Inline TargetMarker in TransitiveTraversalFunction
Simplifies the runtime graph when TransitiveTraversalFunction is used.
Also moves an error reporting method from the base function to the
TransitiveTargetFunction, which is the only one that uses it.
--
MOS_MIGRATED_REVID=106109745
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetMarkerFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetMarkerFunction.java
index 3e3fbd5..74d78dc 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetMarkerFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetMarkerFunction.java
@@ -27,6 +27,8 @@
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
+import javax.annotation.Nullable;
+
/**
* A SkyFunction for {@link TargetMarkerValue}s. Returns a {@link
* TargetMarkerValue#TARGET_MARKER_INSTANCE} if the {@link Label} in the {@link SkyKey}
@@ -35,11 +37,21 @@
*/
public final class TargetMarkerFunction implements SkyFunction {
- public TargetMarkerFunction() {
- }
-
@Override
public SkyValue compute(SkyKey key, Environment env) throws TargetMarkerFunctionException {
+ try {
+ return computeTargetMarkerValue(key, env);
+ } catch (NoSuchTargetException e) {
+ throw new TargetMarkerFunctionException(e);
+ } catch (NoSuchPackageException e) {
+ // Re-throw this exception with our key because root causes should be targets, not packages.
+ throw new TargetMarkerFunctionException(e);
+ }
+ }
+
+ @Nullable
+ static TargetMarkerValue computeTargetMarkerValue(SkyKey key, Environment env)
+ throws NoSuchTargetException, NoSuchPackageException {
Label label = (Label) key.argument();
PathFragment pkgForLabel = label.getPackageFragment();
@@ -47,67 +59,54 @@
// This target is in a subdirectory, therefore it could potentially be invalidated by
// a new BUILD file appearing in the hierarchy.
PathFragment containingDirectory = label.toPathFragment().getParentDirectory();
- ContainingPackageLookupValue containingPackageLookupValue = null;
+ ContainingPackageLookupValue containingPackageLookupValue;
try {
PackageIdentifier newPkgId = PackageIdentifier.create(
label.getPackageIdentifier().getRepository(), containingDirectory);
containingPackageLookupValue = (ContainingPackageLookupValue) env.getValueOrThrow(
ContainingPackageLookupValue.key(newPkgId),
BuildFileNotFoundException.class, InconsistentFilesystemException.class);
- } catch (BuildFileNotFoundException e) {
- // Thrown when there are IO errors looking for BUILD files.
- throw new TargetMarkerFunctionException(e);
} catch (InconsistentFilesystemException e) {
- throw new TargetMarkerFunctionException(new NoSuchTargetException(label,
- e.getMessage()));
+ throw new NoSuchTargetException(label, e.getMessage());
}
if (containingPackageLookupValue == null) {
return null;
}
+
if (!containingPackageLookupValue.hasContainingPackage()) {
// This means the label's package doesn't exist. E.g. there is no package 'a' and we are
// trying to build the target for label 'a:b/foo'.
- throw new TargetMarkerFunctionException(new BuildFileNotFoundException(
- label.getPackageIdentifier(), "BUILD file not found on package path for '"
- + pkgForLabel.getPathString() + "'"));
+ throw new BuildFileNotFoundException(
+ label.getPackageIdentifier(),
+ "BUILD file not found on package path for '" + pkgForLabel.getPathString() + "'");
}
if (!containingPackageLookupValue.getContainingPackageName().equals(
label.getPackageIdentifier())) {
- throw new TargetMarkerFunctionException(new NoSuchTargetException(label,
- String.format("Label '%s' crosses boundary of subpackage '%s'", label,
- containingPackageLookupValue.getContainingPackageName())));
+ throw new NoSuchTargetException(
+ label,
+ String.format(
+ "Label '%s' crosses boundary of subpackage '%s'",
+ label,
+ containingPackageLookupValue.getContainingPackageName()));
}
}
SkyKey pkgSkyKey = PackageValue.key(label.getPackageIdentifier());
- Package pkg;
- try {
- PackageValue value = (PackageValue)
- env.getValueOrThrow(pkgSkyKey, NoSuchPackageException.class);
- if (value == null) {
- return null;
- }
- pkg = value.getPackage();
- } catch (NoSuchPackageException e) {
- // Re-throw this exception with our key because root causes should be targets, not packages.
- throw new TargetMarkerFunctionException(e);
+ PackageValue value =
+ (PackageValue) env.getValueOrThrow(pkgSkyKey, NoSuchPackageException.class);
+ if (value == null) {
+ return null;
}
- Target target;
- try {
- target = pkg.getTarget(label.getName());
- } catch (NoSuchTargetException e) {
- throw new TargetMarkerFunctionException(e);
- }
-
+ Package pkg = value.getPackage();
+ Target target = pkg.getTarget(label.getName());
if (pkg.containsErrors()) {
// There is a target, but its package is in error. We rethrow so that the root cause is the
// target, not the package. Note that targets are only in error when their package is
// "in error" (because a package is in error if there was an error evaluating the package, or
// if one of its targets was in error).
- throw new TargetMarkerFunctionException(
- new NoSuchTargetException(
- target, new BuildFileContainsErrorsException(label.getPackageIdentifier())));
+ throw new NoSuchTargetException(
+ target, new BuildFileContainsErrorsException(label.getPackageIdentifier()));
}
return TargetMarkerValue.TARGET_MARKER_INSTANCE;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java
index 2d7ccc7..8623b78 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveBaseTraversalFunction.java
@@ -17,20 +17,17 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
import com.google.devtools.build.lib.packages.InputFile;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.NoSuchTargetException;
-import com.google.devtools.build.lib.packages.NoSuchThingException;
import com.google.devtools.build.lib.packages.OutputFile;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.PackageGroup;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
@@ -61,8 +58,7 @@
* {@link #computeSkyValue} with the {#code ProcessedTargets} to get the {@link SkyValue} to
* return.
*/
-abstract class TransitiveBaseTraversalFunction<TProcessedTargets>
- implements SkyFunction {
+abstract class TransitiveBaseTraversalFunction<TProcessedTargets> implements SkyFunction {
/**
* Returns a {@link SkyKey} corresponding to the traversal of a target specified by {@code label}
@@ -80,8 +76,7 @@
*/
abstract SkyKey getKey(Label label);
- abstract TProcessedTargets processTarget(Label label,
- TargetAndErrorIfAny targetAndErrorIfAny);
+ abstract TProcessedTargets processTarget(Label label, TargetAndErrorIfAny targetAndErrorIfAny);
abstract void processDeps(TProcessedTargets processedTargets, EventHandler eventHandler,
TargetAndErrorIfAny targetAndErrorIfAny,
@@ -95,6 +90,14 @@
abstract SkyValue computeSkyValue(TargetAndErrorIfAny targetAndErrorIfAny,
TProcessedTargets processedTargets);
+ /**
+ * Returns a {@link TargetMarkerValue} corresponding to the {@param targetMarkerKey} or {@code
+ * null} if the value isn't ready.
+ */
+ @Nullable
+ abstract TargetMarkerValue getTargetMarkerValue(SkyKey targetMarkerKey, Environment env)
+ throws NoSuchTargetException, NoSuchPackageException;
+
@Override
public SkyValue compute(SkyKey key, Environment env)
throws TransitiveBaseTraversalFunctionException {
@@ -150,8 +153,8 @@
/**
* Return an Iterable of SkyKeys corresponding to the Aspect-related dependencies of target.
*
- * <p>This method may return a precise set of aspect keys, but may need to request additional
- * dependencies from the env to do so.
+ * <p>This method may return a precise set of aspect keys, but may need to request additional
+ * dependencies from the env to do so.
*/
private Iterable<SkyKey> getStrictLabelAspectKeys(Target target,
Map<SkyKey, ValueOrException2<NoSuchPackageException, NoSuchTargetException>> depMap,
@@ -179,14 +182,13 @@
return depKeys;
}
- /**
- * Get the Aspect-related Label deps for the given edge.
- */
- protected abstract Collection<Label> getAspectLabels(Target fromTarget, Attribute attr,
- Label toLabel,
- ValueOrException2<NoSuchPackageException, NoSuchTargetException> toVal,
- Environment env);
-
+ /** Get the Aspect-related Label deps for the given edge. */
+ protected abstract Collection<Label> getAspectLabels(
+ Target fromTarget,
+ Attribute attr,
+ Label toLabel,
+ ValueOrException2<NoSuchPackageException, NoSuchTargetException> toVal,
+ Environment env);
private Iterable<SkyKey> getLabelDepKeys(Target target) {
List<SkyKey> depKeys = Lists.newArrayList();
@@ -226,23 +228,6 @@
labels.addAll(packageGroup.getIncludes());
}
- protected void maybeReportErrorAboutMissingEdge(Target target, Label depLabel,
- NoSuchThingException e, EventHandler eventHandler) {
- if (e instanceof NoSuchTargetException) {
- NoSuchTargetException nste = (NoSuchTargetException) e;
- if (depLabel.equals(nste.getLabel())) {
- eventHandler.handle(Event.error(TargetUtils.getLocationMaybe(target),
- TargetUtils.formatMissingEdge(target, depLabel, e)));
- }
- } else if (e instanceof NoSuchPackageException) {
- NoSuchPackageException nspe = (NoSuchPackageException) e;
- if (nspe.getPackageId().equals(depLabel.getPackageIdentifier())) {
- eventHandler.handle(Event.error(TargetUtils.getLocationMaybe(target),
- TargetUtils.formatMissingEdge(target, depLabel, e)));
- }
- }
- }
-
enum LoadTargetResultsType {
VALUES_MISSING,
TARGET_AND_ERROR_IF_ANY
@@ -308,7 +293,7 @@
}
}
- private static LoadTargetResults loadTarget(Environment env, Label label)
+ private LoadTargetResults loadTarget(Environment env, Label label)
throws NoSuchTargetException, NoSuchPackageException {
SkyKey packageKey = PackageValue.key(label.getPackageIdentifier());
SkyKey targetKey = TargetMarkerValue.key(label);
@@ -317,9 +302,10 @@
Target target;
NoSuchTargetException errorLoadingTarget = null;
try {
- TargetMarkerValue targetValue = (TargetMarkerValue) env.getValueOrThrow(targetKey,
- NoSuchTargetException.class, NoSuchPackageException.class);
- if (targetValue == null) {
+ TargetMarkerValue targetValue = getTargetMarkerValue(targetKey, env);
+ boolean targetValueMissing = targetValue == null;
+ Preconditions.checkState(targetValueMissing == env.valuesMissing(), targetKey);
+ if (targetValueMissing) {
return ValuesMissing.INSTANCE;
}
PackageValue packageValue = (PackageValue) env.getValueOrThrow(packageKey,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
index 8053543..a5e6320 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTargetFunction.java
@@ -23,6 +23,7 @@
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.AspectDefinition;
import com.google.devtools.build.lib.packages.Attribute;
@@ -34,6 +35,7 @@
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.skyframe.TransitiveTargetFunction.TransitiveTargetValueBuilder;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
@@ -180,6 +182,35 @@
}
}
+ @Override
+ TargetMarkerValue getTargetMarkerValue(SkyKey targetMarkerKey, Environment env)
+ throws NoSuchTargetException, NoSuchPackageException {
+ return (TargetMarkerValue)
+ env.getValueOrThrow(
+ targetMarkerKey, NoSuchTargetException.class, NoSuchPackageException.class);
+ }
+
+ private void maybeReportErrorAboutMissingEdge(
+ Target target, Label depLabel, NoSuchThingException e, EventHandler eventHandler) {
+ if (e instanceof NoSuchTargetException) {
+ NoSuchTargetException nste = (NoSuchTargetException) e;
+ if (depLabel.equals(nste.getLabel())) {
+ eventHandler.handle(
+ Event.error(
+ TargetUtils.getLocationMaybe(target),
+ TargetUtils.formatMissingEdge(target, depLabel, e)));
+ }
+ } else if (e instanceof NoSuchPackageException) {
+ NoSuchPackageException nspe = (NoSuchPackageException) e;
+ if (nspe.getPackageId().equals(depLabel.getPackageIdentifier())) {
+ eventHandler.handle(
+ Event.error(
+ TargetUtils.getLocationMaybe(target),
+ TargetUtils.formatMissingEdge(target, depLabel, e)));
+ }
+ }
+ }
+
/**
* Returns every configuration fragment known to the system.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java
index 7084ecf..abd6b5c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TransitiveTraversalFunction.java
@@ -114,6 +114,12 @@
: TransitiveTraversalValue.unsuccessfulTransitiveTraversal(firstErrorMessage);
}
+ @Override
+ TargetMarkerValue getTargetMarkerValue(SkyKey targetMarkerKey, Environment env)
+ throws NoSuchTargetException, NoSuchPackageException {
+ return TargetMarkerFunction.computeTargetMarkerValue(targetMarkerKey, env);
+ }
+
/**
* Keeps track of the first error message encountered while traversing itself and its
* dependencies.
