[7.3.0] Be resilient to outdated exec paths in action cache entries (#23230)
https://github.com/bazelbuild/bazel/commit/60924fdc2972184494f6382d39e8c786aa14b9a9
changed the canonical repo name separator from `~` to `+`. Older repo
names containing `~` now trigger a syntax error. If an action cache
entry refers to an exec path from a previous version of Bazel that used
`~`, we need to be resilient and treat the cache entry as corrupted,
rather than just crash.
Fixes https://github.com/bazelbuild/bazel/issues/23180.
Closes #23227.
PiperOrigin-RevId: 660105601
Change-Id: Iea5d86c635056d12ba20598383da463bdde03ab0
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
index 6e814e1..6f8d153 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
@@ -23,6 +23,7 @@
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyKey.SkyKeyInterner;
+import java.util.Optional;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
@@ -77,8 +78,11 @@
*
* In this case, this method returns a package identifier for foo/bar, even though that is not a
* package. Callers need to look up the actual package if needed.
+ *
+ * <p>Returns {@link Optional#empty()} if the path corresponds to an invalid label (e.g. with a
+ * malformed repo name).
*/
- public static PackageIdentifier discoverFromExecPath(
+ public static Optional<PackageIdentifier> discoverFromExecPath(
PathFragment execPath, boolean forFiles, boolean siblingRepositoryLayout) {
Preconditions.checkArgument(!execPath.isAbsolute(), execPath);
PathFragment tofind =
@@ -93,10 +97,15 @@
if (tofind.startsWith(prefix)) {
// Using the path prefix can be either "external" or "..", depending on whether the sibling
// repository layout is used.
- RepositoryName repository = RepositoryName.createUnvalidated(tofind.getSegment(1));
- return PackageIdentifier.create(repository, tofind.subFragment(2));
+ try {
+ RepositoryName repository = RepositoryName.create(tofind.getSegment(1));
+ return Optional.of(PackageIdentifier.create(repository, tofind.subFragment(2)));
+ } catch (LabelSyntaxException e) {
+ // The path corresponds to an invalid label.
+ return Optional.empty();
+ }
} else {
- return PackageIdentifier.createInMainRepo(tofind);
+ return Optional.of(PackageIdentifier.createInMainRepo(tofind));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java
index 40f1f71..bb3d068 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java
@@ -22,7 +22,6 @@
import com.google.devtools.build.lib.actions.PathMapper;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
-import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -32,6 +31,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import javax.annotation.Nullable;
/**
@@ -178,10 +178,13 @@
}
Artifact artifact = regularDerivedArtifacts.get(execPathFragment);
if (artifact == null) {
- RepositoryName repository =
- PackageIdentifier.discoverFromExecPath(execPathFragment, false, siblingRepositoryLayout)
- .getRepository();
- artifact = artifactResolver.resolveSourceArtifact(execPathFragment, repository);
+ Optional<PackageIdentifier> pkgId =
+ PackageIdentifier.discoverFromExecPath(
+ execPathFragment, false, siblingRepositoryLayout);
+ if (pkgId.isPresent()) {
+ artifact =
+ artifactResolver.resolveSourceArtifact(execPathFragment, pkgId.get().getRepository());
+ }
}
if (artifact != null) {
// We don't need to add the sourceFile itself as it is a mandatory input.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index eef14e8..5958e27 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -116,6 +116,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
@@ -686,11 +687,13 @@
PathFragment parent =
checkNotNull(path.getParentDirectory(), "Must pass in files, not root directory");
checkArgument(!parent.isAbsolute(), path);
- ContainingPackageLookupValue.Key depKey =
- ContainingPackageLookupValue.key(
- PackageIdentifier.discoverFromExecPath(path, true, siblingRepositoryLayout));
- depKeys.put(path, depKey);
- packageLookupsRequested.add(depKey);
+ Optional<PackageIdentifier> pkgId =
+ PackageIdentifier.discoverFromExecPath(path, true, siblingRepositoryLayout);
+ if (pkgId.isPresent()) {
+ ContainingPackageLookupValue.Key depKey = ContainingPackageLookupValue.key(pkgId.get());
+ depKeys.put(path, depKey);
+ packageLookupsRequested.add(depKey);
+ }
}
SkyframeLookupResult values = env.getValuesAndExceptions(depKeys.values());
