fix some case issues in sage-fix-pkg-checksums

[jhpalmieri]

The Sage developer's guide insists that the directories in build/pkgs should be all lowercase. At the same time, we have started allowing tarballs in upstream which have mixed case (e.g., Pillow and Sphinx). The script sage-fix-pkg-checksums should allow for this, converting the tarball name to lowercase before looking for the corresponding directory.

Component: build

Author: John Palmieri

Branch: 02ba704

Reviewer: Leif Leonhardy

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

[jhpalmieri]

Branch: u/jhpalmieri/case

[jhpalmieri]

comment:2

Since we want to support OS X, we cannot allow different files whose names are the same except for case differences. So I hope this change is safe.

---

New commits:

02ba704

#18344: case issues in sage-fix-pkg-checksums

[jhpalmieri]

Commit: 02ba704

[nexttime]

comment:3

While your patch apparently fixes the issue with uppercase letters in upstream tarballs (haven't tested it yet though), I don't like the concept of the script, which is pretty upside-down: Instead of iterating over build/pkgs/* and taking the name of the upstream tarball from there, it does the opposite.

I'd rather have

sage-pkg-checksums [--verbose] [--create|--check|--update] [<package name>]*

with useful information about missing upstream tarballs, checksum mismatches etc., but that's presumably beyond this ticket.

(And we have a wild mixture of tr, sed, shopt -s nocaseglob, and shell parameter expansion in various shell scripts... The Python script sage-pkg also seems to be obsolete or at least not yet suited for "new style" spkgs.)

[nexttime]

Reviewer: Leif Leonhardy

[nexttime]

comment:5

P.S.: How am I supposed to call that script btw.? It relies on SAGE_ROOT being set, and doesn't even check whether that's the case.

./sage -fix-pkg-checksums doesn't work (and isn't documented), so one has to enter a Sage subshell or, as I did, for example run

SAGE_ROOT=. src/bin/sage-fix-pkg-checksums

./sage --sh -c sage-fix-pkg-checksums isn't less cryptic or tedious either.

[nexttime]

comment:6

... we could at least add

if [ -z "$SAGE_ROOT" ]; then
    if [ -d upstream ]; then # probably add further sanity checks
        SAGE_ROOT=`pwd`
    else
        echo >&2 "Error: SAGE_ROOT not set."
        exit 1
    fi
fi

Otherwise the script does nothing and exits without an error, even when we're in the Sage root directory but SAGE_ROOT isn't set (just because [ -d /build/pkgs/... ] usually yields false).

[jhpalmieri]

comment:7

Replying to @nexttime:

P.S.: How am I supposed to call that script btw.? It relies on SAGE_ROOT being set, and doesn't even check whether that's the case.

The developer's guide says to do

$ sage -sh sage-fix-pkg-checksums

Anyway, feel free to open another ticket with general cleanup of the script.

[vbraun]

Changed branch from u/jhpalmieri/case to 02ba704

[jdemeyer]

comment:9

What's the point of this

 ** Note: case mismatch between upstream/Pillow-2.2.2.tar.gz and build/pkgs/pillow. **

[jdemeyer]

Changed commit from 02ba704 to none

[nexttime]

comment:10

Replying to @jdemeyer:

What's the point of this

 ** Note: case mismatch between upstream/Pillow-2.2.2.tar.gz and build/pkgs/pillow. **

It's a note.

(On partially case-insensitive filesystems, using uppercase letters in tarball names might be unintentional, I think. It's not an error though, since we can deal with such. But we IMHO shouldn't randomly change case upon upgrades.)

[jhpalmieri]

comment:11

One point behind the note is to somehow warn people that because of OS X, we can't have packages whose names depend on case. So at least this way they see that "Pillow.tar.gz" is associated with the directory "pillow". Maybe if someone tries to create a package "GAP", in addition to the "gap" package, rather than have things perhaps mysteriously break during building, a message like this will pop up earlier letting them know something is going on.

[jdemeyer]

comment:12

I don't really like notes which don't seem to have any meaning...

Is it a warning? Is there a problem? Should I avoid that?

Do you agree to remove that note in a follow-up ticket?

[jdemeyer]

comment:13

Replying to @jhpalmieri:

Maybe if someone tries to create a package "GAP", in addition to the "gap" package, rather than have things perhaps mysteriously break during building, a message like this will pop up earlier letting them know something is going on.

Let me explain this one more time: sage-fix-pkg-checksums has nothing to do with building Sage. It's a development tool and only that.

[jhpalmieri]

comment:14

Replying to @jdemeyer:

Replying to @jhpalmieri:

Maybe if someone tries to create a package "GAP", in addition to the "gap" package, rather than have things perhaps mysteriously break during building, a message like this will pop up earlier letting them know something is going on.

Let me explain this one more time: sage-fix-pkg-checksums has nothing to do with building Sage. It's a development tool and only that.

Do I need to explain that, presumably, someone developing a new package for Sage will try to build Sage with that package? That was the context for my comment: I want to provide some feedback when they are still developing the package (and therefore running sage-fix-pkg-checksums), before they even get to the building/testing stage. And there may be developers who never use OS X and are unaware of how it treats case.

[jhpalmieri]

comment:15

In any case, I don't care too much about the note, so if you want to remove it, go ahead.

[jdemeyer]

comment:16

Replying to @jhpalmieri:

I want to provide some feedback when they are still developing the package (and therefore running sage-fix-pkg-checksums), before they even get to the building/testing stage. And there may be developers who never use OS X and are unaware of how it treats case.

OK, got it. But the current note is completely meaningless: I still don't understand what it really is trying to tell me...

Follow-up at #18402