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 optional attr python_interpreter to run pip with python 3 #158

Closed
wants to merge 2 commits into from

Conversation

acumon
Copy link

@acumon acumon commented Jan 14, 2019

While it's possible to select which interpreter to run python rules with, pip will always be run with python from the system path.

This is an alternative solution to PR #82 and PR #85. it doesn't add pip3_import(). instead it adds python_interpreter attribute to pip_import(). Rationale is that if people want to use python3 for pip without changing the system's default python, they are likely also want to use python3.6 or python3.7 without changing the system's default python3 (for me it points to python 3.5) Using this approach users can any version of python, including customized built python with the interpreter path specified.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@acumon
Copy link
Author

acumon commented Jan 14, 2019

I signed it!

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

@acumon
Copy link
Author

acumon commented Jan 14, 2019

I signed it!

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

XMC added 2 commits January 14, 2019 10:46
This commit also added --python_interpreter argument to piptool.py to
support this attribute.

Example usage:
pip_import(
    name = 'pip_deps',
    requirements = '//:requirements.txt',
    python_interpreter = 'python3.7',
)
@acumon
Copy link
Author

acumon commented Jan 14, 2019

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@acumon
Copy link
Author

acumon commented Jan 20, 2019

Hi @brandjon, As this PR supersedes #82, Could you review this PR instead and have it merged or leaves some comments?

@beneisner
Copy link

Has there been any progress on this? Very excited to see this change, will help our workflow out a lot.

@akashgangil
Copy link

Can someone please look into this?

@pickledgator
Copy link

+1 for this!

@khurshidatcadre
Copy link

+1

chirayuk added a commit to chirayuk/rules_python that referenced this pull request Apr 23, 2019
Good enough for me until a solution is available in the main repo at
bazelbuild#158
@llchan
Copy link

llchan commented May 16, 2019

Should this relate to --python_top in some way? It seems that if I'm already specifying a py_runtime(...), the pip steps should use the same python, no?

@llchan
Copy link

llchan commented May 16, 2019

Nevermind, seems like that is on its way out. The python toolchain stuff is very actively changing (see e.g. bazelbuild/bazel#7899), so it might be good to let the dust settle there before we make a move here.

@bshashank
Copy link

Bump.

The python toolchain stuff is very actively changing (see e.g. bazelbuild/bazel#7899), so it might be good to let the dust settle there before we make a move here.

I'm not sure it'll help. The rules here are used at the analysis phase - in the WORKSPACE file, and these rules only have access to the repository_ctx, and this object does not have access to toolchains.

So we the workaround of specifying the interpreter in every pip_import invocation seems like a good workaround.

tcwalther pushed a commit to tcwalther/rules_python that referenced this pull request Sep 3, 2019
tcwalther pushed a commit to tcwalther/rules_python that referenced this pull request Sep 3, 2019
tcwalther pushed a commit to tcwalther/rules_python that referenced this pull request Sep 3, 2019
@cristifalcas
Copy link

Hi. We also really need this. Any love for this PR?

@Ragora
Copy link

Ragora commented Oct 15, 2019

I had to create a custom version of this repo just for this capability for work. Would be great to see this pulled officially.

@brandjon
Copy link
Member

brandjon commented Nov 7, 2019

Thanks for the PR Chen. I've prioritized this feature as #249 and rebased your commit in #252.

As @bshashank pointed out, this is pretty independent of Python toolchains because this happens at WORKSPACE evaluation time. I imagine most users will only ever need to set it to "python", "python2", or "python3", so it should be sufficient to add pip2_import / pip3_import wrappers around this feature. But in a pinch, if you had some weird system with a non-standard Python command, you could hack up your WORKSPACE to override the pip_import repo rules with equivalent versions that have a modified python_interpreter line.

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

Successfully merging this pull request may close these issues.