Report fetch failures also in the BEP

We already have an internal event reporting the failure of a
fetch of an external repository. Make this a BuildEvent so
that it also gets reported in the Build Event Protocol; also,
refer to this event as root cause in case of fetch failures
rather than to any needed file of the external repository
individually.

Fixes bazelbuild#6670.

Change-Id: I4f31940fe0c5c709673a99e5657274e5db922cff
PiperOrigin-RevId: 251846972

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);
+  }
+}

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<BuildEventId> getChildrenEvents() {
+    return ImmutableList.<BuildEventId>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();
+  }
 }

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;
     }
 

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);

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java

@@ -13,19 +13,23 @@
 // 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;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 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<Label, Target> 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;

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(

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.");

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"