From 5caf811e0d21b3187c8e2d20fde10e7bb4628d8c Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 16 May 2024 13:50:32 +0200 Subject: [PATCH 1/4] refactor: Remove need to pass account to envForRemoteOperation Co-Authored-By: Sergio Padrino <1083228+sergiou87@users.noreply.github.com> --- app/src/lib/git/branch.ts | 4 +- app/src/lib/git/checkout.ts | 7 - app/src/lib/git/clone.ts | 2 +- app/src/lib/git/environment.ts | 6 +- app/src/lib/git/fetch.ts | 16 +- app/src/lib/git/pull.ts | 12 +- app/src/lib/git/push.ts | 4 +- app/src/lib/git/remote.ts | 4 +- app/src/lib/git/revert.ts | 3 - app/src/lib/git/tag.ts | 4 +- app/src/lib/stores/app-store.ts | 252 ++++-------------- app/src/lib/stores/git-store.ts | 27 +- .../helpers/create-tutorial-repository.ts | 2 +- app/src/models/clone-options.ts | 4 - 14 files changed, 76 insertions(+), 271 deletions(-) diff --git a/app/src/lib/git/branch.ts b/app/src/lib/git/branch.ts index 78169f90d5..a4a394fc3c 100644 --- a/app/src/lib/git/branch.ts +++ b/app/src/lib/git/branch.ts @@ -1,7 +1,6 @@ import { git, gitNetworkArguments } from './core' import { Repository } from '../../models/repository' import { Branch } from '../../models/branch' -import { IGitAccount } from '../../models/git-account' import { formatAsLocalRef } from './refs' import { deleteRef } from './update-ref' import { GitError as DugiteError } from 'dugite' @@ -69,7 +68,6 @@ export async function deleteLocalBranch( */ export async function deleteRemoteBranch( repository: Repository, - account: IGitAccount | null, remote: IRemote, remoteBranchName: string ): Promise { @@ -83,7 +81,7 @@ export async function deleteRemoteBranch( // If the user is not authenticated, the push is going to fail // Let this propagate and leave it to the caller to handle const result = await git(args, repository.path, 'deleteRemoteBranch', { - env: await envForRemoteOperation(account, remote.url), + env: await envForRemoteOperation(remote.url), expectedErrors: new Set([DugiteError.BranchDeletionFailed]), }) diff --git a/app/src/lib/git/checkout.ts b/app/src/lib/git/checkout.ts index ae9e4b28d5..7294b24dc5 100644 --- a/app/src/lib/git/checkout.ts +++ b/app/src/lib/git/checkout.ts @@ -2,7 +2,6 @@ import { git, IGitExecutionOptions, gitNetworkArguments } from './core' import { Repository } from '../../models/repository' import { Branch, BranchType } from '../../models/branch' import { ICheckoutProgress } from '../../models/progress' -import { IGitAccount } from '../../models/git-account' import { CheckoutProgressParser, executionOptionsWithProgress, @@ -47,7 +46,6 @@ async function getBranchCheckoutArgs(branch: Branch) { async function getCheckoutOpts( repository: Repository, - account: IGitAccount | null, title: string, target: string, currentRemote: IRemote | null, @@ -56,7 +54,6 @@ async function getCheckoutOpts( ): Promise { const opts: IGitExecutionOptions = { env: await envForRemoteOperation( - account, getFallbackUrlForProxyResolve(repository, currentRemote) ), expectedErrors: AuthenticationErrors, @@ -113,14 +110,12 @@ async function getCheckoutOpts( */ export async function checkoutBranch( repository: Repository, - account: IGitAccount | null, branch: Branch, currentRemote: IRemote | null, progressCallback?: ProgressCallback ): Promise { const opts = await getCheckoutOpts( repository, - account, `Checking out branch ${branch.name}`, branch.name, currentRemote, @@ -155,7 +150,6 @@ export async function checkoutBranch( */ export async function checkoutCommit( repository: Repository, - account: IGitAccount | null, commit: CommitOneLine, currentRemote: IRemote | null, progressCallback?: ProgressCallback @@ -163,7 +157,6 @@ export async function checkoutCommit( const title = `Checking out ${__DARWIN__ ? 'Commit' : 'commit'}` const opts = await getCheckoutOpts( repository, - account, title, shortenSHA(commit.sha), currentRemote, diff --git a/app/src/lib/git/clone.ts b/app/src/lib/git/clone.ts index 8b0dc36c0b..20a4e5ad64 100644 --- a/app/src/lib/git/clone.ts +++ b/app/src/lib/git/clone.ts @@ -31,7 +31,7 @@ export async function clone( progressCallback?: (progress: ICloneProgress) => void ): Promise { const env = { - ...(await envForRemoteOperation(options.account, url)), + ...(await envForRemoteOperation(url)), GIT_CLONE_PROTECTION_ACTIVE: 'false', } diff --git a/app/src/lib/git/environment.ts b/app/src/lib/git/environment.ts index 276a085a6c..90680d5c23 100644 --- a/app/src/lib/git/environment.ts +++ b/app/src/lib/git/environment.ts @@ -1,5 +1,4 @@ import { envForAuthentication } from './authentication' -import { IGitAccount } from '../../models/git-account' import { resolveGitProxy } from '../resolve-git-proxy' import { getHTMLURL } from '../api' import { @@ -74,10 +73,7 @@ export function getFallbackUrlForProxyResolve( * pointing to another host entirely. Used to resolve which * proxy (if any) should be used for the operation. */ -export async function envForRemoteOperation( - account: IGitAccount | null, - remoteUrl: string -) { +export async function envForRemoteOperation(remoteUrl: string) { return { ...envForAuthentication(), ...(await envForProxy(remoteUrl)), diff --git a/app/src/lib/git/fetch.ts b/app/src/lib/git/fetch.ts index e50fb1b1d1..0ad5ff9a4c 100644 --- a/app/src/lib/git/fetch.ts +++ b/app/src/lib/git/fetch.ts @@ -1,6 +1,5 @@ import { git, IGitExecutionOptions, gitNetworkArguments } from './core' import { Repository } from '../../models/repository' -import { IGitAccount } from '../../models/git-account' import { IFetchProgress } from '../../models/progress' import { FetchProgressParser, executionOptionsWithProgress } from '../progress' import { enableRecurseSubmodulesFlag } from '../feature-flag' @@ -9,9 +8,7 @@ import { ITrackingBranch } from '../../models/branch' import { envForRemoteOperation } from './environment' async function getFetchArgs( - repository: Repository, remote: string, - account: IGitAccount | null, progressCallback?: (progress: IFetchProgress) => void ) { if (enableRecurseSubmodulesFlag()) { @@ -57,14 +54,13 @@ async function getFetchArgs( */ export async function fetch( repository: Repository, - account: IGitAccount | null, remote: IRemote, progressCallback?: (progress: IFetchProgress) => void, isBackgroundTask = false ): Promise { let opts: IGitExecutionOptions = { successExitCodes: new Set([0]), - env: await envForRemoteOperation(account, remote.url), + env: await envForRemoteOperation(remote.url), } if (progressCallback) { @@ -103,12 +99,7 @@ export async function fetch( progressCallback({ kind, title, value: 0, remote: remote.name }) } - const args = await getFetchArgs( - repository, - remote.name, - account, - progressCallback - ) + const args = await getFetchArgs(remote.name, progressCallback) await git(args, repository.path, 'fetch', opts) } @@ -116,7 +107,6 @@ export async function fetch( /** Fetch a given refspec from the given remote. */ export async function fetchRefspec( repository: Repository, - account: IGitAccount | null, remote: IRemote, refspec: string ): Promise { @@ -126,7 +116,7 @@ export async function fetchRefspec( 'fetchRefspec', { successExitCodes: new Set([0, 128]), - env: await envForRemoteOperation(account, remote.url), + env: await envForRemoteOperation(remote.url), } ) } diff --git a/app/src/lib/git/pull.ts b/app/src/lib/git/pull.ts index fa95a8c910..ec980adbbb 100644 --- a/app/src/lib/git/pull.ts +++ b/app/src/lib/git/pull.ts @@ -7,7 +7,6 @@ import { } from './core' import { Repository } from '../../models/repository' import { IPullProgress } from '../../models/progress' -import { IGitAccount } from '../../models/git-account' import { PullProgressParser, executionOptionsWithProgress } from '../progress' import { AuthenticationErrors } from './authentication' import { enableRecurseSubmodulesFlag } from '../feature-flag' @@ -18,7 +17,6 @@ import { getConfigValue } from './config' async function getPullArgs( repository: Repository, remote: string, - account: IGitAccount | null, progressCallback?: (progress: IPullProgress) => void ) { const divergentPathArgs = await getDefaultPullDivergentBranchArguments( @@ -60,12 +58,11 @@ async function getPullArgs( */ export async function pull( repository: Repository, - account: IGitAccount | null, remote: IRemote, progressCallback?: (progress: IPullProgress) => void ): Promise { let opts: IGitExecutionOptions = { - env: await envForRemoteOperation(account, remote.url), + env: await envForRemoteOperation(remote.url), expectedErrors: AuthenticationErrors, } @@ -106,12 +103,7 @@ export async function pull( progressCallback({ kind, title, value: 0, remote: remote.name }) } - const args = await getPullArgs( - repository, - remote.name, - account, - progressCallback - ) + const args = await getPullArgs(repository, remote.name, progressCallback) const result = await git(args, repository.path, 'pull', opts) if (result.gitErrorDescription) { diff --git a/app/src/lib/git/push.ts b/app/src/lib/git/push.ts index eb81701baf..b392f5e240 100644 --- a/app/src/lib/git/push.ts +++ b/app/src/lib/git/push.ts @@ -8,7 +8,6 @@ import { } from './core' import { Repository } from '../../models/repository' import { IPushProgress } from '../../models/progress' -import { IGitAccount } from '../../models/git-account' import { PushProgressParser, executionOptionsWithProgress } from '../progress' import { AuthenticationErrors } from './authentication' import { IRemote } from '../../models/remote' @@ -50,7 +49,6 @@ export type PushOptions = { */ export async function push( repository: Repository, - account: IGitAccount | null, remote: IRemote, localBranch: string, remoteBranch: string | null, @@ -80,7 +78,7 @@ export async function push( expectedErrors.add(DugiteError.ProtectedBranchForcePush) let opts: IGitExecutionOptions = { - env: await envForRemoteOperation(account, remote.url), + env: await envForRemoteOperation(remote.url), expectedErrors, } diff --git a/app/src/lib/git/remote.ts b/app/src/lib/git/remote.ts index 20b9e0f0b7..e264c842d4 100644 --- a/app/src/lib/git/remote.ts +++ b/app/src/lib/git/remote.ts @@ -4,7 +4,6 @@ import { GitError } from 'dugite' import { Repository } from '../../models/repository' import { IRemote } from '../../models/remote' import { envForRemoteOperation } from './environment' -import { IGitAccount } from '../../models/git-account' import { getSymbolicRef } from './refs' import { gitNetworkArguments } from '.' @@ -101,13 +100,12 @@ export async function getRemoteURL( */ export async function updateRemoteHEAD( repository: Repository, - account: IGitAccount | null, remote: IRemote, isBackgroundTask: boolean ): Promise { const options = { successExitCodes: new Set([0, 1, 128]), - env: await envForRemoteOperation(account, remote.url), + env: await envForRemoteOperation(remote.url), isBackgroundTask, } diff --git a/app/src/lib/git/revert.ts b/app/src/lib/git/revert.ts index 5c22336bfe..708a452a75 100644 --- a/app/src/lib/git/revert.ts +++ b/app/src/lib/git/revert.ts @@ -3,7 +3,6 @@ import { git, gitNetworkArguments, IGitExecutionOptions } from './core' import { Repository } from '../../models/repository' import { Commit } from '../../models/commit' import { IRevertProgress } from '../../models/progress' -import { IGitAccount } from '../../models/git-account' import { executionOptionsWithProgress } from '../progress/from-process' import { RevertProgressParser } from '../progress/revert' @@ -23,7 +22,6 @@ import { IRemote } from '../../models/remote' export async function revertCommit( repository: Repository, commit: Commit, - account: IGitAccount | null, currentRemote: IRemote | null, progressCallback?: (progress: IRevertProgress) => void ) { @@ -37,7 +35,6 @@ export async function revertCommit( let opts: IGitExecutionOptions = {} if (progressCallback) { const env = await envForRemoteOperation( - account, getFallbackUrlForProxyResolve(repository, currentRemote) ) opts = await executionOptionsWithProgress( diff --git a/app/src/lib/git/tag.ts b/app/src/lib/git/tag.ts index 50f8d38c79..5e2968a732 100644 --- a/app/src/lib/git/tag.ts +++ b/app/src/lib/git/tag.ts @@ -1,6 +1,5 @@ import { git, gitNetworkArguments } from './core' import { Repository } from '../../models/repository' -import { IGitAccount } from '../../models/git-account' import { IRemote } from '../../models/remote' import { envForRemoteOperation } from './environment' @@ -86,7 +85,6 @@ export async function getAllTags( */ export async function fetchTagsToPush( repository: Repository, - account: IGitAccount | null, remote: IRemote, branchName: string ): Promise> { @@ -102,7 +100,7 @@ export async function fetchTagsToPush( ] const result = await git(args, repository.path, 'fetchTagsToPush', { - env: await envForRemoteOperation(account, remote.url), + env: await envForRemoteOperation(remote.url), successExitCodes: new Set([0, 1, 128]), }) diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index b93507a3ac..7d5c290389 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -135,11 +135,7 @@ import { import { assertNever, fatalError, forceUnwrap } from '../fatal-error' import { formatCommitMessage } from '../format-commit-message' -import { - getGenericHostname, - getGenericUsername, - removePathFromGenericGitAuthCreds, -} from '../generic-git-auth' +import { removePathFromGenericGitAuthCreds } from '../generic-git-auth' import { getAccountForRepository } from '../get-account-for-repository' import { abortMerge, @@ -3550,12 +3546,12 @@ export class AppStore extends TypedBaseStore { * of the current branch to its upstream tracking branch. */ private fetchForRepositoryIndicator(repo: Repository) { - return this.withAuthenticatingUser(repo, async (repo, account) => { + return this.withAuthenticatingUser(repo, async repo => { const isBackgroundTask = true const gitStore = this.gitStoreCache.get(repo) await this.withPushPullFetch(repo, () => - gitStore.fetch(account, isBackgroundTask, progress => + gitStore.fetch(isBackgroundTask, progress => this.updatePushPullFetchProgress(repo, progress) ) ) @@ -3863,11 +3859,11 @@ export class AppStore extends TypedBaseStore { } } - return this.withAuthenticatingUser(repository, (repository, account) => { + return this.withAuthenticatingUser(repository, repository => { // We always want to end with refreshing the repository regardless of // whether the checkout succeeded or not in order to present the most // up-to-date information to the user. - return this.checkoutImplementation(repository, branch, account, strategy) + return this.checkoutImplementation(repository, branch, strategy) .then(() => this.onSuccessfulCheckout(repository, branch)) .catch(e => this.emitError(new CheckoutError(e, repository, branch))) .then(() => this.refreshAfterCheckout(repository, branch.name)) @@ -3879,32 +3875,16 @@ export class AppStore extends TypedBaseStore { private checkoutImplementation( repository: Repository, branch: Branch, - account: IGitAccount | null, strategy: UncommittedChangesStrategy ) { const { currentRemote } = this.gitStoreCache.get(repository) if (strategy === UncommittedChangesStrategy.StashOnCurrentBranch) { - return this.checkoutAndLeaveChanges( - repository, - branch, - account, - currentRemote - ) + return this.checkoutAndLeaveChanges(repository, branch, currentRemote) } else if (strategy === UncommittedChangesStrategy.MoveToNewBranch) { - return this.checkoutAndBringChanges( - repository, - branch, - account, - currentRemote - ) + return this.checkoutAndBringChanges(repository, branch, currentRemote) } else { - return this.checkoutIgnoringChanges( - repository, - branch, - account, - currentRemote - ) + return this.checkoutIgnoringChanges(repository, branch, currentRemote) } } @@ -3912,18 +3892,11 @@ export class AppStore extends TypedBaseStore { private async checkoutIgnoringChanges( repository: Repository, branch: Branch, - account: IGitAccount | null, currentRemote: IRemote | null ) { - await checkoutBranch( - repository, - account, - branch, - currentRemote, - progress => { - this.updateCheckoutProgress(repository, progress) - } - ) + await checkoutBranch(repository, branch, currentRemote, progress => { + this.updateCheckoutProgress(repository, progress) + }) } /** @@ -3934,7 +3907,6 @@ export class AppStore extends TypedBaseStore { private async checkoutAndLeaveChanges( repository: Repository, branch: Branch, - account: IGitAccount | null, currentRemote: IRemote | null ) { const repositoryState = this.repositoryStateCache.get(repository) @@ -3946,12 +3918,7 @@ export class AppStore extends TypedBaseStore { this.statsStore.increment('stashCreatedOnCurrentBranchCount') } - return this.checkoutIgnoringChanges( - repository, - branch, - account, - currentRemote - ) + return this.checkoutIgnoringChanges(repository, branch, currentRemote) } /** @@ -3967,16 +3934,10 @@ export class AppStore extends TypedBaseStore { private async checkoutAndBringChanges( repository: Repository, branch: Branch, - account: IGitAccount | null, currentRemote: IRemote | null ) { try { - await this.checkoutIgnoringChanges( - repository, - branch, - account, - currentRemote - ) + await this.checkoutIgnoringChanges(repository, branch, currentRemote) } catch (checkoutError) { if (!isLocalChangesOverwrittenError(checkoutError)) { throw checkoutError @@ -3994,12 +3955,7 @@ export class AppStore extends TypedBaseStore { throw checkoutError } - await this.checkoutIgnoringChanges( - repository, - branch, - account, - currentRemote - ) + await this.checkoutIgnoringChanges(repository, branch, currentRemote) await popStashEntry(repository, stash.stashSha) this.statsStore.increment('changesTakenToNewBranchCount') @@ -4070,14 +4026,13 @@ export class AppStore extends TypedBaseStore { return repository } - return this.withAuthenticatingUser(repository, (repository, account) => { + return this.withAuthenticatingUser(repository, repository => { // We always want to end with refreshing the repository regardless of // whether the checkout succeeded or not in order to present the most // up-to-date information to the user. return this.checkoutCommitDefaultBehaviour( repository, commit, - account, currentRemote ) .catch(e => this.emitError(new Error(e))) @@ -4091,18 +4046,11 @@ export class AppStore extends TypedBaseStore { private async checkoutCommitDefaultBehaviour( repository: Repository, commit: CommitOneLine, - account: IGitAccount | null, currentRemote: IRemote | null ) { - await checkoutCommit( - repository, - account, - commit, - currentRemote, - progress => { - this.updateCheckoutProgress(repository, progress) - } - ) + await checkoutCommit(repository, commit, currentRemote, progress => { + this.updateCheckoutProgress(repository, progress) + }) } /** @@ -4280,7 +4228,7 @@ export class AppStore extends TypedBaseStore { includeUpstream?: boolean, toCheckout?: Branch | null ): Promise { - return this.withAuthenticatingUser(repository, async (r, account) => { + return this.withAuthenticatingUser(repository, async r => { const gitStore = this.gitStoreCache.get(r) // If solely a remote branch, there is no need to checkout a branch. @@ -4305,7 +4253,7 @@ export class AppStore extends TypedBaseStore { } await gitStore.performFailableOperation(() => - deleteRemoteBranch(r, account, remote, nameWithoutRemote) + deleteRemoteBranch(r, remote, nameWithoutRemote) ) // We log the remote branch's sha so that the user can recover it. @@ -4323,7 +4271,7 @@ export class AppStore extends TypedBaseStore { if (branchToCheckout !== null) { await gitStore.performFailableOperation(() => - checkoutBranch(r, account, branchToCheckout, gitStore.currentRemote) + checkoutBranch(r, branchToCheckout, gitStore.currentRemote) ) } @@ -4331,7 +4279,6 @@ export class AppStore extends TypedBaseStore { return this.deleteLocalBranchAndUpstreamBranch( repository, branch, - account, includeUpstream ) }) @@ -4347,7 +4294,6 @@ export class AppStore extends TypedBaseStore { private async deleteLocalBranchAndUpstreamBranch( repository: Repository, branch: Branch, - account: IGitAccount | null, includeUpstream?: boolean ): Promise { await deleteLocalBranch(repository, branch.name) @@ -4370,12 +4316,7 @@ export class AppStore extends TypedBaseStore { throw new Error(`Could not determine remote url from: ${branch.ref}.`) } - await deleteRemoteBranch( - repository, - account, - remote, - branch.upstreamWithoutRemote - ) + await deleteRemoteBranch(repository, remote, branch.upstreamWithoutRemote) } return } @@ -4424,14 +4365,13 @@ export class AppStore extends TypedBaseStore { repository: Repository, options?: PushOptions ): Promise { - return this.withAuthenticatingUser(repository, (repository, account) => { - return this.performPush(repository, account, options) + return this.withAuthenticatingUser(repository, repository => { + return this.performPush(repository, options) }) } private async performPush( repository: Repository, - account: IGitAccount | null, options?: PushOptions ): Promise { const state = this.repositoryStateCache.get(repository) @@ -4535,7 +4475,6 @@ export class AppStore extends TypedBaseStore { async () => { await pushRepo( repository, - account, safeRemote, branch.name, branch.upstreamWithoutRemote, @@ -4551,17 +4490,12 @@ export class AppStore extends TypedBaseStore { ) gitStore.clearTagsToPush() - await gitStore.fetchRemotes( - account, - [safeRemote], - false, - fetchProgress => { - this.updatePushPullFetchProgress(repository, { - ...fetchProgress, - value: pushWeight + fetchProgress.value * fetchWeight, - }) - } - ) + await gitStore.fetchRemotes([safeRemote], false, fetchProgress => { + this.updatePushPullFetchProgress(repository, { + ...fetchProgress, + value: pushWeight + fetchProgress.value * fetchWeight, + }) + }) const refreshTitle = __DARWIN__ ? 'Refreshing Repository' @@ -4659,16 +4593,13 @@ export class AppStore extends TypedBaseStore { } public async _pull(repository: Repository): Promise { - return this.withAuthenticatingUser(repository, (repository, account) => { - return this.performPull(repository, account) + return this.withAuthenticatingUser(repository, repository => { + return this.performPull(repository) }) } /** This shouldn't be called directly. See `Dispatcher`. */ - private async performPull( - repository: Repository, - account: IGitAccount | null - ): Promise { + private async performPull(repository: Repository): Promise { return this.withPushPullFetch(repository, async () => { const gitStore = this.gitStoreCache.get(repository) const remote = gitStore.currentRemote @@ -4743,7 +4674,7 @@ export class AppStore extends TypedBaseStore { const pullSucceeded = await gitStore.performFailableOperation( async () => { - await pullRepo(repository, account, remote, progress => { + await pullRepo(repository, remote, progress => { this.updatePushPullFetchProgress(repository, { ...progress, value: progress.value * pullWeight, @@ -4763,8 +4694,8 @@ export class AppStore extends TypedBaseStore { // Updating the local HEAD symref isn't critical so we don't want // to show an error message to the user and have them retry the // entire pull operation if it fails. - await updateRemoteHEAD(repository, account, remote, false).catch( - e => log.error('Failed updating remote HEAD', e) + await updateRemoteHEAD(repository, remote, false).catch(e => + log.error('Failed updating remote HEAD', e) ) } @@ -4842,7 +4773,7 @@ export class AppStore extends TypedBaseStore { // skip pushing if the current branch is a detached HEAD or the repository // is unborn if (gitStore.tip.kind === TipState.Valid) { - await this.performPush(repository, account) + await this.performPush(repository) } await gitStore.refreshDefaultBranch() @@ -4850,47 +4781,16 @@ export class AppStore extends TypedBaseStore { return this.repositoryWithRefreshedGitHubRepository(repository) } - private getAccountForRemoteURL(remote: string): IGitAccount | null { - const account = matchGitHubRepository(this.accounts, remote)?.account - if (account !== undefined) { - const hasValidToken = - account.token.length > 0 ? 'has token' : 'empty token' - log.info( - `[AppStore.getAccountForRemoteURL] account found for remote: ${remote} - ${account.login} (${hasValidToken})` - ) - return account - } - - const hostname = getGenericHostname(remote) - const username = getGenericUsername(hostname) - if (username != null) { - log.info( - `[AppStore.getAccountForRemoteURL] found generic credentials for '${hostname}' and '${username}'` - ) - return { login: username, endpoint: hostname } - } - - log.info( - `[AppStore.getAccountForRemoteURL] no generic credentials found for '${remote}'` - ) - - return null - } - /** This shouldn't be called directly. See `Dispatcher`. */ public _clone( url: string, path: string, - options?: { branch?: string; defaultBranch?: string } + options: { branch?: string; defaultBranch?: string } = {} ): { promise: Promise repository: CloningRepository } { - const account = this.getAccountForRemoteURL(url) - const promise = this.cloningRepositoriesStore.clone(url, path, { - ...options, - account, - }) + const promise = this.cloningRepositoriesStore.clone(url, path, options) const repository = this.cloningRepositoriesStore.repositories.find( r => r.url === url && r.path === path )! @@ -5091,15 +4991,12 @@ export class AppStore extends TypedBaseStore { repository: Repository, refspec: string ): Promise { - return this.withAuthenticatingUser( - repository, - async (repository, account) => { - const gitStore = this.gitStoreCache.get(repository) - await gitStore.fetchRefspec(account, refspec) + return this.withAuthenticatingUser(repository, async repository => { + const gitStore = this.gitStoreCache.get(repository) + await gitStore.fetchRefspec(refspec) - return this._refreshRepository(repository) - } - ) + return this._refreshRepository(repository) + }) } /** @@ -5111,8 +5008,8 @@ export class AppStore extends TypedBaseStore { * if _any_ fetches or pulls are currently in-progress. */ public _fetch(repository: Repository, fetchType: FetchType): Promise { - return this.withAuthenticatingUser(repository, (repository, account) => { - return this.performFetch(repository, account, fetchType) + return this.withAuthenticatingUser(repository, repository => { + return this.performFetch(repository, fetchType) }) } @@ -5127,8 +5024,8 @@ export class AppStore extends TypedBaseStore { remote: IRemote, fetchType: FetchType ): Promise { - return this.withAuthenticatingUser(repository, (repository, account) => { - return this.performFetch(repository, account, fetchType, [remote]) + return this.withAuthenticatingUser(repository, repository => { + return this.performFetch(repository, fetchType, [remote]) }) } @@ -5141,7 +5038,6 @@ export class AppStore extends TypedBaseStore { */ private async performFetch( repository: Repository, - account: IGitAccount | null, fetchType: FetchType, remotes?: IRemote[] ): Promise { @@ -5161,10 +5057,9 @@ export class AppStore extends TypedBaseStore { } if (remotes === undefined) { - await gitStore.fetch(account, isBackgroundTask, progressCallback) + await gitStore.fetch(isBackgroundTask, progressCallback) } else { await gitStore.fetchRemotes( - account, remotes, isBackgroundTask, progressCallback @@ -6148,10 +6043,10 @@ export class AppStore extends TypedBaseStore { private async withAuthenticatingUser( repository: Repository, - fn: (repository: Repository, account: IGitAccount | null) => Promise + fn: (repository: Repository) => Promise ): Promise { let updatedRepository = repository - let account: IGitAccount | null = getAccountForRepository( + const account: IGitAccount | null = getAccountForRepository( this.accounts, updatedRepository ) @@ -6164,30 +6059,9 @@ export class AppStore extends TypedBaseStore { updatedRepository = await this.repositoryWithRefreshedGitHubRepository( repository ) - account = getAccountForRepository(this.accounts, updatedRepository) } - if (!account) { - const gitStore = this.gitStoreCache.get(repository) - const remote = gitStore.currentRemote - if (remote) { - const hostname = getGenericHostname(remote.url) - const username = getGenericUsername(hostname) - if (username != null) { - account = { login: username, endpoint: hostname } - } - } - } - - if (account instanceof Account) { - const hasValidToken = - account.token.length > 0 ? 'has token' : 'empty token' - log.info( - `[AppStore.withAuthenticatingUser] account found for repository: ${repository.name} - ${account.login} (${hasValidToken})` - ) - } - - return fn(updatedRepository, account) + return fn(updatedRepository) } private updateRevertProgress( @@ -6208,10 +6082,10 @@ export class AppStore extends TypedBaseStore { repository: Repository, commit: Commit ): Promise { - return this.withAuthenticatingUser(repository, async (repo, account) => { + return this.withAuthenticatingUser(repository, async repo => { const gitStore = this.gitStoreCache.get(repo) - await gitStore.revertCommit(repo, commit, account, progress => { + await gitStore.revertCommit(repo, commit, progress => { this.updateRevertProgress(repo, progress) }) @@ -7062,14 +6936,9 @@ export class AppStore extends TypedBaseStore { const checkoutSuccessful = await this.withAuthenticatingUser( repository, - (r, account) => { + r => { return gitStore.performFailableOperation(() => - checkoutBranch( - repository, - account, - targetBranch, - gitStore.currentRemote - ) + checkoutBranch(repository, targetBranch, gitStore.currentRemote) ) } ) @@ -7180,14 +7049,9 @@ export class AppStore extends TypedBaseStore { } const gitStore = this.gitStoreCache.get(repository) - await this.withAuthenticatingUser(repository, async (r, account) => { + await this.withAuthenticatingUser(repository, async r => { await gitStore.performFailableOperation(() => - checkoutBranch( - repository, - account, - sourceBranch, - gitStore.currentRemote - ) + checkoutBranch(repository, sourceBranch, gitStore.currentRemote) ) }) } diff --git a/app/src/lib/stores/git-store.ts b/app/src/lib/stores/git-store.ts index 34a615f307..2ad8865583 100644 --- a/app/src/lib/stores/git-store.ts +++ b/app/src/lib/stores/git-store.ts @@ -89,7 +89,6 @@ import { findDefaultRemote } from './helpers/find-default-remote' import { Author, isKnownAuthor } from '../../models/author' import { formatCommitMessage } from '../format-commit-message' import { GitAuthor } from '../../models/git-author' -import { IGitAccount } from '../../models/git-account' import { BaseStore } from './base-store' import { getStashes, getStashedFiles } from '../git/stash' import { IStashEntry, StashedChangesLoadStates } from '../../models/stash-entry' @@ -950,7 +949,6 @@ export class GitStore extends BaseStore { * the overall fetch progress. */ public async fetch( - account: IGitAccount | null, backgroundTask: boolean, progressCallback?: (fetchProgress: IFetchProgress) => void ): Promise { @@ -976,7 +974,6 @@ export class GitStore extends BaseStore { if (remotes.size > 0) { await this.fetchRemotes( - account, [...remotes.values()], backgroundTask, progressCallback @@ -1016,7 +1013,6 @@ export class GitStore extends BaseStore { * the overall fetch progress. */ public async fetchRemotes( - account: IGitAccount | null, remotes: ReadonlyArray, backgroundTask: boolean, progressCallback?: (fetchProgress: IFetchProgress) => void @@ -1031,7 +1027,7 @@ export class GitStore extends BaseStore { const remote = remotes[i] const startProgressValue = i * weight - await this.fetchRemote(account, remote, backgroundTask, progress => { + await this.fetchRemote(remote, backgroundTask, progress => { if (progress && progressCallback) { progressCallback({ ...progress, @@ -1052,7 +1048,6 @@ export class GitStore extends BaseStore { * the overall fetch progress. */ public async fetchRemote( - account: IGitAccount | null, remote: IRemote, backgroundTask: boolean, progressCallback?: (fetchProgress: IFetchProgress) => void @@ -1064,7 +1059,7 @@ export class GitStore extends BaseStore { } const fetchSucceeded = await this.performFailableOperation( async () => { - await fetchRepo(repo, account, remote, progressCallback, backgroundTask) + await fetchRepo(repo, remote, progressCallback, backgroundTask) return true }, { backgroundTask, retryAction } @@ -1079,7 +1074,7 @@ export class GitStore extends BaseStore { // Updating the local HEAD symref isn't critical so we don't want // to show an error message to the user and have them retry the // entire pull operation if it fails. - await updateRemoteHEAD(repo, account, remote, backgroundTask).catch(e => + await updateRemoteHEAD(repo, remote, backgroundTask).catch(e => log.error('Failed updating remote HEAD', e) ) } @@ -1093,16 +1088,13 @@ export class GitStore extends BaseStore { * part of this action. Refer to git-scm for more * information on refspecs: https://www.git-scm.com/book/tr/v2/Git-Internals-The-Refspec */ - public async fetchRefspec( - account: IGitAccount | null, - refspec: string - ): Promise { + public async fetchRefspec(refspec: string): Promise { // TODO: we should favour origin here const remotes = await getRemotes(this.repository) for (const remote of remotes) { await this.performFailableOperation(() => - fetchRefspec(this.repository, account, remote, refspec) + fetchRefspec(this.repository, remote, refspec) ) } } @@ -1618,17 +1610,10 @@ export class GitStore extends BaseStore { public async revertCommit( repository: Repository, commit: Commit, - account: IGitAccount | null, progressCallback?: (fetchProgress: IRevertProgress) => void ): Promise { await this.performFailableOperation(() => - revertCommit( - repository, - commit, - account, - this.currentRemote, - progressCallback - ) + revertCommit(repository, commit, this.currentRemote, progressCallback) ) this.emitUpdate() diff --git a/app/src/lib/stores/helpers/create-tutorial-repository.ts b/app/src/lib/stores/helpers/create-tutorial-repository.ts index dea55ed220..ceacd9e94f 100644 --- a/app/src/lib/stores/helpers/create-tutorial-repository.ts +++ b/app/src/lib/stores/helpers/create-tutorial-repository.ts @@ -73,7 +73,7 @@ async function pushRepo( const pushOpts = await executionOptionsWithProgress( { - env: await envForRemoteOperation(account, remote.url), + env: await envForRemoteOperation(remote.url), }, new PushProgressParser(), progress => { diff --git a/app/src/models/clone-options.ts b/app/src/models/clone-options.ts index b50cccf4e8..2138191b35 100644 --- a/app/src/models/clone-options.ts +++ b/app/src/models/clone-options.ts @@ -1,9 +1,5 @@ -import { IGitAccount } from './git-account' - /** Additional arguments to provide when cloning a repository */ export type CloneOptions = { - /** The optional identity to provide when cloning. */ - readonly account: IGitAccount | null /** The branch to checkout after the clone has completed. */ readonly branch?: string /** The default branch name in case we're cloning an empty repository. */ From 70dea38a020bdde6415d2cf69ef7183d0d4f9a55 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 16 May 2024 13:51:41 +0200 Subject: [PATCH 2/4] Intentionally shadow previously closed-over repository variable We don't want to accidentally reference it Co-Authored-By: Sergio Padrino <1083228+sergiou87@users.noreply.github.com> --- app/src/lib/stores/app-store.ts | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index 7d5c290389..baf5bc01f9 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -4228,8 +4228,8 @@ export class AppStore extends TypedBaseStore { includeUpstream?: boolean, toCheckout?: Branch | null ): Promise { - return this.withAuthenticatingUser(repository, async r => { - const gitStore = this.gitStoreCache.get(r) + return this.withAuthenticatingUser(repository, async repository => { + const gitStore = this.gitStoreCache.get(repository) // If solely a remote branch, there is no need to checkout a branch. if (branch.type === BranchType.Remote) { @@ -4253,7 +4253,7 @@ export class AppStore extends TypedBaseStore { } await gitStore.performFailableOperation(() => - deleteRemoteBranch(r, remote, nameWithoutRemote) + deleteRemoteBranch(repository, remote, nameWithoutRemote) ) // We log the remote branch's sha so that the user can recover it. @@ -4261,17 +4261,17 @@ export class AppStore extends TypedBaseStore { `Deleted branch ${branch.upstreamWithoutRemote} (was ${tip.sha})` ) - return this._refreshRepository(r) + return this._refreshRepository(repository) } // If a local branch, user may have the branch to delete checked out and // we need to switch to a different branch (default or recent). const branchToCheckout = - toCheckout ?? this.getBranchToCheckoutAfterDelete(branch, r) + toCheckout ?? this.getBranchToCheckoutAfterDelete(branch, repository) if (branchToCheckout !== null) { await gitStore.performFailableOperation(() => - checkoutBranch(r, branchToCheckout, gitStore.currentRemote) + checkoutBranch(repository, branchToCheckout, gitStore.currentRemote) ) } @@ -4283,7 +4283,7 @@ export class AppStore extends TypedBaseStore { ) }) - return this._refreshRepository(r) + return this._refreshRepository(repository) }) } @@ -6082,14 +6082,14 @@ export class AppStore extends TypedBaseStore { repository: Repository, commit: Commit ): Promise { - return this.withAuthenticatingUser(repository, async repo => { - const gitStore = this.gitStoreCache.get(repo) + return this.withAuthenticatingUser(repository, async repository => { + const gitStore = this.gitStoreCache.get(repository) - await gitStore.revertCommit(repo, commit, progress => { - this.updateRevertProgress(repo, progress) + await gitStore.revertCommit(repository, commit, progress => { + this.updateRevertProgress(repository, progress) }) - this.updateRevertProgress(repo, null) + this.updateRevertProgress(repository, null) await this._refreshRepository(repository) }) } @@ -6936,7 +6936,7 @@ export class AppStore extends TypedBaseStore { const checkoutSuccessful = await this.withAuthenticatingUser( repository, - r => { + repository => { return gitStore.performFailableOperation(() => checkoutBranch(repository, targetBranch, gitStore.currentRemote) ) @@ -7049,7 +7049,7 @@ export class AppStore extends TypedBaseStore { } const gitStore = this.gitStoreCache.get(repository) - await this.withAuthenticatingUser(repository, async r => { + await this.withAuthenticatingUser(repository, async repository => { await gitStore.performFailableOperation(() => checkoutBranch(repository, sourceBranch, gitStore.currentRemote) ) From 19e6353338e054af1d871cabacd158085dc38dd7 Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 16 May 2024 13:55:41 +0200 Subject: [PATCH 3/4] Bettererer naming Co-Authored-By: Sergio Padrino <1083228+sergiou87@users.noreply.github.com> --- app/src/lib/stores/app-store.ts | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/app/src/lib/stores/app-store.ts b/app/src/lib/stores/app-store.ts index baf5bc01f9..a59175a119 100644 --- a/app/src/lib/stores/app-store.ts +++ b/app/src/lib/stores/app-store.ts @@ -3546,7 +3546,7 @@ export class AppStore extends TypedBaseStore { * of the current branch to its upstream tracking branch. */ private fetchForRepositoryIndicator(repo: Repository) { - return this.withAuthenticatingUser(repo, async repo => { + return this.withRefreshedGitHubRepository(repo, async repo => { const isBackgroundTask = true const gitStore = this.gitStoreCache.get(repo) @@ -3859,7 +3859,7 @@ export class AppStore extends TypedBaseStore { } } - return this.withAuthenticatingUser(repository, repository => { + return this.withRefreshedGitHubRepository(repository, repository => { // We always want to end with refreshing the repository regardless of // whether the checkout succeeded or not in order to present the most // up-to-date information to the user. @@ -4026,7 +4026,7 @@ export class AppStore extends TypedBaseStore { return repository } - return this.withAuthenticatingUser(repository, repository => { + return this.withRefreshedGitHubRepository(repository, repository => { // We always want to end with refreshing the repository regardless of // whether the checkout succeeded or not in order to present the most // up-to-date information to the user. @@ -4228,7 +4228,7 @@ export class AppStore extends TypedBaseStore { includeUpstream?: boolean, toCheckout?: Branch | null ): Promise { - return this.withAuthenticatingUser(repository, async repository => { + return this.withRefreshedGitHubRepository(repository, async repository => { const gitStore = this.gitStoreCache.get(repository) // If solely a remote branch, there is no need to checkout a branch. @@ -4365,7 +4365,7 @@ export class AppStore extends TypedBaseStore { repository: Repository, options?: PushOptions ): Promise { - return this.withAuthenticatingUser(repository, repository => { + return this.withRefreshedGitHubRepository(repository, repository => { return this.performPush(repository, options) }) } @@ -4593,7 +4593,7 @@ export class AppStore extends TypedBaseStore { } public async _pull(repository: Repository): Promise { - return this.withAuthenticatingUser(repository, repository => { + return this.withRefreshedGitHubRepository(repository, repository => { return this.performPull(repository) }) } @@ -4991,7 +4991,7 @@ export class AppStore extends TypedBaseStore { repository: Repository, refspec: string ): Promise { - return this.withAuthenticatingUser(repository, async repository => { + return this.withRefreshedGitHubRepository(repository, async repository => { const gitStore = this.gitStoreCache.get(repository) await gitStore.fetchRefspec(refspec) @@ -5008,7 +5008,7 @@ export class AppStore extends TypedBaseStore { * if _any_ fetches or pulls are currently in-progress. */ public _fetch(repository: Repository, fetchType: FetchType): Promise { - return this.withAuthenticatingUser(repository, repository => { + return this.withRefreshedGitHubRepository(repository, repository => { return this.performFetch(repository, fetchType) }) } @@ -5024,7 +5024,7 @@ export class AppStore extends TypedBaseStore { remote: IRemote, fetchType: FetchType ): Promise { - return this.withAuthenticatingUser(repository, repository => { + return this.withRefreshedGitHubRepository(repository, repository => { return this.performFetch(repository, fetchType, [remote]) }) } @@ -6041,7 +6041,7 @@ export class AppStore extends TypedBaseStore { }` } - private async withAuthenticatingUser( + private async withRefreshedGitHubRepository( repository: Repository, fn: (repository: Repository) => Promise ): Promise { @@ -6082,7 +6082,7 @@ export class AppStore extends TypedBaseStore { repository: Repository, commit: Commit ): Promise { - return this.withAuthenticatingUser(repository, async repository => { + return this.withRefreshedGitHubRepository(repository, async repository => { const gitStore = this.gitStoreCache.get(repository) await gitStore.revertCommit(repository, commit, progress => { @@ -6934,7 +6934,7 @@ export class AppStore extends TypedBaseStore { ): Promise { const gitStore = this.gitStoreCache.get(repository) - const checkoutSuccessful = await this.withAuthenticatingUser( + const checkoutSuccessful = await this.withRefreshedGitHubRepository( repository, repository => { return gitStore.performFailableOperation(() => @@ -7049,7 +7049,7 @@ export class AppStore extends TypedBaseStore { } const gitStore = this.gitStoreCache.get(repository) - await this.withAuthenticatingUser(repository, async repository => { + await this.withRefreshedGitHubRepository(repository, async repository => { await gitStore.performFailableOperation(() => checkoutBranch(repository, sourceBranch, gitStore.currentRemote) ) From 6b575c7b82952d8becc8b53b0f09e5b58d02feac Mon Sep 17 00:00:00 2001 From: Markus Olsson Date: Thu, 16 May 2024 14:01:53 +0200 Subject: [PATCH 4/4] We no longer need to pass an account Co-Authored-By: Sergio Padrino <1083228+sergiou87@users.noreply.github.com> --- app/test/unit/git/branch-test.ts | 6 +-- app/test/unit/git/checkout-test.ts | 12 ++--- .../unit/git/pull/ahead-and-behind-test.ts | 10 ++-- app/test/unit/git/pull/only-ahead-test.ts | 8 ++-- app/test/unit/git/pull/only-behind-test.ts | 6 +-- app/test/unit/git/reflog-test.ts | 2 +- app/test/unit/git/submodule-test.ts | 4 +- app/test/unit/git/tag-test.ts | 46 ++++++------------- 8 files changed, 38 insertions(+), 56 deletions(-) diff --git a/app/test/unit/git/branch-test.ts b/app/test/unit/git/branch-test.ts index af152c356d..81765d5175 100644 --- a/app/test/unit/git/branch-test.ts +++ b/app/test/unit/git/branch-test.ts @@ -216,7 +216,7 @@ describe('git/branch', () => { const [remoteBranch] = await getBranches(mockLocal, remoteRef) expect(remoteBranch).not.toBeUndefined() - await checkoutBranch(mockLocal, null, remoteBranch, null) + await checkoutBranch(mockLocal, remoteBranch, null) await git(['checkout', '-'], mockLocal.path, 'checkoutPrevious') expect(await getBranches(mockLocal, localRef)).toBeArrayOfSize(1) @@ -229,7 +229,6 @@ describe('git/branch', () => { await deleteRemoteBranch( mockLocal, - null, { name: localBranch.upstreamRemoteName!, url: '' }, localBranch.upstreamWithoutRemote! ) @@ -253,7 +252,7 @@ describe('git/branch', () => { const [remoteBranch] = await getBranches(mockLocal, remoteRef) expect(remoteBranch).not.toBeUndefined() - await checkoutBranch(mockLocal, null, remoteBranch, null) + await checkoutBranch(mockLocal, remoteBranch, null) await git(['checkout', '-'], mockLocal.path, 'checkoutPrevious') expect(await getBranches(mockLocal, localRef)).toBeArrayOfSize(1) @@ -271,7 +270,6 @@ describe('git/branch', () => { await deleteRemoteBranch( mockLocal, - null, { name: localBranch.upstreamRemoteName!, url: '' }, localBranch.upstreamWithoutRemote! ) diff --git a/app/test/unit/git/checkout-test.ts b/app/test/unit/git/checkout-test.ts index 0a1be6b536..e7bc635305 100644 --- a/app/test/unit/git/checkout-test.ts +++ b/app/test/unit/git/checkout-test.ts @@ -52,7 +52,7 @@ describe('git/checkout', () => { let errorRaised = false try { - await checkoutBranch(repository, null, branch, null) + await checkoutBranch(repository, branch, null) } catch (error) { errorRaised = true expect(error.message).toBe('fatal: invalid reference: ..\n') @@ -74,7 +74,7 @@ describe('git/checkout', () => { throw new Error(`Could not find branch: commit-with-long-description`) } - await checkoutBranch(repository, null, branches[0], null) + await checkoutBranch(repository, branches[0], null) const store = new GitStore(repository, shell, statsStore) await store.loadStatus() @@ -109,7 +109,7 @@ describe('git/checkout', () => { throw new Error(`Could not find branch: '${secondBranch}'`) } - await checkoutBranch(repository, null, firstRemoteBranch, null) + await checkoutBranch(repository, firstRemoteBranch, null) const store = new GitStore(repository, shell, statsStore) await store.loadStatus() @@ -143,7 +143,7 @@ describe('git/checkout', () => { let errorRaised = false try { - await checkoutBranch(repository, null, remoteBranch, null) + await checkoutBranch(repository, remoteBranch, null) } catch (error) { errorRaised = true expect(error.message).toBe('A branch with that name already exists.') @@ -170,7 +170,7 @@ describe('git/checkout', () => { throw new Error(`Could not find branch: 'master'`) } - await checkoutBranch(repository, null, masterBranch, null) + await checkoutBranch(repository, masterBranch, null) const status = await getStatusOrThrow(repository) @@ -194,7 +194,7 @@ describe('git/checkout', () => { throw new Error(`Could not find branch: 'dev'`) } - await checkoutBranch(repository, null, devBranch, null) + await checkoutBranch(repository, devBranch, null) const status = await getStatusOrThrow(repository) expect(status.workingDirectory.files).toHaveLength(0) diff --git a/app/test/unit/git/pull/ahead-and-behind-test.ts b/app/test/unit/git/pull/ahead-and-behind-test.ts index 04e595135b..cc262ac96b 100644 --- a/app/test/unit/git/pull/ahead-and-behind-test.ts +++ b/app/test/unit/git/pull/ahead-and-behind-test.ts @@ -52,7 +52,7 @@ describe('git/pull', () => { } await makeCommit(repository, changesForLocalRepository) - await fetch(repository, null, remote) + await fetch(repository, remote) }) describe('with pull.rebase=false and pull.ff=false set in config', () => { @@ -67,7 +67,7 @@ describe('git/pull', () => { previousTip = await getTipOrError(repository) - await pull(repository, null, remote) + await pull(repository, remote) newTip = await getTipOrError(repository) }) @@ -102,7 +102,7 @@ describe('git/pull', () => { previousTip = await getTipOrError(repository) - await pull(repository, null, remote) + await pull(repository, remote) newTip = await getTipOrError(repository) }) @@ -132,7 +132,7 @@ describe('git/pull', () => { previousTip = await getTipOrError(repository) - await pull(repository, null, remote) + await pull(repository, remote) newTip = await getTipOrError(repository) }) @@ -162,7 +162,7 @@ describe('git/pull', () => { }) it(`throws an error as the user blocks merge commits on pull`, () => { - expect(pull(repository, null, remote)).rejects.toThrow() + expect(pull(repository, remote)).rejects.toThrow() }) }) }) diff --git a/app/test/unit/git/pull/only-ahead-test.ts b/app/test/unit/git/pull/only-ahead-test.ts index a09020445a..363d4e0c0c 100644 --- a/app/test/unit/git/pull/only-ahead-test.ts +++ b/app/test/unit/git/pull/only-ahead-test.ts @@ -40,7 +40,7 @@ describe('git/pull', () => { } await makeCommit(repository, changesForLocalRepository) - await fetch(repository, null, remote) + await fetch(repository, remote) }) describe('by default', () => { @@ -50,7 +50,7 @@ describe('git/pull', () => { beforeEach(async () => { previousTip = await getTipOrError(repository) - await pull(repository, null, remote) + await pull(repository, remote) newTip = await getTipOrError(repository) }) @@ -85,7 +85,7 @@ describe('git/pull', () => { previousTip = await getTipOrError(repository) - await pull(repository, null, remote) + await pull(repository, remote) newTip = await getTipOrError(repository) }) @@ -119,7 +119,7 @@ describe('git/pull', () => { previousTip = await getTipOrError(repository) - await pull(repository, null, remote) + await pull(repository, remote) newTip = await getTipOrError(repository) }) diff --git a/app/test/unit/git/pull/only-behind-test.ts b/app/test/unit/git/pull/only-behind-test.ts index d02dec8843..cf9b66a48c 100644 --- a/app/test/unit/git/pull/only-behind-test.ts +++ b/app/test/unit/git/pull/only-behind-test.ts @@ -52,7 +52,7 @@ describe('git/pull', () => { await makeCommit(remoteRepository, firstCommit) await makeCommit(remoteRepository, secondCommit) - await fetch(repository, null, remote) + await fetch(repository, remote) }) describe('with pull.rebase=false and pull.ff=false set in config', () => { @@ -67,7 +67,7 @@ describe('git/pull', () => { previousTip = await getTipOrError(repository) - await pull(repository, null, remote) + await pull(repository, remote) newTip = await getTipOrError(repository) }) @@ -102,7 +102,7 @@ describe('git/pull', () => { previousTip = await getTipOrError(repository) - await pull(repository, null, remote) + await pull(repository, remote) newTip = await getTipOrError(repository) }) diff --git a/app/test/unit/git/reflog-test.ts b/app/test/unit/git/reflog-test.ts index 0cc477d575..65533b64bc 100644 --- a/app/test/unit/git/reflog-test.ts +++ b/app/test/unit/git/reflog-test.ts @@ -20,7 +20,7 @@ async function createAndCheckout( if (branch === undefined) { throw new Error(`Unable to create branch: ${name}`) } - await checkoutBranch(repository, null, branch, null) + await checkoutBranch(repository, branch, null) } describe('git/reflog', () => { diff --git a/app/test/unit/git/submodule-test.ts b/app/test/unit/git/submodule-test.ts index e611393968..b6edc76dbc 100644 --- a/app/test/unit/git/submodule-test.ts +++ b/app/test/unit/git/submodule-test.ts @@ -37,7 +37,7 @@ describe('git/submodule', () => { throw new Error(`Could not find branch: feature-branch`) } - await checkoutBranch(submoduleRepository, null, branches[0], null) + await checkoutBranch(submoduleRepository, branches[0], null) const result = await listSubmodules(repository) expect(result).toHaveLength(1) @@ -64,7 +64,7 @@ describe('git/submodule', () => { throw new Error(`Could not find branch: feature-branch`) } - await checkoutBranch(submoduleRepository, null, branches[0], null) + await checkoutBranch(submoduleRepository, branches[0], null) let result = await listSubmodules(repository) expect(result[0].describe).toBe('heads/feature-branch') diff --git a/app/test/unit/git/tag-test.ts b/app/test/unit/git/tag-test.ts index b344ea405f..21c4e9d2d0 100644 --- a/app/test/unit/git/tag-test.ts +++ b/app/test/unit/git/tag-test.ts @@ -19,8 +19,6 @@ import { setupFixtureRepository, setupLocalForkOfRepository, } from '../../helpers/repositories' -import { Account } from '../../../src/models/account' -import { getDotComAPIEndpoint } from '../../../src/lib/api' import { IRemote } from '../../../src/models/remote' import { findDefaultRemote } from '../../../src/lib/stores/helpers/find-default-remote' import { getStatusOrThrow } from '../../helpers/status' @@ -28,22 +26,10 @@ import { assertNonNullable } from '../../../src/lib/fatal-error' describe('git/tag', () => { let repository: Repository - let account: Account beforeEach(async () => { const testRepoPath = await setupFixtureRepository('test-repo') repository = new Repository(testRepoPath, -1, null, false) - - account = new Account( - 'monalisa', - getDotComAPIEndpoint(), - '', - [], - '', - -1, - 'Mona Lisa', - 'free' - ) }) describe('createTag', () => { @@ -138,28 +124,26 @@ describe('git/tag', () => { it('returns an empty array when there are no tags to get pushed', async () => { expect( - await fetchTagsToPush(repository, account, originRemote, 'master') + await fetchTagsToPush(repository, originRemote, 'master') ).toIncludeAllMembers([]) }) it("returns local tags that haven't been pushed", async () => { await createTag(repository, 'my-new-tag', 'HEAD') - expect( - await fetchTagsToPush(repository, account, originRemote, 'master') - ).toEqual(['my-new-tag']) + expect(await fetchTagsToPush(repository, originRemote, 'master')).toEqual( + ['my-new-tag'] + ) }) it('returns an empty array after pushing the tag', async () => { await createTag(repository, 'my-new-tag', 'HEAD') - await push(repository, account, originRemote, 'master', null, [ - 'my-new-tag', - ]) + await push(repository, originRemote, 'master', null, ['my-new-tag']) - expect( - await fetchTagsToPush(repository, account, originRemote, 'master') - ).toEqual([]) + expect(await fetchTagsToPush(repository, originRemote, 'master')).toEqual( + [] + ) }) it('does not return a tag created on a non-pushed branch', async () => { @@ -173,13 +157,13 @@ describe('git/tag', () => { const status = await getStatusOrThrow(repository) const files = status.workingDirectory.files - await checkoutBranch(repository, account, branch!, null) + await checkoutBranch(repository, branch!, null) const commitSha = await createCommit(repository, 'a commit', files) await createTag(repository, 'my-new-tag', commitSha) - expect( - await fetchTagsToPush(repository, account, originRemote, 'master') - ).toEqual([]) + expect(await fetchTagsToPush(repository, originRemote, 'master')).toEqual( + [] + ) }) it('returns unpushed tags even if it fails to push the branch', async () => { @@ -195,9 +179,9 @@ describe('git/tag', () => { await createTag(repository, 'my-new-tag', 'HEAD') - expect( - await fetchTagsToPush(repository, account, originRemote, 'master') - ).toEqual(['my-new-tag']) + expect(await fetchTagsToPush(repository, originRemote, 'master')).toEqual( + ['my-new-tag'] + ) }) }) })