From 63ba74fe4e7f5ea9cb07b866647488cb76adf066 Mon Sep 17 00:00:00 2001 From: Joey Perrott Date: Fri, 14 Aug 2020 16:49:07 -0700 Subject: [PATCH] feat(dev-infra): tooling to check out pending PR (#38474) Creates a tool within ng-dev to checkout a pending PR from the upstream repository. This automates an action that many developers on the Angular team need to do periodically in the process of testing and reviewing incoming PRs. Example usage: ng-dev pr checkout PR Close #38474 --- dev-infra/pr/BUILD.bazel | 1 + dev-infra/pr/checkout/BUILD.bazel | 13 ++ dev-infra/pr/checkout/cli.ts | 50 +++++++ dev-infra/pr/cli.ts | 4 +- dev-infra/pr/common/BUILD.bazel | 12 ++ dev-infra/pr/common/checkout-pr.ts | 135 +++++++++++++++++++ dev-infra/pr/discover-new-conflicts/index.ts | 2 +- dev-infra/pr/rebase/index.ts | 2 +- dev-infra/utils/git/github.ts | 8 +- dev-infra/utils/git/index.ts | 19 +++ dev-infra/utils/github.ts | 71 +++------- 11 files changed, 261 insertions(+), 56 deletions(-) create mode 100644 dev-infra/pr/checkout/BUILD.bazel create mode 100644 dev-infra/pr/checkout/cli.ts create mode 100644 dev-infra/pr/common/BUILD.bazel create mode 100644 dev-infra/pr/common/checkout-pr.ts diff --git a/dev-infra/pr/BUILD.bazel b/dev-infra/pr/BUILD.bazel index 023bbd69ac..bd2aaaa512 100644 --- a/dev-infra/pr/BUILD.bazel +++ b/dev-infra/pr/BUILD.bazel @@ -6,6 +6,7 @@ ts_library( module_name = "@angular/dev-infra-private/pr", visibility = ["//dev-infra:__subpackages__"], deps = [ + "//dev-infra/pr/checkout", "//dev-infra/pr/discover-new-conflicts", "//dev-infra/pr/merge", "//dev-infra/pr/rebase", diff --git a/dev-infra/pr/checkout/BUILD.bazel b/dev-infra/pr/checkout/BUILD.bazel new file mode 100644 index 0000000000..b6ce76177d --- /dev/null +++ b/dev-infra/pr/checkout/BUILD.bazel @@ -0,0 +1,13 @@ +load("@npm_bazel_typescript//:index.bzl", "ts_library") + +ts_library( + name = "checkout", + srcs = glob(["*.ts"]), + module_name = "@angular/dev-infra-private/pr/checkout", + visibility = ["//dev-infra:__subpackages__"], + deps = [ + "//dev-infra/pr/common", + "//dev-infra/utils", + "@npm//@types/yargs", + ], +) diff --git a/dev-infra/pr/checkout/cli.ts b/dev-infra/pr/checkout/cli.ts new file mode 100644 index 0000000000..de9d8b59f8 --- /dev/null +++ b/dev-infra/pr/checkout/cli.ts @@ -0,0 +1,50 @@ +/** + * @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 {Arguments, Argv, CommandModule} from 'yargs'; + +import {error} from '../../utils/console'; +import {checkOutPullRequestLocally} from '../common/checkout-pr'; + +export interface CheckoutOptions { + prNumber: number; + 'github-token'?: string; +} + +/** URL to the Github page where personal access tokens can be generated. */ +export const GITHUB_TOKEN_GENERATE_URL = `https://github.com/settings/tokens`; + +/** Builds the checkout pull request command. */ +function builder(yargs: Argv) { + return yargs.positional('prNumber', {type: 'number', demandOption: true}).option('github-token', { + type: 'string', + description: 'Github token. If not set, token is retrieved from the environment variables.' + }); +} + +/** Handles the checkout pull request command. */ +async function handler({prNumber, 'github-token': token}: Arguments) { + const githubToken = token || process.env.GITHUB_TOKEN || process.env.TOKEN; + if (!githubToken) { + error('No Github token set. Please set the `GITHUB_TOKEN` environment variable.'); + error('Alternatively, pass the `--github-token` command line flag.'); + error(`You can generate a token here: ${GITHUB_TOKEN_GENERATE_URL}`); + process.exitCode = 1; + return; + } + const prCheckoutOptions = {allowIfMaintainerCannotModify: true, branchName: `pr-${prNumber}`}; + await checkOutPullRequestLocally(prNumber, githubToken, prCheckoutOptions); +} + +/** yargs command module for checking out a PR */ +export const CheckoutCommandModule: CommandModule<{}, CheckoutOptions> = { + handler, + builder, + command: 'checkout ', + describe: 'Checkout a PR from the upstream repo', +}; diff --git a/dev-infra/pr/cli.ts b/dev-infra/pr/cli.ts index 0f4d312d44..fb080ed5c2 100644 --- a/dev-infra/pr/cli.ts +++ b/dev-infra/pr/cli.ts @@ -8,6 +8,7 @@ import * as yargs from 'yargs'; +import {CheckoutCommandModule} from './checkout/cli'; import {buildDiscoverNewConflictsCommand, handleDiscoverNewConflictsCommand} from './discover-new-conflicts/cli'; import {buildMergeCommand, handleMergeCommand} from './merge/cli'; import {buildRebaseCommand, handleRebaseCommand} from './rebase/cli'; @@ -24,7 +25,8 @@ export function buildPrParser(localYargs: yargs.Argv) { buildDiscoverNewConflictsCommand, handleDiscoverNewConflictsCommand) .command( 'rebase ', 'Rebase a pending PR and push the rebased commits back to Github', - buildRebaseCommand, handleRebaseCommand); + buildRebaseCommand, handleRebaseCommand) + .command(CheckoutCommandModule); } if (require.main === module) { diff --git a/dev-infra/pr/common/BUILD.bazel b/dev-infra/pr/common/BUILD.bazel new file mode 100644 index 0000000000..a25c3b90ea --- /dev/null +++ b/dev-infra/pr/common/BUILD.bazel @@ -0,0 +1,12 @@ +load("@npm_bazel_typescript//:index.bzl", "ts_library") + +ts_library( + name = "common", + srcs = glob(["*.ts"]), + visibility = ["//dev-infra:__subpackages__"], + deps = [ + "//dev-infra/utils", + "@npm//@types/node", + "@npm//typed-graphqlify", + ], +) diff --git a/dev-infra/pr/common/checkout-pr.ts b/dev-infra/pr/common/checkout-pr.ts new file mode 100644 index 0000000000..5fd8a4a36f --- /dev/null +++ b/dev-infra/pr/common/checkout-pr.ts @@ -0,0 +1,135 @@ +/** + * @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 {types as graphQLTypes} from 'typed-graphqlify'; +import {URL} from 'url'; + +import {info} from '../../utils/console'; +import {GitClient} from '../../utils/git'; +import {getPr} from '../../utils/github'; + +/* GraphQL schema for the response body for a pending PR. */ +const PR_SCHEMA = { + state: graphQLTypes.string, + maintainerCanModify: graphQLTypes.boolean, + viewerDidAuthor: graphQLTypes.boolean, + headRefOid: graphQLTypes.string, + headRef: { + name: graphQLTypes.string, + repository: { + url: graphQLTypes.string, + nameWithOwner: graphQLTypes.string, + }, + }, + baseRef: { + name: graphQLTypes.string, + repository: { + url: graphQLTypes.string, + nameWithOwner: graphQLTypes.string, + }, + }, +}; + + +export class UnexpectedLocalChangesError extends Error { + constructor(m: string) { + super(m); + Object.setPrototypeOf(this, UnexpectedLocalChangesError.prototype); + } +} + +export class MaintainerModifyAccessError extends Error { + constructor(m: string) { + super(m); + Object.setPrototypeOf(this, MaintainerModifyAccessError.prototype); + } +} + +/** Options for checking out a PR */ +export interface PullRequestCheckoutOptions { + /** Whether the PR should be checked out if the maintainer cannot modify. */ + allowIfMaintainerCannotModify?: boolean; +} + +/** + * Rebase the provided PR onto its merge target branch, and push up the resulting + * commit to the PRs repository. + */ +export async function checkOutPullRequestLocally( + prNumber: number, githubToken: string, opts: PullRequestCheckoutOptions = {}) { + /** Authenticated Git client for git and Github interactions. */ + const git = new GitClient(githubToken); + + // In order to preserve local changes, checkouts cannot occur if local changes are present in the + // git environment. Checked before retrieving the PR to fail fast. + if (git.hasLocalChanges()) { + throw new UnexpectedLocalChangesError('Unable to checkout PR due to uncommitted changes.'); + } + + /** + * The branch or revision originally checked out before this method performed + * any Git operations that may change the working branch. + */ + const previousBranchOrRevision = git.getCurrentBranchOrRevision(); + /* The PR information from Github. */ + const pr = await getPr(PR_SCHEMA, prNumber, git); + /** The branch name of the PR from the repository the PR came from. */ + const headRefName = pr.headRef.name; + /** The full ref for the repository and branch the PR came from. */ + const fullHeadRef = `${pr.headRef.repository.nameWithOwner}:${headRefName}`; + /** The full URL path of the repository the PR came from with github token as authentication. */ + const headRefUrl = addAuthenticationToUrl(pr.headRef.repository.url, githubToken); + // Note: Since we use a detached head for rebasing the PR and therefore do not have + // remote-tracking branches configured, we need to set our expected ref and SHA. This + // allows us to use `--force-with-lease` for the detached head while ensuring that we + // never accidentally override upstream changes that have been pushed in the meanwhile. + // See: + // https://git-scm.com/docs/git-push#Documentation/git-push.txt---force-with-leaseltrefnamegtltexpectgt + /** Flag for a force push with leage back to upstream. */ + const forceWithLeaseFlag = `--force-with-lease=${headRefName}:${pr.headRefOid}`; + + // If the PR does not allow maintainers to modify it, exit as the rebased PR cannot + // be pushed up. + if (!pr.maintainerCanModify && !pr.viewerDidAuthor && !opts.allowIfMaintainerCannotModify) { + throw new MaintainerModifyAccessError('PR is not set to allow maintainers to modify the PR'); + } + + try { + // Fetch the branch at the commit of the PR, and check it out in a detached state. + info(`Checking out PR #${prNumber} from ${fullHeadRef}`); + git.run(['fetch', headRefUrl, headRefName]); + git.run(['checkout', '--detach', 'FETCH_HEAD']); + } catch (e) { + git.checkout(previousBranchOrRevision, true); + throw e; + } + + return { + /** + * Pushes the current local branch to the PR on the upstream repository. + * + * @returns true If the command did not fail causing a GitCommandError to be thrown. + * @throws GitCommandError Thrown when the push back to upstream fails. + */ + pushToUpstream: (): true => { + git.run(['push', headRefUrl, `HEAD:${headRefName}`, forceWithLeaseFlag]); + return true; + }, + /** Restores the state of the local repository to before the PR checkout occured. */ + resetGitState: (): boolean => { + return git.checkout(previousBranchOrRevision, true); + } + }; +} + +/** Adds the provided token as username to the provided url. */ +function addAuthenticationToUrl(urlString: string, token: string) { + const url = new URL(urlString); + url.username = token; + return url.toString(); +} diff --git a/dev-infra/pr/discover-new-conflicts/index.ts b/dev-infra/pr/discover-new-conflicts/index.ts index 790d809438..c59f1a0698 100644 --- a/dev-infra/pr/discover-new-conflicts/index.ts +++ b/dev-infra/pr/discover-new-conflicts/index.ts @@ -72,7 +72,7 @@ export async function discoverNewConflictsForPr( info(`Requesting pending PRs from Github`); /** List of PRs from github currently known as mergable. */ - const allPendingPRs = (await getPendingPrs(PR_SCHEMA, config.github)).map(processPr); + const allPendingPRs = (await getPendingPrs(PR_SCHEMA, git)).map(processPr); /** The PR which is being checked against. */ const requestedPr = allPendingPRs.find(pr => pr.number === newPrNumber); if (requestedPr === undefined) { diff --git a/dev-infra/pr/rebase/index.ts b/dev-infra/pr/rebase/index.ts index 8c0af93324..149de6513a 100644 --- a/dev-infra/pr/rebase/index.ts +++ b/dev-infra/pr/rebase/index.ts @@ -55,7 +55,7 @@ export async function rebasePr( */ const previousBranchOrRevision = git.getCurrentBranchOrRevision(); /* Get the PR information from Github. */ - const pr = await getPr(PR_SCHEMA, prNumber, config.github); + const pr = await getPr(PR_SCHEMA, prNumber, git); const headRefName = pr.headRef.name; const baseRefName = pr.baseRef.name; diff --git a/dev-infra/utils/git/github.ts b/dev-infra/utils/git/github.ts index 85d0614358..cea6979cd7 100644 --- a/dev-infra/utils/git/github.ts +++ b/dev-infra/utils/git/github.ts @@ -26,7 +26,7 @@ export class GithubApiRequestError extends Error { **/ export class GithubClient extends Octokit { /** The Github GraphQL (v4) API. */ - graqhql: GithubGraphqlClient; + graphql: GithubGraphqlClient; /** The current user based on checking against the Github API. */ private _currentUser: string|null = null; @@ -42,7 +42,7 @@ export class GithubClient extends Octokit { }); // Create authenticated graphql client. - this.graqhql = new GithubGraphqlClient(token); + this.graphql = new GithubGraphqlClient(token); } /** Retrieve the login of the current user from Github. */ @@ -51,7 +51,7 @@ export class GithubClient extends Octokit { if (this._currentUser !== null) { return this._currentUser; } - const result = await this.graqhql.query({ + const result = await this.graphql.query({ viewer: { login: types.string, } @@ -80,7 +80,7 @@ class GithubGraphqlClient { // Set the default headers to include authorization with the provided token for all // graphQL calls. if (token) { - this.graqhql.defaults({headers: {authorization: `token ${token}`}}); + this.graqhql = this.graqhql.defaults({headers: {authorization: `token ${token}`}}); } } diff --git a/dev-infra/utils/git/index.ts b/dev-infra/utils/git/index.ts index 030a25c4ca..93364731ad 100644 --- a/dev-infra/utils/git/index.ts +++ b/dev-infra/utils/git/index.ts @@ -150,6 +150,25 @@ export class GitClient { return value.replace(this._githubTokenRegex, ''); } + /** + * Checks out a requested branch or revision, optionally cleaning the state of the repository + * before attempting the checking. Returns a boolean indicating whether the branch or revision + * was cleanly checked out. + */ + checkout(branchOrRevision: string, cleanState: boolean): boolean { + if (cleanState) { + // Abort any outstanding ams. + this.runGraceful(['am', '--abort'], {stdio: 'ignore'}); + // Abort any outstanding cherry-picks. + this.runGraceful(['cherry-pick', '--abort'], {stdio: 'ignore'}); + // Abort any outstanding rebases. + this.runGraceful(['rebase', '--abort'], {stdio: 'ignore'}); + // Clear any changes in the current repo. + this.runGraceful(['reset', '--hard'], {stdio: 'ignore'}); + } + return this.runGraceful(['checkout', branchOrRevision], {stdio: 'ignore'}).status === 0; + } + /** * Assert the GitClient instance is using a token with permissions for the all of the * provided OAuth scopes. diff --git a/dev-infra/utils/github.ts b/dev-infra/utils/github.ts index 04cd93ed97..7e187a24a4 100644 --- a/dev-infra/utils/github.ts +++ b/dev-infra/utils/github.ts @@ -6,29 +6,15 @@ * found in the LICENSE file at https://angular.io/license */ -import {graphql as unauthenticatedGraphql} from '@octokit/graphql'; +import {params, types} from 'typed-graphqlify'; -import {params, query as graphqlQuery, types} from 'typed-graphqlify'; -import {NgDevConfig} from './config'; - -/** The configuration required for github interactions. */ -type GithubConfig = NgDevConfig['github']; - -/** - * Authenticated instance of Github GraphQl API service, relies on a - * personal access token being available in the TOKEN environment variable. - */ -const graphql = unauthenticatedGraphql.defaults({ - headers: { - // TODO(josephperrott): Remove reference to TOKEN environment variable as part of larger - // effort to migrate to expecting tokens via GITHUB_ACCESS_TOKEN environment variables. - authorization: `token ${process.env.TOKEN || process.env.GITHUB_ACCESS_TOKEN}`, - } -}); +import {GitClient} from './git'; /** Get a PR from github */ -export async function getPr( - prSchema: PrSchema, prNumber: number, {owner, name}: GithubConfig) { +export async function getPr(prSchema: PrSchema, prNumber: number, git: GitClient) { + /** The owner and name of the repository */ + const {owner, name} = git.remoteConfig; + /** The GraphQL query object to get a the PR */ const PR_QUERY = params( { $number: 'Int!', // The PR number @@ -41,14 +27,15 @@ export async function getPr( }) }); - const result = - await graphql(graphqlQuery(PR_QUERY), {number: prNumber, owner, name}) as typeof PR_QUERY; + const result = (await git.github.graphql.query(PR_QUERY, {number: prNumber, owner, name})); return result.repository.pullRequest; } /** Get all pending PRs from github */ -export async function getPendingPrs(prSchema: PrSchema, {owner, name}: GithubConfig) { - // The GraphQL query object to get a page of pending PRs +export async function getPendingPrs(prSchema: PrSchema, git: GitClient) { + /** The owner and name of the repository */ + const {owner, name} = git.remoteConfig; + /** The GraphQL query object to get a page of pending PRs */ const PRS_QUERY = params( { $first: 'Int', // How many entries to get with each request @@ -73,36 +60,22 @@ export async function getPendingPrs(prSchema: PrSchema, {owner, name}: }), }) }); - const query = graphqlQuery('members', PRS_QUERY); - - /** - * Gets the query and queryParams for a specific page of entries. - */ - const queryBuilder = (count: number, cursor?: string) => { - return { - query, - params: { - after: cursor || null, - first: count, - owner, - name, - }, - }; - }; - - // The current cursor + /** The current cursor */ let cursor: string|undefined; - // If an additional page of members is expected + /** If an additional page of members is expected */ let hasNextPage = true; - // Array of pending PRs + /** Array of pending PRs */ const prs: Array = []; - // For each page of the response, get the page and add it to the - // list of PRs + // For each page of the response, get the page and add it to the list of PRs while (hasNextPage) { - const {query, params} = queryBuilder(100, cursor); - const results = await graphql(query, params) as typeof PRS_QUERY; - + const params = { + after: cursor || null, + first: 100, + owner, + name, + }; + const results = await git.github.graphql.query(PRS_QUERY, params) as typeof PRS_QUERY; prs.push(...results.repository.pullRequests.nodes); hasNextPage = results.repository.pullRequests.pageInfo.hasNextPage; cursor = results.repository.pullRequests.pageInfo.endCursor;