Automated rollback of commit 2127e4595b1cad8baae9647dbf3cc5607bc4c8c1.
*** Reason for rollback ***
Breaks blaze self-build
*** Original change description ***
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: 683605569
Change-Id: I54cc05b9ccc91e18fc06c97bfaa1c6cb32c47e75
diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc
index df08aa0..9fb9422 100644
--- a/src/main/cpp/option_processor.cc
+++ b/src/main/cpp/option_processor.cc
@@ -657,9 +657,7 @@
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 2facd49..0353aff 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,7 +18,6 @@
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;
@@ -121,7 +120,6 @@
public static FlagSetValue modifyBuildOptionsWithFlagSets(
Label projectFile,
BuildOptions targetOptions,
- ImmutableSet<String> userOptions,
boolean enforceCanonicalConfigs,
ExtendedEventHandler eventHandler,
SkyframeExecutor skyframeExecutor)
@@ -132,7 +130,6 @@
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 b0ce8b4..02f3a22 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,7 +202,6 @@
static ProjectEvaluationResult evaluateProjectFile(
BuildRequest request,
BuildOptions buildOptions,
- ImmutableSet<String> userOptions,
TargetPatternPhaseValue targetPatternPhaseValue,
CommandEnvironment env)
throws LoadingFailedException, InvalidConfigurationException {
@@ -264,7 +263,6 @@
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 067fa91..3496572 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,7 +21,6 @@
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;
@@ -53,9 +52,10 @@
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,7 +189,10 @@
/** 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;
@@ -202,7 +205,6 @@
private final boolean runTests;
private final boolean checkForActionConflicts;
private final boolean reportIncompatibleTargets;
- private final ImmutableSet<String> userOptions;
private BuildRequest(
String commandName,
@@ -222,8 +224,6 @@
this.targets = targets;
this.id = id;
this.startTimeMillis = startTimeMillis;
- this.userOptions =
- options.getUserOptions() == null ? ImmutableSet.of() : options.getUserOptions();
this.optionsCache =
Caffeine.newBuilder()
.build(
@@ -278,20 +278,15 @@
}
/**
- * Returns the list of options that were parsed from either a user blazerc file or the command
- * line.
+ * Returns a unique identifier that universally identifies this build.
*/
- @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;
}
@@ -300,19 +295,24 @@
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,7 +324,10 @@
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);
}
@@ -334,12 +337,17 @@
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);
}
@@ -353,20 +361,24 @@
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;
@@ -391,10 +403,8 @@
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.
@@ -413,7 +423,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 5ac84b3..7cb5062 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
@@ -236,8 +236,7 @@
env.setWorkspaceName(targetPatternPhaseValue.getWorkspaceName());
ProjectEvaluationResult projectEvaluationResult =
- evaluateProjectFile(
- request, buildOptions, request.getUserOptions(), targetPatternPhaseValue, env);
+ evaluateProjectFile(request, buildOptions, targetPatternPhaseValue, env);
var analysisCachingDeps =
RemoteAnalysisCachingDependenciesProviderImpl.forAnalysis(
@@ -1075,7 +1074,6 @@
/** 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,
@@ -1086,7 +1084,6 @@
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 f541dbdc..da79fb4 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,11 +3254,6 @@
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 b40d4b5..5b95668 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,16 +13,10 @@
// 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;
@@ -90,9 +84,6 @@
key.getSclConfig(),
key.enforceCanonical(),
env.getListener());
- if (key.enforceCanonical()) {
- validateNoExtraFlagsSet(key.getTargetOptions(), key.getUserOptions());
- }
ParsedFlagsValue parsedFlags = parseFlags(sclConfigAsStarlarkList, env);
if (parsedFlags == null) {
return null;
@@ -128,9 +119,18 @@
// is silently consider a no-op.
return sclConfigValue == null ? ImmutableList.of() : ImmutableList.copyOf(sclConfigValue);
} else if (sclConfigName.isEmpty()) {
- ImmutableList<String> defaultConfigValue = ImmutableList.of();
try {
- defaultConfigValue = validateDefaultConfig(defaultConfigName, configs, supportedConfigs);
+ 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;
} catch (InvalidProjectFileException e) {
// there's no default config set.
throw new FlagSetFunctionException(
@@ -140,15 +140,6 @@
e.getMessage(), supportedConfigsDesc(projectFile, supportedConfigs))),
Transience.PERSISTENT);
}
- 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(
@@ -159,11 +150,6 @@
Transience.PERSISTENT);
}
- eventHandler.handle(
- Event.info(
- String.format(
- "Applying flags from the config defined in %s: %s ", projectFile, sclConfigValue)));
-
return ImmutableList.copyOf(sclConfigValue);
}
@@ -190,42 +176,6 @@
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<>();
- 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()
- .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 425163b..cf58fb9 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,7 +16,6 @@
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;
@@ -41,7 +40,6 @@
private final Label projectFile;
private final String sclConfig;
private final BuildOptions targetOptions;
- private final ImmutableSet<String> userOptions;
private final boolean enforceCanonical;
@@ -49,23 +47,16 @@
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,
- ImmutableSet<String> userOptions,
- boolean enforceCanonical) {
- return interner.intern(
- new Key(projectFile, sclConfig, targetOptions, userOptions, enforceCanonical));
+ Label projectFile, String sclConfig, BuildOptions targetOptions, boolean enforceCanonical) {
+ return interner.intern(new Key(projectFile, sclConfig, targetOptions, enforceCanonical));
}
public Label getProjectFile() {
@@ -80,10 +71,6 @@
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}.
@@ -114,13 +101,12 @@
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, userOptions, enforceCanonical);
+ return Objects.hash(projectFile, sclConfig, targetOptions, 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 3ab6951..97a9109 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"})
-public final class ImmutableSetCodec extends DeferredObjectCodec<Set> {
+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 8d5ae92..71f4e41 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,7 +15,6 @@
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;
@@ -115,9 +114,4 @@
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
deleted file mode 100644
index 7d8da99..0000000
--- a/src/main/java/com/google/devtools/common/options/GlobalRcUtils.java
+++ /dev/null
@@ -1,34 +0,0 @@
-// 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 8678e10..0fa39ee 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParser.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java
@@ -160,7 +160,6 @@
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;
@@ -233,13 +232,6 @@
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) {
@@ -264,7 +256,7 @@
/** Returns a new {@link OptionsParser}. */
public OptionsParser build() {
- return new OptionsParser(implBuilder.build(), allowResidue, ignoreUserOptions);
+ return new OptionsParser(implBuilder.build(), allowResidue);
}
}
@@ -281,16 +273,13 @@
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, boolean ignoreUserOptions) {
+ private OptionsParser(OptionsParserImpl impl, boolean allowResidue) {
this.impl = impl;
this.allowResidue = allowResidue;
- this.ignoreUserOptions = ignoreUserOptions;
}
public Object getConversionContext() {
@@ -875,33 +864,6 @@
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 2974e74..b45f887 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsProvider.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsProvider.java
@@ -14,14 +14,13 @@
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 =
@@ -42,22 +41,16 @@
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.
@@ -75,10 +68,4 @@
* 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 a6d23d3..c25f6b9 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,10 +310,7 @@
Iterables.addAll(options, module.getCommandOptions(commandAnnotation));
}
options.addAll(runtime.getRuleClassProvider().getFragmentRegistry().getOptionsClasses());
- // 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();
+ return OptionsParser.builder().optionsClasses(options).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 a61a2e3..293d271 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,7 +17,6 @@
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;
@@ -92,7 +91,6 @@
Label.parseCanonical("//test:PROJECT.scl"),
"test_config",
buildOptions,
- /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ false);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -123,7 +121,6 @@
Label.parseCanonical("//test:PROJECT.scl"),
"unknown_config",
buildOptions,
- /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ false);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -142,7 +139,6 @@
Label.parseCanonical("//test:PROJECT.scl"),
"",
buildOptions,
- /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ false);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -163,7 +159,6 @@
Label.parseCanonical("//test:PROJECT.scl"),
"random_config_name",
buildOptions,
- /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ false);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -196,7 +191,6 @@
Label.parseCanonical("//test:PROJECT.scl"),
"test_config",
buildOptions,
- /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -209,140 +203,6 @@
}
@Test
- public void enforceCanonicalConfigsExtraNativeFlag_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 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(
@@ -366,7 +226,6 @@
Label.parseCanonical("//test:PROJECT.scl"),
"other_config",
buildOptions,
- /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
var thrown = assertThrows(Exception.class, () -> executeFunction(key));
@@ -399,7 +258,6 @@
Label.parseCanonical("//test:PROJECT.scl"),
"non_existent_config",
buildOptions,
- /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
var thrown = assertThrows(Exception.class, () -> executeFunction(key));
@@ -432,7 +290,6 @@
Label.parseCanonical("//test:PROJECT.scl"),
/* sclConfig= */ "",
buildOptions,
- /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
var thrown = assertThrows(Exception.class, () -> executeFunction(key));
@@ -467,7 +324,6 @@
Label.parseCanonical("//test:PROJECT.scl"),
/* sclConfig= */ "",
buildOptions,
- /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
var thrown = assertThrows(Exception.class, () -> executeFunction(key));
@@ -503,7 +359,6 @@
Label.parseCanonical("//test:PROJECT.scl"),
/* sclConfig= */ "",
buildOptions,
- /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
var thrown = assertThrows(Exception.class, () -> executeFunction(key));
@@ -539,7 +394,6 @@
Label.parseCanonical("//test:PROJECT.scl"),
/* sclConfig= */ "",
buildOptions,
- /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -574,7 +428,6 @@
Label.parseCanonical("//test:PROJECT.scl"),
/* sclConfig= */ "",
buildOptions,
- /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -602,7 +455,6 @@
Label.parseCanonical("//test:PROJECT.scl"),
/* sclConfig= */ "test_config",
buildOptions,
- /* userOptions= */ ImmutableSet.of(),
/* enforceCanonical= */ true);
FlagSetValue flagSetsValue = executeFunction(key);
@@ -639,7 +491,6 @@
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 a599712..846d2e7 100644
--- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
+++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java
@@ -2625,22 +2625,6 @@
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 cd530af..4342824 100644
--- a/src/test/shell/integration/BUILD
+++ b/src/test/shell/integration/BUILD
@@ -676,17 +676,6 @@
],
)
-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
deleted file mode 100755
index 4aebbc6..0000000
--- a/src/test/shell/integration/flagset_test.sh
+++ /dev/null
@@ -1,100 +0,0 @@
-#!/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
