[MinGW-notify] [ mingw-Bugs-3295526 ] mingw-get remove + read-only files = fail

[SF/projects/mingw n. l. <min...@li...>]

Bugs item #3295526, was opened at 2011-04-30 21:41
Message generated for change (Comment added) made by keithmarshall
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=102435&aid=3295526&group_id=2435

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: MinGW Installer
Group: None
Status: Open
Resolution: None
Priority: 7
Private: No
Submitted By: Charles Wilson (cwilso11)
Assigned to: Keith Marshall (keithmarshall)
Summary: mingw-get remove + read-only files = fail

Initial Comment:
It appears that mingw-get is able to install files that it cannot uninstall. This seems wrong. I think mingw-get remove should check if a file is read-only, and if so then attempt to remove the read-only flag before removing the file.  Only if it fails after all of that should an error be reported to the user.

Alternatively, mingw-get install should ensure that any file unpacked has write permissions at least for the current user, regardless of the permission bits in the tarball. I think this is inferior to the first solution, because this alternative solution means there would be a difference between using mingw-get install to install a package, and untarring it manually.

See this report 
http://thread.gmane.org/gmane.comp.gnu.mingw.user/36263/focus=36291

Relevant bits quoted below:
=======================
> When I tried the upgrade I got a huge swarm of messages like the one
> below. I have never seen this before,
> all previous mingw and msys upgrades and installs have worked perfectly.
> mingw-get: *** WARNING ***
> c:\mingw\/msys/1.0/lib/perl5/5.6/unicode/In/IPAExtensions.pl:unlink
> failed; Permission denied 

It seems this is because both the perl-5.6.1-*-bin tarball AND the new
perl-5.8.8-1-bin tarball have files like this:

5.8.8:
-r-xr-xr-x user/group 7236 2011-04-27 00:23 bin/config_data
-r--r--r-- user/group  838 2011-04-27 00:24 lib/perl5/5.8/abbrev.pl

5.6.1:
-r-xr-xr-x user/group  119 2010-01-30 14:41 bin/cpan
-r--r--r-- user/group  838 2010-01-30 14:35 lib/perl5/5.6/abbrev.pl

That is, inside the tarball the file is listed with no write permission. The list of such files with missing write perms is different between the two releases, but there is some overlap.

When mingw-get unpacks these tarballs, the affected file(s) are created like this:

cygwin reports:
-r-xr-xr-x+ 1 user group 7.1K Apr 27 10:10 bin/config_data
-r-xr-xr-x+ 1 user group  838 Apr 27 10:10 lib/perl5/5.8/abbrev.pl

msys reports:
-r-xr-xr-x 1 user group 7236 Apr 27 10:10 /bin/config_data
-r--r--r-- 1 user group  838 Apr 27 10:10 /lib/perl5/5.8/abbrev.pl

windows gui properties reports:
config_data: The Read-only attribute is checked.
abbrev.pl:   Ditto.

I think this is a bug in mingw-get's 'remove' operation.  If it can create the file, when running as user A, then it should be able to delete the same file, when running as that same user A. I think mingw-get should be modified to check if a to-be-removed file is "read only" and remove the read-only attribute before trying to delete it.  Only if THAT sequence fails should it report a problem to the user.
========================


----------------------------------------------------------------------

>Comment By: Keith Marshall (keithmarshall)
Date: 2011-05-01 17:22

Message:
I did notice this, when it arose on the ML.

My first thought was that mingw-get never calls chmod(2), so how were the
files being set read-only in the first place?  I had to go back and review
my original code, to confirm that the mode bits from the tarball file
header are indeed passed in the creat(2) call, which prepares to save the
extracted file; of course, this has the effect of an implicit chmod() on
close().  I think this is the correct behaviour; I don't plan to change
it.

Now, the next thing which surprised me was that the original package
maintainer would choose to schedule any files to be marked as read-only on
installation.  I presume he/she must have some good reason for making that
choice, but perhaps that should also bring with it a responsibility for
providing a pre-remove script, to clear that attribute so that a clean
removal is supported.

Okay, I'm kind of playing Devil's Advocate with that preceding statement. 
Obviously, it is moot because mingw-get doesn't (yet) provide a capability
for running pre-remove scripts.  The real issue is that I hadn't
anticipated having to deal with read-only package content, so the remove
function simply calls unlink(2), thus having the effect of "rm foo", where
perhaps "rm -f foo" would be preferred.  (The Devil's Advocacy relates to
the desirability of such a cavalier treatment of a file which the package
maintainer considered precious enough to mark as read-only).

That said, it is a trivial change to add a chmod(2) call, immediately
preceding unlink(2), in pkgunst.cpp's pkg_unlink() function, so forcing the
"rm -f foo" behaviour.  I've already prepared that, in my sandbox, so I
could push out a patch release just as soon as I can find time to update
the release notes accordingly.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=102435&aid=3295526&group_id=2435