Use meson in sage-the-distro #39030
Use meson in sage-the-distro #39030
Conversation
|
Sure, I'm ready to help as much as I can. Could you share an outline of how this is meant to work? I imagine it is an adaptation to the Sage's venv (which is a more or less standard venv, no?) of what one can get e.g. with a standard venv on Gentoo Linux, or another environment where all the dependencies are available. Would this approach also work for a Conda-based environment? |
|
As a first step, I would not change how sage-the-distro works. So leave all the dependency installation (python and non-python deps) untouched for now. (We can discuss later how to transfer the configure checks to meson, and perhaps even completely replace the install scripts by mesons wrap files) So, basically the only required change would be |
|
running |
|
that's cause |
|
I think these requirements files can be safely deleted. At least I'm not aware of any usage of them. |
these are for building all these sagemath-* pseudo-packages. By the way, can you resolve the git conflict? |
You are right, these are actually used. Does ae33069 works for you? (I cannot test it atm on linux) |
|
something in your branch broke the build system, with recursive make running wild. Not much fun to debug. |
|
by the way, can this be based on the grayskull branch - this would make working a bit faster, as |
|
the recursion (I ran So the problem is that |
|
in the output of ./configure. It needs, and always needed, the following fix: --- a/configure.ac
+++ b/configure.ac
@@ -431,7 +431,7 @@ AS_IF([test "x$enable_download_from_upstream_url" = "xyes"], [
])
AC_SUBST([SAGE_SPKG_OPTIONS])
-AC_ARG_ENABLE([sagelib],
+AC_ARG_ENABLE([sage_conf],
AS_HELP_STRING([--disable-sage_conf],
[disable build of the sage_conf package]), [
for pkg in sage_conf; do |
|
your fix for VERSION.txt didn't work: |
|
To unblock this, one needs to fix
? After these changes, |
|
with changes as above, the ERROR comes from Indeed, there is no One way or another, With --- a/build/pkgs/sagelib/spkg-install.in
+++ b/build/pkgs/sagelib/spkg-install.in
@@ -1,7 +1,7 @@
if [ "$SAGE_EDITABLE" = yes ]; then
- cd ../../..
+ cd $SAGE_ROOT
else
- cd ../../..
+ cd $SAGE_ROOT
# Issue #34181: Do not allow scripts with shebang lines from old
# venvs leak into new venvs. (Changes only seem to be necessary
# for non-editable builds.)one gets to where the log says So |
build/pkgs/sagelib/spkg-install.in
Outdated
| else | ||
| # Now implied: "$SAGE_WHEELS" = yes | ||
| # We should remove the egg-link that may have been installed previously. | ||
| (cd "$SITEPACKAGESDIR" && rm -f sagemath-standard.egg-link) | ||
| # Use --no-build-isolation to avoid rebuilds because of dependencies: | ||
| # Compiling sage/interfaces/sagespawn.pyx because it depends on /private/var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/pip-build-env-609n5985/overlay/lib/python3.10/site-packages/Cython/Includes/posix/unistd.pxd | ||
| sdh_pip_install --no-build-isolation . | ||
| sdh_pip_install --no-build-isolation . --config-settings=setup-args="-DSAGE_LOCAL=$SAGE_LOCAL" |
There was a problem hiding this comment.
So, a concrete comment @dimpase in the non-editable install you do not set the build-dir. So, I am expecting that if you try to build the documentation with make doc-html after a non editable install, it will fail because it specifically looks for build/sage-distro and I am fairly sure that will not be what pip uses by default.
There was a problem hiding this comment.
SAGE_EDITABLE=no make build quickly errors out:
[sagelib-10.8.beta0] [spkg-install] Installing sagelib-10.8.beta0
[sagelib-10.8.beta0] [spkg-install] usage: python -m build [-h] [--version] [--verbose] [--sdist] [--wheel]
[sagelib-10.8.beta0] [spkg-install] [--outdir PATH] [--skip-dependency-check]
[sagelib-10.8.beta0] [spkg-install] [--no-isolation | --installer {pip,uv}]
[sagelib-10.8.beta0] [spkg-install] [--config-setting KEY[=VALUE]]
[sagelib-10.8.beta0] [spkg-install] [srcdir]
[sagelib-10.8.beta0] [spkg-install] python -m build: error: unrecognized arguments: --config-settings=setup-args=-DSAGE_LOCAL=/home/dima/software/sage-src/local
@tobiasdiez - what's the correct way to do this, or it's indeed broken?
It seems to have been broken before this PR, too, no?
There was a problem hiding this comment.
Thanks a lot @kiwifb! I indeed missed to change the builddir for non-editable builds. Should be fixed now.
@dimpase just a typo due to different conventions of build and pip install (config-setting vs config-settings). What I don't understand is why the non-editable install needs to create wheels using build, instead of using pip install without -e. But that's for another day.
|
I'm guessing this breaks incremental builds, ok but maybe make it more explicit then there will be fewer failure reports from people that test incremental builds |
before trying this PR one most probably needs Then it should be fine (perhaps more cleaning up of things triggering senseless recursive rebuilds might be needed) |
@vbraun Would it be possible to test this branch here on isolation on the build bots. Since it completely changes how sagelib is built, it's very likely that other PRs are indeed changing things in a way that needs slight modification afterwards. |
|
I confirm that the build works with the current branch on the Ubuntu machine where it failed because of NumPy. |
Awesome, thanks for testing! |
Co-authored-by: Kwankyu Lee <[email protected]>
Fixed that now by removing the conf.py files also here in this PR. Since the CI is passing, setting it back to positive review. |
|
merge conflict |
Resolved now. @vbraun |
<!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> Replace the old setuptools-based build by the new meson-based one in sage-the-distro. Delete most of the old stuff that is no longer needed now. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] The title is concise and informative. - [ ] 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 and checked the documentation preview. ### ⌛ Dependencies - sagemath#40133 - sagemath#39973 - sagemath#40071 - sagemath#40597 <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39030 Reported by: Tobias Diez Reviewer(s): Dima Pasechnik, François Bissey, Tobias Diez
Replace the old setuptools-based build by the new meson-based one in sage-the-distro. Delete most of the old stuff that is no longer needed now.
📝 Checklist
⌛ Dependencies