Skip to content

Commit

Permalink
Move --repo_env to common options
Browse files Browse the repository at this point in the history
Move --repo_env into common options and inject the value through CommandEnvironment since CommonCommandOptions cannot be accessed in SkyframeExecutor directly due to a circular dependency in the build.

Addresses bazelbuild#12689

Closes bazelbuild#13003.

PiperOrigin-RevId: 365035772
  • Loading branch information
chancila authored and copybara-github committed Mar 25, 2021
1 parent b65e38d commit c2bdd03
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -366,20 +366,6 @@ public String getTypeDescription() {
+ " wins, options for different variables accumulate.")
public List<Map.Entry<String, String>> hostActionEnvironment;

@Option(
name = "repo_env",
converter = Converters.OptionalAssignmentConverter.class,
allowMultiple = true,
defaultValue = "null",
documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
effectTags = {OptionEffectTag.ACTION_COMMAND_LINES},
help =
"Specifies additional environment variables to be available only for repository rules."
+ " Note that repository rules see the full environment anyway, but in this way"
+ " configuration information can be passed to repositories through options without"
+ " invalidating the action graph.")
public List<Map.Entry<String, String>> repositoryEnvironment;

@Option(
name = "collect_code_coverage",
defaultValue = "false",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ public class CommandEnvironment {
private final Set<String> visibleActionEnv = new TreeSet<>();
private final Set<String> visibleTestEnv = new TreeSet<>();
private final Map<String, String> repoEnv = new TreeMap<>();
private final Map<String, String> repoEnvFromOptions = new TreeMap<>();
private final TimestampGranularityMonitor timestampGranularityMonitor;
private final Thread commandThread;
private final Command command;
Expand Down Expand Up @@ -245,17 +246,15 @@ public void exit(AbruptExitException exception) {
}
}

CoreOptions configOpts = options.getOptions(CoreOptions.class);
if (configOpts != null) {
for (Map.Entry<String, String> entry : configOpts.repositoryEnvironment) {
String name = entry.getKey();
String value = entry.getValue();
if (value == null) {
value = System.getenv(name);
}
if (value != null) {
repoEnv.put(name, value);
}
for (Map.Entry<String, String> entry : commandOptions.repositoryEnvironment) {
String name = entry.getKey();
String value = entry.getValue();
if (value == null) {
value = System.getenv(name);
}
if (value != null) {
repoEnv.put(entry.getKey(), entry.getValue());
repoEnvFromOptions.put(entry.getKey(), entry.getValue());
}
}
}
Expand Down Expand Up @@ -709,6 +708,7 @@ public void syncPackageLoading(OptionsProvider options)
options.getOptions(BuildLanguageOptions.class),
getCommandId(),
clientEnv,
repoEnvFromOptions,
timestampGranularityMonitor,
options);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,20 @@ public String getTypeDescription() {
+ "one.")
public boolean keepStateAfterBuild;

@Option(
name = "repo_env",
converter = Converters.OptionalAssignmentConverter.class,
allowMultiple = true,
defaultValue = "null",
documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
effectTags = {OptionEffectTag.ACTION_COMMAND_LINES},
help =
"Specifies additional environment variables to be available only for repository rules."
+ " Note that repository rules see the full environment anyway, but in this way"
+ " configuration information can be passed to repositories through options without"
+ " invalidating the action graph.")
public List<Map.Entry<String, String>> repositoryEnvironment;

/** The option converter to check that the user can only specify legal profiler tasks. */
public static class ProfilerTaskConverter extends EnumConverter<ProfilerTask> {
public ProfilerTaskConverter() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ public WorkspaceInfoFromDiff sync(
BuildLanguageOptions buildLanguageOptions,
UUID commandId,
Map<String, String> clientEnv,
Map<String, String> repoEnvOption,
TimestampGranularityMonitor tsgm,
OptionsProvider options)
throws InterruptedException, AbruptExitException {
Expand All @@ -261,6 +262,7 @@ public WorkspaceInfoFromDiff sync(
buildLanguageOptions,
commandId,
clientEnv,
repoEnvOption,
tsgm,
options);
long startTime = System.nanoTime();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2695,11 +2695,12 @@ public WorkspaceInfoFromDiff sync(
BuildLanguageOptions buildLanguageOptions,
UUID commandId,
Map<String, String> clientEnv,
Map<String, String> repoEnvOption,
TimestampGranularityMonitor tsgm,
OptionsProvider options)
throws InterruptedException, AbruptExitException {
getActionEnvFromOptions(options.getOptions(CoreOptions.class));
setRepoEnv(options.getOptions(CoreOptions.class));
PrecomputedValue.REPO_ENV.set(injectable(), new LinkedHashMap<>(repoEnvOption));
RemoteOptions remoteOptions = options.getOptions(RemoteOptions.class);
setRemoteExecutionEnabled(remoteOptions != null && remoteOptions.isRemoteExecutionEnabled());
syncPackageLoading(
Expand Down Expand Up @@ -2785,16 +2786,6 @@ public void setActionEnv(Map<String, String> actionEnv) {
PrecomputedValue.ACTION_ENV.set(injectable(), actionEnv);
}

private void setRepoEnv(CoreOptions opt) {
LinkedHashMap<String, String> repoEnv = new LinkedHashMap<>();
if (opt != null) {
for (Map.Entry<String, String> v : opt.repositoryEnvironment) {
repoEnv.put(v.getKey(), v.getValue());
}
}
PrecomputedValue.REPO_ENV.set(injectable(), repoEnv);
}

public PathPackageLocator createPackageLocator(
ExtendedEventHandler eventHandler, List<String> packagePaths, Path workingDirectory) {
return PathPackageLocator.create(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ protected void initTargetPatternEvaluator(ConfiguredRuleClassProvider ruleClassP
this.toolsRepository = ruleClassProvider.getToolsRepository();
skyframeExecutor = createSkyframeExecutor(ruleClassProvider);
PackageOptions packageOptions = Options.getDefaults(PackageOptions.class);

packageOptions.defaultVisibility = ConstantRuleVisibility.PRIVATE;
packageOptions.showLoadingProgress = true;
packageOptions.globbingThreads = 7;
Expand All @@ -296,6 +297,7 @@ protected void initTargetPatternEvaluator(ConfiguredRuleClassProvider ruleClassP
Options.getDefaults(BuildLanguageOptions.class),
UUID.randomUUID(),
ImmutableMap.<String, String>of(),
ImmutableMap.<String, String>of(),
new TimestampGranularityMonitor(BlazeClock.instance()),
OptionsProvider.EMPTY);
} catch (InterruptedException | AbruptExitException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ private void initBuildDriver() throws AbruptExitException, InterruptedException,
Options.getDefaults(BuildLanguageOptions.class),
UUID.randomUUID(),
/*clientEnv=*/ ImmutableMap.of(),
/*repoEnvOption=*/ ImmutableMap.of(),
new TimestampGranularityMonitor(BlazeClock.instance()),
OptionsProvider.EMPTY);
buildDriver = skyframeExecutor.getDriver();
Expand Down
4 changes: 2 additions & 2 deletions src/test/shell/bazel/starlark_repository_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -887,9 +887,9 @@ genrule(
)
EOF
cat > .bazelrc <<EOF
build:foo --repo_env=FOO=foo
common:foo --repo_env=FOO=foo
build:bar --repo_env=FOO=bar
build:qux --repo_env=FOO
common:qux --repo_env=FOO
EOF

bazel build --config=foo //:repoenv //:unrelated
Expand Down

0 comments on commit c2bdd03

Please sign in to comment.