Make ActionOwner a final class, since all non-test implementations were basically doing the same thing with it.
This simplifies the code and avoids unnecessary re-wrapping, which saves memory.
--
MOS_MIGRATED_REVID=117693050
diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
index 7ce6b21..3196d70 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
@@ -26,7 +26,6 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.EventKind;
-import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
import com.google.devtools.build.lib.syntax.Printer;
import com.google.devtools.build.lib.util.Preconditions;
@@ -123,7 +122,7 @@
Iterable<Artifact> outputs) {
Preconditions.checkNotNull(owner);
// TODO(bazel-team): Use RuleContext.actionOwner here instead
- this.owner = new ActionOwnerDescription(owner);
+ this.owner = owner;
this.tools = CollectionUtils.makeImmutable(tools);
this.inputs = CollectionUtils.makeImmutable(inputs);
this.outputs = ImmutableSet.copyOf(outputs);
@@ -427,59 +426,4 @@
return getInputs();
}
- /**
- * A copying implementation of {@link ActionOwner}.
- *
- * <p>ConfiguredTargets implement ActionOwner themselves, but we do not want actions
- * to keep direct references to configured targets just for a label and a few strings.
- */
- @Immutable
- private static class ActionOwnerDescription implements ActionOwner {
-
- private final Location location;
- private final Label label;
- private final String configurationMnemonic;
- private final String configurationChecksum;
- private final String targetKind;
- private final String additionalProgressInfo;
-
- private ActionOwnerDescription(ActionOwner originalOwner) {
- this.location = originalOwner.getLocation();
- this.label = originalOwner.getLabel();
- this.configurationMnemonic = originalOwner.getConfigurationMnemonic();
- this.configurationChecksum = originalOwner.getConfigurationChecksum();
- this.targetKind = originalOwner.getTargetKind();
- this.additionalProgressInfo = originalOwner.getAdditionalProgressInfo();
- }
-
- @Override
- public Location getLocation() {
- return location;
- }
-
- @Override
- public Label getLabel() {
- return label;
- }
-
- @Override
- public String getConfigurationMnemonic() {
- return configurationMnemonic;
- }
-
- @Override
- public String getConfigurationChecksum() {
- return configurationChecksum;
- }
-
- @Override
- public String getTargetKind() {
- return targetKind;
- }
-
- @Override
- public String getAdditionalProgressInfo() {
- return additionalProgressInfo;
- }
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractActionOwner.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractActionOwner.java
deleted file mode 100644
index 0acfa52..0000000
--- a/src/main/java/com/google/devtools/build/lib/actions/AbstractActionOwner.java
+++ /dev/null
@@ -1,60 +0,0 @@
-// Copyright 2014 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.actions;
-
-import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.events.Location;
-
-/**
- * An action owner base class that provides default implementations for some of
- * the {@link ActionOwner} methods.
- */
-public abstract class AbstractActionOwner implements ActionOwner {
-
- @Override
- public String getAdditionalProgressInfo() {
- return null;
- }
-
- @Override
- public Location getLocation() {
- return null;
- }
-
- @Override
- public Label getLabel() {
- return null;
- }
-
- @Override
- public String getTargetKind() {
- return "empty target kind";
- }
-
- /**
- * An action owner for special cases. Usage is strongly discouraged.
- */
- public static final ActionOwner SYSTEM_ACTION_OWNER = new AbstractActionOwner() {
- @Override
- public String getConfigurationMnemonic() {
- return "system";
- }
-
- @Override
- public final String getConfigurationChecksum() {
- return "system";
- }
- };
-}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java b/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java
index b82b025..b0b163e 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java
@@ -15,52 +15,86 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.util.Preconditions;
+
+import javax.annotation.Nullable;
/**
- * The owner of an action is responsible for reporting conflicts in the action
- * graph (two actions attempting to generate the same artifact).
+ * Contains metadata used for reporting the progress and status of an action.
*
- * Typically an action's owner is the RuleConfiguredTarget instance responsible
- * for creating it, but to avoid coupling between the view and actions
- * packages, the RuleConfiguredTarget is hidden behind this interface, which
- * exposes only the error reporting functionality.
+ * <p>Morally an action's owner is the RuleConfiguredTarget instance responsible for creating it,
+ * but to avoid storing heavyweight analysis objects in actions, and to avoid coupling between the
+ * analysis and actions packages, the RuleConfiguredTarget provides an instance of this class.
*/
-public interface ActionOwner {
+public final class ActionOwner {
+ /** An action owner for special cases. Usage is strongly discouraged. */
+ public static final ActionOwner SYSTEM_ACTION_OWNER =
+ new ActionOwner(null, null, "system", "empty target kind", "system", null);
- /**
- * Returns the location of this ActionOwner, if any; null otherwise.
- */
- Location getLocation();
+ @Nullable private final Label label;
+ @Nullable private final Location location;
+ @Nullable private final String mnemonic;
+ @Nullable private final String targetKind;
+ private final String configurationChecksum;
+ @Nullable private final String additionalProgressInfo;
+
+ public ActionOwner(
+ @Nullable Label label,
+ @Nullable Location location,
+ @Nullable String mnemonic,
+ @Nullable String targetKind,
+ String configurationChecksum,
+ @Nullable String additionalProgressInfo) {
+ this.label = label;
+ this.location = location;
+ this.mnemonic = mnemonic;
+ this.targetKind = targetKind;
+ this.configurationChecksum = Preconditions.checkNotNull(configurationChecksum);
+ this.additionalProgressInfo = additionalProgressInfo;
+ }
+
+ /** Returns the location of this ActionOwner, if any; null otherwise. */
+ @Nullable
+ public Location getLocation() {
+ return location;
+ }
/**
* Returns the label for this ActionOwner, if any; null otherwise.
*/
- Label getLabel();
+ @Nullable
+ public Label getLabel() {
+ return label;
+ }
- /**
- * Returns the configuration's mnemonic.
- */
- String getConfigurationMnemonic();
+ /** Returns the configuration's mnemonic. */
+ @Nullable
+ public String getConfigurationMnemonic() {
+ return mnemonic;
+ }
/**
* Returns the short cache key for the configuration of the action owner.
*
- * <p>Special action owners that are not targets can return any string here as long as it is
- * constant. If the configuration is null, this should return "null".
- *
- * <p>These requirements exist so that {@link ActionOwner} instances are consistent with
- * {@code BuildView.ActionOwnerIdentity(ConfiguredTargetValue)}.
+ * <p>Special action owners that are not targets can return any string here. If the
+ * underlying configuration is null, this should return "null".
*/
- String getConfigurationChecksum();
+ public String getConfigurationChecksum() {
+ return configurationChecksum;
+ }
- /**
- * Returns the target kind (rule class name) for this ActionOwner, if any; null otherwise.
- */
- String getTargetKind();
+ /** Returns the target kind (rule class name) for this ActionOwner, if any; null otherwise. */
+ @Nullable
+ public String getTargetKind() {
+ return targetKind;
+ }
/**
* Returns additional information that should be displayed in progress messages, or {@code null}
* if nothing should be added.
*/
- String getAdditionalProgressInfo();
+ @Nullable
+ String getAdditionalProgressInfo() {
+ return additionalProgressInfo;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildInfoHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildInfoHelper.java
deleted file mode 100644
index 3510eb0..0000000
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildInfoHelper.java
+++ /dev/null
@@ -1,31 +0,0 @@
-// Copyright 2014 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.analysis;
-
-import com.google.devtools.build.lib.actions.AbstractActionOwner;
-import com.google.devtools.build.lib.actions.ActionOwner;
-
-// TODO(bazel-team): move BUILD_INFO_ACTION_OWNER somewhere else and remove this class.
-/**
- * Helper class for the CompatibleWriteBuildInfoAction, which holds the
- * methods for generating build information.
- * Abstracted away to allow non-action code to also generate build info under
- * --nobuild or --check_up_to_date.
- */
-public abstract class BuildInfoHelper {
- /** ActionOwner for BuildInfoActions. */
- public static final ActionOwner BUILD_INFO_ACTION_OWNER =
- AbstractActionOwner.SYSTEM_ACTION_OWNER;
-}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 1b69685..273d562 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.analysis;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
@@ -130,7 +131,7 @@
}
}
- static final String HOST_CONFIGURATION_PROGRESS_TAG = "for host";
+ private static final String HOST_CONFIGURATION_PROGRESS_TAG = "for host";
private final Rule rule;
private final ListMultimap<String, ConfiguredTarget> targetMap;
@@ -286,7 +287,7 @@
@Override
public ActionOwner getActionOwner() {
if (actionOwner == null) {
- actionOwner = new RuleActionOwner(rule, getConfiguration());
+ actionOwner = createActionOwner(rule, getConfiguration());
}
return actionOwner;
}
@@ -361,55 +362,15 @@
return getAnalysisEnvironment().getBuildInfo(this, key);
}
- // TODO(bazel-team): This class could be simpler if Rule and BuildConfiguration classes
- // were immutable. Then we would need to store only references those two.
- @Immutable
- private static final class RuleActionOwner implements ActionOwner {
- private final Label label;
- private final Location location;
- private final String mnemonic;
- private final String targetKind;
- private final String configurationChecksum;
- private final boolean hostConfiguration;
-
- private RuleActionOwner(Rule rule, BuildConfiguration configuration) {
- this.label = rule.getLabel();
- this.location = rule.getLocation();
- this.targetKind = rule.getTargetKind();
- this.mnemonic = configuration.getMnemonic();
- this.configurationChecksum = configuration.checksum();
- this.hostConfiguration = configuration.isHostConfiguration();
- }
-
- @Override
- public Location getLocation() {
- return location;
- }
-
- @Override
- public Label getLabel() {
- return label;
- }
-
- @Override
- public String getConfigurationMnemonic() {
- return mnemonic;
- }
-
- @Override
- public String getConfigurationChecksum() {
- return configurationChecksum;
- }
-
- @Override
- public String getTargetKind() {
- return targetKind;
- }
-
- @Override
- public String getAdditionalProgressInfo() {
- return hostConfiguration ? HOST_CONFIGURATION_PROGRESS_TAG : null;
- }
+ @VisibleForTesting
+ public static ActionOwner createActionOwner(Rule rule, BuildConfiguration configuration) {
+ return new ActionOwner(
+ rule.getLabel(),
+ rule.getLocation(),
+ configuration.getMnemonic(),
+ rule.getTargetKind(),
+ configuration.checksum(),
+ configuration.isHostConfiguration() ? HOST_CONFIGURATION_PROGRESS_TAG : null);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
index c47388b..51b582c 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
@@ -23,6 +23,7 @@
import com.google.devtools.build.lib.actions.ActionContextProvider;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionException;
+import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactFactory;
import com.google.devtools.build.lib.actions.ArtifactOwner;
@@ -32,7 +33,6 @@
import com.google.devtools.build.lib.actions.Root;
import com.google.devtools.build.lib.actions.SimpleActionContextProvider;
import com.google.devtools.build.lib.analysis.BuildInfo;
-import com.google.devtools.build.lib.analysis.BuildInfoHelper;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction.Key;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction.KeyType;
@@ -80,7 +80,9 @@
Path workspace,
Artifact stableStatus,
Artifact volatileStatus) {
- super(BuildInfoHelper.BUILD_INFO_ACTION_OWNER, Artifact.NO_ARTIFACTS,
+ super(
+ ActionOwner.SYSTEM_ACTION_OWNER,
+ Artifact.NO_ARTIFACTS,
ImmutableList.of(stableStatus, volatileStatus));
this.options = Preconditions.checkNotNull(options);
this.stableStatus = stableStatus;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/WriteBuildInfoHeaderAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/WriteBuildInfoHeaderAction.java
index da42dfd..b0151eb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/WriteBuildInfoHeaderAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/WriteBuildInfoHeaderAction.java
@@ -19,8 +19,8 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
+import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.BuildInfoHelper;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
import com.google.devtools.build.lib.util.Fingerprint;
@@ -64,8 +64,7 @@
*/
public WriteBuildInfoHeaderAction(Collection<Artifact> inputs,
Artifact output, boolean writeVolatileInfo, boolean writeStableInfo) {
- super(BuildInfoHelper.BUILD_INFO_ACTION_OWNER,
- inputs, output, /*makeExecutable=*/false);
+ super(ActionOwner.SYSTEM_ACTION_OWNER, inputs, output, /*makeExecutable=*/ false);
valueArtifacts = ImmutableList.copyOf(inputs);
if (!inputs.isEmpty()) {
// With non-empty inputs we should not generate both volatile and non-volatile data
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/WriteBuildInfoPropertiesAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/WriteBuildInfoPropertiesAction.java
index a60c322..4439a04 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/WriteBuildInfoPropertiesAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/WriteBuildInfoPropertiesAction.java
@@ -19,8 +19,8 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
+import com.google.devtools.build.lib.actions.ActionOwner;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.BuildInfoHelper;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction.Key;
import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
@@ -126,7 +126,7 @@
public WriteBuildInfoPropertiesAction(Collection<Artifact> inputs, Artifact output,
BuildInfoPropertiesTranslator keyTranslations, boolean includeVolatile,
boolean includeNonVolatile, TimestampFormatter timestampFormatter) {
- super(BuildInfoHelper.BUILD_INFO_ACTION_OWNER, inputs, output, /* makeExecutable= */false);
+ super(ActionOwner.SYSTEM_ACTION_OWNER, inputs, output, /* makeExecutable= */ false);
this.keyTranslations = keyTranslations;
this.includeVolatile = includeVolatile;
this.includeNonVolatile = includeNonVolatile;
