Opened 4 years ago
Closed 3 years ago
#25044 closed enhancement (duplicate)
Add DESTDIR support for gap
Reported by: | embray | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | build | Keywords: | destdir gap |
Cc: | Merged in: | ||
Authors: | Erik Bray | Reviewers: | |
Report Upstream: | N/A | Work issues: | |
Branch: | u/embray/build/destdir-gap (Commits, GitHub, GitLab) | Commit: | ee25fe758ae28af6ca79ea49fbe3bfc2d3b0f65b |
Dependencies: | #24599, #25039, #23733 | Stopgaps: |
Description
Implements #24024 for gap; this one is slightly non-trivial.
Change History (22)
comment:1 Changed 4 years ago by
- Status changed from new to needs_review
comment:2 Changed 4 years ago by
comment:3 Changed 4 years ago by
Hmm, for some reason I thought it wasn't a standard autoconf configure, but it looks like it is, actually. I don't know why it's being passed some PREFIX
argument. That doesn't seem to do anything.
comment:4 Changed 4 years ago by
- Commit changed from 990b6adcdd3eac21f6243d1c739ca1194677e1b9 to 1af0242207918efa243cb405d69c89f6656b7d51
Branch pushed to git repo; I updated commit sha1. New commits:
1af0242 | Use sdh_configure; clean up some superfluous arguments to the configure script
|
comment:5 Changed 4 years ago by
- Dependencies changed from #24599, #25039 to #24599, #25039, #23733
(already merges without conflict with #23733)
comment:6 Changed 4 years ago by
- Status changed from needs_review to needs_work
This appears to have some kind of problem installing gap.sh
...
comment:7 Changed 4 years ago by
- Commit changed from 1af0242207918efa243cb405d69c89f6656b7d51 to c14afaa0ec53cd34d91a65aa3d29ecc325ca4a44
Branch pushed to git repo; I updated commit sha1. New commits:
b4ecda9 | trac 23733: deprecate SAGE64 and CFLAG64
|
905e4d4 | Stop supporting SAGE64 except in Numpy
|
4f85314 | Merge branch 'u/jdemeyer/no-sage64' into u/embray/build/destdir-gap
|
c14afaa | turns out bin/gap.sh is a symlink to either gap-default64.sh or gap-default32.sh (only one or the other should exist). rather than add explicit support for dereferencing symlinks, for now just copy the real file
|
comment:8 Changed 4 years ago by
- Status changed from needs_work to needs_review
comment:9 Changed 4 years ago by
- Milestone changed from sage-8.2 to sage-8.3
comment:10 Changed 4 years ago by
- Status changed from needs_review to needs_work
- Work issues set to merge conflicts
comment:11 Changed 4 years ago by
- Commit changed from c14afaa0ec53cd34d91a65aa3d29ecc325ca4a44 to 6d1493a8cc65b6a2fe869e20e1d92b7c52bd1b87
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
b03b468 | Update to use sage-dist-helpers; add DESTDIR support where possible
|
19a00c4 | update package version for buildbot
|
0a0ae72 | Use sdh_configure; clean up some superfluous arguments to the configure script
|
6d1493a | turns out bin/gap.sh is a symlink to either gap-default64.sh or gap-default32.sh (only one or the other should exist). rather than add explicit support for dereferencing symlinks, for now just copy the real file
|
comment:12 Changed 4 years ago by
- Status changed from needs_work to needs_review
- Work issues merge conflicts deleted
I think it should be good now.
comment:13 Changed 4 years ago by
- Milestone changed from sage-8.3 to sage-8.4
I believe this issue can reasonably be addressed for Sage 8.4.
comment:14 Changed 4 years ago by
Since SAGE_GAP
is no longer used in the script, it would be more clear to replace
SAGE_GAP="$SAGE_LOCAL/gap" INSTALL_DIR="$SAGE_GAP/$GAP_DIR"
by
INSTALL_DIR="$SAGE_LOCAL/gap/$GAP_DIR"
comment:15 follow-up: ↓ 17 Changed 4 years ago by
I would remove
[ -f bin/gap.sh ] || sdh_die "Error building GAP ('gap.sh' not found)."
You have a commit message turns out bin/gap.sh is a symlink to either gap-default64.sh or gap-default32.sh (only one or the other should exist). rather than add explicit support for dereferencing symlinks, for now just copy the real file
. I would rather see something like that in a comment in the script, otherwise it's quite obscure what sdh_install -T bin/gap-default*.sh "$SAGE_LOCAL/bin/gap"
does.
Given that we have sage-legacy-uninstall
, shouldn't this be moved there?
rm -rf "$INSTALL_DIR" rm -f "$SAGE_LOCAL/gap/latest" rm -f "$SAGE_LOCAL/bin/gap"
(just a question, if there is a good reason to keep it there: fine for me)
comment:16 Changed 4 years ago by
- Status changed from needs_review to needs_work
comment:17 in reply to: ↑ 15 Changed 4 years ago by
Replying to jdemeyer:
Given that we have
sage-legacy-uninstall
, shouldn't this be moved there?rm -rf "$INSTALL_DIR" rm -f "$SAGE_LOCAL/gap/latest" rm -f "$SAGE_LOCAL/bin/gap"(just a question, if there is a good reason to keep it there: fine for me)
Yes, this ticket just predates that being merged; as long as this is still open it makes sense to go ahead and move that stuff into an spkg-legacy-uninstall.
comment:18 Changed 4 years ago by
- Commit changed from 6d1493a8cc65b6a2fe869e20e1d92b7c52bd1b87 to ee25fe758ae28af6ca79ea49fbe3bfc2d3b0f65b
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
25e9ae1 | Update to use sage-dist-helpers; add DESTDIR support where possible
|
392056a | update package version for buildbot
|
2da622a | Use sdh_configure; clean up some superfluous arguments to the configure script
|
b50ce9f | turns out bin/gap.sh is a symlink to either gap-default64.sh or gap-default32.sh (only one or the other should exist). rather than add explicit support for dereferencing symlinks, for now just copy the real file
|
448b6de | some review details
|
ee25fe7 | move the old cleanup code to an spkg-legacy-uninstall
|
comment:19 Changed 4 years ago by
- Status changed from needs_work to needs_review
Done all of the above.
comment:20 Changed 4 years ago by
Bump.
comment:21 Changed 4 years ago by
- Milestone changed from sage-8.4 to sage-8.5
comment:22 Changed 3 years ago by
- Milestone changed from sage-8.5 to sage-duplicate/invalid/wontfix
- Resolution set to duplicate
- Status changed from needs_review to closed
This is also implemented by #22626 which supersedes this.
Why not use
sdh_configure
here?