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

Running pip from python and versioning #119

Closed
ghost opened this issue Sep 12, 2018 · 6 comments
Closed

Running pip from python and versioning #119

ghost opened this issue Sep 12, 2018 · 6 comments

Comments

@ghost
Copy link

ghost commented Sep 12, 2018

I was about to make a PR for this, but I thought I'd first ask the question to see if there was a reason for this design decision.

--

In short: I suggest you make both the python version (at least python2 vs python3) used by this tool and the pip version (at least pip2 vs pip3) configurable. In doing so, move from using the pip module directly to invoking pip as a subprocess.

In long:

I ran into a problem using this to manage pip dependencies that require python 3 (pytorch, in my example). The root of the issue is that these rules are hard-coded to use python2. So I cloned the repo and changed it to python3. This was mostly fine, but now it had an issue invoking pip, essentially because pip's internal APIs are not supported and should not be called from production code (see, for example, this issue). The solution, as suggested by pip documentation, is to call pip as a subprocess.

How do you feel about changes along these lines? What was the original reasoning behind 1) hard-coding python2 and 2) calling pip from internal unsupported APIs?

@jmhodges
Copy link

I think I'm hitting this.

I'm trying to depend the tensorflow pip package when building on darwin, and tensorflow-gpu when running on linux, but at pip_import time, I get an error saying there's no matching tensorflow-gpu 1.12.0 version. (I know that tensorflow-gpu doesn't support python2, so I suspect that's the issue.)

(Tangentially, if there's another way to do platform specific requirements.txt's, I love to hear it! select wasn't working in my WORKSPACE)

@ashwin153
Copy link

@johnmarkwayve Could you upload the patch to get it working with python3-only dependencies?

@ghost
Copy link
Author

ghost commented Mar 25, 2019

@ashwin153 I'm afraid that 6 months later we've moved on to a different way to manage pip dependencies (by calling pip in a genrule, for those interested). I don't have the patch to hand or the time to dig it out I'm afraid. I don't remember it being a difficult change though.

@ashwin153
Copy link

ashwin153 commented Mar 26, 2019

Could you post a minimal example of getting pip3 working in a genrule? I can't seem to find a good example online.

@ghost
Copy link
Author

ghost commented Mar 26, 2019

Sure, this isn't precisely what we use (which is wrapped in a custom repository rule that does a bunch of things I'm not comfortable sharing) and may not work out-of-the-box, but should be a start:

genrule(
    name = "pip-install",
    srcs = [],  # You may need to add dependencies here if the pip install requires them
    cmd = """
DUMMY_HOME=/tmp/$$(head /dev/urandom | tr -dc A-Za-z0-9 | head -c 8)
PYTHON_LOCATION="/usr/bin/env python3"
rm -rf $$DUMMY_HOME
mkdir -p $$DUMMY_HOME
HOME=$$DUMMY_HOME $$PYTHON_LOCATION -m \
  pip install --no-cache-dir --disable-pip-version-check --no-build-isolation \
  --target=$@ {{PIP_SPEC}}
rm -rf $$DUMMY_HOME
""",
    outs = ["pip_install"],
)

Where {{PIP_SPEC}} is what you'd normally write with pip install {{PIP_SPEC}}.

The idea is to install into a directory called pip_install, which you can then use for input to other rules. The whole DUMMY_HOME idea is to avoid pip from picking up any libraries that may exist in the user's ~/.local/lib (that urandom spell is just to create a random direcetory so concurrent builds won't conflict).

Like I say, this is incomplete (and we're not entirely happy with it either), but it should be enough to get you going. I also very much doubt this works on Windows.

@brandjon
Copy link
Member

Dupping this out to #249 (pip3 support) and #132 (remove call to pip.main()).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants