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.

Sign up for GitHub

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

Jump to bottom

Crash on tilde separators in cache #23322

Closed
cramertj opened this issue Aug 16, 2024 · 4 comments · May be fixed by #23642
Closed

Crash on tilde separators in cache #23322

cramertj opened this issue Aug 16, 2024 · 4 comments · May be fixed by #23642
Assignees
@Wyverald
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

cramertj commented Aug 16, 2024 •
edited
Loading

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
@Wyverald
Ignore invalid entries in the marker file
3bab496 
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 Wyverald mentioned this issue Aug 19, 2024
Closed
Wyverald added a commit that referenced this issue Aug 19, 2024
@Wyverald
Ignore invalid entries in the marker file
0a6880d 
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
Author

cramertj commented Aug 21, 2024 •
edited
Loading

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
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
@Wyverald @bazel-io
Consider invalid entries in the marker file reason for refetch
697fcc5 
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
@bazel-io bazel-io mentioned this issue Sep 10, 2024
Open
iancha1992 pushed a commit to bazel-io/bazel that referenced this issue Sep 12, 2024
@Wyverald @iancha1992
Consider invalid entries in the marker file reason for refetch
cd40eef 
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
@moroten
Make repo marker file parsing robust to version changes
1524409 
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
@moroten
Make repo marker file parsing robust to version changes
6164d18 
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.
@moroten moroten mentioned this issue Sep 17, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Wyverald Wyverald

Labels
P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: bug
Projects
None yet
Milestone
No milestone
5 participants
@Wyverald @cramertj @sgowroji @iancha1992 @satyanandak