Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move --repo_env to common options #13003

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,11 +246,9 @@ public void exit(AbruptExitException exception) {
}
}

CoreOptions configOpts = options.getOptions(CoreOptions.class);
if (configOpts != null) {
for (Map.Entry<String, String> entry : configOpts.repositoryEnvironment) {
repoEnv.put(entry.getKey(), entry.getValue());
}
for (Map.Entry<String, String> entry : commandOptions.repositoryEnvironment) {
repoEnv.put(entry.getKey(), entry.getValue());
repoEnvFromOptions.put(entry.getKey(), entry.getValue());
}
}

Expand Down Expand Up @@ -702,6 +701,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 @@ -235,6 +235,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 @@ -260,6 +261,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 @@ -2681,11 +2681,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 @@ -2756,16 +2757,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 @@ -309,6 +309,7 @@ private void initBuildDriver() throws AbruptExitException, InterruptedException,
Options.getDefaults(BuildLanguageOptions.class),
UUID.randomUUID(),
/*clientEnv=*/ ImmutableMap.of(),
/*repoEnv=*/ 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,8 +887,8 @@ genrule(
)
EOF
cat > .bazelrc <<EOF
build:foo --repo_env=FOO=foo
build:bar --repo_env=FOO=bar
common:foo --repo_env=FOO=foo
common:bar --repo_env=FOO=bar
EOF

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