Opened 3 years ago
Closed 3 years ago
#25044 closed enhancement (duplicate)
Add DESTDIR support for gap
Reported by:  embray  Owned by:  

Priority:  major  Milestone:  sageduplicate/invalid/wontfix 
Component:  build  Keywords:  destdir gap 
Cc:  Merged in:  
Authors:  Erik Bray  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  u/embray/build/destdirgap (Commits, GitHub, GitLab)  Commit:  ee25fe758ae28af6ca79ea49fbe3bfc2d3b0f65b 
Dependencies:  #24599, #25039, #23733  Stopgaps: 
Description
Implements #24024 for gap; this one is slightly nontrivial.
Change History (22)
comment:1 Changed 3 years ago by
 Status changed from new to needs_review
comment:2 Changed 3 years ago by
comment:3 Changed 3 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 3 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 3 years ago by
 Dependencies changed from #24599, #25039 to #24599, #25039, #23733
(already merges without conflict with #23733)
comment:6 Changed 3 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 3 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/nosage64' into u/embray/build/destdirgap

c14afaa  turns out bin/gap.sh is a symlink to either gapdefault64.sh or gapdefault32.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 3 years ago by
 Status changed from needs_work to needs_review
comment:9 Changed 3 years ago by
 Milestone changed from sage8.2 to sage8.3
comment:10 Changed 3 years ago by
 Status changed from needs_review to needs_work
 Work issues set to merge conflicts
comment:11 Changed 3 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 sagedisthelpers; 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 gapdefault64.sh or gapdefault32.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 3 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 3 years ago by
 Milestone changed from sage8.3 to sage8.4
I believe this issue can reasonably be addressed for Sage 8.4.
comment:14 Changed 3 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 followup: ↓ 17 Changed 3 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 gapdefault64.sh or gapdefault32.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/gapdefault*.sh "$SAGE_LOCAL/bin/gap"
does.
Given that we have sagelegacyuninstall
, 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 3 years ago by
 Status changed from needs_review to needs_work
comment:17 in reply to: ↑ 15 Changed 3 years ago by
Replying to jdemeyer:
Given that we have
sagelegacyuninstall
, 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 spkglegacyuninstall.
comment:18 Changed 3 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 sagedisthelpers; 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 gapdefault64.sh or gapdefault32.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 spkglegacyuninstall

comment:19 Changed 3 years ago by
 Status changed from needs_work to needs_review
Done all of the above.
comment:20 Changed 3 years ago by
Bump.
comment:21 Changed 3 years ago by
 Milestone changed from sage8.4 to sage8.5
comment:22 Changed 3 years ago by
 Milestone changed from sage8.5 to sageduplicate/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?