Opened 3 years ago

Closed 17 months ago

Last modified 17 months ago

#25486 closed enhancement (fixed)

Discover SAGE_SCRIPTS_DIR to make $SAGE_LOCAL/bin/sage work from any directory, in an environment without SAGE_* variables

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.2
Component: build Keywords: SAGE_LOCAL, SAGE_ROOT
Cc: jdemeyer, nbruin, embray, fbissey, charpent, thansen, saraedum, slelievre, gh-timokau, dimpase Merged in:
Authors: Matthias Koeppe Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 4415da5 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

This ticket makes $SAGE_LOCAL/bin/sage work without environment variables set, or the current working directory to be something particular.

It does that by

  • adding SAGE_ROOT to $SAGE_LOCAL/bin/sage-env-config (via src/bin/sage-env-config.in).
  • adding code to the top of sage-env that determines SAGE_SCRIPTS_DIR from the directory of itself.

Previous discussion in https://groups.google.com/d/msg/sage-devel/oODph9-yjaI/FVeBsCk_CgAJ:

On Wednesday, May 30, 2018 at 9:01:59 AM UTC-7, Nils Bruin wrote:

Currently, if I run the script $SAGE_LOCAL/bin/sage in my normal environment (where SAGE_ROOT is not set) then I get:

Error: You must set the SAGE_ROOT environment variable or run this script from the SAGE_ROOT or SAGE_ROOT/local/bin/ directory. Error setting environment variables by sourcing '/usr/local/sage/sage-git/local/bin/sage-env'; possibly contact sage-devel (see http://groups.google.com/group/sage-devel).

If instead I run the script $SAGE_ROOT/sage a value for SAGE_ROOT is figured out and everything works fine when called from my normal environment.

Change History (37)

comment:1 Changed 3 years ago by slelievre

  • Cc slelievre added
  • Keywords SAGE_LOCAL SAGE_ROOT added

Likewise, a naive trial to start the GAP shipped by Sage fails:

$ /path/to/sagedir/local/bin/gap
Set the environment variable SAGE_LOCAL.

and one instead has to run:

$ /path/to/sagedir/sage --gap

or

$ SAGE_LOCAL='/path/to/sagedir' /path/to/sagedir/local/bin/gap

(where /path/to/sagedir should be replaced by the actual path to the Sage installation directory).

This makes it hard to use Gap.app, the macOS interface to GAP by Russ Woodroofe, with the GAP shipped by Sage.

Last edited 3 years ago by slelievre (previous) (diff)

comment:2 Changed 3 years ago by gh-RussWoodroofe

One comment: the new version of GAP deprecates the gap.sh as the right way to start GAP. (The gap binary detects the directory that it lives in, and uses that as the default library directory.) Relying on the shell script to check for SAGE_LOCAL probably isn't a great idea for GAP 4.9.1 or later. That might make this more pressing.

If you do need to check for the SAGE_LOCAL variable, it seems to me like a better place to check would be in GAP code. (The IO_environ() function from the IO package will read the environment, which you could do on package load.) But I'm coming from outside, and might not be aware of all issues.

Also, I'm the Gap.app author. If it still looks necessary when I next make updates, I'll consider adding logic to set SAGE_LOCAL to a sensible guess on startup. I ran out of time for this release (as I wanted to get something out at the same time as GAP 4.9.1). I'd love for it to no longer be necessary!

Last edited 3 years ago by gh-RussWoodroofe (previous) (diff)

comment:3 Changed 3 years ago by embray

I don't think there should be anything in GAP that cares about $SAGE_LOCAL. If anything Sage's bin/gap should just be a wrapper script that sets the necessary environment variables, rather than patching gap.sh which it currently does =_=

comment:4 Changed 3 years ago by gh-timokau

  • Cc gh-timokau added

comment:5 Changed 23 months ago by mkoeppe

A step into this direction: #29022 "Create module src/sage/env_config.py from src/sage/env_config.py.in, defining variables for use in sage.env.

comment:6 Changed 23 months ago by mkoeppe

  • Dependencies set to #29022

comment:7 Changed 23 months ago by mkoeppe

  • Milestone changed from sage-8.3 to sage-9.1

comment:8 Changed 23 months ago by mkoeppe

  • Branch set to u/mkoeppe/_sage_local_bin_sage_should_work_in_an_environment_without_sage___variables

comment:9 Changed 23 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Cc dimpase added
  • Commit set to c7b109099922ef6cdcceb6e4510cdeb096a94661
  • Description modified (diff)
  • Status changed from new to needs_review

This is adapted from an old branch of #21479 (./configure --prefix=SAGE_LOCAL), when $SAGE_LOCAL/bin/sage-env-config was introduced; but then a less ambitious solution was implemented for that ticket.


New commits:

a53e0dfsrc/bin/sage-env-config.in: Define SAGE_ROOT
c7b1090src/bin/sage-env: Obtain SAGE_SCRIPTS_DIR from sage-env invocation

comment:10 Changed 23 months ago by mkoeppe

  • Dependencies #29022 deleted
  • Description modified (diff)

Removed a dependency that is not needed.

comment:11 Changed 23 months ago by mkoeppe

  • Summary changed from $SAGE_LOCAL/bin/sage should work in an environment without SAGE_* variables to Discover SAGE_SCRIPTS_DIR to make $SAGE_LOCAL/bin/sage work from any directory, in an environment without SAGE_* variables

comment:12 follow-ups: Changed 23 months ago by embray

  • Status changed from needs_review to needs_work

A few suggestions:

rename SOURCED_SAGE_ENV_CONFIG to SAGE_ENV_CONFIG_SOURCED (we already have a SAGE_ENV_SOURCED) variable as well.

Make sage-env-config itself responsible for setting SAGE_ENV_CONFIG_SOURCED, rather than setting it in sage-env.

+# SAGE_ROOT is the location of the Sage source/build tree.
+export SAGE_ROOT="@abs_top_srcdir@"

Wouldn't this permanently encode the directory in which Sage happens to be built in any Sage installation, including for binary builds? E.g. if Volker does a binary build and publishes it, then it will contain in local/bin/sage-env-config something like SAGE_ROOT=/home/vbraun/src/sage/ and won't work when moved. So I'm not sure this make sense to me...

How do distros currently handle $SAGE_ROOT, a path that doesn't have significant meaning in standard installs? Perhaps what we need is to do away with any dependency on $SAGE_ROOT at runtime.

comment:13 Changed 23 months ago by embray

I think otherwise in principle I'm +1 to this but I don't see how it resolves that last question...

comment:14 Changed 23 months ago by git

  • Commit changed from c7b109099922ef6cdcceb6e4510cdeb096a94661 to 37c68fb9361c2a4c488ed8a8e1450d36af364eed

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

37c68fbRename SOURCED_SAGE_ENV_CONFIG -> SAGE_ENV_CONFIG_SOURCED, set it in src/bin/sage-env-config.in

comment:15 in reply to: ↑ 12 Changed 23 months ago by mkoeppe

Replying to embray:

A few suggestions:

rename SOURCED_SAGE_ENV_CONFIG to SAGE_ENV_CONFIG_SOURCED (we already have a SAGE_ENV_SOURCED) variable as well.

Make sage-env-config itself responsible for setting SAGE_ENV_CONFIG_SOURCED, rather than setting it in sage-env.

Thanks! Done.

comment:16 in reply to: ↑ 12 Changed 23 months ago by mkoeppe

Replying to embray:

+# SAGE_ROOT is the location of the Sage source/build tree.
+export SAGE_ROOT="@abs_top_srcdir@"

Wouldn't this permanently encode the directory in which Sage happens to be built in any Sage installation,

Yes, that's what it does. Sage installations (SAGE_LOCAL) and non-distclean source trees (SAGE_ROOT) are not relocatable with or without this ticket.

including for binary builds? E.g. if Volker does a binary build and publishes it, then it will contain in local/bin/sage-env-config something like SAGE_ROOT=/home/vbraun/src/sage/ and won't work when moved.

The binary build uses SAGE_ROOT = a long pseudorandom path and then rewrites all paths, including this one, using https://github.com/sagemath/binary-pkg/blob/master/binary_pkg/templates/relocate-once.py the first time that SAGE_ROOT/sage is used.

How do distros currently handle $SAGE_ROOT, a path that doesn't have significant meaning in standard installs?

Distributions tend to replace all of sage-env by their own tooling anyway.

Perhaps what we need is to do away with any dependency on $SAGE_ROOT at runtime.

Yes, in fact, there's very little left (see #25828/#28815). In any case, that's orthogonal to the present ticket.

comment:17 Changed 23 months ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:18 Changed 22 months ago by git

  • Commit changed from 37c68fb9361c2a4c488ed8a8e1450d36af364eed to 6382db9647f59d550026ea0ea63cf4f2eb13a453

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

4184f49src/bin/sage-env-config.in: Define SAGE_ROOT
8cbeafasrc/bin/sage-env: Obtain SAGE_SCRIPTS_DIR from sage-env invocation
6382db9Rename SOURCED_SAGE_ENV_CONFIG -> SAGE_ENV_CONFIG_SOURCED, set it in src/bin/sage-env-config.in

comment:19 Changed 22 months ago by mkoeppe

Rebased on 9.1.beta2, needs review

comment:20 Changed 22 months ago by git

  • Commit changed from 6382db9647f59d550026ea0ea63cf4f2eb13a453 to b6c35a715bd2d496a3aea8982f80b470e68aa98d

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

b6c35a7Merge tag '9.1.beta4' into t/25486/_sage_local_bin_sage_should_work_in_an_environment_without_sage___variables

comment:21 Changed 21 months ago by git

  • Commit changed from b6c35a715bd2d496a3aea8982f80b470e68aa98d to b4277ec6fdc0250ffc81360837d1b82d09b1aa5b

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

b4277ecMerge tag '9.1.beta6' into t/25486/_sage_local_bin_sage_should_work_in_an_environment_without_sage___variables

comment:22 Changed 21 months ago by git

  • Commit changed from b4277ec6fdc0250ffc81360837d1b82d09b1aa5b to c6879b51afa4235733fdbb713613995a4fe23afc

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

c6879b5Merge tag '9.1.beta7' into t/25486/_sage_local_bin_sage_should_work_in_an_environment_without_sage___variables

comment:23 Changed 21 months ago by git

  • Commit changed from c6879b51afa4235733fdbb713613995a4fe23afc to 3671eb340e39a730c4595f9c81b06a09313ad335

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

afd05b3src/bin/sage-env-config.in: Define SAGE_ROOT
50d2e09src/bin/sage-env: Obtain SAGE_SCRIPTS_DIR from sage-env invocation
3671eb3Rename SOURCED_SAGE_ENV_CONFIG -> SAGE_ENV_CONFIG_SOURCED, set it in src/bin/sage-env-config.in

comment:24 Changed 21 months ago by mkoeppe

Rebased on 9.1.beta8, needs review

comment:25 Changed 20 months ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

pushing these forward to 9.2

comment:26 Changed 18 months ago by git

  • Commit changed from 3671eb340e39a730c4595f9c81b06a09313ad335 to 4415da592193863627a835ebf7f44d8a8e1347fd

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

37d79acsrc/bin/sage-env-config.in: Define SAGE_ROOT
68c1762src/bin/sage-env: Obtain SAGE_SCRIPTS_DIR from sage-env invocation
4415da5Rename SOURCED_SAGE_ENV_CONFIG -> SAGE_ENV_CONFIG_SOURCED, set it in src/bin/sage-env-config.in

comment:27 Changed 18 months ago by mkoeppe

Rebased on 9.2.beta0

comment:28 Changed 18 months ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

lgtm

comment:29 Changed 18 months ago by dimpase

  • Status changed from positive_review to needs_info

oops, sorry, doesn't it need a shebang to make sure it is running in bash?

comment:30 Changed 18 months ago by mkoeppe

  • Status changed from needs_info to needs_work

Well, I guess this ticket conflicts somehow with #29345

comment:31 follow-up: Changed 17 months ago by mkoeppe

  • Status changed from needs_work to needs_review

Actually it is already documented that sage-env "uses bash features, so it cannot be used with other shells." It is sourced by other scripts, so it does not need a #! line.

comment:32 Changed 17 months ago by dimpase

  • Status changed from needs_review to positive_review

lgtm

comment:33 Changed 17 months ago by mkoeppe

Thanks!

comment:34 Changed 17 months ago by vbraun

  • Branch changed from u/mkoeppe/_sage_local_bin_sage_should_work_in_an_environment_without_sage___variables to 4415da592193863627a835ebf7f44d8a8e1347fd
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:35 Changed 17 months ago by mkoeppe

  • Commit 4415da592193863627a835ebf7f44d8a8e1347fd deleted

Follow up: #29446

comment:36 in reply to: ↑ 31 Changed 17 months ago by mjo

Replying to mkoeppe:

Actually it is already documented that sage-env "uses bash features, so it cannot be used with other shells." It is sourced by other scripts, so it does not need a #! line.

I fixed all of the bashisms in #29345, but missed that comment. The sage build now fails again with any other /bin/sh:

cd '/home/mjo/src/sage.git/build/pkgs/sage_conf' && . '/home/mjo/src/sage.git/src/bin/sage-env' && . '/home/mjo/src/sage.git/build/bin/sage-build-env-config' && sage-logger -p '/home/mjo/src/sage.git/build/pkgs/sage_conf/spkg-install' '/home/mjo/src/sage.git/logs/pkgs/sage_conf-none.log'
/bin/sh: 123: /home/mjo/src/sage.git/src/bin/sage-env: Bad substitution
Error: SAGE_SCRIPTS_DIR is set to a bad value:
SAGE_SCRIPTS_DIR=/home/mjo/src/sage.git/build/pkgs/sage_conf
You must correct it or erase it and rerun this script
make[3]: *** [Makefile:2022: /home/mjo/src/sage.git/local/var/lib/sage/installed/sage_conf-none] Error 1

Debian's dash shell is crippled, but would it be possible to symlink /bin/sh to dash on e.g. Arch linux, or maybe to ksh on some other distro that we have CI for? (To prevent regressions in the future?)

comment:37 Changed 17 months ago by mjo

Follow-up on #30128

Note: See TracTickets for help on using tickets.