Opened 11 months ago
Closed 9 months ago
#32904 closed defect (fixed)
Add version constraints for sageconf, clean the sageconf sdist
Reported by:  Tobias Diez  Owned by:  

Priority:  major  Milestone:  sage9.5 
Component:  build  Keywords:  
Cc:  Matthias Köppe, John Palmieri, Dima Pasechnik, ghkliem  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Tobias Diez, Dima Pasechnik 
Report Upstream:  N/A  Work issues:  
Branch:  25cd86b (Commits, GitHub, GitLab)  Commit:  25cd86b788d68ea5b84d927735975f48d6fea906 
Dependencies:  #29039  Stopgaps: 
Description (last modified by )
Currently, if you try to install sagemathstandard using pip install ./src
(from SAGE_ROOT) then this fails with the following error
Installing build dependencies: started Installing build dependencies: finished with status 'done' Getting requirements to build wheel: started Getting requirements to build wheel: finished with status 'done' Preparing wheel metadata: started Preparing wheel metadata: finished with status 'error' Getting requirements to build wheel ... done Running command /home/tobias/sage/src/.venv/bin/python /home/tobias/sage/src/.venv/lib/python3.8/sitepackages/pip/_vendor/pep517/in_process/_in_process.py prepare_metadata_for_build_wheel /tmp/tmpj9q6knbr Exception while cythonizing source files: FileNotFoundError(2, 'No such file or directory') Building interpreters for fast_callable ************************************************************************ Traceback (most recent call last): File "/home/tobias/sage/src/.venv/lib/python3.8/sitepackages/pip/_vendor/pep517/in_process/_in_process.py", line 349, in <module> main() File "/home/tobias/sage/src/.venv/lib/python3.8/sitepackages/pip/_vendor/pep517/in_process/_in_process.py", line 331, in main json_out['return_val'] = hook(**hook_input['kwargs']) File "/home/tobias/sage/src/.venv/lib/python3.8/sitepackages/pip/_vendor/pep517/in_process/_in_process.py", line 151, in prepare_metadata_for_build_wheel return hook(metadata_directory, config_settings) File "/tmp/pipbuildenvmke9oai7/overlay/lib/python3.8/sitepackages/setuptools/build_meta.py", line 174, in prepare_metadata_for_build_wheel self.run_setup() File "/tmp/pipbuildenvmke9oai7/overlay/lib/python3.8/sitepackages/setuptools/build_meta.py", line 158, in run_setup exec(compile(code, __file__, 'exec'), locals()) File "setup.py", line 114, in <module> aliases=cython_aliases(), File "/tmp/pipreqbuildtz70fj41/sage/env.py", line 557, in cython_aliases ecl_cflags = subprocess.run([ECL_CONFIG, "cflags"], check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True).stdout.split() File "/usr/lib/python3.8/subprocess.py", line 493, in run with Popen(*popenargs, **kwargs) as process: File "/usr/lib/python3.8/subprocess.py", line 858, in __init__ self._execute_child(args, executable, preexec_fn, close_fds, File "/usr/lib/python3.8/subprocess.py", line 1704, in _execute_child raise child_exception_type(errno_num, err_msg, err_filename) FileNotFoundError: [Errno 2] No such file or directory: '/Users/mkoeppe/s/sage/sagerebasing/worktreedist/.tox/localmacos10.15nohomebrewpython3_xcode/local/bin/eclconfig'
In this ticket, we add version constraints for the buildtime and runtime dependencies on sage_conf
, and clean up the sdist of sage_conf
, in particular so that it no longer ship configuration artifacts from the machine where the sdist was built.
Change History (36)
comment:1 Changed 11 months ago by
Summary:  Installation of sagemathstandard with build isolution → Installation of sagemathstandard with build isolation 

comment:2 Changed 11 months ago by
comment:3 Changed 11 months ago by
The log in the ticket description is not very useful. Best to use pip install v v
so that sufficient detail is presented.
comment:4 Changed 11 months ago by
Status:  new → needs_info 

comment:5 Changed 11 months ago by
I looked a bit deeper into this, and the problem is that https://pypi.org/project/sageconf/9.4.post3/#files indeed contains your config (and is automatically used as there is no version specification in the pyproject file). But as far as I could see this is improved in the most recent prerelease.
But the main question remains, I believe: How to specify the correct build config for sage if you use build isolation (which is the preferred way of installing packages).
comment:6 Changed 11 months ago by
What's missing is certainly a version constraint for the sage_conf
dependency (both in pyproject.toml
and setup.cfg
). We should add this via build/pkgs/sage_conf/installrequires.txt
.
comment:7 followup: 13 Changed 11 months ago by
Once there is a proper version constraint, if a correct version of sage_conf
is already installed, then the isolated build environment should take sage_conf
from the existing installation instead of building a fresh one from the PyPI sdist.
comment:8 Changed 11 months ago by
Dependencies:  → #29039 

Looking at the 9.4.post3 tarball, yes, this sdist is not clean.
These files (all artifacts generated by configure
) should not be included:
sage_root/src/bin/sageenvconfig
sage_root/src/bin/sagesrcenvconfig
sage_root/build/bin/sagebuildenvconfig
sage_root/build/make/Makefileauto
sage_root/pkgs/sageconf/sage_conf.py
sage_root/pkgs/sageconf/bin/sageenvconfig
Also sage_root/src/bin
has a bunch of other scripts that don't belong there.
Some of this would just go away by running the sdist building in a clean environment (#32062 does this on GH Actions).
But it's best to fix it using MANIFEST.in
.
Let's use the present ticket to address this.
comment:9 Changed 11 months ago by
Branch:  → u/mkoeppe/installation_of_sagemath_standard_with_build_isolation 

comment:10 Changed 11 months ago by
Commit:  → b1301b172c7095ab94b95735ed7b240220213cc8 

Replying to ghtobiasdiez:
One option, recommended by https://setuptools.pypa.io/en/latest/deprecated/distutils/configfile.html?highlight=header#writingthesetupconfigurationfile, is
As long as that information is fairly simple—a list of directories to search for C header files or libraries, for example—then providing a configuration file, setup.cfg, for users to edit is a cheap and easy way to solicit it.
Yeah, no, these are ancient practices and we will not start with this
Last 10 new commits:
02505ca  pkgs/sageconf_pypi/MANIFEST.in: Update from #31396

aeca1a0  Makefile (pypisdists): Add sage_setup

cd3dce9  pkgs/sageconf_pypi/setup.py: Update directory of configured sage_conf.py

c695921  pkgs/sageconf/sage_conf.py.in (SAGE_SPKG_WHEELS): Update so it works if SAGE_VENV != SAGE_LOCAL

cebcda3  Merge tag '9.5.beta4' into t/29039/pip_installable_package_sage_bootstrap

0026892  Merge tag '9.5.beta5' into t/29039/pip_installable_package_sage_bootstrap

2a4a323  Merge tag '9.5.beta7' into t/29039/pip_installable_package_sage_bootstrap

e24090a  Merge #29039

5aba4a1  build/pkgs/sage_conf/installrequires.txt: Add version constraint

b1301b1  src/{pyproject.toml.m4, setup.cfg.m4}: Include version constraint for sageconf

comment:11 Changed 11 months ago by
Replying to ghtobiasdiez:
one cannot control only the version of the other build_requires packages to installed, but not where they are coming from.
Actually one can, at least in some use cases, by using noindex findlinks ...
.
See for example pkgs/sagemathstandard/tox.ini
.
This makes it hard to use sage_conf as a build_requires dependency, since it will be usually fetched from pypi and thus doesn't take the users config into account.
Actually the version of sageconf on PyPI comes from pkgs/sageconf_pypi
(from #29039) and runs the configure
script upon installation.
comment:12 Changed 11 months ago by
Authors:  → Matthias Koeppe 

Status:  needs_info → needs_work 
Summary:  Installation of sagemathstandard with build isolation → Add version constraints for sageconf, clean the sageconf sdist 
comment:13 Changed 11 months ago by
Replying to mkoeppe:
Once there is a proper version constraint, if a correct version of
sage_conf
is already installed, then the isolated build environment should takesage_conf
from the existing installation instead of building a fresh one from the PyPI sdist.
I thought so too but it turns out that pip
specifies ignoreinstalled
when constructing the isolated build environment, see https://github.com/pypa/pip/issues/6482.
Thus it will most likely fetch pypi, or if you are lucky use the a locally cached wheel. So that doesn't seem to work.
The version constraint makes sense in either case in my opinion.
comment:14 Changed 11 months ago by
Commit:  b1301b172c7095ab94b95735ed7b240220213cc8 → 1dbe02358eec3b1f1f45e8e641d139f4d80ba3c7 

Branch pushed to git repo; I updated commit sha1. New commits:
1dbe023  pkgs/sageconf_pypi/sage_root: Reduce src/bin to the needed files

comment:15 Changed 11 months ago by
Commit:  1dbe02358eec3b1f1f45e8e641d139f4d80ba3c7 → 25cd86b788d68ea5b84d927735975f48d6fea906 

Branch pushed to git repo; I updated commit sha1. New commits:
25cd86b  pkgs/sageconf_pypi/MANIFEST.in: Exclude unneeded files

comment:16 Changed 11 months ago by
Status:  needs_work → needs_review 

comment:17 Changed 10 months ago by
Cc:  Dima Pasechnik ghkliem added 

comment:18 Changed 10 months ago by
After this patch, it ends up with
Successfully installed sagemathstandard9.5b7
What's the correct name of the module it installs? (Neither 
not .
are allowed in names, as far as I understand, and indeed importing the name above fails).
comment:19 followup: 22 Changed 10 months ago by
Read the documentation added in https://trac.sagemath.org/ticket/32899, it explains all of that
comment:20 Changed 10 months ago by
Does this really work for you with build isolation?
For me it still fetches the locallycached version or the one from pypi if the version of sage_conf
specified in src/pyproject.toml
is available there. That's also the expected behavior as I've outlined above in https://trac.sagemath.org/ticket/32904#comment:7
Thus adding the version constraint might work for production (i.e. external people downloading both sagemath and sageconf from pypi) but not for development. Moreover, if I understand the documentation correctly than sageconf from pypi is very expensive to install and needs to be done twice (once to be in the final venv and once to build sagemathstandard). Thus this is also not ideal.
comment:21 followup: 24 Changed 10 months ago by
Yes, installing the version from PyPI is expensive, as it is equivalent to building the Sage distribution from source. It is only done once, though, as it reuses
If you use noindex
, it will not fetch the version from PyPI.
If you combine it with findlinks=SAGE_VENV/var/lib/sage/wheels
, then it will take the prebuilt sage_conf
wheel from there, both for build and for install.
(Hence, #32913  Revert #32713 (Apply "configure enableeditable" to other packages) for sageconf, sagesetup... waiting for review)
comment:22 Changed 10 months ago by
Replying to mkoeppe:
Read the documentation added in https://trac.sagemath.org/ticket/32899, it explains all of that
I don't think it really answers my question  what is the name of the Python module being installed here?
comment:23 Changed 10 months ago by
It does:
 "A distribution that provides Python modules in the :mod:
sage.*
namespace, say mainly from :mod:sage.PAC.KAGE
, should be named sagemathDISTRIBUTION."  "Other distributions should not use the prefix sagemath in the distribution name."
Therefore, sagemathstandard provides modules in the sage.*
namespace.
Namely, all of the Sage library
comment:24 followups: 25 27 Changed 10 months ago by
Replying to mkoeppe:
Yes, installing the version from PyPI is expensive, as it is equivalent to building the Sage distribution from source. It is only done once, though, as it reuses
As I said above, pip specifies ignoreinstalled
while constructing the build environment and thus will build sageconf twice.
Please convince yourself if you don't believe me:
https://github.com/pypa/pip/blob/main/src/pip/_internal/build_env.py#L221
This is discussed in many issues on the pip repo, some examples: https://github.com/pypa/pip/issues/6482 https://github.com/pypa/pip/issues/9542 https://github.com/pypa/pip/issues/9439 All of these issues deal with (different) problems that come from taking "build isolation" as literally as pip is doing. The two major issues being that build and runtime dependencies are downloaded and build twice, and that there is no guarantee that build and runtime dependencies use the same version. It also seems that this is not something that will be fixed relatively soon.
In particular, the following will definitely build sageconf twice, especially with the changes in this ticket:
pip install sageconf==9.5 pip install sagemath==9.6
That may look like a silly example and no user would probably run this directly, but such a situation may happen quickly if you use packages that specify different but compatible versions of sagemath; or if you e.g. build sageconf yourself locally; or if you upgrade sagemath later (which would trigger another installation of sageconf). Also note that sagemath 9.6 will probably work well with sageconf 9.5 since the latter is after installation nothing more than a glorified configuration file and thus rarely changes (once the changes to the build system due to the modularization settle down).
Your suggestion to use noindex
or findlinks
works for development, but not for enduser that want to use the sagemath package from pypi.
The options I see are:
 Accept that sage doesn't work with build isolation (which would be unfortunate)
 Split
sageconf
into build and runtime configuration. There is still an overlap (and thus duplication) but at built time sage needs less dependencies than at runtime. In addition, maybe directly incorporate the buildtime sageconf in the sagemath package.  Merge sageconf in sagemath (I guess there was a reason to split it out in the first place; which?)
 Explore different options to pass the builttime configuration to sagemath, e.g. using a script that sets the correct env variables or writes an
env
file; usepackageconfig
as described in PEP 517 to set the configuration or point to a config file.
comment:25 followup: 30 Changed 10 months ago by
Replying to ghtobiasdiez:
Replying to mkoeppe:
Yes, installing the version from PyPI is expensive, as it is equivalent to building the Sage distribution from source. It is only done once, though, as it reuses
As I said above, pip specifies
ignoreinstalled
while constructing the build environment and thus will build sageconf twice.
It reuses SAGE_LOCAL
, which resides in a subdirectory of ~/.sage
, so it does NOT build the distribution twice.
comment:26 Changed 10 months ago by
With the present ticket, sagemathstandard
declares the same version constraints on sage_conf
for both build and install.
How strict this version constraint has to be will have to be seen as we gain more experience with it. Fixing it to the same version as sagemathstandard
will be safe but perhaps unnecessarily strict.
comment:27 Changed 10 months ago by
Replying to ghtobiasdiez:
 Merge sageconf in sagemath
That would be equivalent to just pinning the version of sage_conf
with ==
,
and is less flexible.
comment:28 Changed 10 months ago by
Description:  modified (diff) 

comment:29 Changed 10 months ago by
I have narrowed the ticket description to what is being done on the branch.
For more general discussion of how to improve configuration, let's use Metaticket #21707 ("Split sageenv
into 5 to clean up sage configuration")
comment:30 followups: 31 32 Changed 10 months ago by
Replying to mkoeppe:
Replying to ghtobiasdiez:
Replying to mkoeppe:
Yes, installing the version from PyPI is expensive, as it is equivalent to building the Sage distribution from source. It is only done once, though, as it reuses
As I said above, pip specifies
ignoreinstalled
while constructing the build environment and thus will build sageconf twice.It reuses
SAGE_LOCAL
, which resides in a subdirectory of~/.sage
, so it does NOT build the distribution twice.
So you mean that sageconf would be installed twice, but the second installation is considerably quicker since most of the built output is 'cached' in SAGE_LOCAL?
That's nice and probably takes care of the issue for enduser. For development, the workarounds of not using build isolation or noindex/findfiles are working.
The ticket generally looks good to me, especially with the narrowed focus. Two questions:
sageconf ~= 9.5.b6
shouldn't this be ==
to reduce the potential for issues in the future? Once a bit of experience is collected one may play around and see if less strict constraints are sufficient?
What's the purpose of the added symlinks?
comment:31 Changed 10 months ago by
Replying to ghtobiasdiez:
Replying to mkoeppe:
It reuses
SAGE_LOCAL
, which resides in a subdirectory of~/.sage
, so it does NOT build the distribution twice.So you mean that sageconf would be installed twice, but the second installation is considerably quicker since most of the built output is 'cached' in SAGE_LOCAL?
Yes, that's right.
comment:32 Changed 10 months ago by
Replying to ghtobiasdiez:
What's the purpose of the added symlinks?
The branch is replacing the symlink to src/bin
by separate symlinks to the scripts that it actually needs
comment:33 Changed 10 months ago by
Reviewers:  → Tobias Diez 

okay, thanks for the clarification.
LGTM. Dima, feel free to set it to positive review if it's working for you.
comment:34 Changed 10 months ago by
Reviewers:  Tobias Diez → Tobias Diez, Dima Pasechnik 

Status:  needs_review → positive_review 
OK
comment:36 Changed 9 months ago by
Branch:  u/mkoeppe/installation_of_sagemath_standard_with_build_isolation → 25cd86b788d68ea5b84d927735975f48d6fea906 

Resolution:  → fixed 
Status:  positive_review → closed 
Which wheel of
sage_conf
are you referring to? On PyPI there are only sdists, not wheels.