Opened 2 years ago
Closed 22 months ago
#30888 closed enhancement (fixed)
Make $SAGE_LOCAL/bin/sage work again from any directory, in an environment without SAGE_* variables, following symlinks
Reported by: | Matthias Köppe | Owned by: | |
---|---|---|---|
Priority: | blocker | Milestone: | sage-9.3 |
Component: | scripts | Keywords: | sd111 |
Cc: | Michael Orlitzky, Dima Pasechnik, John Palmieri | Merged in: | |
Authors: | Michael Orlitzky | Reviewers: | Matthias Koeppe |
Report Upstream: | N/A | Work issues: | |
Branch: | a63d256 (Commits, GitHub, GitLab) | Commit: | a63d256e1f1968544822ff2af8b4ef146b7c9660 |
Dependencies: | Stopgaps: |
Description (last modified by )
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 (enforce sourcing of sage-env-config
before src/bin/sage-env
).
We repair it, making sure that it works even if $0 is a symlink.
Change History (26)
comment:1 Changed 2 years ago by
Dependencies: | → #29850 |
---|
comment:2 Changed 2 years ago by
Branch: | → u/mkoeppe/remove_unused_env_variable_sage_scripts_dir |
---|
comment:3 Changed 2 years ago by
Authors: | → Matthias Koeppe |
---|---|
Commit: | → a318e9e3c03bc466f966965df378ff4685ff6d12 |
Status: | new → needs_review |
comment:4 Changed 2 years ago by
Status: | needs_review → needs_info |
---|
comment:5 Changed 2 years ago by
comment:6 Changed 2 years ago by
Description: | modified (diff) |
---|---|
Status: | needs_info → needs_work |
Summary: | Remove unused env variable SAGE_SCRIPTS_DIR → Repair SAGE_SCRIPTS_DIR discovery to make $SAGE_LOCAL/bin/sage work again from any directory, in an environment without SAGE_* variables |
Unfortunately I did not notice it when I reviewed #30128.
comment:7 Changed 2 years ago by
Authors: | Matthias Koeppe |
---|---|
Branch: | u/mkoeppe/remove_unused_env_variable_sage_scripts_dir |
Commit: | a318e9e3c03bc466f966965df378ff4685ff6d12 |
Dependencies: | #29850 → #29951 |
Any takers for this one?
comment:8 follow-up: 9 Changed 2 years ago by
What kind of solution is acceptable here? I've lost track of the big picture, but it looks like all of the important ./configure
-time settings are in sage-env-config
, but local/bin/sage
can't find sage-env-config
because the path to it (determined during ./configure
) is contained in sage-env-config
.
Are we allowed to stick autoconf values directly into local/bin/sage
(after renaming with an .in
suffix), like
: ${SAGE_ROOT:=@abs_top_srcdir@}
which would set a default value of SAGE_ROOT
at ./configure
-time, but still allow custom values of SAGE_ROOT
to be set in the environment?
comment:9 Changed 2 years ago by
Replying to mjo:
What kind of solution is acceptable here? I've lost track of the big picture, but it looks like all of the important
./configure
-time settings are insage-env-config
, butlocal/bin/sage
can't findsage-env-config
because the path to it (determined during./configure
) is contained insage-env-config
.
The directory can be found from $0, with resolving symlinks.
Similar to what used to be in sage-env
, but not bash-specific because it's for an executed script rather than a sourced script.
Are we allowed to stick autoconf values directly into
local/bin/sage
(after renaming with an.in
suffix), like: ${SAGE_ROOT:=@abs_top_srcdir@}which would set a default value of
SAGE_ROOT
at./configure
-time, but still allow custom values ofSAGE_ROOT
to be set in the environment?
No, because the sage
script is installed by setup.py
and we plan to make this work even when configure
has not been run.
comment:10 Changed 2 years ago by
It could be done on top of #29852, just adding code to resolve a $0 symlink instead of using $0 directly.
comment:11 Changed 2 years ago by
Dependencies: | #29951 → #29951, #29852 |
---|
comment:12 follow-up: 13 Changed 2 years ago by
We can fake realpath
by copy & pasting the 70-line resolvelinks()
function from sage-env, but I think that's pretty ugly:
- It's copy/pasting a ton of code. We can't source sage-env for it, because we don't know where sage-env lives.
- The function still doesn't work in some common cases, like hardlinks into the tree or symlinks out of the tree.
- It's a waste of time recomputing the same paths every time "sage" is run, in slow shell script.
If setup.py installs the script, it knows the correct location. It could replace a placeholder at install-time as in e.g. https://git.launchpad.net/dkimpy-milter/tree/setup.py
comment:13 follow-up: 14 Changed 2 years ago by
Replying to mjo:
We can fake
realpath
by copy & pasting the 70-lineresolvelinks()
function from sage-env, but I think that's pretty ugly:
- It's copy/pasting a ton of code. We can't source sage-env for it, because we don't know where sage-env lives.
I agree it's a bit ugly... that would be the third copy of the code (another one is in SAGE_ROOT/sage). Perhaps we can get rid of the copy in sage-env
?
- The function still doesn't work in some common cases, like hardlinks into the tree or symlinks out of the tree.
Hardlinks - granted.
What do you mean by "symlinks out of the tree"?
We would just document what is supported. The installation manual already has some guidance regarding "system wide installation" that needs updating anyway.
- It's a waste of time recomputing the same paths every time "sage" is run, in slow shell script.
True, but perhaps the resolving code would only be run if the files cannot be found without resolving. Then it would impose the runtime penalty only if symlinks are in use.
If setup.py installs the script, it knows the correct location. It could replace a placeholder at install-time as in e.g. https://git.launchpad.net/dkimpy-milter/tree/setup.py
No, this is unfortunately not true. The final install location of the Python package is not known at setup.py
time. When we build a wheel (which is the default installation mechanism in modern pip), the resulting wheel can be installed in any venv location (and the wheel format does not support post-install hooks of any kind).
comment:14 follow-ups: 15 21 Changed 2 years ago by
Replying to mkoeppe:
What do you mean by "symlinks out of the tree"?
If you move local/bin/sage
elsewhere, and then create a symlink from local/bin/sage
to that location. In that case, you don't want to use the absolute physical path of the script that's being run to find SAGE_ROOT
, rather the absolute physical path of its original install location.
If setup.py installs the script, it knows the correct location. It could replace a placeholder at install-time as in e.g. https://git.launchpad.net/dkimpy-milter/tree/setup.py
No, this is unfortunately not true. The final install location of the Python package is not known at
setup.py
time. When we build a wheel (which is the default installation mechanism in modern pip), the resulting wheel can be installed in any venv location (and the wheel format does not support post-install hooks of any kind).
Ok, I don't know enough about how this works to propose anything else. Copy/paste it is, then.
comment:15 Changed 2 years ago by
Replying to mjo:
Replying to mkoeppe:
What do you mean by "symlinks out of the tree"?
If you move
local/bin/sage
elsewhere, and then create a symlink fromlocal/bin/sage
to that location. In that case, you don't want to use the absolute physical path of the script that's being run to findSAGE_ROOT
, rather the absolute physical path of its original install location.
Right, let's just say that this is not supported.
comment:16 Changed 2 years ago by
Summary: | Repair SAGE_SCRIPTS_DIR discovery to make $SAGE_LOCAL/bin/sage work again from any directory, in an environment without SAGE_* variables → Make $SAGE_LOCAL/bin/sage work again from any directory, in an environment without SAGE_* variables, following symlinks |
---|
comment:17 Changed 2 years ago by
Dependencies: | #29951, #29852 → #29951, #29852, #30013 |
---|
Best to work on it on top of #30013
comment:18 Changed 2 years ago by
Keywords: | sd111 added |
---|
Hoping we can make progress on this ticket this week - https://wiki.sagemath.org/days111
comment:19 Changed 2 years ago by
User bug report - https://groups.google.com/g/sage-devel/c/tejOsRxfC9w/m/ulKTsowHBAAJ
comment:20 Changed 2 years ago by
Dependencies: | #29951, #29852, #30013 |
---|
comment:21 Changed 23 months ago by
comment:22 Changed 23 months ago by
Authors: | → Michael Orlitzky |
---|---|
Branch: | → u/mjo/ticket/30888 |
Commit: | → a63d256e1f1968544822ff2af8b4ef146b7c9660 |
Status: | needs_work → needs_review |
Further cleanup is possible, but I think this fixes the issue and makes the diff nice and clear.
New commits:
a63d256 | Trac #30888: resolve $0 links in src/bin/sage.
|
comment:23 Changed 23 months ago by
Reviewers: | → Matthias Koeppe |
---|---|
Status: | needs_review → positive_review |
This works well, thanks very much.
comment:24 Changed 22 months ago by
Priority: | major → critical |
---|
comment:25 Changed 22 months ago by
Priority: | critical → blocker |
---|
Setting priority to blocker to bring this ticket to the attention of the release bot.
comment:26 Changed 22 months ago by
Branch: | u/mjo/ticket/30888 → a63d256e1f1968544822ff2af8b4ef146b7c9660 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Last 10 new commits:
Merge tag '9.3.beta1' into t/29951/src_bin_sage_env__make_sage_env_config_and_sage_local_optional
Install sage-env-config with sage_conf
build/pkgs/sage_conf/spkg-src: New
build/pkgs/sagelib/src/setup.py: Do not install sage-env-config
src/bin/sage: Only source sage-env-config if it exists
src/bin/sage: Use python3 etc. from PATH instead of using SAGE_LOCAL
build/pkgs/sage_conf/spkg-install: Build wheel manually
src/bin/sage-env: Make sage-env-config optional
Merge branch 't/29850/install_sage_env_config_with_sage_conf' into t/30888/remove_unused_env_variable_sage_scripts_dir
src/bin/sage-env: Do not set SAGE_SCRIPTS_DIR