diff options
Diffstat (limited to 'src')
26 files changed, 95 insertions, 1289 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD index 08fb8a1fed..adb0f19f1b 100644 --- a/src/main/java/com/google/devtools/build/lib/BUILD +++ b/src/main/java/com/google/devtools/build/lib/BUILD @@ -38,7 +38,6 @@ filegroup( "//src/main/java/com/google/devtools/build/lib/rules/genrule:srcs", "//src/main/java/com/google/devtools/build/lib/rules/objc:srcs", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:srcs", - "//src/main/java/com/google/devtools/build/lib/analysis/featurecontrol:srcs", "//src/main/java/com/google/devtools/build/lib/analysis/platform:srcs", "//src/main/java/com/google/devtools/build/lib/analysis/whitelisting:srcs", "//src/main/java/com/google/devtools/build/lib/rules/platform:srcs", @@ -715,7 +714,6 @@ java_library( ":util", ":vfs", "//src/main/java/com/google/devtools/build/lib/actions", - "//src/main/java/com/google/devtools/build/lib/analysis/featurecontrol", "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto", "//src/main/java/com/google/devtools/build/lib/query2:query-output", "//src/main/java/com/google/devtools/build/lib/rules/apple", @@ -1072,7 +1070,6 @@ java_library( ), deps = [ ":build-base", - "//src/main/java/com/google/devtools/build/lib/analysis/featurecontrol", "//src/main/java/com/google/devtools/build/lib/rules/config", "//third_party:guava", ], @@ -1103,7 +1100,6 @@ java_library( ":vfs", "//src/main/java/com/google/devtools/build/lib:skylarkinterface", "//src/main/java/com/google/devtools/build/lib/actions", - "//src/main/java/com/google/devtools/build/lib/analysis/featurecontrol", "//src/main/java/com/google/devtools/build/lib/rules/config", "//src/main/java/com/google/devtools/build/lib/rules/cpp", "//src/main/java/com/google/devtools/common/options", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/BUILD deleted file mode 100644 index a20987fbe4..0000000000 --- a/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/BUILD +++ /dev/null @@ -1,26 +0,0 @@ -# Description: -# Configuration mechanisms for rolling out and deprecating pieces of Bazel functionality. - -package( - default_visibility = ["//src:__subpackages__"], -) - -filegroup( - name = "srcs", - srcs = glob(["**"]), -) - -java_library( - name = "featurecontrol", - srcs = glob(["*.java"]), - deps = [ - "//src/main/java/com/google/devtools/build/lib:build-base", - "//src/main/java/com/google/devtools/build/lib:packages-internal", - "//src/main/java/com/google/devtools/build/lib:util", - "//src/main/java/com/google/devtools/build/lib/cmdline", - "//src/main/java/com/google/devtools/common/options", - "//third_party:auto_value", - "//third_party:guava", - "//third_party:jsr305", - ], -) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyConfiguration.java deleted file mode 100644 index 37adc1a0b1..0000000000 --- a/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyConfiguration.java +++ /dev/null @@ -1,165 +0,0 @@ -// Copyright 2017 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.featurecontrol; - -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSetMultimap; -import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.analysis.config.BuildConfiguration; -import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.packages.PackageSpecification; -import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; -import com.google.devtools.build.lib.util.Preconditions; - -/** - * A configuration fragment which controls access to features being rolled out or deprecated, - * allowing such features to be limited to particular packages to avoid excessive spread when - * rolling out or deprecating a feature. It's controlled by {@link FeaturePolicyOptions}. - * - * <p>A "feature", for this package's purposes, is any combination of related functionality which - * should be limited to specified packages. Because this is part of the configuration, it can only - * be accessed during the analysis phase; decisions which are made during the loading phase can't - * use this information. Some examples of use cases for this fragment: - * - * <ul> - * <li>A new rule class which could have a major impact on Blaze's memory usage is added. To limit - * this impact during the experimental phase, a feature policy is added which makes it an - * error for rules of that class to be created - or used by other rules - in any package other - * than those defined by the policy. The policy is populated with projects who are doing - * guided experimentation with the feature, and gradually expands as the feature rolls out. - * Then the feature's policy can be removed, making it generally available. - * <li>An attribute is being deprecated. To prevent rollback, a feature policy is added which - * makes it an error for rules in packages other than those defined by the policy to specify a - * value for that attribute. The policy is populated with existing users, and as those users - * are migrated, they are removed from the policy until it is completely empty. Then the - * attribute can be removed entirely. - * </ul> - * - * <p>To use this package: - * - * <ol> - * <li>Define a feature ID in the {@link FeaturePolicyLoader}'s constructor (in the - * RuleClassProvider). This is the string that will be used when checking for the feature in - * rule code, as well as the string used in the flag value for {@link FeaturePolicyOptions}. - * <li>In the RuleClass(es) which will change based on the feature's state, declare {@link - * FeaturePolicyConfiguration} as a required configuration fragment. - * <li>In the ConfiguredTargetFactory of those rules, get the FeaturePolicyConfiguration and check - * {@link #isFeatureEnabledForRule(String,Label)} with the feature ID created in step 1 and - * the label of the current rule. In the event that an error needs to be displayed, use {@link - * #getPolicyForFeature(String)} to show the user where the policy is. - * <li>Create a package_group containing the list of packages which should have access to this - * feature. It can be empty (no packages can access the feature) or contain //... (all - * packages can access the feature) to begin with. - * <li>After the a release containing the feature ID has been pushed, update the global RC file - * with a --feature_control_policy=(your_feature)=(your_package_group) flag. You can now alter - * access to your feature by changing the package_group. - * </ol> - * - * <p>To stop using this package: - * - * <ol> - * <li>Your policy should be at an end state - containing all packages (a rollout which has become - * generally available) or no packages (a deprecated feature which has been totally cleaned - * up). - * <li>Make the behavior the policy controlled permanent - remove a deprecated feature, or remove - * the check on a feature which is being rolled out. - * <li>After this new version is released, remove the flag from the global rc file, and remove the - * feature ID from the constructor for {@link FeaturePolicyLoader}. - * </ol> - * - * @see FeaturePolicyLoader - * @deprecated This is deprecated because the dependency on the package group used to hold the - * whitelist is not accessible through blaze query. Use {@link Whitelist}. - */ -@Deprecated -public final class FeaturePolicyConfiguration extends BuildConfiguration.Fragment { - - private final ImmutableSetMultimap<String, PackageSpecification> features; - private final ImmutableMap<String, String> policies; - - /** - * Creates a new FeaturePolicyConfiguration. - * - * @param features Map mapping from a feature ID to the packages which are able to access it. If a - * feature ID is not present in this mapping, it is not accessible from any package. - * @param policies Map mapping from a feature ID to a string policy to be shown to the user. A - * string must be present as a key in this mapping to be considered a valid feature ID. - */ - public FeaturePolicyConfiguration( - ImmutableSetMultimap<String, PackageSpecification> features, - ImmutableMap<String, String> policies) { - this.features = features; - this.policies = policies; - } - - /** - * Returns whether, according to the current policy, the given feature is enabled for the given - * rule. - */ - public boolean isFeatureEnabledForRule(String feature, Label rule) { - Preconditions.checkArgument(policies.containsKey(feature), "No such feature: %s", feature); - ImmutableSet<PackageSpecification> result = features.get(feature); - for (PackageSpecification spec : result) { - if (spec.containsPackage(rule.getPackageIdentifier())) { - return true; - } - } - return false; - } - - /** - * Checks whether the rule in the given RuleContext has access to a given feature and reports an - * error if not. - * - * @param ruleContext The context in which this check is being executed. - * @param policyName The name of the policy. - * @param feature The name of the feature being used, to be printed in the error if the policy - * forbids access to this rule. - * @param additionalErrorMessage Additional text for the error message. - */ - public static void checkAvailable( - RuleContext ruleContext, String policyName, String feature, String additionalErrorMessage) - throws RuleErrorException { - FeaturePolicyConfiguration policy = ruleContext.getFragment(FeaturePolicyConfiguration.class); - Label label = ruleContext.getLabel(); - if (!policy.isFeatureEnabledForRule(policyName, label)) { - String message = "%s is not available in package '%s' according to policy '%s'.%s"; - ruleContext.ruleError( - String.format( - message, - feature, - label.getPackageIdentifier(), - policy.getPolicyForFeature(policyName), - additionalErrorMessage == null ? "" : " " + additionalErrorMessage + ".")); - throw new RuleErrorException(); - } - } - - public static void checkAvailable(RuleContext ruleContext, String policyName, String feature) - throws RuleErrorException { - checkAvailable(ruleContext, policyName, feature, null); - } - - /** - * Returns a String suitable for presenting to the user which represents the policy used for the - * given feature. - */ - public String getPolicyForFeature(String feature) { - String result = policies.get(feature); - Preconditions.checkArgument(result != null, "No such feature: %s", feature); - return result; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyLoader.java b/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyLoader.java deleted file mode 100644 index 7704c8c6cd..0000000000 --- a/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyLoader.java +++ /dev/null @@ -1,171 +0,0 @@ -// Copyright 2017 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.featurecontrol; - -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSetMultimap; -import com.google.devtools.build.lib.analysis.RedirectChaser; -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.ConfigurationEnvironment; -import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; -import com.google.devtools.build.lib.analysis.config.FragmentOptions; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; -import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.packages.NoSuchPackageException; -import com.google.devtools.build.lib.packages.NoSuchTargetException; -import com.google.devtools.build.lib.packages.PackageGroup; -import com.google.devtools.build.lib.packages.PackageSpecification; -import com.google.devtools.build.lib.packages.Target; -import com.google.devtools.build.lib.util.Preconditions; -import java.util.ArrayDeque; -import java.util.LinkedHashSet; -import java.util.Queue; -import java.util.Set; -import javax.annotation.Nullable; - -/** - * A loader for the FeaturePolicyConfiguration fragment. - * - * @deprecated This is deprecated because the dependency on the package group used to hold the - * whitelist is not accessible through blaze query. Use {@link Whitelist}. - */ -@Deprecated -public final class FeaturePolicyLoader implements ConfigurationFragmentFactory { - - private final ImmutableSet<String> permittedFeatures; - - public FeaturePolicyLoader(Iterable<String> permittedFeatures) { - this.permittedFeatures = ImmutableSet.copyOf(permittedFeatures); - for (String permittedFeature : this.permittedFeatures) { - Preconditions.checkArgument( - !permittedFeature.contains("="), "Feature names cannot contain ="); - } - } - - @Override - @Nullable - public BuildConfiguration.Fragment create(ConfigurationEnvironment env, BuildOptions buildOptions) - throws InvalidConfigurationException, InterruptedException { - ImmutableSetMultimap.Builder<String, PackageSpecification> features = - new ImmutableSetMultimap.Builder<>(); - ImmutableMap.Builder<String, String> policies = new ImmutableMap.Builder<>(); - LinkedHashSet<String> unseenFeatures = new LinkedHashSet<>(permittedFeatures); - - for (PolicyEntry entry : buildOptions.get(FeaturePolicyOptions.class).policies) { - if (!permittedFeatures.contains(entry.getFeature())) { - // It would be so nice to be able to do this during parsing... but because options are - // parsed statically through reflection, we have no way of doing this until we get to the - // configuration loader. - throw new InvalidConfigurationException("No such feature: " + entry.getFeature()); - } - if (!unseenFeatures.remove(entry.getFeature())) { - // TODO(mstaib): Perhaps we should allow for overriding or concatenation here? - throw new InvalidConfigurationException( - "Multiple definitions of the rollout policy for feature " - + entry.getFeature() - + ". To use multiple package_groups, use a package_group with the includes " - + "attribute instead."); - } - - Iterable<PackageSpecification> packageSpecifications = - getAllPackageSpecificationsForPackageGroup( - env, entry.getPackageGroupLabel(), entry.getFeature()); - - if (packageSpecifications == null) { - return null; - } - - features.putAll(entry.getFeature(), packageSpecifications); - policies.put(entry.getFeature(), entry.getPackageGroupLabel().toString()); - } - - // Default to universal access for all features not declared. - for (String unseenFeature : unseenFeatures) { - features.put(unseenFeature, PackageSpecification.everything()); - policies.put(unseenFeature, "//..."); - } - - return new FeaturePolicyConfiguration(features.build(), policies.build()); - } - - /** - * Evaluates, recursively, the given package group. Returns {@code null} in the case of missing - * Skyframe dependencies. - */ - @Nullable - private static Iterable<PackageSpecification> getAllPackageSpecificationsForPackageGroup( - ConfigurationEnvironment env, Label packageGroupLabel, String feature) - throws InvalidConfigurationException, InterruptedException { - String context = feature + " feature policy"; - Label actualPackageGroupLabel = RedirectChaser.followRedirects(env, packageGroupLabel, context); - if (actualPackageGroupLabel == null) { - return null; - } - - ImmutableSet.Builder<PackageSpecification> packages = new ImmutableSet.Builder<>(); - Set<Label> alreadyVisitedPackageGroups = new LinkedHashSet<>(); - Queue<Label> packageGroupsToVisit = new ArrayDeque<>(); - packageGroupsToVisit.add(actualPackageGroupLabel); - alreadyVisitedPackageGroups.add(actualPackageGroupLabel); - - while (!packageGroupsToVisit.isEmpty()) { - Target target; - try { - // This is guaranteed to succeed, because the RedirectChaser was used to get this label, - // and it will throw an InvalidConfigurationException if the target doesn't exist. - target = env.getTarget(packageGroupsToVisit.remove()); - } catch (NoSuchPackageException | NoSuchTargetException impossible) { - throw new AssertionError(impossible); - } - if (target == null) { - return null; - } - - if (!(target instanceof PackageGroup)) { - throw new InvalidConfigurationException( - target.getLabel() + " is not a package_group in " + context); - } - - PackageGroup packageGroup = (PackageGroup) target; - - packages.addAll(packageGroup.getPackageSpecifications()); - - for (Label include : packageGroup.getIncludes()) { - Label actualInclude = RedirectChaser.followRedirects(env, include, context); - if (actualInclude == null) { - return null; - } - - if (alreadyVisitedPackageGroups.add(actualInclude)) { - packageGroupsToVisit.add(actualInclude); - } - } - } - - return packages.build(); - } - - @Override - public Class<? extends BuildConfiguration.Fragment> creates() { - return FeaturePolicyConfiguration.class; - } - - @Override - public ImmutableSet<Class<? extends FragmentOptions>> requiredOptions() { - return ImmutableSet.<Class<? extends FragmentOptions>>of(FeaturePolicyOptions.class); - } -} diff --git a/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyOptions.java deleted file mode 100644 index 737c1c878c..0000000000 --- a/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyOptions.java +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright 2017 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.featurecontrol; - -import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.analysis.config.FragmentOptions; -import com.google.devtools.common.options.Option; -import com.google.devtools.common.options.OptionDocumentationCategory; -import com.google.devtools.common.options.OptionEffectTag; -import java.util.List; - -/** - * The options fragment which defines {@link FeaturePolicyConfiguration}. - * - * @deprecated This is deprecated because the dependency on the package group used to hold the - * whitelist is not accessible through blaze query. Use {@link Whitelist}. - */ -@Deprecated -public final class FeaturePolicyOptions extends FragmentOptions { - /** The mapping from features to their associated package groups. */ - @Option( - name = "feature_control_policy", - help = - "Policy used to limit the rollout or deprecation of features within the Bazel binary to " - + "specific packages. Pass a mapping from a feature name to the package group used to " - + "control access to that feature, in the form feature=//label/of:package_group (note " - + "that unlike visibility, packages cannot be directly specified a la " - + "//package:__pkg__ or //visibility:public). Can be repeated to specify multiple " - + "features, but each feature must be specified only once.", - valueHelp = "a feature=label pair", - documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, - effectTags = {OptionEffectTag.UNKNOWN}, - converter = PolicyEntryConverter.class, - defaultValue = "n/a (default ignored for allowMultiple)", - allowMultiple = true - ) - public List<PolicyEntry> policies = ImmutableList.<PolicyEntry>of(); - - @Override - public FeaturePolicyOptions getHost(boolean fallback) { - // host options are the same as target options - return (FeaturePolicyOptions) clone(); - } -} diff --git a/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/PolicyEntry.java b/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/PolicyEntry.java deleted file mode 100644 index b871637d17..0000000000 --- a/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/PolicyEntry.java +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright 2017 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.featurecontrol; - -import com.google.auto.value.AutoValue; -import com.google.devtools.build.lib.cmdline.Label; - -/** - * Policy value object encoding the package group which can access a given feature. - * - * @deprecated This is deprecated because the dependency on the package group used to hold the - * whitelist is not accessible through blaze query. Use {@link Whitelist}. - */ -@Deprecated -@AutoValue -public abstract class PolicyEntry { - /** Creates a new PolicyEntry for the given feature and package_group label. */ - public static PolicyEntry create(String feature, Label packageGroupLabel) { - return new AutoValue_PolicyEntry(feature, packageGroupLabel); - } - - /** Gets the feature identifier this policy is for. */ - public abstract String getFeature(); - - /** Gets the label for the package group which controls access to this feature. */ - public abstract Label getPackageGroupLabel(); -} diff --git a/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/PolicyEntryConverter.java b/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/PolicyEntryConverter.java deleted file mode 100644 index ec245313a9..0000000000 --- a/src/main/java/com/google/devtools/build/lib/analysis/featurecontrol/PolicyEntryConverter.java +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright 2017 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.featurecontrol; - -import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.cmdline.LabelSyntaxException; -import com.google.devtools.common.options.Converter; -import com.google.devtools.common.options.OptionsParsingException; - -/** - * A converter which creates a PolicyEntry from a flag value. - * - * @deprecated This is deprecated because the dependency on the package group used to hold the - * whitelist is not accessible through blaze query. Use {@link Whitelist}. - */ -@Deprecated -public final class PolicyEntryConverter implements Converter<PolicyEntry> { - @Override - public PolicyEntry convert(String input) throws OptionsParsingException { - int divider = input.indexOf('='); - if (divider == -1) { - throw new OptionsParsingException( - "value must be of the form feature=label; missing ="); - } - String feature = input.substring(0, divider); - if (feature.isEmpty()) { - throw new OptionsParsingException( - "value must be of the form feature=label; feature cannot be empty"); - } - String label = input.substring(divider + 1); - if (label.isEmpty()) { - throw new OptionsParsingException( - "value must be of the form feature=label; label cannot be empty"); - } - try { - return PolicyEntry.create(feature, Label.parseAbsolute(label)); - } catch (LabelSyntaxException ex) { - throw new OptionsParsingException( - "value must be of the form feature=label; " + ex.getMessage()); - } - } - - @Override - public String getTypeDescription() { - return "a feature=label pair"; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java index 520272edc4..6111fe9757 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java @@ -93,7 +93,6 @@ import com.google.devtools.build.lib.rules.apple.XcodeVersionRule; import com.google.devtools.build.lib.rules.apple.cpp.AppleCcToolchainRule; import com.google.devtools.build.lib.rules.apple.swift.SwiftCommandLineOptions; import com.google.devtools.build.lib.rules.apple.swift.SwiftConfiguration; -import com.google.devtools.build.lib.rules.config.ConfigFeatureFlag; import com.google.devtools.build.lib.rules.config.ConfigFeatureFlagConfiguration; import com.google.devtools.build.lib.rules.config.ConfigRuleClasses; import com.google.devtools.build.lib.rules.config.ConfigSkylarkCommon; @@ -226,9 +225,6 @@ public class BazelRuleClassProvider { } }; - public static final ImmutableSet<String> FEATURE_POLICY_FEATURES = - ImmutableSet.of(ConfigFeatureFlag.POLICY_NAME); - public static final RuleSet PLATFORM_RULES = new RuleSet() { @Override diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidBinaryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidBinaryRule.java index 457f068ade..4028796281 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidBinaryRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BazelAndroidBinaryRule.java @@ -18,7 +18,6 @@ import static com.google.devtools.build.lib.packages.Attribute.attr; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; -import com.google.devtools.build.lib.analysis.featurecontrol.FeaturePolicyConfiguration; import com.google.devtools.build.lib.bazel.rules.cpp.BazelCppRuleClasses; import com.google.devtools.build.lib.bazel.rules.java.BazelJavaRuleClasses; import com.google.devtools.build.lib.packages.BuildType; @@ -45,7 +44,6 @@ public class BazelAndroidBinaryRule implements RuleDefinition { .requiresConfigurationFragments( AndroidConfiguration.class, JavaConfiguration.class, - FeaturePolicyConfiguration.class, CppConfiguration.class) .override( attr("manifest", BuildType.LABEL).mandatory().allowedFileTypes(FileType.of(".xml"))) diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java index 0d63fd156f..66767b8942 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java @@ -21,7 +21,6 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; -import com.google.devtools.build.lib.analysis.featurecontrol.FeaturePolicyConfiguration; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.packages.BuildType; @@ -68,8 +67,15 @@ public abstract class AndroidFeatureFlagSetProvider implements TransitiveInfoPro return ImmutableMap.of(); } - FeaturePolicyConfiguration.checkAvailable( - ruleContext, ConfigFeatureFlag.POLICY_NAME, "the " + FEATURE_FLAG_ATTR + " attribute"); + if (!ConfigFeatureFlag.isAvailable(ruleContext)) { + ruleContext.attributeError( + FEATURE_FLAG_ATTR, + String.format( + "the %s attribute is not available in package '%s'", + FEATURE_FLAG_ATTR, + ruleContext.getLabel().getPackageIdentifier())); + throw new RuleErrorException(); + } Iterable<? extends TransitiveInfoCollection> actualTargets = ruleContext.getPrerequisites(FEATURE_FLAG_ATTR, Mode.TARGET); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java index 4e1fe7ffdc..9f4cbfcc09 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java @@ -54,6 +54,7 @@ import com.google.devtools.build.lib.packages.TriState; import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion; import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidManifestMerger; import com.google.devtools.build.lib.rules.android.AndroidConfiguration.ConfigurationDistinguisher; +import com.google.devtools.build.lib.rules.config.ConfigFeatureFlag; import com.google.devtools.build.lib.rules.config.ConfigFeatureFlagProvider; import com.google.devtools.build.lib.rules.cpp.CppOptions; import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider; @@ -923,6 +924,7 @@ public final class AndroidRuleClasses { .nonconfigurable("defines an aspect of configuration") .mandatoryProviders( ImmutableList.of(ConfigFeatureFlagProvider.id()))) + .add(ConfigFeatureFlag.getWhitelistAttribute(env)) // The resource extractor is used at the binary level to extract java resources from the // deploy jar so that they can be added to the APK. .add( diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/BUILD b/src/main/java/com/google/devtools/build/lib/rules/config/BUILD index 29ecc98b99..647c284106 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/config/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/config/BUILD @@ -19,7 +19,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib:preconditions", "//src/main/java/com/google/devtools/build/lib:skylarkinterface", "//src/main/java/com/google/devtools/build/lib/actions", - "//src/main/java/com/google/devtools/build/lib/analysis/featurecontrol", + "//src/main/java/com/google/devtools/build/lib/analysis/whitelisting", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/common/options", "//third_party:guava", diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java index 72b04dc28c..70fa67d8d1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java +++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java @@ -29,9 +29,12 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory; import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.analysis.RunfilesProvider; -import com.google.devtools.build.lib.analysis.featurecontrol.FeaturePolicyConfiguration; +import com.google.devtools.build.lib.analysis.whitelisting.Whitelist; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.syntax.Printer; import java.util.List; @@ -41,13 +44,36 @@ import java.util.List; */ public class ConfigFeatureFlag implements RuleConfiguredTargetFactory { /** The name of the policy that is used to restrict access to the config_feature_flag rule. */ - public static final String POLICY_NAME = "config_feature_flag"; + private static final String WHITELIST_NAME = "config_feature_flag"; + + /** Constructs a definition for the attribute used to restrict access to config_feature_flag. */ + public static Attribute.Builder<Label> getWhitelistAttribute(RuleDefinitionEnvironment env) { + return Whitelist.getAttributeFromWhitelistName( + WHITELIST_NAME, + env.getToolsLabel("//tools/whitelists/config_feature_flag:config_feature_flag")); + } + + /** + * Returns whether config_feature_flag and related features are available to the current rule. + * + * <p>The current rule must have an attribute defined on it created with {@link + * #getWhitelistAttribute}. + */ + public static boolean isAvailable(RuleContext ruleContext) { + return Whitelist.isAvailable(ruleContext, WHITELIST_NAME); + } @Override public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedException, RuleErrorException { - FeaturePolicyConfiguration.checkAvailable( - ruleContext, POLICY_NAME, "the " + ruleContext.getRuleClassNameForLogging() + " rule"); + if (!ConfigFeatureFlag.isAvailable(ruleContext)) { + ruleContext.ruleError( + String.format( + "the %s rule is not available in package '%s'", + ruleContext.getRuleClassNameForLogging(), + ruleContext.getLabel().getPackageIdentifier())); + throw new RuleErrorException(); + } List<String> specifiedValues = ruleContext.attributes().get("allowed_values", STRING_LIST); ImmutableSet<String> values = ImmutableSet.copyOf(specifiedValues); diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java index a6b30a4baa..587e81e10f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigRuleClasses.java @@ -25,7 +25,6 @@ import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; -import com.google.devtools.build.lib.analysis.featurecontrol.FeaturePolicyConfiguration; import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.syntax.Type; @@ -202,7 +201,7 @@ public class ConfigRuleClasses { .mandatoryProviders( ImmutableList.of(ConfigFeatureFlagProvider.id())) .nonconfigurable(NONCONFIGURABLE_ATTRIBUTE_REASON)) - .requiresConfigurationFragments(FeaturePolicyConfiguration.class) + .add(ConfigFeatureFlag.getWhitelistAttribute(env)) .setIsConfigMatcherForConfigSettingOnly() .setOptionReferenceFunctionForConfigSettingOnly( rule -> @@ -292,9 +291,7 @@ config_setting( public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) { return builder .setUndocumented(/* the feature flag feature has not yet been launched */) - .requiresConfigurationFragments( - ConfigFeatureFlagConfiguration.class, - FeaturePolicyConfiguration.class) + .requiresConfigurationFragments(ConfigFeatureFlagConfiguration.class) .add( attr("allowed_values", STRING_LIST) .mandatory() @@ -305,6 +302,7 @@ config_setting( attr("default_value", STRING) .mandatory() .nonconfigurable(NONCONFIGURABLE_ATTRIBUTE_REASON)) + .add(ConfigFeatureFlag.getWhitelistAttribute(env)) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java index b45e11b2d3..97c5b6beb2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java +++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java @@ -39,7 +39,6 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.config.BuildConfigurationOptionDetails; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.analysis.config.TransitiveOptionDetails; -import com.google.devtools.build.lib.analysis.featurecontrol.FeaturePolicyConfiguration; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.BuildType; @@ -88,11 +87,14 @@ public class ConfigSetting implements RuleConfiguredTargetFactory { ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE, BuildType.LABEL_KEYED_STRING_DICT); - if (!userDefinedFlagSettings.isEmpty()) { - FeaturePolicyConfiguration.checkAvailable( - ruleContext, - ConfigFeatureFlag.POLICY_NAME, - "the " + ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE + " attribute"); + if (!userDefinedFlagSettings.isEmpty() && !ConfigFeatureFlag.isAvailable(ruleContext)) { + ruleContext.attributeError( + ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE, + String.format( + "the %s attribute is not available in package '%s'", + ConfigSettingRule.FLAG_SETTINGS_ATTRIBUTE, + ruleContext.getLabel().getPackageIdentifier())); + throw new RuleErrorException(); } List<? extends TransitiveInfoCollection> flagValues = diff --git a/src/main/java/com/google/devtools/build/lib/rules/core/CoreRules.java b/src/main/java/com/google/devtools/build/lib/rules/core/CoreRules.java index 3cebd9850c..50f1de6ff1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/core/CoreRules.java +++ b/src/main/java/com/google/devtools/build/lib/rules/core/CoreRules.java @@ -14,30 +14,21 @@ package com.google.devtools.build.lib.rules.core; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.analysis.BaseRuleClasses; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider.Builder; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider.RuleSet; -import com.google.devtools.build.lib.analysis.featurecontrol.FeaturePolicyLoader; -import com.google.devtools.build.lib.analysis.featurecontrol.FeaturePolicyOptions; import com.google.devtools.build.lib.analysis.test.TestConfiguration; -import com.google.devtools.build.lib.rules.config.ConfigFeatureFlag; /** A set of basic rules - Bazel won't work correctly without these. */ public final class CoreRules implements RuleSet { public static final CoreRules INSTANCE = new CoreRules(); - public static final ImmutableSet<String> FEATURE_POLICY_FEATURES = - ImmutableSet.of(ConfigFeatureFlag.POLICY_NAME); - private CoreRules() { // Use the static INSTANCE field instead. } @Override public void init(Builder builder) { - builder.addConfigurationOptions(FeaturePolicyOptions.class); - builder.addConfigurationFragment(new FeaturePolicyLoader(FEATURE_POLICY_FEATURES)); builder.addDynamicTransitionMaps(BaseRuleClasses.DYNAMIC_TRANSITIONS_MAP); builder.addConfig(TestConfiguration.TestOptions.class, new TestConfiguration.Loader()); diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index cd0ea1419f..7348bd6a1b 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -39,7 +39,6 @@ filegroup( "//src/test/java/com/google/devtools/build/lib/rules/android:srcs", "//src/test/java/com/google/devtools/build/lib/rules/apple:srcs", "//src/test/java/com/google/devtools/build/lib/rules/config:srcs", - "//src/test/java/com/google/devtools/build/lib/analysis/featurecontrol:srcs", "//src/test/java/com/google/devtools/build/lib/analysis/platform:srcs", "//src/test/java/com/google/devtools/build/lib/analysis/whitelisting:srcs", "//src/test/java/com/google/devtools/build/lib/rules/platform:srcs", @@ -366,7 +365,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib:vfs", "//src/main/java/com/google/devtools/build/lib/actions", - "//src/main/java/com/google/devtools/build/lib/analysis/featurecontrol", "//src/main/java/com/google/devtools/build/lib/query2", "//src/main/java/com/google/devtools/build/lib/query2:query-output", "//src/main/java/com/google/devtools/build/lib/rules/apple", diff --git a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/BUILD deleted file mode 100644 index 2f26a91fd6..0000000000 --- a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/BUILD +++ /dev/null @@ -1,32 +0,0 @@ -# Description: -# Tests for configuration mechanisms for rolling out and deprecating pieces of Bazel functionality. - -licenses(["notice"]) # Apache 2.0 - -filegroup( - name = "srcs", - srcs = glob( - ["**"], - ), - visibility = ["//src/test/java/com/google/devtools/build/lib:__pkg__"], -) - -java_test( - name = "FeatureRestrictionTests", - srcs = glob(["*.java"]), - test_class = "com.google.devtools.build.lib.AllTests", - deps = [ - "//src/main/java/com/google/devtools/build/lib:build-base", - "//src/main/java/com/google/devtools/build/lib:packages-internal", - "//src/main/java/com/google/devtools/build/lib/analysis/featurecontrol", - "//src/main/java/com/google/devtools/build/lib/cmdline", - "//src/main/java/com/google/devtools/common/options", - "//src/test/java/com/google/devtools/build/lib:analysis_testutil", - "//src/test/java/com/google/devtools/build/lib:packages_testutil", - "//src/test/java/com/google/devtools/build/lib:test_runner", - "//third_party:guava", - "//third_party:guava-testlib", - "//third_party:junit4", - "//third_party:truth", - ], -) diff --git a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyConfigurationTest.java deleted file mode 100644 index eaaf438215..0000000000 --- a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyConfigurationTest.java +++ /dev/null @@ -1,121 +0,0 @@ -// Copyright 2017 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.featurecontrol; - -import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.fail; - -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSetMultimap; -import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.build.lib.cmdline.RepositoryName; -import com.google.devtools.build.lib.packages.PackageSpecification; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests for the FeaturePolicyConfiguration. */ -@RunWith(JUnit4.class) -public final class FeaturePolicyConfigurationTest { - - @Test - public void isFeatureEnabledForRule_FalseIfAbsentFromFeatureMap() throws Exception { - FeaturePolicyConfiguration config = - new FeaturePolicyConfiguration( - ImmutableSetMultimap.<String, PackageSpecification>of(), - ImmutableMap.of("newFeature", "//package_group:empty")); - - assertThat(config.isFeatureEnabledForRule("newFeature", Label.parseAbsolute("//:rule"))) - .isFalse(); - } - - @Test - public void isFeatureEnabledForRule_TrueIfMappedToEverything() throws Exception { - FeaturePolicyConfiguration config = - new FeaturePolicyConfiguration( - ImmutableSetMultimap.of("newFeature", PackageSpecification.everything()), - ImmutableMap.of("newFeature", "//...")); - - assertThat(config.isFeatureEnabledForRule("newFeature", Label.parseAbsolute("//:rule"))) - .isTrue(); - } - - @Test - public void isFeatureEnabledForRule_TrueIfInPackageSpecification() throws Exception { - FeaturePolicyConfiguration config = - new FeaturePolicyConfiguration( - ImmutableSetMultimap.of( - "newFeature", - PackageSpecification.fromString(RepositoryName.MAIN, "//allowed/...")), - ImmutableMap.of("newFeature", "//allowed/...")); - - assertThat(config.isFeatureEnabledForRule("newFeature", Label.parseAbsolute("//allowed:rule"))) - .isTrue(); - } - - @Test - public void isFeatureEnabledForRule_FalseIfNotInPackageSpecification() throws Exception { - FeaturePolicyConfiguration config = - new FeaturePolicyConfiguration( - ImmutableSetMultimap.of( - "newFeature", - PackageSpecification.fromString(RepositoryName.MAIN, "//allowed/...")), - ImmutableMap.of("newFeature", "//allowed/...")); - - assertThat( - config.isFeatureEnabledForRule("newFeature", Label.parseAbsolute("//forbidden:rule"))) - .isFalse(); - } - - @Test - public void isFeatureEnabledForRule_FailsIfNotPresentInPolicyList() throws Exception { - FeaturePolicyConfiguration config = - new FeaturePolicyConfiguration( - ImmutableSetMultimap.of("newFeature", PackageSpecification.everything()), - ImmutableMap.<String, String>of()); - - try { - config.isFeatureEnabledForRule("newFeature", Label.parseAbsolute("//:rule")); - fail("Expected an exception."); - } catch (IllegalArgumentException expected) { - assertThat(expected).hasMessageThat().contains("No such feature: newFeature"); - } - } - - @Test - public void getPolicyForFeature_ReturnsValueFromPolicy() throws Exception { - FeaturePolicyConfiguration config = - new FeaturePolicyConfiguration( - ImmutableSetMultimap.of("newFeature", PackageSpecification.everything()), - ImmutableMap.<String, String>of("newFeature", "my policy")); - - assertThat(config.getPolicyForFeature("newFeature")).isEqualTo("my policy"); - } - - @Test - public void getPolicyForFeature_FailsIfNotPresentInPolicyList() throws Exception { - FeaturePolicyConfiguration config = - new FeaturePolicyConfiguration( - ImmutableSetMultimap.of("newFeature", PackageSpecification.everything()), - ImmutableMap.<String, String>of()); - - try { - config.getPolicyForFeature("newFeature"); - fail("Expected an exception."); - } catch (IllegalArgumentException expected) { - assertThat(expected).hasMessageThat().contains("No such feature: newFeature"); - } - } -} diff --git a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyLoaderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyLoaderTest.java deleted file mode 100644 index a28752a0fd..0000000000 --- a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyLoaderTest.java +++ /dev/null @@ -1,322 +0,0 @@ -// Copyright 2017 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.featurecontrol; - -import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.fail; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; -import com.google.devtools.build.lib.analysis.config.BuildOptions; -import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment; -import com.google.devtools.build.lib.analysis.config.FragmentOptions; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; -import com.google.devtools.build.lib.analysis.util.AnalysisTestCase; -import com.google.devtools.build.lib.cmdline.Label; -import java.util.Collection; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests for the FeaturePolicyLoader. */ -@RunWith(JUnit4.class) -public final class FeaturePolicyLoaderTest extends AnalysisTestCase { - - private FeaturePolicyConfiguration getFragmentWithFeatures( - Iterable<String> allowedFeatures, Collection<String> args) throws Exception { - ConfigurationEnvironment env = - new ConfigurationEnvironment.TargetProviderEnvironment( - skyframeExecutor.getPackageManager(), reporter, directories); - BuildOptions options = - BuildOptions.of( - ImmutableList.<Class<? extends FragmentOptions>>of(FeaturePolicyOptions.class), - args.toArray(new String[0])); - return (FeaturePolicyConfiguration) - new FeaturePolicyLoader(allowedFeatures).create(env, options); - } - - @Test - public void specifiedFeaturesGetListedAccessPolicy() throws Exception { - scratch.file( - "policy/BUILD", - "package_group(", - " name='featured',", - " packages=[", - " '//direct',", - " '//recursive/...',", - " ])"); - FeaturePolicyConfiguration fragment = - getFragmentWithFeatures( - ImmutableSet.of("defaultFeature", "policiedFeature"), - ImmutableList.of("--feature_control_policy=policiedFeature=//policy:featured")); - assertThat(fragment.getPolicyForFeature("policiedFeature")).isEqualTo("//policy:featured"); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//:rule"))) - .isFalse(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//arbitrary:rule"))) - .isFalse(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//policy:featured"))) - .isFalse(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//direct:allow"))) - .isTrue(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//direct/child:allow"))) - .isFalse(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//recursive:allow"))) - .isTrue(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//recursive/child:allow"))) - .isTrue(); - } - - @Test - public void resolvesIncludedPackageGroups() throws Exception { - scratch.file( - "policy/BUILD", - "package_group(", - " name='main',", - " packages=['//a'],", - " includes=[':include'])", - "package_group(", - " name='include',", - " packages=['//b'])"); - FeaturePolicyConfiguration fragment = - getFragmentWithFeatures( - ImmutableSet.of("defaultFeature", "policiedFeature"), - ImmutableList.of("--feature_control_policy=policiedFeature=//policy:main")); - assertThat(fragment.getPolicyForFeature("policiedFeature")).isEqualTo("//policy:main"); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//arbitrary:rule"))) - .isFalse(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//a:a"))) - .isTrue(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//b:b"))) - .isTrue(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//c:c"))) - .isFalse(); - } - - @Test - public void resolvesAliasToPolicy() throws Exception { - scratch.file( - "policy/BUILD", - "alias(", - " name='aliased',", - " actual=':featured')", - "package_group(", - " name='featured',", - " packages=[", - " '//direct',", - " '//recursive/...',", - " ])"); - FeaturePolicyConfiguration fragment = - getFragmentWithFeatures( - ImmutableSet.of("defaultFeature", "policiedFeature"), - ImmutableList.of("--feature_control_policy=policiedFeature=//policy:aliased")); - assertThat(fragment.getPolicyForFeature("policiedFeature")).isEqualTo("//policy:aliased"); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//arbitrary:rule"))) - .isFalse(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//policy:aliased"))) - .isFalse(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//policy:featured"))) - .isFalse(); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//direct:allow"))) - .isTrue(); - } - - @Test - public void resolvesAliasToIncludesInPackageGroups() throws Exception { - scratch.file( - "policy/BUILD", - "package_group(", - " name='main',", - " packages=['//a'],", - " includes=[':aliased'])", - "alias(", - " name='aliased',", - " actual=':include')", - "package_group(", - " name='include',", - " packages=['//b'])"); - FeaturePolicyConfiguration fragment = - getFragmentWithFeatures( - ImmutableSet.of("defaultFeature", "policiedFeature"), - ImmutableList.of("--feature_control_policy=policiedFeature=//policy:main")); - assertThat(fragment.getPolicyForFeature("policiedFeature")).isEqualTo("//policy:main"); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//arbitrary:rule"))) - .isFalse(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//a:a"))) - .isTrue(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//b:b"))) - .isTrue(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//c:c"))) - .isFalse(); - } - - @Test - public void allowsCyclesInPackageGroupsAndCoversAllMembersOfCycle() throws Exception { - scratch.file( - "policy/BUILD", - "package_group(", - " name='cycle',", - " packages=['//a'],", - " includes=[':elcyc'])", - "package_group(", - " name='elcyc',", - " packages=['//b'],", - " includes=[':cycle'])"); - FeaturePolicyConfiguration fragment = - getFragmentWithFeatures( - ImmutableSet.of("defaultFeature", "policiedFeature"), - ImmutableList.of("--feature_control_policy=policiedFeature=//policy:cycle")); - assertThat(fragment.getPolicyForFeature("policiedFeature")).isEqualTo("//policy:cycle"); - assertThat( - fragment.isFeatureEnabledForRule( - "policiedFeature", Label.parseAbsolute("//arbitrary:rule"))) - .isFalse(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//a:a"))) - .isTrue(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//b:b"))) - .isTrue(); - assertThat(fragment.isFeatureEnabledForRule("policiedFeature", Label.parseAbsolute("//c:c"))) - .isFalse(); - } - - @Test - public void unspecifiedFeaturesGetUniversalAccessPolicy() throws Exception { - scratch.file("null/BUILD", "package_group(name='null', packages=[])"); - FeaturePolicyConfiguration fragment = - getFragmentWithFeatures( - ImmutableSet.of("defaultFeature", "policiedFeature"), - ImmutableList.of("--feature_control_policy=policiedFeature=//null:null")); - assertThat(fragment.getPolicyForFeature("defaultFeature")).isEqualTo("//..."); - assertThat(fragment.isFeatureEnabledForRule("defaultFeature", Label.parseAbsolute("//:rule"))) - .isTrue(); - assertThat( - fragment.isFeatureEnabledForRule( - "defaultFeature", Label.parseAbsolute("//arbitrary:rule"))) - .isTrue(); - } - - @Test - public void throwsForFeatureWithMultiplePolicyDefinitions() throws Exception { - scratch.file( - "null/BUILD", - "package_group(name='null', packages=[])", - "package_group(name='empty', packages=[])"); - try { - getFragmentWithFeatures( - ImmutableSet.of("duplicateFeature"), - ImmutableList.of( - "--feature_control_policy=duplicateFeature=//null:null", - "--feature_control_policy=duplicateFeature=//null:empty")); - fail("Expected an exception"); - } catch (InvalidConfigurationException expected) { - assertThat(expected).hasMessageThat().contains("Multiple definitions"); - assertThat(expected).hasMessageThat().contains("duplicateFeature"); - } - } - - @Test - public void throwsForFeatureNotSpecifiedInLoader() throws Exception { - scratch.file("null/BUILD", "package_group(name='null', packages=[])"); - try { - getFragmentWithFeatures( - ImmutableSet.of("otherFeature"), - ImmutableList.of("--feature_control_policy=missingFeature=//null:null")); - fail("Expected an exception"); - } catch (InvalidConfigurationException expected) { - assertThat(expected).hasMessageThat().contains("No such feature"); - assertThat(expected).hasMessageThat().contains("missingFeature"); - } - } - - @Test - public void throwsForFeatureWithNonexistentPolicy() throws Exception { - scratch.file("null/BUILD", "package_group(name='null', packages=[])"); - try { - getFragmentWithFeatures( - ImmutableSet.of("brokenFeature"), - ImmutableList.of("--feature_control_policy=brokenFeature=//null:missing")); - fail("Expected an exception"); - } catch (InvalidConfigurationException expected) { - assertThat(expected).hasMessageThat().contains("no such target '//null:missing'"); - } - } - - @Test - public void throwsForFeatureWithPolicyInNonexistentPackage() throws Exception { - try { - getFragmentWithFeatures( - ImmutableSet.of("brokenFeature"), - ImmutableList.of("--feature_control_policy=brokenFeature=//missing:missing")); - fail("Expected an exception"); - } catch (InvalidConfigurationException expected) { - assertThat(expected).hasMessageThat().contains("no such package 'missing'"); - } - } - - @Test - public void throwsForFeatureWithNonPackageGroupPolicy() throws Exception { - scratch.file("policy/BUILD", "filegroup(name='non_package_group')"); - try { - getFragmentWithFeatures( - ImmutableSet.of("brokenFeature"), - ImmutableList.of("--feature_control_policy=brokenFeature=//policy:non_package_group")); - fail("Expected an exception"); - } catch (InvalidConfigurationException expected) { - assertThat(expected) - .hasMessageThat() - .contains( - "//policy:non_package_group is not a package_group in brokenFeature feature policy"); - } - } - - @Test - public void throwsForFeatureWithNonRulePolicy() throws Exception { - scratch.file("policy/BUILD", "exports_files(['not_even_a_rule'])"); - try { - getFragmentWithFeatures( - ImmutableSet.of("brokenFeature"), - ImmutableList.of("--feature_control_policy=brokenFeature=//policy:not_even_a_rule")); - fail("Expected an exception"); - } catch (InvalidConfigurationException expected) { - assertThat(expected) - .hasMessageThat() - .contains( - "//policy:not_even_a_rule is not a package_group in brokenFeature feature policy"); - } - } -} diff --git a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyOptionsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyOptionsTest.java deleted file mode 100644 index 72f17c66d2..0000000000 --- a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/FeaturePolicyOptionsTest.java +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright 2017 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.featurecontrol; - -import static com.google.common.truth.Truth.assertThat; - -import com.google.common.testing.EqualsTester; -import com.google.devtools.common.options.Options; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests for the FeaturePolicyOptions. */ -@RunWith(JUnit4.class) -public final class FeaturePolicyOptionsTest { - - @Test - public void testEquality() throws Exception { - new EqualsTester() - .addEqualityGroup( - Options.getDefaults(FeaturePolicyOptions.class), - Options.getDefaults(FeaturePolicyOptions.class)) - .addEqualityGroup( - Options.parse( - FeaturePolicyOptions.class, - "--feature_control_policy=feature=//test:rest", - "--feature_control_policy=future=//nest:best") - .getOptions()) - .testEquals(); - } - - @Test - public void testHostVersionCopiesPolicies() throws Exception { - FeaturePolicyOptions base = - Options.parse( - FeaturePolicyOptions.class, - "--feature_control_policy=feature=//test:rest", - "--feature_control_policy=future=//nest:best") - .getOptions(); - FeaturePolicyOptions host = base.getHost(false); - FeaturePolicyOptions hostFallback = base.getHost(true); - assertThat(host.policies).containsExactlyElementsIn(base.policies); - assertThat(hostFallback.policies).containsExactlyElementsIn(base.policies); - } -} diff --git a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/PolicyEntryConverterTest.java b/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/PolicyEntryConverterTest.java deleted file mode 100644 index d673450efb..0000000000 --- a/src/test/java/com/google/devtools/build/lib/analysis/featurecontrol/PolicyEntryConverterTest.java +++ /dev/null @@ -1,142 +0,0 @@ -// Copyright 2017 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.featurecontrol; - -import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.fail; - -import com.google.devtools.build.lib.cmdline.Label; -import com.google.devtools.common.options.OptionsParsingException; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests for the policy entry option converter. */ -@RunWith(JUnit4.class) -public final class PolicyEntryConverterTest { - - @Test - public void convert_successfullyConvertsValidPair() throws Exception { - assertThat(new PolicyEntryConverter().convert("jeepers=//creepers:peepers")) - .isEqualTo(PolicyEntry.create("jeepers", Label.parseAbsolute("//creepers:peepers"))); - } - - @Test - public void convert_successfullyConvertsValidLabelWithEqualsSign() throws Exception { - assertThat(new PolicyEntryConverter().convert("slamjam=//whoomp:it=there")) - .isEqualTo(PolicyEntry.create("slamjam", Label.parseAbsolute("//whoomp:it=there"))); - } - - @Test - public void convert_acceptsBarePackageNameAsDefaultTarget() throws Exception { - assertThat(new PolicyEntryConverter().convert("two=//go")) - .isEqualTo(PolicyEntry.create("two", Label.parseAbsolute("//go:go"))); - } - - @Test - public void convert_acceptsRepositoryPrefixedLabel() throws Exception { - assertThat(new PolicyEntryConverter().convert("here=@somewhere//else:where")) - .isEqualTo(PolicyEntry.create("here", Label.parseAbsolute("@somewhere//else:where"))); - } - - @Test - public void convert_acceptsBareRepository() throws Exception { - assertThat(new PolicyEntryConverter().convert("aliens=@space")) - .isEqualTo(PolicyEntry.create("aliens", Label.parseAbsolute("@space//:space"))); - } - - @Test - public void convert_failsToConvertWithoutDivider() throws Exception { - try { - new PolicyEntryConverter().convert("hack//sign:on"); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat().contains("missing ="); - } - } - - @Test - public void convert_failsToConvertLabelAlone() throws Exception { - try { - new PolicyEntryConverter().convert("//plain:label"); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat().contains("missing ="); - } - } - - @Test - public void convert_failsToConvertFeatureIdAlone() throws Exception { - try { - new PolicyEntryConverter().convert("plainFeature"); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat().contains("missing ="); - } - } - - @Test - public void convert_failsToConvertEmptyFeature() throws Exception { - try { - new PolicyEntryConverter().convert("=//R1:C1"); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat().contains("feature cannot be empty"); - } - } - - @Test - public void convert_failsToConvertEmptyLabel() throws Exception { - try { - new PolicyEntryConverter().convert("warp="); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat().contains("label cannot be empty"); - } - } - - @Test - public void convert_failsToConvertInvalidLabel() throws Exception { - try { - new PolicyEntryConverter().convert("wrong=//wrong:wrong//wrong"); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat() - .contains("target names may not contain '//' path separators"); - } - } - - @Test - public void convert_failsToConvertRelativeLabel() throws Exception { - try { - new PolicyEntryConverter().convert("wrong=wrong:wrong"); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat() - .contains("invalid label: wrong:wrong"); - } - } - - @Test - public void convert_failsToConvertFilesystemPathLabel() throws Exception { - try { - new PolicyEntryConverter().convert("wrong=/usr/local/google/tmp/filename.ext"); - fail("Expected an exception."); - } catch (OptionsParsingException expected) { - assertThat(expected).hasMessageThat() - .contains("invalid label: /usr/local/google/tmp/filename.ext"); - } - } -} diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java index ad149eab70..96e6d17f7c 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java @@ -13,8 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.analysis.mock; -import static com.google.devtools.build.lib.rules.core.CoreRules.FEATURE_POLICY_FEATURES; - import com.google.common.base.Functions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -22,7 +20,6 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.PlatformConfigurationLoader; import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory; -import com.google.devtools.build.lib.analysis.featurecontrol.FeaturePolicyLoader; import com.google.devtools.build.lib.analysis.util.AnalysisMock; import com.google.devtools.build.lib.bazel.rules.BazelConfiguration; import com.google.devtools.build.lib.bazel.rules.python.BazelPythonConfiguration; @@ -139,6 +136,18 @@ public final class BazelAnalysisMock extends AnalysisMock { "exports_files(['precompile.py'])", "sh_binary(name='2to3', srcs=['2to3.sh'])"); + // Use an alias package group to allow for modification at the simpler path + config.create( + "/bazel_tools_workspace/tools/whitelists/config_feature_flag/BUILD", + "package_group(", + " name='config_feature_flag',", + " includes=['@//tools/whitelists/config_feature_flag'],", + ")"); + + config.create( + "tools/whitelists/config_feature_flag/BUILD", + "package_group(name='config_feature_flag', packages=['//...'])"); + config.create( "/bazel_tools_workspace/tools/zip/BUILD", "package(default_visibility=['//visibility:public'])", @@ -247,7 +256,6 @@ public final class BazelAnalysisMock extends AnalysisMock { new J2ObjcConfiguration.Loader(), new ProtoConfiguration.Loader(), new ConfigFeatureFlagConfiguration.Loader(), - new FeaturePolicyLoader(FEATURE_POLICY_FEATURES), new AndroidConfiguration.Loader(), new PlatformConfigurationLoader()); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java index 9295234d76..2eba0f6d52 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java @@ -3042,10 +3042,10 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { @Test public void testFeatureFlagPolicyMustContainRuleToUseFeatureFlags() throws Exception { reporter.removeHandler(failFastHandler); // expecting an error - scratch.file( - "policy/BUILD", + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", "package_group(", - " name = 'feature_flag_users',", + " name = 'config_feature_flag',", " packages = ['//flag'])"); scratch.file( "flag/BUILD", @@ -3065,22 +3065,19 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " '//flag:flag': 'right',", " }", ")"); - useConfiguration( - "--experimental_dynamic_configs=notrim", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//java/com/google/android/foo:foo")).isNull(); assertContainsEvent( - "in android_binary rule //java/com/google/android/foo:foo: the feature_flags attribute is " - + "not available in package 'java/com/google/android/foo' according to policy " - + "'//policy:feature_flag_users'"); + "in feature_flags attribute of android_binary rule //java/com/google/android/foo:foo: " + + "the feature_flags attribute is not available in package " + + "'java/com/google/android/foo'"); } @Test public void testFeatureFlagPolicyDoesNotBlockRuleIfInPolicy() throws Exception { - scratch.file( - "policy/BUILD", + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", "package_group(", - " name = 'feature_flag_users',", + " name = 'config_feature_flag',", " packages = ['//flag', '//java/com/google/android/foo'])"); scratch.file( "flag/BUILD", @@ -3100,19 +3097,16 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " '//flag:flag': 'right',", " }", ")"); - useConfiguration( - "--experimental_dynamic_configs=notrim", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//java/com/google/android/foo:foo")).isNotNull(); assertNoEvents(); } @Test public void testFeatureFlagPolicyDoesNotBlockRuleIfFlagValuesNotUsed() throws Exception { - scratch.file( - "policy/BUILD", + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", "package_group(", - " name = 'feature_flag_users',", + " name = 'config_feature_flag',", " packages = ['//flag'])"); scratch.file("flag/BUILD"); scratch.file( @@ -3122,9 +3116,6 @@ public class AndroidBinaryTest extends AndroidBuildViewTestCase { " manifest = 'AndroidManifest.xml',", " srcs = [':FooFlags.java'],", ")"); - useConfiguration( - "--experimental_dynamic_configs=notrim", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//java/com/google/android/foo:foo")).isNotNull(); assertNoEvents(); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java index c56ab884b4..2ee5755c40 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlagTest.java @@ -291,9 +291,9 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase { @Test public void policy_mustContainRulesPackage() throws Exception { reporter.removeHandler(failFastHandler); // expecting an error - scratch.file( - "policy/BUILD", - "package_group(name = 'feature_flag_users', packages = ['//some/other'])"); + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", + "package_group(name = 'config_feature_flag', packages = ['//some/other'])"); scratch.file( "test/BUILD", "config_feature_flag(", @@ -301,20 +301,17 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase { " allowed_values = ['default', 'configured', 'other'],", " default_value = 'default',", ")"); - useConfiguration( - "--experimental_dynamic_configs=on", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//test:flag")).isNull(); assertContainsEvent( "in config_feature_flag rule //test:flag: the config_feature_flag rule is not available in " - + "package 'test' according to policy '//policy:feature_flag_users'"); + + "package 'test'"); } @Test public void policy_doesNotBlockRuleIfInPackageGroup() throws Exception { - scratch.file( - "policy/BUILD", - "package_group(name = 'feature_flag_users', packages = ['//test'])"); + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", + "package_group(name = 'config_feature_flag', packages = ['//test'])"); scratch.file( "test/BUILD", "config_feature_flag(", @@ -322,9 +319,6 @@ public final class ConfigFeatureFlagTest extends SkylarkTestCase { " allowed_values = ['default', 'configured', 'other'],", " default_value = 'default',", ")"); - useConfiguration( - "--experimental_dynamic_configs=on", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//test:flag")).isNotNull(); assertNoEvents(); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java index b5935056a8..c79332b4f3 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java @@ -1126,10 +1126,10 @@ public class ConfigSettingTest extends BuildViewTestCase { @Test public void policyMustContainRuleToUseFlagValues() throws Exception { reporter.removeHandler(failFastHandler); // expecting an error - scratch.file( - "policy/BUILD", + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", "package_group(", - " name = 'feature_flag_users',", + " name = 'config_feature_flag',", " packages = ['//flag'])"); scratch.file( "flag/BUILD", @@ -1147,22 +1147,18 @@ public class ConfigSettingTest extends BuildViewTestCase { " '//flag:flag': 'right',", " },", ")"); - useConfiguration( - "--experimental_dynamic_configs=on", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//test:flag_values_user")).isNull(); assertContainsEvent( - "in config_setting rule //test:flag_values_user: the flag_values attribute is not " - + "available in package 'test' according to policy " - + "'//policy:feature_flag_users'"); + "in flag_values attribute of config_setting rule //test:flag_values_user: the flag_values " + + "attribute is not available in package 'test'"); } @Test public void policyDoesNotBlockRuleIfInPolicy() throws Exception { - scratch.file( - "policy/BUILD", + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", "package_group(", - " name = 'feature_flag_users',", + " name = 'config_feature_flag',", " packages = ['//flag', '//test'])"); scratch.file( "flag/BUILD", @@ -1180,19 +1176,16 @@ public class ConfigSettingTest extends BuildViewTestCase { " '//flag:flag': 'right',", " },", ")"); - useConfiguration( - "--experimental_dynamic_configs=on", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//test:flag_values_user")).isNotNull(); assertNoEvents(); } @Test public void policyDoesNotBlockRuleIfFlagValuesNotUsed() throws Exception { - scratch.file( - "policy/BUILD", + scratch.overwriteFile( + "tools/whitelists/config_feature_flag/BUILD", "package_group(", - " name = 'feature_flag_users',", + " name = 'config_feature_flag',", " packages = ['//flag'])"); scratch.file("flag/BUILD"); scratch.file( @@ -1203,9 +1196,6 @@ public class ConfigSettingTest extends BuildViewTestCase { " 'cpu': 'k8',", " },", ")"); - useConfiguration( - "--experimental_dynamic_configs=on", - "--feature_control_policy=config_feature_flag=//policy:feature_flag_users"); assertThat(getConfiguredTarget("//test:flag_values_user")).isNotNull(); assertNoEvents(); } |