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

Treat --upgrade-packages PKGSPECs as constraints (not just minimums), consistently #1578

Merged
merged 5 commits into from
Dec 12, 2022

Conversation

AndydeCleyre
Copy link
Contributor

@AndydeCleyre AndydeCleyre commented Feb 9, 2022

Fixes #1550

This aims to address two problems:

  • When --upgrade and --upgrade-packages PKGSPEC are used together, and PKGSPEC's package is not in the input file (but is a subdependency), then that package is upgraded beyond PKGSPEC
  • When --upgrade-packages PKGSPEC is used, and PKGSPEC's package is in neither the input file nor a preexisting output file (but is a subdependency), then that package is upgraded beyond PKGSPEC

In other words:

--upgrade-packages fails to constrain subdeps, if either absent from a preexisting output file, or if --upgrade is also passed

Some of the current behavior was informed by discussion at #759.

However my understanding now is that it's useful, expected, and without significant cost to additionally treat --upgrade-packages specs as constraints, thereby resolving the above cases.

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).

@AndydeCleyre

This comment was marked as outdated.

@AndydeCleyre AndydeCleyre force-pushed the bugfix/1550 branch 2 times, most recently from afe728c to 6a6b4c7 Compare February 9, 2022 17:58
@AndydeCleyre AndydeCleyre changed the title additional constraints handling for --upgrade-packages additionally treat --upgrade-packages pkgspecs as constraints Feb 10, 2022
@AndydeCleyre AndydeCleyre changed the title additionally treat --upgrade-packages pkgspecs as constraints treat --upgrade-packages PKGSPECs as constraints, even for yet unpinned subdependencies, or when combined with --upgrade Feb 10, 2022
@AndydeCleyre AndydeCleyre changed the title treat --upgrade-packages PKGSPECs as constraints, even for yet unpinned subdependencies, or when combined with --upgrade treat --upgrade-packages PKGSPECs as constraints, even for yet unpinned subdependencies, or when combined with --upgrade Feb 10, 2022
@AndydeCleyre

This comment was marked as outdated.

@AndydeCleyre
Copy link
Contributor Author

OK I think the new parametrization for test_upgrade_packages_version_option_and_upgrade covers the first bullet, so I'm going to promote this out of draft state.

@AndydeCleyre AndydeCleyre marked this pull request as ready for review February 10, 2022 04:58
@AndydeCleyre AndydeCleyre changed the title treat --upgrade-packages PKGSPECs as constraints, even for yet unpinned subdependencies, or when combined with --upgrade treat --upgrade-packages PKGSPECs as constraints, consistently Feb 10, 2022
Copy link
Contributor

@richafrank richafrank left a comment

Choose a reason for hiding this comment

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

Thanks for working to restore this behavior @AndydeCleyre !

piptools/scripts/compile.py Show resolved Hide resolved
piptools/scripts/compile.py Show resolved Hide resolved
@AndydeCleyre AndydeCleyre force-pushed the bugfix/1550 branch 4 times, most recently from cd3002a to e248a40 Compare April 1, 2022 15:32
@AndydeCleyre AndydeCleyre changed the title treat --upgrade-packages PKGSPECs as constraints, consistently Treat --upgrade-packages PKGSPECs as constraints (not just minimums), consistently Apr 1, 2022
@AndydeCleyre AndydeCleyre force-pushed the bugfix/1550 branch 2 times, most recently from 583104c to e8fd4b0 Compare April 4, 2022 20:44
@AndydeCleyre AndydeCleyre force-pushed the bugfix/1550 branch 2 times, most recently from e187eaa to efaf7e9 Compare June 30, 2022 21:42
@AndydeCleyre AndydeCleyre force-pushed the bugfix/1550 branch 2 times, most recently from 06708b0 to 51d4d41 Compare July 18, 2022 15:49
Ensure temporary constraints file gets cleaned up
in a way that avoids Windows file handling complications
Remove leftovers of now unused existing_pins_to_upgrade set
... to cover when the subdependency is not already pinned
... to cover when the -P pkgs are only subdependencies
@atugushev atugushev changed the title Treat --upgrade-packages PKGSPECs as constraints (not just minimums), consistently Treat --upgrade-packages PKGSPECs as constraints (not just minimums), consistently Dec 12, 2022
@atugushev atugushev added the bug Something is not working label Dec 12, 2022
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.

Finally got to this PR. That looks awesome, thanks!

@atugushev atugushev enabled auto-merge (squash) December 12, 2022 23:39
@atugushev atugushev merged commit 6fc9b91 into jazzband:main Dec 12, 2022
@atugushev atugushev added this to the 6.12.0 milestone Dec 13, 2022
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
3 participants