Opened 3 years ago

Closed 22 months ago

#26996 closed defect (fixed)

Copying package files: cannot overwrite non-directory

Reported by: vbraun Owned by:
Priority: critical Milestone: sage-9.1
Component: build Keywords:
Cc: embray, mafra, vdelecroix, dimpase Merged in:
Authors: Matthias Koeppe Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: e83e831 (Commits, GitHub, GitLab) Commit: e83e8317eba8a54a2c8e1676c2be8c8bd2ef2ef3
Dependencies: Stopgaps:

Status badges

Description (last modified by embray)

As reported at https://groups.google.com/d/msg/sage-devel/HHC-CmQ6X60/KCLUP0_aDQAJ

66509-real      31m50.729s 
66510-user      28m47.266s
66511-sys       2m43.839s
66512-Copying package files from temporary location /home/sverre/sage/local/var/tmp/sage/build/gfortran-7.2.0/inst to /home/sverre/sage/local
66513-cp: cannot overwrite non-directory '/home/sverre/sage/local/./lib64' with directory '/home/sverre/sage/local/var/tmp/sage/build/gfortran-7.2.0/inst/home/sverre/sage/local/./lib64'
66514-************************************************************************
66515:Error copying files for gfortran-7.2.0.
66516-************************************************************************
66517-Please email sage-devel (http://groups.google.com/group/sage-devel)
66518-explaining the problem and including the log file
66519-  /home/sverre/sage/logs/pkgs/gfortran-7.2.0.log
66520-Describe your computer, operating system, etc.
66521-************************************************************************

Presumably /home/sverre/sage/local/lib64 is a symlink, and overwriting it with an actual directory fails.

Some informative background for anyone else wanting to report issues with copying over lib64:

When installing Sage SPKGs all library files (at least on Linux, BSD, and OSX) should go under $SAGE_LOCAL/lib. This is because, in order for executables and other libraries to be able to find them, we typically link with an RPATH set to $SAGE_LOCAL/lib. Libraries installed in $SAGE_LOCAL/lib64 thus won't be found. This was worked around in the past by installing lib64 is a symlink to lib. This generally works but can be a bit misleading; it's better if SPKGs are properly configured to not install files into lib64 iin the first place.

An unintended consequence of #22509, where packages are first installed to a temporary location, and then copied from there into the $SAGE_LOCAL prefix, is that trying to overwrite a non-directory (i.e. the lib64 symlink) with a real directory (a "lib64" that gets created in the temporary install location) will fail. In general this is behavior we want to keep, because part of the point of the new install system from #22509 is that different SPKGs should not be overwriting things like in an unintended manner. In this case it actively exposes cases where some packages are still trying to install files into lib64/, which are better to fix directly

Note: For the case of gfortran this is fixed by #27016.

Change History (49)

comment:1 Changed 3 years ago by embray

So? That's intentional. Package installations shouldn't overwrite existing non-directories with directories. There's no reason they should have a symlink named $SAGE_LOCAL/lib64. The real question is why that's there.

comment:2 Changed 3 years ago by vbraun

Afair its there because there is no easy way to convince gcc/gfortran to use /lib instead of /lib64

comment:3 follow-up: Changed 3 years ago by embray

I see. It seems Debian just patches it. If we don't want to have a real lib64 we should do the same.

comment:4 in reply to: ↑ 3 Changed 3 years ago by embray

Replying to embray:

I see. It seems Debian just patches it. If we don't want to have a real lib64 we should do the same.

I've spent more time understanding the gcc build system and the "multilib" settings, and now I understand Debian's patches better as well, which include patching many of the target config files under gcc/config to not use lib64. I can see this on my own GCC on Debian by running

$ gcc -print-multi-os-directory
../lib

This path is appended to ${libdir}/ by the build system to specify the installation path for the target native libraries installed by gcc, so for example libstdc++.so would get installed to ${libdir}/../lib}, or in other words just lib/.

However, with the target fragments in the gcc source, the bootstrap gcc that is built during stage 1 one of the build process ends up with ../lib64 for this, and hence builds all its libraries to also go in lib64.

However, I think I've figured out a bit of a hack to change the multilib settings without patching, in such a way that should work on any platform we care about. I'll probably also propose an upstream patch to make this easier, because really it's pretty straightforward and otherwise supported by the build system. It's just trickier than it should be to get the right flags specified in the right place.

Last edited 3 years ago by embray (previous) (diff)

comment:5 follow-up: Changed 3 years ago by embray

Since this is only really an issue with installing the gcc and gfortran SPKGs on Linux, the specific case reported in this ticket is fixed by #27016, since those packages will no longer create files in lib64.

I opted for now to keep the symlink to lib64 just in case something needs it (though I can't find any specific case), and because having it will alert us in the future to packages trying to install files to lib64.

comment:6 Changed 3 years ago by embray

  • Milestone changed from sage-8.6 to sage-8.7

Retarging tickets optimistically to the next milestone. If you are responsible for this ticket (either its reporter or owner) and don't believe you are likely to complete this ticket before the next release (8.7) please retarget this ticket's milestone to sage-pending or sage-wishlist.

comment:7 in reply to: ↑ 5 Changed 3 years ago by jdemeyer

Replying to embray:

Since this is only really an issue with installing the gcc and gfortran SPKGs on Linux

On a different system, I'm also getting

Copying package files from temporary location /Users/jdemeyer/sage/local/var/tmp/sage/build/ecl-16.1.2.p5/inst to /Users/jdemeyer/sage/local
cp: cannot overwrite directory /Users/jdemeyer/sage/local/./lib/ecl with non-directory /Users/jdemeyer/sage/local/var/tmp/sage/build/ecl-16.1.2.p5/inst/Users/jdemeyer/sage/local/./lib/ecl
************************************************************************
Error copying files for ecl-16.1.2.p5.
************************************************************************

comment:8 Changed 3 years ago by embray

It appears lib/ecl is a symlink to lib/ecl-<version>. Is it possible the symlink was not removed for some reason?

comment:9 Changed 3 years ago by embray

In the spkg-install for ecl there's a line

rm -rf "$SAGE_LOCAL/lib/ecl-"*

but no

rm -f "$SAGE_LOCAL/lib/ecl"

This should also go in a spkg-legacy-uninstall script, but regardless, if you were updating from some older version, and it didn't delete the symlink, you would get this.

comment:10 Changed 3 years ago by jdemeyer

Also with brial (when upgrading from an older version of Sage):

Copying package files from temporary location /Users/jdemeyer/sage/local/var/tmp/sage/build/brial-1.2.4/inst to /Users/jdemeyer/sage/local
cp: symlink: libbrial_groebner.3.dylib: File exists
cp: symlink: libbrial.3.dylib: File exists
************************************************************************
Error copying files for brial-1.2.4.
************************************************************************

comment:11 Changed 3 years ago by embray

That's a little more strange. Actually, now that I think about it, I did see some behavior like this a couple weeks ago, and I think I saw some evidence for a bug that could actually cause the spkg-legacy-uninstall scripts not to run, which in the case of brial it definitely should have.

Let me confirm that and make a ticket if so...

comment:12 Changed 3 years ago by jdemeyer

And also

Copying package files from temporary location /Users/jdemeyer/sage/local/var/tmp/sage/build/gap-4.10.0/inst to /Users/jdemeyer/sage/local
cp: cannot overwrite directory /Users/jdemeyer/sage/local/./share/gap/bin/x86_64-apple-darwin18.2.0-default64/src with non-directory /Users/jdemeyer/sage/local/var/tmp/sage/build/gap-4.10.0/inst/Users/jdemeyer/sage/local/./share/gap/bin/x86_64-apple-darwin18.2.0-default64/src
cp: /Users/jdemeyer/sage/local/./share/gap/pkg/ctbllib/tst/docxpl.tst: Permission denied
************************************************************************
Error copying files for gap-4.10.0.
************************************************************************

comment:13 follow-up: Changed 3 years ago by embray

That one is a known issue from #22626 comment:423. The upstream tarball actually contains a file that randomly has read-only permissions for no good reason.

It seems that still has not been fixed. Not clear what the appropriate solution is. Just chmod everything to u+rw when extracting a tarball?

comment:14 Changed 3 years ago by jdemeyer

[gsl-2.5] Copying package files from temporary location /Users/jdemeyer/sage/local/var/tmp/sage/build/gsl-2.5/inst to /Users/jdemeyer/sage/local
[gsl-2.5] cp: symlink: libgsl.23.dylib: File exists

comment:15 in reply to: ↑ 13 ; follow-up: Changed 3 years ago by jdemeyer

Replying to embray:

Just chmod everything to u+rw when extracting a tarball?

I thought that we already fixed permissions when extracting?

comment:16 Changed 3 years ago by jhpalmieri

Any way this could be related to #27124?

comment:17 follow-up: Changed 3 years ago by jdemeyer

The situation is particularly bad on OS X because cp is not willing to overwrite existing symlinks for some reason. So basically any package which includes a library (i.e. many packages) potentially causes this problem.

comment:18 Changed 3 years ago by jdemeyer

So I'll give up on writing spkg-legacy-uninstall scripts, it's not a scalable solution.

comment:19 in reply to: ↑ 15 Changed 3 years ago by embray

Replying to jdemeyer:

Replying to embray:

Just chmod everything to u+rw when extracting a tarball?

I thought that we already fixed permissions when extracting?

We apply a umask but that's different. And I think it has 0 in the user bits, so that means if a file happens to be read-only for some reason that will be preserved.

comment:20 in reply to: ↑ 17 Changed 3 years ago by embray

Replying to jdemeyer:

The situation is particularly bad on OS X because cp is not willing to overwrite existing symlinks for some reason. So basically any package which includes a library (i.e. many packages) potentially causes this problem.

To be clear: Only if you're trying to upgrade some relatively old build. It would be nice if this can always work, and I'm not saying this isn't a problem. But sometimes it's okay to just require a make distclean.

comment:21 Changed 3 years ago by embray

  • Description modified (diff)

comment:22 Changed 3 years ago by mafra

  • Cc mafra added

comment:23 follow-ups: Changed 3 years ago by jhpalmieri

I'm hitting this problem (or I think it's this problem) with the 8.7.beta4 -> 8.7.beta5 upgrade.

gap:

Copying package files from temporary location /Users/jpalmier/Desktop/Sage/git/patchbot/sage/local/var/tmp/sage/build/gap-4.10.0.p0/inst to /Users/jpalmier/Desktop/Sage/git/patchbot/sage/local
cp: cannot overwrite directory /Users/jpalmier/Desktop/Sage/git/patchbot/sage/local/./share/gap/bin/x86_64-apple-darwin18.2.0-default64/src with non-directory /Users/jpalmier/Desktop/Sage/git/patchbot/sage/local/var/tmp/sage/build/gap-4.10.0.p0/inst/Users/jpalmier/Desktop/Sage/git/patchbot/sage/local/./share/gap/bin/x86_64-apple-darwin18.2.0-default64/src
cp: /Users/jpalmier/Desktop/Sage/git/patchbot/sage/local/./share/gap/pkg/ctbllib/tst/docxpl.tst: Permission denied
************************************************************************
Error copying files for gap-4.10.0.p0.

ecl:

Copying package files from temporary location /Users/jpalmier/Desktop/Sage/git/patchbot/sage/local/var/tmp/sage/build/ecl-16.1.3.p0/inst to /Users/jpalmier/Desktop/Sage/git/patchbot/sage/local
cp: symlink: ecl-16.1.3: File exists
************************************************************************
Error copying files for ecl-16.1.3.p0.

I've also seen it with Python 2 and Python 3, with the symlinks in local/lib/pkgconfig.

comment:24 in reply to: ↑ 23 Changed 3 years ago by embray

Replying to jhpalmieri:

I'm hitting this problem (or I think it's this problem) with the 8.7.beta4 -> 8.7.beta5 upgrade.

The former is a problem with the GAP tarball. I think we should just make a special case fix for that in the GAP spkg for now; see comment:13.

I think the latter might be because of #27124 or possibly #27223. In other words, it's likely that some previous bug meant the package could not be correctly uninstalled first.

comment:25 follow-up: Changed 3 years ago by jhpalmieri

For GAP, and indeed in general, I would be tempted to do this:

  • build/sage_bootstrap/uncompress/tar_file.py

    diff --git a/build/sage_bootstrap/uncompress/tar_file.py b/build/sage_bootstrap/uncompress/tar_file.py
    index 57621527ac..cf707e78a3 100644
    a b class SageBaseTarFile(tarfile.TarFile): 
    6868        """Apply ``self.umask`` instead of the permissions in the TarInfo."""
    6969        tarinfo = copy.copy(tarinfo)
    7070        tarinfo.mode &= ~self.umask
     71        tarinfo.mode |= stat.S_IWUSR
    7172        tarinfo.mode &= ~(stat.S_ISUID | stat.S_ISGID)
    7273        return super(SageBaseTarFile, self).chmod(tarinfo, targetpath)
    7374

I can't tell whether that's a good idea.

It looks like Sage's version of tar_file.py helps if the permissions in the tarball are too lenient, but not if they're too strict, as in this case.

Last edited 3 years ago by jhpalmieri (previous) (diff)

comment:26 in reply to: ↑ 25 Changed 3 years ago by embray

Replying to jhpalmieri:

For GAP, and indeed in general, I would be tempted to do this:

  • build/sage_bootstrap/uncompress/tar_file.py

    diff --git a/build/sage_bootstrap/uncompress/tar_file.py b/build/sage_bootstrap/uncompress/tar_file.py
    index 57621527ac..cf707e78a3 100644
    a b class SageBaseTarFile(tarfile.TarFile): 
    6868        """Apply ``self.umask`` instead of the permissions in the TarInfo."""
    6969        tarinfo = copy.copy(tarinfo)
    7070        tarinfo.mode &= ~self.umask
     71        tarinfo.mode |= stat.S_IWUSR
    7172        tarinfo.mode &= ~(stat.S_ISUID | stat.S_ISGID)
    7273        return super(SageBaseTarFile, self).chmod(tarinfo, targetpath)
    7374

I can't tell whether that's a good idea.

It looks like Sage's version of tar_file.py helps if the permissions in the tarball are too lenient, but not if they're too strict, as in this case.

Yeah, I wouldn't be opposed to that. I don't think there's any good reason for a package to be installing read-only files.

comment:27 Changed 3 years ago by jhpalmieri

See #27388 for this permissions change.

comment:28 follow-up: Changed 3 years ago by dimpase

  • Cc vdelecroix added

Another instance of this problem is primecount (on Linux), see also https://groups.google.com/d/msg/sage-devel/w6Os2s6_eGk/SyFxT7cJBgAJ for report on this

comment:29 in reply to: ↑ 28 Changed 3 years ago by embray

Replying to dimpase:

Another instance of this problem is primecount (on Linux), see also https://groups.google.com/d/msg/sage-devel/w6Os2s6_eGk/SyFxT7cJBgAJ for report on this

See #27485

comment:30 Changed 3 years ago by embray

  • Description modified (diff)

Tried to add some more explanation to this ticket.

comment:31 Changed 3 years ago by embray

  • Milestone changed from sage-8.7 to sage-8.8

Ticket retargeted after milestone closed (if you don't believe this ticket is appropriate for the Sage 8.8 release please retarget manually)

comment:32 in reply to: ↑ 23 Changed 3 years ago by embray

Replying to jhpalmieri:

I'm hitting this problem (or I think it's this problem) with the 8.7.beta4 -> 8.7.beta5 upgrade.

gap:

Copying package files from temporary location /Users/jpalmier/Desktop/Sage/git/patchbot/sage/local/var/tmp/sage/build/gap-4.10.0.p0/inst to /Users/jpalmier/Desktop/Sage/git/patchbot/sage/local
cp: cannot overwrite directory /Users/jpalmier/Desktop/Sage/git/patchbot/sage/local/./share/gap/bin/x86_64-apple-darwin18.2.0-default64/src with non-directory /Users/jpalmier/Desktop/Sage/git/patchbot/sage/local/var/tmp/sage/build/gap-4.10.0.p0/inst/Users/jpalmier/Desktop/Sage/git/patchbot/sage/local/./share/gap/bin/x86_64-apple-darwin18.2.0-default64/src
cp: /Users/jpalmier/Desktop/Sage/git/patchbot/sage/local/./share/gap/pkg/ctbllib/tst/docxpl.tst: Permission denied
************************************************************************
Error copying files for gap-4.10.0.p0.

ecl:

Copying package files from temporary location /Users/jpalmier/Desktop/Sage/git/patchbot/sage/local/var/tmp/sage/build/ecl-16.1.3.p0/inst to /Users/jpalmier/Desktop/Sage/git/patchbot/sage/local
cp: symlink: ecl-16.1.3: File exists
************************************************************************
Error copying files for ecl-16.1.3.p0.

I've also seen it with Python 2 and Python 3, with the symlinks in local/lib/pkgconfig.

This is now #27544.

comment:33 Changed 3 years ago by embray

  • Milestone changed from sage-8.8 to sage-duplicate/invalid/wontfix
  • Resolution set to worksforme
  • Status changed from new to closed

ISTM all known cases of this have been fixed. In particular, fixing #27223 should fix many cases where this could crop up, since some files were not being properly uninstalled.

If it comes up again I would suggest opening a new ticket for that specific case.

comment:34 Changed 2 years ago by mkoeppe

  • Cc dimpase added
  • Milestone changed from sage-duplicate/invalid/wontfix to sage-9.1
  • Resolution worksforme deleted
  • Status changed from closed to new

I'm hitting the "cannot overwrite non-directory" from the original ticket description with gfortran (now 9.2.0) on fedora-31-minimal and fedora-32-minimal - see https://github.com/mkoeppe/sage/actions/runs/33491442

comment:35 Changed 2 years ago by mkoeppe

The symbolic link lib64->lib is set in the AC_CONFIG_COMMANDS mkdirs.

The gfortran staging install creates the following:

ls -l /sage/local/var/tmp/sage/build/gfortran-9.2.0/inst/sage/local/lib*
/sage/local/var/tmp/sage/build/gfortran-9.2.0/inst/sage/local/lib:
total 48296
drwxr-xr-x 3 root root     4096 Jan 31 20:56 gcc
-rw-r--r-- 1 root root   464650 Jan 31 20:56 libatomic.a
lrwxrwxrwx 1 root root       18 Jan 31 20:56 libatomic.so -> libatomic.so.1.2.0
lrwxrwxrwx 1 root root       18 Jan 31 20:56 libatomic.so.1 -> libatomic.so.1.2.0
-rwxr-xr-x 1 root root   170064 Jan 31 20:56 libatomic.so.1.2.0
-rw-r--r-- 1 root root      132 Jan 31 20:56 libgcc_s.so
-rw-r--r-- 1 root root   872480 Jan 31 20:56 libgcc_s.so.1
-rw-r--r-- 1 root root 28051754 Jan 31 20:56 libgfortran.a
lrwxrwxrwx 1 root root       20 Jan 31 20:56 libgfortran.so -> libgfortran.so.5.0.0
lrwxrwxrwx 1 root root       20 Jan 31 20:56 libgfortran.so.5 -> libgfortran.so.5.0.0
-rwxr-xr-x 1 root root 11718472 Jan 31 20:56 libgfortran.so.5.0.0
-rw-r--r-- 1 root root      269 Jan 31 20:56 libgfortran.spec
-rw-r--r-- 1 root root  3239510 Jan 31 20:56 libgomp.a
lrwxrwxrwx 1 root root       16 Jan 31 20:56 libgomp.so -> libgomp.so.1.0.0
lrwxrwxrwx 1 root root       16 Jan 31 20:56 libgomp.so.1 -> libgomp.so.1.0.0
-rwxr-xr-x 1 root root  1468416 Jan 31 20:56 libgomp.so.1.0.0
-rw-r--r-- 1 root root      169 Jan 31 20:56 libgomp.spec
-rw-r--r-- 1 root root  2055148 Jan 31 20:56 libquadmath.a
lrwxrwxrwx 1 root root       20 Jan 31 20:56 libquadmath.so -> libquadmath.so.0.0.0
lrwxrwxrwx 1 root root       20 Jan 31 20:56 libquadmath.so.0 -> libquadmath.so.0.0.0
-rwxr-xr-x 1 root root  1224592 Jan 31 20:56 libquadmath.so.0.0.0
-rw-r--r-- 1 root root   101410 Jan 31 20:56 libssp.a
lrwxrwxrwx 1 root root       15 Jan 31 20:56 libssp.so -> libssp.so.0.0.0
lrwxrwxrwx 1 root root       15 Jan 31 20:56 libssp.so.0 -> libssp.so.0.0.0
-rwxr-xr-x 1 root root    51328 Jan 31 20:56 libssp.so.0.0.0
-rw-r--r-- 1 root root     3222 Jan 31 20:56 libssp_nonshared.a

/sage/local/var/tmp/sage/build/gfortran-9.2.0/inst/sage/local/lib64:
total 1196
-rwxr-xr-x 1 root root     932 Jan 31 20:56 libcc1.la
lrwxrwxrwx 1 root root      15 Jan 31 20:56 libcc1.so -> libcc1.so.0.0.0
lrwxrwxrwx 1 root root      15 Jan 31 20:56 libcc1.so.0 -> libcc1.so.0.0.0
-rwxr-xr-x 1 root root 1217840 Jan 31 20:56 libcc1.so.0.0.0

/sage/local/var/tmp/sage/build/gfortran-9.2.0/inst/sage/local/libexec:
total 4
drwxr-xr-x 3 root root 4096 Jan 31 20:56 gcc

comment:36 Changed 2 years ago by mkoeppe

I'm fixing this for gfortran in #29089 but the faulty copying from staging to $SAGE_LOCAL should be cleaned up in #22509.

comment:37 follow-up: Changed 22 months ago by mkoeppe

  • Priority changed from major to critical

comment:38 in reply to: ↑ 37 Changed 22 months ago by dimpase

Replying to mkoeppe:

Showing up in pillow as well, as reported in https://groups.google.com/d/msg/sage-release/kU5M1QVuQQY/DY3l67DUAwAJ

an unpleasant part of it is that the installation attempt happens from within distutils or setuptools, called from within pillow's setup.py.

would it be better to make lib64/ a proper directory, install stuff there, and then do a kind of postinstall that would move/link/copy stuff over to lib/ ?

comment:39 Changed 22 months ago by mkoeppe

I will just move the fix that I have in the gfortran install script into sage-spkg.

comment:40 Changed 22 months ago by mkoeppe

  • Authors set to Matthias Koeppe

comment:41 Changed 22 months ago by mkoeppe

  • Branch set to u/mkoeppe/copying_package_files__cannot_overwrite_non_directory

comment:42 Changed 22 months ago by mkoeppe

  • Commit set to cf6999e3b28d5940cf29828a26b813e570ddc63c
  • Status changed from new to needs_review

New commits:

cf6999eMove lib64 symlink workaround from build/pkgs/gfortran/spkg-install.in to build/bin/sage-spkg

comment:43 Changed 22 months ago by dimpase

A diffrent error now (File exists):

writing manifest file 'src/Pillow.egg-info/SOURCES.txt'
running install_lib
creating /home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/pillow-5.3.0.p0/inst/home/scratch2/dimpase/sage/sage/local/lib64
error: could not create '/home/scratch2/dimpase/sage/sage/local/var/tmp/sage/build/pillow-5.3.0.p0/inst/home/scratch2/dimpase/sage/sage/local/lib64': File exists
***************************************************************************************************************************************
Error building/installing Pillow

comment:44 Changed 22 months ago by git

  • Commit changed from cf6999e3b28d5940cf29828a26b813e570ddc63c to e83e8317eba8a54a2c8e1676c2be8c8bd2ef2ef3

Branch pushed to git repo; I updated commit sha1. New commits:

e83e831build/bin/sage-spkg: Also create $SAGE_DESTDIR_LOCAL/lib

comment:45 Changed 22 months ago by mkoeppe

Try with this version

comment:46 Changed 22 months ago by dimpase

pillow installs now. Moar testing underway

comment:47 Changed 22 months ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

lgtm

comment:48 Changed 22 months ago by mkoeppe

Thanks!

comment:49 Changed 22 months ago by vbraun

  • Branch changed from u/mkoeppe/copying_package_files__cannot_overwrite_non_directory to e83e8317eba8a54a2c8e1676c2be8c8bd2ef2ef3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.