platform release is not a version-like marker

[j-baker]

Version like markers are validated against PEP-440. This allows things like '1.0.0' and disallows things like '1.8.0_60-redhat'. However, this is very valid in platform_release, as described in
PEP-508 which gives samples 3.14.1-x86_64-linode39, 14.5.0, 1.8.0_51.

platform_release should be a generic marker, which I think allows for equality checking only.

Resolves: python-poetry#7418

• Added tests for changed code.
• Updated documentation for changed code.

[j-baker]

this test fails before the change in markers.py

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[jemaw]

This would be helpful to us. With it we can install certain packages/versions only for specific platforms. Could you give this a review poetry-team?

[jemaw]

@dimbleby I see you interacted on the original issue (python-poetry/poetry#7418) could you give this mr a quick review?

[dimbleby]

looks fine to me but i don't have the commit bit so that doesn't help you

[radoering]

I'm a bit on the fence here. If we change platform_release from version-like to generic, packages that use it in a version-like way will not be supported anymore. Doing a quick search for platform_release, I only found pyobjc, which uses platform_release with >=. As it seems this marker is version-like on some platforms...

[dimbleby]

https://packaging.python.org/en/latest/specifications/dependency-specifiers/#environment-markers

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. Otherwise an error should be raised.

ie it seems as though the intended behaviour is a kind of duck-typing: if everything looks like a version then treat everything as a version, and if not then don't.

In particular: if platform_release actually is a version then it's cool to treat it as one.

so yeah, poetry is currently doing it wrong with or without this MR

[dimbleby]

this is probably unsolvable in the context of a tool that is trying to build a cross-platform lockfile - because it can't possibly know whether platform_release will or will not turn out to be version-like at install time

[jemaw]

current behaviour is, that it is assumed that some markers are version like and some are not:
https://github.com/python-poetry/poetry-core/blob/main/src/poetry/core/version/markers.py#L331

        if name in self._VERSION_LIKE_MARKER_NAME:
            parser = parse_version_constraint

From my understanding the standard wants to determine this on the fly and then treat the markers either as version (if both sides are version like) or as generic otherwise.

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.

Is this possible from the way poetry is designed? I could imagine that at install time we just check what type the constraints are, but I'm unclear what happens at lock time.

If this is not possible I would argue that we should treat platform_relaese as generic marker (as proposed in this mr), because the assumption that it is a version is not true in general.

[dimbleby]

because the assumption that it is a version is not true in general.

the assumption that it is not a version is also not true in general, so unfortunately this reasoning is equally convincing as an argument for doing the exact opposite.

[jemaw]

that's true. But from my understanding it is true that it will always work on all platforms, whereas the approach that assumes a version-like marker breaks on a lot of platforms. Meaning the generic marker is less powerful but always works with explicit comparisons, whereas the version-like marker is sometimes more powerful but breaks on a lot of platforms. I would argue that we should go for the approach that is less powerful but always works in this case.

What about the other option, doing both, do you know if that is realistic?

[dimbleby]

from my understanding it is true that it will always work on all platforms

this is not true. If this were true then everything might as well be a non-version marker. version-comparisons are not the same as string-comparisons.

[jemaw]

sorry I think my comment was not precise, I meant that using generic markers with string comparisons for the platform_release marker will work for every platform. It only allows for string comparisons but works on every platform. Whereas converting platform_release to a version-like marker is of course more powerful but will break for many platforms.
If that's not the case then there is a misunderstanding on my side.

[dimbleby]

using string comparisons does not "work" on every platform, it gives wrong answers where version comparisons are required. Thats not just "less powerful": it is broken.

[radoering]

I have not thought it through to the end yet, but a possible solution might be:

• allow the operators <, >, <=, >= in generic constraints
• try to do version comparisons and fallback to string comparisons if necessary

That's some work because all the methods like allows(), allows_all(), allows_any(), invert(), difference(), intersect() and union() have to be considered and there might be blockers that I haven't thought of yet.

Another alternative would be to introduce a new version class (e.g. LegacyVersion) for versions which are not PEP 440 compliant. I already took a look at this approach some time ago, because it would allow to support arbitrary equality constraints. However, it seemed to be quite difficult so I considered it not worth the effort at this point.