Use meson instead of configure for conda install #36489

tobiasdiez · 2023-10-20T10:07:13Z

Switches from configure to a simple meson-based setup (when using conda) by implementing a sage-conf_meson package that calls meson to looks for the dependencies. I've kept the sage-conf_conda for now, as comparison and fall-back.

This is preparation for #34630.

📝 Checklist

The title is concise, informative, and self-explanatory.
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

Fix mamba install in conda ci workflow #36468 to fix the conda workflow

…onfig

…on-config

This reverts commit 9694046.

src/doc/en/installation/conda.rst

mkoeppe · 2023-10-20T17:51:15Z

meson.build

@@ -0,0 +1,40 @@
+project('sage', 'c')


I'd be more comfortable with this file being local to sage-conf_meson, so as not to create expectations that Sage-the-distribution will switch to meson

The idea in the middle to long term is to support running meson compile from the root directory to compile sagelib with meson (using conda). For this the meson build file needs to be there (and its pretty much convention to have such a build file in the root). Sage-the-distro supporting meson would mean that there is a meson file in the build directory, or not?

I'm not aware of such a plan, and I'd suggest that if you have a plan to use a new Issue in which you explain it.

From what you wrote above, it seems unlikely that it would be the same meson.build file that would be suitable for both purposes, so nothing is gained by putting it there.

It's part of #34630. In the long-term you want to use of course meson-python (configured via pyproject.toml). But in the short to midterm its easier to just call meson directly. That's at least how scipy/numpy proceeded: https://github.com/scipy/scipy/pull/14847/files#diff-e42998d51257e6f84aa51ffa5f4ed1544cb12f97a94978c44f3bc281b5324d79R390-R500

And yes, the meson file added in this PR can be used for this purpose. In fact, I believe we can get ride of the conf and setup packages, but of course there still needs to be some way to provide the runtime-config (probably by just putting the generated conf file in the correct place in the builddir).

Well, #34630 has nothing to with stuff that belongs in SAGE_ROOT.

So it's for compiling the Sage library, for the single use case of the all-conda install?

There is nothing conda-specific about the meson files, it's only that conda provides a nice fixed environment and thus is a good starting point. You can try to run meson setup outside of a conda environment and it should work (if you have all the necessary packages installed in the system).

I'll explain and summarize once more:

What you just described ("it should work (if you have all the necessary packages installed in the system).") means that you are configuring / building the Sage library.

This is different from what our configure.ac and Makefile do. They configure / build the Sage distribution.

There is no plan to switch the Sage distribution to using meson.

When people see a file meson.build next to configure.ac and Makefile, they are bound to think that using meson is an alternative for building the same thing. So this has the potential to mislead.

When people see a file meson.build next to configure.ac and Makefile, they are bound to think that using meson is an alternative for building the same thing. So this has the potential to mislead.

So if you see a hammer and a saw next to each other you are bound to see them as alternatives for building the same thing?

Conceptually, meson setup does the same thing as configure: go through the declared dependencies and write out a config file. Similarly, meson compile will do the same thing as make sage-all (or whatever the correct target is for building sagelib).

It's also not technically possible to move the meson file in the root to src since meson's subdir only walks down the tree.

Anyway, I'm tired of this stupid discussion and will stop participating in it. If you think that the harm created by the meson file in the root outweighs the positives of this PR, then please go ahead and continue blocking it. Please leave the "ready for review" tag on, as there are no work items that referee and PR creator agree on (it just that you don't like a small design choice in the PR).

Similarly, meson compile will do the same thing as make sage-all (or whatever the correct target is for building sagelib).

The target make sage-all does not exist. make all, the default target, builds the Sage distribution.

Sorry, but there are a lot of files in the root folder that are not related to the distribution.
[...] tox ini [...]

Actually SAGE_ROOT/tox.ini defines the portability tests of the Sage distribution.

bootstrap-conda

mkoeppe · 2023-10-20T18:01:49Z

pkgs/sage-conf_meson/sage_conf.py

+from _sage_conf.__main__ import _main
+
+from builddir._conf import *
+from builddir.build_info import CONDA_PREFIX, SAGE_ROOT


This "from builddir... import" makes me a bit uncomfortable.
This seems to assume that . is in sys.path
Please test that also non-editable installs (via wheels) of sage-conf_meson work.

The builddir here is generated under pkgs/sage-conf_meson

Yes, I knew that when I wrote the above. So?

I don't see the difference between configure creating the _conf.py file and here we are using meson to create the builddir folder. Can you please be more specific what problem you encounter?

I don't see the difference

In https://github.com/sagemath/sage/blob/develop/pkgs/sage-conf/sage_conf.py#L1 we have:
from _sage_conf._conf import *

_sage_conf is an installed top-level package.

Here: from builddir._conf import *

builddir is not an installed top-level package.

More questions?

At this time, I don't have a comment on what variables provided by sage_conf are needed at the runtime of the Sage library.

That's my observation: apparently none. The fallbacks are good enough for conda.

You may be right about this, but a lot currently still depends on the assumption "$SAGE_LOCAL = $CONDA_PREFIX" (e.g., see #30914 for pointers), which is guaranteed by the all-conda installation instructions, but I am not sure that we will want to keep it in the future.

A problem with this assumption is that we currently have an inflexible either-or situation:

We either take all packages from conda and do not use the Sage build system at all, for anything;

or we use the Sage build system for everything and then cannot use Python packages from conda. (For the purpose of this discussion, let's ignore the --enable-system-site-packages switch.)

sage-conf is a declared install-requires of sagelib, so some version of it needs to be installed.

So can I change that to an optional dependency? (since it seems to be optional)

No, by design it's required, but as is explained in https://github.com/sagemath/sage/blob/develop/pkgs/sage-conf/README.rst, you can pick your own implementation.

At this time, I don't have a comment on what variables provided by sage_conf are needed at the runtime of the Sage library.

That's my observation: apparently none. The fallbacks are good enough for conda.

You may be right about this, but a lot currently still depends on the assumption "$SAGE_LOCAL = $CONDA_PREFIX" (e.g., see #30914 for pointers), which is guaranteed by the all-conda installation instructions, but I am not sure that we will want to keep it in the future.

A problem with this assumption is that we currently have an inflexible either-or situation:

* We either take all packages from conda and do not use the Sage build system at all, for anything; * or we use the Sage build system for everything and then cannot use Python packages from conda. (For the purpose of this discussion, let's ignore the `--enable-system-site-packages` switch.)

Right, but in the "we take everything from conda" the assumption will always be valid, right? So in this case we don't need a sage_conf package. If in the future some semi-mixed versions are supported, then this method needs its own sage_conf package.

So can I change that to an optional dependency? (since it seems to be optional)

No, by design it's required, but as is explained in https://github.com/sagemath/sage/blob/develop/pkgs/sage-conf/README.rst, you can pick your own implementation.

This document doesn't say anything on why we need the package nor why the design cannot be changed.

Either way, I've fixed the package now. Please give it a try.

Right, but in the "we take everything from conda" the assumption will always be valid, right?

My point is that this dichotomy of the two installation methods is not sustainable; so it would be unwise to be invested in a "pure" mode that seeks to eliminate sage_conf altogether.

mkoeppe · 2023-10-20T18:12:30Z

This goes in a great direction, thanks for working on it.

For the conda CI, I think it would be crucial to keep a run of the configure-based sage-conf_conda around, just for the tests that it runs (instead of just trusting that the package versions installed in the environment are OK). (For example, by building but not installing the sage-conf_conda wheel.)

Co-authored-by: Matthias Köppe <[email protected]>

tobiasdiez · 2023-10-21T15:50:36Z

Thanks for the review. Should have addressed all points now.

pkgs/sage-conf/pyproject.toml

tobiasdiez · 2023-10-29T23:45:06Z

@mkoeppe "Please leave the "ready for review" tag on, as there are no work items that referee and PR creator agree on (it just that you don't like a small design choice in the PR)." from above. Thanks!

mkoeppe · 2023-10-29T23:47:45Z

There are clear work items.

which is blocked for stupid reasons

github-actions · 2023-11-15T01:22:55Z

Documentation preview for this PR (built with commit 7a44082; changes) is ready! 🎉

… `pyproject.toml`     - In part split out from sagemath#36489 - Part of sagemath#33577  ### 📝 Checklist     - [x] The title is concise, informative, and self-explanatory. - [ ] 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  - Depends on sagemath#36533 (which changes setup.cfg, merged here) - Depends on sagemath#36885  URL: sagemath#36561 Reported by: Matthias Köppe Reviewer(s): François Bissey

As noted in sagemath#37024 and in sagemath#36489, `sage-conf` is actually not needed on a few systems now. For this reason, we make the installation of it optional (as there are no optional install requires as far as I know, this means we remove it from `pyproject.toml` and `setup.cfg`). We also remove it from "install all dependencies via conda" as its not needed there.

tobiasdiez · 2024-10-28T15:01:08Z

No longer needed.

tobiasdiez added 8 commits October 20, 2023 10:06

Use meson instead of configure for conda install

bfac057

remove editable wheel customization

9694046

remove configure from conda ci

376742a

Merge remote-tracking branch 'tobiasdiez/ci-conda-error' into meson-c…

e270392

…onfig

Merge remote-tracking branch 'mkoeppe/conda_legacy_editable' into mes…

a21655b

…on-config

Revert "remove editable wheel customization"

84ff7ff

This reverts commit 9694046.

split setup and conf for cleaner display

885e2b4

add meson to conda env

f755a8e

tobiasdiez marked this pull request as ready for review October 20, 2023 15:32

tobiasdiez added the s: needs review label Oct 20, 2023

tobiasdiez requested review from mkoeppe and dimpase October 20, 2023 15:32

mkoeppe reviewed Oct 20, 2023

View reviewed changes

src/doc/en/installation/conda.rst Outdated Show resolved Hide resolved

mkoeppe reviewed Oct 20, 2023

View reviewed changes

bootstrap-conda Outdated Show resolved Hide resolved

mkoeppe reviewed Oct 20, 2023

View reviewed changes

mkoeppe added the c: build: configure label Oct 20, 2023

tobiasdiez and others added 4 commits October 21, 2023 23:32

Fix style in conda docs

4406868

Co-authored-by: Matthias Köppe <[email protected]>

add meson only in src env files

c0cdb5f

keep configure around in the ci

b408311

Merge remote-tracking branch 'origin/develop' into meson-config

4c8b56b

fix conda-ci

f6f27d9

mkoeppe reviewed Oct 21, 2023

View reviewed changes

pkgs/sage-conf/pyproject.toml Show resolved Hide resolved

mkoeppe added s: needs work and removed s: needs review labels Oct 22, 2023

tobiasdiez added s: needs review and removed s: needs work labels Oct 23, 2023

mkoeppe removed the s: needs review label Oct 23, 2023

tobiasdiez added the s: needs review label Oct 29, 2023

mkoeppe added s: needs work and removed s: needs review labels Oct 29, 2023

tobiasdiez added s: needs review and removed s: needs work labels Oct 29, 2023

mkoeppe added s: needs work and removed s: needs review labels Oct 29, 2023

tobiasdiez added s: needs review and removed s: needs work labels Oct 29, 2023

mkoeppe added s: needs work and removed s: needs review labels Oct 30, 2023

tobiasdiez added s: needs review and removed s: needs work labels Nov 2, 2023

vbraun force-pushed the develop branch from 883e05f to e349b00 Compare November 12, 2023 16:25

tobiasdiez added a commit to tobiasdiez/sage that referenced this pull request Nov 14, 2023

Remove dependency on sagemath#36489

1c1bfa3

which is blocked for stupid reasons

Merge branch 'develop' into meson-config

7a44082

tobiasdiez added the disputed PR is waiting for community vote, see https://groups.google.com/g/sage-devel/c/IgBYUJl33SQ label Dec 23, 2023

tobiasdiez mentioned this pull request Jan 22, 2024

Make sage-conf optional #37138

Merged

5 tasks

vbraun force-pushed the develop branch from eba5e19 to e5f42fa Compare June 3, 2024 22:15

tobiasdiez closed this Oct 28, 2024

github-actions bot removed the s: needs review label Oct 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use meson instead of configure for conda install #36489

Use meson instead of configure for conda install #36489

tobiasdiez commented Oct 20, 2023 •

edited

Loading

mkoeppe Oct 20, 2023

tobiasdiez Oct 21, 2023

mkoeppe Oct 21, 2023

tobiasdiez Oct 22, 2023 •

edited

Loading

mkoeppe Oct 22, 2023

tobiasdiez Oct 29, 2023

mkoeppe Oct 29, 2023

tobiasdiez Oct 29, 2023 •

edited

Loading

mkoeppe Oct 29, 2023

mkoeppe Nov 5, 2023

mkoeppe Oct 20, 2023

tobiasdiez Oct 21, 2023

mkoeppe Oct 21, 2023

tobiasdiez Oct 22, 2023

mkoeppe Oct 22, 2023

mkoeppe Oct 28, 2023

mkoeppe Oct 28, 2023

tobiasdiez Oct 28, 2023

tobiasdiez Oct 28, 2023

mkoeppe Oct 28, 2023

mkoeppe commented Oct 20, 2023

tobiasdiez commented Oct 21, 2023

tobiasdiez commented Oct 29, 2023

mkoeppe commented Oct 29, 2023

github-actions bot commented Nov 15, 2023

tobiasdiez commented Oct 28, 2024

Use meson instead of configure for conda install #36489

Use meson instead of configure for conda install #36489

Conversation

tobiasdiez commented Oct 20, 2023 • edited Loading

📝 Checklist

⌛ Dependencies

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasdiez Oct 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasdiez Oct 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkoeppe commented Oct 20, 2023

tobiasdiez commented Oct 21, 2023

tobiasdiez commented Oct 29, 2023

mkoeppe commented Oct 29, 2023

github-actions bot commented Nov 15, 2023

tobiasdiez commented Oct 28, 2024

tobiasdiez commented Oct 20, 2023 •

edited

Loading

tobiasdiez Oct 22, 2023 •

edited

Loading

tobiasdiez Oct 29, 2023 •

edited

Loading