commit 89312fbebfdab4c4ab977063782237cd8369a8c7
author Francois-Rene Rideau <tunes@google.com> Thu Sep 10 18:53:03 2015 +0000
committer Damien Martin-Guillerez <dmarting@google.com> Fri Sep 11 09:45:33 2015 +0000
tree 7a238467e05767608d9d7a122f65a10011092e71
parent 5831c6957d29e8fda0383bfe337533444c5155c8

Refactor Skylark Environment-s

Make Environment-s freezable: Introduce a class Mutability
as a revokable capability to mutate objects in an Environment.
For now, only Environment-s carry this capability.
Make sure that every Mutability is revoked in the same function that create...

This reinstates a change that previously rolled-back because it broke the
serializability of SkylarkLookupValue. Bad news: serializing it succeeds for the
wrong reason, because a SkylarkEnvironment was stored as a result (now an
Environment.Extension) that was Serializable but inherited its bindings from an Environment (now an Environment.BaseExtension) which wasn't Serializable.
Apparently, Java doesn't try to serialize the bindings then (or at least doesn't
error out when it fails), because these bindings map variable names to pretty
arbitrary objects, and a lot of those we find in practice aren't Serializable.
Thus the current code passes the same tests as the previous code, but obviously
the serialization is just as ineffective as it used to be.

--
MOS_MIGRATED_REVID=102776694

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index 18d605d..2f4f86e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -38,10 +38,11 @@
 import com.google.devtools.build.lib.packages.RuleClassProvider;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.rules.SkylarkModules;
+import com.google.devtools.build.lib.syntax.Environment;
+import com.google.devtools.build.lib.syntax.Environment.Extension;
 import com.google.devtools.build.lib.syntax.Label;
-import com.google.devtools.build.lib.syntax.SkylarkEnvironment;
+import com.google.devtools.build.lib.syntax.Mutability;
 import com.google.devtools.build.lib.syntax.SkylarkType;
-import com.google.devtools.build.lib.syntax.ValidationEnvironment;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.common.options.OptionsClassProvider;
 
@@ -310,9 +311,7 @@
 
   private final PrerequisiteValidator prerequisiteValidator;
 
-  private final ImmutableMap<String, SkylarkType> skylarkAccessibleJavaClasses;
-
-  private final ValidationEnvironment skylarkValidationEnvironment;
+  private final Environment.Frame globals;
 
   public ConfiguredRuleClassProvider(
       PathFragment preludePath,
@@ -338,10 +337,7 @@
     this.configurationFragments = configurationFragments;
     this.configurationCollectionFactory = configurationCollectionFactory;
     this.prerequisiteValidator = prerequisiteValidator;
-    this.skylarkAccessibleJavaClasses = skylarkAccessibleJavaClasses;
-    this.skylarkValidationEnvironment = new ValidationEnvironment(
-        // TODO(bazel-team): refactor constructors so we don't have those null-s
-        createSkylarkRuleClassEnvironment(null, null));
+    this.globals = createGlobals(skylarkAccessibleJavaClasses);
   }
 
   public PrerequisiteValidator getPrerequisiteValidator() {
@@ -425,22 +421,46 @@
     return BuildOptions.of(configurationOptions, optionsProvider);
   }
 
-  @Override
-  public SkylarkEnvironment createSkylarkRuleClassEnvironment(
-      EventHandler eventHandler, String astFileContentHashCode) {
-    SkylarkEnvironment env = SkylarkModules.getNewEnvironment(eventHandler, astFileContentHashCode);
-    for (Map.Entry<String, SkylarkType> entry : skylarkAccessibleJavaClasses.entrySet()) {
-      env.update(entry.getKey(), entry.getValue().getType());
+  private Environment.Frame createGlobals(
+      ImmutableMap<String, SkylarkType> skylarkAccessibleJavaClasses) {
+    try (Mutability mutability = Mutability.create("ConfiguredRuleClassProvider globals")) {
+      Environment env = createSkylarkRuleClassEnvironment(
+          mutability, SkylarkModules.GLOBALS, null, null, null);
+      for (Map.Entry<String, SkylarkType> entry : skylarkAccessibleJavaClasses.entrySet()) {
+        env.setup(entry.getKey(), entry.getValue().getType());
+      }
+      return env.getGlobals();
     }
-    env.setLoadingPhase();
+  }
+
+  private Environment createSkylarkRuleClassEnvironment(
+      Mutability mutability,
+      Environment.Frame globals,
+      EventHandler eventHandler,
+      String astFileContentHashCode,
+      Map<PathFragment, Extension> importMap) {
+    Environment env = Environment.builder(mutability)
+        .setSkylark()
+        .setGlobals(globals)
+        .setEventHandler(eventHandler)
+        .setFileContentHashCode(astFileContentHashCode)
+        .setImportedExtensions(importMap)
+        .setLoadingPhase()
+        .build();
     return env;
   }
 
   @Override
-  public ValidationEnvironment getSkylarkValidationEnvironment() {
-    return skylarkValidationEnvironment;
+  public Environment createSkylarkRuleClassEnvironment(
+      Mutability mutability,
+      EventHandler eventHandler,
+      String astFileContentHashCode,
+      Map<PathFragment, Extension> importMap) {
+    return createSkylarkRuleClassEnvironment(
+        mutability, globals, eventHandler, astFileContentHashCode, importMap);
   }
 
+
   @Override
   public String getDefaultWorkspaceFile() {
     return defaultWorkspaceFile;