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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 5 commits into from Dec 12, 2022
Merged

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

merged 5 commits into from Dec 12, 2022

Conversation

AndydeCleyre
Copy link
Contributor

@AndydeCleyre AndydeCleyre commented Feb 9, 2022 •
edited by atugushev

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 AndydeCleyre mentioned this pull request Feb 9, 2022
Closed
@AndydeCleyre

This comment was marked as outdated.

Sign in to view
@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.

Sign in to view
@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
richafrank
richafrank reviewed Feb 20, 2022
View reviewed changes
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
@AndydeCleyre
Also treat --upgrade-packages as '-c constraints.txt' style constraints
103623c 
Ensure temporary constraints file gets cleaned up
in a way that avoids Windows file handling complications
@AndydeCleyre
Don't add upgrade pkgs which are also existing pins as regular reqs
10808bf 
Remove leftovers of now unused existing_pins_to_upgrade set
@AndydeCleyre
Parametrize test_upgrade_packages_option_subdependency
051c72c 
... to cover when the subdependency is not already pinned
@AndydeCleyre
Parametrize combined --upgrade and -P test
91a1c0e 
... 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
atugushev
atugushev approved these changes Dec 12, 2022
View reviewed changes
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
@chrysle chrysle mentioned this pull request May 22, 2023
Draft
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richafrank richafrank richafrank left review comments

@atugushev atugushev atugushev approved these changes

Assignees
No one assigned
Labels
bug Something is not working
Projects
None yet
Milestone
6.12.0
3 participants
@AndydeCleyre @richafrank @atugushev