#29038 closed enhancement (fixed)
Python package sage_conf: Provides optional configuration information for sagelib
Reported by: | mkoeppe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.1 |
Component: | build | Keywords: | |
Cc: | fbissey, arojas, isuruf, embray, infinity0, gh-timokau, jdemeyer, dimpase, jhpalmieri, vbraun | Merged in: | |
Authors: | Matthias Koeppe | Reviewers: | Dima Pasechnik, Erik Bray |
Report Upstream: | N/A | Work issues: | |
Branch: | af18a01 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
This ticket introduces a new Python package sage_conf
.
- sage-the-distribution will generate this Python package at
./configure
time and install it at build time before starting to build/install sagelib usingsrc/setup.py
. - Distributions will generate and install their own
sage_conf
by a method of their choice before starting to build/install sagelib.
A console_script
allows to query individual variable values from the shell, or output all variables in .env format:
$ local/bin/sage-config MAXIMA /Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/bin/maxima $ local/bin/sage-config VERSION=9.1.beta0 MAXIMA=/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local/bin/maxima SAGE_LOCAL=/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring/local SAGE_ROOT=/Users/mkoeppe/s/sage/sage-rebasing/worktree-algebraic-2018-spring
The module and the script are used as follows:
- It provides configuration information to sagelib at installation time (
src/setup.py
).
Ultimately we want to be able to install sagelib with
pip
. To support installation bypip
from PyPI, from an URL, etc., we cannot expect to configure the build like we do now, by writing the configuration to the filesrc/sage-env-config
. Moreover, if pip is running in--isolated
mode, also no environment variables are passed. Then the only information flow is through arguments topip build
and through the installed Python packages. By installingsage_conf
first, we make the configuration available to the pip install of sagelib.
- It provides configuration information to the docbuild, in particular about the install locations of documentation of external packages.
- It provides configuration information to the runtime of sagelib, making sagelib more independent from the environment variables set by
src/bin/sage-env
(local/bin/sage-env
).
- By providing
SAGE_LOCAL
as one the configuration variables, it removes assumptions regarding install locations ofsagelib
relative to$SAGE_LOCAL
. This enables the following:
- Installing an experimental version of
sagelib
in other install locations, such as in a user site packages directory. - 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
- For a broader context: see #21707 Task: Split
sage-env
into 5
Change History (74)
comment:1 Changed 3 years ago by
- Description modified (diff)
comment:2 Changed 3 years ago by
- Branch set to u/mkoeppe/python_package_sage_conf__provides_optional_configuration_information_for_sagelib
comment:3 Changed 3 years ago by
- Cc fbissey arojas isuruf embray infinity0 gh-timokau jdemeyer dimpase jhpalmieri added
- Commit set to fe5c890e66a7dd37395720b6c8e08a3837e5a74d
- Status changed from new to needs_review
comment:4 Changed 3 years ago by
- Commit changed from fe5c890e66a7dd37395720b6c8e08a3837e5a74d to aefd827981481f015c696328402278f2f8d30912
Branch pushed to git repo; I updated commit sha1. New commits:
aefd827 | build/pkgs/sage_conf/spkg-install: Fix up path
|
comment:5 Changed 3 years ago by
why are you checking in files which apparently are meant to be generated from .in templates?
comment:6 Changed 3 years ago by
A mistake, which I will fix in a moment. Thanks!
comment:7 Changed 3 years ago by
- Commit changed from aefd827981481f015c696328402278f2f8d30912 to 88fd03c7784eb82eeb8cb06102ed4e93f5f1d758
comment:8 Changed 3 years ago by
- Description modified (diff)
comment:9 Changed 3 years ago by
- Description modified (diff)
comment:10 Changed 3 years ago by
- Description modified (diff)
comment:11 Changed 3 years ago by
- Description modified (diff)
comment:12 follow-up: ↓ 13 Changed 3 years ago by
the example with sage --python -m venv
- is it going to work if Sage is using venv on its own?
comment:13 in reply to: ↑ 12 Changed 3 years ago by
Replying to dimpase:
the example with
sage --python -m venv
- is it going to work if Sage is using venv on its own?
Not without installing sage packages into the new venv. As far as I can see, one can only control whether system-packages are made available but not whether the originating venv's packages are made available. But I'd expect that this option will be invented at some point in Python.
comment:14 Changed 3 years ago by
- Description modified (diff)
comment:15 follow-up: ↓ 16 Changed 3 years ago by
is sage_conf
also meant to be used while building Sage's dependencies?
comment:16 in reply to: ↑ 15 Changed 3 years ago by
comment:17 Changed 3 years ago by
author
ought to be either "The Sage Developers" (cf https://wiki.sagemath.org/Publications_using_SageMath?action=show&redirect=Publications_using_SAGE)
or yourself...
comment:18 follow-up: ↓ 19 Changed 3 years ago by
I copied the metadata from src/setup.py
comment:19 in reply to: ↑ 18 Changed 3 years ago by
Replying to mkoeppe:
I copied the metadata from
src/setup.py
that line is 5.5 years old. With all respect, William has not contributed a line of code to sagemath since 2017.
comment:20 Changed 3 years ago by
- Commit changed from 88fd03c7784eb82eeb8cb06102ed4e93f5f1d758 to f2863760bea085d285709ef848c53e6ce17a65fa
Branch pushed to git repo; I updated commit sha1. New commits:
f286376 | build/pkgs/sage_conf/src/setup.cfg.in: Change author to 'The Sage Developers'
|
comment:21 Changed 3 years ago by
- Description modified (diff)
comment:22 follow-up: ↓ 24 Changed 3 years ago by
I don't seem to get local/bin/sage-config
- it is not on the branch?
comment:23 Changed 3 years ago by
otherwise the branch builds and passes tests on a Debian system
comment:24 in reply to: ↑ 22 Changed 3 years ago by
Replying to dimpase:
I don't seem to get
local/bin/sage-config
- it is not on the branch?
Strange. Can you post the output of "sage -f sage_conf"?
comment:25 Changed 3 years ago by
- Commit changed from f2863760bea085d285709ef848c53e6ce17a65fa to a4205a776e9563a9d1db72541df7fef2d7a9f8c0
Branch pushed to git repo; I updated commit sha1. New commits:
a4205a7 | build/pkgs/sage_conf/src/setup.cfg.in: Use py_modules instead of packages
|
comment:26 Changed 3 years ago by
with the latest commit I get an error from sage -f sage_conf
make[1]: Entering directory '/home/dimpase/sage/build/make' cd '/home/dimpase/sage' && source '/home/dimpase/sage/src/bin/sage-env' && sage-logger -p '/home/dimpase/sage/build/pkgs/sage_conf/spkg-install' '/home/dimpase/sage/logs/pkgs/sage_conf.log' [sage_conf] /home/dimpase/sage/build/pkgs/sage_conf/spkg-install: 4: /home/dimpase/sage/build/pkgs/sage_conf/spkg-install: source: not found [sage_conf] Error: failed to source /home/dimpase/sage/build/bin/sage-dist-helpers [sage_conf] Is /home/dimpase/sage the correct SAGE_ROOT? make[1]: *** [Makefile:2240: sage_conf] Error 1 make[1]: Leaving directory '/home/dimpase/sage/build/make' real 0m0.137s user 0m0.076s sys 0m0.066s *************************************************************** Error building Sage. The following package(s) may have failed to build (not necessarily during this run of 'make sage_conf'): * package: sage_conf log file: /home/dimpase/sage/logs/pkgs/sage_conf.log build directory: /home/dimpase/sage/local/var/tmp/sage/build/sage_conf
comment:27 Changed 3 years ago by
simply due to Debian's /bin/sh not knowing what source
is:
$ /bin/sh $ source /bin/sh: 1: source: not found
comment:28 Changed 3 years ago by
I suppose the 1st line got to be
#!/usr/bin/env bash
Trying this now.
comment:29 Changed 3 years ago by
OK, with the latter fix, it completes, and also
$ ./local/bin/sage-config VERSION=9.1.beta0 MAXIMA=/home/dimpase/sage/local/bin/maxima SAGE_LOCAL=/home/dimpase/sage/local SAGE_ROOT=/home/dimpase/sage
works.
\begin{lament}
An annoying thing is that sage -f sage_conf
did cause ./configure
to be run like 4 or 5 times, I really lost count.
This is a by-product of sage -f/-i
changing the configuration.
See #27373.
I really think we should get rid of these snowflake sage -f/-i
and do ./configure && make
:-)
\end{lament}
comment:30 Changed 3 years ago by
$ sage -python -m venv --system-site-packages ~/personal-sage-venv/
also works as advertised by this ticket.
comment:31 Changed 3 years ago by
- Reviewers set to Dima Pasechnik
- Status changed from needs_review to needs_work
- Work issues set to use bash, not /bin/sh
comment:32 Changed 3 years ago by
- Commit changed from a4205a776e9563a9d1db72541df7fef2d7a9f8c0 to 747b05a5364d1e939533648dbae6dde9fbc75363
Branch pushed to git repo; I updated commit sha1. New commits:
747b05a | build/pkgs/sage_conf/spkg-install: Use bash, not sh
|
comment:33 Changed 3 years ago by
- Status changed from needs_work to needs_review
- Work issues use bash, not /bin/sh deleted
comment:35 Changed 3 years ago by
Thank you!
comment:36 follow-up: ↓ 58 Changed 3 years ago by
- Status changed from positive_review to needs_work
For the argument parsing please just use argparse.
comment:37 follow-up: ↓ 38 Changed 3 years ago by
I don't understand this line:
MAXIMA = "@prefix@/bin/maxima"
Why assume that's the location of maxima? It might be if using SAGE_LOCAL, but this should instead come from a path to maxima determined by configure.
comment:38 in reply to: ↑ 37 ; follow-ups: ↓ 40 ↓ 51 Changed 3 years ago by
Replying to embray:
I don't understand this line:
MAXIMA = "@prefix@/bin/maxima"Why assume that's the location of maxima? It might be if using SAGE_LOCAL, but this should instead come from a path to maxima determined by configure.
As long as we don't have spkg-configure.m4 for maxima, it's OK.
comment:39 follow-ups: ↓ 50 ↓ 55 Changed 3 years ago by
The idea of making this type=script
make sense to me, but that isn't used very often so it hasn't been well tested. If you look at what happens with a type=script package, it's installed like this:
# ============================= script packages ============================== # Generate build rules for 'script' packages; this template is used to generate # two rules in the form: # # <pkgname>: <dependencies> # $(AM_V_at)cd '$SAGE_ROOT' && \\ # source '$SAGE_ROOT/src/bin/sage-env' && \\ # sage-logger -p '$SAGE_ROOT/build/pkgs/<pkgname>/spkg-install' '$(SAGE_LOGS)/<pkgname>.log'
So it already sources sage-env
, but not sage-dist-helpers
.
Looking at the handful of other spkg-install
scripts for the type=script packages, they might not work properly anyways since they don't get the "wrapper" treatment that other spkg-install scripts now get from sage-spkg
(that didn't used to be the case.
I would make the splitting up of sage-env a prerequisite to this, and at a minimum ensure that sage-dist-helpers is already sourced in the build env. Then you could do away with the extra boilerplate at the top of the spkg-install
for this.
comment:40 in reply to: ↑ 38 Changed 3 years ago by
Replying to dimpase:
Replying to embray:
I don't understand this line:
MAXIMA = "@prefix@/bin/maxima"Why assume that's the location of maxima? It might be if using SAGE_LOCAL, but this should instead come from a path to maxima determined by configure.
As long as we don't have spkg-configure.m4 for maxima, it's OK.
Yes, I suppose, though this would have to be changed later if it were added.
comment:41 follow-up: ↓ 53 Changed 3 years ago by
In the setup.cfg
this should not be necessary:
+setup_requires = + setuptools
The standard now for specifying build-time dependencies is to add a pyproject.toml file.
comment:42 follow-ups: ↓ 44 ↓ 56 Changed 3 years ago by
Not to sound like a broken record, but I don't understand the purpose of making this a whole Python module instead of a plain text file. In some ways this makes things even worse for packagers, because now to package sage_config
they have to hand-patch this Python module.
All of the data provided by this module can go in a config file, similarly to Numpy's site.cfg
. It can even go directly in setup.cfg
under a Sage-specific section, but probably better a separate file that doesn't need to be patched.
If you want to install the data along with the sage package it can be added to package_data
. If you look at Sage's setup.py it already has one other package_data
file. Adding one more is not really a problem, issues like #22655 nonwithstanding. The file can be generated by the setup.py
, which is not that unusual a thing to do. If all this is to just avoid questions of how to update setup.py
, I can help with that.
comment:43 Changed 3 years ago by
This fix (adding the MAXIMA variable) is definitely good anything else in this ticket aside, and I'd approve right away as a separate ticket:
@@ -186,6 +192,7 @@ var('MTXLIB', join(SAGE_SHARE, 'meataxe')) var('THREEJS_DIR', join(SAGE_SHARE, 'threejs')) var('SINGULARPATH', join(SAGE_SHARE, 'singular')) var('PPLPY_DOCS', join(SAGE_SHARE, 'doc', 'pplpy')) +var('MAXIMA', 'maxima') var('MAXIMA_FAS') var('SAGE_NAUTY_BINS_PREFIX', '') diff --git a/src/sage/interfaces/maxima.py b/src/sage/interfaces/maxima.py index eacbc4c..d6d37fe 100644 --- a/src/sage/interfaces/maxima.py +++ b/src/sage/interfaces/maxima.py @@ -472,7 +472,7 @@ import pexpect from random import randrange -from sage.env import DOT_SAGE, SAGE_LOCAL +from sage.env import DOT_SAGE, SAGE_LOCAL, MAXIMA from sage.misc.misc import ECL_TMP from .expect import (Expect, ExpectElement, gc_disabled) @@ -546,7 +546,7 @@ class Maxima(MaximaAbstract, Expect): Expect.__init__(self, name = 'maxima', prompt = r'\(\%i[0-9]+\) ', - command = 'maxima -p "{0}"'.format(STARTUP), + command = '"{0}" -p "{1}"'.format(MAXIMA, STARTUP), env = {'TMPDIR': str(ECL_TMP)}, script_subdirectory = script_subdirectory, restart_on_ctrlc = False,
comment:44 in reply to: ↑ 42 ; follow-ups: ↓ 45 ↓ 47 Changed 3 years ago by
Replying to embray:
Not to sound like a broken record, but I don't understand the purpose of making this a whole Python module instead of a plain text file.
it is easier to make data available in Python if it is packaged as a Python module. Otherwise you need to read a plain text file from somewhere etc etc.
comment:45 in reply to: ↑ 44 ; follow-up: ↓ 46 Changed 3 years ago by
Replying to dimpase:
Replying to embray:
Not to sound like a broken record, but I don't understand the purpose of making this a whole Python module instead of a plain text file.
it is easier to make data available in Python if it is packaged as a Python module. Otherwise you need to read a plain text file from somewhere etc etc.
perhaps needless to say, but sysconfig
is a Python package, and your objections -- why it's not a text file -- apply to it just as well.
comment:46 in reply to: ↑ 45 Changed 3 years ago by
Replying to dimpase:
Replying to dimpase:
Replying to embray:
Not to sound like a broken record, but I don't understand the purpose of making this a whole Python module instead of a plain text file.
it is easier to make data available in Python if it is packaged as a Python module. Otherwise you need to read a plain text file from somewhere etc etc.
perhaps needless to say, but
sysconfig
is a Python package, and your objections -- why it's not a text file -- apply to it just as well.
Have you looked at the sysconfig module? None of it is generated. In fact, Python installs the Makefile it was built with, and sysconfig.py reads it. I'm not advocating that approach either, but something closer to it.
That said, having a generated Python file for this configuration is something I'd bend on. I don't think it needs to be a .py module but shrug. I added a comment on #29022 which I hope clarifies what I think this should be.
I do like that addition of a sage-config
script. That could just be part of the sage
package itself though; no need for an module external to sage
that I can see.
comment:47 in reply to: ↑ 44 Changed 3 years ago by
Replying to dimpase:
Replying to embray:
Not to sound like a broken record, but I don't understand the purpose of making this a whole Python module instead of a plain text file.
it is easier to make data available in Python if it is packaged as a Python module. Otherwise you need to read a plain text file from somewhere etc etc.
# sage/env.py ... SAGE_ENV_FILE = os.path.join(os.path.dirname(__file__), 'sage.env') with open(SAGE_ENV_FILE) as sage_env: for line in sage_env: key, value = line.split('=', 1) os.environ[key] = value ...
This format has the advantage that it can also be sourced by a shell script as it's valid shell syntax.
This is just a suggestion though; the .py equivalent is almost the same except with spaces around the =
.
Another version, if you require that the values are quoted (so that it's simultaneously valid shell syntax and Python syntax):
with open(SAGE_ENV_FILE) as sage_env: exec(sage_env.read(), os.environ)
Or, if we prefer not to override things in os.environ
we can use globals()
instead.
Point being, it's a few lines.
I like this because even if $SAGE_LOCAL
is not "activated" (sage-env
not sourced), you can run $SAGE_LOCAL/bin/python -c 'import sage'
and have a working environment. The necessary environment variables can then also be passed to Sage's external dependencies, without any requirement of modifying the outer shell environment. To me, that's the primary motivation. That motivation can still be achieved even if the generated file is a .py module, which is why I'm more open to that; I'm just trying to show that it doesn't gain all that much in simplicity either.
comment:48 Changed 3 years ago by
IMHO the internal design of sysconfig
is a technical detail - given Python's historical dislike of non-Python tooling, no wonder they do it their way.
comment:49 Changed 3 years ago by
globals()
, exec()
, etc are really measures of last resort.
And a plain text file is much more vulnerable to all sorts of problems, too, e.g. it's not checked for syntax by Python...
comment:50 in reply to: ↑ 39 Changed 3 years ago by
comment:51 in reply to: ↑ 38 Changed 3 years ago by
Replying to dimpase:
Replying to embray:
I don't understand this line:
MAXIMA = "@prefix@/bin/maxima"Why assume that's the location of maxima? It might be if using SAGE_LOCAL, but this should instead come from a path to maxima determined by configure.
As long as we don't have spkg-configure.m4 for maxima, it's OK.
Yes, of course the intention is that a follow-up ticket will add spkg-configure.m4
.
I picked this invocation of maxima
as an illustrating example for this ticket - one of many places where the Python code assumes that externally to the Python process, an environment has been set up. In the systematic of #21707 ("Split sage-env into 5"), this example belongs to category 5.
comment:52 Changed 3 years ago by
- Commit changed from 747b05a5364d1e939533648dbae6dde9fbc75363 to 5f7adc62326b63292f798fb7b6f276f49b2229d3
Branch pushed to git repo; I updated commit sha1. New commits:
5f7adc6 | build/pkgs/sage_conf/src/setup.cfg.in: Remove unnecessary setup_requires
|
comment:53 in reply to: ↑ 41 Changed 3 years ago by
Replying to embray:
In the
setup.cfg
this should not be necessary:+setup_requires = + setuptools
Thanks, I've removed it.
The standard now for specifying build-time dependencies is to add a pyproject.toml file.
Our present tooling does not need that file either. I'd suggest we do pyproject.toml business after #21508.
comment:54 Changed 3 years ago by
- Commit changed from 5f7adc62326b63292f798fb7b6f276f49b2229d3 to 8f8d8e3976d1c891b501e64ecdc094da65cf8239
Branch pushed to git repo; I updated commit sha1. New commits:
8f8d8e3 | build/pkgs/sage_conf/spkg-install: Don't source sage-env, as it is sourced already by the build/make/Makefile rule
|
comment:55 in reply to: ↑ 39 Changed 3 years ago by
Replying to embray:
The idea of making this
type=script
make sense to me, but that isn't used very often so it hasn't been well tested. If you look at what happens with a type=script package, [...] it already sourcessage-env
, but notsage-dist-helpers
.
Thanks! I've simplified spkg-install
accordingly.
comment:56 in reply to: ↑ 42 ; follow-up: ↓ 66 Changed 3 years ago by
Replying to embray:
In some ways this makes things even worse for packagers, because now to package
sage_config
they have to hand-patch this Python module.
No, to the contrary. This ticket defines a functional interface to configuration: to the shell, by sage-config
, to Python, by importing the module sage_conf
and reading its variables.
The version of sage_conf
provided in build/pkgs
is just a reference implementation of the functional interface that sage-the-distribution uses.
Packagers will just be able to generate a sage_conf
module in any way they like.
I have chosen this design (external to the src
directory, a Python module rather than a configuration file) for the following reasons:
- committing to a functional interface is more flexible than (1) committing to configuration by a static file written at installation time; (2) (less importantly) to a particular file format for that.
- for example, a binary distribution may decide to ship a version of
sage_conf
that is more dynamic: It could discover some settings at its runtime. - it is not part of the
src
directory; in this way it is working towards the goals of (a) distributors being able to use the whole unmodifiedsrc
tree; (b) pip installability of the unmodifiedsrc
tree.
comment:57 Changed 3 years ago by
- Commit changed from 8f8d8e3976d1c891b501e64ecdc094da65cf8239 to 7e34340291bce50f341a047f32f018ae97af6f08
Branch pushed to git repo; I updated commit sha1. New commits:
7e34340 | build/pkgs/sage_conf/src/sage_conf.py.in: Reimplement _main using argparse
|
comment:58 in reply to: ↑ 36 Changed 3 years ago by
Replying to embray:
For the argument parsing please just use argparse.
Thanks for the suggestion. Done.
comment:59 Changed 3 years ago by
- Status changed from needs_work to needs_review
comment:60 Changed 3 years ago by
- Reviewers changed from Dima Pasechnik to Dima Pasechnik, Erik Bray
comment:61 follow-up: ↓ 63 Changed 3 years ago by
from __future__ import print_function +del print_function
by the way, can you explain these 2 lines
comment:62 Changed 3 years ago by
- Commit changed from 7e34340291bce50f341a047f32f018ae97af6f08 to af18a01e25b70406cf5ea948f550493f065282ae
Branch pushed to git repo; I updated commit sha1. New commits:
af18a01 | Use sys.stdout.write instead of print
|
comment:63 in reply to: ↑ 61 Changed 3 years ago by
Replying to dimpase:
from __future__ import print_function +del print_functionby the way, can you explain these 2 lines
Without the del print_function
, sage-config
would print a line print_function=_Feature((2, 6, 0, 'alpha', 2), (3, 0, 0, 'alpha', 0), 65536)
.
But I have simplified the module by avoiding print
altogether.
comment:65 Changed 3 years ago by
- Branch changed from u/mkoeppe/python_package_sage_conf__provides_optional_configuration_information_for_sagelib to af18a01e25b70406cf5ea948f550493f065282ae
- Resolution set to fixed
- Status changed from positive_review to closed
comment:66 in reply to: ↑ 56 Changed 3 years ago by
- Commit af18a01e25b70406cf5ea948f550493f065282ae deleted
Replying to mkoeppe:
Replying to embray:
In some ways this makes things even worse for packagers, because now to package
sage_config
they have to hand-patch this Python module.No, to the contrary. This ticket defines a functional interface to configuration: to the shell, by
sage-config
, to Python, by importing the modulesage_conf
and reading its variables.The version of
sage_conf
provided inbuild/pkgs
is just a reference implementation of the functional interface that sage-the-distribution uses.Packagers will just be able to generate a
sage_conf
module in any way they like.I have chosen this design (external to the
src
directory, a Python module rather than a configuration file) for the following reasons:
- committing to a functional interface is more flexible than (1) committing to configuration by a static file written at installation time; (2) (less importantly) to a particular file format for that.
- for example, a binary distribution may decide to ship a version of
sage_conf
that is more dynamic: It could discover some settings at its runtime.- it is not part of the
src
directory; in this way it is working towards the goals of (a) distributors being able to use the whole unmodifiedsrc
tree; (b) pip installability of the unmodifiedsrc
tree.
But it's not even documented what this is supposed to provide or how it's supposed to work. This is all very confusing.
I don't really know what you mean by "functional" here. Do you mean executable? Why? Is there a specific reason for that? In any case I know of all that's needed here is at most a list of environment variables. In many cases this will just be static. If a packager has a need to generate some values for those variables programmatically they can do so however they like, but now they have to write a whole Python script that implements some undocumented interface.
comment:67 follow-up: ↓ 68 Changed 3 years ago by
Once again, it's more Pythonic to wrap Python-needed things into Python modules, as sysconfig
does, as opposed to keep stuff around in text files, which need to be read, parsed, etc etc. I guess that's what Matthias calls "functional".
comment:68 in reply to: ↑ 67 ; follow-up: ↓ 73 Changed 3 years ago by
Replying to dimpase:
Once again, it's more Pythonic to wrap Python-needed things into Python modules
![citation needed]
as
sysconfig
does, as opposed to keep stuff around in text files, which need to be read, parsed, etc etc. I guess that's what Matthias calls "functional".
Perhaps you missed it, but as I already explained above this is exactly what sysconfig
does. You are saying that sysconfig
is "unpythonic".
comment:69 follow-up: ↓ 71 Changed 3 years ago by
The larger plan is laid out in #21707 - Split sage-env into 5. It explains how pieces fit together. A recommended read.
comment:70 Changed 3 years ago by
For reference, again sysconfig, which is a hard-coded Python module which reads from a plain text-file.
Also recommended, the venv module which is actually a lot less mysterious than you guys seem to be making it out to be. The only magic involved is some support bits coded directly into the Python interpreter used for setting sys.prefix
correctly (this is one of the few ways venv
differs from the original virtualenv
which needed some hacks to achieve this).
comment:71 in reply to: ↑ 69 Changed 3 years ago by
Replying to mkoeppe:
The larger plan is laid out in #21707 - Split sage-env into 5. It explains how pieces fit together. A recommended read.
Thanks, that is useful. Obviously I've participated in that ticket before, but I wasn't sure if it was the ticket tying the rest of this together. I still believe that steps 3/4/5 can be achieved by different means which merit more discussion. But I'm on board with the overall plan :)
comment:72 follow-up: ↓ 74 Changed 3 years ago by
- Cc vbraun added
No problem. I know that it is not always possible to keep up with all developments when there are more important things to do.
As you see in that plan in #21707, the present ticket on sage_conf
was a step to cleaning up the sage-env mess.
It has little to do with my venv-python3 tickets (which I haven't had time to work on). (In fact, I now think that your version at #29032 may, after all, be the simpler way to go at it -- but I have to get back to the python3-spkg-configure business later.)
So there's no need to mix any "venv" discussion into the "sagelib configuration" discussion.
The "regression" caused by this ticket is one that is affecting developers who switch between new and old branches. It is fixed by just merging current develop into the old branch.
comment:73 in reply to: ↑ 68 Changed 3 years ago by
Replying to embray:
Replying to dimpase:
Once again, it's more Pythonic to wrap Python-needed things into Python modules
![citation needed]
cf. [sysconfig
].
as
sysconfig
does, as opposed to keep stuff around in text files, which need to be read, parsed, etc etc. I guess that's what Matthias calls "functional".Perhaps you missed it, but as I already explained above this is exactly what
sysconfig
does. You are saying thatsysconfig
is "unpythonic".
No, obviously Python needs to do some non-Pythonic things under the hood, but then the interface is Pythonic, and sysconfig
is an example of such an approach.
You are somehow against this approach, you say that such wrapping is too complicated, so it should not be done - if you were Python BDFL, you'd not have allowed sysconfig
in.
comment:74 in reply to: ↑ 72 Changed 3 years ago by
Replying to mkoeppe:
The "regression" caused by this ticket is one that is affecting developers who switch between new and old branches. It is fixed by just merging current develop into the old branch.
For tickets where merging the current develop
is not easy, I have prepared #29120 - One-line fix for "./configure is too sensitive to stray files/subdirectories".
New commits:
29038: Python package sage_conf: Provides configuration information for sagelib