sagemath/sage

src/bin/sage: Delegate handling of options of sage-the-distribution, specifying a subset that is supported by sagelib proper

mkoeppe opened this issue · 146 comments

The sage script has a number of options that only make sense for sage-the-distribution, but do not make sense in downstream packaging of sage. Consequently, downstream packagers may install simple custom versions of the script that only provide a subset of the options.

Examples:

  • sage -sqlite -- as discussed in #29002/#29092 -- because from a downstream packaging point of view, sage is not really responsible for providing sqlite, it's provided by justs another system package
  • sage -upgrade -- only makes sense with sage-the-distribution built from source

We should specify the subset of sage command line options that must be supported in any deployment/packaging of sage. This is so that user packages can work reliably.

Examples:

  • sage -c SAGECOMMAND -- obviously
  • sage -python -c COMMAND
  • sage -t -- because it is used by user packages for testing sage sources
  • sage -sh ..... ?

In this ticket, we actually split src/bin/sage into sagelib functionality and sage-the-distribution functionality. src/bin/sage delegates unknown options to build/bin/sage-site.

Ideally, distributions should be able to use an unmodified src/bin/sage.

Context:

See also:

  • #29884: src/doc/bootstrap: Generate src/doc/en/reference/repl/options.rst

Depends on #29878
Depends on #29289
Depends on #29884

CC: @kiwifb @isuruf @dimpase @embray @saraedum @antonio-rojas @slel @EmmanuelCharpentier @orlitzky @kcrisman

Component: scripts

Author: Matthias Koeppe, John Palmieri

Branch: 3953671

Reviewer: Matthias Koeppe, John Palmieri, François Bissey

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

comment:1

As I said on #29092, we should also move corresponding sage-the-distribution tests (such as those for sage -sqlite) mentioned in #29002) from src (which is for sagelib) to somewhere in build (which is for sage-the-distribution). Downstream packagers would ignore the sage-the-distribution tests.

comment:3

pushing these forward to 9.2

Description changed:

--- 
+++ 
@@ -12,7 +12,7 @@
 - `sage -t` -- because it is used by user packages for testing sage sources
 - `sage -sh` ..... ?
 
-(In addition to specifying this, we might actually want to split `src/bin/sage` into sagelib functionality and sage-the-distribution functionality.)
+(In addition to specifying this, we might actually want to split `src/bin/sage` into sagelib functionality and sage-the-distribution functionality. Ideally, distributions should be able to use an unmodified `src/bin/sage`, which would delegate unknown options to an optional "site" script that would be provided by sage-the-distribution.)
 
 Context:
 - https://groups.google.com/d/msg/sage-packaging/BmkxIBdwbvE/fRMl2sjdBQAJ
comment:6

I've asked this question before: if Sage is built with the system's Python, should we still keep sage --python ...? What is the difference between sage --python and sage --sqlite3 (and what about sage --gap, sage --singular, etc.)?

comment:7

Here are some arguments that should always be allowed:

  • sage -t
  • sage -c
  • sage -h
  • sage -n, sage --notebook
  • sage --nodotsage
  • sage -v, sage --version

"Advanced" arguments:

  • sage --preparse
  • sage --min
  • sage -q
  • sage --dumpversion
  • sage --startuptime
  • sage --fixdoctests
  • sage --coverage
  • sage --coverageall
  • sage --grep, sage --search_src, sage --grepdoc, sage --search_doc
  • sage --rst2ipynb, sage --ipynb2rst

Maybe also sage --sh.

(I'm not saying is a complete list, but I would suggest these as a starting point.)

comment:8

By the way, sage -twistd doesn't work: there is no twistd executable since Sage 9.1. Plus we are removing the twisted package in #29754. See #29878.

comment:9

Hum, this ticket reminds me that I need to clean the version in sage-on-gentoo

fbissey@moonloop ~ $ sage -advanced
SageMath version 9.2.beta1, Release Date: 2020-06-13

Running Sage:
  file.[sage|py|spyx] -- run given .sage, .py or .spyx file
  -advanced           -- print this advanced help message
  -c <cmd>            -- Evaluates cmd as sage code
  -h, -?              -- print short help message
  -min [...]          -- do not populate global namespace (must be first
                         option)
  -preparse <file.sage> -- preparse file.sage and produce corresponding file.sage.py
  -q                  -- quiet; start with no banner
  -gthread, -qthread, -q4thread, -wthread, -pylab
                      -- pass the option through to ipython
  -v, -version        -- display Sage version information
  -dumpversion        -- print Sage version

Running the notebook:
  --notebook=[...]    -- start the Sage notebook (valid options are
                         'default', 'sagenb', 'jupyter' and 'export').
                         Current default is 'export' sagenb to jupyter.
                         See the output of sage --notebook --help
                         for more details and examples of how to pass
                         optional arguments
  -inotebook [...]    -- start the *insecure* Sage notebook (deprecated)
  -n, -notebook [...] -- start the default Sage notebook (options are the
                         same as for the notebook command in Sage).  See the
                         output of sage -n -h for more details

Running external programs:
  -cleaner            -- run the Sage cleaner
  -cython [...]       -- run Cython with given arguments
  -ecl [...]          -- run Common Lisp
  -gap [...]          -- run Sage's Gap with given arguments
  -gdb                -- run Sage under the control of gdb
  -gp [...]           -- run Sage's PARI/GP calculator with given arguments
  -ipython [...]      -- run Sage's IPython using the default environment (not
                         Sage), passing additional options to IPython
  -ipython3 [...]     -- same as above, but using Python 3
  -jupyter [...]      -- run Sage's Jupyter with given arguments
  -kash [...]         -- run Sage's Kash with given arguments
                         (not installed currently, run sage -i kash)
  -lisp [...]         -- run Lisp interpreter included with Sage
  -M2 [...]           -- run Sage's Macaulay2 with given arguments
                         (not installed currently, run sage -i macaulay2)
  -maxima [...]       -- run Sage's Maxima with given arguments
  -mwrank [...]       -- run Sage's mwrank with given arguments
  -polymake [...]     -- run Sage's Polymake with given arguments
                         (not installed currently, run sage -i polymake)
  -python [...]       -- run the Python interpreter
  -python2 [...]      -- run the Python 2 interpreter
  -python3 [...]      -- run the Python 3 interpreter
  -R [...]            -- run Sage's R with given arguments
  -sh [...]           -- run $SHELL (/bin/bash) with Sage environment variables
  -singular [...]     -- run Sage's singular with given arguments
  -sqlite3 [...]      -- run Sage's sqlite3 with given arguments
  -twistd [...]       -- run Twisted server

Testing the Sage library:
  -startuptime [module] -- display how long each component of Sage takes to
                         start up; optionally specify a module to get more
                         details about that particular module
  -t [options] <--all|files|dir>
                      -- test examples in .py, .pyx, .sage, .tex or .rst files
                         selected options:
                           --long - include lines with the phrase 'long time'
                           --verbose - print debugging output during the test
                           --all - test all files
                           --sagenb - test all sagenb files
                           --optional - controls which optional tests are run
                           --initial - only show the first failure per block
                           --debug - drop into PDB after an unexpected error
                           --failed - only test files that failed last test
                           --warn-long [timeout] - warning if doctest is slow
                           --only-errors - only output failures, not successes
                           --gc=GC - control garbarge collection (ALWAYS:
                                     collect garbage before every test; NEVER:
                                     disable gc; DEFAULT: Python default)
                           --help - show all testing options
  -tp <N> [...]       -- like -t above, but tests in parallel using N threads
                         with 0 interpreted as a sensible default
  -testall [options]  -- test all source files, docs, and examples.  options
                         like -t

File conversion:
  -rst2ipynb [...]    -- Generates Jupyter notebook (.ipynb) from standalone
                         reStructuredText source.
                         (not installed currently, run sage -i rst2ipynb)
  -ipynb2rst [...]    -- Generates a reStructuredText source file from
                         a Jupyter notebook (.ipynb).

Valgrind memory debugging:
  -cachegrind         -- run Sage using Valgrind's cachegrind tool.  The log
                         files are named sage-cachegrind.PID can be found in
                         
  -callgrind          -- run Sage using Valgrind's callgrind tool.  The log
                         files are named sage-callgrind.PID can be found in
                         
  -massif             -- run Sage using Valgrind's massif tool.  The log
                         files are named sage-massif.PID can be found in
                         
  -memcheck           -- run Sage using Valgrind's memcheck tool.  The log
                         files are named sage-memcheck.PID can be found in
                         
  -omega              -- run Sage using Valgrind's omega tool.  The log
                         files are named sage-omega.PID can be found in
                         
  -valgrind           -- this is an alias for -memcheck

You can also use -- before a long option, e.g., 'sage --optional'.

I have to drop the py2/py3 stuff for just defaut python. I dropped -sh the day I dropped sage-env altogether, the main point of keeping sage -sh around was to check or use the environment set by sage-env.

comment:10

Dropping sage -sh is a bad idea. User packages need this

comment:11

Replying to @jhpalmieri:

I've asked this question before: if Sage is built with the system's Python, should we still keep sage --python ...? What is the difference between sage --python and sage --sqlite3 (and what about sage --gap, sage --singular, etc.)?

sage -python means: run the python that sage runs in.
In distributions, this could just be plain Python.

Install scripts of user packages need this.

Dependencies: #29878

comment:13

Replying to @mkoeppe:

Dropping sage -sh is a bad idea. User packages need this

Certainly, vanilla sage does need it. In sage-on-gentoo at best you would have a normal shell since I dropped sage-env altogether.

comment:14

Replying to @kiwifb:

Replying to @mkoeppe:

Dropping sage -sh is a bad idea. User packages need this

Certainly, vanilla sage does need it. In sage-on-gentoo at best you would have a normal shell since I dropped sage-env altogether.

Yes, vanilla shell is fine. The point is that user packages need the interface.

Changed dependencies from #29878 to #29878, #29289

comment:16

Now, I understand what you mean. User scripts may start with sage -sh and what not. I completely forgot about that when I removed it - even though that was one of the reason I was keeping it around. I will re-instate it as a vanilla shell or no-op for compatibility.

comment:17

Great!

comment:19

Here's a beginning of what I have in mind.


New commits:

b200d70trac 29878: remove "sage --twistd" command-line argument
e0b6112build/bin/sage-spkg, src/bin/sage: Remove support for installing old-style SPKGs
20b2a93Merge branch 't/29289/remove_support_for_installing_old_style_spkgs__deprecated_in_sage_6_9' into t/29111/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution
152e581src/bin/sage: Delegate options that pertain to SAGE_ROOT or sage-the-distribution to the new script build/bin/sage-site

Commit: 152e581

comment:20

Okay, I'm fine with keeping sage --python. What about sage --singular et al.? Should src/bin/sage be constructed by ./configure, adding arguments when the corresponding packages are going to be installed by Sage? Or is that too complicated?

Description changed:

--- 
+++ 
@@ -12,7 +12,9 @@
 - `sage -t` -- because it is used by user packages for testing sage sources
 - `sage -sh` ..... ?
 
-(In addition to specifying this, we might actually want to split `src/bin/sage` into sagelib functionality and sage-the-distribution functionality. Ideally, distributions should be able to use an unmodified `src/bin/sage`, which would delegate unknown options to an optional "site" script that would be provided by sage-the-distribution.)
+In this ticket, we actually split `src/bin/sage` into sagelib functionality and sage-the-distribution functionality. `src/bin/sage` delegates unknown options to `build/bin/sage-site`. 
+
+Ideally, distributions should be able to use an unmodified `src/bin/sage`.
 
 Context:
 - https://groups.google.com/d/msg/sage-packaging/BmkxIBdwbvE/fRMl2sjdBQAJ
comment:22

Replying to @jhpalmieri:

Okay, I'm fine with keeping sage --python. What about sage --singular et al.?

I think those should be moved to sage-site as well. (Also the printing of these in the help text...)

Should src/bin/sage be constructed by ./configure, adding arguments when the corresponding packages are going to be installed by Sage? Or is that too complicated?

I think that's too complicated

comment:23

sage --grep, sage --search_src, sage --grepdoc, sage --search_doc

At the risk of starting another holy war, we shouldn't expect these to work on a distribution because we shouldn't expect the source files to be available after installation. My current sage.git checkout contains 71MiB of *.py files that are entirely redundant after the optimized .opt-2.pyc files are generated. Someone is eventually going to realize that they shouldn't be installed after this gets more popular.

comment:24

We could just conditionalize these ones based on whether SAGE_SRC is set or not by sage-env...

Changed commit from 152e581 to e734d55

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

e734d55src/bin/sage: Disable options -grep, -grepdoc on distributions without SAGE_SRC

Changed commit from e734d55 to 096ca92

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

096ca92src/bin/sage: Move option -fix-pkg-checksums to sage-site

Author: Matthias Koeppe

comment:27

All, please review, or feel free to push more changes to this ticket.

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

eadae5asrc/bin/sage: Move parts of the help message to sage-site

Changed commit from 096ca92 to eadae5a

Description changed:

--- 
+++ 
@@ -20,3 +20,5 @@
 - https://groups.google.com/d/msg/sage-packaging/BmkxIBdwbvE/fRMl2sjdBQAJ
 - #29060: Meta-ticket: Add Dockerfiles and CI scripts for integration testing of source and binary distributions and of downstream packages
 
+See also:
+- #29884: `src/doc/bootstrap`: Generate `src/doc/en/reference/repl/options.rst`
comment:30

There are also various options regarding notebooks that may need updating. If someone knows about the details, please push changes to the branch

comment:31

Speaking of holy wars, this has reminded me of #21, which I am thinking of trying to revive. I'll see if I have time to work on that. For this ticket, we could remove -inotebook, and sagenb is no longer a valid option for -notebook.

Changed commit from eadae5a to 0856913

comment:33

Speaking of holy wars, this has reminded me of #21, which I am thinking of trying to revive. I'll see if I have time to work on that.

Or holy grails!

For this ticket, we could remove -inotebook, and sagenb is no longer a valid option for -notebook.

I don't remember all the options for -notebook, but I think it would be helpful to have some sort of warning message (or page?) that sagenb would raise, rather than removing it completely. For instance, it could just open the export-to-Jupyter page by default, with an extra little warning template that "if you want to use the old sagenb, please use Sage prior to 9.x". Probably what anyone who tries to use sagenb would want is the export at this point, and this is one way to inform them.

John, what does your branch give for sage -n -h or sage --notebook --help which is somewhat prominently mentioned in the sage -advanced help? Is any of what comes out of that still valid?


New commits:

0856913trac 29111: remove --inotebook, references to sagenb
comment:34

I think that the default for sage --notebook should now be jupyter. This involves only editing sage-notebook, and belongs on a separate ticket: #29885.


New commits:

0856913trac 29111: remove --inotebook, references to sagenb
comment:35

The branch here doesn't change sage -n -h. The branch at #29885 removes "sagenb" from this part of the help message:

  --notebook [NOTEBOOK], -n [NOTEBOOK], -notebook [NOTEBOOK]
                        The notebook to run [one of: default, sagenb, ipython,
                        jupyter, jupyterlab, export]. Default is export
comment:36

Oh, and with the branch at #29885, running sage -n sagenb prints

CRITICAL:root:cannot use the legacy notebook SageNB with Python 3
The legacy notebook does not work under Python 3; use the Jupyter notebook.
See https://wiki.sagemath.org/Python3-Switch

The branch here prints something similar, just a bit less grammatical ;)

comment:37

One more thing:

it could just open the export-to-Jupyter page by default

this has been the standard behavior for a while. At #29885, I am proposing changing this to make running the Jupyter notebook the default behavior. We should probably continue the notebook discussion at #29885.

comment:38

We should probably continue the notebook discussion at #29885.

Sure, it's not a big deal in any case.

comment:39

Should #29884 be a dependency for this ticket, or are we also going to manually fix src/doc/en/reference/repl/options.rst?

comment:40

Replying to @jhpalmieri:

Should #29884 be a dependency for this ticket, or are we also going to manually fix src/doc/en/reference/repl/options.rst?

I'll work on the automatic generation in #29884 now.
If you could check if anything from options.rst should be added to the help text, that would be helpful.

comment:41

#29884 is ready now

New commits:

fb9dcdcsrc/bin/sage: Move parts of the help message to sage-site
885092esrc/doc/bootstrap: Generate src/doc/en/reference/repl/options.txt
1ecc840Merge branch 't/29884/src_doc_bootstrap__generate_src_doc_en_reference_repl_options_rst' into t/29111/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution

Changed commit from 0856913 to 1ecc840

Changed dependencies from #29878, #29289 to #29878, #29289, #29884

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

6ba580bsrc/doc/en/reference/repl/options.rst: Replace copypasta by include of generated file options.txt

Changed commit from 1ecc840 to 6ba580b

comment:45

Replying to @mkoeppe:

Replying to @jhpalmieri:

Should #29884 be a dependency for this ticket, or are we also going to manually fix src/doc/en/reference/repl/options.rst?

I'll work on the automatic generation in #29884 now.
If you could check if anything from options.rst should be added to the help text, that would be helpful.

I tend to like the extra details options.rst more than what is printed by sage --advanced. Would it be too verbose if we used (more or less) the current options.rst for the output to sage --advanced?

I would also organize it differently, distinguishing the options that are not available in distros.

comment:46

Replying to @jhpalmieri:

Replying to @mkoeppe:

Replying to @jhpalmieri:

Should #29884 be a dependency for this ticket, or are we also going to manually fix src/doc/en/reference/repl/options.rst?

I'll work on the automatic generation in #29884 now.
If you could check if anything from options.rst should be added to the help text, that would be helpful.

I tend to like the extra details options.rst more than what is printed by sage --advanced. Would it be too verbose if we used (more or less) the current options.rst for the output to sage --advanced?

Sounds good to me.

I would also organize it differently, distinguishing the options that are not available in distros.

Right, that would be done by moving these into the sage-site function usage_advanced. They are printed below the ones from sage -advanced.

Changed author from Matthias Koeppe to Matthias Koeppe, John Palmieri

Changed commit from 6ba580b to 4b303a2

comment:49

Here are some proposed changes to sage --advanced. I have a strong preference for two hyphens in sage --long-option over sage -long-option. I don't object to Sage parsing the two the same way, but I think we should advertise the two-hyphen version. (If we had a different argument parser, then sage -info would be interpreted as sage -i -n -f -o, whereas sage --info is less ambiguous.)

I wasn't sure how to divide everything between the two files, but it's another draft to work with.


New commits:

4b303a2trac 29111: revising "sage --advanced" message.
comment:50

Other comments: --fix-pkg-checksums was deprecated, so I removed it from the help string. --gap3 does not need to be in the basic sage -h message: it comes from an experimental package. For me, sage -? doesn't work, because the shell tries to parse ?. So I removed that from the sage -h message.

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

611b8d6trac 29111: revising "sage --advanced" message.

Changed commit from 4b303a2 to 611b8d6

comment:52

I agree with these changes.

Some comments on sage --advanced:

  1. I think --grep ...--search_doc should go to src/bin/sage, conditionalized on $SAGE_SRC

  2. I think --gdb*, --python*, --ipython*, --pip, and perhaps --jupyter* should go to src/bin/sage

  3. Also --sh (but NOT --buildsh) and --cleaner should go to src/bin/sage

comment:53

It is very close to something I am willing to ship out of the box in Gentoo. I will mostly echo @mkoeppe last comment. (2) and (3) are totally acceptable in distros. I certainly can run under gdb for example, in fact it is bizarre that gdb is treated differently from valgrind et al. (1) in the way suggested is acceptable.

Also, I think you have an alignment problem in src/bin/sage right now.

+    echo "  --dumpversion        -- print brief Sage version"
+    echo "  --preparse file.sage -- preparse \"file.sage\", and produce"
+    echo "                          the corresponding Python file"
+    echo "                          \"file.sage.py\""
+    echo "  -q                   -- quiet; start with no banner"
+    echo "  --min                -- do not populate global namespace"
+    echo "                          (must be first option)"
+    echo "   --nodotsage          -- run Sage without using the user's"
+    echo "                          .sage directory: create and use a temporary"
+    echo "                          .sage directory instead."
+    echo "   --gthread, --qthread, --q4thread, --wthread, --pylab"
+    echo "                        -- pass the option through to IPython"

--nodotsage and --gthread are one space further to the left than -q and --min.

comment:54

Is there a reason for this difference in the sage script?

usage() {
...
        exec "$SAGE_ROOT/build/bin/sage-site" "-h"
...

vs

usage_advanced() {
...
       "$SAGE_ROOT/build/bin/sage-site" "--advanced"
...

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

073577btrac 29111: more reorganization of "sage --advanced" message

Changed commit from 611b8d6 to 073577b

comment:56

Replying to @jhpalmieri:

Is there a reason for this difference in the sage script?

usage() {
...
        exec "$SAGE_ROOT/build/bin/sage-site" "-h"
...

vs

usage_advanced() {
...
       "$SAGE_ROOT/build/bin/sage-site" "--advanced"
...

FYI: I changed both to use exec. Feel free to change back if this is wrong.

Changed commit from 073577b to c8daf80

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

c8daf80trac 29111: delete sage-fix-pkg-checksums
comment:58

sage-fix-pkg-checksums was deprecated four years ago (#19984), so I deleted it.

Changed commit from c8daf80 to 5c3ced6

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

5c3ced6trac 29111: delete sage-fix-pkg-checksums
comment:60

One last thing I should have commented on earlier

+    echo "  -bn [...], --build-and-notebook [...]"
+    echo "                      -- build the Sage library (as by running \"sage -b\")"
+    echo "                         and then start the notebook"

is currently in src/bin/sage but really any -b options belong to sage-site and nowhere else.

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

6841389trac 29111: re "sage --advanced" message:

Changed commit from 5c3ced6 to 6841389

comment:62

Replying to @kiwifb:

One last thing I should have commented on earlier

+    echo "  -bn [...], --build-and-notebook [...]"
+    echo "                      -- build the Sage library (as by running \"sage -b\")"
+    echo "                         and then start the notebook"

is currently in src/bin/sage but really any -b options belong to sage-site and nowhere else.

Okay, moved, but on the other hand, I think that the coverage options belong with doctesting, so I moved those from sage-site to sage.

comment:63

Replying to @jhpalmieri:

sage-fix-pkg-checksums was deprecated four years ago (#19984), so I deleted it.

Excellent!

comment:64

Replying to @jhpalmieri:

Okay, moved, but on the other hand, I think that the coverage options belong with doctesting, so I moved those from sage-site to sage.

Never shipped them before, they don't look harmful but it is more development focused than a test suite is. A test suite is run by anyone who wants to check that their install has a certain level of "sanity". coverage is really a tool for devs that look for something to do.

In any case that's not super harmful unlike -b.

comment:65

Replying to @jhpalmieri:

FYI: I changed both to use exec. Feel free to change back if this is wrong.

That's fine. In a previous version, -advanced printed a line after returning from sage-site. That's no longer needed.

comment:66

Replying to @kiwifb:

Replying to @jhpalmieri:

Okay, moved, but on the other hand, I think that the coverage options belong with doctesting, so I moved those from sage-site to sage.

Never shipped them before, they don't look harmful but it is more development focused than a test suite is. A test suite is run by anyone who wants to check that their install has a certain level of "sanity". coverage is really a tool for devs that look for something to do.

Note that like sage -t, also sage -coverage can be run on user code, not just on Sage sources.

comment:67

Replying to @mkoeppe:

Replying to @kiwifb:

Replying to @jhpalmieri:

Okay, moved, but on the other hand, I think that the coverage options belong with doctesting, so I moved those from sage-site to sage.

Never shipped them before, they don't look harmful but it is more development focused than a test suite is. A test suite is run by anyone who wants to check that their install has a certain level of "sanity". coverage is really a tool for devs that look for something to do.

Note that like sage -t, also sage -coverage can be run on user code, not just on Sage sources.

Right, that was my thought, too. And then it didn't make sense to separate coverage and coverageall.

comment:68

It is fine. I will ship those two then. It makes sense.

Reviewer: Matthias Koeppe, ...

comment:69

Looks great to me. Let's get this in.

comment:70

I have a few more questions:

  • I would like to delete the -rsyncdist option. Can we also delete sage-rsyncdist? The file itself says "NOTE: This script is of little use after the git transition and will be deleted eventually."

  • Is it worth moving handling of -gap et al. to sage-site, which is where they are documented?

  • A comment: it would be nice to move sage --sdist handling to sage-site, but then we have to duplicate maybe_sage_location, so it's probably not worth it. Although then we could move sage -b handling, too. This can be done later, if someone feels motivated.

comment:71

Replying to @jhpalmieri:

  • I would like to delete the -rsyncdist option. Can we also delete sage-rsyncdist? The file itself says "NOTE: This script is of little use after the git transition and will be deleted eventually."

Sure, let's get rid of it. There are a number of references to what looks like old-style SPKGs in this script. It was updated last in 2013. So it is likely outdated and broken already.

  • Is it worth moving handling of -gap et al. to sage-site, which is where they are documented?

Yes, good idea, please go ahead.

  • A comment: it would be nice to move sage --sdist handling to sage-site, but then we have to duplicate maybe_sage_location, so it's probably not worth it. Although then we could move sage -b handling, too. This can be done later, if someone feels motivated.

I would suggest to do these in a follow-up ticket. Likewise, for the option handling for sage -i that would really belong to sage-site; this is complicated by the fact that sage-env must not be sourced before sage-spkg is executed.

comment:72

Here's a problem: I currently have for the sage --advanced message

    if [ -n "$SAGE_SRC" -a -d "$SAGE_SRC" ]; then
        echo "  --grep [options] <string>"
        ...

At this point, sage-env hasn't been run. Can I change $SAGE_SRC to $SAGE_ROOT/src? Failing this, we can move this part of the help message to sage-site.

comment:73

The intention of using SAGE_SRC from sage-env here is that packagers would leave it undefined if they do not package a distribution.

How about moving the handling of -h, -advanced below the sourcing of sage-env?

Changed commit from 6841389 to 778fcd5

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

778fcd5trac 29111: more tinkering. Fix doctests in tests/cmdline.py.
comment:75

I used SAGE_ROOT/src, but of course this can be changed. There were a few failing doctests in tests/cmdline.py (mainly because of rewordings in the documentation), now fixed.

comment:76

And now I'm confused about sage -p. This was being actively tested in tests/cmdline.py:

    Test ``sage --info [packages]`` and the equivalent
    ``sage -p --info --info [packages]`` (the doubling of ``--info``
    is intentional, that option should be idempotent)::

Do we indeed remove it?

comment:77

Replying to @jhpalmieri:

I used SAGE_ROOT/src

Fine. Note [ -n "$SAGE_ROOT/src" ] is always true, so it can be simplified

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

3cfb60ctrac 29111: more tinkering. Fix doctests in tests/cmdline.py.

Changed commit from 778fcd5 to 3cfb60c

comment:79

Replying to @mkoeppe:

Replying to @jhpalmieri:

I used SAGE_ROOT/src

Fine. Note [ -n "$SAGE_ROOT/src" ] is always true, so it can be simplified

Sorry, I missed this before pushing the branch. I've moved usage_advanced to after sourcing sage-env. Now --grep et al. show up in the message for me. I've also restored sage -p: I don't think this is the ticket to debate whether to remove it, that should be done on another ticket. (Admittedly, now it is no longer listed in sage -h or sage --advanced, so either it should be listed or it should be removed. But I don't know which way to go with it.)

comment:80

I also did a lot of reorganizing of the sage script, trying to group the different options into sensible ways. The patch is a bit of a mess, so the file itself might be easier to browse through for changes.

comment:81

Replying to @jhpalmieri:

I've also restored sage -p: I don't think this is the ticket to debate whether to remove it, that should be done on another ticket.

It was removed in #29289, a dependency of this ticket.

Changed commit from 3cfb60c to 9c28ee4

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

9c28ee4trac 29111: more tinkering. Fix doctests in tests/cmdline.py.
comment:83

I've tried to undo my changes to sage -p.