-
-
Notifications
You must be signed in to change notification settings - Fork 434
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
Refactor bootstrap-conda
, m4/sage_spkg_collect.m4
through sage-package; handle dependencies_build
#36740
Open
mkoeppe
wants to merge
8
commits into
sagemath:develop
Choose a base branch
from
mkoeppe:dependencies_build
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5 tasks
5 tasks
9504420
to
6173224
Compare
5 tasks
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Dec 9, 2023
…eaning separately from build/install <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> The `sage-spkg` script installs normal and wheel packages. We create new options that make it possible to invoke the phases of the installation separately. ``` -d: only download the package -b: build and install (stage) only, do not run post-install or check -p: post-install only -x: exclusively run the test suite; this may assume that: * the package has been installed already and/or that * the temporary build directory has not been deleted -e: erase (delete) the temporary build directory only ``` This is preparation for improvements in `Makefile.in` in combination with sagemath#36740 <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36738 Reported by: Matthias Köppe Reviewer(s): Michael Orlitzky
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Dec 9, 2023
… to the post-install phase <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> Previously, on `sdh_pip_install`, the wheel file is staged in DESTDIR, but the wheel is installed immediately. Now we store a new script `spkg-pipinst`, which is run after unloading DESTDIR (and before any `spkg-postinst` script), which does the actual installation of the wheel. - This resolves sagemath#30956 (fixing the wheel URLs shown in `sage -pip freeze`) -- except when `SAGE_SUDO` is set. Apart from this and some changes to the messages displayed during package installation, this should make no difference for any of our packages. Just so that it is tested for at least one package in CI, we include a small package update. Together with - sagemath#36738 and - sagemath#36740, this is preparation for requiring only the build dependencies ("build- system requires") while building a wheel for the package, and to require the runtime dependencies ("install-requires") only later, when the wheel is to be installed. <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36743 Reported by: Matthias Köppe Reviewer(s): John H. Palmieri
6173224
to
c51ffd8
Compare
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Dec 13, 2023
…cript packages <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> Split out and updated from sagemath#29386. The section of `build/make/Makefile.in` that deals with script packages can be consolidated with the section on normal packages later, in a follow-up PR after sagemath#36740, sagemath#36738. We switch the `spkg-install` scripts of the `sage*` script packages to the templated versions (`spkg-install.in`), which is a simplification. `spkg-check` remains as is because we still invoke it directly (changing this will need sagemath#36738). Resolves sagemath#29386. <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> - Depends on sagemath#36737 (for testing) <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36747 Reported by: Matthias Köppe Reviewer(s): Michael Orlitzky
c51ffd8
to
b12e093
Compare
37aad3c
to
74b1b83
Compare
build/pkgs/*/dependencies_build
bootstrap
, bootstrap-conda
, m4/sage_spkg_collect.m4
through sage-package; handle dependencies_build
433b9d9
to
cfbc053
Compare
cfbc053
to
107b3a7
Compare
This was referenced Dec 27, 2023
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Apr 8, 2024
…nts.txt` <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> As discussed in sagemath#36982: - the name "install-requires" has become outdated with the transition to the new names set by the `pyproject.toml` format (see https://setupto ols.pypa.io/en/latest/userguide/dependency_management.html#declaring- required-dependency, compare the tabs "pyproject.toml" vs. "setup.cfg") - this will help with sagemath#35890, as GitHub will be able to just read the `version_requirements.txt` files sagemath#36982 (comment) Notes for reviewers: - **This is a simple, limited-scope improvement and not intended as a long-term commitment to the format of the files in build/pkgs/....** - PRs sagemath#37430, sagemath#37350, sagemath#36740 remove direct access to build/pkgs in favor of using the sage_bootstrap API (sage --package). <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> - Depends on sagemath#37401 <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36999 Reported by: Matthias Köppe Reviewer(s): Dima Pasechnik, Kwankyu Lee, Nathan Dunfield
baa5606
to
7b7d4a6
Compare
7b7d4a6
to
a006616
Compare
e36eddd
to
41403cc
Compare
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Jun 16, 2024
…pkg_collect.m4`, `sage-spkg-info` through `sage --package properties`, `sage-get-system-packages` <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> Using the command `sage --package properties` added in sagemath#37018, we eliminate some direct references to `build/pkgs/` from various scripts and reduce code duplication in reading SPKG metadata. Using other `sage --package` commands, we also simplify the code for `bootstrap -s` and `bootstrap -D`. Review instructions: Note that there are changes to the `bootstrap`-generated file `m4/sage_spkg_configures.m4`: - The macro `SAGE_SPKG_COLLECT_INIT` is now called explicitly, with an argument (the full list of packages). - Lines such as `m4_define([SPKG_INSTALL_REQUIRES_exceptiongroup], [['exceptiongroup; python_version<\"3.11\"',]])dnl` are added for use by the macro `SAGE_PYTHON_PACKAGE_CHECK` (replacing direct access to `version_requirements.txt` files) <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> This is: - Split out from sagemath#36740 <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#37430 Reported by: Matthias Köppe Reviewer(s): Julian Rüth, Kwankyu Lee, Matthias Köppe, Tobias Diez
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Jun 17, 2024
…pkg_collect.m4`, `sage-spkg-info` through `sage --package properties`, `sage-get-system-packages` <!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> Using the command `sage --package properties` added in sagemath#37018, we eliminate some direct references to `build/pkgs/` from various scripts and reduce code duplication in reading SPKG metadata. Using other `sage --package` commands, we also simplify the code for `bootstrap -s` and `bootstrap -D`. Review instructions: Note that there are changes to the `bootstrap`-generated file `m4/sage_spkg_configures.m4`: - The macro `SAGE_SPKG_COLLECT_INIT` is now called explicitly, with an argument (the full list of packages). - Lines such as `m4_define([SPKG_INSTALL_REQUIRES_exceptiongroup], [['exceptiongroup; python_version<\"3.11\"',]])dnl` are added for use by the macro `SAGE_PYTHON_PACKAGE_CHECK` (replacing direct access to `version_requirements.txt` files) <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> This is: - Split out from sagemath#36740 <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#37430 Reported by: Matthias Köppe Reviewer(s): Julian Rüth, Kwankyu Lee, Matthias Köppe, Tobias Diez
41403cc
to
fbf7d90
Compare
bootstrap
, bootstrap-conda
, m4/sage_spkg_collect.m4
through sage-package; handle dependencies_build
bootstrap-conda
, m4/sage_spkg_collect.m4
through sage-package; handle dependencies_build
a4137cd
to
f2b8011
Compare
…Makefile variables build_deps_... m4/sage_spkg_collect.m4: Change handling of 'dependencies_check' files, generate Makefile variables check_deps_... build/make/Makefile.in: Handle build_deps, deps, check_deps separately in package templates m4/sage_spkg_collect.m4, build/make/Makefile.in: Simplify handling of optional deps bootstrap, m4/sage_spkg_collect.m4: Get dependencies via 'sage --package dependencies --format=shell' m4/sage_spkg_collect.m4: Handle wheel packages like normal packages
bootstrap: Remove last direct use of 'build/pkgs' bootstrap-conda: Use 'sage-package properties' and 'sage-package dependencies' Update shell uses of 'sage-package properties'
f2b8011
to
004746e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Using new commands
sage --package dependencies
andsage --package properties
, we eliminate direct references tobuild/pkgs/
from various scripts and reduce code duplication in reading SPKG metadata.In particular, we simplify the
configure
-time compilation of dependency information forbuild/make/Makefile
inm4/sage_spkg_collect.m4
; this also gives a bit of a speedup when running./configure
.We complement the
build/make/Makefile
variablesdeps_SPKG
by new variablesbuild_deps_SPKG
andcheck_deps_SPKG
. This is refactoring only; there should be no change in behavior (also note that nodependencies_build
file is added here.) Together with #36738, this is also preparation for improving the parallelization of build phases of various packages. As it gives us a clearer mapping of "build-system requires" and "install requires" of Python packages, it is also preparation for automating more steps of the creation of Python SPKGs (sage -package create --pypi
).Review instructions:
bootstrap
-generated filem4/sage_spkg_configures.m4
: The macroSAGE_SPKG_COLLECT_INIT
is now called explicitly, with an argument (the full list of packages)make DEBUG_RULES=1
. (Note there is no change to the macro for script packages here; these rules will be merged into the macro for normal packages as a follow-up tobuild/bin/sage-spkg
: Add support for installing script packages #36747._📝 Checklist
⌛ Dependencies
sage --package properties
#37018 (split out from here)sage -package dependencies
#37350 (split out from here)bootstrap
,bootstrap-conda
,m4/sage_spkg_collect.m4
,sage-spkg-info
throughsage --package properties
,sage-get-system-packages
#37430 (split out from here)