Add needed pip packages to `src/environment.yml`, minimize conda environments by mkoeppe · Pull Request #35593 · sagemath/sage

mkoeppe · 2023-05-01T16:19:41Z

📚 Description

Adds a pip section to src/environment.yml for the missing standard package sagenb_export (this was left out in bootstrap-conda: Refactor, generate versioned environment files #36405)
This pip section is also good for adding the new pip package in Update to new conway-polynomials python package #36765
Resolves Minimize conda environment #34626
Update CI conda to use pkgs/sage-conf_conda (= what is documented in the installation guide)
Update pkgs/sage-conf_conda to use --enable-system-site-packages (this was left out in Simplify experimental all-conda installation instructions via pkgs/sage-conf_conda #36367 (review)) and to avoid --with-system-SPKG=force for packages that may be omitted by the environment minimizer.
Fixes some failures with src/environment-optional.yml

📝 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

Depends on sage --package list: Sort output, add switches --{in,ex}clude-dependencies #36393
Depends on bootstrap-conda: Refactor, generate versioned environment files #36405
Depends on Fix conda install instructions after setuptools update #36411
Depends on build/pkgs/setuptools_scm: Update to 8.0.4, add fixes for version 8 #36400
Depends on Fix mamba install in conda ci workflow #36468
Depends on Remove conda patchelf distro #36499 (split out from here)
Depends on networkx, scipy, ipywidgets: Update version ranges in conda.txt #36513 (split out from here)
Depends on build/pkgs/sagenb_export: Fix install-requires.txt #36514 (split out from here)
Depends on ipython: Fix conda version range #36560 (split out from here)

build/pkgs/pari_jupyter/distros/conda.txt

mkoeppe · 2023-05-02T00:10:12Z

Our upper bound on setuptools conflicted with cvxpy, which for unknown reasons has setuptools in install-requires (see cvxpy/cvxpy#2118)

mkoeppe · 2023-05-02T00:18:04Z

Still lots of conflicts, involving narrowly restricted version of boost-cpp etc.

build/pkgs/mathics/distros/conda.txt

.github/workflows/ci-conda.yml

build/pkgs/jupymake/distros/conda.txt

mkoeppe · 2023-05-03T01:58:53Z

@tobiasdiez do you think we can have the "s: run conda ci" label also run the workflow on PR syncs?

mkoeppe · 2023-05-09T07:06:33Z

@tobiasdiez do you think we can have the "s: run conda ci" label also run the workflow on PR syncs?

This is now part of #35380

mkoeppe · 2023-09-27T21:18:23Z

Solves and package installs take pretty long - on the CI runners about 30 mins; see https://github.com/sagemath/sage/actions/runs/6331256567?pr=35593
@isuruf Not sure what's wrong here, or how to ask the resolver for hints what we should be doing differently.

tobiasdiez

I propose you split this PR in multiple small ones with a single purpose to make reviewing easier.

src/doc/en/installation/conda.rst

tobiasdiez · 2023-09-27T23:54:05Z

src/doc/en/installation/conda.rst

      $ ./bootstrap
      $ ./configure --with-python=$CONDA_PREFIX/bin/python             \
                    --prefix=$CONDA_PREFIX                             \
+                    --enable-system-site-packages                      \


Why is this needed / what is it doing in this context? Also since the word 'system' package might be misleading for conda, and the list of additional commands is getting rather big, I propose you introduce a new toggle "with-conda=$CONDA_PREFIX" to configure that takes care of all this config.

what is it doing in this context?

It's unfortunately not documented yet (@orlitzky, am I right about this?), and is as experimental as this conda installation method.

Here I am using it so that the version constraints of those Python packages with spkg-configure are checked. Extra checks = good.

How long this command is does not really matter -- it is to be copy-pasted anyway. In any case, inventing new switches for conda is beyond the scope of this PR.

Is it also changing how the venv is setup/used?

Here I am using it so that the version constraints of those Python packages with spkg-configure are checked. Extra checks = good.

I don't see the point in that. We have ci checks already in place and the lock files I proposed would make it even more unnecessary.

Is it also changing how the venv is setup/used?

Note that in these instructions, no venv is created.

An important point of running the configure script with the "force" options is to check that the installed versions are suitable for Sage. It is of course just a partial check because it is only done for those package for which we have the spkg-configure.m4. Now thanks to @orlitzky's work, we can check more, so here I'm adding it. It's a natural improvement.

The configuration step won't go away. This has nothing to do with using or not using the Sage build system. (We aren't using it in this mode, as you know.)

Concerning 1), I'm right in the assumption that this is not relevant here as we don't create a venv when used with conda.

I can provide a better answer to this now.

Right: it's not directly relevant to the conda installation. But it does provide a sanity check on the compatibility of the versions used by conda and sage. In the long run, that will help conda.

We are currently adding spkg-configure.m4 files for all of sage's python packages, testing the version bounds in install-requires.txt, adding compatibility patches, etc. When all that is done, and when enough people have tested it, my hope is that we can turn --enable-system-site-packages on by default. If that happens, using conda to install dependencies for sage becomes much simpler because you don't have to circumvent make and the venv.

For that to work smoothly, the conda versions all have to be acceptable to ./configure; otherwise, ./configure will either crash or want to install an SPKG. Adding --enable-system-site-packages now, while not having any direct effect, does flag those incompatibilities to us.

using conda to install dependencies for sage becomes much simpler because you don't have to circumvent make

It's actually a feature and not a bug that the conda install doesn't use the make interface. One of the longterm goals is to replace sages package management and build infrastructure with conda. Right now this is already working very well for python packages. You could remove every python package (but keep the auto generated conda env and setup.cfg files) and everything is still working. That's why I think this flag here is a step backwards (for the conda install, I'm sure it has other good usages).

I would propose we remove this flag from the docs for now, but keep it in the ci workflow. Then you do get the consistency checks, just that not everyone has to run them.

I don't think anyone is proposing to add the use of make to the all-conda installation method.

That was also not my point...

mkoeppe · 2023-09-28T00:09:19Z

I propose you split this PR in multiple small ones with a single purpose to make reviewing easier.

I don't think so. The changes are needed to restore the workflow from broken to working.

dimpase · 2023-09-28T19:31:55Z

can we only run ./bootstrap-conda on conda?

mkoeppe · 2023-09-28T19:52:11Z

No. Its output files are part of the configure tarball.

dimpase · 2023-09-28T21:59:13Z

there should be an option to bootstrap to build this tarball. We run thus mostly useless bunch of stuff a lot, during routine rebuilds.

orlitzky · 2023-09-29T13:30:26Z

I must be out of the loop. How do we avoid the virtualenv when using conda to provide only the dependencies for sage? I thought the process was, at a high level,

Install all sage dependencies into a conda environment
Install sage normally, but pointing ./configure to the conda environment

Where, in my head at least, that second phase still uses a venv.

build/pkgs/cachetools/distros/conda.txt

dimpase · 2023-09-29T14:43:14Z

I presume this https://doc.sagemath.org/html/en/installation/conda.html#using-conda-to-provide-all-dependencies-for-the-sage-library-experimental can be simplified now, with --enable-system-site-packages available. But that's for another ticket.

mkoeppe · 2023-09-29T17:46:11Z

I must be out of the loop. How do we avoid the virtualenv when using conda to provide only the dependencies for sage?

By telling users not to run make. That's the experimental method in section 3 ("Using conda to provide all dependencies") of https://doc.sagemath.org/html/en/installation/conda.html

I thought the process was, at a high level,

Install all sage dependencies into a conda environment

Install sage normally, but pointing ./configure to the conda environment

Where, in my head at least, that second phase still uses a venv.

Yes, that's the method of section 2 ("Using conda to provide system packages for the Sage distribution") of https://doc.sagemath.org/html/en/installation/conda.html

mkoeppe · 2023-09-29T17:51:20Z

I presume this https://doc.sagemath.org/html/en/installation/conda.html#using-conda-to-provide-all-dependencies-for-the-sage-library-experimental can be simplified now, with --enable-system-site-packages available.

This method is what we have been talking about here on the PR. I have added --enable-system-site-packages here (because it makes sense). It doesn't simplify anything, but it brings what ./configure says closer to what is actually going to happen. (./configure still says "... will be installed from SPKG" for a few packages that don't have spkg-configure or are not available on conda -- but we instruct the user to never type "make", so nothing will be installed from SPKG.)

orlitzky · 2023-09-29T18:07:28Z

Ah, thanks. Now that I know that make is missing, I can see that it's missing.

mkoeppe · 2023-09-30T01:24:18Z

I'm more concerned about the long solve/install time of the environment (see current run https://github.com/sagemath/sage/actions/runs/6358819359?pr=35593). Any help there would be very welcome.

mkoeppe · 2023-09-30T22:24:10Z

I've reduced the scope of the PR; it now no longer makes changes to the installation manual.

mkoeppe · 2023-09-30T22:26:08Z

I presume this https://doc.sagemath.org/html/en/installation/conda.html#using-conda-to-provide-all-dependencies-for-the-sage-library-experimental can be simplified now, with --enable-system-site-packages available.

This method is what we have been talking about here on the PR. I have added --enable-system-site-packages here (because it makes sense). It doesn't simplify anything, but it brings what ./configure says closer to what is actually going to happen. (./configure still says "... will be installed from SPKG" for a few packages that don't have spkg-configure or are not available on conda -- but we instruct the user to never type "make", so nothing will be installed from SPKG.)

In #36367 I simplify the installation instructions by moving the run of configure inside the installation of the sage-conf package.

tobiasdiez · 2023-10-02T18:57:00Z

This change did not go in a good direction, I've backed it out. "Setting up mamba in strict mode, then solving the environment", but only on macOS, seems a rather obscure workaround.

According to conda/conda-libmamba-solver#283, the issue we faced "is a known incompatibility between the libarchive packages published on defaults and conda-forge. The root cause is a problem in the CMake build configuration upstream (libarchive/libarchive#1857)." So forcing mamba and libarchive to be from conda-forge is a according to the official "recommendation [..] to make sure that libarchive and libmamba come from the same channel."

The version I just pushed removes mention of mamba (which https://github.com/marketplace/actions/setup-miniconda flags as not fully supported)

Please not, mamba will soon be the official solver in conda (in the slightly different form of https://github.com/conda/conda-libmamba-solver). As long as this conda-mamba solver is not supported by the gh action, I prefer to stay with mamba.

I'll implement a full solution for listing only top-level packages (unless there are version constraints) here.

Again, please not. This PR is (almost) ready to go, and it is needed to fix a) test failures that come from the setuptools upgrade (to be merged) and b) macos. The added package stuff is orthogonal to this.

mkoeppe · 2023-10-02T19:44:37Z

So forcing mamba and libarchive to be from conda-forge is a according to the official "recommendation [..] to make sure that libarchive and libmamba come from the same channel."

The version of 54cee7d was obviously not working well, with the solve times for macOS just for getting mamba = 30 mins.

mkoeppe · 2023-10-02T19:46:34Z

Please not, mamba will soon be the official solver in conda

Well, when this is done, there will certainly not be a need to force https://github.com/marketplace/actions/setup-miniconda to use it. The current version is already ready for that.

mkoeppe · 2023-10-02T19:47:24Z

This PR is (almost) ready to go

No worries, I'll have it ready later today.

dimpase

seems OK to me

tobiasdiez

I don't think this is ready yet. The build step on conda now fails, it still installs hatch and meson ect in the environment and there is no reason to not use mamba on Linux (I'm fine with using plain conda for now on macos as a temporary workaround).

In light of these, I would like to ask you Matthias to extract some parts to other PRs. For example the package updates for setup tools ect are very good to go in my opinion, so is the splitting by python version.

mkoeppe · 2023-10-05T01:14:34Z

Yes, this PR has gotten too big and unfocused. I've opened #36405 for the refactoring changes.

The changes for setuptools depend on #36123, which is yet to be merged, so that will be done here or on another partial PR

tobiasdiez · 2023-10-05T06:30:23Z

The changes for setuptools depend on #36123, which is yet to be merged, so that will be done here or on another partial PR

Would be nice to have it in as soon as possible as #36123 actually breaks a few doctests on conda without these updates.

mkoeppe · 2023-10-05T16:02:48Z

That's now #36411

isuruf · 2023-10-22T18:51:11Z

build/pkgs/patchelf/distros/conda.txt

@@ -1 +0,0 @@
-patchelf


patchelf is available on conda-forge, but only on Linux platforms

mkoeppe · 2023-11-25T19:35:40Z

Still some rough edges:

  /usr/share/miniconda3/envs/sage/bin/x86_64-conda-linux-gnu-cc -shared -Wl,--allow-shlib-undefined -Wl,-rpath,/usr/share/miniconda3/envs/sage/lib -Wl,-rpath-link,/usr/share/miniconda3/envs/sage/lib -L/usr/share/miniconda3/envs/sage/lib -Wl,--allow-shlib-undefined -Wl,-rpath,/usr/share/miniconda3/envs/sage/lib -Wl,-rpath-link,/usr/share/miniconda3/envs/sage/lib -L/usr/share/miniconda3/envs/sage/lib -Wl,-O2 -Wl,--sort-common -Wl,--as-needed -Wl,-z,relro -Wl,-z,now -Wl,--disable-new-dtags -Wl,--gc-sections -Wl,--allow-shlib-undefined -Wl,-rpath,/usr/share/miniconda3/envs/sage/lib -Wl,-rpath-link,/usr/share/miniconda3/envs/sage/lib -L/usr/share/miniconda3/envs/sage/lib -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /usr/share/miniconda3/envs/sage/include -DNDEBUG -D_FORTIFY_SOURCE=2 -O2 -isystem /usr/share/miniconda3/envs/sage/include /tmp/tmpi4cbpdyd.build-temp/sage/graphs/chrompoly.o -lgmp -o /tmp/tmpjgan9ucn.build-lib/sage/graphs/chrompoly.cpython-310-x86_64-linux-gnu.so -lpari
  sage/graphs/cliquer.c:1217:10: fatal error: cliquer/graph.h: No such file or directory
   1217 | #include "cliquer/graph.h"
        |          ^~~~~~~~~~~~~~~~~
  compilation terminated.

tobiasdiez · 2023-11-26T14:21:06Z

bootstrap-conda

    (
        echo >&4 "  - pip:"
        echo >&5 "  - pip:"
+        echo >&6 "  - pip:"


The standard environment file doesn't contain a pip section by design.

Well, if that's by design (whose?), then that's a severe bug in it, and this is the fix.

It's not there to check that all standard packages are installed via conda. If anything is a bug, then it's the presence of the pip section for optional and dev environments.

It's not there to check that all standard packages are installed via conda.

No, it is not being checked by anything. sagenb_export is a standard package, and so far it has just been omitted.

The bigger flaw is related to what we explained to you over in #36765 (comment):

Creating conda packages is a downstream activity. If we were to take the design idea that "all standard packages need to have a conda package" seriously, it would mean that we could never add a standard package in our capacity as the upstream project.

But neither is this current practice (neither with conda nor with any other downstream package repository), nor has such a change of policy been established or even discussed anywhere. And certainly this change cannot come into existence by gaslighting PR authors.

tobiasdiez · 2023-11-26T14:27:41Z

bootstrap-conda

    if [ -n "$PKG_SYSTEM_PACKAGES" ]; then
+        case "$PKG_SYSTEM_PACKAGES" in
+            *([-A-Za-z0-9_ ]))  # just a package name
+                UNCONSTRAINED_PACKAGES+=" $PKG_BASE"


I think it's a fragile design to use "unconstrained version" as an indicator for including or excluding a certain dependency in the environment. It would prevent us from adding a version constraint to non-direct dependencies (although I have to admit I don't even understand the purpose of having conda.txt files for such dependencies) and forces us to specify a version constraint for those packages that do should end up.
Going with Python's 'explicit is better than implicit' slogan, I think it would be better to have a certain indicator file that says 'I'm a direct dependency, so include me please'.

It would prevent us from adding a version constraint to non-direct dependencies

To the contrary. It does the right thing -- namely including the dependency with the version constraint in the environment file -- for the non-direct dependencies.

design to use "unconstrained version" as an indicator for including or excluding a certain dependency in the environment.

That's not what it does. Packages are excluded by the minimizer from the environment file when they are implied by dependence. But we never exclude packages that have their own version constraints, so that the version constraint is not lost.

There is no need for any of the version constraints you added in this PR (intrinsic need, as in "if you install an older version that it's known to fail"). Thus you use version constraints as a a signal to include the given package in the environment file. I think this is not a good use of version constraints.

Yes, there is an "intrinsic need" for these version constraints; they are the same version bounds that configure checks, each with good reasons.

It's just that "extrinsically" we have so far gotten away with not setting them -- simply because there is nothing that would force an older unsuitable version. But when users mix our environment with other conda packages, we can run into trouble. (And no, it's not better to wait for the bug report.)

Yes, I reuse this mechanism here for another purpose -- so that not another mechanism needs to be invented.

tobiasdiez · 2023-11-26T14:31:39Z

pkgs/sage-conf_conda/setup.py

        cmd = f"cd {SAGE_ROOT} && ./configure --enable-build-as-root --with-system-python3=force --disable-notebook --disable-sagelib --disable-sage_conf --disable-doc"
-        cmd += ' --with-python=$CONDA_PREFIX/bin/python --prefix="$CONDA_PREFIX"'
-        cmd += ' $(for pkg in $(PATH="build/bin:$PATH" build/bin/sage-package list :standard: --exclude rpy2 --has-file spkg-configure.m4 --has-file distros/conda.txt); do echo --with-system-$pkg=force; done)'
+        cmd += ' --with-python=$CONDA_PREFIX/bin/python --prefix="$CONDA_PREFIX" --enable-system-site-packages'


I still think it's to early for that change.

I just re-read the comments in the thread above from 2 months ago, I don't think there's anything that supports delaying it further.

It's still "experimental" right? Given that the conda workflow is not as stable right now as we want it to be, I don't think we should add any non-stable elements to it.

Building the Sage distribution with --enable-system-site-packages is experimental.
Running the configure script with it is safe.

build/pkgs/cachetools/distros/conda.txt

mkoeppe · 2023-11-27T04:27:08Z

I would also suggest a bit more restraint when relabeling as "needs work". None of your comments made identified a clear work item.

mkoeppe · 2023-12-22T06:07:26Z

Let's get this in.

github-actions · 2024-04-01T09:05:49Z

Documentation preview for this PR (built with commit a5e4c8f; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

… be eliminated by the bootstrap-conda environment minimizer

isuruf reviewed May 1, 2023

View reviewed changes

build/pkgs/pari_jupyter/distros/conda.txt Show resolved Hide resolved

tobiasdiez reviewed May 2, 2023

View reviewed changes

build/pkgs/mathics/distros/conda.txt Show resolved Hide resolved

isuruf reviewed May 2, 2023

View reviewed changes

.github/workflows/ci-conda.yml Outdated Show resolved Hide resolved

isuruf reviewed May 3, 2023

View reviewed changes

build/pkgs/jupymake/distros/conda.txt Show resolved Hide resolved

tobiasdiez added s: run conda ci Run the conda workflow on this PR. and removed s: run conda ci Run the conda workflow on this PR. labels May 3, 2023

tobiasdiez suggested changes Sep 27, 2023

View reviewed changes

tobiasdiez added s: needs work and removed s: needs review labels Sep 28, 2023

tobiasdiez reviewed Sep 29, 2023

View reviewed changes

build/pkgs/cachetools/distros/conda.txt Show resolved Hide resolved

dimpase approved these changes Oct 4, 2023

View reviewed changes

tobiasdiez suggested changes Oct 5, 2023

View reviewed changes

isuruf reviewed Oct 22, 2023

View reviewed changes

tobiasdiez suggested changes Nov 26, 2023

View reviewed changes

Matthias Koeppe added 15 commits July 25, 2024 03:51

Remove conda.txt for mathics, pari_jupyter

65277dc

build/pkgs/jupymake: Omit from src/environment-optional.yml

e18a7ab

build/pkgs/pari_jupyter: Restore conda.txt

addd6e4

build/pkgs/jupymake/distros/conda.txt: Fix up

346b16d

build/pkgs: Add some more conda.txt

d46bfa9

build/pkgs/jupyterlab_widgets/distros/conda.txt: Fix up

0da1ac2

build/pkgs/pycygwin: Do not install on conda

aafebf4

build/pkgs: Add missing conda.txt

3fb54ff

bootstrap-conda: Minimize environments

22ba07b

build/pkgs: Make boost_cropped an order-only dep (it's header-only)

fcb537d

bootstrap-conda: Add pip lines also in src/environment.yml

38c8c2c

build/pkgs/cliquer/distros/conda.txt: Add lower bound

db482e2

build/pkgs/{ecl,gc,libatomic_ops}: Add version bounds for conda

8b8c45e

pkgs/sage-conf_conda/setup.py: Do not use force for packages that may…

5af6f07

… be eliminated by the bootstrap-conda environment minimizer

pkgs/sage-conf_conda/setup.py: Use --enable-system-sitepackages

a5e4c8f

Uh oh!

Conversation

mkoeppe commented May 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📚 Description

📝 Checklist

⌛ Dependencies

Uh oh!

Uh oh!

mkoeppe commented May 2, 2023

Uh oh!

mkoeppe commented May 2, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkoeppe commented May 3, 2023

Uh oh!

mkoeppe commented May 9, 2023

Uh oh!

mkoeppe commented Sep 27, 2023

Uh oh!

tobiasdiez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkoeppe Sep 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkoeppe commented Sep 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dimpase commented Sep 28, 2023

Uh oh!

mkoeppe commented Sep 28, 2023

Uh oh!

dimpase commented Sep 28, 2023

Uh oh!

orlitzky commented Sep 29, 2023

Uh oh!

Uh oh!

dimpase commented Sep 29, 2023

Uh oh!

mkoeppe commented Sep 29, 2023

Uh oh!

mkoeppe commented Sep 29, 2023

Uh oh!

orlitzky commented Sep 29, 2023

Uh oh!

mkoeppe commented Sep 30, 2023

Uh oh!

mkoeppe commented Sep 30, 2023

Uh oh!

mkoeppe commented Sep 30, 2023

Uh oh!

tobiasdiez commented Oct 2, 2023

Uh oh!

mkoeppe commented Oct 2, 2023

Uh oh!

mkoeppe commented Oct 2, 2023

Uh oh!

mkoeppe commented May 1, 2023 •

edited

Loading

mkoeppe Sep 28, 2023 •

edited

Loading

mkoeppe commented Sep 28, 2023 •

edited

Loading