Skip to content

Commit

Permalink
feat(git): Make shorten_hash() guaranteed to produce a unique hash
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@AtkinsSJ @KernelDeimos
AtkinsSJ authored and KernelDeimos committed Jun 28, 2024
1 parent fa3df72 commit dd10a37
Show file tree
Hide file tree
Showing 11 changed files with 74 additions and 35 deletions.
33 changes: 21 additions & 12 deletions packages/git/src/format.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,14 @@ export const process_commit_formatting_options = (options) => {

/**
* Format the given oid hash, followed by any refs that point to it
* @param git_context {{ fs, dir, gitdir, cache }} as taken by most isomorphic-git methods.
* @param oid
* @param short_hashes Whwther to shorten the hash
* @returns {String}
*/
export const format_oid = (oid, { short_hashes = false } = {}) => {
export const format_oid = async (git_context, oid, { short_hashes = false } = {}) => {
// TODO: List refs at this commit, after the hash
return short_hashes ? shorten_hash(oid) : oid;
return short_hashes ? shorten_hash(git_context, oid) : oid;
}

/**
Expand Down Expand Up @@ -110,30 +111,31 @@ export const format_timestamp_and_offset = (owner) => {

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

switch (options.format || 'medium') {
// TODO: Other formats
case 'oneline':
return `${chalk.yellow(format_oid(oid, options))} ${title_line()}`;
return `${chalk.yellow(await format_oid(git_context, oid, options))} ${title_line()}`;
case 'short': {
let s = '';
s += chalk.yellow(`commit ${format_oid(oid, options)}\n`);
s += chalk.yellow(`commit ${await format_oid(git_context, oid, options)}\n`);
s += `Author: ${format_person(commit.author)}\n`;
s += '\n';
s += indent(title_line());
return s;
}
case 'medium': {
let s = '';
s += chalk.yellow(`commit ${format_oid(oid, options)}\n`);
s += chalk.yellow(`commit ${await format_oid(git_context, oid, options)}\n`);
s += `Author: ${format_person(commit.author)}\n`;
s += `Date: ${format_date(commit.author)}\n`;
s += '\n';
Expand All @@ -142,7 +144,7 @@ export const format_commit = (commit, oid, options = {}) => {
}
case 'full': {
let s = '';
s += chalk.yellow(`commit ${format_oid(oid, options)}\n`);
s += chalk.yellow(`commit ${await format_oid(git_context, oid, options)}\n`);
s += `Author: ${format_person(commit.author)}\n`;
s += `Commit: ${format_person(commit.committer)}\n`;
s += '\n';
Expand All @@ -151,7 +153,7 @@ export const format_commit = (commit, oid, options = {}) => {
}
case 'fuller': {
let s = '';
s += chalk.yellow(`commit ${format_oid(oid, options)}\n`);
s += chalk.yellow(`commit ${await format_oid(git_context, oid, options)}\n`);
s += `Author: ${format_person(commit.author)}\n`;
s += `AuthorDate: ${format_date(commit.author)}\n`;
s += `Commit: ${format_person(commit.committer)}\n`;
Expand Down Expand Up @@ -330,6 +332,7 @@ export const process_diff_formatting_options = (options, { show_patch_by_default

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

let s = '';
if (options.raw) {
// https://git-scm.com/docs/diff-format#_raw_output_format
for (const { a, b, diff } of diffs) {
s += `:${a.mode} ${b.mode} ${shorten_hash(a.oid)} ${shorten_hash(b.oid)} `;
const short_a_oid = await shorten_hash(git_context, a.oid);
const short_b_oid = await shorten_hash(git_context, b.oid);

s += `:${a.mode} ${b.mode} ${short_a_oid} ${short_b_oid} `;
// Status. For now, we just support A/D/M
if (a.mode === '000000') {
s += 'A'; // Added
Expand Down Expand Up @@ -409,19 +415,22 @@ export const format_diffs = (diffs, options) => {
const a_path = diff.oldFileName.startsWith('/') ? diff.oldFileName : `${options.source_prefix}${diff.oldFileName}`;
const b_path = diff.newFileName.startsWith('/') ? diff.newFileName : `${options.dest_prefix}${diff.newFileName}`;

const short_a_oid = await shorten_hash(git_context, a.oid);
const short_b_oid = await shorten_hash(git_context, b.oid);

// NOTE: This first line shows `a/$newFileName` for files that are new, not `/dev/null`.
const first_line_a_path = a_path !== '/dev/null' ? a_path : `${options.source_prefix}${diff.newFileName}`;
s += chalk.bold(`diff --git ${first_line_a_path} ${b_path}\n`);
if (a.mode === b.mode) {
s += chalk.bold(`index ${shorten_hash(a.oid)}..${shorten_hash(b.oid)} ${a.mode}\n`);
s += chalk.bold(`index ${short_a_oid}..${short_b_oid} ${a.mode}\n`);
} else {
if (a.mode === '000000') {
s += chalk.bold(`new file mode ${b.mode}\n`);
} else {
s += chalk.bold(`old mode ${a.mode}\n`);
s += chalk.bold(`new mode ${b.mode}\n`);
}
s += chalk.bold(`index ${shorten_hash(a.oid)}..${shorten_hash(b.oid)}\n`);
s += chalk.bold(`index ${short_a_oid}..${short_b_oid}\n`);
}
if (!diff.hunks.length)
continue;
Expand Down
29 changes: 23 additions & 6 deletions packages/git/src/git-helpers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,31 @@ export const find_repo_root = async (fs, pwd) => {

/**
* Produce a shortened version of the given hash, which is still unique within the repo.
* TODO: Ensure that whatever we produce is unique within the repo.
* For now this is just a convenience function, so there's one place to change later.
* @param git_context {{ fs, dir, gitdir, cache }} as taken by most isomorphic-git methods.
* @param hash
* @returns {String} The shortened hash
* @returns {Promise<String>} The shortened hash
*/
export const shorten_hash = (hash) => {
// TODO: Ensure that whatever we produce is unique within the repo
return hash.slice(0, 7);
export const shorten_hash = async (git_context, hash) => {
// Repeatedly take the prefix of the hash, and try and resolve it into a full hash.
// git.expandOid() will only succeed if there is exactly one possibility, so if it fails,
// we make the prefix longer and try again.
let short_hash = hash.slice(0, 7);
while (true) {
try {
const expanded = await git.expandOid({ ...git_context, oid: short_hash });
// Sanity-check: Ensure we got the original hash back.
if (expanded === hash)
return short_hash;
} catch (e) {
// Failed, so try again with a longer one
}
if (short_hash.length < hash.length) {
short_hash = hash.slice(0, short_hash.length + 1);
continue;
}
// Otherwise, we failed, so just return the original hash.
return hash;
}
}

/**
Expand Down
6 changes: 4 additions & 2 deletions packages/git/src/subcommands/branch.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ const BRANCH = {
const { io, fs, env, args } = ctx;
const { stdout, stderr } = io;
const { options, positionals, tokens } = args;
const cache = {};

for (const token of tokens) {
if (token.kind !== 'option') continue;
Expand Down Expand Up @@ -204,7 +205,7 @@ const BRANCH = {
stderr(`error: ${result.reason}`);
} else {
const oid = result.value;
const hash = shorten_hash(result.value);
const hash = await shorten_hash({fs, dir, gitdir, cache}, result.value);
stdout(`Deleted branch ${branch} (was ${hash}).`);
}
}
Expand Down Expand Up @@ -271,7 +272,8 @@ const BRANCH = {

if (!current_branch) {
const oid = await git.resolveRef({ fs, dir, gitdir, ref: 'HEAD' });
stdout(`* ${chalk.greenBright(`(HEAD detached at ${shorten_hash(oid)})`)}`);
const short_oid = await shorten_hash({ fs, dir, gitdir, cache }, oid);
stdout(`* ${chalk.greenBright(`(HEAD detached at ${short_oid})`)}`);
}

for (const branch of branches) {
Expand Down
7 changes: 5 additions & 2 deletions packages/git/src/subcommands/checkout.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,11 @@ const CHECKOUT = {
const commit_title = commit.commit.message.split('\n', 2)[0];
const old_commit_title = old_commit.commit.message.split('\n', 2)[0];

stdout(`Previous HEAD position was ${shorten_hash(old_commit.oid)} ${old_commit_title}`);
stdout(`HEAD is now at ${shorten_hash(commit.oid)} ${commit_title}`);
const short_oid = await shorten_hash({ fs, dir, gitdir, cache }, commit.oid);
const short_old_oid = await shorten_hash({ fs, dir, gitdir, cache }, old_commit.oid);

stdout(`Previous HEAD position was ${short_old_oid} ${old_commit_title}`);
stdout(`HEAD is now at ${short_oid} ${commit_title}`);
}
}
}
Expand Down
8 changes: 5 additions & 3 deletions packages/git/src/subcommands/cherry-pick.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export default {
for (const commit_data of commits) {
const commit = commit_data.commit;
const commit_title = commit.message.split('\n')[0];
const short_commit_oid = await shorten_hash({ fs, dir, gitdir, cache }, commit_data.oid);

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

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

// Print out information about the new commit.
// TODO: Should be a lot more output. See commit.js for a similar list of TODOs.
stdout(`[${branch} ${shorten_hash(head_oid)}] ${commit_title}`);
stdout(`[${branch} ${short_head_oid}] ${commit_title}`);
}
}
}
3 changes: 2 additions & 1 deletion packages/git/src/subcommands/commit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export default {
const { io, fs, env, args } = ctx;
const { stdout, stderr } = io;
const { options, positionals } = args;
const cache = {};

if (!options.message) {
// TODO: Support opening a temporary file in an editor,
Expand Down Expand Up @@ -97,7 +98,7 @@ export default {
gitdir,
});
const commit_title = options.message.split('\n')[0];
const short_hash = shorten_hash(commit_hash);
const short_hash = await shorten_hash({ fs, dir, gitdir, cache }, commit_hash);
let output = `[${branch} ${short_hash}] ${commit_title}\n`;
// TODO: --amend prints out the date of the original commit here, as:
// ` Date: Fri May 17 15:45:47 2024 +0100`
Expand Down
4 changes: 2 additions & 2 deletions packages/git/src/subcommands/diff.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export default {

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

return;
}
Expand Down Expand Up @@ -193,7 +193,7 @@ export default {
}

if (!diff_options.no_patch)
stdout(format_diffs(diffs, diff_options));
stdout(await format_diffs({ fs, dir, gitdir, cache }, diffs, diff_options));

if (options['exit-code'])
return diffs.length > 0 ? 1 : 0;
Expand Down
4 changes: 2 additions & 2 deletions packages/git/src/subcommands/log.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export default {
const read_tree = walker => walker?.content()?.then(it => new TextDecoder().decode(it));

for (const commit of log) {
stdout(format_commit(commit.commit, commit.oid, options));
stdout(await format_commit({ fs, dir, gitdir, cache }, commit.commit, commit.oid, options));
if (diff_options.display_diff()) {
const diffs = await diff_git_trees({
...diff_ctx,
Expand All @@ -95,7 +95,7 @@ export default {
read_a: read_tree,
read_b: read_tree,
});
stdout(format_diffs(diffs, diff_options));
stdout(await format_diffs({ fs, dir, gitdir, cache }, diffs, diff_options));
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions packages/git/src/subcommands/push.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,9 @@ export default {
...authenticator.get_auth_callbacks(stderr),
});
let flag = ' ';
let summary = `${shorten_hash(old_dest_oid)}..${shorten_hash(source_oid)}`;
const short_old_oid = await shorten_hash({ fs, dir, gitdir, cache }, old_dest_oid);
const short_new_oid = await shorten_hash({ fs, dir, gitdir, cache }, source_oid);
let summary = `${short_old_oid}..${short_new_oid}`;
if (delete_) {
flag = '-';
summary = '[deleted]';
Expand All @@ -242,7 +244,7 @@ export default {
summary = '[new branch]';
} else if (force) {
flag = '+';
summary = `${shorten_hash(old_dest_oid)}...${shorten_hash(source_oid)}`;
summary = `${short_old_oid}...${short_new_oid}`;
} else if (is_up_to_date) {
flag = '=';
summary = `[up to date]`;
Expand Down
4 changes: 2 additions & 2 deletions packages/git/src/subcommands/show.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export default {
case 'blob':
return new TextDecoder().decode(parsed_object.object);
case 'commit': {
let s = format_commit(parsed_object.object, parsed_object.oid, options);
let s = await format_commit({ fs, dir, gitdir, cache }, parsed_object.object, parsed_object.oid, options);
if (diff_options.display_diff()) {
const diffs = await diff_git_trees({
...diff_ctx,
Expand All @@ -81,7 +81,7 @@ export default {
read_b: read_tree,
});
s += '\n';
s += format_diffs(diffs, diff_options);
s += await format_diffs({ fs, dir, gitdir, cache }, diffs, diff_options);
}
return s;
}
Expand Down
5 changes: 4 additions & 1 deletion packages/git/src/subcommands/status.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export default {
const { io, fs, env, args } = ctx;
const { stdout, stderr } = io;
const { options, positionals } = args;
const cache = {};

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

Expand All @@ -42,6 +43,7 @@ export default {
fs,
dir,
gitdir,
cache,
ignored: false,
});

Expand Down Expand Up @@ -117,7 +119,8 @@ export default {
}
} else {
const oid = await git.resolveRef({ fs, dir, gitdir, ref: 'HEAD' });
stdout(`${chalk.redBright('HEAD detached at')} ${shorten_hash(oid)}`);
const short_oid = await shorten_hash({ fs, dir, gitdir, cache }, oid);
stdout(`${chalk.redBright('HEAD detached at')} ${short_oid}`);
}

if (staged.length) {
Expand Down

0 comments on commit dd10a37

Please sign in to comment.