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

Crash on tilde separators in cache #23322

Closed
cramertj opened this issue Aug 16, 2024 · 5 comments
Closed

Crash on tilde separators in cache #23322

cramertj opened this issue Aug 16, 2024 · 5 comments
Assignees
Labels
P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug

Comments

@cramertj
Copy link
Contributor

cramertj commented Aug 16, 2024

Description of the bug:

When attempting to update Pigweed from 8.0.0-pre.20240618.2 to 8.0.0-pre.20240812.1 we hit the following error when building any target (e.g. bazelisk build //pw_chrono:system_clock):

Starting local Bazel server and connecting to it...
Computing main repo mapping:
    Fetching https://bcr.bazel.build/modules/stardoc/0.7.0/source.json
FATAL: bazel crashed due to an internal error. Printing stack trace:
java.lang.RuntimeException: Unrecoverable error while evaluating node 'REPOSITORY_DIRECTORY:@@fuchsia_sdk' (requested by nodes 'PACKAGE_LOOKUP:@@fuchsia_sdk//fuchsia')
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:547)
	at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:435)
	at java.base/java.util.concurrent.ForkJoinTask$AdaptedRunnableAction.exec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.scan(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(Unknown Source)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(Unknown Source)
Caused by: java.util.concurrent.CompletionException: com.google.devtools.build.lib.cmdline.LabelSyntaxException: invalid repository name '_main~_repo_rules~cipd_client': repo names may contain only A-Z, a-z, 0-9, '-', '_', '.' and '+'
	at com.github.benmanes.caffeine.cache.LocalLoadingCache.lambda$newMappingFunction$3(LocalLoadingCache.java:204)
	at com.github.benmanes.caffeine.cache.BoundedLocalCache.lambda$doComputeIfAbsent$13(BoundedLocalCache.java:2451)
	at java.base/java.util.concurrent.ConcurrentHashMap.compute(Unknown Source)
	at com.github.benmanes.caffeine.cache.BoundedLocalCache.doComputeIfAbsent(BoundedLocalCache.java:2449)
	at com.github.benmanes.caffeine.cache.BoundedLocalCache.computeIfAbsent(BoundedLocalCache.java:2432)
	at com.github.benmanes.caffeine.cache.LocalCache.computeIfAbsent(LocalCache.java:107)
	at com.github.benmanes.caffeine.cache.LocalLoadingCache.get(LocalLoadingCache.java:57)
	at com.google.devtools.build.lib.cmdline.RepositoryName.createUnvalidated(RepositoryName.java:98)
	at com.google.devtools.build.lib.rules.repository.RepoRecordedInput$RepoCacheFriendlyPath.parse(RepoRecordedInput.java:221)
	at com.google.devtools.build.lib.rules.repository.RepoRecordedInput$File$1.parse(RepoRecordedInput.java:267)
	at com.google.devtools.build.lib.rules.repository.RepoRecordedInput.parse(RepoRecordedInput.java:103)
	at com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction$DigestWriter.readMarkerFile(RepositoryDelegatorFunction.java:712)
	at com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction$DigestWriter.areRepositoryAndMarkerFileConsistent(RepositoryDelegatorFunction.java:675)
	at com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction$DigestWriter.areRepositoryAndMarkerFileConsistent(RepositoryDelegatorFunction.java:649)
	at com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction.compute(RepositoryDelegatorFunction.java:199)
	at com.google.devtools.build.skyframe.AbstractParallelEvaluator$Evaluate.run(AbstractParallelEvaluator.java:467)
	... 7 more
Caused by: com.google.devtools.build.lib.cmdline.LabelSyntaxException: invalid repository name '_main~_repo_rules~cipd_client': repo names may contain only A-Z, a-z, 0-9, '-', '_', '.' and '+'
	at com.google.devtools.build.lib.cmdline.LabelParser.syntaxErrorf(LabelParser.java:208)
	at com.google.devtools.build.lib.cmdline.RepositoryName.validate(RepositoryName.java:177)
	at com.google.devtools.build.lib.cmdline.RepositoryName.lambda$static$0(RepositoryName.java:68)
	at com.github.benmanes.caffeine.cache.LocalLoadingCache.lambda$newMappingFunction$3(LocalLoadingCache.java:197)
	... 22 more

A bisect points at #23103. It seems like there was an intentional choice to change the separator. Deleting MODULE.bazel.lock does not solve the issue, nor does bazelisk clean --expunge.

Which category does this issue belong to?

Core

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Clone https://pigweed.googlesource.com/pigweed/pigweed/
Update .bazelversion to 8.0.0-pre.20240812.1
bazelisk build //pw_chrono:system_clock (or any other target)

@github-actions github-actions bot added the team-Core Skyframe, bazel query, BEP, options parsing, bazelrc label Aug 16, 2024
@Wyverald Wyverald added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. P1 I'll work on this now. (Assignee required) and removed team-Core Skyframe, bazel query, BEP, options parsing, bazelrc untriaged labels Aug 19, 2024
@Wyverald Wyverald self-assigned this Aug 19, 2024
@Wyverald
Copy link
Member

Thanks for the report!

This is pretty bad, but I think it should be fine for people on 7.3.1 as @@abc~ is still a valid repo name on that version, even if you flip the flag. IOW, this crash should only show up for people on rolling.

The immediate fix is to run bazel clean --expunge; I'll send a proper fix for this.

Wyverald added a commit that referenced this issue Aug 19, 2024
Especially due to #23127, older marker files may contain entries that don't even parse correctly. Instead of crashing Bazel, we should just ignore such entries.

Fixes #23322.
Wyverald added a commit that referenced this issue Aug 19, 2024
Especially due to #23127, older marker files may contain entries that don't even parse correctly. Instead of crashing Bazel, we should just ignore such entries.

Fixes #23322.
@cramertj
Copy link
Contributor Author

cramertj commented Aug 21, 2024

FYI bazel clean --expunge did not fix this issue for me-- thanks for sending a quick fix, though!

@Wyverald
Copy link
Member

Wait, that's surprising. Do you get the exact same error message? I don't understand how that's possible, then.

@cramertj
Copy link
Contributor Author

@Wyverald My apologies-- i'm not sure what I did before when I tested. I must've early-aborted the expunge or something. I tried again just now, and building after bazelisk clean --expunge worked.

bazel-io pushed a commit to bazel-io/bazel that referenced this issue Sep 10, 2024
Especially due to bazelbuild#23127, older marker files may contain entries that don't even parse correctly. Instead of crashing Bazel, we should signal a refetch.

Fixes bazelbuild#23322.

Closes bazelbuild#23336.

PiperOrigin-RevId: 666416942
Change-Id: Ie5507654f69825d93f9523d9c209d417fb3cdaf6
iancha1992 pushed a commit to bazel-io/bazel that referenced this issue Sep 12, 2024
Especially due to bazelbuild#23127, older marker files may contain entries that don't even parse correctly. Instead of crashing Bazel, we should signal a refetch.

Fixes bazelbuild#23322.

Closes bazelbuild#23336.

PiperOrigin-RevId: 666416942
Change-Id: Ie5507654f69825d93f9523d9c209d417fb3cdaf6
moroten added a commit to meroton/bazel that referenced this issue Sep 17, 2024
Bazel 7.1.0 and 7.2.0 contains a bug where + characters in labels in the
repository marker files cannot be parsed.

Improves bazelbuild#23336 that fixed bazelbuild#23322.
moroten added a commit to meroton/bazel that referenced this issue Sep 17, 2024
Bazel 7.1.0 and 7.2.0 contains a bug where + characters in labels in the
repository marker files cannot be parsed. This was fixed in
commit d62e0a0. To reduce the rusk of
future bugs in the same area, this change skips parsing the file if the
first line shows that the content will not be used anyway, which should
be reasonably safe if introducing new formats.

Improves bazelbuild#23336 that fixed bazelbuild#23322.
copybara-service bot pushed a commit that referenced this issue Sep 26, 2024
Bazel 7.1.0 and 7.2.0 contains a bug where + characters in labels in the repository marker files cannot be parsed. This was fixed in commit d62e0a0. To reduce the rusk of future bugs in the same area, this change skips parsing the file if the first line shows that the content will not be used anyway, which should be reasonably safe if introducing new formats.

Improves #23336 that fixed #23322.

Closes #23642.

PiperOrigin-RevId: 679330823
Change-Id: I8123bdde047735ced3eabed3df68112d44318b08
Wyverald added a commit that referenced this issue Oct 9, 2024
Especially due to #23127, older marker files may contain entries that don't even parse correctly. Instead of crashing Bazel, we should signal a refetch.

Fixes #23322.

Closes #23336.

PiperOrigin-RevId: 666416942
Change-Id: Ie5507654f69825d93f9523d9c209d417fb3cdaf6
Wyverald pushed a commit that referenced this issue Oct 9, 2024
Bazel 7.1.0 and 7.2.0 contains a bug where + characters in labels in the repository marker files cannot be parsed. This was fixed in commit d62e0a0. To reduce the rusk of future bugs in the same area, this change skips parsing the file if the first line shows that the content will not be used anyway, which should be reasonably safe if introducing new formats.

Improves #23336 that fixed #23322.

Closes #23642.

PiperOrigin-RevId: 679330823
Change-Id: I8123bdde047735ced3eabed3df68112d44318b08
@iancha1992
Copy link
Member

A fix for this issue has been included in Bazel 7.4.0 RC1. Please test out the release candidate and report any issues as soon as possible.
If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=7.4.0rc1. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug
Projects
None yet
5 participants
@Wyverald @cramertj @iancha1992 @satyanandak and others