-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
setuptools pkg_resources fails to detect version properly #419
Comments
Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco): The issue is that with PEP 440, your local version is parsed as a LegacyVersion:
And all LegacyVersions compare less than a proper Version of 0. It's actually the post release tag with the dash in it that's forcing the LegacyVersion. If you can omit that tag, then the version will parse as a proper Version and the requirement will be satisfied. @dstufft: Do you have any recommendations for this issue? |
Original comment by Eric89GXL (Bitbucket: Eric89GXL, GitHub: Eric89GXL): So I looked, and |
Original comment by dstufft (Bitbucket: dstufft, GitHub: dstufft): It sounds like somewhere along the lines someone is using an older version of setuptools. Only older versions mangle the >>> import pkg_resources._vendor.packaging as packaging
>>> packaging.version.parse('1.11.0.dev0+2329eae')
<Version('1.11.0.dev0+2329eae')>
>>> import pkg_resources
>>> pkg_resources.safe_version('1.11.0.dev0+2329eae')
'1.11.0.dev0+2329eae' |
Original comment by dstufft (Bitbucket: dstufft, GitHub: dstufft): Yes (see https://caremad.io/s/IaPtgPLQVE/). I think 8.0 might have been when PEP 440 was implemented in setuptools (and thus when |
Original comment by Eric89GXL (Bitbucket: Eric89GXL, GitHub: Eric89GXL): I figured that the filename was being changed to be safe... I'll try to track down who is actually changing the filename, but I suspect it is their desired (and not unreasonable) behavior. I'm not sure I quite follow your example, though. The problem I'm having here is that the
But that is a bit immaterial I think, since that file isn't actually parsed in FWIW I'm currently running |
Original comment by dstufft (Bitbucket: dstufft, GitHub: dstufft): It works when installed too: https://caremad.io/s/SsluEIrRRO/ |
Original comment by Eric89GXL (Bitbucket: Eric89GXL, GitHub: Eric89GXL): Turns out |
Original comment by dstufft (Bitbucket: dstufft, GitHub: dstufft): We considered doing that in the PEP, however the problem was a version like 1.0-1 is valid as an alternative spelling of 1.0.post1 (1.0-1 was supported in setuptools previously and had use on PyPI). When you create a wheel the |
Original comment by Eric89GXL (Bitbucket: Eric89GXL, GitHub: Eric89GXL): Wouldn't it be more robust to parse the file itself for the version information? Or is that expected to do some bad things / be unreliable / be slow? In any case, I've opened an issue for It looks like back in 2013 it was tried and reverted, due to scipy/scipy@a4e93fb#diff-2eeaed663bd0d25b7e608891384b7298L185 |
Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco): I don't understand the system well enough to say what the implications might be to reading out the file. I'd be open to someone exploring that technique. If the implementation is straightforward and works well enough in a few isolated environments, I'd be willing to push it out to the larger community (as a beta) for comment. |
Original comment by Eric89GXL (Bitbucket: Eric89GXL, GitHub: Eric89GXL): I have a limited knowledge of I could open a PR to open and read the file, perhaps with a |
Original comment by Eric89GXL (Bitbucket: Eric89GXL, GitHub: Eric89GXL): Done, I think -- https://bitbucket.org/pypa/setuptools/pull-requests/144/fix-for-issue-419-read-egg-info-files-to/diff |
Original comment by Eric89GXL (Bitbucket: Eric89GXL, GitHub: Eric89GXL): Yep, works over here. A point of curiosity from my end -- IMO the subsequent changes to |
Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco): Well, I prefer that style personally. It's not a pypa preference, AFAIK. The reason I prefer it is because it's more functional in style and more robust from an analysis point of view. I strive to keep code as flat as possible, as each indented block creates a logical branch, each branch which needs to be analyzed/tested. For example, it wasn't clear to me originally that a return of None when no Version was found was important. Using a functional style means the return value is clearer. I do see, though, how the approach might strike some as convoluted, but in my experience, formulating code in that functional programming way is more robust, and worth recognizing as a useful pattern. Ideally, I'd probably re-write it again to (a) have key/value pairs returned by the iterator, (b) filter on the key, and (c) construct a dictionary whose .get result is the desired value. Good feedback, though. |
Original comment by reinout (Bitbucket: reinout, GitHub: reinout): If I read it correctly, this feature was added between 18.7 and 18.7.1, right? I'm seeing an error (on a colleague's laptop) related to numpy and it smells like the 18.7.1 fix could be the cause. It is probably a corner case. And it probably doesn't even have anything to do with numpy specifically, I suspect that is only a funny coincidence :-) It occurs when I run buildout's bootstrap script (the very latest), which builds a setuptools egg in the tmp directory. The corner case is that there is a
It works just fine with 18.5 and 18.7:
Symlinks that are broken are tricky in python. They exist, but cannot be opened. I've had problems with them, too. Two questions:
|
Original comment by Eric89GXL (Bitbucket: Eric89GXL, GitHub: Eric89GXL): Yeah it looks like the fix causes this problem, because it added looking at the file for more information. I guess the offending line(s) could be put in a |
Original comment by reinout (Bitbucket: reinout, GitHub: reinout): If I understand correctly, opening and reading the file is intended as a fall-back mechanism, right? If so, a |
Original comment by Eric89GXL (Bitbucket: Eric89GXL, GitHub: Eric89GXL): I'm not sure about the performance problems, but IIRC only the Jason, I think that's one advantage of the approach that I originally had with |
Original comment by Eric89GXL (Bitbucket: Eric89GXL, GitHub: Eric89GXL): @reinout reading the contents of the file is actually not a fallback, it's a first choice, and using the filename is now the fallback. It should be more robust to I'll wait to hear back from Jason about if a FWIW the traceback actually points to what seems like the correct offending line: |
Original comment by Eric89GXL (Bitbucket: Eric89GXL, GitHub: Eric89GXL): Although looking at the code, the opening of the file is a bit far from where it's used now, so I'm not actually sure how I'd get it back to using |
Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco): @Eric89GXL First, I did update 'filter' to use ifilter on Python 2 to avoid unnecessary iteration in However, in investigating #469, I decided that there shouldn't be many distinct methods of parsing the metadata, which is why |
Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco): The more I think about the situation that @reinout has encountered, the more I think setuptools is doing the right thing here, because there is a distutils package there that is presumed to have a .egg-info directory, but that directory is improperly configured. In that case, raising an exception seems like the right thing to do. I do recognize that this new error is a regression from the 18.7 and 18.7.1 behavior, which did have an explicit check for 'isfile'. I've updated the FileMetadata class to perform this check in has_metadata, which seems right to me. @reinout, can you test against the latest commit and confirm that this fix addresses the issue? |
Originally reported by: Eric89GXL (Bitbucket: Eric89GXL, GitHub: Eric89GXL)
When installing a package:
In other words, my numpy 1.11.0.dev0 is not seen as satisfying a >= 1.6.1 version requirement. It seems like using
disutils.version.LooseVersion
or so should solve this. I'm happy to look into making a PR if this seems like the right solution.The text was updated successfully, but these errors were encountered: