Skip to content

Conversation

@dqkqd
Copy link
Contributor

@dqkqd dqkqd commented Aug 26, 2022

Thank you for contributing to Pipenv!

The issue

Fixes #5239

The fix

Packages are added into resolver in this order:

  1. Load from Pipfile
  2. Pass into environment variables PIPENV_PACKAGES
  3. Pass into Resolver with the name constraints.
  4. Parsed by pip's parsed_requirements
  5. Convert into InstallRequirement by install_req_from_parsed_requirement
  6. Pass into pip's resolver

After that, we would have all package's markers from resolver.

Thanks @bakhtiary for providing a nice way to overcome this issue by sorting packages.
I think if we want to sort packages, we better sort them after step 5, which is the last step we could intervene.

But even if those packages are sorted, this might not be the best solution. Because markers are generated by pip's resolver, which is something we couldn't control (for example those packages might be reordered inside resolver).

The checklist

  • Associated issue
  • A news fragment in the news/ directory to describe this fix with the extension .bugfix.rst, .feature.rst, .behavior.rst, .doc.rst. .vendor.rst. or .trivial.rst (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

@dqkqd
Copy link
Contributor Author

dqkqd commented Aug 26, 2022

I don't know how to write a test for this one. Testing by running a loop seems heuristic.
I ran on my laptop without 4b9fc02, and look like it works fine, so just sort self._constraints might be enough for this issue.

./stress-lock.sh
attempt 1: greenlet markers="platform_python_implementation == 'CPython'"
attempt 2: greenlet markers="platform_python_implementation == 'CPython'"
attempt 3: greenlet markers="platform_python_implementation == 'CPython'"
attempt 4: greenlet markers="platform_python_implementation == 'CPython'"
attempt 5: greenlet markers="platform_python_implementation == 'CPython'"
attempt 6: greenlet markers="platform_python_implementation == 'CPython'"
attempt 7: greenlet markers="platform_python_implementation == 'CPython'"
attempt 8: greenlet markers="platform_python_implementation == 'CPython'"
^C

@@ -0,0 +1 @@
Sorting ``constraints`` before resolving, which fixes ``pipenv lock`` generates nondeterminism environment markers.
Copy link
Member

Choose a reason for hiding this comment

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

Please rename this file to 5299.bugfix.rst to avoid the merge conflict.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I didn't check that. Fixed.

@matteius matteius requested a review from oz123 August 26, 2022 23:44
@matteius matteius merged commit d33f4ec into pypa:main Aug 27, 2022
@dqkqd dqkqd deleted the issue-5239-nondeterminism-with-markers branch August 28, 2022 04:48
matteius added a commit that referenced this pull request Sep 1, 2022
yeisonvargasf pushed a commit to yeisonvargasf/pipenv that referenced this pull request Nov 19, 2022
* Fixes pipenv lock nondeterminism with environment markers
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.

pipenv lock nondeterminism with environment markers (again)

2 participants