Treat .pth injected paths as extras. #370
Conversation
kwlzn
force-pushed
the
kwlzn/osxtras
branch
2 times, most recently
from
c1e05bf
to
af6b8bb
Compare
pex/pex.py
Outdated
| if not os.path.exists(dir_path): | ||
| continue | ||
|
|
||
| pth_filenames = (f for f in os.listdir(dir_path) if f.endswith(os.extsep + 'pth')) |
There was a problem hiding this comment.
I've never understood why os.extsep exists. This isn't something that changes across systems. I think .pth would be more readable and technically more precise. Also, it would be more uniform since we don't use extsep anywhere today, and we use . all over the place.
pex/util.py
Outdated
| try: | ||
| f = open(filename, 'rU') # noqa | ||
| except IOError: | ||
| return |
There was a problem hiding this comment.
I can see why site.py does this, but is this still what we want to do in our case?
There was a problem hiding this comment.
I don't think it hurts - otherwise it's possible we accidentally bomb trying to open a potentially problematic pth file that we're trying to passively consume.
| continue | ||
| except Exception: | ||
| # Defer error handling to the higher level site.py logic invoked at startup. | ||
| return |
There was a problem hiding this comment.
Same here. We're not deferring error handling to site.py any more, since we've copy-pasted this out of startup code and into regular userland code.
There was a problem hiding this comment.
the error handling I'm referring to here is the dumping of the traceback and other sys.stderr logging that happens in this case in site.py here.
I don't think we strictly need to do the import bits in our function - but they're basically no-ops afaict - and without testing the import it's possible we'd consume subsequent lines that site.py itself wouldn't respect because of how it breaks out of the processing loop in this case - so I left it in because it felt more correct.
open to stronger opinions on better handling of this tho.
There was a problem hiding this comment.
Oh I see, I misunderstood what gets called when. Seems fine.
| @@ -174,3 +174,35 @@ def test_distributionhelper_egg_assert(): | |||
| 'setuptools' | |||
| ) | |||
| assert len(d.resource_listdir('/')) > 3 | |||
|
|
|||
|
|
|||
| @mock.patch('os.path.exists', autospec=True, spec_set=True) | |||
There was a problem hiding this comment.
Good thing this change fixes the problem that makes mock a pain on OSX... :)
| continue | ||
| except Exception: | ||
| # Defer error handling to the higher level site.py logic invoked at startup. | ||
| return |
There was a problem hiding this comment.
Oh I see, I misunderstood what gets called when. Seems fine.
|
Yay! Thanks for fixing this. It was unbelievably irritating. |
Fixes pex-tool#302 before: [omerta pex (kwlzn/osxtras)]$ pex --version pex 1.2.3 [omerta pex (kwlzn/osxtras)]$ pex six -o /tmp/six_broke.pex [omerta pex (kwlzn/osxtras)]$ PEX_VERBOSE=9 PEX_PYTHON=/usr/bin/python /tmp/six_broke.pex pex: Detected PEX_PYTHON, re-exec to /usr/bin/python ... Python 2.7.10 (default, Jul 30 2016, 19:40:32) [GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.34)] on darwin Type "help", "copyright", "credits" or "license" for more information. (InteractiveConsole) >>> from six import wraps Traceback (most recent call last): File "<console>", line 1, in <module> ImportError: cannot import name wraps >>> import six; six.__file__ '/System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/six.pyc' >>> after: [omerta pex (kwlzn/osxtras)]$ tox -e py27-package ... [omerta pex (kwlzn/osxtras)]$ ./dist/pex27 six -o /tmp/six.pex ... [omerta pex (kwlzn/osxtras)]$ PEX_VERBOSE=9 PEX_PYTHON=/usr/bin/python /tmp/six.pex pex: Detected PEX_PYTHON, re-exec to /usr/bin/python ... pex: Found .pth file: /Library/Python/2.7/site-packages/Extras.pth pex: Found site extra: /System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python pex: Found site extra: /System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/PyObjC ... pex: Tainted path element: /System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python pex: Tainted path element: /System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/PyObjC ... pex: Scrubbing from site-packages: /System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python pex: Scrubbing from site-packages: /System/Library/Frameworks/Python.framework/Versions/2.7/Extras/lib/python/PyObjC ... Python 2.7.10 (default, Jul 30 2016, 19:40:32) [GCC 4.2.1 Compatible Apple LLVM 8.0.0 (clang-800.0.34)] on darwin Type "help", "copyright", "credits" or "license" for more information. (InteractiveConsole) >>> from six import wraps >>> import six; six.__file__ '/Users/kwilson/.pex/install/six-1.10.0-py2.py3-none-any.whl.a99dfb27e60da3957f6667853b91ea52e173da0a/six-1.10.0-py2.py3-none-any.whl/six.pyc' >>>
Fixes #302
before:
after: