diff --git a/src/lib/__snapshots__/cherrypickAndCreateTargetPullRequest.test.ts.snap b/src/lib/__snapshots__/cherrypickAndCreateTargetPullRequest.test.ts.snap index 7e66b386..73e5877a 100644 --- a/src/lib/__snapshots__/cherrypickAndCreateTargetPullRequest.test.ts.snap +++ b/src/lib/__snapshots__/cherrypickAndCreateTargetPullRequest.test.ts.snap @@ -100,9 +100,9 @@ Array [ "git", Array [ "-c", - "user.name=\\"Søren Louv-Jansen\\"", + "user.name=\\"Soren L\\"", "-c", - "user.email=\\"soren.louv@elastic.co\\"", + "user.email=\\"soren@louv.dk\\"", "cherry-pick", "-x", "mySha", @@ -112,6 +112,10 @@ Array [ Array [ "git", Array [ + "-c", + "user.name=\\"Soren L\\"", + "-c", + "user.email=\\"soren@louv.dk\\"", "commit", "--no-edit", ], @@ -121,9 +125,9 @@ Array [ "git", Array [ "-c", - "user.name=\\"Søren Louv-Jansen\\"", + "user.name=\\"Soren L\\"", "-c", - "user.email=\\"soren.louv@elastic.co\\"", + "user.email=\\"soren@louv.dk\\"", "cherry-pick", "-x", "mySha2", @@ -133,6 +137,10 @@ Array [ Array [ "git", Array [ + "-c", + "user.name=\\"Soren L\\"", + "-c", + "user.email=\\"soren@louv.dk\\"", "commit", "--no-edit", ], diff --git a/src/lib/author.ts b/src/lib/author.ts new file mode 100644 index 00000000..706ad81f --- /dev/null +++ b/src/lib/author.ts @@ -0,0 +1,42 @@ +import { Commit } from '../entrypoint.module'; +import { ValidConfigOptions } from '../options/options'; +import { getGitConfig, getLocalSourceRepoPath } from './git'; + +export type GitConfigAuthor = { name?: string; email?: string }; +export async function getGitConfigAuthor( + options: ValidConfigOptions +): Promise { + const localRepoPath = await getLocalSourceRepoPath(options); + if (!localRepoPath) { + return; + } + + return { + name: await getGitConfig({ dir: localRepoPath, key: 'user.name' }), + email: await getGitConfig({ dir: localRepoPath, key: 'user.email' }), + }; +} + +export type CommitAuthor = Required; +export function getCommitAuthor({ + options, + commit, + gitConfigAuthor, +}: { + options: ValidConfigOptions; + commit: Commit; + gitConfigAuthor?: GitConfigAuthor; +}): CommitAuthor { + if (options.resetAuthor) { + return { + name: options.authenticatedUsername, + email: `<${options.authenticatedUsername}@users.noreply.github.com>`, + }; + } + + return { + name: options.gitAuthorName ?? gitConfigAuthor?.name ?? commit.author.name, + email: + options.gitAuthorEmail ?? gitConfigAuthor?.email ?? commit.author.email, + }; +} diff --git a/src/lib/cherrypickAndCreateTargetPullRequest.test.ts b/src/lib/cherrypickAndCreateTargetPullRequest.test.ts index 8b74126d..0e4be085 100644 --- a/src/lib/cherrypickAndCreateTargetPullRequest.test.ts +++ b/src/lib/cherrypickAndCreateTargetPullRequest.test.ts @@ -47,6 +47,8 @@ describe('cherrypickAndCreateTargetPullRequest', () => { const options = { assignees: [] as string[], authenticatedUsername: 'sqren_authenticated', + gitAuthorName: 'Soren L', + gitAuthorEmail: 'soren@louv.dk', author: 'sqren', fork: true, interactive: true, diff --git a/src/lib/cherrypickAndCreateTargetPullRequest.ts b/src/lib/cherrypickAndCreateTargetPullRequest.ts index a95f8964..392e3307 100644 --- a/src/lib/cherrypickAndCreateTargetPullRequest.ts +++ b/src/lib/cherrypickAndCreateTargetPullRequest.ts @@ -2,10 +2,15 @@ import chalk from 'chalk'; import { isEmpty, difference, flatten } from 'lodash'; import { ValidConfigOptions } from '../options/options'; import { BackportError } from './BackportError'; +import { + CommitAuthor, + getCommitAuthor, + getGitConfigAuthor, + GitConfigAuthor, +} from './author'; import { spawnPromise } from './child-process-promisified'; import { getRepoPath } from './env'; import { getCommitsWithoutBackports } from './getCommitsWithoutBackports'; -import { getGitConfigAuthor, GitConfigAuthor } from './getGitConfigAuthor'; import { cherrypick, createBackportBranch, @@ -200,20 +205,21 @@ async function waitForCherrypick( getFirstLine(commit.sourceCommit.message) )}`; const cherrypickSpinner = ora(options.interactive, spinnerText).start(); + const commitAuthor = getCommitAuthor({ options, commit, gitConfigAuthor }); - await cherrypickAndHandleConflicts( + await cherrypickAndHandleConflicts({ options, commit, + commitAuthor, targetBranch, cherrypickSpinner, - gitConfigAuthor - ); + }); // Conflicts should be resolved and files staged at this point try { - // Run `git commit` - await commitChanges(commit, options); + // Run `git commit` in case conflicts were not manually committed + await commitChanges({ options, commit, commitAuthor }); cherrypickSpinner.succeed(); } catch (e) { cherrypickSpinner.fail(); @@ -221,17 +227,22 @@ async function waitForCherrypick( } } -async function cherrypickAndHandleConflicts( - options: ValidConfigOptions, - commit: Commit, - targetBranch: string, - cherrypickSpinner: Ora, - gitConfigAuthor?: GitConfigAuthor -) { +async function cherrypickAndHandleConflicts({ + options, + commit, + commitAuthor, + targetBranch, + cherrypickSpinner, +}: { + options: ValidConfigOptions; + commit: Commit; + commitAuthor: CommitAuthor; + targetBranch: string; + cherrypickSpinner: Ora; +}) { const mergedTargetPullRequest = commit.expectedTargetPullRequests.find( (pr) => pr.state === 'MERGED' && pr.branch === targetBranch ); - const commitAuthor = getCommitAuthor({ options, gitConfigAuthor, commit }); let conflictingFiles: ConflictingFiles; let unstagedFiles: string[]; @@ -398,26 +409,3 @@ async function listConflictingAndUnstagedFiles({ unstagedFiles: _unstagedFiles, }); } - -function getCommitAuthor({ - options, - gitConfigAuthor, - commit, -}: { - options: ValidConfigOptions; - gitConfigAuthor?: GitConfigAuthor; - commit: Commit; -}) { - if (options.resetAuthor) { - return { - name: options.authenticatedUsername, - email: `<${options.authenticatedUsername}@users.noreply.github.com>`, - }; - } - - return { - name: options.gitAuthorName ?? gitConfigAuthor?.name ?? commit.author.name, - email: - options.gitAuthorEmail ?? gitConfigAuthor?.email ?? commit.author.email, - }; -} diff --git a/src/lib/getGitConfigAuthor.ts b/src/lib/getGitConfigAuthor.ts deleted file mode 100644 index 9471ffec..00000000 --- a/src/lib/getGitConfigAuthor.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { ValidConfigOptions } from '../options/options'; -import { getGitConfig, getLocalSourceRepoPath } from './git'; - -export type GitConfigAuthor = { - name?: string; - email?: string; -}; - -export async function getGitConfigAuthor( - options: ValidConfigOptions -): Promise { - const localRepoPath = await getLocalSourceRepoPath(options); - if (!localRepoPath) { - return; - } - - return { - name: await getGitConfig({ dir: localRepoPath, key: 'user.name' }), - email: await getGitConfig({ dir: localRepoPath, key: 'user.email' }), - }; -} diff --git a/src/lib/git.private.test.ts b/src/lib/git.private.test.ts index 4bf25881..1d88ade8 100644 --- a/src/lib/git.private.test.ts +++ b/src/lib/git.private.test.ts @@ -354,7 +354,7 @@ describe('git.integration', () => { }); expect(async () => { - return await commitChanges(commit, options); + return await commitChanges({ commit, commitAuthor, options }); }).not.toThrowError(); const message = await getMostRecentCommitMessage(cwd); @@ -374,7 +374,7 @@ describe('git.integration', () => { cwd, }); - await commitChanges(commit, options); + await commitChanges({ commit, commitAuthor, options }); const message = await getMostRecentCommitMessage(cwd); expect(message).toBe('my fallback commit message'); @@ -407,7 +407,7 @@ describe('git.integration', () => { // disregard conflicts and stage all files await childProcess.exec('git add -A', { cwd }); - await commitChanges(commit, options); + await commitChanges({ commit, commitAuthor, options }); const message = await getMostRecentCommitMessage(cwd); expect(message).toMatchInlineSnapshot(` diff --git a/src/lib/git.test.ts b/src/lib/git.test.ts index 891f3e9b..e6fe901a 100644 --- a/src/lib/git.test.ts +++ b/src/lib/git.test.ts @@ -596,7 +596,9 @@ describe('commitChanges', () => { .spyOn(childProcess, 'spawnPromise') .mockResolvedValueOnce({ stderr: '', stdout: '', code: 0, cmdArgs: [] }); - await expect(await commitChanges(commit, options)).toBe(undefined); + await expect(await commitChanges({ commit, commitAuthor, options })).toBe( + undefined + ); }); it('should swallow error if changes have already been committed manaully', async () => { @@ -609,7 +611,9 @@ describe('commitChanges', () => { }); jest.spyOn(childProcess, 'spawnPromise').mockRejectedValueOnce(err); - await expect(await commitChanges(commit, options)).toBe(undefined); + await expect(await commitChanges({ commit, commitAuthor, options })).toBe( + undefined + ); }); describe('when commit fails due to empty message', () => { @@ -633,21 +637,36 @@ describe('commitChanges', () => { cmdArgs: [], }); - res = await commitChanges(commit, options); + res = await commitChanges({ commit, commitAuthor, options }); }); it('should manually set the commit message', () => { expect(spy).toHaveBeenCalledTimes(2); - expect(spy).toHaveBeenCalledWith( + expect(spy.mock.calls[0]).toEqual([ 'git', - ['commit', '--no-edit'], - expect.any(String) - ); - expect(spy).toHaveBeenCalledWith( + [ + '-c', + 'user.name="Soren L"', + '-c', + 'user.email="soren@mail.dk"', + 'commit', + '--no-edit', + ], + expect.any(String), + ]); + + expect(spy.mock.calls[1]).toEqual([ 'git', - ['commit', '--message=The original commit message'], - expect.any(String) - ); + [ + '-c', + 'user.name="Soren L"', + '-c', + 'user.email="soren@mail.dk"', + 'commit', + '--message=The original commit message', + ], + expect.any(String), + ]); }); it('should handle the error and resolve successfully', async () => { @@ -661,7 +680,7 @@ describe('commitChanges', () => { expect.assertions(1); await expect( - commitChanges(commit, options) + commitChanges({ commit, commitAuthor, options }) ).rejects.toThrowErrorMatchingInlineSnapshot(`"another error"`); }); }); diff --git a/src/lib/git.ts b/src/lib/git.ts index 0a083143..5a960927 100644 --- a/src/lib/git.ts +++ b/src/lib/git.ts @@ -4,6 +4,7 @@ import { ora } from '../lib/ora'; import { ValidConfigOptions } from '../options/options'; import { filterNil } from '../utils/filterEmpty'; import { BackportError } from './BackportError'; +import { CommitAuthor } from './author'; import { spawnPromise, SpawnError, @@ -288,16 +289,16 @@ export async function cherrypick({ options: ValidConfigOptions; sha: string; mergedTargetPullRequest?: ExpectedTargetPullRequest; - commitAuthor: { name: string; email: string }; + commitAuthor: CommitAuthor; }): Promise<{ conflictingFiles: { absolute: string; relative: string }[]; unstagedFiles: string[]; needsResolving: boolean; }> { const cmdArgs = [ - '-c', + `-c`, `user.name="${commitAuthor.name}"`, - '-c', + `-c`, `user.email="${commitAuthor.email}"`, `cherry-pick`, ...(options.mainline != undefined @@ -359,10 +360,15 @@ export async function cherrypick({ } } -export async function commitChanges( - commit: Commit, - options: ValidConfigOptions -) { +export async function commitChanges({ + options, + commit, + commitAuthor, +}: { + options: ValidConfigOptions; + commit: Commit; + commitAuthor: CommitAuthor; +}) { const noVerifyFlag = options.noVerify ? ['--no-verify'] : []; const cwd = getRepoPath(options); @@ -370,6 +376,10 @@ export async function commitChanges( await spawnPromise( 'git', [ + `-c`, + `user.name="${commitAuthor.name}"`, + `-c`, + `user.email="${commitAuthor.email}"`, 'commit', '--no-edit', // Use the selected commit message without launching an editor. ...noVerifyFlag, // bypass pre-commit and commit-msg hooks @@ -396,6 +406,10 @@ export async function commitChanges( await spawnPromise( 'git', [ + `-c`, + `user.name="${commitAuthor.name}"`, + `-c`, + `user.email="${commitAuthor.email}"`, 'commit', `--message=${commit.sourceCommit.message}`, ...noVerifyFlag, // bypass pre-commit and commit-msg hooks