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

Fail early when writing a lockfile with invalid labels #22054

Closed
wants to merge 1 commit into from

Conversation

Wyverald
Copy link
Member

Today, a repo rule's attributes might contain invalid labels because of unresolved apparent repo names. This could be due to either module extensions generating bad repo definitions, or users providing e.g. bad labels to patches attribute of overrides. This causes us to write a label like @@[unknown repo 'foo' requested from @@]//:bar into the lockfile, which will cause Bazel to crash on a subsequent attempt to parse the lockfile.

This PR fixes it so that we throw an error at the time of writing the lockfile, instead of writing a corrupted lockfile that users can't recover from unless they manually delete it. In the unlikely event that we do have a corrupted lockfile, we also no longer crash, but give a helpful error message instead.

Fixes #21845.

Today, a repo rule's attributes might contain invalid labels because of unresolved apparent repo names. This could be due to either module extensions generating bad repo definitions, or users providing e.g. bad labels to `patches` attribute of overrides. This causes us to write a label like `@@[unknown repo 'foo' requested from @@]//:bar` into the lockfile, which will cause Bazel to crash on a subsequent attempt to parse the lockfile.

This PR fixes it so that we throw an error at the time of writing the lockfile, instead of writing a corrupted lockfile that users can't recover from unless they manually delete it. In the unlikely event that we do have a corrupted lockfile, we also no longer crash, but give a helpful error message instead.

Fixes #21845.
@github-actions github-actions bot added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. awaiting-review PR is awaiting review from an assigned reviewer labels Apr 18, 2024
@fmeum
Copy link
Collaborator

fmeum commented Apr 18, 2024

Could we instead fail when the module extension instantiates these repos? That would seem like a more fitting spot with better context on the error.

@Wyverald Wyverald closed this May 23, 2024
@Wyverald Wyverald deleted the wyv-bad-label-lockfile branch May 23, 2024 23:44
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using invalid patch path with bzlmod + lockfile results in crash and bad lockfile
2 participants