fix(fetch, push): recognize more git config settings (#1033)

billiegoose · web-flow · commit a4e5aeeb9ecc · 2020-02-08T16:03:19.000-05:00
BREAKING CHANGE: the `fetch` and `push` functions now respect previously ignored git config settings like: `branch.${ref}.pushRemote`, `remote.pushDefault`, `remote.${remote}.pushurl`, and `branch.${ref}.merge`. (Note the `pull` command already respected `branch.${ref}.merge` but `fetch` did not.)
diff --git a/README.md b/README.md
@@ -69,7 +69,7 @@ At the time of writing, the following breaking changes are planned:
 - [x] Remove the `noOverwrite` option from `init` and make that the new behavior.
 - [ ] Oh, and I should move all the error strings into a separate JSON file that can be fetched lazily in production.
 - [ ] I should probably remove `username`, `password`, `token`, and `oauth2format` and make everyone use `onAuth` callback for that.
-- [ ] Fix `push` to use the remote tracking branch by default for `remtoeRef`
+- [x] Fix `push` to use the remote tracking branch by default for `remtoeRef`
 - [x] Auto-generate `docs/alphabetic.md` and `README` list from filenames in `src/api`.
 
 ## Getting Started
@@ -174,7 +174,7 @@ unless there is a major version bump.
 <!-- API-LIST:START - Do not remove or modify this section -->
 <!-- prettier-ignore-start -->
 <!-- markdownlint-disable -->
-    
+
 <!-- autogenerated_by: __tests__/__helpers__/generate-docs.js -->
 
 - [add](https://isomorphic-git.github.io/docs/add.html)
diff --git a/src/api/clone.js b/src/api/clone.js
@@ -9,8 +9,6 @@ import { join } from '../utils/join.js'
 /**
  * Clone a repository
  *
- * To monitor progress events, see the documentation for the [`'emitter'` plugin](./plugin_emitter.md).
- *
  * @param {object} args
  * @param {FsClient} args.fs - a file system implementation
  * @param {HttpClient} args.http - an HTTP client
diff --git a/src/api/fetch.js b/src/api/fetch.js
@@ -21,10 +21,6 @@ import { join } from '../utils/join.js'
 /**
  * Fetch commits from a remote repository
  *
- * Future versions of isomorphic-git might return additional metadata.
- *
- * To monitor progress events, see the documentation for the [`'emitter'` plugin](./plugin_emitter.md).
- *
  * @param {object} args
  * @param {FsClient} args.fs - a file system client
  * @param {HttpClient} args.http - an HTTP client
@@ -35,24 +31,25 @@ import { join } from '../utils/join.js'
  * @param {AuthFailureCallback} [args.onAuthFailure] - optional auth rejected callback
  * @param {string} [args.dir] - The [working tree](dir-vs-gitdir.md) directory path
  * @param {string} [args.gitdir=join(dir,'.git')] - [required] The [git directory](dir-vs-gitdir.md) path
- * @param {string} [args.url] - The URL of the remote repository. Will be gotten from gitconfig if absent.
- * @param {string} [args.corsProxy] - Optional [CORS proxy](https://www.npmjs.com/%40isomorphic-git/cors-proxy). Overrides value in repo config.
- * @param {string} [args.ref = 'HEAD'] - Which branch to fetch. By default this is the currently checked out branch.
+ * @param {string} [args.ref] - Which branch to fetch. By default this is the currently checked out branch.
+ * @param {string} [args.url] - The URL of the remote repository. The default is the value set in the git config for that remote.
+ * @param {string} [args.remote] - If URL is not specified, determines which remote to use.
+ * @param {string} [args.remoteRef] - The name of the branch on the remote to fetch. By default this is the configured remote tracking branch.
  * @param {boolean} [args.singleBranch = false] - Instead of the default behavior of fetching all the branches, only fetch a single branch.
- * @param {boolean} [args.noGitSuffix = false] - If true, clone will not auto-append a `.git` suffix to the `url`. (**AWS CodeCommit needs this option**)
  * @param {boolean} [args.tags = false] - Also fetch tags
- * @param {string} [args.remote] - What to name the remote that is created.
  * @param {number} [args.depth] - Integer. Determines how much of the git repository's history to retrieve
+ * @param {boolean} [args.relative = false] - Changes the meaning of `depth` to be measured from the current shallow depth rather than from the branch tip.
  * @param {Date} [args.since] - Only fetch commits created after the given date. Mutually exclusive with `depth`.
  * @param {string[]} [args.exclude = []] - A list of branches or tags. Instructs the remote server not to send us any commits reachable from these refs.
- * @param {boolean} [args.relative = false] - Changes the meaning of `depth` to be measured from the current shallow depth rather than from the branch tip.
+ * @param {boolean} [args.prune] - Delete local remote-tracking branches that are not present on the remote
+ * @param {boolean} [args.pruneTags] - Prune local tags that don’t exist on the remote, and force-update those tags that differ
+ * @param {boolean} [args.noGitSuffix = false] - If true, clone will not auto-append a `.git` suffix to the `url`. (**AWS CodeCommit needs this option**)
+ * @param {string} [args.corsProxy] - Optional [CORS proxy](https://www.npmjs.com/%40isomorphic-git/cors-proxy). Overrides value in repo config.
  * @param {string} [args.username] - See the [Authentication](./authentication.html) documentation
  * @param {string} [args.password] - See the [Authentication](./authentication.html) documentation
  * @param {string} [args.token] - See the [Authentication](./authentication.html) documentation
  * @param {'github' | 'bitbucket' | 'gitlab'} [args.oauth2format] - See the [Authentication](./authentication.html) documentation
  * @param {Object<string, string>} [args.headers] - Additional headers to include in HTTP requests, similar to git's `extraHeader` config
- * @param {boolean} [args.prune] - Delete local remote-tracking branches that are not present on the remote
- * @param {boolean} [args.pruneTags] - Prune local tags that don’t exist on the remote, and force-update those tags that differ
  *
  * @returns {Promise<FetchResult>} Resolves successfully when fetch completes
  * @see FetchResult
@@ -81,9 +78,8 @@ export async function fetch ({
   dir,
   gitdir = join(dir, '.git'),
   ref = 'HEAD',
-  // @ts-ignore
-  refs = [ref],
   remote,
+  remoteRef,
   url,
   noGitSuffix = false,
   corsProxy,
@@ -125,8 +121,8 @@ export async function fetch ({
       onAuthFailure,
       gitdir,
       ref,
-      refs,
       remote,
+      remoteRef,
       url,
       noGitSuffix,
       corsProxy,
diff --git a/src/api/pull.js b/src/api/pull.js
@@ -10,9 +10,7 @@ import { normalizeAuthorObject } from '../utils/normalizeAuthorObject.js'
 import { normalizeCommitterObject } from '../utils/normalizeCommitterObject.js'
 
 /**
- * Fetch and merge commits from a remote repository *(Currently, only fast-forward merges are implemented.)*
- *
- * To monitor progress events, see the documentation for the [`'emitter'` plugin](./plugin_emitter.md).
+ * Fetch and merge commits from a remote repository
  *
  * @param {object} args
  * @param {FsClient} args.fs - a file system client
diff --git a/src/api/push.js b/src/api/push.js
@@ -18,8 +18,6 @@ import { join } from '../utils/join.js'
 /**
  * Push a branch or tag
  *
- * > *Note:* The behavior of `remoteRef` is reasonable but not the _correct_ behavior. It _should_ be using the configured remote tracking branch! TODO: I need to fix this
- *
  * The push command returns an object that describes the result of the attempted push operation.
  * *Notes:* If there were no errors, then there will be no `errors` property. There can be a mix of `ok` messages and `errors` messages.
  *
@@ -28,8 +26,6 @@ import { join } from '../utils/join.js'
  * | ok     | Array\<string\>  | The first item is "unpack" if the overall operation was successful. The remaining items are the names of refs that were updated successfully.                                                                    |
  * | errors | Array\<string\>  | If the overall operation threw and error, the first item will be "unpack {Overall error message}". The remaining items are individual refs that failed to be updated in the format "{ref name} {error message}". |
  *
- * To monitor progress events, see the documentation for the [`'emitter'` plugin](./plugin_emitter.md).
- *
  * @param {object} args
  * @param {FsClient} args.fs - a file system client
  * @param {HttpClient} args.http - an HTTP client
@@ -41,12 +37,12 @@ import { join } from '../utils/join.js'
  * @param {string} [args.dir] - The [working tree](dir-vs-gitdir.md) directory path
  * @param {string} [args.gitdir=join(dir,'.git')] - [required] The [git directory](dir-vs-gitdir.md) path
  * @param {string} [args.ref] - Which branch to push. By default this is the currently checked out branch.
- * @param {string} [args.remoteRef] - The name of the receiving branch on the remote. By default this is the same as `ref`. (See note below)
+ * @param {string} [args.url] - The URL of the remote repository. The default is the value set in the git config for that remote.
  * @param {string} [args.remote] - If URL is not specified, determines which remote to use.
+ * @param {string} [args.remoteRef] - The name of the receiving branch on the remote. By default this is the configured remote tracking branch.
  * @param {boolean} [args.force = false] - If true, behaves the same as `git push --force`
  * @param {boolean} [args.delete = false] - If true, delete the remote ref
  * @param {boolean} [args.noGitSuffix = false] - If true, do not auto-append a `.git` suffix to the `url`. (**AWS CodeCommit needs this option**)
- * @param {string} [args.url] - The URL of the remote git server. The default is the value set in the git config for that remote.
  * @param {string} [args.corsProxy] - Optional [CORS proxy](https://www.npmjs.com/%40isomorphic-git/cors-proxy). Overrides value in repo config.
  * @param {string} [args.username] - See the [Authentication](./authentication.html) documentation
  * @param {string} [args.password] - See the [Authentication](./authentication.html) documentation
diff --git a/src/commands/fetch.js b/src/commands/fetch.js
@@ -1,5 +1,8 @@
 // @ts-check
-import { getConfig } from '../commands/getConfig.js'
+import '../commands/typedefs.js'
+
+import { currentBranch } from '../commands/currentBranch.js'
+import { GitConfigManager } from '../managers/GitConfigManager.js'
 import { GitRefManager } from '../managers/GitRefManager.js'
 import { GitRemoteManager } from '../managers/GitRemoteManager.js'
 import { GitShallowManager } from '../managers/GitShallowManager.js'
@@ -42,12 +45,12 @@ import { writeUploadPackRequest } from '../wire/writeUploadPackRequest.js'
  * @param {string} args.gitdir
  * @param {string|void} [args.url]
  * @param {string} [args.corsProxy]
- * @param {string} [args.ref = 'HEAD']
- * @param {string[]} [args.refs = [ref]]
+ * @param {string} [args.ref]
+ * @param {string} [args.remoteRef]
+ * @param {string} [args.remote]
  * @param {boolean} [args.singleBranch = false]
  * @param {boolean} [args.noGitSuffix = false]
  * @param {boolean} [args.tags = false]
- * @param {string} [args.remote]
  * @param {number} [args.depth]
  * @param {Date} [args.since]
  * @param {string[]} [args.exclude = []]
@@ -72,10 +75,10 @@ export async function fetch ({
   onAuthSuccess,
   onAuthFailure,
   gitdir,
-  ref = 'HEAD',
-  refs = [ref],
-  remote,
-  url,
+  ref: _ref,
+  remoteRef: _remoteRef,
+  remote: _remote,
+  url: _url,
   noGitSuffix = false,
   corsProxy,
   username,
@@ -92,33 +95,31 @@ export async function fetch ({
   prune = false,
   pruneTags = false
 }) {
-  // Lookup remote tracking branch.
-  remote =
-    remote ||
-    (await getConfig({
-      fs,
-      gitdir,
-      path: `branch.${ref}.remote`
-    })) ||
-    'origin'
-  // Set missing values
+  const ref = _ref || (await currentBranch({ fs, gitdir }))
+  if (typeof ref === 'undefined') {
+    throw new GitError(E.MissingRequiredParameterError, {
+      parameter: 'ref'
+    })
+  }
+  const config = await GitConfigManager.get({ fs, gitdir })
+  // Figure out what remote to use.
+  const remote =
+    _remote || (await config.get(`branch.${ref}.remote`)) || 'origin'
+  // Lookup the URL for the given remote.
+  const url = _url || (await config.get(`remote.${remote}.url`))
   if (typeof url === 'undefined') {
-    url = await getConfig({
-      fs,
-      gitdir,
-      path: `remote.${remote}.url`
+    throw new GitError(E.MissingRequiredParameterError, {
+      parameter: 'remote OR url'
     })
-    // TODO: throw better error
-    if (typeof url === 'undefined') {
-      throw new GitError(E.MissingRequiredParameterError, {
-        parameter: 'remote OR url'
-      })
-    }
   }
+  // Figure out what remote ref to use.
+  const remoteRef =
+    _remoteRef || (await config.get(`branch.${ref}.merge`)) || ref
 
   if (corsProxy === void 0) {
-    corsProxy = await getConfig({ fs, gitdir, path: 'http.corsProxy' })
+    corsProxy = await config.get('http.corsProxy')
   }
+
   let auth = { username, password, token, oauth2format }
   const GitRemoteHTTP = GitRemoteManager.getRemoteHelperFor({ url })
   const remoteHTTP = await GitRemoteHTTP.discover({
@@ -158,7 +159,7 @@ export async function fetch ({
   }
   // Figure out the SHA for the requested ref
   const { oid, fullref } = GitRefManager.resolveAgainstMap({
-    ref,
+    ref: remoteRef,
     map: remoteRefs
   })
   // Filter out refs we want to ignore: only keep ref we're cloning, HEAD, branches, and tags (if we're keeping them)
@@ -195,7 +196,7 @@ export async function fetch ({
   // Come up with a reasonable list of oids to tell the remote we already have
   // (preferably oids that are close ancestors of the branch heads we're fetching)
   const haveRefs = singleBranch
-    ? refs
+    ? [ref]
     : await GitRefManager.listRefs({
       fs,
       gitdir,
diff --git a/src/commands/push.js b/src/commands/push.js
@@ -1,12 +1,13 @@
 // @ts-check
 import '../commands/typedefs.js'
 
+import { currentBranch } from '../commands/currentBranch.js'
 import { findMergeBase } from '../commands/findMergeBase.js'
-import { getConfig } from '../commands/getConfig.js'
 import { isDescendent } from '../commands/isDescendent.js'
 import { listCommitsAndTags } from '../commands/listCommitsAndTags.js'
 import { listObjects } from '../commands/listObjects.js'
 import { pack } from '../commands/pack.js'
+import { GitConfigManager } from '../managers/GitConfigManager.js'
 import { GitRefManager } from '../managers/GitRefManager.js'
 import { GitRemoteManager } from '../managers/GitRemoteManager.js'
 import { E, GitError } from '../models/GitError.js'
@@ -73,10 +74,10 @@ export async function push ({
   onAuthSuccess,
   onAuthFailure,
   gitdir,
-  ref,
-  remoteRef,
-  remote = 'origin',
-  url,
+  ref: _ref,
+  remoteRef: _remoteRef,
+  remote,
+  url: _url,
   force = false,
   delete: _delete = false,
   noGitSuffix = false,
@@ -87,25 +88,43 @@ export async function push ({
   oauth2format,
   headers = {}
 }) {
-  // TODO: Figure out how pushing tags works. (This only works for branches.)
-  if (url === undefined) {
-    url = await getConfig({ fs, gitdir, path: `remote.${remote}.url` })
+  const ref = _ref || (await currentBranch({ fs, gitdir }))
+  if (typeof ref === 'undefined') {
+    throw new GitError(E.MissingRequiredParameterError, {
+      parameter: 'ref'
+    })
   }
-
-  if (corsProxy === undefined) {
-    corsProxy = await getConfig({ fs, gitdir, path: 'http.corsProxy' })
+  const config = await GitConfigManager.get({ fs, gitdir })
+  // Figure out what remote to use.
+  remote =
+    remote ||
+    (await config.get(`branch.${ref}.pushRemote`)) ||
+    (await config.get('remote.pushDefault')) ||
+    (await config.get(`branch.${ref}.remote`)) ||
+    'origin'
+  // Lookup the URL for the given remote.
+  const url =
+    _url ||
+    (await config.get(`remote.${remote}.pushurl`)) ||
+    (await config.get(`remote.${remote}.url`))
+  if (typeof url === 'undefined') {
+    throw new GitError(E.MissingRequiredParameterError, {
+      parameter: 'remote OR url'
+    })
   }
-  let fullRef
-  if (!ref) {
-    fullRef = await GitRefManager.resolve({
-      fs,
-      gitdir,
-      ref: 'HEAD',
-      depth: 2
+  // Figure out what remote ref to use.
+  const remoteRef = _remoteRef || (await config.get(`branch.${ref}.merge`))
+  if (typeof url === 'undefined') {
+    throw new GitError(E.MissingRequiredParameterError, {
+      parameter: 'remoteRef'
     })
-  } else {
-    fullRef = await GitRefManager.expand({ fs, gitdir, ref })
   }
+
+  if (corsProxy === undefined) {
+    corsProxy = await config.get('http.corsProxy')
+  }
+
+  const fullRef = await GitRefManager.expand({ fs, gitdir, ref })
   const oid = _delete
     ? '0000000000000000000000000000000000000000'
     : await GitRefManager.resolve({ fs, gitdir, ref: fullRef })
@@ -229,6 +248,7 @@ export async function push ({
   }
 
   // Update the local copy of the remote ref
+  // TODO: I think this should actually be using a refspec transform rather than assuming 'refs/remotes/{remote}'
   if (remote && result.ok && result.ok.includes(fullRemoteRef)) {
     const ref = `refs/remotes/${remote}/${fullRemoteRef.replace(
       'refs/heads',

Original file line number	Diff line number	Diff line change
`@@ -9,8 +9,6 @@ import { join } from '../utils/join.js'`
`9`	`9`	`/**`
`10`	`10`	`* Clone a repository`
`11`	`11`	`*`
`12`		- * To monitor progress events, see the documentation for the [`'emitter'` plugin](./plugin_emitter.md).
`13`		`- *`
`14`	`12`	`* @param {object} args`
`15`	`13`	`* @param {FsClient} args.fs - a file system implementation`
`16`	`14`	`* @param {HttpClient} args.http - an HTTP client`