diff --git a/__test__/create-or-update-branch.int.test.ts b/__test__/create-or-update-branch.int.test.ts index 213dc83392..aaa45bf6c1 100644 --- a/__test__/create-or-update-branch.int.test.ts +++ b/__test__/create-or-update-branch.int.test.ts @@ -14,7 +14,7 @@ const REMOTE_NAME = 'origin' const TRACKED_FILE = 'a/tracked-file.txt' const UNTRACKED_FILE = 'b/untracked-file.txt' -const DEFAULT_BRANCH = 'tests/master' +const DEFAULT_BRANCH = 'tests/main' const NOT_BASE_BRANCH = 'tests/branch-that-is-not-the-base' const NOT_EXIST_BRANCH = 'tests/branch-that-does-not-exist' @@ -108,10 +108,10 @@ describe('create-or-update-branch tests', () => { // Check there are no local changes that might be destroyed by running these tests expect(await git.isDirty(true)).toBeFalsy() // Fetch the default branch - await git.fetch(['master:refs/remotes/origin/master']) + await git.fetch(['main:refs/remotes/origin/main']) // Create a "not base branch" for the test run - await git.checkout('master') + await git.checkout('main') await git.checkout(NOT_BASE_BRANCH, 'HEAD') await createFile(TRACKED_FILE) await git.exec(['add', '-A']) @@ -123,7 +123,7 @@ describe('create-or-update-branch tests', () => { ]) // Create a new default branch for the test run with a tracked file - await git.checkout('master') + await git.checkout('main') await git.checkout(DEFAULT_BRANCH, 'HEAD') await createFile(TRACKED_FILE) await git.exec(['add', '-A']) @@ -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) @@ -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) diff --git a/__test__/entrypoint.sh b/__test__/entrypoint.sh index dd13fe9357..7dc7ccdcd0 100755 --- a/__test__/entrypoint.sh +++ b/__test__/entrypoint.sh @@ -6,6 +6,7 @@ WORKINGDIR=$PWD # Create and serve a remote repo mkdir -p /git/remote +git config --global init.defaultBranch main git init --bare /git/remote/test-base.git git daemon --verbose --enable=receive-pack --base-path=/git/remote --export-all /git/remote &>/dev/null & diff --git a/dist/index.js b/dist/index.js index 23b649f32c..81ec93919f 100644 --- a/dist/index.js +++ b/dist/index.js @@ -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 @@ -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); diff --git a/docs/concepts-guidelines.md b/docs/concepts-guidelines.md index f305ac24ea..8431e84676 100644 --- a/docs/concepts-guidelines.md +++ b/docs/concepts-guidelines.md @@ -214,8 +214,9 @@ How to use SSH (deploy keys) with create-pull-request action: Instead of pushing pull request branches to the repository you want to update, you can push them to a fork of that repository. This allows you to employ the [principle of least privilege](https://en.wikipedia.org/wiki/Principle_of_least_privilege) by using a dedicated user acting as a [machine account](https://docs.github.com/en/github/site-policy/github-terms-of-service#3-account-requirements). -This user has no access to the main repository. +This user only has `read` access to the main repository. It will use their own fork to push code and create the pull request. +Note that if you choose to use this method (not give the machine account `write` access to the repository) the following inputs cannot be used: `labels`, `assignees`, `reviewers`, `team-reviewers` and `milestone`. 1. Create a new GitHub user and login. 2. Fork the repository that you will be creating pull requests in. diff --git a/docs/examples.md b/docs/examples.md index 66973e76a2..86295e967a 100644 --- a/docs/examples.md +++ b/docs/examples.md @@ -282,8 +282,10 @@ jobs: - name: Get Latest Swagger UI Release id: swagger-ui run: | - echo ::set-output name=release_tag::$(curl -sL https://api.github.com/repos/swagger-api/swagger-ui/releases/latest | jq -r ".tag_name") - echo ::set-output name=current_tag::$(> $GITHUB_OUTPUT + current_tag=$(> $GITHUB_OUTPUT - name: Update Swagger UI if: steps.swagger-ui.outputs.current_tag != steps.swagger-ui.outputs.release_tag env: @@ -475,7 +477,9 @@ jobs: args: --exit-code --recursive --in-place --aggressive --aggressive . - name: Set autopep8 branch name id: vars - run: echo ::set-output name=branch-name::"autopep8-patches/${{ github.head_ref }}" + run: | + branch-name="autopep8-patches/${{ github.head_ref }}" + echo "branch-name=$branch-name" >> $GITHUB_OUTPUT - name: Create Pull Request if: steps.autopep8.outputs.exit-code == 2 uses: peter-evans/create-pull-request@v4 @@ -525,16 +529,17 @@ jobs: ### Dynamic configuration using variables The following examples show how configuration for the action can be dynamically defined in a previous workflow step. - -The recommended method is to use [`set-output`](https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-output-parameter). Note that the step where output variables are defined must have an id. +Note that the step where output variables are defined must have an id. ```yml - name: Set output variables id: vars run: | - echo ::set-output name=pr_title::"[Test] Add report file $(date +%d-%m-%Y)" - echo ::set-output name=pr_body::"This PR was auto-generated on $(date +%d-%m-%Y) \ + pr_title="[Test] Add report file $(date +%d-%m-%Y)" + pr_body="This PR was auto-generated on $(date +%d-%m-%Y) \ by [create-pull-request](https://github.com/peter-evans/create-pull-request)." + echo "pr_title=$pr_title" >> $GITHUB_OUTPUT + echo "pr_body=$pr_body" >> $GITHUB_OUTPUT - name: Create Pull Request uses: peter-evans/create-pull-request@v4 with: @@ -545,16 +550,15 @@ The recommended method is to use [`set-output`](https://docs.github.com/en/actio ### Setting the pull request body from a file This example shows how file content can be read into a variable and passed to the action. -The content must be [escaped to preserve newlines](https://github.amrom.workers.devmunity/t/set-output-truncates-multiline-strings/16852/3). ```yml - id: get-pr-body run: | body=$(cat pr-body.txt) - body="${body//'%'/'%25'}" - body="${body//$'\n'/'%0A'}" - body="${body//$'\r'/'%0D'}" - echo ::set-output name=body::$body + delimiter="$(openssl rand -hex 8)" + echo "body<<$delimiter" >> $GITHUB_OUTPUT + echo "$body" >> $GITHUB_OUTPUT + echo "$delimiter" >> $GITHUB_OUTPUT - name: Create Pull Request uses: peter-evans/create-pull-request@v4 diff --git a/src/create-or-update-branch.ts b/src/create-or-update-branch.ts index 7dcd9fcdf5..ebdef12cda 100644 --- a/src/create-or-update-branch.ts +++ b/src/create-or-update-branch.ts @@ -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 { +): Promise { 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 { + 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 { 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 { + return (await commitsBehind(git, branch1, branch2)) > 0 } // Return true if branch2 is even with branch1 @@ -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