Opened 2 years ago

Closed 2 years ago

#26013 closed defect (fixed)

lots of warnings when uninstalling spkgs

Reported by: jhpalmieri Owned by:
Priority: major Milestone: sage-8.4
Component: build Keywords:
Cc: jdemeyer, embray Merged in:
Authors: John Palmieri Reviewers: Erik Bray
Report Upstream: N/A Work issues:
Branch: 48f7cdc (Commits) Commit: 48f7cdc4f7f80041ba843b459696415cac488e55
Dependencies: Stopgaps:

Description

When upgrading Sage from 8.3.rc2 to 8.4.beta0: mpir-3.0.0-644faf502c56f97d9accd301965fc57d6ec70868.p0.log:

Uninstalling existing 'mpir'
Warning: File '/share/info/dir' not found
Warning: Directory '/share/info' not found
Warning: File '/share/info/mpir.info' not found
Warning: Directory '/share/info' not found
Warning: File '/share/info/mpir.info-1' not found
Warning: Directory '/share/info' not found
Warning: File '/share/info/mpir.info-2' not found
Warning: Directory '/share/info' not found
Warning: File '/include/gmp.h' not found
Warning: Directory '/include' not found
Warning: File '/include/gmpxx.h' not found
Warning: Directory '/include' not found
Warning: File '/include/mpir.h' not found
Warning: Directory '/include' not found
Warning: File '/include/mpirxx.h' not found
Warning: Directory '/include' not found
Warning: File '/lib/libgmp.23.dylib' not found
Warning: Directory '/lib' not found
Warning: File '/lib/libgmp.a' not found
Warning: Directory '/lib' not found
Warning: File '/lib/libgmp.dylib' not found
Warning: Directory '/lib' not found
Warning: File '/lib/libgmpxx.8.dylib' not found
Warning: Directory '/lib' not found
Warning: File '/lib/libgmpxx.a' not found
Warning: Directory '/lib' not found
Warning: File '/lib/libgmpxx.dylib' not found
Warning: Directory '/lib' not found
Warning: File '/lib/libmpir.23.dylib' not found
Warning: Directory '/lib' not found
Warning: File '/lib/libmpir.a' not found
Warning: Directory '/lib' not found
Warning: File '/lib/libmpir.dylib' not found
Warning: Directory '/lib' not found
Warning: File '/lib/libmpirxx.8.dylib' not found
Warning: Directory '/lib' not found
Warning: File '/lib/libmpirxx.a' not found
Warning: Directory '/lib' not found
Warning: File '/lib/libmpirxx.dylib' not found
Warning: Directory '/lib' not found

mpc-1.1.0.log:

Uninstalling existing 'mpc'
Warning: File '/share/info/dir' not found
Warning: Directory '/share/info' not found
Warning: File '/share/info/mpc.info' not found
Warning: Directory '/share/info' not found
Warning: File '/include/mpc.h' not found
Warning: Directory '/include' not found
Warning: File '/lib/libmpc.3.dylib' not found
Warning: Directory '/lib' not found
Warning: File '/lib/libmpc.a' not found
Warning: Directory '/lib' not found
Warning: File '/lib/libmpc.dylib' not found
Warning: Directory '/lib' not found

and similarly in many other log files. As far as I can tell, files are not being uninstalled before the new versions are built. Is the problem just that the path names should have SAGE_LOCAL prepended?

Change History (21)

comment:1 in reply to: ↑ description Changed 2 years ago by jdemeyer

Replying to jhpalmieri:

Is the problem just that the path names should have SAGE_LOCAL prepended?

Indeed. However, it works for me.

comment:2 Changed 2 years ago by jhpalmieri

It happens on two different machines. Maybe something is broken. I added some print statements, and they do not clarify things. Here is the change I made:

  • build/sage_bootstrap/uninstall.py

    diff --git a/build/sage_bootstrap/uninstall.py b/build/sage_bootstrap/uninstall.py
    index 94a1203f0b..ed00ceac46 100644
    a b def modern_uninstall(spkg_name, sage_local, files, verbose=False): 
    187187    # Remove the files; if a directory is empty after removing a file
    188188    # from it, remove the directory too.
    189189    for filename in files:
     190        print("  * sage_local = {}".format(sage_local))
     191        print("  * orig filename = {}".format(filename))
    190192        filename = pth.join(sage_local, filename)
     193        print("  ** new filename = {}".format(filename))
    191194        dirname = pth.dirname(filename)
    192195        if os.path.exists(filename):
    193196            if verbose:

Here is typical output:

Uninstalling existing 'glpk'
  * sage_local = /Users/jpalmier/Desktop/Sage_stuff/git/sage/local
  * orig filename = /bin/glpsol
  ** new filename = /bin/glpsol

I don't understand.

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

comment:3 Changed 2 years ago by jhpalmieri

Oh, I do understand. From the documentation for os.path.join:

If a component is an absolute path, all previous components are thrown away and joining continues from the absolute path component.

So we need to strip os.sep from the left end of filename.

comment:4 Changed 2 years ago by jhpalmieri

  • Branch set to u/jhpalmieri/spkg-uninstall-os-sep

comment:5 Changed 2 years ago by jhpalmieri

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

New commits:

f1aa849trac 26022: "![ -z ...]" is not valid, at least on OS X
b31446atrac 26013: use filename.lstrip(os.sep) instead of filename

comment:6 Changed 2 years ago by jhpalmieri

  • Authors set to John Palmieri

comment:7 Changed 2 years ago by git

  • Commit changed from b31446accb1a28c65d5dcb1a2a1976fa9349d6da to 40e0cfb75ef3a0a18f9ba00ad9a1eabcf5d7fc41

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

40e0cfbtrac 26013: apply lstrip(os.sep) to filename in os.path.join(..., filename)

comment:8 Changed 2 years ago by embray

Some other OSX users have pointed this issue out before, but I could never figure out why it was happening. Let me look into this a bit more for a sec because there shouldn't be absolute paths in those filenames in the first place. They should be written as relative paths. For example I have:

$ cat local/var/lib/sage/installed/mpc-1.1.0
{
    "package_name": "mpc",
    "package_version": "1.1.0",
    "install_date": "Thu Jul 19 08:10:31 UTC 2018",
    "system_uname": "Linux sage-docker 3.13.0-123-generic #172-Ubuntu SMP Mon Jun 26 18:04:35 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux",
    "sage_version": "SageMath version 8.3.rc1, Release Date: 2018-07-14",
    "test_result": "",
    "files": [
        "include/mpc.h",
        "lib/libmpc.a",
        "lib/libmpc.so",
        "lib/libmpc.so.3",
        "lib/libmpc.so.3.1.0",
        "share/info/dir",
        "share/info/mpc.info"
    ]
}

Do you see absolute paths being written into those files?

comment:9 Changed 2 years ago by embray

Unfortunately the OSX machine I used to have access to appears to be down, and everyone here is on vacation...

comment:10 Changed 2 years ago by embray

The relevant code is in sage-spkg (yet more reason this should probably be rewritten in Python as it is growing in complexity, though as Jeroen wrote elsewhere it would be a tedious task...). A summarized version is as follows:

PREFIX="${SAGE_DESTDIR_LOCAL%/}/"
old_IFS="$IFS"; IFS=$'\n'
for filename in $(find "$PREFIX" -type f -o -type l | sort); do
    filename="${filename#$PREFIX}"
    echo "$filename"
done

in the above, $PREFIX should end with a /, and ${filename#$PREFIX} should have the effect of stripping $PREFIX (up to and including the final /) from the front of $filename. But maybe there's a bug or some other subtlety with that I was unaware of.

comment:11 Changed 2 years ago by embray

Another way maybe it could fail is if your $SAGE_LOCAL somehow had a superfluous double-slash at the end of it...?

comment:12 Changed 2 years ago by embray

If nothing else, something like this should do the trick. But I think I will be rewriting a chunk of this code anyways...

  • build/bin/sage-spkg

    diff --git a/build/bin/sage-spkg b/build/bin/sage-spkg
    index 125dd15..ded872f 100755
    a b if [ -d "$SAGE_DESTDIR" ]; then 
    889889    old_IFS="$IFS"; IFS=$'\n'
    890890    for filename in $(find "$PREFIX" -type f -o -type l | sort); do
    891891        filename="${filename#$PREFIX}"
     892        # Make really sure there are still no leading slashes on the filename
     893        # (which could happen if enough superfluous slashes were added
     894        # elsewhere)
     895        while [[ "$filename" == /* ]]; do
     896            filename="${filename#/}"
     897        done
     898
    892899        if [ $FIRST -eq 1 ]; then
    893900            FILE_LIST="\"$filename\""
    894901            FIRST=0

comment:13 Changed 2 years ago by jhpalmieri

I added echo "$filename" in sage-spkg, and there are indeed leading backslashes. My $SAGE_LOCAL has no trailing backslashes (at least when I do sage -sh and echo $SAGE_LOCAL), and the $PREFIX variable in sage-spkg has one. As you probably know, bash behavior can vary depending on whether it's GNU/linux or BSD. Maybe that's what's going on. Is there an issue with just using lstrip(os.sep) as my branch does?

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

comment:14 Changed 2 years ago by embray

I'd be curious if you could figure out exactly which part is behaving differently in your bash? What happens with filename="${filename#$PREFIX}. Not even necessarily in this script, but just in general? What does something like:

PREFIX=/a/
filename=/a/b
filename=${filename#$PREFIX}
echo "$filename"

If that returns anything other than just b this is a bug in your bash I think.

Is there an issue with just using lstrip(os.sep) as my branch does?

The point is that shouldn't even be necessary. It might not be a bad idea anyways just as a sanity check, though I might also issue a warning. The intent is that these files contain a list of relative paths. If they don't then the files are malformatted, which means the real problem is in the code producing those files (whether the bug ultimately originates from a buggy bash or not).

comment:15 Changed 2 years ago by embray

Alternatively, it's also possible that your find is slipping in some superfluous slashes.

comment:16 Changed 2 years ago by embray

This seems to suggest that it is in fact the find at fault: https://apple.stackexchange.com/a/254224/44101

comment:17 Changed 2 years ago by embray

This patch should do it:

  • build/bin/sage-spkg

    diff --git a/build/bin/sage-spkg b/build/bin/sage-spkg
    index 9b90421eaf..9dffb481df 100755
    a b fi 
    869869# case DESTDIR=$SAGE_DESTDIR installation was not used
    870870echo "Copying package files from temporary location $SAGE_DESTDIR to $SAGE_LOCAL"
    871871if [ -d "$SAGE_DESTDIR" ]; then
    872     PREFIX="${SAGE_DESTDIR_LOCAL%/}/"
     872    # Some `find` implementations will put superfluous slashes in the
     873    # output if we give them a directory name with a slash; so make sure
     874    # any trailing slash is removed; https://trac.sagemath.org/ticket/26013
     875    PREFIX="${SAGE_DESTDIR_LOCAL%/}"
    873876
    874     rm -f "$PREFIX"lib/*.la
     877    rm -f "$PREFIX"/lib/*.la
    875878    if [ $? -ne 0 ]; then
    876879        error_msg "Error deleting unnecessary libtool archive files"
    877880        exit 1
    if [ -d "$SAGE_DESTDIR" ]; then 
    882885    FIRST=1
    883886    old_IFS="$IFS"; IFS=$'\n'
    884887    for filename in $(find "$PREFIX" -type f -o -type l | sort); do
    885         filename="${filename#$PREFIX}"
     888        filename="${filename#$PREFIX/}"
    886889        if [ $FIRST -eq 1 ]; then
    887890            FILE_LIST="\"$filename\""
    888891            FIRST=0
    if [ -d "$SAGE_DESTDIR" ]; then 
    893896        if [ ! -d "$SAGE_LOCAL/$(dirname "$filename")" ]; then
    894897            $SAGE_SUDO mkdir -p "$SAGE_LOCAL/$(dirname "$filename")"
    895898        fi
    896         $SAGE_SUDO mv "$PREFIX$filename" "${SAGE_LOCAL%/}/$filename"
     899        $SAGE_SUDO mv "$PREFIX/$filename" "${SAGE_LOCAL%/}/$filename"
    897900        if [ $? -ne 0 ]; then
    898901            error_msg "Error moving files for $PKG_NAME."
    899902            exit 1

I'm fine with adding this to your existing patch to uninstall.py as long a comment is also added pointing back to this ticket (and assuming this patch does in fact work).

comment:18 Changed 2 years ago by git

  • Commit changed from 40e0cfb75ef3a0a18f9ba00ad9a1eabcf5d7fc41 to 48f7cdc4f7f80041ba843b459696415cac488e55

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

48f7cdctrac 26013: make sure that sage-spkg generates relative filenames for

comment:19 Changed 2 years ago by jhpalmieri

Your patch works for me. Here is a new branch.

comment:20 Changed 2 years ago by embray

  • Reviewers set to Erik Bray
  • Status changed from needs_review to positive_review

Thanks!

comment:21 Changed 2 years ago by vbraun

  • Branch changed from u/jhpalmieri/spkg-uninstall-os-sep to 48f7cdc4f7f80041ba843b459696415cac488e55
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.