Fix `pip-sync --python-executable` evaluating markers for the wrong environment

jazzband / pip-tools

A set of tools to keep your pinned Python dependencies fresh.

https://pip-tools.rtfd.io

BSD 3-Clause "New" or "Revised" License

7.69k stars 610 forks source link

Fix `pip-sync --python-executable` evaluating markers for the wrong environment #2116

Open gschaffner opened 1 month ago

gschaffner commented 1 month ago

Proposed patchset that fixes #2115.

I've marked this as a draft PR because it's just a fix; it does not yet include an automated test. The test for this seems difficult to fit into the current testing framework—the test requires access to multiple (differing) interpreters—so I was hoping someone with more familiarity could help with that.

Contributor checklist

[ ] Included tests for the changes.
[x] PR title is short, clear, and ready to be included in the user-facing changelog.

Maintainer checklist

[x] Verified one of these labels is present: backwards incompatible, feature, enhancement, deprecation, bug, dependency, docs or skip-changelog as they determine changelog listing.
[x] Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

gschaffner commented 1 month ago

I do not currently understand the failures of test_python_executable_option (assert 0 == 2) on various piplatest on GHA. tox -r -e "$(echo py{38,39,310,311,312,py3}-pip{previous,latest,main}-coverage | tr ' ' ',')" -- --no-cov -n=0 -v -k test_python_executable_option passes locally.

chrysle commented 1 month ago

The test for this seems difficult to fit into the current testing framework—the test requires access to multiple (differing) interpreters—so I was hoping someone with more familiarity could help with that.

Basically, we might just as well use the same interpreter and a corresponding marker?

gschaffner commented 1 month ago

Basically, we might just as well use the same interpreter and a corresponding marker?

I don't think I understand, where are you suggesting putting a marker?

My understanding is that for the bug to occur, InstallRequirement.markers.evaluate(environment) needs to produce a different result with the pip-sync interpreter's environment and with the --python-executable=... interpreter's environment. AFAIK this is only possible if the two environments differ, and AFAIK that is only possible if they are not the same interpreter.

gschaffner commented 1 month ago

Applied review suggestions and fixed a crash on Windows.