diff --git a/.github/workflows/sync-branch.yml b/.github/workflows/sync-branch.yml index 09ed31afa3b7..2a594687809b 100644 --- a/.github/workflows/sync-branch.yml +++ b/.github/workflows/sync-branch.yml @@ -35,4 +35,4 @@ jobs: echo "$(pwd)" >> $GITHUB_PATH - name: Run merge command - run: flub merge branches -a ${{ secrets.BOT_MAIN_NEXT_WORKFLOW_PAT }} -s $SOURCE_BRANCH -t $TARGET_BRANCH -b 10 + run: flub merge branches -a ${{ secrets.GITHUB_TOKEN }} -p ${{ secrets.BOT_MAIN_NEXT_WORKFLOW_PAT }} -s $SOURCE_BRANCH -t $TARGET_BRANCH -b 10 --reviewers sonalideshpandemsft diff --git a/build-tools/packages/build-cli/docs/merge.md b/build-tools/packages/build-cli/docs/merge.md index 16820416d18c..afd52edcc5bd 100644 --- a/build-tools/packages/build-cli/docs/merge.md +++ b/build-tools/packages/build-cli/docs/merge.md @@ -12,15 +12,18 @@ Sync branches depending on the batch size passed ``` USAGE - $ flub merge branches -a -s -t -b [-v | --quiet] [-r ] + $ flub merge branches -a -p -s -t -b --reviewers [-v | --quiet] + [-r ] FLAGS -a, --auth= (required) GitHub authentication token. For security reasons, this value should be passed using the GITHUB_TOKEN environment variable. -b, --batchSize= (required) Maximum number of commits to include in the pull request + -p, --pat= (required) GitHub Personal Access Token -r, --remote= [default: origin] -s, --source= (required) Source branch name -t, --target= (required) Target branch name + --reviewers=... (required) Add reviewers to PR LOGGING FLAGS -v, --verbose Enable verbose logging. diff --git a/build-tools/packages/build-cli/src/commands/merge/branches.ts b/build-tools/packages/build-cli/src/commands/merge/branches.ts index 307fdccf703b..5a159d69c8ce 100644 --- a/build-tools/packages/build-cli/src/commands/merge/branches.ts +++ b/build-tools/packages/build-cli/src/commands/merge/branches.ts @@ -7,13 +7,7 @@ import { Flags } from "@oclif/core"; import chalk from "chalk"; import { BaseCommand } from "../../base"; -import { - Repository, - createPullRequest, - getUserAccess, - pullRequestExists, - pullRequestInfo, -} from "../../lib"; +import { Repository, createPullRequest, pullRequestExists, pullRequestInfo } from "../../lib"; /** * This command class is used to merge two branches based on the batch size provided. @@ -31,6 +25,11 @@ export default class MergeBranch extends BaseCommand { required: true, env: "GITHUB_TOKEN", }), + pat: Flags.string({ + description: "GitHub Personal Access Token", + char: "p", + required: true, + }), source: Flags.string({ description: "Source branch name", char: "s", @@ -51,6 +50,11 @@ export default class MergeBranch extends BaseCommand { char: "r", default: "origin", }), + reviewers: Flags.string({ + description: "Add reviewers to PR", + required: true, + multiple: true, + }), ...BaseCommand.flags, }; @@ -87,6 +91,7 @@ export default class MergeBranch extends BaseCommand { this.initialBranch = (await this.gitRepo.gitClient.status()).current ?? "main"; this.remote = flags.remote; + const prExists: boolean = await pullRequestExists( flags.auth, prTitle, @@ -112,6 +117,8 @@ export default class MergeBranch extends BaseCommand { `refs/remotes/${this.remote}/${flags.source}`, ); + this.log(`Unmerged commit list: ${unmergedCommitList}`); + if (unmergedCommitList.length === 0) { this.log( chalk.green( @@ -132,7 +139,6 @@ export default class MergeBranch extends BaseCommand { await this.gitRepo.gitClient .checkoutBranch(tempBranchToCheckConflicts, flags.target) - .push(this.remote, tempBranchToCheckConflicts) .branch(["--set-upstream-to", `${this.remote}/${tempBranchToCheckConflicts}`]); const [commitListHasConflicts, conflictingCommitIndex] = await hasConflicts( @@ -212,7 +218,9 @@ export default class MergeBranch extends BaseCommand { git push`; if (prWillConflict === false) { - await this.gitRepo.gitClient.merge([flags.target, "-m", prTitle]); + await this.gitRepo.gitClient + .merge([flags.target, "-m", prTitle]) + .push(this.remote, branchName); /** * The below description is intended for PRs which may have CI failures with next. @@ -236,30 +244,31 @@ export default class MergeBranch extends BaseCommand { /** * fetch name of owner associated to the pull request */ - const pr = await pullRequestInfo(flags.auth, owner, repo, prHeadCommit, this.logger); - console.debug(pr); - const author = pr.data?.[0]?.assignee?.login; + const pr = await pullRequestInfo(flags.pat, owner, repo, prHeadCommit, this.logger); + if (pr === undefined) { + this.warning(`Unable to add assignee`); + } + const username = pr.data.author.login; this.info( - `Fetching pull request info for commit id ${prHeadCommit} and assignee ${author}`, + `Fetching pull request info for commit id ${prHeadCommit} and assignee ${username}`, ); - const user = await getUserAccess(flags.auth, owner, repo, this.logger); - this.verbose(`List users with push access to main branch: ${JSON.stringify(user.data)}`); const prObject = { - token: flags.auth, + token: flags.pat, owner, repo, source: branchName, target: flags.target, - assignee: author, + assignee: username, title: prTitle, description, + reviewers: flags.reviewers, }; + this.log(`Initiate PR creation: ${prObject}}`); + const prNumber = await createPullRequest(prObject, this.logger); - this.log( - `Opened pull request ${prNumber} for commit id ${prHeadCommit}. Please resolve the merge conflicts.`, - ); + this.log(`Opened pull request ${prNumber} for commit id ${prHeadCommit}`); } /** diff --git a/build-tools/packages/build-cli/src/lib/git.ts b/build-tools/packages/build-cli/src/lib/git.ts index 13ff4db4989c..2206049941d3 100644 --- a/build-tools/packages/build-cli/src/lib/git.ts +++ b/build-tools/packages/build-cli/src/lib/git.ts @@ -187,16 +187,24 @@ export class Repository { * @returns An array of all commits between the base and head commits. */ public async revList(baseCommit: string, headCommit: string = "HEAD"): Promise { - const result = await this.git.raw("rev-list", `${baseCommit}..${headCommit}`); + const result = await this.git.raw("rev-list", `${baseCommit}..${headCommit}`, "--reverse"); return result .split(/\r?\n/) .filter((value) => value !== null && value !== undefined && value !== ""); } public async canMergeWithoutConflicts(commit: string): Promise { - const mergeResult = await this.git.merge([commit, "--no-commit"]); - await this.git.merge(["--abort"]); - const canMerge = mergeResult.result === "success"; - return canMerge; + let mergeResult; + try { + console.log(`Checking merge conflicts for: ${commit}`); + mergeResult = await this.git.merge([commit, "--no-commit", "--no-ff"]); + console.log(`Git merge result: ${mergeResult}`); + await this.git.merge(["--abort"]); + } catch { + await this.git.merge(["--abort"]); + return false; + } + + return mergeResult.result === "success"; } } diff --git a/build-tools/packages/build-cli/src/lib/github.ts b/build-tools/packages/build-cli/src/lib/github.ts index 577dfdba78a6..e85132f8846d 100644 --- a/build-tools/packages/build-cli/src/lib/github.ts +++ b/build-tools/packages/build-cli/src/lib/github.ts @@ -6,12 +6,11 @@ import { Octokit } from "@octokit/core"; import { CommandLogger } from "../logging"; const PULL_REQUEST_EXISTS = "GET /repos/{owner}/{repo}/pulls"; -const PULL_REQUEST_INFO = "GET /repos/{owner}/{repo}/commits/{commit_sha}/pulls"; +const PULL_REQUEST_INFO = "GET /repos/{owner}/{repo}/commits/{ref}"; const PULL_REQUEST = "POST /repos/{owner}/{repo}/pulls"; const ASSIGNEE = "POST /repos/{owner}/{repo}/issues/{issue_number}/assignees"; -const REVIEWER = "POST /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers"; const LABEL = "POST /repos/{owner}/{repo}/issues/{issue_number}/labels"; -const GET_USER = "GET /repos/{owner}/{repo}/branches/{branch}/protection/restrictions/users"; +const REVIEWER = "POST /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers"; /** * @@ -45,39 +44,17 @@ export async function pullRequestInfo( log: CommandLogger, ): Promise { const octokit = new Octokit({ auth: token }); + const prInfo = await octokit.request(PULL_REQUEST_INFO, { owner, repo, - commit_sha, + ref: commit_sha, }); - log.verbose(`Get pull request info for ${commit_sha}: ${JSON.stringify(prInfo)}`); + log.verbose(`Get info from ref: ${JSON.stringify(prInfo)}`); return prInfo; } -/** - * - * @param token - GitHub authentication token - * @returns Lists the user who have push access to this branch - */ -export async function getUserAccess( - token: string, - owner: string, - repo: string, - log: CommandLogger, -): Promise { - const octokit = new Octokit({ auth: token }); - - const user = await octokit.request(GET_USER, { - owner, - repo, - branch: "main", - }); - - log.verbose(`Get list of users with push access ${JSON.stringify(user)}`); - return user; -} - /** * * @param auth - GitHub authentication token @@ -96,6 +73,7 @@ export async function createPullRequest( assignee: string; title: string; description: string; + reviewers: string[]; }, log: CommandLogger, ): Promise { @@ -118,12 +96,13 @@ export async function createPullRequest( assignees: [pr.assignee], }); - log.verbose(`Adding reviewer to pull request ${newPr.data.number}`); + log.log(`Adding reviewer to pull request ${newPr.data.number}`); await octokit.request(REVIEWER, { owner: pr.owner, repo: pr.repo, pull_number: newPr.data.number, - reviewer: [], + reviewers: pr.reviewers, + team_reviewers: [""], }); log.verbose(`Adding label to pull request ${newPr.data.number}`); diff --git a/build-tools/packages/build-cli/src/lib/index.ts b/build-tools/packages/build-cli/src/lib/index.ts index 6adfd1d91848..17c853cde1a4 100644 --- a/build-tools/packages/build-cli/src/lib/index.ts +++ b/build-tools/packages/build-cli/src/lib/index.ts @@ -31,7 +31,7 @@ export { } from "./package"; export { difference } from "./sets"; export { getIndent, indentString } from "./text"; -export { createPullRequest, getUserAccess, pullRequestExists, pullRequestInfo } from "./github"; +export { createPullRequest, pullRequestExists, pullRequestInfo } from "./github"; export { getRanges, PackageVersionList,