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

bootstrap, src/doc/Makefile: Remove use of the SAGE_ROOT/sage script #33852

Closed
mkoeppe opened this issue May 14, 2022 · 34 comments
Closed

bootstrap, src/doc/Makefile: Remove use of the SAGE_ROOT/sage script #33852

mkoeppe opened this issue May 14, 2022 · 34 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented May 14, 2022

We change the bootstrap scripts so that they invoke build/bin/sage-package directly instead of going through the SAGE_ROOT/sage script.

This is faster and also more robust because it separates build phases better by eliminating the (unnecessary) use of configure-generated files such as src/bin/sage-src-env-config.

We also move the generation of src/doc/en/reference/repl/options.txt (by calling sage -advanced) from bootstrap time to src/doc/Makefile. The information displayed there depends on what packages are installed, so running it at bootstrap time was not correct.

Depends on #33740

CC: @jhpalmieri @tobiasdiez @orlitzky @dimpase @kiwifb

Component: scripts

Author: Matthias Koeppe

Branch/Commit: 67c8238

Reviewer: John Palmieri, François Bissey

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

@mkoeppe mkoeppe added this to the sage-9.7 milestone May 14, 2022
@mkoeppe
Copy link
Member Author

mkoeppe commented May 14, 2022

@mkoeppe
Copy link
Member Author

mkoeppe commented May 14, 2022

Commit: ccbc21a

@mkoeppe
Copy link
Member Author

mkoeppe commented May 14, 2022

Last 10 new commits:

10338a7Change ci as well
0cf7334Add sage-setup as install-requires dep
6d2b149Use relative paths to not install src package
1dba3baMaybe thats the correct shebang?
c1b2fbfRevert install_requires change
ad56ba3Go back to two pip installs for now
b42f5f5Readd how to use specific python version
87f360cReadd name arg
765dff7Merge #33740
ccbc21abootstrap, bootstrap-conda, src/doc/bootstrap: Use build/bin/sage-package directly

@mkoeppe mkoeppe changed the title bootstrap: Remove use of the SAGE_ROOT/sage script bootstrap, src/doc/Makefile: Remove use of the SAGE_ROOT/sage script May 14, 2022
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 14, 2022

Changed commit from ccbc21a to 4560895

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 14, 2022

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

4560895src/doc/Makefile: Remove explicit use of SAGE_ROOT and the top-level sage script

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 14, 2022

Changed commit from 4560895 to 7759f3c

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 14, 2022

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

7759f3cGenerate en/reference/repl/options.txt at build time, not bootstrap time

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

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

67c8238src/bin/sage: Print error to error output

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

Changed commit from 7759f3c to 67c8238

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

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

c671cbbm4/sage_spkg_collect.m4: Read files dependencies_{optional,order_only,check}
9f3a296build/pkgs: Split dependencies_optional and dependencies_check out from dependencies

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

Changed commit from 67c8238 to 9f3a296

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

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

46144d1build/bin/sage-spkg-info: Restore message '(none known)' for packages without system package info
3faf0abbuild/bin/sage-spkg-info: Format dependencies

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

Changed commit from 9f3a296 to 3faf0ab

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

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

facb1fcbuild/bin/sage-spkg-info: Format dependencies

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

Changed commit from 3faf0ab to facb1fc

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

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

8ab0814build/pkgs/memory_allocator/SPKG.rst: Fix markup

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

Changed commit from facb1fc to 8ab0814

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

Changed commit from 8ab0814 to 2d12c0b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

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

2d12c0bbuild/pkgs/memory_allocator/SPKG.rst: Fix markup

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

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

0e5162abuild/pkgs/mathics/SPKG.rst: Fix title
51eb305build/bin/sage-spkg-info: Fix rst label

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

Changed commit from 2d12c0b to 51eb305

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 15, 2022

Changed commit from 51eb305 to 67c8238

@kiwifb
Copy link
Member

kiwifb commented May 26, 2022

comment:17

I found it a bit hard to figure out what came from #33740 and what is this ticket :(

Once I get past that issue, I agree with the changes. It is much cleaner and simpler. May not go far enough from a sage-on-gentoo point of view (I will still patch but we had that discussion sage --docbuild vs python -m sage_docbuild somewhere else and that step is not ready yet). Just one remark, all the targets are phony in src/doc/Makefile by necessity but we could have had en/reference/repl/options.txt as a real target instead of a phony one, not that it matter much in that sea of stuff that rebuild every time you call make.

@mkoeppe
Copy link
Member Author

mkoeppe commented May 26, 2022

comment:18

Replying to @kiwifb:

we could have had en/reference/repl/options.txt as a real target instead of a phony one

Yes but what would be its dependencies?

@kiwifb
Copy link
Member

kiwifb commented May 26, 2022

comment:19

Replying to @mkoeppe:

Replying to @kiwifb:

we could have had en/reference/repl/options.txt as a real target instead of a phony one

Yes but what would be its dependencies?

I am not sure I follow. It doesn't need to have dependencies or have I forgotten something fundamental about makefiles.

@mkoeppe
Copy link
Member Author

mkoeppe commented May 26, 2022

comment:20

In that case it would never be updated, which can't be right

@kiwifb
Copy link
Member

kiwifb commented May 26, 2022

comment:21

Replying to @mkoeppe:

In that case it would never be updated, which can't be right

Thank you for reminding me of some essentials :) I'd like to brush it off as it being too early in the morning to make a proper comment but I think that's just part of a pattern where I am too deep on the packager side, and thinking of workflow in which ultimately, makefile such as this one are "single use", and when something doesn't work you just restart from the top because you need the flow to be right. I am not spending enough time on the pure dev side, that's what

@jhpalmieri
Copy link
Member

comment:22

I think this looks okay, too. François, do you agree with a positive review?

@kiwifb
Copy link
Member

kiwifb commented May 27, 2022

comment:23

Yes, let's get it in the next beta.

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri, François Bissey

@mkoeppe
Copy link
Member Author

mkoeppe commented May 28, 2022

comment:25

Thanks!

@vbraun
Copy link
Member

vbraun commented Jun 6, 2022

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

4 participants