Skip to content

Treat marker environment values as literal strings - #985

Open
harbinresearcher wants to merge 1 commit into
python-poetry:mainfrom
harbinresearcher:codex/literal-marker-environment
Open

harbinresearcher wants to merge 1 commit into
python-poetry:mainfrom
harbinresearcher:codex/literal-marker-environment

Conversation

@harbinresearcher

@harbinresearcher harbinresearcher commented Oct 9, 2026 •

Copy link
Copy Markdown

Description

Environment-marker evaluation can interpret a platform string as constraint syntax. For example:

from poetry.core.version.markers import parse_marker

marker = parse_marker('"kernel" in platform_version')
marker.validate({"platform_version": "#1 SMP, kernel"})

This raises ValueError on main instead of returning True: the comma turns the environment value into a compound constraint. Pipes, comparison prefixes, empty strings and * can similarly raise errors or change comparisons; leading/trailing spaces are also stripped.

Construct a literal equality Constraint for non-version environment values instead of parsing them as expressions. Version-marker parsing and the special handling of extras stay unchanged. This also fixes apply() and atomic marker unions through their existing validation path.

Add 34 parameterized cases for positive/negative substring comparisons, equality, atomic unions and partial evaluation. On the original implementation, 26 fail and 8 pass; all 34 pass with this change. The tests enter through the public parse_marker() API.

Validation

  • Dependencies installed from the repository's Poetry lock; poetry check --lock passes.
  • Windows Python 3.14.7 default suite: original 2,954 passed; patched 2,988 passed. Both have 3 skipped and 9 integration tests deselected.
  • Linux Python 3.14.8 default suite: original 2,950 passed; patched 2,984 passed. Both have 7 skipped and 9 integration tests deselected.
  • Separate integration suites pass on both platforms: 9 tests each on the original and patched sources, including PEP 517 and pip wheel builds. Linux required installing pip into its initially pip-free test environment; repository dependencies and checks were unchanged.
  • Measured patched product statement coverage is 93% on both platforms, with the changed environment path exercised.
  • Windows Python 3.10.21 and Linux Python 3.15.0rc3 marker suite: 590 passed each.
  • Full pinned pre-commit run --all-files passes; Mypy reports no issues in 152 source files on Windows and Linux.
  • Windows tests run from an ASCII working directory: the original test_path_to_url_win expects an unescaped cwd and fails when run from our Chinese cwd. No tests, exclusions or product code were changed to address that separate baseline issue. Linux uses a local C compiler wrapper for extension fixtures; the initial six missing-compiler failures are resolved in both original and patched runs.

Downstream CI limitation

The downstream run 37893165139 reports 3,309 passed, 34 skipped, 2 expected failures and one failure in test_extractall_sdist_no_symlink_path_traversal_via_directory_symlink: it expects OutsideDestinationError on Python 3.10.22, but no exception is raised.

I reproduced this independently using the unchanged downstream extractall() function at Poetry commit 63bba274f8ea6bc9a064f5328fa6c67bce617cc1 on Linux Python 3.10.22, with no poetry.core modules loaded. The extracted file stays inside the destination and does not escape it. This failure is independent of the marker change; I have not changed extraction code or weakened the downstream test. The core-library matrix has now completed successfully: all 19 platform/Python jobs pass. pre-commit.ci also passes.

AI assistance was used for implementation, tests and this description. A second coding agent performed read-only design and diff reviews and found no blockers; it ran no tests. This is a reproducible code-level bug report, without a claim of a production incident or human review.

  • Added tests for changed code.
  • Updated documentation for changed code. No documentation change is needed for evaluating existing environment values literally; the public API is unchanged.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Needs a human reviewer. If the literal comparison is wrong, Poetry could select or record an incorrect dependency set in a lockfile or environment; reverting would not remove those installed packages or already-written lock data, though they can be repaired by re-resolving and reinstalling.


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant