-
-
Notifications
You must be signed in to change notification settings - Fork 292
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
pex fails with InvalidSpecifier if any dist has a bad version specifier #2289
Comments
@travisdowns I think #2264 may be a bad diagnosis. What prior version of Pex was this same scenario working with? |
Yeah, the bisect on Pex versions leads to some change in 2.1.140:
|
Yeah, this bisects to #2188 which upgraded vendored |
So, @travisdowns Pex could contemplate ignoring bad Instead of continuing down that line of thought, perhaps this can be pre-empted by just fixing libcloud? VCS specifiers are very handy for this sort of thing. |
Closing the loop on the exact error cause. It looks to be right here (which was included in the packaging 23.1 release): pypa/packaging@068a0b5#diff-91bbf04171e694df3ef07a8fffc168bfa43e2f00cb531ee37035987210ae0371R230 IOW, PyPA adding better enforcement of their own spec. |
You could be right: I thought before this change the
2.1.100 |
Libcloud was fixed a couple of years ago but I can't go and fix all the transitive dependees to update their version. Among other places this version of libcloud is the default shipped with the most recent Ubuntu LTS 22.04, a very popular distro.
I don't know much about pex to make a very intelligent comment, but wouldn't treating a failed check in the same way as an unsatisfied python version be better? I.e., don't activate the dist.
Fair enough, but as it is it is totally not hermetric: installing any random package (even non-python one) on the host may bring in this file and its mere existence on the filesystem means that any existing pex just stops working. So in this sense it seems like it is not at all hermetic. |
It would absolutely not avoid this. The same metadata is present in the whl METADATA file.
Ok. But then you'd fail in a different way when the code that needs libcloud failed to import it!
That is just straight up false. That is not what is happening here. See my note above about .whl METADATA.
It's as easy as this...
That is my fork of the project with a patch applied on top of the v3.2.0 tagged commit. |
@travisdowns let me know if my explanation and workaround are sticking for you. If so, I'd like to close this as an answered question. If not, we can discuss in more detail why the workaround is not sufficient or other means of amelioration. |
We did not find any METADATA file with this string, only the PKG-INFO. It comes from the
That seems like a good approach: after all libcloud is broken, so it would be up to the user to update it. "Breaking any pex that tries to use libcloud" seems much better than "breaking every pex on the system as soon as the libcloud package is even installed". Note that we did not need libcloud in any way, it was just incidentally installed on the system for some other reason (probably not even explicitly but just as a transitive dependency of some other package).
I am reporting what I saw.
As above, and perhaps I did not make this clear enough initially, we are not using libcloud. It was just brought in at some unknown point by the host package manager, probably as a transitive dependency. We don't need a fix: we have already "fixed" this issue by removing the unused package, before this bug was reported. I thought I'd report it here for the benefit of pex maintainers and other users but I'm sure starting to regret my choice. |
Yeah, that was totally unclear. You did alot of work figuring out what was going on it seems, which is great, but it was not detailed here. If the bug report included the Pex build time command line and runtime command line (this is a runtime error), for example, it would have made it clear libcloud was not a direct or transitive requirement. You have provided more details now though, which is great.
Ok, that is new information - for me. Thank you. I repro as such: $ cat Dockerfile
FROM ubuntu:22.04
RUN apt update && \
apt install -y \
python3-dev \
python3-distutils \
python3-venv \
python3-libcloud
RUN python3 -m venv pex.venv && \
pex.venv/bin/pip install pex
ENV PATH=pex.venv/bin:$PATH $ docker build -tpex-issues-2289 .
$ docker run --rm -it pex-issues-2289
root@83e1b2ff9d09:/# pex -V
2.1.152
root@83e1b2ff9d09:/# pex cowsay -c cowsay -o cowsay.pex
root@83e1b2ff9d09:/# ./cowsay.pex
Traceback (most recent call last):
File "/usr/lib/python3.10/runpy.py", line 196, in _run_module_as_main
return _run_code(code, main_globals, None,
File "/usr/lib/python3.10/runpy.py", line 86, in _run_code
exec(code, run_globals)
File "/root/.pex/unzipped_pexes/994f8ec4b043ab1eb64b76a7f53d81e99fce2604/__main__.py", line 108, in <module>
bootstrap_pex(__entry_point__, execute=__execute__, venv_dir=__venv_dir__)
File "/root/.pex/unzipped_pexes/994f8ec4b043ab1eb64b76a7f53d81e99fce2604/.bootstrap/pex/pex_bootstrapper.py", line 627, in bootstrap_pex
pex.PEX(entry_point).execute()
File "/root/.pex/unzipped_pexes/994f8ec4b043ab1eb64b76a7f53d81e99fce2604/.bootstrap/pex/pex.py", line 542, in execute
self.activate()
File "/root/.pex/unzipped_pexes/994f8ec4b043ab1eb64b76a7f53d81e99fce2604/.bootstrap/pex/pex.py", line 242, in activate
self.patch_sys()
File "/root/.pex/unzipped_pexes/994f8ec4b043ab1eb64b76a7f53d81e99fce2604/.bootstrap/pex/pex.py", line 445, in patch_sys
new_sys_path, new_sys_path_importer_cache, new_sys_modules = self.minimum_sys(inherit_path)
File "/root/.pex/unzipped_pexes/994f8ec4b043ab1eb64b76a7f53d81e99fce2604/.bootstrap/pex/pex.py", line 415, in minimum_sys
sys_path, sys_path_importer_cache = self.minimum_sys_path(isolated_sys_path, inherit_path)
File "/root/.pex/unzipped_pexes/994f8ec4b043ab1eb64b76a7f53d81e99fce2604/.bootstrap/pex/pex.py", line 351, in minimum_sys_path
site_distributions.update(all_distribution_paths(path_element))
File "/root/.pex/unzipped_pexes/994f8ec4b043ab1eb64b76a7f53d81e99fce2604/.bootstrap/pex/pex.py", line 345, in all_distribution_paths
locations = set(dist.location for dist in find_distributions([path]))
File "/root/.pex/unzipped_pexes/994f8ec4b043ab1eb64b76a7f53d81e99fce2604/.bootstrap/pex/pex.py", line 345, in <genexpr>
locations = set(dist.location for dist in find_distributions([path]))
File "/root/.pex/unzipped_pexes/994f8ec4b043ab1eb64b76a7f53d81e99fce2604/.bootstrap/pex/dist_metadata.py", line 1004, in find_distributions
location=location, metadata=DistMetadata.from_metadata_files(metadata_files)
File "/root/.pex/unzipped_pexes/994f8ec4b043ab1eb64b76a7f53d81e99fce2604/.bootstrap/pex/dist_metadata.py", line 758, in from_metadata_files
requires_python=requires_python(metadata_files),
File "/root/.pex/unzipped_pexes/994f8ec4b043ab1eb64b76a7f53d81e99fce2604/.bootstrap/pex/dist_metadata.py", line 516, in requires_python
return SpecifierSet(python_requirement)
File "/root/.pex/unzipped_pexes/994f8ec4b043ab1eb64b76a7f53d81e99fce2604/.bootstrap/pex/vendor/_vendored/packaging_23_1/packaging/specifiers.py", line 714, in __init__
parsed.add(Specifier(specifier))
File "/root/.pex/unzipped_pexes/994f8ec4b043ab1eb64b76a7f53d81e99fce2604/.bootstrap/pex/vendor/_vendored/packaging_23_1/packaging/specifiers.py", line 245, in __init__
raise InvalidSpecifier(f"Invalid specifier: '{spec}'")
pex.vendor._vendored.packaging_23_1.packaging.specifiers.InvalidSpecifier: Invalid specifier: '>=3.5.*'
root@83e1b2ff9d09:/# apt remove -y python3-libcloud
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
The following packages were automatically installed and are no longer required:
python3-certifi python3-cffi-backend python3-chardet python3-cryptography python3-idna python3-lockfile python3-pkg-resources python3-requests python3-simplejson python3-six python3-urllib3
Use 'apt autoremove' to remove them.
The following packages will be REMOVED:
python3-libcloud
0 upgraded, 0 newly installed, 1 to remove and 12 not upgraded.
After this operation, 19.6 MB disk space will be freed.
(Reading database ... 14211 files and directories currently installed.)
Removing python3-libcloud (3.2.0-2) ...
root@83e1b2ff9d09:/# ./cowsay.pex -t 'Moo!'
____
| Moo! |
====
\
\
^__^
(oo)\_______
(__)\ )\/\
||----w |
|| ||
root@83e1b2ff9d09:/# |
With this reproducer where which pex commit does the failure bisect to? |
I'm sure its the one you identified. That seems clear without bisecting. The fuller picture is what I needed here. I apologize for not being able to read the tea leaves of the initial bug report. |
Alrighty, this should clean up easily. The Pex runtime was doing alot of un-necessary work since ~this date 9 years ago it appears in its sys.path scrubbing implementation. Before this change, each scrubbed distribution in each scrubbed path element was logged, but no more. Since then even that bit of use has been absent, and collecting dists was un-needed work. |
Aha, thinking more, in 2014 there were egg zips as possible |
Thanks for the RCA, and apology accepted. |
I am becoming pretty frustrated with OSS expectations. What part of my behavior do you consider unsatisfactory? I apologized for nothing here. I was just trying to run down a bug, and sometimes that takes 20 questions. |
Ah, I guess I did apologize - but that was a bit tongue in cheek. |
FWIW this bug will be fixed and released in under 8 hours start to finish. I would guess you don't get that good a response time in many OSS projects; especially those maintained gratis. I am not payed for this or any other software work. |
This was a relic of the days of egg zips; and the `dist.location` data is now fully redundant with the `sys.path` element input to find the dist in the 1st place. Fixes pex-tool#2289
The fix will go out in 2.1.153: #2291 |
Thanks, the quick turnaround is much appreciated. |
This was a relic of the days of egg zips; and the `dist.location` data is now fully redundant with the `sys.path` element input to find the dist in the 1st place. Fixes #2289
Ok, using the same Dockerfile from above, the repro case is now fixed:
|
Very thorough and quick, thanks John. |
For the record, we ran into a similar case which seems to be fixed with this fix:
which got glued together to a bad requirement string (an attempt at nesting quotes):
|
After upgrading from 2.1.100 to 2.1.151
and so pulling #2264pex
started failing to execute locally with the following error:We managed to trace this back to
apache_libcloud-3.2.0.egg-info/PKG-INFO
which has the following line:Although this seems to be a bad specifier , some components presumably accept it as the package was working in that state for years, but not the
packaging
stuff used bypex
.Now this was a bug in libcloud [1], but I think
pex
should not fail entirely if some package has this in its version info.This was exposed by #2264 because before that thisPKG-INFO
file was not parsed at all.In any case libcloud has been fixed years ago apparently but of course not every transitive dependency has necessarily been updated to use a fixed version, etc.
The text was updated successfully, but these errors were encountered: