Consider pkg in error on any symlink cycle in deps
While evaluating a package value, whether the package should be in
error was checked several times for different kinds of symlink cycle
deps: subincluded target's package, subincluded target, and glob.
But each check overrode the result of previous checks. Now, the
error state is a big OR of the results of each cycle check, not just
the last one (which happened to be for globs).
--
MOS_MIGRATED_REVID=95854169
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index 8aac499..121e114 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -218,7 +218,8 @@
private static boolean markDependenciesAndPropagateInconsistentFilesystemExceptions(
Package pkg, Environment env, Collection<Pair<String, Boolean>> globPatterns,
Map<Label, Path> subincludes) throws InternalInconsistentFilesystemException {
- boolean packageShouldBeInError = pkg.containsErrors();
+ boolean packageWasOriginallyInError = pkg.containsErrors();
+ boolean packageShouldBeInError = packageWasOriginallyInError;
// TODO(bazel-team): This means that many packages will have to be preprocessed twice. Ouch!
// We need a better continuation mechanism to avoid repeating work. [skyframe-loading]
@@ -234,10 +235,10 @@
}
Pair<? extends Map<PathFragment, PackageLookupValue>, Boolean> subincludePackageLookupResult =
getPackageLookupDepsAndPropagateInconsistentFilesystemExceptions(pkg.getName(),
- subincludePackageLookupDepKeys, env, pkg.containsErrors());
+ subincludePackageLookupDepKeys, env, packageWasOriginallyInError);
Map<PathFragment, PackageLookupValue> subincludePackageLookupDeps =
subincludePackageLookupResult.getFirst();
- packageShouldBeInError = subincludePackageLookupResult.getSecond();
+ packageShouldBeInError |= subincludePackageLookupResult.getSecond();
List<SkyKey> subincludeFileDepKeys = Lists.newArrayList();
for (Entry<Label, Path> subincludeEntry : subincludes.entrySet()) {
// Ideally, we would have a direct dependency on the target with the given label, but then
@@ -273,8 +274,8 @@
}
}
}
- packageShouldBeInError = markFileDepsAndPropagateInconsistentFilesystemExceptions(
- pkg.getName(), subincludeFileDepKeys, env, pkg.containsErrors());
+ packageShouldBeInError |= markFileDepsAndPropagateInconsistentFilesystemExceptions(
+ pkg.getName(), subincludeFileDepKeys, env, packageWasOriginallyInError);
// TODO(bazel-team): In the long term, we want to actually resolve the glob patterns within
// Skyframe. For now, just logging the glob requests provides correct incrementality and
@@ -293,8 +294,8 @@
}
globDepKeys.add(globSkyKey);
}
- packageShouldBeInError = markGlobDepsAndPropagateInconsistentFilesystemExceptions(
- pkg.getName(), globDepKeys, env, pkg.containsErrors());
+ packageShouldBeInError |= markGlobDepsAndPropagateInconsistentFilesystemExceptions(
+ pkg.getName(), globDepKeys, env, packageWasOriginallyInError);
return packageShouldBeInError;
}
@@ -480,7 +481,7 @@
Map<Label, Path> subincludes = legacyPkgBuilder.getSubincludes();
Package pkg = legacyPkgBuilder.finishBuild();
Event.replayEventsOn(env.getListener(), pkg.getEvents());
- boolean packageShouldBeConsideredInError = pkg.containsErrors();
+ boolean packageShouldBeConsideredInError;
try {
packageShouldBeConsideredInError =
markDependenciesAndPropagateInconsistentFilesystemExceptions(pkg, env,
