Stop removing onlyLoadingPhase bindings

They are now disabled based on a dynamic flag instead. It's all too easy
to bypass removal with duplication, anyway, as in my_native_glob = native.glob

--
MOS_MIGRATED_REVID=101714237

6 files changed, 2 insertions, 39 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
index c2186b9337..1f32cbd0b2 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkNativeModule.java
@@ -29,8 +29,8 @@ import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor;
 /**
  * A class for the Skylark native module.
  */
-@SkylarkModule(name = "native", namespace = true, onlyLoadingPhase = true, doc =
-      "A built-in module to support native rules and other package helper functions. "
+@SkylarkModule(name = "native", namespace = true, doc =
+    "A built-in module to support native rules and other package helper functions. "
     + "All native rules appear as functions in this module. Note that the native module is only "
     + "available in the loading phase (i.e. for macros, not for rule implementations).<br/>"
     + "Extra helper functions:")
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
index 35dfa4d481..635b1ed37e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java
@@ -51,7 +51,6 @@ import java.util.Map;
 @SkylarkModule(
   name = "attr",
   namespace = true,
-  onlyLoadingPhase = true,
   doc =
       "Module for creating new attributes. "
           + "They are only for use with the <a href=\"globals.html#rule\">rule</a> function."
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
index 274d527285..a7c9b4f91e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
@@ -59,9 +59,6 @@ public final class SkylarkRuleConfiguredTargetBuilder {
       SkylarkEnvironment env = ruleContext.getRule().getRuleClassObject()
           .getRuleDefinitionEnvironment().cloneEnv(
               ruleContext.getAnalysisEnvironment().getEventHandler());
-      // Collect the symbols to disable statically and pass at the next call, so we don't need to
-      // clone the RuleDefinitionEnvironment.
-      env.disableOnlyLoadingPhaseObjects();
       Object target = ruleImplementation.call(ImmutableList.<Object>of(skylarkRuleContext),
           ImmutableMap.<String, Object>of(), null, env);
 
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 b6acb050a9..3b4342a5e5 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
@@ -90,12 +90,6 @@ public class Environment {
   protected Map<PathFragment, SkylarkEnvironment> importedExtensions;
 
   /**
-   * A set of disabled namespaces propagating through function calling. This is needed because
-   * UserDefinedFunctions lock the definition Environment which should be immutable.
-   */
-  protected Set<Class<?>> disabledNameSpaces = new HashSet<>();
-
-  /**
    * A set of variables propagating through function calling. It's only used to call
    * native rules from Skylark build extensions.
    */
@@ -360,10 +354,6 @@ public class Environment {
 
   private Map<String, BaseFunction> getNamespaceFunctions(Class<?> nameSpace) {
     nameSpace = getCanonicalRepresentation(nameSpace);
-    if (disabledNameSpaces.contains(nameSpace)
-        || (parent != null && parent.disabledNameSpaces.contains(nameSpace))) {
-      return null;
-    }
     Environment topLevel = this;
     while (topLevel.parent != null) {
       topLevel = topLevel.parent;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkEnvironment.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkEnvironment.java
index c200b839a2..cc238179d8 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkEnvironment.java
@@ -22,9 +22,7 @@ import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.util.Fingerprint;
 
 import java.io.Serializable;
-import java.util.ArrayList;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
@@ -81,7 +79,6 @@ public class SkylarkEnvironment extends Environment implements Serializable {
       // This should never happen.
       throw new IllegalStateException(e);
     }
-    childEnv.disabledNameSpaces = callerEnv.disabledNameSpaces;
     return childEnv;
   }
 
@@ -198,24 +195,6 @@ public class SkylarkEnvironment extends Environment implements Serializable {
     return variable != null ? EvalUtils.getSkylarkType(variable.getClass()) : null;
   }
 
-  /**
-   * Removes the functions and the modules (i.e. the symbol of the module from the top level
-   * Environment and the functions attached to it) from the Environment which should be present
-   * only during the loading phase.
-   */
-  public void disableOnlyLoadingPhaseObjects() {
-    List<Class<?>> modulesToRemove = new ArrayList<>();
-    for (Map.Entry<String, Object> entry : env.entrySet()) {
-      Object object = entry.getValue();
-      if (object.getClass().isAnnotationPresent(SkylarkModule.class)) {
-        if (object.getClass().getAnnotation(SkylarkModule.class).onlyLoadingPhase()) {
-          modulesToRemove.add(entry.getValue().getClass());
-        }
-      }
-    }
-    disabledNameSpaces.addAll(modulesToRemove);
-  }
-
   public void handleEvent(Event event) {
     eventHandler.handle(event);
   }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModule.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModule.java
index a9c0f9b1f4..477bffb98d 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModule.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkModule.java
@@ -33,6 +33,4 @@ public @interface SkylarkModule {
   boolean documented() default true;
 
   boolean namespace() default false;
-
-  boolean onlyLoadingPhase() default false;
 }