Skip to content

Commit d7db273

Browse files
peter-evanspeter-evans
authored
fix: handle update after force pushing base to a new commit (#1307)
1 parent ee93d78 commit d7db273

File tree

3 files changed

+188
-13
lines changed

3 files changed

+188
-13
lines changed

‎__test__/create-or-update-branch.int.test.ts

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,69 @@ describe('create-or-update-branch tests', () => {
631631
).toBeTruthy()
632632
})
633633

634+
it('tests create, force push of base branch, and update with identical changes', async () => {
635+
// If the base branch is force pushed to a different commit when there is an open
636+
// pull request, the branch must be reset to rebase the changes on the base.
637+
638+
// Create tracked and untracked file changes
639+
const changes = await createChanges()
640+
const commitMessage = uuidv4()
641+
const result = await createOrUpdateBranch(
642+
git,
643+
commitMessage,
644+
'',
645+
BRANCH,
646+
REMOTE_NAME,
647+
false,
648+
ADD_PATHS_DEFAULT
649+
)
650+
expect(result.action).toEqual('created')
651+
expect(await getFileContent(TRACKED_FILE)).toEqual(changes.tracked)
652+
expect(await getFileContent(UNTRACKED_FILE)).toEqual(changes.untracked)
653+
expect(
654+
await gitLogMatches([commitMessage, INIT_COMMIT_MESSAGE])
655+
).toBeTruthy()
656+
657+
// Push pull request branch to remote
658+
await git.push([
659+
'--force-with-lease',
660+
REMOTE_NAME,
661+
`HEAD:refs/heads/${BRANCH}`
662+
])
663+
664+
await afterTest(false)
665+
await beforeTest()
666+
667+
// Force push the base branch to a different commit
668+
const amendedCommitMessage = uuidv4()
669+
await git.commit(['--amend', '-m', amendedCommitMessage])
670+
await git.push([
671+
'--force',
672+
REMOTE_NAME,
673+
`HEAD:refs/heads/${DEFAULT_BRANCH}`
674+
])
675+
676+
// Create the same tracked and untracked file changes (no change on update)
677+
const _changes = await createChanges(changes.tracked, changes.untracked)
678+
const _commitMessage = uuidv4()
679+
const _result = await createOrUpdateBranch(
680+
git,
681+
_commitMessage,
682+
'',
683+
BRANCH,
684+
REMOTE_NAME,
685+
false,
686+
ADD_PATHS_DEFAULT
687+
)
688+
expect(_result.action).toEqual('updated')
689+
expect(_result.hasDiffWithBase).toBeTruthy()
690+
expect(await getFileContent(TRACKED_FILE)).toEqual(_changes.tracked)
691+
expect(await getFileContent(UNTRACKED_FILE)).toEqual(_changes.untracked)
692+
expect(
693+
await gitLogMatches([_commitMessage, amendedCommitMessage])
694+
).toBeTruthy()
695+
})
696+
634697
it('tests create and update with commits on the working base (during the workflow)', async () => {
635698
// Create commits on the working base
636699
const commits = await createCommits(git)
@@ -1519,6 +1582,75 @@ describe('create-or-update-branch tests', () => {
15191582
).toBeTruthy()
15201583
})
15211584

1585+
it('tests create, force push of base branch, and update with identical changes (WBNB)', async () => {
1586+
// If the base branch is force pushed to a different commit when there is an open
1587+
// pull request, the branch must be reset to rebase the changes on the base.
1588+
1589+
// Set the working base to a branch that is not the pull request base
1590+
await git.checkout(NOT_BASE_BRANCH)
1591+
1592+
// Create tracked and untracked file changes
1593+
const changes = await createChanges()
1594+
const commitMessage = uuidv4()
1595+
const result = await createOrUpdateBranch(
1596+
git,
1597+
commitMessage,
1598+
BASE,
1599+
BRANCH,
1600+
REMOTE_NAME,
1601+
false,
1602+
ADD_PATHS_DEFAULT
1603+
)
1604+
expect(result.action).toEqual('created')
1605+
expect(await getFileContent(TRACKED_FILE)).toEqual(changes.tracked)
1606+
expect(await getFileContent(UNTRACKED_FILE)).toEqual(changes.untracked)
1607+
expect(
1608+
await gitLogMatches([commitMessage, INIT_COMMIT_MESSAGE])
1609+
).toBeTruthy()
1610+
1611+
// Push pull request branch to remote
1612+
await git.push([
1613+
'--force-with-lease',
1614+
REMOTE_NAME,
1615+
`HEAD:refs/heads/${BRANCH}`
1616+
])
1617+
1618+
await afterTest(false)
1619+
await beforeTest()
1620+
1621+
// Force push the base branch to a different commit
1622+
const amendedCommitMessage = uuidv4()
1623+
await git.commit(['--amend', '-m', amendedCommitMessage])
1624+
await git.push([
1625+
'--force',
1626+
REMOTE_NAME,
1627+
`HEAD:refs/heads/${DEFAULT_BRANCH}`
1628+
])
1629+
1630+
// Set the working base to a branch that is not the pull request base
1631+
await git.checkout(NOT_BASE_BRANCH)
1632+
1633+
// Create the same tracked and untracked file changes (no change on update)
1634+
const _changes = await createChanges(changes.tracked, changes.untracked)
1635+
const _commitMessage = uuidv4()
1636+
const _result = await createOrUpdateBranch(
1637+
git,
1638+
_commitMessage,
1639+
BASE,
1640+
BRANCH,
1641+
REMOTE_NAME,
1642+
false,
1643+
ADD_PATHS_DEFAULT
1644+
)
1645+
expect(_result.action).toEqual('updated')
1646+
expect(_result.hasDiffWithBase).toBeTruthy()
1647+
expect(await getFileContent(TRACKED_FILE)).toEqual(_changes.tracked)
1648+
expect(await getFileContent(UNTRACKED_FILE)).toEqual(_changes.untracked)
1649+
expect(
1650+
await gitLogMatches([_commitMessage, amendedCommitMessage])
1651+
).toBeTruthy()
1652+
})
1653+
15221654
it('tests create and update with commits on the working base (during the workflow) (WBNB)', async () => {
15231655
// Set the working base to a branch that is not the pull request base
15241656
await git.checkout(NOT_BASE_BRANCH)

‎dist/index.js

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,18 +74,30 @@ function tryFetch(git, remote, branch) {
7474
});
7575
}
7676
exports.tryFetch = tryFetch;
77+
// Return the number of commits that branch2 is ahead of branch1
78+
function commitsAhead(git, branch1, branch2) {
79+
return __awaiter(this, void 0, void 0, function* () {
80+
const result = yield git.revList([`${branch1}...${branch2}`], ['--right-only', '--count']);
81+
return Number(result);
82+
});
83+
}
7784
// Return true if branch2 is ahead of branch1
7885
function isAhead(git, branch1, branch2) {
7986
return __awaiter(this, void 0, void 0, function* () {
80-
const result = yield git.revList([`${branch1}...${branch2}`], ['--right-only', '--count']);
81-
return Number(result) > 0;
87+
return (yield commitsAhead(git, branch1, branch2)) > 0;
88+
});
89+
}
90+
// Return the number of commits that branch2 is behind branch1
91+
function commitsBehind(git, branch1, branch2) {
92+
return __awaiter(this, void 0, void 0, function* () {
93+
const result = yield git.revList([`${branch1}...${branch2}`], ['--left-only', '--count']);
94+
return Number(result);
8295
});
8396
}
8497
// Return true if branch2 is behind branch1
8598
function isBehind(git, branch1, branch2) {
8699
return __awaiter(this, void 0, void 0, function* () {
87-
const result = yield git.revList([`${branch1}...${branch2}`], ['--left-only', '--count']);
88-
return Number(result) > 0;
100+
return (yield commitsBehind(git, branch1, branch2)) > 0;
89101
});
90102
}
91103
// Return true if branch2 is even with branch1
@@ -214,9 +226,16 @@ function createOrUpdateBranch(git, commitMessage, base, branch, branchRemoteName
214226
// branches after merging. In particular, it catches a case where the branch was
215227
// squash merged but not deleted. We need to reset to make sure it doesn't appear
216228
// to have a diff with the base due to different commits for the same changes.
229+
// - If the number of commits ahead of the base branch differs between the branch and
230+
// temp branch. This catches a case where the base branch has been force pushed to
231+
// a new commit.
217232
// For changes on base this reset is equivalent to a rebase of the pull request branch.
233+
const tempBranchCommitsAhead = yield commitsAhead(git, base, tempBranch);
234+
const branchCommitsAhead = yield commitsAhead(git, base, branch);
218235
if ((yield git.hasDiff([`${branch}..${tempBranch}`])) ||
219-
!(yield isAhead(git, base, tempBranch))) {
236+
branchCommitsAhead != tempBranchCommitsAhead ||
237+
!(tempBranchCommitsAhead > 0) // !isAhead
238+
) {
220239
core.info(`Resetting '${branch}'`);
221240
// Alternatively, git switch -C branch tempBranch
222241
yield git.checkout(branch, tempBranch);

‎src/create-or-update-branch.ts

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,30 +43,48 @@ export async function tryFetch(
4343
}
4444
}
4545

46-
// Return true if branch2 is ahead of branch1
47-
async function isAhead(
46+
// Return the number of commits that branch2 is ahead of branch1
47+
async function commitsAhead(
4848
git: GitCommandManager,
4949
branch1: string,
5050
branch2: string
51-
): Promise<boolean> {
51+
): Promise<number> {
5252
const result = await git.revList(
5353
[`${branch1}...${branch2}`],
5454
['--right-only', '--count']
5555
)
56-
return Number(result) > 0
56+
return Number(result)
5757
}
5858

59-
// Return true if branch2 is behind branch1
60-
async function isBehind(
59+
// Return true if branch2 is ahead of branch1
60+
async function isAhead(
6161
git: GitCommandManager,
6262
branch1: string,
6363
branch2: string
6464
): Promise<boolean> {
65+
return (await commitsAhead(git, branch1, branch2)) > 0
66+
}
67+
68+
// Return the number of commits that branch2 is behind branch1
69+
async function commitsBehind(
70+
git: GitCommandManager,
71+
branch1: string,
72+
branch2: string
73+
): Promise<number> {
6574
const result = await git.revList(
6675
[`${branch1}...${branch2}`],
6776
['--left-only', '--count']
6877
)
69-
return Number(result) > 0
78+
return Number(result)
79+
}
80+
81+
// Return true if branch2 is behind branch1
82+
async function isBehind(
83+
git: GitCommandManager,
84+
branch1: string,
85+
branch2: string
86+
): Promise<boolean> {
87+
return (await commitsBehind(git, branch1, branch2)) > 0
7088
}
7189

7290
// Return true if branch2 is even with branch1
@@ -226,10 +244,16 @@ export async function createOrUpdateBranch(
226244
// branches after merging. In particular, it catches a case where the branch was
227245
// squash merged but not deleted. We need to reset to make sure it doesn't appear
228246
// to have a diff with the base due to different commits for the same changes.
247+
// - If the number of commits ahead of the base branch differs between the branch and
248+
// temp branch. This catches a case where the base branch has been force pushed to
249+
// a new commit.
229250
// For changes on base this reset is equivalent to a rebase of the pull request branch.
251+
const tempBranchCommitsAhead = await commitsAhead(git, base, tempBranch)
252+
const branchCommitsAhead = await commitsAhead(git, base, branch)
230253
if (
231254
(await git.hasDiff([`${branch}..${tempBranch}`])) ||
232-
!(await isAhead(git, base, tempBranch))
255+
branchCommitsAhead != tempBranchCommitsAhead ||
256+
!(tempBranchCommitsAhead > 0) // !isAhead
233257
) {
234258
core.info(`Resetting '${branch}'`)
235259
// Alternatively, git switch -C branch tempBranch

0 commit comments

Comments
 (0)