Skip to content

Commit dd10a37

Browse files
AtkinsSJKernelDeimos
AtkinsSJ
authored and
KernelDeimos
committed
feat(git): Make shorten_hash() guaranteed to produce a unique hash
Until now we just relied on it being very unlikely that there would be a collision. This new method ensures that the returned hash is unique.
1 parent fa3df72 commit dd10a37

File tree

11 files changed

+74
-35
lines changed

11 files changed

+74
-35
lines changed

‎packages/git/src/format.js

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,14 @@ export const process_commit_formatting_options = (options) => {
6363

6464
/**
6565
* Format the given oid hash, followed by any refs that point to it
66+
* @param git_context {{ fs, dir, gitdir, cache }} as taken by most isomorphic-git methods.
6667
* @param oid
6768
* @param short_hashes Whwther to shorten the hash
6869
* @returns {String}
6970
*/
70-
export const format_oid = (oid, { short_hashes = false } = {}) => {
71+
export const format_oid = async (git_context, oid, { short_hashes = false } = {}) => {
7172
// TODO: List refs at this commit, after the hash
72-
return short_hashes ? shorten_hash(oid) : oid;
73+
return short_hashes ? shorten_hash(git_context, oid) : oid;
7374
}
7475

7576
/**
@@ -110,30 +111,31 @@ export const format_timestamp_and_offset = (owner) => {
110111

111112
/**
112113
* Produce a string representation of a commit.
114+
* @param git_context {{ fs, dir, gitdir, cache }} as taken by most isomorphic-git methods.
113115
* @param commit A CommitObject
114116
* @param oid Commit hash
115117
* @param options Options returned by parsing the command arguments in `commit_formatting_options`
116118
* @returns {string}
117119
*/
118-
export const format_commit = (commit, oid, options = {}) => {
120+
export const format_commit = async (git_context, commit, oid, options = {}) => {
119121
const title_line = () => commit.message.split('\n')[0];
120122
const indent = (text) => text.split('\n').map(it => ` ${it}`).join('\n');
121123

122124
switch (options.format || 'medium') {
123125
// TODO: Other formats
124126
case 'oneline':
125-
return `${chalk.yellow(format_oid(oid, options))} ${title_line()}`;
127+
return `${chalk.yellow(await format_oid(git_context, oid, options))} ${title_line()}`;
126128
case 'short': {
127129
let s = '';
128-
s += chalk.yellow(`commit ${format_oid(oid, options)}\n`);
130+
s += chalk.yellow(`commit ${await format_oid(git_context, oid, options)}\n`);
129131
s += `Author: ${format_person(commit.author)}\n`;
130132
s += '\n';
131133
s += indent(title_line());
132134
return s;
133135
}
134136
case 'medium': {
135137
let s = '';
136-
s += chalk.yellow(`commit ${format_oid(oid, options)}\n`);
138+
s += chalk.yellow(`commit ${await format_oid(git_context, oid, options)}\n`);
137139
s += `Author: ${format_person(commit.author)}\n`;
138140
s += `Date: ${format_date(commit.author)}\n`;
139141
s += '\n';
@@ -142,7 +144,7 @@ export const format_commit = (commit, oid, options = {}) => {
142144
}
143145
case 'full': {
144146
let s = '';
145-
s += chalk.yellow(`commit ${format_oid(oid, options)}\n`);
147+
s += chalk.yellow(`commit ${await format_oid(git_context, oid, options)}\n`);
146148
s += `Author: ${format_person(commit.author)}\n`;
147149
s += `Commit: ${format_person(commit.committer)}\n`;
148150
s += '\n';
@@ -151,7 +153,7 @@ export const format_commit = (commit, oid, options = {}) => {
151153
}
152154
case 'fuller': {
153155
let s = '';
154-
s += chalk.yellow(`commit ${format_oid(oid, options)}\n`);
156+
s += chalk.yellow(`commit ${await format_oid(git_context, oid, options)}\n`);
155157
s += `Author: ${format_person(commit.author)}\n`;
156158
s += `AuthorDate: ${format_date(commit.author)}\n`;
157159
s += `Commit: ${format_person(commit.committer)}\n`;
@@ -330,6 +332,7 @@ export const process_diff_formatting_options = (options, { show_patch_by_default
330332

331333
/**
332334
* Produce a string representation of the given diffs.
335+
* @param git_context {{ fs, dir, gitdir, cache }} as taken by most isomorphic-git methods.
333336
* @param diffs A single object, or array of them, in the format:
334337
* {
335338
* a: { mode, oid },
@@ -339,15 +342,18 @@ export const process_diff_formatting_options = (options, { show_patch_by_default
339342
* @param options Object returned by process_diff_formatting_options()
340343
* @returns {string}
341344
*/
342-
export const format_diffs = (diffs, options) => {
345+
export const format_diffs = async (git_context, diffs, options) => {
343346
if (!(diffs instanceof Array))
344347
diffs = [diffs];
345348

346349
let s = '';
347350
if (options.raw) {
348351
// https://git-scm.com/docs/diff-format#_raw_output_format
349352
for (const { a, b, diff } of diffs) {
350-
s += `:${a.mode} ${b.mode} ${shorten_hash(a.oid)} ${shorten_hash(b.oid)} `;
353+
const short_a_oid = await shorten_hash(git_context, a.oid);
354+
const short_b_oid = await shorten_hash(git_context, b.oid);
355+
356+
s += `:${a.mode} ${b.mode} ${short_a_oid} ${short_b_oid} `;
351357
// Status. For now, we just support A/D/M
352358
if (a.mode === '000000') {
353359
s += 'A'; // Added
@@ -409,19 +415,22 @@ export const format_diffs = (diffs, options) => {
409415
const a_path = diff.oldFileName.startsWith('/') ? diff.oldFileName : `${options.source_prefix}${diff.oldFileName}`;
410416
const b_path = diff.newFileName.startsWith('/') ? diff.newFileName : `${options.dest_prefix}${diff.newFileName}`;
411417

418+
const short_a_oid = await shorten_hash(git_context, a.oid);
419+
const short_b_oid = await shorten_hash(git_context, b.oid);
420+
412421
// NOTE: This first line shows `a/$newFileName` for files that are new, not `/dev/null`.
413422
const first_line_a_path = a_path !== '/dev/null' ? a_path : `${options.source_prefix}${diff.newFileName}`;
414423
s += chalk.bold(`diff --git ${first_line_a_path} ${b_path}\n`);
415424
if (a.mode === b.mode) {
416-
s += chalk.bold(`index ${shorten_hash(a.oid)}..${shorten_hash(b.oid)} ${a.mode}\n`);
425+
s += chalk.bold(`index ${short_a_oid}..${short_b_oid} ${a.mode}\n`);
417426
} else {
418427
if (a.mode === '000000') {
419428
s += chalk.bold(`new file mode ${b.mode}\n`);
420429
} else {
421430
s += chalk.bold(`old mode ${a.mode}\n`);
422431
s += chalk.bold(`new mode ${b.mode}\n`);
423432
}
424-
s += chalk.bold(`index ${shorten_hash(a.oid)}..${shorten_hash(b.oid)}\n`);
433+
s += chalk.bold(`index ${short_a_oid}..${short_b_oid}\n`);
425434
}
426435
if (!diff.hunks.length)
427436
continue;

‎packages/git/src/git-helpers.js

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,31 @@ export const find_repo_root = async (fs, pwd) => {
7474

7575
/**
7676
* Produce a shortened version of the given hash, which is still unique within the repo.
77-
* TODO: Ensure that whatever we produce is unique within the repo.
78-
* For now this is just a convenience function, so there's one place to change later.
77+
* @param git_context {{ fs, dir, gitdir, cache }} as taken by most isomorphic-git methods.
7978
* @param hash
80-
* @returns {String} The shortened hash
79+
* @returns {Promise<String>} The shortened hash
8180
*/
82-
export const shorten_hash = (hash) => {
83-
// TODO: Ensure that whatever we produce is unique within the repo
84-
return hash.slice(0, 7);
81+
export const shorten_hash = async (git_context, hash) => {
82+
// Repeatedly take the prefix of the hash, and try and resolve it into a full hash.
83+
// git.expandOid() will only succeed if there is exactly one possibility, so if it fails,
84+
// we make the prefix longer and try again.
85+
let short_hash = hash.slice(0, 7);
86+
while (true) {
87+
try {
88+
const expanded = await git.expandOid({ ...git_context, oid: short_hash });
89+
// Sanity-check: Ensure we got the original hash back.
90+
if (expanded === hash)
91+
return short_hash;
92+
} catch (e) {
93+
// Failed, so try again with a longer one
94+
}
95+
if (short_hash.length < hash.length) {
96+
short_hash = hash.slice(0, short_hash.length + 1);
97+
continue;
98+
}
99+
// Otherwise, we failed, so just return the original hash.
100+
return hash;
101+
}
85102
}
86103

87104
/**

‎packages/git/src/subcommands/branch.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ const BRANCH = {
7474
const { io, fs, env, args } = ctx;
7575
const { stdout, stderr } = io;
7676
const { options, positionals, tokens } = args;
77+
const cache = {};
7778

7879
for (const token of tokens) {
7980
if (token.kind !== 'option') continue;
@@ -204,7 +205,7 @@ const BRANCH = {
204205
stderr(`error: ${result.reason}`);
205206
} else {
206207
const oid = result.value;
207-
const hash = shorten_hash(result.value);
208+
const hash = await shorten_hash({fs, dir, gitdir, cache}, result.value);
208209
stdout(`Deleted branch ${branch} (was ${hash}).`);
209210
}
210211
}
@@ -271,7 +272,8 @@ const BRANCH = {
271272

272273
if (!current_branch) {
273274
const oid = await git.resolveRef({ fs, dir, gitdir, ref: 'HEAD' });
274-
stdout(`* ${chalk.greenBright(`(HEAD detached at ${shorten_hash(oid)})`)}`);
275+
const short_oid = await shorten_hash({ fs, dir, gitdir, cache }, oid);
276+
stdout(`* ${chalk.greenBright(`(HEAD detached at ${short_oid})`)}`);
275277
}
276278

277279
for (const branch of branches) {

‎packages/git/src/subcommands/checkout.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,11 @@ const CHECKOUT = {
158158
const commit_title = commit.commit.message.split('\n', 2)[0];
159159
const old_commit_title = old_commit.commit.message.split('\n', 2)[0];
160160

161-
stdout(`Previous HEAD position was ${shorten_hash(old_commit.oid)} ${old_commit_title}`);
162-
stdout(`HEAD is now at ${shorten_hash(commit.oid)} ${commit_title}`);
161+
const short_oid = await shorten_hash({ fs, dir, gitdir, cache }, commit.oid);
162+
const short_old_oid = await shorten_hash({ fs, dir, gitdir, cache }, old_commit.oid);
163+
164+
stdout(`Previous HEAD position was ${short_old_oid} ${old_commit_title}`);
165+
stdout(`HEAD is now at ${short_oid} ${commit_title}`);
163166
}
164167
}
165168
}

‎packages/git/src/subcommands/cherry-pick.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ export default {
6767
for (const commit_data of commits) {
6868
const commit = commit_data.commit;
6969
const commit_title = commit.message.split('\n')[0];
70+
const short_commit_oid = await shorten_hash({ fs, dir, gitdir, cache }, commit_data.oid);
7071

7172
// We can't just add the old commit directly:
7273
// - Its parent is wrong
@@ -107,7 +108,7 @@ export default {
107108
const new_file_contents = Diff.applyPatch(existing_file_contents, diff);
108109
if (!new_file_contents) {
109110
// TODO: We should insert merge conflict markers and wait for the user resolve the conflict.
110-
throw new Error(`Merge conflict: Unable to apply commit ${shorten_hash(commit_data.oid)} ${commit_title}`);
111+
throw new Error(`Merge conflict: Unable to apply commit ${short_commit_oid} ${commit_title}`);
111112
}
112113
// Now, stage the new file contents
113114
const file_oid = await git.writeBlob({
@@ -131,7 +132,7 @@ export default {
131132
if (! await has_staged_changes({ fs, dir, gitdir, cache })) {
132133
// For now, just skip empty commits.
133134
// TODO: cherry-picking should be a multi-step process.
134-
stderr(`Skipping empty commit ${shorten_hash(commit_data.oid)} ${commit_title}`);
135+
stderr(`Skipping empty commit ${short_commit_oid} ${commit_title}`);
135136
continue;
136137
}
137138

@@ -142,10 +143,11 @@ export default {
142143
author: commit.author,
143144
committer: commit.committer,
144145
});
146+
const short_head_oid = await shorten_hash({ fs, dir, gitdir, cache }, head_oid);
145147

146148
// Print out information about the new commit.
147149
// TODO: Should be a lot more output. See commit.js for a similar list of TODOs.
148-
stdout(`[${branch} ${shorten_hash(head_oid)}] ${commit_title}`);
150+
stdout(`[${branch} ${short_head_oid}] ${commit_title}`);
149151
}
150152
}
151153
}

‎packages/git/src/subcommands/commit.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ export default {
4141
const { io, fs, env, args } = ctx;
4242
const { stdout, stderr } = io;
4343
const { options, positionals } = args;
44+
const cache = {};
4445

4546
if (!options.message) {
4647
// TODO: Support opening a temporary file in an editor,
@@ -97,7 +98,7 @@ export default {
9798
gitdir,
9899
});
99100
const commit_title = options.message.split('\n')[0];
100-
const short_hash = shorten_hash(commit_hash);
101+
const short_hash = await shorten_hash({ fs, dir, gitdir, cache }, commit_hash);
101102
let output = `[${branch} ${short_hash}] ${commit_title}\n`;
102103
// TODO: --amend prints out the date of the original commit here, as:
103104
// ` Date: Fri May 17 15:45:47 2024 +0100`

‎packages/git/src/subcommands/diff.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ export default {
112112

113113
const a = { path: a_rel_path, oid: '00000000', mode: mode_string(a_stat) };
114114
const b = { path: b_rel_path, oid: '00000000', mode: mode_string(b_stat) };
115-
stdout(format_diffs({ a, b, diff }, diff_options));
115+
stdout(await format_diffs({ fs, dir, gitdir, cache }, { a, b, diff }, diff_options));
116116

117117
return;
118118
}
@@ -193,7 +193,7 @@ export default {
193193
}
194194

195195
if (!diff_options.no_patch)
196-
stdout(format_diffs(diffs, diff_options));
196+
stdout(await format_diffs({ fs, dir, gitdir, cache }, diffs, diff_options));
197197

198198
if (options['exit-code'])
199199
return diffs.length > 0 ? 1 : 0;

‎packages/git/src/subcommands/log.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export default {
8484
const read_tree = walker => walker?.content()?.then(it => new TextDecoder().decode(it));
8585

8686
for (const commit of log) {
87-
stdout(format_commit(commit.commit, commit.oid, options));
87+
stdout(await format_commit({ fs, dir, gitdir, cache }, commit.commit, commit.oid, options));
8888
if (diff_options.display_diff()) {
8989
const diffs = await diff_git_trees({
9090
...diff_ctx,
@@ -95,7 +95,7 @@ export default {
9595
read_a: read_tree,
9696
read_b: read_tree,
9797
});
98-
stdout(format_diffs(diffs, diff_options));
98+
stdout(await format_diffs({ fs, dir, gitdir, cache }, diffs, diff_options));
9999
}
100100
}
101101
}

‎packages/git/src/subcommands/push.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,9 @@ export default {
233233
...authenticator.get_auth_callbacks(stderr),
234234
});
235235
let flag = ' ';
236-
let summary = `${shorten_hash(old_dest_oid)}..${shorten_hash(source_oid)}`;
236+
const short_old_oid = await shorten_hash({ fs, dir, gitdir, cache }, old_dest_oid);
237+
const short_new_oid = await shorten_hash({ fs, dir, gitdir, cache }, source_oid);
238+
let summary = `${short_old_oid}..${short_new_oid}`;
237239
if (delete_) {
238240
flag = '-';
239241
summary = '[deleted]';
@@ -242,7 +244,7 @@ export default {
242244
summary = '[new branch]';
243245
} else if (force) {
244246
flag = '+';
245-
summary = `${shorten_hash(old_dest_oid)}...${shorten_hash(source_oid)}`;
247+
summary = `${short_old_oid}...${short_new_oid}`;
246248
} else if (is_up_to_date) {
247249
flag = '=';
248250
summary = `[up to date]`;

‎packages/git/src/subcommands/show.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ export default {
6969
case 'blob':
7070
return new TextDecoder().decode(parsed_object.object);
7171
case 'commit': {
72-
let s = format_commit(parsed_object.object, parsed_object.oid, options);
72+
let s = await format_commit({ fs, dir, gitdir, cache }, parsed_object.object, parsed_object.oid, options);
7373
if (diff_options.display_diff()) {
7474
const diffs = await diff_git_trees({
7575
...diff_ctx,
@@ -81,7 +81,7 @@ export default {
8181
read_b: read_tree,
8282
});
8383
s += '\n';
84-
s += format_diffs(diffs, diff_options);
84+
s += await format_diffs({ fs, dir, gitdir, cache }, diffs, diff_options);
8585
}
8686
return s;
8787
}

‎packages/git/src/subcommands/status.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export default {
3434
const { io, fs, env, args } = ctx;
3535
const { stdout, stderr } = io;
3636
const { options, positionals } = args;
37+
const cache = {};
3738

3839
const { dir, gitdir } = await find_repo_root(fs, env.PWD);
3940

@@ -42,6 +43,7 @@ export default {
4243
fs,
4344
dir,
4445
gitdir,
46+
cache,
4547
ignored: false,
4648
});
4749

@@ -117,7 +119,8 @@ export default {
117119
}
118120
} else {
119121
const oid = await git.resolveRef({ fs, dir, gitdir, ref: 'HEAD' });
120-
stdout(`${chalk.redBright('HEAD detached at')} ${shorten_hash(oid)}`);
122+
const short_oid = await shorten_hash({ fs, dir, gitdir, cache }, oid);
123+
stdout(`${chalk.redBright('HEAD detached at')} ${short_oid}`);
121124
}
122125

123126
if (staged.length) {

0 commit comments

Comments
 (0)