#29022 closed enhancement (wontfix)
Make sagelib installation directory more flexible by creating a module src/sage/env_config.py from src/sage/env_config.py.in, defining variables for use in sage.env
Reported by: | mkoeppe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | build | Keywords: | |
Cc: | fbissey, arojas, isuruf, embray, infinity0, gh-timokau, jdemeyer, dimpase, jhpalmieri | Merged in: | |
Authors: | Matthias Koeppe | Reviewers: | Dima Pasechnik, Erik Bray |
Report Upstream: | N/A | Work issues: | |
Branch: | u/mkoeppe/create_module_src_sage_env_config_py_from_src_sage_env_config_py_in__defining_variables_for_use_in_sage_env (Commits, GitHub, GitLab) | Commit: | 7a435448517b4cae9825943447d2eaf77757a4a6 |
Dependencies: | Stopgaps: |
Description (last modified by )
This is a follow-up from #28225 - "Allow sage to run in the absence of sage-env
". The present ticket also allows $SAGE_LOCAL/bin/python
to run and import sage.all
in the absence of the shell script sage-env
. (Getting a fully functional sage.all
is future work.)
This ticket also makes sagelib more independent from the environment variables set by src/bin/sage-env
(local/bin/sage-env
), and to remove assumptions regarding install locations of sagelib relative to $SAGE_LOCAL
.
- To support users who want to install an experimental version of sagelib in other install locations, such as in a user site packages directory.
- For making sagelib available in a user's venv, as in the following example:
Without this ticket:
$ sage -python -m venv --system-site-packages ~/personal-sage-venv/ $ source ~/personal-sage-venv/bin/activate (personal-sage-venv) $ python >>> import sage.env >>> sage.env.SAGE_LOCAL '/Users/mkoeppe/personal-sage-venv' # wrong >>> import sage.all RuntimeError: You must get the file local/bin/sage-maxima.lispWith this ticket:
>>> sage.env.SAGE_LOCAL '/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local' >>> import sage.all >>> sage.all.maxima('1') 1
- To support #29013/#27824: spkg-configure.m4 for python3
- To make it possible for distributions to use a stock
src/sage/env.py
(providing a customsrc/sage/env_conf.py
).
In a first step, we just set SAGE_LOCAL
and MAXIMA
(to the location of the maxima
binary), but also SAGE_ROOT
could be added (for developer conveniences such as sage.misc.edit_module; see also #25486); and later packages' spkg-configure
might be setting MATHJAX_DIR
etc.
In a follow-up ticket, src/sage/env_config.py
would actually be generated by src/setup.py
, not configure
.
Ticket #29038 provides an alternative implementation that uses an standalone installed Python module sage_conf
, rather than writing into src/sage
Related:
Change History (55)
comment:1 Changed 3 years ago by
Summary: | src/setup.py: Create module sage.env_config, which defines variables for use in sage.env → Create module src/sage/env_config.py from src/sage/env_config.py.in, defining variables for use in sage.env |
---|
comment:2 Changed 3 years ago by
Cc: | fbissey arojas isuruf embray infinity0 gh-timokau added |
---|---|
Description: | modified (diff) |
comment:3 Changed 3 years ago by
Branch: | → u/mkoeppe/create_module_src_sage_env_config_py_from_src_sage_env_config_py_in__defining_variables_for_use_in_sage_env |
---|
comment:4 Changed 3 years ago by
Authors: | → Matthias Koeppe |
---|---|
Cc: | jdemeyer added |
Commit: | → bba08ec0c8008f925643eaf56742115d83253e23 |
Status: | new → needs_review |
comment:5 follow-up: 6 Changed 3 years ago by
Interesting concept. But your branch won't achieve much since SAGE_LOCAL
is not None
. In fact the problem of the value of SAGE_LOCAL
is solved in most case by the current setting of os.path.abspath(sys.prefix)
. Did you mean to set SAGE_ROOT
?
comment:6 follow-ups: 8 14 Changed 3 years ago by
Replying to fbissey:
Interesting concept. But your branch won't achieve much since
SAGE_LOCAL
is notNone
. In fact the problem of the value ofSAGE_LOCAL
is solved in most case by the current setting ofos.path.abspath(sys.prefix)
.
1) With #29013, os.path.abspath(sys.prefix)
is the location of the venv, and not equal to SAGE_LOCAL
.
2) If one starts Sage's Python directly, without going through sage-env
, then the environment variable SAGE_LOCAL
is not set; and in this case it is retrieved from env_config.py
.
comment:8 follow-up: 9 Changed 3 years ago by
I misunderstood the way the code works in var
. I thought your value from env_config.py
wouldn't be used.
For your points
1) OK - I get it, that's a problem
2) I am perfectly happy with it being set as
os.path.abspath(sys.prefix)
ifsage-env
is not loaded as it is now. But your alternative is acceptable and of course solves your issue at (1).
I removed sage-env
completely from sage-on-gentoo a number of releases ago and it does work nicely with very little patching (I upstreamed most of it). There is only one place now where I have to replace a call to SAGE_ROOT
at runtime (in sage.misc.copying
). I'd say sage works at 99.99% out of the box without needing sage-env
, in particular you should be able to call it directly from python.
However
1) your venv work may introduce a few disparities
2) being able to override some default from a standardised configuration file instead of patching for your distro values is very appealing.
I don't particularly need SAGE_ROOT
to be set (in fact None
suits me just fine since it doesn't make much sense on a distro), it was just my misconception on what you were doing.
comment:9 follow-up: 10 Changed 3 years ago by
Replying to fbissey:
I don't particularly need
SAGE_ROOT
to be set (in factNone
suits me just fine since it doesn't make much sense on a distro), it was just my misconception on what you were doing.
Yes, I agree that SAGE_ROOT is not very important, but it's still used for developer convenience functions like those in sage.misc.edit_module
.
comment:10 Changed 3 years ago by
Replying to mkoeppe:
Replying to fbissey:
I don't particularly need
SAGE_ROOT
to be set (in factNone
suits me just fine since it doesn't make much sense on a distro), it was just my misconception on what you were doing.Yes, I agree that SAGE_ROOT is not very important, but it's still used for developer convenience functions like those in
sage.misc.edit_module
.
SAGE_ROOT
is mentioned in the documentation string for edit_devel
but is otherwise absent. The bit in documentation should have been amended at the same time as https://github.com/sagemath/sage/commit/c352b1d5920ce9ffaebada2fe172784b2ba3e3fb#diff-141c6e649507c15775d157f417db1e26 I guess.
Almost all of the SAGE_ROOT
stuff left pertains to packaging I think.
comment:12 Changed 3 years ago by
Replying to mkoeppe:
Note
SAGE_SRC
is defined throughSAGE_ROOT
.
var('SAGE_SRC', join(SAGE_ROOT, 'src'), SAGE_LIB)
I made sure of that :) - so if SAGE_ROOT
is None
, SAGE_SRC
is set to SAGE_LIB
. I guess you may prefer the SAGE_ROOT
based value. But on a distro it will give me a read only view of the code (which is the best you should hope for if you install stuff with your distro package manager).
comment:13 follow-up: 15 Changed 3 years ago by
I don't see why it needs to explicitly be a Python module. I think my proposal in #22652 is simpler, because the same file could be used both for configuring the shell environment, and can be read by the Python code.
To me this just seems like extra cruft.
comment:14 follow-up: 16 Changed 3 years ago by
Replying to mkoeppe:
Replying to fbissey:
Interesting concept. But your branch won't achieve much since
SAGE_LOCAL
is notNone
. In fact the problem of the value ofSAGE_LOCAL
is solved in most case by the current setting ofos.path.abspath(sys.prefix)
.1) With #29013,
os.path.abspath(sys.prefix)
is the location of the venv, and not equal toSAGE_LOCAL
.
sys.prefix
may not be, but even in a venv sysconfig.get_config_var('prefix')
will always give you the original install prefix for Python, which when using Sage's Python will be SAGE_LOCAL
.
comment:15 Changed 3 years ago by
Replying to embray:
I don't see why it needs to explicitly be a Python module. I think my proposal in #22652 is simpler, because the same file could be used both for configuring the shell environment, and can be read by the Python code.
To me this just seems like extra cruft.
For example, a file like this could be generated at build time by running something like:
$ (source src/bin/sage-env-config && env | grep '^SAGE_' | sort) SAGE_ARB_LIBRARY=arb SAGE_CONFIGURE_FLINT=--with-flint=/home/embray/src/sagemath/sage/local SAGE_CONFIGURE_GMP=--with-gmp=/home/embray/src/sagemath/sage/local SAGE_CONFIGURE_MPC=--with-mpc=/home/embray/src/sagemath/sage/local SAGE_CONFIGURE_MPFR=--with-mpfr=/home/embray/src/sagemath/sage/local SAGE_CONFIGURE_NTL=--with-ntl=/home/embray/src/sagemath/sage/local SAGE_CONFIGURE_PARI=--with-pari=/home/embray/src/sagemath/sage/local SAGE_CRTI_DIR= SAGE_FLINT_PREFIX=/home/embray/src/sagemath/sage/local SAGE_FREETYPE_PREFIX= SAGE_GLPK_PREFIX=/home/embray/src/sagemath/sage/local SAGE_GMP_INCLUDE=/home/embray/src/sagemath/sage/local/include SAGE_GMP_PREFIX=/home/embray/src/sagemath/sage/local SAGE_KEEP_BUILT_SPKGS=yes SAGE_LOCAL=/home/embray/src/sagemath/sage/local SAGE_MPC_PREFIX=/home/embray/src/sagemath/sage/local SAGE_MPFR_PREFIX=/home/embray/src/sagemath/sage/local SAGE_NTL_PREFIX=/home/embray/src/sagemath/sage/local SAGE_PARI_CFG=/home/embray/src/sagemath/sage/local/lib/pari/pari.cfg SAGE_PARI_PREFIX=/home/embray/src/sagemath/sage/local SAGE_PKG_CONFIG_PATH= SAGE_PYTHON_VERSION=2
It could be output to a .py module that's (optionally) imported by sage.env
, or just a ".env" file (a common extension being used in recent years for a file like this that specifies some environment variables for a project). Distros, if they wish, could write their own version of this file without patching.
comment:16 Changed 3 years ago by
Replying to embray:
Replying to mkoeppe:
Replying to fbissey:
Interesting concept. But your branch won't achieve much since
SAGE_LOCAL
is notNone
. In fact the problem of the value ofSAGE_LOCAL
is solved in most case by the current setting ofos.path.abspath(sys.prefix)
.1) With #29013,
os.path.abspath(sys.prefix)
is the location of the venv, and not equal toSAGE_LOCAL
.
sys.prefix
may not be, but even in a venvsysconfig.get_config_var('prefix')
will always give you the original install prefix for Python, which when using Sage's Python will beSAGE_LOCAL
.
Thanks for this useful info. The purpose of this ticket, however, is to get rid of assumptions like that, to enable use of system python, see #27824.
comment:17 Changed 3 years ago by
Yeah, I see what you're saying. Long term I think it would be great if we could get rid of "SAGE_LOCAL" altogether, though I'm not exactly sure what the alternative looks like either...
comment:18 Changed 3 years ago by
Cc: | dimpase added |
---|
comment:20 follow-up: 21 Changed 3 years ago by
Status: | needs_review → needs_info |
---|
Sorry, I'm just not convinced this is useful. At least not yet. It's not needed for setting SAGE_LOCAL. I do agree something like this might be useful for setting other paths related to Sage's dependencies, but I don't think that's necessarily something that should be output by a configure script either, but rather should be loaded from a config file that downstream packagers can modify without patching if need be.
To expand on that a bit, the sage-the-distribution configure script is not generally run by packagers. Instead, this should be handled at the level of the sagelib package itself.
comment:21 follow-up: 22 Changed 3 years ago by
Replying to embray:
I don't think that's necessarily something that should be output by a configure script either, but rather should be loaded from a config file that downstream packagers can modify without patching if need be.
Sage-the-distribution generates the file using configure script. Downstream packagers would generate the file by any means that they prefer.
comment:22 follow-ups: 24 39 Changed 3 years ago by
Replying to mkoeppe:
Replying to embray:
I don't think that's necessarily something that should be output by a configure script either, but rather should be loaded from a config file that downstream packagers can modify without patching if need be.
Sage-the-distribution generates the file using configure script. Downstream packagers would generate the file by any means that they prefer.
If that's the case, I don't think env_config.py.in
should even be directly in the sage package. But I also think this file should not go directly into the sage package at all. It would be better if it were external to the sage package's source tree; but it could still be generated and installed along with the package, but that would be a step handled by setup.py. I also still fail to see the point of making it an actual Python module.
comment:23 follow-up: 31 Changed 3 years ago by
I think what I also need to see here is a concrete use case...
comment:24 follow-up: 25 Changed 3 years ago by
Replying to embray:
Replying to mkoeppe:
Sage-the-distribution generates the file using configure script. Downstream packagers would generate the file by any means that they prefer.
If that's the case, I don't think
env_config.py.in
should even be directly in the sage package.
It's the autoconf standard to have templates right next to the file they are templating.
But I also think this file should not go directly into the sage package at all. It would be better if it were external to the sage package's source tree; but it could still be generated and installed along with the package, but that would be a step handled by setup.py.
I actually agree with that, as you can see in the ticket description: In a follow-up ticket, src/sage/env_config.py would actually be generated by src/setup.py, not configure.
I also still fail to see the point of making it an actual Python module.
It's the easiest way to import values into Python, without having to fight about the "correct" file format to use. toml, anyone?
comment:25 follow-up: 26 Changed 3 years ago by
Replying to mkoeppe:
Replying to embray:
Replying to mkoeppe:
Sage-the-distribution generates the file using configure script. Downstream packagers would generate the file by any means that they prefer.
If that's the case, I don't think
env_config.py.in
should even be directly in the sage package.It's the autoconf standard to have templates right next to the file they are templating.
But I also think this file should not go directly into the sage package at all. It would be better if it were external to the sage package's source tree; but it could still be generated and installed along with the package, but that would be a step handled by setup.py.
I actually agree with that, as you can see in the ticket description: In a follow-up ticket, src/sage/env_config.py would actually be generated by src/setup.py, not configure.
I think if we started with that, and put the .in file just directly under src/ instead of src/sage I'd be less concerned about this (I still don't see the need, but I wouldn't fight it as much).
I also still fail to see the point of making it an actual Python module.
It's the easiest way to import values into Python, without having to fight about the "correct" file format to use. toml, anyone?
I already suggested a ".env" format which is just one "KEY=VALUE" per line, and can also be sourced by shell scripts.
comment:26 Changed 3 years ago by
Replying to embray:
But I also think this file should not go directly into the sage package at all. It would be better if it were external to the sage package's source tree; but it could still be generated and installed along with the package, but that would be a step handled by setup.py.
I actually agree with that, as you can see in the ticket description: In a follow-up ticket, src/sage/env_config.py would actually be generated by src/setup.py, not configure.
I think if we started with that, and put the .in file just directly under src/ instead of src/sage I'd be less concerned about this
By making it a Python module in src/sage, it is installed by the standard facilities (setup.py
) that already work now, not in some unspecified future.
comment:27 Changed 3 years ago by
I note here that everything about our installation of non-Python components that reside in src/
is ad-hoc and we seem to have been unable to make progress on this in years. Relevant tickets:
- #21785 - Installation of SAGE_SRC/ext/ in SAGE_LOCAL/share/sage/ext/ should be done by setup.py, not build/make/Makefile
- #22655 - Support package_data-like of non-Python resource files in Python packages
- #21569 - Install src/bin/* scripts via setup.py (scripts, console_scripts)
That's why I prefer an actionable solution that uses a solid part - the installation of Python modules.
comment:28 Changed 3 years ago by
Description: | modified (diff) |
---|---|
Summary: | Create module src/sage/env_config.py from src/sage/env_config.py.in, defining variables for use in sage.env → Make sagelib installation directory more flexible by creating a module src/sage/env_config.py from src/sage/env_config.py.in, defining variables for use in sage.env |
I've expanded on the explanation of the goals of this ticket.
comment:29 Changed 3 years ago by
Description: | modified (diff) |
---|
comment:30 Changed 3 years ago by
Description: | modified (diff) |
---|
comment:31 Changed 3 years ago by
Replying to embray:
I think what I also need to see here is a concrete use case...
Take a look a the new ticket description please.
comment:32 Changed 3 years ago by
Status: | needs_info → needs_review |
---|
comment:33 Changed 3 years ago by
Description: | modified (diff) |
---|
comment:34 Changed 3 years ago by
Commit: | bba08ec0c8008f925643eaf56742115d83253e23 → 7a435448517b4cae9825943447d2eaf77757a4a6 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
7a43544 | src/sage/env.py, envconfig.py.in: Add MAXIMA; src/sage/interfaces/maxima.py: Use it
|
comment:35 Changed 3 years ago by
Description: | modified (diff) |
---|
comment:36 Changed 3 years ago by
Description: | modified (diff) |
---|
comment:37 Changed 3 years ago by
Cc: | jhpalmieri added |
---|
comment:38 Changed 3 years ago by
Description: | modified (diff) |
---|
comment:39 follow-up: 42 Changed 3 years ago by
Replying to embray:
Sage-the-distribution generates the file using configure script. Downstream packagers would generate the file by any means that they prefer.
If that's the case, I don't think
env_config.py.in
should even be directly in the sage package. But I also think this file should not go directly into the sage package at all. It would be better if it were external to the sage package's source tree; but it could still be generated and installed along with the package
In #29038 I now do exactly that. Thanks for the input.
comment:40 Changed 3 years ago by
Milestone: | sage-9.1 → sage-duplicate/invalid/wontfix |
---|
This ticket is superseded by #29038 - Python package sage_conf
: Provides optional configuration information for sagelib.
comment:41 Changed 3 years ago by
Reviewers: | → Dima Pasechnik, Erik Bray |
---|---|
Status: | needs_review → positive_review |
comment:42 follow-up: 44 Changed 3 years ago by
Status: | positive_review → needs_info |
---|
Replying to mkoeppe:
Replying to embray:
Sage-the-distribution generates the file using configure script. Downstream packagers would generate the file by any means that they prefer.
If that's the case, I don't think
env_config.py.in
should even be directly in the sage package. But I also think this file should not go directly into the sage package at all. It would be better if it were external to the sage package's source tree; but it could still be generated and installed along with the packageIn #29038 I now do exactly that. Thanks for the input.
I'm sorry, what you did in that ticket is not what I meant at all, and that's my fault for not being clearer and more explicit. What I had in mind here was this:
- I was unclear when I wrote "external to the sage package's source tree". What I meant was (relative to the git repository), still in
src/
, but not insrc/sage
.
- These settings (whether written by sage-the-distribution's configure script, or by hand by a packager) would be written by
src/setup.py
into a file that *is* installed in thesage
package.
An analogy I would use is numpy.__config__
. If you look at it, and Numpy's setup.py, you can see that it's generated and installed inside the numpy package. This is close to what you are trying to do, but just a different approach to how the file is installed.
I'm still -1 on it being a .py module, but if you feel strongly about that I'd take it as long as we can do the rest more like Numpy does.
comment:43 Changed 3 years ago by
numpy's configuration/setup is IMHO a good example of how NOT to do such things.
comment:44 follow-up: 48 Changed 3 years ago by
Replying to embray:
I'd take it as long as we can do the rest more like Numpy does.
Numpy's solution, expecting a configuration file in its to-be-installed sources, is outdated (and, by the way, untested by upstream) because it is incompatible with pip-installing from a source other than a local directory.
Let's continue the discussion in #29038.
comment:45 Changed 3 years ago by
Status: | needs_info → needs_review |
---|
This ticket can now be closed because superseded by #29038.
comment:46 Changed 3 years ago by
Status: | needs_review → positive_review |
---|
comment:47 Changed 3 years ago by
Resolution: | → wontfix |
---|---|
Status: | positive_review → closed |
comment:48 Changed 3 years ago by
Replying to mkoeppe:
Replying to embray:
I'd take it as long as we can do the rest more like Numpy does.
Numpy's solution, expecting a configuration file in its to-be-installed sources, is outdated (and, by the way, untested by upstream) because it is incompatible with pip-installing from a source other than a local directory.
Let's continue the discussion in #29038.
I dont think so; you could also pass the configuration by other means but it's just an example. I don't see how #29038 makes this any better. Now you have to have a whole external Python module to do the same thing.
comment:49 Changed 3 years ago by
wrapping up configuration into a Python package is more Pythonic than "other means". "A whole external Python" module is way cleaner than a bunch of text files (maybe sourceable from shell, maybe not) for a Python library.
comment:50 follow-ups: 51 53 Changed 3 years ago by
I need to do a write up about this. I guess I exchanged a bigger patch for a smaller one and an extra file. The issue with your python module is that it is not a proper package. I don't have a tarball for the source apart from sage's own source and it is not versioned. So as a distro packager I am looking at this as extra work if I want to package it as a separate python module because I have to do the packaging and versioning myself.
So right now I have a patch to look for sage.sage_conf in env.py, and I am putting sage_conf.py next to env.py. There may be something wrong with putting sage_cong.py.in in the source themselves. But replacing it by an half assed package is hardly ideal.
comment:51 Changed 3 years ago by
Replying to fbissey:
I need to do a write up about this. I guess I exchanged a bigger patch for a smaller one and an extra file. The issue with your python module is that it is not a proper package. I don't have a tarball for the source apart from sage's own source and it is not versioned. So as a distro packager I am looking at this as extra work if I want to package it as a separate python module because I have to do the packaging and versioning myself.
We can make ./bootstrap to generate a tarball of it in upstream/ just as it's done for configure package.
Will it help?
comment:52 Changed 3 years ago by
It'd possibly be an improvement if it is properly versioned. But it would still need to be pre-processed before it being usable. You are running away from the complexity by heaping a bit more :)
I can adapt with most of the stuff that you throw at me. I have been doing that for about 12 years now. But I am fairly much in agreement with Erik that a different design should have been chosen altogether. In fact it's been years that I have been expecting sage to get a setup.cfg - at the very least for optional packages (the new design for coinor/gurobi/cplex rocks by the way).
comment:53 Changed 3 years ago by
Replying to fbissey:
I need to do a write up about this. I guess I exchanged a bigger patch for a smaller one and an extra file. The issue with your python module is that it is not a proper package. I don't have a tarball for the source apart from sage's own source and it is not versioned. So as a distro packager I am looking at this as extra work if I want to package it as a separate python module because I have to do the packaging and versioning myself.
So right now I have a patch to look for sage.sage_conf in env.py, and I am putting sage_conf.py next to env.py. There may be something wrong with putting sage_cong.py.in in the source themselves. But replacing it by an half assed package is hardly ideal.
Are you commenting on the right ticket? #29038 was merged, not this one.
comment:54 Changed 3 years ago by
You are right I should have commented on the other ticket. I just took the conversation on this ticket when I found it first in my inbox this morning. That being said I have been thinking of doing a comment for a week so I just took the opportunity before postponing it and never writing it. Of course the order I read things in my inbox in the morning is also a big influence in this case. But if I had waited to read all my inbox before thinking "I should put my comment there" I may not have written it at all :(
New commits:
Create module src/sage/env_config.py from src/sage/env_config.py.in, defining SAGE_LOCAL