Merge pull request #84 from npm/lk/handle-pathspec-error

fix: rethrow git pathspec errors from git operations on hosted services

Author: lukekarrys

lib/git.js

@@ -85,6 +85,9 @@ class GitFetcher extends Fetcher {
   [_resolvedFromHosted] (hosted) {
     return this[_resolvedFromRepo](hosted.https && hosted.https())
       .catch(er => {
+        // Throw early since we know pathspec errors will fail again if retried
+        if (er instanceof git.errors.GitPathspecError)
+          throw er
         const ssh = hosted.sshurl && hosted.sshurl()
         // no fallthrough if we can't fall through or have https auth
         if (!ssh || hosted.auth)
@@ -260,9 +263,11 @@ class GitFetcher extends Fetcher {
   // is present, otherwise ssh if the hosted type provides it
   [_cloneHosted] (ref, tmp) {
     const hosted = this.spec.hosted
-    const https = hosted.https()
     return this[_cloneRepo](hosted.https({ noCommittish: true }), ref, tmp)
       .catch(er => {
+        // Throw early since we know pathspec errors will fail again if retried
+        if (er instanceof git.errors.GitPathspecError)
+          throw er
         const ssh = hosted.sshurl && hosted.sshurl({ noCommittish: true })
         // no fallthrough if we can't fall through or have https auth
         if (!ssh || hosted.auth)

test/git.js

@@ -35,6 +35,7 @@ ghi.localhostssh = {
   protocols: ['git+ssh:'],
   tarballtemplate: () => `${hostedUrl}/repo-HEAD.tgz`,
   sshurltemplate: (h) => `git://127.0.0.1:${gitPort}/${h.user}${h.committish ? `#${h.committish}` : ''}`,
+  httpstemplate: (h) => `git://127.0.0.1:${gitPort}/${h.user}${h.committish ? `#${h.committish}` : ''}`,
   shortcuttemplate: (h) => `localhostssh:${h.user}/${h.project}${h.committish ? `#${h.committish}` : ''}`,
   extract: (url) => {
     const [, user, project] = url.pathname.split('/')
@@ -618,3 +619,16 @@ require('fs').writeFileSync('log', JSON.stringify(data,0,2))
     }, 'does not continue installing once loop is detected')
   }
 })
+
+t.test('missing branch name throws pathspec error', async (t) => {
+  const domains = ['localhostssh', 'localhosthttps', 'localhost']
+
+  for (const domain of domains) {
+    await t.rejects(new GitFetcher(`${domain}:repo/x#this-branch-does-not-exist`, {cache}).resolve(), {
+      constructor: /GitPathspecError/
+    }, domain)
+    await t.rejects(new GitFetcher(`${domain}:repo/x#this-branch-does-not-exist`, {cache}).manifest(), {
+      constructor: /GitPathspecError/
+    }, domain)
+  }
+})