From c949a2b103b3fc54f58b9eeb6c33391a3b997481 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 15 May 2020 17:19:13 +0200 Subject: [PATCH 1/6] feat(dev-infra): move merge script over from components repo Moves the merge script from the components repository over to the shared dev-infra package. The merge script has been orginally built for all Angular repositories, but we just kept it in the components repo temporarily to test it. Since everything went well on the components side, we now move the script over and integrate it into the dev-infra package. --- dev-infra/pr/merge/config.ts | 95 ++++++++ dev-infra/pr/merge/console.ts | 20 ++ dev-infra/pr/merge/failures.ts | 79 ++++++ dev-infra/pr/merge/git.ts | 115 +++++++++ dev-infra/pr/merge/index.ts | 114 +++++++++ dev-infra/pr/merge/pull-request.ts | 111 +++++++++ dev-infra/pr/merge/strategies/api-merge.ts | 225 ++++++++++++++++++ .../pr/merge/strategies/autosquash-merge.ts | 73 ++++++ .../merge/strategies/commit-message-filter.js | 38 +++ dev-infra/pr/merge/strategies/strategy.ts | 131 ++++++++++ dev-infra/pr/merge/string-pattern.ts | 12 + dev-infra/pr/merge/target-label.ts | 28 +++ dev-infra/pr/merge/task.ts | 105 ++++++++ package.json | 1 + yarn.lock | 99 +++++++- 15 files changed, 1244 insertions(+), 2 deletions(-) create mode 100644 dev-infra/pr/merge/config.ts create mode 100644 dev-infra/pr/merge/console.ts create mode 100644 dev-infra/pr/merge/failures.ts create mode 100644 dev-infra/pr/merge/git.ts create mode 100644 dev-infra/pr/merge/index.ts create mode 100644 dev-infra/pr/merge/pull-request.ts create mode 100644 dev-infra/pr/merge/strategies/api-merge.ts create mode 100644 dev-infra/pr/merge/strategies/autosquash-merge.ts create mode 100644 dev-infra/pr/merge/strategies/commit-message-filter.js create mode 100644 dev-infra/pr/merge/strategies/strategy.ts create mode 100644 dev-infra/pr/merge/string-pattern.ts create mode 100644 dev-infra/pr/merge/target-label.ts create mode 100644 dev-infra/pr/merge/task.ts diff --git a/dev-infra/pr/merge/config.ts b/dev-infra/pr/merge/config.ts new file mode 100644 index 0000000000000..b500fd4fc61e4 --- /dev/null +++ b/dev-infra/pr/merge/config.ts @@ -0,0 +1,95 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {getAngularDevConfig} from '../../utils/config'; + +import {GithubApiMergeStrategyConfig} from './strategies/api-merge'; + +/** + * Possible merge methods supported by the Github API. + * https://developer.github.com/v3/pulls/#merge-a-pull-request-merge-button. + */ +export type GithubApiMergeMethod = 'merge'|'squash'|'rebase'; + +/** + * Target labels represent Github pull requests labels. These labels instruct the merge + * script into which branches a given pull request should be merged to. + */ +export interface TargetLabel { + /** Pattern that matches the given target label. */ + pattern: RegExp|string; + /** + * List of branches a pull request with this target label should be merged into. + * Can also be wrapped in a function that accepts the target branch specified in the + * Github Web UI. This is useful for supporting labels like `target: development-branch`. + */ + branches: string[]|((githubTargetBranch: string) => string[]); +} + +/** Configuration for the merge script. */ +export interface MergeConfig { + /** Configuration for the upstream repository. */ + repository: {user: string; name: string; useSsh?: boolean}; + /** List of target labels. */ + labels: TargetLabel[]; + /** Required base commits for given branches. */ + requiredBaseCommits?: {[branchName: string]: string}; + /** Pattern that matches labels which imply a signed CLA. */ + claSignedLabel: string|RegExp; + /** Pattern that matches labels which imply a merge ready pull request. */ + mergeReadyLabel: string|RegExp; + /** Label which can be applied to fixup commit messages in the merge script. */ + commitMessageFixupLabel: string|RegExp; + /** + * Whether pull requests should be merged using the Github API. This can be enabled + * if projects want to have their pull requests show up as `Merged` in the Github UI. + * The downside is that fixup or squash commits no longer work as the Github API does + * not support this. + */ + githubApiMerge: false|GithubApiMergeStrategyConfig; +} + +/** Loads and validates the merge configuration. */ +export function loadAndValidateConfig(): {config?: MergeConfig, errors?: string[]} { + const config = getAngularDevConfig<'merge', MergeConfig>().merge; + if (config === undefined) { + return { + errors: ['No merge configuration found. Set the `merge` configuration.'] + } + } + const errors = validateConfig(config); + if (errors.length) { + return {errors}; + } + return {config}; +} + +/** Validates the specified configuration. Returns a list of failure messages. */ +function validateConfig(config: MergeConfig): string[] { + const errors: string[] = []; + if (!config.labels) { + errors.push('No label configuration.'); + } else if (!Array.isArray(config.labels)) { + errors.push('Label configuration needs to be an array.'); + } + if (!config.repository) { + errors.push('No repository is configured.'); + } else if (!config.repository.user || !config.repository.name) { + errors.push('Repository configuration needs to specify a `user` and repository `name`.'); + } + if (!config.claSignedLabel) { + errors.push('No CLA signed label configured.'); + } + if (!config.mergeReadyLabel) { + errors.push('No merge ready label configured.'); + } + if (config.githubApiMerge === undefined) { + errors.push('No explicit choice of merge strategy. Please set `githubApiMerge`.'); + } + return errors; +} diff --git a/dev-infra/pr/merge/console.ts b/dev-infra/pr/merge/console.ts new file mode 100644 index 0000000000000..f6ca0c9d9e4db --- /dev/null +++ b/dev-infra/pr/merge/console.ts @@ -0,0 +1,20 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {prompt} from 'inquirer'; + +/** Prompts the user with a confirmation question and a specified message. */ +export async function promptConfirm(message: string, defaultValue = false): Promise { + return (await prompt<{result: boolean}>({ + type: 'confirm', + name: 'result', + message: message, + default: defaultValue, + })) + .result; +} diff --git a/dev-infra/pr/merge/failures.ts b/dev-infra/pr/merge/failures.ts new file mode 100644 index 0000000000000..3966692e5064a --- /dev/null +++ b/dev-infra/pr/merge/failures.ts @@ -0,0 +1,79 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +/** + * Class that can be used to describe pull request failures. A failure + * is described through a human-readable message and a flag indicating + * whether it is non-fatal or not. + */ +export class PullRequestFailure { + constructor( + /** Human-readable message for the failure */ + public message: string, + /** Whether the failure is non-fatal and can be forcibly ignored. */ + public nonFatal = false) {} + + static claUnsigned() { + return new this(`CLA has not been signed. Please make sure the PR author has signed the CLA.`); + } + + static failingCiJobs() { + return new this(`Failing CI jobs.`, true); + } + + static pendingCiJobs() { + return new this(`Pending CI jobs.`, true); + } + + static notMergeReady() { + return new this(`Not marked as merge ready.`); + } + + static noTargetLabel() { + return new this(`No target branch could be determined. Please ensure a target label is set.`); + } + + static mismatchingTargetBranch(allowedBranches: string[]) { + return new this( + `Pull request is set to wrong base branch. Please update the PR in the Github UI ` + + `to one of the following branches: ${allowedBranches.join(', ')}.`); + } + + static unsatisfiedBaseSha() { + return new this( + `Pull request has not been rebased recently and could be bypassing CI checks. ` + + `Please rebase the PR.`); + } + + static mergeConflicts(failedBranches: string[]) { + return new this( + `Could not merge pull request into the following branches due to merge ` + + `conflicts: ${ + failedBranches.join(', ')}. Please rebase the PR or update the target label.`); + } + + static unknownMergeError() { + return new this(`Unknown merge error occurred. Please see console output above for debugging.`); + } + + static unableToFixupCommitMessageSquashOnly() { + return new this( + `Unable to fixup commit message of pull request. Commit message can only be ` + + `modified if the PR is merged using squash.`); + } + + static notFound() { + return new this(`Pull request could not be found upstream.`); + } + + static insufficientPermissionsToMerge() { + return new this( + `Insufficient Github API permissions to merge pull request. Please ` + + `ensure that your auth token has write access.`); + } +} diff --git a/dev-infra/pr/merge/git.ts b/dev-infra/pr/merge/git.ts new file mode 100644 index 0000000000000..34468001f6d91 --- /dev/null +++ b/dev-infra/pr/merge/git.ts @@ -0,0 +1,115 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import * as Octokit from '@octokit/rest'; +import {spawnSync, SpawnSyncOptions, SpawnSyncReturns} from 'child_process'; +import {MergeConfig} from './config'; + +/** Error for failed Github API requests. */ +export class GithubApiRequestError extends Error { + constructor(public status: number, message: string) { + super(message); + } +} + +/** Error for failed Git commands. */ +export class GitCommandError extends Error { + constructor(client: GitClient, public args: string[]) { + // Errors are not guaranteed to be caught. To ensure that we don't + // accidentally leak the Github token that might be used in a command, + // we sanitize the command that will be part of the error message. + super(`Command failed: git ${client.omitGithubTokenFromMessage(args.join(' '))}`); + } +} + +export class GitClient { + /** Short-hand for accessing the repository configuration. */ + repoConfig = this._config.repository; + /** Octokit request parameters object for targeting the configured repository. */ + repoParams = {owner: this.repoConfig.user, repo: this.repoConfig.name}; + /** URL that resolves to the configured repository. */ + repoGitUrl = this.repoConfig.useSsh ? + `git@github.com:${this.repoConfig.user}/${this.repoConfig.name}.git` : + `https://${this._githubToken}@github.com/${this.repoConfig.user}/${this.repoConfig.name}.git`; + /** Instance of the authenticated Github octokit API. */ + api: Octokit; + + /** Regular expression that matches the provided Github token. */ + private _tokenRegex = new RegExp(this._githubToken, 'g'); + + constructor( + private _projectRoot: string, private _githubToken: string, private _config: MergeConfig) { + this.api = new Octokit({auth: _githubToken}); + this.api.hook.error('request', error => { + // Wrap API errors in a known error class. This allows us to + // expect Github API errors better and in a non-ambiguous way. + throw new GithubApiRequestError(error.status, error.message); + }); + } + + /** Executes the given git command. Throws if the command fails. */ + run(args: string[], options?: SpawnSyncOptions): Omit, 'status'> { + const result = this.runGraceful(args, options); + if (result.status !== 0) { + throw new GitCommandError(this, args); + } + // Omit `status` from the type so that it's obvious that the status is never + // non-zero as explained in the method description. + return result as Omit, 'status'>; + } + + /** + * Spawns a given Git command process. Does not throw if the command fails. Additionally, + * if there is any stderr output, the output will be printed. This makes it easier to + * debug failed commands. + */ + runGraceful(args: string[], options: SpawnSyncOptions = {}): SpawnSyncReturns { + // To improve the debugging experience in case something fails, we print all executed + // Git commands. Note that we do not want to print the token if is contained in the + // command. It's common to share errors with others if the tool failed. + console.info('Executing: git', this.omitGithubTokenFromMessage(args.join(' '))); + + const result = spawnSync('git', args, { + cwd: this._projectRoot, + stdio: 'pipe', + ...options, + // Encoding is always `utf8` and not overridable. This ensures that this method + // always returns `string` as output instead of buffers. + encoding: 'utf8', + }); + + if (result.stderr !== null) { + // Git sometimes prints the command if it failed. This means that it could + // potentially leak the Github token used for accessing the remote. To avoid + // printing a token, we sanitize the string before printing the stderr output. + process.stderr.write(this.omitGithubTokenFromMessage(result.stderr)); + } + + return result; + } + + /** Whether the given branch contains the specified SHA. */ + hasCommit(branchName: string, sha: string): boolean { + return this.run(['branch', branchName, '--contains', sha]).stdout !== ''; + } + + /** Gets the currently checked out branch. */ + getCurrentBranch(): string { + return this.run(['rev-parse', '--abbrev-ref', 'HEAD']).stdout.trim(); + } + + /** Gets whether the current Git repository has uncommitted changes. */ + hasUncommittedChanges(): boolean { + return this.runGraceful(['diff-index', '--quiet', 'HEAD']).status !== 0; + } + + /** Sanitizes a given message by omitting the provided Github token if present. */ + omitGithubTokenFromMessage(value: string): string { + return value.replace(this._tokenRegex, ''); + } +} diff --git a/dev-infra/pr/merge/index.ts b/dev-infra/pr/merge/index.ts new file mode 100644 index 0000000000000..4bc1e2a842263 --- /dev/null +++ b/dev-infra/pr/merge/index.ts @@ -0,0 +1,114 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {MergeResult, MergeStatus, PullRequestMergeTask} from './task'; +import {GithubApiRequestError} from './git'; +import chalk from 'chalk'; +import {promptConfirm} from './console'; +import {loadAndValidateConfig} from './config'; +import {getRepoBaseDir} from '../../utils/config'; + +/** URL to the Github page where personal access tokens can be generated. */ +export const GITHUB_TOKEN_GENERATE_URL = `https://github.com/settings/tokens`; + + +/** + * Entry-point for the merge script CLI. The script can be used to merge individual pull requests + * into branches based on the `PR target` labels that have been set in a configuration. The script + * aims to reduce the manual work that needs to be performed to cherry-pick a PR into multiple + * branches based on a target label. + */ +export async function mergePullRequest(prNumber: number, githubToken: string) { + const projectRoot = getRepoBaseDir(); + const {config, errors} = loadAndValidateConfig(); + + if (errors) { + console.error(chalk.red('Invalid configuration:')); + errors.forEach(desc => console.error(chalk.yellow(` - ${desc}`))); + process.exit(1); + } + + const api = new PullRequestMergeTask(projectRoot, config, githubToken); + + // Perform the merge. Force mode can be activated through a command line flag. + // Alternatively, if the merge fails with non-fatal failures, the script + // will prompt whether it should rerun in force mode. + if (!await performMerge(false)) { + process.exit(1); + } + + /** Performs the merge and returns whether it was successful or not. */ + async function performMerge(ignoreFatalErrors: boolean): Promise { + try { + const result = await api.merge(prNumber, ignoreFatalErrors); + return await handleMergeResult(result, ignoreFatalErrors); + } catch (e) { + // Catch errors to the Github API for invalid requests. We want to + // exit the script with a better explanation of the error. + if (e instanceof GithubApiRequestError && e.status === 401) { + console.error(chalk.red('Github API request failed. ' + e.message)); + console.error(chalk.yellow('Please ensure that your provided token is valid.')); + console.error(chalk.yellow(`You can generate a token here: ${GITHUB_TOKEN_GENERATE_URL}`)); + process.exit(1); + } + throw e; + } + } + + /** + * Prompts whether the specified pull request should be forcibly merged. If so, merges + * the specified pull request forcibly (ignoring non-critical failures). + * @returns Whether the specified pull request has been forcibly merged. + */ + async function promptAndPerformForceMerge(): Promise { + if (await promptConfirm('Do you want to forcibly proceed with merging?')) { + // Perform the merge in force mode. This means that non-fatal failures + // are ignored and the merge continues. + return performMerge(true); + } + return false; + } + + /** + * Handles the merge result by printing console messages, exiting the process + * based on the result, or by restarting the merge if force mode has been enabled. + * @returns Whether the merge was successful or not. + */ + async function handleMergeResult(result: MergeResult, disableForceMergePrompt = false) { + const {failure, status} = result; + const canForciblyMerge = failure && failure.nonFatal; + + switch (status) { + case MergeStatus.SUCCESS: + console.info(chalk.green(`Successfully merged the pull request: ${prNumber}`)); + return true; + case MergeStatus.DIRTY_WORKING_DIR: + console.error(chalk.red( + `Local working repository not clean. Please make sure there are ` + + `no uncommitted changes.`)); + return false; + case MergeStatus.UNKNOWN_GIT_ERROR: + console.error(chalk.red( + 'An unknown Git error has been thrown. Please check the output ' + + 'above for details.')); + return false; + case MergeStatus.FAILED: + console.error(chalk.yellow(`Could not merge the specified pull request.`)); + console.error(chalk.red(failure!.message)); + if (canForciblyMerge && !disableForceMergePrompt) { + console.info(); + console.info(chalk.yellow('The pull request above failed due to non-critical errors.')); + console.info(chalk.yellow(`This error can be forcibly ignored if desired.`)); + return await promptAndPerformForceMerge(); + } + return false; + default: + throw Error(`Unexpected merge result: ${status}`); + } + } +} diff --git a/dev-infra/pr/merge/pull-request.ts b/dev-infra/pr/merge/pull-request.ts new file mode 100644 index 0000000000000..de9cfe13559b4 --- /dev/null +++ b/dev-infra/pr/merge/pull-request.ts @@ -0,0 +1,111 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import * as Octokit from '@octokit/rest'; + +import {PullRequestFailure} from './failures'; +import {GitClient} from './git'; +import {matchesPattern} from './string-pattern'; +import {getBranchesFromTargetLabel, getTargetLabelFromPullRequest} from './target-label'; +import {PullRequestMergeTask} from './task'; + +/** Interface that describes a pull request. */ +export interface PullRequest { + /** Number of the pull request. */ + prNumber: number; + /** Title of the pull request. */ + title: string; + /** Labels applied to the pull request. */ + labels: string[]; + /** List of branches this PR should be merged into. */ + targetBranches: string[]; + /** Branch that the PR targets in the Github UI. */ + githubTargetBranch: string; + /** Count of commits in this pull request. */ + commitCount: number; + /** Optional SHA that this pull request needs to be based on. */ + requiredBaseSha?: string; + /** Whether the pull request commit message fixup. */ + needsCommitMessageFixup: boolean; +} + +/** + * Loads and validates the specified pull request against the given configuration. + * If the pull requests fails, a pull request failure is returned. + */ +export async function loadAndValidatePullRequest( + {git, config}: PullRequestMergeTask, prNumber: number, + ignoreNonFatalFailures = false): Promise { + const prData = await fetchPullRequestFromGithub(git, prNumber); + + if (prData === null) { + return PullRequestFailure.notFound(); + } + + const labels = prData.labels.map(l => l.name); + + if (!labels.some(name => matchesPattern(name, config.mergeReadyLabel))) { + return PullRequestFailure.notMergeReady(); + } + if (!labels.some(name => matchesPattern(name, config.claSignedLabel))) { + return PullRequestFailure.claUnsigned(); + } + + const targetLabel = getTargetLabelFromPullRequest(config, labels); + if (targetLabel === null) { + return PullRequestFailure.noTargetLabel(); + } + + const {data: {state}} = + await git.api.repos.getCombinedStatusForRef({...git.repoParams, ref: prData.head.sha}); + + if (state === 'failure' && !ignoreNonFatalFailures) { + return PullRequestFailure.failingCiJobs(); + } + if (state === 'pending' && !ignoreNonFatalFailures) { + return PullRequestFailure.pendingCiJobs(); + } + + const githubTargetBranch = prData.base.ref; + const requiredBaseSha = + config.requiredBaseCommits && config.requiredBaseCommits[githubTargetBranch]; + const needsCommitMessageFixup = !!config.commitMessageFixupLabel && + labels.some(name => matchesPattern(name, config.commitMessageFixupLabel)); + + return { + prNumber, + labels, + requiredBaseSha, + githubTargetBranch, + needsCommitMessageFixup, + title: prData.title, + targetBranches: getBranchesFromTargetLabel(targetLabel, githubTargetBranch), + commitCount: prData.commits, + }; +} + +/** Fetches a pull request from Github. Returns null if an error occurred. */ +async function fetchPullRequestFromGithub( + git: GitClient, prNumber: number): Promise { + try { + const result = await git.api.pulls.get({...git.repoParams, pull_number: prNumber}); + return result.data; + } catch (e) { + // If the pull request could not be found, we want to return `null` so + // that the error can be handled gracefully. + if (e.status === 404) { + return null; + } + throw e; + } +} + +/** Whether the specified value resolves to a pull request. */ +export function isPullRequest(v: PullRequestFailure|PullRequest): v is PullRequest { + return (v as PullRequest).targetBranches !== undefined; +} diff --git a/dev-infra/pr/merge/strategies/api-merge.ts b/dev-infra/pr/merge/strategies/api-merge.ts new file mode 100644 index 0000000000000..9b7766e0a20de --- /dev/null +++ b/dev-infra/pr/merge/strategies/api-merge.ts @@ -0,0 +1,225 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {PullsListCommitsResponse, PullsMergeParams} from '@octokit/rest'; +import {prompt} from 'inquirer'; + +import {parseCommitMessage} from '../../../commit-message/validate'; +import {GithubApiMergeMethod} from '../config'; +import {PullRequestFailure} from '../failures'; +import {GitClient} from '../git'; +import {PullRequest} from '../pull-request'; +import {matchesPattern} from '../string-pattern'; + +import {MergeStrategy, TEMP_PR_HEAD_BRANCH} from './strategy'; + +/** Configuration for the Github API merge strategy. */ +export interface GithubApiMergeStrategyConfig { + /** Default method used for merging pull requests */ + default: GithubApiMergeMethod; + /** Labels which specify a different merge method than the default. */ + labels?: {pattern: string, method: GithubApiMergeMethod}[]; +} + +/** Separator between commit message header and body. */ +const COMMIT_HEADER_SEPARATOR = '\n\n'; + +/** + * Merge strategy that primarily leverages the Github API. The strategy merges a given + * pull request into a target branch using the API. This ensures that Github displays + * the pull request as merged. The merged commits are then cherry-picked into the remaining + * target branches using the local Git instance. The benefit is that the Github merged state + * is properly set, but a notable downside is that PRs cannot use fixup or squash commits. + */ +export class GithubApiMergeStrategy extends MergeStrategy { + constructor(git: GitClient, private _config: GithubApiMergeStrategyConfig) { + super(git); + } + + async merge(pullRequest: PullRequest): Promise { + const {githubTargetBranch, prNumber, targetBranches, requiredBaseSha, needsCommitMessageFixup} = + pullRequest; + // If the pull request does not have its base branch set to any determined target + // branch, we cannot merge using the API. + if (targetBranches.every(t => t !== githubTargetBranch)) { + return PullRequestFailure.mismatchingTargetBranch(targetBranches); + } + + // In cases where a required base commit is specified for this pull request, check if + // the pull request contains the given commit. If not, return a pull request failure. + // This check is useful for enforcing that PRs are rebased on top of a given commit. + // e.g. a commit that changes the code ownership validation. PRs which are not rebased + // could bypass new codeowner ship rules. + if (requiredBaseSha && !this.git.hasCommit(TEMP_PR_HEAD_BRANCH, requiredBaseSha)) { + return PullRequestFailure.unsatisfiedBaseSha(); + } + + const method = this._getMergeActionFromPullRequest(pullRequest); + const cherryPickTargetBranches = targetBranches.filter(b => b !== githubTargetBranch); + + // First cherry-pick the PR into all local target branches in dry-run mode. This is + // purely for testing so that we can figure out whether the PR can be cherry-picked + // into the other target branches. We don't want to merge the PR through the API, and + // then run into cherry-pick conflicts after the initial merge already completed. + const failure = await this._checkMergability(pullRequest, cherryPickTargetBranches); + + // If the PR could not be cherry-picked into all target branches locally, we know it can't + // be done through the Github API either. We abort merging and pass-through the failure. + if (failure !== null) { + return failure; + } + + const mergeOptions: PullsMergeParams = { + pull_number: prNumber, + merge_method: method, + ...this.git.repoParams, + }; + + if (needsCommitMessageFixup) { + // Commit message fixup does not work with other merge methods as the Github API only + // allows commit message modifications for squash merging. + if (method !== 'squash') { + return PullRequestFailure.unableToFixupCommitMessageSquashOnly(); + } + await this._promptCommitMessageEdit(pullRequest, mergeOptions); + } + + let mergeStatusCode: number; + let targetSha: string; + + try { + // Merge the pull request using the Github API into the selected base branch. + const result = await this.git.api.pulls.merge(mergeOptions); + + mergeStatusCode = result.status; + targetSha = result.data.sha; + } catch (e) { + // Note: Github usually returns `404` as status code if the API request uses a + // token with insufficient permissions. Github does this because it doesn't want + // to leak whether a repository exists or not. In our case we expect a certain + // repository to exist, so we always treat this as a permission failure. + if (e.status === 403 || e.status === 404) { + return PullRequestFailure.insufficientPermissionsToMerge(); + } + throw e; + } + + // https://developer.github.com/v3/pulls/#response-if-merge-cannot-be-performed + // Pull request cannot be merged due to merge conflicts. + if (mergeStatusCode === 405) { + return PullRequestFailure.mergeConflicts([githubTargetBranch]); + } + if (mergeStatusCode !== 200) { + return PullRequestFailure.unknownMergeError(); + } + + // If the PR does not need to be merged into any other target branches, + // we exit here as we already completed the merge. + if (!cherryPickTargetBranches.length) { + return null; + } + + // Refresh the target branch the PR has been merged into through the API. We need + // to re-fetch as otherwise we cannot cherry-pick the new commits into the remaining + // target branches. + this.fetchTargetBranches([githubTargetBranch]); + + // Number of commits that have landed in the target branch. This could vary from + // the count of commits in the PR due to squashing. + const targetCommitsCount = method === 'squash' ? 1 : pullRequest.commitCount; + + // Cherry pick the merged commits into the remaining target branches. + const failedBranches = await this.cherryPickIntoTargetBranches( + `${targetSha}~${targetCommitsCount}..${targetSha}`, cherryPickTargetBranches); + + // We already checked whether the PR can be cherry-picked into the target branches, + // but in case the cherry-pick somehow fails, we still handle the conflicts here. The + // commits created through the Github API could be different (i.e. through squash). + if (failedBranches.length) { + return PullRequestFailure.mergeConflicts(failedBranches); + } + + this.pushTargetBranchesUpstream(cherryPickTargetBranches); + return null; + } + + /** + * Prompts the user for the commit message changes. Unlike as in the autosquash merge + * strategy, we cannot start an interactive rebase because we merge using the Github API. + * The Github API only allows modifications to PR title and body for squash merges. + */ + async _promptCommitMessageEdit(pullRequest: PullRequest, mergeOptions: PullsMergeParams) { + const commitMessage = await this._getDefaultSquashCommitMessage(pullRequest); + const {result} = await prompt<{result: string}>({ + type: 'editor', + name: 'result', + message: 'Please update the commit message', + default: commitMessage, + }); + + // Split the new message into title and message. This is necessary because the + // Github API expects title and message to be passed separately. + const [newTitle, ...newMessage] = result.split(COMMIT_HEADER_SEPARATOR); + + // Update the merge options so that the changes are reflected in there. + mergeOptions.commit_title = `${newTitle} (#${pullRequest.prNumber})`; + mergeOptions.commit_message = newMessage.join(COMMIT_HEADER_SEPARATOR); + } + + /** + * Gets a commit message for the given pull request. Github by default concatenates + * multiple commit messages if a PR is merged in squash mode. We try to replicate this + * behavior here so that we have a default commit message that can be fixed up. + */ + private async _getDefaultSquashCommitMessage(pullRequest: PullRequest): Promise { + const commits = (await this._getPullRequestCommitMessages(pullRequest)) + .map(message => ({message, parsed: parseCommitMessage(message)})); + const messageBase = `${pullRequest.title}${COMMIT_HEADER_SEPARATOR}`; + if (commits.length <= 1) { + return `${messageBase}${commits[0].parsed.body}`; + } + const joinedMessages = commits.map(c => `* ${c.message}`).join(COMMIT_HEADER_SEPARATOR); + return `${messageBase}${joinedMessages}`; + } + + /** Gets all commit messages of commits in the pull request. */ + private async _getPullRequestCommitMessages({prNumber}: PullRequest) { + const request = this.git.api.pulls.listCommits.endpoint.merge( + {...this.git.repoParams, pull_number: prNumber}); + const allCommits: PullsListCommitsResponse = await this.git.api.paginate(request); + return allCommits.map(({commit}) => commit.message); + } + + /** + * Checks if given pull request could be merged into its target branches. + * @returns A pull request failure if it the PR could not be merged. + */ + private async _checkMergability(pullRequest: PullRequest, targetBranches: string[]): + Promise { + const revisionRange = this.getPullRequestRevisionRange(pullRequest); + const failedBranches = + this.cherryPickIntoTargetBranches(revisionRange, targetBranches, {dryRun: true}); + + if (failedBranches.length) { + return PullRequestFailure.mergeConflicts(failedBranches); + } + return null; + } + + /** Determines the merge action from the given pull request. */ + private _getMergeActionFromPullRequest({labels}: PullRequest): GithubApiMergeMethod { + if (this._config.labels) { + const matchingLabel = + this._config.labels.find(({pattern}) => labels.some(l => matchesPattern(l, pattern))); + if (matchingLabel !== undefined) { + return matchingLabel.method; + } + } + return this._config.default; + } +} diff --git a/dev-infra/pr/merge/strategies/autosquash-merge.ts b/dev-infra/pr/merge/strategies/autosquash-merge.ts new file mode 100644 index 0000000000000..1c10525a11c6b --- /dev/null +++ b/dev-infra/pr/merge/strategies/autosquash-merge.ts @@ -0,0 +1,73 @@ +import {join} from 'path'; +import {PullRequestFailure} from '../failures'; +import {PullRequest} from '../pull-request'; +import {MergeStrategy, TEMP_PR_HEAD_BRANCH} from './strategy'; + +/** Path to the commit message filter script. Git expects this paths to use forward slashes. */ +const MSG_FILTER_SCRIPT = join(__dirname, './commit-message-filter.js').replace(/\\/g, '/'); + +/** + * Merge strategy that does not use the Github API for merging. Instead, it fetches + * all target branches and the PR locally. The PR is then cherry-picked with autosquash + * enabled into the target branches. The benefit is the support for fixup and squash commits. + * A notable downside though is that Github does not show the PR as `Merged` due to non + * fast-forward merges + */ +export class AutosquashMergeStrategy extends MergeStrategy { + /** + * Merges the specified pull request into the target branches and pushes the target + * branches upstream. This method requires the temporary target branches to be fetched + * already as we don't want to fetch the target branches per pull request merge. This + * would causes unnecessary multiple fetch requests when multiple PRs are merged. + * @throws {GitCommandError} An unknown Git command error occurred that is not + * specific to the pull request merge. + * @returns A pull request failure or null in case of success. + */ + async merge(pullRequest: PullRequest): Promise { + const {prNumber, targetBranches, requiredBaseSha, needsCommitMessageFixup} = pullRequest; + // In case a required base is specified for this pull request, check if the pull + // request contains the given commit. If not, return a pull request failure. This + // check is useful for enforcing that PRs are rebased on top of a given commit. e.g. + // a commit that changes the codeowner ship validation. PRs which are not rebased + // could bypass new codeowner ship rules. + if (requiredBaseSha && !this.git.hasCommit(TEMP_PR_HEAD_BRANCH, requiredBaseSha)) { + return PullRequestFailure.unsatisfiedBaseSha(); + } + + // Git revision range that matches the pull request commits. + const revisionRange = this.getPullRequestRevisionRange(pullRequest); + // Git revision for the first commit the pull request is based on. + const baseRevision = this.getPullRequestBaseRevision(pullRequest); + + // By default, we rebase the pull request so that fixup or squash commits are + // automatically collapsed. Optionally, if a commit message fixup is needed, we + // make this an interactive rebase so that commits can be selectively modified + // before the merge completes. + const branchBeforeRebase = this.git.getCurrentBranch(); + const rebaseArgs = ['--autosquash', baseRevision, TEMP_PR_HEAD_BRANCH]; + if (needsCommitMessageFixup) { + this.git.run(['rebase', '--interactive', ...rebaseArgs], {stdio: 'inherit'}); + } else { + this.git.run(['rebase', ...rebaseArgs]); + } + + // Update pull requests commits to reference the pull request. This matches what + // Github does when pull requests are merged through the Web UI. The motivation is + // that it should be easy to determine which pull request contained a given commit. + // **Note**: The filter-branch command relies on the working tree, so we want to make + // sure that we are on the initial branch where the merge script has been run. + this.git.run(['checkout', '-f', branchBeforeRebase]); + this.git.run( + ['filter-branch', '-f', '--msg-filter', `${MSG_FILTER_SCRIPT} ${prNumber}`, revisionRange]); + + // Cherry-pick the pull request into all determined target branches. + const failedBranches = this.cherryPickIntoTargetBranches(revisionRange, targetBranches); + + if (failedBranches.length) { + return PullRequestFailure.mergeConflicts(failedBranches); + } + + this.pushTargetBranchesUpstream(targetBranches); + return null; + } +} diff --git a/dev-infra/pr/merge/strategies/commit-message-filter.js b/dev-infra/pr/merge/strategies/commit-message-filter.js new file mode 100644 index 0000000000000..228c6076c571c --- /dev/null +++ b/dev-infra/pr/merge/strategies/commit-message-filter.js @@ -0,0 +1,38 @@ +#!/usr/bin/env node + +/** + * Script that can be passed as commit message filter to `git filter-branch --msg-filter`. + * The script rewrites commit messages to contain a Github instruction to close the + * corresponding pull request. For more details. See: https://git.io/Jv64r. + */ + +if (require.main === module) { + const [prNumber] = process.argv.slice(2); + if (!prNumber) { + console.error('No pull request number specified.'); + process.exit(1); + } + + let commitMessage = ''; + process.stdin.setEncoding('utf8'); + process.stdin.on('readable', () => { + const chunk = process.stdin.read(); + if (chunk !== null) { + commitMessage += chunk; + } + }); + + process.stdin.on('end', () => { + console.info(rewriteCommitMessage(commitMessage, prNumber)); + }); +} + +function rewriteCommitMessage(message, prNumber) { + const lines = message.split(/\n/); + // Add the pull request number to the commit message title. This matches what + // Github does when PRs are merged on the web through the `Squash and Merge` button. + lines[0] += ` (#${prNumber})`; + // Push a new line that instructs Github to close the specified pull request. + lines.push(`PR Close #${prNumber}`); + return lines.join('\n'); +} diff --git a/dev-infra/pr/merge/strategies/strategy.ts b/dev-infra/pr/merge/strategies/strategy.ts new file mode 100644 index 0000000000000..63209ef9bcb25 --- /dev/null +++ b/dev-infra/pr/merge/strategies/strategy.ts @@ -0,0 +1,131 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {PullRequestFailure} from '../failures'; +import {GitClient} from '../git'; +import {PullRequest} from '../pull-request'; + +/** + * Name of a temporary branch that contains the head of a currently-processed PR. Note + * that a branch name should be used that most likely does not conflict with other local + * development branches. + */ +export const TEMP_PR_HEAD_BRANCH = 'merge_pr_head'; + +/** + * Base class for merge strategies. A merge strategy accepts a pull request and + * merges it into the determined target branches. + */ +export abstract class MergeStrategy { + constructor(protected git: GitClient) {} + + /** + * Prepares a merge of the given pull request. The strategy by default will + * fetch all target branches and the pull request into local temporary branches. + */ + async prepare(pullRequest: PullRequest) { + this.fetchTargetBranches( + pullRequest.targetBranches, `pull/${pullRequest.prNumber}/head:${TEMP_PR_HEAD_BRANCH}`); + } + + /** + * Performs the merge of the given pull request. This needs to be implemented + * by individual merge strategies. + */ + abstract merge(pullRequest: PullRequest): Promise; + + /** Cleans up the pull request merge. e.g. deleting temporary local branches. */ + async cleanup(pullRequest: PullRequest) { + // Delete all temporary target branches. + pullRequest.targetBranches.forEach( + branchName => this.git.run(['branch', '-D', this.getLocalTargetBranchName(branchName)])); + + // Delete temporary branch for the pull request head. + this.git.run(['branch', '-D', TEMP_PR_HEAD_BRANCH]); + } + + /** Gets the revision range for all commits in the given pull request. */ + protected getPullRequestRevisionRange(pullRequest: PullRequest): string { + return `${this.getPullRequestBaseRevision(pullRequest)}..${TEMP_PR_HEAD_BRANCH}`; + } + + /** Gets the base revision of a pull request. i.e. the commit the PR is based on. */ + protected getPullRequestBaseRevision(pullRequest: PullRequest): string { + return `${TEMP_PR_HEAD_BRANCH}~${pullRequest.commitCount}`; + } + + /** Gets a deterministic local branch name for a given branch. */ + protected getLocalTargetBranchName(targetBranch: string): string { + return `merge_pr_target_${targetBranch.replace(/\//g, '_')}`; + } + + /** + * Cherry-picks the given revision range into the specified target branches. + * @returns A list of branches for which the revisions could not be cherry-picked into. + */ + protected cherryPickIntoTargetBranches(revisionRange: string, targetBranches: string[], options: { + dryRun?: boolean + } = {}) { + const cherryPickArgs = [revisionRange]; + const failedBranches: string[] = []; + + if (options.dryRun) { + // https://git-scm.com/docs/git-cherry-pick#Documentation/git-cherry-pick.txt---no-commit + // This causes `git cherry-pick` to not generate any commits. Instead, the changes are + // applied directly in the working tree. This allow us to easily discard the changes + // for dry-run purposes. + cherryPickArgs.push('--no-commit'); + } + + // Cherry-pick the refspec into all determined target branches. + for (const branchName of targetBranches) { + const localTargetBranch = this.getLocalTargetBranchName(branchName); + // Checkout the local target branch. + this.git.run(['checkout', localTargetBranch]); + // Cherry-pick the refspec into the target branch. + if (this.git.runGraceful(['cherry-pick', ...cherryPickArgs]).status !== 0) { + // Abort the failed cherry-pick. We do this because Git persists the failed + // cherry-pick state globally in the repository. This could prevent future + // pull request merges as a Git thinks a cherry-pick is still in progress. + this.git.runGraceful(['cherry-pick', '--abort']); + failedBranches.push(branchName); + } + // If we run with dry run mode, we reset the local target branch so that all dry-run + // cherry-pick changes are discard. Changes are applied to the working tree and index. + if (options.dryRun) { + this.git.run(['reset', '--hard', 'HEAD']); + } + } + return failedBranches; + } + + /** + * Fetches the given target branches. Also accepts a list of additional refspecs that + * should be fetched. This is helpful as multiple slow fetches could be avoided. + */ + protected fetchTargetBranches(names: string[], ...extraRefspecs: string[]) { + const fetchRefspecs = names.map(targetBranch => { + const localTargetBranch = this.getLocalTargetBranchName(targetBranch); + return `refs/heads/${targetBranch}:${localTargetBranch}`; + }); + // Fetch all target branches with a single command. We don't want to fetch them + // individually as that could cause an unnecessary slow-down. + this.git.run(['fetch', '-f', this.git.repoGitUrl, ...fetchRefspecs, ...extraRefspecs]); + } + + /** Pushes the given target branches upstream. */ + protected pushTargetBranchesUpstream(names: string[]) { + const pushRefspecs = names.map(targetBranch => { + const localTargetBranch = this.getLocalTargetBranchName(targetBranch); + return `${localTargetBranch}:refs/heads/${targetBranch}`; + }); + // Push all target branches with a single command if we don't run in dry-run mode. + // We don't want to push them individually as that could cause an unnecessary slow-down. + this.git.run(['push', this.git.repoGitUrl, ...pushRefspecs]); + } +} diff --git a/dev-infra/pr/merge/string-pattern.ts b/dev-infra/pr/merge/string-pattern.ts new file mode 100644 index 0000000000000..01c9c7122ff47 --- /dev/null +++ b/dev-infra/pr/merge/string-pattern.ts @@ -0,0 +1,12 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +/** Checks whether the specified value matches the given pattern. */ +export function matchesPattern(value: string, pattern: RegExp|string): boolean { + return typeof pattern === 'string' ? value === pattern : pattern.test(value); +} diff --git a/dev-infra/pr/merge/target-label.ts b/dev-infra/pr/merge/target-label.ts new file mode 100644 index 0000000000000..dec769b1ee86c --- /dev/null +++ b/dev-infra/pr/merge/target-label.ts @@ -0,0 +1,28 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {MergeConfig, TargetLabel} from './config'; +import {matchesPattern} from './string-pattern'; + +/** Gets the target label from the specified pull request labels. */ +export function getTargetLabelFromPullRequest(config: MergeConfig, labels: string[]): TargetLabel| + null { + for (const label of labels) { + const match = config.labels.find(({pattern}) => matchesPattern(label, pattern)); + if (match !== undefined) { + return match; + } + } + return null; +} + +/** Gets the branches from the specified target label. */ +export function getBranchesFromTargetLabel( + label: TargetLabel, githubTargetBranch: string): string[] { + return typeof label.branches === 'function' ? label.branches(githubTargetBranch) : label.branches; +} diff --git a/dev-infra/pr/merge/task.ts b/dev-infra/pr/merge/task.ts new file mode 100644 index 0000000000000..37f841bd37fe7 --- /dev/null +++ b/dev-infra/pr/merge/task.ts @@ -0,0 +1,105 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {MergeConfig} from './config'; +import {PullRequestFailure} from './failures'; +import {GitClient, GitCommandError} from './git'; +import {isPullRequest, loadAndValidatePullRequest,} from './pull-request'; +import {GithubApiMergeStrategy} from './strategies/api-merge'; +import {AutosquashMergeStrategy} from './strategies/autosquash-merge'; + +/** Describes the status of a pull request merge. */ +export const enum MergeStatus { + UNKNOWN_GIT_ERROR, + DIRTY_WORKING_DIR, + SUCCESS, + FAILED, +} + +/** Result of a pull request merge. */ +export interface MergeResult { + /** Overall status of the merge. */ + status: MergeStatus; + /** List of pull request failures. */ + failure?: PullRequestFailure; +} + +/** + * Class that accepts a merge script configuration and Github token. It provides + * a programmatic interface for merging multiple pull requests based on their + * labels that have been resolved through the merge script configuration. + */ +export class PullRequestMergeTask { + /** Git client that can be used to execute Git commands. */ + git = new GitClient(this.projectRoot, this._githubToken, this.config); + + constructor( + public projectRoot: string, public config: MergeConfig, private _githubToken: string) {} + + /** + * Merges the given pull request and pushes it upstream. + * @param prNumber Pull request that should be merged. + * @param force Whether non-critical pull request failures should be ignored. + */ + async merge(prNumber: number, force = false): Promise { + if (this.git.hasUncommittedChanges()) { + return {status: MergeStatus.DIRTY_WORKING_DIR}; + } + + const pullRequest = await loadAndValidatePullRequest(this, prNumber, force); + + if (!isPullRequest(pullRequest)) { + return {status: MergeStatus.FAILED, failure: pullRequest}; + } + + const strategy = this.config.githubApiMerge ? + new GithubApiMergeStrategy(this.git, this.config.githubApiMerge) : + new AutosquashMergeStrategy(this.git); + + // Branch that is currently checked out so that we can switch back to it once + // the pull request has been merged. + let previousBranch: null|string = null; + + // The following block runs Git commands as child processes. These Git commands can fail. + // We want to capture these command errors and return an appropriate merge request status. + try { + previousBranch = this.git.getCurrentBranch(); + + // Run preparations for the merge (e.g. fetching branches). + await strategy.prepare(pullRequest); + + // Perform the merge and capture potential failures. + const failure = await strategy.merge(pullRequest); + if (failure !== null) { + return {status: MergeStatus.FAILED, failure}; + } + + // Switch back to the previous branch. We need to do this before deleting the temporary + // branches because we cannot delete branches which are currently checked out. + this.git.run(['checkout', '-f', previousBranch]); + + await strategy.cleanup(pullRequest); + + // Return a successful merge status. + return {status: MergeStatus.SUCCESS}; + } catch (e) { + // Catch all git command errors and return a merge result w/ git error status code. + // Other unknown errors which aren't caused by a git command are re-thrown. + if (e instanceof GitCommandError) { + return {status: MergeStatus.UNKNOWN_GIT_ERROR}; + } + throw e; + } finally { + // Always try to restore the branch if possible. We don't want to leave + // the repository in a different state than before. + if (previousBranch !== null) { + this.git.runGraceful(['checkout', '-f', previousBranch]); + } + } + } +} diff --git a/package.json b/package.json index 41e4f1b544ee7..0875dd7163c3a 100644 --- a/package.json +++ b/package.json @@ -57,6 +57,7 @@ "@bazel/terser": "1.6.0", "@bazel/typescript": "1.6.0", "@microsoft/api-extractor": "~7.6.0", + "@octokit/rest": "16.28.7", "@schematics/angular": "9.0.3", "@types/angular": "^1.6.47", "@types/babel__core": "^7.1.6", diff --git a/yarn.lock b/yarn.lock index a11084e53a2fb..f2a70c47d5d39 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1054,6 +1054,15 @@ is-plain-object "^3.0.0" universal-user-agent "^5.0.0" +"@octokit/endpoint@^6.0.1": + version "6.0.1" + resolved "https://registry.yarnpkg.com/@octokit/endpoint/-/endpoint-6.0.1.tgz#16d5c0e7a83e3a644d1ddbe8cded6c3d038d31d7" + integrity sha512-pOPHaSz57SFT/m3R5P8MUu4wLPszokn5pXcB/pzavLTQf2jbU+6iayTvzaY6/BiotuRS0qyEUkx3QglT4U958A== + dependencies: + "@octokit/types" "^2.11.1" + is-plain-object "^3.0.0" + universal-user-agent "^5.0.0" + "@octokit/graphql@^4.3.1": version "4.3.1" resolved "https://registry.yarnpkg.com/@octokit/graphql/-/graphql-4.3.1.tgz#9ee840e04ed2906c7d6763807632de84cdecf418" @@ -1063,6 +1072,15 @@ "@octokit/types" "^2.0.0" universal-user-agent "^4.0.0" +"@octokit/request-error@^1.0.2": + version "1.2.1" + resolved "https://registry.yarnpkg.com/@octokit/request-error/-/request-error-1.2.1.tgz#ede0714c773f32347576c25649dc013ae6b31801" + integrity sha512-+6yDyk1EES6WK+l3viRDElw96MvwfJxCt45GvmjDUKWjYIb3PJZQkq3i46TwGwoPD4h8NmTrENmtyA1FwbmhRA== + dependencies: + "@octokit/types" "^2.0.0" + deprecation "^2.0.0" + once "^1.4.0" + "@octokit/request-error@^2.0.0": version "2.0.0" resolved "https://registry.yarnpkg.com/@octokit/request-error/-/request-error-2.0.0.tgz#94ca7293373654400fbb2995f377f9473e00834b" @@ -1072,6 +1090,20 @@ deprecation "^2.0.0" once "^1.4.0" +"@octokit/request@^5.0.0": + version "5.4.2" + resolved "https://registry.yarnpkg.com/@octokit/request/-/request-5.4.2.tgz#74f8e5bbd39dc738a1b127629791f8ad1b3193ee" + integrity sha512-zKdnGuQ2TQ2vFk9VU8awFT4+EYf92Z/v3OlzRaSh4RIP0H6cvW1BFPXq4XYvNez+TPQjqN+0uSkCYnMFFhcFrw== + dependencies: + "@octokit/endpoint" "^6.0.1" + "@octokit/request-error" "^2.0.0" + "@octokit/types" "^2.11.1" + deprecation "^2.0.0" + is-plain-object "^3.0.0" + node-fetch "^2.3.0" + once "^1.4.0" + universal-user-agent "^5.0.0" + "@octokit/request@^5.3.0": version "5.3.4" resolved "https://registry.yarnpkg.com/@octokit/request/-/request-5.3.4.tgz#fbc950bf785d59da3b0399fc6d042c8cf52e2905" @@ -1086,6 +1118,25 @@ once "^1.4.0" universal-user-agent "^5.0.0" +"@octokit/rest@16.28.7": + version "16.28.7" + resolved "https://registry.yarnpkg.com/@octokit/rest/-/rest-16.28.7.tgz#a2c2db5b318da84144beba82d19c1a9dbdb1a1fa" + integrity sha512-cznFSLEhh22XD3XeqJw51OLSfyL2fcFKUO+v2Ep9MTAFfFLS1cK1Zwd1yEgQJmJoDnj4/vv3+fGGZweG+xsbIA== + dependencies: + "@octokit/request" "^5.0.0" + "@octokit/request-error" "^1.0.2" + atob-lite "^2.0.0" + before-after-hook "^2.0.0" + btoa-lite "^1.0.0" + deprecation "^2.0.0" + lodash.get "^4.4.2" + lodash.set "^4.3.2" + lodash.uniq "^4.5.0" + octokit-pagination-methods "^1.1.0" + once "^1.4.0" + universal-user-agent "^3.0.0" + url-template "^2.0.8" + "@octokit/types@^2.0.0": version "2.5.1" resolved "https://registry.yarnpkg.com/@octokit/types/-/types-2.5.1.tgz#22563b3bb50034bea3176eac1860340c5e812e2a" @@ -1093,6 +1144,13 @@ dependencies: "@types/node" ">= 8" +"@octokit/types@^2.11.1": + version "2.15.0" + resolved "https://registry.yarnpkg.com/@octokit/types/-/types-2.15.0.tgz#b2070520207727bc6ab3a9caa1e4f60b0434bfa8" + integrity sha512-0mnpenB8rLhBVu8VUklp38gWi+EatjvcEcLWcdProMKauSaQWWepOAybZ714sOGsEyhXPlIcHICggn8HUsCXVw== + dependencies: + "@types/node" ">= 8" + "@protobufjs/aspromise@^1.1.1", "@protobufjs/aspromise@^1.1.2": version "1.1.2" resolved "https://registry.yarnpkg.com/@protobufjs/aspromise/-/aspromise-1.1.2.tgz#9b8b0cc663d669a7d8f6f5d0893a14d348f30fbf" @@ -2251,6 +2309,11 @@ asynckit@^0.4.0: resolved "https://registry.yarnpkg.com/asynckit/-/asynckit-0.4.0.tgz#c79ed97f7f34cb8f2ba1bc9790bcc366474b4b79" integrity sha1-x57Zf380y48robyXkLzDZkdLS3k= +atob-lite@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/atob-lite/-/atob-lite-2.0.0.tgz#0fef5ad46f1bd7a8502c65727f0367d5ee43d696" + integrity sha1-D+9a1G8b16hQLGVyfwNn1e5D1pY= + atob@^2.1.2: version "2.1.2" resolved "https://registry.yarnpkg.com/atob/-/atob-2.1.2.tgz#6d9517eb9e030d2436666651e86bd9f6f13533c9" @@ -2407,6 +2470,11 @@ beeper@^1.0.0: resolved "https://registry.yarnpkg.com/beeper/-/beeper-1.1.1.tgz#e6d5ea8c5dad001304a70b22638447f69cb2f809" integrity sha1-5tXqjF2tABMEpwsiY4RH9pyy+Ak= +before-after-hook@^2.0.0: + version "2.1.0" + resolved "https://registry.yarnpkg.com/before-after-hook/-/before-after-hook-2.1.0.tgz#b6c03487f44e24200dd30ca5e6a1979c5d2fb635" + integrity sha512-IWIbu7pMqyw3EAJHzzHbWa85b6oud/yfKYg5rqB5hNE8CeMi3nX+2C2sj0HswfblST86hpVEOAb9x34NZd6P7A== + better-assert@~1.0.0: version "1.0.2" resolved "https://registry.yarnpkg.com/better-assert/-/better-assert-1.0.2.tgz#40866b9e1b9e0b55b481894311e68faffaebc522" @@ -2704,6 +2772,11 @@ browserstacktunnel-wrapper@2.0.1: https-proxy-agent "^1.0.0" unzip "~0.1.9" +btoa-lite@^1.0.0: + version "1.0.0" + resolved "https://registry.yarnpkg.com/btoa-lite/-/btoa-lite-1.0.0.tgz#337766da15801210fdd956c22e9c6891ab9d0337" + integrity sha1-M3dm2hWAEhD92VbCLpxokaudAzc= + buffer-alloc-unsafe@^1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/buffer-alloc-unsafe/-/buffer-alloc-unsafe-1.1.0.tgz#bd7dc26ae2972d0eda253be061dba992349c19f0" @@ -8550,7 +8623,7 @@ lodash.flatten@^4.4.0: resolved "https://registry.yarnpkg.com/lodash.flatten/-/lodash.flatten-4.4.0.tgz#f31c22225a9632d2bbf8e4addbef240aa765a61f" integrity sha1-8xwiIlqWMtK7+OSt2+8kCqdlph8= -lodash.get@^4.0.0: +lodash.get@^4.0.0, lodash.get@^4.4.2: version "4.4.2" resolved "https://registry.yarnpkg.com/lodash.get/-/lodash.get-4.4.2.tgz#2d177f652fa31e939b4438d5341499dfa3825e99" integrity sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk= @@ -8660,6 +8733,11 @@ lodash.restparam@^3.0.0: resolved "https://registry.yarnpkg.com/lodash.restparam/-/lodash.restparam-3.6.1.tgz#936a4e309ef330a7645ed4145986c85ae5b20805" integrity sha1-k2pOMJ7zMKdkXtQUWYbIWuWyCAU= +lodash.set@^4.3.2: + version "4.3.2" + resolved "https://registry.yarnpkg.com/lodash.set/-/lodash.set-4.3.2.tgz#d8757b1da807dde24816b0d6a84bea1a76230b23" + integrity sha1-2HV7HagH3eJIFrDWqEvqGnYjCyM= + lodash.snakecase@^4.1.1: version "4.1.1" resolved "https://registry.yarnpkg.com/lodash.snakecase/-/lodash.snakecase-4.1.1.tgz#39d714a35357147837aefd64b5dcbb16becd8f8d" @@ -9918,6 +9996,11 @@ obuf@^1.0.0, obuf@^1.1.2: resolved "https://registry.yarnpkg.com/obuf/-/obuf-1.1.2.tgz#09bea3343d41859ebd446292d11c9d4db619084e" integrity sha512-PX1wu0AmAdPqOL1mWhqmlOd8kOIZQwGZw6rh7uby9fTc5lhaOWFLX3I6R1hrF9k3zUY40e6igsLGkDXK92LJNg== +octokit-pagination-methods@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/octokit-pagination-methods/-/octokit-pagination-methods-1.1.0.tgz#cf472edc9d551055f9ef73f6e42b4dbb4c80bea4" + integrity sha512-fZ4qZdQ2nxJvtcasX7Ghl+WlWS/d9IgnBIwFZXVNNZUmzpno91SX5bc5vuxiuKoCtK78XxGGNuSCrDC7xYB3OQ== + on-finished@^2.2.0, on-finished@~2.3.0: version "2.3.0" resolved "https://registry.yarnpkg.com/on-finished/-/on-finished-2.3.0.tgz#20f1336481b083cd75337992a16971aa2d906947" @@ -10093,7 +10176,7 @@ os-locale@^3.0.0, os-locale@^3.1.0: lcid "^2.0.0" mem "^4.0.0" -os-name@^3.1.0: +os-name@^3.0.0, os-name@^3.1.0: version "3.1.0" resolved "https://registry.yarnpkg.com/os-name/-/os-name-3.1.0.tgz#dec19d966296e1cd62d701a5a66ee1ddeae70801" integrity sha512-h8L+8aNjNcMpo/mAIBPn5PXCM16iyPGjHNWo6U1YO8sJTMHtEtyczI6QJnLoplswm6goopQkqc7OAnjhWcugVg== @@ -13973,6 +14056,13 @@ universal-analytics@^0.4.16, universal-analytics@^0.4.20: request "^2.88.0" uuid "^3.0.0" +universal-user-agent@^3.0.0: + version "3.0.0" + resolved "https://registry.yarnpkg.com/universal-user-agent/-/universal-user-agent-3.0.0.tgz#4cc88d68097bffd7ac42e3b7c903e7481424b4b9" + integrity sha512-T3siHThqoj5X0benA5H0qcDnrKGXzU8TKoX15x/tQHw1hQBvIEBHjxQ2klizYsqBOO/Q+WuxoQUihadeeqDnoA== + dependencies: + os-name "^3.0.0" + universal-user-agent@^4.0.0: version "4.0.1" resolved "https://registry.yarnpkg.com/universal-user-agent/-/universal-user-agent-4.0.1.tgz#fd8d6cb773a679a709e967ef8288a31fcc03e557" @@ -14101,6 +14191,11 @@ url-parse@^1.4.3: querystringify "^2.1.1" requires-port "^1.0.0" +url-template@^2.0.8: + version "2.0.8" + resolved "https://registry.yarnpkg.com/url-template/-/url-template-2.0.8.tgz#fc565a3cccbff7730c775f5641f9555791439f21" + integrity sha1-/FZaPMy/93MMd19WQflVV5FDnyE= + url@^0.11.0: version "0.11.0" resolved "https://registry.yarnpkg.com/url/-/url-0.11.0.tgz#3838e97cfc60521eb73c525a8e55bfdd9e2e28f1" From 6d549e90114ad721f5303984453d16fb93b28e57 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 15 May 2020 17:21:01 +0200 Subject: [PATCH 2/6] feat(dev-infra): integrate merge script into ng-dev cli Integrates the merge script into the `ng-dev` CLI. The goal is that caretakers can run the same command across repositories to merge a pull request. The command is as followed: `yarn ng-dev pr merge `. --- dev-infra/pr/BUILD.bazel | 14 +--- dev-infra/pr/cli.ts | 41 +++-------- .../pr/discover-new-conflicts/BUILD.bazel | 19 +++++ dev-infra/pr/discover-new-conflicts/cli.ts | 33 +++++++++ .../index.ts} | 10 ++- dev-infra/pr/merge/BUILD.bazel | 17 +++++ dev-infra/pr/merge/cli.ts | 33 +++++++++ dev-infra/pr/merge/config.ts | 69 +++++++++++++++---- dev-infra/pr/merge/git.ts | 20 +++--- dev-infra/pr/merge/index.ts | 46 ++++++++----- dev-infra/pr/merge/pull-request.ts | 4 +- dev-infra/pr/merge/strategies/api-merge.ts | 4 +- dev-infra/pr/merge/task.ts | 5 +- dev-infra/utils/BUILD.bazel | 1 + dev-infra/utils/config.ts | 17 +++-- dev-infra/{pr/merge => utils}/console.ts | 0 16 files changed, 236 insertions(+), 97 deletions(-) create mode 100644 dev-infra/pr/discover-new-conflicts/BUILD.bazel create mode 100644 dev-infra/pr/discover-new-conflicts/cli.ts rename dev-infra/pr/{discover-new-conflicts.ts => discover-new-conflicts/index.ts} (94%) create mode 100644 dev-infra/pr/merge/BUILD.bazel create mode 100644 dev-infra/pr/merge/cli.ts rename dev-infra/{pr/merge => utils}/console.ts (100%) diff --git a/dev-infra/pr/BUILD.bazel b/dev-infra/pr/BUILD.bazel index ad1765c96ed08..b700e03f74551 100644 --- a/dev-infra/pr/BUILD.bazel +++ b/dev-infra/pr/BUILD.bazel @@ -2,20 +2,12 @@ load("@npm_bazel_typescript//:index.bzl", "ts_library") ts_library( name = "pr", - srcs = glob([ - "*.ts", - ]), + srcs = ["cli.ts"], module_name = "@angular/dev-infra-private/pr", visibility = ["//dev-infra:__subpackages__"], deps = [ - "//dev-infra/utils", - "@npm//@types/cli-progress", - "@npm//@types/node", - "@npm//@types/shelljs", + "//dev-infra/pr/discover-new-conflicts", + "//dev-infra/pr/merge", "@npm//@types/yargs", - "@npm//cli-progress", - "@npm//shelljs", - "@npm//typed-graphqlify", - "@npm//yargs", ], ) diff --git a/dev-infra/pr/cli.ts b/dev-infra/pr/cli.ts index a27d39eb022d9..d5f4c7253ff15 100644 --- a/dev-infra/pr/cli.ts +++ b/dev-infra/pr/cli.ts @@ -7,39 +7,20 @@ */ import * as yargs from 'yargs'; -import {discoverNewConflictsForPr} from './discover-new-conflicts'; -/** A Date object 30 days ago. */ -const THIRTY_DAYS_AGO = (() => { - const date = new Date(); - // Set the hours, minutes and seconds to 0 to only consider date. - date.setHours(0, 0, 0, 0); - // Set the date to 30 days in the past. - date.setDate(date.getDate() - 30); - return date; -})(); +import {buildDiscoverNewConflictsCommand, handleDiscoverNewConflictsCommand} from './discover-new-conflicts/cli'; +import {buildMergeCommand, handleMergeCommand} from './merge/cli'; -/** Build the parser for the pr commands. */ +/** Build the parser for pull request commands. */ export function buildPrParser(localYargs: yargs.Argv) { - return localYargs.help().strict().demandCommand().command( - 'discover-new-conflicts ', - 'Check if a pending PR causes new conflicts for other pending PRs', - args => { - return args.option('date', { - description: 'Only consider PRs updated since provided date', - defaultDescription: '30 days ago', - coerce: Date.parse, - default: THIRTY_DAYS_AGO, - }); - }, - ({pr, date}) => { - // If a provided date is not able to be parsed, yargs provides it as NaN. - if (isNaN(date)) { - console.error('Unable to parse the value provided via --date flag'); - process.exit(1); - } - discoverNewConflictsForPr(pr, date); - }); + return localYargs.help() + .strict() + .demandCommand() + .command('merge ', 'Merge pull requests', buildMergeCommand, handleMergeCommand) + .command( + 'discover-new-conflicts ', + 'Check if a pending PR causes new conflicts for other pending PRs', + buildDiscoverNewConflictsCommand, handleDiscoverNewConflictsCommand) } if (require.main === module) { diff --git a/dev-infra/pr/discover-new-conflicts/BUILD.bazel b/dev-infra/pr/discover-new-conflicts/BUILD.bazel new file mode 100644 index 0000000000000..a0dbdca87a1ca --- /dev/null +++ b/dev-infra/pr/discover-new-conflicts/BUILD.bazel @@ -0,0 +1,19 @@ +load("@npm_bazel_typescript//:index.bzl", "ts_library") + +ts_library( + name = "discover-new-conflicts", + srcs = [ + "cli.ts", + "index.ts", + ], + module_name = "@angular/dev-infra-private/pr/discover-new-conflicts", + visibility = ["//dev-infra:__subpackages__"], + deps = [ + "//dev-infra/utils", + "@npm//@types/cli-progress", + "@npm//@types/node", + "@npm//@types/shelljs", + "@npm//@types/yargs", + "@npm//typed-graphqlify", + ], +) diff --git a/dev-infra/pr/discover-new-conflicts/cli.ts b/dev-infra/pr/discover-new-conflicts/cli.ts new file mode 100644 index 0000000000000..ec0b8ff9f620e --- /dev/null +++ b/dev-infra/pr/discover-new-conflicts/cli.ts @@ -0,0 +1,33 @@ +import {Arguments, Argv} from 'yargs'; + +import {discoverNewConflictsForPr} from './index'; + +/** Builds the discover-new-conflicts pull request command. */ +export function buildDiscoverNewConflictsCommand(yargs: Argv) { + return yargs.option('date', { + description: 'Only consider PRs updated since provided date', + defaultDescription: '30 days ago', + coerce: Date.parse, + default: getThirtyDaysAgoDate, + }); +} + +/** Handles the discover-new-conflicts pull request command. */ +export async function handleDiscoverNewConflictsCommand({prNumber, date}: Arguments) { + // If a provided date is not able to be parsed, yargs provides it as NaN. + if (isNaN(date)) { + console.error('Unable to parse the value provided via --date flag'); + process.exit(1); + } + await discoverNewConflictsForPr(prNumber, date); +} + +/** Gets a date object 30 days ago from today. */ +function getThirtyDaysAgoDate(): Date { + const date = new Date(); + // Set the hours, minutes and seconds to 0 to only consider date. + date.setHours(0, 0, 0, 0); + // Set the date to 30 days in the past. + date.setDate(date.getDate() - 30); + return date; +} diff --git a/dev-infra/pr/discover-new-conflicts.ts b/dev-infra/pr/discover-new-conflicts/index.ts similarity index 94% rename from dev-infra/pr/discover-new-conflicts.ts rename to dev-infra/pr/discover-new-conflicts/index.ts index 3f5d0f9474728..c688b4aa90d67 100644 --- a/dev-infra/pr/discover-new-conflicts.ts +++ b/dev-infra/pr/discover-new-conflicts/index.ts @@ -9,10 +9,10 @@ import {Bar} from 'cli-progress'; import {types as graphQLTypes} from 'typed-graphqlify'; -import {getConfig, NgDevConfig} from '../utils/config'; -import {getCurrentBranch, hasLocalChanges} from '../utils/git'; -import {getPendingPrs} from '../utils/github'; -import {exec} from '../utils/shelljs'; +import {getConfig, NgDevConfig} from '../../utils/config'; +import {getCurrentBranch, hasLocalChanges} from '../../utils/git'; +import {getPendingPrs} from '../../utils/github'; +import {exec} from '../../utils/shelljs'; /* GraphQL schema for the response body for each pending PR. */ @@ -67,8 +67,6 @@ export async function discoverNewConflictsForPr( const progressBar = new Bar({format: `[{bar}] ETA: {eta}s | {value}/{total}`}); /* PRs which were found to be conflicting. */ const conflicts: Array = []; - /* String version of the updatedAfter value, for logging. */ - const updatedAfterString = new Date(updatedAfter).toLocaleDateString(); console.info(`Requesting pending PRs from Github`); /** List of PRs from github currently known as mergable. */ diff --git a/dev-infra/pr/merge/BUILD.bazel b/dev-infra/pr/merge/BUILD.bazel new file mode 100644 index 0000000000000..68609c668a253 --- /dev/null +++ b/dev-infra/pr/merge/BUILD.bazel @@ -0,0 +1,17 @@ +load("@npm_bazel_typescript//:index.bzl", "ts_library") + +ts_library( + name = "merge", + srcs = glob(["**/*.ts"]), + module_name = "@angular/dev-infra-private/pr/merge", + visibility = ["//dev-infra:__subpackages__"], + deps = [ + "//dev-infra/commit-message", + "//dev-infra/utils", + "@npm//@octokit/rest", + "@npm//@types/inquirer", + "@npm//@types/node", + "@npm//@types/yargs", + "@npm//chalk", + ], +) diff --git a/dev-infra/pr/merge/cli.ts b/dev-infra/pr/merge/cli.ts new file mode 100644 index 0000000000000..8bec6a223143e --- /dev/null +++ b/dev-infra/pr/merge/cli.ts @@ -0,0 +1,33 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import chalk from 'chalk'; +import {Arguments, Argv} from 'yargs'; +import {GITHUB_TOKEN_GENERATE_URL, mergePullRequest} from './index'; + +/** Builds the options for the merge command. */ +export function buildMergeCommand(yargs: Argv) { + return yargs.help().strict().option('github-token', { + type: 'string', + description: 'Github token. If not set, token is retrieved from the environment variables.' + }) +} + +/** Handles the merge command. i.e. performs the merge of a specified pull request. */ +export async function handleMergeCommand(args: Arguments) { + const githubToken = args.githubToken || process.env.GITHUB_TOKEN || process.env.TOKEN; + if (!githubToken) { + console.error( + chalk.red('No Github token set. Please set the `GITHUB_TOKEN` environment variable.')); + console.error(chalk.red('Alternatively, pass the `--github-token` command line flag.')); + console.error(chalk.yellow(`You can generate a token here: ${GITHUB_TOKEN_GENERATE_URL}`)); + process.exit(1); + } + + await mergePullRequest(args.prNumber, githubToken); +} diff --git a/dev-infra/pr/merge/config.ts b/dev-infra/pr/merge/config.ts index b500fd4fc61e4..bf725008983c5 100644 --- a/dev-infra/pr/merge/config.ts +++ b/dev-infra/pr/merge/config.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {getAngularDevConfig} from '../../utils/config'; +import {getConfig, NgDevConfig} from '../../utils/config'; import {GithubApiMergeStrategyConfig} from './strategies/api-merge'; @@ -31,10 +31,30 @@ export interface TargetLabel { branches: string[]|((githubTargetBranch: string) => string[]); } +/** Describes the remote used for merging pull requests. */ +export interface MergeRemote { + /** Owner name of the repository. */ + owner: string; + /** Name of the repository. */ + name: string; + /** Whether SSH should be used for merging pull requests. */ + useSsh?: boolean +} + +/** + * Configuration for the merge script with all remote options specified. The + * default `MergeConfig` has does not require any of these options as defaults + * are provided by the common dev-infra github configuration. + */ +export type MergeConfigWithRemote = MergeConfig&{remote: MergeRemote}; + /** Configuration for the merge script. */ export interface MergeConfig { - /** Configuration for the upstream repository. */ - repository: {user: string; name: string; useSsh?: boolean}; + /** + * Configuration for the upstream remote. All of these options are optional as + * defaults are provided by the common dev-infra github configuration. + */ + remote?: Partial; /** List of target labels. */ labels: TargetLabel[]; /** Required base commits for given branches. */ @@ -54,34 +74,55 @@ export interface MergeConfig { githubApiMerge: false|GithubApiMergeStrategyConfig; } +/** + * Configuration of the merge script in the dev-infra configuration. Note that the + * merge configuration is retrieved lazily as usually these configurations rely + * on branch name computations. We don't want to run these immediately whenever + * the dev-infra configuration is loaded as that could slow-down other commands. + */ +export type DevInfraMergeConfig = NgDevConfig<{'merge': () => MergeConfig}>; + /** Loads and validates the merge configuration. */ -export function loadAndValidateConfig(): {config?: MergeConfig, errors?: string[]} { - const config = getAngularDevConfig<'merge', MergeConfig>().merge; - if (config === undefined) { +export function loadAndValidateConfig(): {config?: MergeConfigWithRemote, errors?: string[]} { + const config: Partial = getConfig(); + + if (config.merge === undefined) { return { errors: ['No merge configuration found. Set the `merge` configuration.'] } } - const errors = validateConfig(config); + + if (typeof config.merge !== 'function') { + return { + errors: ['Expected merge configuration to be defined lazily through a function.'] + } + } + + const mergeConfig = config.merge(); + const errors = validateMergeConfig(mergeConfig); + if (errors.length) { return {errors}; } - return {config}; + + if (mergeConfig.remote) { + mergeConfig.remote = {...config.github, ...mergeConfig.remote}; + } else { + mergeConfig.remote = config.github; + } + + // We ensures that the `remote` is set, so we can cast it to a `MergeConfigWithRemote`. + return {config: mergeConfig as MergeConfigWithRemote}; } /** Validates the specified configuration. Returns a list of failure messages. */ -function validateConfig(config: MergeConfig): string[] { +function validateMergeConfig(config: Partial): string[] { const errors: string[] = []; if (!config.labels) { errors.push('No label configuration.'); } else if (!Array.isArray(config.labels)) { errors.push('Label configuration needs to be an array.'); } - if (!config.repository) { - errors.push('No repository is configured.'); - } else if (!config.repository.user || !config.repository.name) { - errors.push('Repository configuration needs to specify a `user` and repository `name`.'); - } if (!config.claSignedLabel) { errors.push('No CLA signed label configured.'); } diff --git a/dev-infra/pr/merge/git.ts b/dev-infra/pr/merge/git.ts index 34468001f6d91..e43b7e0aab40a 100644 --- a/dev-infra/pr/merge/git.ts +++ b/dev-infra/pr/merge/git.ts @@ -8,7 +8,7 @@ import * as Octokit from '@octokit/rest'; import {spawnSync, SpawnSyncOptions, SpawnSyncReturns} from 'child_process'; -import {MergeConfig} from './config'; +import {MergeConfigWithRemote} from './config'; /** Error for failed Github API requests. */ export class GithubApiRequestError extends Error { @@ -28,14 +28,15 @@ export class GitCommandError extends Error { } export class GitClient { - /** Short-hand for accessing the repository configuration. */ - repoConfig = this._config.repository; - /** Octokit request parameters object for targeting the configured repository. */ - repoParams = {owner: this.repoConfig.user, repo: this.repoConfig.name}; + /** Short-hand for accessing the remote configuration. */ + remoteConfig = this._config.remote; + /** Octokit request parameters object for targeting the configured remote. */ + remoteParams = {owner: this.remoteConfig.owner, repo: this.remoteConfig.name}; /** URL that resolves to the configured repository. */ - repoGitUrl = this.repoConfig.useSsh ? - `git@github.com:${this.repoConfig.user}/${this.repoConfig.name}.git` : - `https://${this._githubToken}@github.com/${this.repoConfig.user}/${this.repoConfig.name}.git`; + repoGitUrl = this.remoteConfig.useSsh ? + `git@github.com:${this.remoteConfig.owner}/${this.remoteConfig.name}.git` : + `https://${this._githubToken}@github.com/${this.remoteConfig.owner}/${ + this.remoteConfig.name}.git`; /** Instance of the authenticated Github octokit API. */ api: Octokit; @@ -43,7 +44,8 @@ export class GitClient { private _tokenRegex = new RegExp(this._githubToken, 'g'); constructor( - private _projectRoot: string, private _githubToken: string, private _config: MergeConfig) { + private _projectRoot: string, private _githubToken: string, + private _config: MergeConfigWithRemote) { this.api = new Octokit({auth: _githubToken}); this.api.hook.error('request', error => { // Wrap API errors in a known error class. This allows us to diff --git a/dev-infra/pr/merge/index.ts b/dev-infra/pr/merge/index.ts index 4bc1e2a842263..3efb5e8cc165b 100644 --- a/dev-infra/pr/merge/index.ts +++ b/dev-infra/pr/merge/index.ts @@ -6,31 +6,45 @@ * found in the LICENSE file at https://angular.io/license */ -import {MergeResult, MergeStatus, PullRequestMergeTask} from './task'; -import {GithubApiRequestError} from './git'; import chalk from 'chalk'; -import {promptConfirm} from './console'; -import {loadAndValidateConfig} from './config'; + import {getRepoBaseDir} from '../../utils/config'; +import {promptConfirm} from '../../utils/console'; + +import {loadAndValidateConfig, MergeConfigWithRemote} from './config'; +import {GithubApiRequestError} from './git'; +import {MergeResult, MergeStatus, PullRequestMergeTask} from './task'; /** URL to the Github page where personal access tokens can be generated. */ export const GITHUB_TOKEN_GENERATE_URL = `https://github.com/settings/tokens`; /** - * Entry-point for the merge script CLI. The script can be used to merge individual pull requests - * into branches based on the `PR target` labels that have been set in a configuration. The script - * aims to reduce the manual work that needs to be performed to cherry-pick a PR into multiple - * branches based on a target label. + * Merges a given pull request based on labels configured in the given merge configuration. + * Pull requests can be merged with different strategies such as the Github API merge + * strategy, or the local autosquash strategy. Either strategy has benefits and downsides. + * More information on these strategies can be found in their dedicated strategy classes. + * + * See {@link GithubApiMergeStrategy} and {@link AutosquashMergeStrategy} + * + * @param prNumber Number of the pull request that should be merged. + * @param githubToken Github token used for merging (i.e. fetching and pushing) + * @param projectRoot Path to the local Git project that is used for merging. + * @param config Configuration for merging pull requests. */ -export async function mergePullRequest(prNumber: number, githubToken: string) { - const projectRoot = getRepoBaseDir(); - const {config, errors} = loadAndValidateConfig(); - - if (errors) { - console.error(chalk.red('Invalid configuration:')); - errors.forEach(desc => console.error(chalk.yellow(` - ${desc}`))); - process.exit(1); +export async function mergePullRequest( + prNumber: number, githubToken: string, projectRoot: string = getRepoBaseDir(), + config?: MergeConfigWithRemote) { + // If no explicit configuration has been specified, we load and validate + // the configuration from the shared dev-infra configuration. + if (config === undefined) { + const {config: _config, errors} = loadAndValidateConfig(); + if (errors) { + console.error(chalk.red('Invalid configuration:')); + errors.forEach(desc => console.error(chalk.yellow(` - ${desc}`))); + process.exit(1); + } + config = _config!; } const api = new PullRequestMergeTask(projectRoot, config, githubToken); diff --git a/dev-infra/pr/merge/pull-request.ts b/dev-infra/pr/merge/pull-request.ts index de9cfe13559b4..bf866dfe2ecc5 100644 --- a/dev-infra/pr/merge/pull-request.ts +++ b/dev-infra/pr/merge/pull-request.ts @@ -62,7 +62,7 @@ export async function loadAndValidatePullRequest( } const {data: {state}} = - await git.api.repos.getCombinedStatusForRef({...git.repoParams, ref: prData.head.sha}); + await git.api.repos.getCombinedStatusForRef({...git.remoteParams, ref: prData.head.sha}); if (state === 'failure' && !ignoreNonFatalFailures) { return PullRequestFailure.failingCiJobs(); @@ -93,7 +93,7 @@ export async function loadAndValidatePullRequest( async function fetchPullRequestFromGithub( git: GitClient, prNumber: number): Promise { try { - const result = await git.api.pulls.get({...git.repoParams, pull_number: prNumber}); + const result = await git.api.pulls.get({...git.remoteParams, pull_number: prNumber}); return result.data; } catch (e) { // If the pull request could not be found, we want to return `null` so diff --git a/dev-infra/pr/merge/strategies/api-merge.ts b/dev-infra/pr/merge/strategies/api-merge.ts index 9b7766e0a20de..2da887e9d2338 100644 --- a/dev-infra/pr/merge/strategies/api-merge.ts +++ b/dev-infra/pr/merge/strategies/api-merge.ts @@ -77,7 +77,7 @@ export class GithubApiMergeStrategy extends MergeStrategy { const mergeOptions: PullsMergeParams = { pull_number: prNumber, merge_method: method, - ...this.git.repoParams, + ...this.git.remoteParams, }; if (needsCommitMessageFixup) { @@ -190,7 +190,7 @@ export class GithubApiMergeStrategy extends MergeStrategy { /** Gets all commit messages of commits in the pull request. */ private async _getPullRequestCommitMessages({prNumber}: PullRequest) { const request = this.git.api.pulls.listCommits.endpoint.merge( - {...this.git.repoParams, pull_number: prNumber}); + {...this.git.remoteParams, pull_number: prNumber}); const allCommits: PullsListCommitsResponse = await this.git.api.paginate(request); return allCommits.map(({commit}) => commit.message); } diff --git a/dev-infra/pr/merge/task.ts b/dev-infra/pr/merge/task.ts index 37f841bd37fe7..e85065322d1a5 100644 --- a/dev-infra/pr/merge/task.ts +++ b/dev-infra/pr/merge/task.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {MergeConfig} from './config'; +import {MergeConfigWithRemote} from './config'; import {PullRequestFailure} from './failures'; import {GitClient, GitCommandError} from './git'; import {isPullRequest, loadAndValidatePullRequest,} from './pull-request'; @@ -39,7 +39,8 @@ export class PullRequestMergeTask { git = new GitClient(this.projectRoot, this._githubToken, this.config); constructor( - public projectRoot: string, public config: MergeConfig, private _githubToken: string) {} + public projectRoot: string, public config: MergeConfigWithRemote, + private _githubToken: string) {} /** * Merges the given pull request and pushes it upstream. diff --git a/dev-infra/utils/BUILD.bazel b/dev-infra/utils/BUILD.bazel index fe3c27e4776f1..48b015feeb3b7 100644 --- a/dev-infra/utils/BUILD.bazel +++ b/dev-infra/utils/BUILD.bazel @@ -7,6 +7,7 @@ ts_library( visibility = ["//dev-infra:__subpackages__"], deps = [ "@npm//@octokit/graphql", + "@npm//@types/inquirer", "@npm//@types/node", "@npm//@types/shelljs", "@npm//shelljs", diff --git a/dev-infra/utils/config.ts b/dev-infra/utils/config.ts index a20279ae69698..260c5a7930de1 100644 --- a/dev-infra/utils/config.ts +++ b/dev-infra/utils/config.ts @@ -9,13 +9,20 @@ import {join} from 'path'; import {exec} from 'shelljs'; +/** + * Describes the Github configuration for dev-infra. This configuration is + * used for API requests, determining the upstream remote, etc. + */ +export interface GithubConfig { + /** Owner name of the repository. */ + owner: string; + /** Name of the repository. */ + name: string; +} + /** The common configuration for ng-dev. */ type CommonConfig = { - /* Github repository configuration used for API Requests, determining upstream remote, etc. */ - github: { - owner: string, - name: string, - } + github: GithubConfig }; /** diff --git a/dev-infra/pr/merge/console.ts b/dev-infra/utils/console.ts similarity index 100% rename from dev-infra/pr/merge/console.ts rename to dev-infra/utils/console.ts From d74b2d1af7df29a3358e5575da88431e3ed76bfb Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 15 May 2020 17:21:47 +0200 Subject: [PATCH 3/6] build: configure dev-infra merge script Sets up the dev-infa merge script in the framework ng-dev configuration file. This allow us to use the script in the future. --- .ng-dev-config.ts | 63 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/.ng-dev-config.ts b/.ng-dev-config.ts index 0ee02484a2d9d..d85be46331d13 100644 --- a/.ng-dev-config.ts +++ b/.ng-dev-config.ts @@ -1,3 +1,7 @@ +import {exec} from 'shelljs'; + +import {MergeConfig} from './dev-infra/pr/merge/config'; + // The configuration for `ng-dev commit-message` commands. const commitMessage = { 'maxLength': 120, @@ -72,15 +76,72 @@ const format = { 'buildifier': true }; -// Github metadata information for `ng-dev` commands. +/** Github metadata information for `ng-dev` commands. */ const github = { owner: 'angular', name: 'angular', }; +/** + * Gets the name of the current patch branch. The patch branch is determined by + * looking for upstream branches that follow the format of `{major}.{minor}.x`. + */ +const getPatchBranchName = (): string => { + const branches = + exec( + `git ls-remote --heads https://github.com/${github.owner}/${github.name}.git`, + {silent: true}) + .trim() + .split('\n'); + + for (let i = branches.length - 1; i >= 0; i--) { + const branchName = branches[i]; + const matches = branchName.match(/refs\/heads\/([0-9]+\.[0-9]+\.x)/); + if (matches !== null) { + return matches[1]; + } + } + + throw Error('Could not determine patch branch name.'); +}; + +// Configuration for the `ng-dev pr merge` command. The command can be used +// for merging upstream pull requests into branches based on a PR target label. +const merge = () => { + const patchBranch = getPatchBranchName(); + const config: MergeConfig = { + githubApiMerge: false, + claSignedLabel: 'cla: yes', + mergeReadyLabel: /^PR action: merge(-assistance)?/, + commitMessageFixupLabel: 'commit message fixup', + labels: [ + { + pattern: 'PR target: master-only', + branches: ['master'], + }, + { + pattern: 'PR target: patch-only', + branches: [patchBranch], + }, + { + pattern: 'PR target: master & patch', + branches: ['master', patchBranch], + }, + ], + requiredBaseCommits: { + // PRs that target either `master` or the patch branch, need to be rebased + // on top of the latest commit message validation fix. + 'master': '4341743b4a6d7e23c6f944aa9e34166b701369a1', + [patchBranch]: '2a53f471592f424538802907aca1f60f1177a86d' + }, + }; + return config; +}; + // Export function to build ng-dev configuration object. module.exports = { commitMessage, format, github, + merge, }; From 9e1098290dc3e8ea8596a462133d707f113588ac Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 15 May 2020 15:49:38 +0200 Subject: [PATCH 4/6] fix(dev-infra): merge script autosquash should be interactive The components repo does not use the autosquash merge strategy, so recent changes to that seem to broke the autosquash strategy. Since we don't run the rebase in interactive mode, the `--autosquash` flag has no effect. This is by design in Git. We can make it work by setting the git sequence editor to `true` so that the rebase seems like an interactive one to Git, while it isn't one for the user. This matches conceptually with the merge script currently used in framework. The only difference is that we allow a real interactive rebase if the `commit message fixup` label is applied. This allows commit message modifications (and others) if needed. --- .../pr/merge/strategies/autosquash-merge.ts | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/dev-infra/pr/merge/strategies/autosquash-merge.ts b/dev-infra/pr/merge/strategies/autosquash-merge.ts index 1c10525a11c6b..25d9c8a0955b3 100644 --- a/dev-infra/pr/merge/strategies/autosquash-merge.ts +++ b/dev-infra/pr/merge/strategies/autosquash-merge.ts @@ -39,17 +39,18 @@ export class AutosquashMergeStrategy extends MergeStrategy { // Git revision for the first commit the pull request is based on. const baseRevision = this.getPullRequestBaseRevision(pullRequest); - // By default, we rebase the pull request so that fixup or squash commits are - // automatically collapsed. Optionally, if a commit message fixup is needed, we - // make this an interactive rebase so that commits can be selectively modified - // before the merge completes. + // We always rebase the pull request so that fixup or squash commits are automatically + // collapsed. Git's autosquash functionality does only work in interactive rebases, so + // our rebase is always interactive. In reality though, unless a commit message fixup + // is desired, we set the `GIT_SEQUENCE_EDITOR` environment variable to `true` so that + // the rebase seems interactive to Git, while it's not interactive to the user. + // See: https://github.com/git/git/commit/891d4a0313edc03f7e2ecb96edec5d30dc182294. const branchBeforeRebase = this.git.getCurrentBranch(); - const rebaseArgs = ['--autosquash', baseRevision, TEMP_PR_HEAD_BRANCH]; - if (needsCommitMessageFixup) { - this.git.run(['rebase', '--interactive', ...rebaseArgs], {stdio: 'inherit'}); - } else { - this.git.run(['rebase', ...rebaseArgs]); - } + const rebaseEnv = + needsCommitMessageFixup ? undefined : {...process.env, GIT_SEQUENCE_EDITOR: 'true'}; + this.git.run( + ['rebase', '--interactive', '--autosquash', baseRevision, TEMP_PR_HEAD_BRANCH], + {stdio: 'inherit', env: rebaseEnv}); // Update pull requests commits to reference the pull request. This matches what // Github does when pull requests are merged through the Web UI. The motivation is From 55ad4162c8b2526dafdb213f73a0635713a22843 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 15 May 2020 16:19:19 +0200 Subject: [PATCH 5/6] fix(dev-infra): incorrect base revision for merge script autosquash As mentioned in the previous commit, the autosquash strategy has not been used in the components repo, so we could easily regress. After thorough manual testing of the autosquash strategy again, now that the merge script will be moved to framework, it came to mind that there is a bug with the base revision in the autosquash merge strategy. The problem is that the base revision of a given PR is relying on the amount of commits in a PR. This is prone to error because the amount of commits could easily change in the autosquash merge strategy, because fixup or squash commits will be collapsed. Basically invalidating the base revision. To fix this, we fixate the base revision by determining the actual SHA. This one is guaranteed to not change after the autosquash rebase. The current merge script in framework fixates the revision by creating a separate branch, but there is no benefit in that, compared to just using an explicit SHA that doesn't need to be cleaned up.. --- dev-infra/pr/merge/strategies/autosquash-merge.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/dev-infra/pr/merge/strategies/autosquash-merge.ts b/dev-infra/pr/merge/strategies/autosquash-merge.ts index 25d9c8a0955b3..f4abffb1f51f8 100644 --- a/dev-infra/pr/merge/strategies/autosquash-merge.ts +++ b/dev-infra/pr/merge/strategies/autosquash-merge.ts @@ -34,10 +34,16 @@ export class AutosquashMergeStrategy extends MergeStrategy { return PullRequestFailure.unsatisfiedBaseSha(); } + // SHA for the first commit the pull request is based on. Usually we would able + // to just rely on the base revision provided by `getPullRequestBaseRevision`, but + // the revision would rely on the amount of commits in a pull request. This is not + // reliable as we rebase the PR with autosquash where the amount of commits could + // change. We work around this by parsing the base revision so that we have a fixated + // SHA before the autosquash rebase is performed. + const baseSha = + this.git.run(['rev-parse', this.getPullRequestBaseRevision(pullRequest)]).stdout.trim(); // Git revision range that matches the pull request commits. - const revisionRange = this.getPullRequestRevisionRange(pullRequest); - // Git revision for the first commit the pull request is based on. - const baseRevision = this.getPullRequestBaseRevision(pullRequest); + const revisionRange = `${baseSha}..${TEMP_PR_HEAD_BRANCH}`; // We always rebase the pull request so that fixup or squash commits are automatically // collapsed. Git's autosquash functionality does only work in interactive rebases, so @@ -49,7 +55,7 @@ export class AutosquashMergeStrategy extends MergeStrategy { const rebaseEnv = needsCommitMessageFixup ? undefined : {...process.env, GIT_SEQUENCE_EDITOR: 'true'}; this.git.run( - ['rebase', '--interactive', '--autosquash', baseRevision, TEMP_PR_HEAD_BRANCH], + ['rebase', '--interactive', '--autosquash', baseSha, TEMP_PR_HEAD_BRANCH], {stdio: 'inherit', env: rebaseEnv}); // Update pull requests commits to reference the pull request. This matches what From 18228890d3090ea9ed8db0a4f58796bc94b54f4f Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 18 May 2020 19:08:47 +0200 Subject: [PATCH 6/6] fixup! feat(dev-infra): integrate merge script into ng-dev cli Fix comment typo --- dev-infra/pr/merge/config.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev-infra/pr/merge/config.ts b/dev-infra/pr/merge/config.ts index bf725008983c5..5476c611d90f8 100644 --- a/dev-infra/pr/merge/config.ts +++ b/dev-infra/pr/merge/config.ts @@ -111,7 +111,8 @@ export function loadAndValidateConfig(): {config?: MergeConfigWithRemote, errors mergeConfig.remote = config.github; } - // We ensures that the `remote` is set, so we can cast it to a `MergeConfigWithRemote`. + // We always set the `remote` option, so we can safely cast the + // config to `MergeConfigWithRemote`. return {config: mergeConfig as MergeConfigWithRemote}; }