Skip to content

refactor(ng-dev): separate out logic for takeover, target and regular checkouts #2560

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

refactor(ng-dev): separate out logic for takeover, target and regular checkouts #2560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 41 additions & 146 deletions ng-dev/pr/checkout/checkout.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,184 +1,79 @@
import {GithubConfig, NgDevConfig} from '../../utils/config.js';
import {dirname, join} from 'path';
/**
* @license
* Copyright Google LLC
*
* 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 {AuthenticatedGitClient} from '../../utils/git/authenticated-git-client.js';
import {Log, bold, green} from '../../utils/logging.js';
import {green, Log, red} from '../../utils/logging.js';
import {checkOutPullRequestLocally} from '../common/checkout-pr.js';
import {fileURLToPath} from 'url';
import {ActiveReleaseTrains} from '../../release/versioning/active-release-trains.js';
import {getNextBranchName} from '../../release/versioning/version-branches.js';
import {addTokenToGitHttpsUrl} from '../../utils/git/github-urls.js';
import {Prompt} from '../../utils/prompt.js';

/** List of accounts that are supported for takeover. */
const takeoverAccounts = ['angular-robot'];
import {checkoutToTargetBranch} from './target.js';
import {checkoutAsPrTakeover} from './takeover.js';

export interface CheckoutPullRequestParams {
pr: number;
takeover?: boolean;
target?: string;
}

export async function checkoutPullRequest(
params: CheckoutPullRequestParams,
config: NgDevConfig<{github: GithubConfig}>,
): Promise<void> {
export async function checkoutPullRequest(params: CheckoutPullRequestParams): Promise<void> {
const {pr, takeover, target} = params;
/** An authenticated git client. */
const git = await AuthenticatedGitClient.get();

if (takeover && target) {
Log.error(` ✘ You cannot specify both takeover and target branch at the same time`);
Log.error(` ${red('✘')} The --takeover and --target flags cannot be provided simultaneously`);
return;
}

// Make sure the local repository is clean.
if (git.hasUncommittedChanges()) {
Log.error(
` ✘ Local working repository not clean. Please make sure there are no uncommitted changes`,
` ${red('✘')} Local working repository not clean. Please make sure there are no uncommitted changes`,
);
return;
}

const {resetGitState, pullRequest, pushToUpstreamCommand} = await checkOutPullRequestLocally(pr, {
const localCheckoutResult = await checkOutPullRequestLocally(pr, {
allowIfMaintainerCannotModify: true,
});

if (!target) {
const branchName = `pr-takeover-${pr}`;
// if maintainer can modify is false or if takeover is provided do takeover

if (pullRequest.maintainerCanModify === false || takeover) {
if (takeover !== true) {
Log.info('The author of this pull request does not allow maintainers to modify the pull');
Log.info(
'request. Since you will not be able to push changes to the original pull request',
);
Log.info('you will instead need to perform a "takeover." In a takeover the original pull');
Log.info('request will be checked out, the commits are modified to close the original on');
Log.info('merge of the newly created branch.\n');

if (
!(await Prompt.confirm({
message: `Would you like to create a takeover pull request?`,
default: true,
}))
) {
Log.info('Aborting takeover..');
await resetGitState();
return;
}
}

if (git.runGraceful(['rev-parse', '-q', '--verify', branchName]).status === 0) {
Log.error(` ✘ Expected branch name \`${branchName}\` already exists locally`);
return;
}

// Confirm that the takeover request is being done on a valid pull request.
if (!takeoverAccounts.includes(pullRequest.author.login)) {
Log.warn(
` ⚠ ${bold(pullRequest.author.login)} is not an account fully supported for takeover.`,
);
Log.warn(` Supported accounts: ${bold(takeoverAccounts.join(', '))}`);
if (
await Prompt.confirm({
message: `Continue with pull request takeover anyway?`,
default: true,
})
) {
Log.debug('Continuing per user confirmation in prompt');
} else {
Log.info('Aborting takeover..');
await resetGitState();
return;
}
}

Log.info(`Setting local branch name based on the pull request`);
git.run(['checkout', '-q', '-b', branchName]);

Log.info('Updating commit messages to close previous pull request');
git.run([
'filter-branch',
'-f',
'--msg-filter',
`${getCommitMessageFilterScriptPath()} ${pr}`,
`${pullRequest.baseRefOid}..HEAD`,
]);
if (takeover) {
return await checkoutAsPrTakeover(pr, localCheckoutResult);
}

Log.info(` ${green('✔')} Checked out pull request #${pr} into branch: ${branchName}`);
return;
}
if (target) {
return await checkoutToTargetBranch(pr, target, localCheckoutResult);
}

Log.info(`Checked out the remote branch for pull request #${pr}\n`);
Log.info('To push the checked out branch back to its PR, run the following command:');
Log.info(` $ ${pushToUpstreamCommand}`);
} else {
const branchName = `pr-${target.toLowerCase().replaceAll(/[\W_]/gm, '-')}-${pr}`;
const {owner, name: repo} = config.github;
const activeReleaseTrains = await ActiveReleaseTrains.fetch({
name: repo,
owner: owner,
nextBranchName: getNextBranchName(config.github),
api: git.github,
});
/**
* Whether the pull request is configured to allow for the maintainers to modify the pull request.
*/
const maintainerCanModify = localCheckoutResult.pullRequest.maintainerCanModify;

let targetBranch = target;
let targetName = target;
if (!maintainerCanModify) {
Log.info('The author of this pull request does not allow maintainers to modify the pull');
Log.info('request. Since you will not be able to push changes to the original pull request');
Log.info('you will instead need to perform a "takeover." In a takeover, the original pull');
Log.info('request will be checked out, the commits are modified to close the original on');
Log.info('merge of the newly created branch.');

if (
target === 'patch' ||
target === 'latest' ||
activeReleaseTrains.latest.branchName === target
await Prompt.confirm({
message: `Would you like to create a takeover pull request?`,
default: true,
})
) {
targetName = 'patch';
targetBranch = activeReleaseTrains.latest.branchName;
} else if (
target === 'main' ||
target === 'next' ||
target === 'minor' ||
activeReleaseTrains.next.branchName === target
) {
targetName = 'main';
targetBranch = activeReleaseTrains.next.branchName;
} else if (
activeReleaseTrains.releaseCandidate &&
(target === 'rc' || activeReleaseTrains.releaseCandidate.branchName === target)
) {
targetName = 'rc';
targetBranch = activeReleaseTrains.releaseCandidate.branchName;
}
Log.info(`Targeting '${targetBranch}' branch\n`);

const baseRefUrl = addTokenToGitHttpsUrl(pullRequest.baseRef.repository.url, git.githubToken);

git.run(['checkout', '-q', targetBranch]);
git.run(['fetch', '-q', baseRefUrl, targetBranch, '--deepen=500']);
git.run(['checkout', '-b', branchName]);

Log.info(`Running cherry-pick\n`);

try {
const revisionRange = `${pullRequest.baseRefOid}..${pullRequest.headRefOid}`;
git.run(['cherry-pick', revisionRange]);
Log.info(`Cherry-pick is complete. You can now push to create a new pull request.`);
} catch {
Log.info(
`Cherry-pick resulted in conflicts. Please resolve them manually and push to create your patch PR`,
);
return;
return await checkoutAsPrTakeover(pr, localCheckoutResult);
}

return;
}
}

/** Gets the absolute file path to the commit-message filter script. */
function getCommitMessageFilterScriptPath(): string {
// This file is getting bundled and ends up in `<pkg-root>/bundles/<chunk>`. We also
// bundle the commit-message-filter script as another entry-point and can reference
// it relatively as the path is preserved inside `bundles/`.
// *Note*: Relying on package resolution is problematic within ESM and with `local-dev.sh`
const bundlesDir = dirname(fileURLToPath(import.meta.url));
return join(bundlesDir, './pr/checkout/commit-message-filter.mjs');
Log.info(` ${green('✔')} Checked out the remote branch for pull request #${pr}`);
if (maintainerCanModify) {
Log.info('To push the checked out branch back to its PR, run the following command:');
Log.info(` $ ${localCheckoutResult.pushToUpstreamCommand}`);
}
}
6 changes: 1 addition & 5 deletions ng-dev/pr/checkout/cli.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import {Argv, Arguments, CommandModule} from 'yargs';

import {assertValidGithubConfig, getConfig, GithubConfig, NgDevConfig} from '../../utils/config.js';
import {addGithubTokenOption} from '../../utils/git/github-yargs.js';
import {checkoutPullRequest, CheckoutPullRequestParams} from './checkout.js';

Expand All @@ -34,10 +33,7 @@ function builder(yargs: Argv) {

/** Handles the checkout pull request command. */
async function handler({pr, takeover, target}: Arguments<CheckoutPullRequestParams>) {
const config = await getConfig();
assertValidGithubConfig(config);

await checkoutPullRequest({pr, takeover, target}, config);
await checkoutPullRequest({pr, takeover, target});
}

/** yargs command module for checking out a PR */
Expand Down
80 changes: 80 additions & 0 deletions ng-dev/pr/checkout/takeover.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/**
* @license
* Copyright Google LLC
*
* 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 {dirname, join} from 'path';
import {AuthenticatedGitClient} from '../../utils/git/authenticated-git-client.js';
import {bold, green, Log} from '../../utils/logging.js';
import {Prompt} from '../../utils/prompt.js';
import {checkOutPullRequestLocally} from '../common/checkout-pr.js';
import {fileURLToPath} from 'url';

/** List of accounts that are supported for takeover. */
const takeoverAccounts = ['angular-robot'];

/**
* Checkout the provided pull request in preperation for a new takeover pull request to be made
*/
export async function checkoutAsPrTakeover(
prNumber: number,
{resetGitState, pullRequest}: Awaited<ReturnType<typeof checkOutPullRequestLocally>>,
) {
/** An authenticated git client. */
const git = await AuthenticatedGitClient.get();
/** The branch name to be used for the takeover attempt. */
const branchName = `pr-takeover-${prNumber}`;

if (git.runGraceful(['rev-parse', '-q', '--verify', branchName]).status === 0) {
Log.error(` ✘ Expected branch name \`${branchName}\` already exists locally`);
return;
}

// Validate that the takeover attempt is being made against a pull request created by an
// expected account.
if (!takeoverAccounts.includes(pullRequest.author.login)) {
Log.warn(
` ⚠ ${bold(pullRequest.author.login)} is not an account fully supported for takeover.`,
);
Log.warn(` Supported accounts: ${bold(takeoverAccounts.join(', '))}`);
if (
await Prompt.confirm({
message: `Continue with pull request takeover anyway?`,
default: true,
})
) {
Log.debug('Continuing per user confirmation in prompt');
} else {
Log.info('Aborting takeover..');
resetGitState();
return;
}
}

Log.info(`Setting local branch name based on the pull request`);
git.run(['checkout', '-q', '-b', branchName]);

Log.info('Updating commit messages to close previous pull request');
git.run([
'filter-branch',
'-f',
'--msg-filter',
`${getCommitMessageFilterScriptPath()} ${prNumber}`,
`${pullRequest.baseRefOid}..HEAD`,
]);

Log.info(` ${green('✔')} Checked out pull request #${prNumber} into branch: ${branchName}`);
}

/** Gets the absolute file path to the commit-message filter script. */
function getCommitMessageFilterScriptPath(): string {
// This file is getting bundled and ends up in `<pkg-root>/bundles/<chunk>`. We also
// bundle the commit-message-filter script as another entry-point and can reference
// it relatively as the path is preserved inside `bundles/`.
// *Note*: Relying on package resolution is problematic within ESM and with `local-dev.sh`
const bundlesDir = dirname(fileURLToPath(import.meta.url));
return join(bundlesDir, './pr/checkout/commit-message-filter.mjs');
}
Loading