null out pythonpath #252
null out pythonpath #252
Conversation
Fix bugs in logging.info statements.
pipx/util.py
Outdated
| @@ -94,11 +94,12 @@ def get_site_packages(python: Path) -> Path: | |||
| def run(cmd: Sequence[Union[str, Path]], check=True) -> int: | |||
| """Run arbitrary command as subprocess""" | |||
|
|
|||
| env = {k:v for k,v in os.environ.items() if k.upper() != 'PYTHONPATH'} | |||
There was a problem hiding this comment.
@cs01 Should we add some changelog for this?
|
LGTM, but the test case you added failed. |
|
Let me fix the test before merging. Let me know if there’s an easier way to test this. |
|
@laixintao I fixed the tests for python 3.6, added a changelog entry for |
|
A force push was recently done, so this PR will have to be rebased onto origin/master. Apologies for the inconvenience. See #270. |
There was a problem hiding this comment.
Besides black complaining
and your branch having some conflicts now, this looks 👌 to me.
tests/test_run.py
Outdated
| assert 'None' in subprocess.run( | ||
| [sys.executable, "-m", "pipx", "run", "ipython", "-c", "import os; print(os.environ.get('PYTHONPATH'))"], | ||
| universal_newlines=True, | ||
| env={'PYTHONPATH': 'test'}, |
There was a problem hiding this comment.
To fix the windows test run, you'll need to update the existing environment here.
mgedmin/check-manifest@90b8961 👀
There was a problem hiding this comment.
done, waiting for tests 826afdd
docs/changelog.md
Outdated
| @@ -1,3 +1,7 @@ | |||
| 0.14.0.1 | |||
|
|
|||
| - Null out the PYTHONPATH when executing any command to prevent conflicts between pipx dependencices and package dependenies when pipx is installed via homebrew. Homebrew can use pythonpath manipulation instead of virtual environments. (#233) | |||
There was a problem hiding this comment.
Perhaps: Remove the PYTHONPATH environment variable when...?
Also, dependencies
|
@cs01 @michaeljoseph @laixintao just bumping this PR. tests now pass. anything left to fix? |
|
Looks good to me! Thanks |
| @@ -2,6 +2,7 @@ dev | |||
|
|
|||
| - Handle missing interpreters more gracefully (#146) | |||
| - Change `reinstall-all` to use system python by default for apps. Now use `--python` option to specify a different python version. | |||
| - Remove the PYTHONPATH environment variable when executing any command to prevent conflicts between pipx dependencies and package dependenies when pipx is installed via homebrew. Homebrew can use pythonpath manipulation instead of virtual environments. (#233) | |||
There was a problem hiding this comment.
sorry, thought I fixed this. pushed again in my branch but looks like the PR is already merged.
fixes #233
cc @cs01 @laixintao @jayvdb