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

Remove PIP_INSTALL from sage-env #24018

Closed
embray opened this issue Oct 12, 2017 · 15 comments
Closed

Remove PIP_INSTALL from sage-env #24018

embray opened this issue Oct 12, 2017 · 15 comments
Assignees
Milestone

Comments

@embray
Copy link
Contributor

embray commented Oct 12, 2017

#24014 deprecated the PIP_INSTALL environment variable in favor of the sdh_pip_install helper function. This ticket is to remove PIP_INSTALL entirely.

CC: @slel @tscrim @embray @jdemeyer

Component: build

Keywords: pip

Author: Frédéric Chapoton

Branch/Commit: ef11f05

Reviewer: Erik Bray

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

@embray embray added this to the sage-8.2 milestone Oct 12, 2017
@embray embray self-assigned this Oct 12, 2017
@jdemeyer
Copy link

comment:2

Related: some Python packages don't use pip at all:

gambit
numpy
pillow
scons
setuptools

For setuptools, this is obvious because it's a dependency of pip.

@slel
Copy link
Member

slel commented Oct 30, 2017

Changed keywords from none to pip

@slel
Copy link
Member

slel commented Oct 30, 2017

Changed dependencies from #24014 to #23983, #24014, #24092

@slel
Copy link
Member

slel commented Oct 30, 2017

comment:3

In #24014, getting rid of the PIP_INSTALL environment variable was postponed for the sake of work in progress on brial and dot2tex.

Since #24092, brial no longer uses PIP_INSTALL.

Now that #23983 is closed, all that remains to do is to stop using PIP_INSTALL for dot2tex, allowing to remove PIP_INSTALL entirely.

@tscrim
Copy link
Collaborator

tscrim commented Oct 30, 2017

comment:4

Because I care about dot2tex.

@embray embray modified the milestones: sage-8.2, sage-8.3 Apr 26, 2018
@embray embray modified the milestones: sage-8.3, sage-8.4 Jul 18, 2018
@embray embray modified the milestones: sage-8.4, sage-8.5 Oct 28, 2018
@fchapoton
Copy link
Contributor

New commits:

ef11f05work on dot2tex spkg

@fchapoton
Copy link
Contributor

Commit: ef11f05

@fchapoton
Copy link
Contributor

Changed dependencies from #23983, #24014, #24092 to none

@fchapoton
Copy link
Contributor

Branch: u/chapoton/24018

@fchapoton
Copy link
Contributor

comment:9

Jeroen, would you have some time to look at this one ? This should be pretty clear, removal of the last instance of this variable. I have tested the package installation and it worked.

@embray
Copy link
Contributor Author

embray commented Dec 21, 2018

comment:10

In retrospect there is one small possible use for a variable like this, in case users for some particular reason needed to override and/or extend the default flags passed to pip. I could sort of envision a reason for that in theory, though I don't know one explicitly.

In any case, the current implementation wouldn't work that way anyways, and something more like PIP_INSTALL_FLAGS would be better for that use case.

@embray
Copy link
Contributor Author

embray commented Dec 21, 2018

Author: Frédéric Chapoton

@embray
Copy link
Contributor Author

embray commented Dec 21, 2018

Reviewer: Erik Bray

@vbraun
Copy link
Member

vbraun commented Dec 23, 2018

Changed branch from u/chapoton/24018 to ef11f05

@embray
Copy link
Contributor Author

embray commented Dec 28, 2018

comment:12

This tickets were closed as fixed after the Sage 8.5 release.

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

6 participants