diff --git a/src/main/java/com/google/devtools/build/lib/packages/RepositoryFetchException.java b/src/main/java/com/google/devtools/build/lib/packages/RepositoryFetchException.java new file mode 100644 index 00000000000000..0aa2fe14107863 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/packages/RepositoryFetchException.java @@ -0,0 +1,31 @@ +// 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. + +package com.google.devtools.build.lib.packages; + +import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import java.io.IOException; + +/** Exception indicating an attempt to access a package which is not found or does not exist. */ +public class RepositoryFetchException extends NoSuchPackageException { + + public RepositoryFetchException(PackageIdentifier packageIdentifier, String message) { + super(packageIdentifier, message); + } + + public RepositoryFetchException( + PackageIdentifier packageIdentifier, String message, IOException cause) { + super(packageIdentifier, message, cause); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/repository/RepositoryFailedEvent.java b/src/main/java/com/google/devtools/build/lib/repository/RepositoryFailedEvent.java index 61aec63feada4d..41112dd9757a26 100644 --- a/src/main/java/com/google/devtools/build/lib/repository/RepositoryFailedEvent.java +++ b/src/main/java/com/google/devtools/build/lib/repository/RepositoryFailedEvent.java @@ -13,20 +13,65 @@ // limitations under the License. package com.google.devtools.build.lib.repository; +import static com.google.devtools.build.lib.cmdline.LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER; + +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.buildeventstream.BuildEvent; +import com.google.devtools.build.lib.buildeventstream.BuildEventContext; +import com.google.devtools.build.lib.buildeventstream.BuildEventId; +import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos; +import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.events.ExtendedEventHandler.ProgressLike; +import java.util.Collection; /** * Event indicating that a failure is related to a given external repository; this is in particular * the case, if fetching that repository failed. */ -public class RepositoryFailedEvent implements ProgressLike { +public class RepositoryFailedEvent implements ProgressLike, BuildEvent { private final String repo; + private final String message; - public RepositoryFailedEvent(String repo) { + public RepositoryFailedEvent(String repo, String message) { this.repo = repo; + this.message = message; } public String getRepo() { return repo; } + + @Override + public BuildEventId getEventId() { + String strippedRepoName = repo; + if (strippedRepoName.startsWith("@")) { + strippedRepoName = strippedRepoName.substring(1); + } + try { + Label label = Label.create(EXTERNAL_PACKAGE_IDENTIFIER, strippedRepoName); + return BuildEventId.unconfiguredLabelId(label); + } catch (LabelSyntaxException e) { + // As the repository name was accepted earlier, the label construction really shouldn't fail. + // In any case, return something still referring to the repository. + return BuildEventId.unknownBuildEventId(EXTERNAL_PACKAGE_IDENTIFIER + ":" + strippedRepoName); + } + } + + @Override + public Collection getChildrenEvents() { + return ImmutableList.of(); + } + + @Override + public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext context) { + return GenericBuildEvent.protoChaining(this) + .setAborted( + BuildEventStreamProtos.Aborted.newBuilder() + .setReason(BuildEventStreamProtos.Aborted.AbortReason.LOADING_FAILURE) + .setDescription(message) + .build()) + .build(); + } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java index f0acce2e66a2bd..7d3ddcfdc47675 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java @@ -284,7 +284,7 @@ private RepositoryDirectoryValue.Builder fetchRepository( } catch (SkyFunctionException e) { // Upon an exceptional exit, the fetching of that repository is over as well. env.getListener().post(new RepositoryFetching(repositoryName, true)); - env.getListener().post(new RepositoryFailedEvent(repositoryName)); + env.getListener().post(new RepositoryFailedEvent(repositoryName, e.getMessage())); throw e; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java index 3c6a84c04e8aaf..080d1bd392eff1 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java @@ -25,6 +25,7 @@ import com.google.devtools.build.lib.packages.BuildFileNotFoundException; import com.google.devtools.build.lib.packages.ErrorDeterminingRepositoryException; import com.google.devtools.build.lib.packages.NoSuchPackageException; +import com.google.devtools.build.lib.packages.RepositoryFetchException; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.vfs.PathFragment; @@ -340,9 +341,12 @@ private PackageLookupValue computeExternalPackageLookupValue( if (repositoryValue == null) { return null; } - } catch (NoSuchPackageException | IOException | EvalException e) { + } catch (NoSuchPackageException e) { throw new PackageLookupFunctionException(new BuildFileNotFoundException(id, e.getMessage()), Transience.PERSISTENT); + } catch (IOException | EvalException e) { + throw new PackageLookupFunctionException( + new RepositoryFetchException(id, e.getMessage()), Transience.PERSISTENT); } if (!repositoryValue.repositoryExists()) { // TODO(ulfjack): Maybe propagate the error message from the repository delegator function? @@ -378,6 +382,10 @@ public PackageLookupFunctionException(BuildFileNotFoundException e, Transience t super(e, transience); } + public PackageLookupFunctionException(RepositoryFetchException e, Transience transience) { + super(e, transience); + } + public PackageLookupFunctionException(InconsistentFilesystemException e, Transience transience) { super(e, transience); 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 9709b26da76c9a..e67fbb58110eca 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 @@ -13,12 +13,15 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; +import static com.google.devtools.build.lib.cmdline.LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER; + import com.google.common.collect.Maps; import com.google.devtools.build.lib.analysis.DependencyResolver; import com.google.devtools.build.lib.analysis.TargetAndConfiguration; import com.google.devtools.build.lib.causes.Cause; import com.google.devtools.build.lib.causes.LoadingFailedCause; 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.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.events.Event; @@ -26,6 +29,7 @@ import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.packages.NoSuchThingException; import com.google.devtools.build.lib.packages.Package; +import com.google.devtools.build.lib.packages.RepositoryFetchException; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.util.OrderedSetMultimap; @@ -119,6 +123,32 @@ protected Map getTargets( continue; } } catch (NoSuchPackageException e) { + if (e instanceof RepositoryFetchException) { + Label repositoryLabel; + try { + repositoryLabel = + Label.create( + EXTERNAL_PACKAGE_IDENTIFIER, + label.getPackageIdentifier().getRepository().strippedName()); + } catch (LabelSyntaxException lse) { + // We're taking the repository name from something that was already + // part of a label, so it should be valid. If we really get into this + // strange we situation, better not try to be smart and report the original + // label. + repositoryLabel = label; + } + rootCauses.add(new LoadingFailedCause(repositoryLabel, e.getMessage())); + env.getListener() + .handle( + Event.error( + TargetUtils.getLocationMaybe(fromTarget), + String.format( + "%s depends on %s in repository %s which failed to fetch", + fromTarget.getLabel(), + label, + label.getPackageIdentifier().getRepository()))); + continue; + } rootCauses.add(new LoadingFailedCause(label, e.getMessage())); missingEdgeHook(fromTarget, entry.getKey(), label, e); continue; diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryTest.java index 4bfc3cc0dcbdcd..9f8d57fdeaeacd 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryTest.java @@ -22,7 +22,7 @@ import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.packages.AttributeContainer; -import com.google.devtools.build.lib.packages.BuildFileNotFoundException; +import com.google.devtools.build.lib.packages.RepositoryFetchException; import com.google.devtools.build.lib.packages.util.BazelMockCcSupport; import com.google.devtools.build.lib.packages.util.ResourceLoader; import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; @@ -206,8 +206,8 @@ public void testMissingPlatformsDirectory() throws Exception { " path = '/ndk',", ")"); invalidatePackages(false); - BuildFileNotFoundException e = - assertThrows(BuildFileNotFoundException.class, () -> getTarget("@androidndk//:files")); + RepositoryFetchException e = + assertThrows(RepositoryFetchException.class, () -> getTarget("@androidndk//:files")); assertThat(e) .hasMessageThat() .contains( diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryTest.java index 5b6dd7817e5499..cdce62a305887e 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryTest.java @@ -20,7 +20,7 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; -import com.google.devtools.build.lib.packages.BuildFileNotFoundException; +import com.google.devtools.build.lib.packages.RepositoryFetchException; import com.google.devtools.build.lib.packages.util.ResourceLoader; import com.google.devtools.build.lib.rules.android.AndroidSdkProvider; import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; @@ -277,7 +277,7 @@ public void testMissingApiLevel() throws Exception { try { getTarget("@androidsdk//:files"); fail("android_sdk_repository should have failed due to missing SDK api level."); - } catch (BuildFileNotFoundException e) { + } catch (RepositoryFetchException e) { assertThat(e.getMessage()) .contains( "Android SDK api level 25 was requested but it is not installed in the Android SDK " @@ -318,7 +318,7 @@ public void testMissingPlatformsDirectory() throws Exception { try { getTarget("@androidsdk//:files"); fail("android_sdk_repository should have failed due to missing SDK platforms dir."); - } catch (BuildFileNotFoundException e) { + } catch (RepositoryFetchException e) { assertThat(e.getMessage()) .contains("Expected directory at /sdk/platforms but it is not a directory or it does " + "not exist."); @@ -340,7 +340,7 @@ public void testMissingBuildToolsDirectory() throws Exception { try { getTarget("@androidsdk//:files"); fail("android_sdk_repository should have failed due to missing SDK build tools dir."); - } catch (BuildFileNotFoundException e) { + } catch (RepositoryFetchException e) { assertThat(e.getMessage()) .contains("Expected directory at /sdk/build-tools but it is not a directory or it does " + "not exist."); diff --git a/src/test/shell/bazel/bazel_build_event_stream_test.sh b/src/test/shell/bazel/bazel_build_event_stream_test.sh index b6c9c7fcef6048..266f4baffc31e4 100755 --- a/src/test/shell/bazel/bazel_build_event_stream_test.sh +++ b/src/test/shell/bazel/bazel_build_event_stream_test.sh @@ -150,4 +150,44 @@ function test_query() { expect_log 'last_message: true' } +function test_fetch_failure() { + # We expect that if a build is failing due to an error fetching an external + # repository, we get a reasonable attribution of the root cause. + cat > WORKSPACE <<'EOF' +load("//:failing_repo.bzl", "failing") + +failing(name="remote") +EOF + touch BUILD + cat > failing_repo.bzl <<'EOF' +def _impl(ctx): + fail("This is the error message") + +failing = repository_rule( + implementation = _impl, + attrs = {}, +) +EOF + cat > pkg/BUILD <<'EOF' +genrule( + name="main", + srcs=["@remote//file"], + outs = ["main.out"], + cmd = "cp $< $@", +) +genrule( + name="main2", + srcs=["@remote//file2"], + outs = ["main2.out"], + cmd = "cp $< $@", +) +EOF + bazel build -k --build_event_text_file=$TEST_log //pkg:main //pkg:main2 \ + && fail "expected failure" || : + + expect_log 'label: "//external:remote"' + expect_log 'description:.*This is the error message' + expect_not_log 'label.*@remote//file' +} + run_suite "Bazel-specific integration tests for the build-event stream"