{"thread":{"id":"63468","subject":"[PATCH 0/13] dropping support for non-standard object types","startedAt":"2025-05-16T04:49:19Z","lastAt":"2025-05-21T13:44:20Z","messageCount":35,"participants":["Jeff King","Patrick Steinhardt","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":13},"messages":[{"id":"518198","messageId":"20250516044916.GA21985@coredump.intra.peff.net","threadId":"63468","inReplyTo":null,"subject":"[PATCH 0/13] dropping support for non-standard object types","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-16T04:49:16Z","receivedAt":"2025-05-16T04:49:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"While fixing some bugs last month in c39e5cbaa5 (Merge branch\n'jk/zlib-inflate-fixes', 2025-04-15), I noted that objects with\nnon-standard types are not really usable. You can get their size and\ntype, but nothing else, not even their contents. And you can't transfer\nthem to other repositories, as packfiles have no way to represent them.\n\nWe've had that code since 2015, but beyond using it in a few tests,\nit's never gone anywhere. So I'd like to consider the whole direction a\nfailed experiment and rip it out, which simplifies some of the core\nobject code.\n\nIMHO this doesn't need to follow the breaking-change flow and wait until\nGit 3.0, because what's there is not really usable in any useful way.\nBut others may disagree.\n\nI've tried to group the patches logically:\n\n  [01/13]: object-file.h: fix typo in variable declaration\n\n    Nearby cleanup that can be taken independently.\n\n  [02/13]: cat-file: make --allow-unknown-type a noop\n  [03/13]: object-file: drop OBJECT_INFO_ALLOW_UNKNOWN_TYPE flag\n\n    This drops the user-facing reading feature, and the hairiest bits of\n    the reading code.\n\n  [04/13]: cat-file: use type enum instead of buffer for -t option\n  [05/13]: oid_object_info_convert(): stop using string for object type\n  [06/13]: fsck: stop using object_info->type_name strbuf\n  [07/13]: oid_object_info(): drop type_name strbuf\n\n    This drops the rest of the unknown-type code. The first three are\n    refactors to prepare for it, then the final one drops the code.\n    These are mostly not user-facing, though patch 6 does change some\n    fsck stderr output.\n\n    This is not strictly necessary to happen along with patches 2+3, but\n    I think the resulting code is an improvement.\n\n    All the patches after this deal with the writing side (the two are\n    conceptually independent, but of course many of the reading-side\n    tests removed by earlier commits did depend on the writing side for\n    setup).\n\n  [08/13]: t/helper: add zlib test-tool\n  [09/13]: t: add lib-loose.sh\n  [10/13]: hash-object: stop allowing unknown types\n\n    This drops the user-facing support for writing objects with\n    non-standard types. We do use that feature in the test suite (e.g.,\n    to see how fsck reacts), so there's a new helper to enable that. So\n    in a sense we are trading code removed from the object-writing\n    system and putting it in the test suite. But IMHO that is still a\n    win, because we care more about the \"production\" code in git itself.\n\n  [11/13]: hash-object: merge HASH_* and INDEX_* flags\n  [12/13]: hash-object: handle --literally with OPT_NEGBIT\n  [13/13]: object-file: drop support for writing objects with unknown types\n\n    These are some cleanups enabled by patch 10, culminating in dropping\n    write_object_file_literally().\n\n Documentation/git-cat-file.adoc     |   6 +-\n Makefile                            |   1 +\n builtin/cat-file.c                  |  31 ++--\n builtin/fsck.c                      |  13 +-\n builtin/hash-object.c               |  69 +++------\n object-file.c                       | 142 +++---------------\n object-file.h                       |  17 +--\n object-store.c                      |  17 +--\n object-store.h                      |   3 -\n packfile.c                          |   7 +-\n streaming.c                         |   2 +-\n t/helper/meson.build                |   1 +\n t/helper/test-tool.c                |   1 +\n t/helper/test-tool.h                |   1 +\n t/helper/test-zlib.c                |  62 ++++++++\n t/lib-loose.sh                      |  30 ++++\n t/t1006-cat-file.sh                 | 216 +++++++---------------------\n t/t1007-hash-object.sh              |  11 +-\n t/t1450-fsck.sh                     |  32 +----\n t/t1512-rev-parse-disambiguation.sh |   5 +-\n 20 files changed, 220 insertions(+), 447 deletions(-)\n create mode 100644 t/helper/test-zlib.c\n create mode 100644 t/lib-loose.sh\n\n-Peff\n"},{"id":"518199","messageId":"20250516044928.GA22242@coredump.intra.peff.net","threadId":"63468","inReplyTo":"20250516044916.GA21985@coredump.intra.peff.net","subject":"[PATCH 01/13] object-file.h: fix typo in variable declaration","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-16T04:49:28Z","receivedAt":"2025-05-16T04:49:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This should be \"compat\", not \"comapt\".\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n object-file.h | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/object-file.h b/object-file.h\nindex a85b2e5b49..fd715663fb 100644\n--- a/object-file.h\n+++ b/object-file.h\n@@ -180,7 +180,7 @@ enum {\n \n int write_object_file_flags(const void *buf, unsigned long len,\n \t\t\t    enum object_type type, struct object_id *oid,\n-\t\t\t    struct object_id *comapt_oid_in, unsigned flags);\n+\t\t\t    struct object_id *compat_oid_in, unsigned flags);\n static inline int write_object_file(const void *buf, unsigned long len,\n \t\t\t\t    enum object_type type, struct object_id *oid)\n {\n-- \n2.49.0.896.g93578ceaaf\n\n"},{"id":"518200","messageId":"20250516044935.GB22242@coredump.intra.peff.net","threadId":"63468","inReplyTo":"20250516044916.GA21985@coredump.intra.peff.net","subject":"[PATCH 02/13] cat-file: make --allow-unknown-type a noop","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-16T04:49:35Z","receivedAt":"2025-05-16T04:49:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The cat-file command has some minor support for handling objects with\n\"unknown\" types. I.e., strings that are not \"blob\", \"commit\", \"tree\", or\n\"tag\".\n\nIn theory this could be used for debugging or experimenting with\nextensions to Git. But in practice this support is not very useful:\n\n  1. You can get the type and size of such objects, but nothing else.\n     Not even the contents!\n\n  2. Only loose objects are supported, since packfiles use numeric ids\n     for the types, rather than strings.\n\n  3. Likewise you cannot ever transfer objects between repositories,\n     because they cannot be represented in the packfiles used for the\n     on-the-wire protocol.\n\nThe support for these unknown types complicates the object-parsing code,\nand has led to bugs such as b748ddb7a4 (unpack_loose_header(): fix\ninfinite loop on broken zlib input, 2025-02-25). So let's drop it.\n\nThe first step is to remove the user-facing parts, which are accessible\nonly via cat-file. This is technically backwards-incompatible, but given\nthe limitations listed above, these objects couldn't possibly be useful\nin any workflow.\n\nHowever, we can't just rip out the option entirely. That would hurt a\ncaller who ran:\n\n  git cat-file -t --allow-unknown-object <oid>\n\nand fed it normal, well-formed objects. There --allow-unknown-type was\ndoing nothing, but we wouldn't want to start bailing with an error. So\nto protect any such callers, we'll retain --allow-unknown-type as a\nnoop.\n\nThe code change is fairly small (but we'll able to clean up more code in\nfollow-on patches). The test updates drop any use of the option. We\nstill retain tests that feed the broken objects to cat-file without\n--allow-unknown-type, as we should continue to confirm that those\nobjects are rejected. Note that in one spot we can drop a layer of loop,\nre-indenting the body; viewing the diff with \"-w\" helps there.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/git-cat-file.adoc |   6 +-\n builtin/cat-file.c              |  18 +--\n t/t1006-cat-file.sh             | 211 ++++++++------------------------\n 3 files changed, 56 insertions(+), 179 deletions(-)\n\ndiff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc\nindex fc4b92f104..cde79ad242 100644\n--- a/Documentation/git-cat-file.adoc\n+++ b/Documentation/git-cat-file.adoc\n@@ -9,8 +9,7 @@ SYNOPSIS\n --------\n [verse]\n 'git cat-file' <type> <object>\n-'git cat-file' (-e | -p) <object>\n-'git cat-file' (-t | -s) [--allow-unknown-type] <object>\n+'git cat-file' (-e | -p | -t | -s) <object>\n 'git cat-file' (--textconv | --filters)\n \t     [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]\n 'git cat-file' (--batch | --batch-check | --batch-command) [--batch-all-objects]\n@@ -202,9 +201,6 @@ flush::\n \tonly once, even if it is stored multiple times in the\n \trepository.\n \n---allow-unknown-type::\n-\tAllow `-s` or `-t` to query broken/corrupt objects of unknown type.\n-\n --follow-symlinks::\n \tWith `--batch` or `--batch-check`, follow symlinks inside the\n \trepository when requesting objects with extended SHA-1\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 3914a2a3f6..4adc19aa29 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -100,8 +100,7 @@ static int stream_blob(const struct object_id *oid)\n \treturn 0;\n }\n \n-static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n-\t\t\tint unknown_type)\n+static int cat_one_file(int opt, const char *exp_type, const char *obj_name)\n {\n \tint ret;\n \tstruct object_id oid;\n@@ -121,9 +120,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \tif (!path && opt_cw)\n \t\tget_oid_flags |= GET_OID_REQUIRE_PATH;\n \n-\tif (unknown_type)\n-\t\tflags |= OBJECT_INFO_ALLOW_UNKNOWN_TYPE;\n-\n \tif (get_oid_with_context(the_repository, obj_name, get_oid_flags, &oid,\n \t\t\t\t &obj_context))\n \t\tdie(\"Not a valid object name %s\", obj_name);\n@@ -1038,8 +1034,7 @@ int cmd_cat_file(int argc,\n \n \tconst char * const builtin_catfile_usage[] = {\n \t\tN_(\"git cat-file <type> <object>\"),\n-\t\tN_(\"git cat-file (-e | -p) <object>\"),\n-\t\tN_(\"git cat-file (-t | -s) [--allow-unknown-type] <object>\"),\n+\t\tN_(\"git cat-file (-e | -p | -t | -s) <object>\"),\n \t\tN_(\"git cat-file (--textconv | --filters)\\n\"\n \t\t   \"             [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]\"),\n \t\tN_(\"git cat-file (--batch | --batch-check | --batch-command) [--batch-all-objects]\\n\"\n@@ -1057,8 +1052,8 @@ int cmd_cat_file(int argc,\n \t\tOPT_GROUP(N_(\"Emit [broken] object attributes\")),\n \t\tOPT_CMDMODE('t', NULL, &opt, N_(\"show object type (one of 'blob', 'tree', 'commit', 'tag', ...)\"), 't'),\n \t\tOPT_CMDMODE('s', NULL, &opt, N_(\"show object size\"), 's'),\n-\t\tOPT_BOOL(0, \"allow-unknown-type\", &unknown_type,\n-\t\t\t  N_(\"allow -s and -t to work with broken/corrupt objects\")),\n+\t\tOPT_HIDDEN_BOOL(0, \"allow-unknown-type\", &unknown_type,\n+\t\t\t  N_(\"historical option -- no-op\")),\n \t\tOPT_BOOL(0, \"use-mailmap\", &use_mailmap, N_(\"use mail map file\")),\n \t\tOPT_ALIAS(0, \"mailmap\", \"use-mailmap\"),\n \t\t/* Batch mode */\n@@ -1209,10 +1204,7 @@ int cmd_cat_file(int argc,\n \t\tobj_name = argv[1];\n \t}\n \n-\tif (unknown_type && opt != 't' && opt != 's')\n-\t\tdie(\"git cat-file --allow-unknown-type: use with -s or -t\");\n-\n-\tret = cat_one_file(opt, exp_type, obj_name, unknown_type);\n+\tret = cat_one_file(opt, exp_type, obj_name);\n \n out:\n \tlist_objects_filter_release(&batch.objects_filter);\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex ce8b27bf54..d96d02ad7d 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -136,18 +136,6 @@ $content\"\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 $oid >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 $oid >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@@ -677,95 +665,67 @@ test_expect_success 'setup bogus data' '\n \tbogus_long_oid=$(echo_without_newline \"$bogus_long_content\" | git hash-object -t $bogus_long_type --literally -w --stdin)\n '\n \n-for arg1 in '' --allow-unknown-type\n+for arg1 in -s -t -p\n do\n-\tfor arg2 in -s -t -p\n-\tdo\n-\t\tif test \"$arg1\" = \"--allow-unknown-type\" && test \"$arg2\" = \"-p\"\n-\t\tthen\n-\t\t\tcontinue\n-\t\tfi\n+\ttest_expect_success \"cat-file $arg1 error on bogus short OID\" '\n+\t\tcat >expect <<-\\EOF &&\n+\t\tfatal: invalid object type\n+\t\tEOF\n \n+\t\ttest_must_fail git cat-file $arg1 $bogus_short_oid >out 2>actual &&\n+\t\ttest_must_be_empty out &&\n+\t\ttest_cmp expect actual\n+\t'\n \n-\t\ttest_expect_success \"cat-file $arg1 $arg2 error on bogus short OID\" '\n-\t\t\tcat >expect <<-\\EOF &&\n-\t\t\tfatal: invalid object type\n+\ttest_expect_success \"cat-file $arg1 error on bogus full OID\" '\n+\t\tif test \"$arg1\" = \"-p\"\n+\t\tthen\n+\t\t\tcat >expect <<-EOF\n+\t\t\terror: header for $bogus_long_oid too long, exceeds 32 bytes\n+\t\t\tfatal: Not a valid object name $bogus_long_oid\n+\t\t\tEOF\n+\t\telse\n+\t\t\tcat >expect <<-EOF\n+\t\t\terror: header for $bogus_long_oid too long, exceeds 32 bytes\n+\t\t\tfatal: git cat-file: could not get object info\n \t\t\tEOF\n+\t\tfi &&\n \n-\t\t\tif test \"$arg1\" = \"--allow-unknown-type\"\n-\t\t\tthen\n-\t\t\t\tgit cat-file $arg1 $arg2 $bogus_short_oid\n-\t\t\telse\n-\t\t\t\ttest_must_fail git cat-file $arg1 $arg2 $bogus_short_oid >out 2>actual &&\n-\t\t\t\ttest_must_be_empty out &&\n-\t\t\t\ttest_cmp expect actual\n-\t\t\tfi\n-\t\t'\n+\t\ttest_must_fail git cat-file $arg1 $bogus_long_oid >out 2>actual &&\n+\t\ttest_must_be_empty out &&\n+\t\ttest_cmp expect actual\n+\t'\n \n-\t\ttest_expect_success \"cat-file $arg1 $arg2 error on bogus full OID\" '\n-\t\t\tif test \"$arg2\" = \"-p\"\n-\t\t\tthen\n-\t\t\t\tcat >expect <<-EOF\n-\t\t\t\terror: header for $bogus_long_oid too long, exceeds 32 bytes\n-\t\t\t\tfatal: Not a valid object name $bogus_long_oid\n-\t\t\t\tEOF\n-\t\t\telse\n-\t\t\t\tcat >expect <<-EOF\n-\t\t\t\terror: header for $bogus_long_oid too long, exceeds 32 bytes\n-\t\t\t\tfatal: git cat-file: could not get object info\n-\t\t\t\tEOF\n-\t\t\tfi &&\n-\n-\t\t\tif test \"$arg1\" = \"--allow-unknown-type\"\n-\t\t\tthen\n-\t\t\t\tgit cat-file $arg1 $arg2 $bogus_short_oid\n-\t\t\telse\n-\t\t\t\ttest_must_fail git cat-file $arg1 $arg2 $bogus_long_oid >out 2>actual &&\n-\t\t\t\ttest_must_be_empty out &&\n-\t\t\t\ttest_cmp expect actual\n-\t\t\tfi\n-\t\t'\n+\ttest_expect_success \"cat-file $arg1 error on missing short OID\" '\n+\t\tcat >expect.err <<-EOF &&\n+\t\tfatal: Not a valid object name $(test_oid deadbeef_short)\n+\t\tEOF\n+\t\ttest_must_fail git cat-file $arg1 $(test_oid deadbeef_short) >out 2>err.actual &&\n+\t\ttest_must_be_empty out &&\n+\t\ttest_cmp expect.err err.actual\n+\t'\n \n-\t\ttest_expect_success \"cat-file $arg1 $arg2 error on missing short OID\" '\n-\t\t\tcat >expect.err <<-EOF &&\n-\t\t\tfatal: Not a valid object name $(test_oid deadbeef_short)\n+\ttest_expect_success \"cat-file $arg1 error on missing full OID\" '\n+\t\tif test \"$arg1\" = \"-p\"\n+\t\tthen\n+\t\t\tcat >expect.err <<-EOF\n+\t\t\tfatal: Not a valid object name $(test_oid deadbeef)\n \t\t\tEOF\n-\t\t\ttest_must_fail git cat-file $arg1 $arg2 $(test_oid deadbeef_short) >out 2>err.actual &&\n-\t\t\ttest_must_be_empty out &&\n-\t\t\ttest_cmp expect.err err.actual\n-\t\t'\n-\n-\t\ttest_expect_success \"cat-file $arg1 $arg2 error on missing full OID\" '\n-\t\t\tif test \"$arg2\" = \"-p\"\n-\t\t\tthen\n-\t\t\t\tcat >expect.err <<-EOF\n-\t\t\t\tfatal: Not a valid object name $(test_oid deadbeef)\n-\t\t\t\tEOF\n-\t\t\telse\n-\t\t\t\tcat >expect.err <<-\\EOF\n-\t\t\t\tfatal: git cat-file: could not get object info\n-\t\t\t\tEOF\n-\t\t\tfi &&\n-\t\t\ttest_must_fail git cat-file $arg1 $arg2 $(test_oid deadbeef) >out 2>err.actual &&\n-\t\t\ttest_must_be_empty out &&\n-\t\t\ttest_cmp expect.err err.actual\n-\t\t'\n-\tdone\n+\t\telse\n+\t\t\tcat >expect.err <<-\\EOF\n+\t\t\tfatal: git cat-file: could not get object info\n+\t\t\tEOF\n+\t\tfi &&\n+\t\ttest_must_fail git cat-file $arg1 $(test_oid deadbeef) >out 2>err.actual &&\n+\t\ttest_must_be_empty out &&\n+\t\ttest_cmp expect.err err.actual\n+\t'\n done\n \n-test_expect_success '-e is OK with a broken object without --allow-unknown-type' '\n+test_expect_success '-e is OK with a broken object' '\n \tgit cat-file -e $bogus_short_oid\n '\n \n-test_expect_success '-e can not be combined with --allow-unknown-type' '\n-\ttest_expect_code 128 git cat-file -e --allow-unknown-type $bogus_short_oid\n-'\n-\n-test_expect_success '-p cannot print a broken object even with --allow-unknown-type' '\n-\ttest_must_fail git cat-file -p $bogus_short_oid &&\n-\ttest_expect_code 128 git cat-file -p --allow-unknown-type $bogus_short_oid\n-'\n-\n test_expect_success '<type> <hash> does not work with objects of broken types' '\n \tcat >err.expect <<-\\EOF &&\n \tfatal: invalid object type \"bogus\"\n@@ -788,60 +748,8 @@ test_expect_success 'broken types combined with --batch and --batch-check' '\n \ttest_cmp err.expect err.actual\n '\n \n-test_expect_success 'the --batch and --batch-check options do not combine with --allow-unknown-type' '\n-\ttest_expect_code 128 git cat-file --batch --allow-unknown-type <bogus-oid &&\n-\ttest_expect_code 128 git cat-file --batch-check --allow-unknown-type <bogus-oid\n-'\n-\n-test_expect_success 'the --allow-unknown-type option does not consider replacement refs' '\n-\tcat >expect <<-EOF &&\n-\t$bogus_short_type\n-\tEOF\n-\tgit cat-file -t --allow-unknown-type $bogus_short_oid >actual &&\n-\ttest_cmp expect actual &&\n-\n-\t# Create it manually, as \"git replace\" will die on bogus\n-\t# types.\n-\thead=$(git rev-parse --verify HEAD) &&\n-\ttest_when_finished \"test-tool ref-store main delete-refs 0 msg refs/replace/$bogus_short_oid\" &&\n-\ttest-tool ref-store main update-ref msg \"refs/replace/$bogus_short_oid\" $head $ZERO_OID REF_SKIP_OID_VERIFICATION &&\n-\n-\tcat >expect <<-EOF &&\n-\tcommit\n-\tEOF\n-\tgit cat-file -t --allow-unknown-type $bogus_short_oid >actual &&\n-\ttest_cmp expect actual\n-'\n-\n-test_expect_success \"Type of broken object is correct\" '\n-\techo $bogus_short_type >expect &&\n-\tgit cat-file -t --allow-unknown-type $bogus_short_oid >actual &&\n-\ttest_cmp expect actual\n-'\n-\n-test_expect_success \"Size of broken object is correct\" '\n-\techo $bogus_short_size >expect &&\n-\tgit cat-file -s --allow-unknown-type $bogus_short_oid >actual &&\n-\ttest_cmp expect actual\n-'\n-\n-test_expect_success 'clean up broken object' '\n-\trm .git/objects/$(test_oid_to_path $bogus_short_oid)\n-'\n-\n-test_expect_success \"Type of broken object is correct when type is large\" '\n-\techo $bogus_long_type >expect &&\n-\tgit cat-file -t --allow-unknown-type $bogus_long_oid >actual &&\n-\ttest_cmp expect actual\n-'\n-\n-test_expect_success \"Size of large broken object is correct when type is large\" '\n-\techo $bogus_long_size >expect &&\n-\tgit cat-file -s --allow-unknown-type $bogus_long_oid >actual &&\n-\ttest_cmp expect actual\n-'\n-\n-test_expect_success 'clean up broken object' '\n+test_expect_success 'clean up broken objects' '\n+\trm .git/objects/$(test_oid_to_path $bogus_short_oid) &&\n \trm .git/objects/$(test_oid_to_path $bogus_long_oid)\n '\n \n@@ -903,25 +811,6 @@ test_expect_success 'cat-file -t and -s on corrupt loose object' '\n \t)\n '\n \n-test_expect_success 'truncated object with --allow-unknown-type' - <<\\EOT\n-\tobjtype='a really long type name that exceeds the 32-byte limit' &&\n-\tblob=$(git hash-object -w --literally -t \"$objtype\" /dev/null) &&\n-\tobjpath=.git/objects/$(test_oid_to_path \"$blob\") &&\n-\n-\t# We want to truncate the object far enough in that we don't hit the\n-\t# end while inflating the first 32 bytes (since we want to have to dig\n-\t# for the trailing NUL of the header). But we don't want to go too far,\n-\t# since our header isn't very big. And of course we are counting\n-\t# deflated zlib bytes in the on-disk file, so it's a bit of a guess.\n-\t# Empirically 50 seems to work.\n-\tmv \"$objpath\" obj.bak &&\n-\ttest_when_finished 'mv obj.bak \"$objpath\"' &&\n-\ttest_copy_bytes 50 <obj.bak >\"$objpath\" &&\n-\n-\ttest_must_fail git cat-file --allow-unknown-type -t $blob 2>err &&\n-\ttest_grep \"unable to unpack $blob header\" err\n-EOT\n-\n test_expect_success 'object reading handles zlib dictionary' - <<\\EOT\n \techo 'content that will be recompressed' >file &&\n \tblob=$(git hash-object -w file) &&\n-- \n2.49.0.896.g93578ceaaf\n\n"},{"id":"518201","messageId":"20250516044945.GC22242@coredump.intra.peff.net","threadId":"63468","inReplyTo":"20250516044916.GA21985@coredump.intra.peff.net","subject":"[PATCH 03/13] object-file: drop OBJECT_INFO_ALLOW_UNKNOWN_TYPE flag","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-16T04:49:45Z","receivedAt":"2025-05-16T04:49:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Since cat-file dropped its \"--allow-unknown-type\" option in the previous\ncommit, there are no more uses of the internal flag that implemented it.\nLet's drop it.\n\nThat in turn lets us drop the strbuf parameter of unpack_loose_header(),\nwhich now is always NULL. And without that, we can drop all of the\nadditional code to inflate larger headers into the strbuf.\n\nArguably we could drop ULHR_TOO_LONG, as no callers really care about\nthe distinction from ULHR_BAD. But it's easy enough to retain, and it\ndoes let us produce a slightly more specific message in one instance.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n object-file.c  | 45 +++++++--------------------------------------\n object-file.h  | 10 ++--------\n object-store.h |  2 --\n streaming.c    |  2 +-\n 4 files changed, 10 insertions(+), 49 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex dc56a4766d..1127e154f6 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -299,8 +299,7 @@ enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,\n \t\t\t\t\t\t    unsigned char *map,\n \t\t\t\t\t\t    unsigned long mapsize,\n \t\t\t\t\t\t    void *buffer,\n-\t\t\t\t\t\t    unsigned long bufsiz,\n-\t\t\t\t\t\t    struct strbuf *header)\n+\t\t\t\t\t\t    unsigned long bufsiz)\n {\n \tint status;\n \n@@ -325,32 +324,9 @@ enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,\n \t\treturn ULHR_OK;\n \n \t/*\n-\t * We have a header longer than MAX_HEADER_LEN. The \"header\"\n-\t * here is only non-NULL when we run \"cat-file\n-\t * --allow-unknown-type\".\n+\t * We have a header longer than MAX_HEADER_LEN.\n \t */\n-\tif (!header)\n-\t\treturn ULHR_TOO_LONG;\n-\n-\t/*\n-\t * buffer[0..bufsiz] was not large enough.  Copy the partial\n-\t * result out to header, and then append the result of further\n-\t * reading the stream.\n-\t */\n-\tstrbuf_add(header, buffer, stream->next_out - (unsigned char *)buffer);\n-\n-\tdo {\n-\t\tstream->next_out = buffer;\n-\t\tstream->avail_out = bufsiz;\n-\n-\t\tobj_read_unlock();\n-\t\tstatus = git_inflate(stream, 0);\n-\t\tobj_read_lock();\n-\t\tstrbuf_add(header, buffer, stream->next_out - (unsigned char *)buffer);\n-\t\tif (memchr(buffer, '\\0', stream->next_out - (unsigned char *)buffer))\n-\t\t\treturn 0;\n-\t} while (status == Z_OK);\n-\treturn ULHR_BAD;\n+\treturn ULHR_TOO_LONG;\n }\n \n static void *unpack_loose_rest(git_zstream *stream,\n@@ -476,10 +452,8 @@ int loose_object_info(struct repository *r,\n \tvoid *map;\n \tgit_zstream stream;\n \tchar hdr[MAX_HEADER_LEN];\n-\tstruct strbuf hdrbuf = STRBUF_INIT;\n \tunsigned long size_scratch;\n \tenum object_type type_scratch;\n-\tint allow_unknown = flags & OBJECT_INFO_ALLOW_UNKNOWN_TYPE;\n \n \tif (oi->delta_base_oid)\n \t\toidclr(oi->delta_base_oid, the_repository->hash_algo);\n@@ -521,18 +495,15 @@ int loose_object_info(struct repository *r,\n \tif (oi->disk_sizep)\n \t\t*oi->disk_sizep = mapsize;\n \n-\tswitch (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr),\n-\t\t\t\t    allow_unknown ? &hdrbuf : NULL)) {\n+\tswitch (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr))) {\n \tcase ULHR_OK:\n-\t\tif (parse_loose_header(hdrbuf.len ? hdrbuf.buf : hdr, oi) < 0)\n+\t\tif (parse_loose_header(hdr, oi) < 0)\n \t\t\tstatus = error(_(\"unable to parse %s header\"), oid_to_hex(oid));\n-\t\telse if (!allow_unknown && *oi->typep < 0)\n+\t\telse if (*oi->typep < 0)\n \t\t\tdie(_(\"invalid object type\"));\n \n \t\tif (!oi->contentp)\n \t\t\tbreak;\n-\t\tif (hdrbuf.len)\n-\t\t\tBUG(\"unpacking content with unknown types not yet supported\");\n \t\t*oi->contentp = unpack_loose_rest(&stream, hdr, *oi->sizep, oid);\n \t\tif (*oi->contentp)\n \t\t\tgoto cleanup;\n@@ -558,7 +529,6 @@ int loose_object_info(struct repository *r,\n \tmunmap(map, mapsize);\n \tif (oi->sizep == &size_scratch)\n \t\toi->sizep = NULL;\n-\tstrbuf_release(&hdrbuf);\n \tif (oi->typep == &type_scratch)\n \t\toi->typep = NULL;\n \toi->whence = OI_LOOSE;\n@@ -1682,8 +1652,7 @@ int read_loose_object(const char *path,\n \t\tgoto out;\n \t}\n \n-\tif (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr),\n-\t\t\t\tNULL) != ULHR_OK) {\n+\tif (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr)) != ULHR_OK) {\n \t\terror(_(\"unable to unpack header of %s\"), path);\n \t\tgoto out_inflate;\n \t}\ndiff --git a/object-file.h b/object-file.h\nindex fd715663fb..a979fd5e4d 100644\n--- a/object-file.h\n+++ b/object-file.h\n@@ -133,12 +133,7 @@ int format_object_header(char *str, size_t size, enum object_type type,\n  * - ULHR_BAD on error\n  * - ULHR_TOO_LONG if the header was too long\n  *\n- * It will only parse up to MAX_HEADER_LEN bytes unless an optional\n- * \"hdrbuf\" argument is non-NULL. This is intended for use with\n- * OBJECT_INFO_ALLOW_UNKNOWN_TYPE to extract the bad type for (error)\n- * reporting. The full header will be extracted to \"hdrbuf\" for use\n- * with parse_loose_header(), ULHR_TOO_LONG will still be returned\n- * from this function to indicate that the header was too long.\n+ * It will only parse up to MAX_HEADER_LEN bytes.\n  */\n enum unpack_loose_header_result {\n \tULHR_OK,\n@@ -149,8 +144,7 @@ enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,\n \t\t\t\t\t\t    unsigned char *map,\n \t\t\t\t\t\t    unsigned long mapsize,\n \t\t\t\t\t\t    void *buffer,\n-\t\t\t\t\t\t    unsigned long bufsiz,\n-\t\t\t\t\t\t    struct strbuf *hdrbuf);\n+\t\t\t\t\t\t    unsigned long bufsiz);\n \n /**\n  * parse_loose_header() parses the starting \"<type> <len>\\0\" of an\ndiff --git a/object-store.h b/object-store.h\nindex c2fe5a1960..cf908fe68e 100644\n--- a/object-store.h\n+++ b/object-store.h\n@@ -240,8 +240,6 @@ struct object_info {\n \n /* Invoke lookup_replace_object() on the given hash */\n #define OBJECT_INFO_LOOKUP_REPLACE 1\n-/* Allow reading from a loose object file of unknown/bogus type */\n-#define OBJECT_INFO_ALLOW_UNKNOWN_TYPE 2\n /* Do not retry packed storage after checking packed and loose storage */\n #define OBJECT_INFO_QUICK 8\n /*\ndiff --git a/streaming.c b/streaming.c\nindex 127d6b5d6a..6d6512e2e0 100644\n--- a/streaming.c\n+++ b/streaming.c\n@@ -238,7 +238,7 @@ static int open_istream_loose(struct git_istream *st, struct repository *r,\n \t\treturn -1;\n \tswitch (unpack_loose_header(&st->z, st->u.loose.mapped,\n \t\t\t\t    st->u.loose.mapsize, st->u.loose.hdr,\n-\t\t\t\t    sizeof(st->u.loose.hdr), NULL)) {\n+\t\t\t\t    sizeof(st->u.loose.hdr))) {\n \tcase ULHR_OK:\n \t\tbreak;\n \tcase ULHR_BAD:\n-- \n2.49.0.896.g93578ceaaf\n\n"},{"id":"518202","messageId":"20250516044947.GD22242@coredump.intra.peff.net","threadId":"63468","inReplyTo":"20250516044916.GA21985@coredump.intra.peff.net","subject":"[PATCH 04/13] cat-file: use type enum instead of buffer for -t option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-16T04:49:47Z","receivedAt":"2025-05-16T04:49:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Now that we no longer support OBJECT_INFO_ALLOW_UNKNOWN_TYPE, there is\nno need to pass a strbuf into oid_object_info_extended() to record the\ntype. The regular object_type enum is sufficient to capture all of the\ntypes we will allow.\n\nThis simplifies the code a bit, and will eventually let us drop\nobject_info's type_name strbuf support.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/cat-file.c | 13 ++++---------\n 1 file changed, 4 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 4adc19aa29..67a5ff2b9e 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -109,7 +109,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)\n \tunsigned long size;\n \tstruct object_context obj_context = {0};\n \tstruct object_info oi = OBJECT_INFO_INIT;\n-\tstruct strbuf sb = STRBUF_INIT;\n \tunsigned flags = OBJECT_INFO_LOOKUP_REPLACE;\n \tunsigned get_oid_flags =\n \t\tGET_OID_RECORD_PATH |\n@@ -132,16 +131,12 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)\n \tbuf = NULL;\n \tswitch (opt) {\n \tcase 't':\n-\t\toi.type_name = &sb;\n+\t\toi.typep = &type;\n \t\tif (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-\t\t\tstrbuf_release(&sb);\n-\t\t\tret = 0;\n-\t\t\tgoto cleanup;\n-\t\t}\n-\t\tbreak;\n+\t\tprintf(\"%s\\n\", type_name(type));\n+\t\tret = 0;\n+\t\tgoto cleanup;\n \n \tcase 's':\n \t\toi.sizep = &size;\n-- \n2.49.0.896.g93578ceaaf\n\n"},{"id":"518203","messageId":"20250516044950.GE22242@coredump.intra.peff.net","threadId":"63468","inReplyTo":"20250516044916.GA21985@coredump.intra.peff.net","subject":"[PATCH 05/13] oid_object_info_convert(): stop using string for object type","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-16T04:49:50Z","receivedAt":"2025-05-16T04:49:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In oid_object_info_convert(), we convert objects between their sha1 and\nsha256 variants. To do this, we naturally need to know the type, which\nwe get from oid_object_info_extended() using its type_name strbuf\noption.\n\nBut getting the value as a string (versus an object_type enum) is not\nhelpful. Since we do not allow unknown types, the regular enum is\nsufficient. And the resulting code is a bit simpler, as we no longer\nhave to manage the extra allocation nor convert the string to an enum\nourselves.\n\nNote that at first glance, it might seem like we should retain the error\ncheck for \"type == -1\" to catch bogus types found by the underlying\nparser. But we don't need it, as an unknown type would have yielded an\nerror from the call to oid_object_info_extended(), which would already\nhave caused us to return an error.\n\nIn fact, I suspect this was always impossible to trigger. Even when we\nwere converting the string to a type enum ourselves, an invalid type\nwould never have escaped oid_object_info_extended(), since we never\npassed the (now removed) OBJECT_INFO_ALLOW_UNKNOWN_TYPE option.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n object-store.c | 15 ++++-----------\n 1 file changed, 4 insertions(+), 11 deletions(-)\n\ndiff --git a/object-store.c b/object-store.c\nindex 2f51d0e3b0..b8f6955ea7 100644\n--- a/object-store.c\n+++ b/object-store.c\n@@ -727,7 +727,7 @@ static int oid_object_info_convert(struct repository *r,\n {\n \tconst struct git_hash_algo *input_algo = &hash_algos[input_oid->algo];\n \tint do_die = flags & OBJECT_INFO_DIE_IF_CORRUPT;\n-\tstruct strbuf type_name = STRBUF_INIT;\n+\tenum object_type type;\n \tstruct object_id oid, delta_base_oid;\n \tstruct object_info new_oi, *oi;\n \tunsigned long size;\n@@ -753,7 +753,7 @@ static int oid_object_info_convert(struct repository *r,\n \t\tif (input_oi->sizep || input_oi->contentp) {\n \t\t\tnew_oi.contentp = &content;\n \t\t\tnew_oi.sizep = &size;\n-\t\t\tnew_oi.type_name = &type_name;\n+\t\t\tnew_oi.typep = &type;\n \t\t}\n \t\toi = &new_oi;\n \t}\n@@ -766,12 +766,7 @@ static int oid_object_info_convert(struct repository *r,\n \n \tif (new_oi.contentp) {\n \t\tstruct strbuf outbuf = STRBUF_INIT;\n-\t\tenum object_type type;\n \n-\t\ttype = type_from_string_gently(type_name.buf, type_name.len,\n-\t\t\t\t\t       !do_die);\n-\t\tif (type == -1)\n-\t\t\treturn -1;\n \t\tif (type != OBJ_BLOB) {\n \t\t\tret = convert_object_file(the_repository, &outbuf,\n \t\t\t\t\t\t  the_hash_algo, input_algo,\n@@ -788,10 +783,8 @@ static int oid_object_info_convert(struct repository *r,\n \t\t\t*input_oi->contentp = content;\n \t\telse\n \t\t\tfree(content);\n-\t\tif (input_oi->type_name)\n-\t\t\t*input_oi->type_name = type_name;\n-\t\telse\n-\t\t\tstrbuf_release(&type_name);\n+\t\tif (input_oi->typep)\n+\t\t\t*input_oi->typep = type;\n \t}\n \tif (new_oi.delta_base_oid == &delta_base_oid) {\n \t\tif (repo_oid_to_algop(r, &delta_base_oid, input_algo,\n-- \n2.49.0.896.g93578ceaaf\n\n"},{"id":"518204","messageId":"20250516044953.GF22242@coredump.intra.peff.net","threadId":"63468","inReplyTo":"20250516044916.GA21985@coredump.intra.peff.net","subject":"[PATCH 06/13] fsck: stop using object_info->type_name strbuf","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-16T04:49:53Z","receivedAt":"2025-05-16T04:49:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When fsck-ing a loose object, we use object_info's type_name strbuf to\nrecord the parsed object type as a string. For most objects this is\nredundant with the object_type enum, but it does let us report the\nstring when we encounter an object with an unknown type (for which there\nis no matching enum value).\n\nThere are a few downsides, though:\n\n  1. The code to report these cases is not actually robust. Since we did\n     not pass a strbuf to unpack_loose_header(), we only retrieved types\n     from headers up to 32 bytes. In longer cases, we'd simply say\n     \"object corrupt or missing\".\n\n  2. This is the last caller that uses object_info's type_name strbuf\n     support. It would be nice to refactor it so that we can simplify\n     that code.\n\n  3. Likewise, we'll check the hash of the object using its unknown type\n     (again, as long as that type is short enough). That depends on the\n     hash_object_file_literally() code, which we'd eventually like to\n     get rid of.\n\nSo we can simplify things by bailing immediately in read_loose_object()\nwhen we encounter an unknown type. This has a few user-visible effects:\n\n  a. Instead of producing a single line of error output like this:\n\n       error: 26ed13ce3564fbbb44e35bde42c7da717ea004a6: object is of unknown type 'bogus': .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6\n\n     we'll now issue two lines (the first from read_loose_object() when\n     we see the unparsable header, and the second from the fsck code,\n     since we couldn't read the object):\n\n       error: unable to parse type from header 'bogus 4' of .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6\n       error: 26ed13ce3564fbbb44e35bde42c7da717ea004a6: object corrupt or missing: .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6\n\n     This is a little more verbose, but this sort of error should be\n     rare (such objects are almost impossible to work with, and cannot\n     be transferred between repositories as they are not representable\n     in packfiles). And as a bonus, reporting the broken header in full\n     could help with debugging other cases (e.g., a header like \"blob\n     xyzzy\\0\" would fail in parsing the size, but previously we'd not\n     have showed the offending bytes).\n\n  b. An object with an unknown type will be reported as corrupt, without\n     actually doing a hash check. Again, I think this is unlikely to\n     matter in practice since such objects are totally unusable.\n\nWe'll update one fsck test to match the new error strings. And we can\nremove another test that covered the case of an object with an unknown\ntype _and_ a hash corruption. Since we'll skip the hash check now in\nthis case, the test is no longer interesting.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/fsck.c  | 13 ++-----------\n object-file.c   | 12 +++++++++---\n t/t1450-fsck.sh | 29 +++--------------------------\n 3 files changed, 14 insertions(+), 40 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 6cac28356c..e7d96a9c8e 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -614,12 +614,11 @@ static void get_default_heads(void)\n struct for_each_loose_cb\n {\n \tstruct progress *progress;\n-\tstruct strbuf obj_type;\n };\n \n-static int fsck_loose(const struct object_id *oid, const char *path, void *data)\n+static int fsck_loose(const struct object_id *oid, const char *path,\n+\t\t      void *data UNUSED)\n {\n-\tstruct for_each_loose_cb *cb_data = data;\n \tstruct object *obj;\n \tenum object_type type = OBJ_NONE;\n \tunsigned long size;\n@@ -629,8 +628,6 @@ static int fsck_loose(const struct object_id *oid, const char *path, void *data)\n \tstruct object_id real_oid = *null_oid(the_hash_algo);\n \tint err = 0;\n \n-\tstrbuf_reset(&cb_data->obj_type);\n-\toi.type_name = &cb_data->obj_type;\n \toi.sizep = &size;\n \toi.typep = &type;\n \n@@ -642,10 +639,6 @@ static int fsck_loose(const struct object_id *oid, const char *path, void *data)\n \t\t\terr = error(_(\"%s: object corrupt or missing: %s\"),\n \t\t\t\t    oid_to_hex(oid), path);\n \t}\n-\tif (type != OBJ_NONE && type < 0)\n-\t\terr = error(_(\"%s: object is of unknown type '%s': %s\"),\n-\t\t\t    oid_to_hex(&real_oid), cb_data->obj_type.buf,\n-\t\t\t    path);\n \tif (err < 0) {\n \t\terrors_found |= ERROR_OBJECT;\n \t\tfree(contents);\n@@ -697,7 +690,6 @@ static void fsck_object_dir(const char *path)\n {\n \tstruct progress *progress = NULL;\n \tstruct for_each_loose_cb cb_data = {\n-\t\t.obj_type = STRBUF_INIT,\n \t\t.progress = progress,\n \t};\n \n@@ -712,7 +704,6 @@ static void fsck_object_dir(const char *path)\n \t\t\t\t      &cb_data);\n \tdisplay_progress(progress, 256);\n \tstop_progress(&progress);\n-\tstrbuf_release(&cb_data.obj_type);\n }\n \n static int fsck_head_link(const char *head_ref_name,\ndiff --git a/object-file.c b/object-file.c\nindex 1127e154f6..7a35bde96e 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -1662,6 +1662,12 @@ int read_loose_object(const char *path,\n \t\tgoto out_inflate;\n \t}\n \n+\tif (*oi->typep < 0) {\n+\t\terror(_(\"unable to parse type from header '%s' of %s\"),\n+\t\t      hdr, path);\n+\t\tgoto out_inflate;\n+\t}\n+\n \tif (*oi->typep == OBJ_BLOB &&\n \t    *size > repo_settings_get_big_file_threshold(the_repository)) {\n \t\tif (check_stream_oid(&stream, hdr, *size, path, expected_oid) < 0)\n@@ -1672,9 +1678,9 @@ int read_loose_object(const char *path,\n \t\t\terror(_(\"unable to unpack contents of %s\"), path);\n \t\t\tgoto out_inflate;\n \t\t}\n-\t\thash_object_file_literally(the_repository->hash_algo,\n-\t\t\t\t\t   *contents, *size,\n-\t\t\t\t\t   oi->type_name->buf, real_oid);\n+\t\thash_object_file(the_repository->hash_algo,\n+\t\t\t\t *contents, *size,\n+\t\t\t\t *oi->typep, real_oid);\n \t\tif (!oideq(expected_oid, real_oid))\n \t\t\tgoto out_inflate;\n \t}\ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex 0105045376..3f52dd5abc 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -71,30 +71,6 @@ test_expect_success 'object with hash mismatch' '\n \t)\n '\n \n-test_expect_success 'object with hash and type mismatch' '\n-\tgit init --bare hash-type-mismatch &&\n-\t(\n-\t\tcd hash-type-mismatch &&\n-\n-\t\toid=$(echo blob | git hash-object -w --stdin -t garbage --literally) &&\n-\t\toldoid=$oid &&\n-\t\told=$(test_oid_to_path \"$oid\") &&\n-\t\tnew=$(dirname $old)/$(test_oid ff_2) &&\n-\t\toid=\"$(dirname $new)$(basename $new)\" &&\n-\n-\t\tmv objects/$old objects/$new &&\n-\t\tgit update-index --add --cacheinfo 100644 $oid foo &&\n-\t\ttree=$(git write-tree) &&\n-\t\tcmt=$(echo bogus | git commit-tree $tree) &&\n-\t\tgit update-ref refs/heads/bogus $cmt &&\n-\n-\n-\t\ttest_must_fail git fsck 2>out &&\n-\t\tgrep \"^error: $oldoid: hash-path mismatch, found at: .*$new\" out &&\n-\t\tgrep \"^error: $oldoid: object is of unknown type '\"'\"'garbage'\"'\"'\" out\n-\t)\n-'\n-\n test_expect_success 'zlib corrupt loose object output ' '\n \tgit init --bare corrupt-loose-output &&\n \t(\n@@ -1001,8 +977,9 @@ test_expect_success 'fsck error and recovery on invalid object type' '\n \n \t\ttest_must_fail git fsck 2>err &&\n \t\tgrep -e \"^error\" -e \"^fatal\" err >errors &&\n-\t\ttest_line_count = 1 errors &&\n-\t\tgrep \"$garbage_blob: object is of unknown type '\"'\"'garbage'\"'\"':\" err\n+\t\ttest_line_count = 2 errors &&\n+\t\ttest_grep \"unable to parse type from header .garbage\" err &&\n+\t\ttest_grep \"$garbage_blob: object corrupt or missing:\" err\n \t)\n '\n \n-- \n2.49.0.896.g93578ceaaf\n\n"},{"id":"518205","messageId":"20250516044956.GG22242@coredump.intra.peff.net","threadId":"63468","inReplyTo":"20250516044916.GA21985@coredump.intra.peff.net","subject":"[PATCH 07/13] oid_object_info(): drop type_name strbuf","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-16T04:49:56Z","receivedAt":"2025-05-16T04:49:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We provide a mechanism for callers to get the object type as a raw\nstring, rather than an object_type enum. This was in theory useful for\nreturning types that are not representable in the enum, but we consider\nany such type to be an error, and there are no callers that use the\nstrbuf anymore.\n\nLet's drop support to simplify the code a bit.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n object-file.c  | 4 +---\n object-store.c | 2 --\n object-store.h | 1 -\n packfile.c     | 7 +------\n 4 files changed, 2 insertions(+), 12 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex 7a35bde96e..b10e283529 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -403,8 +403,6 @@ int parse_loose_header(const char *hdr, struct object_info *oi)\n \t}\n \n \ttype = type_from_string_gently(type_buf, type_len, 1);\n-\tif (oi->type_name)\n-\t\tstrbuf_add(oi->type_name, type_buf, type_len);\n \tif (oi->typep)\n \t\t*oi->typep = type;\n \n@@ -466,7 +464,7 @@ int loose_object_info(struct repository *r,\n \t * return value implicitly indicates whether the\n \t * object even exists.\n \t */\n-\tif (!oi->typep && !oi->type_name && !oi->sizep && !oi->contentp) {\n+\tif (!oi->typep && !oi->sizep && !oi->contentp) {\n \t\tstruct stat st;\n \t\tif (!oi->disk_sizep && (flags & OBJECT_INFO_QUICK))\n \t\t\treturn quick_has_loose(r, oid) ? 0 : -1;\ndiff --git a/object-store.c b/object-store.c\nindex b8f6955ea7..216c61dcf2 100644\n--- a/object-store.c\n+++ b/object-store.c\n@@ -646,8 +646,6 @@ static int do_oid_object_info_extended(struct repository *r,\n \t\t\t*(oi->disk_sizep) = 0;\n \t\tif (oi->delta_base_oid)\n \t\t\toidclr(oi->delta_base_oid, the_repository->hash_algo);\n-\t\tif (oi->type_name)\n-\t\t\tstrbuf_addstr(oi->type_name, type_name(co->type));\n \t\tif (oi->contentp)\n \t\t\t*oi->contentp = xmemdupz(co->buf, co->size);\n \t\toi->whence = OI_CACHED;\ndiff --git a/object-store.h b/object-store.h\nindex cf908fe68e..6b55c245eb 100644\n--- a/object-store.h\n+++ b/object-store.h\n@@ -205,7 +205,6 @@ struct object_info {\n \tunsigned long *sizep;\n \toff_t *disk_sizep;\n \tstruct object_id *delta_base_oid;\n-\tstruct strbuf *type_name;\n \tvoid **contentp;\n \n \t/* Response */\ndiff --git a/packfile.c b/packfile.c\nindex d91016f1c7..80e35f1032 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -1598,17 +1598,12 @@ int packed_object_info(struct repository *r, struct packed_git *p,\n \t\t*oi->disk_sizep = pack_pos_to_offset(p, pos + 1) - obj_offset;\n \t}\n \n-\tif (oi->typep || oi->type_name) {\n+\tif (oi->typep) {\n \t\tenum object_type ptot;\n \t\tptot = packed_to_object_type(r, p, obj_offset,\n \t\t\t\t\t     type, &w_curs, curpos);\n \t\tif (oi->typep)\n \t\t\t*oi->typep = ptot;\n-\t\tif (oi->type_name) {\n-\t\t\tconst char *tn = type_name(ptot);\n-\t\t\tif (tn)\n-\t\t\t\tstrbuf_addstr(oi->type_name, tn);\n-\t\t}\n \t\tif (ptot < 0) {\n \t\t\ttype = OBJ_BAD;\n \t\t\tgoto out;\n-- \n2.49.0.896.g93578ceaaf\n\n"},{"id":"518206","messageId":"20250516044959.GH22242@coredump.intra.peff.net","threadId":"63468","inReplyTo":"20250516044916.GA21985@coredump.intra.peff.net","subject":"[PATCH 08/13] t/helper: add zlib test-tool","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-16T04:49:59Z","receivedAt":"2025-05-16T04:50:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"It's occasionally useful when testing or debugging to be able to do raw\nzlib inflate/deflate operations (e.g., to check the bytes of a specific\nloose or packed object).\n\nEven though zlib's deflate algorithm is used by many other programs,\nthis is surprisingly hard to do in a portable way. E.g., gzip can do\nthis if you manually munge some header bytes. But the result is somewhat\narcane, and we don't assume gzip is available anyway. Likewise, pigz\nwill handle raw zlib, but we can't assume it is available.\n\nSo let's introduce a short test helper for just doing zlib operations.\nWe'll use it in subsequent patches to add some new tests, but it would\nalso have come in handy a few times in the past:\n\n  - The hard-coded pack data from 3b910d0c5e (add tests for indexing\n    packs with delta cycles, 2013-08-23) could probably be generated on\n    the fly.\n\n  - Likewise we could avoid the hard-coded data from 0b1493c2d4\n    (git_inflate(): skip zlib_post_call() sanity check on Z_NEED_DICT,\n    2025-02-25). Though note this would require support for more zlib\n    options.\n\n  - It would have helped with the debugging documented in 41dfbb2dbe\n    (howto: add article on recovering a corrupted object, 2013-10-25).\n\nI'll leave refactoring existing tests for another day, but I hope the\nexamples above show the general utility.\n\nI aimed for simplicity in the code. In particular, it will read all\ninput into a memory buffer, rather than streaming. That makes the zlib\nloops harder to get wrong (which has been a source of subtle bugs in the\npast).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Makefile             |  1 +\n t/helper/meson.build |  1 +\n t/helper/test-tool.c |  1 +\n t/helper/test-tool.h |  1 +\n t/helper/test-zlib.c | 62 ++++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 66 insertions(+)\n create mode 100644 t/helper/test-zlib.c\n\ndiff --git a/Makefile b/Makefile\nindex de73c6ddcd..14616ff625 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -859,6 +859,7 @@ TEST_BUILTINS_OBJS += test-wildmatch.o\n TEST_BUILTINS_OBJS += test-windows-named-pipe.o\n TEST_BUILTINS_OBJS += test-write-cache.o\n TEST_BUILTINS_OBJS += test-xml-encode.o\n+TEST_BUILTINS_OBJS += test-zlib.o\n \n # Do not add more tests here unless they have extra dependencies. Add\n # them in TEST_BUILTINS_OBJS above.\ndiff --git a/t/helper/meson.build b/t/helper/meson.build\nindex d4e8b26df8..675e64c010 100644\n--- a/t/helper/meson.build\n+++ b/t/helper/meson.build\n@@ -77,6 +77,7 @@ test_tool_sources = [\n   'test-windows-named-pipe.c',\n   'test-write-cache.c',\n   'test-xml-encode.c',\n+  'test-zlib.c',\n ]\n \n test_tool = executable('test-tool',\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex 74812ed86d..a7abc618b3 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -91,6 +91,7 @@ static struct test_cmd cmds[] = {\n \t{ \"windows-named-pipe\", cmd__windows_named_pipe },\n #endif\n \t{ \"write-cache\", cmd__write_cache },\n+\t{ \"zlib\", cmd__zlib },\n };\n \n static NORETURN void die_usage(void)\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex 2571a3ccfe..7f150fa1eb 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -84,6 +84,7 @@ int cmd__wildmatch(int argc, const char **argv);\n int cmd__windows_named_pipe(int argc, const char **argv);\n #endif\n int cmd__write_cache(int argc, const char **argv);\n+int cmd__zlib(int argc, const char **argv);\n \n int cmd_hash_impl(int ac, const char **av, int algo, int unsafe);\n \ndiff --git a/t/helper/test-zlib.c b/t/helper/test-zlib.c\nnew file mode 100644\nindex 0000000000..de7e9edee1\n--- /dev/null\n+++ b/t/helper/test-zlib.c\n@@ -0,0 +1,62 @@\n+#include \"test-tool.h\"\n+#include \"git-zlib.h\"\n+#include \"strbuf.h\"\n+\n+static const char *zlib_usage = \"test-tool zlib [inflate|deflate]\";\n+\n+static void do_zlib(struct git_zstream *stream,\n+\t\t    int (*zlib_func)(git_zstream *, int),\n+\t\t    int fd_in, int fd_out)\n+{\n+\tstruct strbuf buf_in = STRBUF_INIT;\n+\tint status = Z_OK;\n+\n+\tif (strbuf_read(&buf_in, fd_in, 0) < 0)\n+\t\tdie_errno(\"read error\");\n+\n+\tstream->next_in = (unsigned char *)buf_in.buf;\n+\tstream->avail_in = buf_in.len;\n+\n+\twhile (status == Z_OK ||\n+\t       (status == Z_BUF_ERROR && !stream->avail_out)) {\n+\t\tunsigned char buf_out[4096];\n+\n+\t\tstream->next_out = buf_out;\n+\t\tstream->avail_out = sizeof(buf_out);\n+\n+\t\tstatus = zlib_func(stream, Z_FINISH);\n+\t\tif (write_in_full(fd_out, buf_out,\n+\t\t\t\t  sizeof(buf_out) - stream->avail_out) < 0)\n+\t\t\tdie_errno(\"write error\");\n+\t}\n+\n+\tif (status != Z_STREAM_END)\n+\t\tdie(\"zlib error %d\", status);\n+\n+\tstrbuf_release(&buf_in);\n+}\n+\n+int cmd__zlib(int argc, const char **argv)\n+{\n+\tgit_zstream stream;\n+\n+\tif (argc != 2)\n+\t\tusage(zlib_usage);\n+\n+\tmemset(&stream, 0, sizeof(stream));\n+\n+\tif (!strcmp(argv[1], \"inflate\")) {\n+\t\tgit_inflate_init(&stream);\n+\t\tdo_zlib(&stream, git_inflate, 0, 1);\n+\t\tgit_inflate_end(&stream);\n+\t} else if (!strcmp(argv[1], \"deflate\")) {\n+\t\tgit_deflate_init(&stream, Z_DEFAULT_COMPRESSION);\n+\t\tdo_zlib(&stream, git_deflate, 0, 1);\n+\t\tgit_deflate_end(&stream);\n+\t} else {\n+\t\terror(\"unknown mode: %s\", argv[1]);\n+\t\tusage(zlib_usage);\n+\t}\n+\n+\treturn 0;\n+}\n-- \n2.49.0.896.g93578ceaaf\n\n"},{"id":"518207","messageId":"20250516045002.GI22242@coredump.intra.peff.net","threadId":"63468","inReplyTo":"20250516044916.GA21985@coredump.intra.peff.net","subject":"[PATCH 09/13] t: add lib-loose.sh","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-16T04:50:02Z","receivedAt":"2025-05-16T04:50:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This commit adds a shell library for writing raw loose objects into the\nobject database. Normally this is done with hash-object, but the\nspecific intent here is to allow broken objects that hash-object may not\nsupport.\n\nWe'll convert several cases that use \"hash-object --literally\" to write\nobjects with invalid types. That works currently, but dropping this\ndependency will allow us to remove that feature and simplify the\nobject-writing code.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/lib-loose.sh                      | 30 +++++++++++++++++++++++++++++\n t/t1006-cat-file.sh                 |  5 +++--\n t/t1450-fsck.sh                     |  3 ++-\n t/t1512-rev-parse-disambiguation.sh |  5 +++--\n 4 files changed, 38 insertions(+), 5 deletions(-)\n create mode 100644 t/lib-loose.sh\n\ndiff --git a/t/lib-loose.sh b/t/lib-loose.sh\nnew file mode 100644\nindex 0000000000..3613631eaf\n--- /dev/null\n+++ b/t/lib-loose.sh\n@@ -0,0 +1,30 @@\n+# Support routines for hand-crafting loose objects.\n+\n+# Write a loose object into the odb at $1, with object type $2 and contents\n+# from stdin. Writes the oid to stdout. Example:\n+#\n+#   oid=$(echo foo | loose_obj .git/objects blob)\n+#\n+loose_obj () {\n+\tcat >tmp_loose.content &&\n+\tsize=$(wc -c <tmp_loose.content) &&\n+\t{\n+\t\t# Do not quote $size here; we want the shell\n+\t\t# to strip whitespace that \"wc\" adds on some platforms.\n+\t\tprintf \"%s %s\\0\" \"$2\" $size &&\n+\t\tcat tmp_loose.content\n+\t} >tmp_loose.raw &&\n+\n+\toid=$(test-tool $test_hash_algo <tmp_loose.raw) &&\n+\tsuffix=${oid#??} &&\n+\tprefix=${oid%$suffix} &&\n+\tdir=$1/$prefix &&\n+\tfile=$dir/$suffix &&\n+\n+\ttest-tool zlib deflate <tmp_loose.raw >tmp_loose.zlib &&\n+\tmkdir -p \"$dir\" &&\n+\tmv tmp_loose.zlib \"$file\" &&\n+\n+\trm tmp_loose.raw tmp_loose.content &&\n+\techo \"$oid\"\n+}\ndiff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\nindex d96d02ad7d..317da6869c 100755\n--- a/t/t1006-cat-file.sh\n+++ b/t/t1006-cat-file.sh\n@@ -3,6 +3,7 @@\n test_description='git cat-file'\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY/lib-loose.sh\"\n \n test_cmdmode_usage () {\n \ttest_expect_code 129 \"$@\" 2>err &&\n@@ -657,12 +658,12 @@ test_expect_success 'setup bogus data' '\n \tbogus_short_type=\"bogus\" &&\n \tbogus_short_content=\"bogus\" &&\n \tbogus_short_size=$(strlen \"$bogus_short_content\") &&\n-\tbogus_short_oid=$(echo_without_newline \"$bogus_short_content\" | git hash-object -t $bogus_short_type --literally -w --stdin) &&\n+\tbogus_short_oid=$(echo_without_newline \"$bogus_short_content\" | loose_obj .git/objects $bogus_short_type) &&\n \n \tbogus_long_type=\"abcdefghijklmnopqrstuvwxyz1234679\" &&\n \tbogus_long_content=\"bogus\" &&\n \tbogus_long_size=$(strlen \"$bogus_long_content\") &&\n-\tbogus_long_oid=$(echo_without_newline \"$bogus_long_content\" | git hash-object -t $bogus_long_type --literally -w --stdin)\n+\tbogus_long_oid=$(echo_without_newline \"$bogus_long_content\" | loose_obj .git/objects $bogus_long_type)\n '\n \n for arg1 in -s -t -p\ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex 3f52dd5abc..5ae86c42be 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -7,6 +7,7 @@ test_description='git fsck random collection of tests\n '\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY/lib-loose.sh\"\n \n test_expect_success setup '\n \tgit config gc.auto 0 &&\n@@ -973,7 +974,7 @@ test_expect_success 'fsck error and recovery on invalid object type' '\n \t(\n \t\tcd garbage-type &&\n \n-\t\tgarbage_blob=$(git hash-object --stdin -w -t garbage --literally </dev/null) &&\n+\t\tgarbage_blob=$(loose_obj objects garbage </dev/null) &&\n \n \t\ttest_must_fail git fsck 2>err &&\n \t\tgrep -e \"^error\" -e \"^fatal\" err >errors &&\ndiff --git a/t/t1512-rev-parse-disambiguation.sh b/t/t1512-rev-parse-disambiguation.sh\nindex 70f1e0a998..1a380a4184 100755\n--- a/t/t1512-rev-parse-disambiguation.sh\n+++ b/t/t1512-rev-parse-disambiguation.sh\n@@ -24,6 +24,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY/lib-loose.sh\"\n \n test_cmp_failed_rev_parse () {\n \tdir=$1\n@@ -67,8 +68,8 @@ test_expect_success 'ambiguous loose bad object parsed as OBJ_BAD' '\n \t\tcd blob.bad &&\n \n \t\t# Both have the prefix \"bad0\"\n-\t\techo xyzfaowcoh | git hash-object -t bad -w --stdin --literally &&\n-\t\techo xyzhjpyvwl | git hash-object -t bad -w --stdin --literally\n+\t\techo xyzfaowcoh | loose_obj objects bad &&\n+\t\techo xyzhjpyvwl | loose_obj objects bad\n \t) &&\n \n \ttest_cmp_failed_rev_parse blob.bad bad0 <<-\\EOF\n-- \n2.49.0.896.g93578ceaaf\n\n"},{"id":"518208","messageId":"20250516045005.GJ22242@coredump.intra.peff.net","threadId":"63468","inReplyTo":"20250516044916.GA21985@coredump.intra.peff.net","subject":"[PATCH 10/13] hash-object: stop allowing unknown types","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-16T04:50:05Z","receivedAt":"2025-05-16T04:50:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When passed the \"--literally\" option, hash-object will allow any\narbitrary string for its \"-t\" type option. Such objects are only useful\nfor testing or debugging, as they cannot be used in the normal way\n(e.g., you cannot fetch their contents!).\n\nLet's drop this feature, which will eventually let us simplify the\nobject-writing code. This is technically backwards incompatible, but\nsince such objects were never really functional, it seems unlikely that\nanybody will notice.\n\nWe will retain the --literally flag, as it also instructs hash-object\nnot to worry about other format issues (e.g., type-specific things that\nfsck would complain about). The documentation does not need to be\nupdated, as it was always vague about which checks we're loosening (it\nuses only the phrase \"any garbage\").\n\nThe code change is a bit hard to verify from just the patch text. We can\ndrop our local hash_literally() helper, but it was really just wrapping\nwrite_object_file_literally(). We now replace that with calling\nindex_fd(), as we do for the non-literal code path, but dropping the\nINDEX_FORMAT_CHECK flag. This ends up being the same semantically as\nwhat the _literally() code path was doing (modulo handling unknown\ntypes, which is our goal).\n\nWe'll be able to clean up these code paths a bit more in subsequent\npatches.\n\nThe existing test is flipped to show that we now reject the unknown\ntype. The additional \"extra-long type\" test is now redundant, as we bail\nearly upon seeing a bogus type.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/hash-object.c  | 29 +++++------------------------\n t/t1007-hash-object.sh | 11 ++---------\n 2 files changed, 7 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin/hash-object.c b/builtin/hash-object.c\nindex cd53fa3bde..3c6949b3fa 100644\n--- a/builtin/hash-object.c\n+++ b/builtin/hash-object.c\n@@ -24,26 +24,6 @@ enum {\n \tHASH_OBJECT_WRITE = (1 << 1),\n };\n \n-/*\n- * This is to create corrupt objects for debugging and as such it\n- * needs to bypass the data conversion performed by, and the type\n- * limitation imposed by, index_fd() and its callees.\n- */\n-static int hash_literally(struct object_id *oid, int fd, const char *type, unsigned flags)\n-{\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tint ret;\n-\n-\tif (strbuf_read(&buf, fd, 4096) < 0)\n-\t\tret = -1;\n-\telse\n-\t\tret = write_object_file_literally(buf.buf, buf.len, type, oid,\n-\t\t\t\t\t\t  (flags & HASH_OBJECT_WRITE) ? WRITE_OBJECT_FILE_PERSIST : 0);\n-\tclose(fd);\n-\tstrbuf_release(&buf);\n-\treturn ret;\n-}\n-\n static void hash_fd(int fd, const char *type, const char *path, unsigned flags,\n \t\t    int literally)\n {\n@@ -56,11 +36,12 @@ static void hash_fd(int fd, const char *type, const char *path, unsigned flags,\n \tif (flags & HASH_OBJECT_CHECK)\n \t\tindex_flags |= INDEX_FORMAT_CHECK;\n \n+\tif (literally)\n+\t\tindex_flags &= ~INDEX_FORMAT_CHECK;\n+\n \tif (fstat(fd, &st) < 0 ||\n-\t    (literally\n-\t     ? hash_literally(&oid, fd, type, flags)\n-\t     : index_fd(the_repository->index, &oid, fd, &st,\n-\t\t\ttype_from_string(type), path, index_flags)))\n+\t    index_fd(the_repository->index, &oid, fd, &st,\n+\t\t     type_from_string(type), path, index_flags))\n \t\tdie((flags & HASH_OBJECT_WRITE)\n \t\t    ? \"Unable to add %s to database\"\n \t\t    : \"Unable to hash %s\", path);\ndiff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh\nindex b3cf53ff8c..dbbe9fb0d4 100755\n--- a/t/t1007-hash-object.sh\n+++ b/t/t1007-hash-object.sh\n@@ -248,15 +248,8 @@ test_expect_success 'hash-object complains about truncated type name' '\n \ttest_must_fail git hash-object -t bl --stdin </dev/null\n '\n \n-test_expect_success '--literally' '\n-\tt=1234567890 &&\n-\techo example | git hash-object -t $t --literally --stdin\n-'\n-\n-test_expect_success '--literally with extra-long type' '\n-\tt=12345678901234567890123456789012345678901234567890 &&\n-\tt=\"$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t$t\" &&\n-\techo example | git hash-object -t $t --literally --stdin\n+test_expect_success '--literally complains about non-standard types' '\n+\ttest_must_fail git hash-object -t bogus --literally --stdin\n '\n \n test_expect_success '--stdin outside of repository (uses SHA-1)' '\n-- \n2.49.0.896.g93578ceaaf\n\n"},{"id":"518209","messageId":"20250516045008.GK22242@coredump.intra.peff.net","threadId":"63468","inReplyTo":"20250516044916.GA21985@coredump.intra.peff.net","subject":"[PATCH 11/13] hash-object: merge HASH_* and INDEX_* flags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-16T04:50:08Z","receivedAt":"2025-05-16T04:50:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The hash-object command has its own custom flag bits that it sets based\non command-line options. But since we dropped hash_literally() in the\nprevious commit, the only thing we do with those flag bits is convert\nthem directly into \"index_flags\" to pass to index_fd().\n\nThis extra layer of indirection makes the code harder to read and reason\nabout. Let's just use the INDEX_* flags directly.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/hash-object.c | 23 ++++++-----------------\n 1 file changed, 6 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/hash-object.c b/builtin/hash-object.c\nindex 3c6949b3fa..1ecb70b551 100644\n--- a/builtin/hash-object.c\n+++ b/builtin/hash-object.c\n@@ -19,30 +19,19 @@\n #include \"strbuf.h\"\n #include \"write-or-die.h\"\n \n-enum {\n-\tHASH_OBJECT_CHECK = (1 << 0),\n-\tHASH_OBJECT_WRITE = (1 << 1),\n-};\n-\n static void hash_fd(int fd, const char *type, const char *path, unsigned flags,\n \t\t    int literally)\n {\n-\tunsigned int index_flags = 0;\n \tstruct stat st;\n \tstruct object_id oid;\n \n-\tif (flags & HASH_OBJECT_WRITE)\n-\t\tindex_flags |= INDEX_WRITE_OBJECT;\n-\tif (flags & HASH_OBJECT_CHECK)\n-\t\tindex_flags |= INDEX_FORMAT_CHECK;\n-\n \tif (literally)\n-\t\tindex_flags &= ~INDEX_FORMAT_CHECK;\n+\t\tflags &= ~INDEX_FORMAT_CHECK;\n \n \tif (fstat(fd, &st) < 0 ||\n \t    index_fd(the_repository->index, &oid, fd, &st,\n-\t\t     type_from_string(type), path, index_flags))\n-\t\tdie((flags & HASH_OBJECT_WRITE)\n+\t\t     type_from_string(type), path, flags))\n+\t\tdie((flags & INDEX_WRITE_OBJECT)\n \t\t    ? \"Unable to add %s to database\"\n \t\t    : \"Unable to hash %s\", path);\n \tprintf(\"%s\\n\", oid_to_hex(&oid));\n@@ -94,13 +83,13 @@ int cmd_hash_object(int argc,\n \tint no_filters = 0;\n \tint literally = 0;\n \tint nongit = 0;\n-\tunsigned flags = HASH_OBJECT_CHECK;\n+\tunsigned flags = INDEX_FORMAT_CHECK;\n \tconst char *vpath = NULL;\n \tchar *vpath_free = NULL;\n \tconst struct option hash_object_options[] = {\n \t\tOPT_STRING('t', NULL, &type, N_(\"type\"), N_(\"object type\")),\n \t\tOPT_BIT('w', NULL, &flags, N_(\"write the object into the object database\"),\n-\t\t\tHASH_OBJECT_WRITE),\n+\t\t\tINDEX_WRITE_OBJECT),\n \t\tOPT_COUNTUP( 0 , \"stdin\", &hashstdin, N_(\"read the object from stdin\")),\n \t\tOPT_BOOL( 0 , \"stdin-paths\", &stdin_paths, N_(\"read file names from stdin\")),\n \t\tOPT_BOOL( 0 , \"no-filters\", &no_filters, N_(\"store file as is without filters\")),\n@@ -114,7 +103,7 @@ int cmd_hash_object(int argc,\n \targc = parse_options(argc, argv, prefix, hash_object_options,\n \t\t\t     hash_object_usage, 0);\n \n-\tif (flags & HASH_OBJECT_WRITE)\n+\tif (flags & INDEX_WRITE_OBJECT)\n \t\tprefix = setup_git_directory();\n \telse\n \t\tprefix = setup_git_directory_gently(&nongit);\n-- \n2.49.0.896.g93578ceaaf\n\n"},{"id":"518210","messageId":"20250516045010.GL22242@coredump.intra.peff.net","threadId":"63468","inReplyTo":"20250516044916.GA21985@coredump.intra.peff.net","subject":"[PATCH 12/13] hash-object: handle --literally with OPT_NEGBIT","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-16T04:50:10Z","receivedAt":"2025-05-16T04:50:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Since we recently removed the hash_literally() function, the hash-object\n--literally option has been simplified to just removing the\nINDEX_FORMAT_CHECK flag. Rather than pass it around as a separate bool,\nwe can just have the option parser remove the bit from the set of flags\ndirectly. This simplifies the helper functions.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/hash-object.c | 27 +++++++++++----------------\n 1 file changed, 11 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/hash-object.c b/builtin/hash-object.c\nindex 1ecb70b551..6a99ec250d 100644\n--- a/builtin/hash-object.c\n+++ b/builtin/hash-object.c\n@@ -19,15 +19,11 @@\n #include \"strbuf.h\"\n #include \"write-or-die.h\"\n \n-static void hash_fd(int fd, const char *type, const char *path, unsigned flags,\n-\t\t    int literally)\n+static void hash_fd(int fd, const char *type, const char *path, unsigned flags)\n {\n \tstruct stat st;\n \tstruct object_id oid;\n \n-\tif (literally)\n-\t\tflags &= ~INDEX_FORMAT_CHECK;\n-\n \tif (fstat(fd, &st) < 0 ||\n \t    index_fd(the_repository->index, &oid, fd, &st,\n \t\t     type_from_string(type), path, flags))\n@@ -39,15 +35,14 @@ static void hash_fd(int fd, const char *type, const char *path, unsigned flags,\n }\n \n static void hash_object(const char *path, const char *type, const char *vpath,\n-\t\t\tunsigned flags, int literally)\n+\t\t\tunsigned flags)\n {\n \tint fd;\n \tfd = xopen(path, O_RDONLY);\n-\thash_fd(fd, type, vpath, flags, literally);\n+\thash_fd(fd, type, vpath, flags);\n }\n \n-static void hash_stdin_paths(const char *type, int no_filters, unsigned flags,\n-\t\t\t     int literally)\n+static void hash_stdin_paths(const char *type, int no_filters, unsigned flags)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct strbuf unquoted = STRBUF_INIT;\n@@ -59,8 +54,7 @@ static void hash_stdin_paths(const char *type, int no_filters, unsigned flags,\n \t\t\t\tdie(\"line is badly quoted\");\n \t\t\tstrbuf_swap(&buf, &unquoted);\n \t\t}\n-\t\thash_object(buf.buf, type, no_filters ? NULL : buf.buf, flags,\n-\t\t\t    literally);\n+\t\thash_object(buf.buf, type, no_filters ? NULL : buf.buf, flags);\n \t}\n \tstrbuf_release(&buf);\n \tstrbuf_release(&unquoted);\n@@ -81,7 +75,6 @@ int cmd_hash_object(int argc,\n \tint hashstdin = 0;\n \tint stdin_paths = 0;\n \tint no_filters = 0;\n-\tint literally = 0;\n \tint nongit = 0;\n \tunsigned flags = INDEX_FORMAT_CHECK;\n \tconst char *vpath = NULL;\n@@ -93,7 +86,9 @@ int cmd_hash_object(int argc,\n \t\tOPT_COUNTUP( 0 , \"stdin\", &hashstdin, N_(\"read the object from stdin\")),\n \t\tOPT_BOOL( 0 , \"stdin-paths\", &stdin_paths, N_(\"read file names from stdin\")),\n \t\tOPT_BOOL( 0 , \"no-filters\", &no_filters, N_(\"store file as is without filters\")),\n-\t\tOPT_BOOL( 0, \"literally\", &literally, N_(\"just hash any random garbage to create corrupt objects for debugging Git\")),\n+\t\tOPT_NEGBIT( 0, \"literally\", &flags,\n+\t\t\t    N_(\"just hash any random garbage to create corrupt objects for debugging Git\"),\n+\t\t\t    INDEX_FORMAT_CHECK),\n \t\tOPT_STRING( 0 , \"path\", &vpath, N_(\"file\"), N_(\"process file as it were from this path\")),\n \t\tOPT_END()\n \t};\n@@ -139,7 +134,7 @@ int cmd_hash_object(int argc,\n \t}\n \n \tif (hashstdin)\n-\t\thash_fd(0, type, vpath, flags, literally);\n+\t\thash_fd(0, type, vpath, flags);\n \n \tfor (i = 0 ; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n@@ -148,12 +143,12 @@ int cmd_hash_object(int argc,\n \t\tif (prefix)\n \t\t\targ = to_free = prefix_filename(prefix, arg);\n \t\thash_object(arg, type, no_filters ? NULL : vpath ? vpath : arg,\n-\t\t\t    flags, literally);\n+\t\t\t    flags);\n \t\tfree(to_free);\n \t}\n \n \tif (stdin_paths)\n-\t\thash_stdin_paths(type, no_filters, flags, literally);\n+\t\thash_stdin_paths(type, no_filters, flags);\n \n \tfree(vpath_free);\n \n-- \n2.49.0.896.g93578ceaaf\n\n"},{"id":"518211","messageId":"20250516045013.GM22242@coredump.intra.peff.net","threadId":"63468","inReplyTo":"20250516044916.GA21985@coredump.intra.peff.net","subject":"[PATCH 13/13] object-file: drop support for writing objects with unknown types","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-16T04:50:13Z","receivedAt":"2025-05-16T04:50:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Since \"hash-object --literally\" no longer supports objects with unknown\ntypes, there are now no callers of write_object_file_literally() and its\nhelpers. Let's drop them to simplify the code.\n\nIn particular, this gets rid of some ugly copy-and-paste code from\nwrite_object_file_literally(), which is a parallel implementation of\nwrite_object_file(). When the split was originally made, the two weren't\nthat long, but commits like 63a6745a07 (object-file: update the loose\nobject map when writing loose objects, 2023-10-01) ended up having to\nduplicate some tricky code.\n\nThis patch drops all of that duplication and should make things less\nerror-prone going forward.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n object-file.c | 81 ++++-----------------------------------------------\n object-file.h |  5 +---\n 2 files changed, 6 insertions(+), 80 deletions(-)\n\ndiff --git a/object-file.c b/object-file.c\nindex b10e283529..1ac04c2891 100644\n--- a/object-file.c\n+++ b/object-file.c\n@@ -130,12 +130,6 @@ int has_loose_object(const struct object_id *oid)\n \treturn check_and_freshen(oid, 0);\n }\n \n-static int format_object_header_literally(char *str, size_t size,\n-\t\t\t\t\t  const char *type, size_t objsize)\n-{\n-\treturn xsnprintf(str, size, \"%s %\"PRIuMAX, type, (uintmax_t)objsize) + 1;\n-}\n-\n int format_object_header(char *str, size_t size, enum object_type type,\n \t\t\t size_t objsize)\n {\n@@ -144,7 +138,7 @@ int format_object_header(char *str, size_t size, enum object_type type,\n \tif (!name)\n \t\tBUG(\"could not get a type name for 'enum object_type' value %d\", type);\n \n-\treturn format_object_header_literally(str, size, name, objsize);\n+\treturn xsnprintf(str, size, \"%s %\"PRIuMAX, name, (uintmax_t)objsize) + 1;\n }\n \n int check_object_signature(struct repository *r, const struct object_id *oid,\n@@ -558,17 +552,6 @@ static void write_object_file_prepare(const struct git_hash_algo *algo,\n \thash_object_body(algo, &c, buf, len, oid, hdr, hdrlen);\n }\n \n-static void write_object_file_prepare_literally(const struct git_hash_algo *algo,\n-\t\t\t\t      const void *buf, unsigned long len,\n-\t\t\t\t      const char *type, struct object_id *oid,\n-\t\t\t\t      char *hdr, int *hdrlen)\n-{\n-\tstruct git_hash_ctx c;\n-\n-\t*hdrlen = format_object_header_literally(hdr, *hdrlen, type, len);\n-\thash_object_body(algo, &c, buf, len, oid, hdr, hdrlen);\n-}\n-\n #define CHECK_COLLISION_DEST_VANISHED -2\n \n static int check_collision(const char *source, const char *dest)\n@@ -698,21 +681,14 @@ int finalize_object_file_flags(const char *tmpfile, const char *filename,\n \treturn 0;\n }\n \n-static void hash_object_file_literally(const struct git_hash_algo *algo,\n-\t\t\t\t       const void *buf, unsigned long len,\n-\t\t\t\t       const char *type, struct object_id *oid)\n-{\n-\tchar hdr[MAX_HEADER_LEN];\n-\tint hdrlen = sizeof(hdr);\n-\n-\twrite_object_file_prepare_literally(algo, buf, len, type, oid, hdr, &hdrlen);\n-}\n-\n void hash_object_file(const struct git_hash_algo *algo, const void *buf,\n \t\t      unsigned long len, enum object_type type,\n \t\t      struct object_id *oid)\n {\n-\thash_object_file_literally(algo, buf, len, type_name(type), oid);\n+\tchar hdr[MAX_HEADER_LEN];\n+\tint hdrlen = sizeof(hdr);\n+\n+\twrite_object_file_prepare(algo, buf, len, type, oid, hdr, &hdrlen);\n }\n \n /* Finalize a file on disk, and close it. */\n@@ -1114,53 +1090,6 @@ int write_object_file_flags(const void *buf, unsigned long len,\n \treturn 0;\n }\n \n-int write_object_file_literally(const void *buf, unsigned long len,\n-\t\t\t\tconst char *type, struct object_id *oid,\n-\t\t\t\tunsigned flags)\n-{\n-\tchar *header;\n-\tstruct repository *repo = the_repository;\n-\tconst struct git_hash_algo *algo = repo->hash_algo;\n-\tconst struct git_hash_algo *compat = repo->compat_hash_algo;\n-\tstruct object_id compat_oid;\n-\tint hdrlen, status = 0;\n-\tint compat_type = -1;\n-\n-\tif (compat) {\n-\t\tcompat_type = type_from_string_gently(type, -1, 1);\n-\t\tif (compat_type == OBJ_BLOB)\n-\t\t\thash_object_file(compat, buf, len, compat_type,\n-\t\t\t\t\t &compat_oid);\n-\t\telse if (compat_type != -1) {\n-\t\t\tstruct strbuf converted = STRBUF_INIT;\n-\t\t\tconvert_object_file(the_repository,\n-\t\t\t\t\t    &converted, algo, compat,\n-\t\t\t\t\t    buf, len, compat_type, 0);\n-\t\t\thash_object_file(compat, converted.buf, converted.len,\n-\t\t\t\t\t compat_type, &compat_oid);\n-\t\t\tstrbuf_release(&converted);\n-\t\t}\n-\t}\n-\n-\t/* type string, SP, %lu of the length plus NUL must fit this */\n-\thdrlen = strlen(type) + MAX_HEADER_LEN;\n-\theader = xmalloc(hdrlen);\n-\twrite_object_file_prepare_literally(the_hash_algo, buf, len, type,\n-\t\t\t\t\t    oid, header, &hdrlen);\n-\n-\tif (!(flags & WRITE_OBJECT_FILE_PERSIST))\n-\t\tgoto cleanup;\n-\tif (freshen_packed_object(oid) || freshen_loose_object(oid))\n-\t\tgoto cleanup;\n-\tstatus = write_loose_object(oid, header, hdrlen, buf, len, 0, 0);\n-\tif (compat_type != -1)\n-\t\treturn repo_add_loose_object_map(repo, oid, &compat_oid);\n-\n-cleanup:\n-\tfree(header);\n-\treturn status;\n-}\n-\n int force_object_loose(const struct object_id *oid, time_t mtime)\n {\n \tstruct repository *repo = the_repository;\ndiff --git a/object-file.h b/object-file.h\nindex a979fd5e4d..6f41142452 100644\n--- a/object-file.h\n+++ b/object-file.h\n@@ -159,7 +159,7 @@ int parse_loose_header(const char *hdr, struct object_info *oi);\n \n enum {\n \t/*\n-\t * By default, `write_object_file_literally()` does not actually write\n+\t * By default, `write_object_file()` does not actually write\n \t * anything into the object store, but only computes the object ID.\n \t * This flag changes that so that the object will be written as a loose\n \t * object and persisted.\n@@ -187,9 +187,6 @@ struct input_stream {\n \tint is_finished;\n };\n \n-int write_object_file_literally(const void *buf, unsigned long len,\n-\t\t\t\tconst char *type, struct object_id *oid,\n-\t\t\t\tunsigned flags);\n int stream_loose_object(struct input_stream *in_stream, size_t len,\n \t\t\tstruct object_id *oid);\n \n-- \n2.49.0.896.g93578ceaaf\n"},{"id":"518222","messageId":"aCcK0p7wMURHppe7@pks.im","threadId":"63468","inReplyTo":"20250516044935.GB22242@coredump.intra.peff.net","subject":"Re: [PATCH 02/13] cat-file: make --allow-unknown-type a noop","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-16T09:52:18Z","receivedAt":"2025-05-16T09:52:26Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, May 16, 2025 at 12:49:35AM -0400, Jeff King wrote:\n> The cat-file command has some minor support for handling objects with\n> \"unknown\" types. I.e., strings that are not \"blob\", \"commit\", \"tree\", or\n> \"tag\".\n> \n> In theory this could be used for debugging or experimenting with\n> extensions to Git. But in practice this support is not very useful:\n> \n>   1. You can get the type and size of such objects, but nothing else.\n>      Not even the contents!\n> \n>   2. Only loose objects are supported, since packfiles use numeric ids\n>      for the types, rather than strings.\n> \n>   3. Likewise you cannot ever transfer objects between repositories,\n>      because they cannot be represented in the packfiles used for the\n>      on-the-wire protocol.\n\nAll of these are good reasons. To add one more: they would probably\nprove to be a pain for pluggable object databases, too. No need to add\nthat to the list here though given that there isn't even a design doc\nfor those yet.\n\n> The support for these unknown types complicates the object-parsing code,\n> and has led to bugs such as b748ddb7a4 (unpack_loose_header(): fix\n> infinite loop on broken zlib input, 2025-02-25). So let's drop it.\n> \n> The first step is to remove the user-facing parts, which are accessible\n> only via cat-file. This is technically backwards-incompatible, but given\n> the limitations listed above, these objects couldn't possibly be useful\n> in any workflow.\n\nI agree.\n\n> However, we can't just rip out the option entirely. That would hurt a\n> caller who ran:\n> \n>   git cat-file -t --allow-unknown-object <oid>\n> \n> and fed it normal, well-formed objects. There --allow-unknown-type was\n> doing nothing, but we wouldn't want to start bailing with an error. So\n> to protect any such callers, we'll retain --allow-unknown-type as a\n> noop.\n\nOkay, that sounds reasonable to me.\n\n> The code change is fairly small (but we'll able to clean up more code in\n> follow-on patches). The test updates drop any use of the option. We\n> still retain tests that feed the broken objects to cat-file without\n> --allow-unknown-type, as we should continue to confirm that those\n> objects are rejected. Note that in one spot we can drop a layer of loop,\n> re-indenting the body; viewing the diff with \"-w\" helps there.\n\nShouldn't we have a test though that the option is still accepted, even\nthough it doesn't do anything?\n\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  Documentation/git-cat-file.adoc |   6 +-\n>  builtin/cat-file.c              |  18 +--\n>  t/t1006-cat-file.sh             | 211 ++++++++------------------------\n>  3 files changed, 56 insertions(+), 179 deletions(-)\n> \n> diff --git a/Documentation/git-cat-file.adoc b/Documentation/git-cat-file.adoc\n> index fc4b92f104..cde79ad242 100644\n> --- a/Documentation/git-cat-file.adoc\n> +++ b/Documentation/git-cat-file.adoc\n> @@ -9,8 +9,7 @@ SYNOPSIS\n>  --------\n>  [verse]\n>  'git cat-file' <type> <object>\n> -'git cat-file' (-e | -p) <object>\n> -'git cat-file' (-t | -s) [--allow-unknown-type] <object>\n> +'git cat-file' (-e | -p | -t | -s) <object>\n>  'git cat-file' (--textconv | --filters)\n>  \t     [<rev>:<path|tree-ish> | --path=<path|tree-ish> <rev>]\n>  'git cat-file' (--batch | --batch-check | --batch-command) [--batch-all-objects]\n> @@ -202,9 +201,6 @@ flush::\n>  \tonly once, even if it is stored multiple times in the\n>  \trepository.\n>  \n> ---allow-unknown-type::\n> -\tAllow `-s` or `-t` to query broken/corrupt objects of unknown type.\n> -\n\nShould we maybe introduce a \"deprecated\" section and spell out that this\noption is a no-op nowadays that will be removed for example in Git 3.0?\n\nPatrick\n"},{"id":"518223","messageId":"aCcK2lfr8048Kh7E@pks.im","threadId":"63468","inReplyTo":"20250516044953.GF22242@coredump.intra.peff.net","subject":"Re: [PATCH 06/13] fsck: stop using object_info->type_name strbuf","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-16T09:52:26Z","receivedAt":"2025-05-16T09:52:30Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, May 16, 2025 at 12:49:53AM -0400, Jeff King wrote:\n> When fsck-ing a loose object, we use object_info's type_name strbuf to\n> record the parsed object type as a string. For most objects this is\n> redundant with the object_type enum, but it does let us report the\n> string when we encounter an object with an unknown type (for which there\n> is no matching enum value).\n> \n> There are a few downsides, though:\n> \n>   1. The code to report these cases is not actually robust. Since we did\n>      not pass a strbuf to unpack_loose_header(), we only retrieved types\n>      from headers up to 32 bytes. In longer cases, we'd simply say\n>      \"object corrupt or missing\".\n> \n>   2. This is the last caller that uses object_info's type_name strbuf\n>      support. It would be nice to refactor it so that we can simplify\n>      that code.\n> \n>   3. Likewise, we'll check the hash of the object using its unknown type\n>      (again, as long as that type is short enough). That depends on the\n>      hash_object_file_literally() code, which we'd eventually like to\n>      get rid of.\n\nOh, I'd very much welcome if this code path went away completely.\n\n> So we can simplify things by bailing immediately in read_loose_object()\n> when we encounter an unknown type. This has a few user-visible effects:\n> \n>   a. Instead of producing a single line of error output like this:\n> \n>        error: 26ed13ce3564fbbb44e35bde42c7da717ea004a6: object is of unknown type 'bogus': .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6\n> \n>      we'll now issue two lines (the first from read_loose_object() when\n>      we see the unparsable header, and the second from the fsck code,\n>      since we couldn't read the object):\n> \n>        error: unable to parse type from header 'bogus 4' of .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6\n>        error: 26ed13ce3564fbbb44e35bde42c7da717ea004a6: object corrupt or missing: .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6\n> \n>      This is a little more verbose, but this sort of error should be\n>      rare (such objects are almost impossible to work with, and cannot\n>      be transferred between repositories as they are not representable\n>      in packfiles). And as a bonus, reporting the broken header in full\n>      could help with debugging other cases (e.g., a header like \"blob\n>      xyzzy\\0\" would fail in parsing the size, but previously we'd not\n>      have showed the offending bytes).\n\nYup, I would claim this is an improvement, as well.\n\n>   b. An object with an unknown type will be reported as corrupt, without\n>      actually doing a hash check. Again, I think this is unlikely to\n>      matter in practice since such objects are totally unusable.\n\nAgreed.\n\n> We'll update one fsck test to match the new error strings. And we can\n> remove another test that covered the case of an object with an unknown\n> type _and_ a hash corruption. Since we'll skip the hash check now in\n> this case, the test is no longer interesting.\n> \n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  builtin/fsck.c  | 13 ++-----------\n>  object-file.c   | 12 +++++++++---\n>  t/t1450-fsck.sh | 29 +++--------------------------\n>  3 files changed, 14 insertions(+), 40 deletions(-)\n> \n> diff --git a/builtin/fsck.c b/builtin/fsck.c\n> index 6cac28356c..e7d96a9c8e 100644\n> --- a/builtin/fsck.c\n> +++ b/builtin/fsck.c\n> @@ -614,12 +614,11 @@ static void get_default_heads(void)\n>  struct for_each_loose_cb\n>  {\n>  \tstruct progress *progress;\n> -\tstruct strbuf obj_type;\n>  };\n>  \n> -static int fsck_loose(const struct object_id *oid, const char *path, void *data)\n> +static int fsck_loose(const struct object_id *oid, const char *path,\n> +\t\t      void *data UNUSED)\n>  {\n> -\tstruct for_each_loose_cb *cb_data = data;\n>  \tstruct object *obj;\n>  \tenum object_type type = OBJ_NONE;\n>  \tunsigned long size;\n> @@ -629,8 +628,6 @@ static int fsck_loose(const struct object_id *oid, const char *path, void *data)\n>  \tstruct object_id real_oid = *null_oid(the_hash_algo);\n>  \tint err = 0;\n>  \n> -\tstrbuf_reset(&cb_data->obj_type);\n> -\toi.type_name = &cb_data->obj_type;\n>  \toi.sizep = &size;\n>  \toi.typep = &type;\n>  \n> @@ -642,10 +639,6 @@ static int fsck_loose(const struct object_id *oid, const char *path, void *data)\n>  \t\t\terr = error(_(\"%s: object corrupt or missing: %s\"),\n>  \t\t\t\t    oid_to_hex(oid), path);\n>  \t}\n> -\tif (type != OBJ_NONE && type < 0)\n> -\t\terr = error(_(\"%s: object is of unknown type '%s': %s\"),\n> -\t\t\t    oid_to_hex(&real_oid), cb_data->obj_type.buf,\n> -\t\t\t    path);\n\nThis one is a bit curious. But because we know that we have reported\nthis error in `read_loose_object()` already we don't need to print the\nerror over here anymore.\n\n> diff --git a/object-file.c b/object-file.c\n> index 1127e154f6..7a35bde96e 100644\n> --- a/object-file.c\n> +++ b/object-file.c\n> @@ -1662,6 +1662,12 @@ int read_loose_object(const char *path,\n>  \t\tgoto out_inflate;\n>  \t}\n>  \n> +\tif (*oi->typep < 0) {\n> +\t\terror(_(\"unable to parse type from header '%s' of %s\"),\n> +\t\t      hdr, path);\n> +\t\tgoto out_inflate;\n> +\t}\n> +\n>  \tif (*oi->typep == OBJ_BLOB &&\n>  \t    *size > repo_settings_get_big_file_threshold(the_repository)) {\n>  \t\tif (check_stream_oid(&stream, hdr, *size, path, expected_oid) < 0)\n\nSo this is where we report the new error now. Makes sense.\n\nPatrick\n"},{"id":"518224","messageId":"aCcK4bZVxuO2iTMS@pks.im","threadId":"63468","inReplyTo":"20250516045002.GI22242@coredump.intra.peff.net","subject":"Re: [PATCH 09/13] t: add lib-loose.sh","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-16T09:52:33Z","receivedAt":"2025-05-16T09:52:36Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, May 16, 2025 at 12:50:02AM -0400, Jeff King wrote:\n> This commit adds a shell library for writing raw loose objects into the\n> object database. Normally this is done with hash-object, but the\n> specific intent here is to allow broken objects that hash-object may not\n> support.\n> \n> We'll convert several cases that use \"hash-object --literally\" to write\n> objects with invalid types. That works currently, but dropping this\n> dependency will allow us to remove that feature and simplify the\n> object-writing code.\n> \n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  t/lib-loose.sh                      | 30 +++++++++++++++++++++++++++++\n>  t/t1006-cat-file.sh                 |  5 +++--\n>  t/t1450-fsck.sh                     |  3 ++-\n>  t/t1512-rev-parse-disambiguation.sh |  5 +++--\n>  4 files changed, 38 insertions(+), 5 deletions(-)\n>  create mode 100644 t/lib-loose.sh\n> \n> diff --git a/t/lib-loose.sh b/t/lib-loose.sh\n> new file mode 100644\n> index 0000000000..3613631eaf\n> --- /dev/null\n> +++ b/t/lib-loose.sh\n> @@ -0,0 +1,30 @@\n> +# Support routines for hand-crafting loose objects.\n> +\n> +# Write a loose object into the odb at $1, with object type $2 and contents\n> +# from stdin. Writes the oid to stdout. Example:\n> +#\n> +#   oid=$(echo foo | loose_obj .git/objects blob)\n> +#\n> +loose_obj () {\n\nNit: I would have called this `write_loose_obj ()` to indicate that it's\nwriting an object. But ultimately doesn't matter too much, so please\nfeel free to ignore this comment.\n\n> +\tcat >tmp_loose.content &&\n> +\tsize=$(wc -c <tmp_loose.content) &&\n> +\t{\n> +\t\t# Do not quote $size here; we want the shell\n> +\t\t# to strip whitespace that \"wc\" adds on some platforms.\n> +\t\tprintf \"%s %s\\0\" \"$2\" $size &&\n> +\t\tcat tmp_loose.content\n> +\t} >tmp_loose.raw &&\n> +\n> +\toid=$(test-tool $test_hash_algo <tmp_loose.raw) &&\n> +\tsuffix=${oid#??} &&\n> +\tprefix=${oid%$suffix} &&\n> +\tdir=$1/$prefix &&\n> +\tfile=$dir/$suffix &&\n> +\n> +\ttest-tool zlib deflate <tmp_loose.raw >tmp_loose.zlib &&\n> +\tmkdir -p \"$dir\" &&\n> +\tmv tmp_loose.zlib \"$file\" &&\n> +\n> +\trm tmp_loose.raw tmp_loose.content &&\n> +\techo \"$oid\"\n> +}\n\nAll of this look sensible to me.\n\nPatrick\n"},{"id":"518225","messageId":"aCcK5iF3h2j4gQf7@pks.im","threadId":"63468","inReplyTo":"20250516045008.GK22242@coredump.intra.peff.net","subject":"Re: [PATCH 11/13] hash-object: merge HASH_* and INDEX_* flags","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-16T09:52:38Z","receivedAt":"2025-05-16T09:52:41Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, May 16, 2025 at 12:50:08AM -0400, Jeff King wrote:\n> The hash-object command has its own custom flag bits that it sets based\n> on command-line options. But since we dropped hash_literally() in the\n> previous commit, the only thing we do with those flag bits is convert\n> them directly into \"index_flags\" to pass to index_fd().\n> \n> This extra layer of indirection makes the code harder to read and reason\n> about. Let's just use the INDEX_* flags directly.\n\nHeh, coming full circle with 70c0f9db4e0 (object-file: split up concerns\nof `HASH_*` flags, 2025-04-15). But I agree, now that we have dropped\nthe `hash_literally()` function this is a sensible change.\n\nPatrick\n"},{"id":"518226","messageId":"aCcK6quAx_q28ltu@pks.im","threadId":"63468","inReplyTo":"20250516045013.GM22242@coredump.intra.peff.net","subject":"Re: [PATCH 13/13] object-file: drop support for writing objects with unknown types","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-16T09:52:42Z","receivedAt":"2025-05-16T09:52:45Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, May 16, 2025 at 12:50:13AM -0400, Jeff King wrote:\n> Since \"hash-object --literally\" no longer supports objects with unknown\n> types, there are now no callers of write_object_file_literally() and its\n> helpers. Let's drop them to simplify the code.\n> \n> In particular, this gets rid of some ugly copy-and-paste code from\n> write_object_file_literally(), which is a parallel implementation of\n> write_object_file(). When the split was originally made, the two weren't\n> that long, but commits like 63a6745a07 (object-file: update the loose\n> object map when writing loose objects, 2023-10-01) ended up having to\n> duplicate some tricky code.\n> \n> This patch drops all of that duplication and should make things less\n> error-prone going forward.\n\nJust today I was looking at this code and pondered what to do about it\nwith pluggable object databases. I started unifying those code paths,\nbut all the results looked quite ugly. I am thus very happy to see that\nit just goes away completely. Thank you for making my life easier!\n\nPatrick\n"},{"id":"518266","messageId":"xmqqcyc8wlzw.fsf@gitster.g","threadId":"63468","inReplyTo":"20250516044916.GA21985@coredump.intra.peff.net","subject":"Re: [PATCH 0/13] dropping support for non-standard object types","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-16T16:36:51Z","receivedAt":"2025-05-16T16:36: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> While fixing some bugs last month in c39e5cbaa5 (Merge branch\n> 'jk/zlib-inflate-fixes', 2025-04-15), I noted that objects with\n> non-standard types are not really usable. You can get their size and\n> type, but nothing else, not even their contents. And you can't transfer\n> them to other repositories, as packfiles have no way to represent them.\n>\n> We've had that code since 2015, but beyond using it in a few tests,\n> it's never gone anywhere. So I'd like to consider the whole direction a\n> failed experiment and rip it out, which simplifies some of the core\n> object code.\n>\n> IMHO this doesn't need to follow the breaking-change flow and wait until\n> Git 3.0, because what's there is not really usable in any useful way.\n> But others may disagree.\n\nFWIW, they weren't for exprimenting to see how feasible adding more\ntypes to the object system at all.  Rather, they were primarily to\nhelp testing how the production code reacted to unknown object type.\n\nSo I am all for removal of the support.  I didn't know its fallout\nwas this widely spread across the system to need 13 patches to\nremove all ;-)\n"},{"id":"518268","messageId":"xmqq34d4wlhz.fsf@gitster.g","threadId":"63468","inReplyTo":"20250516044935.GB22242@coredump.intra.peff.net","subject":"Re: [PATCH 02/13] cat-file: make --allow-unknown-type a noop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-16T16:47:36Z","receivedAt":"2025-05-16T16:47:38Z","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> However, we can't just rip out the option entirely. That would hurt a\n> caller who ran:\n>\n>   git cat-file -t --allow-unknown-object <oid>\n>\n> and fed it normal, well-formed objects. There --allow-unknown-type was\n> doing nothing, but we wouldn't want to start bailing with an error. So\n> to protect any such callers, we'll retain --allow-unknown-type as a\n> noop.\n\nHeh, unlike my usual self, I started reading this patch from the\nchanges before coming back to the proposed log message, and was\nwondering why we still take the option.  It is obvious when the\nreason is spelled out like the above and I fully support it.  My\nsense of backward compatibility may have deteriorated over time X-<.\n"},{"id":"518269","messageId":"xmqqy0uwv6im.fsf@gitster.g","threadId":"63468","inReplyTo":"20250516044947.GD22242@coredump.intra.peff.net","subject":"Re: [PATCH 04/13] cat-file: use type enum instead of buffer for -t option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-16T16:56:33Z","receivedAt":"2025-05-16T16:56:36Z","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> Now that we no longer support OBJECT_INFO_ALLOW_UNKNOWN_TYPE, there is\n> no need to pass a strbuf into oid_object_info_extended() to record the\n> type. The regular object_type enum is sufficient to capture all of the\n> types we will allow.\n>\n> This simplifies the code a bit, and will eventually let us drop\n> object_info's type_name strbuf support.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  builtin/cat-file.c | 13 ++++---------\n>  1 file changed, 4 insertions(+), 9 deletions(-)\n\nNice.  It is sad that it takes more to lose .type_name but we'll see\nthat happen in a later step.\n\n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index 4adc19aa29..67a5ff2b9e 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -109,7 +109,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)\n>  \tunsigned long size;\n>  \tstruct object_context obj_context = {0};\n>  \tstruct object_info oi = OBJECT_INFO_INIT;\n> -\tstruct strbuf sb = STRBUF_INIT;\n>  \tunsigned flags = OBJECT_INFO_LOOKUP_REPLACE;\n>  \tunsigned get_oid_flags =\n>  \t\tGET_OID_RECORD_PATH |\n> @@ -132,16 +131,12 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name)\n>  \tbuf = NULL;\n>  \tswitch (opt) {\n>  \tcase 't':\n> -\t\toi.type_name = &sb;\n> +\t\toi.typep = &type;\n>  \t\tif (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> -\t\t\tstrbuf_release(&sb);\n> -\t\t\tret = 0;\n> -\t\t\tgoto cleanup;\n> -\t\t}\n> -\t\tbreak;\n> +\t\tprintf(\"%s\\n\", type_name(type));\n> +\t\tret = 0;\n> +\t\tgoto cleanup;\n>  \n>  \tcase 's':\n>  \t\toi.sizep = &size;\n"},{"id":"518378","messageId":"20250519061617.GA102701@coredump.intra.peff.net","threadId":"63468","inReplyTo":"aCcK0p7wMURHppe7@pks.im","subject":"Re: [PATCH 02/13] cat-file: make --allow-unknown-type a noop","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-19T06:16:17Z","receivedAt":"2025-05-19T06:16:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 16, 2025 at 11:52:18AM +0200, Patrick Steinhardt wrote:\n\n> > The code change is fairly small (but we'll able to clean up more code in\n> > follow-on patches). The test updates drop any use of the option. We\n> > still retain tests that feed the broken objects to cat-file without\n> > --allow-unknown-type, as we should continue to confirm that those\n> > objects are rejected. Note that in one spot we can drop a layer of loop,\n> > re-indenting the body; viewing the diff with \"-w\" helps there.\n> \n> Shouldn't we have a test though that the option is still accepted, even\n> though it doesn't do anything?\n\nI dunno. It is obvious-ish from looking at the code that the option does\nnothing, so we know that it will behave the same whether it is provided\nor not. I guess it depends on how white/black-box we want our tests to\nbe.\n\nIf we did this on top:\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 67a5ff2b9e..ff92b14201 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -1022,7 +1022,6 @@ int cmd_cat_file(int argc,\n \tstruct batch_options batch = {\n \t\t.objects_filter = LIST_OBJECTS_FILTER_INIT,\n \t};\n-\tint unknown_type = 0;\n \tint input_nul_terminated = 0;\n \tint nul_terminated = 0;\n \tint ret;\n@@ -1047,8 +1046,7 @@ int cmd_cat_file(int argc,\n \t\tOPT_GROUP(N_(\"Emit [broken] object attributes\")),\n \t\tOPT_CMDMODE('t', NULL, &opt, N_(\"show object type (one of 'blob', 'tree', 'commit', 'tag', ...)\"), 't'),\n \t\tOPT_CMDMODE('s', NULL, &opt, N_(\"show object size\"), 's'),\n-\t\tOPT_HIDDEN_BOOL(0, \"allow-unknown-type\", &unknown_type,\n-\t\t\t  N_(\"historical option -- no-op\")),\n+\t\tOPT_NOOP_NOARG(0, \"allow-unknown-type\"),\n \t\tOPT_BOOL(0, \"use-mailmap\", &use_mailmap, N_(\"use mail map file\")),\n \t\tOPT_ALIAS(0, \"mailmap\", \"use-mailmap\"),\n \t\t/* Batch mode */\n\nthat would perhaps remove the \"-ish\" from \"obvious-ish\". I had left the\nflag in place because I wondered if we might want to produce a\ndeprecation warning before dropping it completely.\n\n> > @@ -202,9 +201,6 @@ flush::\n> >  \tonly once, even if it is stored multiple times in the\n> >  \trepository.\n> >  \n> > ---allow-unknown-type::\n> > -\tAllow `-s` or `-t` to query broken/corrupt objects of unknown type.\n> > -\n> \n> Should we maybe introduce a \"deprecated\" section and spell out that this\n> option is a no-op nowadays that will be removed for example in Git 3.0?\n\nI don't have a strong opinion there. It mostly seems like clutter to me\nin the manpage. In theory it could help somebody who had learned about\nthe option previously and wondered what happened. OTOH, the release\nnotes can help with that. With the patch above, \"cat-file --help-all\"\nwould also produce:\n\n     --[no-]allow-unknown-type\n                            no-op (backward compatibility)\n\nthough I don't really expect anybody to find that casually.\n\n-Peff\n"},{"id":"518379","messageId":"20250519061753.GB102701@coredump.intra.peff.net","threadId":"63468","inReplyTo":"aCcK4bZVxuO2iTMS@pks.im","subject":"Re: [PATCH 09/13] t: add lib-loose.sh","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-19T06:17:53Z","receivedAt":"2025-05-19T06:17:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 16, 2025 at 11:52:33AM +0200, Patrick Steinhardt wrote:\n\n> > +++ b/t/lib-loose.sh\n> > @@ -0,0 +1,30 @@\n> > +# Support routines for hand-crafting loose objects.\n> > +\n> > +# Write a loose object into the odb at $1, with object type $2 and contents\n> > +# from stdin. Writes the oid to stdout. Example:\n> > +#\n> > +#   oid=$(echo foo | loose_obj .git/objects blob)\n> > +#\n> > +loose_obj () {\n> \n> Nit: I would have called this `write_loose_obj ()` to indicate that it's\n> writing an object. But ultimately doesn't matter too much, so please\n> feel free to ignore this comment.\n\nI named it this way to match \"pack_obj\" in lib-pack.sh. But that one is\nprobably my doing, too, and arguably both should be named\nwrite_foo_obj(). ;)\n\nFor such a little corner of the test suite I agree it probably doesn't\nmatter too much.\n\n-Peff\n"},{"id":"518389","messageId":"aCrcRZ1sv-YCfLLA@pks.im","threadId":"63468","inReplyTo":"20250519061617.GA102701@coredump.intra.peff.net","subject":"Re: [PATCH 02/13] cat-file: make --allow-unknown-type a noop","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-05-19T07:22:45Z","receivedAt":"2025-05-19T07:22:48Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, May 19, 2025 at 02:16:17AM -0400, Jeff King wrote:\n> On Fri, May 16, 2025 at 11:52:18AM +0200, Patrick Steinhardt wrote:\n> \n> > > The code change is fairly small (but we'll able to clean up more code in\n> > > follow-on patches). The test updates drop any use of the option. We\n> > > still retain tests that feed the broken objects to cat-file without\n> > > --allow-unknown-type, as we should continue to confirm that those\n> > > objects are rejected. Note that in one spot we can drop a layer of loop,\n> > > re-indenting the body; viewing the diff with \"-w\" helps there.\n> > \n> > Shouldn't we have a test though that the option is still accepted, even\n> > though it doesn't do anything?\n> \n> I dunno. It is obvious-ish from looking at the code that the option does\n> nothing, so we know that it will behave the same whether it is provided\n> or not. I guess it depends on how white/black-box we want our tests to\n> be.\n> \n> If we did this on top:\n> \n> diff --git a/builtin/cat-file.c b/builtin/cat-file.c\n> index 67a5ff2b9e..ff92b14201 100644\n> --- a/builtin/cat-file.c\n> +++ b/builtin/cat-file.c\n> @@ -1022,7 +1022,6 @@ int cmd_cat_file(int argc,\n>  \tstruct batch_options batch = {\n>  \t\t.objects_filter = LIST_OBJECTS_FILTER_INIT,\n>  \t};\n> -\tint unknown_type = 0;\n>  \tint input_nul_terminated = 0;\n>  \tint nul_terminated = 0;\n>  \tint ret;\n> @@ -1047,8 +1046,7 @@ int cmd_cat_file(int argc,\n>  \t\tOPT_GROUP(N_(\"Emit [broken] object attributes\")),\n>  \t\tOPT_CMDMODE('t', NULL, &opt, N_(\"show object type (one of 'blob', 'tree', 'commit', 'tag', ...)\"), 't'),\n>  \t\tOPT_CMDMODE('s', NULL, &opt, N_(\"show object size\"), 's'),\n> -\t\tOPT_HIDDEN_BOOL(0, \"allow-unknown-type\", &unknown_type,\n> -\t\t\t  N_(\"historical option -- no-op\")),\n> +\t\tOPT_NOOP_NOARG(0, \"allow-unknown-type\"),\n>  \t\tOPT_BOOL(0, \"use-mailmap\", &use_mailmap, N_(\"use mail map file\")),\n>  \t\tOPT_ALIAS(0, \"mailmap\", \"use-mailmap\"),\n>  \t\t/* Batch mode */\n> \n> that would perhaps remove the \"-ish\" from \"obvious-ish\". I had left the\n> flag in place because I wondered if we might want to produce a\n> deprecation warning before dropping it completely.\n\nFair. I don't have a strong opinion here.\n\n> > > @@ -202,9 +201,6 @@ flush::\n> > >  \tonly once, even if it is stored multiple times in the\n> > >  \trepository.\n> > >  \n> > > ---allow-unknown-type::\n> > > -\tAllow `-s` or `-t` to query broken/corrupt objects of unknown type.\n> > > -\n> > \n> > Should we maybe introduce a \"deprecated\" section and spell out that this\n> > option is a no-op nowadays that will be removed for example in Git 3.0?\n> \n> I don't have a strong opinion there. It mostly seems like clutter to me\n> in the manpage. In theory it could help somebody who had learned about\n> the option previously and wondered what happened. OTOH, the release\n> notes can help with that. With the patch above, \"cat-file --help-all\"\n> would also produce:\n> \n>      --[no-]allow-unknown-type\n>                             no-op (backward compatibility)\n> \n> though I don't really expect anybody to find that casually.\n\nWell, the use case I'm worried about is person X that inherits a script\nthat uses this option. They may wonder what the option does, but if we\ndon't mention it at all in the man page they won't have a chance to\nlearn that it is a no-op without digging into Git's history.\n\nPatrick\n"},{"id":"518420","messageId":"xmqqy0uspthe.fsf@gitster.g","threadId":"63468","inReplyTo":"20250516044953.GF22242@coredump.intra.peff.net","subject":"Re: [PATCH 06/13] fsck: stop using object_info->type_name strbuf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-19T14:26:05Z","receivedAt":"2025-05-19T14:26:08Z","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> So we can simplify things by bailing immediately in read_loose_object()\n> when we encounter an unknown type. This has a few user-visible effects:\n>\n>   a. Instead of producing a single line of error output like this:\n>\n>        error: 26ed13ce3564fbbb44e35bde42c7da717ea004a6: object is of unknown type 'bogus': .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6\n>\n>      we'll now issue two lines (the first from read_loose_object() when\n>      we see the unparsable header, and the second from the fsck code,\n>      since we couldn't read the object):\n>\n>        error: unable to parse type from header 'bogus 4' of .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6\n>        error: 26ed13ce3564fbbb44e35bde42c7da717ea004a6: object corrupt or missing: .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6\n\nJust curious; is the difference between 'bogus' and 'bogus 4'\nsignificant in the above examples?\n\n>\n>      This is a little more verbose, but this sort of error should be\n>      rare (such objects are almost impossible to work with, and cannot\n>      be transferred between repositories as they are not representable\n>      in packfiles). And as a bonus, reporting the broken header in full\n>      could help with debugging other cases (e.g., a header like \"blob\n>      xyzzy\\0\" would fail in parsing the size, but previously we'd not\n>      have showed the offending bytes).\n\nYup, I do not think anybody minds the message that has overly long\nobject name toward the end, instead of the beginning, of line,\neither.\n\n>   b. An object with an unknown type will be reported as corrupt, without\n>      actually doing a hash check. Again, I think this is unlikely to\n>      matter in practice since such objects are totally unusable.\n\nAgreed.\n\n> We'll update one fsck test to match the new error strings. And we can\n> remove another test that covered the case of an object with an unknown\n> type _and_ a hash corruption. Since we'll skip the hash check now in\n> this case, the test is no longer interesting.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  builtin/fsck.c  | 13 ++-----------\n>  object-file.c   | 12 +++++++++---\n>  t/t1450-fsck.sh | 29 +++--------------------------\n>  3 files changed, 14 insertions(+), 40 deletions(-)\n\nPleasant.\n\nThanks.\n"},{"id":"518421","messageId":"xmqqtt5gpryr.fsf@gitster.g","threadId":"63468","inReplyTo":"20250516044956.GG22242@coredump.intra.peff.net","subject":"Re: [PATCH 07/13] oid_object_info(): drop type_name strbuf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-19T14:58:52Z","receivedAt":"2025-05-19T14:58:55Z","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> We provide a mechanism for callers to get the object type as a raw\n> string, rather than an object_type enum. This was in theory useful for\n> returning types that are not representable in the enum, but we consider\n> any such type to be an error, and there are no callers that use the\n> strbuf anymore.\n>\n> Let's drop support to simplify the code a bit.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  object-file.c  | 4 +---\n>  object-store.c | 2 --\n>  object-store.h | 1 -\n>  packfile.c     | 7 +------\n>  4 files changed, 2 insertions(+), 12 deletions(-)\n\nYup, now we have fixed vocabulary, we can pass around the enum and\nmap them into fixed strings on demand.  Nice.\n"},{"id":"518422","messageId":"xmqqo6voprrk.fsf@gitster.g","threadId":"63468","inReplyTo":"20250516044959.GH22242@coredump.intra.peff.net","subject":"Re: [PATCH 08/13] t/helper: add zlib test-tool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-19T15:03:11Z","receivedAt":"2025-05-19T15:03:14Z","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> So let's introduce a short test helper for just doing zlib operations.\n\nOooh. I remember that I did want to use something like this from\ntime to time.  Nice.\n\n> +static void do_zlib(struct git_zstream *stream,\n> +\t\t    int (*zlib_func)(git_zstream *, int),\n> +\t\t    int fd_in, int fd_out)\n> +{\n> +\tstruct strbuf buf_in = STRBUF_INIT;\n> +\tint status = Z_OK;\n> +\n> +\tif (strbuf_read(&buf_in, fd_in, 0) < 0)\n> +\t\tdie_errno(\"read error\");\n> +\n> +\tstream->next_in = (unsigned char *)buf_in.buf;\n> +\tstream->avail_in = buf_in.len;\n> +\n> +\twhile (status == Z_OK ||\n> +\t       (status == Z_BUF_ERROR && !stream->avail_out)) {\n> +\t\tunsigned char buf_out[4096];\n> +\n> +\t\tstream->next_out = buf_out;\n> +\t\tstream->avail_out = sizeof(buf_out);\n> +\n> +\t\tstatus = zlib_func(stream, Z_FINISH);\n> +\t\tif (write_in_full(fd_out, buf_out,\n> +\t\t\t\t  sizeof(buf_out) - stream->avail_out) < 0)\n> +\t\t\tdie_errno(\"write error\");\n> +\t}\n\nEven though I may have written this as do {} while() loop, I do not\nmind a while () loop that depends on status being initialized to\nZ_OK.\n\n> +\tif (status != Z_STREAM_END)\n> +\t\tdie(\"zlib error %d\", status);\n> +\n> +\tstrbuf_release(&buf_in);\n> +}\n\nLooking good.\n\n\n> +int cmd__zlib(int argc, const char **argv)\n> +{\n> +\tgit_zstream stream;\n> +\n> +\tif (argc != 2)\n> +\t\tusage(zlib_usage);\n> +\n> +\tmemset(&stream, 0, sizeof(stream));\n> +\n> +\tif (!strcmp(argv[1], \"inflate\")) {\n> +\t\tgit_inflate_init(&stream);\n> +\t\tdo_zlib(&stream, git_inflate, 0, 1);\n> +\t\tgit_inflate_end(&stream);\n> +\t} else if (!strcmp(argv[1], \"deflate\")) {\n> +\t\tgit_deflate_init(&stream, Z_DEFAULT_COMPRESSION);\n> +\t\tdo_zlib(&stream, git_deflate, 0, 1);\n> +\t\tgit_deflate_end(&stream);\n> +\t} else {\n> +\t\terror(\"unknown mode: %s\", argv[1]);\n> +\t\tusage(zlib_usage);\n> +\t}\n> +\n> +\treturn 0;\n> +}\n"},{"id":"518423","messageId":"xmqqh61gprbe.fsf@gitster.g","threadId":"63468","inReplyTo":"20250516045002.GI22242@coredump.intra.peff.net","subject":"Re: [PATCH 09/13] t: add lib-loose.sh","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-19T15:12:53Z","receivedAt":"2025-05-19T15:12:55Z","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> +# Write a loose object into the odb at $1, with object type $2 and contents\n> +# from stdin. Writes the oid to stdout. Example:\n> +#\n> +#   oid=$(echo foo | loose_obj .git/objects blob)\n> +#\n> +loose_obj () {\n> +\tcat >tmp_loose.content &&\n> +\tsize=$(wc -c <tmp_loose.content) &&\n> +\t{\n> +\t\t# Do not quote $size here; we want the shell\n> +\t\t# to strip whitespace that \"wc\" adds on some platforms.\n> +\t\tprintf \"%s %s\\0\" \"$2\" $size &&\n\nNice to have this comment.\n\nWe probably could also do\n\n\t\tprintf \"%s %d\\0\" \"$2\" \"$size\"\n\nto cope with possible leading space padding, but your version makes\nthe intent a lot clearer.\n\n> +\t\tcat tmp_loose.content\n> +\t} >tmp_loose.raw &&\n\nOK, a loose object file is a deflated bytestream of object header\nfollowed by the payload, and because the header records the size\nof the payload, we need to read the payload to its end first.\n\n> +\toid=$(test-tool $test_hash_algo <tmp_loose.raw) &&\n\nAnd hashing the raw gives us the object name.\n\n> +\tsuffix=${oid#??} &&\n> +\tprefix=${oid%$suffix} &&\n> +\tdir=$1/$prefix &&\n> +\tfile=$dir/$suffix &&\n\nOK.\n\n> +\ttest-tool zlib deflate <tmp_loose.raw >tmp_loose.zlib &&\n> +\tmkdir -p \"$dir\" &&\n> +\tmv tmp_loose.zlib \"$file\" &&\n\nAh, you are being very careful.  As this is merely a test, we\nprobably could do without tmp_loose.zlib but I do not mind it being\ncareful.\n\n> +\trm tmp_loose.raw tmp_loose.content &&\n> +\techo \"$oid\"\n> +}\n> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh\n> index d96d02ad7d..317da6869c 100755\n> --- a/t/t1006-cat-file.sh\n> +++ b/t/t1006-cat-file.sh\n> @@ -3,6 +3,7 @@\n>  test_description='git cat-file'\n>  \n>  . ./test-lib.sh\n> +. \"$TEST_DIRECTORY/lib-loose.sh\"\n>  \n>  test_cmdmode_usage () {\n>  \ttest_expect_code 129 \"$@\" 2>err &&\n> @@ -657,12 +658,12 @@ test_expect_success 'setup bogus data' '\n>  \tbogus_short_type=\"bogus\" &&\n>  \tbogus_short_content=\"bogus\" &&\n>  \tbogus_short_size=$(strlen \"$bogus_short_content\") &&\n> -\tbogus_short_oid=$(echo_without_newline \"$bogus_short_content\" | git hash-object -t $bogus_short_type --literally -w --stdin) &&\n> +\tbogus_short_oid=$(echo_without_newline \"$bogus_short_content\" | loose_obj .git/objects $bogus_short_type) &&\n>  \n>  \tbogus_long_type=\"abcdefghijklmnopqrstuvwxyz1234679\" &&\n>  \tbogus_long_content=\"bogus\" &&\n>  \tbogus_long_size=$(strlen \"$bogus_long_content\") &&\n> -\tbogus_long_oid=$(echo_without_newline \"$bogus_long_content\" | git hash-object -t $bogus_long_type --literally -w --stdin)\n> +\tbogus_long_oid=$(echo_without_newline \"$bogus_long_content\" | loose_obj .git/objects $bogus_long_type)\n>  '\n\nNice.\n"},{"id":"518424","messageId":"xmqqcyc4pr75.fsf@gitster.g","threadId":"63468","inReplyTo":"20250516045005.GJ22242@coredump.intra.peff.net","subject":"Re: [PATCH 10/13] hash-object: stop allowing unknown types","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-19T15:15:26Z","receivedAt":"2025-05-19T15:15:29Z","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> We will retain the --literally flag, as it also instructs hash-object\n> not to worry about other format issues (e.g., type-specific things that\n> fsck would complain about). The documentation does not need to be\n> updated, as it was always vague about which checks we're loosening (it\n> uses only the phrase \"any garbage\").\n\n;-)\n\n> +test_expect_success '--literally complains about non-standard types' '\n> +\ttest_must_fail git hash-object -t bogus --literally --stdin\n>  '\n\nYup, we obviously want to fail this now.\n\n"},{"id":"518425","messageId":"xmqq5xhwpqhh.fsf@gitster.g","threadId":"63468","inReplyTo":"20250516045010.GL22242@coredump.intra.peff.net","subject":"Re: [PATCH 12/13] hash-object: handle --literally with OPT_NEGBIT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-19T15:30:50Z","receivedAt":"2025-05-19T15:30: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> Since we recently removed the hash_literally() function, the hash-object\n> --literally option has been simplified to just removing the\n> INDEX_FORMAT_CHECK flag. Rather than pass it around as a separate bool,\n> we can just have the option parser remove the bit from the set of flags\n> directly. This simplifies the helper functions.\n\nInteresting.\n\nI never anticipated this coming after reading the previous steps,\nbut when presented, it is so obvious a thing to do.\n\nIndeed, the code path to hash_fd() gets simplified quite a lot.\n\nThanks.\n"},{"id":"518426","messageId":"xmqq1pskpqdw.fsf@gitster.g","threadId":"63468","inReplyTo":"20250516045013.GM22242@coredump.intra.peff.net","subject":"Re: [PATCH 13/13] object-file: drop support for writing objects with unknown types","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-19T15:32:59Z","receivedAt":"2025-05-19T15:33:01Z","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> Since \"hash-object --literally\" no longer supports objects with unknown\n> types, there are now no callers of write_object_file_literally() and its\n> helpers. Let's drop them to simplify the code.\n>\n> In particular, this gets rid of some ugly copy-and-paste code from\n> write_object_file_literally(), which is a parallel implementation of\n> write_object_file(). When the split was originally made, the two weren't\n> that long, but commits like 63a6745a07 (object-file: update the loose\n> object map when writing loose objects, 2023-10-01) ended up having to\n> duplicate some tricky code.\n>\n> This patch drops all of that duplication and should make things less\n> error-prone going forward.\n\nGood.  Creating broken loose object for the purpose of testing was\nthe only reason to have this \"feature\", and that is reimplemented in\na shell script in the test suite, so this can safely go.  Nice.\n\nThanks.\n"},{"id":"518438","messageId":"20250519170032.GA772062@coredump.intra.peff.net","threadId":"63468","inReplyTo":"xmqqy0uspthe.fsf@gitster.g","subject":"Re: [PATCH 06/13] fsck: stop using object_info->type_name strbuf","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-19T17:00:32Z","receivedAt":"2025-05-19T17:00:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 19, 2025 at 07:26:05AM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > So we can simplify things by bailing immediately in read_loose_object()\n> > when we encounter an unknown type. This has a few user-visible effects:\n> >\n> >   a. Instead of producing a single line of error output like this:\n> >\n> >        error: 26ed13ce3564fbbb44e35bde42c7da717ea004a6: object is of unknown type 'bogus': .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6\n> >\n> >      we'll now issue two lines (the first from read_loose_object() when\n> >      we see the unparsable header, and the second from the fsck code,\n> >      since we couldn't read the object):\n> >\n> >        error: unable to parse type from header 'bogus 4' of .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6\n> >        error: 26ed13ce3564fbbb44e35bde42c7da717ea004a6: object corrupt or missing: .git/objects/26/ed13ce3564fbbb44e35bde42c7da717ea004a6\n> \n> Just curious; is the difference between 'bogus' and 'bogus 4'\n> significant in the above examples?\n\nThe \"4\" is the size of the object. Knowing me, it was almost certainly\n\"foo\\n\". ;)\n\nIn the original, we had parsed the type name into its own strbuf, so the\nerror message reported that. Now we just get OBJ_BAD, but we still have\nthe full header sitting in a buffer (we unpack_loose_header() to zlib\ninflate up until the NUL byte, and then we parse_loose_header() to\nactually interpret it).\n\nWe _could_ do something like:\n\n  p = strchrnul(hdr, ' ');\n  *p = '\\0'; /* truncate header at end of type */\n  error(\"bad type: %s\", hdr);\n\nto just print \"bogus\". But showing the whole header is less work, and\narguably could be more informative, depending on exactly how it's\nmalformed.\n\nNote that we _are_ guaranteed to have a NUL byte, or else\nunpack_loose_header() would have complained and we'd have said:\n\n  error: unable to unpack header of .git/objects/...\n\nThat's what you see if you put in a very long type name (we only inflate\nup to MAX_HEADER_LEN, so we never find the NUL).\n\n-Peff\n"},{"id":"518439","messageId":"20250519170335.GB772062@coredump.intra.peff.net","threadId":"63468","inReplyTo":"xmqqo6voprrk.fsf@gitster.g","subject":"Re: [PATCH 08/13] t/helper: add zlib test-tool","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-05-19T17:03:35Z","receivedAt":"2025-05-19T17:03:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 19, 2025 at 08:03:11AM -0700, Junio C Hamano wrote:\n\n> > +static void do_zlib(struct git_zstream *stream,\n> > +\t\t    int (*zlib_func)(git_zstream *, int),\n> > +\t\t    int fd_in, int fd_out)\n> > +{\n> > +\tstruct strbuf buf_in = STRBUF_INIT;\n> > +\tint status = Z_OK;\n> > +\n> > +\tif (strbuf_read(&buf_in, fd_in, 0) < 0)\n> > +\t\tdie_errno(\"read error\");\n> > +\n> > +\tstream->next_in = (unsigned char *)buf_in.buf;\n> > +\tstream->avail_in = buf_in.len;\n> > +\n> > +\twhile (status == Z_OK ||\n> > +\t       (status == Z_BUF_ERROR && !stream->avail_out)) {\n> > +\t\tunsigned char buf_out[4096];\n> > +\n> > +\t\tstream->next_out = buf_out;\n> > +\t\tstream->avail_out = sizeof(buf_out);\n> > +\n> > +\t\tstatus = zlib_func(stream, Z_FINISH);\n> > +\t\tif (write_in_full(fd_out, buf_out,\n> > +\t\t\t\t  sizeof(buf_out) - stream->avail_out) < 0)\n> > +\t\t\tdie_errno(\"write error\");\n> > +\t}\n> \n> Even though I may have written this as do {} while() loop, I do not\n> mind a while () loop that depends on status being initialized to\n> Z_OK.\n\nHmm, yeah. I agree that is slightly nicer, but probably not worth caring\nabout too much.\n\nI thought at first there might be an opportunity to also simplify some\nof the assignments, similar to 03e7c454e9 (unpack_loose_header():\nsimplify next_out assignment, 2025-02-25), but I don't think so (in that\ncommit the complication was that we inflated a little before hitting the\nloop).\n\n-Peff\n"},{"id":"518574","messageId":"xmqqbjrmdqoe.fsf@gitster.g","threadId":"63468","inReplyTo":"20250519170335.GB772062@coredump.intra.peff.net","subject":"Re: [PATCH 08/13] t/helper: add zlib test-tool","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-21T13:44:17Z","receivedAt":"2025-05-21T13:44:20Z","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 thought at first there might be an opportunity to also simplify some\n> of the assignments, similar to 03e7c454e9 (unpack_loose_header():\n> simplify next_out assignment, 2025-02-25), but I don't think so (in that\n> commit the complication was that we inflated a little before hitting the\n> loop).\n\nYeah, you're right.  This one was a bit simpler that everything is\ndone inside the loop.\n"}]}