Marker referencing 'platform_release' fails to evaluate on Linux systems with non-PEP 440 kernel versions #774
Comments
|
The relevant part of PEP 508 seems to be this: Comparisons in marker expressions are typed by the comparison operator. The <marker_op> operators that are not in <version_cmp> perform the same as they do for strings in Python. The <version_cmp> operators use the PEP 440 version comparison rules when those are defined (that is when both sides have a valid version specifier). If there is no defined PEP 440 behaviour and the operator exists in Python, then the operator falls back to the Python behaviour. so in this case it should fall back to a string comparison? |
|
Huh, it should indeed. A PR fixing this would be welcome! |
|
I took a stab in PR #816. I hope that there aren't any devious edge cases in the details, although honestly I would not be surprised 🙃 |
|
Thanks! I think the wording of the PEP could lead to what might be considered some odd edge cases, like the example you put where |
|
The odd edge cases are also mentioned here |
|
In packaging<22 (pip<24.1), it seems these cases were handled with something more intuitive than a string comparison, that was not standard compliant. Today, we are in a situation that fails explicitly in such cases which, in a way, is good, or at least better than changing behavior silently. So if I read correctly, no packaging version has ever been standard compliant for these cases. Since the standard will lead to a non-intuitive solution, and (pip) users did rely on the intuitive solution of packaging<22, maybe there is a case to be made to go back to the standard drawing board first, instead of merging #816. Even if that takes quite some time, affected users can stick to packaging<22 or pip<24.1. If we implement the standard today, it will be a breaking changes for folks who relied on the previous behavior, it will likely please no-one, and it will also lead to a situation that will be much harder to change if/when we want to change the standard because it would then be another breaking change. |
|
I agree with @sbidoul that the spec should be updated, and updating pip/packaging to be in line with the existing spec first is a step in the wrong direction. The string comparison fallback does not seem sensible or useful. Is that |
|
Hmm okay... if the spec is to be updated, would the goal be to formalize the behavior that pip/packaging has used in the past? Or would this be an opportunity to rethink the whole thing and come up with a potentially fresh and more sensible way of handling non-PEP 440 version numbers than lexicographic comparison? In the latter case, I'd be willing to help advance a discussion about what the spec should say, if I knew some people with more established reputations than myself - like you all - would be interested in having it. (as per this discussion on the PR) Or, even in the former case (formalize the historical behavior) I'd be happy to help too, but I think I have little value to add in that situation. Some ideas I've been throwing around in my head
This is not to derail the issue with discussion of what a change proposal (if there is one) should be; just making the point that there's enough material to start that discussion in whatever venue is appropriate. |
|
FYI the spec for environment markers can be found at https://packaging.python.org/en/latest/specifications/dependency-specifiers/#environment-markers and it mirrors what's in PEP 508, but the spec is authoritative.
That means it hasn't been that way since 2021, so a good amount of users have moved on.
If someone wants to bring this up on discuss.python.org and try to get consensus then please feel free to! We don't have to rush to fix this, but we shouldn't leave it in a non-compliant state forever, either.
It will at least please me as this issue can be closed. 😁
The problem w/ that is it assumes that even that is lax enough to parse every Linux distro version. The string fallback has the feature of being simple and it never fails for anyone. But if we try to be clever it could still lead to complaints about why doesn't my Linux distro's version get parsed? And as the project that's going to have to field those complaints, it doesn't make me want to try and change the spec to add a potential 3rd layer to the marker comparison logic. |
I disagree on this point, pip was vendoring packaging < 22 until pretty recently, and the majority of users would be using packaging via
Personally, I noticed because |
|
Yeah, only pip users who have moved to pip 24.1+ have been subject to these rules. And IMO pip users most likely to struggle with these rules are the ones using OS distributed versions of pip, on LTS versions of their OS, and may not upgrade to pip 24.1+ for several years still, e.g. I work with a team currently standardized on pip 19.1, so there may be a very long tail of users hitting this problem. |
Fair enough, my mistake as I thought pip was keeping up more. Regardless, this project is not in the business of going against standards, so someone will have to get the spec changed for us to implement something different than what's on packaging.python.org. |
FYI updating to packaging 22.0+ was a difficult vendoring for pip pypa/pip#11715 / pypa/pip#12300 |
In a conversation on Mastodon, @kevinbowen777 reported an error with a failing version comparison when installing py5 with pdm. With @py5coding and myself, we tracked the cause to markers like
platform_release >= "20.0" and sys_platform == "darwin"that appear in the pdm lock file. Specifically, on Linux,platform_releaseevaluates to the full Linux kernel version as returned byuname -r, which might be something like6.1.0-17-amd64(on Kevin's system) or6.7.0-gentoo(on mine), and so when packaging applies PEP 440 version comparison logic to that version number, it raises anInvalidVersionerror.Here's a simple reproduction on my system:
I know it's not terribly common for packages to use the
platform_releasemarker, but evidently it does happen; in this case when installing py5, something is conditionally depending on pyobjc and is usingplatform_releaseto express that condition. And I didn't find anything in the documentation indicating that this sort of thing shouldn't be allowed. So it certainly at least looks like an issue that needs to be worked around.This is pretty similar to #678, but that issue was about the Python runtime version, where there's a bit of an argument to be made that CPython should stick to PEP 440-compatible version numbers. I'm pretty sure that argument isn't going to fly for distribution-packaged Linux kernel versions. I wasn't sure if you'd rather have a separate issue or just a comment on #678, but I figured I'd start with this and you can absorb it into that other issue if you want.
Original example with pdm and py5
It's worth noting that we haven't figured out how to reproduce this with any other package installer besides pdm. We haven't figured out why exactly that is; evidently there's something that pdm is doing and others don't which is involved in triggering the error. But, considering the simple example I posted above, this clearly doesn't need pdm to be reproducible, and it didn't seem like pdm is using
Marker.evaluate()incorrectly, which is why I'm reporting it here rather than to pdm.The text was updated successfully, but these errors were encountered: