feat(dev-infra): integrate merge script into ng-dev cli (#37138)

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 <number>`.

PR Close #37138
This commit is contained in:
Paul Gschwendtner 2020-05-15 17:21:01 +02:00 committed by Kara Erickson
parent 318e9372c9
commit 8a3493af47
16 changed files with 237 additions and 97 deletions

View File

@ -2,20 +2,12 @@ load("@npm_bazel_typescript//:index.bzl", "ts_library")
ts_library( ts_library(
name = "pr", name = "pr",
srcs = glob([ srcs = ["cli.ts"],
"*.ts",
]),
module_name = "@angular/dev-infra-private/pr", module_name = "@angular/dev-infra-private/pr",
visibility = ["//dev-infra:__subpackages__"], visibility = ["//dev-infra:__subpackages__"],
deps = [ deps = [
"//dev-infra/utils", "//dev-infra/pr/discover-new-conflicts",
"@npm//@types/cli-progress", "//dev-infra/pr/merge",
"@npm//@types/node",
"@npm//@types/shelljs",
"@npm//@types/yargs", "@npm//@types/yargs",
"@npm//cli-progress",
"@npm//shelljs",
"@npm//typed-graphqlify",
"@npm//yargs",
], ],
) )

View File

@ -7,39 +7,20 @@
*/ */
import * as yargs from 'yargs'; import * as yargs from 'yargs';
import {discoverNewConflictsForPr} from './discover-new-conflicts';
/** A Date object 30 days ago. */ import {buildDiscoverNewConflictsCommand, handleDiscoverNewConflictsCommand} from './discover-new-conflicts/cli';
const THIRTY_DAYS_AGO = (() => { import {buildMergeCommand, handleMergeCommand} from './merge/cli';
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;
})();
/** Build the parser for the pr commands. */ /** Build the parser for pull request commands. */
export function buildPrParser(localYargs: yargs.Argv) { export function buildPrParser(localYargs: yargs.Argv) {
return localYargs.help().strict().demandCommand().command( return localYargs.help()
'discover-new-conflicts <pr>', .strict()
'Check if a pending PR causes new conflicts for other pending PRs', .demandCommand()
args => { .command('merge <pr-number>', 'Merge pull requests', buildMergeCommand, handleMergeCommand)
return args.option('date', { .command(
description: 'Only consider PRs updated since provided date', 'discover-new-conflicts <pr-number>',
defaultDescription: '30 days ago', 'Check if a pending PR causes new conflicts for other pending PRs',
coerce: Date.parse, buildDiscoverNewConflictsCommand, handleDiscoverNewConflictsCommand)
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);
});
} }
if (require.main === module) { if (require.main === module) {

View File

@ -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",
],
)

View File

@ -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;
}

View File

@ -9,10 +9,10 @@
import {Bar} from 'cli-progress'; import {Bar} from 'cli-progress';
import {types as graphQLTypes} from 'typed-graphqlify'; import {types as graphQLTypes} from 'typed-graphqlify';
import {getConfig, NgDevConfig} from '../utils/config'; import {getConfig, NgDevConfig} from '../../utils/config';
import {getCurrentBranch, hasLocalChanges} from '../utils/git'; import {getCurrentBranch, hasLocalChanges} from '../../utils/git';
import {getPendingPrs} from '../utils/github'; import {getPendingPrs} from '../../utils/github';
import {exec} from '../utils/shelljs'; import {exec} from '../../utils/shelljs';
/* GraphQL schema for the response body for each pending PR. */ /* 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}`}); const progressBar = new Bar({format: `[{bar}] ETA: {eta}s | {value}/{total}`});
/* PRs which were found to be conflicting. */ /* PRs which were found to be conflicting. */
const conflicts: Array<PullRequest> = []; const conflicts: Array<PullRequest> = [];
/* String version of the updatedAfter value, for logging. */
const updatedAfterString = new Date(updatedAfter).toLocaleDateString();
console.info(`Requesting pending PRs from Github`); console.info(`Requesting pending PRs from Github`);
/** List of PRs from github currently known as mergable. */ /** List of PRs from github currently known as mergable. */

View File

@ -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",
],
)

33
dev-infra/pr/merge/cli.ts Normal file
View File

@ -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);
}

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license * 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'; import {GithubApiMergeStrategyConfig} from './strategies/api-merge';
@ -31,10 +31,30 @@ export interface TargetLabel {
branches: string[]|((githubTargetBranch: string) => string[]); 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. */ /** Configuration for the merge script. */
export interface MergeConfig { 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<MergeRemote>;
/** List of target labels. */ /** List of target labels. */
labels: TargetLabel[]; labels: TargetLabel[];
/** Required base commits for given branches. */ /** Required base commits for given branches. */
@ -54,34 +74,56 @@ export interface MergeConfig {
githubApiMerge: false|GithubApiMergeStrategyConfig; 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. */ /** Loads and validates the merge configuration. */
export function loadAndValidateConfig(): {config?: MergeConfig, errors?: string[]} { export function loadAndValidateConfig(): {config?: MergeConfigWithRemote, errors?: string[]} {
const config = getAngularDevConfig<'merge', MergeConfig>().merge; const config: Partial<DevInfraMergeConfig> = getConfig();
if (config === undefined) {
if (config.merge === undefined) {
return { return {
errors: ['No merge configuration found. Set the `merge` configuration.'] 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) { if (errors.length) {
return {errors}; return {errors};
} }
return {config};
if (mergeConfig.remote) {
mergeConfig.remote = {...config.github, ...mergeConfig.remote};
} else {
mergeConfig.remote = config.github;
}
// We always set the `remote` option, so we can safely cast the
// config to `MergeConfigWithRemote`.
return {config: mergeConfig as MergeConfigWithRemote};
} }
/** Validates the specified configuration. Returns a list of failure messages. */ /** Validates the specified configuration. Returns a list of failure messages. */
function validateConfig(config: MergeConfig): string[] { function validateMergeConfig(config: Partial<MergeConfig>): string[] {
const errors: string[] = []; const errors: string[] = [];
if (!config.labels) { if (!config.labels) {
errors.push('No label configuration.'); errors.push('No label configuration.');
} else if (!Array.isArray(config.labels)) { } else if (!Array.isArray(config.labels)) {
errors.push('Label configuration needs to be an array.'); 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) { if (!config.claSignedLabel) {
errors.push('No CLA signed label configured.'); errors.push('No CLA signed label configured.');
} }

View File

@ -8,7 +8,7 @@
import * as Octokit from '@octokit/rest'; import * as Octokit from '@octokit/rest';
import {spawnSync, SpawnSyncOptions, SpawnSyncReturns} from 'child_process'; import {spawnSync, SpawnSyncOptions, SpawnSyncReturns} from 'child_process';
import {MergeConfig} from './config'; import {MergeConfigWithRemote} from './config';
/** Error for failed Github API requests. */ /** Error for failed Github API requests. */
export class GithubApiRequestError extends Error { export class GithubApiRequestError extends Error {
@ -28,14 +28,15 @@ export class GitCommandError extends Error {
} }
export class GitClient { export class GitClient {
/** Short-hand for accessing the repository configuration. */ /** Short-hand for accessing the remote configuration. */
repoConfig = this._config.repository; remoteConfig = this._config.remote;
/** Octokit request parameters object for targeting the configured repository. */ /** Octokit request parameters object for targeting the configured remote. */
repoParams = {owner: this.repoConfig.user, repo: this.repoConfig.name}; remoteParams = {owner: this.remoteConfig.owner, repo: this.remoteConfig.name};
/** URL that resolves to the configured repository. */ /** URL that resolves to the configured repository. */
repoGitUrl = this.repoConfig.useSsh ? repoGitUrl = this.remoteConfig.useSsh ?
`git@github.com:${this.repoConfig.user}/${this.repoConfig.name}.git` : `git@github.com:${this.remoteConfig.owner}/${this.remoteConfig.name}.git` :
`https://${this._githubToken}@github.com/${this.repoConfig.user}/${this.repoConfig.name}.git`; `https://${this._githubToken}@github.com/${this.remoteConfig.owner}/${
this.remoteConfig.name}.git`;
/** Instance of the authenticated Github octokit API. */ /** Instance of the authenticated Github octokit API. */
api: Octokit; api: Octokit;
@ -43,7 +44,8 @@ export class GitClient {
private _tokenRegex = new RegExp(this._githubToken, 'g'); private _tokenRegex = new RegExp(this._githubToken, 'g');
constructor( 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 = new Octokit({auth: _githubToken});
this.api.hook.error('request', error => { this.api.hook.error('request', error => {
// Wrap API errors in a known error class. This allows us to // Wrap API errors in a known error class. This allows us to

View File

@ -6,31 +6,45 @@
* found in the LICENSE file at https://angular.io/license * 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 chalk from 'chalk';
import {promptConfirm} from './console';
import {loadAndValidateConfig} from './config';
import {getRepoBaseDir} from '../../utils/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. */ /** URL to the Github page where personal access tokens can be generated. */
export const GITHUB_TOKEN_GENERATE_URL = `https://github.com/settings/tokens`; 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 * Merges a given pull request based on labels configured in the given merge configuration.
* into branches based on the `PR target` labels that have been set in a configuration. The script * Pull requests can be merged with different strategies such as the Github API merge
* aims to reduce the manual work that needs to be performed to cherry-pick a PR into multiple * strategy, or the local autosquash strategy. Either strategy has benefits and downsides.
* branches based on a target label. * 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) { export async function mergePullRequest(
const projectRoot = getRepoBaseDir(); prNumber: number, githubToken: string, projectRoot: string = getRepoBaseDir(),
const {config, errors} = loadAndValidateConfig(); config?: MergeConfigWithRemote) {
// If no explicit configuration has been specified, we load and validate
if (errors) { // the configuration from the shared dev-infra configuration.
console.error(chalk.red('Invalid configuration:')); if (config === undefined) {
errors.forEach(desc => console.error(chalk.yellow(` - ${desc}`))); const {config: _config, errors} = loadAndValidateConfig();
process.exit(1); 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); const api = new PullRequestMergeTask(projectRoot, config, githubToken);

View File

@ -62,7 +62,7 @@ export async function loadAndValidatePullRequest(
} }
const {data: {state}} = 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) { if (state === 'failure' && !ignoreNonFatalFailures) {
return PullRequestFailure.failingCiJobs(); return PullRequestFailure.failingCiJobs();
@ -93,7 +93,7 @@ export async function loadAndValidatePullRequest(
async function fetchPullRequestFromGithub( async function fetchPullRequestFromGithub(
git: GitClient, prNumber: number): Promise<Octokit.PullsGetResponse|null> { git: GitClient, prNumber: number): Promise<Octokit.PullsGetResponse|null> {
try { 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; return result.data;
} catch (e) { } catch (e) {
// If the pull request could not be found, we want to return `null` so // If the pull request could not be found, we want to return `null` so

View File

@ -77,7 +77,7 @@ export class GithubApiMergeStrategy extends MergeStrategy {
const mergeOptions: PullsMergeParams = { const mergeOptions: PullsMergeParams = {
pull_number: prNumber, pull_number: prNumber,
merge_method: method, merge_method: method,
...this.git.repoParams, ...this.git.remoteParams,
}; };
if (needsCommitMessageFixup) { if (needsCommitMessageFixup) {
@ -190,7 +190,7 @@ export class GithubApiMergeStrategy extends MergeStrategy {
/** Gets all commit messages of commits in the pull request. */ /** Gets all commit messages of commits in the pull request. */
private async _getPullRequestCommitMessages({prNumber}: PullRequest) { private async _getPullRequestCommitMessages({prNumber}: PullRequest) {
const request = this.git.api.pulls.listCommits.endpoint.merge( 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); const allCommits: PullsListCommitsResponse = await this.git.api.paginate(request);
return allCommits.map(({commit}) => commit.message); return allCommits.map(({commit}) => commit.message);
} }

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {MergeConfig} from './config'; import {MergeConfigWithRemote} from './config';
import {PullRequestFailure} from './failures'; import {PullRequestFailure} from './failures';
import {GitClient, GitCommandError} from './git'; import {GitClient, GitCommandError} from './git';
import {isPullRequest, loadAndValidatePullRequest,} from './pull-request'; import {isPullRequest, loadAndValidatePullRequest,} from './pull-request';
@ -39,7 +39,8 @@ export class PullRequestMergeTask {
git = new GitClient(this.projectRoot, this._githubToken, this.config); git = new GitClient(this.projectRoot, this._githubToken, this.config);
constructor( 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. * Merges the given pull request and pushes it upstream.

View File

@ -7,6 +7,7 @@ ts_library(
visibility = ["//dev-infra:__subpackages__"], visibility = ["//dev-infra:__subpackages__"],
deps = [ deps = [
"@npm//@octokit/graphql", "@npm//@octokit/graphql",
"@npm//@types/inquirer",
"@npm//@types/node", "@npm//@types/node",
"@npm//@types/shelljs", "@npm//@types/shelljs",
"@npm//shelljs", "@npm//shelljs",

View File

@ -9,13 +9,20 @@
import {join} from 'path'; import {join} from 'path';
import {exec} from 'shelljs'; 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. */ /** The common configuration for ng-dev. */
type CommonConfig = { type CommonConfig = {
/* Github repository configuration used for API Requests, determining upstream remote, etc. */ github: GithubConfig
github: {
owner: string,
name: string,
}
}; };
/** /**