#29111 closed enhancement (fixed)

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

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.2
Component: scripts Keywords:
Cc: fbissey, isuruf, dimpase, embray, saraedum, arojas, slelievre, charpent, mjo, kcrisman Merged in:
Authors: Matthias Koeppe, John Palmieri Reviewers: Matthias Koeppe, John Palmieri, François Bissey
Report Upstream: N/A Work issues:
Branch: 3953671 (Commits, GitHub, GitLab) Commit: 39536716aef05e8953bdc50038c49e5959ffdb6a
Dependencies: #29878, #29289, #29884 Stopgaps:

Status badges

Description (last modified by mkoeppe)

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

Change History (125)

comment:1 Changed 20 months ago by mkoeppe

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.

Last edited 20 months ago by mkoeppe (previous) (diff)

comment:2 Changed 20 months ago by mkoeppe

  • Component changed from PLEASE CHANGE to scripts

comment:3 Changed 17 months ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

pushing these forward to 9.2

comment:4 Changed 16 months ago by mjo

  • Cc mjo added

comment:5 Changed 15 months ago by mkoeppe

  • Description modified (diff)

comment:6 follow-up: Changed 15 months ago by 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.)?

comment:7 Changed 15 months ago by jhpalmieri

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 Changed 15 months ago by jhpalmieri

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.

Last edited 15 months ago by jhpalmieri (previous) (diff)

comment:9 Changed 15 months ago by fbissey

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 follow-up: Changed 15 months ago by mkoeppe

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

comment:11 in reply to: ↑ 6 Changed 15 months ago by mkoeppe

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.

comment:12 Changed 15 months ago by mkoeppe

  • Dependencies set to #29878

comment:13 in reply to: ↑ 10 ; follow-up: Changed 15 months ago by fbissey

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 in reply to: ↑ 13 Changed 15 months ago by mkoeppe

Replying to fbissey:

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_.

comment:15 Changed 15 months ago by mkoeppe

  • Dependencies changed from #29878 to #29878, #29289

comment:16 Changed 15 months ago by fbissey

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 Changed 15 months ago by mkoeppe

Great!

comment:18 Changed 15 months ago by mkoeppe

  • Branch set to u/mkoeppe/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution

comment:19 Changed 15 months ago by mkoeppe

  • Commit set to 152e58155fe193fd9215bee3bf20755c5e0c51a2

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

comment:20 follow-up: Changed 15 months ago by jhpalmieri

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?

comment:21 Changed 15 months ago by mkoeppe

  • Description modified (diff)
  • Summary changed from Specify a subset of sage command line options that are supported by sagelib - rather than sage-the-distribution to src/bin/sage: Delegate handling of options of sage-the-distribution, specifying a subset that is supported by sagelib proper

comment:22 in reply to: ↑ 20 Changed 15 months ago by mkoeppe

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 Changed 15 months ago by mjo

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 Changed 15 months ago by mkoeppe

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

comment:25 Changed 15 months ago by git

  • Commit changed from 152e58155fe193fd9215bee3bf20755c5e0c51a2 to e734d55ff8bcff42a7c9b0f750dc066d2afa0252

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

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

comment:26 Changed 15 months ago by git

  • Commit changed from e734d55ff8bcff42a7c9b0f750dc066d2afa0252 to 096ca9293b7ddef99c4b722f8604af0a2a420799

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

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

comment:27 Changed 15 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Status changed from new to needs_review

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

comment:28 Changed 15 months ago by git

  • Commit changed from 096ca9293b7ddef99c4b722f8604af0a2a420799 to eadae5aad05ed13b4fc78db9ddf7ee8c67331274

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

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

comment:29 Changed 15 months ago by mkoeppe

  • Description modified (diff)

comment:30 Changed 15 months ago by mkoeppe

  • Cc kcrisman added

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 follow-up: Changed 15 months ago by jhpalmieri

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.

comment:32 Changed 15 months ago by jhpalmieri

  • Branch changed from u/mkoeppe/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution to u/jhpalmieri/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution

comment:33 in reply to: ↑ 31 Changed 15 months ago by kcrisman

  • Commit changed from eadae5aad05ed13b4fc78db9ddf7ee8c67331274 to 08569132d2b8dfd54a4446df7da200d303ad9ce1

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 Changed 15 months ago by jhpalmieri

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 Changed 15 months ago by jhpalmieri

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 Changed 15 months ago by jhpalmieri

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 follow-up: Changed 15 months ago by jhpalmieri

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 in reply to: ↑ 37 Changed 15 months ago by kcrisman

We should probably continue the notebook discussion at #29885.

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

comment:39 follow-up: Changed 15 months ago by jhpalmieri

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 in reply to: ↑ 39 ; follow-up: Changed 15 months ago by 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.

comment:41 Changed 15 months ago by mkoeppe

#29884 is ready now

comment:42 Changed 15 months ago by mkoeppe

  • Branch changed from u/jhpalmieri/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution to u/mkoeppe/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution

comment:43 Changed 15 months ago by mkoeppe

  • Commit changed from 08569132d2b8dfd54a4446df7da200d303ad9ce1 to 1ecc84042fbdc77eabb23dc33954658ca3e5b276
  • Dependencies changed from #29878, #29289 to #29878, #29289, #29884

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

comment:44 Changed 15 months ago by git

  • Commit changed from 1ecc84042fbdc77eabb23dc33954658ca3e5b276 to 6ba580b6523280f1510a200ef519f61b512bbaf2

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

comment:45 in reply to: ↑ 40 ; follow-up: Changed 15 months ago by 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?

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

comment:46 in reply to: ↑ 45 Changed 15 months ago by mkoeppe

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.

comment:47 Changed 15 months ago by mkoeppe

  • Authors changed from Matthias Koeppe to Matthias Koeppe, John Palmieri

comment:48 Changed 15 months ago by jhpalmieri

  • Branch changed from u/mkoeppe/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution to u/jhpalmieri/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution

comment:49 follow-up: Changed 15 months ago by jhpalmieri

  • Commit changed from 6ba580b6523280f1510a200ef519f61b512bbaf2 to 4b303a215c8b6df3cecb57621f19afc7e65ab567

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 Changed 15 months ago by jhpalmieri

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.

comment:51 Changed 15 months ago by git

  • Commit changed from 4b303a215c8b6df3cecb57621f19afc7e65ab567 to 611b8d626b0f370830689e6c038287fb2c0a5ea6

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

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

comment:52 in reply to: ↑ 49 ; follow-up: Changed 15 months ago by mkoeppe

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 Changed 15 months ago by fbissey

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 in reply to: ↑ 52 ; follow-up: Changed 15 months ago by 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"
...

comment:55 Changed 15 months ago by git

  • Commit changed from 611b8d626b0f370830689e6c038287fb2c0a5ea6 to 073577b867f440b139ceb26fa6fddf5df5c3b93a

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

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

comment:56 in reply to: ↑ 54 ; follow-up: Changed 15 months ago by jhpalmieri

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.

comment:57 Changed 15 months ago by git

  • Commit changed from 073577b867f440b139ceb26fa6fddf5df5c3b93a to c8daf8054418491abbc2e147aad024a8eb98c00c

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

c8daf80trac 29111: delete sage-fix-pkg-checksums

comment:58 follow-up: Changed 15 months ago by jhpalmieri

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

comment:59 Changed 15 months ago by git

  • Commit changed from c8daf8054418491abbc2e147aad024a8eb98c00c to 5c3ced6afb74de327f067145a49a5506a9a54ea1

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 follow-up: Changed 15 months ago by fbissey

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.

comment:61 Changed 15 months ago by git

  • Commit changed from 5c3ced6afb74de327f067145a49a5506a9a54ea1 to 684138996fc44134dc18d2e8761eecc8caafb39e

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

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

comment:62 in reply to: ↑ 60 ; follow-up: Changed 15 months ago by jhpalmieri

Replying to fbissey:

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 in reply to: ↑ 58 Changed 15 months ago by mkoeppe

Replying to jhpalmieri:

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

Excellent!

comment:64 in reply to: ↑ 62 ; follow-up: Changed 15 months ago by fbissey

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.

Last edited 15 months ago by fbissey (previous) (diff)

comment:65 in reply to: ↑ 56 Changed 15 months ago by mkoeppe

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 in reply to: ↑ 64 ; follow-up: Changed 15 months ago by mkoeppe

Replying to fbissey:

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 in reply to: ↑ 66 Changed 15 months ago by jhpalmieri

Replying to mkoeppe:

Replying to fbissey:

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 Changed 15 months ago by fbissey

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

comment:69 Changed 15 months ago by mkoeppe

  • Reviewers set to Matthias Koeppe, ...

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

comment:70 follow-up: Changed 15 months ago by jhpalmieri

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 in reply to: ↑ 70 Changed 15 months ago by mkoeppe

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 Changed 15 months ago by jhpalmieri

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 Changed 15 months ago by mkoeppe

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?

comment:74 Changed 15 months ago by git

  • Commit changed from 684138996fc44134dc18d2e8761eecc8caafb39e to 778fcd5817ce3f5455f3b7e34a3b362280cd6a7d

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

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

comment:75 follow-up: Changed 15 months ago by jhpalmieri

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 Changed 15 months ago by jhpalmieri

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 in reply to: ↑ 75 ; follow-up: Changed 15 months ago by mkoeppe

Replying to jhpalmieri:

I used SAGE_ROOT/src

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

comment:78 Changed 15 months ago by git

  • Commit changed from 778fcd5817ce3f5455f3b7e34a3b362280cd6a7d to 3cfb60cc0207109d2a64c41933f3cc7c0c5a6d34

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.

comment:79 in reply to: ↑ 77 ; follow-up: Changed 15 months ago by jhpalmieri

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 Changed 15 months ago by jhpalmieri

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 in reply to: ↑ 79 Changed 15 months ago by mkoeppe

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.

comment:82 Changed 15 months ago by git

  • Commit changed from 3cfb60cc0207109d2a64c41933f3cc7c0c5a6d34 to 9c28ee4551def34545d1cc603e7db2d11bb6802a

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 Changed 15 months ago by jhpalmieri

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

comment:84 Changed 15 months ago by mkoeppe

  • Status changed from needs_review to needs_work

comment:85 Changed 15 months ago by mkoeppe

  • Branch changed from u/jhpalmieri/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution to u/mkoeppe/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution

comment:86 Changed 15 months ago by mkoeppe

  • Commit changed from 9c28ee4551def34545d1cc603e7db2d11bb6802a to 875940dea7853575e42f567759bcaa5392d6c5b3

Rebased on top of the new version of #29289


Last 10 new commits:

225dbb4src/bin/sage: Move option -fix-pkg-checksums to sage-site
df1cd8atrac 29111: remove --inotebook, references to sagenb
55c5680src/bin/sage: Move parts of the help message to sage-site
05ba86aMerge 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
72c7e23src/doc/en/reference/repl/options.rst: Replace copypasta by include of generated file options.txt
e9a883dtrac 29111: revising "sage --advanced" message.
5dca421trac 29111: more reorganization of "sage --advanced" message
8f0ee05trac 29111: delete sage-fix-pkg-checksums
21fc231trac 29111: re "sage --advanced" message:
875940dtrac 29111: more tinkering. Fix doctests in tests/cmdline.py.

comment:87 Changed 15 months ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:88 follow-up: Changed 15 months ago by mkoeppe

By the way, sage-site -- is this a good name for this script?

comment:89 in reply to: ↑ 88 Changed 15 months ago by jhpalmieri

Replying to mkoeppe:

By the way, sage-site -- is this a good name for this script?

I can't think of anything better.

comment:90 Changed 15 months ago by jhpalmieri

  • Reviewers changed from Matthias Koeppe, ... to Matthias Koeppe, John Palmieri, ...

comment:91 follow-up: Changed 15 months ago by mkoeppe

@fbissey, could you take another look so we can finalize this ticket?

comment:92 in reply to: ↑ 91 Changed 15 months ago by fbissey

Replying to mkoeppe:

@fbissey, could you take another look so we can finalize this ticket?

I'll have a look after my lunch.

comment:93 follow-up: Changed 15 months ago by fbissey

OK, so I am having this look over, mainly at sage and what is now called sage-site. And I have to say splitting the stuff is not easy and subject to controversy so what I say is not going to be the final word on the matter.

This branch version of sage still execute numerous options that are not listed in the sage menu but that would be displayed by sage-site.

  • --upgrade
  • --buildsh
  • -i
  • -f
  • the function build_sage (I would have thought it belonged to sage-site)
  • same with maybe_sage_location
  • all -b options

I recognise that some of these may have to live there for technical reason. Splitting --sh and --buildsh for example would lead to code duplication - unless someone also does some refactoring. But as it is now, they could be accidentally called from a sage-on-distro shipping this file. If they have to stay there, we should get a check on SAGE_ROOT before executing them and display some error message if it is not defined. Like "not available in this distribution of sagemath".

Conversely I thought calling on sage's version of various packages (gap, singular, maxima, etc...) was to stay in the main sage script rather than move to sage-site. They usually execute normally with system installed packages and may be required for running some user scripts.

Last edited 15 months ago by fbissey (previous) (diff)

comment:94 in reply to: ↑ 93 ; follow-up: Changed 15 months ago by mkoeppe

Replying to fbissey:

This branch version of sage still execute numerous options that are not listed in the sage menu but that would be displayed by sage-site. [...] I recognise that some of these may have to live there for technical reason.

Right, this ticket is certainly not meant as the final word on splitting the script. But by splitting the help strings on this ticket, we define the interface, and this is to me the most important part. The patch is already huge, and it is best to do further refactoring and refinements in narrower follow-up tickets.

as it is now, they could be accidentally called from a sage-on-distro shipping this file. If they have to stay there, we should get a check on SAGE_ROOT before executing them and display some error message if it is not defined. Like "not available in this distribution of sagemath".

This sounds like a good idea, but I would also prefer to do this on a follow-up ticket.

Conversely I thought calling on sage's version of various packages (gap, singular, maxima, etc...) was to stay in the main sage script rather than move to sage-site. They usually execute normally with system installed packages and may be required for running some user scripts.

It would be fine with me to move them back to sage.

comment:95 in reply to: ↑ 94 ; follow-up: Changed 15 months ago by fbissey

Replying to mkoeppe:

Replying to fbissey:

This branch version of sage still execute numerous options that are not listed in the sage menu but that would be displayed by sage-site. [...] I recognise that some of these may have to live there for technical reason.

Right, this ticket is certainly not meant as the final word on splitting the script. But by splitting the help strings on this ticket, we define the interface, and this is to me the most important part. The patch is already huge, and it is best to do further refactoring and refinements in narrower follow-up tickets.

I am OK with that. Let's get the interface where we want it then.

as it is now, they could be accidentally called from a sage-on-distro shipping this file. If they have to stay there, we should get a check on SAGE_ROOT before executing them and display some error message if it is not defined. Like "not available in this distribution of sagemath".

This sounds like a good idea, but I would also prefer to do this on a follow-up ticket.

This is acceptable. 9.2 is not getting anywhere very fast :) To put things in perspective I was thinking of having a function rather than writing a very similar piece of code all over the place.

Conversely I thought calling on sage's version of various packages (gap, singular, maxima, etc...) was to stay in the main sage script rather than move to sage-site. They usually execute normally with system installed packages and may be required for running some user scripts.

It would be fine with me to move them back to sage.

Right, since we want to set the interface right we have to decide if this is something that is important for the distribution to support. On one side, it just works like it is, on the other side it is redundant with calling the application directly. The only interesting point is whether there are scripts out there that needs this for compatibility.

These direct calls to various applications could also be replaced by sage --sh $app, sage --sh was actually developed some time after those direct calls were put in place (IIRC). So a migration path could be suggested and they could be deprecated. That would be yet another follow up ticket.

comment:96 in reply to: ↑ 95 Changed 15 months ago by mkoeppe

Replying to fbissey:

Conversely I thought calling on sage's version of various packages (gap, singular, maxima, etc...) was to stay in the main sage script rather than move to sage-site. They usually execute normally with system installed packages and may be required for running some user scripts.

It would be fine with me to move them back to sage.

Right, since we want to set the interface right we have to decide if this is something that is important for the distribution to support. On one side, it just works like it is, on the other side it is redundant with calling the application directly. The only interesting point is whether there are scripts out there that needs this for compatibility.

These direct calls to various applications could also be replaced by sage --sh $app, sage --sh was actually developed some time after those direct calls were put in place (IIRC).

Thanks for the context.

So a migration path could be suggested and they could be deprecated. That would be yet another follow up ticket.

I agree. We should go in this direction by deprecating these shortcuts sage -$app first. That would be a separate ticket.

So for this ticket, I will move them back into the sage script unless @jhpalmieri objects.

comment:97 Changed 15 months ago by jhpalmieri

Go ahead, I don't have strong feelings about it.

I also like the idea of testing SAGE_ROOT before executing sage -b etc. But doing it on a followup ticket is fine.

comment:98 Changed 15 months ago by git

  • Commit changed from 875940dea7853575e42f567759bcaa5392d6c5b3 to 4a3d36eb78976d31d9d27623712b2af7b6b74006

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

4a3d36eMove 'sage -app' back to src/bin/sage

comment:99 Changed 15 months ago by mkoeppe

done, please check if you have a chance

comment:100 Changed 15 months ago by jhpalmieri

What is sage --axiom supposed to do? What package installs an axiom executable?

comment:101 Changed 15 months ago by fbissey

Did we drop the packaging of axiom? http://axiom.axiom-developer.org/

comment:102 Changed 15 months ago by jhpalmieri

Not as a new-style package, and I couldn't find the website with old-style packages. Does it still exist?

comment:104 Changed 15 months ago by jhpalmieri

Oh, so it's from 2007?!

comment:105 Changed 15 months ago by jhpalmieri

See #29930 for a discussion of sage --gap and friends.

Last edited 15 months ago by jhpalmieri (previous) (diff)

comment:106 Changed 15 months ago by jhpalmieri

The Sage axiom package requires clisp. I changed the spkg-install file to use ecl instead, but it failed. I am going to out on a limb and suggest that no one is using this spkg with a recent version of Sage.

comment:107 Changed 15 months ago by fbissey

You have my full support to remove this piece of cruft.

comment:108 Changed 15 months ago by mkoeppe

Does perhaps fricas install itself under the name axiom?

comment:109 Changed 15 months ago by jhpalmieri

Good idea. From http://fricas.sourceforge.net/history.html, it looks like FriCAS is a fork of Axiom, but I just installed it, and it doesn't install axiom (at least not in SAGE_ROOT/local/bin, nor is axiom listed in its package manifest). It installs fricas and efricas, so we could eventually add those, but it's not like anyone's been clamoring for them.

comment:110 Changed 15 months ago by mkoeppe

Then let's just drop sage --axiom. Good catch!

comment:111 Changed 15 months ago by dimpase

is axiom package an old-style one?

It's probably very outdated. also, my impression is that fricas fork is the most active of all the forks of the original axiom forks. The others are http://www.axiom-developer.org/ (axiom) and http://www.open-axiom.org/links.html (OpenAxiom)

comment:112 Changed 15 months ago by mkoeppe

That's right, there is no new-style axiom package.

comment:113 Changed 15 months ago by git

  • Commit changed from 4a3d36eb78976d31d9d27623712b2af7b6b74006 to 3a0193c9f0a12407ac3708fea6d70c9b72d4f864

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

3a0193csrc/bin/sage: Remove handling of 'sage -axiom'

comment:114 Changed 15 months ago by mkoeppe

Done. Needs review

comment:115 Changed 15 months ago by jhpalmieri

I'm happy with everything. Who else would like to weigh in?

comment:116 Changed 15 months ago by mkoeppe

  • Reviewers changed from Matthias Koeppe, John Palmieri, ... to Matthias Koeppe, John Palmieri, François Bissey

comment:117 Changed 15 months ago by fbissey

  • Status changed from needs_review to positive_review

Sorry, I was busy and forgot about it. We have reached an agreement with what goes in and what's next. I think we have bashed it enough and it is ready to go.

comment:118 Changed 15 months ago by mkoeppe

Thanks!

comment:119 Changed 15 months ago by jhpalmieri

<thumbs up emoji>

comment:120 Changed 15 months ago by mkoeppe

Follow-ups:

  • #21559: Install src/bin scripts by sagelib's setup.py, not make
  • #29930: Change handling of sage --APP

comment:121 Changed 15 months ago by vbraun

  • Status changed from positive_review to needs_work

Merge confilct

comment:122 Changed 15 months ago by git

  • Commit changed from 3a0193c9f0a12407ac3708fea6d70c9b72d4f864 to c8bab39ca0c44333484007cc0f4778016c01ab0c

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

c8bab39Merge tag '9.2.beta4' into t/29111/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution

comment:123 Changed 15 months ago by git

  • Commit changed from c8bab39ca0c44333484007cc0f4778016c01ab0c to 39536716aef05e8953bdc50038c49e5959ffdb6a

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

3953671Merge tag '9.2.beta4' into t/29111/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution

comment:124 Changed 15 months ago by mkoeppe

  • Status changed from needs_work to positive_review

comment:125 Changed 15 months ago by vbraun

  • Branch changed from u/mkoeppe/specify_a_subset_of_sage_command_line_options_that_are_supported_by_sagelib___rather_than_sage_the_distribution to 39536716aef05e8953bdc50038c49e5959ffdb6a
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.