Skip to content

Commit 268e6b8

Browse files
gitstergitster
committed
Merge branch 'ab/ambiguous-object-name'
Error output given in response to an ambiguous object name has been improved. * ab/ambiguous-object-name: object-name: re-use "struct strbuf" in show_ambiguous_object() object-name: iterate ambiguous objects before showing header object-name: show date for ambiguous tag objects object-name: make ambiguous object output translatable object-name: explicitly handle bad tags in show_ambiguous_object() object-name: explicitly handle OBJ_BAD in show_ambiguous_object() object-name tests: add tests for ambiguous object blind spots
2 parents dab1b79 + 3a73c1d commit 268e6b8

File tree

2 files changed

+190
-12
lines changed

2 files changed

+190
-12
lines changed

‎object-name.c

Lines changed: 109 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -351,35 +351,118 @@ static int init_object_disambiguation(struct repository *r,
351351
return 0;
352352
}
353353

354+
struct ambiguous_output {
355+
const struct disambiguate_state *ds;
356+
struct strbuf advice;
357+
struct strbuf sb;
358+
};
359+
354360
static int show_ambiguous_object(const struct object_id *oid, void *data)
355361
{
356-
const struct disambiguate_state *ds = data;
357-
struct strbuf desc = STRBUF_INIT;
362+
struct ambiguous_output *state = data;
363+
const struct disambiguate_state *ds = state->ds;
364+
struct strbuf *advice = &state->advice;
365+
struct strbuf *sb = &state->sb;
358366
int type;
367+
const char *hash;
359368

360369
if (ds->fn && !ds->fn(ds->repo, oid, ds->cb_data))
361370
return 0;
362371

372+
hash = repo_find_unique_abbrev(ds->repo, oid, DEFAULT_ABBREV);
363373
type = oid_object_info(ds->repo, oid, NULL);
374+
375+
if (type < 0) {
376+
/*
377+
* TRANSLATORS: This is a line of ambiguous object
378+
* output shown when we cannot look up or parse the
379+
* object in question. E.g. "deadbeef [bad object]".
380+
*/
381+
strbuf_addf(sb, _("%s [bad object]"), hash);
382+
goto out;
383+
}
384+
385+
assert(type == OBJ_TREE || type == OBJ_COMMIT ||
386+
type == OBJ_BLOB || type == OBJ_TAG);
387+
364388
if (type == OBJ_COMMIT) {
389+
struct strbuf date = STRBUF_INIT;
390+
struct strbuf msg = STRBUF_INIT;
365391
struct commit *commit = lookup_commit(ds->repo, oid);
392+
366393
if (commit) {
367394
struct pretty_print_context pp = {0};
368395
pp.date_mode.type = DATE_SHORT;
369-
format_commit_message(commit, " %ad - %s", &desc, &pp);
396+
format_commit_message(commit, "%ad", &date, &pp);
397+
format_commit_message(commit, "%s", &msg, &pp);
370398
}
399+
400+
/*
401+
* TRANSLATORS: This is a line of ambiguous commit
402+
* object output. E.g.:
403+
*
404+
* "deadbeef commit 2021-01-01 - Some Commit Message"
405+
*/
406+
strbuf_addf(sb, _("%s commit %s - %s"), hash, date.buf,
407+
msg.buf);
408+
409+
strbuf_release(&date);
410+
strbuf_release(&msg);
371411
} else if (type == OBJ_TAG) {
372412
struct tag *tag = lookup_tag(ds->repo, oid);
373-
if (!parse_tag(tag) && tag->tag)
374-
strbuf_addf(&desc, " %s", tag->tag);
413+
414+
if (!parse_tag(tag) && tag->tag) {
415+
/*
416+
* TRANSLATORS: This is a line of ambiguous
417+
* tag object output. E.g.:
418+
*
419+
* "deadbeef tag 2022-01-01 - Some Tag Message"
420+
*
421+
* The second argument is the YYYY-MM-DD found
422+
* in the tag.
423+
*
424+
* The third argument is the "tag" string
425+
* from object.c.
426+
*/
427+
strbuf_addf(sb, _("%s tag %s - %s"), hash,
428+
show_date(tag->date, 0, DATE_MODE(SHORT)),
429+
tag->tag);
430+
} else {
431+
/*
432+
* TRANSLATORS: This is a line of ambiguous
433+
* tag object output where we couldn't parse
434+
* the tag itself. E.g.:
435+
*
436+
* "deadbeef [bad tag, could not parse it]"
437+
*/
438+
strbuf_addf(sb, _("%s [bad tag, could not parse it]"),
439+
hash);
440+
}
441+
} else if (type == OBJ_TREE) {
442+
/*
443+
* TRANSLATORS: This is a line of ambiguous <type>
444+
* object output. E.g. "deadbeef tree".
445+
*/
446+
strbuf_addf(sb, _("%s tree"), hash);
447+
} else if (type == OBJ_BLOB) {
448+
/*
449+
* TRANSLATORS: This is a line of ambiguous <type>
450+
* object output. E.g. "deadbeef blob".
451+
*/
452+
strbuf_addf(sb, _("%s blob"), hash);
375453
}
376454

377-
advise(" %s %s%s",
378-
repo_find_unique_abbrev(ds->repo, oid, DEFAULT_ABBREV),
379-
type_name(type) ? type_name(type) : "unknown type",
380-
desc.buf);
381455

382-
strbuf_release(&desc);
456+
out:
457+
/*
458+
* TRANSLATORS: This is line item of ambiguous object output
459+
* from describe_ambiguous_object() above. For RTL languages
460+
* you'll probably want to swap the "%s" and leading " " space
461+
* around.
462+
*/
463+
strbuf_addf(advice, _(" %s\n"), sb->buf);
464+
465+
strbuf_reset(sb);
383466
return 0;
384467
}
385468

@@ -476,6 +559,11 @@ static enum get_oid_result get_short_oid(struct repository *r,
476559

477560
if (!quietly && (status == SHORT_NAME_AMBIGUOUS)) {
478561
struct oid_array collect = OID_ARRAY_INIT;
562+
struct ambiguous_output out = {
563+
.ds = &ds,
564+
.sb = STRBUF_INIT,
565+
.advice = STRBUF_INIT,
566+
};
479567

480568
error(_("short object ID %s is ambiguous"), ds.hex_pfx);
481569

@@ -488,13 +576,22 @@ static enum get_oid_result get_short_oid(struct repository *r,
488576
if (!ds.ambiguous)
489577
ds.fn = NULL;
490578

491-
advise(_("The candidates are:"));
492579
repo_for_each_abbrev(r, ds.hex_pfx, collect_ambiguous, &collect);
493580
sort_ambiguous_oid_array(r, &collect);
494581

495-
if (oid_array_for_each(&collect, show_ambiguous_object, &ds))
582+
if (oid_array_for_each(&collect, show_ambiguous_object, &out))
496583
BUG("show_ambiguous_object shouldn't return non-zero");
584+
585+
/*
586+
* TRANSLATORS: The argument is the list of ambiguous
587+
* objects composed in show_ambiguous_object(). See
588+
* its "TRANSLATORS" comments for details.
589+
*/
590+
advise(_("The candidates are:\n%s"), out.advice.buf);
591+
497592
oid_array_clear(&collect);
593+
strbuf_release(&out.advice);
594+
strbuf_release(&out.sb);
498595
}
499596

500597
return status;

‎t/t1512-rev-parse-disambiguation.sh

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,87 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
2525

2626
. ./test-lib.sh
2727

28+
test_cmp_failed_rev_parse () {
29+
dir=$1
30+
rev=$2
31+
32+
cat >expect &&
33+
test_must_fail git -C "$dir" rev-parse "$rev" 2>actual.raw &&
34+
sed "s/\($rev\)[0-9a-f]*/\1.../" <actual.raw >actual &&
35+
test_cmp expect actual
36+
}
37+
38+
test_expect_success 'ambiguous blob output' '
39+
git init --bare blob.prefix &&
40+
(
41+
cd blob.prefix &&
42+
43+
# Both start with "dead..", under both SHA-1 and SHA-256
44+
echo brocdnra | git hash-object -w --stdin &&
45+
echo brigddsv | git hash-object -w --stdin &&
46+
47+
# Both start with "beef.."
48+
echo 1agllotbh | git hash-object -w --stdin &&
49+
echo 1bbfctrkc | git hash-object -w --stdin
50+
) &&
51+
52+
test_must_fail git -C blob.prefix rev-parse dead &&
53+
test_cmp_failed_rev_parse blob.prefix beef <<-\EOF
54+
error: short object ID beef... is ambiguous
55+
hint: The candidates are:
56+
hint: beef... blob
57+
hint: beef... blob
58+
fatal: ambiguous argument '\''beef...'\'': unknown revision or path not in the working tree.
59+
Use '\''--'\'' to separate paths from revisions, like this:
60+
'\''git <command> [<revision>...] -- [<file>...]'\''
61+
EOF
62+
'
63+
64+
test_expect_success 'ambiguous loose bad object parsed as OBJ_BAD' '
65+
git init --bare blob.bad &&
66+
(
67+
cd blob.bad &&
68+
69+
# Both have the prefix "bad0"
70+
echo xyzfaowcoh | git hash-object -t bad -w --stdin --literally &&
71+
echo xyzhjpyvwl | git hash-object -t bad -w --stdin --literally
72+
) &&
73+
74+
test_cmp_failed_rev_parse blob.bad bad0 <<-\EOF
75+
error: short object ID bad0... is ambiguous
76+
fatal: invalid object type
77+
EOF
78+
'
79+
80+
test_expect_success POSIXPERM 'ambigous zlib corrupt loose blob' '
81+
git init --bare blob.corrupt &&
82+
(
83+
cd blob.corrupt &&
84+
85+
# Both have the prefix "cafe"
86+
echo bnkxmdwz | git hash-object -w --stdin &&
87+
oid=$(echo bmwsjxzi | git hash-object -w --stdin) &&
88+
89+
oidf=objects/$(test_oid_to_path "$oid") &&
90+
chmod 755 $oidf &&
91+
echo broken >$oidf
92+
) &&
93+
94+
test_cmp_failed_rev_parse blob.corrupt cafe <<-\EOF
95+
error: short object ID cafe... is ambiguous
96+
error: inflate: data stream error (incorrect header check)
97+
error: unable to unpack cafe... header
98+
error: inflate: data stream error (incorrect header check)
99+
error: unable to unpack cafe... header
100+
hint: The candidates are:
101+
hint: cafe... [bad object]
102+
hint: cafe... blob
103+
fatal: ambiguous argument '\''cafe...'\'': unknown revision or path not in the working tree.
104+
Use '\''--'\'' to separate paths from revisions, like this:
105+
'\''git <command> [<revision>...] -- [<file>...]'\''
106+
EOF
107+
'
108+
28109
if ! test_have_prereq SHA1
29110
then
30111
skip_all='not using SHA-1 for objects'

0 commit comments

Comments
 (0)