#21726 closed enhancement (fixed)
Support splitting spkg install into `spkg-build` and `spkg-install` (for SAGE_SUDO)
Reported by: | mkoeppe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.5 |
Component: | build | Keywords: | |
Cc: | jdemeyer, fbissey, embray, vbraun, dimpase, tscrim | Merged in: | |
Authors: | Matthias Koeppe | Reviewers: | Erik Bray |
Report Upstream: | N/A | Work issues: | |
Branch: | 48053d0 (Commits) | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
Here's a new attempt toward #21537 that is perhaps more elegant.
Let's change build/bin/sage-spkg
as follows:
- If
spkg-build
exists, call that, followed by$SAGE_SUDO spkg-install
. - Otherwise, just call
spkg-install
(withOUT $SAGE_SUDO). (Such packages cannot be installed into a SAGE_ROOT that is only root-writable, unless they use $SAGE_SUDO themselves.)
The patch changes two example packages, to demonstrate this: 4ti2
and lcalc
.
Before the patch:
$ rm -Rf local/share/4ti2 $ mkdir local/share/4ti2 $ sudo chown root local/share/4ti2 $ ./sage -f 4ti2 [...] [4ti2-1.6.7] mkdir: /Users/mkoeppe/s/sage/sage-rebasing/local/share/4ti2/doc: Permission denied [4ti2-1.6.7] make[4]: *** [install-docDATA] Error 1 [...] $ sudo chown root local/bin/ local/bin/lcalc $ ./sage -f lcalc [...] [lcalc-1.23.p14] cp -f lcalc /Users/mkoeppe/s/sage/sage-rebasing/local//bin/. [lcalc-1.23.p14] cp: /Users/mkoeppe/s/sage/sage-rebasing/local//bin/./lcalc: Permission denied [...]
Then, after the patch:
$ export SAGE_SUDO="sudo -E" $ ./sage -f 4ti2 lcalc # succeeds
Other packages will be handled in follow-up tickets.
See also:
Change History (19)
comment:1 Changed 3 years ago by
- Description modified (diff)
comment:2 Changed 3 years ago by
- Description modified (diff)
comment:3 Changed 3 years ago by
- Branch set to u/mkoeppe/support_splitting_spkg_install_into__spkg_build__and__spkg_install___for_sage_sudo_
comment:4 Changed 3 years ago by
- Commit set to 48053d0af7bef79e41a581aa492e58a285c9feb6
- Description modified (diff)
- Status changed from new to needs_review
comment:5 Changed 3 years ago by
- Description modified (diff)
comment:6 Changed 3 years ago by
- Cc vbraun dimpase added
comment:7 Changed 3 years ago by
- Cc tscrim added
comment:8 follow-up: ↓ 11 Changed 3 years ago by
So, if I understand correctly, the point of calling spkg-install
without $SAGE_SUDO
if no spkg-build
exists, is that for backwards-compatibility such packages should be assumed to be "special" in some way, and have to handle $SAGE_SUDO
on their own. Is that correct?
comment:9 follow-up: ↓ 12 Changed 3 years ago by
What is this about?
+ if [ ! -x spkg-build ]; then + echo >&2 "WARNING: spkg-build is not executable, making it executable" + chmod +x spkg-build + fi
Why not just require it to be executable in the first place?
comment:10 Changed 3 years ago by
LGTM otherwise. This looks like what I had in mind in #21537.
comment:11 in reply to: ↑ 8 Changed 3 years ago by
Replying to embray:
So, if I understand correctly, the point of calling
spkg-install
without$SAGE_SUDO
if nospkg-build
exists, is that for backwards-compatibility such packages should be assumed to be "special" in some way, and have to handle$SAGE_SUDO
on their own. Is that correct?
That's right.
comment:12 in reply to: ↑ 9 Changed 3 years ago by
Replying to embray:
What is this about?
+ if [ ! -x spkg-build ]; then + echo >&2 "WARNING: spkg-build is not executable, making it executable" + chmod +x spkg-build + fiWhy not just require it to be executable in the first place?
It's imitating existing code that does the same for spkg-install
. I don't know why it does that. Hysterical raisins?
comment:13 Changed 3 years ago by
I see that now. Might as well keep it for now then :(
comment:14 Changed 3 years ago by
- Status changed from needs_review to positive_review
comment:15 Changed 3 years ago by
- Reviewers set to Erik Bray
comment:16 Changed 3 years ago by
Thanks!
comment:17 Changed 3 years ago by
- Description modified (diff)
comment:18 Changed 3 years ago by
- Branch changed from u/mkoeppe/support_splitting_spkg_install_into__spkg_build__and__spkg_install___for_sage_sudo_ to 48053d0af7bef79e41a581aa492e58a285c9feb6
- Resolution set to fixed
- Status changed from positive_review to closed
comment:19 Changed 3 years ago by
- Commit 48053d0af7bef79e41a581aa492e58a285c9feb6 deleted
Semi-related, see #22510.
New commits:
If spkg-build exists, run it, followed by $SAGE_SUDO spkg-install
4ti2: Call make install within $SUDO
lcalc: Split out spkg-build from spkg-install