-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Attempt to clarify environment marker evaluation #1988
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
base: main
Are you sure you want to change the base?
Attempt to clarify environment marker evaluation #1988
Conversation
Preparation for the release of packaging 25.1 revealed multiple deficiencies in the specification of environment marker evaluation. Review of the proposed amendments to resolve those deficiencies highlighted multiple other problems, including some dating from the original PEP 508 specification: * other pages still referencing PEP 508 instead of the living spec * direct reference to PEP 685 instead of the core metadata spec * the "extra" special case not being properly defined * lacking guidance to tool developers regarding what should be considered errors to disallow entirely vs issues to work around Inspired by the initial PR at pypa#1971
for more information, see https://pre-commit.ci
|
|
||
| For ``Set of String`` fields, as there is no marker syntax for set literals, | ||
| the only valid operations are ``in`` and ``not in`` comparisons with a user | ||
| supplied string literal as the left operand. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Publishing tools should emit an error for other operations, consuming tools may just treat the operations as false
| defined as they are for ``String`` fields. However, there is no expectation | ||
| that the parsing of the marker field value or the user supplied constant as a | ||
| valid version will succeed, so tools MUST fall back to processing the field as | ||
| a ``String`` field. Alternatively, tools MAY unconditionally treat such fields |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The established use of platform_release as a version on macOS means that this field really does need to be checked as a version first.
Due to iOS and Android, platform_version potentially needs the same handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
platform_release is only useful if it's a Version. The string fallback means you can't use it (unless you really are shipping a package for literally only one exact version of a specific platform - maybe also on Windows if this just always reports "11"). So we do want the "Version" behavior to be required, with a string fallback, just like Version.
To me, the difference between Version and Version | String is that we are telling the user that we expect it to always be a Version, vs. we expect it to fallback to something useless (but not an error). This generally means a user can't rely on Version | String unless it's gated by something that ensures it's a Version, like a check for the platform.
It would be nice if we had some sort of "always Version" field instead, but it's defined as just the raw underlying platform.release and it would be a much bigger effort to develop some sort of always-Version transform for every platform, so I think we should just try to make the best of what we have for now.
I think platform_version is always a String? It's not convertible to a version on macOS, only platform.release() is. Also Pydodide (emscripten) it's not a version. We don't consider it to be possible to convert it to a version here: https://github.com/pypa/packaging/blob/7c0307115f734c1a6b7b54bf676ed1b635fc7c16/src/packaging/markers.py#L37-L41
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I realize there's nothing actionable above. I'd like to require Version | String to do the Version processing if possible for platform_release; that way it is possible for a package to build a marker using platform-specific versioning on some platforms, like macOS. If we leave that up to tools, then that becomes tool specific, instead of just platform specific.
| ''''''''''''''''''''' | ||
|
|
||
| References to unknown marker fields MUST raise an error rather than resulting | ||
| in a comparison that evaluates to True or False. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explain that this so unknown markers result in a clear installation failure rather than an installation with potentially missing dependencies that then fails at runtime. Soften it to a SHOULD.
Preparation for the release of packaging 25.1 revealed multiple deficiencies in the specification of environment marker evaluation. Review of the proposed amendments to resolve those deficiencies highlighted multiple other problems, including some dating from the original PEP 508 specification:
Inspired by the initial PR at #1971
discuss.python.org thread: https://discuss.python.org/t/spec-change-bugfix-dependency-specifiers-simplification-pep-508
📚 Documentation preview 📚: https://python-packaging-user-guide--1988.org.readthedocs.build/en/1988/