./sage -f sagelib no longer works

[jdemeyer]

This used to be a convenient way to force-rebuild the whole Sage library:

$ ./sage -f sagelib
...
Error: package 'sagelib' not found
Note: if it is an old-style package, use -p instead of -i to install it

Similar for ./sage -f doc.

CC: @embray

Component: build

Author: Erik Bray

Branch/Commit: 2ad88ae

Reviewer: Jeroen Demeyer

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

[embray]

comment:2

This reminds me, it's been on my TODO list to just make "sagelib" another spkg (with some special disposition that allows its "source" to be found and built in a local directory rather than downloaded as a tarball and extracted to a temp dir). I'm not sure why this would be broken now though.

[embray]

comment:3

In src/bin/sage there's a kind of hackish "grep" of build/make/Makefile to ensure that the argument to ./sage -f (mostly assumed to be name of an SPKG) is actually a make target. But it seems from the usage you describe here, really ./sage -f should work for almost any make target, not just SPKGs.

I guess either the check could be made more generic or just removed entirely.

Also is this actually documented behavior? If not I wouldn't consider it a "blocker". The --help just says of -f "force build of the given Sage packages". It's not clear to me whether "sagelib" and "doc" are considered "Sage packages" (per my above comment maybe the should be...)

[jdemeyer]

comment:4

Replying to @embray:

Also is this actually documented behavior?

I don't think so, but it should be. I have certainly recommended running ./sage -f sagelib to people.

[embray]

comment:5

Well how would you suggest handling it? IMO the actual behavior you desire should be documented before moving forward on fixing it. What targets should it work for? Just "sagelib" and "doc"? Or others as well? Previously, it technically worked for any target in the Makefile.

[jdemeyer]

comment:6

Replying to @embray:

Previously, it technically worked for any target in the Makefile.

It is actually by design that ./sage -i works that way. So maybe the easiest fix is removing this check:

        # First check that $PKG is actually a Makefile target
        # Since https://github.com/sagemath/sage-prod/issues/21524 there is not explicitly
        # a target for the package, but if it has a vers_<pkgname> variable
        # there will also be a generated rule for it
        if ! grep "^vers_$PKG = " build/make/Makefile >/dev/null; then
            echo >&2 "Error: package '$PKG' not found"
            echo >&2 "Note: if it is an old-style package, use -p instead of -i to install it"
            exit 1
        fi

[jdemeyer]

Branch: u/jdemeyer/__sage__f_sagelib_no_longer_works

[jdemeyer]

Commit: 98db16e

[jdemeyer]

New commits:

98db16e

Remove Makefile target check

[jdemeyer]

Author: Jeroen Demeyer

[embray]

Reviewer: Erik Bray

[embray]

comment:9

FWIW I don't think 'build' commands should be combined into Sage's runtime interface at all. The sole exception might be sage -i (or an equivalent) for installing optional packages, and even that has its problems.

[embray]

comment:10

Actually, upon testing this I take it back. This is rather user-hostile because if you run ./sage -i <unknown_or_misspelled_package> it will still just fire up make, possibly rebuild some other things (as it did for me, since I had to rebuild my toolchain packages due to previous tinkering) and only much later give an error from make that the target is not found (and a hard to understand error, at that, if you don't know what you're looking for).

[embray]

Changed branch from u/jdemeyer/__sage__f_sagelib_no_longer_works to u/embray/build/sage-i-valid-targets

[embray]

comment:11

What about something like this? If you run make list it simply prints a list of all the packages the makefile knows about, as well as some additional "packages" listed in the SAGE_I_TARGETS variable. Ccurrently just "sagelib" and "doc" but others could be added--though I don't think every makefile target is necessarily useful or valid in this context.

[embray]

Changed commit from 98db16e to none

[sagetrac-git]

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

a72231b

Have the makefile itself provide a list of the valid targets that can be used with sage -i, for example.

[sagetrac-git]

Commit: a72231b

[sagetrac-git]

Changed commit from a72231b to 5a0e558

[sagetrac-git]

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

5a0e558

Some minor tweaks--in particular package names must be spelled exactly

[dimpase]

comment:14

I agree with Erik that the misspelt package names are a nuisance, resulting in misleading logs.

[embray]

Changed branch from u/jdemeyer/__sage__f_sagelib_no_longer_works to u/embray/build/sage-i-valid-targets

[dimpase]

comment:16

apologies for messing up the branch.
I forgot that this happens if the ticket was changed in the meantime, and I didn't click somthing (I am hopeless with GUIs in general :-))

[embray]

comment:17

No problem I've done that too.

[jdemeyer]

comment:18

needs review?

[embray]

comment:19

Replying to @jdemeyer:

needs review?

I guess, if you agree with my general analysis of the situation. I posted my branch as an offer for an alternative solution, but it only "needs review" if you agree with my previous comments.

[jdemeyer]

Changed reviewer from Erik Bray to Jeroen Demeyer

[jdemeyer]

Changed author from Jeroen Demeyer to Erik Bray

[jdemeyer]

comment:20

What's the point of

@$(MAKE) -q build/make/Makefile >&2

Wouldn't it make more sense to actually build build/make/Makefile?

[jdemeyer]

comment:21

Apart from that, this works fine.

[embray]

comment:22

I think you misread that line. That's exactly what it's doing.

[jdemeyer]

comment:23

I think you either misread the make manpage or you have a different version of make than me.

       -q, --question
            ``Question mode''.  Do not run any commands, or print anything; just return an exit status that  is  zero  if  the  specified  targets  are
            already up to date, nonzero otherwise.

[embray]

comment:25

Oops, sorry, you're right. I had it confused with -s for --silent (which confusingly can also be spelled --quiet, but not -q).

[sagetrac-git]

Changed commit from 5a0e558 to 2ad88ae

[sagetrac-git]

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

2ad88ae

this was supposed to be -s, not -q; let's use long names instead so we don't confuse ourselves again

[embray]

comment:27

The overall intent was so that in the (unusual) case that build/make/Makefile has to be generated in order to produce the list, the only output to stdout would still be the list itself.

[vbraun]

Changed branch from u/embray/build/sage-i-valid-targets to 2ad88ae