Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DESTDIR support for gap #25044

Closed
embray opened this issue Mar 27, 2018 · 23 comments
Closed

Add DESTDIR support for gap #25044

embray opened this issue Mar 27, 2018 · 23 comments

Comments

@embray
Copy link
Contributor

embray commented Mar 27, 2018

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

Depends on #24599
Depends on #25039
Depends on #23733

Component: build

Keywords: destdir gap

Author: Erik Bray

Branch/Commit: u/embray/build/destdir-gap @ ee25fe7

Issue created by migration from https://trac.sagemath.org/ticket/25044

@embray embray added this to the sage-8.2 milestone Mar 27, 2018
@jdemeyer
Copy link

comment:2

Why not use sdh_configure here?

@embray
Copy link
Contributor Author

embray commented Mar 29, 2018

comment:3

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 29, 2018

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

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 29, 2018

Changed commit from 990b6ad to 1af0242

@embray
Copy link
Contributor Author

embray commented Apr 4, 2018

Changed dependencies from #24599, #25039 to #24599, #25039, #23733

@embray
Copy link
Contributor Author

embray commented Apr 4, 2018

comment:5

(already merges without conflict with #23733)

@embray
Copy link
Contributor Author

embray commented Apr 5, 2018

comment:6

This appears to have some kind of problem installing gap.sh...

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 5, 2018

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 5, 2018

Changed commit from 1af0242 to c14afaa

@embray embray modified the milestones: sage-8.2, sage-8.3 Apr 26, 2018
@saraedum
Copy link
Member

Work Issues: merge conflicts

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 17, 2018

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 17, 2018

Changed commit from c14afaa to 6d1493a

@embray
Copy link
Contributor Author

embray commented Jul 17, 2018

comment:12

I think it should be good now.

@embray
Copy link
Contributor Author

embray commented Jul 17, 2018

Changed work issues from merge conflicts to none

@embray
Copy link
Contributor Author

embray commented Jul 18, 2018

comment:13

I believe this issue can reasonably be addressed for Sage 8.4.

@embray embray modified the milestones: sage-8.3, sage-8.4 Jul 18, 2018
@jdemeyer
Copy link

jdemeyer commented Aug 7, 2018

comment:14

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"

@jdemeyer
Copy link

jdemeyer commented Aug 7, 2018

comment:15

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)

@embray
Copy link
Contributor Author

embray commented Aug 8, 2018

comment:17

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 8, 2018

Changed commit from 6d1493a to ee25fe7

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 8, 2018

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

@embray
Copy link
Contributor Author

embray commented Aug 8, 2018

comment:19

Done all of the above.

@embray
Copy link
Contributor Author

embray commented Sep 6, 2018

comment:20

Bump.

@embray embray modified the milestones: sage-8.4, sage-8.5 Oct 28, 2018
@embray
Copy link
Contributor Author

embray commented Dec 20, 2018

comment:22

This is also implemented by #22626 which supersedes this.

@embray embray removed this from the sage-8.5 milestone Dec 20, 2018
@embray embray closed this as completed Dec 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants