src/doc/bootstrap: Simplify by using new options of "sage -package list"

Question

src/doc/bootstrap: Simplify by using new options of "sage -package list"

Closed this issue 5 years ago · 32 comments

mkoeppe commented 5 years ago

Depends on #30865
Depends on #28745

CC: @seblabbe @slel

Component: documentation

Keywords: sd111

Author: Matthias Koeppe

Branch/Commit: 084fbf6

Reviewer: Sébastien Labbé

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

Answer 1 · 2020-11-22T20:38:56.000Z

Changed dependencies from #30865 to #30865, #28745

Answer 2 · 2020-11-24T01:31:06.000Z

Branch: u/mkoeppe/src_doc_bootstrap__simplify_by_using_new_options_of__sage__package_list_

Answer 3 · 2020-11-24T01:31:34.000Z

Commit: 5ad29d5

Answer 4 · 2020-11-24T01:31:34.000Z

Author: Matthias Koeppe

Answer 5 · 2020-11-24T01:31:34.000Z

Last 10 new commits:

`be5bdf2`	`build/pkgs/pyparsing: Update to 2.4.7`
`88b8427`	`build/pkgs/pytz: Update to 2020.4`
`df7972c`	`build/pkgs/rpy2: Update to 3.3.6`
`7cd96b2`	`build/pkgs/scipy/patches/extern_decls.patch: Remove`
`84d778c`	`Fixing upstream url for networkx`
`13af6c6`	`fix tarball name`
`45b3408`	`Merge commit '13af6c6b8c1533ba9d1b45b127e1a7b7d30000c6' of git://trac.sagemath.org/sage into t/28745/environment_yaml`
`736e006`	`build/pkgs/pathpy/distros/conda.txt: Remove`
`cdfbe3a`	`Merge branch 't/28745/environment_yaml' into t/30947/src_doc_bootstrap__simplify_by_using_new_options_of__sage__package_list_`
`5ad29d5`	`src/doc/bootstrap: Simplify by using new options of "sage -package list"`

Answer 6 · 2020-11-24T08:58:52.000Z

comment:4

I assume the only commit to be reviewed here is this one: https://github.com/sagemath/sagetrac-mirror/commits/5ad29d5e5686a403f51d6b09d5ba0ea02face4e9

I see that it simplifies the code by removing two if ... fi clauses twice.

Do you accept to add a commit which unindent twice the content of the two for loops?

Answer 7 · 2020-11-24T10:59:22.000Z

comment:5

Running make ptestlong with the current branch finished with no error (except the unrelated sage -t --long --warn-long 55.7 --random-seed=0 src/sage/interfaces/singular.py # Killed due to segmentation fault).

You can change the status to positive review on my behalf once the unindentation is done.

Answer 8 · 2020-11-24T10:59:22.000Z

Reviewer: Sébastien Labbé

Answer 9 · 2020-11-24T17:30:28.000Z

comment:6

Replying to @seblabbe:

Do you accept to add a commit which unindent twice the content of the two for loops?

I'd rather not on this ticket because there are several tickets out there that touch the same code.

Answer 10 · 2020-11-24T19:27:39.000Z

Description changed:

--- 
+++ 
@@ -1 +1 @@
-
+ok, then unindentation can wait.

Answer 11 · 2020-11-24T19:28:08.000Z

comment:8

ok, then unindentation can wait.

Answer 12 · 2020-11-24T19:28:08.000Z

Description changed:

--- 
+++ 
@@ -1 +1 @@
-ok, then unindentation can wait.
+

Answer 13 · 2020-11-24T19:46:27.000Z

comment:9

Thank you!

Answer 14 · 2020-11-25T09:57:45.000Z

Description changed:

--- 
+++ 
@@ -1 +1 @@
-
+The branch has now a merge conflict on top of 9.3.beta2.

Answer 15 · 2020-11-25T09:58:45.000Z

Description changed:

--- 
+++ 
@@ -1 +1 @@
-The branch has now a merge conflict on top of 9.3.beta2.
+

Answer 16 · 2020-11-25T09:58:45.000Z

comment:11

(Oups, since the description of the ticket is empty, I end up writing my comment in that box...)

The branch has now a merge conflict on top of 9.3.beta2.

Answer 17 · 2020-11-25T22:16:14.000Z

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

`5e9366d`	`Merge tag '9.3.beta2' into t/30947/src_doc_bootstrap__simplify_by_using_new_options_of__sage__package_list_`

Answer 18 · 2020-11-25T22:16:14.000Z

Changed commit from 5ad29d5 to 5e9366d

Answer 19 · 2020-11-25T22:17:05.000Z

comment:13

Merged cleanly

Answer 20 · 2020-12-09T09:26:47.000Z

comment:14

The color of the branch name is red again. Is there a merge conflict with beta3 ?

Answer 21 · 2020-12-09T09:33:51.000Z

comment:15

I just tested on my machine and it merges cleanly on 9.3.beta3. So I don't know what is hapenning with the red color here.

Answer 22 · 2020-12-09T10:17:20.000Z

comment:16

While reviewing #29124, I see an issue, but it seems related to this ticket instead.

When I run tox -e docker-archlinux-minimal -- config.status, the command finishes with
docker-archlinux-minimal: commands succeeded congratulations :), but some red lines showing sage: command not found related to file src/doc/bootstrap are printed in the process:

...
+ ./bootstrap
rm -rf config configure build/make/Makefile-auto.in
rm -f src/doc/en/installation/*.txt
rm -rf src/doc/en/reference/spkg/*.rst
rm -f src/doc/en/reference/repl/*.txt
rm -f environment.yml
rm -f src/environment.yml
rm -f environment-optional.yml
rm -f src/environment-optional.yml
src/doc/bootstrap: line 34: sage: command not found
src/doc/bootstrap:92: installing src/doc/en/installation/arch.txt and src/doc/en/installation/arch-optional.txt
src/doc/bootstrap: line 34: sage: command not found
src/doc/bootstrap:92: installing src/doc/en/installation/debian.txt and src/doc/en/installation/debian-optional.txt
src/doc/bootstrap: line 34: sage: command not found
src/doc/bootstrap:92: installing src/doc/en/installation/fedora.txt and src/doc/en/installation/fedora-optional.txt
src/doc/bootstrap: line 34: sage: command not found
src/doc/bootstrap:92: installing src/doc/en/installation/cygwin.txt and src/doc/en/installation/cygwin-optional.txt
src/doc/bootstrap: line 34: sage: command not found
src/doc/bootstrap:92: installing src/doc/en/installation/homebrew.txt and src/doc/en/installation/homebrew-optional.txt
src/doc/bootstrap: line 34: sage: command not found
src/doc/bootstrap:66: installing environment.yml, environment-optional.yml, src/environment.yml and src/environment-optional.yml
src/doc/bootstrap:103: installing src/doc/en/reference/spkg/*.rst
src/doc/bootstrap: line 115: sage: command not found
src/doc/bootstrap:131: installing src/doc/en/reference/repl/options.txt
bootstrap:73: installing 'config/config.rpath'
configure.ac:305: installing 'config/compile'
configure.ac:89: installing 'config/config.guess'
configure.ac:89: installing 'config/config.sub'
configure.ac:43: installing 'config/install-sh'
configure.ac:43: installing 'config/missing'
...

Below is the same part of the log on top of 9.3.beta3 which do not have the sage command not found issue:

...
+ ./bootstrap
rm -rf config configure build/make/Makefile-auto.in
rm -f src/doc/en/installation/*.txt
rm -rf src/doc/en/reference/spkg/*.rst
rm -f src/doc/en/reference/repl/*.txt
rm -f environment.yml
rm -f src/environment.yml
rm -f environment-optional.yml
rm -f src/environment-optional.yml
src/doc/bootstrap:100: installing src/doc/en/installation/arch.txt and src/doc/en/installation/arch-optional.txt
src/doc/bootstrap:100: installing src/doc/en/installation/debian.txt and src/doc/en/installation/debian-optional.txt
src/doc/bootstrap:100: installing src/doc/en/installation/fedora.txt and src/doc/en/installation/fedora-optional.txt
src/doc/bootstrap:100: installing src/doc/en/installation/cygwin.txt and src/doc/en/installation/cygwin-optional.txt
src/doc/bootstrap:100: installing src/doc/en/installation/homebrew.txt and src/doc/en/installation/homebrew-optional.txt
src/doc/bootstrap:74: installing environment.yml, environment-optional.yml, src/environment.yml and src/environment-optional.yml
src/doc/bootstrap:111: installing src/doc/en/reference/spkg/*.rst
src/doc/bootstrap:143: installing src/doc/en/reference/repl/options.txt
bootstrap:73: installing 'config/config.rpath'
configure.ac:305: installing 'config/compile'
configure.ac:89: installing 'config/config.guess'
configure.ac:89: installing 'config/config.sub'
configure.ac:43: installing 'config/install-sh'
configure.ac:43: installing 'config/missing'

Answer 23 · 2020-12-10T04:56:13.000Z

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

`184ae6d`	`Merge tag '9.3.beta3' into t/30947/src_doc_bootstrap__simplify_by_using_new_options_of__sage__package_list_`

Answer 24 · 2020-12-10T04:56:13.000Z

Changed commit from 5e9366d to 184ae6d

Answer 25 · 2020-12-10T04:57:22.000Z

comment:19

Replying to @seblabbe:

I just tested on my machine and it merges cleanly on 9.3.beta3. So I don't know what is hapenning with the red color here.

The trac merger is configured to be a bit pickier than default git, it seems.
I've merged the beta and pushed it to the ticket.

Answer 26 · 2020-12-10T05:08:01.000Z

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

`084fbf6`	`src/doc/bootstrap: Use ./sage`

Answer 27 · 2020-12-10T05:08:01.000Z

Changed commit from 184ae6d to 084fbf6

Answer 28 · 2020-12-10T05:34:35.000Z

comment:21

Replying to @seblabbe:

When I run tox -e docker-archlinux-minimal -- config.status, the command finishes with
docker-archlinux-minimal: commands succeeded congratulations :), but some red lines showing sage: command not found related to file src/doc/bootstrap are printed in the process:
src/doc/bootstrap: line 34: sage: command not found

Thanks for catching this! Fixed.

Answer 29 · 2020-12-10T08:39:04.000Z

comment:23

I confirm the issue is fixed.

Answer 30 · 2020-12-10T16:29:05.000Z

comment:24

Thank you!

Answer 31 · 2020-12-10T16:29:05.000Z

Changed keywords from none to sd111

Answer 32 · 2020-12-14T23:52:49.000Z

Changed branch from u/mkoeppe/src_doc_bootstrap__simplify_by_using_new_options_of__sage__package_list_ to 084fbf6