diff options
Diffstat (limited to 'src')
10 files changed, 69 insertions, 40 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AliasProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/AliasProvider.java index 3633fd2190..c704b83ea6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AliasProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AliasProvider.java @@ -66,12 +66,41 @@ public final class AliasProvider implements TransitiveInfoProvider { return aliasChain; } - public static String printLabelWithAliasChain(ConfiguredTargetAndData target) { + /** The way {@link #describeTargetWithAliases(ConfiguredTargetAndData, TargetMode) reports the + * kind of a target. */ + public enum TargetMode { + WITH_KIND, // Specify the kind of the target + WITHOUT_KIND, // Only say "target" + } + + /** + * Prints a nice description of a target. + * + * Also adds the aliases it was reached through, if any. + * + * @param target the target to describe + * @param targetMode how to express the kind of the target + * @return + */ + public static String describeTargetWithAliases( + ConfiguredTargetAndData target, TargetMode targetMode) { + String kind = targetMode == TargetMode.WITH_KIND + ? target.getTarget().getTargetKind() : "target"; AliasProvider aliasProvider = target.getConfiguredTarget().getProvider(AliasProvider.class); - String suffix = aliasProvider == null - ? "" - : " (aliased through '" + Joiner.on("' -> '").join(aliasProvider.getAliasChain()) + "')"; + if (aliasProvider == null) { + return kind + " '" + target.getTarget().getLabel() + "'"; + } + + ImmutableList<Label> aliasChain = aliasProvider.getAliasChain(); + StringBuilder result = new StringBuilder(); + result.append("alias '" + aliasChain.get(0) + "'"); + result.append(" referring to " + kind + " '" + target.getTarget().getLabel() + "'"); + if (aliasChain.size() > 1) { + result.append(" through '" + + Joiner.on("' -> '").join(aliasChain.subList(1, aliasChain.size())) + + "'"); + } - return "'" + target.getTarget().getLabel() + "'" + suffix; + return result.toString(); } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index 6bab955fca..c8c477524f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java @@ -41,6 +41,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; import com.google.devtools.build.lib.actions.ArtifactOwner; import com.google.devtools.build.lib.actions.ArtifactRoot; +import com.google.devtools.build.lib.analysis.AliasProvider.TargetMode; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider.PrerequisiteValidator; import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext; import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildInfoKey; @@ -1679,26 +1680,25 @@ public final class RuleContext extends TargetContext } private String badPrerequisiteMessage( - String targetKind, ConfiguredTargetAndData prerequisite, String reason, boolean isWarning) { - String msgPrefix = targetKind != null ? targetKind + " " : ""; + ConfiguredTargetAndData prerequisite, String reason, boolean isWarning) { + String targetKind = prerequisite.getTarget().getTargetKind(); String msgReason = reason != null ? " (" + reason + ")" : ""; if (isWarning) { return String.format( - "%s%s is unexpected here%s; continuing anyway", - msgPrefix, AliasProvider.printLabelWithAliasChain(prerequisite), + "%s is unexpected here%s; continuing anyway", + AliasProvider.describeTargetWithAliases(prerequisite, TargetMode.WITH_KIND), msgReason); } - return String.format("%s%s is misplaced here%s", - msgPrefix, AliasProvider.printLabelWithAliasChain(prerequisite), msgReason); + return String.format("%s is misplaced here%s", + AliasProvider.describeTargetWithAliases(prerequisite, TargetMode.WITH_KIND), msgReason); } private void reportBadPrerequisite( Attribute attribute, - String targetKind, ConfiguredTargetAndData prerequisite, String reason, boolean isWarning) { - String message = badPrerequisiteMessage(targetKind, prerequisite, reason, isWarning); + String message = badPrerequisiteMessage(prerequisite, reason, isWarning); if (isWarning) { attributeWarning(attribute.getName(), message); } else { @@ -1716,8 +1716,7 @@ public final class RuleContext extends TargetContext String reason = attribute.getValidityPredicate().checkValid(rule, prerequisiteRule); if (reason != null) { - reportBadPrerequisite(attribute, prerequisiteTarget.getTargetKind(), - prerequisite, reason, false); + reportBadPrerequisite(attribute, prerequisite, reason, false); } } @@ -1742,7 +1741,7 @@ public final class RuleContext extends TargetContext } } else { // The file exists but has a bad extension - reportBadPrerequisite(attribute, "file", prerequisite, + reportBadPrerequisite(attribute, prerequisite, "expected " + attribute.getAllowedFileTypesPredicate(), false); } } @@ -1880,7 +1879,6 @@ public final class RuleContext extends TargetContext // but maybe prerequisite provides required providers? do not reject yet. unfulfilledRequirements.add( badPrerequisiteMessage( - prerequisite.getTarget().getTargetKind(), prerequisite, "expected " + attribute.getAllowedRuleClassesPredicate(), false)); @@ -1901,7 +1899,6 @@ public final class RuleContext extends TargetContext Predicate<RuleClass> allowedRuleClasses = attribute.getAllowedRuleClassesPredicate(); reportBadPrerequisite( attribute, - prerequisite.getTarget().getTargetKind(), prerequisite, allowedRuleClasses == Predicates.<RuleClass>alwaysTrue() ? null diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java index bfb346e421..a4f2f3d011 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.bazel.rules; import com.google.devtools.build.lib.analysis.AliasProvider; +import com.google.devtools.build.lib.analysis.AliasProvider.TargetMode; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.packages.Attribute; @@ -56,18 +57,20 @@ public class BazelPrerequisiteValidator if (!context.getConfiguration().checkVisibility()) { errorMessage = String.format( - "Target '%s' violates visibility of target " + "Target '%s' violates visibility of " + "%s. Continuing because --nocheck_visibility is active", - rule.getLabel(), AliasProvider.printLabelWithAliasChain(prerequisite)); + rule.getLabel(), AliasProvider.describeTargetWithAliases(prerequisite, + TargetMode.WITHOUT_KIND)); context.ruleWarning(errorMessage); } else { // Oddly enough, we use reportError rather than ruleError here. errorMessage = String.format( - "Target %s is not visible from target '%s'. Check " + "%s is not visible from target '%s'. Check " + "the visibility declaration of the former target if you think " + "the dependency is legitimate", - AliasProvider.printLabelWithAliasChain(prerequisite), rule.getLabel()); + AliasProvider.describeTargetWithAliases(prerequisite, TargetMode.WITHOUT_KIND), + rule.getLabel()); context.reportError(rule.getLocation(), errorMessage); } // We can always post the visibility error as, regardless of the value of keep going, @@ -91,8 +94,8 @@ public class BazelPrerequisiteValidator + rule.getRuleClass() + " rule " + rule.getLabel() - + ": package group " - + AliasProvider.printLabelWithAliasChain(prerequisite) + + ": " + + AliasProvider.describeTargetWithAliases(prerequisite, TargetMode.WITH_KIND) + " is misplaced here " + "(they are only allowed in the visibility attribute)"); } @@ -116,8 +119,8 @@ public class BazelPrerequisiteValidator String message = "non-test target '" + rule.getLabel() - + "' depends on testonly target " - + AliasProvider.printLabelWithAliasChain(prerequisite) + + "' depends on testonly " + + AliasProvider.describeTargetWithAliases(prerequisite, TargetMode.WITHOUT_KIND) + " and doesn't have testonly attribute set"; if (thisPackage.startsWith("experimental/")) { context.ruleWarning(message); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index 5f37d87b8b..7580624ac6 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -1353,7 +1353,7 @@ public abstract class BuildViewTestCase extends FoundationTestCase { "'" + packageName + ":gvalid' does not produce any " + descriptionPlural, "'" + packageName + ":gmix' does not produce any " + descriptionPlural); assertSrcsValidity(ruleType, packageName + ":invalid", true, - "file '" + packageName + ":a.foo' is misplaced here " + descriptionPluralFile, + "source file '" + packageName + ":a.foo' is misplaced here " + descriptionPluralFile, "'" + packageName + ":ginvalid' does not produce any " + descriptionPlural); assertSrcsValidity(ruleType, packageName + ":mix", true, "'" + packageName + ":a.foo' does not produce any " + descriptionPlural); @@ -1736,7 +1736,7 @@ public abstract class BuildViewTestCase extends FoundationTestCase { protected String getErrorMsgMisplacedFiles(String attrName, String ruleType, String ruleName, String fileName) { - return "in " + attrName + " attribute of " + ruleType + " rule " + ruleName + ": file '" + return "in " + attrName + " attribute of " + ruleType + " rule " + ruleName + ": source file '" + fileName + "' is misplaced here"; } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java index 2b329552df..8f1a37384e 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryIntegrationTest.java @@ -432,7 +432,7 @@ public class SkylarkRepositoryIntegrationTest extends BuildViewTestCase { invalidatePackages(); getConfiguredTarget("//:x"); assertContainsEvent( - "Target '//external:zlib' is not visible from target '//:x'. " + "target '//external:zlib' is not visible from target '//:x'. " + "Check the visibility declaration of the former target if you think the " + "dependency is legitimate"); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/AliasTest.java b/src/test/java/com/google/devtools/build/lib/rules/AliasTest.java index 4784acaed6..6bc52b5315 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/AliasTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/AliasTest.java @@ -78,7 +78,7 @@ public class AliasTest extends BuildViewTestCase { reporter.removeHandler(failFastHandler); getConfiguredTarget("//c:c"); assertContainsEvent( - "Target '//a:a' (aliased through '//b:b') is not visible from target '//c:c'"); + "alias '//b:b' referring to target '//a:a' is not visible from target '//c:c'"); } @Test @@ -94,8 +94,8 @@ public class AliasTest extends BuildViewTestCase { reporter.removeHandler(failFastHandler); getConfiguredTarget("//d:d"); - assertContainsEvent( - "Target '//a:a' (aliased through '//c:c' -> '//b:b') is not visible from target '//d:d'"); + assertContainsEvent("alias '//c:c' referring to target '//a:a' through '//b:b' " + + "is not visible from target '//d:d'"); } @Test @@ -131,7 +131,7 @@ public class AliasTest extends BuildViewTestCase { reporter.removeHandler(failFastHandler); getConfiguredTarget("//a:a"); - assertContainsEvent("filegroup rule '//a:c' (aliased through '//a:b') is misplaced here"); + assertContainsEvent("alias '//a:b' referring to filegroup rule '//a:c' is misplaced here"); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidCommonTest.java index 8c36bdde81..bb33ecb654 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidCommonTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidCommonTest.java @@ -74,7 +74,7 @@ public class AndroidCommonTest extends BuildViewTestCase { "'//java/srcs:gvalid' is misplaced " + descriptionPlural, "'//java/srcs:gmix' does not produce any " + descriptionPlural); assertSrcsValidity(rule, "//java/srcs:invalid", true, - "file '//java/srcs:a.properties' is misplaced here " + descriptionPluralFile, + "source file '//java/srcs:a.properties' is misplaced here " + descriptionPluralFile, "'//java/srcs:ginvalid' does not produce any " + descriptionPlural); assertSrcsValidity(rule, "//java/srcs:mix", true, "'//java/srcs:a.properties' does not produce any " + descriptionPlural); diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java index 2e0f7b0bc8..8c4b7df066 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java @@ -1077,7 +1077,7 @@ public class SkylarkRuleContextTest extends SkylarkTestCase { invalidatePackages(); getConfiguredTarget("//:r"); assertContainsEvent("in label_dict attribute of my_rule rule //:r: " - + "file '//:myfile.cpp' is misplaced here (expected no files)"); + + "source file '//:myfile.cpp' is misplaced here (expected no files)"); } @Test diff --git a/src/test/shell/bazel/external_correctness_test.sh b/src/test/shell/bazel/external_correctness_test.sh index d497aa9e05..b663acc544 100755 --- a/src/test/shell/bazel/external_correctness_test.sh +++ b/src/test/shell/bazel/external_correctness_test.sh @@ -163,11 +163,11 @@ EOF echo "local_repository(name='r', path='$REMOTE')" > WORKSPACE bazel build @r//v:rv >& $TEST_log || fail "Build failed" bazel build @r//a:ra >& $TEST_log && fail "Build succeeded" - expect_log "Target '@r//:fg' is not visible" + expect_log "target '@r//:fg' is not visible" bazel build //a:ma >& $TEST_log && fail "Build succeeded" - expect_log "Target '@r//:fg' is not visible" + expect_log "target '@r//:fg' is not visible" bazel build //v:mv >& $TEST_log && fail "Build succeeded" - expect_log "Target '@r//:fg' is not visible" + expect_log "target '@r//:fg' is not visible" } @@ -235,7 +235,7 @@ EOF bazel build @r//:fg || fail "Build failed" bazel build //:fg >& $TEST_log && fail "Build succeeded" - expect_log "Target '@r//r:fg1' is not visible" + expect_log "target '@r//r:fg1' is not visible" } diff --git a/src/test/shell/bazel/local_repository_test.sh b/src/test/shell/bazel/local_repository_test.sh index 3f6e2dbde1..2e6b763f49 100755 --- a/src/test/shell/bazel/local_repository_test.sh +++ b/src/test/shell/bazel/local_repository_test.sh @@ -798,7 +798,7 @@ EOF bazel build @r//:public >& $TEST_log || fail "failed to build public target" bazel build @r//:private >& $TEST_log && fail "could build private target" - expect_log "Target '//:private' is not visible from target '//external:private'" + expect_log "target '//:private' is not visible from target '//external:private'" } function test_load_in_remote_repository() { |