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 PEX environment setup. #531

Merged
merged 1 commit into from
Jul 28, 2018
Merged

Fix PEX environment setup. #531

merged 1 commit into from
Jul 28, 2018

Conversation

jsirois
Copy link
Member

@jsirois jsirois commented Jul 27, 2018

Previously, if a PEX had a custom interpreter, it was not threaded
through to the environments it activated. Add a test that the
designated interpreter is used.

Fixes #522

@jsirois
Copy link
Member Author

jsirois commented Jul 27, 2018

With the new test added but the fix not applied, the test failed like so:

$ tox -epy27
...
tests/test_pex.py::test_activate_interpreter_different_from_current Failed to execute PEX file, missing linux_x86_64-cp-27-cp27mu compatible dependencies for:
my-project
...

@jsirois
Copy link
Member Author

jsirois commented Jul 27, 2018

Hrm. Locally I was seeing the errors of the form seen in CI - randomly :(:

tests/test_pex.py::test_activate_interpreter_different_from_current **** Failed to install tmpiEkmjW (caused by: NonZeroExit("received exit code 1 during execution of `['/home/travis/build/pantsbuild/pex/.pyenv_test/versions/3.6.3/bin/python3.6', '-', 'bdist_wheel', '--dist-dir=/tmp/tmpnPulvz']` while trying to execute `['/home/travis/build/pantsbuild/pex/.pyenv_test/versions/3.6.3/bin/python3.6', '-', 'bdist_wheel', '--dist-dir=/tmp/tmpnPulvz']`",)
):
stdout:
stderr:
usage: setup.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
   or: setup.py --help [cmd1 cmd2 ...]
   or: setup.py --help-commands
   or: setup.py cmd --help
error: invalid command 'bdist_wheel'

On successful runs I could copy the PYTHONHASHSEED and then re-run with tox --hashseed=<value> ... and get consistent success. I'll dig into the randomness in the am.

@jsirois
Copy link
Member Author

jsirois commented Jul 27, 2018

OK - this exposes a longstanding bug with extras - order is important when importing setuptools. If wheel is not on the sys.path when it is imported, no wheel distutils commands will be discovered. I'll try a fix to get CI green and then break that out as a seperate PR.

Copy link
Contributor

@CMLivingston CMLivingston left a comment

Choose a reason for hiding this comment

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

This LGTM, however I hit a test failure on the py36-requests environment:

**** Failed to install tmps18ib62q (caused by: NonZeroExit("received exit code 1 during execution of `['/Users/clivingston/workspace/pex/.pyenv_test/versions/2.7.10/bin/python2.7', '-', 'bdist_wheel', '--dist-dir=/var/folders/7n/x4k4tqh94k9gwm61g046m2340000gn/T/tmp39eob7fs']` while trying to execute `['/Users/clivingston/workspace/pex/.pyenv_test/versions/2.7.10/bin/python2.7', '-', 'bdist_wheel', '--dist-dir=/var/folders/7n/x4k4tqh94k9gwm61g046m2340000gn/T/tmp39eob7fs']`",)
):
stdout:

stderr:
usage: setup.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
   or: setup.py --help [cmd1 cmd2 ...]
   or: setup.py --help-commands
   or: setup.py cmd --help

error: invalid command 'bdist_wheel'

I suspect this is specific to my machine, but shouldn't tox be installing wheel for this run?

@jsirois
Copy link
Member Author

jsirois commented Jul 27, 2018

@CMLivingston did your test include 2ec7c0b? That has the fix for the error: invalid command 'bdist_wheel'.

FWIW: without the fix, running tox -epy27 -- -vsktest_activate_interpreter_different_from_current is much quicker and would reveal flake. If the PYTHONHASHSEED rolled the right way, setuptools would be inserted on the sys.path and imported before wheel was on the sys.path, and so distutils command discovery in setuptools would fail to see not-yet-present wheel.

@CMLivingston
Copy link
Contributor

Nope, just pulled and it now works for me! Thanks.

@jsirois
Copy link
Member Author

jsirois commented Jul 27, 2018

OK great. Thanks for giving this a spin @CMLivingston. I'll split that unrelated fix off to its own PR - deserves a test of its own. I'll then rebase this after that PR hits master.

@jsirois
Copy link
Member Author

jsirois commented Jul 27, 2018

And the error: invalid command 'bdist_wheel' issue is old!: #158

@jsirois
Copy link
Member Author

jsirois commented Jul 27, 2018

Depends on #532 which is the 1st two commits - only ff01ec0 needs review in this PR.

Previously, if a PEX had a custom interpreter, it was not threaded
through to the environments it activated. Add a test that the
designated interpreter is used.

Fixes pex-tool#522
@jsirois jsirois merged commit 0b416c7 into pex-tool:master Jul 28, 2018
@jsirois jsirois deleted the issues/522 branch July 28, 2018 04:55
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

Successfully merging this pull request may close these issues.

3 participants