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 incorrect removal of ruby platform when auto-healing corrupted lockfiles #6495

Merged
merged 4 commits into from
Mar 20, 2023

Conversation

deivid-rodriguez
Copy link
Member

What was the end-user or developer problem that led to this PR?

In certain situations, when auto-healing a corrupted lockfile we will end up removing the "RUBY" platform from the lockfile, when not necessary.

This is because we sometimes also "heal" the lockfile PLATFORMS section, because some old lockfiles including the "RUBY" platform incorrectly and can't bundle in newer bundler versions, so we remove it for backwards compatibility.

However, our check to detect these corrupt lockfiles that are locked to the RUBY platform but don't work with it, sometimes was triggering incorrectly.

One is for lockfile with empty SPECS section, another one is for lockfile missing dependencies.

I found these bugs while working on #5700, where I plan to generate a "universal lockfile" by default (with more than just the current platform), so this kind of issue surfaces more since we deal with multiple lockfile platforms in more cases.

What is your fix for the problem, implemented in this PR?

  • For the empty SPECS section case, we explicitly avoid the "incomplete ruby check".
  • For the missing dependencies case, we need to make sure the pre-existing "incomplete" condition of the lockfile does not trick us when checking for incompleteness with respect to the "RUBY" platform.

Fixing the latter bug made me realize we were relying too much on state (keeping @incomplete_platforms and propagating it through SpecSets), making this bug easy to happen. I refactored things by extracting an IncompleteSpecification class that makes things easier to follow and more consistent with how we handle missing specifications.

Make sure the following tasks are checked

@deivid-rodriguez deivid-rodriguez force-pushed the dont-remove-ruby-platform-when-healing branch 2 times, most recently from 1db4569 to 488379e Compare March 17, 2023 12:01
Recent bugs fixed made me realize we were relying on state too much
here. We only need to keep incomplete specs to be able to expire them
and retry resolution without them locked. If we use a separate class, we
can do that more transparently and handle them just like we handle
"missing specs".
@deivid-rodriguez deivid-rodriguez force-pushed the dont-remove-ruby-platform-when-healing branch from 488379e to a20585b Compare March 17, 2023 13:06
@deivid-rodriguez deivid-rodriguez added this pull request to the merge queue Mar 20, 2023
@deivid-rodriguez deivid-rodriguez merged commit 9eeb6a6 into master Mar 20, 2023
@deivid-rodriguez deivid-rodriguez deleted the dont-remove-ruby-platform-when-healing branch March 20, 2023 16:06
deivid-rodriguez added a commit that referenced this pull request Mar 20, 2023
…-healing

Fix incorrect removal of ruby platform when auto-healing corrupted lockfiles

(cherry picked from commit 9eeb6a6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant