Fix lockfile-only versioning strategy not creating some updates that are expected (v2)#5902
Merged
deivid-rodriguez merged 2 commits intomainfrom May 5, 2023
Merged
Conversation
deivid-rodriguez
force-pushed
the
deivid-rodriguez/fix-bad-lockfile-only-updates
branch
from
9d5eded to
e3776ef
Compare
deivid-rodriguez
changed the title
lockfile-only versioning strategy not creating some updates that are expected (v2)
deivid-rodriguez
force-pushed
the
deivid-rodriguez/fix-bad-lockfile-only-updates
branch
from
e3776ef to
e4d2428
Compare
deivid-rodriguez
force-pushed
the
deivid-rodriguez/fix-bad-lockfile-only-updates
branch
4 times, most recently
from
6c63855 to
f033fbc
Compare
Contributor
Author
|
This PR is now rebased and green again 🎉. Still left to do is the verify that both the original problem and the regression that motivated the original revert no longer happen with this PR. |
deivid-rodriguez
force-pushed
the
deivid-rodriguez/fix-bad-lockfile-only-updates
branch
from
f033fbc to
1521427
Compare
Contributor
Author
This was referenced Apr 13, 2023
deivid-rodriguez
force-pushed
the
deivid-rodriguez/fix-bad-lockfile-only-updates
branch
from
1521427 to
463bc2b
Compare
github-actions
bot
added
L: javascript
L: php:composer
deivid-rodriguez
force-pushed
the
deivid-rodriguez/fix-bad-lockfile-only-updates
branch
from
463bc2b to
d800116
Compare
Contributor
Author
|
I extracted one change to a separate PR so this should be a bit more digestible now. |
jeffwidman
approved these changes
Apr 20, 2023
Member
jeffwidman
left a comment
jeffwidman
left a comment
There was a problem hiding this comment.
This looks 👍 to me, with the caveat that it touches a lot of code and I didn't spend time to deeply investigate every bit.
But what I can see makes sense, and there's reasonable CI around it, so I think we should 🚢 and if anything comes up we can course correct as needed.
deivid-rodriguez
force-pushed
the
deivid-rodriguez/fix-bad-lockfile-only-updates
branch
from
d800116 to
2d1affe
Compare
Currently only the "lockfile_only" and "auto" strategies are supported, and the lockfile_only one is handled separately. However, once the lockfile_only strategy is handled as just another requirement update strategy, this will need to support custom strategies, since that will be passed around. Fix that for now, also making cargo look like all the other update checkers, that support custom strategies.
In addition to avoiding updating Requirements in `RequirementUpdater` classes, the lockfile-only strategy needs to also be checked when we decide which requirements to unlock in order to figure out the target version, because otherwise, even if we don't allow the manifest file to change, we'll still may be allowing higher target versions than the manifest allows, causing the lock file to fall out of sync.
deivid-rodriguez
force-pushed
the
deivid-rodriguez/fix-bad-lockfile-only-updates
branch
from
14a08ac to
c1fa6d8
Compare
Contributor
Author
|
Let's try this again! 🤞 |
deivid-rodriguez
deleted the
deivid-rodriguez/fix-bad-lockfile-only-updates
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Trying to retry #5581, but in a correct way now.
Fixes #5569.