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

Auto-heal on corrupted lockfile with missing deps #6400

Merged

Conversation

composerinteralia
Copy link
Contributor

@composerinteralia composerinteralia commented Feb 22, 2023

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

See #6355 for the original problem.

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

#6355 turned a crash into a nicer error. This commit auto-heals the corrupt lockfile instead.

In this particular case (a corrupt Gemfile.lock with missing dependencies) the LazySpecification will not have accurate dependency information, so we have to materialize the SpecSet to determine there are missing dependencies. We've already got a way to handle this, via SpecSet#incomplete_specs, but it wasn't quite working for this case because we'd get to @incomplete_specs += lookup[name] and lookup[name] would be empty for the dependency.

With this commit we catch it a bit earlier, marking the spec containing the missing dependency as incomplete.

(I had originally opened #6399, but misunderstood a few things there. I closed it and then force pushed to the branch, preventing me from reopening it.)

Make sure the following tasks are checked

@composerinteralia composerinteralia force-pushed the auto-heal-corrupted-lockfile branch 3 times, most recently from 0234ac7 to f7ab4da Compare February 22, 2023 20:05
Following up on rubygems#6355, which
turned a crash into a nicer error message, this commit auto-heals the
corrupt lockfile instead.

In this particular case (a corrupt Gemfile.lock with missing
dependencies) the LazySpecification will not have accurate dependency
information, we have to materialize the SpecSet to determine there are
missing dependencies. We've already got a way to handle this, via
`SpecSet#incomplete_specs`, but it wasn't quite working for this case
because we'd get to `@incomplete_specs += lookup[name]` and
`lookup[name]` would be empty for the dependency.

With this commit we catch it a bit earlier, marking the parent spec
containing the missing dependency as incomplete.
Copy link
Member

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for the nice follow up!

@simi simi added this pull request to the merge queue Feb 28, 2023
Merged via the queue into rubygems:master with commit 6e38d37 Mar 1, 2023
deivid-rodriguez pushed a commit that referenced this pull request Mar 8, 2023
…ockfile

Auto-heal on corrupted lockfile with missing deps

(cherry picked from commit 6e38d37)
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

3 participants