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: sage-9.3
Component: porting Keywords:
Cc: mkoeppe, jhpalmieri, mjo, gh-tobiasdiez, isuruf, arojas, gh-mwageringel 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:

Status badges

Description (last modified by mkoeppe)

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 spkg-configure scripts. (Supporting --bindir would be another step toward more autotools compliance, see Meta-ticket #21566.)

2a. Scripts such as src/bin/sage, src/bin/sage-eval 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 sage-the-distribution --with-system-python, 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/sage-eval 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 user-facing). 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 follow-up tickets.

We also make sage-env-config optional from the viewpoint of the other scripts. This is preparation for #29850 / #29852.

See also:

Change History (128)

comment:1 Changed 4 years ago by thansen

  • 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 thansen

  • Authors set to Tobias Hanen
  • 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

New commits:

6ae9bf1Use SAGE_SCRIPTS_DIR in more places and define a fallback in env.py.

comment:3 Changed 4 years ago by thansen

  • Authors changed from Tobias Hanen to Tobias Hansen

comment:4 Changed 4 years ago by jdemeyer

  • 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 sage-env 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 saraedum

  • 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 git

  • Commit changed from 6ae9bf199cc52f40a59913222876bd6d98ffd011 to 502f6abb14c4088bc6fe9cd9610400725af93c54

Branch pushed to git repo; I updated commit sha1. New commits:

502f6abRename SAGE_SCRIPTS_DIR to SAGE_BIN in places where we are not looking for sage-env's friends

comment:7 Changed 3 years ago by saraedum

  • Authors changed from Tobias Hansen to Tobias Hansen, Julian Rüth
  • 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 git

  • Commit changed from 502f6abb14c4088bc6fe9cd9610400725af93c54 to 7b6d51fce967cea3a2d9163561925bca251cf6a3

Branch pushed to git repo; I updated commit sha1. New commits:

07649aaReplace more occurences of SAGE_LOCAL/bin with SAGE_BIN
7b6d51fFixed typos related to SAGE_BIN

comment:9 Changed 3 years ago by jdemeyer

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 git

  • Commit changed from 7b6d51fce967cea3a2d9163561925bca251cf6a3 to b91dbcb58ff20256f37f3a73f9c97713240abcd9

Branch pushed to git repo; I updated commit sha1. New commits:

b91dbcbDrop SAGE_LOCAL/bin from PATH

comment:11 Changed 3 years ago by jdemeyer

  • 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
  1. You need to define SAGE_BIN somewhere in sage-env
  1. This conflicts with #25056

comment:12 Changed 3 years ago by git

  • Commit changed from b91dbcb58ff20256f37f3a73f9c97713240abcd9 to 2d4fa67978250cd7729b3c6904c158b54b5f9f41

Branch pushed to git repo; I updated commit sha1. New commits:

2d4fa67Set SAGE_BIN in sage-env

comment:13 follow-up: Changed 3 years ago by saraedum

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 saraedum

  • Status changed from needs_work to needs_review

comment:15 Changed 3 years ago by saraedum

  • 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 jdemeyer

  • 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 jdemeyer

  1. SAGE_BIN should be set to the --bindir determined by ./configure. Use src/bin/sage-env-config.in to implement this.
  1. 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 to SAGE_LOCAL.

comment:18 Changed 3 years ago by git

  • Commit changed from 2d4fa67978250cd7729b3c6904c158b54b5f9f41 to 5b3d0541a77f9b69453b3f87e628172c7c402d2f

Branch pushed to git repo; I updated commit sha1. New commits:

542dbb6Do not allow resetting SAGE_BIN at runtime
5b3d054Fix another occurrence for SAGE_LOCAL/bin

comment:19 Changed 3 years ago by saraedum

  • Work issues changed from rebase, test to test with and without bindir, test relocation

comment:20 Changed 3 years ago by fbissey

  • Authors changed from Tobias Hansen, Julian Rüth to Tobias Hansen, Julian Rüth, fbissey

comment:21 Changed 3 years ago by saraedum

  • Authors changed from Tobias Hansen, Julian Rüth, fbissey to Tobias Hansen, Julian Rüth, François Bissey

comment:22 Changed 3 years ago by fbissey

  • Authors changed from Tobias Hansen, Julian Rüth, François Bissey to Tobias Hansen, Julian Rüth

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 sage-on-distro, 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 saraedum

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 fbissey

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 saraedum

You're right. I'll try to change the scope of SAGE_BIN.

comment:26 Changed 3 years ago by saraedum

  • 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 git

  • Commit changed from 5b3d0541a77f9b69453b3f87e628172c7c402d2f to 3224b7c15e0009b7b1bec0f9c883469b2d6ef73f

Branch pushed to git repo; I updated commit sha1. New commits:

3224b7cMerge remote-tracking branch 'trac/develop' into t/22731/allow_installation_of_sage_scripts_to_an_arbotrary_location_by_using_sage_scripts_dir_

comment:28 follow-up: Changed 3 years ago by saraedum

  • 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 where sage 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 the sage-* 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 non-Sage 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/Directory-Variables.html.)
  • Make $SAGE_SCRIPTS_DIR available like any other $SAGE_ variable everywhere and use it as the directory where all the sage-* 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/sage-env 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 fbissey

Well, I don't use the configure script but that sound relatively reasonable. I personally ship stuff like sage-env 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 saraedum

  • 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 saraedum

Thanks for the feedback. I think I prefer the non-breaking change at the moment.

comment:32 Changed 3 years ago by git

  • Commit changed from 3224b7c15e0009b7b1bec0f9c883469b2d6ef73f to c897d2b9e3e2a7d4a2220af60250f01b9b419746

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

c897d2bA first draft of using SAGE_BIN and SAGE_SCRIPTS_DIR differently

comment:33 Changed 3 years ago by saraedum

  • Work issues set to use features of 20382

comment:34 Changed 3 years ago by saraedum

  • Description modified (diff)
  • Work issues use features of 20382 deleted

I think I want to the binaries differently if they are non-optional dependencies. So I think this should not be part of this ticket.

comment:35 Changed 3 years ago by git

  • Commit changed from c897d2b9e3e2a7d4a2220af60250f01b9b419746 to 5e597441a4435e2033100611512113095834bdbb

Branch pushed to git repo; I updated commit sha1. New commits:

5e59744Add configure switch for scriptsdir

comment:36 Changed 3 years ago by saraedum

  • Status changed from needs_work to needs_review
  • Work issues set to requires testing

comment:37 Changed 3 years ago by saraedum

This still needs quite some testing. I just want to see what the patchbot thinks about this.

comment:38 Changed 3 years ago by saraedum

  • 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 git

  • Commit changed from 5e597441a4435e2033100611512113095834bdbb to 6b1ec2e573978815d770f0baff01bffb9ec7d2e4

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

be3a0f7The docbuild crashes happen during an os.fork to spawn tachyon
7d85dc7Something related to the sphinxbuild seems to be leaking memory
50898feTry to build-from-scratch on GitLab's underpowered CI machines
95c6275Comment on fork logic in docbuilding
c0574caMerge remote-tracking branch 'trac/develop' into u/saraedum/gitlabci
329fdabfix tags for build-from-latest
048e2d8silence unicode warnings from the patchbot
f9c772eFix filtering logic in sphinx logger
f043163Fixed some minor doctest issues
6b1ec2eMerge remote-tracking branch 'remotes/trac/u/saraedum/gitlabci' into sage-scripts-dir

comment:40 Changed 3 years ago by saraedum

  • 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 git

  • Commit changed from 6b1ec2e573978815d770f0baff01bffb9ec7d2e4 to 795d0faef202cf603ceebddcff8ef4113dab8e2e

Branch pushed to git repo; I updated commit sha1. New commits:

8f3480eIt seems that MAKEOPTS/SAGE_NUM_THREADS parsing has been fixed now
99355adBuild images in the same order in the Dockerfile and in build-docker.sh
81babb8Simplify caching during docker build
ad39cc9Complicate parallelization quite a bit
795d0faMerge remote-tracking branch 'trac/u/saraedum/gitlabci' into HEAD

comment:42 Changed 3 years ago by saraedum

  • Description modified (diff)

comment:43 Changed 3 years ago by saraedum

  • Description modified (diff)

comment:44 Changed 3 years ago by git

  • Commit changed from 795d0faef202cf603ceebddcff8ef4113dab8e2e to 4de96c2cfd063680492a5e066d1ee508980871f4

Branch pushed to git repo; I updated commit sha1. New commits:

4de96c2Merge remote-tracking 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 saraedum

  • 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 saraedum

  • Branch set to u/saraedum/22731

comment:47 Changed 3 years ago by git

  • Commit set to 03c7aa4660e808f080f97a572b0b331c5bce4060

Branch pushed to git repo; I updated commit sha1. New commits:

03c7aa4Merge remote-tracking branch 'trac/develop' into 22731

comment:48 Changed 3 years ago by saraedum

  • Description modified (diff)

comment:49 Changed 3 years ago by git

  • Commit changed from 03c7aa4660e808f080f97a572b0b331c5bce4060 to 7e37bedcb1d086aa5eb2e1516beec495db39c4c5

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

a532fceMerge remote-tracking branch 'trac/develop' into u/saraedum/gitlabci
686fe86An attempt to use workflows on CircleCI
79003abDo not deduce CPUs/RAM by looking at the local machine
906ebeaCollect system information on the docker host
60ddb4bsimplify apk add command for GitLab CI
25f934eBuild from the latest develop image
aa49196Backport https://trac.sagemath.org/ticket/24655 for automated docker builds
a0fdc3cMerge commit 'aa49196c4f0ed33da7a45b80c7a4a01bd46b7d61' into u/saraedum/gitlabci
b704268Merge branch '24655' into 24854
7e37bedMerge branch '24854' into 22731

comment:50 Changed 3 years ago by saraedum

  • Dependencies changed from #25056, #20382, #24655 to #25056, #20382, #24655, #24854

comment:51 Changed 3 years ago by nbruin

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: sage-devel 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 saraedum

  • Description modified (diff)

comment:53 Changed 3 years ago by saraedum

  • Description modified (diff)

comment:54 Changed 3 years ago by git

  • Commit changed from 7e37bedcb1d086aa5eb2e1516beec495db39c4c5 to 6d89d614dc1f363622e21b22e213e815b528425b

Branch pushed to git repo; I updated commit sha1. New commits:

11f8a6cno port exposure needed for jupyter anymore
1960ba1add openssl/libssl-dev to the docker images
14926cfRevert "add openssl/libssl-dev to the docker images"
f2c641aAdd debug information on patch size
6d89d61Merge branch '24655' into 22731

comment:55 Changed 3 years ago by saraedum

  • Description modified (diff)

comment:56 Changed 3 years ago by git

  • Commit changed from 6d89d614dc1f363622e21b22e213e815b528425b to d7cb021121279a5d3645df7c1519dfff382ac033

Branch pushed to git repo; I updated commit sha1. New commits:

864843cfix syntax error in CircleCI configuration
e1c8536Run short and new doctests on CircleCI for every branch
45579a1Call the right scripts in CircleCI
83f71e7Don't pipe "echo" into "rm"
8d3caebMerge branch '24655' into 24854
d7cb021Merge branch '24854' into 22731

comment:57 Changed 3 years ago by saraedum

  • 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 saraedum

  • Dependencies changed from #25056, #20382, #24655, #24854 to #25056, #20382, #24655, #24854, #25431

comment:59 Changed 3 years ago by git

  • Commit changed from d7cb021121279a5d3645df7c1519dfff382ac033 to bed5af50d2560826420e23b716193c95672ffe3e

Branch pushed to git repo; I updated commit sha1. New commits:

e67314cSkip _test_matrix_smith for lattice precision rings
bed5af5Merge branch '25431' into 22731

comment:60 Changed 3 years ago by mkoeppe

  • Cc mkoeppe added

comment:61 in reply to: ↑ 28 Changed 3 years ago by mkoeppe

Replying to saraedum:

Alternatively the second point could be replaced by the following two:

+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 mkoeppe

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 git

  • Commit changed from bed5af50d2560826420e23b716193c95672ffe3e to 7ba2ee2059f43222aabf3ce5de7a282f2687832d

Branch pushed to git repo; I updated commit sha1. New commits:

3c0d6a3Merge branch '22731' into HEAD
d28f601Make sage -pip work
b8e6077Do not install libssl-dev twice
7f44254Merge remote-tracking branch 'trac/develop' into 24655
91d5271Merge branch '24655' into HEAD
b413748Merge branch '24854' into HEAD
4ddd346Backport #24655 to build docker images
ab71cfaMerge branch 'docker-8.3.beta3' into 24655
5086548Merge branch '24655' into 24854
7ba2ee2Merge branch '24854' into 22731

comment:64 Changed 3 years ago by git

  • Commit changed from 7ba2ee2059f43222aabf3ce5de7a282f2687832d to 2130060ef6ab1490368bd266ceaa3c190e2366a8

Branch pushed to git repo; I updated commit sha1. New commits:

6708512Call sage -tp with --all so it actually does something
2130060Merge branch '24854' into 22731

comment:65 Changed 3 years ago by mkoeppe

See also #21559, #21569, #21570.

comment:66 Changed 3 years ago by git

  • Commit changed from 2130060ef6ab1490368bd266ceaa3c190e2366a8 to 08f93c738438eea285881837340a0211e71e8187

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

4ef4e18Where to report bugs for the docker images
b8a94e2Merge remote-tracking branch 'trac/develop' into 24655
6a3d5d0drop trailing space
f7ea0f3Make update-env posix compliant again
44e931cMerge branch '24655' into 24854
e8dadc8Merge remote-tracking branch 'trac/develop' into 24655
5877f65Merge branch '24655' into 24854
569d492Merge remote-tracking branch 'trac/develop' into 24655
e9c2cf8Merge branch '24655' into 24854
08f93c7Merge branch '24854' into HEAD

comment:67 Changed 3 years ago by git

  • Commit changed from 08f93c738438eea285881837340a0211e71e8187 to a9ca8a1e356eb6508036e9c986f04f2d97131428

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

57d653dMerge branch '25270' into 24854
d29058eMerge remote-tracking branch 'trac/develop' into 24655
2cec8acReplace do tag with big
7bb3fafMerge branch '24655' into 24854
6c9c2ceMerge remote-tracking branch 'trac/develop' into 25270
e38d7deMerge branch '25270' into 24854
662d3f8Use --short to run --short doctests
6241011Fix namespacing for CircleCI
9f15655Merge branch '24655' into 24854
a9ca8a1Merge branch '24854' into 22731

comment:68 Changed 3 years ago by git

  • Commit changed from a9ca8a1e356eb6508036e9c986f04f2d97131428 to 9c9caef7b23b5782d230dd52aa9f06cb7aee96d9

Branch pushed to git repo; I updated commit sha1. New commits:

6e267b6Fix namespacing for GitLab CI
9c9caefMerge branch '24655' into 22731

comment:69 Changed 2 years ago by jdemeyer

  • Status changed from needs_review to needs_work
  1. There is a merge conflicts.
  1. As I mentioned in 4, don't use SAGE_SCRIPTS_DIR.

comment:70 Changed 10 months ago by mkoeppe

  • Milestone changed from sage-8.0 to sage-9.2

comment:71 Changed 10 months ago by mkoeppe

  • Work issues changed from requires testing to clarify and prepare to support #30036

comment:72 Changed 10 months ago by mkoeppe

  • 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 mkoeppe

  • 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 mkoeppe

  • Description modified (diff)

comment:75 Changed 9 months ago by mkoeppe

  • Description modified (diff)

comment:76 Changed 9 months ago by mkoeppe

  • Priority changed from minor to major

comment:77 Changed 8 months ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-9.3

comment:78 Changed 8 months ago by mkoeppe

  • Cc gh-tobiasdiez 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 mkoeppe

  • 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 mkoeppe

  • Branch changed from u/saraedum/22731 to u/mkoeppe/22731

comment:81 Changed 8 months ago by mkoeppe

  • Commit changed from 9c9caef7b23b5782d230dd52aa9f06cb7aee96d9 to b9dfd1a4cf1e789672f09a464757d50c2036b34a
  • Dependencies #25056, #20382, #24655, #24854, #25431 deleted

Rebased away from #24854


New commits:

7325325A first draft of using SAGE_BIN and SAGE_SCRIPTS_DIR differently
b9dfd1aAdd configure switch for scriptsdir

comment:82 Changed 8 months ago by mkoeppe

  • 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 mkoeppe

  • Description modified (diff)

comment:84 Changed 7 months ago by mkoeppe

  • Dependencies set to #30818

comment:85 Changed 6 months ago by mkoeppe

  • Cc isuruf added
  • Dependencies changed from #30818 to #29850, #30818

comment:86 Changed 6 months ago by git

  • Commit changed from b9dfd1a4cf1e789672f09a464757d50c2036b34a to 545d118b9bc907340eca5342e380ce32fa72a923

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

9ba8cf2Merge tag '9.3.beta0' into t/29951/src_bin_sage_env__make_sage_env_config_and_sage_local_optional
c8e6910Merge tag '9.3.beta1' into t/29951/src_bin_sage_env__make_sage_env_config_and_sage_local_optional
9ab593cInstall sage-env-config with sage_conf
a361580build/pkgs/sage_conf/spkg-src: New
d02b597build/pkgs/sagelib/src/setup.py: Do not install sage-env-config
1b4bc99src/bin/sage: Only source sage-env-config if it exists
a1a6df5src/bin/sage: Use python3 etc. from PATH instead of using SAGE_LOCAL
d30b410build/pkgs/sage_conf/spkg-install: Build wheel manually
19f7eaesrc/bin/sage-env: Make sage-env-config optional
545d118sage.env: Add SAGE_VENV

comment:87 Changed 6 months ago by git

  • Commit changed from 545d118b9bc907340eca5342e380ce32fa72a923 to 6348c5c7c0eb2caba9f72f511847bea550b23709

Branch pushed to git repo; I updated commit sha1. New commits:

6348c5csrc/sage: Replace some SAGE_LOCAL by SAGE_VENV

comment:88 Changed 6 months ago by mkoeppe

  • Authors changed from Tobias Hansen, Julian Rüth to Matthias Koeppe
  • Status changed from needs_work to needs_review
  • Work issues Implement as described deleted

comment:89 Changed 6 months ago by mkoeppe

  • Dependencies changed from #29850, #30818 to #29850

comment:90 Changed 6 months ago by git

  • Commit changed from 6348c5c7c0eb2caba9f72f511847bea550b23709 to 122bf7890c17de88de973be8ac9c5165e9d47447

Branch pushed to git repo; I updated commit sha1. New commits:

ecde605Merge branch 't/29850/install_sage_env_config_with_sage_conf' into t/30888/remove_unused_env_variable_sage_scripts_dir
a318e9esrc/bin/sage-env: Do not set SAGE_SCRIPTS_DIR
122bf78Merge branch 't/30888/remove_unused_env_variable_sage_scripts_dir' into t/22731/22731

comment:91 Changed 6 months ago by mkoeppe

  • Dependencies changed from #29850 to #29850, #30888

comment:92 Changed 6 months ago by git

  • 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 mkoeppe

  • Dependencies changed from #29850, #30888 to #29850

comment:94 Changed 6 months ago by git

  • Commit changed from 6348c5c7c0eb2caba9f72f511847bea550b23709 to c585d94b95c5675f5a68690d13fa6a949642c48f

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

8f88709src/bin/sage: Only source sage-env-config if it exists
ab2655esrc/bin/sage: Use python3 etc. from PATH instead of using SAGE_LOCAL
4577f37src/bin/sage-env: Make sage-env-config optional
c35c170sage.env: Add SAGE_VENV
c585d94src/sage: Replace some SAGE_LOCAL by SAGE_VENV

comment:95 Changed 6 months ago by mkoeppe

  • 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 git

  • Commit changed from c585d94b95c5675f5a68690d13fa6a949642c48f to 2fd195f5f044d125b0d5a743fec9d79e449126af

Branch pushed to git repo; I updated commit sha1. New commits:

2fd195fsrc/bin/sage: Fix reference to trac ticket

comment:97 Changed 6 months ago by mkoeppe

  • Description modified (diff)

comment:98 Changed 6 months ago by mkoeppe

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 sage-env 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 follow-ups: Changed 6 months ago by gh-tobiasdiez

  • 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 git

  • Commit changed from 2fd195f5f044d125b0d5a743fec9d79e449126af to f3b7a9cce247457f07ef42069cdadb3f6409b008

Branch pushed to git repo; I updated commit sha1. New commits:

f3b7a9csrc/sage/env.py: Add documentation

comment:101 in reply to: ↑ 99 ; follow-up: Changed 6 months ago by mkoeppe

Replying to gh-tobiasdiez:

... 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

Done, please take a look

comment:102 Changed 6 months ago by mkoeppe

  • Description modified (diff)

comment:103 in reply to: ↑ 99 Changed 6 months ago by mkoeppe

Replying to gh-tobiasdiez:

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).

Sure, that will make sense to do in the follow-up ticket regarding SAGE_BIN (if the downstream packagers still need that...)

comment:104 Changed 6 months ago by mkoeppe

  • Reviewers changed from Tobias Diez, to Tobias Diez, ...

comment:105 in reply to: ↑ 101 Changed 6 months ago by gh-tobiasdiez

Replying to mkoeppe:

Replying to gh-tobiasdiez:

... 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

Done, please take a look

Thanks! Looks good.

comment:106 Changed 6 months ago by mkoeppe

  • Cc arojas added

comment:107 Changed 6 months ago by mkoeppe

  • Cc gh-mwageringel added

comment:108 Changed 6 months ago by dimpase

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 mkoeppe

See ticket description - "In this ticket, ..."

comment:110 Changed 6 months ago by dimpase

  • 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 mkoeppe

Thanks!

comment:112 Changed 6 months ago by vbraun

  • 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 vbraun

  • 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/Makefile-auto.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/sage-env';
possibly contact sage-devel (see http://groups.google.com/group/sage-devel).

comment:114 follow-up: Changed 6 months ago by fbissey

I noticed it as well in sage-on-gentoo. 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 mkoeppe

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 dimpase

Is at the error point the value of SAGE_SCRIPTS_DIR already known?

comment:117 Changed 6 months ago by fbissey

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.

Last edited 6 months ago by fbissey (previous) (diff)

comment:118 follow-up: Changed 6 months ago by mkoeppe

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 fbissey

Replying to mkoeppe:

The problem is solved in #30013, so I need to move some of the changes to this ticket.

Yes, it looks like it would help.

comment:120 Changed 6 months ago by mkoeppe

I will work on this after the next beta comes out.

comment:121 Changed 6 months ago by dimpase

Anyhow, just in case, here one could do

  • bootstrap

    a b SAGE_SPKG_CONFIGURE_$(echo ${pkgname} | tr '[a-z]' '[A-Z]')" 
    113113    # ONLY stderr, and to re-output the results back to stderr leaving
    114114    # stdout alone. Basically we swap the two descriptors using a
    115115    # third, filter, and then swap them back.
     116    touch src/bin/sage-env-config
     117    SAGE_SCRIPTS_DIR=./src/bin
    116118    BOOTSTRAP_QUIET="${BOOTSTRAP_QUIET}" \
    117119    SAGE_ROOT="$SAGE_ROOT" \
    118120    src/doc/bootstrap && \

and it all goes through.

comment:122 Changed 6 months ago by git

  • Commit changed from f3b7a9cce247457f07ef42069cdadb3f6409b008 to 1fe77a819d2477bf4b53d228388a4ff3e2c7c45d

Branch pushed to git repo; I updated commit sha1. New commits:

1fe77a8src/bin/sage-env: Do not set SAGE_SCRIPTS_DIR

comment:123 Changed 6 months ago by mkoeppe

  • Status changed from new to needs_review

Cherry-picked 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 re-introduce this feature using a different mechanism.)

comment:124 Changed 6 months ago by git

  • Commit changed from 1fe77a819d2477bf4b53d228388a4ff3e2c7c45d to 38eebc3d9f790c81b8ffcc963cf2513184c9ae6a

Branch pushed to git repo; I updated commit sha1. New commits:

38eebc3Merge tag '9.3.beta2' into t/22731/22731

comment:125 Changed 6 months ago by mkoeppe

Merged the latest beta. Ready for another round of review.

comment:126 Changed 6 months ago by dimpase

  • Status changed from needs_review to positive_review

this works, after "git clean" as above, too.

comment:127 Changed 6 months ago by mkoeppe

Thanks!

comment:128 Changed 5 months ago by vbraun

  • Branch changed from u/mkoeppe/22731 to 38eebc3d9f790c81b8ffcc963cf2513184c9ae6a
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.