Opened 3 years ago
Last modified 3 months ago
#21707 needs_review enhancement
Split sageenv into sagebuildenv and sageenv
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage8.9 
Component:  build  Keywords:  
Cc:  embray, jdemeyer, leif, fbissey, dimpase  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  public/split_sage_env_into_sage_build_env_and_sage_env (Commits)  Commit:  ff0712125a3d05500a1f77e62c371d695d1a9794 
Dependencies:  #25130  Stopgaps: 
Description (last modified by )
sageenv
is used by both Sagethedistribution while building packages,
and by Sage at runtime.
It should be split into two or three separate scripts.
sagebuildenv
is for the buildtime environment variables for sagethedistribution, should go intobuild/bin
and not be installed. Or perhaps more stuff should be put intobuild/make/install
sagelibbuildenv
is for the buildtime environment variables for sagelibsageenv
is for the runtime environment variables of sage and should be installed bysrc/setup.py
(thus very late in the build process), rather than bybuild/make/Makefile
. (sageenv
will be very short and at some point hopefully disappear, if we want sagelib to become a standalone Python library  #21507)
This is a step towards the cleaning of src/bin
as described in #21569, #21570, #21559.
Change History (22)
comment:1 Changed 3 years ago by
comment:3 Changed 3 years ago by
 Description modified (diff)
comment:4 Changed 3 years ago by
I agree.
Consider a slight alternative: Rewrite sageenv as a Python script. This would give it much more flexibility and probably more legibility, including some commandline options for different environments (build
, especially).
The output would of course be the shell commands to actually set the variables. So rather than source sageenv
one would eval $(sageenv)
. 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 sageenv.
comment:5 Changed 3 years ago by
I have a tendency to mix up sageenv
the script and sage.env.py
, my comment is really an example of that. However a lot is common between the two.
comment:6 followup: ↓ 7 Changed 3 years ago by
Rightthey could be one in the same.
comment:7 in reply to: ↑ 6 Changed 3 years ago by
Replying to embray:
Rightthey 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 sageongentoo where I ship a very simplified sageenv (in /etc as well) they are virtually doing the same thing, one in shell, the other in python.
comment:8 Changed 3 years ago by
In either casewhether 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 3 years ago by
Probably, the current sageenv
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 3 years ago by
Ideally, the configuration for sagelib runtime should take place in a generated Python file and not involve environment variables at all.
comment:11 Changed 3 years ago by
More likely parts of it would be static, and other parts could be generated, but yes, what I meant above in terms of rewriting sageenv
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 4 months ago by
 Branch set to u/mkoeppe/split_sage_env_into_sage_build_env_and_sage_env
comment:13 Changed 4 months ago by
 Cc dimpase added
 Commit set to 9189d609fa8cc90d616cb7ea05be405ce196295e
 Dependencies set to #25130
 Milestone changed from sage7.5 to sage8.8
 Status changed from new to needs_review
comment:14 Changed 4 months ago by
Asis this is going to conflict with tickets like #27825 that are adding still more environment variables needed at buildtime to sageenvconfig.
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 4 months ago by
comment:16 Changed 4 months ago by
The tickets would be trivial to rebase on top of this one.
comment:17 Changed 4 months ago by
Probably one should introduce a command sage buildsh
, which would provide the larger environment.
comment:18 Changed 3 months ago by
For the sake of a bit more terseness I wonder if we could call it just sageenvbuild.in
and sageenvbuild
. The only reason for the config
in the other one is to distinguish it from plain sageenv
.
Now that I look I'm not even entirely sure we need to separate sageenvconfig
from sageenv
.
I think we could probably have a sageenv.in
produce sageenv
and get rid of the separate sageenvconfig
entirely, though it's possible there's some subtlety to that that I'm missing.
comment:19 Changed 3 months ago by
 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 asis.
New commits:
a34a9f1  Move sagedisthelpers from src/bin to build/bin

ff07121  Split out build/bin/sagebuildenvconfig from sageenvconfig

comment:20 Changed 3 months ago by
The only subtility to sageenvconfig
in my mind, is that you can touch it or even regenerate it without touching sageenv
. But I am sure you don't need a separate file to achieve your objectives in either case.
comment:21 Changed 3 months ago by
No, probably not. I might try it, though we'll leave that as a separate exercise from this ticket.
comment:22 Changed 3 months ago by
 Milestone changed from sage8.8 to sage8.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).
One comment. Currently
sage_setup
is installed because it needs to be installed to be doctested  like the rest of sage. But nothing insage_setup
is really needed at runtime and distro can choose not to install it. I don't in sageongentoo. I tend to push for thing not needed at runtime to be moved in there. And that's where I would have putsagebuildenv
but I am not going to fight it going intobuild/bin
although I'd like everything used bysetup.py
to be neatly undersrc
.