#30947 closed enhancement (fixed)

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

Reported by: mkoeppe Owned by:
Priority: minor Milestone: sage-9.3
Component: documentation Keywords: sd111
Cc: slabbe, slelievre Merged in:
Authors: Matthias Koeppe Reviewers: Sébastien Labbé
Report Upstream: N/A Work issues:
Branch: 084fbf6 (Commits, GitHub, GitLab) Commit: 084fbf6b995e304d126000e5be2797b51fbd4592
Dependencies: #30865, #28745 Stopgaps:

Status badges

Change History (25)

comment:1 Changed 10 months ago by mkoeppe

  • Dependencies changed from #30865 to #30865, #28745

comment:2 Changed 10 months ago by mkoeppe

  • Branch set to u/mkoeppe/src_doc_bootstrap__simplify_by_using_new_options_of__sage__package_list_

comment:3 Changed 10 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Cc slabbe slelievre added
  • Commit set to 5ad29d5e5686a403f51d6b09d5ba0ea02face4e9
  • Status changed from new to needs_review

Last 10 new commits:

be5bdf2build/pkgs/pyparsing: Update to 2.4.7
88b8427build/pkgs/pytz: Update to 2020.4
df7972cbuild/pkgs/rpy2: Update to 3.3.6
7cd96b2build/pkgs/scipy/patches/extern_decls.patch: Remove
84d778cFixing upstream url for networkx
13af6c6fix tarball name
45b3408Merge commit '13af6c6b8c1533ba9d1b45b127e1a7b7d30000c6' of git://trac.sagemath.org/sage into t/28745/environment_yaml
736e006build/pkgs/pathpy/distros/conda.txt: Remove
cdfbe3aMerge branch 't/28745/environment_yaml' into t/30947/src_doc_bootstrap__simplify_by_using_new_options_of__sage__package_list_
5ad29d5src/doc/bootstrap: Simplify by using new options of "sage -package list"

comment:4 follow-up: Changed 10 months ago by slabbe

I assume the only commit to be reviewed here is this one: https://git.sagemath.org/sage.git/commit/?h=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?

comment:5 Changed 10 months ago by slabbe

  • Reviewers set to Sébastien Labbé

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.

comment:6 in reply to: ↑ 4 Changed 10 months ago by mkoeppe

Replying to slabbe:

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.

comment:7 Changed 10 months ago by slabbe

  • Description modified (diff)
  • Status changed from needs_review to positive_review

comment:8 Changed 10 months ago by slabbe

  • Description modified (diff)

ok, then unindentation can wait.

comment:9 Changed 10 months ago by mkoeppe

Thank you!

comment:10 Changed 10 months ago by slabbe

  • Description modified (diff)
  • Status changed from positive_review to needs_work

comment:11 Changed 10 months ago by slabbe

  • Description modified (diff)

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

comment:12 Changed 10 months ago by git

  • Commit changed from 5ad29d5e5686a403f51d6b09d5ba0ea02face4e9 to 5e9366d545d744840c2de716dd6d070c97555b13

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

5e9366dMerge tag '9.3.beta2' into t/30947/src_doc_bootstrap__simplify_by_using_new_options_of__sage__package_list_

comment:13 Changed 10 months ago by mkoeppe

  • Status changed from needs_work to positive_review

Merged cleanly

comment:14 Changed 10 months ago by slabbe

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

comment:15 follow-up: Changed 10 months ago by slabbe

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.

comment:16 follow-up: Changed 10 months ago by slabbe

  • Status changed from positive_review to needs_info

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'
Last edited 10 months ago by slabbe (previous) (diff)

comment:17 Changed 10 months ago by slabbe

  • Status changed from needs_info to needs_work

comment:18 Changed 10 months ago by git

  • Commit changed from 5e9366d545d744840c2de716dd6d070c97555b13 to 184ae6d63c28e4462ec3711dc60c6d357218c3c8

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

184ae6dMerge tag '9.3.beta3' into t/30947/src_doc_bootstrap__simplify_by_using_new_options_of__sage__package_list_

comment:19 in reply to: ↑ 15 Changed 10 months ago by mkoeppe

Replying to slabbe:

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.

comment:20 Changed 10 months ago by git

  • Commit changed from 184ae6d63c28e4462ec3711dc60c6d357218c3c8 to 084fbf6b995e304d126000e5be2797b51fbd4592

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

084fbf6src/doc/bootstrap: Use ./sage

comment:21 in reply to: ↑ 16 Changed 10 months ago by mkoeppe

Replying to slabbe:

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.

comment:22 Changed 10 months ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:23 Changed 10 months ago by slabbe

  • Status changed from needs_review to positive_review

I confirm the issue is fixed.

comment:24 Changed 10 months ago by mkoeppe

  • Keywords sd111 added

Thank you!

comment:25 Changed 10 months ago by vbraun

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