From 095c53659be5f2cd40e3ffab2507082e194d6326 Mon Sep 17 00:00:00 2001 From: Peter Evans Date: Sat, 29 Aug 2020 17:27:00 +0900 Subject: [PATCH 1/3] feat: support checkout on a commit in addition to a ref --- __test__/create-or-update-branch.int.test.ts | 150 ++++++++++++++++++- dist/index.js | 59 +++++--- src/create-or-update-branch.ts | 41 ++++- src/create-pull-request.ts | 28 ++-- 4 files changed, 240 insertions(+), 38 deletions(-) diff --git a/__test__/create-or-update-branch.int.test.ts b/__test__/create-or-update-branch.int.test.ts index c3594f7..397cfbc 100644 --- a/__test__/create-or-update-branch.int.test.ts +++ b/__test__/create-or-update-branch.int.test.ts @@ -1,4 +1,8 @@ -import {createOrUpdateBranch, tryFetch} from '../lib/create-or-update-branch' +import { + createOrUpdateBranch, + tryFetch, + getWorkingBaseAndType +} from '../lib/create-or-update-branch' import * as fs from 'fs' import {GitCommandManager} from '../lib/git-command-manager' import * as path from 'path' @@ -193,6 +197,21 @@ describe('create-or-update-branch tests', () => { expect(await tryFetch(git, REMOTE_NAME, NOT_EXIST_BRANCH)).toBeFalsy() }) + it('tests getWorkingBaseAndType on a checked out ref', async () => { + const [workingBase, workingBaseType] = await getWorkingBaseAndType(git) + expect(workingBase).toEqual(BASE) + expect(workingBaseType).toEqual('branch') + }) + + it('tests getWorkingBaseAndType on a checked out commit', async () => { + // Checkout the HEAD commit SHA + const headSha = await git.revParse('HEAD') + await git.exec(['checkout', headSha]) + const [workingBase, workingBaseType] = await getWorkingBaseAndType(git) + expect(workingBase).toEqual(headSha) + expect(workingBaseType).toEqual('commit') + }) + it('tests no changes resulting in no new branch being created', async () => { const commitMessage = uuidv4() const result = await createOrUpdateBranch( @@ -1450,4 +1469,133 @@ describe('create-or-update-branch tests', () => { await gitLogMatches([_commitMessage, INIT_COMMIT_MESSAGE]) ).toBeTruthy() }) + + // Working Base is Not a Ref (WBNR) + // A commit is checked out leaving the repository in a "detached HEAD" state + + it('tests create and update in detached HEAD state (WBNR)', async () => { + // Checkout the HEAD commit SHA + const headSha = await git.revParse('HEAD') + await git.checkout(headSha) + + // Create tracked and untracked file changes + const changes = await createChanges() + const commitMessage = uuidv4() + const result = await createOrUpdateBranch( + git, + commitMessage, + BASE, + BRANCH, + REMOTE_NAME, + false + ) + 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() + + // Checkout the HEAD commit SHA + const _headSha = await git.revParse('HEAD') + await git.checkout(_headSha) + + // Create tracked and untracked file changes + const _changes = await createChanges() + const _commitMessage = uuidv4() + const _result = await createOrUpdateBranch( + git, + _commitMessage, + BASE, + BRANCH, + REMOTE_NAME, + false + ) + 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, INIT_COMMIT_MESSAGE]) + ).toBeTruthy() + }) + + it('tests create and update with commits on the base inbetween, in detached HEAD state (WBNR)', async () => { + // Checkout the HEAD commit SHA + const headSha = await git.revParse('HEAD') + await git.checkout(headSha) + + // Create tracked and untracked file changes + const changes = await createChanges() + const commitMessage = uuidv4() + const result = await createOrUpdateBranch( + git, + commitMessage, + BASE, + BRANCH, + REMOTE_NAME, + false + ) + 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() + + // Create commits on the base + const commitsOnBase = await createCommits(git) + await git.push([ + '--force', + REMOTE_NAME, + `HEAD:refs/heads/${DEFAULT_BRANCH}` + ]) + + // Checkout the HEAD commit SHA + const _headSha = await git.revParse('HEAD') + await git.checkout(_headSha) + + // Create tracked and untracked file changes + const _changes = await createChanges() + const _commitMessage = uuidv4() + const _result = await createOrUpdateBranch( + git, + _commitMessage, + BASE, + BRANCH, + REMOTE_NAME, + false + ) + 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, + ...commitsOnBase.commitMsgs, + INIT_COMMIT_MESSAGE + ]) + ).toBeTruthy() + }) }) diff --git a/dist/index.js b/dist/index.js index 7482497..1b848f2 100644 --- a/dist/index.js +++ b/dist/index.js @@ -2932,10 +2932,30 @@ var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, ge }); }; Object.defineProperty(exports, "__esModule", { value: true }); -exports.createOrUpdateBranch = exports.tryFetch = void 0; +exports.createOrUpdateBranch = exports.tryFetch = exports.getWorkingBaseAndType = exports.WorkingBaseType = void 0; const core = __importStar(__webpack_require__(186)); const uuid_1 = __webpack_require__(840); const CHERRYPICK_EMPTY = 'The previous cherry-pick is now empty, possibly due to conflict resolution.'; +var WorkingBaseType; +(function (WorkingBaseType) { + WorkingBaseType["Branch"] = "branch"; + WorkingBaseType["Commit"] = "commit"; +})(WorkingBaseType = exports.WorkingBaseType || (exports.WorkingBaseType = {})); +function getWorkingBaseAndType(git) { + return __awaiter(this, void 0, void 0, function* () { + const symbolicRefResult = yield git.exec(['symbolic-ref', 'HEAD', '--short'], true); + if (symbolicRefResult.exitCode == 0) { + // A ref is checked out + return [symbolicRefResult.stdout.trim(), WorkingBaseType.Branch]; + } + else { + // A commit is checked out (detached HEAD) + const headSha = yield git.revParse('HEAD'); + return [headSha, WorkingBaseType.Commit]; + } + }); +} +exports.getWorkingBaseAndType = getWorkingBaseAndType; function tryFetch(git, remote, branch) { return __awaiter(this, void 0, void 0, function* () { try { @@ -2983,8 +3003,14 @@ function splitLines(multilineString) { } function createOrUpdateBranch(git, commitMessage, base, branch, branchRemoteName, signoff) { return __awaiter(this, void 0, void 0, function* () { - // Get the working base. This may or may not be the actual base. - const workingBase = yield git.symbolicRef('HEAD', ['--short']); + // Get the working base. + // When a ref, it may or may not be the actual base. + // When a commit, we must rebase onto the actual base. + const [workingBase, workingBaseType] = yield getWorkingBaseAndType(git); + core.info(`Working base is ${workingBaseType} '${workingBase}'`); + if (workingBaseType == WorkingBaseType.Commit && !base) { + throw new Error(`When in 'detached HEAD' state, 'base' must be supplied.`); + } // If the base is not specified it is assumed to be the working base. base = base ? base : workingBase; const baseRemote = 'origin'; @@ -3009,10 +3035,14 @@ function createOrUpdateBranch(git, commitMessage, base, branch, branchRemoteName } // Perform fetch and reset the working base // Commits made during the workflow will be removed - yield git.fetch([`${workingBase}:${workingBase}`], baseRemote, ['--force']); + if (workingBaseType == WorkingBaseType.Branch) { + core.info(`Resetting working base branch '${workingBase}' to its remote`); + yield git.fetch([`${workingBase}:${workingBase}`], baseRemote, ['--force']); + } // If the working base is not the base, rebase the temp branch commits + // This will also be true if the working base type is a commit if (workingBase != base) { - core.info(`Rebasing commits made to branch '${workingBase}' on to base branch '${base}'`); + core.info(`Rebasing commits made to ${workingBaseType} '${workingBase}' on to base branch '${base}'`); // Checkout the actual base yield git.fetch([`${base}:${base}`], baseRemote, ['--force']); yield git.checkout(base); @@ -6927,19 +6957,14 @@ function createPullRequest(inputs) { yield gitAuthHelper.configureToken(inputs.token); core.endGroup(); } - // Determine if the checked out ref is a valid base for a pull request - // The action needs the checked out HEAD ref to be a branch - // This check will fail in the following cases: - // - HEAD is detached - // - HEAD is a merge commit (pull_request events) - // - HEAD is a tag - core.startGroup('Checking the checked out ref'); - const symbolicRefResult = yield git.exec(['symbolic-ref', 'HEAD', '--short'], true); - if (symbolicRefResult.exitCode != 0) { - core.debug(`${symbolicRefResult.stderr}`); - throw new Error('The checked out ref is not a valid base for a pull request. Unable to continue.'); + core.startGroup('Checking the base repository state'); + const [workingBase, workingBaseType] = yield create_or_update_branch_1.getWorkingBaseAndType(git); + core.info(`Working base is ${workingBaseType} '${workingBase}'`); + // When in detached HEAD state (checked out on a commit), we need to + // know the 'base' branch in order to rebase changes. + if (workingBaseType == create_or_update_branch_1.WorkingBaseType.Commit && !inputs.base) { + throw new Error(`When the repository is checked out on a commit instead of a branch, the 'base' input must be supplied.`); } - const workingBase = symbolicRefResult.stdout.trim(); // If the base is not specified it is assumed to be the working base. const base = inputs.base ? inputs.base : workingBase; // Throw an error if the base and branch are not different branches diff --git a/src/create-or-update-branch.ts b/src/create-or-update-branch.ts index d5d11f9..bd67a6a 100644 --- a/src/create-or-update-branch.ts +++ b/src/create-or-update-branch.ts @@ -5,6 +5,28 @@ import {v4 as uuidv4} from 'uuid' const CHERRYPICK_EMPTY = 'The previous cherry-pick is now empty, possibly due to conflict resolution.' +export enum WorkingBaseType { + Branch = 'branch', + Commit = 'commit' +} + +export async function getWorkingBaseAndType( + git: GitCommandManager +): Promise<[string, WorkingBaseType]> { + const symbolicRefResult = await git.exec( + ['symbolic-ref', 'HEAD', '--short'], + true + ) + if (symbolicRefResult.exitCode == 0) { + // A ref is checked out + return [symbolicRefResult.stdout.trim(), WorkingBaseType.Branch] + } else { + // A commit is checked out (detached HEAD) + const headSha = await git.revParse('HEAD') + return [headSha, WorkingBaseType.Commit] + } +} + export async function tryFetch( git: GitCommandManager, remote: string, @@ -80,8 +102,15 @@ export async function createOrUpdateBranch( branchRemoteName: string, signoff: boolean ): Promise { - // Get the working base. This may or may not be the actual base. - const workingBase = await git.symbolicRef('HEAD', ['--short']) + // Get the working base. + // When a ref, it may or may not be the actual base. + // When a commit, we must rebase onto the actual base. + const [workingBase, workingBaseType] = await getWorkingBaseAndType(git) + core.info(`Working base is ${workingBaseType} '${workingBase}'`) + if (workingBaseType == WorkingBaseType.Commit && !base) { + throw new Error(`When in 'detached HEAD' state, 'base' must be supplied.`) + } + // If the base is not specified it is assumed to be the working base. base = base ? base : workingBase const baseRemote = 'origin' @@ -109,12 +138,16 @@ export async function createOrUpdateBranch( // Perform fetch and reset the working base // Commits made during the workflow will be removed - await git.fetch([`${workingBase}:${workingBase}`], baseRemote, ['--force']) + if (workingBaseType == WorkingBaseType.Branch) { + core.info(`Resetting working base branch '${workingBase}' to its remote`) + await git.fetch([`${workingBase}:${workingBase}`], baseRemote, ['--force']) + } // If the working base is not the base, rebase the temp branch commits + // This will also be true if the working base type is a commit if (workingBase != base) { core.info( - `Rebasing commits made to branch '${workingBase}' on to base branch '${base}'` + `Rebasing commits made to ${workingBaseType} '${workingBase}' on to base branch '${base}'` ) // Checkout the actual base await git.fetch([`${base}:${base}`], baseRemote, ['--force']) diff --git a/src/create-pull-request.ts b/src/create-pull-request.ts index d16ff6b..ce46a36 100644 --- a/src/create-pull-request.ts +++ b/src/create-pull-request.ts @@ -1,5 +1,9 @@ import * as core from '@actions/core' -import {createOrUpdateBranch} from './create-or-update-branch' +import { + createOrUpdateBranch, + getWorkingBaseAndType, + WorkingBaseType +} from './create-or-update-branch' import {GitHubHelper} from './github-helper' import {GitCommandManager} from './git-command-manager' import {GitAuthHelper} from './git-auth-helper' @@ -81,24 +85,16 @@ export async function createPullRequest(inputs: Inputs): Promise { core.endGroup() } - // Determine if the checked out ref is a valid base for a pull request - // The action needs the checked out HEAD ref to be a branch - // This check will fail in the following cases: - // - HEAD is detached - // - HEAD is a merge commit (pull_request events) - // - HEAD is a tag - core.startGroup('Checking the checked out ref') - const symbolicRefResult = await git.exec( - ['symbolic-ref', 'HEAD', '--short'], - true - ) - if (symbolicRefResult.exitCode != 0) { - core.debug(`${symbolicRefResult.stderr}`) + core.startGroup('Checking the base repository state') + const [workingBase, workingBaseType] = await getWorkingBaseAndType(git) + core.info(`Working base is ${workingBaseType} '${workingBase}'`) + // When in detached HEAD state (checked out on a commit), we need to + // know the 'base' branch in order to rebase changes. + if (workingBaseType == WorkingBaseType.Commit && !inputs.base) { throw new Error( - 'The checked out ref is not a valid base for a pull request. Unable to continue.' + `When the repository is checked out on a commit instead of a branch, the 'base' input must be supplied.` ) } - const workingBase = symbolicRefResult.stdout.trim() // If the base is not specified it is assumed to be the working base. const base = inputs.base ? inputs.base : workingBase // Throw an error if the base and branch are not different branches From cbd5e97fef39b99fe1379383ce10b719f43cb43e Mon Sep 17 00:00:00 2001 From: Peter Evans Date: Sun, 30 Aug 2020 09:55:07 +0900 Subject: [PATCH 2/3] docs: update concepts-guidelines --- docs/concepts-guidelines.md | 96 +++++++++---------------------------- 1 file changed, 23 insertions(+), 73 deletions(-) diff --git a/docs/concepts-guidelines.md b/docs/concepts-guidelines.md index 81b6ce6..8b4dcce 100644 --- a/docs/concepts-guidelines.md +++ b/docs/concepts-guidelines.md @@ -7,7 +7,7 @@ This document covers terminology, how the action works, general usage guidelines - [How the action works](#how-the-action-works) - [Guidelines](#guidelines) - [Providing a consistent base](#providing-a-consistent-base) - - [Pull request events](#pull-request-events) + - [Events which checkout a commit](#events-which-checkout-a-commit) - [Restrictions on repository forks](#restrictions-on-repository-forks) - [Triggering further workflow runs](#triggering-further-workflow-runs) - [Security](#security) @@ -17,7 +17,6 @@ This document covers terminology, how the action works, general usage guidelines - [Push pull request branches to a fork](#push-pull-request-branches-to-a-fork) - [Authenticating with GitHub App generated tokens](#authenticating-with-github-app-generated-tokens) - [Running in a container or on self-hosted runners](#running-in-a-container-or-on-self-hosted-runners) - - [Creating pull requests on tag push](#creating-pull-requests-on-tag-push) ## Terminology @@ -32,8 +31,6 @@ A pull request references two branches: For each [event type](https://docs.github.com/en/actions/reference/events-that-trigger-workflows) there is a default `GITHUB_SHA` that will be checked out by the GitHub Actions [checkout](https://github.com/actions/checkout) action. -The majority of events will default to checking out the "last commit on default branch," which in most cases will be the latest commit on `master`. - The default can be overridden by specifying a `ref` on checkout. ```yml @@ -44,7 +41,7 @@ The default can be overridden by specifying a `ref` on checkout. ## How the action works -By default, the action expects to be executed on the pull request `base`—the branch you intend to modify with the proposed changes. +Unless the `base` input is supplied, the action expects the target repository to be checked out on the pull request `base`—the branch you intend to modify with the proposed changes. Workflow steps: @@ -60,11 +57,11 @@ The following git diagram shows how the action creates and updates a pull reques ### Providing a consistent base -For the action to work correctly it should be executed in a workflow that checks out a *consistent base* branch. This will be the base of the pull request unless overridden with the `base` input. +For the action to work correctly it should be executed in a workflow that checks out a *consistent* base branch. This will be the base of the pull request unless overridden with the `base` input. This means your workflow should be consistently checking out the branch that you intend to modify once the PR is merged. -In the following example, the [`push`](https://docs.github.com/en/actions/reference/events-that-trigger-workflows#push) and [`create`](https://docs.github.com/en/actions/reference/events-that-trigger-workflows#create) events both trigger the same workflow. This will cause the checkout action to checkout commits from inconsistent branches. Do *not* do this. It will cause multiple pull requests to be created for each additional `base` the action is executed against. +In the following example, the [`push`](https://docs.github.com/en/actions/reference/events-that-trigger-workflows#push) and [`create`](https://docs.github.com/en/actions/reference/events-that-trigger-workflows#create) events both trigger the same workflow. This will cause the checkout action to checkout inconsistent branches and commits. Do *not* do this. It will cause multiple pull requests to be created for each additional `base` the action is executed against. ```yml on: @@ -77,16 +74,29 @@ jobs: - uses: actions/checkout@v2 ``` -Although rare, there may be use cases where it makes sense to execute the workflow on a branch that is not the base of the pull request. In these cases, the base branch can be specified with the `base` action input. The action will attempt to rebase changes made during the workflow on to the actual base. +There may be use cases where it makes sense to execute the workflow on a branch that is not the base of the pull request. In these cases, the base branch can be specified with the `base` action input. The action will attempt to rebase changes made during the workflow on to the actual base. -### Pull request events +### Events which checkout a commit -Workflows triggered by `pull_request` events will by default check out a [merge commit](https://docs.github.com/en/actions/reference/events-that-trigger-workflows#pull_request). To prevent the merge commit being included in created pull requests it is necessary to checkout the `head_ref`. +The [default checkout](#events-and-checkout) for the majority of events will leave the repository checked out on a branch. +However, some events such as `release` and `pull_request` will leave the repository in a "detached HEAD" state. +This is because they checkout a commit, not a branch. +In these cases, you *must supply* the `base` input so the action can rebase changes made during the workflow for the pull request. + +Workflows triggered by [`pull_request`](https://docs.github.com/en/actions/reference/events-that-trigger-workflows#pull_request) events will by default check out a merge commit. Set the `base` input as follows to base the new pull request on the current pull request's branch. ```yml - - uses: actions/checkout@v2 + - uses: peter-evans/create-pull-request@v3 with: - ref: ${{ github.head_ref }} + base: ${{ github.head_ref }} +``` + +Workflows triggered by [`release`](https://docs.github.com/en/actions/reference/events-that-trigger-workflows#release) events will by default check out a tag. For most use cases, you will need to set the `base` input to the branch name of the tagged commit. + +```yml + - uses: peter-evans/create-pull-request@v3 + with: + base: master ``` ### Restrictions on repository forks @@ -146,7 +156,7 @@ Alternatively, use the action directly and reference the commit hash for the ver - uses: thirdparty/foo-action@172ec762f2ac8e050062398456fccd30444f8f30 ``` -This action uses [ncc](https://github.com/zeit/ncc) to compile the Node.js code and dependencies into a single JavaScript file under the [dist](https://github.com/peter-evans/create-pull-request/tree/master/dist) directory. +This action uses [ncc](https://github.com/vercel/ncc) to compile the Node.js code and dependencies into a single JavaScript file under the [dist](https://github.com/peter-evans/create-pull-request/tree/master/dist) directory. ## Advanced usage @@ -306,63 +316,3 @@ jobs: - name: Create Pull Request uses: peter-evans/create-pull-request@v3 ``` - -### Creating pull requests on tag push - -An `on: push` workflow will also trigger when tags are pushed. -During these events, the `actions/checkout` action will check out the `ref/tags/` git ref by default. -This means the repository will *not* be checked out on an active branch. - -If you would like to run `create-pull-request` action on the tagged commit you can achieve this by creating a temporary branch as follows. - -```yml -on: - push: - tags: - - 'v*.*.*' -jobs: - example: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - - name: Create a temporary tag branch - run: | - git config --global user.name 'GitHub' - git config --global user.email 'noreply@github.com' - git checkout -b temp-${GITHUB_REF:10} - git push --set-upstream origin temp-${GITHUB_REF:10} - - # Make changes to pull request here - - - name: Create Pull Request - uses: peter-evans/create-pull-request@v3 - with: - base: master - - - name: Delete tag branch - run: | - git push --delete origin temp-${GITHUB_REF:10} -``` - -This is an alternative, simpler workflow to the one above. However, this is not guaranteed to checkout the tagged commit. -There is a chance that in between the tag being pushed and checking out the `master` branch in the workflow, another commit is made to `master`. If that possibility is not a concern, this workflow will work fine. - -```yml -on: - push: - tags: - - 'v*.*.*' -jobs: - example: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - ref: master - - # Make changes to pull request here - - - name: Create Pull Request - uses: peter-evans/create-pull-request@v3 -``` From da6a868b861a7509e7b116f671f0b14b6694feaa Mon Sep 17 00:00:00 2001 From: Peter Evans Date: Sun, 30 Aug 2020 09:55:17 +0900 Subject: [PATCH 3/3] docs: update readme --- README.md | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/README.md b/README.md index c088a45..284e0ed 100644 --- a/README.md +++ b/README.md @@ -73,17 +73,6 @@ Note that in order to read the step output the action step must have an id. echo "Pull Request Number - ${{ steps.cpr.outputs.pull-request-number }}" ``` -### Checkout - -This action expects repositories to be checked out with `actions/checkout@v2`. - -If there is some reason you need to use `actions/checkout@v1` the following step can be added to checkout the branch. - -```yml - - uses: actions/checkout@v1 - - run: git checkout "${GITHUB_REF:11}" -``` - ### Action behaviour The default behaviour of the action is to create a pull request that will be continually updated with new changes until it is merged or closed. @@ -115,6 +104,7 @@ To use this strategy, set input `branch-suffix` with one of the following option ### Controlling commits As well as relying on the action to handle uncommitted changes, you can additionally make your own commits before the action runs. +Note that the repository must be checked out on a branch with a remote, it won't work for [events which checkout a commit](docs/concepts-guidelines.md#events-which-checkout-a-commit). ```yml steps: