Skip to content

Commit

Permalink
fix: handle update after force pushing base to a new commit (#1307)
Browse files Browse the repository at this point in the history
  • Loading branch information
peter-evans committed Nov 22, 2022
1 parent ee93d78 commit d7db273
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 13 deletions.
132 changes: 132 additions & 0 deletions __test__/create-or-update-branch.int.test.ts
Expand Up @@ -631,6 +631,69 @@ describe('create-or-update-branch tests', () => {
).toBeTruthy()
})

it('tests create, force push of base branch, and update with identical changes', async () => {
// If the base branch is force pushed to a different commit when there is an open
// pull request, the branch must be reset to rebase the changes on the base.

// Create tracked and untracked file changes
const changes = await createChanges()
const commitMessage = uuidv4()
const result = await createOrUpdateBranch(
git,
commitMessage,
'',
BRANCH,
REMOTE_NAME,
false,
ADD_PATHS_DEFAULT
)
expect(result.action).toEqual('created')
expect(await getFileContent(TRACKED_FILE)).toEqual(changes.tracked)
expect(await getFileContent(UNTRACKED_FILE)).toEqual(changes.untracked)
expect(
await gitLogMatches([commitMessage, INIT_COMMIT_MESSAGE])
).toBeTruthy()

// Push pull request branch to remote
await git.push([
'--force-with-lease',
REMOTE_NAME,
`HEAD:refs/heads/${BRANCH}`
])

await afterTest(false)
await beforeTest()

// Force push the base branch to a different commit
const amendedCommitMessage = uuidv4()
await git.commit(['--amend', '-m', amendedCommitMessage])
await git.push([
'--force',
REMOTE_NAME,
`HEAD:refs/heads/${DEFAULT_BRANCH}`
])

// Create the same tracked and untracked file changes (no change on update)
const _changes = await createChanges(changes.tracked, changes.untracked)
const _commitMessage = uuidv4()
const _result = await createOrUpdateBranch(
git,
_commitMessage,
'',
BRANCH,
REMOTE_NAME,
false,
ADD_PATHS_DEFAULT
)
expect(_result.action).toEqual('updated')
expect(_result.hasDiffWithBase).toBeTruthy()
expect(await getFileContent(TRACKED_FILE)).toEqual(_changes.tracked)
expect(await getFileContent(UNTRACKED_FILE)).toEqual(_changes.untracked)
expect(
await gitLogMatches([_commitMessage, amendedCommitMessage])
).toBeTruthy()
})

it('tests create and update with commits on the working base (during the workflow)', async () => {
// Create commits on the working base
const commits = await createCommits(git)
Expand Down Expand Up @@ -1519,6 +1582,75 @@ describe('create-or-update-branch tests', () => {
).toBeTruthy()
})

it('tests create, force push of base branch, and update with identical changes (WBNB)', async () => {
// If the base branch is force pushed to a different commit when there is an open
// pull request, the branch must be reset to rebase the changes on the base.

// Set the working base to a branch that is not the pull request base
await git.checkout(NOT_BASE_BRANCH)

// Create tracked and untracked file changes
const changes = await createChanges()
const commitMessage = uuidv4()
const result = await createOrUpdateBranch(
git,
commitMessage,
BASE,
BRANCH,
REMOTE_NAME,
false,
ADD_PATHS_DEFAULT
)
expect(result.action).toEqual('created')
expect(await getFileContent(TRACKED_FILE)).toEqual(changes.tracked)
expect(await getFileContent(UNTRACKED_FILE)).toEqual(changes.untracked)
expect(
await gitLogMatches([commitMessage, INIT_COMMIT_MESSAGE])
).toBeTruthy()

// Push pull request branch to remote
await git.push([
'--force-with-lease',
REMOTE_NAME,
`HEAD:refs/heads/${BRANCH}`
])

await afterTest(false)
await beforeTest()

// Force push the base branch to a different commit
const amendedCommitMessage = uuidv4()
await git.commit(['--amend', '-m', amendedCommitMessage])
await git.push([
'--force',
REMOTE_NAME,
`HEAD:refs/heads/${DEFAULT_BRANCH}`
])

// Set the working base to a branch that is not the pull request base
await git.checkout(NOT_BASE_BRANCH)

// Create the same tracked and untracked file changes (no change on update)
const _changes = await createChanges(changes.tracked, changes.untracked)
const _commitMessage = uuidv4()
const _result = await createOrUpdateBranch(
git,
_commitMessage,
BASE,
BRANCH,
REMOTE_NAME,
false,
ADD_PATHS_DEFAULT
)
expect(_result.action).toEqual('updated')
expect(_result.hasDiffWithBase).toBeTruthy()
expect(await getFileContent(TRACKED_FILE)).toEqual(_changes.tracked)
expect(await getFileContent(UNTRACKED_FILE)).toEqual(_changes.untracked)
expect(
await gitLogMatches([_commitMessage, amendedCommitMessage])
).toBeTruthy()
})

it('tests create and update with commits on the working base (during the workflow) (WBNB)', async () => {
// Set the working base to a branch that is not the pull request base
await git.checkout(NOT_BASE_BRANCH)
Expand Down
29 changes: 24 additions & 5 deletions dist/index.js
Expand Up @@ -74,18 +74,30 @@ function tryFetch(git, remote, branch) {
});
}
exports.tryFetch = tryFetch;
// Return the number of commits that branch2 is ahead of branch1
function commitsAhead(git, branch1, branch2) {
return __awaiter(this, void 0, void 0, function* () {
const result = yield git.revList([`${branch1}...${branch2}`], ['--right-only', '--count']);
return Number(result);
});
}
// Return true if branch2 is ahead of branch1
function isAhead(git, branch1, branch2) {
return __awaiter(this, void 0, void 0, function* () {
const result = yield git.revList([`${branch1}...${branch2}`], ['--right-only', '--count']);
return Number(result) > 0;
return (yield commitsAhead(git, branch1, branch2)) > 0;
});
}
// Return the number of commits that branch2 is behind branch1
function commitsBehind(git, branch1, branch2) {
return __awaiter(this, void 0, void 0, function* () {
const result = yield git.revList([`${branch1}...${branch2}`], ['--left-only', '--count']);
return Number(result);
});
}
// Return true if branch2 is behind branch1
function isBehind(git, branch1, branch2) {
return __awaiter(this, void 0, void 0, function* () {
const result = yield git.revList([`${branch1}...${branch2}`], ['--left-only', '--count']);
return Number(result) > 0;
return (yield commitsBehind(git, branch1, branch2)) > 0;
});
}
// Return true if branch2 is even with branch1
Expand Down Expand Up @@ -214,9 +226,16 @@ function createOrUpdateBranch(git, commitMessage, base, branch, branchRemoteName
// branches after merging. In particular, it catches a case where the branch was
// squash merged but not deleted. We need to reset to make sure it doesn't appear
// to have a diff with the base due to different commits for the same changes.
// - If the number of commits ahead of the base branch differs between the branch and
// temp branch. This catches a case where the base branch has been force pushed to
// a new commit.
// For changes on base this reset is equivalent to a rebase of the pull request branch.
const tempBranchCommitsAhead = yield commitsAhead(git, base, tempBranch);
const branchCommitsAhead = yield commitsAhead(git, base, branch);
if ((yield git.hasDiff([`${branch}..${tempBranch}`])) ||
!(yield isAhead(git, base, tempBranch))) {
branchCommitsAhead != tempBranchCommitsAhead ||
!(tempBranchCommitsAhead > 0) // !isAhead
) {
core.info(`Resetting '${branch}'`);
// Alternatively, git switch -C branch tempBranch
yield git.checkout(branch, tempBranch);
Expand Down
40 changes: 32 additions & 8 deletions src/create-or-update-branch.ts
Expand Up @@ -43,30 +43,48 @@ export async function tryFetch(
}
}

// Return true if branch2 is ahead of branch1
async function isAhead(
// Return the number of commits that branch2 is ahead of branch1
async function commitsAhead(
git: GitCommandManager,
branch1: string,
branch2: string
): Promise<boolean> {
): Promise<number> {
const result = await git.revList(
[`${branch1}...${branch2}`],
['--right-only', '--count']
)
return Number(result) > 0
return Number(result)
}

// Return true if branch2 is behind branch1
async function isBehind(
// Return true if branch2 is ahead of branch1
async function isAhead(
git: GitCommandManager,
branch1: string,
branch2: string
): Promise<boolean> {
return (await commitsAhead(git, branch1, branch2)) > 0
}

// Return the number of commits that branch2 is behind branch1
async function commitsBehind(
git: GitCommandManager,
branch1: string,
branch2: string
): Promise<number> {
const result = await git.revList(
[`${branch1}...${branch2}`],
['--left-only', '--count']
)
return Number(result) > 0
return Number(result)
}

// Return true if branch2 is behind branch1
async function isBehind(
git: GitCommandManager,
branch1: string,
branch2: string
): Promise<boolean> {
return (await commitsBehind(git, branch1, branch2)) > 0
}

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

0 comments on commit d7db273

Please sign in to comment.