For missing implicit dependencies show doc string
If an implicit dependency is not met for a target, and the attribute
introducing that implicit dependency has a doc string, show it
to the user. Most likely, it will contain information on where to
obtain that dependency from. In this way, the rule authors, who
are the only ones having knowledge about best practices using their
rules, can provide the needed information.
Improves on #7165.
Change-Id: I2ece552080aa2bd7581a81b790891d60d943e839
PiperOrigin-RevId: 236701041
diff --git a/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java b/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java
index 1f947e6..acb0352 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java
@@ -19,6 +19,7 @@
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
+import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.cmdline.Label;
@@ -320,19 +321,27 @@
* Target} target did not exist, due to {@link NoSuchThingException} e.
*/
public static String formatMissingEdge(
- @Nullable Target target, Label label, NoSuchThingException e) {
+ @Nullable Target target, Label label, NoSuchThingException e, @Nullable Attribute attr) {
// instanceof returns false if target is null (which is exploited here)
if (target instanceof Rule) {
Rule rule = (Rule) target;
if (isExplicitDependency(rule, label)) {
return String.format("%s and referenced by '%s'", e.getMessage(), target.getLabel());
} else {
+ String additionalInfo = "";
+ if (attr != null && !Strings.isNullOrEmpty(attr.getDoc())) {
+ additionalInfo =
+ String.format(
+ "\nDocumentation for implicit attribute %s of rules of type %s:\n%s",
+ attr.getPublicName(), rule.getRuleClass(), attr.getDoc());
+ }
// N.B. If you see this error message in one of our integration tests during development of
// a change that adds a new implicit dependency when running Blaze, maybe you forgot to add
// a new mock target to the integration test's setup.
- return String.format("every rule of type %s implicitly depends upon the target '%s', but "
- + "this target could not be found because of: %s", rule.getRuleClass(), label,
- e.getMessage());
+ return String.format(
+ "every rule of type %s implicitly depends upon the target '%s', but "
+ + "this target could not be found because of: %s%s",
+ rule.getRuleClass(), label, e.getMessage(), additionalInfo);
}
} else if (target instanceof InputFile) {
return e.getMessage() + " (this is usually caused by a missing package group in the"
@@ -345,4 +354,9 @@
return e.getMessage();
}
}
+
+ public static String formatMissingEdge(
+ @Nullable Target target, Label label, NoSuchThingException e) {
+ return formatMissingEdge(target, label, e, null);
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java
index 7ccdca4..1d7cc27 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java
@@ -77,7 +77,7 @@
"Target '%s' depends on toolchain '%s', which cannot be found: %s'",
from.getLabel(), to, e.getMessage());
} else {
- message = TargetUtils.formatMissingEdge(from, to, e);
+ message = TargetUtils.formatMissingEdge(from, to, e, dependencyKind.getAttribute());
}
env.getListener().handle(Event.error(TargetUtils.getLocationMaybe(from), message));
diff --git a/src/test/shell/integration/BUILD b/src/test/shell/integration/BUILD
index 60d96b4..2e4ae0d 100644
--- a/src/test/shell/integration/BUILD
+++ b/src/test/shell/integration/BUILD
@@ -36,6 +36,16 @@
)
sh_test(
+ name = "implicit_dependency_reporting_test",
+ size = "medium",
+ srcs = ["implicit_dependency_reporting_test.sh"],
+ data = [
+ ":test-deps",
+ "@bazel_tools//tools/bash/runfiles",
+ ],
+)
+
+sh_test(
name = "execution_strategies_test",
size = "medium",
srcs = ["execution_strategies_test.sh"],
diff --git a/src/test/shell/integration/implicit_dependency_reporting_test.sh b/src/test/shell/integration/implicit_dependency_reporting_test.sh
new file mode 100755
index 0000000..842b65d
--- /dev/null
+++ b/src/test/shell/integration/implicit_dependency_reporting_test.sh
@@ -0,0 +1,105 @@
+#!/bin/bash
+#
+# Copyright 2019 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# An end-to-end test that Bazel's experimental UI produces reasonable output.
+
+# --- begin runfiles.bash initialization ---
+set -euo pipefail
+if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
+ if [[ -f "$0.runfiles_manifest" ]]; then
+ export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest"
+ elif [[ -f "$0.runfiles/MANIFEST" ]]; then
+ export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST"
+ elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then
+ export RUNFILES_DIR="$0.runfiles"
+ fi
+fi
+if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then
+ source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash"
+elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
+ source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \
+ "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)"
+else
+ echo >&2 "ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash"
+ exit 1
+fi
+# --- end runfiles.bash initialization ---
+
+source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \
+ || { echo "integration_test_setup.sh not found!" >&2; exit 1; }
+
+case "$(uname -s | tr [:upper:] [:lower:])" in
+msys*|mingw*|cygwin*)
+ declare -r is_windows=true
+ ;;
+*)
+ declare -r is_windows=false
+ ;;
+esac
+
+if "$is_windows"; then
+ export MSYS_NO_PATHCONV=1
+ export MSYS2_ARG_CONV_EXCL="*"
+fi
+
+#### SETUP #############################################################
+
+test_custom_message() {
+ rm -rf custommessage
+ mkdir custommessage
+ cd custommessage
+
+ touch WORKSPACE
+ cat > rule.bzl <<'EOF'
+def _rule_impl(ctx):
+ out = ctx.new_file(ctx.label.name + ".txt")
+ ctx.action(
+ inputs = ctx.files._data,
+ outputs = [out],
+ command = ["cp"] + [f.path for f in ctx.files._data] + [out.path],
+ mnemonic = "copying",
+ progress_message = "Copying implict data dependency for %s" % ctx.label
+ )
+
+implicit_rule = rule(
+ implementation = _rule_impl,
+ attrs = {
+ "_data": attr.label(allow_files=True, default = "//magic/place:data",
+ doc="You must manually put the data to magic/place.")
+ },
+)
+EOF
+ cat > BUILD <<'EOF'
+load("//:rule.bzl", "implicit_rule")
+
+implicit_rule(name = "it")
+EOF
+ mkdir -p magic/place
+ echo Hello World > magic/place/data
+ echo 'exports_files(["data"])' > magic/place/BUILD
+
+ bazel build //:it || fail "Rule should work"
+
+ rm -rf magic
+
+ bazel build //:it > "${TEST_log}" 2>&1 \
+ && fail "Missing implict dependency should be detected" || :
+ expect_log 'rule.*implicit_rule.*implicitly depends'
+ expect_log 'attribute _data of.*implicit_rule'
+ expect_log 'You must manually put the data to magic/place'
+}
+
+run_suite "Integration tests for reporing missing implicit dependencies"
