Reimplement "sage -p SPKG" as "cd $SAGE_ROOT && make SPKG-no-deps"

[mkoeppe]

This option installs a package without its dependencies, enabling experiments by experienced developers.

After #29013 (Support installation of Python packages into separate venvs depending on the python version), sage-spkg requires an installation-tree argument. As a result, currently sage -p is broken, as reported in #32751.

We remove this direct use of sage-spkg.

Making this change also removes the last use of SAGE_LOGS from src/

Since this now goes through a make target instead of using sage-spkg directly, after switching to a branch that adds a package, developers may need to manually invoke bootstrap and configure before using this option.

Depends on #33127

CC: @jhpalmieri @orlitzky @kliem @vbraun

Component: build

Author: Matthias Koeppe, John Palmieri

Branch/Commit: 9ed0d60

Reviewer: John Palmieri, Matthias Koeppe

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

[mkoeppe]

Dependencies: #30657

[mkoeppe]

Changed dependencies from #30657 to #31263

[mkoeppe]

Branch: u/mkoeppe/reimplement__sage__p_spkg__as__cd__sage_root_build_make____install_spkg_no_deps_

[mkoeppe]

Commit: 427f0bd

[mkoeppe]

Author: Matthias Koeppe

[mkoeppe]

New commits:

427f0bd

src/bin/sage (-p): Use build/make/install SPKG-no-deps

[jhpalmieri]

comment:6

I think that this does not behave according to the documentation: "Options are the same as for the -i command." For example ./sage -p -c bzip2 does not run the test suite, and ./sage -p -s biopython does not preserve the build directory.

[mkoeppe]

comment:7

You are right, this needs more thought

[mkoeppe]

comment:8

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

[mkoeppe]

comment:9

Setting a new milestone for this ticket based on a cursory review.

[mkoeppe]

comment:29

Yes, I'm in favor of deleting this nonsensical test

[jhpalmieri]

Changed branch from u/mkoeppe/reimplement__sage__p_spkg__as__cd__sage_root_build_make____install_spkg_no_deps_ to u/jhpalmieri/reimplement__sage__p_spkg__as__cd__sage_root_build_make____install_spkg_no_deps_

[jhpalmieri]

comment:31

The issues in comment:6 have been fixed.

---

New commits:

8ebd85b

trac 30649: remove "sage -p --info --info ..." test

[jhpalmieri]

Changed commit from d42fabe to 8ebd85b

[mkoeppe]

Changed author from Matthias Koeppe to Matthias Koeppe, John Palmieri

[mkoeppe]

Changed branch from u/jhpalmieri/reimplement__sage__p_spkg__as__cd__sage_root_build_make____install_spkg_no_deps_ to u/mkoeppe/reimplement__sage__p_spkg__as__cd__sage_root_build_make____install_spkg_no_deps_

[jhpalmieri]

comment:34

Any ideas on what else needs to be done with this?

---

New commits:

33d3d1c	src/bin/sage-env: Merge code that sets SAGE_ROOT
fdada1c	src/bin/sage-env: Update comments on SAGE_ENV_SOURCED
7828a79	src/bin/sage-env: Fixup: Move setting of SAGE_ROOT out of the inner-most if-block
2bd0742	src/bin/sage-env: Suppress error message from probing for NEW_SAGE_ROOT
702b779	src/bin/sage-env: Do not set SAGE_ROOT to empty; sage-location does not like it
c9fe2ed	Merge #33127

[jhpalmieri]

Changed commit from 8ebd85b to c9fe2ed

[mkoeppe]

comment:35

I think it is ready

[jhpalmieri]

comment:36

The change from comment:30 and comment:31 seems to have been lost, and I am getting an error when I try to push. I don't want to mess up the rest of the branch, so it might be better if you do it:

diff --git a/src/sage/tests/cmdline.py b/src/sage/tests/cmdline.py
index 681ff703e25..fccba3f12fb 100644
--- a/src/sage/tests/cmdline.py
+++ b/src/sage/tests/cmdline.py
@@ -210,9 +210,7 @@ def test_executable(args, input="", timeout=100.0, pydebug_ignore_warnings=False
         sage: ret  # optional - sage_spkg
         0
 
-    Test ``sage --info [packages]`` and the equivalent
-    ``sage -p --info --info [packages]`` (the doubling of ``--info``
-    is intentional, that option should be idempotent)::
+    Test ``sage --info [packages]``::
 
         sage: out, err, ret = test_executable(["sage", "--info", "sqlite"])  # optional - sage_spkg
         sage: print(out)  # optional - sage_spkg
@@ -225,17 +223,6 @@ def test_executable(args, input="", timeout=100.0, pydebug_ignore_warnings=False
         sage: ret  # optional - sage_spkg
         0
 
-        sage: out, err, ret = test_executable(["sage", "-p", "--info", "--info", "sqlite"])  # optional - sage_spkg
-        sage: print(out)  # optional - sage_spkg
-        sqlite...
-        SQLite is a software library that implements a self-contained,
-        serverless, zero-configuration, transactional SQL database engine.
-        ...
-        sage: err  # optional - sage_spkg
-        ''
-        sage: ret  # optional - sage_spkg
-        0
-
     Test ``sage-run`` on a Python file, both with an absolute and with a relative path::
 
         sage: dir = tmp_dir(); name = 'python_test_file.py'

[sagetrac-git]

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

65989cc

trac 30649: remove "sage -p --info --info ..." test

[sagetrac-git]

Changed commit from c9fe2ed to 65989cc

[mkoeppe]

comment:38

Sorry for dropping this! Here it is

[jhpalmieri]

comment:39

I propose the following two changes (the first one because this is already in sage-site and the option is handled twenty lines later, the second is just a typo not related to the changes here but one that you see when you run ./sage -p):

diff --git a/build/bin/sage-site b/build/bin/sage-site
index 78848c389e..f7164503b7 100755
--- a/build/bin/sage-site
+++ b/build/bin/sage-site
@@ -129,7 +129,7 @@ if [ "$1" = '-standard' -o "$1" = "--standard" ]; then
 fi
 
 if [ "$1" = '-p' ]; then
-    # If there are no further arguments, display usage help (via an option handled by sage-site)
+    # If there are no further arguments, display usage help
     if [ $# -eq 1 ]; then
         set -- --info
     else
diff --git a/build/bin/sage-spkg b/build/bin/sage-spkg
index 1af55f6671..4478bc7f2d 100755
--- a/build/bin/sage-spkg
+++ b/build/bin/sage-spkg
@@ -99,7 +99,7 @@ Options:
    -y: automatically reply "y" for all prompts regarding
        experimental and old-style packages; warning: there
        is no guarantee that these packages will build correctly;
-       use at your own risk"
+       use at your own risk
    -n: automatically reply "n" for all prompts regarding
        experimental and old-style packages
    -o: allow fetching the package from its upstream URL

Also for a future ticket: should ./sage -i and ./sage -p (with no further arguments) behave similarly? ./sage -i builds some stuff, while ./sage -p just prints a usage message and exits.

[sagetrac-git]

Changed commit from 65989cc to 9ed0d60

[sagetrac-git]

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

9ed0d60

build/bin/sage-site: Edits to comments, help text

[mkoeppe]

comment:41

Replying to @jhpalmieri:

should ./sage -i and ./sage -p (with no further arguments) behave similarly? ./sage -i builds some stuff, while ./sage -p just prints a usage message and exits.

I suppose sage -p could be reduced to just saying "Nothing to do." instead of showing usage.

[mkoeppe]

comment:42

And it's not clear if sage -i running make all-build is really useful, so we could consider changing this behavior too.

But as you say, that would be a future ticket.

[mkoeppe]

Reviewer: ..., Matthias Koeppe

[jhpalmieri]

Changed reviewer from ..., Matthias Koeppe to John Palmieri, Matthias Koeppe

[jhpalmieri]

comment:44

Looks good to me. It has some advantages that I didn't expect. Without this branch:

% ./sage -p pytest
Error: Installing old-style SPKGs is no longer supported

whereas with this branch, ./sage -p pytest works fine (although it says "Successfully installed iniconfig-1.1.1 pytest-7.0.1" so it is still doing some dependency checking — probably pip or similar, not Sage's build system).

[mkoeppe]

comment:45

Thanks!

[vbraun]

Changed branch from u/mkoeppe/reimplement__sage__p_spkg__as__cd__sage_root_build_make____install_spkg_no_deps_ to 9ed0d60