Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
mkoeppe opened this issue Nov 22, 2020 · 32 comments
Closed

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

mkoeppe opened this issue Nov 22, 2020 · 32 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Nov 22, 2020

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

@mkoeppe mkoeppe added this to the sage-9.3 milestone Nov 22, 2020
@mkoeppe
Copy link
Member Author

mkoeppe commented Nov 22, 2020

Changed dependencies from #30865 to #30865, #28745

@mkoeppe
Copy link
Member Author

mkoeppe commented Nov 24, 2020

@mkoeppe
Copy link
Member Author

mkoeppe commented Nov 24, 2020

Commit: 5ad29d5

@mkoeppe
Copy link
Member Author

mkoeppe commented Nov 24, 2020

Author: Matthias Koeppe

@mkoeppe
Copy link
Member Author

mkoeppe commented Nov 24, 2020

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"

@seblabbe
Copy link
Contributor

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?

@seblabbe
Copy link
Contributor

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.

@seblabbe
Copy link
Contributor

Reviewer: Sébastien Labbé

@mkoeppe
Copy link
Member Author

mkoeppe commented Nov 24, 2020

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.

@seblabbe

This comment has been minimized.

@seblabbe
Copy link
Contributor

comment:8

ok, then unindentation can wait.

@seblabbe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented Nov 24, 2020

comment:9

Thank you!

@seblabbe

This comment has been minimized.

@seblabbe

This comment has been minimized.

@seblabbe
Copy link
Contributor

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 25, 2020

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_

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 25, 2020

Changed commit from 5ad29d5 to 5e9366d

@mkoeppe
Copy link
Member Author

mkoeppe commented Nov 25, 2020

comment:13

Merged cleanly

@seblabbe
Copy link
Contributor

seblabbe commented Dec 9, 2020

comment:14

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

@seblabbe
Copy link
Contributor

seblabbe commented Dec 9, 2020

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.

@seblabbe
Copy link
Contributor

seblabbe commented Dec 9, 2020

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'

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 10, 2020

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_

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 10, 2020

Changed commit from 5e9366d to 184ae6d

@mkoeppe
Copy link
Member Author

mkoeppe commented Dec 10, 2020

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 10, 2020

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

084fbf6src/doc/bootstrap: Use ./sage

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 10, 2020

Changed commit from 184ae6d to 084fbf6

@mkoeppe
Copy link
Member Author

mkoeppe commented Dec 10, 2020

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.

@seblabbe
Copy link
Contributor

comment:23

I confirm the issue is fixed.

@mkoeppe
Copy link
Member Author

mkoeppe commented Dec 10, 2020

comment:24

Thank you!

@mkoeppe
Copy link
Member Author

mkoeppe commented Dec 10, 2020

Changed keywords from none to sd111

@vbraun
Copy link
Member

vbraun commented Dec 14, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants