Skip to content

Commit

Permalink
Improve error message if select references non-existent package
Browse files Browse the repository at this point in the history
If a key in a `select` references a package that does not exist, the error message now includes a reference to the target with the `select`:

```
ERROR: no such package 'hello/c': BUILD file not found in any of the following directories. Add a BUILD file to a directory to mark it as a package.
 - /tmp/bazel-crash/hello/c
ERROR: /tmp/bazel-crash/hello/b/BUILD:1:13: errors encountered resolving select() keys for //hello/b:b
ERROR: Analysis of target '//hello/a:a' failed; build aborted
```

Previously, it didn't:
```
ERROR: Analysis of target '//hello/a:a' failed; build aborted: no such package 'hello/c': BUILD file not found in any of the following directories. Add a BUILD file to a directory to mark it as a package.
 - /tmp/bazel-crash/hello/c
```

Fixes bazelbuild#17689

Closes bazelbuild#18454.

PiperOrigin-RevId: 533635167
Change-Id: I714b1c1c539d9dfb76dea9b6f1899bb98ae558dd
  • Loading branch information
fmeum authored and fweikert committed May 25, 2023
1 parent ef3cb91 commit 0bdee2c
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public ViewCreationFailedException(String message, FailureDetail failureDetail)
}

public ViewCreationFailedException(String message, FailureDetail failureDetail, Throwable cause) {
super(message + ": " + cause.getMessage(), cause);
super(combineMessages(message, cause), cause);
this.failureDetail = checkNotNull(failureDetail);
}

Expand All @@ -44,4 +44,12 @@ public ViewCreationFailedException(FailureDetail failureDetail, Throwable cause)
public FailureDetail getFailureDetail() {
return failureDetail;
}

private static String combineMessages(String message, Throwable cause) {
if (cause.getMessage().isEmpty()) {
return message;
} else {
return message + ": " + cause.getMessage();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@
public class DependencyEvaluationException extends Exception {
/* Null denotes whatever default exit code callers choose. */
@Nullable private final DetailedExitCode detailedExitCode;
private final Location location;
@Nullable private final Location location;
private final boolean depReportedOwnError;

private DependencyEvaluationException(
Exception cause,
@Nullable DetailedExitCode detailedExitCode,
Location location,
@Nullable Location location,
boolean depReportedOwnError) {
super(cause.getMessage(), cause);
this.detailedExitCode = detailedExitCode;
Expand All @@ -68,6 +68,7 @@ public DetailedExitCode getDetailedExitCode() {
return detailedExitCode;
}

@Nullable
public Location getLocation() {
return location;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@
public final class ConfiguredValueCreationException extends Exception
implements SaneAnalysisException {

private final Location location;
@Nullable private final Location location;
@Nullable private final BuildEventId configuration;
private final NestedSet<Cause> rootCauses;
// TODO(b/138456686): if warranted by a need for finer-grained details, replace the constructors
// that specify the general Code.CONFIGURED_VALUE_CREATION_FAILED
private final DetailedExitCode detailedExitCode;

public ConfiguredValueCreationException(
Location location,
@Nullable Location location,
String message,
Label label,
@Nullable BuildEventId configuration,
Expand Down Expand Up @@ -83,6 +83,7 @@ public ConfiguredValueCreationException(TargetAndConfiguration ctgValue, String
this(ctgValue, message, /*rootCauses=*/ null, /*detailedExitCode=*/ null);
}

@Nullable
public Location getLocation() {
return location;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.Aspect;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
import com.google.devtools.build.lib.packages.Package;
Expand Down Expand Up @@ -553,11 +554,21 @@ private static TargetAndConfiguration computeTargetAndConfiguration(
if (env.valuesMissing()) {
return null;
}
PackageValue packageValue = (PackageValue) packageAndMaybeConfigurationValues.get(packageKey);
if (packageValue == null) {
return null;
Package pkg;
try {
pkg =
((PackageValue)
packageAndMaybeConfigurationValues.getOrThrow(
packageKey, NoSuchPackageException.class))
.getPackage();
} catch (NoSuchPackageException e) {
if (!e.getMessage().isEmpty()) {
env.getListener().handle(Event.error(e.getMessage()));
}
throw new ReportedException(
new ConfiguredValueCreationException(
null, e.getMessage(), label, null, null, e.getDetailedExitCode()));
}
Package pkg = packageValue.getPackage();
if (configurationKeyMaybe != null) {
configuration =
(BuildConfigurationValue) packageAndMaybeConfigurationValues.get(configurationKeyMaybe);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1401,4 +1401,26 @@ public void refiningReplacesRemovedEnvironmentWithInvalidFulfillingSubset() thro
assertContainsEvent("//hello:lib: the current command line flags disqualify all supported "
+ "environments because of incompatible select() paths");
}

@Test
public void invalidSelectKeyError() throws Exception {
scratch.file(
"hello/a/BUILD",
"java_library(",
" name = 'a',",
" runtime_deps = ['//hello/b'],",
")");
scratch.file(
"hello/b/BUILD",
"java_library(",
" name = 'b',",
" runtime_deps = select({'//hello/c': []}),",
")");
reporter.removeHandler(failFastHandler);
assertThat(getConfiguredTarget("//hello/a")).isNull();
assertContainsEvent(
"no such package 'hello/c': BUILD file not found in any of the following directories. Add"
+ " a BUILD file to a directory to mark it as a package");
assertContainsEvent("errors encountered resolving select() keys for //hello/b:b");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ public void testConstraintValueLookup_targetDoesNotExist() throws Exception {
.build();
GetConstraintValueInfoKey key = GetConstraintValueInfoKey.create(ImmutableList.of(targetKey));

reporter.removeHandler(failFastHandler);
EvaluationResult<GetConstraintValueInfoValue> result = getConstraintValueInfo(key);

assertThatEvaluationResult(result).hasError();
Expand All @@ -137,8 +138,10 @@ public void testConstraintValueLookup_targetDoesNotExist() throws Exception {
assertThatEvaluationResult(result)
.hasErrorEntryForKeyThat(key)
.hasExceptionThat()
.hasMessageThat()
.contains("no such package 'fake': BUILD file not found");
.hasCauseThat()
.isInstanceOf(ConfiguredValueCreationException.class);

assertContainsEvent("no such package 'fake': BUILD file not found");
}

// Calls ConstraintValueLookupUtil.getConstraintValueInfo.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ public void testToolchainTypeLookup_targetDoesNotExist() throws Exception {
.build();
GetToolchainTypeInfoKey key = GetToolchainTypeInfoKey.create(ImmutableList.of(targetKey));

reporter.removeHandler(failFastHandler);
EvaluationResult<GetToolchainTypeInfoValue> result = getToolchainTypeInfo(key);

assertThatEvaluationResult(result).hasError();
Expand All @@ -160,8 +161,10 @@ public void testToolchainTypeLookup_targetDoesNotExist() throws Exception {
assertThatEvaluationResult(result)
.hasErrorEntryForKeyThat(key)
.hasExceptionThat()
.hasMessageThat()
.contains("no such package 'fake': BUILD file not found");
.hasCauseThat()
.isInstanceOf(ConfiguredValueCreationException.class);

assertContainsEvent("no such package 'fake': BUILD file not found");
}

// Calls ToolchainTypeLookupUtil.getToolchainTypeInfo.
Expand Down

0 comments on commit 0bdee2c

Please sign in to comment.