cat-file: use GET_OID_ONLY_TO_DIE in --(textconv|filters)

Change the cat_one_file() logic that calls get_oid_with_context() under --textconv and --filters to use the GET_OID_ONLY_TO_DIE flag, thus improving the error messaging emitted when e.g. <path> is missing but <rev> is not. To service the "cat-file" use-case we need to introduce a new "GET_OID_REQUIRE_PATH" flag, otherwise it would exit early as soon as a valid "HEAD" was resolved, but in the "cat-file" case being changed we always need a valid revision and path. This arguably makes the "<bad rev>:<bad path>" and "<bad rev>:<good (in HEAD) path>" use cases worse, as we won't quote the <path> component at the user anymore, but let's just use the existing logic "git log" et al use for now. We can improve the messaging for those cases as a follow-up for all callers. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Dec 28, 2021 at 14:28 UTC 245b948815048821e73edc1a2b4224eaa94a8fc0
4 files changed +14 -14
builtin/cat-file.c
+6 -10
@@ -73,14 +73,17 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
73 struct object_info oi = OBJECT_INFO_INIT;
74 struct strbuf sb = STRBUF_INIT;
75 unsigned flags = OBJECT_INFO_LOOKUP_REPLACE;
76 + unsigned get_oid_flags = GET_OID_RECORD_PATH | GET_OID_ONLY_TO_DIE;
77 const char *path = force_path;
78 + const int opt_cw = (opt == 'c' || opt == 'w');
79 + if (!path && opt_cw)
80 + get_oid_flags |= GET_OID_REQUIRE_PATH;
81
82 if (unknown_type)
83 flags |= OBJECT_INFO_ALLOW_UNKNOWN_TYPE;
84
81 - if (get_oid_with_context(the_repository, obj_name,
82 - GET_OID_RECORD_PATH,
83 - &oid, &obj_context))
85 + if (get_oid_with_context(the_repository, obj_name, get_oid_flags, &oid,
86 + &obj_context))
87 die("Not a valid object name %s", obj_name);
88
89 if (!path)
@@ -112,9 +115,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
115 return !has_object_file(&oid);
116
117 case 'w':
115 - if (!path)
116 - die("git cat-file --filters %s: <object> must be "
117 - "<sha1:path>", obj_name);
118
119 if (filter_object(path, obj_context.mode,
120 &oid, &buf, &size))
@@ -122,10 +122,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
122 break;
123
124 case 'c':
125 - if (!path)
126 - die("git cat-file --textconv %s: <object> must be <sha1:path>",
127 - obj_name);
128 -
125 if (textconv_object(the_repository, path, obj_context.mode,
126 &oid, 1, &buf, &size))
127 break;
cache.h
+1
@@ -1366,6 +1366,7 @@ struct object_context {
1366 #define GET_OID_FOLLOW_SYMLINKS 0100
1367 #define GET_OID_RECORD_PATH 0200
1368 #define GET_OID_ONLY_TO_DIE 04000
1369 +#define GET_OID_REQUIRE_PATH 010000
1370
1371 #define GET_OID_DISAMBIGUATORS \
1372 (GET_OID_COMMIT | GET_OID_COMMITTISH | \
object-name.c
+3
@@ -1799,6 +1799,9 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,
1799 oc->mode = S_IFINVALID;
1800 strbuf_init(&oc->symlink_path, 0);
1801 ret = get_oid_1(repo, name, namelen, oid, flags);
1802 + if (!ret && flags & GET_OID_REQUIRE_PATH)
1803 + die(_("<object>:<path> required, only <object> '%s' given"),
1804 + name);
1805 if (!ret)
1806 return ret;
1807 /*
t/t8007-cat-file-textconv.sh
+4 -4
@@ -29,7 +29,7 @@ test_expect_success 'usage: <bad rev>' '
29
30 test_expect_success 'usage: <bad rev>:<bad path>' '
31 cat >expect <<-\EOF &&
32 - fatal: Not a valid object name HEAD2:two.bin
32 + fatal: invalid object name '\''HEAD2'\''.
33 EOF
34 test_must_fail git cat-file --textconv HEAD2:two.bin 2>actual &&
35 test_cmp expect actual
@@ -37,7 +37,7 @@ test_expect_success 'usage: <bad rev>:<bad path>' '
37
38 test_expect_success 'usage: <rev>:<bad path>' '
39 cat >expect <<-\EOF &&
40 - fatal: Not a valid object name HEAD:two.bin
40 + fatal: path '\''two.bin'\'' does not exist in '\''HEAD'\''
41 EOF
42 test_must_fail git cat-file --textconv HEAD:two.bin 2>actual &&
43 test_cmp expect actual
@@ -46,7 +46,7 @@ test_expect_success 'usage: <rev>:<bad path>' '
46
47 test_expect_success 'usage: <rev> with no <path>' '
48 cat >expect <<-\EOF &&
49 - fatal: git cat-file --textconv HEAD: <object> must be <sha1:path>
49 + fatal: <object>:<path> required, only <object> '\''HEAD'\'' given
50 EOF
51 test_must_fail git cat-file --textconv HEAD 2>actual &&
52 test_cmp expect actual
@@ -55,7 +55,7 @@ test_expect_success 'usage: <rev> with no <path>' '
55
56 test_expect_success 'usage: <bad rev>:<good (in HEAD) path>' '
57 cat >expect <<-\EOF &&
58 - fatal: Not a valid object name HEAD2:one.bin
58 + fatal: invalid object name '\''HEAD2'\''.
59 EOF
60 test_must_fail git cat-file --textconv HEAD2:one.bin 2>actual &&
61 test_cmp expect actual