From 53615f1a7a37f28691f822dd916887a1a84c37ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9r=C3=A8?= Date: Fri, 29 Dec 2023 13:51:56 -0800 Subject: [PATCH] Use treeless clone to fetch enough history for diff A treeless clone contains all git history (and so can always find the correct merge base), but is more efficient than a normal clone/fetch. --- README.md | 15 ++++-- action.yml | 4 +- badges/coverage.svg | 2 +- dist/index.js | 98 +++++++++++++++++++-------------------- src/initializer.ts | 109 ++++++++++++++++++++++---------------------- src/main.ts | 7 ++- 6 files changed, 122 insertions(+), 113 deletions(-) diff --git a/README.md b/README.md index 8287737..3d5a8d5 100644 --- a/README.md +++ b/README.md @@ -27,10 +27,15 @@ jobs: runs-on: ubuntu-latest steps: - # Check out a copy of this repository so that we can compute a diff to - # evaluate and load the Action's configuration. + # Check out a copy of this repository so that we can retrieve a diff to + # evaluate and load the `sizeup` configuration file. + # + # This step must fetch the pull request's base branch using the `ref` + # and `filter` arguments as shown below. - name: Checkout this repository uses: actions/checkout@v4 + with: + filter: tree:0 # Run the estimation tool - name: Run sizeup @@ -49,13 +54,13 @@ jobs: # This input is required. token: "${{ secrets.GITHUB_TOKEN }}" - # Optional arguments that will be forwarded to `git diff` when - # computing the diff to evaluate with this Action. + # Options that will be forwarded to `git diff` when computing the + # diff to evaluate with this Action. # # Defaults to "--ignore-space-change", which ignores lines of the # diff in which the only change is to the amount of whitespace on the # line. - git-diff-args: "" + git-diff-options: "" # Path to a YAML configuration file for this Action that is stored in # this repository. diff --git a/action.yml b/action.yml index d81583f..659bf8a 100644 --- a/action.yml +++ b/action.yml @@ -6,9 +6,9 @@ inputs: token: description: 'GitHub API token with read permissions for pull requests in this repository.' required: true - git-diff-args: + git-diff-options: description: | - Optional custom arguments to forward to `git diff` when retrieving the diff of the pull request that triggered this workflow. + Custom options to forward to `git diff` when retrieving the diff of the pull request that triggered this workflow. The result of that `git diff` command will be used for evaluation with sizeup. default: '--ignore-space-change' configuration-file-path: diff --git a/badges/coverage.svg b/badges/coverage.svg index f7c1988..d179b1d 100644 --- a/badges/coverage.svg +++ b/badges/coverage.svg @@ -1 +1 @@ -Coverage: 61.59%Coverage61.59% \ No newline at end of file +Coverage: 63.23%Coverage63.23% \ No newline at end of file diff --git a/dist/index.js b/dist/index.js index 9ad1763..bff0393 100644 --- a/dist/index.js +++ b/dist/index.js @@ -16348,7 +16348,7 @@ var __importStar = (this && this.__importStar) || function (mod) { return result; }; Object.defineProperty(exports, "__esModule", ({ value: true })); -exports.pullRequestAuthorHasNotOptedIn = exports.workflowTriggeredForUnsupportedEvent = exports.fetchDiff = exports.loadConfiguration = void 0; +exports.fetchDiff = exports.pullRequestAuthorHasNotOptedIn = exports.workflowTriggeredForUnsupportedEvent = exports.loadConfiguration = void 0; const core = __importStar(__nccwpck_require__(2186)); const github = __importStar(__nccwpck_require__(5438)); const fs = __importStar(__nccwpck_require__(7147)); @@ -16373,51 +16373,6 @@ function loadConfiguration() { return YAML.parse(fs.readFileSync(configFile, 'utf8')); } exports.loadConfiguration = loadConfiguration; -/** - * Retrieves the diff of the pull request that triggered this workflow which we - * will use for evaluation. - * - * @param pull The pull request that triggered this workflow. - * @returns The diff of the given pull request. - */ -async function fetchDiff(pull) { - const git = (0, simple_git_1.simpleGit)('.', { trimmed: true }); - const diffArgs = ['--merge-base', `origin/${pull.base.ref}`].concat(core.getInput('git-diff-args').split(/\s+/)); - core.info(`Retrieving diff with \`git diff ${diffArgs.join(' ')}\``); - // Fetch all commits for the head branch back to where it diverged from the base. - core.debug(`Fetching ${pull.commits + 1} commits for ${pull.head.ref}`); - await git.fetch([ - 'origin', - `+${pull.head.ref}:${pull.head.ref}`, - `--depth=${pull.commits + 1}`, - '--no-tags', - '--prune', - '--no-recurse-submodules' - ]); - core.debug(`Switching to branch ${pull.head.ref}`); - await git.raw('switch', pull.head.ref); - // Fetch commits for the base branch that were made since the head diverged from it. - const divergedFrom = await git.raw('rev-list', '--max-parents=0', 'HEAD'); - const divergedAt = await git.show([ - '--quiet', - '--date=unix', - '--format=%cd', - divergedFrom - ]); - core.debug(`Retrieving history for origin/${pull.base.ref} since ${divergedFrom} which was committed at ${divergedAt}`); - await git.fetch([ - 'origin', - `+${pull.base.ref}:${pull.base.ref}`, - `--shallow-since=${divergedAt}`, - '--no-tags', - '--prune', - '--no-recurse-submodules' - ]); - // Now we have all relevant history from both base and head branches, so we - // can compute an accurate diff relative to the base branch. - return git.diff(diffArgs); -} -exports.fetchDiff = fetchDiff; function workflowTriggeredForUnsupportedEvent() { if (github.context.eventName !== 'pull_request') { core.setFailed("This action is only supported on the 'pull_request' event, " + @@ -16438,6 +16393,49 @@ function pullRequestAuthorHasNotOptedIn(config, pullRequest) { return false; } exports.pullRequestAuthorHasNotOptedIn = pullRequestAuthorHasNotOptedIn; +/** + * Retrieves the diff of the pull request that triggered this workflow which we + * will use for evaluation. + * + * @param pull The pull request that triggered this workflow. + * @returns The diff of the given pull request or `undefined` if we failed to retrieve it. + */ +async function fetchDiff(pull) { + const git = (0, simple_git_1.simpleGit)('.', { trimmed: true }); + // let baseRefExists = false + // try { + // baseRefExists = !!(await git.raw('rev-parse', '--verify', pull.base.ref)) + // } catch (e) { + // core.error( + // `Error from 'git rev-parse --verfy ${pull.base.ref}': ${ + // (e as Error).message + // }` + // ) + // } + // if (!baseRefExists) { + // core.setFailed( + // `Could not find pull request base branch ${pull.base.ref}. ` + + // `Please make sure actions/checkout was used beforehand to fetch ${pull.base.ref}.` + // ) + // return + // } + core.debug(`Fetching base ref "${pull.base.ref}" and head ref "${pull.head.ref}"`); + await git.fetch([ + 'origin', + `+${pull.base.ref}:${pull.base.ref}`, + `+${pull.head.ref}:${pull.head.ref}`, + `--filter=tree:0`, + '--no-tags', + '--prune', + '--no-recurse-submodules' + ]); + core.debug(`Switching to head ref "${pull.head.ref}"`); + await git.raw('switch', pull.head.ref); + const diffArgs = ['--merge-base', pull.base.ref].concat(core.getInput('git-diff-options').split(/\s+/)); + core.info(`Retrieving diff with \`git diff ${diffArgs.join(' ')}\``); + return git.diff(diffArgs); +} +exports.fetchDiff = fetchDiff; /***/ }), @@ -16501,7 +16499,10 @@ async function run() { const pullRequest = github.context.payload.pull_request; if ((0, initializer_1.pullRequestAuthorHasNotOptedIn)(config, pullRequest)) return; - const score = await evaluatePullRequest(pullRequest, config); + const diff = await (0, initializer_1.fetchDiff)(pullRequest); + if (!diff) + return; + const score = await evaluatePullRequest(pullRequest, diff, config); await applyCategoryLabel(pullRequest, score, config); await addScoreThresholdExceededComment(pullRequest, score, config); } @@ -16521,7 +16522,7 @@ exports.run = run; * to the `sizeup` library. * @returns The score that the pull request received */ -async function evaluatePullRequest(pull, config) { +async function evaluatePullRequest(pull, diff, config) { const pullRequestNickname = `${pull.base.repo.owner.login}/${pull.base.repo.name}#${pull.number}`; core.info(`Evaluating pull request ${pullRequestNickname}`); let sizeupConfigFile = undefined; @@ -16530,7 +16531,6 @@ async function evaluatePullRequest(pull, config) { fs.mkdirSync(path.dirname(sizeupConfigFile)); fs.writeFileSync(sizeupConfigFile, YAML.stringify(config.sizeup)); } - const diff = await (0, initializer_1.fetchDiff)(pull); const score = sizeup_core_1.SizeUp.evaluate(diff, sizeupConfigFile); if (sizeupConfigFile) { fs.rmSync(sizeupConfigFile, { force: true, recursive: true }); diff --git a/src/initializer.ts b/src/initializer.ts index 34e1992..c4eb934 100644 --- a/src/initializer.ts +++ b/src/initializer.ts @@ -26,60 +26,6 @@ export function loadConfiguration(): Configuration { return YAML.parse(fs.readFileSync(configFile, 'utf8')) as Configuration } -/** - * Retrieves the diff of the pull request that triggered this workflow which we - * will use for evaluation. - * - * @param pull The pull request that triggered this workflow. - * @returns The diff of the given pull request. - */ -export async function fetchDiff(pull: PullRequest): Promise { - const git = simpleGit('.', { trimmed: true }) - const diffArgs = ['--merge-base', `origin/${pull.base.ref}`].concat( - core.getInput('git-diff-args').split(/\s+/) - ) - - core.info(`Retrieving diff with \`git diff ${diffArgs.join(' ')}\``) - - // Fetch all commits for the head branch back to where it diverged from the base. - core.debug(`Fetching ${pull.commits + 1} commits for ${pull.head.ref}`) - await git.fetch([ - 'origin', - `+${pull.head.ref}:${pull.head.ref}`, - `--depth=${pull.commits + 1}`, - '--no-tags', - '--prune', - '--no-recurse-submodules' - ]) - - core.debug(`Switching to branch ${pull.head.ref}`) - await git.raw('switch', pull.head.ref) - - // Fetch commits for the base branch that were made since the head diverged from it. - const divergedFrom = await git.raw('rev-list', '--max-parents=0', 'HEAD') - const divergedAt = await git.show([ - '--quiet', - '--date=unix', - '--format=%cd', - divergedFrom - ]) - core.debug( - `Retrieving history for origin/${pull.base.ref} since ${divergedFrom} which was committed at ${divergedAt}` - ) - await git.fetch([ - 'origin', - `+${pull.base.ref}:${pull.base.ref}`, - `--shallow-since=${divergedAt}`, - '--no-tags', - '--prune', - '--no-recurse-submodules' - ]) - - // Now we have all relevant history from both base and head branches, so we - // can compute an accurate diff relative to the base branch. - return git.diff(diffArgs) -} - export function workflowTriggeredForUnsupportedEvent(): boolean { if (github.context.eventName !== 'pull_request') { core.setFailed( @@ -113,3 +59,58 @@ export function pullRequestAuthorHasNotOptedIn( return false } + +/** + * Retrieves the diff of the pull request that triggered this workflow which we + * will use for evaluation. + * + * @param pull The pull request that triggered this workflow. + * @returns The diff of the given pull request or `undefined` if we failed to retrieve it. + */ +export async function fetchDiff( + pull: PullRequest +): Promise { + const git = simpleGit('.', { trimmed: true }) + + // let baseRefExists = false + // try { + // baseRefExists = !!(await git.raw('rev-parse', '--verify', pull.base.ref)) + // } catch (e) { + // core.error( + // `Error from 'git rev-parse --verfy ${pull.base.ref}': ${ + // (e as Error).message + // }` + // ) + // } + + // if (!baseRefExists) { + // core.setFailed( + // `Could not find pull request base branch ${pull.base.ref}. ` + + // `Please make sure actions/checkout was used beforehand to fetch ${pull.base.ref}.` + // ) + // return + // } + + core.debug( + `Fetching base ref "${pull.base.ref}" and head ref "${pull.head.ref}"` + ) + + await git.fetch([ + 'origin', + `+${pull.base.ref}:${pull.base.ref}`, + `+${pull.head.ref}:${pull.head.ref}`, + `--filter=tree:0`, + '--no-tags', + '--prune', + '--no-recurse-submodules' + ]) + + core.debug(`Switching to head ref "${pull.head.ref}"`) + await git.raw('switch', pull.head.ref) + + const diffArgs = ['--merge-base', pull.base.ref].concat( + core.getInput('git-diff-options').split(/\s+/) + ) + core.info(`Retrieving diff with \`git diff ${diffArgs.join(' ')}\``) + return git.diff(diffArgs) +} diff --git a/src/main.ts b/src/main.ts index 04f9c42..13851ec 100644 --- a/src/main.ts +++ b/src/main.ts @@ -37,7 +37,10 @@ export async function run(): Promise { if (pullRequestAuthorHasNotOptedIn(config, pullRequest)) return - const score = await evaluatePullRequest(pullRequest, config) + const diff = await fetchDiff(pullRequest) + if (!diff) return + + const score = await evaluatePullRequest(pullRequest, diff, config) await applyCategoryLabel(pullRequest, score, config) await addScoreThresholdExceededComment(pullRequest, score, config) @@ -58,6 +61,7 @@ export async function run(): Promise { */ async function evaluatePullRequest( pull: PullRequest, + diff: string, config: Configuration ): Promise { const pullRequestNickname = `${pull.base.repo.owner.login}/${pull.base.repo.name}#${pull.number}` @@ -70,7 +74,6 @@ async function evaluatePullRequest( fs.writeFileSync(sizeupConfigFile, YAML.stringify(config.sizeup)) } - const diff = await fetchDiff(pull) const score = SizeUp.evaluate(diff, sizeupConfigFile) if (sizeupConfigFile) {