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

Plumb requirement blacklist through to the pex resolver #5697

Merged
merged 3 commits into from Apr 16, 2018

Conversation

Projects
None yet
4 participants
@CMLivingston
Copy link
Contributor

CMLivingston commented Apr 13, 2018

Problem

The pex resolver errors on PEP508 environment markers when resolving for an incompatible interpreter. A common use case for such markers is to ensure backport libraries are installed for universal wheels. Pex has a workaround in place but needs Pants to do some plumbing with a new config var for it to take full effect in a Pants project.

Solution

Add a resolver-blacklist option for python-setup. Do not add requirements to the requirements pex if its name (key) shows up in the set of keys of the blacklist dictionary.

Result

Users can now define a blacklist dictionary in pants.ini to ignore problematic requirements during the pex resolver step. An example of a valid blacklist

[python-setup]
# functools32 is a backport from 3 to 2 and will error out on Python 3 resolves.
resolver_blacklist: {"functools32": "CPython>3"}

#5696 tracks the removal of these changes and
pantsbuild/pex#456 tracks the long term solution in the pex codebase.

@CMLivingston CMLivingston changed the title Clivingston/blacklist for pex resolver Plumb requirement blacklist through to the pex resolver Apr 13, 2018

@cosmicexplorer
Copy link
Contributor

cosmicexplorer left a comment

@CMLivingston showed me offline the pex pr and the next issue describing the longer-term solution, so thanks for that talk. this pr looks highly legitimate

requirements=[
python_requirement('jupyter')
]
)

This comment has been minimized.

@cosmicexplorer

cosmicexplorer Apr 14, 2018

Contributor

The spacing here is confusing me -- I would assume this would be indented only two spaces? I also don't know if it's the style of this repo or some other one, but I feel like I usually see BUILD files using commas at the end of the last element in a list, e.g.:

python_requirement_library(
  name='reqlib',
  requirements=[
    python_requirement('jupyter'), # comma here
  ], # and here
)

I do this typically only when elements of the list are aligned vertically (as they are here). It's not necessary.

'`functools32`. For example, a valid blacklist is {"functools32": "CPython>3"}. '
'NOTE: this keyword is a temporary fix and will be reverted per: '
'https://github.com/pantsbuild/pants/issues/5696. The long term '
'solution is tracked by: https://github.com/pantsbuild/pex/issues/456.')

This comment has been minimized.

@cosmicexplorer

cosmicexplorer Apr 14, 2018

Contributor

I love help that actually explains the how and why for options, this is great.

@illicitonion
Copy link
Contributor

illicitonion left a comment

Looks reasonable :)

Do you have a timeline for the real fix + revert?

Chris Livingston
@CMLivingston

This comment has been minimized.

Copy link
Contributor

CMLivingston commented Apr 16, 2018

Thanks for taking a look! I would say that we should expect a pex-side long-term fix merged in ~1 month (conservative).

@illicitonion illicitonion merged commit b07052a into pantsbuild:master Apr 16, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment