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 interpreter identity check for non-blacklisted interpreters #5724

Merged
merged 3 commits into from Apr 20, 2018

Conversation

Projects
None yet
3 participants
@CMLivingston
Copy link
Contributor

CMLivingston commented Apr 19, 2018

Problem

The pex resolver blacklist checking in the Pants codebase is too loose and only checks that a requirement name is present in the blacklist before excluding it from the pex builder object. Requirements blacklisted by pants.ini do not get their requirement strings plumbed though to the PEX-INFO metadata of produced pex files and as a result, these pex files cannot import the dependencies they need.

Solution

Check for interpreter identity matches before excluding requirements from the pex builder object in addition to the requirement name check.

Result

Pex files that use a non-blacklisted interpreter for a particular requirement will not have that requirement excluded from PEX-INFO metadata, hence making the requirement importable in the scripts packaged by the pex file.

@UnrememberMe
Copy link
Contributor

UnrememberMe left a comment

small nit. Free feel to ignore.


# Test non-blacklisted backport usage.
python_binary(
name='test_py2',

This comment has been minimized.

@UnrememberMe

UnrememberMe Apr 19, 2018

Contributor

maybe add compatibility to <3 to be specific and remain runnable if/when we switch default to Python3?

This comment has been minimized.

@CMLivingston

CMLivingston Apr 19, 2018

Contributor

Sure thing.

Chris Livingston added some commits Apr 19, 2018

@wisechengyi wisechengyi merged commit 1d82f7c into pantsbuild:master Apr 20, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

wisechengyi added a commit that referenced this pull request Apr 20, 2018

Add interpreter identity check for non-blacklisted interpreters (#5724)
### Problem

The pex resolver blacklist checking in the Pants codebase is too loose and only checks that a requirement name is present in the blacklist before excluding it from the pex builder object. Requirements blacklisted by pants.ini do not get their requirement strings plumbed though to the PEX-INFO metadata of produced pex files and as a result, these pex files cannot import the dependencies they need.

### Solution

Check for interpreter identity matches before excluding requirements from the pex builder object in addition to the requirement name check.

### Result

Pex files that use a non-blacklisted interpreter for a particular requirement will not have that requirement excluded from PEX-INFO metadata, hence making the requirement importable in the scripts packaged by the pex file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment