{"thread":{"id":"61093","subject":"[PATCH 0/3] cat-file: add %(objectmode) avoid verifying submodules' OIDs","startedAt":"2024-03-11T18:56:05Z","lastAt":"2024-03-13T21:23:39Z","messageCount":15,"participants":["Johannes Schindelin via GitGitGadget","Victoria Dye via GitGitGadget","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"490383","messageId":"pull.1689.git.1710183362.gitgitgadget@gmail.com","threadId":"61093","inReplyTo":null,"subject":"[PATCH 0/3] cat-file: add %(objectmode) avoid verifying submodules' OIDs","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-03-11T18:55:59Z","receivedAt":"2024-03-11T18:56:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The cat-file --batch command is very valuable in server settings, but so far\nit is missing a bit of functionality that would come in handy there.\n\nFor example, it is sometimes necessary to determine the object mode of a\nbatch of tree objects' children.\n\nThis came up in $dayjob recently, and applies cleanly to v2.44.0.\n\nJohannes Schindelin (1):\n  cat-file: avoid verifying submodules' OIDs\n\nVictoria Dye (2):\n  t1006: update 'run_tests' to test generic object specifiers\n  cat-file: add %(objectmode) atom\n\n Documentation/git-cat-file.txt | 10 +++++\n builtin/cat-file.c             | 41 ++++++++++++++----\n t/t1006-cat-file.sh            | 79 +++++++++++++++++++++-------------\n 3 files changed, 92 insertions(+), 38 deletions(-)\n\n\nbase-commit: 3c2a3fdc388747b9eaf4a4a4f2035c1c9ddb26d0\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1689%2Fdscho%2Fcat-file-vs-submodules-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1689/dscho/cat-file-vs-submodules-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1689\n-- \ngitgitgadget\n"},{"id":"490384","messageId":"765d7240311e145b43bbe40f00436f3854a4a177.1710183362.git.gitgitgadget@gmail.com","threadId":"61093","inReplyTo":"pull.1689.git.1710183362.gitgitgadget@gmail.com","subject":"[PATCH 1/3] t1006: update 'run_tests' to test generic object specifiers","fromName":"Victoria Dye via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-03-11T18:56:00Z","receivedAt":"2024-03-11T18:56:06Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"From: Victoria Dye <vdye@github.com>\n\nUpdate the 'run_tests' test wrapper so that the first argument may refer to\nany specifier that uniquely identifies an object (e.g. a ref name,\n'<OID>:<path>', '<OID>^{<type>}', etc.), rather than only a full object ID.\nAlso, add a test that uses a non-OID identifier, ensuring appropriate\nparsing in 'cat-file'.\n\nSigned-off-by: Victoria Dye <vdye@github.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/t1006-cat-file.sh | 46 +++++++++++++++++++++++----------------------\n 1 file changed, 24 insertions(+), 22 deletions(-)\n\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex e0c6482797e..ac1f754ee32 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -112,65 +112,66 @@ strlen () {\n \n run_tests () {\n     type=$1\n-    sha1=$2\n+    object_name=$2\n+    oid=$(git rev-parse --verify $object_name)\n     size=$3\n     content=$4\n     pretty_content=$5\n \n-    batch_output=\"$sha1 $type $size\n+    batch_output=\"$oid $type $size\n $content\"\n \n     test_expect_success \"$type exists\" '\n-\tgit cat-file -e $sha1\n+\tgit cat-file -e $object_name\n     '\n \n     test_expect_success \"Type of $type is correct\" '\n \techo $type >expect &&\n-\tgit cat-file -t $sha1 >actual &&\n+\tgit cat-file -t $object_name >actual &&\n \ttest_cmp expect actual\n     '\n \n     test_expect_success \"Size of $type is correct\" '\n \techo $size >expect &&\n-\tgit cat-file -s $sha1 >actual &&\n+\tgit cat-file -s $object_name >actual &&\n \ttest_cmp expect actual\n     '\n \n     test_expect_success \"Type of $type is correct using --allow-unknown-type\" '\n \techo $type >expect &&\n-\tgit cat-file -t --allow-unknown-type $sha1 >actual &&\n+\tgit cat-file -t --allow-unknown-type $object_name >actual &&\n \ttest_cmp expect actual\n     '\n \n     test_expect_success \"Size of $type is correct using --allow-unknown-type\" '\n \techo $size >expect &&\n-\tgit cat-file -s --allow-unknown-type $sha1 >actual &&\n+\tgit cat-file -s --allow-unknown-type $object_name >actual &&\n \ttest_cmp expect actual\n     '\n \n     test -z \"$content\" ||\n     test_expect_success \"Content of $type is correct\" '\n \techo_without_newline \"$content\" >expect &&\n-\tgit cat-file $type $sha1 >actual &&\n+\tgit cat-file $type $object_name >actual &&\n \ttest_cmp expect actual\n     '\n \n     test_expect_success \"Pretty content of $type is correct\" '\n \techo_without_newline \"$pretty_content\" >expect &&\n-\tgit cat-file -p $sha1 >actual &&\n+\tgit cat-file -p $object_name >actual &&\n \ttest_cmp expect actual\n     '\n \n     test -z \"$content\" ||\n     test_expect_success \"--batch output of $type is correct\" '\n \techo \"$batch_output\" >expect &&\n-\techo $sha1 | git cat-file --batch >actual &&\n+\techo $object_name | git cat-file --batch >actual &&\n \ttest_cmp expect actual\n     '\n \n     test_expect_success \"--batch-check output of $type is correct\" '\n-\techo \"$sha1 $type $size\" >expect &&\n-\techo_without_newline $sha1 | git cat-file --batch-check >actual &&\n+\techo \"$oid $type $size\" >expect &&\n+\techo_without_newline $object_name | git cat-file --batch-check >actual &&\n \ttest_cmp expect actual\n     '\n \n@@ -179,33 +180,33 @@ $content\"\n \ttest -z \"$content\" ||\n \t\ttest_expect_success \"--batch-command $opt output of $type content is correct\" '\n \t\techo \"$batch_output\" >expect &&\n-\t\ttest_write_lines \"contents $sha1\" | git cat-file --batch-command $opt >actual &&\n+\t\ttest_write_lines \"contents $object_name\" | git cat-file --batch-command $opt >actual &&\n \t\ttest_cmp expect actual\n \t'\n \n \ttest_expect_success \"--batch-command $opt output of $type info is correct\" '\n-\t\techo \"$sha1 $type $size\" >expect &&\n-\t\ttest_write_lines \"info $sha1\" |\n+\t\techo \"$oid $type $size\" >expect &&\n+\t\ttest_write_lines \"info $object_name\" |\n \t\tgit cat-file --batch-command $opt >actual &&\n \t\ttest_cmp expect actual\n \t'\n     done\n \n     test_expect_success \"custom --batch-check format\" '\n-\techo \"$type $sha1\" >expect &&\n-\techo $sha1 | git cat-file --batch-check=\"%(objecttype) %(objectname)\" >actual &&\n+\techo \"$type $oid\" >expect &&\n+\techo $object_name | git cat-file --batch-check=\"%(objecttype) %(objectname)\" >actual &&\n \ttest_cmp expect actual\n     '\n \n     test_expect_success \"custom --batch-command format\" '\n-\techo \"$type $sha1\" >expect &&\n-\techo \"info $sha1\" | git cat-file --batch-command=\"%(objecttype) %(objectname)\" >actual &&\n+\techo \"$type $oid\" >expect &&\n+\techo \"info $object_name\" | git cat-file --batch-command=\"%(objecttype) %(objectname)\" >actual &&\n \ttest_cmp expect actual\n     '\n \n     test_expect_success '--batch-check with %(rest)' '\n \techo \"$type this is some extra content\" >expect &&\n-\techo \"$sha1    this is some extra content\" |\n+\techo \"$object_name    this is some extra content\" |\n \t\tgit cat-file --batch-check=\"%(objecttype) %(rest)\" >actual &&\n \ttest_cmp expect actual\n     '\n@@ -216,7 +217,7 @@ $content\"\n \t\techo \"$size\" &&\n \t\techo \"$content\"\n \t} >expect &&\n-\techo $sha1 | git cat-file --batch=\"%(objectsize)\" >actual &&\n+\techo $object_name | git cat-file --batch=\"%(objectsize)\" >actual &&\n \ttest_cmp expect actual\n     '\n \n@@ -226,7 +227,7 @@ $content\"\n \t\techo \"$type\" &&\n \t\techo \"$content\"\n \t} >expect &&\n-\techo $sha1 | git cat-file --batch=\"%(objecttype)\" >actual &&\n+\techo $object_name | git cat-file --batch=\"%(objecttype)\" >actual &&\n \ttest_cmp expect actual\n     '\n }\n@@ -271,6 +272,7 @@ tree_size=$(($(test_oid rawsz) + 13))\n tree_pretty_content=\"100644 blob $hello_sha1\thello${LF}\"\n \n run_tests 'tree' $tree_sha1 $tree_size \"\" \"$tree_pretty_content\"\n+run_tests 'blob' \"$tree_sha1:hello\" $hello_size \"\" \"$hello_content\"\n \n commit_message=\"Initial commit\"\n commit_sha1=$(echo_without_newline \"$commit_message\" | git commit-tree $tree_sha1)\n-- \ngitgitgadget\n\n"},{"id":"490385","messageId":"b836bc64ddc06069d1722ae89ca049e9dfce7eec.1710183362.git.gitgitgadget@gmail.com","threadId":"61093","inReplyTo":"pull.1689.git.1710183362.gitgitgadget@gmail.com","subject":"[PATCH 2/3] cat-file: add %(objectmode) atom","fromName":"Victoria Dye via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-03-11T18:56:01Z","receivedAt":"2024-03-11T18:56:07Z","isPatch":true,"sender":{"key":"vdye@github.com","avatar":"https://avatars.githubusercontent.com/u/3619353?v=4"},"body":"From: Victoria Dye <vdye@github.com>\n\nAdd a formatting atom, used with the --batch-check/--batch-command options,\nthat prints the octal representation of the object mode if a given revision\nincludes that information, e.g. one that follows the format\n<tree-ish>:<path>. If the mode information does not exist, an empty string\nis printed instead.\n\nSigned-off-by: Victoria Dye <vdye@github.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n Documentation/git-cat-file.txt |  5 +++++\n builtin/cat-file.c             |  9 +++++++--\n t/t1006-cat-file.sh            | 23 +++++++++++++++--------\n 3 files changed, 27 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex bd95a6c10a7..de29e6d79d9 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -292,6 +292,11 @@ newline. The available atoms are:\n `objecttype`::\n \tThe type of the object (the same as `cat-file -t` reports).\n \n+`objectmode`::\n+\tIf the specified object has mode information (such as a tree or\n+\tindex entry), the mode expressed as an octal integer. Otherwise,\n+\tempty string.\n+\n `objectsize`::\n \tThe size, in bytes, of the object (the same as `cat-file -s`\n \treports).\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex bbf851138ec..73bd78c0b63 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -272,6 +272,7 @@ struct expand_data {\n \tstruct object_id oid;\n \tenum object_type type;\n \tunsigned long size;\n+\tunsigned short mode;\n \toff_t disk_size;\n \tconst char *rest;\n \tstruct object_id delta_base_oid;\n@@ -303,6 +304,7 @@ struct expand_data {\n \t */\n \tunsigned skip_object_info : 1;\n };\n+#define EXPAND_DATA_INIT  { .mode = S_IFINVALID }\n \n static int is_atom(const char *atom, const char *s, int slen)\n {\n@@ -342,6 +344,9 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len,\n \t\telse\n \t\t\tstrbuf_addstr(sb,\n \t\t\t\t      oid_to_hex(&data->delta_base_oid));\n+\t} else if (is_atom(\"objectmode\", atom, len)) {\n+\t\tif (!data->mark_query && !(S_IFINVALID == data->mode))\n+\t\t\tstrbuf_addf(sb, \"%06o\", data->mode);\n \t} else\n \t\tdie(\"unknown format element: %.*s\", len, atom);\n }\n@@ -562,6 +567,7 @@ static void batch_one_object(const char *obj_name,\n \t\treturn;\n \t}\n \n+\tdata->mode = ctx.mode;\n \tbatch_object_write(obj_name, scratch, opt, data, NULL, 0);\n }\n \n@@ -766,7 +772,7 @@ static int batch_objects(struct batch_options *opt)\n {\n \tstruct strbuf input = STRBUF_INIT;\n \tstruct strbuf output = STRBUF_INIT;\n-\tstruct expand_data data;\n+\tstruct expand_data data = EXPAND_DATA_INIT;\n \tint save_warning;\n \tint retval = 0;\n \n@@ -775,7 +781,6 @@ static int batch_objects(struct batch_options *opt)\n \t * object_info to be handed to oid_object_info_extended for each\n \t * object.\n \t */\n-\tmemset(&data, 0, sizeof(data));\n \tdata.mark_query = 1;\n \texpand_format(&output,\n \t\t      opt->format ? opt->format : DEFAULT_FORMAT,\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex ac1f754ee32..6f25cc20ec6 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -114,9 +114,10 @@ run_tests () {\n     type=$1\n     object_name=$2\n     oid=$(git rev-parse --verify $object_name)\n-    size=$3\n-    content=$4\n-    pretty_content=$5\n+    mode=$3\n+    size=$4\n+    content=$5\n+    pretty_content=$6\n \n     batch_output=\"$oid $type $size\n $content\"\n@@ -211,6 +212,12 @@ $content\"\n \ttest_cmp expect actual\n     '\n \n+    test_expect_success '--batch-check with %(objectmode)' '\n+\techo \"$mode $oid\" >expect &&\n+\techo $object_name | git cat-file --batch-check=\"%(objectmode) %(objectname)\" >actual &&\n+\ttest_cmp expect actual\n+    '\n+\n     test -z \"$content\" ||\n     test_expect_success \"--batch without type ($type)\" '\n \t{\n@@ -241,7 +248,7 @@ test_expect_success \"setup\" '\n \tgit update-index --add hello\n '\n \n-run_tests 'blob' $hello_sha1 $hello_size \"$hello_content\" \"$hello_content\"\n+run_tests 'blob' $hello_sha1 \"\" $hello_size \"$hello_content\" \"$hello_content\"\n \n test_expect_success '--batch-command --buffer with flush for blob info' '\n \techo \"$hello_sha1 blob $hello_size\" >expect &&\n@@ -271,8 +278,8 @@ tree_sha1=$(git write-tree)\n tree_size=$(($(test_oid rawsz) + 13))\n tree_pretty_content=\"100644 blob $hello_sha1\thello${LF}\"\n \n-run_tests 'tree' $tree_sha1 $tree_size \"\" \"$tree_pretty_content\"\n-run_tests 'blob' \"$tree_sha1:hello\" $hello_size \"\" \"$hello_content\"\n+run_tests 'tree' $tree_sha1 \"\" $tree_size \"\" \"$tree_pretty_content\"\n+run_tests 'blob' \"$tree_sha1:hello\" \"100644\" $hello_size \"\" \"$hello_content\"\n \n commit_message=\"Initial commit\"\n commit_sha1=$(echo_without_newline \"$commit_message\" | git commit-tree $tree_sha1)\n@@ -283,7 +290,7 @@ committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE\n \n $commit_message\"\n \n-run_tests 'commit' $commit_sha1 $commit_size \"$commit_content\" \"$commit_content\"\n+run_tests 'commit' $commit_sha1 \"\" $commit_size \"$commit_content\" \"$commit_content\"\n \n tag_header_without_timestamp=\"object $hello_sha1\n type blob\n@@ -297,7 +304,7 @@ $tag_description\"\n tag_sha1=$(echo_without_newline \"$tag_content\" | git hash-object -t tag --stdin -w)\n tag_size=$(strlen \"$tag_content\")\n \n-run_tests 'tag' $tag_sha1 $tag_size \"$tag_content\" \"$tag_content\"\n+run_tests 'tag' $tag_sha1 \"\" $tag_size \"$tag_content\" \"$tag_content\"\n \n test_expect_success \"Reach a blob from a tag pointing to it\" '\n \techo_without_newline \"$hello_content\" >expect &&\n-- \ngitgitgadget\n\n"},{"id":"490386","messageId":"951f73397c15f76da75bbd74a02f36da0116623f.1710183362.git.gitgitgadget@gmail.com","threadId":"61093","inReplyTo":"pull.1689.git.1710183362.gitgitgadget@gmail.com","subject":"[PATCH 3/3] cat-file: avoid verifying submodules' OIDs","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-03-11T18:56:02Z","receivedAt":"2024-03-11T18:56:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nSubmodules are strange creatures. They have OIDs, but the corresponding\nobjects are not expected to be present in the current directory.\n\nLet's teach `cat-file` about this: This command should not even attempt\nto look up those objects, let alone declare them \"missing\".\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n Documentation/git-cat-file.txt |  5 +++++\n builtin/cat-file.c             | 32 ++++++++++++++++++++++++++------\n t/t1006-cat-file.sh            | 10 ++++++++++\n 3 files changed, 41 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\nindex de29e6d79d9..69b50d2042f 100644\n--- a/Documentation/git-cat-file.txt\n+++ b/Documentation/git-cat-file.txt\n@@ -412,6 +412,11 @@ Note also that multiple copies of an object may be present in the object\n database; in this case, it is undefined which copy's size or delta base\n will be reported.\n \n+Submodules are handled specially in `git cat-file`, as the objects\n+corresponding to the recorded OIDs are not expected to be present in the\n+current repository. For that reason, submodules are reported as having\n+type `submodule` and mode 1600000 and all other fields are zeroed out.\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 73bd78c0b63..c59ad682d1f 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -128,7 +128,9 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \tswitch (opt) {\n \tcase 't':\n \t\toi.type_name = &sb;\n-\t\tif (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n+\t\tif (obj_context.mode == S_IFGITLINK)\n+\t\t\tstrbuf_addstr(&sb, \"submodule\");\n+\t\telse if (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n \t\t\tdie(\"git cat-file: could not get object info\");\n \t\tif (sb.len) {\n \t\t\tprintf(\"%s\\n\", sb.buf);\n@@ -319,17 +321,26 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len,\n \t\tif (!data->mark_query)\n \t\t\tstrbuf_addstr(sb, oid_to_hex(&data->oid));\n \t} else if (is_atom(\"objecttype\", atom, len)) {\n-\t\tif (data->mark_query)\n+\t\tif (data->mode == S_IFGITLINK) {\n+\t\t\tif (!data->mark_query)\n+\t\t\t\tstrbuf_addstr(sb, \"submodule\");\n+\t\t} else if (data->mark_query)\n \t\t\tdata->info.typep = &data->type;\n \t\telse\n \t\t\tstrbuf_addstr(sb, type_name(data->type));\n \t} else if (is_atom(\"objectsize\", atom, len)) {\n-\t\tif (data->mark_query)\n+\t\tif (data->mode == S_IFGITLINK) {\n+\t\t\tif (!data->mark_query)\n+\t\t\t\tstrbuf_addstr(sb, \"0\");\n+\t\t} else if (data->mark_query)\n \t\t\tdata->info.sizep = &data->size;\n \t\telse\n \t\t\tstrbuf_addf(sb, \"%\"PRIuMAX , (uintmax_t)data->size);\n \t} else if (is_atom(\"objectsize:disk\", atom, len)) {\n-\t\tif (data->mark_query)\n+\t\tif (data->mode == S_IFGITLINK) {\n+\t\t\tif (!data->mark_query)\n+\t\t\t\tstrbuf_addstr(sb, \"0\");\n+\t\t} else if (data->mark_query)\n \t\t\tdata->info.disk_sizep = &data->disk_size;\n \t\telse\n \t\t\tstrbuf_addf(sb, \"%\"PRIuMAX, (uintmax_t)data->disk_size);\n@@ -448,7 +459,8 @@ static void print_default_format(struct strbuf *scratch, struct expand_data *dat\n \t\t\t\t struct batch_options *opt)\n {\n \tstrbuf_addf(scratch, \"%s %s %\"PRIuMAX\"%c\", oid_to_hex(&data->oid),\n-\t\t    type_name(data->type),\n+\t\t    data->mode == S_IFGITLINK ?\n+\t\t    \"submodule\" : type_name(data->type),\n \t\t    (uintmax_t)data->size, opt->output_delim);\n }\n \n@@ -470,7 +482,15 @@ static void batch_object_write(const char *obj_name,\n \t\tif (use_mailmap)\n \t\t\tdata->info.typep = &data->type;\n \n-\t\tif (pack)\n+\t\tif (data->mode == S_IFGITLINK) {\n+\t\t\tdata->type = OBJ_BAD; /* `type_name()` does not know submodules */\n+\t\t\tdata->size = 0;\n+\t\t\tdata->disk_size = 0;\n+\t\t\tdata->rest = NULL;\n+\t\t\toidcpy(&data->delta_base_oid, null_oid());\n+\t\t\tmemset(&data->info, 0, sizeof(data->info));\n+\t\t\tret = 0; /* no info to look up */\n+\t\t} else if (pack)\n \t\t\tret = packed_object_info(the_repository, pack, offset,\n \t\t\t\t\t\t &data->info);\n \t\telse\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex 6f25cc20ec6..3368b663ef3 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -1178,6 +1178,16 @@ test_expect_success 'cat-file --batch-check respects replace objects' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'batch-command with a submodule' '\n+\tprintf \"160000 commit %0.*d\\tsub\\n\" $(test_oid hexsz) 17 >tree-with-sub &&\n+\ttree=$(git mktree <tree-with-sub) &&\n+\tgit cat-file --batch-check >actual <<-EOF &&\n+\t$tree:sub\n+\tEOF\n+\tprintf \"%0.*d submodule 0\\n\" $(test_oid hexsz) 17 >expect &&\n+\ttest_cmp expect actual\n+'\n+\n # Pull the entry for object with oid \"$1\" out of the output of\n # \"cat-file --batch\", including its object content (which requires\n # parsing and reading a set amount of bytes, hence perl).\n-- \ngitgitgadget\n"},{"id":"490405","messageId":"xmqqwmq8o36j.fsf@gitster.g","threadId":"61093","inReplyTo":"pull.1689.git.1710183362.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] cat-file: add %(objectmode) avoid verifying submodules' OIDs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-11T21:43:00Z","receivedAt":"2024-03-11T21:43:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> The cat-file --batch command is very valuable in server settings, but so far\n> it is missing a bit of functionality that would come in handy there.\n>\n> For example, it is sometimes necessary to determine the object mode of a\n> batch of tree objects' children.\n\nOK.\n\nIt is somewhat unsatisfying that --batch/--batch-check lacks so\nmuch.  Even with %(objectmode) its nature of one-object-at-a-time\nmakes querying children of a large tree a chore, when you compare it\nwith something like \"cat-file -p HEAD:\" that allows you to grab the\nneeded information for all children with a single invocation.\n\nThis is orthogonal to what the patch wants to do, which is to enrich\nthe output side with more formatting, bit I wonder if we want to\nconsider enriching the input side?  e.g. instead of feeding just a\nsingle object name from the standard input of \"cat-file\n--batch/--batch-check\", perhaps a syntax can say \"Here I have the\nobject name for a tree-ish object, but please pretend that I gave\nyou all the objects contained within it\", or something?\n\nThanks, will queue.\n"},{"id":"490413","messageId":"xmqqle6oo2ns.fsf@gitster.g","threadId":"61093","inReplyTo":"765d7240311e145b43bbe40f00436f3854a4a177.1710183362.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] t1006: update 'run_tests' to test generic object specifiers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-11T21:54:15Z","receivedAt":"2024-03-11T21:54:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Victoria Dye via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Victoria Dye <vdye@github.com>\n>\n> Update the 'run_tests' test wrapper so that the first argument may refer to\n> any specifier that uniquely identifies an object (e.g. a ref name,\n> '<OID>:<path>', '<OID>^{<type>}', etc.), rather than only a full object ID.\n> Also, add a test that uses a non-OID identifier, ensuring appropriate\n> parsing in 'cat-file'.\n>\n> Signed-off-by: Victoria Dye <vdye@github.com>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>  t/t1006-cat-file.sh | 46 +++++++++++++++++++++++----------------------\n>  1 file changed, 24 insertions(+), 22 deletions(-)\n>\n> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n> index e0c6482797e..ac1f754ee32 100755\n> --- a/t/t1006-cat-file.sh\n> +++ b/t/t1006-cat-file.sh\n> @@ -112,65 +112,66 @@ strlen () {\n>  \n>  run_tests () {\n>      type=$1\n> -    sha1=$2\n> +    object_name=$2\n> +    oid=$(git rev-parse --verify $object_name)\n>      size=$3\n>      content=$4\n>      pretty_content=$5\n>  \n> -    batch_output=\"$sha1 $type $size\n> +    batch_output=\"$oid $type $size\n>  $content\"\n\nAs \"object_name\" is now allowed to be any name in the 'extended\nSHA-1' syntax (cf. Documentation/revisions.txt), you should be a bit\nmore careful in quoting.\n\n\toid=$(git rev-parse --verify \"$object_name\")\n\n>      test_expect_success \"$type exists\" '\n> -\tgit cat-file -e $sha1\n> +\tgit cat-file -e $object_name\n>      '\n\nLikewise.  You may not currently use a path with SP in it to name a\ntree object, e.g., \"HEAD:Read Me.txt\", but protecting against such a\npathname is a cheap investment for futureproofing.\n\nLooking good otherwise.  Thanks.\n"},{"id":"490414","messageId":"xmqq34swo1p2.fsf@gitster.g","threadId":"61093","inReplyTo":"b836bc64ddc06069d1722ae89ca049e9dfce7eec.1710183362.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] cat-file: add %(objectmode) atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-11T22:15:05Z","receivedAt":"2024-03-11T22:15:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Victoria Dye via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index bbf851138ec..73bd78c0b63 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -272,6 +272,7 @@ struct expand_data {\n>  \tstruct object_id oid;\n>  \tenum object_type type;\n>  \tunsigned long size;\n> +\tunsigned short mode;\n>  \toff_t disk_size;\n\nWe are not saving the storage used in this structure by using\n\"unsigned short\" due to alignment, so I got curious where the choice\ncame from, but I do not think of any sensible explanation.\n\nLet's to be consistent with the remainder of the system, like how\nthe mode is stored in the in-core index (ce_mode) and in the in-core\ntree entry (name_entry.mode) and use \"unsigned int\" instead here.\n\n> +#define EXPAND_DATA_INIT  { .mode = S_IFINVALID }\n\nThanks for knowing about and choosing to use the INVALID thing (I\nwould have naively chosen 0 without looking around enough and made\nthings inconsistent).\n\n> +\t} else if (is_atom(\"objectmode\", atom, len)) {\n> +\t\tif (!data->mark_query && !(S_IFINVALID == data->mode))\n> +\t\t\tstrbuf_addf(sb, \"%06o\", data->mode);\n\nNit.  I think\n\n\t\tif (!data->mark_query && data->mode != S_IFINVALID)\n\nwould be a more common way to write the same thing.\n\n> @@ -766,7 +772,7 @@ static int batch_objects(struct batch_options *opt)\n>  {\n>  \tstruct strbuf input = STRBUF_INIT;\n>  \tstruct strbuf output = STRBUF_INIT;\n> -\tstruct expand_data data;\n> +\tstruct expand_data data = EXPAND_DATA_INIT;\n>  \tint save_warning;\n>  \tint retval = 0;\n>  \n> @@ -775,7 +781,6 @@ static int batch_objects(struct batch_options *opt)\n>  \t * object_info to be handed to oid_object_info_extended for each\n>  \t * object.\n>  \t */\n> -\tmemset(&data, 0, sizeof(data));\n\nNice to see this go with the _INIT thing.\n\n> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n> index ac1f754ee32..6f25cc20ec6 100755\n> --- a/t/t1006-cat-file.sh\n> +++ b/t/t1006-cat-file.sh\n> @@ -114,9 +114,10 @@ run_tests () {\n>      type=$1\n>      object_name=$2\n>      oid=$(git rev-parse --verify $object_name)\n> -    size=$3\n> -    content=$4\n> -    pretty_content=$5\n> +    mode=$3\n> +    size=$4\n> +    content=$5\n> +    pretty_content=$6\n>  \n>      batch_output=\"$oid $type $size\n>  $content\"\n\nI wonder if appending $mode as an optional thing at the end would\nhave made the patch less noisy?  After all, the expectation above\nthat does not have $mode, and the tests that are expected to produce\noutput that match the expectation, do not have to change.  And the\nexisting invocation of run_tests that do not care about $mode do not\nhave to change.\n\nBut I guess if the damage is only with the above 7-lines (which\nwould become just 1 if we made mode the $6 last tthing), it is not a\nhuge deal either way?\n"},{"id":"490455","messageId":"20240312085854.GA94437@coredump.intra.peff.net","threadId":"61093","inReplyTo":"951f73397c15f76da75bbd74a02f36da0116623f.1710183362.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] cat-file: avoid verifying submodules' OIDs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-03-12T08:58:54Z","receivedAt":"2024-03-12T08:58:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 11, 2024 at 06:56:02PM +0000, Johannes Schindelin via GitGitGadget wrote:\n\n> diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt\n> index de29e6d79d9..69b50d2042f 100644\n> --- a/Documentation/git-cat-file.txt\n> +++ b/Documentation/git-cat-file.txt\n> @@ -412,6 +412,11 @@ Note also that multiple copies of an object may be present in the object\n>  database; in this case, it is undefined which copy's size or delta base\n>  will be reported.\n>  \n> +Submodules are handled specially in `git cat-file`, as the objects\n> +corresponding to the recorded OIDs are not expected to be present in the\n> +current repository. For that reason, submodules are reported as having\n> +type `submodule` and mode 1600000 and all other fields are zeroed out.\n\nI think there's an extra 0 in the mode here?\n\nIt may also be worth being more explicit about when Git knows something\nis a submodule. Naively, reading the above I might think that:\n\n  git ls-tree --format='%(objectname)' HEAD | git cat-file --batch-check\n\nwould do something special with submodules. But it can't, as there's no\ncontext carried in just the objectname. This is obvious if you are\nfamiliar with how Git works, but I'm not sure it would be for all end\nusers. So we could say something along the lines of:\n\n  When `cat-file` is given a name within a tree that points to a\n  submodule (e.g., `HEAD:my-submodule`), ...\n\n-Peff\n"},{"id":"490456","messageId":"20240312085901.GA69635@coredump.intra.peff.net","threadId":"61093","inReplyTo":"xmqqwmq8o36j.fsf@gitster.g","subject":"Re: [PATCH 0/3] cat-file: add %(objectmode) avoid verifying submodules' OIDs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-03-12T08:59:01Z","receivedAt":"2024-03-12T08:59:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 11, 2024 at 02:43:00PM -0700, Junio C Hamano wrote:\n\n> It is somewhat unsatisfying that --batch/--batch-check lacks so\n> much.  Even with %(objectmode) its nature of one-object-at-a-time\n> makes querying children of a large tree a chore, when you compare it\n> with something like \"cat-file -p HEAD:\" that allows you to grab the\n> needed information for all children with a single invocation.\n> \n> This is orthogonal to what the patch wants to do, which is to enrich\n> the output side with more formatting, bit I wonder if we want to\n> consider enriching the input side?  e.g. instead of feeding just a\n> single object name from the standard input of \"cat-file\n> --batch/--batch-check\", perhaps a syntax can say \"Here I have the\n> object name for a tree-ish object, but please pretend that I gave\n> you all the objects contained within it\", or something?\n\nThat is an interesting direction. In practice I guess you might want to\nexpand trees (to show their contents) or perhaps commits (to traverse\nhistory and/or look at their trees). And we already have tools to do\nthat.\n\nSo for example you can already do:\n\n  git ls-tree --format='%(objectname) %(objectmode)' HEAD\n\nOr if you wanted to mix-and-match with other cat-file placeholders, you\ncan do:\n\n  git ls-tree --format='%(objectname) %(objectmode)' HEAD |\n  git cat-file --batch-check='%(objectname) %(deltabase) %(rest)'\n\nThat is a little less efficient (we look up the object twice), but once\nyou are working with hex object ids it is not too bad (cat-file is\nheavily optimized here). Of course in the long run I think we should\nmove to a future where the formatting code is shared, and you can just\nask ls-tree for deltabase if you want to.\n\nI think leaving this to specialized tools like ls-tree gives them a lot\nof flexibility that a special input mode to cat-file might find awkward.\nFor example, recurse vs non-recursive tree listing. Or filtering with\npathspecs. And of course when you get into commits and traversal, there\nare many rev-list options. :)\n\nThe strategy so far has been making sure cat-file can efficiently take\nin the output of these other tools to further describe objects. But\nmoving towards a unified output formatting model would be even better, I\nthink. In the meantime, I think cat-file learning %(objectmode) makes\nsense for single names (rather than listing trees), and fortunately it\nuses the same (obvious) name that ls-tree does, so we won't have a\nproblem unifying them later.\n\nThe patch itself looked reasonable to me, modulo the comments you\nalready made.\n\n-Peff\n"},{"id":"490496","messageId":"xmqqh6hbl2mz.fsf@gitster.g","threadId":"61093","inReplyTo":"951f73397c15f76da75bbd74a02f36da0116623f.1710183362.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] cat-file: avoid verifying submodules' OIDs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-12T18:35:16Z","receivedAt":"2024-03-12T18:35:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> +Submodules are handled specially in `git cat-file`, as the objects\n> +corresponding to the recorded OIDs are not expected to be present in the\n> +current repository. For that reason, submodules are reported as having\n> +type `submodule` and mode 1600000 and all other fields are zeroed out.\n\nWhile the above may not be technically wrong per-se, I am not sure\nif that is the more important part of what we want to tell our\nusers.  For example, \"git ls-tree HEAD -- sha1collisiondetection\"\nreports \"160000 commit ...object.name.... sha1collisiondetection\".\nIs it correct to say ...\n\n    Submodules are handled specially in `git ls-tree`, as the\n    objects corresponding to the recorded OIDs are not expected to\n    be present in the current repository.\n\n...?  I do not think so.\n\nFor the same reason, as an explanation for the reason why \"git\ncat-file -t :sha1collisiondetection\" just reports \"submodule\", the\nnew text does not sit well.\n\nI actually have to wonder if the new behaviour proposed by this\npatch is a solution that is in search of a problem, or trying to\nsolve an unstated problem in a wrong way.\n\n    O=$(git rev-parse --verify :sha1collisiondetection)\n    git cat-file -t \"$O\"\n    \nshould fail because the object whose name is $O is not available.\nWhy should then this succeed and give a different result?\n\n    git cat-file -t :sha1collisiondetection\n\nThe \"cat-file\" command is about objects.  While object's type may\nsometimes be inferrable (by being contained in a tree), if the user\nasks us to determine the type of the object, we should actually hit\nthe object store, whether the commit object in question happens to\nbe on our history or somebody else's history that our gitlink points\nat.\n\nSo, I am not yet convinced that I should take this patch.  Previous\ntwo steps looked good, though.\n\nThanks.\n\n> index 73bd78c0b63..c59ad682d1f 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -128,7 +128,9 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n>  \tswitch (opt) {\n>  \tcase 't':\n>  \t\toi.type_name = &sb;\n> -\t\tif (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n> +\t\tif (obj_context.mode == S_IFGITLINK)\n> +\t\t\tstrbuf_addstr(&sb, \"submodule\");\n> +\t\telse if (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)\n>  \t\t\tdie(\"git cat-file: could not get object info\");\n"},{"id":"490498","messageId":"xmqq1q8fl05r.fsf@gitster.g","threadId":"61093","inReplyTo":"20240312085901.GA69635@coredump.intra.peff.net","subject":"Re: [PATCH 0/3] cat-file: add %(objectmode) avoid verifying submodules' OIDs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-12T19:28:48Z","receivedAt":"2024-03-12T19:28:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> That is an interesting direction. In practice I guess you might want to\n> expand trees (to show their contents) or perhaps commits (to traverse\n> history and/or look at their trees). And we already have tools to do\n> that.\n>\n> So for example you can already do:\n>\n>   git ls-tree --format='%(objectname) %(objectmode)' HEAD\n>\n> Or if you wanted to mix-and-match with other cat-file placeholders, you\n> can do:\n>\n>   git ls-tree --format='%(objectname) %(objectmode)' HEAD |\n>   git cat-file --batch-check='%(objectname) %(deltabase) %(rest)'\n>\n> That is a little less efficient (we look up the object twice), but once\n> you are working with hex object ids it is not too bad (cat-file is\n> heavily optimized here). Of course in the long run I think we should\n> move to a future where the formatting code is shared, and you can just\n> ask ls-tree for deltabase if you want to.\n\nI was imagining more about a use case \"cat-file --batch\" was\noriginally designed for---having a long-running single process\nand ask any and all questions you have about various objects in the\nobject database by interacting with it.  So \"yes, ls-tree can\nalready give us that information\", while it is true, shoots at a\ndifferent direction from what I had in mind.\n\n> The strategy so far has been making sure cat-file can efficiently take\n> in the output of these other tools to further describe objects. But\n> moving towards a unified output formatting model would be even better, I\n> think. In the meantime, I think cat-file learning %(objectmode) makes\n> sense for single names (rather than listing trees), and fortunately it\n> uses the same (obvious) name that ls-tree does, so we won't have a\n> problem unifying them later.\n\nYes, enriching the output format side is an orthogonal issue from\nthe input side, and the %(objectmode) thing that gives a piece of\ninformation that is additionally available on top of the various\npieces of information about the object itself does make sense.\n\n> The patch itself looked reasonable to me, modulo the comments you\n> already made.\n>\n> -Peff\n"},{"id":"490516","messageId":"20240312220341.GA106249@coredump.intra.peff.net","threadId":"61093","inReplyTo":"xmqq1q8fl05r.fsf@gitster.g","subject":"Re: [PATCH 0/3] cat-file: add %(objectmode) avoid verifying submodules' OIDs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-03-12T22:03:41Z","receivedAt":"2024-03-12T22:03:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 12, 2024 at 12:28:48PM -0700, Junio C Hamano wrote:\n\n> > Or if you wanted to mix-and-match with other cat-file placeholders, you\n> > can do:\n> >\n> >   git ls-tree --format='%(objectname) %(objectmode)' HEAD |\n> >   git cat-file --batch-check='%(objectname) %(deltabase) %(rest)'\n> >\n> > That is a little less efficient (we look up the object twice), but once\n> > you are working with hex object ids it is not too bad (cat-file is\n> > heavily optimized here). Of course in the long run I think we should\n> > move to a future where the formatting code is shared, and you can just\n> > ask ls-tree for deltabase if you want to.\n> \n> I was imagining more about a use case \"cat-file --batch\" was\n> originally designed for---having a long-running single process\n> and ask any and all questions you have about various objects in the\n> object database by interacting with it.  So \"yes, ls-tree can\n> already give us that information\", while it is true, shoots at a\n> different direction from what I had in mind.\n\nAh, yeah, that is one thing that cat-file does that no other part of the\nsystem will. I do wonder in the long term if it is easier to teach\ncat-file everything that all of the other commands can do, or to teach\nall of the other commands some way of handling multiple requests in a\nsingle process. ;)\n\n(All obviously orthogonal to this patch series).\n\n-Peff\n"},{"id":"490519","messageId":"20240312221758.GA109417@coredump.intra.peff.net","threadId":"61093","inReplyTo":"xmqqh6hbl2mz.fsf@gitster.g","subject":"Re: [PATCH 3/3] cat-file: avoid verifying submodules' OIDs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-03-12T22:17:58Z","receivedAt":"2024-03-12T22:17:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 12, 2024 at 11:35:16AM -0700, Junio C Hamano wrote:\n\n> I actually have to wonder if the new behaviour proposed by this\n> patch is a solution that is in search of a problem, or trying to\n> solve an unstated problem in a wrong way.\n> \n>     O=$(git rev-parse --verify :sha1collisiondetection)\n>     git cat-file -t \"$O\"\n>     \n> should fail because the object whose name is $O is not available.\n> Why should then this succeed and give a different result?\n> \n>     git cat-file -t :sha1collisiondetection\n> \n> The \"cat-file\" command is about objects.  While object's type may\n> sometimes be inferrable (by being contained in a tree), if the user\n> asks us to determine the type of the object, we should actually hit\n> the object store, whether the commit object in question happens to\n> be on our history or somebody else's history that our gitlink points\n> at.\n> \n> So, I am not yet convinced that I should take this patch.  Previous\n> two steps looked good, though.\n\nI'm not sure about \"-t\" in particular, but for batch output, I think if\nwe stop at patch 2 it would be impossible to tell the difference between\na submodule entry and a corrupt repo (or bad request). E.g., if I do\nthis:\n\n  (echo HEAD:Makefile; echo HEAD:sha1collisiondetection) |\n  git cat-file --batch-check='%(objectname) %(objectmode)'\n\nafter only patch 2, I'd get:\n\n  4e255c81f22386389c7460d8f5e59426673b5a5a 100644\n  HEAD:sha1collisiondetection missing\n\nWe can't tell if HEAD didn't resolve, or it doesn't have that path, or\nif it's a regular blob entry and the repository is corrupt. Whereas\nafter patch 3, we get:\n\n  4e255c81f22386389c7460d8f5e59426673b5a5a 100644\n  855827c583bc30645ba427885caa40c5b81764d2 160000\n\nand the mode tells us that we resolved it to a submodule.\n\nThe current behavior is not too surprising for cat-file, since it's\nwhole purpose is to give you information about the objects themselves,\nand we don't have one here. But with this %(objectmode) format, we're\nreally moving into a realm of \"resolve this name for me and show me the\ncontext\". We don't care about the details of the object at all!\n\nI think you could make an argument that the problem is shoe-horning new,\nslightly-mismatched functionality into cat-file. But there are lots of\npractical reasons to want to do so, as we discussed elsewhere. Since\ngitlinks are the only place where we'd expect an object to be missing,\n\"simulating\" them here isn't too bad. But I suspect there's a more\ngeneral solution where cat-file learns to print dummy values for any\nmissing object, letting the caller see what we _could_ find out. And\nthen the submodule case just falls out naturally. I doubt we could make\nit the default for historical compatibility; we'd need a new option.\n\nThis is all speculative on my part, of course. Probably Dscho or\nVictoria can explain their use case better. :)\n\n-Peff\n"},{"id":"490543","messageId":"xmqqmsr2f97b.fsf@gitster.g","threadId":"61093","inReplyTo":"20240312221758.GA109417@coredump.intra.peff.net","subject":"Re: [PATCH 3/3] cat-file: avoid verifying submodules' OIDs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-13T15:22:16Z","receivedAt":"2024-03-13T15:22:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I think you could make an argument that the problem is\n> shoe-horning new, slightly-mismatched functionality into\n> cat-file. But there are lots of practical reasons to want to do\n> so, as we discussed elsewhere.  Since gitlinks are the only place\n> where we'd expect an object to be missing, \"simulating\" them here\n> isn't too bad.\n\n100% agreed.  This is something we should be asking about \"HEAD:\"\ntree object, not about \"HEAD:sha1collisiondetection\" object, if we\nare to ask cat-file.  After all \"cat-file p HEAD:\" tells us that the\nthing is a submodule already.  But unfortunately the \"--batch\" thing\nis limited to \"give me an object and what you want to know about the\nobject, and I'll tell you what I know about it\" exchange, so it is a\nvery bad match when you cannot really give it an object (which you\ndo not have, like the target of the gitlink).  So...\n\n> But I suspect there's a more\n> general solution where cat-file learns to print dummy values for any\n> missing object, letting the caller see what we _could_ find out. And\n> then the submodule case just falls out naturally. I doubt we could make\n> it the default for historical compatibility; we'd need a new option.\n\n... \"--batch\" obviously needs to be extended, and %(objectmode) may\nbe one direction to do so, but it would also work to allow us to ask\nabout \"HEAD:\" and what it has at paths, which match a pathspec\n\"sha1collisiondetection\", an equivalent to give \"cat-file --batch\" a\ncommand to drive \"ls-tree\".\n\n> This is all speculative on my part, of course. Probably Dscho or\n> Victoria can explain their use case better. :)\n\nLikewise.\n"},{"id":"490582","messageId":"xmqqfrwtakrq.fsf@gitster.g","threadId":"61093","inReplyTo":"xmqq34swo1p2.fsf@gitster.g","subject":"Re: [PATCH 2/3] cat-file: add %(objectmode) atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-03-13T21:23:37Z","receivedAt":"2024-03-13T21:23:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n>> index ac1f754ee32..6f25cc20ec6 100755\n>> --- a/t/t1006-cat-file.sh\n>> +++ b/t/t1006-cat-file.sh\n>> @@ -114,9 +114,10 @@ run_tests () {\n>>      type=$1\n>>      object_name=$2\n>>      oid=$(git rev-parse --verify $object_name)\n>> -    size=$3\n>> -    content=$4\n>> -    pretty_content=$5\n>> +    mode=$3\n>> +    size=$4\n>> +    content=$5\n>> +    pretty_content=$6\n>>  \n>>      batch_output=\"$oid $type $size\n>>  $content\"\n>\n> I wonder if appending $mode as an optional thing at the end would\n> have made the patch less noisy?  After all, the expectation above\n> that does not have $mode, and the tests that are expected to produce\n> output that match the expectation, do not have to change.  And the\n> existing invocation of run_tests that do not care about $mode do not\n> have to change.\n>\n> But I guess if the damage is only with the above 7-lines (which\n> would become just 1 if we made mode the $6 last tthing), it is not a\n> huge deal either way?\n\nUnfortunately, not really.\n\nIf we made the optional mode as the last thing, and allow\nrun_tests() to be called without an explicit \"\", it may have avoided\nunnecessary conflicts with eb/hash-transition topic.  Interested\nfolks can see how well these three patches plays with the other\ntopic by trying to merge it to 'seen'.\n"}]}