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:

Status badges

Description

Implements #24024 for gap; this one is slightly non-trivial.

Change History (22)

comment:1 Changed 4 years ago by embray

  • Status changed from new to needs_review

comment:2 Changed 4 years ago by jdemeyer

Why not use sdh_configure here?

comment:3 Changed 4 years ago by embray

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 git

  • Commit changed from 990b6adcdd3eac21f6243d1c739ca1194677e1b9 to 1af0242207918efa243cb405d69c89f6656b7d51

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

1af0242Use sdh_configure; clean up some superfluous arguments to the configure script

comment:5 Changed 4 years ago by embray

  • Dependencies changed from #24599, #25039 to #24599, #25039, #23733

(already merges without conflict with #23733)

comment:6 Changed 4 years ago by embray

  • 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 git

  • Commit changed from 1af0242207918efa243cb405d69c89f6656b7d51 to c14afaa0ec53cd34d91a65aa3d29ecc325ca4a44

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

b4ecda9trac 23733: deprecate SAGE64 and CFLAG64
905e4d4Stop supporting SAGE64 except in Numpy
4f85314Merge branch 'u/jdemeyer/no-sage64' into u/embray/build/destdir-gap
c14afaaturns 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 embray

  • Status changed from needs_work to needs_review

comment:9 Changed 3 years ago by embray

  • Milestone changed from sage-8.2 to sage-8.3

comment:10 Changed 3 years ago by saraedum

  • Status changed from needs_review to needs_work
  • Work issues set to merge conflicts

comment:11 Changed 3 years ago by git

  • Commit changed from c14afaa0ec53cd34d91a65aa3d29ecc325ca4a44 to 6d1493a8cc65b6a2fe869e20e1d92b7c52bd1b87

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

b03b468Update to use sage-dist-helpers; add DESTDIR support where possible
19a00c4update package version for buildbot
0a0ae72Use sdh_configure; clean up some superfluous arguments to the configure script
6d1493aturns 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 3 years ago by embray

  • 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 embray

  • 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 3 years ago by jdemeyer

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: Changed 3 years ago by jdemeyer

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 3 years ago by jdemeyer

  • Status changed from needs_review to needs_work

comment:17 in reply to: ↑ 15 Changed 3 years ago by embray

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 3 years ago by git

  • Commit changed from 6d1493a8cc65b6a2fe869e20e1d92b7c52bd1b87 to ee25fe758ae28af6ca79ea49fbe3bfc2d3b0f65b

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

25e9ae1Update to use sage-dist-helpers; add DESTDIR support where possible
392056aupdate package version for buildbot
2da622aUse sdh_configure; clean up some superfluous arguments to the configure script
b50ce9fturns 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
448b6desome review details
ee25fe7move the old cleanup code to an spkg-legacy-uninstall

comment:19 Changed 3 years ago by embray

  • Status changed from needs_work to needs_review

Done all of the above.

comment:20 Changed 3 years ago by embray

Bump.

comment:21 Changed 3 years ago by embray

  • Milestone changed from sage-8.4 to sage-8.5

comment:22 Changed 3 years ago by embray

  • 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.

Note: See TracTickets for help on using tickets.