aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar Michael Staib <mstaib@google.com>2016-09-16 19:36:49 +0000
committerGravatar Laszlo Csomor <laszlocsomor@google.com>2016-09-19 07:35:07 +0000
commit8618b9d67d20a737908113fa89357ac321a9669b (patch)
treec35e40647b56648f25716aa209525257619ea664
parent1ca32990938c4b126cbe49895b757403e8fbf07a (diff)
Add deprecation support to Bazel.
Bazel has always had a deprecation attribute, but until now it has been a no-op. After this change, Bazel will warn when a target with the deprecated attribute unset depends on one with the deprecated attribute set. Like all other warnings, this warning will only be displayed when it matches the output filter. It is also bypassed if the two targets are in the same package. RELNOTES: The deprecation attribute of all rules now causes warnings to be printed when other targets depend on a target with that attribute set. -- MOS_MIGRATED_REVID=133415232
-rw-r--r--src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java91
-rw-r--r--src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java5
-rw-r--r--src/test/java/com/google/devtools/build/lib/analysis/DeprecationValidatorTest.java129
3 files changed, 223 insertions, 2 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index 42c935e817..3f018d37eb 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -33,13 +33,19 @@ import com.google.devtools.build.lib.analysis.config.DefaultsPackage;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.graph.Digraph;
import com.google.devtools.build.lib.graph.Node;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.NativeAspectClass;
+import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
+import com.google.devtools.build.lib.packages.OutputFile;
+import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClassProvider;
+import com.google.devtools.build.lib.packages.RuleErrorConsumer;
+import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.rules.SkylarkModules;
import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
@@ -47,6 +53,7 @@ import com.google.devtools.build.lib.syntax.Environment;
import com.google.devtools.build.lib.syntax.Environment.Extension;
import com.google.devtools.build.lib.syntax.Environment.Phase;
import com.google.devtools.build.lib.syntax.Mutability;
+import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.common.options.OptionsClassProvider;
import java.lang.reflect.Constructor;
@@ -83,6 +90,90 @@ public class ConfiguredRuleClassProvider implements RuleClassProvider {
RuleContext.Builder contextBuilder, ConfiguredTarget prerequisite, Attribute attribute);
}
+ /** Validator to check for and warn on the deprecation of dependencies. */
+ public static final class DeprecationValidator implements PrerequisiteValidator {
+ /** Checks if the given prerequisite is deprecated and prints a warning if so. */
+ @Override
+ public void validate(
+ RuleContext.Builder contextBuilder, ConfiguredTarget prerequisite, Attribute attribute) {
+ validateDirectPrerequisiteForDeprecation(
+ contextBuilder, contextBuilder.getRule(), prerequisite);
+ }
+
+ /**
+ * Returns whether two packages are considered the same for purposes of deprecation warnings.
+ * Dependencies within the same package do not print deprecation warnings; a package in the
+ * javatests directory may also depend on its corresponding java package without a warning.
+ */
+ public static boolean isSameLogicalPackage(
+ PackageIdentifier thisPackage, PackageIdentifier prerequisitePackage) {
+ if (thisPackage.equals(prerequisitePackage)) {
+ // If the packages are equal, they are the same logical package (and just the same package).
+ return true;
+ }
+ if (!thisPackage.getRepository().equals(prerequisitePackage.getRepository())) {
+ // If the packages are in different repositories, they are not the same logical package.
+ return false;
+ }
+ // If the packages are in the same repository, it's allowed iff this package is the javatests
+ // companion to the prerequisite java package.
+ String thisPackagePath = thisPackage.getPackageFragment().getPathString();
+ String prerequisitePackagePath = prerequisitePackage.getPackageFragment().getPathString();
+ return thisPackagePath.startsWith("javatests/")
+ && prerequisitePackagePath.startsWith("java/")
+ && thisPackagePath.substring("javatests/".length()).equals(
+ prerequisitePackagePath.substring("java/".length()));
+ }
+
+ /** Returns whether a deprecation warning should be printed for the prerequisite described. */
+ private static boolean shouldEmitDeprecationWarningFor(
+ String thisDeprecation, PackageIdentifier thisPackage,
+ String prerequisiteDeprecation, PackageIdentifier prerequisitePackage) {
+ // Don't report deprecation edges from javatests to java or within a package;
+ // otherwise tests of deprecated code generate nuisance warnings.
+ // Don't report deprecation if the current target is also deprecated.
+ return (prerequisiteDeprecation != null
+ && !isSameLogicalPackage(thisPackage, prerequisitePackage)
+ && thisDeprecation == null);
+ }
+
+ /** Checks if the given prerequisite is deprecated and prints a warning if so. */
+ public static void validateDirectPrerequisiteForDeprecation(
+ RuleErrorConsumer errors, Rule rule, ConfiguredTarget prerequisite) {
+ Target prerequisiteTarget = prerequisite.getTarget();
+ Label prerequisiteLabel = prerequisiteTarget.getLabel();
+ PackageIdentifier thatPackage = prerequisiteLabel.getPackageIdentifier();
+ PackageIdentifier thisPackage = rule.getLabel().getPackageIdentifier();
+
+ if (prerequisiteTarget instanceof Rule) {
+ Rule prerequisiteRule = (Rule) prerequisiteTarget;
+ String thisDeprecation =
+ NonconfigurableAttributeMapper.of(rule).get("deprecation", Type.STRING);
+ String thatDeprecation =
+ NonconfigurableAttributeMapper.of(prerequisiteRule).get("deprecation", Type.STRING);
+ if (shouldEmitDeprecationWarningFor(
+ thisDeprecation, thisPackage, thatDeprecation, thatPackage)) {
+ errors.ruleWarning("target '" + rule.getLabel() + "' depends on deprecated target '"
+ + prerequisiteLabel + "': " + thatDeprecation);
+ }
+ }
+
+ if (prerequisiteTarget instanceof OutputFile) {
+ Rule generatingRule = ((OutputFile) prerequisiteTarget).getGeneratingRule();
+ String thisDeprecation =
+ NonconfigurableAttributeMapper.of(rule).get("deprecation", Type.STRING);
+ String thatDeprecation =
+ NonconfigurableAttributeMapper.of(generatingRule).get("deprecation", Type.STRING);
+ if (shouldEmitDeprecationWarningFor(
+ thisDeprecation, thisPackage, thatDeprecation, thatPackage)) {
+ errors.ruleWarning("target '" + rule.getLabel() + "' depends on the output file "
+ + prerequisiteLabel + " of a deprecated rule " + generatingRule.getLabel()
+ + "': " + thatDeprecation);
+ }
+ }
+ }
+ }
+
/**
* Builder for {@link ConfiguredRuleClassProvider}.
*/
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 34cfdaa34e..90b064a929 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
@@ -20,6 +20,7 @@ import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Functions;
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
+import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider.DeprecationValidator;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider.PrerequisiteValidator;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.RuleContext;
@@ -181,6 +182,8 @@ public class BazelRuleClassProvider {
public void validate(RuleContext.Builder context,
ConfiguredTarget prerequisite, Attribute attribute) {
validateDirectPrerequisiteVisibility(context, prerequisite, attribute.getName());
+ DeprecationValidator.validateDirectPrerequisiteForDeprecation(
+ context, context.getRule(), prerequisite);
}
private void validateDirectPrerequisiteVisibility(
@@ -193,8 +196,6 @@ public class BazelRuleClassProvider {
}
Target prerequisiteTarget = prerequisite.getTarget();
Label prerequisiteLabel = prerequisiteTarget.getLabel();
- // We don't check the visibility of late-bound attributes, because it would break some
- // features.
if (!context.getRule().getLabel().getPackageIdentifier().equals(
prerequisite.getTarget().getLabel().getPackageIdentifier())
&& !context.isVisible(prerequisite)) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/DeprecationValidatorTest.java b/src/test/java/com/google/devtools/build/lib/analysis/DeprecationValidatorTest.java
new file mode 100644
index 0000000000..053af1ec6c
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/analysis/DeprecationValidatorTest.java
@@ -0,0 +1,129 @@
+// Copyright 2016 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;
+
+import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
+import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for deprecation warnings in Bazel. */
+@RunWith(JUnit4.class)
+public final class DeprecationValidatorTest extends BuildViewTestCase {
+
+ @Test
+ public void noDeprecationWarningForTopLevelTarget() throws Exception {
+ scratchConfiguredTarget(
+ "a",
+ "a",
+ "filegroup(name='a', deprecation='ignored because this target is on the top level')");
+ assertNoEvents();
+ }
+
+ @Test
+ public void noDeprecationWarningWithinPackage() throws Exception {
+ scratchConfiguredTarget(
+ "a",
+ "a",
+ "filegroup(name='a', srcs=[':b'])",
+ "filegroup(name='b', deprecation='ignored because depending target is in same package')");
+ assertNoEvents();
+ }
+
+ @Test
+ public void noDeprecationWarningForDeprecatedTarget() throws Exception {
+ scratchConfiguredTarget(
+ "b",
+ "b",
+ "filegroup(name='b', deprecation='ignored because depending target is deprecated')");
+ scratchConfiguredTarget(
+ "a",
+ "a",
+ "filegroup(name='a', srcs=['//b:b'], deprecation='ignored for a top level target')");
+ assertNoEvents();
+ }
+
+ @Test
+ public void noDeprecationWarningForJavatestsCompanionOfJavaPackage() throws Exception {
+ scratchConfiguredTarget(
+ "java/a",
+ "b",
+ "filegroup(name='b', deprecation='ignored because depending target is javatests match')");
+ scratchConfiguredTarget("javatests/a", "a", "filegroup(name='a', srcs=['//java/a:b'])");
+ assertNoEvents();
+ }
+
+ @Test
+ public void deprecationWarningForJavaCompanionOfJavatestsPackage() throws Exception {
+ scratchConfiguredTarget(
+ "javatests/a",
+ "b",
+ "filegroup(name='b', deprecation='deprecation warning printed', testonly=0)");
+ checkWarning(
+ "java/a",
+ "a",
+ "target '//java/a:a' depends on deprecated target '//javatests/a:b': "
+ + "deprecation warning printed",
+ "filegroup(name='a', srcs=['//javatests/a:b'])");
+ }
+
+ @Test
+ public void deprecationWarningForDifferentPackage() throws Exception {
+ scratchConfiguredTarget(
+ "b", "b", "filegroup(name='b', deprecation='deprecation warning printed')");
+ checkWarning(
+ "a",
+ "a",
+ "target '//a:a' depends on deprecated target '//b:b': deprecation warning printed",
+ "filegroup(name='a', srcs=['//b:b'])");
+ }
+
+ @Test
+ public void deprecationWarningForSamePackageInDifferentRepository() throws Exception {
+ try (OutputStream output = scratch.resolve("WORKSPACE").getOutputStream(true /* append */)) {
+ output.write(
+ "\nlocal_repository(name = 'r', path = '/r')\n".getBytes(StandardCharsets.UTF_8));
+ }
+ scratch.file("/r/WORKSPACE", "workspace(name = 'r')");
+ scratch.file("/r/a/BUILD", "filegroup(name='b', deprecation='deprecation warning printed')");
+ invalidatePackages();
+ checkWarning(
+ "a",
+ "a",
+ "target '//a:a' depends on deprecated target '@r//a:b': deprecation warning printed",
+ "filegroup(name='a', srcs=['@r//a:b'])");
+ }
+
+ @Test
+ public void deprecationWarningForJavatestsCompanionOfJavaPackageInDifferentRepository()
+ throws Exception {
+ try (OutputStream output = scratch.resolve("WORKSPACE").getOutputStream(true /* append */)) {
+ output.write(
+ "\nlocal_repository(name = 'r', path = '/r')\n".getBytes(StandardCharsets.UTF_8));
+ }
+ scratch.file("/r/WORKSPACE", "workspace(name = 'r')");
+ scratch.file(
+ "/r/java/a/BUILD", "filegroup(name='b', deprecation='deprecation warning printed')");
+ invalidatePackages();
+ checkWarning(
+ "javatests/a",
+ "a",
+ "target '//javatests/a:a' depends on deprecated target '@r//java/a:b': "
+ + "deprecation warning printed",
+ "filegroup(name='a', srcs=['@r//java/a:b'])");
+ }
+}