When --enforce_project_configs is enabled for a valid --scl_config, do not allow any other flags to be set in the user blazerc (~/.blazerc or --blazerc) or command line.
PiperOrigin-RevId: 686252332
Change-Id: I20a1de3a261b9b75141ebe1661ee08e6e6e732d3
diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc
index 9fb9422..df08aa0 100644
--- a/src/main/cpp/option_processor.cc
+++ b/src/main/cpp/option_processor.cc
@@ -657,7 +657,9 @@
const std::vector<std::string>& env) {
// Provide terminal options as coming from the least important rc file.
std::vector<std::string> result = {
+ // LINT.IfChange
"--rc_source=client",
+ // LINT.ThenChange(src/main/java/com/google/devtools/common/options/GlobalRcUtils.java)
"--default_override=0:common=--isatty=" +
blaze_util::ToString(IsStandardTerminal()),
"--default_override=0:common=--terminal_columns=" +
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Project.java b/src/main/java/com/google/devtools/build/lib/analysis/Project.java
index 0353aff..2facd49 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Project.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Project.java
@@ -18,6 +18,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMultimap;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
@@ -120,6 +121,7 @@
public static FlagSetValue modifyBuildOptionsWithFlagSets(
Label projectFile,
BuildOptions targetOptions,
+ ImmutableSet<String> userOptions,
boolean enforceCanonicalConfigs,
ExtendedEventHandler eventHandler,
SkyframeExecutor skyframeExecutor)
@@ -130,6 +132,7 @@
projectFile,
targetOptions.get(CoreOptions.class).sclConfig,
targetOptions,
+ userOptions,
enforceCanonicalConfigs);
EvaluationResult<SkyValue> result =
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
index 02f3a22..b0ce8b4 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
@@ -202,6 +202,7 @@
static ProjectEvaluationResult evaluateProjectFile(
BuildRequest request,
BuildOptions buildOptions,
+ ImmutableSet<String> userOptions,
TargetPatternPhaseValue targetPatternPhaseValue,
CommandEnvironment env)
throws LoadingFailedException, InvalidConfigurationException {
@@ -263,6 +264,7 @@
buildOptions =
BuildTool.applySclConfigs(
buildOptions,
+ userOptions,
projectFile,
request.getBuildOptions().enforceProjectConfigs,
env.getSkyframeExecutor(),
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
index 3496572..067fa91 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
@@ -21,6 +21,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.analysis.AnalysisOptions;
import com.google.devtools.build.lib.analysis.AspectCollection;
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
@@ -52,10 +53,9 @@
import javax.annotation.Nullable;
/**
- * A BuildRequest represents a single invocation of the build tool by a user.
- * A request specifies a list of targets to be built for a single
- * configuration, a pair of output/error streams, and additional options such
- * as --keep_going, --jobs, etc.
+ * A BuildRequest represents a single invocation of the build tool by a user. A request specifies a
+ * list of targets to be built for a single configuration, a pair of output/error streams, and
+ * additional options such as --keep_going, --jobs, etc.
*/
public class BuildRequest implements OptionsProvider {
@@ -189,10 +189,7 @@
/** A human-readable description of all the non-default option settings. */
private final String optionsDescription;
- /**
- * The name of the Blaze command that the user invoked.
- * Used for --announce.
- */
+ /** The name of the Blaze command that the user invoked. Used for --announce. */
private final String commandName;
private final OutErr outErr;
@@ -205,6 +202,7 @@
private final boolean runTests;
private final boolean checkForActionConflicts;
private final boolean reportIncompatibleTargets;
+ private final ImmutableSet<String> userOptions;
private BuildRequest(
String commandName,
@@ -224,6 +222,8 @@
this.targets = targets;
this.id = id;
this.startTimeMillis = startTimeMillis;
+ this.userOptions =
+ options.getUserOptions() == null ? ImmutableSet.of() : options.getUserOptions();
this.optionsCache =
Caffeine.newBuilder()
.build(
@@ -278,15 +278,20 @@
}
/**
- * Returns a unique identifier that universally identifies this build.
+ * Returns the list of options that were parsed from either a user blazerc file or the command
+ * line.
*/
+ @Override
+ public ImmutableSet<String> getUserOptions() {
+ return userOptions;
+ }
+
+ /** Returns a unique identifier that universally identifies this build. */
public UUID getId() {
return id;
}
- /**
- * Returns the name of the Blaze command that the user invoked.
- */
+ /** Returns the name of the Blaze command that the user invoked. */
public String getCommandName() {
return commandName;
}
@@ -295,24 +300,19 @@
return runningInEmacs;
}
- /**
- * Returns true if tests should be run by the build tool.
- */
+ /** Returns true if tests should be run by the build tool. */
public boolean shouldRunTests() {
return runTests;
}
- /**
- * Returns the (immutable) list of targets to build in commandline
- * form.
- */
+ /** Returns the (immutable) list of targets to build in commandline form. */
public List<String> getTargets() {
return targets;
}
/**
- * Returns the output/error streams to which errors and progress messages
- * should be sent during the fulfillment of this request.
+ * Returns the output/error streams to which errors and progress messages should be sent during
+ * the fulfillment of this request.
*/
public OutErr getOutErr() {
return outErr;
@@ -324,10 +324,7 @@
return (T) optionsCache.get(clazz).orNull();
}
-
- /**
- * Returns the set of command-line options specified for this request.
- */
+ /** Returns the set of command-line options specified for this request. */
public BuildRequestOptions getBuildOptions() {
return getOptions(BuildRequestOptions.class);
}
@@ -337,17 +334,12 @@
return getOptions(PackageOptions.class);
}
- /**
- * Returns the set of options related to the loading phase.
- */
+ /** Returns the set of options related to the loading phase. */
public LoadingOptions getLoadingOptions() {
return getOptions(LoadingOptions.class);
}
- /**
- * Returns the set of command-line options related to the view specified for
- * this request.
- */
+ /** Returns the set of command-line options related to the view specified for this request. */
public AnalysisOptions getViewOptions() {
return getOptions(AnalysisOptions.class);
}
@@ -361,24 +353,20 @@
int getLoadingPhaseThreadCount() {
return getOptions(LoadingPhaseThreadsOption.class).threads;
}
- /**
- * Returns the set of execution options specified for this request.
- */
+
+ /** Returns the set of execution options specified for this request. */
public ExecutionOptions getExecutionOptions() {
return getOptions(ExecutionOptions.class);
}
- /**
- * Returns the human-readable description of the non-default options
- * for this build request.
- */
+ /** Returns the human-readable description of the non-default options for this build request. */
public String getOptionsDescription() {
return optionsDescription;
}
/**
- * Return the time (according to System.currentTimeMillis()) at which the
- * service of this request was started.
+ * Return the time (according to System.currentTimeMillis()) at which the service of this request
+ * was started.
*/
public long getStartTime() {
return startTimeMillis;
@@ -403,8 +391,10 @@
int jobs = getBuildOptions().jobs;
if (localTestJobs > jobs) {
warnings.add(
- String.format("High value for --local_test_jobs: %d. This exceeds the value for --jobs: "
- + "%d. Only up to %d local tests will run concurrently.", localTestJobs, jobs, jobs));
+ String.format(
+ "High value for --local_test_jobs: %d. This exceeds the value for --jobs: "
+ + "%d. Only up to %d local tests will run concurrently.",
+ localTestJobs, jobs, jobs));
}
// Validate other BuildRequest options.
@@ -423,7 +413,7 @@
getOptions(BuildEventProtocolOptions.class).expandFilesets,
getOptions(BuildEventProtocolOptions.class).fullyResolveFilesetSymlinks,
OutputGroupInfo.determineOutputGroups(
- buildOptions.outputGroups, validationMode(), /*shouldRunTests=*/ shouldRunTests()));
+ buildOptions.outputGroups, validationMode(), /* shouldRunTests= */ shouldRunTests()));
}
public ImmutableList<String> getAspects() {
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
index 3092930..55ac41e 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -235,7 +235,8 @@
env.setWorkspaceName(targetPatternPhaseValue.getWorkspaceName());
ProjectEvaluationResult projectEvaluationResult =
- evaluateProjectFile(request, buildOptions, targetPatternPhaseValue, env);
+ evaluateProjectFile(
+ request, buildOptions, request.getUserOptions(), targetPatternPhaseValue, env);
var analysisCachingDeps =
RemoteAnalysisCachingDependenciesProviderImpl.forAnalysis(
@@ -1050,6 +1051,7 @@
/** Creates a BuildOptions class for the given options taken from an {@link OptionsProvider}. */
public static BuildOptions applySclConfigs(
BuildOptions buildOptionsBeforeFlagSets,
+ ImmutableSet<String> userOptions,
Label projectFile,
boolean enforceCanonicalConfigs,
SkyframeExecutor skyframeExecutor,
@@ -1060,6 +1062,7 @@
Project.modifyBuildOptionsWithFlagSets(
projectFile,
buildOptionsBeforeFlagSets,
+ userOptions,
enforceCanonicalConfigs,
eventHandler,
skyframeExecutor);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 550dcad..2d38b60 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -3254,6 +3254,11 @@
java.util.function.Predicate<? super ParsedOptionDescription> filter) {
return ImmutableMap.of();
}
+
+ @Override
+ public ImmutableSet<String> getUserOptions() {
+ return ImmutableSet.of();
+ }
});
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/config/FlagSetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/config/FlagSetFunction.java
index 5b95668..6703112 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/config/FlagSetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/config/FlagSetFunction.java
@@ -13,10 +13,16 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe.config;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
+
import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
+import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.Label.RepoContext;
import com.google.devtools.build.lib.cmdline.RepositoryName;
@@ -83,7 +89,9 @@
projectValue,
key.getSclConfig(),
key.enforceCanonical(),
- env.getListener());
+ env.getListener(),
+ key.getTargetOptions(),
+ key.getUserOptions());
ParsedFlagsValue parsedFlags = parseFlags(sclConfigAsStarlarkList, env);
if (parsedFlags == null) {
return null;
@@ -102,7 +110,9 @@
ProjectValue sclContent,
String sclConfigName,
boolean enforceCanonical,
- ExtendedEventHandler eventHandler)
+ ExtendedEventHandler eventHandler,
+ BuildOptions targetOptions,
+ ImmutableSet<String> userOptions)
throws FlagSetFunctionException {
var configs = (Dict<String, Collection<String>>) sclContent.getResidualGlobal(CONFIGS);
var sclConfigValue = configs == null ? null : configs.get(sclConfigName);
@@ -119,18 +129,9 @@
// is silently consider a no-op.
return sclConfigValue == null ? ImmutableList.of() : ImmutableList.copyOf(sclConfigValue);
} else if (sclConfigName.isEmpty()) {
+ ImmutableList<String> defaultConfigValue = ImmutableList.of();
try {
- ImmutableList<String> defaultConfigValue =
- validateDefaultConfig(defaultConfigName, configs, supportedConfigs);
- if (!defaultConfigWarningShown.get()) {
- eventHandler.handle(
- Event.info(
- String.format(
- "Applying flags from the default config defined in %s: %s ",
- projectFile, defaultConfigValue)));
- defaultConfigWarningShown.set(true);
- }
- return defaultConfigValue;
+ defaultConfigValue = validateDefaultConfig(defaultConfigName, configs, supportedConfigs);
} catch (InvalidProjectFileException e) {
// there's no default config set.
throw new FlagSetFunctionException(
@@ -140,6 +141,16 @@
e.getMessage(), supportedConfigsDesc(projectFile, supportedConfigs))),
Transience.PERSISTENT);
}
+ validateNoExtraFlagsSet(targetOptions, userOptions);
+ if (!defaultConfigWarningShown.get()) {
+ eventHandler.handle(
+ Event.info(
+ String.format(
+ "Applying flags from the default config defined in %s: %s ",
+ projectFile, defaultConfigValue)));
+ defaultConfigWarningShown.set(true);
+ }
+ return defaultConfigValue;
} else if (!supportedConfigs.containsKey(sclConfigName)) {
// This project declares supported configs and user set --scl_config to an unsupported config.
throw new FlagSetFunctionException(
@@ -150,6 +161,14 @@
Transience.PERSISTENT);
}
+ if (enforceCanonical) {
+ validateNoExtraFlagsSet(targetOptions, userOptions);
+ }
+ eventHandler.handle(
+ Event.info(
+ String.format(
+ "Applying flags from the config defined in %s: %s ", projectFile, sclConfigValue)));
+
return ImmutableList.copyOf(sclConfigValue);
}
@@ -176,6 +195,44 @@
return ImmutableList.copyOf(configs.get(defaultConfigName));
}
+ /**
+ * Enforces that the user did not set any output-affecting options in a blazerc or on the command
+ * line. Flags set in global RC files and flags that do not affect outputs are allowed.
+ */
+ // TODO(steinman): Allow user options if they are also part of the --scl_config.
+ private void validateNoExtraFlagsSet(BuildOptions targetOptions, ImmutableSet<String> userOptions)
+ throws FlagSetFunctionException {
+ ImmutableList.Builder<String> allOptionsAsStringsBuilder = new ImmutableList.Builder<>();
+ // All potentially conflicting user options also appear in targetOptions
+ targetOptions.getStarlarkOptions().keySet().stream()
+ .map(Object::toString)
+ .forEach(allOptionsAsStringsBuilder::add);
+ for (FragmentOptions fragmentOptions : targetOptions.getNativeOptions()) {
+ fragmentOptions.asMap().keySet().forEach(allOptionsAsStringsBuilder::add);
+ }
+ ImmutableList<String> allOptionsAsStrings = allOptionsAsStringsBuilder.build();
+ ImmutableSet<String> overlap =
+ userOptions.stream()
+ // Remove options that aren't part of BuildOptions
+ .filter(
+ option ->
+ allOptionsAsStrings.contains(
+ Iterables.get(Splitter.on("=").split(option), 0).replaceFirst("--", "")))
+ .filter(option -> !option.startsWith("--scl_config"))
+ .collect(toImmutableSet());
+ // TODO(b/341930725): Allow user options if they are also part of the --scl_config.
+ if (!overlap.isEmpty()) {
+ throw new FlagSetFunctionException(
+ new UnsupportedConfigException(
+ String.format(
+ "When --enforce_project_configs is set, --scl_config must be the only"
+ + " configuration-affecting flag in the build. Found %s in the command line"
+ + " or user blazerc",
+ overlap)),
+ Transience.PERSISTENT);
+ }
+ }
+
/** Returns a user-friendly description of project-supported configurations. */
private static String supportedConfigsDesc(
Label projectFile, Dict<String, String> supportedConfigs) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/config/FlagSetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/config/FlagSetValue.java
index cf58fb9..425163b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/config/FlagSetValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/config/FlagSetValue.java
@@ -16,6 +16,7 @@
import static com.google.common.base.Strings.nullToEmpty;
import com.google.common.base.Verify;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety;
@@ -40,6 +41,7 @@
private final Label projectFile;
private final String sclConfig;
private final BuildOptions targetOptions;
+ private final ImmutableSet<String> userOptions;
private final boolean enforceCanonical;
@@ -47,16 +49,23 @@
Label projectFile,
@Nullable String sclConfig,
BuildOptions targetOptions,
+ ImmutableSet<String> userOptions,
boolean enforceCanonical) {
this.projectFile = Verify.verifyNotNull(projectFile);
this.sclConfig = nullToEmpty(sclConfig);
this.targetOptions = Verify.verifyNotNull(targetOptions);
+ this.userOptions = Verify.verifyNotNull(userOptions);
this.enforceCanonical = enforceCanonical;
}
public static Key create(
- Label projectFile, String sclConfig, BuildOptions targetOptions, boolean enforceCanonical) {
- return interner.intern(new Key(projectFile, sclConfig, targetOptions, enforceCanonical));
+ Label projectFile,
+ String sclConfig,
+ BuildOptions targetOptions,
+ ImmutableSet<String> userOptions,
+ boolean enforceCanonical) {
+ return interner.intern(
+ new Key(projectFile, sclConfig, targetOptions, userOptions, enforceCanonical));
}
public Label getProjectFile() {
@@ -71,6 +80,10 @@
return targetOptions;
}
+ public ImmutableSet<String> getUserOptions() {
+ return userOptions;
+ }
+
/**
* Whether {@code --scl_config} must match an officially supported project configuration. See
* {@link com.google.devtools.build.lib.buildtool.BuildRequestOptions#enforceProjectConfigs}.
@@ -101,12 +114,13 @@
return Objects.equals(projectFile, key.projectFile)
&& Objects.equals(sclConfig, key.sclConfig)
&& Objects.equals(targetOptions, key.targetOptions)
+ && Objects.equals(userOptions, key.userOptions)
&& (enforceCanonical == key.enforceCanonical);
}
@Override
public int hashCode() {
- return Objects.hash(projectFile, sclConfig, targetOptions, enforceCanonical);
+ return Objects.hash(projectFile, sclConfig, targetOptions, userOptions, enforceCanonical);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ImmutableSetCodec.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ImmutableSetCodec.java
index 97a9109..3ab6951 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ImmutableSetCodec.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/ImmutableSetCodec.java
@@ -30,7 +30,7 @@
/** {@link ObjectCodec} for {@link ImmutableSet} and other sets that should be immutable. */
@SuppressWarnings({"rawtypes", "unchecked"})
-final class ImmutableSetCodec extends DeferredObjectCodec<Set> {
+public final class ImmutableSetCodec extends DeferredObjectCodec<Set> {
// Conversion of the types below to ImmutableSet is sound because the underlying types are hidden
// and only referenceable as the Set type.
diff --git a/src/main/java/com/google/devtools/build/lib/testing/common/FakeOptions.java b/src/main/java/com/google/devtools/build/lib/testing/common/FakeOptions.java
index 71f4e41..8d5ae92 100644
--- a/src/main/java/com/google/devtools/build/lib/testing/common/FakeOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/testing/common/FakeOptions.java
@@ -15,6 +15,7 @@
import com.google.common.collect.ImmutableClassToInstanceMap;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.common.options.Options;
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsProvider;
@@ -114,4 +115,9 @@
Predicate<? super ParsedOptionDescription> filter) {
return ImmutableMap.of();
}
+
+ @Override
+ public ImmutableSet<String> getUserOptions() {
+ return ImmutableSet.of();
+ }
}
diff --git a/src/main/java/com/google/devtools/common/options/GlobalRcUtils.java b/src/main/java/com/google/devtools/common/options/GlobalRcUtils.java
new file mode 100644
index 0000000..7d8da99
--- /dev/null
+++ b/src/main/java/com/google/devtools/common/options/GlobalRcUtils.java
@@ -0,0 +1,34 @@
+// Copyright 2024 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.common.options;
+
+import java.util.function.Predicate;
+
+/** Utility functions for global RC files. */
+final class GlobalRcUtils {
+
+ private GlobalRcUtils() {}
+
+ /** No global RC files in Bazel. Consider "client" options to be global. */
+ static final Predicate<ParsedOptionDescription> IS_GLOBAL_RC_OPTION =
+ // LINT.IfChange
+ (option) -> {
+ if (option.getOrigin().getSource() != null
+ && option.getOrigin().getSource().equals("client")) {
+ return true;
+ }
+ return false;
+ };
+ // LINT.ThenChange(//src/main/cpp/option_processor.cc)
+}
diff --git a/src/main/java/com/google/devtools/common/options/OptionsParser.java b/src/main/java/com/google/devtools/common/options/OptionsParser.java
index 0fa39ee..8678e10 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParser.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java
@@ -160,6 +160,7 @@
public static final class Builder {
private final OptionsParserImpl.Builder implBuilder;
private boolean allowResidue = true;
+ private boolean ignoreUserOptions = false;
private Builder(OptionsParserImpl.Builder implBuilder) {
this.implBuilder = implBuilder;
@@ -232,6 +233,13 @@
return this;
}
+ /** Sets whether the parser should ignore user options. If true, returns no user options. */
+ @CanIgnoreReturnValue
+ public Builder ignoreUserOptions() {
+ this.ignoreUserOptions = true;
+ return this;
+ }
+
/** Sets the string the parser should look for as an identifier for flag aliases. */
@CanIgnoreReturnValue
public Builder withAliasFlag(@Nullable String aliasFlag) {
@@ -256,7 +264,7 @@
/** Returns a new {@link OptionsParser}. */
public OptionsParser build() {
- return new OptionsParser(implBuilder.build(), allowResidue);
+ return new OptionsParser(implBuilder.build(), allowResidue, ignoreUserOptions);
}
}
@@ -273,13 +281,16 @@
private final List<String> residue = new ArrayList<>();
private final List<String> postDoubleDashResidue = new ArrayList<>();
private final boolean allowResidue;
+ private final boolean ignoreUserOptions;
+
private ImmutableSortedMap<String, Object> starlarkOptions = ImmutableSortedMap.of();
private final Map<String, String> aliases = new HashMap<>();
private boolean success = true;
- private OptionsParser(OptionsParserImpl impl, boolean allowResidue) {
+ private OptionsParser(OptionsParserImpl impl, boolean allowResidue, boolean ignoreUserOptions) {
this.impl = impl;
this.allowResidue = allowResidue;
+ this.ignoreUserOptions = ignoreUserOptions;
}
public Object getConversionContext() {
@@ -864,6 +875,33 @@
return impl.asCanonicalizedList();
}
+ @Override
+ public ImmutableSet<String> getUserOptions() {
+ if (ignoreUserOptions) {
+ return ImmutableSet.of();
+ }
+ ImmutableSet.Builder<String> userOptions = ImmutableSet.builder();
+
+ return userOptions
+ .addAll(
+ asListOfExplicitOptions().stream()
+ .filter(GlobalRcUtils.IS_GLOBAL_RC_OPTION.negate())
+ .filter(option -> !option.getCanonicalForm().contains("default_override"))
+ .map(option -> option.getCanonicalForm())
+ .collect(toImmutableSet()))
+ .addAll(
+ impl.getSkippedOptions().stream()
+ .filter(GlobalRcUtils.IS_GLOBAL_RC_OPTION.negate())
+ .map(option -> option.getUnconvertedValue())
+ .filter(
+ o ->
+ getStarlarkOptions()
+ .containsKey(
+ Iterables.get(Splitter.on('=').split(o.replace("--", "")), 0)))
+ .collect(toImmutableSet()))
+ .build();
+ }
+
/** Returns all options fields of the given options class, in alphabetic order. */
public static ImmutableList<? extends OptionDefinition> getOptionDefinitions(
Class<? extends OptionsBase> optionsClass) {
diff --git a/src/main/java/com/google/devtools/common/options/OptionsProvider.java b/src/main/java/com/google/devtools/common/options/OptionsProvider.java
index b45f887..2974e74 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsProvider.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsProvider.java
@@ -14,13 +14,14 @@
package com.google.devtools.common.options;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import java.util.Map;
import java.util.function.Predicate;
import javax.annotation.Nullable;
/**
- * A read-only interface for options parser results, which only allows to query the options of
- * a specific class, but not e.g. the residue any other information pertaining to the command line.
+ * A read-only interface for options parser results, which only allows to query the options of a
+ * specific class, but not e.g. the residue any other information pertaining to the command line.
*/
public interface OptionsProvider {
public static final OptionsProvider EMPTY =
@@ -41,16 +42,22 @@
Predicate<? super ParsedOptionDescription> filter) {
return ImmutableMap.of();
}
+
+ @Override
+ public ImmutableSet<String> getUserOptions() {
+ return ImmutableSet.of();
+ }
};
/**
- * Returns the options instance for the given {@code optionsClass}, that is,
- * the parsed options, or null if it is not among those available.
+ * Returns the options instance for the given {@code optionsClass}, that is, the parsed options,
+ * or null if it is not among those available.
*
- * <p>The returned options should be treated by library code as immutable and
- * a provider is permitted to return the same options instance multiple times.
+ * <p>The returned options should be treated by library code as immutable and a provider is
+ * permitted to return the same options instance multiple times.
*/
- @Nullable <O extends OptionsBase> O getOptions(Class<O> optionsClass);
+ @Nullable
+ <O extends OptionsBase> O getOptions(Class<O> optionsClass);
/**
* Returns the starlark options in a name:value map.
@@ -68,4 +75,10 @@
* the given filter criteria.
*/
Map<String, Object> getExplicitStarlarkOptions(Predicate<? super ParsedOptionDescription> filter);
+
+ /**
+ * Returns the options that were parsed from either a user blazerc file or the command line as a
+ * map of option name to option value.
+ */
+ ImmutableSet<String> getUserOptions();
}
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java b/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java
index c25f6b9..a6d23d3 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java
@@ -310,7 +310,10 @@
Iterables.addAll(options, module.getCommandOptions(commandAnnotation));
}
options.addAll(runtime.getRuleClassProvider().getFragmentRegistry().getOptionsClasses());
- return OptionsParser.builder().optionsClasses(options).build();
+ // Because the tests that use this class don't set sources for their options, the normal logic
+ // for determining user options assumes that all options are user options. This causes tests
+ // that enable PROJECT.scl files to fail, so ignore user options instead.
+ return OptionsParser.builder().optionsClasses(options).ignoreUserOptions().build();
}
void executeNonBuildCommand() throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/config/FlagSetsFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/config/FlagSetsFunctionTest.java
index 293d271..1edda61 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/config/FlagSetsFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/config/FlagSetsFunctionTest.java
@@ -17,6 +17,7 @@
import static org.junit.Assert.assertThrows;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.PlatformOptions;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
@@ -91,6 +92,7 @@
Label.parseCanonical("//test:PROJECT.scl"),
"test_config",
buildOptions,
+ /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ false);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -121,6 +123,7 @@
Label.parseCanonical("//test:PROJECT.scl"),
"unknown_config",
buildOptions,
+ /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ false);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -139,6 +142,7 @@
Label.parseCanonical("//test:PROJECT.scl"),
"",
buildOptions,
+ /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ false);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -159,6 +163,7 @@
Label.parseCanonical("//test:PROJECT.scl"),
"random_config_name",
buildOptions,
+ /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ false);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -191,6 +196,7 @@
Label.parseCanonical("//test:PROJECT.scl"),
"test_config",
buildOptions,
+ /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -203,6 +209,179 @@
}
@Test
+ public void enforceCanonicalConfigsExtraNativeFlag_withSclConfig_fails() throws Exception {
+ scratch.file(
+ "test/build_settings.bzl",
+ """
+string_flag = rule(implementation = lambda ctx: [], build_setting = config.string(flag = True))
+""");
+ scratch.file(
+ "test/BUILD",
+ """
+ load("//test:build_settings.bzl", "string_flag")
+ string_flag(
+ name = "myflag",
+ build_setting_default = "default",
+ )
+ """);
+ scratch.file(
+ "test/PROJECT.scl",
+ """
+ configs = {
+ "test_config": ['--//test:myflag=test_config_value'],
+ "other_config": ['--//test:myflag=other_config_value'],
+ }
+ supported_configs = {
+ "test_config": "User documentation for what this config means",
+ }
+ """);
+ setBuildLanguageOptions("--experimental_enable_scl_dialect=true");
+ BuildOptions buildOptions = createBuildOptions("--define=foo=bar");
+
+ FlagSetValue.Key key =
+ FlagSetValue.Key.create(
+ Label.parseCanonical("//test:PROJECT.scl"),
+ "test_config",
+ buildOptions,
+ /* userOptions= */ ImmutableSet.of("--define=foo=bar"),
+ /* enforceCanonical= */ true);
+
+ var thrown = assertThrows(Exception.class, () -> executeFunction(key));
+ assertThat(thrown).hasMessageThat().contains("Found [--define=foo=bar]");
+ }
+
+ @Test
+ public void enforceCanonicalConfigsExtraNativeFlag_noSupportedConfigs_passes() throws Exception {
+ scratch.file(
+ "test/build_settings.bzl",
+ """
+string_flag = rule(implementation = lambda ctx: [], build_setting = config.string(flag = True))
+""");
+ scratch.file(
+ "test/BUILD",
+ """
+ load("//test:build_settings.bzl", "string_flag")
+ string_flag(
+ name = "myflag",
+ build_setting_default = "default",
+ )
+ """);
+ scratch.file(
+ "test/PROJECT.scl",
+ """
+ configs = {
+ "test_config": ['--//test:myflag=test_config_value'],
+ "other_config": ['--//test:myflag=other_config_value'],
+ }
+ """);
+ setBuildLanguageOptions("--experimental_enable_scl_dialect=true");
+ BuildOptions buildOptions = createBuildOptions("--define=foo=bar");
+
+ FlagSetValue.Key key =
+ FlagSetValue.Key.create(
+ Label.parseCanonical("//test:PROJECT.scl"),
+ null,
+ buildOptions,
+ /* userOptions= */ ImmutableSet.of("--define=foo=bar"),
+ /* enforceCanonical= */ true);
+
+ var unused = executeFunction(key);
+ assertDoesNotContainEvent("--scl_config must be the only configuration-affecting flag");
+ }
+
+ @Test
+ public void enforceCanonicalConfigsExtraStarlarkFlag_fails() throws Exception {
+ scratch.file(
+ "test/build_settings.bzl",
+ """
+string_flag = rule(implementation = lambda ctx: [], build_setting = config.string(flag = True))
+""");
+ scratch.file(
+ "test/BUILD",
+ """
+ load("//test:build_settings.bzl", "string_flag")
+ string_flag(
+ name = "myflag",
+ build_setting_default = "default",
+ )
+ string_flag(
+ name = "starlark_flags_always_affect_configuration",
+ build_setting_default = "default",
+ )
+ """);
+ scratch.file(
+ "test/PROJECT.scl",
+ """
+ configs = {
+ "test_config": ['--//test:myflag=test_config_value'],
+ "other_config": ['--//test:myflag=other_config_value'],
+ }
+ supported_configs = {
+ "test_config": "User documentation for what this config means",
+ }
+ """);
+ setBuildLanguageOptions("--experimental_enable_scl_dialect=true");
+ BuildOptions buildOptions =
+ createBuildOptions("--//test:starlark_flags_always_affect_configuration=yes_they_do");
+
+ FlagSetValue.Key key =
+ FlagSetValue.Key.create(
+ Label.parseCanonical("//test:PROJECT.scl"),
+ "test_config",
+ buildOptions,
+ /* userOptions= */ ImmutableSet.of(
+ "--//test:starlark_flags_always_affect_configuration=yes_they_do"),
+ /* enforceCanonical= */ true);
+
+ var thrown = assertThrows(Exception.class, () -> executeFunction(key));
+ assertThat(thrown)
+ .hasMessageThat()
+ .contains("Found [--//test:starlark_flags_always_affect_configuration=yes_they_do]");
+ }
+
+ @Test
+ public void noEnforceCanonicalConfigsExtraFlag_passes() throws Exception {
+ scratch.file(
+ "test/build_settings.bzl",
+ """
+string_flag = rule(implementation = lambda ctx: [], build_setting = config.string(flag = True))
+""");
+ scratch.file(
+ "test/BUILD",
+ """
+ load("//test:build_settings.bzl", "string_flag")
+ string_flag(
+ name = "myflag",
+ build_setting_default = "default",
+ )
+ """);
+ scratch.file(
+ "test/PROJECT.scl",
+ """
+ configs = {
+ "test_config": ['--//test:myflag=test_config_value'],
+ "other_config": ['--//test:myflag=other_config_value'],
+ }
+ supported_configs = {
+ "test_config": "User documentation for what this config means",
+ }
+ """);
+ setBuildLanguageOptions("--experimental_enable_scl_dialect=true");
+ BuildOptions buildOptions = createBuildOptions("--define=foo=bar");
+
+ FlagSetValue.Key key =
+ FlagSetValue.Key.create(
+ Label.parseCanonical("//test:PROJECT.scl"),
+ "test_config",
+ buildOptions,
+ /* userOptions= */ ImmutableSet.of("--define=foo=bar"),
+ /* enforceCanonical= */ false);
+
+ var unused = executeFunction(key);
+ assertDoesNotContainEvent("--scl_config must be the only configuration-affecting flag");
+ }
+
+ @Test
public void enforceCanonicalConfigsUnsupportedConfig() throws Exception {
createStringFlag("//test:myflag", /* defaultValue= */ "default");
scratch.file(
@@ -226,6 +405,7 @@
Label.parseCanonical("//test:PROJECT.scl"),
"other_config",
buildOptions,
+ /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
var thrown = assertThrows(Exception.class, () -> executeFunction(key));
@@ -258,6 +438,7 @@
Label.parseCanonical("//test:PROJECT.scl"),
"non_existent_config",
buildOptions,
+ /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
var thrown = assertThrows(Exception.class, () -> executeFunction(key));
@@ -290,6 +471,7 @@
Label.parseCanonical("//test:PROJECT.scl"),
/* sclConfig= */ "",
buildOptions,
+ /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
var thrown = assertThrows(Exception.class, () -> executeFunction(key));
@@ -324,6 +506,7 @@
Label.parseCanonical("//test:PROJECT.scl"),
/* sclConfig= */ "",
buildOptions,
+ /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
var thrown = assertThrows(Exception.class, () -> executeFunction(key));
@@ -359,6 +542,7 @@
Label.parseCanonical("//test:PROJECT.scl"),
/* sclConfig= */ "",
buildOptions,
+ /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
var thrown = assertThrows(Exception.class, () -> executeFunction(key));
@@ -394,6 +578,7 @@
Label.parseCanonical("//test:PROJECT.scl"),
/* sclConfig= */ "",
buildOptions,
+ /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -428,6 +613,7 @@
Label.parseCanonical("//test:PROJECT.scl"),
/* sclConfig= */ "",
buildOptions,
+ /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -455,6 +641,7 @@
Label.parseCanonical("//test:PROJECT.scl"),
/* sclConfig= */ "test_config",
buildOptions,
+ /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -491,6 +678,7 @@
Label.parseCanonical("//test:PROJECT.scl"),
/* sclConfig= */ "",
buildOptions,
+ /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
var thrown = assertThrows(Exception.class, () -> executeFunction(key));
diff --git a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
index 846d2e7..a599712 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
@@ -2625,6 +2625,22 @@
assertThat(parser.getOptions(ExpandingOptionsFallback.class)).isNull();
}
+ @Test
+ public void testOptionsParser_getUserOptions_excludesClientOptions() throws Exception {
+ OptionsParser parser =
+ OptionsParser.builder()
+ .optionsClasses(ExpandingOptions.class, ExpandingOptionsFallback.class)
+ .build();
+ parser.parseWithSourceFunction(
+ PriorityCategory.RC_FILE, o -> "client", ImmutableList.of("--foo"), null);
+ assertThat(parser.getUserOptions()).isEmpty();
+
+ parser.parseWithSourceFunction(
+ PriorityCategory.RC_FILE, o -> ".bazelrc", ImmutableList.of("--foo"), null);
+
+ assertThat(parser.getUserOptions()).containsExactly("--foo");
+ }
+
private static OptionInstanceOrigin createInvocationPolicyOrigin() {
return createInvocationPolicyOrigin(/*implicitDependent=*/ null, /*expandedFrom=*/ null);
}
diff --git a/src/test/shell/integration/BUILD b/src/test/shell/integration/BUILD
index 4342824..cd530af 100644
--- a/src/test/shell/integration/BUILD
+++ b/src/test/shell/integration/BUILD
@@ -676,6 +676,17 @@
],
)
+sh_test(
+ name = "flagset_test",
+ size = "large",
+ srcs = ["flagset_test.sh"],
+ data = [
+ ":test-deps",
+ "@bazel_tools//tools/bash/runfiles",
+ ],
+ shard_count = 3,
+)
+
# Copy protoc into a known location, since //third_party/protobuf:protoc
# might be an alias. This is referenced from testenv.sh.
genrule(
diff --git a/src/test/shell/integration/flagset_test.sh b/src/test/shell/integration/flagset_test.sh
new file mode 100755
index 0000000..4aebbc6
--- /dev/null
+++ b/src/test/shell/integration/flagset_test.sh
@@ -0,0 +1,100 @@
+#!/bin/bash
+#
+# Copyright 2024 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.
+#
+# An end-to-end test for flagsets.
+
+# --- begin runfiles.bash initialization ---
+set -euo pipefail
+if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
+ if [[ -f "$0.runfiles_manifest" ]]; then
+ export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest"
+ elif [[ -f "$0.runfiles/MANIFEST" ]]; then
+ export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST"
+ elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then
+ export RUNFILES_DIR="$0.runfiles"
+ fi
+fi
+if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then
+ source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash"
+elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
+ source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \
+ "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)"
+else
+ echo >&2 "ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash"
+ exit 1
+fi
+# --- end runfiles.bash initialization ---
+
+source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \
+ || { echo "integration_test_setup.sh not found!" >&2; exit 1; }
+
+function set_up_project_file() {
+ mkdir -p test
+ cat > test/BUILD <<EOF
+load("//test:test.bzl", "string_flag")
+
+genrule(name='test', outs=['test.txt'], cmd='echo hi > \$@')
+
+string_flag(
+ name = "starlark_flags_always_affect_configuration",
+ build_setting_default = "default",
+)
+EOF
+ cat > test/PROJECT.scl <<EOF
+configs = {
+ "test_config": ["--define=foo=bar"],
+}
+supported_configs = {
+ "test_config": "User documentation for what this config means",
+}
+EOF
+
+ touch test/test.bzl
+ cat > test/test.bzl <<EOF
+string_flag = rule(implementation = lambda ctx: [], build_setting = config.string(flag = True))
+EOF
+}
+
+function test_scl_config_plus_user_bazelrc_fails(){
+ set_up_project_file
+ add_to_bazelrc "build '--//test:starlark_flags_always_affect_configuration=yes'"
+ add_to_bazelrc "build --define=bar=baz"
+ cat .bazelrc >> test/test.bazelrc
+ bazel --bazelrc=test/test.bazelrc build --nobuild //test:test --enforce_project_configs --scl_config=test_config --experimental_enable_scl_dialect &> "$TEST_log" && \
+ fail "Scl enabled build expected to fail with starlark flag in user bazelrc"
+ expect_log "--scl_config must be the only configuration-affecting flag"
+ expect_log "--//test:starlark_flags_always_affect_configuration=yes"
+ expect_log "--define=bar=baz"
+}
+
+function test_scl_config_plus_command_line_flag_fails(){
+ set_up_project_file
+ bazel build --nobuild //test:test --enforce_project_configs --scl_config=test_config --experimental_enable_scl_dialect --//test:starlark_flags_always_affect_configuration=yes --define=bar=baz &> "$TEST_log" && \
+ fail "Scl enabled build expected to fail with command-line flags"
+ expect_log "--scl_config must be the only configuration-affecting flag"
+ expect_log "--//test:starlark_flags_always_affect_configuration=yes"
+ expect_log "--define=bar=baz"
+}
+
+function test_scl_config_plus_expanded_command_line_flag_fails(){
+ set_up_project_file
+ bazel build --nobuild //test:test --enforce_project_configs --scl_config=test_config --experimental_enable_scl_dialect -c opt &> "$TEST_log" && \
+ fail "Scl enabled build expected to fail with command line flag"
+ expect_log "--scl_config must be the only configuration-affecting flag"
+ expect_log "--compilation_mode=opt"
+}
+
+run_suite "Integration tests for flagsets/scl_config"
\ No newline at end of file
