Avoid using jGit when we know how to download an archive

Only checking for GitHub right now, this could probably be expanded.

Fixes #2147
PiperOrigin-RevId: 152689610

4 files changed, 125 insertions, 18 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
index a8d72cc0ea..4c7388031f 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
@@ -96,12 +96,12 @@ public class BazelRepositoryModule extends BlazeModule {
         ImmutableMap.<String, RepositoryFunction>builder()
             .put(LocalRepositoryRule.NAME, new LocalRepositoryFunction())
             .put(HttpArchiveRule.NAME, new HttpArchiveFunction(httpDownloader))
-            .put(GitRepositoryRule.NAME, new GitRepositoryFunction())
+            .put(GitRepositoryRule.NAME, new GitRepositoryFunction(httpDownloader))
             .put(HttpJarRule.NAME, new HttpJarFunction(httpDownloader))
             .put(HttpFileRule.NAME, new HttpFileFunction(httpDownloader))
             .put(MavenJarRule.NAME, new MavenJarFunction(mavenDownloader))
             .put(NewHttpArchiveRule.NAME, new NewHttpArchiveFunction(httpDownloader))
-            .put(NewGitRepositoryRule.NAME, new NewGitRepositoryFunction())
+            .put(NewGitRepositoryRule.NAME, new NewGitRepositoryFunction(httpDownloader))
             .put(NewLocalRepositoryRule.NAME, new NewLocalRepositoryFunction())
             .put(AndroidSdkRepositoryRule.NAME, new AndroidSdkRepositoryFunction())
             .put(AndroidNdkRepositoryRule.NAME, new AndroidNdkRepositoryFunction())
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/GitCloner.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/GitCloner.java
index 7d5c9490e8..bf2650486e 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/GitCloner.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/GitCloner.java
@@ -15,6 +15,10 @@
 package com.google.devtools.build.lib.bazel.repository;
 
 import com.google.common.base.Ascii;
+import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableList;
+import com.google.common.net.UrlEscapers;
+import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
 import com.google.devtools.build.lib.bazel.repository.downloader.ProxyHelper;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.packages.Rule;
@@ -22,6 +26,7 @@ import com.google.devtools.build.lib.rules.repository.RepositoryFunction.Reposit
 import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
@@ -31,6 +36,8 @@ import java.net.URL;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.Status;
 import org.eclipse.jgit.api.errors.GitAPIException;
@@ -49,6 +56,10 @@ import org.eclipse.jgit.transport.NetRCCredentialsProvider;
  * clones submodules if specified.
  */
 public class GitCloner {
+
+  private static final Pattern GITHUB_URL = Pattern.compile(
+      "(?:git@|https?://)github\\.com[:/](\\w+)/(\\w+)\\.git");
+
   private GitCloner() {
     // Only static methods in this class
   }
@@ -96,7 +107,8 @@ public class GitCloner {
       Rule rule,
       Path outputDirectory,
       ExtendedEventHandler eventHandler,
-      Map<String, String> clientEnvironment)
+      Map<String, String> clientEnvironment,
+      HttpDownloader downloader)
       throws RepositoryFunctionException {
     WorkspaceAttributeMapper mapper = WorkspaceAttributeMapper.of(rule);
     if (mapper.isAttributeValueExplicitlySpecified("commit")
@@ -107,19 +119,20 @@ public class GitCloner {
     }
 
     GitRepositoryDescriptor descriptor;
-    String startingPoint;
     try {
       if (mapper.isAttributeValueExplicitlySpecified("commit")) {
-        startingPoint = mapper.get("commit", Type.STRING);
+        descriptor = GitRepositoryDescriptor.createWithCommit(
+            mapper.get("remote", Type.STRING),
+            mapper.get("commit", Type.STRING),
+            mapper.get("init_submodules", Type.BOOLEAN),
+            outputDirectory);
       } else {
-        startingPoint = "tags/" + mapper.get("tag", Type.STRING);
+        descriptor = GitRepositoryDescriptor.createWithTag(
+            mapper.get("remote", Type.STRING),
+            mapper.get("tag", Type.STRING),
+            mapper.get("init_submodules", Type.BOOLEAN),
+            outputDirectory);
       }
-
-      descriptor = new GitRepositoryDescriptor(
-          mapper.get("remote", Type.STRING),
-          startingPoint,
-          mapper.get("init_submodules", Type.BOOLEAN),
-          outputDirectory);
     } catch (EvalException e) {
       throw new RepositoryFunctionException(e, Transience.PERSISTENT);
     }
@@ -134,6 +147,7 @@ public class GitCloner {
     }
 
     Git git = null;
+    Exception suppressedException = null;
     try {
       if (descriptor.directory.exists()) {
         if (isUpToDate(descriptor)) {
@@ -145,6 +159,18 @@ public class GitCloner {
           throw new RepositoryFunctionException(e, Transience.TRANSIENT);
         }
       }
+
+      if (repositoryLooksTgzable(descriptor.remote)) {
+        Optional<Exception> maybeException = downloadRepositoryAsHttpArchive(
+            descriptor, eventHandler, clientEnvironment, downloader);
+        if (maybeException.isPresent()) {
+          suppressedException = maybeException.get();
+        } else {
+          return new HttpDownloadValue(descriptor.directory);
+        }
+        // Fallthrough to cloning from git.
+      }
+
       git =
           Git.cloneRepository()
               .setURI(descriptor.remote)
@@ -176,13 +202,22 @@ public class GitCloner {
             .call();
       }
     } catch (InvalidRemoteException e) {
+      if (suppressedException != null) {
+        e.addSuppressed(suppressedException);
+      }
       throw new RepositoryFunctionException(
           new IOException("Invalid Git repository URI: " + e.getMessage()), Transience.PERSISTENT);
     } catch (RefNotFoundException | InvalidRefNameException e) {
+      if (suppressedException != null) {
+        e.addSuppressed(suppressedException);
+      }
       throw new RepositoryFunctionException(
           new IOException("Invalid branch, tag, or commit: " + e.getMessage()),
           Transience.PERSISTENT);
     } catch (GitAPIException e) {
+      if (suppressedException != null) {
+        e.addSuppressed(suppressedException);
+      }
       // This is a sad attempt to actually get a useful error message out of jGit, which will bury
       // the actual (useful) cause of the exception under several throws.
       StringBuilder errmsg = new StringBuilder();
@@ -195,6 +230,9 @@ public class GitCloner {
       throw new RepositoryFunctionException(
           new IOException("Error cloning repository: " + errmsg), Transience.PERSISTENT);
     } catch (JGitInternalException e) {
+      if (suppressedException != null) {
+        e.addSuppressed(suppressedException);
+      }
       // This is a lame catch-all for jgit throwing RuntimeExceptions all over the place because,
       // as the docs put it, "a lot of exceptions are so low-level that is is unlikely that the
       // caller of the command can handle them effectively." Thanks, jgit.
@@ -208,15 +246,58 @@ public class GitCloner {
     return new HttpDownloadValue(descriptor.directory);
   }
 
+  private static boolean repositoryLooksTgzable(String remote) {
+    // Only handles GitHub right now.
+    return GITHUB_URL.matcher(remote).matches();
+  }
+
+  private static Optional<Exception> downloadRepositoryAsHttpArchive(
+      GitRepositoryDescriptor descriptor, ExtendedEventHandler eventHandler,
+      Map<String, String> clientEnvironment, HttpDownloader downloader)
+      throws RepositoryFunctionException {
+    Matcher matcher = GITHUB_URL.matcher(descriptor.remote);
+    Preconditions.checkState(
+        matcher.matches(), "Remote should be checked before calling this method");
+    String user = matcher.group(1);
+    String repositoryName = matcher.group(2);
+    String downloadUrl =
+        "https://github.com/"
+            + UrlEscapers.urlPathSegmentEscaper().escape(
+                user + "/" + repositoryName + "/archive/" + descriptor.ref + ".tar.gz");
+    try {
+      FileSystemUtils.createDirectoryAndParents(descriptor.directory);
+      Path tgz = downloader.download(ImmutableList.of(new URL(downloadUrl)), "",
+          Optional.of("tar.gz"), descriptor.directory, eventHandler, clientEnvironment);
+      DecompressorValue.decompress(DecompressorDescriptor.builder()
+          .setArchivePath(tgz)
+          // GitHub puts the contents under a directory called <repo>-<commit>.
+          .setPrefix(repositoryName + "-" + descriptor.ref)
+          .setRepositoryPath(descriptor.directory)
+          .build());
+    } catch (InterruptedException | IOException e) {
+      try {
+        FileSystemUtils.deleteTree(descriptor.directory);
+      } catch (IOException e1) {
+        throw new RepositoryFunctionException(
+            new IOException("Unable to delete " + descriptor.directory + ": " + e1.getMessage()),
+            Transience.TRANSIENT);
+      }
+      return Optional.<Exception>of(e);
+    }
+    return Optional.absent();
+  }
+
   private static final class GitRepositoryDescriptor {
     private final String remote;
     private final String checkout;
     private final boolean initSubmodules;
     private final Path directory;
+    private final String ref;
 
-    public GitRepositoryDescriptor(String remote, String checkout, boolean initSubmodules,
-        Path directory) {
+    private GitRepositoryDescriptor(String remote, String ref, String checkout,
+        boolean initSubmodules, Path directory) {
       this.remote = remote;
+      this.ref = ref;
       this.checkout = checkout;
       this.initSubmodules = initSubmodules;
       this.directory = directory;
@@ -238,14 +319,26 @@ public class GitCloner {
       }
       GitRepositoryDescriptor other = (GitRepositoryDescriptor) obj;
       return Objects.equals(remote, other.remote)
-          && Objects.equals(checkout, other.checkout)
+          && Objects.equals(ref, other.ref)
           && Objects.equals(initSubmodules, other.initSubmodules)
           && Objects.equals(directory, other.directory);
     }
 
     @Override
     public int hashCode() {
-      return Objects.hash(remote, checkout, initSubmodules, directory);
+      return Objects.hash(remote, ref, initSubmodules, directory);
+    }
+
+    static GitRepositoryDescriptor createWithCommit(String remote, String commit,
+        Boolean initSubmodules, Path outputDirectory) {
+      return new GitRepositoryDescriptor(
+          remote, commit, commit, initSubmodules, outputDirectory);
+    }
+
+    static GitRepositoryDescriptor createWithTag(String remote, String tag,
+        Boolean initSubmodules, Path outputDirectory) {
+      return new GitRepositoryDescriptor(
+          remote, tag, "tags/" + tag, initSubmodules, outputDirectory);
     }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/GitRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/GitRepositoryFunction.java
index a15d220fb5..d9bc8ac0be 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/GitRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/GitRepositoryFunction.java
@@ -16,10 +16,12 @@ package com.google.devtools.build.lib.bazel.repository;
 
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
+import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
 import com.google.devtools.build.lib.bazel.rules.workspace.GitRepositoryRule;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
 import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
+import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
@@ -32,6 +34,13 @@ import java.util.Map;
  */
 public class GitRepositoryFunction extends RepositoryFunction {
 
+  protected HttpDownloader downloader;
+
+  public GitRepositoryFunction(HttpDownloader httpDownloader) {
+    Preconditions.checkNotNull(httpDownloader);
+    this.downloader = httpDownloader;
+  }
+
   @Override
   public boolean isLocal(Rule rule) {
     return false;
@@ -42,7 +51,7 @@ public class GitRepositoryFunction extends RepositoryFunction {
       BlazeDirectories directories, Environment env, Map<String, String> markerData)
       throws InterruptedException, RepositoryFunctionException {
     createDirectory(outputDirectory, rule);
-    GitCloner.clone(rule, outputDirectory, env.getListener(), clientEnvironment);
+    GitCloner.clone(rule, outputDirectory, env.getListener(), clientEnvironment, downloader);
     return RepositoryDirectoryValue.builder().setPath(outputDirectory);
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/NewGitRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/NewGitRepositoryFunction.java
index 0355ce80c4..967e26c7a7 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/NewGitRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/NewGitRepositoryFunction.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.bazel.repository;
 
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.rules.repository.NewRepositoryFileHandler;
 import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue;
@@ -26,6 +27,10 @@ import java.util.Map;
  * Clones a Git repository, creates a WORKSPACE file, and adds a BUILD file for it.
  */
 public class NewGitRepositoryFunction extends GitRepositoryFunction {
+  public NewGitRepositoryFunction(HttpDownloader httpDownloader) {
+    super(httpDownloader);
+  }
+
   @Override
   public RepositoryDirectoryValue.Builder fetch(Rule rule, Path outputDirectory,
       BlazeDirectories directories, Environment env, Map<String, String> markerData)
@@ -36,7 +41,7 @@ public class NewGitRepositoryFunction extends GitRepositoryFunction {
     }
 
     createDirectory(outputDirectory, rule);
-    GitCloner.clone(rule, outputDirectory, env.getListener(), clientEnvironment);
+    GitCloner.clone(rule, outputDirectory, env.getListener(), clientEnvironment, downloader);
     fileHandler.finishFile(outputDirectory);
 
     return RepositoryDirectoryValue.builder().setPath(outputDirectory);