Clean up a few Skylark-Skyframe structures
Add value-class methods to SkylarkFileDependency and SkylarkImportLookupValue. Remove Java serialization hack from Extension.
RELNOTES: None
PiperOrigin-RevId: 162383283
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkFileDependency.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkFileDependency.java
index 36a7516..0167aea 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkFileDependency.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkFileDependency.java
@@ -16,19 +16,20 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.cmdline.Label;
-
-import java.io.Serializable;
+import java.util.Objects;
/**
- * A simple value class to store the direct Skylark file dependencies of a Skylark
- * extension file. It also contains a Label identifying the extension file.
+ * A simple value class to store the direct Skylark file dependencies of a Skylark extension file.
+ * It also contains a Label identifying the extension file.
+ *
+ * <p>The dependency structure must be acyclic.
*/
-class SkylarkFileDependency implements Serializable {
+public class SkylarkFileDependency {
private final Label label;
private final ImmutableList<SkylarkFileDependency> dependencies;
- SkylarkFileDependency(Label label, ImmutableList<SkylarkFileDependency> dependencies) {
+ public SkylarkFileDependency(Label label, ImmutableList<SkylarkFileDependency> dependencies) {
this.label = label;
this.dependencies = dependencies;
}
@@ -37,14 +38,34 @@
* Returns the list of direct Skylark file dependencies of the Skylark extension file
* corresponding to this object.
*/
- ImmutableList<SkylarkFileDependency> getDependencies() {
+ public ImmutableList<SkylarkFileDependency> getDependencies() {
return dependencies;
}
/**
* Returns the Label of the Skylark extension file corresponding to this object.
*/
- Label getLabel() {
+ public Label getLabel() {
return label;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof SkylarkFileDependency)) {
+ return false;
+ }
+ SkylarkFileDependency other = (SkylarkFileDependency) obj;
+ if (!label.equals(other.getLabel())) {
+ return false;
+ }
+ return dependencies.equals(other.getDependencies());
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(label, dependencies);
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
index 583e155..4d63f1b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
@@ -102,4 +102,22 @@
return LegacySkyKey.create(
SkyFunctions.SKYLARK_IMPORTS_LOOKUP, new SkylarkImportLookupKey(importLabel, inWorkspace));
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof SkylarkImportLookupValue)) {
+ return false;
+ }
+ SkylarkImportLookupValue other = (SkylarkImportLookupValue) obj;
+ return environmentExtension.equals(other.getEnvironmentExtension())
+ && dependency.equals(other.getDependency());
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(environmentExtension, dependency);
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
index 38cbb7c..4bec1c6 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
@@ -29,12 +29,12 @@
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.util.SpellChecker;
import com.google.devtools.common.options.Options;
-import java.io.Serializable;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import java.util.TreeSet;
import javax.annotation.Nullable;
@@ -295,38 +295,20 @@
}
}
- // TODO(bazel-team): Eliminate this hack around Java serialization. The bindings are currently
- // factored out into BaseExtension, which is non-Serializable, and which has a default constructor
- // that does not initialize any bindings. This means that when Extension is Java-serialized, all
- // the bindings are simply lost.
- private static class BaseExtension {
-
- protected final ImmutableMap<String, Object> bindings;
-
- BaseExtension(Map<String, Object> bindings) {
- this.bindings = ImmutableMap.copyOf(bindings);
- }
-
- // Hack to "allow" java serialization.
- BaseExtension() {
- this.bindings = ImmutableMap.of();
- }
- }
-
/** An Extension to be imported with load() into a BUILD or .bzl file. */
@Immutable
- public static final class Extension extends BaseExtension implements Serializable {
+ public static final class Extension {
+
+ private final ImmutableMap<String, Object> bindings;
/**
* Cached hash code for the transitive content of this {@code Extension} and its dependencies.
*/
private final String transitiveContentHashCode;
- /**
- * Constructs with the given hash code and bindings.
- */
- public Extension(Map<String, Object> bindings, String transitiveContentHashCode) {
- super(bindings);
+ /** Constructs with the given hash code and bindings. */
+ public Extension(ImmutableMap<String, Object> bindings, String transitiveContentHashCode) {
+ this.bindings = bindings;
this.transitiveContentHashCode = transitiveContentHashCode;
}
@@ -335,8 +317,7 @@
* and that {@code Environment}'s transitive hash code.
*/
public Extension(Environment env) {
- super(env.globalFrame.bindings);
- this.transitiveContentHashCode = env.getTransitiveContentHashCode();
+ this(ImmutableMap.copyOf(env.globalFrame.bindings), env.getTransitiveContentHashCode());
}
public String getTransitiveContentHashCode() {
@@ -346,6 +327,24 @@
public ImmutableMap<String, Object> getBindings() {
return bindings;
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj) {
+ return true;
+ }
+ if (!(obj instanceof Extension)) {
+ return false;
+ }
+ Extension other = (Extension) obj;
+ return transitiveContentHashCode.equals(other.getTransitiveContentHashCode())
+ && bindings.equals(other.getBindings());
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(bindings, transitiveContentHashCode);
+ }
}
/**
