Skip to content

Keep Pipfile shorthand environment markers - #816

Merged
rowkav09 merged 1 commit into
mainfrom
fix-pipfile-shorthand
Oct 2, 2026
Merged

rowkav09 merged 1 commit into
mainfrom
fix-pipfile-shorthand

Conversation

@rowkav09

@rowkav09 rowkav09 commented Oct 2, 2026

Copy link
Copy Markdown
Member

Fixes #815.

Pipfile shorthand keys like sys_platform and python_version were allow-listed but only markers was read, so pywin32 = {version = "*", sys_platform = "== 'win32'"} lost its condition. The marker is now built from the shorthand keys and ANDed with a full markers string, with each part parenthesised so an OR expression keeps its scope. Two regressions fail before the change; the Python adapter suite passes after (245), with Prettier and ESLint clean.

Shorthand keys such as sys_platform and python_version were allow-listed but never read, so the requirement lost its condition. Build the marker from them and AND it with a full markers string, parenthesising each part so an OR expression keeps its scope.
@rowkav09

rowkav09 commented Oct 2, 2026

Copy link
Copy Markdown
Member Author

APPROVE at head 0bde370, with two non-blocking notes.

origin/main (d25c493) is in the head. For each Pipfile package table the parser now collects the full markers string plus any shorthand environment key (sys_platform = "== 'win32'" becomes sys_platform == 'win32') and joins several parts with and, wrapping each part in parentheses. I ran parsePipfileText base vs head on 20 entries and validated every head marker with Python packaging.markers.Marker. A full-marker-only entry (including an OR marker and a marker with quotes) and a plain string dependency are byte-identical to main. A shorthand-only entry used to lose its marker and now keeps it, for one, two and three keys, for in and not in, and in [dev-packages]. Full plus shorthand now gives (platform_machine == 'x86_64') and (sys_platform == 'linux'), and an OR marker plus shorthand gives (os_name == 'nt' or os_name == 'posix') and (python_version < '3.11'), so the OR arm keeps its scope. An empty shorthand value is skipped, a non-string value is ignored as before, and an empty markers plus a shorthand key no longer yields an empty marker. All of these parse as valid markers. Suite 246/246. Fail-first: both new tests fail with main's pipfile.ts under them.

Notes, neither a blocker. First, the key list adds extra, which is not one of pipenv's shorthand marker fields (I read pipenv 2023.12.1 PipenvMarkers: os_name, sys_platform, platform_machine, platform_python_implementation, platform_release, platform_system, platform_version, python_version, python_full_version, implementation_name, implementation_version). A stray extra = "== 'x'" key now becomes an extra == 'x' marker that pipenv would ignore. Second, a shorthand value with no operator (python_version = "3.10") is kept as the invalid marker python_version 3.10; pipenv drops the whole marker when the combined string is invalid. Pipenv also joins parts without parentheses, so for an OR marker plus shorthand the parenthesised form here is the safer reading, not a match for pipenv's exact output.

CI at this head when I looked: CodeFactor, lint, typecheck, image, self-scan, consumer ubuntu x2 and the rest of the jobs succeeded; corpus was still in progress, so not fully CI-green yet. Same-account review.

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.

Pipfile shorthand environment markers are accepted but silently discarded

1 participant