#1538 mingw-get remove + read-only files = fail

closed-fixed
2014-08-27
2011-04-30
No

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.

Discussion

  • Keith Marshall

    Keith Marshall - 2011-05-01

    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.

     
  • Charles Wilson

    Charles Wilson - 2011-05-01

    About mingw-get install + implicit chmod: "I don't plan to change it." I agree. mingw-get should retain, if at all possible, whatever attributes the tarball specifies, when unpacking/installing a component.

    About "original package maintainer [choosing] to schedule any files to be marked as read-only" Don't look at me. That's just what the basic perl installation does -- I'd have to manually chmod u+w after installation (bad idea, because I can't do -R as the files are intermingled, after installation, in the "real" /usr tree (since perl != DESTDIR/reassign-$prefix) with non-package files. I do have a list of all files to be packaged, so I could chmod u+w one at a time...but why? (I could also patch the various makefiles -- AND module MakeMaker -- to use 644 and 755 where it currently uses 444 and 555 but again: why? Perl is what it is, and there's nothing inherently WRONG about installing readonly files...

    Perl is doing this on purpose. It does it on cygwin and unix. But cygwin's setup.exe and (e..g) Fedora's rpm can manage to uninstall "read only" files, because they are run under a user with sufficient privileges to do so. Just like mingw-get should at least attempt to make remove() work, on uninstall, by chmoding u+w -- if the attempt fails, well then, the user obviously didn't have sufficient privileges. Same behavior as on cygwin or linux.

    Now, you've already agreed with that, with reservations, but I just wanted to expand/explain the argument. Concerning your reservations about "cavalier treatment" -- well, realistically, that's why users should be careful when running ANY installer/uninstaller, on ANY platform: the app will assume it owns and has full control over of all files it (previously) installed, and WILL remove them, regardless of "permission bits" -- assuming it has sufficient privileges. That's just the way installers/uninstallers /work/.

    No real hurry on the release; we have a workaround already posted on the list.

     
  • Keith Marshall

    Keith Marshall - 2011-05-20
    • status: open --> closed
     
  • Keith Marshall

    Keith Marshall - 2011-05-20

    The problem you report has caused modification in some fashion in
    the official CVS for the given package. The w32api and
    mingw-runtime official CVS reside in the winsup CVS directory tree for Cygwin. Those package CVS trees are periodically merged into
    the MinGW CVS tree. If you still find problems then please open a
    new report.

     
  • Keith Marshall

    Keith Marshall - 2011-05-20
    • status: closed --> closed-fixed
     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks