#29111 closed enhancement (fixed)
src/bin/sage: Delegate handling of options of sagethedistribution, specifying a subset that is supported by sagelib proper
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage9.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:  
Dependencies:  #29878, #29289, #29884  Stopgaps: 
Description (last modified by )
The sage
script has a number of options that only make sense for sagethedistribution, 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 packagesage upgrade
 only makes sense with sagethedistribution 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
 obviouslysage python c COMMAND
sage t
 because it is used by user packages for testing sage sourcessage sh
..... ?
In this ticket, we actually split src/bin/sage
into sagelib functionality and sagethedistribution functionality. src/bin/sage
delegates unknown options to build/bin/sagesite
.
Ideally, distributions should be able to use an unmodified src/bin/sage
.
Context:
 https://groups.google.com/d/msg/sagepackaging/BmkxIBdwbvE/fRMl2sjdBQAJ
 #29060: Metaticket: Add Dockerfiles and CI scripts for integration testing of source and binary distributions and of downstream packages
See also:
 #29884:
src/doc/bootstrap
: Generatesrc/doc/en/reference/repl/options.rst
Change History (126)
comment:1 Changed 3 years ago by
comment:2 Changed 3 years ago by
 Component changed from PLEASE CHANGE to scripts
comment:3 Changed 2 years ago by
 Milestone changed from sage9.1 to sage9.2
pushing these forward to 9.2
comment:4 Changed 2 years ago by
 Cc mjo added
comment:5 Changed 2 years ago by
 Description modified (diff)
comment:6 followup: ↓ 11 Changed 2 years ago by
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 2 years ago by
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 2 years ago by
comment:9 Changed 2 years ago by
Hum, this ticket reminds me that I need to clean the version in sageongentoo
fbissey@moonloop ~ $ sage advanced SageMath version 9.2.beta1, Release Date: 20200613 Running Sage: file.[sagepyspyx]  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] <allfilesdir>  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 warnlong [timeout]  warning if doctest is slow onlyerrors  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 sagecachegrind.PID can be found in callgrind  run Sage using Valgrind's callgrind tool. The log files are named sagecallgrind.PID can be found in massif  run Sage using Valgrind's massif tool. The log files are named sagemassif.PID can be found in memcheck  run Sage using Valgrind's memcheck tool. The log files are named sagememcheck.PID can be found in omega  run Sage using Valgrind's omega tool. The log files are named sageomega.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 sageenv
altogether, the main point of keeping sage sh
around was to check or use the environment set by sageenv
.
comment:10 followup: ↓ 13 Changed 2 years ago by
Dropping sage sh
is a bad idea. User packages need this
comment:11 in reply to: ↑ 6 Changed 2 years ago by
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 betweensage python
andsage sqlite3
(and what aboutsage 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 2 years ago by
 Dependencies set to #29878
comment:13 in reply to: ↑ 10 ; followup: ↓ 14 Changed 2 years ago by
Replying to mkoeppe:
Dropping
sage sh
is a bad idea. User packages need this
Certainly, vanilla sage does need it. In sageongentoo at best you would have a normal shell since I dropped sageenv altogether.
comment:14 in reply to: ↑ 13 Changed 2 years ago by
Replying to fbissey:
Replying to mkoeppe:
Dropping
sage sh
is a bad idea. User packages need thisCertainly, vanilla sage does need it. In sageongentoo at best you would have a normal shell since I dropped sageenv altogether.
Yes, vanilla shell is fine. The point is that user packages need the _interface_.
comment:15 Changed 2 years ago by
 Dependencies changed from #29878 to #29878, #29289
comment:16 Changed 2 years ago by
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 reinstate it as a vanilla shell or noop for compatibility.
comment:17 Changed 2 years ago by
Great!
comment:18 Changed 2 years ago by
 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 2 years ago by
 Commit set to 152e58155fe193fd9215bee3bf20755c5e0c51a2
Here's a beginning of what I have in mind.
New commits:
b200d70  trac 29878: remove "sage twistd" commandline argument

e0b6112  build/bin/sagespkg, src/bin/sage: Remove support for installing oldstyle SPKGs

20b2a93  Merge 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

152e581  src/bin/sage: Delegate options that pertain to SAGE_ROOT or sagethedistribution to the new script build/bin/sagesite

comment:20 followup: ↓ 22 Changed 2 years ago by
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 2 years ago by
 Description modified (diff)
 Summary changed from Specify a subset of sage command line options that are supported by sagelib  rather than sagethedistribution to src/bin/sage: Delegate handling of options of sagethedistribution, specifying a subset that is supported by sagelib proper
comment:22 in reply to: ↑ 20 Changed 2 years ago by
Replying to jhpalmieri:
Okay, I'm fine with keeping
sage python
. What aboutsage singular
et al.?
I think those should be moved to sagesite
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 2 years ago by
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 .opt2.pyc files are generated. Someone is eventually going to realize that they shouldn't be installed after this gets more popular.
comment:24 Changed 2 years ago by
We could just conditionalize these ones based on whether SAGE_SRC is set or not by sageenv...
comment:25 Changed 2 years ago by
 Commit changed from 152e58155fe193fd9215bee3bf20755c5e0c51a2 to e734d55ff8bcff42a7c9b0f750dc066d2afa0252
Branch pushed to git repo; I updated commit sha1. New commits:
e734d55  src/bin/sage: Disable options grep, grepdoc on distributions without SAGE_SRC

comment:26 Changed 2 years ago by
 Commit changed from e734d55ff8bcff42a7c9b0f750dc066d2afa0252 to 096ca9293b7ddef99c4b722f8604af0a2a420799
Branch pushed to git repo; I updated commit sha1. New commits:
096ca92  src/bin/sage: Move option fixpkgchecksums to sagesite

comment:27 Changed 2 years ago by
 Status changed from new to needs_review
All, please review, or feel free to push more changes to this ticket.
comment:28 Changed 2 years ago by
 Commit changed from 096ca9293b7ddef99c4b722f8604af0a2a420799 to eadae5aad05ed13b4fc78db9ddf7ee8c67331274
Branch pushed to git repo; I updated commit sha1. New commits:
eadae5a  src/bin/sage: Move parts of the help message to sagesite

comment:29 Changed 2 years ago by
 Description modified (diff)
comment:30 Changed 2 years ago by
 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 followup: ↓ 33 Changed 2 years ago by
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 2 years ago by
 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 2 years ago by
 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
, andsagenb
is no longer a valid option fornotebook
.
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 exporttoJupyter 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:
0856913  trac 29111: remove inotebook, references to sagenb

comment:34 Changed 2 years ago by
comment:35 Changed 2 years ago by
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 2 years ago by
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/Python3Switch
The branch here prints something similar, just a bit less grammatical ;)
comment:37 followup: ↓ 38 Changed 2 years ago by
comment:38 in reply to: ↑ 37 Changed 2 years ago by
We should probably continue the notebook discussion at #29885.
Sure, it's not a big deal in any case.
comment:39 followup: ↓ 40 Changed 2 years ago by
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 ; followup: ↓ 45 Changed 2 years ago by
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 2 years ago by
#29884 is ready now
comment:42 Changed 2 years ago by
 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 2 years ago by
 Commit changed from 08569132d2b8dfd54a4446df7da200d303ad9ce1 to 1ecc84042fbdc77eabb23dc33954658ca3e5b276
 Dependencies changed from #29878, #29289 to #29878, #29289, #29884
New commits:
fb9dcdc  src/bin/sage: Move parts of the help message to sagesite

885092e  src/doc/bootstrap: Generate src/doc/en/reference/repl/options.txt

1ecc840  Merge 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 2 years ago by
 Commit changed from 1ecc84042fbdc77eabb23dc33954658ca3e5b276 to 6ba580b6523280f1510a200ef519f61b512bbaf2
Branch pushed to git repo; I updated commit sha1. New commits:
6ba580b  src/doc/en/reference/repl/options.rst: Replace copypasta by include of generated file options.txt

comment:45 in reply to: ↑ 40 ; followup: ↓ 46 Changed 2 years ago by
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 2 years ago by
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 bysage advanced
. Would it be too verbose if we used (more or less) the currentoptions.rst
for the output tosage 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 sagesite
function usage_advanced
. They are printed below the ones from sage advanced
.
comment:47 Changed 2 years ago by
comment:48 Changed 2 years ago by
 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 followup: ↓ 52 Changed 2 years ago by
 Commit changed from 6ba580b6523280f1510a200ef519f61b512bbaf2 to 4b303a215c8b6df3cecb57621f19afc7e65ab567
Here are some proposed changes to sage advanced
. I have a strong preference for two hyphens in sage longoption
over sage longoption
. I don't object to Sage parsing the two the same way, but I think we should advertise the twohyphen 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:
4b303a2  trac 29111: revising "sage advanced" message.

comment:50 Changed 2 years ago by
Other comments: fixpkgchecksums
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 2 years ago by
 Commit changed from 4b303a215c8b6df3cecb57621f19afc7e65ab567 to 611b8d626b0f370830689e6c038287fb2c0a5ea6
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
611b8d6  trac 29111: revising "sage advanced" message.

comment:52 in reply to: ↑ 49 ; followup: ↓ 54 Changed 2 years ago by
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 2 years ago by
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 ; followup: ↓ 56 Changed 2 years ago by
Is there a reason for this difference in the sage
script?
usage() { ... exec "$SAGE_ROOT/build/bin/sagesite" "h" ...
vs
usage_advanced() { ... "$SAGE_ROOT/build/bin/sagesite" "advanced" ...
comment:55 Changed 2 years ago by
 Commit changed from 611b8d626b0f370830689e6c038287fb2c0a5ea6 to 073577b867f440b139ceb26fa6fddf5df5c3b93a
Branch pushed to git repo; I updated commit sha1. New commits:
073577b  trac 29111: more reorganization of "sage advanced" message

comment:56 in reply to: ↑ 54 ; followup: ↓ 65 Changed 2 years ago by
Replying to jhpalmieri:
Is there a reason for this difference in the
sage
script?usage() { ... exec "$SAGE_ROOT/build/bin/sagesite" "h" ...vs
usage_advanced() { ... "$SAGE_ROOT/build/bin/sagesite" "advanced" ...
FYI: I changed both to use exec
. Feel free to change back if this is wrong.
comment:57 Changed 2 years ago by
 Commit changed from 073577b867f440b139ceb26fa6fddf5df5c3b93a to c8daf8054418491abbc2e147aad024a8eb98c00c
Branch pushed to git repo; I updated commit sha1. New commits:
c8daf80  trac 29111: delete sagefixpkgchecksums

comment:58 followup: ↓ 63 Changed 2 years ago by
sagefixpkgchecksums
was deprecated four years ago (#19984), so I deleted it.
comment:59 Changed 2 years ago by
 Commit changed from c8daf8054418491abbc2e147aad024a8eb98c00c to 5c3ced6afb74de327f067145a49a5506a9a54ea1
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
5c3ced6  trac 29111: delete sagefixpkgchecksums

comment:60 followup: ↓ 62 Changed 2 years ago by
One last thing I should have commented on earlier
+ echo " bn [...], buildandnotebook [...]" + 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 sagesite
and nowhere else.
comment:61 Changed 2 years ago by
 Commit changed from 5c3ced6afb74de327f067145a49a5506a9a54ea1 to 684138996fc44134dc18d2e8761eecc8caafb39e
Branch pushed to git repo; I updated commit sha1. New commits:
6841389  trac 29111: re "sage advanced" message:

comment:62 in reply to: ↑ 60 ; followup: ↓ 64 Changed 2 years ago by
Replying to fbissey:
One last thing I should have commented on earlier
+ echo " bn [...], buildandnotebook [...]" + echo "  build the Sage library (as by running \"sage b\")" + echo " and then start the notebook"is currently in
src/bin/sage
but really anyb
options belong tosagesite
and nowhere else.
Okay, moved, but on the other hand, I think that the coverage
options belong with doctesting, so I moved those from sagesite
to sage
.
comment:63 in reply to: ↑ 58 Changed 2 years ago by
Replying to jhpalmieri:
sagefixpkgchecksums
was deprecated four years ago (#19984), so I deleted it.
Excellent!
comment:64 in reply to: ↑ 62 ; followup: ↓ 66 Changed 2 years ago by
Replying to jhpalmieri:
Okay, moved, but on the other hand, I think that the
coverage
options belong with doctesting, so I moved those fromsagesite
tosage
.
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 in reply to: ↑ 56 Changed 2 years ago by
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 sagesite
. That's no longer needed.
comment:66 in reply to: ↑ 64 ; followup: ↓ 67 Changed 2 years ago by
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 fromsagesite
tosage
.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 2 years ago by
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 fromsagesite
tosage
.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
, alsosage 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 2 years ago by
It is fine. I will ship those two then. It makes sense.
comment:69 Changed 2 years ago by
 Reviewers set to Matthias Koeppe, ...
Looks great to me. Let's get this in.
comment:70 followup: ↓ 71 Changed 2 years ago by
I have a few more questions:
 I would like to delete the
rsyncdist
option. Can we also deletesagersyncdist
? 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. tosagesite
, which is where they are documented?
 A comment: it would be nice to move
sage sdist
handling tosagesite
, but then we have to duplicatemaybe_sage_location
, so it's probably not worth it. Although then we could movesage b
handling, too. This can be done later, if someone feels motivated.
comment:71 in reply to: ↑ 70 Changed 2 years ago by
Replying to jhpalmieri:
 I would like to delete the
rsyncdist
option. Can we also deletesagersyncdist
? 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 oldstyle 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. tosagesite
, which is where they are documented?
Yes, good idea, please go ahead.
 A comment: it would be nice to move
sage sdist
handling tosagesite
, but then we have to duplicatemaybe_sage_location
, so it's probably not worth it. Although then we could movesage b
handling, too. This can be done later, if someone feels motivated.
I would suggest to do these in a followup ticket. Likewise, for the option handling for sage i
that would really belong to sagesite
; this is complicated by the fact that sageenv
must not be sourced before sagespkg
is executed.
comment:72 Changed 2 years ago by
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, sageenv
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 sagesite
.
comment:73 Changed 2 years ago by
The intention of using SAGE_SRC
from sageenv
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 sageenv
?
comment:74 Changed 2 years ago by
 Commit changed from 684138996fc44134dc18d2e8761eecc8caafb39e to 778fcd5817ce3f5455f3b7e34a3b362280cd6a7d
Branch pushed to git repo; I updated commit sha1. New commits:
778fcd5  trac 29111: more tinkering. Fix doctests in tests/cmdline.py.

comment:75 followup: ↓ 77 Changed 2 years ago by
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 2 years ago by
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 ; followup: ↓ 79 Changed 2 years ago by
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 2 years ago by
 Commit changed from 778fcd5817ce3f5455f3b7e34a3b362280cd6a7d to 3cfb60cc0207109d2a64c41933f3cc7c0c5a6d34
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
3cfb60c  trac 29111: more tinkering. Fix doctests in tests/cmdline.py.

comment:79 in reply to: ↑ 77 ; followup: ↓ 81 Changed 2 years ago by
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 sageenv. 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 2 years ago by
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 2 years ago by
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 2 years ago by
 Commit changed from 3cfb60cc0207109d2a64c41933f3cc7c0c5a6d34 to 9c28ee4551def34545d1cc603e7db2d11bb6802a
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
9c28ee4  trac 29111: more tinkering. Fix doctests in tests/cmdline.py.

comment:83 Changed 2 years ago by
I've tried to undo my changes to sage p
.
comment:84 Changed 2 years ago by
 Status changed from needs_review to needs_work
comment:85 Changed 2 years ago by
 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 2 years ago by
 Commit changed from 9c28ee4551def34545d1cc603e7db2d11bb6802a to 875940dea7853575e42f567759bcaa5392d6c5b3
Rebased on top of the new version of #29289
Last 10 new commits:
225dbb4  src/bin/sage: Move option fixpkgchecksums to sagesite

df1cd8a  trac 29111: remove inotebook, references to sagenb

55c5680  src/bin/sage: Move parts of the help message to sagesite

05ba86a  Merge 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

72c7e23  src/doc/en/reference/repl/options.rst: Replace copypasta by include of generated file options.txt

e9a883d  trac 29111: revising "sage advanced" message.

5dca421  trac 29111: more reorganization of "sage advanced" message

8f0ee05  trac 29111: delete sagefixpkgchecksums

21fc231  trac 29111: re "sage advanced" message:

875940d  trac 29111: more tinkering. Fix doctests in tests/cmdline.py.

comment:87 Changed 2 years ago by
 Status changed from needs_work to needs_review
comment:88 followup: ↓ 89 Changed 2 years ago by
By the way, sagesite
 is this a good name for this script?
comment:89 in reply to: ↑ 88 Changed 2 years ago by
Replying to mkoeppe:
By the way,
sagesite
 is this a good name for this script?
I can't think of anything better.
comment:90 Changed 2 years ago by
 Reviewers changed from Matthias Koeppe, ... to Matthias Koeppe, John Palmieri, ...
comment:91 followup: ↓ 92 Changed 2 years ago by
@fbissey, could you take another look so we can finalize this ticket?
comment:92 in reply to: ↑ 91 Changed 2 years ago by
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 followup: ↓ 94 Changed 2 years ago by
OK, so I am having this look over, mainly at sage
and what is now called sagesite
. 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 sagesite
.
upgrade
buildsh
i
f
 the function
build_sage
(I would have thought it belonged tosagesite
)  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 sageondistro 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 sagesite
. They usually execute normally with system installed packages and may be required for running some user scripts.
comment:94 in reply to: ↑ 93 ; followup: ↓ 95 Changed 2 years ago by
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 bysagesite
. [...] 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 followup tickets.
as it is now, they could be accidentally called from a sageondistro 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 followup ticket.
Conversely I thought calling on sage's version of various packages (
gap
,singular
,maxima
, etc...) was to stay in the mainsage
script rather than move tosagesite
. 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 ; followup: ↓ 96 Changed 2 years ago by
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 bysagesite
. [...] 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 followup 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 sageondistro 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 followup 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 mainsage
script rather than move tosagesite
. 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 2 years ago by
Replying to fbissey:
Conversely I thought calling on sage's version of various packages (
gap
,singular
,maxima
, etc...) was to stay in the mainsage
script rather than move tosagesite
. 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 2 years ago by
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 2 years ago by
 Commit changed from 875940dea7853575e42f567759bcaa5392d6c5b3 to 4a3d36eb78976d31d9d27623712b2af7b6b74006
Branch pushed to git repo; I updated commit sha1. New commits:
4a3d36e  Move 'sage app' back to src/bin/sage

comment:99 Changed 2 years ago by
done, please check if you have a chance
comment:100 Changed 2 years ago by
What is sage axiom
supposed to do? What package installs an axiom
executable?
comment:101 Changed 2 years ago by
Did we drop the packaging of axiom
? http://axiom.axiomdeveloper.org/
comment:102 Changed 2 years ago by
Not as a newstyle package, and I couldn't find the website with oldstyle packages. Does it still exist?
comment:103 Changed 2 years ago by
There is here http://old.files.sagemath.org/spkg/archive/ which I could go from here http://files.sagemath.org/spkg/
comment:104 Changed 2 years ago by
Oh, so it's from 2007?!
comment:105 Changed 2 years ago by
See #29930 for a discussion of sage gap
and friends.
comment:106 Changed 2 years ago by
The Sage axiom
package requires clisp
. I changed the spkginstall
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 2 years ago by
You have my full support to remove this piece of cruft.
comment:108 Changed 2 years ago by
Does perhaps fricas
install itself under the name axiom
?
comment:109 Changed 2 years ago by
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 2 years ago by
Then let's just drop sage axiom
. Good catch!
comment:111 Changed 2 years ago by
is axiom package an oldstyle 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.axiomdeveloper.org/ (axiom) and http://www.openaxiom.org/links.html (OpenAxiom)
comment:112 Changed 2 years ago by
That's right, there is no newstyle axiom
package.
comment:113 Changed 2 years ago by
 Commit changed from 4a3d36eb78976d31d9d27623712b2af7b6b74006 to 3a0193c9f0a12407ac3708fea6d70c9b72d4f864
Branch pushed to git repo; I updated commit sha1. New commits:
3a0193c  src/bin/sage: Remove handling of 'sage axiom'

comment:114 Changed 2 years ago by
Done. Needs review
comment:115 Changed 2 years ago by
I'm happy with everything. Who else would like to weigh in?
comment:116 Changed 2 years ago by
 Reviewers changed from Matthias Koeppe, John Palmieri, ... to Matthias Koeppe, John Palmieri, François Bissey
comment:117 Changed 2 years ago by
 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 2 years ago by
Thanks!
comment:119 Changed 2 years ago by
<thumbs up emoji>
comment:120 Changed 2 years ago by
comment:122 Changed 2 years ago by
 Commit changed from 3a0193c9f0a12407ac3708fea6d70c9b72d4f864 to c8bab39ca0c44333484007cc0f4778016c01ab0c
Branch pushed to git repo; I updated commit sha1. New commits:
c8bab39  Merge 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 2 years ago by
 Commit changed from c8bab39ca0c44333484007cc0f4778016c01ab0c to 39536716aef05e8953bdc50038c49e5959ffdb6a
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
3953671  Merge 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 2 years ago by
 Status changed from needs_work to positive_review
comment:125 Changed 2 years ago by
 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
comment:126 Changed 4 months ago by
 Commit 39536716aef05e8953bdc50038c49e5959ffdb6a deleted
Followup in #33753
As I said on #29092, we should also move corresponding sagethedistribution tests (such as those for
sage sqlite
) mentioned in #29002) fromsrc
(which is for sagelib) to somewhere inbuild
(which is for sagethedistribution). Downstream packagers would ignore the sagethedistribution tests.