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 support for staged installations in packages that use sdh_pip_install #24646

Closed
embray opened this issue Feb 2, 2018 · 46 comments
Closed

Comments

@embray
Copy link
Contributor

embray commented Feb 2, 2018

This implements #22509 for any Python packages that are installed with sdh_pip_install. Passing --root= to pip is essentially the same as using DESTDIR with make install.

This required a few additional changes to a small handful of packages for them to work with staged installation.

Component: build

Author: Erik Bray

Branch/Commit: ff1ccd3

Reviewer: Julian Rüth

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

@embray embray added this to the sage-8.2 milestone Feb 2, 2018
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 26, 2018

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

a591ffbAdded the ability for packages to have a post-install script called spkg-postinst.
599668cThe postinst script should probably be run with SAGE_SUDO if it is set
ac34ecbAdd a paragraph to the docs about spkg-postinst
6519c3fUse $SAGE_SUDO, in case it's set, when moving files from the $SAGE_DESTDIR to $SAGE_LOCAL
454b75bDon't use SAGE_SUDO in these helpers if SAGE_DESTDIR is set
fcd6e5dAdd support for SAGE_DESTDIR to sdh_pip_install, thus making most Python packages work with staged installation

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 26, 2018

Changed commit from efecbb5 to fcd6e5d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 26, 2018

Changed commit from fcd6e5d to fd1719e

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 26, 2018

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

fd1719eAdd support for SAGE_DESTDIR to sdh_pip_install, thus making most Python packages work with staged installation

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

videlec commented May 12, 2018

comment:5

merge failure on 8.3.b0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 14, 2018

Changed commit from fd1719e to d03844b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 14, 2018

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

d03844bAdd support for SAGE_DESTDIR to sdh_pip_install, thus making most Python packages work with staged installation

@embray
Copy link
Contributor Author

embray commented May 14, 2018

comment:7

Rebased since its dependency #24645 has been satisfied.

@embray
Copy link
Contributor Author

embray commented May 14, 2018

Changed dependencies from #24645 to none

@jdemeyer
Copy link

comment:8

What's up with build/pkgs/widgetsnbextension/spkg-postinst? That should just be removed.

@jdemeyer
Copy link

comment:9

For sagenb, why not keep

if [ ! -d mathjax ]; then
    echo >&2 "Error: Cannot symlink mathjax into the SageNB data directory."
    exit 1
fi

It seems like a useful sanity check.

@embray
Copy link
Contributor Author

embray commented May 15, 2018

comment:10

Replying to @jdemeyer:

What's up with build/pkgs/widgetsnbextension/spkg-postinst?

Let me check whether or not that was intended to be in this branch.

That should just be removed.

I don't see how you can assert that without knowing what it is.

@embray
Copy link
Contributor Author

embray commented May 15, 2018

comment:11

Replying to @jdemeyer:

For sagenb, why not keep

You can see just above that it checks that the directory being symlinked exists before making the symlink.

@jdemeyer
Copy link

comment:12

Replying to @embray:

You can see just above that it checks that the directory being symlinked exists before making the symlink.

It's not at all obvious to me that the check before is equivalent to the ln -s succeeding. The check after is simpler and therefore safer.

@jdemeyer
Copy link

comment:13

Replying to @embray:

I don't see how you can assert that without knowing what it is.

I know what it is because I wrote the original. The code which is in that spkg-postinst script used to be in spkg-install but it's no longer needed because of Jupyter improvements. At some point, you must have had a merge/rebase conflict with spkg-install and this left-over spkg-postinst script is fallout from that.

@embray
Copy link
Contributor Author

embray commented May 15, 2018

comment:14

I see, you even removed the jupyter nbextension call a while ago (but probably after I first wrote this). I believe you then, that it's no longer needed.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 16, 2018

Changed commit from d03844b to 00bd3f3

@saraedum
Copy link
Member

Reviewer: Julian Rüth

@vbraun
Copy link
Member

vbraun commented Jul 15, 2018

comment:29

See patchbot

@embray
Copy link
Contributor Author

embray commented Jul 16, 2018

comment:30

Apparently I had asked Jeroen to make this a prerequisite to #24935 (after all, this ticket was opened first and had no reason to wait so long to be merged). But he forgot about that or ignored it (and I likewise forgot about it).

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 16, 2018

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

b1412f5Add support for SAGE_DESTDIR to sdh_pip_install, thus making most Python packages work with staged installation
4d20506try installing the symlink anyways and don't poke around SAGE_LOCAL

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 16, 2018

Changed commit from 449a6a2 to 4d20506

@embray
Copy link
Contributor Author

embray commented Jul 16, 2018

comment:32

The spkg-postinst for Sphinx was simply no longer necessary so I removed it.

@embray
Copy link
Contributor Author

embray commented Jul 18, 2018

comment:33

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
@vbraun
Copy link
Member

vbraun commented Jul 30, 2018

comment:34

You obviously didn't try to build Sage with the ticket

No record that 'sagenb' was ever installed; skipping uninstall
./spkg-install: line 33: syntax error near unexpected token `fi'
./spkg-install: line 33: `fi'

@embray
Copy link
Contributor Author

embray commented Jul 30, 2018

comment:35

Replying to @vbraun:

You obviously didn't try to build Sage with the ticket

That's a bit flippant. Of course I built Sage with this ticket. It's been opened for 6 months and has had to be rebased many times. Perhaps there was a small merge error along the way. But that's not as if it was never tested...

@embray
Copy link
Contributor Author

embray commented Jul 30, 2018

comment:36

Also, the ticket doesn't bump the package versions of the affected packages, so you would only see the issue if rebuilding from scratch. The "obviously" is pretty unnecessary.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2018

Changed commit from 4d20506 to ff1ccd3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2018

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

7643e66Add support for SAGE_DESTDIR to sdh_pip_install, thus making most Python packages work with staged installation
32f5bbetry installing the symlink anyways and don't poke around SAGE_LOCAL
ff1ccd3bump package version on sagenb since its spkg-install changes are slightly non-trivial

@embray
Copy link
Contributor Author

embray commented Jul 30, 2018

New commits:

7643e66Add support for SAGE_DESTDIR to sdh_pip_install, thus making most Python packages work with staged installation
32f5bbetry installing the symlink anyways and don't poke around SAGE_LOCAL
ff1ccd3bump package version on sagenb since its spkg-install changes are slightly non-trivial

@vbraun
Copy link
Member

vbraun commented Aug 5, 2018

Changed branch from u/embray/build/destdir-pip to ff1ccd3

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

5 participants