Skip to content
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

Fix missing vias when more than two input files are used #1890

Merged
merged 10 commits into from
Jul 1, 2023

Conversation

lpulley
Copy link
Contributor

@lpulley lpulley commented Jun 26, 2023

Fixes #1853.

The gist of the problem is that

self._constraints_map = {key_from_ireq(ireq): ireq for ireq in constraints}

only kept the last ireq for each key, rather than combining them (and thus their sources).

There are two changes here:

  • self._constraints_map is no longer constructed directly from constraints; rather, the items in constraints are first grouped by key into sets, then those sets are "collapsed" via combine_install_requirements() into single InstallRequirements.
  • pinned_ireq._source_ireqs is no longer always a singleton list of the value from self._constraints_map; rather, if the constraint has _source_ireqs, then that list is used as pinned_ireq._source_ireqs. This effectively "unwraps" _source_ireqs (once) if possible.
    • A more sophisticated solution might be to just have writer.py recurse on _source_ireqs when constructing required_by, but that touches more code paths than are necessary to fix this, so I opted for this solution instead. Such recursion would enable combined InstallRequirements to be combined further without issue, but that doesn't seem to be a requirement right now.

The new test_many_inputs_includes_all_annotations is parametrized for 2, 3, and 10 inputs; the 2 case was not broken but should still be tested.


Contributor checklist
  • Provided the tests for the changes.
  • Assure PR title is short, clear, and good to be included in the user-oriented changelog
Maintainer checklist
  • Assure one of these labels is present: backwards incompatible, feature, enhancement, deprecation, bug, dependency, docs or skip-changelog as they determine changelog listing.
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

Copy link
Member

@atugushev atugushev left a comment

Choose a reason for hiding this comment

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

Otherwise, great work! Thanks for the fix 🎉

@atugushev atugushev added the bug Something is not working label Jun 28, 2023
@atugushev atugushev enabled auto-merge (squash) July 1, 2023 07:38
@atugushev atugushev merged commit 7958a71 into jazzband:main Jul 1, 2023
34 checks passed
@lpulley lpulley deleted the fix-jazzband-1853 branch July 3, 2023 14:43
@lpulley
Copy link
Contributor Author

lpulley commented Jul 3, 2023

@atugushev I see this just missed inclusion in 6.14.0 -- any chance of a 6.14.1 to release this sooner rather than later?

And thanks for your review 🙂

@atugushev
Copy link
Member

atugushev commented Jul 6, 2023

@lpulley unfortunately no. The next release will be 7.0.0.

@atugushev atugushev added this to the 7.0.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pip-compile drops annotations when using --resolver=backtracking
3 participants