Opened 5 years ago

Last modified 3 months ago

#21707 needs_work task

Meta-ticket: Split sage-env into 5 to clean up sage configuration

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.5
Component: build Keywords: sd109
Cc: embray, jdemeyer, leif, fbissey, dimpase, isuruf, arojas, infinity0, gh-timokau, mjo, jhpalmieri Merged in:
Authors: Matthias Koeppe Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #29038, #29052 Stopgaps:

Status badges

Description (last modified by mkoeppe)

src/bin/sage-env (with generated configuration in src/bin/sage-env-config; installed in $SAGE_LOCAL/bin) is used in all of the following contexts:

  1. Sage-the-distribution while building spkgs,
  2. Sage-the-distribution for building sagelib,
  3. for environment variables needed to start Sage,
  4. as variables by Sage at runtime (through sage.env), and
  5. for environment variables affecting subprocesses of Python invoked by sagelib modules.

This ticket proposes to split the configuration according to these 5 contexts. This will make the installation of Sage more modular and flexible.

1./2. Create build/bin/sage-build-env(with generated configuration in build/bin/sage-build-env-config) for the build-time environment variables for sage-the-distribution (spkg and sagelib). It is not installed in $SAGE_LOCAL.

  1. In build/make/Makefile.in and the scripts generated by build/bin/sage-spkg, source build/bin/sage-build-env in addition to src/bin/sage-env. This is #29052.
  2. Gradually, we will move settings from src/bin/sage-env[-config] to build/bin/sage-build-env[-config] that are known to be only needed for 1./2. (Note that not all compiler-related environment variables can be moved exclusively to 1./2. – some need to be added to 5 as well for some some calls to compilers needed at runtime to support things like %cython (?) and sage.calculus.desolvers.desolve_mintides.) As well as users' use of pip to install additional Python packages!
  3. Eventually, we will remove the call to src/bin/sage-env from the build scripts.

2./3. Obtain sagelib's build-time configuration such as src/setup.py's library_dirs and runtime information (sage.env) from sage-config (introduced in #29038).

  1. In src/Makefile, poison the SAGE_LOCAL variable.
    • #29411: make sagelib a script package
    • #29779: pkg-config installed from SPKG pkgconf should not depend on environment variable SAGE_LOCAL
  2. Pass SAGE_PKGS (if still needed at all - see #28815 or #29705) via sage-config instead of from src/Makefile.

3./4./5. Using sage_conf (Python module and script sage-config, #29038), make sage.all fully functional when imported from a Python, without setting any environment variables (sage-env).

  1. Initially, phase out src/bin/sage-env-config by using sage-config instead.
    • #29384: Clean up src/bin/sage-env-config.in: Move logic to src/bin/sage-env, move non-environment configuration variables to sage_conf.py
    • #29825: Clean-up for src/bin/sage-env
  2. Set sage.env variables via the Python module sage_conf instead of relying on information from environment variables. Example: CYSIGNALS_CRASH_DAYS
  3. Set environment variables that are needed only by subprocesses invoked by sagelib in the environment of these subprocesses, rather than relying on them begin set in sage-env. This environment could be provided by a variable sage_conf.SAGE_SUBPROCESS_ENV and/or more invidual variables. Example: R_PROFILE
    • #30563: Use configuration variable MAXIMA instead of hardcoding "maxima"
    • #30818 Set environment for subprocesses invoked by Sage
  4. Finally, remove use of sage-env for all purposes except for sage -sh.

Aspects of downstream sage packaging:

  • The goal is to enable downstream sage packagers to use a whole unmodified src directory to build and install sagelib.
    • more precisely, to use the pip-installable source package created by build/pkgs/sagelib/spkg-src (#29950); see also #30036 (pip-installable sage)
  • Downstream packagers could provide their own "implementation" (one Python module sage_conf.py) of the configuration module defined by #29038, instead of patching sources. They would not necessarily use the reference implementation in build/pkgs/sage_conf, which is intended for use with sage-the-distribution.

Binary packages:

  • #31417 version of package sage_conf for relocatable binary distributions

pip-installable Sage

  • #29039 pip-installable version of package sage_conf - installs non-Python bits of the Sage distribution in ~/.sage/
  • #31396 relocatable wheel version of package sage_conf

More related tickets:

  • cleaning of src/bin as described in #21570, #21559.
  • #25486: Discover SAGE_SCRIPTS_DIR to make $SAGE_LOCAL/bin/sage work from any directory, in an environment without SAGE_* variables - this adds a SAGE_ROOT variable to sage-env-config
  • #30724: src/bin/sage-env, src/bin/sage-env-config.in: Remove PYTHON_FOR_VENV
  • #29133 META-META-TICKET: Build, packaging, testing improvements

Change History (58)

comment:1 Changed 5 years ago by fbissey

One comment. Currently sage_setup is installed because it needs to be installed to be doctested - like the rest of sage. But nothing in sage_setup is really needed at runtime and distro can choose not to install it. I don't in sage-on-gentoo. I tend to push for thing not needed at runtime to be moved in there. And that's where I would have put sage-build-env but I am not going to fight it going into build/bin although I'd like everything used by setup.py to be neatly under src.

comment:2 Changed 5 years ago by mkoeppe

  • Description modified (diff)

Thanks - I've updated the description

comment:3 Changed 5 years ago by mkoeppe

  • Description modified (diff)

comment:4 Changed 5 years ago by embray

I agree.

Consider a slight alternative: Rewrite sage-env as a Python script. This would give it much more flexibility and probably more legibility, including some command-line options for different environments (--build, especially).

The output would of course be the shell commands to actually set the variables. So rather than source sage-env one would eval $(sage-env). My one concern is that it would be slower, but ideally it's something that shouldn't happen frequently anyways, but rather only when not already in sage-env.

comment:5 Changed 5 years ago by fbissey

I have a tendency to mix up sage-env the script and sage.env.py, my comment is really an example of that. However a lot is common between the two.

comment:6 follow-up: Changed 5 years ago by embray

Right--they could be one in the same.

comment:7 in reply to: ↑ 6 Changed 5 years ago by fbissey

Replying to embray:

Right--they could be one in the same.

I have thought about it often. Of course that really means that you'll have to removing all the build time stuff. But in sage-on-gentoo where I ship a very simplified sage-env (in /etc as well) they are virtually doing the same thing, one in shell, the other in python.

comment:8 Changed 5 years ago by embray

In either case--whether making a single script, or two separate shell scripts, this would be useful for gentoo (and probably other distros as well) then right?

comment:9 Changed 5 years ago by fbissey

Probably, the current sage-env is a forest mostly dedicated to the build system. I just discard it and bring my own, it takes less space and maintenance than a mega patch.

comment:10 Changed 5 years ago by mkoeppe

Ideally, the configuration for sagelib runtime should take place in a generated Python file and not involve environment variables at all.

comment:11 Changed 5 years ago by embray

More likely parts of it would be static, and other parts could be generated, but yes, what I meant above in terms of rewriting sage-env in Python was that it would not (by default) involve environment variables at all.

But it's still useful for interacting with other tools, especially for build purposes, as well as dropping into ./sage -sh to be able to set environment variables, which is why I suggest a mode to output a list of variables that can be read from the shell.

comment:12 Changed 2 years ago by mkoeppe

  • Branch set to u/mkoeppe/split_sage_env_into_sage_build_env_and_sage_env

comment:13 Changed 2 years ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Cc dimpase added
  • Commit set to 9189d609fa8cc90d616cb7ea05be405ce196295e
  • Dependencies set to #25130
  • Milestone changed from sage-7.5 to sage-8.8
  • Status changed from new to needs_review

New commits:

7405dd5Move sage-dist-helpers from src/bin to build/bin
9189d60Split out build/bin/sage-build-env-config from sage-env-config

comment:14 Changed 2 years ago by embray

As-is this is going to conflict with tickets like #27825 that are adding still more environment variables needed at build-time to sage-env-config.

I'm completely in favor of this separation in principle though. The question is whether we should do this first and then update tickets like #27825 on top of it, or the other way around?

comment:15 Changed 2 years ago by dimpase

I really would like to first get our existing tickets (#27825, #27822) touching sage-env-config in.

comment:16 Changed 2 years ago by mkoeppe

The tickets would be trivial to rebase on top of this one.

comment:17 follow-up: Changed 2 years ago by mkoeppe

Probably one should introduce a command sage --buildsh, which would provide the larger environment.

comment:18 Changed 2 years ago by embray

For the sake of a bit more terseness I wonder if we could call it just sage-env-build.in and sage-env-build. The only reason for the -config in the other one is to distinguish it from plain sage-env.

Now that I look I'm not even entirely sure we need to separate sage-env-config from sage-env.

I think we could probably have a sage-env.in produce sage-env and get rid of the separate sage-env-config entirely, though it's possible there's some subtlety to that that I'm missing.

comment:19 Changed 2 years ago by embray

  • Branch changed from u/mkoeppe/split_sage_env_into_sage_build_env_and_sage_env to public/split_sage_env_into_sage_build_env_and_sage_env
  • Commit changed from 9189d609fa8cc90d616cb7ea05be405ce196295e to ff0712125a3d05500a1f77e62c371d695d1a9794

Went ahead and rebased on current develop so it could incorporate all the additional variables we've since added.

I didn't change the name of the script yet; I would still prefer to shorten it but if anyone disagrees we can leave it as-is.


New commits:

a34a9f1Move sage-dist-helpers from src/bin to build/bin
ff07121Split out build/bin/sage-build-env-config from sage-env-config

comment:20 Changed 2 years ago by fbissey

The only subtility to sage-env-config in my mind, is that you can touch it or even regenerate it without touching sage-env. But I am sure you don't need a separate file to achieve your objectives in either case.

comment:21 Changed 2 years ago by embray

No, probably not. I might try it, though we'll leave that as a separate exercise from this ticket.

comment:22 Changed 2 years ago by embray

  • Milestone changed from sage-8.8 to sage-8.9

Moving tickets from the Sage 8.8 milestone that have been actively worked on in the last six months to the next release milestone (optimistically).

comment:23 Changed 22 months ago by embray

  • Milestone changed from sage-8.9 to sage-9.1

Ticket retargeted after milestone closed

comment:24 Changed 22 months ago by mkoeppe

  • Status changed from needs_review to needs_work

comment:25 follow-up: Changed 22 months ago by mkoeppe

The run-time environment variables of sage could as well be configured by a Python module src/sage/env_config.py - see #29022.

comment:26 in reply to: ↑ 17 Changed 22 months ago by mkoeppe

Replying to mkoeppe:

Probably one should introduce a command sage --buildsh, which would provide the larger environment.

As an extension of this, there could be sage --buildsh SPKG, which would set up the environment in which spkg-install runs. This could be useful for testing new versions of packages.

comment:27 in reply to: ↑ 25 Changed 22 months ago by mkoeppe

Replying to mkoeppe:

The run-time environment variables of sage could as well be configured by a Python module src/sage/env_config.py - see #29022.

Or #29038 - Python package sage_conf: Provides optional configuration information for sagelib

comment:28 Changed 21 months ago by mkoeppe

  • Branch public/split_sage_env_into_sage_build_env_and_sage_env deleted
  • Cc isuruf arojas infinity0 gh-timokau added
  • Commit ff0712125a3d05500a1f77e62c371d695d1a9794 deleted
  • Dependencies changed from #25130 to #29038
  • Description modified (diff)
  • Status changed from needs_work to needs_info
  • Summary changed from Split sage-env into sage-build-env and sage-env to Split sage-env into 5
  • Type changed from enhancement to task

Hoping for input from people involved in packaging.

comment:29 Changed 21 months ago by mkoeppe

  • Status changed from needs_info to needs_review

comment:30 Changed 21 months ago by mkoeppe

  • Description modified (diff)

comment:31 Changed 21 months ago by mkoeppe

  • Description modified (diff)

comment:32 Changed 21 months ago by mkoeppe

  • Dependencies changed from #29038 to #29038, #29052

comment:33 Changed 21 months ago by mkoeppe

  • Summary changed from Split sage-env into 5 to Meta-ticket: Split sage-env into 5 to clean up sage configuration

comment:34 Changed 21 months ago by mkoeppe

  • Description modified (diff)

comment:35 Changed 21 months ago by mkoeppe

  • Description modified (diff)

comment:36 Changed 21 months ago by mkoeppe

  • Cc mjo added

comment:37 Changed 19 months ago by mkoeppe

  • Description modified (diff)
  • Status changed from needs_review to needs_work

comment:38 Changed 19 months ago by mkoeppe

  • Description modified (diff)

comment:39 Changed 18 months ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

comment:40 Changed 18 months ago by mkoeppe

  • Description modified (diff)

comment:41 Changed 18 months ago by mkoeppe

  • Cc jhpalmieri added

comment:42 Changed 17 months ago by mkoeppe

  • Keywords sd109 added

comment:43 Changed 17 months ago by mkoeppe

  • Description modified (diff)

comment:44 Changed 17 months ago by mkoeppe

  • Description modified (diff)

comment:45 Changed 16 months ago by mkoeppe

  • Description modified (diff)

comment:46 Changed 16 months ago by mkoeppe

  • Description modified (diff)

comment:47 Changed 13 months ago by mkoeppe

  • Description modified (diff)

comment:48 Changed 12 months ago by mkoeppe

  • Description modified (diff)

comment:49 Changed 12 months ago by gh-tobiasdiez

Not sure if this is the right place, but I would suggest to use the refactoring process to also convert the environment variables to configuration files.

The idea would be to have a toml file (or yaml or whatever you prefer) in some predefined folder (say src for the start). This file could be generated e.g by the configure script. This config file is then parsed and wrapped by a python class (natural candidate would be the current sage_conf.py). The python class also provides reasonable default values in case the config file doesn't exist.

This has several advantages in my opinion:

  • More transparent, as one sees on one glance which variables influence sage.
  • Easily editable by the user/developer.
  • Local (e.g. think about multiple virtual environments with different configs).
  • Type-safe, i.e, it is ensured when parsing the toml file that the types of the config variables is correct.
  • Easier for downstream package managers, since they only need to provide a proper config file.
  • (Higher security as one cannot inject arbitrary python code, which would be the case if sage_conf.py should be edited.)

What do you think?

comment:50 Changed 12 months ago by mkoeppe

sage_conf defines an interface already - and the plan is indeed to get rid of dependencies on environment variables using this interface.

How sage_conf obtains the configuration is orthogonal to the defined interface.

I am not interested in defining configuration files at the moment. There is just too much potential on wasting energy on discussions regarding the "best" config file format.

comment:51 Changed 12 months ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-9.3

comment:52 follow-ups: Changed 12 months ago by gh-tobiasdiez

Well, you are actually proposing to add a configuration file in the form of a python file (sage_conf.py). One could easily bypass an extensive discussion and follow the arguments of https://www.python.org/dev/peps/pep-0518/#overview-of-file-formats-considered.

But I agree that my suggestion concerning a config file is somewhat out of line with the goal of this ticket. I would still suggest to keep this in mind while designing the interface for sage_conf, so that it might be easy in the future to switch to a design based on a config file. In particular, I would replace the module wide variables (like VERSION) with methods in a config class (like Config.get_version) that can also provide suitable defaults and do slight post-processing.

comment:53 in reply to: ↑ 52 Changed 12 months ago by mkoeppe

Replying to gh-tobiasdiez:

Well, you are actually proposing to add a configuration file in the form of a python file (sage_conf.py).

Note that sage_conf.py is already in use since Sage 9.1

comment:54 in reply to: ↑ 52 Changed 12 months ago by mkoeppe

Replying to gh-tobiasdiez:

I would still suggest to keep this in mind while designing the interface for sage_conf, so that it might be easy in the future to switch to a design based on a config file. In particular, I would replace the module wide variables (like VERSION) with methods in a config class (like Config.get_version) that can also provide suitable defaults and do slight post-processing.

All of this discussion is certainly valid and I am sure that you (and others) have valuable insights there, but now is simply not the right time for it. To get the design right, one needs to know about objectives and constraints. We are in the process of expanding the ways of Sage installation/deployment into several directions, including pip-installability and modularization; and also binary packaging will need an overhaul.

comment:55 Changed 10 months ago by mkoeppe

  • Description modified (diff)

comment:56 Changed 8 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

comment:57 Changed 8 months ago by mkoeppe

  • Description modified (diff)

comment:58 Changed 3 months ago by mkoeppe

  • Milestone changed from sage-9.4 to sage-9.5

Setting a new milestone for this ticket based on a cursory review.

Note: See TracTickets for help on using tickets.