Merge pull request desktop#14781 from desktop/yes-this-is-future

Remove explicit opt-in to protocol version 2

app/src/lib/git/branch.ts

@@ -76,7 +76,6 @@ export async function deleteRemoteBranch(
   remoteName: string,
   remoteBranchName: string
 ): Promise<true> {
-  const networkArguments = await gitNetworkArguments(repository, account)
   const remoteUrl =
     (await getRemoteURL(repository, remoteName).catch(err => {
       // If we can't get the URL then it's very unlikely Git will be able to
@@ -86,7 +85,12 @@ export async function deleteRemoteBranch(
       return null
     })) || getFallbackUrlForProxyResolve(account, repository)
 
-  const args = [...networkArguments, 'push', remoteName, `:${remoteBranchName}`]
+  const args = [
+    ...gitNetworkArguments(),
+    'push',
+    remoteName,
+    `:${remoteBranchName}`,
+  ]
 
   // If the user is not authenticated, the push is going to fail
   // Let this propagate and leave it to the caller to handle

app/src/lib/git/checkout.ts

@@ -24,12 +24,10 @@ async function getCheckoutArgs(
   account: IGitAccount | null,
   progressCallback?: ProgressCallback
 ) {
-  const networkArguments = await gitNetworkArguments(repository, account)
-
   const baseArgs =
     progressCallback != null
-      ? [...networkArguments, 'checkout', '--progress']
-      : [...networkArguments, 'checkout']
+      ? [...gitNetworkArguments(), 'checkout', '--progress']
+      : [...gitNetworkArguments(), 'checkout']
 
   if (enableRecurseSubmodulesFlag()) {
     return branch.type === BranchType.Remote

app/src/lib/git/clone.ts

@@ -31,14 +31,12 @@ export async function clone(
   options: CloneOptions,
   progressCallback?: (progress: ICloneProgress) => void
 ): Promise<void> {
-  const networkArguments = await gitNetworkArguments(null, options.account)
-
   const env = await envForRemoteOperation(options.account, url)
 
   const defaultBranch = options.defaultBranch ?? (await getDefaultBranch())
 
   const args = [
-    ...networkArguments,
+    ...gitNetworkArguments(),
     '-c',
     `init.defaultBranch=${defaultBranch}`,
     'clone',

app/src/lib/git/core.ts

@@ -6,14 +6,8 @@ import {
 } from 'dugite'
 
 import { assertNever } from '../fatal-error'
-import { getDotComAPIEndpoint } from '../api'
-
-import { IGitAccount } from '../../models/git-account'
-
 import * as GitPerf from '../../ui/lib/git-perf'
 import * as Path from 'path'
-import { Repository } from '../../models/repository'
-import { getConfigValue, getGlobalConfigValue } from './config'
 import { isErrnoException } from '../errno-exception'
 import { ChildProcess } from 'child_process'
 import { Readable } from 'stream'
@@ -446,57 +440,15 @@ function getDescriptionForError(error: DugiteError): string | null {
  * the default git configuration values provided by local, global, or system
  * level git configs.
  *
- * These arguments should be inserted before the subcommand, i.e in
- * the case of `git pull` these arguments needs to go before the `pull`
- * argument.
- *
- * @param repository the local repository associated with the command, to check
- *                   local, global and system config for an existing value.
- *                   If `null` if provided (for example, when cloning a new
- *                   repository), this function will check global and system
- *                   config for an existing `protocol.version` setting
- *
- * @param account the identity associated with the repository, or `null` if
- *                unknown. The `protocol.version` behaviour is currently only
- *                enabled for GitHub.com repositories that don't have an
- *                existing `protocol.version` setting.
+ * These arguments should be inserted before the subcommand, i.e in the case of
+ * `git pull` these arguments needs to go before the `pull` argument.
  */
-export async function gitNetworkArguments(
-  repository: Repository | null,
-  account: IGitAccount | null
-): Promise<ReadonlyArray<string>> {
-  const baseArgs = [
-    // Explicitly unset any defined credential helper, we rely on our
-    // own askpass for authentication.
-    '-c',
-    'credential.helper=',
-  ]
-
-  if (account === null) {
-    return baseArgs
-  }
-
-  const isDotComAccount = account.endpoint === getDotComAPIEndpoint()
-
-  if (!isDotComAccount) {
-    return baseArgs
-  }
-
-  const name = 'protocol.version'
-
-  const protocolVersion =
-    repository != null
-      ? await getConfigValue(repository, name)
-      : await getGlobalConfigValue(name)
-
-  if (protocolVersion !== null) {
-    // protocol.version is already set, we should not override it with our own
-    return baseArgs
-  }
-
-  // opt in for v2 of the Git Wire protocol for GitHub repositories
-  return [...baseArgs, '-c', 'protocol.version=2']
-}
+export const gitNetworkArguments = () => [
+  // Explicitly unset any defined credential helper, we rely on our
+  // own askpass for authentication.
+  '-c',
+  'credential.helper=',
+]
 
 /**
  * Returns the arguments to use on any git operation that can end up

app/src/lib/git/fetch.ts

@@ -14,29 +14,27 @@ async function getFetchArgs(
   account: IGitAccount | null,
   progressCallback?: (progress: IFetchProgress) => void
 ) {
-  const networkArguments = await gitNetworkArguments(repository, account)
-
   if (enableRecurseSubmodulesFlag()) {
     return progressCallback != null
       ? [
-          ...networkArguments,
+          ...gitNetworkArguments(),
           'fetch',
           '--progress',
           '--prune',
           '--recurse-submodules=on-demand',
           remote,
         ]
       : [
-          ...networkArguments,
+          ...gitNetworkArguments(),
           'fetch',
           '--prune',
           '--recurse-submodules=on-demand',
           remote,
         ]
   } else {
     return progressCallback != null
-      ? [...networkArguments, 'fetch', '--progress', '--prune', remote]
-      : [...networkArguments, 'fetch', '--prune', remote]
+      ? [...gitNetworkArguments(), 'fetch', '--progress', '--prune', remote]
+      : [...gitNetworkArguments(), 'fetch', '--prune', remote]
   }
 }
 
@@ -119,16 +117,15 @@ export async function fetchRefspec(
   remote: IRemote,
   refspec: string
 ): Promise<void> {
-  const options = {
-    successExitCodes: new Set([0, 128]),
-    env: await envForRemoteOperation(account, remote.url),
-  }
-
-  const networkArguments = await gitNetworkArguments(repository, account)
-
-  const args = [...networkArguments, 'fetch', remote.name, refspec]
-
-  await git(args, repository.path, 'fetchRefspec', options)
+  await git(
+    [...gitNetworkArguments(), 'fetch', remote.name, refspec],
+    repository.path,
+    'fetchRefspec',
+    {
+      successExitCodes: new Set([0, 128]),
+      env: await envForRemoteOperation(account, remote.url),
+    }
+  )
 }
 
 export async function fastForwardBranches(

app/src/lib/git/pull.ts

@@ -21,14 +21,12 @@ async function getPullArgs(
   account: IGitAccount | null,
   progressCallback?: (progress: IPullProgress) => void
 ) {
-  const networkArguments = await gitNetworkArguments(repository, account)
-
   const divergentPathArgs = await getDefaultPullDivergentBranchArguments(
     repository
   )
 
   const args = [
-    ...networkArguments,
+    ...gitNetworkArguments(),
     ...gitRebaseArguments(),
     'pull',
     ...divergentPathArgs,

app/src/lib/git/push.ts

@@ -60,10 +60,8 @@ export async function push(
   },
   progressCallback?: (progress: IPushProgress) => void
 ): Promise<void> {
-  const networkArguments = await gitNetworkArguments(repository, account)
-
   const args = [
-    ...networkArguments,
+    ...gitNetworkArguments(),
     'push',
     remote.name,
     remoteBranch ? `${localBranch}:${remoteBranch}` : localBranch,

app/src/lib/git/revert.ts

@@ -26,9 +26,7 @@ export async function revertCommit(
   account: IGitAccount | null,
   progressCallback?: (progress: IRevertProgress) => void
 ) {
-  const networkArguments = await gitNetworkArguments(repository, account)
-
-  const args = [...networkArguments, 'revert']
+  const args = [...gitNetworkArguments(), 'revert']
   if (commit.parentSHAs.length > 1) {
     args.push('-m', '1')
   }

app/src/lib/git/tag.ts

@@ -90,10 +90,8 @@ export async function fetchTagsToPush(
   remote: IRemote,
   branchName: string
 ): Promise<ReadonlyArray<string>> {
-  const networkArguments = await gitNetworkArguments(repository, account)
-
   const args = [
-    ...networkArguments,
+    ...gitNetworkArguments(),
     'push',
     remote.name,
     branchName,