diff options
author | brandjon <brandjon@google.com> | 2017-10-20 22:53:48 +0200 |
---|---|---|
committer | Dmitry Lomov <dslomov@google.com> | 2017-10-23 17:16:13 +0200 |
commit | e1e7725abffdc8687fea03d1fe134a1819fb8f37 (patch) | |
tree | 2b7c4154b6b3368736a1830898987efd72223a8c | |
parent | 6738c36265c544a116751372ce74209305f44071 (diff) |
Don't set globals in environments that aren't read
There are two Environments involved in running an implementation callback for rules/aspects/repository-rules: (1) The static environment of the function's definition, and (2) the caller's dynamic environment. The function will only consult (1), so trying to set anything on (2) is misleading.
RELNOTES: None
PiperOrigin-RevId: 172929301
4 files changed, 9 insertions, 17 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java index d993620ff8..a04501889e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java @@ -84,20 +84,14 @@ public final class SkylarkRuleConfiguredTargetUtil { Environment env = Environment.builder(mutability) .setCallerLabel(ruleContext.getLabel()) - .setGlobals( - ruleContext - .getRule() - .getRuleClassObject() - .getRuleDefinitionEnvironment() - .getGlobals()) .setSemantics(skylarkSemantics) .setEventHandler(ruleContext.getAnalysisEnvironment().getEventHandler()) .build(); // NB: loading phase functions are not available: this is analysis already, // so we do *not* setLoadingPhase(). Object target = ruleImplementation.call( - ImmutableList.<Object>of(skylarkRuleContext), - ImmutableMap.<String, Object>of(), + /*args=*/ ImmutableList.of(skylarkRuleContext), + /*kwargs*/ ImmutableMap.of(), /*ast=*/ null, env); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java index 18ac7b4060..6f708863fd 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java @@ -59,7 +59,6 @@ public class SkylarkRepositoryFunction extends RepositoryFunction { com.google.devtools.build.lib.syntax.Environment buildEnv = com.google.devtools.build.lib.syntax.Environment.builder(mutability) .useDefaultSemantics() - .setGlobals(rule.getRuleClassObject().getRuleDefinitionEnvironment().getGlobals()) .setEventHandler(env.getListener()) .build(); SkylarkRepositoryContext skylarkRepositoryContext = @@ -72,8 +71,8 @@ public class SkylarkRepositoryFunction extends RepositoryFunction { // structure as it is. Object retValue = function.call( - ImmutableList.<Object>of(skylarkRepositoryContext), - ImmutableMap.<String, Object>of(), + /*args=*/ ImmutableList.of(skylarkRepositoryContext), + /*kwargs=*/ ImmutableMap.of(), null, buildEnv); if (retValue != Runtime.NONE) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java index 39238de5f2..8aeac24dac 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java @@ -66,19 +66,19 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory { } Environment env = Environment.builder(mutability) - .setGlobals(skylarkAspect.getFuncallEnv().getGlobals()) .setSemantics(analysisEnv.getSkylarkSemantics()) .setEventHandler(analysisEnv.getEventHandler()) - .build(); // NB: loading phase functions are not available: this is analysis already, - // so we do *not* setLoadingPhase(). + // NB: loading phase functions are not available: this is analysis already, so we do + // *not* setLoadingPhase(). + .build(); Object aspectSkylarkObject; try { aspectSkylarkObject = skylarkAspect .getImplementation() .call( - ImmutableList.<Object>of(base, skylarkRuleContext), - ImmutableMap.<String, Object>of(), + /*args=*/ ImmutableList.of(base, skylarkRuleContext), + /*kwarg=*/ ImmutableMap.of(), /*ast=*/ null, env); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallbackFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallbackFunction.java index 1786b9bc28..ed79584a05 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallbackFunction.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallbackFunction.java @@ -42,7 +42,6 @@ public class SkylarkCallbackFunction { Environment env = Environment.builder(mutability) .setSemantics(funcallEnv.getSemantics()) .setEventHandler(funcallEnv.getEventHandler()) - .setGlobals(funcallEnv.getGlobals()) .build(); return callback.call(buildArgumentList(ctx, arguments), null, ast, env); } catch (ClassCastException | IllegalArgumentException e) { |