Roll forward config_setting visibility enforcement behind a flag.
This was rolled back in https://github.com/bazelbuild/bazel/commit/36d228bd792f4332c7486c4e5f9c78e4b55f4b06
because of depot breakages.
This version adds incompatible flags to safely introduce enforcement.
See https://github.com/bazelbuild/bazel/issues/12932 and
https://github.com/bazelbuild/bazel/issues/12933 for flag settings.
*** Original change description ***
Roll back https://github.com/bazelbuild/bazel/pull/12877.
***
Fixes #12669.
RELNOTES: enforce config_setting visibility. See https://github.com/bazelbuild/bazel/issues/12932 for details.
PiperOrigin-RevId: 354930807
diff --git a/site/docs/visibility.md b/site/docs/visibility.md
index e8ff2ff..e6cb247 100644
--- a/site/docs/visibility.md
+++ b/site/docs/visibility.md
@@ -62,6 +62,25 @@
target's BUILD file. If there is no such `default_visibility` declaration, the
visibility is `//visibility:private`.
+`config_setting` visibility has historically not been enforced.
+`--incompatible_enforce_config_setting_visibility` and
+`--incompatible_config_setting_private_default_visibility` provide migration
+logic for converging with other rules.
+
+If `--incompatible_enforce_config_setting_visibility=false`, every
+`config_setting` is unconditionally visible to all targets.
+
+Else if `--incompatible_config_setting_private_default_visibility=false`, any
+`config_setting` that doesn't explicitly set visibility is `//visibility:public`
+(ignoring package [`default_visibility`](be/functions.html#package.default_visibility)).
+
+Else if `--incompatible_config_setting_private_default_visibility=true`,
+`config_setting` uses the same visibility logic as all other rules.
+
+Best practice is to treat all `config_setting` targets like other rules:
+explicitly set `visibility` on any `config_setting` used anywhere outside its
+package.
+
### Example
File `//frobber/bin/BUILD`:
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
index 8b731d7..1cc968c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -292,6 +292,7 @@
":buildinfo/build_info_key",
":config/build_configuration",
":config/build_options",
+ ":config/config_conditions",
":config/config_matching_provider",
":config/core_options",
":config/execution_transition_factory",
@@ -1608,6 +1609,20 @@
)
java_library(
+ name = "config/config_conditions",
+ srcs = ["config/ConfigConditions.java"],
+ deps = [
+ ":config/config_matching_provider",
+ ":configured_target",
+ "//src/main/java/com/google/devtools/build/lib/analysis/platform",
+ "//src/main/java/com/google/devtools/build/lib/cmdline",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data",
+ "//third_party:auto_value",
+ "//third_party:guava",
+ ],
+)
+
+java_library(
name = "config/core_option_converters",
srcs = ["config/CoreOptionConverters.java"],
deps = [
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index 4bbc311..1c4c336 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -27,7 +27,7 @@
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.RuleContext.InvalidExecGroupException;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
-import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
+import com.google.devtools.build.lib.analysis.config.ConfigConditions;
import com.google.devtools.build.lib.analysis.config.Fragment;
import com.google.devtools.build.lib.analysis.config.RequiredFragmentsUtil;
import com.google.devtools.build.lib.analysis.configuredtargets.EnvironmentGroupConfiguredTarget;
@@ -181,7 +181,7 @@
BuildConfiguration hostConfig,
ConfiguredTargetKey configuredTargetKey,
OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> prerequisiteMap,
- ImmutableMap<Label, ConfigMatchingProvider> configConditions,
+ ConfigConditions configConditions,
@Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts)
throws InterruptedException, ActionConflictException, InvalidExecGroupException {
if (target instanceof Rule) {
@@ -287,7 +287,7 @@
BuildConfiguration hostConfiguration,
ConfiguredTargetKey configuredTargetKey,
OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> prerequisiteMap,
- ImmutableMap<Label, ConfigMatchingProvider> configConditions,
+ ConfigConditions configConditions,
@Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts)
throws InterruptedException, ActionConflictException, InvalidExecGroupException {
ConfigurationFragmentPolicy configurationFragmentPolicy =
@@ -318,7 +318,7 @@
configuration,
ruleClassProvider.getUniversalFragments(),
configurationFragmentPolicy,
- configConditions,
+ configConditions.asProviders(),
prerequisiteMap.values()))
.build();
@@ -495,7 +495,7 @@
ConfiguredAspectFactory aspectFactory,
Aspect aspect,
OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> prerequisiteMap,
- ImmutableMap<Label, ConfigMatchingProvider> configConditions,
+ ConfigConditions configConditions,
@Nullable ResolvedToolchainContext toolchainContext,
BuildConfiguration aspectConfiguration,
BuildConfiguration hostConfiguration,
@@ -540,7 +540,7 @@
aspectConfiguration,
ruleClassProvider.getUniversalFragments(),
aspect.getDefinition().getConfigurationFragmentPolicy(),
- configConditions,
+ configConditions.asProviders(),
prerequisiteMap.values()))
.build();
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 9dfef7e..946cde7 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -46,6 +46,7 @@
import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoKey;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.analysis.config.ConfigConditions;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.analysis.config.CoreOptions.IncludeConfigFragmentsEnum;
@@ -80,6 +81,7 @@
import com.google.devtools.build.lib.packages.Info;
import com.google.devtools.build.lib.packages.InputFile;
import com.google.devtools.build.lib.packages.OutputFile;
+import com.google.devtools.build.lib.packages.Package.ConfigSettingVisibilityPolicy;
import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents;
import com.google.devtools.build.lib.packages.RawAttributeMapper;
import com.google.devtools.build.lib.packages.RequiredProviders;
@@ -1766,7 +1768,7 @@
private final PrerequisiteValidator prerequisiteValidator;
private final RuleErrorConsumer reporter;
private OrderedSetMultimap<Attribute, ConfiguredTargetAndData> prerequisiteMap;
- private ImmutableMap<Label, ConfigMatchingProvider> configConditions = ImmutableMap.of();
+ private ConfigConditions configConditions;
private String toolsRepository;
private StarlarkSemantics starlarkSemantics;
private Mutability mutability;
@@ -1815,11 +1817,21 @@
Preconditions.checkNotNull(visibility);
Preconditions.checkNotNull(constraintSemantics);
AttributeMap attributes =
- ConfiguredAttributeMapper.of(target.getAssociatedRule(), configConditions);
+ ConfiguredAttributeMapper.of(target.getAssociatedRule(), configConditions.asProviders());
checkAttributesNonEmpty(attributes);
ListMultimap<String, ConfiguredTargetAndData> targetMap = createTargetMap();
+ // This conditionally checks visibility on config_setting rules based on
+ // --config_setting_visibility_policy. This should be removed as soon as it's deemed safe
+ // to unconditionally check visibility. See https://github.com/bazelbuild/bazel/issues/12669.
+ if (target.getPackage().getConfigSettingVisibilityPolicy()
+ != ConfigSettingVisibilityPolicy.LEGACY_OFF) {
+ Attribute configSettingAttr = attributes.getAttributeDefinition("$config_dependencies");
+ for (ConfiguredTargetAndData condition : configConditions.asConfiguredTargets().values()) {
+ validateDirectPrerequisite(configSettingAttr, condition);
+ }
+ }
ListMultimap<String, ConfiguredFilesetEntry> filesetEntryMap =
- createFilesetEntryMap(target.getAssociatedRule(), configConditions);
+ createFilesetEntryMap(target.getAssociatedRule(), configConditions.asProviders());
if (rawExecProperties == null) {
if (!attributes.has(RuleClass.EXEC_PROPERTIES, Type.STRING_DICT)) {
rawExecProperties = ImmutableMap.of();
@@ -1833,7 +1845,7 @@
attributes,
targetMap,
filesetEntryMap,
- configConditions,
+ configConditions.asProviders(),
universalFragments,
getRuleClassNameForLogging(),
actionOwnerSymbol,
@@ -1908,11 +1920,10 @@
}
/**
- * Sets the configuration conditions needed to determine which paths to follow for this
- * rule's configurable attributes.
+ * Sets the configuration conditions needed to determine which paths to follow for this rule's
+ * configurable attributes.
*/
- public Builder setConfigConditions(
- ImmutableMap<Label, ConfigMatchingProvider> configConditions) {
+ public Builder setConfigConditions(ConfigConditions configConditions) {
this.configConditions = Preconditions.checkNotNull(configConditions);
return this;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigConditions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigConditions.java
new file mode 100644
index 0000000..40dd520
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigConditions.java
@@ -0,0 +1,81 @@
+// Copyright 2021 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.analysis.config;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
+import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
+
+/**
+ * Utility class for temporarily tracking {@code select()} keys' {@link ConfigMatchingProvider}s and
+ * {@link ConfiguredTarget}s.
+ *
+ * <p>This is a utility class because its only purpose is to maintain {@link ConfiguredTarget} long
+ * enough for {@link RuleContext.Builder} to do prerequisite validation on it (for example,
+ * visibility checks).
+ *
+ * <p>Once {@link RuleContext} is instantiated, it should only have access to {@link
+ * ConfigMatchingProvider}, on the principle that providers are the correct interfaces for storing
+ * and sharing target metadata. {@link ConfiguredTarget} isn't meant to persist that long.
+ */
+@AutoValue
+public abstract class ConfigConditions {
+ public abstract ImmutableMap<Label, ConfiguredTargetAndData> asConfiguredTargets();
+
+ public abstract ImmutableMap<Label, ConfigMatchingProvider> asProviders();
+
+ public static ConfigConditions create(
+ ImmutableMap<Label, ConfiguredTargetAndData> asConfiguredTargets,
+ ImmutableMap<Label, ConfigMatchingProvider> asProviders) {
+ return new AutoValue_ConfigConditions(asConfiguredTargets, asProviders);
+ }
+
+ public static final ConfigConditions EMPTY =
+ ConfigConditions.create(ImmutableMap.of(), ImmutableMap.of());
+
+ /** Exception for when a {@code select()} has an invalid key (for example, wrong target type). */
+ public static class InvalidConditionException extends Exception {}
+
+ /**
+ * Returns a {@link ConfigMatchingProvider} from the given configured target if appropriate, else
+ * triggers a {@link InvalidConditionException}.
+ *
+ * <p>This is the canonical place to extract {@link ConfigMatchingProvider}s from configured
+ * targets. It's not as simple as {@link ConfiguredTarget#getProvider}.
+ */
+ public static ConfigMatchingProvider fromConfiguredTarget(
+ ConfiguredTargetAndData selectKey, PlatformInfo targetPlatform)
+ throws InvalidConditionException {
+ ConfiguredTarget selectable = selectKey.getConfiguredTarget();
+ // The below handles config_setting (which natively provides ConfigMatchingProvider) and
+ // constraint_value (which needs a custom-built ConfigMatchingProvider).
+ ConfigMatchingProvider matchingProvider = selectable.getProvider(ConfigMatchingProvider.class);
+ if (matchingProvider != null) {
+ return matchingProvider;
+ }
+ ConstraintValueInfo constraintValueInfo = selectable.get(ConstraintValueInfo.PROVIDER);
+ if (constraintValueInfo != null && targetPlatform != null) {
+ // If platformInfo == null, that means the owning target doesn't invoke toolchain
+ // resolution, in which case depending on a constraint_value is nonsensical.
+ return constraintValueInfo.configMatchingProvider(targetPlatform);
+ }
+
+ // Not a valid provider for configuration conditions.
+ throw new InvalidConditionException();
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index eba918c..0c12388 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -144,6 +144,24 @@
private boolean defaultVisibilitySet;
/**
+ * How to enforce config_setting visibility settings.
+ *
+ * <p>This is a temporary setting in service of https://github.com/bazelbuild/bazel/issues/12669.
+ * After enough depot cleanup, config_setting will have the same visibility enforcement as all
+ * other rules.
+ */
+ public enum ConfigSettingVisibilityPolicy {
+ /** Don't enforce visibility for any config_setting. */
+ LEGACY_OFF,
+ /** Honor explicit visibility settings on config_setting, else use //visibility:public. */
+ DEFAULT_PUBLIC,
+ /** Enforce config_setting visibility exactly the same as all other rules. */
+ DEFAULT_STANDARD
+ }
+
+ private ConfigSettingVisibilityPolicy configSettingVisibilityPolicy;
+
+ /**
* Default package-level 'testonly' value for rules that do not specify it.
*/
private boolean defaultTestOnly = false;
@@ -436,6 +454,7 @@
this.targets = ImmutableSortedKeyMap.copyOf(builder.targets);
this.defaultVisibility = builder.defaultVisibility;
this.defaultVisibilitySet = builder.defaultVisibilitySet;
+ this.configSettingVisibilityPolicy = builder.configSettingVisibilityPolicy;
if (builder.defaultCopts == null) {
this.defaultCopts = ImmutableList.of();
} else {
@@ -701,6 +720,14 @@
}
/**
+ * How to enforce visibility on <code>config_setting</code> See
+ * {@link ConfigSettingVisibilityPolicy} for details.
+ */
+ public ConfigSettingVisibilityPolicy getConfigSettingVisibilityPolicy() {
+ return configSettingVisibilityPolicy;
+ }
+
+ /**
* Returns the default testonly value.
*/
public Boolean getDefaultTestOnly() {
@@ -920,6 +947,7 @@
// serialized representation is deterministic.
private final TreeMap<String, String> makeEnv = new TreeMap<>();
private RuleVisibility defaultVisibility = ConstantRuleVisibility.PRIVATE;
+ private ConfigSettingVisibilityPolicy configSettingVisibilityPolicy;
private boolean defaultVisibilitySet;
private List<String> defaultCopts = null;
private final List<String> features = new ArrayList<>();
@@ -1163,6 +1191,12 @@
return this;
}
+ /** Sets visibility enforcement policy for <code>config_setting</code>. */
+ public Builder setConfigSettingVisibilityPolicy(ConfigSettingVisibilityPolicy policy) {
+ this.configSettingVisibilityPolicy = policy;
+ return this;
+ }
+
/** Sets the default value of 'testonly'. Rule-level 'testonly' will override this. */
Builder setDefaultTestonly(boolean defaultTestonly) {
pkg.setDefaultTestOnly(defaultTestonly);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Rule.java b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
index f7aa381..7c78f5a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Rule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
@@ -34,6 +34,7 @@
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.StarlarkImplicitOutputsFunction;
import com.google.devtools.build.lib.packages.License.DistributionType;
+import com.google.devtools.build.lib.packages.Package.ConfigSettingVisibilityPolicy;
import com.google.devtools.build.lib.util.BinaryPredicate;
import java.util.Collection;
import java.util.HashSet;
@@ -842,10 +843,27 @@
*/
@Override
public RuleVisibility getVisibility() {
+ // Temporary logic to relax config_setting's visibility enforcement while depot migrations set
+ // visibility settings properly (legacy code may have visibility settings that would break if
+ // enforced). See https://github.com/bazelbuild/bazel/issues/12669. Ultimately this entire
+ // conditional should be removed.
+ if (ruleClass.getName().equals("config_setting")) {
+ ConfigSettingVisibilityPolicy policy = getPackage().getConfigSettingVisibilityPolicy();
+ if (policy == ConfigSettingVisibilityPolicy.LEGACY_OFF) {
+ return ConstantRuleVisibility.PUBLIC; // Always public, no matter what.
+ } else if (visibility != null) {
+ return visibility; // Use explicitly set visibility
+ } else if (policy == ConfigSettingVisibilityPolicy.DEFAULT_PUBLIC) {
+ return ConstantRuleVisibility.PUBLIC; // Default: //visibility:public.
+ } else {
+ return pkg.getDefaultVisibility(); // Default: same as all other rules.
+ }
+ }
+
+ // All other rules.
if (visibility != null) {
return visibility;
}
-
return pkg.getDefaultVisibility();
}
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java
index a233964..df34501 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageOptions.java
@@ -30,6 +30,7 @@
import com.google.devtools.common.options.Option;
import com.google.devtools.common.options.OptionDocumentationCategory;
import com.google.devtools.common.options.OptionEffectTag;
+import com.google.devtools.common.options.OptionMetadataTag;
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsParsingException;
import java.util.List;
@@ -121,6 +122,38 @@
)
public RuleVisibility defaultVisibility;
+
+ @Option(
+ name = "incompatible_enforce_config_setting_visibility",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+ effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
+ help = "If true, enforce config_setting visibility restrictions. If false, every "
+ + "config_setting is visible to every target. See "
+ + "https://github.com/bazelbuild/bazel/issues/12932."
+ )
+ public boolean enforceConfigSettingVisibility;
+
+ @Option(
+ name = "incompatible_config_setting_private_default_visibility",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+ effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
+ help = "If incompatible_enforce_config_setting_visibility=false, this is a noop. Else, if "
+ + "this flag is false, any config_setting without an explicit visibility attribute is "
+ + "//visibility:public. If this flag is true, config_setting follows the same visibility "
+ + "logic as all other rules. See https://github.com/bazelbuild/bazel/issues/12933."
+ )
+ public boolean configSettingPrivateDefaultVisibility;
+
@Option(
name = "legacy_globbing_threads",
defaultValue = "100",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index ca08292..385c111 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -37,7 +37,7 @@
import com.google.devtools.build.lib.analysis.ToolchainCollection;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
+import com.google.devtools.build.lib.analysis.config.ConfigConditions;
import com.google.devtools.build.lib.analysis.config.DependencyEvaluationException;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.analysis.configuredtargets.MergedConfiguredTarget;
@@ -403,7 +403,7 @@
}
// Get the configuration targets that trigger this rule's configurable attributes.
- ImmutableMap<Label, ConfigMatchingProvider> configConditions =
+ ConfigConditions configConditions =
ConfiguredTargetFunction.getConfigConditions(
env,
originalTargetAndAspectConfiguration,
@@ -423,7 +423,7 @@
resolver,
originalTargetAndAspectConfiguration,
aspectPath,
- configConditions,
+ configConditions.asProviders(),
unloadedToolchainContext == null
? null
: ToolchainCollection.builder()
@@ -640,7 +640,7 @@
ConfiguredAspectFactory aspectFactory,
ConfiguredTargetAndData associatedTarget,
BuildConfiguration aspectConfiguration,
- ImmutableMap<Label, ConfigMatchingProvider> configConditions,
+ ConfigConditions configConditions,
ResolvedToolchainContext toolchainContext,
OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> directDeps,
@Nullable NestedSetBuilder<Package> transitivePackagesForPackageRootResolution)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
index 7726e14e..a021adf 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -236,6 +236,7 @@
"//src/main/java/com/google/devtools/build/lib/analysis:buildinfo/build_info_key",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
+ "//src/main/java/com/google/devtools/build/lib/analysis:config/config_conditions",
"//src/main/java/com/google/devtools/build/lib/analysis:config/config_matching_provider",
"//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment",
@@ -1911,7 +1912,6 @@
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
- "//src/main/java/com/google/devtools/build/lib/remote/options",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/build/skyframe",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index 84e41d1..ba3ebea 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -48,13 +48,13 @@
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
+import com.google.devtools.build.lib.analysis.config.ConfigConditions;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.analysis.config.ConfigurationResolver;
import com.google.devtools.build.lib.analysis.config.DependencyEvaluationException;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
-import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
import com.google.devtools.build.lib.analysis.starlark.StarlarkTransition.TransitionException;
import com.google.devtools.build.lib.causes.AnalysisFailedCause;
@@ -95,7 +95,6 @@
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
-import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -116,9 +115,6 @@
public final class ConfiguredTargetFunction implements SkyFunction {
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
- private static final ImmutableMap<Label, ConfigMatchingProvider> NO_CONFIG_CONDITIONS =
- ImmutableMap.of();
-
/**
* Attempt to find a {@link ConfiguredValueCreationException} in a {@link ToolchainException}, or
* its causes.
@@ -287,7 +283,7 @@
}
// Get the configuration targets that trigger this rule's configurable attributes.
- ImmutableMap<Label, ConfigMatchingProvider> configConditions =
+ ConfigConditions configConditions =
getConfigConditions(
env,
ctgValue,
@@ -307,7 +303,7 @@
// Note that this doesn't apply to AspectFunction, because aspects can't have configurable
// attributes.
if (!transitiveRootCauses.isEmpty()
- && !Objects.equals(configConditions, NO_CONFIG_CONDITIONS)) {
+ && !Objects.equals(configConditions, ConfigConditions.EMPTY)) {
NestedSet<Cause> causes = transitiveRootCauses.build();
throw new ConfiguredTargetFunctionException(
new ConfiguredValueCreationException(
@@ -324,7 +320,7 @@
resolver,
ctgValue,
ImmutableList.<Aspect>of(),
- configConditions,
+ configConditions.asProviders(),
unloadedToolchainContexts == null
? null
: unloadedToolchainContexts.asToolchainContexts(),
@@ -712,14 +708,13 @@
}
/**
- * Returns the set of {@link ConfigMatchingProvider}s that key the configurable attributes used by
- * this rule.
+ * Returns the targets that key the configurable attributes used by this rule.
*
* <p>>If the configured targets supplying those providers aren't yet resolved by the dependency
* resolver, returns null.
*/
@Nullable
- static ImmutableMap<Label, ConfigMatchingProvider> getConfigConditions(
+ static ConfigConditions getConfigConditions(
Environment env,
TargetAndConfiguration ctgValue,
@Nullable NestedSetBuilder<Package> transitivePackagesForPackageRootResolution,
@@ -728,11 +723,11 @@
throws DependencyEvaluationException, InterruptedException {
Target target = ctgValue.getTarget();
if (!(target instanceof Rule)) {
- return NO_CONFIG_CONDITIONS;
+ return ConfigConditions.EMPTY;
}
RawAttributeMapper attrs = RawAttributeMapper.of(((Rule) target));
if (!attrs.has(RuleClass.CONFIG_SETTING_DEPS_ATTRIBUTE)) {
- return NO_CONFIG_CONDITIONS;
+ return ConfigConditions.EMPTY;
}
// Collect the labels of the configured targets we need to resolve.
@@ -741,7 +736,7 @@
.map(configLabel -> target.getLabel().resolveRepositoryRelative(configLabel))
.collect(Collectors.toList());
if (configLabels.isEmpty()) {
- return NO_CONFIG_CONDITIONS;
+ return ConfigConditions.EMPTY;
} else if (ctgValue.getConfiguration().trimConfigurationsRetroactively()) {
String message =
target.getLabel()
@@ -795,33 +790,24 @@
throw e;
}
- Map<Label, ConfigMatchingProvider> configConditions = new LinkedHashMap<>();
+ ImmutableMap.Builder<Label, ConfiguredTargetAndData> asConfiguredTargets =
+ ImmutableMap.builder();
+ ImmutableMap.Builder<Label, ConfigMatchingProvider> asConfigConditions = ImmutableMap.builder();
// Get the configured targets as ConfigMatchingProvider interfaces.
for (Dependency entry : configConditionDeps) {
SkyKey baseKey = entry.getConfiguredTargetKey();
- // The code above guarantees that value is non-null here.
- ConfiguredTarget value = configValues.get(baseKey).getConfiguredTarget();
- // The below handles config_setting (which nativly provides ConfigMatchingProvider) and
- // constraint_value (which needs a custom-built ConfigMatchingProvider). If we ever add
- // support for more rules we should move resolution logic to ConfigMatchingProvider and
- // simplify the logic here.
- ConfigMatchingProvider matchingProvider = value.getProvider(ConfigMatchingProvider.class);
- ConstraintValueInfo constraintValueInfo = value.get(ConstraintValueInfo.PROVIDER);
-
- if (matchingProvider != null) {
- configConditions.put(entry.getLabel(), matchingProvider);
- } else if (constraintValueInfo != null && platformInfo != null) {
- // If platformInfo == null, that means the owning target doesn't invoke toolchain
- // resolution, in which case depending on a constraint_value is non-sensical.
- configConditions.put(
- entry.getLabel(), constraintValueInfo.configMatchingProvider(platformInfo));
- } else {
- // Not a valid provider for configuration conditions.
+ // The code above guarantees that selectKeyTarget is non-null here.
+ ConfiguredTargetAndData selectKeyTarget = configValues.get(baseKey);
+ asConfiguredTargets.put(entry.getLabel(), selectKeyTarget);
+ try {
+ asConfigConditions.put(
+ entry.getLabel(), ConfigConditions.fromConfiguredTarget(selectKeyTarget, platformInfo));
+ } catch (ConfigConditions.InvalidConditionException e) {
String message =
String.format(
"%s is not a valid select() condition for %s.\n",
- entry.getLabel(), target.getLabel())
+ selectKeyTarget.getTarget().getLabel(), target.getLabel())
+ String.format(
"To inspect the select(), run: bazel query --output=build %s.\n",
target.getLabel())
@@ -833,7 +819,7 @@
}
}
- return ImmutableMap.copyOf(configConditions);
+ return ConfigConditions.create(asConfiguredTargets.build(), asConfigConditions.build());
}
/**
@@ -992,7 +978,7 @@
BuildConfiguration configuration,
ConfiguredTargetKey configuredTargetKey,
OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> depValueMap,
- ImmutableMap<Label, ConfigMatchingProvider> configConditions,
+ ConfigConditions configConditions,
@Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts,
@Nullable NestedSetBuilder<Package> transitivePackagesForPackageRootResolution)
throws ConfiguredTargetFunctionException, InterruptedException {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index 848ac73..a8a6647 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -45,6 +45,7 @@
import com.google.devtools.build.lib.packages.LegacyGlobber;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.Package;
+import com.google.devtools.build.lib.packages.Package.ConfigSettingVisibilityPolicy;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.PackageValidator.InvalidPackageException;
import com.google.devtools.build.lib.packages.RuleVisibility;
@@ -477,6 +478,8 @@
FileValue buildFileValue = getBuildFileValue(env, buildFileRootedPath);
RuleVisibility defaultVisibility = PrecomputedValue.DEFAULT_VISIBILITY.get(env);
+ ConfigSettingVisibilityPolicy configSettingVisibilityPolicy =
+ PrecomputedValue.CONFIG_SETTING_VISIBILITY_POLICY.get(env);
StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env);
IgnoredPackagePrefixesValue repositoryIgnoredPackagePrefixes =
(IgnoredPackagePrefixesValue)
@@ -520,6 +523,7 @@
buildFileRootedPath,
buildFileValue,
defaultVisibility,
+ configSettingVisibilityPolicy,
starlarkSemantics,
preludeLabel,
packageLookupValue.getRoot(),
@@ -1187,6 +1191,7 @@
RootedPath buildFilePath,
FileValue buildFileValue,
RuleVisibility defaultVisibility,
+ ConfigSettingVisibilityPolicy configSettingVisibilityPolicy,
StarlarkSemantics starlarkSemantics,
@Nullable Label preludeLabel,
Root packageRoot,
@@ -1289,7 +1294,8 @@
// "defaultVisibility" comes from the command line.
// Let's give the BUILD file a chance to set default_visibility once,
// by resetting the PackageBuilder.defaultVisibilitySet flag.
- .setDefaultVisibilitySet(false);
+ .setDefaultVisibilitySet(false)
+ .setConfigSettingVisibilityPolicy(configSettingVisibilityPolicy);
Set<SkyKey> globDepKeys = ImmutableSet.of();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java
index e039158..19810fb 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java
@@ -19,6 +19,7 @@
import com.google.common.base.Suppliers;
import com.google.common.collect.Interner;
import com.google.devtools.build.lib.concurrent.BlazeInterners;
+import com.google.devtools.build.lib.packages.Package.ConfigSettingVisibilityPolicy;
import com.google.devtools.build.lib.packages.RuleVisibility;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
@@ -75,6 +76,9 @@
public static final Precomputed<RuleVisibility> DEFAULT_VISIBILITY =
new Precomputed<>("default_visibility");
+ public static final Precomputed<ConfigSettingVisibilityPolicy> CONFIG_SETTING_VISIBILITY_POLICY =
+ new Precomputed<>("config_setting_visibility_policy");
+
public static final Precomputed<StarlarkSemantics> STARLARK_SEMANTICS =
new Precomputed<>("starlark_semantics");
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index 90d5ea1..1a0390b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -54,7 +54,7 @@
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.BuildOptions.OptionsDiff;
-import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
+import com.google.devtools.build.lib.analysis.config.ConfigConditions;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.analysis.config.FragmentClassSet;
import com.google.devtools.build.lib.bugreport.BugReport;
@@ -927,7 +927,7 @@
CachingAnalysisEnvironment analysisEnvironment,
ConfiguredTargetKey configuredTargetKey,
OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> prerequisiteMap,
- ImmutableMap<Label, ConfigMatchingProvider> configConditions,
+ ConfigConditions configConditions,
@Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts)
throws InterruptedException, ActionConflictException, InvalidExecGroupException {
Preconditions.checkState(
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 dc36577..07713cf 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
@@ -120,6 +120,7 @@
import com.google.devtools.build.lib.packages.NoSuchThingException;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Package.Builder.PackageSettings;
+import com.google.devtools.build.lib.packages.Package.ConfigSettingVisibilityPolicy;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.RuleVisibility;
@@ -1131,6 +1132,10 @@
PrecomputedValue.DEFAULT_VISIBILITY.set(injectable(), defaultVisibility);
}
+ private void setConfigSettingVisibilityPolicty(ConfigSettingVisibilityPolicy policy) {
+ PrecomputedValue.CONFIG_SETTING_VISIBILITY_POLICY.set(injectable(), policy);
+ }
+
private void setStarlarkSemantics(StarlarkSemantics starlarkSemantics) {
PrecomputedValue.STARLARK_SEMANTICS.set(injectable(), starlarkSemantics);
}
@@ -1370,6 +1375,13 @@
setShowLoadingProgress(packageOptions.showLoadingProgress);
setDefaultVisibility(packageOptions.defaultVisibility);
+ if (!packageOptions.enforceConfigSettingVisibility) {
+ setConfigSettingVisibilityPolicty(ConfigSettingVisibilityPolicy.LEGACY_OFF);
+ } else {
+ setConfigSettingVisibilityPolicty(packageOptions.configSettingPrivateDefaultVisibility
+ ? ConfigSettingVisibilityPolicy.DEFAULT_STANDARD
+ : ConfigSettingVisibilityPolicy.DEFAULT_PUBLIC);
+ }
StarlarkSemantics starlarkSemantics = getEffectiveStarlarkSemantics(buildLanguageOptions);
setStarlarkSemantics(starlarkSemantics);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
index ed1e12b..4576b10 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
@@ -44,6 +44,7 @@
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Package.Builder.DefaultPackageSettings;
+import com.google.devtools.build.lib.packages.Package.ConfigSettingVisibilityPolicy;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension;
import com.google.devtools.build.lib.packages.PackageLoadingListener;
@@ -317,6 +318,8 @@
}
PrecomputedValue.PATH_PACKAGE_LOCATOR.set(injectable, pkgLocator);
PrecomputedValue.DEFAULT_VISIBILITY.set(injectable, ConstantRuleVisibility.PRIVATE);
+ PrecomputedValue.CONFIG_SETTING_VISIBILITY_POLICY
+ .set(injectable, ConfigSettingVisibilityPolicy.LEGACY_OFF);
PrecomputedValue.STARLARK_SEMANTICS.set(injectable, starlarkSemantics);
return new ImmutableDiff(ImmutableList.of(), valuesToInject);
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java b/src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java
index eb1c8ee..4cd7ad3 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/ConfigurableAttributesTest.java
@@ -1291,4 +1291,215 @@
+ " //a:foo\n"
+ " //a:alias_to_foo");
}
+
+ @Test
+ public void defaultVisibilityConfigSetting_noVisibilityEnforcement() throws Exception {
+ // Production builds default to private visibility, but BuildViewTestCase defaults to public.
+ setPackageOptions("--default_visibility=private",
+ "--incompatible_enforce_config_setting_visibility=false");
+ scratch.file("c/BUILD", "config_setting(name = 'foo', define_values = { 'foo': '1' })");
+ scratch.file(
+ "a/BUILD",
+ "rule_with_boolean_attr(",
+ " name = 'binary',",
+ " boolean_attr= select({",
+ " '//c:foo': 0,",
+ " '//conditions:default': 1",
+ " }))");
+ reporter.removeHandler(failFastHandler);
+ assertThat(getConfiguredTarget("//a:binary")).isNotNull();
+ assertNoEvents();
+ }
+
+ @Test
+ public void privateVisibilityConfigSetting_noVisibilityEnforcement() throws Exception {
+ // Production builds default to private visibility, but BuildViewTestCase defaults to public.
+ setPackageOptions("--default_visibility=private",
+ "--incompatible_enforce_config_setting_visibility=false");
+ scratch.file(
+ "c/BUILD",
+ "config_setting(",
+ " name = 'foo',",
+ " define_values = { 'foo': '1' },",
+ " visibility = ['//visibility:private']",
+ ")");
+ scratch.file(
+ "a/BUILD",
+ "rule_with_boolean_attr(",
+ " name = 'binary',",
+ " boolean_attr= select({",
+ " '//c:foo': 0,",
+ " '//conditions:default': 1",
+ " }))");
+ reporter.removeHandler(failFastHandler);
+ assertThat(getConfiguredTarget("//a:binary")).isNotNull();
+ assertNoEvents();
+ }
+
+ @Test
+ public void publicVisibilityConfigSetting__noVisibilityEnforcement() throws Exception {
+ // Production builds default to private visibility, but BuildViewTestCase defaults to public.
+ setPackageOptions("--default_visibility=private",
+ "--incompatible_enforce_config_setting_visibility=false");
+ scratch.file(
+ "c/BUILD",
+ "config_setting(",
+ " name = 'foo',",
+ " define_values = { 'foo': '1' },",
+ " visibility = ['//visibility:public']",
+ ")");
+ scratch.file(
+ "a/BUILD",
+ "rule_with_boolean_attr(",
+ " name = 'binary',",
+ " boolean_attr= select({",
+ " '//c:foo': 0,",
+ " '//conditions:default': 1",
+ " }))");
+ reporter.removeHandler(failFastHandler);
+ assertThat(getConfiguredTarget("//a:binary")).isNotNull();
+ assertNoEvents();
+ }
+ @Test
+ public void defaultVisibilityConfigSetting_defaultIsPublic() throws Exception {
+ // Production builds default to private visibility, but BuildViewTestCase defaults to public.
+ setPackageOptions("--default_visibility=private",
+ "--incompatible_enforce_config_setting_visibility=true",
+ "--incompatible_config_setting_private_default_visibility=false");
+ scratch.file("c/BUILD", "config_setting(name = 'foo', define_values = { 'foo': '1' })");
+ scratch.file(
+ "a/BUILD",
+ "rule_with_boolean_attr(",
+ " name = 'binary',",
+ " boolean_attr= select({",
+ " '//c:foo': 0,",
+ " '//conditions:default': 1",
+ " }))");
+ reporter.removeHandler(failFastHandler);
+ assertThat(getConfiguredTarget("//a:binary")).isNotNull();
+ assertNoEvents();
+ }
+
+ @Test
+ public void privateVisibilityConfigSetting_defaultIsPublic() throws Exception {
+ // Production builds default to private visibility, but BuildViewTestCase defaults to public.
+ setPackageOptions("--default_visibility=private",
+ "--incompatible_enforce_config_setting_visibility=true",
+ "--incompatible_config_setting_private_default_visibility=false");
+ scratch.file(
+ "c/BUILD",
+ "config_setting(",
+ " name = 'foo',",
+ " define_values = { 'foo': '1' },",
+ " visibility = ['//visibility:private']",
+ ")");
+ scratch.file(
+ "a/BUILD",
+ "rule_with_boolean_attr(",
+ " name = 'binary',",
+ " boolean_attr= select({",
+ " '//c:foo': 0,",
+ " '//conditions:default': 1",
+ " }))");
+ reporter.removeHandler(failFastHandler);
+ assertThat(getConfiguredTarget("//a:binary")).isNull();
+ assertContainsEvent("'//c:foo' is not visible from target '//a:binary'");
+ }
+
+ @Test
+ public void publicVisibilityConfigSetting_defaultIsPublic() throws Exception {
+ // Production builds default to private visibility, but BuildViewTestCase defaults to public.
+ setPackageOptions("--default_visibility=private",
+ "--incompatible_enforce_config_setting_visibility=true",
+ "--incompatible_config_setting_private_default_visibility=false");
+ scratch.file(
+ "c/BUILD",
+ "config_setting(",
+ " name = 'foo',",
+ " define_values = { 'foo': '1' },",
+ " visibility = ['//visibility:public']",
+ ")");
+ scratch.file(
+ "a/BUILD",
+ "rule_with_boolean_attr(",
+ " name = 'binary',",
+ " boolean_attr= select({",
+ " '//c:foo': 0,",
+ " '//conditions:default': 1",
+ " }))");
+ reporter.removeHandler(failFastHandler);
+ assertThat(getConfiguredTarget("//a:binary")).isNotNull();
+ assertNoEvents();
+ }
+ @Test
+ public void defaultVisibilityConfigSetting_defaultIsPrivate() throws Exception {
+ // Production builds default to private visibility, but BuildViewTestCase defaults to public.
+ setPackageOptions("--default_visibility=private",
+ "--incompatible_enforce_config_setting_visibility=true",
+ "--incompatible_config_setting_private_default_visibility=true");
+ scratch.file("c/BUILD", "config_setting(name = 'foo', define_values = { 'foo': '1' })");
+ scratch.file(
+ "a/BUILD",
+ "rule_with_boolean_attr(",
+ " name = 'binary',",
+ " boolean_attr= select({",
+ " '//c:foo': 0,",
+ " '//conditions:default': 1",
+ " }))");
+ reporter.removeHandler(failFastHandler);
+ assertThat(getConfiguredTarget("//a:binary")).isNull();
+ assertContainsEvent("'//c:foo' is not visible from target '//a:binary'");
+ }
+
+ @Test
+ public void privateVisibilityConfigSetting_defaultIsPrivate() throws Exception {
+ // Production builds default to private visibility, but BuildViewTestCase defaults to public.
+ setPackageOptions("--default_visibility=private",
+ "--incompatible_enforce_config_setting_visibility=true",
+ "--incompatible_config_setting_private_default_visibility=true");
+ scratch.file(
+ "c/BUILD",
+ "config_setting(",
+ " name = 'foo',",
+ " define_values = { 'foo': '1' },",
+ " visibility = ['//visibility:private']",
+ ")");
+ scratch.file(
+ "a/BUILD",
+ "rule_with_boolean_attr(",
+ " name = 'binary',",
+ " boolean_attr= select({",
+ " '//c:foo': 0,",
+ " '//conditions:default': 1",
+ " }))");
+ reporter.removeHandler(failFastHandler);
+ assertThat(getConfiguredTarget("//a:binary")).isNull();
+ assertContainsEvent("'//c:foo' is not visible from target '//a:binary'");
+ }
+
+ @Test
+ public void publicVisibilityConfigSetting_defaultIsPrivate() throws Exception {
+ // Production builds default to private visibility, but BuildViewTestCase defaults to public.
+ setPackageOptions("--default_visibility=private",
+ "--incompatible_enforce_config_setting_visibility=true",
+ "--incompatible_config_setting_private_default_visibility=true");
+ scratch.file(
+ "c/BUILD",
+ "config_setting(",
+ " name = 'foo',",
+ " define_values = { 'foo': '1' },",
+ " visibility = ['//visibility:public']",
+ ")");
+ scratch.file(
+ "a/BUILD",
+ "rule_with_boolean_attr(",
+ " name = 'binary',",
+ " boolean_attr= select({",
+ " '//c:foo': 0,",
+ " '//conditions:default': 1",
+ " }))");
+ reporter.removeHandler(failFastHandler);
+ assertThat(getConfiguredTarget("//a:binary")).isNotNull();
+ assertNoEvents();
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD
index 45c37ce..a7fce74 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD
@@ -50,6 +50,7 @@
"//src/main/java/com/google/devtools/build/lib/analysis:buildinfo/build_info_key",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
+ "//src/main/java/com/google/devtools/build/lib/analysis:config/config_conditions",
"//src/main/java/com/google/devtools/build/lib/analysis:config/config_matching_provider",
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment",
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
index a71e381..8d10729 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
@@ -49,6 +49,7 @@
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.analysis.config.ConfigConditions;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.analysis.config.ConfigurationResolver;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
@@ -608,7 +609,7 @@
.setPrerequisites(
ConfiguredTargetFactory.transformPrerequisiteMap(
prerequisiteMap, target.getAssociatedRule()))
- .setConfigConditions(ImmutableMap.<Label, ConfigMatchingProvider>of())
+ .setConfigConditions(ConfigConditions.EMPTY)
.setUniversalFragments(ruleClassProvider.getUniversalFragments())
.setToolchainContexts(resolvedToolchainContext.build())
.setConstraintSemantics(ruleClassProvider.getConstraintSemantics())