Opened 4 years ago
Closed 5 months ago
#22731 closed enhancement (fixed)
Replace "$SAGE_LOCAL/bin" by more specific variables to make Sage easier to package, use in venvs
Reported by:  thansen  Owned by:  

Priority:  major  Milestone:  sage9.3 
Component:  porting  Keywords:  
Cc:  mkoeppe, jhpalmieri, mjo, ghtobiasdiez, isuruf, arojas, ghmwageringel  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Tobias Diez, Dima Pasechnik 
Report Upstream:  N/A  Work issues:  
Branch:  38eebc3 (Commits, GitHub, GitLab)  Commit:  38eebc3d9f790c81b8ffcc963cf2513184c9ae6a 
Dependencies:  #29951  Stopgaps: 
Description (last modified by )
Various scripts refer to "$SAGE_LOCAL/bin" in a way that conflates several different uses. We introduce new variables to give packagers more flexibility without having to patch, and to enable new installation schemes, including the use of Sage in users' venvs.
1a. Introduce $SAGE_BIN
as the place where the binaries of all the SPKGs get installed to. Default is $SAGE_LOCAL/bin/
.
1b. (Wishlist item:) Make it configurable through ./configure bindir
, but this would need major changes to spkgconfigure scripts. (Supporting bindir
would be another step toward more autotools compliance, see Metaticket #21566.)
2a. Scripts such as src/bin/sage
, src/bin/sageeval
are installed by the sagelib
package using the scripts
facility of distutils/setuptools. If sagelib
is installed into a venv, these scripts are installed into the bin
directory of the venv. For the default installation in sagethedistribution withsystempython
, this venv is identical with SAGE_LOCAL
. But if other venvs are created by the user, it will be unrelated to SAGE_LOCAL
; in fact, SAGE_LOCAL
may be undefined (in distribution packaging and when installing sagelib through pip). As proposed in #29013, we use the variable SAGE_VENV
for this, so these scripts would be referred to as $SAGE_VENV/bin/sageeval
etc.
2b. Referring to the python3
used for sage. Likewise, this comes from the venv, not $SAGE_LOCAL/bin
. So it should be referred to as $SAGE_VENV/bin/python3
.
2c. The subset of the scripts in src/bin
, currently installed by the sagelib
package, that are "libexec"like (not userfacing). Distribution packagers would like to hide these scripts. (In the old description of this ticket, it was proposed to use $SAGE_SCRIPTS_DIR
for this, but a more specific name is needed: We will use $SAGE_VENV_SCRIPTS_DIR
.)
In this ticket, we introduce SAGE_VENV
for 2abc, ignoring the "libexec" aspect (using $SAGE_VENV/bin
directly instead of going through SAGE_VENV_SCRIPTS_DIR
). Items 1 and the libexec aspect of 2c can be done on followup tickets.
We also make sageenvconfig
optional from the viewpoint of the other scripts. This is preparation for #29850 / #29852.
See also:
 https://salsa.debian.org/scienceteam/sagemath//blob/master/debian/patches/u1scriptsdir.patch
 #30013
src/bin/sageenv
: Make sure$SAGE_VENV/bin
is at the beginning of the PATH  #29951
src/bin/sageenv
: MakeSAGE_ROOT
andSAGE_LOCAL
optional  #30578 Add
src/requirements.txt
for installation ofsagelib
in a venv
Change History (128)
comment:1 Changed 4 years ago by
 Branch set to u/thansen/allow_installation_of_sage_scripts_to_an_arbotrary_location_by_using_sage_scripts_dir_
comment:2 Changed 4 years ago by
 Commit set to 6ae9bf199cc52f40a59913222876bd6d98ffd011
 Component changed from PLEASE CHANGE to scripts
 Description modified (diff)
 Priority changed from major to minor
 Status changed from new to needs_review
 Summary changed from Allow installation of sage scripts to an arbotrary location by using SAGE_SCRIPTS_DIR. to Allow installation of sage scripts to an arbitrary location by using SAGE_SCRIPTS_DIR.
 Type changed from PLEASE CHANGE to enhancement
comment:3 Changed 4 years ago by
comment:4 Changed 4 years ago by
 Component changed from scripts to porting
 Status changed from needs_review to needs_work
The name SAGE_SCRIPTS_DIR
refers to the scripts of Sage itself. It is only meant to find sageenv
and friends, nothing more.
Now, if you want a invent a new variable with the purpose of defining the bindir (to replace SAGE_LOCAL/bin
), that's fine. Just don't call it SAGE_SCRIPTS_DIR
.
comment:5 Changed 3 years ago by
 Branch changed from u/thansen/allow_installation_of_sage_scripts_to_an_arbotrary_location_by_using_sage_scripts_dir_ to u/saraedum/allow_installation_of_sage_scripts_to_an_arbotrary_location_by_using_sage_scripts_dir_
comment:6 Changed 3 years ago by
 Commit changed from 6ae9bf199cc52f40a59913222876bd6d98ffd011 to 502f6abb14c4088bc6fe9cd9610400725af93c54
Branch pushed to git repo; I updated commit sha1. New commits:
502f6ab  Rename SAGE_SCRIPTS_DIR to SAGE_BIN in places where we are not looking for sageenv's friends

comment:7 Changed 3 years ago by
 Status changed from needs_work to needs_review
I have not had a chance to try this out locally (as my Sage is rebuilding.) Is this more or less what you had in mind jdemeyer?
comment:8 Changed 3 years ago by
 Commit changed from 502f6abb14c4088bc6fe9cd9610400725af93c54 to 7b6d51fce967cea3a2d9163561925bca251cf6a3
comment:9 Changed 3 years ago by
It makes no sense to add both $SAGE_LOCAL/bin
and $SAGE_BIN
to the PATH
. The whole point should be to replace $SAGE_LOCAL/bin
by $SAGE_BIN
.
comment:10 Changed 3 years ago by
 Commit changed from 7b6d51fce967cea3a2d9163561925bca251cf6a3 to b91dbcb58ff20256f37f3a73f9c97713240abcd9
Branch pushed to git repo; I updated commit sha1. New commits:
b91dbcb  Drop SAGE_LOCAL/bin from PATH

comment:11 Changed 3 years ago by
 Description modified (diff)
 Status changed from needs_review to needs_work
 Summary changed from Allow installation of sage scripts to an arbitrary location by using SAGE_SCRIPTS_DIR. to New environment variable SAGE_BIN
 You need to define
SAGE_BIN
somewhere insageenv
 This conflicts with #25056
comment:12 Changed 3 years ago by
 Commit changed from b91dbcb58ff20256f37f3a73f9c97713240abcd9 to 2d4fa67978250cd7729b3c6904c158b54b5f9f41
Branch pushed to git repo; I updated commit sha1. New commits:
2d4fa67  Set SAGE_BIN in sageenv

comment:13 followup: ↓ 16 Changed 3 years ago by
Thanks for the feedback. I do not really understand how these scripts interact. Did you mean something like this?
What is your point when it comes to the conflict with #25056. I guess whoever gets merged last needs to fix conflicts? Or is there something more substantial that needs to be taken into account?
comment:14 Changed 3 years ago by
 Status changed from needs_work to needs_review
comment:15 Changed 3 years ago by
 Work issues set to needs more testing
[my Sage is rebuilding as always, so I could not test this yet.]
comment:16 in reply to: ↑ 13 Changed 3 years ago by
 Dependencies set to #25056
 Status changed from needs_review to needs_work
 Work issues changed from needs more testing to rebase, test
Replying to saraedum:
What is your point when it comes to the conflict with #25056. I guess whoever gets merged last needs to fix conflicts? Or is there something more substantial that needs to be taken into account?
In this case, #25056 is a blocker so it will certainly take priority over this ticket. So it might be a good idea to rebase on top of #25056.
comment:17 Changed 3 years ago by
SAGE_BIN
should be set to thebindir
determined by./configure
. Usesrc/bin/sageenvconfig.in
to implement this.
 I don't think that it should be allowed to override
SAGE_BIN
at runtime, it should be set unconditionally to the value determined by./configure
. This would be analogous toSAGE_LOCAL
.
comment:18 Changed 3 years ago by
 Commit changed from 2d4fa67978250cd7729b3c6904c158b54b5f9f41 to 5b3d0541a77f9b69453b3f87e628172c7c402d2f
comment:19 Changed 3 years ago by
 Work issues changed from rebase, test to test with and without bindir, test relocation
comment:20 Changed 3 years ago by
comment:21 Changed 3 years ago by
comment:22 Changed 3 years ago by
Darn I had filled the wrong field when I wanted to "subscribe" to this ticket.
I am not hot to create a new variable especially for calling stuff that should by and large be in the PATH
. In my opinion, for sageondistro, it is even better if there is nothing in front of the name of the executables. It should just work unless you mess your environment and I am not sure we should protect the end users to that level.
comment:23 Changed 3 years ago by
fbissey: So you are saying that you want to have all the sage*
scripts in the PATH? I think it's reasonable for some distros that they want to hide these. I can imagine that one reason to get them off the path is that some distros want MAN pages for everything that's on the PATH. Also not all of these scripts have a nice help
/h
implemented.
comment:24 Changed 3 years ago by
I see. But it feels like it is more than just the sage* scripts that are touched here. In src/sage/graphs/lovasz_theta.py
this is not the path to a sage script but to an executable from an optional package for example. So you end up having to separate the scripts from the normal commands that are installed in SAGE_LOCAL/bin
. I don't see this ticket currently doing that.
comment:25 Changed 3 years ago by
You're right. I'll try to change the scope of SAGE_BIN
.
comment:26 Changed 3 years ago by
 Work issues changed from test with and without bindir, test relocation to test with and without bindir, test relocation, change scope of SAGE_BIN
comment:27 Changed 3 years ago by
 Commit changed from 5b3d0541a77f9b69453b3f87e628172c7c402d2f to 3224b7c15e0009b7b1bec0f9c883469b2d6ef73f
Branch pushed to git repo; I updated commit sha1. New commits:
3224b7c  Merge remotetracking branch 'trac/develop' into t/22731/allow_installation_of_sage_scripts_to_an_arbotrary_location_by_using_sage_scripts_dir_

comment:28 followup: ↓ 61 Changed 3 years ago by
 Dependencies changed from #25056 to #25056, #20382
 Status changed from needs_work to needs_info
I wonder whether we should do the following:
 Introduce
$SAGE_BIN
as the place wheresage
and the binaries of all the SPKGs get installed to. Configurable through./configure bindir
, default$SAGE_LOCAL/bin/
.  Make
$SAGE_SCRIPTS_DIR
available like any other$SAGE_
variable everywhere and use it as the directory where all thesage*
scripts are installed to. The default should be$SAGE_LOCAL/bin/
, however, it's configurable as./configure scriptsdir
.  The explicit calls such as
$SAGE_LOCAL/bin/theta
should go away with #20382. We should always call what's on the PATH when calling a nonSage binary (possibly after we checked that it's sane with a Feature check.)
Alternatively the second point could be replaced by the following two:
 Introduce
$SAGE_LIBEXEC
. Configurable through./configure libexecdir
, default$SAGE_LOCAL/libexec/
. (See https://www.gnu.org/prep/standards/html_node/DirectoryVariables.html.)  Make
$SAGE_SCRIPTS_DIR
available like any other$SAGE_
variable everywhere and use it as the directory where all thesage*
scripts are installed to. The default should be$SAGE_LIBEXEC/sage/
. To keep backwards compatibility, we need the default to be$SAGE_LOCAL/bin
if$SAGE_LOCAL/bin/sageenv
exists.
The $SAGE_SCRIPTS_DIR
fallback is ugly of course. It means that Sage installs vary depending on whether they are clean installs (after this proposal) or upgrades from before. That's unfortunate but we have that already because SPKGs do not uninstall completely. (Or do they by now?) Anyway, I think it makes sense to install things into standard places and I guess it could make the life of packagers easier.
What do you think?
comment:29 Changed 3 years ago by
Well, I don't use the configure
script but that sound relatively reasonable. I personally ship stuff like sageenv
in /etc
where I feel it belongs but that may be hard at this stage.
Uninstall is still being worked on by Erik, but it cannot be helped that there may be a hard block upgrading from a certain version of sage once those things are in place.
comment:30 Changed 3 years ago by
 Description modified (diff)
 Status changed from needs_info to needs_work
 Summary changed from New environment variable SAGE_BIN to Use SAGE_BIN an SAGE_SCRIPTS_DIR to make Sage easier to package
 Work issues test with and without bindir, test relocation, change scope of SAGE_BIN deleted
comment:31 Changed 3 years ago by
Thanks for the feedback. I think I prefer the nonbreaking change at the moment.
comment:32 Changed 3 years ago by
 Commit changed from 3224b7c15e0009b7b1bec0f9c883469b2d6ef73f to c897d2b9e3e2a7d4a2220af60250f01b9b419746
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
c897d2b  A first draft of using SAGE_BIN and SAGE_SCRIPTS_DIR differently

comment:33 Changed 3 years ago by
 Work issues set to use features of 20382
comment:34 Changed 3 years ago by
 Description modified (diff)
 Work issues use features of 20382 deleted
I think I want to the binaries differently if they are nonoptional dependencies. So I think this should not be part of this ticket.
comment:35 Changed 3 years ago by
 Commit changed from c897d2b9e3e2a7d4a2220af60250f01b9b419746 to 5e597441a4435e2033100611512113095834bdbb
Branch pushed to git repo; I updated commit sha1. New commits:
5e59744  Add configure switch for scriptsdir

comment:36 Changed 3 years ago by
 Status changed from needs_work to needs_review
 Work issues set to requires testing
comment:37 Changed 3 years ago by
This still needs quite some testing. I just want to see what the patchbot thinks about this.
comment:38 Changed 3 years ago by
 Summary changed from Use SAGE_BIN an SAGE_SCRIPTS_DIR to make Sage easier to package to Use SAGE_BIN and SAGE_SCRIPTS_DIR to make Sage easier to package
comment:39 Changed 3 years ago by
 Commit changed from 5e597441a4435e2033100611512113095834bdbb to 6b1ec2e573978815d770f0baff01bffb9ec7d2e4
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
be3a0f7  The docbuild crashes happen during an os.fork to spawn tachyon

7d85dc7  Something related to the sphinxbuild seems to be leaking memory

50898fe  Try to buildfromscratch on GitLab's underpowered CI machines

95c6275  Comment on fork logic in docbuilding

c0574ca  Merge remotetracking branch 'trac/develop' into u/saraedum/gitlabci

329fdab  fix tags for buildfromlatest

048e2d8  silence unicode warnings from the patchbot

f9c772e  Fix filtering logic in sphinx logger

f043163  Fixed some minor doctest issues

6b1ec2e  Merge remotetracking branch 'remotes/trac/u/saraedum/gitlabci' into sagescriptsdir

comment:40 Changed 3 years ago by
 Dependencies changed from #25056, #20382 to #25056, #20382, #24655
 Status changed from needs_review to needs_work
Ok, the patchbot does probably not help much here as it does not rebuild if something that fundamental changes. Let's see what CI thinks about it.
comment:41 Changed 3 years ago by
 Commit changed from 6b1ec2e573978815d770f0baff01bffb9ec7d2e4 to 795d0faef202cf603ceebddcff8ef4113dab8e2e
Branch pushed to git repo; I updated commit sha1. New commits:
8f3480e  It seems that MAKEOPTS/SAGE_NUM_THREADS parsing has been fixed now

99355ad  Build images in the same order in the Dockerfile and in builddocker.sh

81babb8  Simplify caching during docker build

ad39cc9  Complicate parallelization quite a bit

795d0fa  Merge remotetracking branch 'trac/u/saraedum/gitlabci' into HEAD

comment:42 Changed 3 years ago by
 Description modified (diff)
comment:43 Changed 3 years ago by
 Description modified (diff)
comment:44 Changed 3 years ago by
 Commit changed from 795d0faef202cf603ceebddcff8ef4113dab8e2e to 4de96c2cfd063680492a5e066d1ee508980871f4
Branch pushed to git repo; I updated commit sha1. New commits:
4de96c2  Merge remotetracking branch 'trac/develop' into t/22731/allow_installation_of_sage_scripts_to_an_arbotrary_location_by_using_sage_scripts_dir_

comment:45 Changed 3 years ago by
 Branch u/saraedum/allow_installation_of_sage_scripts_to_an_arbotrary_location_by_using_sage_scripts_dir_ deleted
 Commit 4de96c2cfd063680492a5e066d1ee508980871f4 deleted
comment:46 Changed 3 years ago by
 Branch set to u/saraedum/22731
comment:47 Changed 3 years ago by
 Commit set to 03c7aa4660e808f080f97a572b0b331c5bce4060
Branch pushed to git repo; I updated commit sha1. New commits:
03c7aa4  Merge remotetracking branch 'trac/develop' into 22731

comment:48 Changed 3 years ago by
 Description modified (diff)
comment:49 Changed 3 years ago by
 Commit changed from 03c7aa4660e808f080f97a572b0b331c5bce4060 to 7e37bedcb1d086aa5eb2e1516beec495db39c4c5
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
a532fce  Merge remotetracking branch 'trac/develop' into u/saraedum/gitlabci

686fe86  An attempt to use workflows on CircleCI

79003ab  Do not deduce CPUs/RAM by looking at the local machine

906ebea  Collect system information on the docker host

60ddb4b  simplify apk add command for GitLab CI

25f934e  Build from the latest develop image

aa49196  Backport https://trac.sagemath.org/ticket/24655 for automated docker builds

a0fdc3c  Merge commit 'aa49196c4f0ed33da7a45b80c7a4a01bd46b7d61' into u/saraedum/gitlabci

b704268  Merge branch '24655' into 24854

7e37bed  Merge branch '24854' into 22731

comment:50 Changed 3 years ago by
 Dependencies changed from #25056, #20382, #24655 to #25056, #20382, #24655, #24854
comment:51 Changed 3 years ago by
Would people on this thread have an idea on what can be done to make $SAGE_LOCAL/bin/sage
work when $SAGE_ROOT
is not yet set and/or how one can figure out if $SAGE_ROOT/sage
(a script that has more logic to figure out a value for $SAGE_ROOT
) is an acceptable executable to use?
It comes up in producing a jupyter kernelspec for sage that is suitable for use by an external jupyter. See: sagedevel thread
If we can make it very easy to install sage's kernelspec in arbitrary jupyter configurations, it becomes significantly easier to integrate sage in computational environments (for instance, should the nteract desktop jupyter client ever become popular, we could just provide easy interfacing with a system install of it rather than packaging it with sage).
comment:52 Changed 3 years ago by
 Description modified (diff)
comment:53 Changed 3 years ago by
 Description modified (diff)
comment:54 Changed 3 years ago by
 Commit changed from 7e37bedcb1d086aa5eb2e1516beec495db39c4c5 to 6d89d614dc1f363622e21b22e213e815b528425b
Branch pushed to git repo; I updated commit sha1. New commits:
11f8a6c  no port exposure needed for jupyter anymore

1960ba1  add openssl/libssldev to the docker images

14926cf  Revert "add openssl/libssldev to the docker images"

f2c641a  Add debug information on patch size

6d89d61  Merge branch '24655' into 22731

comment:55 Changed 3 years ago by
 Description modified (diff)
comment:56 Changed 3 years ago by
 Commit changed from 6d89d614dc1f363622e21b22e213e815b528425b to d7cb021121279a5d3645df7c1519dfff382ac033
Branch pushed to git repo; I updated commit sha1. New commits:
864843c  fix syntax error in CircleCI configuration

e1c8536  Run short and new doctests on CircleCI for every branch

45579a1  Call the right scripts in CircleCI

83f71e7  Don't pipe "echo" into "rm"

8d3caeb  Merge branch '24655' into 24854

d7cb021  Merge branch '24854' into 22731

comment:57 Changed 3 years ago by
 Status changed from needs_work to needs_review
The CI tests were successful (the failed tests are due to #25431)
comment:58 Changed 3 years ago by
 Dependencies changed from #25056, #20382, #24655, #24854 to #25056, #20382, #24655, #24854, #25431
comment:59 Changed 3 years ago by
 Commit changed from d7cb021121279a5d3645df7c1519dfff382ac033 to bed5af50d2560826420e23b716193c95672ffe3e
comment:60 Changed 3 years ago by
 Cc mkoeppe added
comment:61 in reply to: ↑ 28 Changed 3 years ago by
Replying to saraedum:
Alternatively the second point could be replaced by the following two:
 Introduce
$SAGE_LIBEXEC
. Configurable through./configure libexecdir
, default$SAGE_LOCAL/libexec/
. (See https://www.gnu.org/prep/standards/html_node/DirectoryVariables.html.)
+1 for using libexec for all auxiliary scripts that are not useful for the user from the command line.
comment:62 Changed 3 years ago by
It's hard to see the changes on this branch, by the way, since this is on top of some circleci branch; also needs rebasing.
comment:63 Changed 3 years ago by
 Commit changed from bed5af50d2560826420e23b716193c95672ffe3e to 7ba2ee2059f43222aabf3ce5de7a282f2687832d
Branch pushed to git repo; I updated commit sha1. New commits:
3c0d6a3  Merge branch '22731' into HEAD

d28f601  Make sage pip work

b8e6077  Do not install libssldev twice

7f44254  Merge remotetracking branch 'trac/develop' into 24655

91d5271  Merge branch '24655' into HEAD

b413748  Merge branch '24854' into HEAD

4ddd346  Backport #24655 to build docker images

ab71cfa  Merge branch 'docker8.3.beta3' into 24655

5086548  Merge branch '24655' into 24854

7ba2ee2  Merge branch '24854' into 22731

comment:64 Changed 3 years ago by
 Commit changed from 7ba2ee2059f43222aabf3ce5de7a282f2687832d to 2130060ef6ab1490368bd266ceaa3c190e2366a8
comment:65 Changed 3 years ago by
comment:66 Changed 3 years ago by
 Commit changed from 2130060ef6ab1490368bd266ceaa3c190e2366a8 to 08f93c738438eea285881837340a0211e71e8187
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
4ef4e18  Where to report bugs for the docker images

b8a94e2  Merge remotetracking branch 'trac/develop' into 24655

6a3d5d0  drop trailing space

f7ea0f3  Make updateenv posix compliant again

44e931c  Merge branch '24655' into 24854

e8dadc8  Merge remotetracking branch 'trac/develop' into 24655

5877f65  Merge branch '24655' into 24854

569d492  Merge remotetracking branch 'trac/develop' into 24655

e9c2cf8  Merge branch '24655' into 24854

08f93c7  Merge branch '24854' into HEAD

comment:67 Changed 3 years ago by
 Commit changed from 08f93c738438eea285881837340a0211e71e8187 to a9ca8a1e356eb6508036e9c986f04f2d97131428
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
57d653d  Merge branch '25270' into 24854

d29058e  Merge remotetracking branch 'trac/develop' into 24655

2cec8ac  Replace do tag with big

7bb3faf  Merge branch '24655' into 24854

6c9c2ce  Merge remotetracking branch 'trac/develop' into 25270

e38d7de  Merge branch '25270' into 24854

662d3f8  Use short to run short doctests

6241011  Fix namespacing for CircleCI

9f15655  Merge branch '24655' into 24854

a9ca8a1  Merge branch '24854' into 22731

comment:68 Changed 3 years ago by
 Commit changed from a9ca8a1e356eb6508036e9c986f04f2d97131428 to 9c9caef7b23b5782d230dd52aa9f06cb7aee96d9
comment:69 Changed 2 years ago by
 Status changed from needs_review to needs_work
 There is a merge conflicts.
 As I mentioned in 4, don't use
SAGE_SCRIPTS_DIR
.
comment:70 Changed 10 months ago by
 Milestone changed from sage8.0 to sage9.2
comment:71 Changed 10 months ago by
 Work issues changed from requires testing to clarify and prepare to support #30036
comment:72 Changed 10 months ago by
 Cc jhpalmieri mjo added
 Description modified (diff)
 Summary changed from Use SAGE_BIN and SAGE_SCRIPTS_DIR to make Sage easier to package to Replace "$SAGE_LOCAL/bin" by more specific variables to make Sage easier to package
comment:73 Changed 10 months ago by
 Work issues changed from clarify and prepare to support #30036 to clarify and prepare to support #30036, review/rebase changes
comment:74 Changed 10 months ago by
 Description modified (diff)
comment:75 Changed 9 months ago by
 Description modified (diff)
comment:76 Changed 9 months ago by
 Priority changed from minor to major
comment:77 Changed 8 months ago by
 Milestone changed from sage9.2 to sage9.3
comment:78 Changed 8 months ago by
 Cc ghtobiasdiez added
 Description modified (diff)
 Summary changed from Replace "$SAGE_LOCAL/bin" by more specific variables to make Sage easier to package to Replace "$SAGE_LOCAL/bin" by more specific variables to make Sage easier to package, use in venvs
comment:79 Changed 8 months ago by
 Description modified (diff)
 Work issues changed from clarify and prepare to support #30036, review/rebase changes to Implement as described
comment:80 Changed 8 months ago by
 Branch changed from u/saraedum/22731 to u/mkoeppe/22731
comment:81 Changed 8 months ago by
 Commit changed from 9c9caef7b23b5782d230dd52aa9f06cb7aee96d9 to b9dfd1a4cf1e789672f09a464757d50c2036b34a
 Dependencies #25056, #20382, #24655, #24854, #25431 deleted
comment:82 Changed 8 months ago by
 Description modified (diff)
Hoping that we can move this ticket forward next, to reduce the amount of necessary patching in distribution packaging.
comment:83 Changed 8 months ago by
 Description modified (diff)
comment:84 Changed 7 months ago by
 Dependencies set to #30818
comment:85 Changed 6 months ago by
 Cc isuruf added
 Dependencies changed from #30818 to #29850, #30818
comment:86 Changed 6 months ago by
 Commit changed from b9dfd1a4cf1e789672f09a464757d50c2036b34a to 545d118b9bc907340eca5342e380ce32fa72a923
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
9ba8cf2  Merge tag '9.3.beta0' into t/29951/src_bin_sage_env__make_sage_env_config_and_sage_local_optional

c8e6910  Merge tag '9.3.beta1' into t/29951/src_bin_sage_env__make_sage_env_config_and_sage_local_optional

9ab593c  Install sageenvconfig with sage_conf

a361580  build/pkgs/sage_conf/spkgsrc: New

d02b597  build/pkgs/sagelib/src/setup.py: Do not install sageenvconfig

1b4bc99  src/bin/sage: Only source sageenvconfig if it exists

a1a6df5  src/bin/sage: Use python3 etc. from PATH instead of using SAGE_LOCAL

d30b410  build/pkgs/sage_conf/spkginstall: Build wheel manually

19f7eae  src/bin/sageenv: Make sageenvconfig optional

545d118  sage.env: Add SAGE_VENV

comment:87 Changed 6 months ago by
 Commit changed from 545d118b9bc907340eca5342e380ce32fa72a923 to 6348c5c7c0eb2caba9f72f511847bea550b23709
Branch pushed to git repo; I updated commit sha1. New commits:
6348c5c  src/sage: Replace some SAGE_LOCAL by SAGE_VENV

comment:88 Changed 6 months ago by
 Status changed from needs_work to needs_review
 Work issues Implement as described deleted
comment:89 Changed 6 months ago by
 Dependencies changed from #29850, #30818 to #29850
comment:90 Changed 6 months ago by
 Commit changed from 6348c5c7c0eb2caba9f72f511847bea550b23709 to 122bf7890c17de88de973be8ac9c5165e9d47447
Branch pushed to git repo; I updated commit sha1. New commits:
ecde605  Merge branch 't/29850/install_sage_env_config_with_sage_conf' into t/30888/remove_unused_env_variable_sage_scripts_dir

a318e9e  src/bin/sageenv: Do not set SAGE_SCRIPTS_DIR

122bf78  Merge branch 't/30888/remove_unused_env_variable_sage_scripts_dir' into t/22731/22731

comment:91 Changed 6 months ago by
 Dependencies changed from #29850 to #29850, #30888
comment:92 Changed 6 months ago by
 Commit changed from 122bf7890c17de88de973be8ac9c5165e9d47447 to 6348c5c7c0eb2caba9f72f511847bea550b23709
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
comment:93 Changed 6 months ago by
 Dependencies changed from #29850, #30888 to #29850
comment:94 Changed 6 months ago by
 Commit changed from 6348c5c7c0eb2caba9f72f511847bea550b23709 to c585d94b95c5675f5a68690d13fa6a949642c48f
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
8f88709  src/bin/sage: Only source sageenvconfig if it exists

ab2655e  src/bin/sage: Use python3 etc. from PATH instead of using SAGE_LOCAL

4577f37  src/bin/sageenv: Make sageenvconfig optional

c35c170  sage.env: Add SAGE_VENV

c585d94  src/sage: Replace some SAGE_LOCAL by SAGE_VENV

comment:95 Changed 6 months ago by
 Dependencies changed from #29850 to #29951
 Description modified (diff)
Picked some changes from #29850, rebased away from it.
comment:96 Changed 6 months ago by
 Commit changed from c585d94b95c5675f5a68690d13fa6a949642c48f to 2fd195f5f044d125b0d5a743fec9d79e449126af
Branch pushed to git repo; I updated commit sha1. New commits:
2fd195f  src/bin/sage: Fix reference to trac ticket

comment:97 Changed 6 months ago by
 Description modified (diff)
comment:98 Changed 6 months ago by
Although the ticket description is long, the actual changes on top of #29951 are very small. Essentially, the sage
script now relies on the PATH
set by sageenv
instead of hardcoding the directory where python3
etc. is located. $SAGE_VENV/bin
is used explicitly only in a few places that need an absolute filename.
Ready for review
comment:99 followups: ↓ 101 ↓ 103 Changed 6 months ago by
 Reviewers set to Tobias Diez,
Codewise this looks good to me, but may I ask that you add a shortened version of the ticket description as comments in the env.py
file as documentation for the SAGE_VENV
and SAGE_LOCAL
variable as it's not clear from the context when to use which.
Moreover, I was wondering if it makes sense to provide only a variable SAGE_VENV_BIN
pointing to the binary folder in the venv (since at least from your ticket description and code changes would be sufficient and parallels the upcoming SAGE_BIN
).
Needs another review as I'm not yet that familiar with sages inner workings.
comment:100 Changed 6 months ago by
 Commit changed from 2fd195f5f044d125b0d5a743fec9d79e449126af to f3b7a9cce247457f07ef42069cdadb3f6409b008
Branch pushed to git repo; I updated commit sha1. New commits:
f3b7a9c  src/sage/env.py: Add documentation

comment:101 in reply to: ↑ 99 ; followup: ↓ 105 Changed 6 months ago by
Replying to ghtobiasdiez:
... may I ask that you add a shortened version of the ticket description as comments in the
env.py
file as documentation for theSAGE_VENV
andSAGE_LOCAL
variable
Done, please take a look
comment:102 Changed 6 months ago by
 Description modified (diff)
comment:103 in reply to: ↑ 99 Changed 6 months ago by
Replying to ghtobiasdiez:
I was wondering if it makes sense to provide only a variable
SAGE_VENV_BIN
pointing to the binary folder in the venv (since at least from your ticket description and code changes would be sufficient and parallels the upcomingSAGE_BIN
).
Sure, that will make sense to do in the followup ticket regarding SAGE_BIN
(if the downstream packagers still need that...)
comment:104 Changed 6 months ago by
 Reviewers changed from Tobias Diez, to Tobias Diez, ...
comment:105 in reply to: ↑ 101 Changed 6 months ago by
Replying to mkoeppe:
Replying to ghtobiasdiez:
... may I ask that you add a shortened version of the ticket description as comments in the
env.py
file as documentation for theSAGE_VENV
andSAGE_LOCAL
variableDone, please take a look
Thanks! Looks good.
comment:106 Changed 6 months ago by
 Cc arojas added
comment:107 Changed 6 months ago by
 Cc ghmwageringel added
comment:108 Changed 6 months ago by
running
$ grep SAGE_LOCAL/bin build/ R
returns quite a few hits, mostly in build/pkgs/
Shouldn't most of these be replaced with SAGE_BIN
?
comment:109 Changed 6 months ago by
See ticket description  "In this ticket, ..."
comment:110 Changed 6 months ago by
 Reviewers changed from Tobias Diez, ... to Tobias Diez, Dima Pasechnik
 Status changed from needs_review to positive_review
OK, fine
comment:111 Changed 6 months ago by
Thanks!
comment:112 Changed 6 months ago by
 Branch changed from u/mkoeppe/22731 to f3b7a9cce247457f07ef42069cdadb3f6409b008
 Resolution set to fixed
 Status changed from positive_review to closed
comment:113 Changed 6 months ago by
 Branch changed from f3b7a9cce247457f07ef42069cdadb3f6409b008 to u/mkoeppe/22731
 Resolution fixed deleted
 Status changed from closed to new
$ git clean f d x . $ ./bootstrap d rm rf config configure build/make/Makefileauto.in rm f src/doc/en/installation/*.txt rm rf src/doc/en/reference/spkg/*.rst rm f src/doc/en/reference/repl/*.txt src/doc/bootstrap:60: installing src/doc/en/installation/arch*.txt src/doc/bootstrap:60: installing src/doc/en/installation/debian*.txt src/doc/bootstrap:60: installing src/doc/en/installation/fedora*.txt src/doc/bootstrap:60: installing src/doc/en/installation/cygwin*.txt src/doc/bootstrap:60: installing src/doc/en/installation/homebrew*.txt src/doc/bootstrap:70: installing src/doc/en/reference/spkg/*.rst src/doc/bootstrap:102: installing src/doc/en/reference/repl/options.txt Error: You must set either the SAGE_LOCAL or SAGE_SCRIPTS_DIR environment variable to run this Error setting environment variables by sourcing '/Users/vbraun/Sage/src/bin/sageenv'; possibly contact sagedevel (see http://groups.google.com/group/sagedevel).
comment:114 followup: ↓ 115 Changed 6 months ago by
I noticed it as well in sageongentoo. I was trying to minimize the patch to bin/sage
but that got me. This is ultimately caused by the last line of src/doc/bootstrap
./sage advanced > "$OUTPUT"
Why is this line needed, I am wondering?
comment:115 in reply to: ↑ 114 Changed 6 months ago by
Replying to fbissey:
This is ultimately caused by the last line of
src/doc/bootstrap
./sage advanced > "$OUTPUT"Why is this line needed, I am wondering?
It generates part of the documentation.
comment:116 Changed 6 months ago by
Is at the error point the value of SAGE_SCRIPTS_DIR
already known?
comment:117 Changed 6 months ago by
I see how it is used now. And no, SAGE_SCRIPTS_DIR
or anything like that is not known, we are bootstrapping the documentation, which you should be able to do before running configure.
Some dummy values could be set in the bootstrap script on the assumption you do that on a clean tree.
comment:118 followup: ↓ 119 Changed 6 months ago by
The problem is solved in #30013, so I need to move some of the changes to this ticket.
comment:119 in reply to: ↑ 118 Changed 6 months ago by
comment:120 Changed 6 months ago by
I will work on this after the next beta comes out.
comment:121 Changed 6 months ago by
Anyhow, just in case, here one could do

bootstrap
a b SAGE_SPKG_CONFIGURE_$(echo ${pkgname}  tr '[az]' '[AZ]')" 113 113 # ONLY stderr, and to reoutput the results back to stderr leaving 114 114 # stdout alone. Basically we swap the two descriptors using a 115 115 # third, filter, and then swap them back. 116 touch src/bin/sageenvconfig 117 SAGE_SCRIPTS_DIR=./src/bin 116 118 BOOTSTRAP_QUIET="${BOOTSTRAP_QUIET}" \ 117 119 SAGE_ROOT="$SAGE_ROOT" \ 118 120 src/doc/bootstrap && \
and it all goes through.
comment:122 Changed 6 months ago by
 Commit changed from f3b7a9cce247457f07ef42069cdadb3f6409b008 to 1fe77a819d2477bf4b53d228388a4ff3e2c7c45d
Branch pushed to git repo; I updated commit sha1. New commits:
1fe77a8  src/bin/sageenv: Do not set SAGE_SCRIPTS_DIR

comment:123 Changed 6 months ago by
 Status changed from new to needs_review
Cherrypicked a commit from the original branch of #30888, where it was observed that
SAGE_SCRIPTS_DIR
is currently unused because the feature introduced in #25486 ("Discover SAGE_SCRIPTS_DIR
to make $SAGE_LOCAL/bin/sage
work from any directory, in an environment without SAGE_*
variables") was killed by #30128.
(#30888 will reintroduce this feature using a different mechanism.)
comment:124 Changed 6 months ago by
 Commit changed from 1fe77a819d2477bf4b53d228388a4ff3e2c7c45d to 38eebc3d9f790c81b8ffcc963cf2513184c9ae6a
Branch pushed to git repo; I updated commit sha1. New commits:
38eebc3  Merge tag '9.3.beta2' into t/22731/22731

comment:125 Changed 6 months ago by
Merged the latest beta. Ready for another round of review.
comment:126 Changed 6 months ago by
 Status changed from needs_review to positive_review
this works, after "git clean" as above, too.
comment:127 Changed 6 months ago by
Thanks!
comment:128 Changed 5 months ago by
 Branch changed from u/mkoeppe/22731 to 38eebc3d9f790c81b8ffcc963cf2513184c9ae6a
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Use SAGE_SCRIPTS_DIR in more places and define a fallback in env.py.