Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copying package files: cannot overwrite non-directory #26996

Closed
vbraun opened this issue Jan 2, 2019 · 52 comments
Closed

Copying package files: cannot overwrite non-directory #26996

vbraun opened this issue Jan 2, 2019 · 52 comments

Comments

@vbraun
Copy link
Member

vbraun commented Jan 2, 2019

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.

CC: @embray @sagetrac-mafra @videlec @dimpase

Component: build

Author: Matthias Koeppe

Branch/Commit: e83e831

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/26996

@vbraun vbraun added this to the sage-8.6 milestone Jan 2, 2019
@embray
Copy link
Contributor

embray commented Jan 2, 2019

comment:1

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.

@vbraun
Copy link
Member Author

vbraun commented Jan 2, 2019

comment:2

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

@embray
Copy link
Contributor

embray commented Jan 2, 2019

comment:3

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

@embray
Copy link
Contributor

embray commented Jan 2, 2019

comment:4

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.

@embray
Copy link
Contributor

embray commented Jan 4, 2019

comment:5

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.

@embray
Copy link
Contributor

embray commented Jan 15, 2019

comment:6

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.

@embray embray modified the milestones: sage-8.6, sage-8.7 Jan 15, 2019
@jdemeyer
Copy link

jdemeyer commented Feb 4, 2019

comment:7

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.
************************************************************************

@embray
Copy link
Contributor

embray commented Feb 4, 2019

comment:8

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

@embray
Copy link
Contributor

embray commented Feb 4, 2019

comment:9

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.

@jdemeyer
Copy link

jdemeyer commented Feb 4, 2019

comment:10

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.
************************************************************************

@embray
Copy link
Contributor

embray commented Feb 4, 2019

comment:11

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...

@jdemeyer
Copy link

jdemeyer commented Feb 4, 2019

comment:12

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.
************************************************************************

@embray
Copy link
Contributor

embray commented Feb 4, 2019

comment:13

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?

@jdemeyer
Copy link

jdemeyer commented Feb 4, 2019

comment:14
[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

@jdemeyer
Copy link

jdemeyer commented Feb 4, 2019

comment:15

Replying to @embray:

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

I thought that we already fixed permissions when extracting?

@jhpalmieri
Copy link
Member

comment:16

Any way this could be related to #27124?

@jdemeyer
Copy link

jdemeyer commented Feb 5, 2019

comment:17

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.

@jdemeyer
Copy link

jdemeyer commented Feb 5, 2019

comment:18

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

@embray
Copy link
Contributor

embray commented Feb 5, 2019

comment:19

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.

@embray
Copy link
Contributor

embray commented Feb 5, 2019

comment:20

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.

@embray

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:23

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.

@embray
Copy link
Contributor

embray commented Feb 26, 2019

comment:24

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.

@jhpalmieri
Copy link
Member

comment:25

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

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

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.

@mkoeppe mkoeppe reopened this Jan 31, 2020
@mkoeppe mkoeppe added this to the sage-9.1 milestone Jan 31, 2020
@mkoeppe
Copy link
Member

mkoeppe commented Jan 31, 2020

comment:35

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

@mkoeppe
Copy link
Member

mkoeppe commented Jan 31, 2020

comment:36

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

@mkoeppe
Copy link
Member

mkoeppe commented Mar 31, 2020

comment:37

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

@dimpase
Copy link
Member

dimpase commented Mar 31, 2020

comment:38

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/ ?

@mkoeppe
Copy link
Member

mkoeppe commented Mar 31, 2020

comment:39

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

@mkoeppe
Copy link
Member

mkoeppe commented Mar 31, 2020

Author: Matthias Koeppe

@mkoeppe
Copy link
Member

mkoeppe commented Mar 31, 2020

@mkoeppe
Copy link
Member

mkoeppe commented Mar 31, 2020

Commit: cf6999e

@mkoeppe
Copy link
Member

mkoeppe commented Mar 31, 2020

New commits:

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

@dimpase
Copy link
Member

dimpase commented Mar 31, 2020

comment:43

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 31, 2020

Changed commit from cf6999e to e83e831

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 31, 2020

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

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

@mkoeppe
Copy link
Member

mkoeppe commented Mar 31, 2020

comment:45

Try with this version

@dimpase
Copy link
Member

dimpase commented Mar 31, 2020

comment:46

pillow installs now. Moar testing underway

@dimpase
Copy link
Member

dimpase commented Apr 1, 2020

Reviewer: Dima Pasechnik

@dimpase
Copy link
Member

dimpase commented Apr 1, 2020

comment:47

lgtm

@mkoeppe
Copy link
Member

mkoeppe commented Apr 1, 2020

comment:48

Thanks!

@vbraun
Copy link
Member Author

vbraun commented Apr 9, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants