Skip to content

Commit 95fd273

Browse files
committed
updateCommitMappings: run only one big range-diff
Over the time, GitGitGadget accumulated _quite_ a few open PRs. For all of them, the commit mappings are updated whenever `pu` changed. But that can take _quite_ a long time, due to an ancient merge base. See e.g. gitgitgadget/git#15, which still awaits the time when this developer can do some fun stuff again. Even worse: since this task is run on a low-powered Azure VM, it takes quite a few resources, and quite a long time just to update the commit mappings. Let's coalesce all the ranges that need to be compared into a single one, and run only a single range-diff (which will still take something like 90 seconds, but that is way better than the 20 minutes the many range-diffs take right now). To do so, we construct a throw-away octopus merge of all the base commits, and another octopus merge of all the head commits, and then use the range between these octopus merges as the first commit range for the `git range-diff` command. To make sure that we're not missing anything (e.g. when one of the PRs has `pu` as its base commit), we do not use the first octopus merge in the second commit range. Instead, we use `maint~100..pu`, which should be plenty enough. Note: we could save on some running time by using `maint~10` instead (roughly 20 seconds) or even `maint~1` (about a minute), but we want to stay safe in case a PR goes directly to `maint` (which _has_ happened before). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
1 parent 233dded commit 95fd273

File tree

2 files changed

+34
-5
lines changed

2 files changed

+34
-5
lines changed

Diff for: lib/ci-helper.ts

+33-5
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as fs from "fs";
22
import * as util from "util";
33
import { ILintError, LintCommit } from "./commit-lint";
4-
import { commitExists, git } from "./git";
4+
import { commitExists, git, emptyTreeName } from "./git";
55
import { GitNotes } from "./git-notes";
66
import { GitGitGadget, IGitGitGadgetOptions } from "./gitgitgadget";
77
import { GitHubGlue, IGitHubUser, IPRCommit,
@@ -159,6 +159,14 @@ export class CIHelper {
159159
{ workDir: this.workDir })).split("\n"),
160160
);
161161
let result = false;
162+
/*
163+
* Both `bases` and `heads` accumulate the `-p<commit-hash>` parameters
164+
* for the `git commit-tree` command for the two octopus merges. We
165+
* need to make sure that no parent is listed twice, as `git
166+
* commit-tree` would error out on that.
167+
*/
168+
const bases = new Set<string>();
169+
const heads = new Set<string>();
162170
for (const pullRequestURL of Object.keys(options.openPRs)) {
163171
const info = await this.getPRMetadata(pullRequestURL);
164172
if (info === undefined || info.latestTag === undefined ||
@@ -183,24 +191,44 @@ export class CIHelper {
183191
meta.commitInGitGit = undefined;
184192
result = true;
185193
}
194+
bases.add(`-p${info.baseCommit}`);
195+
heads.add(`-p${info.headCommit}`);
196+
}
197+
198+
if (heads.size > 0) {
199+
/*
200+
* Generate throw-away octopus merges to combine multiple commit
201+
* ranges into a single one.
202+
*/
203+
const octopus = async (set: Set<string>): Promise<string> => {
204+
const array = Array.from(set);
205+
if (array.length === 1) {
206+
return array[0];
207+
}
208+
return await
209+
git(["commit-tree", ...array, emptyTreeName, "-m", "()"],
210+
{ workDir: this.workDir });
211+
};
186212

213+
const range1 = `${await octopus(bases)}..${await octopus(heads)}`;
214+
const range2 =
215+
"refs/remotes/upstream/maint~100..refs/remotes/upstream/pu";
187216
const start = Date.now();
188217
const out = await git(["-c", "core.abbrev=40", "range-diff", "-s",
189-
info.baseCommit, info.headCommit,
190-
"refs/remotes/upstream/pu"],
218+
range1, range2],
191219
{ workDir: this.workDir });
192220
const duration = Date.now() - start;
193221
if (duration > 2000)
194222
console.log(`warning: \`git range-diff ${
195-
info.baseCommit} ${info.headCommit} upstream/pu\` took ${
223+
range1} ${range2}\` took ${
196224
duration / 1000} seconds`);
197225
for (const line of out.split("\n")) {
198226
const match =
199227
line.match(/^[^:]*: *([^ ]*) [!=][^:]*: *([^ ]*)/);
200228
if (!match) {
201229
continue;
202230
}
203-
const messageID2 = match[1] === info.headCommit ? messageID :
231+
const messageID2 =
204232
await this.getMessageIdForOriginalCommit(match[1]);
205233
if (messageID2 === undefined) {
206234
continue;

Diff for: lib/git.ts

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export interface IGitOptions {
1313
}
1414

1515
export const emptyBlobName = "e69de29bb2d1d6434b8b29ae775ad8c2e48c5391";
16+
export const emptyTreeName = "4b825dc642cb6eb9a060e54bf8d69288fbee4904";
1617

1718
function trimTrailingNewline(str: string): string {
1819
return str.replace(/\r?\n$/, "");

0 commit comments

Comments
 (0)