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

Fix PEXEnvironment platform determination. #568

Merged
merged 4 commits into from
Oct 4, 2018

Conversation

jsirois
Copy link
Member

@jsirois jsirois commented Oct 1, 2018

Have this use pex intrinsics instead of pkg_resources which is known
to report bad values on Apple-shipped inerpreters.

Also fix resolver extended platform determination to support the test
for the fix.

Fixes #511
Fixes #523

@jsirois jsirois force-pushed the issues/523 branch 3 times, most recently from f6e6745 to b92da02 Compare October 1, 2018 18:40
@jsirois jsirois changed the title WIP: Failing test for #523 Fix PEXEnvironment platform determination. Oct 1, 2018
@jsirois jsirois force-pushed the issues/523 branch 3 times, most recently from 12bb2bb to be78f4f Compare October 2, 2018 20:31
Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks really good! ++ for more tests.

pex/environment.py Outdated Show resolved Hide resolved
pex/interpreter.py Show resolved Hide resolved
pex/resolver.py Outdated Show resolved Hide resolved
pex/resolver.py Outdated Show resolved Hide resolved
tests/test_environment.py Outdated Show resolved Hide resolved
Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good, thanks!

Have this use pex intrinsics instead of pkg_resources which is known
to report bad values on Apple-shipped inerpreters.

Also fix resolver extended platform determination to support the test
for the fix.

Fixes pex-tool#511
Fixes pex-tool#523
@jsirois jsirois merged commit d4a3c9d into pex-tool:master Oct 4, 2018
@jsirois jsirois deleted the issues/523 branch October 4, 2018 17:22
jsirois added a commit to jsirois/pex that referenced this pull request Oct 8, 2018
PR pex-tool#571 regressed the half-broken state of having
`--interpreter_constraint` selected interpreters not setup to also
having `--python` selected interpreters also not setup. In addition,
PR pex-tool#568 incorrectly classified the current Platform passed by
`resolve_multi` as a user-specified extended platform specification
breaking custom interpreter resolution. Fix both and add tests that
failed prior to this combination of fixes.

A more comprehensive fix is tracked in part by pex-tool#579.
jsirois added a commit that referenced this pull request Oct 8, 2018
PR #571 regressed the half-broken state of having
`--interpreter_constraint` selected interpreters not setup to also
having `--python` selected interpreters also not setup. In addition,
PR #568 incorrectly classified the current Platform passed by
`resolve_multi` as a user-specified extended platform specification
breaking custom interpreter resolution. Fix both and add tests that
failed prior to this combination of fixes.

A more comprehensive fix is tracked in part by #579.
Laevos pushed a commit to Laevos/pex that referenced this pull request Oct 9, 2018
# The first commit's message is:
pex-tool#572: Allow import of ctypes to be skipped if use_manylinux is false

# This is the 2nd commit message:

Narrow the env marker test. (pex-tool#578)

The jupyter dist is just a meta-dist with fully unconstrained deps on ~6
other dists. This test was added to test environment marker support in
pex, which ipython - not jupyter - leverages heavily.

# This is the 3rd commit message:

Fix resolve regressions introduced by the 1.4.8. (pex-tool#580)

PR pex-tool#571 regressed the half-broken state of having
`--interpreter_constraint` selected interpreters not setup to also
having `--python` selected interpreters also not setup. In addition,
PR pex-tool#568 incorrectly classified the current Platform passed by
`resolve_multi` as a user-specified extended platform specification
breaking custom interpreter resolution. Fix both and add tests that
failed prior to this combination of fixes.

A more comprehensive fix is tracked in part by pex-tool#579.

# This is the 4th commit message:

Cleanup `PexInfo` and `PythonInterpreter`. (pex-tool#581)

Kill an unused type in `PexInfo` as well as our last remaining use of
`pkg_resources.get_platform`. Also kill unused `COMPATIBLE_SETUPTOOLS`
constants in `PythonInterpreter`.

# This is the 5th commit message:

Support environment markers during pex activation. (pex-tool#582)

We've had support for environment markers on the resolve side for a
while and with just a little plumbing we can now support multi-python
pexes with environment-specific requirements.

Fixes pex-tool#456

# This is the 6th commit message:

Revert "Support environment markers during pex activation. (pex-tool#582)"

This reverts commit 5f1f00f.

We want to do a 1.4.9 bugfix release before this ~API change.

# This is the 7th commit message:

Prepare the 1.4.9 release. (pex-tool#588)

Work towards pex-tool#583

# This is the 8th commit message:

Revert "Revert "Support environment markers during pex activation. (pex-tool#582)""

This reverts commit 44ff463.

This restores pex-tool#582 for the 1.5.0 release tracked by pex-tool#585.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants