Show path for missing BUILD file also in external repositories Since c578764adcf we report additional path information if a package does not exist because of a missing BUILD file. Ensure we also report this, if the packge is supposed to be in an external repository. Change-Id: Iabfd4fe0deb5e43d7cb974a46bdc328c28762db4 PiperOrigin-RevId: 247003674
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java index 88cfd5e..3fee29d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -357,10 +357,15 @@ if (!packageLookupValue.packageExists()) { switch (packageLookupValue.getErrorReason()) { case NO_BUILD_FILE: + throw new PackageFunctionException( + new BuildFileNotFoundException( + packageId, PackageLookupFunction.explainNoBuildFileValue(packageId, env)), + Transience.PERSISTENT); case DELETED_PACKAGE: case REPOSITORY_NOT_FOUND: - throw new PackageFunctionException(new BuildFileNotFoundException(packageId, - packageLookupValue.getErrorMsg()), Transience.PERSISTENT); + throw new PackageFunctionException( + new BuildFileNotFoundException(packageId, packageLookupValue.getErrorMsg()), + Transience.PERSISTENT); case INVALID_PACKAGE_NAME: throw new PackageFunctionException(new InvalidPackageNameException(packageId, packageLookupValue.getErrorMsg()), Transience.PERSISTENT);
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisFailureReportingTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisFailureReportingTest.java index 0c5938d..50cc9d1 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisFailureReportingTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisFailureReportingTest.java
@@ -90,7 +90,9 @@ assertThat(collector.events.get(topLevel)) .containsExactly( new LoadingFailedCause( - causeLabel, "no such package 'bar': BUILD file not found on package path")); + causeLabel, + "no such package 'bar': BUILD file not found in any of the following directories.\n" + + " - /workspace/bar")); } /**
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java index 67478cd..f221cab 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -1242,9 +1242,9 @@ reporter.removeHandler(failFastHandler); update(defaultFlags().with(Flag.KEEP_GOING), "//foo:foo"); assertContainsEvent( - "every rule of type custom_rule implicitly depends upon the target '//bad2:label', but this" - + " target could not be found because of: no such package 'bad2': BUILD file not found " - + "on package path"); + "every rule of type custom_rule implicitly depends upon the target '//bad2:label', but" + + " this target could not be found because of: no such package 'bad2': BUILD file not" + + " found"); assertContainsEvent( "every rule of type custom_rule implicitly depends upon the target '//bad:label', but this " + "target could not be found because of: Target '//bad:label' contains an error and its"
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/whitelisting/WhitelistTest.java b/src/test/java/com/google/devtools/build/lib/analysis/whitelisting/WhitelistTest.java index a5ecd9e..1a77cfc 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/whitelisting/WhitelistTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/whitelisting/WhitelistTest.java
@@ -99,7 +99,7 @@ "x", "every rule of type rule_with_whitelist implicitly depends upon the target" + " '//whitelist:whitelist', but this target could not be found because of: no such" - + " package 'whitelist': BUILD file not found on package path", + + " package 'whitelist': BUILD file not found", "rule_with_whitelist(name='x')"); }
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java index 09d6605..7333812 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
@@ -789,7 +789,7 @@ assertThat(loadingResult.hasError()).isTrue(); ParsingFailedEvent event = tester.findPostOnce(ParsingFailedEvent.class); assertThat(event.getPattern()).isEqualTo("//does_not_exist"); - assertThat(event.getMessage()).contains("BUILD file not found on package path"); + assertThat(event.getMessage()).contains("BUILD file not found"); } @Test
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java index 5fc8679..a2e42ca 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java
@@ -218,8 +218,7 @@ @Test public void testGetNonexistentPackage() throws Exception { - checkGetPackageFails( - "not-there", "no such package 'not-there': " + "BUILD file not found on package path"); + checkGetPackageFails("not-there", "no such package 'not-there': " + "BUILD file not found"); } @Test @@ -256,19 +255,16 @@ */ @Test public void testRepeatedAttemptsToParseMissingPackage() throws Exception { - checkGetPackageFails( - "missing", "no such package 'missing': " + "BUILD file not found on package path"); + checkGetPackageFails("missing", "no such package 'missing': " + "BUILD file not found"); // Still missing: - checkGetPackageFails( - "missing", "no such package 'missing': " + "BUILD file not found on package path"); + checkGetPackageFails("missing", "no such package 'missing': " + "BUILD file not found"); // Update the BUILD file on disk so "missing" is no longer missing: scratch.file("missing/BUILD", "# an ok build file"); // Still missing: - checkGetPackageFails( - "missing", "no such package 'missing': " + "BUILD file not found on package path"); + checkGetPackageFails("missing", "no such package 'missing': " + "BUILD file not found"); invalidatePackages();
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ConstraintValueLookupUtilTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ConstraintValueLookupUtilTest.java index fe9f0ff..aaf1968 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ConstraintValueLookupUtilTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ConstraintValueLookupUtilTest.java
@@ -125,7 +125,7 @@ .hasErrorEntryForKeyThat(key) .hasExceptionThat() .hasMessageThat() - .contains("no such package 'fake': BUILD file not found on package path"); + .contains("no such package 'fake': BUILD file not found"); } // Calls ConstraintValueLookupUtil.getConstraintValueInfo.
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PlatformLookupUtilTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PlatformLookupUtilTest.java index a851e8b..1ed9020 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PlatformLookupUtilTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PlatformLookupUtilTest.java
@@ -124,7 +124,7 @@ .hasErrorEntryForKeyThat(key) .hasExceptionThat() .hasMessageThat() - .contains("no such package 'fake': BUILD file not found on package path"); + .contains("no such package 'fake': BUILD file not found"); } // Calls PlatformLookupUtil.getPlatformInfo.
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoaderTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoaderTest.java index 69a610b..f4e3e93 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoaderTest.java
@@ -74,7 +74,7 @@ assertThrows(NoSuchPackageException.class, () -> pkgLoader.loadPackage(pkgId)); assertThat(expected) .hasMessageThat() - .isEqualTo("no such package 'nope': BUILD file not found on package path"); + .startsWith("no such package 'nope': BUILD file not found"); assertNoEvents(handler.getEvents()); } @@ -179,9 +179,7 @@ PackageIdentifier pkgId = PackageIdentifier.createInMainRepo(PathFragment.create("foo")); NoSuchPackageException expected = assertThrows(NoSuchPackageException.class, () -> pkgLoader.loadPackage(pkgId)); - assertThat(expected) - .hasMessageThat() - .contains("no such package 'foo': BUILD file not found on package path"); + assertThat(expected).hasMessageThat().contains("no such package 'foo': BUILD file not found"); } protected Path path(String rootRelativePath) {
diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh index ad38511..7bb5c4e 100755 --- a/src/test/shell/bazel/external_integration_test.sh +++ b/src/test/shell/bazel/external_integration_test.sh
@@ -2051,6 +2051,42 @@ expect_log 'path/to/workspace/path/to' } +function test_report_package_external() { + # Verify that a useful error message is shown for a BUILD + # file not found at the expected location in an external repository. + WRKDIR=$(mktemp -d "${TEST_TMPDIR}/testXXXXXX") + cd "${WRKDIR}" + + mkdir -p ext/path/too/deep + echo 'data' > ext/path/too/deep/foo.txt + echo 'exports_files(["deep/foo.txt"])' > ext/path/too/BUILD + tar cvf ext.tar ext + rm -rf ext + + mkdir -p path/to/workspace + cd path/to/workspace + cat > WORKSPACE <<EOF +load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") +http_archive( + name="ext", + urls=["file://${WRKDIR}/ext.tar"], +) +EOF + cat > BUILD <<EOF +genrule( + name = "it", + outs = ["it.txt"], + srcs = ["@ext//path/too/deep:foo.txt"], + cmd = "cp $< $@", +) +EOF + + bazel build //:it > "${TEST_log}" 2>&1 \ + && fail "Expected failure" || : + + expect_log 'BUILD file not found.*path/too/deep' +} + function test_location_reported() { # Verify that some useful information is provided about where # a failing repository definition occurred.
diff --git a/src/test/shell/integration/build_event_stream_test.sh b/src/test/shell/integration/build_event_stream_test.sh index 04bb5e4..90aa862 100755 --- a/src/test/shell/integration/build_event_stream_test.sh +++ b/src/test/shell/integration/build_event_stream_test.sh
@@ -661,7 +661,7 @@ //does/not/exist && fail "build failure expected") || true expect_log_once 'aborted' expect_log_once 'reason: LOADING_FAILURE' - expect_log 'description.*BUILD file not found on package path' + expect_log 'description.*BUILD file not found' expect_not_log 'expanded' expect_log 'last_message: true' expect_log_once '^build_tool_logs' @@ -716,7 +716,7 @@ expect_log_once 'aborted' expect_log_once 'reason: LOADING_FAILURE' # We don't expect an expanded message in this case, since all patterns failed. - expect_log 'description.*BUILD file not found on package path' + expect_log 'description.*BUILD file not found' expect_log 'last_message: true' expect_log_once '^build_tool_logs' } @@ -727,7 +727,7 @@ expect_log_once 'aborted' expect_log_once 'reason: LOADING_FAILURE' expect_log_once '^expanded' - expect_log 'description.*BUILD file not found on package path' + expect_log 'description.*BUILD file not found' expect_log 'last_message: true' expect_log_once '^build_tool_logs' }