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

1 files changed, 8 insertions, 6 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index 5cb76854ff..12eea02a7d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -46,10 +46,10 @@ import com.google.devtools.build.lib.skyframe.ASTFileLookupValue.ASTLookupInputE
 import com.google.devtools.build.lib.skyframe.GlobValue.InvalidGlobPatternException;
 import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction.SkylarkImportFailedException;
 import com.google.devtools.build.lib.syntax.BuildFileAST;
+import com.google.devtools.build.lib.syntax.Environment.Extension;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Label;
 import com.google.devtools.build.lib.syntax.ParserInputSource;
-import com.google.devtools.build.lib.syntax.SkylarkEnvironment;
 import com.google.devtools.build.lib.syntax.Statement;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.Path;
@@ -553,7 +553,8 @@ public class PackageFunction implements SkyFunction {
     if (eventHandler.hasErrors()) {
       importResult =
           new SkylarkImportResult(
-              ImmutableMap.<PathFragment, SkylarkEnvironment>of(), ImmutableList.<Label>of());
+              ImmutableMap.<PathFragment, Extension>of(),
+              ImmutableList.<Label>of());
       includeRepositoriesFetched = true;
     } else {
       importResult =
@@ -581,7 +582,7 @@ public class PackageFunction implements SkyFunction {
       Environment env)
       throws PackageFunctionException {
     ImmutableMap<Location, PathFragment> imports = buildFileAST.getImports();
-    Map<PathFragment, SkylarkEnvironment> importMap = new HashMap<>();
+    Map<PathFragment, Extension> importMap = new HashMap<>();
     ImmutableList.Builder<SkylarkFileDependency> fileDependencies = ImmutableList.builder();
     try {
       for (Map.Entry<Location, PathFragment> entry : imports.entrySet()) {
@@ -601,7 +602,7 @@ public class PackageFunction implements SkyFunction {
                 InconsistentFilesystemException.class, ASTLookupInputException.class,
                 BuildFileNotFoundException.class);
         if (importLookupValue != null) {
-          importMap.put(importFile, importLookupValue.getImportedEnvironment());
+          importMap.put(importFile, importLookupValue.getEnvironmentExtension());
           fileDependencies.add(importLookupValue.getDependency());
         }
       }
@@ -882,9 +883,10 @@ public class PackageFunction implements SkyFunction {
 
   /** A simple value class to store the result of the Skylark imports.*/
   private static final class SkylarkImportResult {
-    private final Map<PathFragment, SkylarkEnvironment> importMap;
+    private final Map<PathFragment, Extension> importMap;
     private final ImmutableList<Label> fileDependencies;
-    private SkylarkImportResult(Map<PathFragment, SkylarkEnvironment> importMap,
+    private SkylarkImportResult(
+        Map<PathFragment, Extension> importMap,
         ImmutableList<Label> fileDependencies) {
       this.importMap = importMap;
       this.fileDependencies = fileDependencies;