diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index 721a422eba3e78..97b8b8cfd84108 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -133,7 +133,6 @@ java_library( ":test/execution_info", ":test/instrumented_files_info", ":test/test_configuration", - ":test/test_environment_info", ":test/test_sharding_strategy", ":test/test_trimming_transition_factory", ":toolchain_collection", @@ -340,6 +339,7 @@ java_library( ":resolved_toolchain_context", ":rule_configured_object_value", ":rule_definition_environment", + ":run_environment_info", ":starlark/args", ":starlark/bazel_build_api_globals", ":starlark/function_transition_util", @@ -357,7 +357,6 @@ java_library( ":test/execution_info", ":test/instrumented_files_info", ":test/test_configuration", - ":test/test_environment_info", ":test/test_sharding_strategy", ":toolchain_collection", ":toolchain_context", @@ -390,7 +389,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto", "//src/main/java/com/google/devtools/build/lib/causes", "//src/main/java/com/google/devtools/build/lib/cmdline", - "//src/main/java/com/google/devtools/build/lib/cmdline:LabelValidator", "//src/main/java/com/google/devtools/build/lib/collect", "//src/main/java/com/google/devtools/build/lib/collect/compacthashset", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", @@ -1003,6 +1001,18 @@ java_library( ], ) +java_library( + name = "run_environment_info", + srcs = ["RunEnvironmentInfo.java"], + deps = [ + "//src/main/java/com/google/devtools/build/lib/concurrent", + "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi", + "//src/main/java/net/starlark/java/eval", + "//third_party:guava", + ], +) + java_library( name = "rule_definition_environment", srcs = ["RuleDefinitionEnvironment.java"], @@ -2412,17 +2422,6 @@ java_library( ], ) -java_library( - name = "test/test_environment_info", - srcs = ["test/TestEnvironmentInfo.java"], - deps = [ - "//src/main/java/com/google/devtools/build/lib/concurrent", - "//src/main/java/com/google/devtools/build/lib/packages", - "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test", - "//third_party:guava", - ], -) - java_library( name = "test/test_sharding_strategy", srcs = ["test/TestShardingStrategy.java"], diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java index 244a575a1ca5f2..fed227896f0ccb 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java @@ -40,7 +40,6 @@ import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo; import com.google.devtools.build.lib.analysis.test.TestActionBuilder; import com.google.devtools.build.lib.analysis.test.TestConfiguration; -import com.google.devtools.build.lib.analysis.test.TestEnvironmentInfo; import com.google.devtools.build.lib.analysis.test.TestProvider; import com.google.devtools.build.lib.analysis.test.TestProvider.TestParams; import com.google.devtools.build.lib.analysis.test.TestTagsProvider; @@ -470,9 +469,8 @@ private TestProvider initializeTestProvider(FilesToRunProvider filesToRunProvide providersBuilder.getProvider( InstrumentedFilesInfo.STARLARK_CONSTRUCTOR.getKey())); - TestEnvironmentInfo environmentProvider = - (TestEnvironmentInfo) - providersBuilder.getProvider(TestEnvironmentInfo.PROVIDER.getKey()); + RunEnvironmentInfo environmentProvider = + (RunEnvironmentInfo) providersBuilder.getProvider(RunEnvironmentInfo.PROVIDER.getKey()); if (environmentProvider != null) { testActionBuilder.addExtraEnv(environmentProvider.getEnvironment()); testActionBuilder.addExtraInheritedEnv(environmentProvider.getInheritedEnvironment()); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunEnvironmentInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/RunEnvironmentInfo.java new file mode 100644 index 00000000000000..cb40eceefe43aa --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/RunEnvironmentInfo.java @@ -0,0 +1,106 @@ +// Copyright 2022 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.analysis; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.packages.BuiltinProvider; +import com.google.devtools.build.lib.packages.NativeInfo; +import com.google.devtools.build.lib.starlarkbuildapi.RunEnvironmentInfoApi; +import java.util.List; +import java.util.Map; +import net.starlark.java.eval.Dict; +import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.Sequence; +import net.starlark.java.eval.StarlarkList; + +/** + * Provider containing any additional environment variables for use in the executable or test + * action. + */ +@Immutable +public final class RunEnvironmentInfo extends NativeInfo implements RunEnvironmentInfoApi { + + /** Singleton instance of the provider type for {@link DefaultInfo}. */ + public static final RunEnvironmentInfoProvider PROVIDER = new RunEnvironmentInfoProvider(); + + private final ImmutableMap environment; + private final ImmutableList inheritedEnvironment; + private final boolean shouldErrorOnNonExecutableRule; + + /** Constructs a new provider with the given fixed and inherited environment variables. */ + public RunEnvironmentInfo( + Map environment, + List inheritedEnvironment, + boolean shouldErrorOnNonExecutableRule) { + this.environment = ImmutableMap.copyOf(Preconditions.checkNotNull(environment)); + this.inheritedEnvironment = + ImmutableList.copyOf(Preconditions.checkNotNull(inheritedEnvironment)); + this.shouldErrorOnNonExecutableRule = shouldErrorOnNonExecutableRule; + } + + @Override + public RunEnvironmentInfoProvider getProvider() { + return PROVIDER; + } + + /** + * Returns environment variables which should be set when the target advertising this provider is + * executed. + */ + @Override + public ImmutableMap getEnvironment() { + return environment; + } + + /** + * Returns names of environment variables whose value should be inherited from the shell + * environment when the target advertising this provider is executed. + */ + @Override + public ImmutableList getInheritedEnvironment() { + return inheritedEnvironment; + } + + /** + * Returns whether advertising this provider on a non-executable (and thus non-test) rule should + * result in an error or a warning. The latter is required to not break testing.TestEnvironment, + * which is now implemented via RunEnvironmentInfo. + */ + public boolean shouldErrorOnNonExecutableRule() { + return shouldErrorOnNonExecutableRule; + } + + /** Provider implementation for {@link RunEnvironmentInfoApi}. */ + public static class RunEnvironmentInfoProvider extends BuiltinProvider + implements RunEnvironmentInfoApi.RunEnvironmentInfoApiProvider { + + private RunEnvironmentInfoProvider() { + super("RunEnvironmentInfo", RunEnvironmentInfo.class); + } + + @Override + public RunEnvironmentInfoApi constructor( + Dict environment, Sequence inheritedEnvironment) throws EvalException { + return new RunEnvironmentInfo( + Dict.cast(environment, String.class, String.class, "environment"), + StarlarkList.immutableCopyOf( + Sequence.cast(inheritedEnvironment, String.class, "inherited_environment")), + /* shouldErrorOnNonExecutableRule */ true); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java index 0166b2d5e971d2..4c4d96cb14bf5a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java @@ -475,9 +475,7 @@ private static CommandLine computeArgs(RuleContext ruleContext, CommandLine addi } private static ActionEnvironment computeActionEnvironment(RuleContext ruleContext) { - // Currently, "env" and "env_inherit" are not added to Starlark-defined rules (unlike "args"), - // in order to avoid breaking existing Starlark rules that use those attribute names. - // TODO(b/176554800): Support "env" and "env_inherit" for Starlark-defined rules. + // Executable Starlark rules can use RunEnvironmentInfo to specify environment variables. boolean isNativeRule = ruleContext.getRule().getRuleClassObject().getRuleDefinitionEnvironmentLabel() == null; if (!isNativeRule diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkModules.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkModules.java index a3105a713c1b6c..c44ae4c4a9a2bb 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkModules.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkModules.java @@ -18,6 +18,7 @@ import com.google.devtools.build.lib.analysis.ActionsProvider; import com.google.devtools.build.lib.analysis.DefaultInfo; import com.google.devtools.build.lib.analysis.OutputGroupInfo; +import com.google.devtools.build.lib.analysis.RunEnvironmentInfo; import com.google.devtools.build.lib.packages.StarlarkLibrary; import com.google.devtools.build.lib.packages.StructProvider; import net.starlark.java.eval.Starlark; @@ -44,5 +45,6 @@ public static void addPredeclared(ImmutableMap.Builder predeclar predeclared.put("OutputGroupInfo", OutputGroupInfo.STARLARK_CONSTRUCTOR); predeclared.put("Actions", ActionsProvider.INSTANCE); predeclared.put("DefaultInfo", DefaultInfo.PROVIDER); + predeclared.put("RunEnvironmentInfo", RunEnvironmentInfo.PROVIDER); } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleConfiguredTargetUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleConfiguredTargetUtil.java index 9a74ec1ef0d6c0..3a18f0afb86a14 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleConfiguredTargetUtil.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleConfiguredTargetUtil.java @@ -24,6 +24,7 @@ import com.google.devtools.build.lib.analysis.DefaultInfo; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.RunEnvironmentInfo; import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.RunfilesProvider; import com.google.devtools.build.lib.analysis.RunfilesSupport; @@ -348,6 +349,17 @@ private static void addProviders( if (getProviderKey(declaredProvider).equals(DefaultInfo.PROVIDER.getKey())) { parseDefaultProviderFields((DefaultInfo) declaredProvider, context, builder); defaultProviderProvidedExplicitly = true; + } else if (getProviderKey(declaredProvider).equals(RunEnvironmentInfo.PROVIDER.getKey()) + && !(context.isExecutable() || context.getRuleContext().isTestTarget())) { + String message = + "Returning RunEnvironmentInfo from a non-executable, non-test target has no effect"; + RunEnvironmentInfo runEnvironmentInfo = (RunEnvironmentInfo) declaredProvider; + if (runEnvironmentInfo.shouldErrorOnNonExecutableRule()) { + context.getRuleContext().ruleError(message); + } else { + context.getRuleContext().ruleWarning(message); + builder.addStarlarkDeclaredProvider(declaredProvider); + } } else { builder.addStarlarkDeclaredProvider(declaredProvider); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestEnvironmentInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestEnvironmentInfo.java deleted file mode 100644 index d5245338342d2d..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestEnvironmentInfo.java +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright 2015 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.analysis.test; - -import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; -import com.google.devtools.build.lib.packages.BuiltinProvider; -import com.google.devtools.build.lib.packages.NativeInfo; -import com.google.devtools.build.lib.starlarkbuildapi.test.TestEnvironmentInfoApi; -import java.util.List; -import java.util.Map; - -/** Provider containing any additional environment variables for use in the test action. */ -@Immutable -public final class TestEnvironmentInfo extends NativeInfo implements TestEnvironmentInfoApi { - - /** Starlark constructor and identifier for TestEnvironmentInfo. */ - public static final BuiltinProvider PROVIDER = - new BuiltinProvider("TestEnvironment", TestEnvironmentInfo.class) {}; - - private final ImmutableMap environment; - private final ImmutableList inheritedEnvironment; - - /** Constructs a new provider with the given fixed and inherited environment variables. */ - public TestEnvironmentInfo(Map environment, List inheritedEnvironment) { - this.environment = ImmutableMap.copyOf(Preconditions.checkNotNull(environment)); - this.inheritedEnvironment = - ImmutableList.copyOf(Preconditions.checkNotNull(inheritedEnvironment)); - } - - @Override - public BuiltinProvider getProvider() { - return PROVIDER; - } - - /** - * Returns environment variables which should be set on the test action. - */ - @Override - public Map getEnvironment() { - return environment; - } - - /** Returns names of environment variables whose value should be obtained from the environment. */ - @Override - public ImmutableList getInheritedEnvironment() { - return inheritedEnvironment; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/rules/BUILD b/src/main/java/com/google/devtools/build/lib/rules/BUILD index 15337da99ff737..2763e8412795e2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/BUILD @@ -123,12 +123,12 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", "//src/main/java/com/google/devtools/build/lib/analysis:rule_definition_environment", + "//src/main/java/com/google/devtools/build/lib/analysis:run_environment_info", "//src/main/java/com/google/devtools/build/lib/analysis:test/analysis_failure_info", "//src/main/java/com/google/devtools/build/lib/analysis:test/analysis_test_result_info", "//src/main/java/com/google/devtools/build/lib/analysis:test/execution_info", "//src/main/java/com/google/devtools/build/lib/analysis:test/instrumented_files_info", "//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration", - "//src/main/java/com/google/devtools/build/lib/analysis:test/test_environment_info", "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_collection", "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_provider", "//src/main/java/com/google/devtools/build/lib/concurrent", diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java b/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java index 1c6fcb6c23dad9..5373dd0eb6dedf 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java @@ -13,8 +13,8 @@ // limitations under the License. package com.google.devtools.build.lib.rules.test; +import com.google.devtools.build.lib.analysis.RunEnvironmentInfo; import com.google.devtools.build.lib.analysis.test.ExecutionInfo; -import com.google.devtools.build.lib.analysis.test.TestEnvironmentInfo; import com.google.devtools.build.lib.starlarkbuildapi.test.TestingModuleApi; import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; @@ -31,13 +31,14 @@ public ExecutionInfo executionInfo(Dict requirements /* * } @Override - public TestEnvironmentInfo testEnvironment( + public RunEnvironmentInfo testEnvironment( Dict environment /* */, Sequence inheritedEnvironment /* */) throws EvalException { - return new TestEnvironmentInfo( + return new RunEnvironmentInfo( Dict.cast(environment, String.class, String.class, "environment"), StarlarkList.immutableCopyOf( - Sequence.cast(inheritedEnvironment, String.class, "inherited_environment"))); + Sequence.cast(inheritedEnvironment, String.class, "inherited_environment")), + /* shouldErrorOnNonExecutableRule */ false); } } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD b/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD index df7249438877ba..2de67618a3c8c0 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD @@ -47,6 +47,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:no_build_event", "//src/main/java/com/google/devtools/build/lib/analysis:no_build_request_finished_event", "//src/main/java/com/google/devtools/build/lib/analysis:print_action_visitor", + "//src/main/java/com/google/devtools/build/lib/analysis:run_environment_info", "//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration", "//src/main/java/com/google/devtools/build/lib/buildeventstream", "//src/main/java/com/google/devtools/build/lib/cmdline", diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java index 442fd4477eb69b..d398b1e0c00abf 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java @@ -20,9 +20,11 @@ import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; +import com.google.devtools.build.lib.actions.ActionEnvironment; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.CommandLine; @@ -32,6 +34,7 @@ import com.google.devtools.build.lib.analysis.AliasProvider; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FilesToRunProvider; +import com.google.devtools.build.lib.analysis.RunEnvironmentInfo; import com.google.devtools.build.lib.analysis.RunfilesSupport; import com.google.devtools.build.lib.analysis.ShToolchain; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; @@ -487,11 +490,18 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti } else { workingDir = targetToRunRunfilesDir != null ? targetToRunRunfilesDir : env.getWorkingDirectory(); + ActionEnvironment actionEnvironment = ActionEnvironment.EMPTY; if (targetToRunRunfilesSupport != null) { - targetToRunRunfilesSupport - .getActionEnvironment() - .resolve(runEnvironment, env.getClientEnv()); + actionEnvironment = targetToRunRunfilesSupport.getActionEnvironment(); } + RunEnvironmentInfo environmentProvider = targetToRun.get(RunEnvironmentInfo.PROVIDER); + if (environmentProvider != null) { + actionEnvironment = + actionEnvironment.withAdditionalVariables( + environmentProvider.getEnvironment(), + ImmutableSet.copyOf(environmentProvider.getInheritedEnvironment())); + } + actionEnvironment.resolve(runEnvironment, env.getClientEnv()); try { List args = computeArgs(targetToRun, commandLineArgs); constructCommandLine( @@ -523,7 +533,7 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti runEnvironment, workingDir.getPathString(), configuration.checksum(), - /* executionPlatform= */ null); + /* executionPlatformAsLabelString= */ null); PathFragment shExecutable = ShToolchain.getPath(configuration); if (shExecutable.isEmpty()) { diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/RunEnvironmentInfoApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/RunEnvironmentInfoApi.java new file mode 100644 index 00000000000000..3de4fca081a126 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/RunEnvironmentInfoApi.java @@ -0,0 +1,103 @@ +// Copyright 2022 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.starlarkbuildapi; + +import com.google.devtools.build.docgen.annot.DocCategory; +import com.google.devtools.build.docgen.annot.StarlarkConstructor; +import com.google.devtools.build.lib.starlarkbuildapi.core.ProviderApi; +import com.google.devtools.build.lib.starlarkbuildapi.core.StructApi; +import java.util.List; +import java.util.Map; +import net.starlark.java.annot.Param; +import net.starlark.java.annot.ParamType; +import net.starlark.java.annot.StarlarkBuiltin; +import net.starlark.java.annot.StarlarkMethod; +import net.starlark.java.eval.Dict; +import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.Sequence; + +/** + * Provider containing any additional environment variables for use when running executables, either + * in test actions or when executed via the run command. + */ +@StarlarkBuiltin( + name = "RunEnvironmentInfo", + category = DocCategory.PROVIDER, + doc = + "A provider that can be returned from executable rules to control the environment in" + + " which their executable is executed.") +public interface RunEnvironmentInfoApi extends StructApi { + + @StarlarkMethod( + name = "environment", + doc = + "A map of string keys and values that represent environment variables and their values." + + " These will be made available when the target that returns this provider is" + + " executed, either as a test or via the run command.", + structField = true) + Map getEnvironment(); + + @StarlarkMethod( + name = "inherited_environment", + doc = + "A sequence of names of environment variables. These variables are made available with" + + " their current value taken from the shell environment when the target that returns" + + " this provider is executed, either as a test or via the run command. If a variable" + + " is contained in both environment and" + + " inherited_environment, the value inherited from the shell" + + " environment will take precedence if set.", + structField = true) + List getInheritedEnvironment(); + + /** Provider for {@link RunEnvironmentInfoApi}. */ + @StarlarkBuiltin(name = "Provider", category = DocCategory.PROVIDER, documented = false, doc = "") + interface RunEnvironmentInfoApiProvider extends ProviderApi { + + @StarlarkMethod( + name = "RunEnvironmentInfo", + doc = "", + documented = false, + parameters = { + @Param( + name = "environment", + defaultValue = "{}", + named = true, + positional = true, + doc = + "A map of string keys and values that represent environment variables and their" + + " values. These will be made available when the target that returns this" + + " provider is executed, either as a test or via the run command."), + @Param( + name = "inherited_environment", + allowedTypes = {@ParamType(type = Sequence.class, generic1 = String.class)}, + defaultValue = "[]", + named = true, + positional = true, + doc = + "A sequence of names of environment variables. These variables are made " + + " available with their current value taken from the shell environment" + + " when the target that returns this provider is executed, either as a" + + " test or via the run command. If a variable is contained in both " + + "environment and inherited_environment, the value" + + " inherited from the shell environment will take precedence if set.") + }, + selfCall = true) + @StarlarkConstructor + RunEnvironmentInfoApi constructor( + Dict environment, // expected + Sequence inheritedEnvironment /* expected */) + throws EvalException; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestEnvironmentInfoApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestEnvironmentInfoApi.java deleted file mode 100644 index ed11b24af45ddf..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestEnvironmentInfoApi.java +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2018 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.starlarkbuildapi.test; - -import com.google.devtools.build.lib.starlarkbuildapi.core.StructApi; -import java.util.List; -import java.util.Map; -import net.starlark.java.annot.StarlarkBuiltin; -import net.starlark.java.annot.StarlarkMethod; - -/** Provider containing any additional environment variables for use in the test action. */ -@StarlarkBuiltin(name = "TestEnvironmentInfo", doc = "", documented = false) -public interface TestEnvironmentInfoApi extends StructApi { - - @StarlarkMethod( - name = "environment", - doc = "A dict containing environment variables which should be set on the test action.", - structField = true) - Map getEnvironment(); - - @StarlarkMethod( - name = "inherited_environment", - doc = "A list of variables that the test action should inherit from the shell environment.", - structField = true) - List getInheritedEnvironment(); -} diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestingModuleApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestingModuleApi.java index 1570f8039bbfbe..dd91fdc7739a1b 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestingModuleApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/test/TestingModuleApi.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.starlarkbuildapi.test; +import com.google.devtools.build.lib.starlarkbuildapi.RunEnvironmentInfoApi; import net.starlark.java.annot.Param; import net.starlark.java.annot.ParamType; import net.starlark.java.annot.StarlarkBuiltin; @@ -49,12 +50,12 @@ public interface TestingModuleApi extends StarlarkValue { ExecutionInfoApi executionInfo(Dict requirements // expected ) throws EvalException; - // TODO(bazel-team): Change this function to be the actual TestEnvironmentInfo.PROVIDER. @StarlarkMethod( name = "TestEnvironment", doc = - "Creates a new test environment provider. Use this provider to specify extra" - + "environment variables to be made available during test execution.", + "Deprecated: Use RunEnvironmentInfo instead. Creates a new test environment " + + "provider. Use this provider to specify extra environment variables to be made " + + "available during test execution.", parameters = { @Param( name = "environment", @@ -76,7 +77,7 @@ ExecutionInfoApi executionInfo(Dict requirements // expec + " and inherited_environment, the value inherited from the" + " shell environment will take precedence if set.") }) - TestEnvironmentInfoApi testEnvironment( + RunEnvironmentInfoApi testEnvironment( Dict environment, // expected Sequence inheritedEnvironment /* expected */) throws EvalException; diff --git a/src/test/java/com/google/devtools/build/lib/rules/test/BUILD b/src/test/java/com/google/devtools/build/lib/rules/test/BUILD index 8f34480f06425c..764210c5b4484f 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/test/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/test/BUILD @@ -20,8 +20,8 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", + "//src/main/java/com/google/devtools/build/lib/analysis:run_environment_info", "//src/main/java/com/google/devtools/build/lib/analysis:test/execution_info", - "//src/main/java/com/google/devtools/build/lib/analysis:test/test_environment_info", "//src/test/java/com/google/devtools/build/lib/analysis/util", "//third_party:junit4", "//third_party:truth", diff --git a/src/test/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModuleTest.java b/src/test/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModuleTest.java index 77943fef1eaaf7..4a63b185ab9f2e 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModuleTest.java @@ -16,8 +16,8 @@ import static com.google.common.truth.Truth.assertThat; import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.RunEnvironmentInfo; import com.google.devtools.build.lib.analysis.test.ExecutionInfo; -import com.google.devtools.build.lib.analysis.test.TestEnvironmentInfo; import com.google.devtools.build.lib.analysis.test.TestProvider; import com.google.devtools.build.lib.analysis.test.TestRunnerAction; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; @@ -74,7 +74,7 @@ public void testStarlarkRulePropagatesTestEnvironmentProvider() throws Exception ")"); ConfiguredTarget starlarkTarget = getConfiguredTarget("//examples/apple_starlark:my_target"); - TestEnvironmentInfo provider = starlarkTarget.get(TestEnvironmentInfo.PROVIDER); + RunEnvironmentInfo provider = starlarkTarget.get(RunEnvironmentInfo.PROVIDER); assertThat(provider.getEnvironment().get("XCODE_VERSION_OVERRIDE")).isEqualTo("7.3.1"); } @@ -105,7 +105,8 @@ public void testStarlarkRulePropagatesTestEnvironmentProviderWithInheritedEnv() ")"); ConfiguredTarget starlarkTarget = getConfiguredTarget("//examples/apple_starlark:my_target"); - TestEnvironmentInfo provider = starlarkTarget.get(TestEnvironmentInfo.PROVIDER); + RunEnvironmentInfo provider = + (RunEnvironmentInfo) starlarkTarget.get(RunEnvironmentInfo.PROVIDER.getKey()); assertThat(provider.getEnvironment()).containsEntry("XCODE_VERSION_OVERRIDE", "7.3.1"); assertThat(provider.getInheritedEnvironment()).contains("DEVELOPER_DIR"); diff --git a/src/test/java/com/google/devtools/build/lib/starlark/BUILD b/src/test/java/com/google/devtools/build/lib/starlark/BUILD index 3f237073bfb5f7..0ab20ce6319a18 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/BUILD +++ b/src/test/java/com/google/devtools/build/lib/starlark/BUILD @@ -45,6 +45,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", "//src/main/java/com/google/devtools/build/lib/analysis:file_provider", "//src/main/java/com/google/devtools/build/lib/analysis:resolved_toolchain_context", + "//src/main/java/com/google/devtools/build/lib/analysis:run_environment_info", "//src/main/java/com/google/devtools/build/lib/analysis:starlark/args", "//src/main/java/com/google/devtools/build/lib/analysis:starlark/starlark_exec_group_collection", "//src/main/java/com/google/devtools/build/lib/analysis:test/analysis_test_result_info", diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java index c7ffc30df3adfb..52fbac82d2ef1c 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java @@ -22,6 +22,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; @@ -29,6 +30,7 @@ import com.google.devtools.build.lib.analysis.FileProvider; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.OutputGroupInfo; +import com.google.devtools.build.lib.analysis.RunEnvironmentInfo; import com.google.devtools.build.lib.analysis.RunfilesProvider; import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition; import com.google.devtools.build.lib.analysis.configuredtargets.FileConfiguredTarget; @@ -42,6 +44,7 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.Depset; import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; import com.google.devtools.build.lib.packages.BuildSetting; import com.google.devtools.build.lib.packages.FunctionSplitTransitionAllowlist; @@ -3570,4 +3573,88 @@ public void bzlFileWithErrorsLoadedThroughMultipleLoadPathsWithTheLatterOneNotHa .hasMessageThat() .contains("compilation of module 'test/starlark/error.bzl' failed"); } + + @Test + public void testStarlarkRulePropagatesRunEnvironmentProvider() throws Exception { + scratch.file( + "examples/rules.bzl", + "def my_rule_impl(ctx):", + " script = ctx.actions.declare_file(ctx.attr.name)", + " ctx.actions.write(script, '', is_executable = True)", + " run_env = RunEnvironmentInfo(", + " {'FIXED': 'fixed'},", + " ['INHERITED']", + " )", + " return [", + " DefaultInfo(executable = script),", + " run_env,", + " ]", + "my_rule = rule(", + " implementation = my_rule_impl,", + " attrs = {},", + " executable = True,", + ")"); + scratch.file( + "examples/BUILD", + "load(':rules.bzl', 'my_rule')", + "my_rule(", + " name = 'my_target',", + ")"); + + ConfiguredTarget starlarkTarget = getConfiguredTarget("//examples:my_target"); + RunEnvironmentInfo provider = starlarkTarget.get(RunEnvironmentInfo.PROVIDER); + + assertThat(provider.getEnvironment()).containsExactly("FIXED", "fixed"); + assertThat(provider.getInheritedEnvironment()).containsExactly("INHERITED"); + } + + @Test + public void nonExecutableStarlarkRuleReturningRunEnvironmentInfoErrors() throws Exception { + scratch.file( + "examples/rules.bzl", + "def my_rule_impl(ctx):", + " return [RunEnvironmentInfo()]", + "my_rule = rule(", + " implementation = my_rule_impl,", + " attrs = {},", + ")"); + scratch.file( + "examples/BUILD", + "load(':rules.bzl', 'my_rule')", + "my_rule(", + " name = 'my_target',", + ")"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//examples:my_target"); + assertContainsEvent( + "in my_rule rule //examples:my_target: Returning RunEnvironmentInfo from a non-executable," + + " non-test target has no effect", + ImmutableSet.of(EventKind.ERROR)); + } + + @Test + public void nonExecutableStarlarkRuleReturningTestEnvironmentProducesAWarning() throws Exception { + scratch.file( + "examples/rules.bzl", + "def my_rule_impl(ctx):", + " return [testing.TestEnvironment(environment = {})]", + "my_rule = rule(", + " implementation = my_rule_impl,", + " attrs = {},", + ")"); + scratch.file( + "examples/BUILD", + "load(':rules.bzl', 'my_rule')", + "my_rule(", + " name = 'my_target',", + ")"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//examples:my_target"); + assertContainsEvent( + "in my_rule rule //examples:my_target: Returning RunEnvironmentInfo from a non-executable," + + " non-test target has no effect", + ImmutableSet.of(EventKind.WARNING)); + } } diff --git a/src/test/shell/bazel/bazel_rules_test.sh b/src/test/shell/bazel/bazel_rules_test.sh index 5650f0d458e283..5807d4fd45085e 100755 --- a/src/test/shell/bazel/bazel_rules_test.sh +++ b/src/test/shell/bazel/bazel_rules_test.sh @@ -612,6 +612,7 @@ if not "%FIXED_ONLY%" == "fixed" exit /B 1 if not "%FIXED_AND_INHERITED%" == "inherited" exit /B 1 if not "%FIXED_AND_INHERITED_BUT_NOT_SET%" == "fixed" exit /B 1 if not "%INHERITED_ONLY%" == "inherited" exit /B 1 +if defined INHERITED_BUT_UNSET exit /B 1 """ EOF else @@ -621,7 +622,8 @@ _SCRIPT_CONTENT = """#!/bin/bash [[ "$FIXED_ONLY" == "fixed" \ && "$FIXED_AND_INHERITED" == "inherited" \ && "$FIXED_AND_INHERITED_BUT_NOT_SET" == "fixed" \ - && "$INHERITED_ONLY" == "inherited" ]] + && "$INHERITED_ONLY" == "inherited" \ + && -z "${INHERITED_BUT_UNSET+default}" ]] """ EOF fi @@ -644,13 +646,14 @@ def _my_test_impl(ctx): "FIXED_AND_INHERITED", "FIXED_AND_INHERITED_BUT_NOT_SET", "INHERITED_ONLY", + "INHERITED_BUT_UNSET", ] ) return [ DefaultInfo( executable = test_sh, ), - test_env + test_env, ] my_test = rule( @@ -661,7 +664,83 @@ my_test = rule( EOF FIXED_AND_INHERITED=inherited INHERITED_ONLY=inherited \ - bazel test //pkg:my_test &> /dev/null || fail "Test should pass" + bazel test //pkg:my_test >$TEST_log 2>&1 || fail "Test should pass" +} + +function test_starlark_rule_with_run_environment() { + mkdir pkg + cat >pkg/BUILD <<'EOF' +load(":rules.bzl", "my_executable") +my_executable( + name = "my_executable", +) +EOF + + # On Windows this file needs to be acceptable by CreateProcessW(), rather + # than a Bourne script. + if "$is_windows"; then + cat >pkg/rules.bzl <<'EOF' +_SCRIPT_EXT = ".bat" +_SCRIPT_CONTENT = """@ECHO OFF +if not "%FIXED_ONLY%" == "fixed" exit /B 1 +if not "%FIXED_AND_INHERITED%" == "inherited" exit /B 1 +if not "%FIXED_AND_INHERITED_BUT_NOT_SET%" == "fixed" exit /B 1 +if not "%INHERITED_ONLY%" == "inherited" exit /B 1 +if defined INHERITED_BUT_UNSET exit /B 1 +""" +EOF + else + cat >pkg/rules.bzl <<'EOF' +_SCRIPT_EXT = ".sh" +_SCRIPT_CONTENT = """#!/bin/bash +set -x +env +[[ "$FIXED_ONLY" == "fixed" \ + && "$FIXED_AND_INHERITED" == "inherited" \ + && "$FIXED_AND_INHERITED_BUT_NOT_SET" == "fixed" \ + && "$INHERITED_ONLY" == "inherited" \ + && -z "${INHERITED_BUT_UNSET+default}" ]] +""" +EOF + fi + + cat >>pkg/rules.bzl <<'EOF' +def _my_executable_impl(ctx): + executable_sh = ctx.actions.declare_file(ctx.attr.name + _SCRIPT_EXT) + ctx.actions.write( + output = executable_sh, + content = _SCRIPT_CONTENT, + is_executable = True, + ) + run_env = RunEnvironmentInfo( + { + "FIXED_AND_INHERITED": "fixed", + "FIXED_AND_INHERITED_BUT_NOT_SET": "fixed", + "FIXED_ONLY": "fixed", + }, + [ + "FIXED_AND_INHERITED", + "FIXED_AND_INHERITED_BUT_NOT_SET", + "INHERITED_ONLY", + "INHERITED_BUT_UNSET", + ] + ) + return [ + DefaultInfo( + executable = executable_sh, + ), + run_env, + ] + +my_executable = rule( + implementation = _my_executable_impl, + attrs = {}, + executable = True, +) +EOF + + FIXED_AND_INHERITED=inherited INHERITED_ONLY=inherited \ + bazel run //pkg:my_executable >$TEST_log 2>&1 || fail "Binary should have exit code 0" } run_suite "rules test"