Skip to content

Commit

Permalink
Add a method to get BuildConfigurationKey from ActionLookupKey.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 406831509
  • Loading branch information
justinhorvitz authored and copybara-github committed Nov 1, 2021
1 parent 2c9721f commit 216b2b6
Show file tree
Hide file tree
Showing 20 changed files with 116 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
// limitations under the License.
package com.google.devtools.build.lib.actions;

import com.google.devtools.build.lib.skyframe.BuildConfigurationKey;
import com.google.devtools.build.skyframe.CPUHeavySkyKey;
import com.google.devtools.build.skyframe.SkyKey;
import javax.annotation.Nullable;

/**
* {@link SkyKey} for an "analysis object": either an {@link ActionLookupValue} or a {@link
Expand All @@ -28,4 +30,12 @@
* are subclasses of {@link ActionLookupKey}. This allows callers to easily find the value key,
* while remaining agnostic to what action lookup values actually exist.
*/
public interface ActionLookupKey extends ArtifactOwner, CPUHeavySkyKey {}
public interface ActionLookupKey extends ArtifactOwner, CPUHeavySkyKey {

/**
* Returns the {@link BuildConfigurationKey} for the configuration associated with this key, or
* {@code null} if this key has no associated configuration.
*/
@Nullable
BuildConfigurationKey getConfigurationKey();
}
2 changes: 2 additions & 0 deletions src/main/java/com/google/devtools/build/lib/actions/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,10 @@ java_library(
srcs = ["ActionLookupKey.java"],
deps = [
":artifact_owner",
"//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration",
"//src/main/java/com/google/devtools/build/skyframe:cpu_heavy_skykey",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//third_party:jsr305",
],
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ public Label getLabel() {
return actionLookupKey.getLabel();
}

@Override
public BuildConfigurationKey getConfigurationKey() {
return actionLookupKey.getConfigurationKey();
}

public ActionLookupKey getActionLookupKey() {
return actionLookupKey;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,11 @@ public SkyValue compute(SkyKey skyKey, Environment env)
new BuildFileContainsErrorsException(key.getLabel().getPackageIdentifier()));
}

boolean aspectHasConfiguration = key.getAspectConfigurationKey() != null;
boolean aspectHasConfiguration = key.getConfigurationKey() != null;

ImmutableSet<SkyKey> keys =
aspectHasConfiguration
? ImmutableSet.of(key.getBaseConfiguredTargetKey(), key.getAspectConfigurationKey())
? ImmutableSet.of(key.getBaseConfiguredTargetKey(), key.getConfigurationKey())
: ImmutableSet.of(key.getBaseConfiguredTargetKey());

Map<SkyKey, ValueOrException<ConfiguredValueCreationException>> baseAndAspectValues =
Expand All @@ -248,11 +248,12 @@ public SkyValue compute(SkyKey skyKey, Environment env)
if (aspectHasConfiguration) {
try {
aspectConfiguration =
(BuildConfigurationValue)
baseAndAspectValues.get(key.getAspectConfigurationKey()).get();
(BuildConfigurationValue) baseAndAspectValues.get(key.getConfigurationKey()).get();
} catch (ConfiguredValueCreationException e) {
throw new IllegalStateException("Unexpected exception from BuildConfigurationFunction when "
+ "computing " + key.getAspectConfigurationKey(), e);
throw new IllegalStateException(
"Unexpected exception from BuildConfigurationFunction when computing "
+ key.getConfigurationKey(),
e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,22 +192,10 @@ public String getDescription() {
/**
* Returns the key of the configured target of the aspect; that is, the configuration in which
* the aspect will be evaluated.
*
* <p>In trimmed configuration mode, the aspect may require more fragments than the target on
* which it is being evaluated; in addition to configuration fragments required by the target
* and its dependencies, an aspect has configuration fragment requirements of its own, as well
* as dependencies of its own with their own configuration fragment requirements.
*
* <p>The aspect configuration contains all of these fragments, and is used to create the
* aspect's RuleContext and to retrieve the dependencies. Note that dependencies will have their
* configurations trimmed from this one as normal.
*
* <p>Because of these properties, this configuration is always a superset of the base target's
* configuration. In untrimmed configuration mode, this configuration will be equivalent to the
* base target's configuration.
*/
@Override
@Nullable
public BuildConfigurationKey getAspectConfigurationKey() {
public BuildConfigurationKey getConfigurationKey() {
return aspectConfigurationKey;
}

Expand Down Expand Up @@ -303,6 +291,11 @@ public SkyFunctionName functionName() {
return SkyFunctions.TOP_LEVEL_ASPECTS;
}

@Override
public BuildConfigurationKey getConfigurationKey() {
return getBaseConfiguredTargetKey().getConfigurationKey();
}

ImmutableList<AspectClass> getTopLevelAspectsClasses() {
return topLevelAspectsClasses;
}
Expand All @@ -313,7 +306,7 @@ public Label getLabel() {
}

String getDescription() {
return topLevelAspectsClasses + " on " + getLabel();
return topLevelAspectsClasses + " on " + targetLabel;
}

@Override
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,7 @@ java_library(
name = "action_template_expansion_value",
srcs = ["ActionTemplateExpansionValue.java"],
deps = [
":build_configuration",
":sky_functions",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
Expand Down Expand Up @@ -1226,6 +1227,7 @@ java_library(
name = "coverage_report_value",
srcs = ["CoverageReportValue.java"],
deps = [
":build_configuration",
":sky_functions",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
Expand Down Expand Up @@ -2768,6 +2770,7 @@ java_library(
name = "workspace_status_value",
srcs = ["WorkspaceStatusValue.java"],
deps = [
":build_configuration",
":sky_functions",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept
ImmutableSet.of(
WorkspaceStatusValue.BUILD_INFO_KEY,
WorkspaceNameValue.key(),
keyAndConfig.getConfigKey());
keyAndConfig.getConfigurationKey());
Map<SkyKey, SkyValue> result = env.getValues(keysToRequest);
if (env.valuesMissing()) {
return null;
Expand All @@ -66,7 +66,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept
(WorkspaceStatusValue) result.get(WorkspaceStatusValue.BUILD_INFO_KEY);

BuildConfigurationValue config =
(BuildConfigurationValue) result.get(keyAndConfig.getConfigKey());
(BuildConfigurationValue) result.get(keyAndConfig.getConfigurationKey());
Map<BuildInfoKey, BuildInfoFactory> buildInfoFactories = BUILD_INFO_FACTORIES.get(env);
BuildInfoFactory buildInfoFactory = buildInfoFactories.get(keyAndConfig.getInfoKey());
Preconditions.checkState(buildInfoFactory.isEnabled(config));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,13 @@ public SkyFunctionName functionName() {
return SkyFunctions.BUILD_INFO_COLLECTION;
}

BuildInfoKey getInfoKey() {
return infoKey;
@Override
public BuildConfigurationKey getConfigurationKey() {
return configKey;
}

BuildConfigurationKey getConfigKey() {
return configKey;
BuildInfoKey getInfoKey() {
return infoKey;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ public SkyFunctionName functionName() {
return SkyFunctions.COVERAGE_REPORT;
}

@Nullable
@Override
public BuildConfigurationKey getConfigurationKey() {
return null;
}

@Nullable
@Override
public Label getLabel() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -561,15 +561,12 @@ public SkyframeAnalysisResult configureTargets(
for (ActionLookupKey ctKey : Iterables.concat(ctKeys, aspectKeys)) {
if (!topLevelActionConflictReport.isErrorFree(ctKey)) {
Optional<ConflictException> e = topLevelActionConflictReport.getConflictException(ctKey);
if (!e.isPresent()) {
if (e.isEmpty()) {
continue;
}
AnalysisFailedCause failedCause =
makeArtifactConflictAnalysisFailedCause(configurationLookupSupplier, e.get());
BuildConfigurationKey configKey =
ctKey instanceof ConfiguredTargetKey
? ((ConfiguredTargetKey) ctKey).getConfigurationKey()
: ((AspectKey) ctKey).getAspectConfigurationKey();
BuildConfigurationKey configKey = ctKey.getConfigurationKey();
eventBus.post(
new AnalysisFailureEvent(
ctKey,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,11 @@ public SkyFunctionName functionName() {
public Label getLabel() {
return null;
}

@Nullable
@Override
public BuildConfigurationKey getConfigurationKey() {
return null;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.rules.cpp.CppFileTypes;
import com.google.devtools.build.lib.rules.java.JavaSemantics;
import com.google.devtools.build.lib.skyframe.BuildConfigurationKey;
import com.google.devtools.build.lib.skyframe.serialization.AutoRegistry;
import com.google.devtools.build.lib.skyframe.serialization.ObjectCodecs;
import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationDepsUtils;
Expand Down Expand Up @@ -414,6 +415,11 @@ public Label getLabel() {
return null;
}

@Override
public BuildConfigurationKey getConfigurationKey() {
return null;
}

@Override
public SkyFunctionName functionName() {
return null;
Expand All @@ -426,6 +432,11 @@ public Label getLabel() {
return null;
}

@Override
public BuildConfigurationKey getConfigurationKey() {
return null;
}

@Override
public SkyFunctionName functionName() {
return null;
Expand Down
1 change: 1 addition & 0 deletions src/test/java/com/google/devtools/build/lib/actions/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/exec:single_build_file_cache",
"//src/main/java/com/google/devtools/build/lib/rules/cpp",
"//src/main/java/com/google/devtools/build/lib/rules/java:java-compilation",
"//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration",
"//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
import com.google.devtools.build.lib.exec.SingleBuildFileCache;
import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue;
import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue.ActionTemplateExpansionKey;
import com.google.devtools.build.lib.skyframe.BuildConfigurationKey;
import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
import com.google.devtools.build.lib.util.FileType;
Expand Down Expand Up @@ -387,6 +388,7 @@ public boolean restartPermitted() {
@SerializationConstant
public static final ActionLookupKey NULL_ARTIFACT_OWNER =
new ActionLookupKey() {

@Override
public SkyFunctionName functionName() {
return null;
Expand All @@ -397,6 +399,12 @@ public Label getLabel() {
return NULL_LABEL;
}

@Nullable
@Override
public BuildConfigurationKey getConfigurationKey() {
return null;
}

@Override
public String toString() {
return "NULL_ARTIFACT_OWNER";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/exec:single_build_file_cache",
"//src/main/java/com/google/devtools/build/lib/skyframe:action_template_expansion_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration",
"//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import com.google.devtools.build.lib.actions.ActionLookupKey;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.skyframe.BuildConfigurationKey;
import com.google.devtools.build.skyframe.SkyFunctionName;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -44,6 +45,12 @@ public Label getLabel() {
return null;
}

@Nullable
@Override
public BuildConfigurationKey getConfigurationKey() {
return null;
}

@Override
public int hashCode() {
return name.hashCode();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.events.EventKind;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.skyframe.BuildConfigurationKey;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
Expand Down Expand Up @@ -71,7 +72,7 @@ private static StarlarkThread newStarlarkThread(String... options)
}

@Test
public void testFilterListForObscuringSymlinksCatchesBadObscurer() throws Exception {
public void testFilterListForObscuringSymlinksCatchesBadObscurer() {
Map<PathFragment, Artifact> obscuringMap = new HashMap<>();
PathFragment pathA = PathFragment.create("a");
ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.resolve("/workspace")));
Expand All @@ -84,7 +85,7 @@ public void testFilterListForObscuringSymlinksCatchesBadObscurer() throws Except
}

@Test
public void testFilterListForObscuringSymlinksCatchesBadGrandParentObscurer() throws Exception {
public void testFilterListForObscuringSymlinksCatchesBadGrandParentObscurer() {
Map<PathFragment, Artifact> obscuringMap = new HashMap<>();
PathFragment pathA = PathFragment.create("a");
ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.resolve("/workspace")));
Expand All @@ -97,7 +98,7 @@ public void testFilterListForObscuringSymlinksCatchesBadGrandParentObscurer() th
}

@Test
public void testFilterListForObscuringSymlinksCatchesBadObscurerNoListener() throws Exception {
public void testFilterListForObscuringSymlinksCatchesBadObscurerNoListener() {
Map<PathFragment, Artifact> obscuringMap = new HashMap<>();
PathFragment pathA = PathFragment.create("a");
ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.resolve("/workspace")));
Expand All @@ -109,7 +110,7 @@ public void testFilterListForObscuringSymlinksCatchesBadObscurerNoListener() thr
}

@Test
public void testFilterListForObscuringSymlinksIgnoresOkObscurer() throws Exception {
public void testFilterListForObscuringSymlinksIgnoresOkObscurer() {
Map<PathFragment, Artifact> obscuringMap = new HashMap<>();
PathFragment pathA = PathFragment.create("a");
ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.resolve("/workspace")));
Expand All @@ -123,7 +124,7 @@ public void testFilterListForObscuringSymlinksIgnoresOkObscurer() throws Excepti
}

@Test
public void testFilterListForObscuringSymlinksNoObscurers() throws Exception {
public void testFilterListForObscuringSymlinksNoObscurers() {
Map<PathFragment, Artifact> obscuringMap = new HashMap<>();
PathFragment pathA = PathFragment.create("a");
ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.resolve("/workspace")));
Expand Down Expand Up @@ -171,6 +172,12 @@ public SkyFunctionName functionName() {
public Label getLabel() {
return null;
}

@Nullable
@Override
public BuildConfigurationKey getConfigurationKey() {
return null;
}
}

@Test
Expand Down
Loading

0 comments on commit 216b2b6

Please sign in to comment.