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

Include PYTHONPATH in --inherit-path logic. #765

Merged
merged 3 commits into from
Sep 10, 2019

Conversation

jsirois
Copy link
Member

@jsirois jsirois commented Sep 9, 2019

Previously Pex would inherit PYTHONPATH by default despite the default
--inherit-path setting being false. This was buggy behavior running
against the primary Pex goal of providing an isolated execution
environment. Now, to use PYTHONPATH to amend the sys.path of a
running is accomplished by either of:

PEX_INHERIT_PATH=prefer PYTHONPATH=... ./my.pex
PEX_INHERIT_PATH=fallback PYTHONPATH=... ./my.pex

Update corresponding settings documentation and fix pex to consider
PYTHONPATH when adjusting sys.path.

Fixes #707

Previously Pex would inherit `PYTHONPATH` by default despite the default
`--inherit-path` setting being false. This was buggy behavior running
against the primary Pex goal of providing an isolated execution
environment. Now, to use PYTHONPATH to ammend the `sys.path` of a
running is accomplished by either of:
```
PEX_INHERIT_PATH=prefer PYTHONPATH=... ./my.pex
PEX_INHERIT_PATH=fallback PYTHONPATH=... ./my.pex
```

Update corresponding settings documentation and fix pex to consider
PYTHONPATH when adjusting sys.path.

Fixes pex-tool#707
Copy link

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Thanks!

pex/pex.py Outdated Show resolved Hide resolved
pex/pex.py Outdated Show resolved Hide resolved
@jsirois jsirois mentioned this pull request Sep 10, 2019
2 tasks
@jsirois jsirois merged commit e33ccd6 into pex-tool:master Sep 10, 2019
@jsirois jsirois deleted the issues/707/isolation/PYTHONPATH branch September 10, 2019 15:26
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.

Thanks John for fixing this! Looks good.

return pex_info

def assert_isolation(self, inherit_path, expected_output):
env = dict(PYTHONPATH=self.pythonpath)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: collection literal preferred, per Pants style guide. Not worth losing sleep over or fixing retroactively, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hrm, that definitely is not the style guide used in PEX today.String interpolation doesn't even apply for example due to PEX supporting 2.7. Even if it did though, this one I might have wanted to debate a little. For dicts in particular, dict can be viewed as nicer than a literal since the kwarg form used here puts sane restrictions on env var keys as well as making them appropriately "pop out" visually in syntax highlighters. Thats why I tend to use it for env dicts anyhow.

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.

Use of virtualenv-burrito results in a tainted PYTHONPATH element
3 participants