{"thread":{"id":"62767","subject":"[PATCH] rev-list: print missing object type with --missing=print-type","startedAt":"2025-01-08T03:43:02Z","lastAt":"2025-02-11T16:51:53Z","messageCount":37,"participants":["Justin Tobler","Christian Couder","Junio C Hamano","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"510141","messageId":"20250108034012.211043-1-jltobler@gmail.com","threadId":"62767","inReplyTo":null,"subject":"[PATCH] rev-list: print missing object type with --missing=print-type","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-01-08T03:40:12Z","receivedAt":"2025-01-08T03:43:02Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Handling of missing objects encounted by git-rev-list(1) can be\nconfigured with the `--missing=<action>` option and specifying the\ndesired action. Of the available missing actions, none provide a way to\nprint additional information about the missing object such as its type.\n\nAdd a new missing action called `print-type`. Similar to `print`, this\naction prints a list of missing objects but also includes the object\ntype if available in the form: `?<oid> [type]`.\n\nSigned-off-by: Justin Tobler <jltobler@gmail.com>\n---\nInstead of adding an additional missing action type for the explicit\npurpose of printing type info, I also considered a separate option\n(something like `--object-types`, or maybe a `--missing-format`) that\ncould be used in combination with the `--missing=print` option to\nachieve the same result. The main intent is for the missing object type\nand I wasn't sure if type info would be useful in the general case so I\nopted to choose the former approach.\n\nThanks,\n-Justin\n---\n Documentation/rev-list-options.txt |  3 ++\n builtin/rev-list.c                 | 74 +++++++++++++++++++++++++-----\n t/t6022-rev-list-missing.sh        | 13 +++++-\n 3 files changed, 77 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 459e5a02f5..277a0b645e 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -1024,6 +1024,9 @@ Unexpected missing objects will raise an error.\n The form '--missing=print' is like 'allow-any', but will also print a\n list of the missing objects.  Object IDs are prefixed with a ``?'' character.\n +\n+The form '--missing=print-type' is like 'print', but will also print the\n+missing object type information if available in the form `?<oid> [type]`.\n++\n If some tips passed to the traversal are missing, they will be\n considered as missing too, and the traversal will ignore them. In case\n we cannot get their Object ID though, an error will be raised.\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 3196da7b2d..ee473da52c 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -22,6 +22,7 @@\n #include \"progress.h\"\n #include \"reflog-walk.h\"\n #include \"oidset.h\"\n+#include \"oidmap.h\"\n #include \"packfile.h\"\n \n static const char rev_list_usage[] =\n@@ -73,11 +74,16 @@ static unsigned progress_counter;\n static struct oidset omitted_objects;\n static int arg_print_omitted; /* print objects omitted by filter */\n \n-static struct oidset missing_objects;\n+struct missing_objects_map_entry {\n+\tstruct oidmap_entry entry;\n+\tunsigned type : TYPE_BITS;\n+};\n+static struct oidmap missing_objects;\n enum missing_action {\n \tMA_ERROR = 0,    /* fail if any missing objects are encountered */\n \tMA_ALLOW_ANY,    /* silently allow ALL missing objects */\n \tMA_PRINT,        /* print ALL missing objects in special section */\n+\tMA_PRINT_TYPE,   /* same as MA_PRINT but includes available type info */\n \tMA_ALLOW_PROMISOR, /* silently allow all missing PROMISOR objects */\n };\n static enum missing_action arg_missing_action;\n@@ -103,6 +109,8 @@ static off_t get_object_disk_usage(struct object *obj)\n \n static inline void finish_object__ma(struct object *obj)\n {\n+\tstruct missing_objects_map_entry *entry, *old;\n+\n \t/*\n \t * Whether or not we try to dynamically fetch missing objects\n \t * from the server, we currently DO NOT have the object.  We\n@@ -119,7 +127,12 @@ static inline void finish_object__ma(struct object *obj)\n \t\treturn;\n \n \tcase MA_PRINT:\n-\t\toidset_insert(&missing_objects, &obj->oid);\n+\tcase MA_PRINT_TYPE:\n+\t\tCALLOC_ARRAY(entry, 1);\n+\t\tentry->entry.oid = obj->oid;\n+\t\tentry->type = obj->type;\n+\t\told = oidmap_put(&missing_objects, entry);\n+\t\tfree(old);\n \t\treturn;\n \n \tcase MA_ALLOW_PROMISOR:\n@@ -414,6 +427,12 @@ static inline int parse_missing_action_value(const char *value)\n \t\treturn 1;\n \t}\n \n+\tif (!strcmp(value, \"print-type\")) {\n+\t\targ_missing_action = MA_PRINT_TYPE;\n+\t\tfetch_if_missing = 0;\n+\t\treturn 1;\n+\t}\n+\n \tif (!strcmp(value, \"allow-promisor\")) {\n \t\targ_missing_action = MA_ALLOW_PROMISOR;\n \t\tfetch_if_missing = 0;\n@@ -781,10 +800,26 @@ int cmd_rev_list(int argc,\n \n \tif (arg_print_omitted)\n \t\toidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);\n-\tif (arg_missing_action == MA_PRINT) {\n-\t\toidset_init(&missing_objects, DEFAULT_OIDSET_SIZE);\n-\t\t/* Add missing tips */\n-\t\toidset_insert_from_set(&missing_objects, &revs.missing_commits);\n+\tif (arg_missing_action == MA_PRINT ||\n+\t    arg_missing_action == MA_PRINT_TYPE) {\n+\t\tstruct oidset_iter iter;\n+\t\tstruct object_id *oid;\n+\n+\t\toidmap_init(&missing_objects, DEFAULT_OIDSET_SIZE);\n+\t\toidset_iter_init(&revs.missing_commits, &iter);\n+\n+\t\t/*\n+\t\t * Revisions pointing to missing objects lack the context\n+\t\t * required to determine object type.\n+\t\t */\n+\t\twhile ((oid = oidset_iter_next(&iter))) {\n+\t\t\tstruct missing_objects_map_entry *entry;\n+\n+\t\t\tCALLOC_ARRAY(entry, 1);\n+\t\t\toidcpy(&entry->entry.oid, oid);\n+\t\t\toidmap_put(&missing_objects, entry);\n+\t\t}\n+\n \t\toidset_clear(&revs.missing_commits);\n \t}\n \n@@ -801,12 +836,27 @@ int cmd_rev_list(int argc,\n \t\toidset_clear(&omitted_objects);\n \t}\n \tif (arg_missing_action == MA_PRINT) {\n-\t\tstruct oidset_iter iter;\n-\t\tstruct object_id *oid;\n-\t\toidset_iter_init(&missing_objects, &iter);\n-\t\twhile ((oid = oidset_iter_next(&iter)))\n-\t\t\tprintf(\"?%s\\n\", oid_to_hex(oid));\n-\t\toidset_clear(&missing_objects);\n+\t\tstruct missing_objects_map_entry *entry;\n+\t\tstruct oidmap_iter iter;\n+\n+\t\toidmap_iter_init(&missing_objects, &iter);\n+\n+\t\twhile ((entry = oidmap_iter_next(&iter)))\n+\t\t\tprintf(\"?%s\\n\", oid_to_hex(&entry->entry.oid));\n+\n+\t\toidmap_free(&missing_objects, true);\n+\t}\n+\tif (arg_missing_action == MA_PRINT_TYPE) {\n+\t\tstruct missing_objects_map_entry *entry;\n+\t\tstruct oidmap_iter iter;\n+\n+\t\toidmap_iter_init(&missing_objects, &iter);\n+\n+\t\twhile ((entry = oidmap_iter_next(&iter)))\n+\t\t\tprintf(\"?%s %s\\n\", oid_to_hex(&entry->entry.oid),\n+\t\t\t       entry->type ? type_name(entry->type) : \"\");\n+\n+\t\toidmap_free(&missing_objects, true);\n \t}\n \n \tstop_progress(&progress);\ndiff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh\nindex 7553a9cca2..1606082d4b 100755\n--- a/t/t6022-rev-list-missing.sh\n+++ b/t/t6022-rev-list-missing.sh\n@@ -37,7 +37,7 @@ done\n \n for obj in \"HEAD~1\" \"HEAD~1^{tree}\" \"HEAD:1.t\"\n do\n-\tfor action in \"allow-any\" \"print\"\n+\tfor action in \"allow-any\" \"print\" \"print-type\"\n \tdo\n \t\ttest_expect_success \"rev-list --missing=$action with missing $obj\" '\n \t\t\toid=\"$(git rev-parse $obj)\" &&\n@@ -48,6 +48,13 @@ do\n \t\t\tgit rev-list --objects --no-object-names \\\n \t\t\t\tHEAD ^$obj >expect.raw &&\n \n+\t\t\t# When the action is to print the type, we should also\n+\t\t\t# record the expected type of the missing object.\n+\t\t\tif test \"$action\" = \"print-type\"\n+\t\t\tthen\n+\t\t\t\ttype=\"$(git cat-file -t $obj)\"\n+\t\t\tfi &&\n+\n \t\t\t# Blobs are shared by all commits, so even though a commit/tree\n \t\t\t# might be skipped, its blob must be accounted for.\n \t\t\tif test $obj != \"HEAD:1.t\"\n@@ -71,6 +78,10 @@ do\n \t\t\t\tgrep ?$oid actual.raw &&\n \t\t\t\techo ?$oid >>expect.raw\n \t\t\t\t;;\n+\t\t\tprint-type)\n+\t\t\t\tgrep \"?$oid $type\" actual.raw &&\n+\t\t\t\techo \"?$oid $type\" >>expect.raw\n+\t\t\t\t;;\n \t\t\tesac &&\n \n \t\t\tsort actual.raw >actual &&\n\nbase-commit: b74ff38af58464688b211140b90ec90598d340c6\n-- \n2.47.1\n\n"},{"id":"510151","messageId":"CAP8UFD1Q0C22TH7V7K9QZAJmLFDp+DPus1uB1O0oiAvJnWQgig@mail.gmail.com","threadId":"62767","inReplyTo":"20250108034012.211043-1-jltobler@gmail.com","subject":"Re: [PATCH] rev-list: print missing object type with --missing=print-type","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-01-08T10:08:44Z","receivedAt":"2025-01-08T10:08:57Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Jan 8, 2025 at 4:43 AM Justin Tobler <jltobler@gmail.com> wrote:\n>\n> Handling of missing objects encounted by git-rev-list(1) can be\n> configured with the `--missing=<action>` option and specifying the\n> desired action. Of the available missing actions, none provide a way to\n> print additional information about the missing object such as its type.\n\nWhat kind of additional information could we also print except for the type?\n\n> Add a new missing action called `print-type`. Similar to `print`, this\n> action prints a list of missing objects but also includes the object\n> type if available in the form: `?<oid> [type]`.\n>\n> Signed-off-by: Justin Tobler <jltobler@gmail.com>\n> ---\n> Instead of adding an additional missing action type for the explicit\n> purpose of printing type info, I also considered a separate option\n> (something like `--object-types`, or maybe a `--missing-format`) that\n> could be used in combination with the `--missing=print` option to\n> achieve the same result. The main intent is for the missing object type\n> and I wasn't sure if type info would be useful in the general case so I\n> opted to choose the former approach.\n\nIf there are many other kinds of information we could also print, then\nmaybe a `--missing-format=<format>` option would make more sense\nrather than adding `print-X`, `print-Y`, `print-X-Y`, etc. Otherwise,\nyeah, I would say that `print-type` makes sense.\n\n> @@ -103,6 +109,8 @@ static off_t get_object_disk_usage(struct object *obj)\n>\n>  static inline void finish_object__ma(struct object *obj)\n>  {\n> +       struct missing_objects_map_entry *entry, *old;\n> +\n>         /*\n>          * Whether or not we try to dynamically fetch missing objects\n>          * from the server, we currently DO NOT have the object.  We\n> @@ -119,7 +127,12 @@ static inline void finish_object__ma(struct object *obj)\n>                 return;\n>\n>         case MA_PRINT:\n> -               oidset_insert(&missing_objects, &obj->oid);\n> +       case MA_PRINT_TYPE:\n> +               CALLOC_ARRAY(entry, 1);\n> +               entry->entry.oid = obj->oid;\n> +               entry->type = obj->type;\n> +               old = oidmap_put(&missing_objects, entry);\n> +               free(old);\n>                 return;\n\nMaybe a function like:\n\nstatic void add_missing_object_entry(struct object_id *oid, unsigned int type)\n{\n    struct missing_objects_map_entry *entry, *old;\n\n    CALLOC_ARRAY(entry, 1);\n    entry->entry.oid = *oid;\n    entry->type = type;\n    old = oidmap_put(&missing_objects, entry);\n    free(old);\n}\n\nand then:\n\n    case MA_PRINT:\n    case MA_PRINT_TYPE:\n        add_missing_object_entry(&obj->oid, obj->type);\n        return;\n\ncould help keep finish_object__ma() (which is inlined) short and also\navoid some code duplication in the code below.\n\n>         case MA_ALLOW_PROMISOR:\n> @@ -414,6 +427,12 @@ static inline int parse_missing_action_value(const char *value)\n>                 return 1;\n>         }\n>\n> +       if (!strcmp(value, \"print-type\")) {\n> +               arg_missing_action = MA_PRINT_TYPE;\n> +               fetch_if_missing = 0;\n> +               return 1;\n> +       }\n> +\n>         if (!strcmp(value, \"allow-promisor\")) {\n>                 arg_missing_action = MA_ALLOW_PROMISOR;\n>                 fetch_if_missing = 0;\n> @@ -781,10 +800,26 @@ int cmd_rev_list(int argc,\n>\n>         if (arg_print_omitted)\n>                 oidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);\n> -       if (arg_missing_action == MA_PRINT) {\n> -               oidset_init(&missing_objects, DEFAULT_OIDSET_SIZE);\n> -               /* Add missing tips */\n> -               oidset_insert_from_set(&missing_objects, &revs.missing_commits);\n> +       if (arg_missing_action == MA_PRINT ||\n> +           arg_missing_action == MA_PRINT_TYPE) {\n> +               struct oidset_iter iter;\n> +               struct object_id *oid;\n> +\n> +               oidmap_init(&missing_objects, DEFAULT_OIDSET_SIZE);\n> +               oidset_iter_init(&revs.missing_commits, &iter);\n> +\n> +               /*\n> +                * Revisions pointing to missing objects lack the context\n> +                * required to determine object type.\n> +                */\n> +               while ((oid = oidset_iter_next(&iter))) {\n> +                       struct missing_objects_map_entry *entry;\n> +\n> +                       CALLOC_ARRAY(entry, 1);\n> +                       oidcpy(&entry->entry.oid, oid);\n> +                       oidmap_put(&missing_objects, entry);\n> +               }\n\nUsing the function suggested above this could be:\n\n        /*\n         * Revisions pointing to missing objects lack the context\n         * required to determine object type.\n         */\n        while ((oid = oidset_iter_next(&iter)))\n            add_missing_object_entry(oid, 0);\n\n> +\n>                 oidset_clear(&revs.missing_commits);\n>         }\n>\n> @@ -801,12 +836,27 @@ int cmd_rev_list(int argc,\n>                 oidset_clear(&omitted_objects);\n>         }\n>         if (arg_missing_action == MA_PRINT) {\n> -               struct oidset_iter iter;\n> -               struct object_id *oid;\n> -               oidset_iter_init(&missing_objects, &iter);\n> -               while ((oid = oidset_iter_next(&iter)))\n> -                       printf(\"?%s\\n\", oid_to_hex(oid));\n> -               oidset_clear(&missing_objects);\n> +               struct missing_objects_map_entry *entry;\n> +               struct oidmap_iter iter;\n> +\n> +               oidmap_iter_init(&missing_objects, &iter);\n> +\n> +               while ((entry = oidmap_iter_next(&iter)))\n> +                       printf(\"?%s\\n\", oid_to_hex(&entry->entry.oid));\n> +\n> +               oidmap_free(&missing_objects, true);\n> +       }\n> +       if (arg_missing_action == MA_PRINT_TYPE) {\n> +               struct missing_objects_map_entry *entry;\n> +               struct oidmap_iter iter;\n> +\n> +               oidmap_iter_init(&missing_objects, &iter);\n> +\n> +               while ((entry = oidmap_iter_next(&iter)))\n> +                       printf(\"?%s %s\\n\", oid_to_hex(&entry->entry.oid),\n> +                              entry->type ? type_name(entry->type) : \"\");\n> +\n> +               oidmap_free(&missing_objects, true);\n>         }\n\nMaybe a function like:\n\nstatic void print_missing_objects(unsigned int with_type)\n{\n    struct missing_objects_map_entry *entry;\n    struct oidmap_iter iter;\n\n    oidmap_iter_init(&missing_objects, &iter);\n\n    while ((entry = oidmap_iter_next(&iter)))\n        if (with_type && entry->type)\n            printf(\"?%s %s\\n\", oid_to_hex(&entry->entry.oid),\n                   type_name(entry->type));\n        else\n            printf(\"?%s\\n\", oid_to_hex(&entry->entry.oid));\n\n    oidmap_free(&missing_objects, true);\n}\n\ncould avoid some code duplication. It could also make the output a bit\ncleaner as when there is no type, there would be no space after the\noid in the output.\n\nThanks.\n"},{"id":"510160","messageId":"xmqqjzb5pc6t.fsf@gitster.g","threadId":"62767","inReplyTo":"20250108034012.211043-1-jltobler@gmail.com","subject":"Re: [PATCH] rev-list: print missing object type with --missing=print-type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-08T15:17:30Z","receivedAt":"2025-01-08T15:17:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Justin Tobler <jltobler@gmail.com> writes:\n\n> Handling of missing objects encounted by git-rev-list(1) can be\n> configured with the `--missing=<action>` option and specifying the\n> desired action. Of the available missing actions, none provide a way to\n> print additional information about the missing object such as its type.\n>\n> Add a new missing action called `print-type`. Similar to `print`, this\n> action prints a list of missing objects but also includes the object\n> type if available in the form: `?<oid> [type]`.\n\nThis part needs to explain where the type information comes from and\nwhat its significance is (see below for more details).\n\n> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\n> index 459e5a02f5..277a0b645e 100644\n> --- a/Documentation/rev-list-options.txt\n> +++ b/Documentation/rev-list-options.txt\n> @@ -1024,6 +1024,9 @@ Unexpected missing objects will raise an error.\n>  The form '--missing=print' is like 'allow-any', but will also print a\n>  list of the missing objects.  Object IDs are prefixed with a ``?'' character.\n>  +\n> +The form '--missing=print-type' is like 'print', but will also print the\n> +missing object type information if available in the form `?<oid> [type]`.\n> ++\n\nThe users need to be told what this \"type\" information really means,\nas its meaning is quite different from what \"git cat-file -t <oid>\"\nwould give them.  We do not have the object, so we are not learning\nits type from the object itself.  How much trust should the users\nput in this information, for example?\n\nThat comes back to the \"where does it come from\" that the future\nreaders of \"git log\" and reviewers need to be told by the proposed\nlog message.  Knowing the internals, I know you'd be getting it from\nthe \"containing\" objects, e.g., an object name that was found on the\n\"parent\" object header field of another commit, which is _expected_\nto be a commit, or an object name that was found in a tree entry\nwhose mode bits were 100644, which is _expected_ to be a blob, etc.\n\nThere are other places that you _could_ glean information about\n(possibly missing) objects.  An object that is found during\n\"rev-list --objects\" traversal (which is the topic of this patch\nafter all) but turned out to be missing may not just have an\nexpected type (because it was found in a tree object that we\nsuccessfully read) but also the full path to the object in the\ntop-level tree, for example.\n\nIn modern Git, there are even more places that you may be able to\nuse, like commit-graph that not just hints the object itself is a\ncommit, but what its parents are and when the commit was created.\n\nNote that I am not suggesting to implement more code to learn \"type\"\ninformation from more places than the current patch is doing.  At\nleast not in this iteration of the patch.  What I am getting at is\nthat it would help us to avoid unnecessarily limiting ourselves by\nstressing on \"type\" too much if we at least imagine what the\npossible sources of these extra pieces of information are and what\nthey could provide.\n\nAs I suspect that we would want to leave the door open for us to\nextend this later, I would perhaps suggest an output format format\nlike:\n\n    ?<object name> [<token>=<value>]...\n\nwhere <token> tells what kind of extra information it is.  I expect\nthat the initial implementation only knows about \"type\" as the\n<token>.  For future extensibility, we only need to say that under\nthe syntax:\n\n (1) How multiple attributes are shown?\n (2) How would a <value> with SP or LF in it is represented?\n\nMy suggestion is to have multiple <token>=<value> on the same line,\nwith a SP in between, and problematic bytes in <value> are quoted,\nusing cquote(). i.e. a <token>=<value> whose <value> part does not\nbegin with a double-quote ends at the first SP after it, otherwise\n<value> is taken as a C-quoted string inside a pair of double-quote.\n\nIf you are adventurous, I would not mind seeing \"path\" implemented\nas another token, since that would be fairly easily obtainable, but\nit does not have to be in the initial attempt.\n\nThanks.\n"},{"id":"510204","messageId":"onddu4fjgo2q7f6r7f6kgqctrbhshkll23z6ocsxiiqxbynap5@msq7qtj2un4w","threadId":"62767","inReplyTo":"xmqqjzb5pc6t.fsf@gitster.g","subject":"Re: [PATCH] rev-list: print missing object type with --missing=print-type","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-01-08T22:18:35Z","receivedAt":"2025-01-08T22:21:12Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/01/08 07:17AM, Junio C Hamano wrote:\n> The users need to be told what this \"type\" information really means,\n> as its meaning is quite different from what \"git cat-file -t <oid>\"\n> would give them.  We do not have the object, so we are not learning\n> its type from the object itself.  How much trust should the users\n> put in this information, for example?\n> \n> That comes back to the \"where does it come from\" that the future\n> readers of \"git log\" and reviewers need to be told by the proposed\n> log message.  Knowing the internals, I know you'd be getting it from\n> the \"containing\" objects, e.g., an object name that was found on the\n> \"parent\" object header field of another commit, which is _expected_\n> to be a commit, or an object name that was found in a tree entry\n> whose mode bits were 100644, which is _expected_ to be a blob, etc.\n\nI'll update the log message in the next version to explain how\ninformation about a missing object gets inferred. As you explained, this\nis relying on the containing object to figure this out. This is why for\nsome missing objects it may not be possible to infer the type. For\nexample, if a missing object is only reffered to by a reference, there\nis not a containing object that can be used.\n\n> There are other places that you _could_ glean information about\n> (possibly missing) objects.  An object that is found during\n> \"rev-list --objects\" traversal (which is the topic of this patch\n> after all) but turned out to be missing may not just have an\n> expected type (because it was found in a tree object that we\n> successfully read) but also the full path to the object in the\n> top-level tree, for example.\n> \n> In modern Git, there are even more places that you may be able to\n> use, like commit-graph that not just hints the object itself is a\n> commit, but what its parents are and when the commit was created.\n> \n> Note that I am not suggesting to implement more code to learn \"type\"\n> information from more places than the current patch is doing.  At\n> least not in this iteration of the patch.  What I am getting at is\n> that it would help us to avoid unnecessarily limiting ourselves by\n> stressing on \"type\" too much if we at least imagine what the\n> possible sources of these extra pieces of information are and what\n> they could provide.\n> \n> As I suspect that we would want to leave the door open for us to\n> extend this later, I would perhaps suggest an output format format\n> like:\n> \n>     ?<object name> [<token>=<value>]...\n\nI think this is a great idea. To select which attributes get printed\nwith the missing object we could add an option. Something like:\n\n  $ git rev-list --objects --missing=print \\\n  --missing-attr=path --missing-attr=type\n\nI like the idea of also adding a path attribute, but this raises a\ncouple of questions. The way `--missing=print` currently works is that\nit prints the unique set of missing object IDs. A missing object could\npossibly be referenced by multiple trees and thus have multiple valid\npaths. There are a couple of different ways this situation could be\nhandled:\n\n  - We could record each of the encounted paths for an object and print\n    out each. Something like: `?<oid> path=foo path=bar`\n\n  - Historically, `--missing=print` would only ever print a single\n    instance of the OID, but we could print a missing object with\n    multiple paths each on a separate line. Something like this:\n          \n      ?e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 path=foo\n      ?e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 path=bar\n\n  - We could keep it simple a just report a single path per missing\n    object. It doesn't capture the whole picture, but it does provide\n    some insight into the missing object.\n\nSince this missing object type is also inferred from the containing\nobject, in theory different containing objects could indicate different\ntypes for the missing object. I'm not sure though if this is a scenario\nworth accounting for. Maybe it would be fine to rely on a single\ncontaining object to provide the type and assume it is consistent across\nthe others.\n\nFor paths, I'm currently leaning towards having each identified path\nprinted out as a separate attribute on the same line. For types, I'm\nthinking we can just print a single type and assume it is consistent.\nI'm certainly open to suggestions though. :)\n\n> where <token> tells what kind of extra information it is.  I expect\n> that the initial implementation only knows about \"type\" as the\n> <token>.  For future extensibility, we only need to say that under\n> the syntax:\n> \n>  (1) How multiple attributes are shown?\n>  (2) How would a <value> with SP or LF in it is represented?\n> \n> My suggestion is to have multiple <token>=<value> on the same line,\n> with a SP in between, and problematic bytes in <value> are quoted,\n> using cquote(). i.e. a <token>=<value> whose <value> part does not\n> begin with a double-quote ends at the first SP after it, otherwise\n> <value> is taken as a C-quoted string inside a pair of double-quote.\n\nOk, so using a path value as an example, if it contains a SP, or a\nspecial character that would be handled by `quote_c_style()` it should\nbe wrapped in double-quotes to indicate that it is all part of the\nsingle attribute. That makes sense.\n\n> If you are adventurous, I would not mind seeing \"path\" implemented\n> as another token, since that would be fairly easily obtainable, but\n> it does not have to be in the initial attempt.\n\nHandling paths appears pretty straight-forward to add and seems like a\ngood idea. In my next version I'll also add support for a path\nattribute. Thanks for the feedback.\n\n-Justin\n"},{"id":"510205","messageId":"u5bchsjfjnr2wm2vfxgtugesh5t26oxe7weajqopemlrfmaaio@wispfrii67gb","threadId":"62767","inReplyTo":"CAP8UFD1Q0C22TH7V7K9QZAJmLFDp+DPus1uB1O0oiAvJnWQgig@mail.gmail.com","subject":"Re: [PATCH] rev-list: print missing object type with --missing=print-type","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-01-08T22:28:14Z","receivedAt":"2025-01-08T22:30:49Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/01/08 11:08AM, Christian Couder wrote:\n> On Wed, Jan 8, 2025 at 4:43 AM Justin Tobler <jltobler@gmail.com> wrote:\n> >\n> > Handling of missing objects encounted by git-rev-list(1) can be\n> > configured with the `--missing=<action>` option and specifying the\n> > desired action. Of the available missing actions, none provide a way to\n> > print additional information about the missing object such as its type.\n> \n> What kind of additional information could we also print except for the type?\n\nIn [1], Junio suggested path information as another option.\n\n> \n> > Add a new missing action called `print-type`. Similar to `print`, this\n> > action prints a list of missing objects but also includes the object\n> > type if available in the form: `?<oid> [type]`.\n> >\n> > Signed-off-by: Justin Tobler <jltobler@gmail.com>\n> > ---\n> > Instead of adding an additional missing action type for the explicit\n> > purpose of printing type info, I also considered a separate option\n> > (something like `--object-types`, or maybe a `--missing-format`) that\n> > could be used in combination with the `--missing=print` option to\n> > achieve the same result. The main intent is for the missing object type\n> > and I wasn't sure if type info would be useful in the general case so I\n> > opted to choose the former approach.\n> \n> If there are many other kinds of information we could also print, then\n> maybe a `--missing-format=<format>` option would make more sense\n> rather than adding `print-X`, `print-Y`, `print-X-Y`, etc. Otherwise,\n> yeah, I would say that `print-type` makes sense.\n\nSince there is some other information that we may also want to print, I\nthink it makes sense to follow a more generic approach now. I'm\ncurrently thinking could continue to use `--missing=print`, but add a\n`--missing-attr` option to specify additional information we want to\nprint. Something like:\n\n  $ git rev-list --objects --missing=print \\\n  --missing-attr=type --missing-attr=type\n\n> > @@ -103,6 +109,8 @@ static off_t get_object_disk_usage(struct object *obj)\n> >\n> >  static inline void finish_object__ma(struct object *obj)\n> >  {\n> > +       struct missing_objects_map_entry *entry, *old;\n> > +\n> >         /*\n> >          * Whether or not we try to dynamically fetch missing objects\n> >          * from the server, we currently DO NOT have the object.  We\n> > @@ -119,7 +127,12 @@ static inline void finish_object__ma(struct object *obj)\n> >                 return;\n> >\n> >         case MA_PRINT:\n> > -               oidset_insert(&missing_objects, &obj->oid);\n> > +       case MA_PRINT_TYPE:\n> > +               CALLOC_ARRAY(entry, 1);\n> > +               entry->entry.oid = obj->oid;\n> > +               entry->type = obj->type;\n> > +               old = oidmap_put(&missing_objects, entry);\n> > +               free(old);\n> >                 return;\n> \n> Maybe a function like:\n> \n> static void add_missing_object_entry(struct object_id *oid, unsigned int type)\n> {\n>     struct missing_objects_map_entry *entry, *old;\n> \n>     CALLOC_ARRAY(entry, 1);\n>     entry->entry.oid = *oid;\n>     entry->type = type;\n>     old = oidmap_put(&missing_objects, entry);\n>     free(old);\n> }\n> \n> and then:\n> \n>     case MA_PRINT:\n>     case MA_PRINT_TYPE:\n>         add_missing_object_entry(&obj->oid, obj->type);\n>         return;\n> \n> could help keep finish_object__ma() (which is inlined) short and also\n> avoid some code duplication in the code below.\n\nGood idea, I'll factor this out in the next version.\n\n> \n> >         case MA_ALLOW_PROMISOR:\n> > @@ -414,6 +427,12 @@ static inline int parse_missing_action_value(const char *value)\n> >                 return 1;\n> >         }\n> >\n> > +       if (!strcmp(value, \"print-type\")) {\n> > +               arg_missing_action = MA_PRINT_TYPE;\n> > +               fetch_if_missing = 0;\n> > +               return 1;\n> > +       }\n> > +\n> >         if (!strcmp(value, \"allow-promisor\")) {\n> >                 arg_missing_action = MA_ALLOW_PROMISOR;\n> >                 fetch_if_missing = 0;\n> > @@ -781,10 +800,26 @@ int cmd_rev_list(int argc,\n> >\n> >         if (arg_print_omitted)\n> >                 oidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);\n> > -       if (arg_missing_action == MA_PRINT) {\n> > -               oidset_init(&missing_objects, DEFAULT_OIDSET_SIZE);\n> > -               /* Add missing tips */\n> > -               oidset_insert_from_set(&missing_objects, &revs.missing_commits);\n> > +       if (arg_missing_action == MA_PRINT ||\n> > +           arg_missing_action == MA_PRINT_TYPE) {\n> > +               struct oidset_iter iter;\n> > +               struct object_id *oid;\n> > +\n> > +               oidmap_init(&missing_objects, DEFAULT_OIDSET_SIZE);\n> > +               oidset_iter_init(&revs.missing_commits, &iter);\n> > +\n> > +               /*\n> > +                * Revisions pointing to missing objects lack the context\n> > +                * required to determine object type.\n> > +                */\n> > +               while ((oid = oidset_iter_next(&iter))) {\n> > +                       struct missing_objects_map_entry *entry;\n> > +\n> > +                       CALLOC_ARRAY(entry, 1);\n> > +                       oidcpy(&entry->entry.oid, oid);\n> > +                       oidmap_put(&missing_objects, entry);\n> > +               }\n> \n> Using the function suggested above this could be:\n> \n>         /*\n>          * Revisions pointing to missing objects lack the context\n>          * required to determine object type.\n>          */\n>         while ((oid = oidset_iter_next(&iter)))\n>             add_missing_object_entry(oid, 0);\n> \n> > +\n> >                 oidset_clear(&revs.missing_commits);\n> >         }\n> >\n> > @@ -801,12 +836,27 @@ int cmd_rev_list(int argc,\n> >                 oidset_clear(&omitted_objects);\n> >         }\n> >         if (arg_missing_action == MA_PRINT) {\n> > -               struct oidset_iter iter;\n> > -               struct object_id *oid;\n> > -               oidset_iter_init(&missing_objects, &iter);\n> > -               while ((oid = oidset_iter_next(&iter)))\n> > -                       printf(\"?%s\\n\", oid_to_hex(oid));\n> > -               oidset_clear(&missing_objects);\n> > +               struct missing_objects_map_entry *entry;\n> > +               struct oidmap_iter iter;\n> > +\n> > +               oidmap_iter_init(&missing_objects, &iter);\n> > +\n> > +               while ((entry = oidmap_iter_next(&iter)))\n> > +                       printf(\"?%s\\n\", oid_to_hex(&entry->entry.oid));\n> > +\n> > +               oidmap_free(&missing_objects, true);\n> > +       }\n> > +       if (arg_missing_action == MA_PRINT_TYPE) {\n> > +               struct missing_objects_map_entry *entry;\n> > +               struct oidmap_iter iter;\n> > +\n> > +               oidmap_iter_init(&missing_objects, &iter);\n> > +\n> > +               while ((entry = oidmap_iter_next(&iter)))\n> > +                       printf(\"?%s %s\\n\", oid_to_hex(&entry->entry.oid),\n> > +                              entry->type ? type_name(entry->type) : \"\");\n> > +\n> > +               oidmap_free(&missing_objects, true);\n> >         }\n> \n> Maybe a function like:\n> \n> static void print_missing_objects(unsigned int with_type)\n> {\n>     struct missing_objects_map_entry *entry;\n>     struct oidmap_iter iter;\n> \n>     oidmap_iter_init(&missing_objects, &iter);\n> \n>     while ((entry = oidmap_iter_next(&iter)))\n>         if (with_type && entry->type)\n>             printf(\"?%s %s\\n\", oid_to_hex(&entry->entry.oid),\n>                    type_name(entry->type));\n>         else\n>             printf(\"?%s\\n\", oid_to_hex(&entry->entry.oid));\n> \n>     oidmap_free(&missing_objects, true);\n> }\n> \n> could avoid some code duplication. It could also make the output a bit\n> cleaner as when there is no type, there would be no space after the\n> oid in the output.\n\nI agree with this suggestion and will also factor this out in the next\nversion. Thanks!\n\n-Justin\n"},{"id":"510206","messageId":"xmqq8qrllydy.fsf@gitster.g","threadId":"62767","inReplyTo":"onddu4fjgo2q7f6r7f6kgqctrbhshkll23z6ocsxiiqxbynap5@msq7qtj2un4w","subject":"Re: [PATCH] rev-list: print missing object type with --missing=print-type","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-08T22:43:53Z","receivedAt":"2025-01-08T22:43:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Justin Tobler <jltobler@gmail.com> writes:\n\n>> As I suspect that we would want to leave the door open for us to\n>> extend this later, I would perhaps suggest an output format format\n>> like:\n>> \n>>     ?<object name> [<token>=<value>]...\n>\n> I think this is a great idea. To select which attributes get printed\n> with the missing object we could add an option. Something like:\n>\n>   $ git rev-list --objects --missing=print \\\n>   --missing-attr=path --missing-attr=type\n\nMy knee-jerk reaction is that this is over-engineered; wouldn't it\nbe possible for us to simply dump everything we know about the\nobject, and let the receiving end pick and choose?\n\n> I like the idea of also adding a path attribute, but this raises a\n> couple of questions. The way `--missing=print` currently works is that\n> it prints the unique set of missing object IDs. A missing object could\n> possibly be referenced by multiple trees and thus have multiple valid\n> paths.\n\nThat is not an issue at all, I think.  \"rev-list --objects\" that\nshows objects that are not missing already has the same issue, and\nthe solution is \"show the path when the object gets shown for the\nfirst time\".  Even when the same object is encountered during the\nhistory-and-then-tree walk later, that object is simply not listed.\n\nThe code path that collects \"I thought this blob should exist\nbecause a tree wants to see it at this path, but the repository is\ncorrupt and I cannot see it there\" into the missing object table\nwith attributes should do the same.  If the table does not yet have\nthe object, record the attributes (like \"expected type\", \"path at\nwhich the object was found\") when inserting the object into the\ntable for the first time.  If you have a missing object and the\ntable already has it recorded there, don't do anything extra.\n"},{"id":"510208","messageId":"23akc5fftnnkzrrxb3oojlskmcxgiwfwqpkxgncaubzbmaurm5@uj5tgy5q4bji","threadId":"62767","inReplyTo":"xmqq8qrllydy.fsf@gitster.g","subject":"Re: [PATCH] rev-list: print missing object type with --missing=print-type","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-01-08T23:13:33Z","receivedAt":"2025-01-08T23:16:09Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/01/08 02:43PM, Junio C Hamano wrote:\n> Justin Tobler <jltobler@gmail.com> writes:\n> \n> >> As I suspect that we would want to leave the door open for us to\n> >> extend this later, I would perhaps suggest an output format format\n> >> like:\n> >> \n> >>     ?<object name> [<token>=<value>]...\n> >\n> > I think this is a great idea. To select which attributes get printed\n> > with the missing object we could add an option. Something like:\n> >\n> >   $ git rev-list --objects --missing=print \\\n> >   --missing-attr=path --missing-attr=type\n> \n> My knee-jerk reaction is that this is over-engineered; wouldn't it\n> be possible for us to simply dump everything we know about the\n> object, and let the receiving end pick and choose?\n\nI think that should also be fine and much simpler. We may not want to\neffect the existing output of `--missing=print` though, so we could have\na simple boolean option like `--missing-attr` or just add a new missing\naction type like `--missing=print-attr`. When enabled it would just\nprint all the identified attributes.\n\nIf we don't care though, we could keep it very simple and just add all\nthe information whenever `--missing=print` is set. \n\n> > I like the idea of also adding a path attribute, but this raises a\n> > couple of questions. The way `--missing=print` currently works is that\n> > it prints the unique set of missing object IDs. A missing object could\n> > possibly be referenced by multiple trees and thus have multiple valid\n> > paths.\n> \n> That is not an issue at all, I think.  \"rev-list --objects\" that\n> shows objects that are not missing already has the same issue, and\n> the solution is \"show the path when the object gets shown for the\n> first time\".  Even when the same object is encountered during the\n> history-and-then-tree walk later, that object is simply not listed.\n> \n> The code path that collects \"I thought this blob should exist\n> because a tree wants to see it at this path, but the repository is\n> corrupt and I cannot see it there\" into the missing object table\n> with attributes should do the same.  If the table does not yet have\n> the object, record the attributes (like \"expected type\", \"path at\n> which the object was found\") when inserting the object into the\n> table for the first time.  If you have a missing object and the\n> table already has it recorded there, don't do anything extra.\n\nGood to know! That makes things a bit simpler. I'll follow this approach\nand only add the missing object to the table if an object with the same\nOID is not already recorded.\n\n-Justin\n"},{"id":"510275","messageId":"20250110053417.2602109-2-jltobler@gmail.com","threadId":"62767","inReplyTo":"20250108034012.211043-1-jltobler@gmail.com","subject":"[PATCH v2 0/2] rev-list: print additional missing object information","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-01-10T05:34:16Z","receivedAt":"2025-01-10T05:38:48Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Greetings,\n\nIt is possible to configure git-rev-list(1) to print the OID of missing\nobjects by setting the `--missing=print` option. While it is useful\nknowing about these objects, it would be nice to have even more context\nabout the objects that are missing. Luckily, from an object containing\nthe missing object, it is possible to infer additional information the\nmissing object. For example, if the tree containing a missing blob still\nexists, the tree entry for the missing object should contain path and\ntype information.\n\nThis series aims to provide a git-rev-list(1) option that, when combined\nwith the existing `--missing=print` option, prints other potentially\ninteresting information about the missing object.\n\n- Patch 1 introduces the `--missing-info` option and supports printing\n  the missing object path information.\n\n- Patch 2 extends the `--missing-info` option to also print object type\n  information about the missing object.\n\nChanges in V2:\n\n- A `--missing-info` option is now used instead of an explicit\n  `--missing=print-type` option to print additional missing object info\n  in a more generalized fashion. This enables other use cases outside of\n  just the missing object type.\n\n- Support for printing the full path of the missing object from the\n  top-level tree is also added with `--missing-info`.\n\nDue to all rearranging/changes in this version, I opted not to include a\nrange-diff.\n\nThanks,\n-Justin\n\nJustin Tobler (2):\n  rev-list: add --missing-info to print missing object path\n  rev-list: extend --missing-info to print missing object type\n\n Documentation/rev-list-options.txt |  15 +++++\n builtin/rev-list.c                 | 103 ++++++++++++++++++++++++-----\n t/t6022-rev-list-missing.sh        |  53 +++++++++++++++\n 3 files changed, 156 insertions(+), 15 deletions(-)\n\n\nbase-commit: b74ff38af58464688b211140b90ec90598d340c6\n-- \n2.47.1\n\n"},{"id":"510276","messageId":"20250110053417.2602109-3-jltobler@gmail.com","threadId":"62767","inReplyTo":"20250108034012.211043-1-jltobler@gmail.com","subject":"[PATCH v2 1/2] rev-list: add --missing-info to print missing object path","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-01-10T05:34:17Z","receivedAt":"2025-01-10T05:38:51Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Missing objects identified through git-rev-list(1) can be printed by\nsetting the `--missing=print` option. Additional information about the\nmissing object, such as its path and type, may be present in its\ncontaining object.\n\nAdd the `--missing-info` option that, when also set with\n`--missing=print`, prints additional insight about the missing object\ninferred from its containing object. Each line of output for a missing\nobject is in the form: `?<oid> [<token>=<value>]...`. This format is\nkept generic so it can support multiple different attributes and be\nextended in the future as needed. Each additional attribute is separated\nby a SP. Any value containing a SP or special character is enclosed in\ndouble-quotes in the C style as needed.\n\nFor now, only a missing object path attribute is implemented. It follows\nthe form `path=<path>` and specifies the full path to the object from\nthe top-level tree. In a subsequent commit, a missing object type\nattribute will also be added.\n\nSigned-off-by: Justin Tobler <jltobler@gmail.com>\n---\n Documentation/rev-list-options.txt | 12 ++++\n builtin/rev-list.c                 | 98 +++++++++++++++++++++++++-----\n t/t6022-rev-list-missing.sh        | 52 ++++++++++++++++\n 3 files changed, 147 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 459e5a02f5..8e363f5ae7 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -1028,6 +1028,18 @@ If some tips passed to the traversal are missing, they will be\n considered as missing too, and the traversal will ignore them. In case\n we cannot get their Object ID though, an error will be raised.\n \n+--missing-info::\n+\tOnly useful with `--missing=print`; prints any additional information\n+\tabout the missing object inferred from its containing object. The\n+\tinformation is all printed on the same line with the missing object ID\n+\tin the form: `?<oid> [<token>=<value>]...`. Additional attributes are\n+\teach separated by a SP. Any value containing a SP or special character\n+\tis enclosed in double-quotes in the C style as needed. Each\n+\t`<token>=<value>` may be one of the following:\n++\n+The `path=<path>` shows the path of the missing object inferred from a\n+containing object.\n+\n --exclude-promisor-objects::\n \t(For internal use only.)  Prefilter object traversal at\n \tpromisor boundary.  This is used with partial clone.  This is\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 3196da7b2d..861ffdaf21 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -22,7 +22,10 @@\n #include \"progress.h\"\n #include \"reflog-walk.h\"\n #include \"oidset.h\"\n+#include \"oidmap.h\"\n #include \"packfile.h\"\n+#include \"quote.h\"\n+#include \"strbuf.h\"\n \n static const char rev_list_usage[] =\n \"git rev-list [<options>] <commit>... [--] [<path>...]\\n\"\n@@ -73,7 +76,11 @@ static unsigned progress_counter;\n static struct oidset omitted_objects;\n static int arg_print_omitted; /* print objects omitted by filter */\n \n-static struct oidset missing_objects;\n+struct missing_objects_map_entry {\n+\tstruct oidmap_entry entry;\n+\tconst char *path;\n+};\n+static struct oidmap missing_objects;\n enum missing_action {\n \tMA_ERROR = 0,    /* fail if any missing objects are encountered */\n \tMA_ALLOW_ANY,    /* silently allow ALL missing objects */\n@@ -101,7 +108,47 @@ static off_t get_object_disk_usage(struct object *obj)\n \treturn size;\n }\n \n-static inline void finish_object__ma(struct object *obj)\n+static void add_missing_object_entry(struct object_id *oid, const char *path)\n+{\n+\tstruct missing_objects_map_entry *entry;\n+\n+\tif (oidmap_get(&missing_objects, oid))\n+\t\treturn;\n+\n+\tCALLOC_ARRAY(entry, 1);\n+\tentry->entry.oid = *oid;\n+\tif (path)\n+\t\tentry->path = xstrdup(path);\n+\toidmap_put(&missing_objects, entry);\n+}\n+\n+static void print_missing_object(struct missing_objects_map_entry *entry,\n+\t\t\t\t int print_missing_info)\n+{\n+\tstruct strbuf sb;\n+\n+\tif (!print_missing_info) {\n+\t\tprintf(\"?%s\\n\", oid_to_hex(&entry->entry.oid));\n+\t\treturn;\n+\t}\n+\n+\tstrbuf_init(&sb, 0);\n+\tif (entry->path && *entry->path) {\n+\t\tstrbuf_addstr(&sb, \" path=\");\n+\n+\t\tif (quote_c_style(entry->path, NULL, NULL, 0))\n+\t\t\tquote_c_style(entry->path, &sb, NULL, 0);\n+\t\telse if (strchr(entry->path, ' '))\n+\t\t\tstrbuf_addf(&sb, \"\\\"%s\\\"\", entry->path);\n+\t\telse\n+\t\t\tstrbuf_addstr(&sb, entry->path);\n+\t}\n+\n+\tprintf(\"?%s%s\\n\", oid_to_hex(&entry->entry.oid), sb.buf);\n+\tstrbuf_release(&sb);\n+}\n+\n+static inline void finish_object__ma(struct object *obj, const char *name)\n {\n \t/*\n \t * Whether or not we try to dynamically fetch missing objects\n@@ -119,7 +166,7 @@ static inline void finish_object__ma(struct object *obj)\n \t\treturn;\n \n \tcase MA_PRINT:\n-\t\toidset_insert(&missing_objects, &obj->oid);\n+\t\tadd_missing_object_entry(&obj->oid, name);\n \t\treturn;\n \n \tcase MA_ALLOW_PROMISOR:\n@@ -152,7 +199,7 @@ static void show_commit(struct commit *commit, void *data)\n \n \tif (revs->do_not_die_on_missing_objects &&\n \t    oidset_contains(&revs->missing_commits, &commit->object.oid)) {\n-\t\tfinish_object__ma(&commit->object);\n+\t\tfinish_object__ma(&commit->object, NULL);\n \t\treturn;\n \t}\n \n@@ -268,12 +315,11 @@ static void show_commit(struct commit *commit, void *data)\n \tfinish_commit(commit);\n }\n \n-static int finish_object(struct object *obj, const char *name UNUSED,\n-\t\t\t void *cb_data)\n+static int finish_object(struct object *obj, const char *name, void *cb_data)\n {\n \tstruct rev_list_info *info = cb_data;\n \tif (oid_object_info_extended(the_repository, &obj->oid, NULL, 0) < 0) {\n-\t\tfinish_object__ma(obj);\n+\t\tfinish_object__ma(obj, name);\n \t\treturn 1;\n \t}\n \tif (info->revs->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)\n@@ -538,6 +584,7 @@ int cmd_rev_list(int argc,\n \tint bisect_show_vars = 0;\n \tint bisect_find_all = 0;\n \tint use_bitmap_index = 0;\n+\tint print_missing_info = 0;\n \tint filter_provided_objects = 0;\n \tconst char *show_progress = NULL;\n \tint ret = 0;\n@@ -656,6 +703,15 @@ int cmd_rev_list(int argc,\n \t\tif (skip_prefix(arg, \"--missing=\", &arg))\n \t\t\tcontinue; /* already handled above */\n \n+\t\tif (!strcmp(arg, \"--missing-info\")) {\n+\t\t\tif (arg_missing_action != MA_PRINT)\n+\t\t\t\tdie(_(\"the option '%s' requires '%s'\"),\n+\t\t\t\t    \"--missing-info\", \"--missing=print\");\n+\n+\t\t\tprint_missing_info = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\tif (!strcmp(arg, (\"--no-object-names\"))) {\n \t\t\targ_show_object_names = 0;\n \t\t\tcontinue;\n@@ -782,9 +838,16 @@ int cmd_rev_list(int argc,\n \tif (arg_print_omitted)\n \t\toidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);\n \tif (arg_missing_action == MA_PRINT) {\n-\t\toidset_init(&missing_objects, DEFAULT_OIDSET_SIZE);\n+\t\tstruct oidset_iter iter;\n+\t\tstruct object_id *oid;\n+\n+\t\toidmap_init(&missing_objects, DEFAULT_OIDSET_SIZE);\n+\t\toidset_iter_init(&revs.missing_commits, &iter);\n+\n \t\t/* Add missing tips */\n-\t\toidset_insert_from_set(&missing_objects, &revs.missing_commits);\n+\t\twhile ((oid = oidset_iter_next(&iter)))\n+\t\t\tadd_missing_object_entry(oid, NULL);\n+\n \t\toidset_clear(&revs.missing_commits);\n \t}\n \n@@ -801,12 +864,17 @@ int cmd_rev_list(int argc,\n \t\toidset_clear(&omitted_objects);\n \t}\n \tif (arg_missing_action == MA_PRINT) {\n-\t\tstruct oidset_iter iter;\n-\t\tstruct object_id *oid;\n-\t\toidset_iter_init(&missing_objects, &iter);\n-\t\twhile ((oid = oidset_iter_next(&iter)))\n-\t\t\tprintf(\"?%s\\n\", oid_to_hex(oid));\n-\t\toidset_clear(&missing_objects);\n+\t\tstruct missing_objects_map_entry *entry;\n+\t\tstruct oidmap_iter iter;\n+\n+\t\toidmap_iter_init(&missing_objects, &iter);\n+\n+\t\twhile ((entry = oidmap_iter_next(&iter))) {\n+\t\t\tprint_missing_object(entry, print_missing_info);\n+\t\t\tfree((void *)entry->path);\n+\t\t}\n+\n+\t\toidmap_free(&missing_objects, true);\n \t}\n \n \tstop_progress(&progress);\ndiff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh\nindex 7553a9cca2..2eb051028e 100755\n--- a/t/t6022-rev-list-missing.sh\n+++ b/t/t6022-rev-list-missing.sh\n@@ -145,4 +145,56 @@ do\n \tdone\n done\n \n+for obj in \"HEAD~1\" \"HEAD^{tree}\" \"HEAD:foo\" \"HEAD:foo/bar\" \"HEAD:baz baz\" \"HEAD:bat\\\"\"\n+do\n+\ttest_expect_success \"--missing-info with missing '$obj'\" '\n+\t\ttest_when_finished rm -rf missing-info &&\n+\n+\t\tgit init missing-info &&\n+\t\t(\n+\t\t\tcd missing-info &&\n+\t\t\tgit commit --allow-empty -m first &&\n+\n+\t\t\tmkdir foo &&\n+\t\t\techo bar >foo/bar &&\n+\t\t\techo baz >\"baz baz\" &&\n+\t\t\techo bat >bat\\\" &&\n+\t\t\tgit add -A &&\n+\t\t\tgit commit -m second &&\n+\n+\t\t\toid=\"$(git rev-parse \"$obj\")\" &&\n+\t\t\tpath=\".git/objects/$(test_oid_to_path $oid)\" &&\n+\n+\t\t\tcase $obj in\n+\t\t\tHEAD:foo)\n+\t\t\t\tpath_info=\" path=foo\"\n+\t\t\t\t;;\n+\t\t\tHEAD:foo/bar)\n+\t\t\t\tpath_info=\" path=foo/bar\"\n+\t\t\t\t;;\n+\t\t\t\"HEAD:baz baz\")\n+\t\t\t\tpath_info=\" path=\\\"baz baz\\\"\"\n+\t\t\t\t;;\n+\t\t\t\"HEAD:bat\\\"\")\n+\t\t\t\tpath_info=\" path=\\\"bat\\\\\\\"\\\"\"\n+\t\t\t\t;;\n+\t\t\tesac &&\n+\n+\t\t\t# Before the object is made missing, we use rev-list to\n+\t\t\t# get the expected oids.\n+\t\t\tgit rev-list --objects --no-object-names \\\n+\t\t\t\tHEAD ^\"$obj\" >expect.raw &&\n+\t\t\techo \"?$oid$path_info\" >>expect.raw &&\n+\n+\t\t\tmv \"$path\" \"$path.hidden\" &&\n+\t\t\tgit rev-list --objects --no-object-names \\\n+\t\t\t\t--missing=print --missing-info HEAD >actual.raw &&\n+\n+\t\t\tsort actual.raw >actual &&\n+\t\t\tsort expect.raw >expect &&\n+\t\t\ttest_cmp expect actual\n+\t\t)\n+\t'\n+done\n+\n test_done\n-- \n2.47.1\n\n"},{"id":"510277","messageId":"20250110053417.2602109-4-jltobler@gmail.com","threadId":"62767","inReplyTo":"20250108034012.211043-1-jltobler@gmail.com","subject":"[PATCH v2 2/2] rev-list: extend --missing-info to print missing object type","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-01-10T05:34:18Z","receivedAt":"2025-01-10T05:38:56Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Additional information about missing objects found in git-rev-list(1)\ncan be printed by specifying the `--missing=print` and `--missing-info`\noptions. Extend this option to also print missing object type\ninformation inferred from its containing object. This attribute follows\nthe form `type=<type>` and specifies the expected object type of the\nmissing object.\n\nSigned-off-by: Justin Tobler <jltobler@gmail.com>\n---\n Documentation/rev-list-options.txt |  3 +++\n builtin/rev-list.c                 | 11 ++++++++---\n t/t6022-rev-list-missing.sh        |  3 ++-\n 3 files changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 8e363f5ae7..9722eefc79 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -1039,6 +1039,9 @@ we cannot get their Object ID though, an error will be raised.\n +\n The `path=<path>` shows the path of the missing object inferred from a\n containing object.\n++\n+The `type=<type>` shows the type of the missing object inferred from a\n+containing object.\n \n --exclude-promisor-objects::\n \t(For internal use only.)  Prefilter object traversal at\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 861ffdaf21..93d173039d 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -79,6 +79,7 @@ static int arg_print_omitted; /* print objects omitted by filter */\n struct missing_objects_map_entry {\n \tstruct oidmap_entry entry;\n \tconst char *path;\n+\tunsigned type;\n };\n static struct oidmap missing_objects;\n enum missing_action {\n@@ -108,7 +109,8 @@ static off_t get_object_disk_usage(struct object *obj)\n \treturn size;\n }\n \n-static void add_missing_object_entry(struct object_id *oid, const char *path)\n+static void add_missing_object_entry(struct object_id *oid, const char *path,\n+\t\t\t\t     unsigned type)\n {\n \tstruct missing_objects_map_entry *entry;\n \n@@ -117,6 +119,7 @@ static void add_missing_object_entry(struct object_id *oid, const char *path)\n \n \tCALLOC_ARRAY(entry, 1);\n \tentry->entry.oid = *oid;\n+\tentry->type = type;\n \tif (path)\n \t\tentry->path = xstrdup(path);\n \toidmap_put(&missing_objects, entry);\n@@ -143,6 +146,8 @@ static void print_missing_object(struct missing_objects_map_entry *entry,\n \t\telse\n \t\t\tstrbuf_addstr(&sb, entry->path);\n \t}\n+\tif (entry->type)\n+\t\tstrbuf_addf(&sb, \" type=%s\", type_name(entry->type));\n \n \tprintf(\"?%s%s\\n\", oid_to_hex(&entry->entry.oid), sb.buf);\n \tstrbuf_release(&sb);\n@@ -166,7 +171,7 @@ static inline void finish_object__ma(struct object *obj, const char *name)\n \t\treturn;\n \n \tcase MA_PRINT:\n-\t\tadd_missing_object_entry(&obj->oid, name);\n+\t\tadd_missing_object_entry(&obj->oid, name, obj->type);\n \t\treturn;\n \n \tcase MA_ALLOW_PROMISOR:\n@@ -846,7 +851,7 @@ int cmd_rev_list(int argc,\n \n \t\t/* Add missing tips */\n \t\twhile ((oid = oidset_iter_next(&iter)))\n-\t\t\tadd_missing_object_entry(oid, NULL);\n+\t\t\tadd_missing_object_entry(oid, NULL, 0);\n \n \t\toidset_clear(&revs.missing_commits);\n \t}\ndiff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh\nindex 2eb051028e..95bee17cab 100755\n--- a/t/t6022-rev-list-missing.sh\n+++ b/t/t6022-rev-list-missing.sh\n@@ -164,6 +164,7 @@ do\n \n \t\t\toid=\"$(git rev-parse \"$obj\")\" &&\n \t\t\tpath=\".git/objects/$(test_oid_to_path $oid)\" &&\n+\t\t\ttype_info=\" type=$(git cat-file -t $oid)\" &&\n \n \t\t\tcase $obj in\n \t\t\tHEAD:foo)\n@@ -184,7 +185,7 @@ do\n \t\t\t# get the expected oids.\n \t\t\tgit rev-list --objects --no-object-names \\\n \t\t\t\tHEAD ^\"$obj\" >expect.raw &&\n-\t\t\techo \"?$oid$path_info\" >>expect.raw &&\n+\t\t\techo \"?$oid$path_info$type_info\" >>expect.raw &&\n \n \t\t\tmv \"$path\" \"$path.hidden\" &&\n \t\t\tgit rev-list --objects --no-object-names \\\n-- \n2.47.1\n\n"},{"id":"510280","messageId":"CAP8UFD2Cfriv4puPe8agaTZOpLHr-=4CkK-yrzw8fH-k5mPkAA@mail.gmail.com","threadId":"62767","inReplyTo":"20250110053417.2602109-3-jltobler@gmail.com","subject":"Re: [PATCH v2 1/2] rev-list: add --missing-info to print missing object path","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-01-10T08:47:08Z","receivedAt":"2025-01-10T08:47:22Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Jan 10, 2025 at 6:38 AM Justin Tobler <jltobler@gmail.com> wrote:\n\n> +--missing-info::\n> +       Only useful with `--missing=print`; prints any additional information\n> +       about the missing object inferred from its containing object. The\n> +       information is all printed on the same line with the missing object ID\n> +       in the form: `?<oid> [<token>=<value>]...`. Additional attributes are\n> +       each separated by a SP.\n\nNit: I'd rather say \"The `<token>=<value>` pairs containing additional\ninformation are separated from each other by a SP.\" to avoid\nintroducing \"attributes\" which might not be very clear.\n\n> Any value containing a SP or special character\n> +       is enclosed in double-quotes in the C style as needed. Each\n> +       `<token>=<value>` may be one of the following:\n\nIt might be a bit better to decide for each token-value pair how the\nvalue is encoded, instead of deciding in advance for all of them.\n\n> ++\n> +The `path=<path>` shows the path of the missing object inferred from a\n> +containing object.\n\nFor example for the path, I think it might be easier to always enclose\nit in double-quotes in the C style rather than checking first if it\ncontains spaces or other special characters, see below.\n\n> +static void print_missing_object(struct missing_objects_map_entry *entry,\n> +                                int print_missing_info)\n> +{\n> +       struct strbuf sb;\n> +\n> +       if (!print_missing_info) {\n> +               printf(\"?%s\\n\", oid_to_hex(&entry->entry.oid));\n> +               return;\n> +       }\n> +\n> +       strbuf_init(&sb, 0);\n\nI am not sure it's worth initializing the sb separately from its\ndeclaration above. Using \"struct strbuf sb = STRBUF_INIT;\" is more\nstandard in the code base and I think most compilers these days are\nlikely to be able to optimize away the initialization in the\n\"!print_missing_info\" case.\n\n> +       if (entry->path && *entry->path) {\n> +               strbuf_addstr(&sb, \" path=\");\n> +\n> +               if (quote_c_style(entry->path, NULL, NULL, 0))\n> +                       quote_c_style(entry->path, &sb, NULL, 0);\n> +               else if (strchr(entry->path, ' '))\n> +                       strbuf_addf(&sb, \"\\\"%s\\\"\", entry->path);\n> +               else\n> +                       strbuf_addstr(&sb, entry->path);\n\nI think the above code paragraph could be simplified to just:\n\n            quote_c_style(entry->path, &sb, NULL, 0);\n\nif we decided to always quote the path. The decoding part would likely\nbe simplified too.\n\n> +       }\n> +\n> +       printf(\"?%s%s\\n\", oid_to_hex(&entry->entry.oid), sb.buf);\n> +       strbuf_release(&sb);\n> +}\n\n> @@ -656,6 +703,15 @@ int cmd_rev_list(int argc,\n>                 if (skip_prefix(arg, \"--missing=\", &arg))\n>                         continue; /* already handled above */\n>\n> +               if (!strcmp(arg, \"--missing-info\")) {\n> +                       if (arg_missing_action != MA_PRINT)\n> +                               die(_(\"the option '%s' requires '%s'\"),\n> +                                   \"--missing-info\", \"--missing=print\");\n\nIt seems to me that this check should be performed outside the arg\nparsing loop so that this check passes if the user passes\n\"--missing-info\" before \"--missing=print\" on the command line.\n\n> +                       print_missing_info = 1;\n> +                       continue;\n> +               }\n> +\n"},{"id":"510328","messageId":"xmqqtta68zhz.fsf@gitster.g","threadId":"62767","inReplyTo":"CAP8UFD2Cfriv4puPe8agaTZOpLHr-=4CkK-yrzw8fH-k5mPkAA@mail.gmail.com","subject":"Re: [PATCH v2 1/2] rev-list: add --missing-info to print missing object path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-01-10T15:22:48Z","receivedAt":"2025-01-10T15:22:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> On Fri, Jan 10, 2025 at 6:38 AM Justin Tobler <jltobler@gmail.com> wrote:\n>\n>> +--missing-info::\n>> +       Only useful with `--missing=print`; prints any additional information\n>> +       about the missing object inferred from its containing object. The\n>> +       information is all printed on the same line with the missing object ID\n>> +       in the form: `?<oid> [<token>=<value>]...`. Additional attributes are\n>> +       each separated by a SP.\n>\n> Nit: I'd rather say \"The `<token>=<value>` pairs containing additional\n> information are separated from each other by a SP.\" to avoid\n> introducing \"attributes\" which might not be very clear.\n\nExcellent.\n\n>> Any value containing a SP or special character\n>> +       is enclosed in double-quotes in the C style as needed. Each\n>> +       `<token>=<value>` may be one of the following:\n>\n> It might be a bit better to decide for each token-value pair how the\n> value is encoded, instead of deciding in advance for all of them.\n\nI strongly advise against it.\n\nDeclaring the syntax we will use for forseeable future for new\nattributes upfront would allow the receiving end, the scripts that\ninterpret our output, to be written in a futureproof way.\n\nAn alternative is \"value is encoded in token specific fashion, but\none thing that is common across these future encodings is that SP or\nLF contained in value will be represented in such a way that the\nresulting encoded value will not have either of these two\nproblematic bytes\".\n\n>> +       if (entry->path && *entry->path) {\n>> +               strbuf_addstr(&sb, \" path=\");\n>> +\n>> +               if (quote_c_style(entry->path, NULL, NULL, 0))\n>> +                       quote_c_style(entry->path, &sb, NULL, 0);\n>> +               else if (strchr(entry->path, ' '))\n>> +                       strbuf_addf(&sb, \"\\\"%s\\\"\", entry->path);\n>> +               else\n>> +                       strbuf_addstr(&sb, entry->path);\n>\n> I think the above code paragraph could be simplified to just:\n>\n>             quote_c_style(entry->path, &sb, NULL, 0);\n\nHmph, you two may be both wrong ;-)  It is unfortunate that you\ncannot easily configure what is considered \"must be quoted\" bytes\nper invocation of the quote_c_style() function.  Most problematic\nis that in the cq_lookup[] table, SP and \"!\" are treated the same\nway, i.e., <a b c> does not need to be quoted.  This comes from the\nfact that quote_c_style() was written for the sole purpose of\nshowing the pathnames on the header lines of \"git diff\" output.\n\nWe may be able to (ab)use quote_path() with QUOTE_PATH_QUOTE_SP\nbit set for this purpose, though, to work around the above.\n\ncq_must_quote() also is affected by core.quotepath, which is not a\ndesirable feature here---we want our machine readable program output\nstable regardless of end-user preference.\n\nBy the way, perhaps we should propose to make the default value for\ncore.quotepath to \"no\" at Git 3.0 boundary?\n\nThanks.\n"},{"id":"511641","messageId":"20250201201658.11562-1-jltobler@gmail.com","threadId":"62767","inReplyTo":"20250110053417.2602109-2-jltobler@gmail.com","subject":"[PATCH v3 0/4] rev-list: print additional missing object information","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-02-01T20:16:54Z","receivedAt":"2025-02-01T20:20:02Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Greetings,\n\nIt is possible to configure git-rev-list(1) to print the OID of missing\nobjects by setting the `--missing=print` option. While it is useful\nknowing about these objects, it would be nice to have even more context\nabout the objects that are missing. Luckily, from an object containing\nthe missing object, it is possible to infer additional information the\nmissing object. For example, if the tree containing a missing blob still\nexists, the tree entry for the missing object should contain path and\ntype information.\n\nThis series aims to provide git-rev-list(1) with a new `print-info`\nmissing action for the `--missing` option that, when set, behaves like\nthe existing `print` action but also prints other potentially\ninteresting information about the missing object.\n\nMissing object info is printed in the form `?<oid> [<token>=<value>]...`\nwhere multiple `<token>=<value>` pairs may be specified each separated\nfrom each other with a SP. Values that contain SP or LF characters are\nexpected to be encoded in a manner such that these problematic bytes are\nhandled. For missing object path information this is handled by quoting\nthe path in the C style if it contains SP or special characters.\n\nOne concern I currently have with this quoting approach is that it is a\nbit more challenging to machine parse compared to something like using a\nnull byte to delimit between missing info. One option is, in a followup\nseries, introduce a git-for-each-ref(1) style format syntax. Maybe\nsomething like: `--missing=print-info:%(path)%00%(type)`. I'm curious if\nanyone may have thoughts around this. My goal is to ensure that there is\nan easy to use machine parsable interface to get this information. I\ncould see something like `?<oid> path=\"foo \\\"bar\" type=blob`, being a\nbit complex.\n\nThe series is set up as follows:\n\n- Pathes 1 and 2 provide the `quote_c_style` and `quote_path` functions\n  with a way print consistent output regardless of how core.quotePath is\n  set.\n\n- Patch 3 introduces the `print-info` missing action and supports\n  printing missing object path information.\n\n- Patch 4 extends the `print-info` missing action to also print object\n  type information about the missing object.\n\nChanges in V3:\n\n- This missing info is provided once again through an explicit\n  `--missing=print-info` action instead of combining the\n  `--missing=print` and `--missing-info` flag. This was done to make the\n  interface a bit straightforward to use and not introduce a flag and is\n  dependent on another.\n\n- Added flag to `quote_path()` to disable core.quotePath from impacting\n  quoted output and use it when print missing object path info.\n\n- Update documentation to not use the term \"attribute\" which could be\n  confused with gitattributes.\n\nDue to all rearranging/changes in this version, I opted not to include a\nrange-diff.\n\nThanks,\n-Justin\n\nJustin Tobler (4):\n  quote: add c quote flag to ignore core.quotePath\n  quote: add quote_path() flag to ignore config\n  rev-list: add print-info action to print missing object path\n  rev-list: extend print-info to print missing object type\n\n Documentation/rev-list-options.txt |  19 +++++\n builtin/rev-list.c                 | 107 ++++++++++++++++++++++++-----\n quote.c                            |  27 +++++---\n quote.h                            |   6 +-\n t/t6022-rev-list-missing.sh        |  53 ++++++++++++++\n 5 files changed, 184 insertions(+), 28 deletions(-)\n\n\nbase-commit: b74ff38af58464688b211140b90ec90598d340c6\n-- \n2.48.1.157.g3b0d05c4a7\n\n"},{"id":"511642","messageId":"20250201201658.11562-2-jltobler@gmail.com","threadId":"62767","inReplyTo":"20250201201658.11562-1-jltobler@gmail.com","subject":"[PATCH v3 1/4] quote: add c quote flag to ignore core.quotePath","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-02-01T20:16:55Z","receivedAt":"2025-02-01T20:20:07Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"The output of `cq_must_quote()` is affected by `core.quotePath`. This is\nundesirable for operations that want to ensure consistent output\nindependent of config settings.\n\nIntroduce the `CQUOTE_IGNORE_CONFIG` flag for the `quote_c_style*`\nfunctions which when set makes `cq_must_quote()` always follow the\ndefault behavior (core.quotePath=true) regardless of how its set in the\nconfig.\n\nSigned-off-by: Justin Tobler <jltobler@gmail.com>\n---\n quote.c | 14 ++++++++------\n quote.h |  3 ++-\n 2 files changed, 10 insertions(+), 7 deletions(-)\n\ndiff --git a/quote.c b/quote.c\nindex b9f6bdc775..d129c1de70 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -232,21 +232,22 @@ static signed char const cq_lookup[256] = {\n \t/* 0x80 */ /* set to 0 */\n };\n \n-static inline int cq_must_quote(char c)\n+static inline int cq_must_quote(char c, int ignore_config)\n {\n-\treturn cq_lookup[(unsigned char)c] + quote_path_fully > 0;\n+\treturn cq_lookup[(unsigned char)c] + (quote_path_fully || ignore_config) > 0;\n }\n \n /* returns the longest prefix not needing a quote up to maxlen if positive.\n    This stops at the first \\0 because it's marked as a character needing an\n    escape */\n-static size_t next_quote_pos(const char *s, ssize_t maxlen)\n+static size_t next_quote_pos(const char *s, ssize_t maxlen, int ignore_config)\n {\n \tsize_t len;\n \tif (maxlen < 0) {\n-\t\tfor (len = 0; !cq_must_quote(s[len]); len++);\n+\t\tfor (len = 0; !cq_must_quote(s[len], ignore_config); len++);\n \t} else {\n-\t\tfor (len = 0; len < maxlen && !cq_must_quote(s[len]); len++);\n+\t\tfor (len = 0;\n+\t\t     len < maxlen && !cq_must_quote(s[len], ignore_config); len++);\n \t}\n \treturn len;\n }\n@@ -282,13 +283,14 @@ static size_t quote_c_style_counted(const char *name, ssize_t maxlen,\n \t} while (0)\n \n \tint no_dq = !!(flags & CQUOTE_NODQ);\n+\tint ignore_config = !!(flags & CQUOTE_IGNORE_CONFIG);\n \tsize_t len, count = 0;\n \tconst char *p = name;\n \n \tfor (;;) {\n \t\tint ch;\n \n-\t\tlen = next_quote_pos(p, maxlen);\n+\t\tlen = next_quote_pos(p, maxlen, ignore_config);\n \t\tif (len == maxlen || (maxlen < 0 && !p[len]))\n \t\t\tbreak;\n \ndiff --git a/quote.h b/quote.h\nindex 0300c29104..2a793fbef6 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -83,7 +83,8 @@ int sq_dequote_to_strvec(char *arg, struct strvec *);\n int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);\n \n /* Bits in the flags parameter to quote_c_style() */\n-#define CQUOTE_NODQ 01\n+#define CQUOTE_NODQ\t     (1u << 0)\n+#define CQUOTE_IGNORE_CONFIG (1u << 1)\n size_t quote_c_style(const char *name, struct strbuf *, FILE *, unsigned);\n void quote_two_c_style(struct strbuf *, const char *, const char *, unsigned);\n \n-- \n2.48.1.157.g3b0d05c4a7\n\n"},{"id":"511643","messageId":"20250201201658.11562-3-jltobler@gmail.com","threadId":"62767","inReplyTo":"20250201201658.11562-1-jltobler@gmail.com","subject":"[PATCH v3 2/4] quote: add quote_path() flag to ignore config","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-02-01T20:16:56Z","receivedAt":"2025-02-01T20:20:09Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"The `quote_path()` function invokes `quote_c_style_counted()` to handle\nquoting. This means the output `quote_path()` is ultimately affected by\n`core.quotePath` configuration. In a subsequent commit, `quote_path()`\nwill be used in a scenario where the output should remain consistent\nregardless of the current configuration.\n\nIntroduce the `QUOTE_PATH_IGNORE_CONFIG` flag for `quote_path()`which\nwhen set instructs the underlying `quote_c_style_counted()` to also\nignore the `core.quotePath` configuration when executed.\n\nSigned-off-by: Justin Tobler <jltobler@gmail.com>\n---\n quote.c | 13 ++++++++++---\n quote.h |  3 ++-\n 2 files changed, 12 insertions(+), 4 deletions(-)\n\ndiff --git a/quote.c b/quote.c\nindex d129c1de70..baec34ca94 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -370,10 +370,18 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *rel = relative_path(in, prefix, &sb);\n-\tint force_dq = ((flags & QUOTE_PATH_QUOTE_SP) && strchr(rel, ' '));\n+\tunsigned cquote_flags = 0;\n+\tint force_dq = 0;\n \n \tstrbuf_reset(out);\n \n+\tif ((flags & QUOTE_PATH_QUOTE_SP) && strchr(rel, ' ')) {\n+\t\tforce_dq = 1;\n+\t\tcquote_flags &= CQUOTE_NODQ;\n+\t}\n+\tif (flags & QUOTE_PATH_IGNORE_CONFIG)\n+\t\tcquote_flags &= CQUOTE_IGNORE_CONFIG;\n+\n \t/*\n \t * If the caller wants us to enclose the output in a dq-pair\n \t * whether quote_c_style_counted() needs to, we do it ourselves\n@@ -381,8 +389,7 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne\n \t */\n \tif (force_dq)\n \t\tstrbuf_addch(out, '\"');\n-\tquote_c_style_counted(rel, strlen(rel), out, NULL,\n-\t\t\t      force_dq ? CQUOTE_NODQ : 0);\n+\tquote_c_style_counted(rel, strlen(rel), out, NULL, cquote_flags);\n \tif (force_dq)\n \t\tstrbuf_addch(out, '\"');\n \tstrbuf_release(&sb);\ndiff --git a/quote.h b/quote.h\nindex 2a793fbef6..84903951ef 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -94,7 +94,8 @@ void write_name_quoted_relative(const char *name, const char *prefix,\n \n /* quote path as relative to the given prefix */\n char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags);\n-#define QUOTE_PATH_QUOTE_SP 01\n+#define QUOTE_PATH_QUOTE_SP\t (1u << 0)\n+#define QUOTE_PATH_IGNORE_CONFIG (1u << 1)\n \n /* quoting as a string literal for other languages */\n void perl_quote_buf(struct strbuf *sb, const char *src);\n-- \n2.48.1.157.g3b0d05c4a7\n\n"},{"id":"511644","messageId":"20250201201658.11562-4-jltobler@gmail.com","threadId":"62767","inReplyTo":"20250201201658.11562-1-jltobler@gmail.com","subject":"[PATCH v3 3/4] rev-list: add print-info action to print missing object path","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-02-01T20:16:57Z","receivedAt":"2025-02-01T20:20:11Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Missing objects identified through git-rev-list(1) can be printed by\nsetting the `--missing=print` option. Additional information about the\nmissing object, such as its path and type, may be present in its\ncontaining object.\n\nAdd the `print-info` missing action for the `--missing` option that,\nwhen set, prints additional insight about the missing object inferred\nfrom its containing object. Each line of output for a missing object is\nin the form: `?<oid> [<token>=<value>]...`. The `<token>=<value>` pairs\ncontaining additional information are separated from each other by a SP.\nThe value is encoded in a token specific fashion, but SP or LF contained\nin value are always expected to be represented in such a way that the\nresulting encoded value does not have either of these two problematic\nbytes. This format is kept generic so it can be extended in the future\nto support additional information.\n\nFor now, only a missing object path info is implemented. It follows the\nform `path=<path>` and specifies the full path to the object from the\ntop-level tree. A path containing SP or special characters is enclosed\nin double-quotes in the C style as needed. In a subsequent commit,\nmissing object type info will also be added.\n\nSigned-off-by: Justin Tobler <jltobler@gmail.com>\n---\n Documentation/rev-list-options.txt |  16 +++++\n builtin/rev-list.c                 | 102 ++++++++++++++++++++++++-----\n t/t6022-rev-list-missing.sh        |  52 +++++++++++++++\n 3 files changed, 153 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 459e5a02f5..0bea9d4ad3 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -1024,6 +1024,22 @@ Unexpected missing objects will raise an error.\n The form '--missing=print' is like 'allow-any', but will also print a\n list of the missing objects.  Object IDs are prefixed with a ``?'' character.\n +\n+The form '--missing=print-info' is like 'print', but will also print additional\n+information about the missing object inferred from its containing object. The\n+information is all printed on the same line with the missing object ID in the\n+form: `?<oid> [<token>=<value>]...`. The `<token>=<value>` pairs containing\n+additional information are separated from each other by a SP. The value is\n+encoded in a token specific fashion, but SP or LF contained in value are always\n+expected to be represented in such a way that the resulting encoded value does\n+not have either of these two problematic bytes. Each `<token>=<value>` may be\n+one of the following:\n++\n+--\n+* The `path=<path>` shows the path of the missing object inferred from a\n+  containing object. A path containing SP or special characters is enclosed in\n+  double-quotes in the C style as needed.\n+--\n++\n If some tips passed to the traversal are missing, they will be\n considered as missing too, and the traversal will ignore them. In case\n we cannot get their Object ID though, an error will be raised.\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 3196da7b2d..4a45a4e555 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -22,7 +22,10 @@\n #include \"progress.h\"\n #include \"reflog-walk.h\"\n #include \"oidset.h\"\n+#include \"oidmap.h\"\n #include \"packfile.h\"\n+#include \"quote.h\"\n+#include \"strbuf.h\"\n \n static const char rev_list_usage[] =\n \"git rev-list [<options>] <commit>... [--] [<path>...]\\n\"\n@@ -73,11 +76,16 @@ static unsigned progress_counter;\n static struct oidset omitted_objects;\n static int arg_print_omitted; /* print objects omitted by filter */\n \n-static struct oidset missing_objects;\n+struct missing_objects_map_entry {\n+\tstruct oidmap_entry entry;\n+\tconst char *path;\n+};\n+static struct oidmap missing_objects;\n enum missing_action {\n \tMA_ERROR = 0,    /* fail if any missing objects are encountered */\n \tMA_ALLOW_ANY,    /* silently allow ALL missing objects */\n \tMA_PRINT,        /* print ALL missing objects in special section */\n+\tMA_PRINT_INFO,   /* same as MA_PRINT but also prints missing object info */\n \tMA_ALLOW_PROMISOR, /* silently allow all missing PROMISOR objects */\n };\n static enum missing_action arg_missing_action;\n@@ -101,7 +109,46 @@ static off_t get_object_disk_usage(struct object *obj)\n \treturn size;\n }\n \n-static inline void finish_object__ma(struct object *obj)\n+static void add_missing_object_entry(struct object_id *oid, const char *path)\n+{\n+\tstruct missing_objects_map_entry *entry;\n+\n+\tif (oidmap_get(&missing_objects, oid))\n+\t\treturn;\n+\n+\tCALLOC_ARRAY(entry, 1);\n+\tentry->entry.oid = *oid;\n+\tif (path)\n+\t\tentry->path = xstrdup(path);\n+\toidmap_put(&missing_objects, entry);\n+}\n+\n+static void print_missing_object(struct missing_objects_map_entry *entry,\n+\t\t\t\t int print_missing_info)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tif (!print_missing_info) {\n+\t\tprintf(\"?%s\\n\", oid_to_hex(&entry->entry.oid));\n+\t\treturn;\n+\t}\n+\n+\tif (entry->path && *entry->path) {\n+\t\tstruct strbuf path = STRBUF_INIT;\n+\n+\t\tstrbuf_addstr(&sb, \" path=\");\n+\t\tquote_path(entry->path, NULL, &path,\n+\t\t\t   QUOTE_PATH_QUOTE_SP | QUOTE_PATH_IGNORE_CONFIG);\n+\t\tstrbuf_addbuf(&sb, &path);\n+\n+\t\tstrbuf_release(&path);\n+\t}\n+\n+\tprintf(\"?%s%s\\n\", oid_to_hex(&entry->entry.oid), sb.buf);\n+\tstrbuf_release(&sb);\n+}\n+\n+static inline void finish_object__ma(struct object *obj, const char *name)\n {\n \t/*\n \t * Whether or not we try to dynamically fetch missing objects\n@@ -119,7 +166,8 @@ static inline void finish_object__ma(struct object *obj)\n \t\treturn;\n \n \tcase MA_PRINT:\n-\t\toidset_insert(&missing_objects, &obj->oid);\n+\tcase MA_PRINT_INFO:\n+\t\tadd_missing_object_entry(&obj->oid, name);\n \t\treturn;\n \n \tcase MA_ALLOW_PROMISOR:\n@@ -152,7 +200,7 @@ static void show_commit(struct commit *commit, void *data)\n \n \tif (revs->do_not_die_on_missing_objects &&\n \t    oidset_contains(&revs->missing_commits, &commit->object.oid)) {\n-\t\tfinish_object__ma(&commit->object);\n+\t\tfinish_object__ma(&commit->object, NULL);\n \t\treturn;\n \t}\n \n@@ -268,12 +316,11 @@ static void show_commit(struct commit *commit, void *data)\n \tfinish_commit(commit);\n }\n \n-static int finish_object(struct object *obj, const char *name UNUSED,\n-\t\t\t void *cb_data)\n+static int finish_object(struct object *obj, const char *name, void *cb_data)\n {\n \tstruct rev_list_info *info = cb_data;\n \tif (oid_object_info_extended(the_repository, &obj->oid, NULL, 0) < 0) {\n-\t\tfinish_object__ma(obj);\n+\t\tfinish_object__ma(obj, name);\n \t\treturn 1;\n \t}\n \tif (info->revs->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)\n@@ -414,6 +461,12 @@ static inline int parse_missing_action_value(const char *value)\n \t\treturn 1;\n \t}\n \n+\tif (!strcmp(value, \"print-info\")) {\n+\t\targ_missing_action = MA_PRINT_INFO;\n+\t\tfetch_if_missing = 0;\n+\t\treturn 1;\n+\t}\n+\n \tif (!strcmp(value, \"allow-promisor\")) {\n \t\targ_missing_action = MA_ALLOW_PROMISOR;\n \t\tfetch_if_missing = 0;\n@@ -781,10 +834,18 @@ int cmd_rev_list(int argc,\n \n \tif (arg_print_omitted)\n \t\toidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);\n-\tif (arg_missing_action == MA_PRINT) {\n-\t\toidset_init(&missing_objects, DEFAULT_OIDSET_SIZE);\n+\tif (arg_missing_action == MA_PRINT ||\n+\t    arg_missing_action == MA_PRINT_INFO) {\n+\t\tstruct oidset_iter iter;\n+\t\tstruct object_id *oid;\n+\n+\t\toidmap_init(&missing_objects, DEFAULT_OIDSET_SIZE);\n+\t\toidset_iter_init(&revs.missing_commits, &iter);\n+\n \t\t/* Add missing tips */\n-\t\toidset_insert_from_set(&missing_objects, &revs.missing_commits);\n+\t\twhile ((oid = oidset_iter_next(&iter)))\n+\t\t\tadd_missing_object_entry(oid, NULL);\n+\n \t\toidset_clear(&revs.missing_commits);\n \t}\n \n@@ -800,13 +861,20 @@ int cmd_rev_list(int argc,\n \t\t\tprintf(\"~%s\\n\", oid_to_hex(oid));\n \t\toidset_clear(&omitted_objects);\n \t}\n-\tif (arg_missing_action == MA_PRINT) {\n-\t\tstruct oidset_iter iter;\n-\t\tstruct object_id *oid;\n-\t\toidset_iter_init(&missing_objects, &iter);\n-\t\twhile ((oid = oidset_iter_next(&iter)))\n-\t\t\tprintf(\"?%s\\n\", oid_to_hex(oid));\n-\t\toidset_clear(&missing_objects);\n+\tif (arg_missing_action == MA_PRINT ||\n+\t    arg_missing_action == MA_PRINT_INFO) {\n+\t\tstruct missing_objects_map_entry *entry;\n+\t\tstruct oidmap_iter iter;\n+\n+\t\toidmap_iter_init(&missing_objects, &iter);\n+\n+\t\twhile ((entry = oidmap_iter_next(&iter))) {\n+\t\t\tprint_missing_object(entry, arg_missing_action ==\n+\t\t\t\t\t\t\t    MA_PRINT_INFO);\n+\t\t\tfree((void *)entry->path);\n+\t\t}\n+\n+\t\toidmap_free(&missing_objects, true);\n \t}\n \n \tstop_progress(&progress);\ndiff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh\nindex 7553a9cca2..38afca6f09 100755\n--- a/t/t6022-rev-list-missing.sh\n+++ b/t/t6022-rev-list-missing.sh\n@@ -145,4 +145,56 @@ do\n \tdone\n done\n \n+for obj in \"HEAD~1\" \"HEAD^{tree}\" \"HEAD:foo\" \"HEAD:foo/bar\" \"HEAD:baz baz\"\n+do\n+\ttest_expect_success \"--missing=print-info with missing '$obj'\" '\n+\t\ttest_when_finished rm -rf missing-info &&\n+\n+\t\tgit init missing-info &&\n+\t\t(\n+\t\t\tcd missing-info &&\n+\t\t\tgit commit --allow-empty -m first &&\n+\n+\t\t\tmkdir foo &&\n+\t\t\techo bar >foo/bar &&\n+\t\t\techo baz >\"baz baz\" &&\n+\t\t\techo bat >bat\\\" &&\n+\t\t\tgit add -A &&\n+\t\t\tgit commit -m second &&\n+\n+\t\t\toid=\"$(git rev-parse \"$obj\")\" &&\n+\t\t\tpath=\".git/objects/$(test_oid_to_path $oid)\" &&\n+\n+\t\t\tcase $obj in\n+\t\t\tHEAD:foo)\n+\t\t\t\tpath_info=\" path=foo\"\n+\t\t\t\t;;\n+\t\t\tHEAD:foo/bar)\n+\t\t\t\tpath_info=\" path=foo/bar\"\n+\t\t\t\t;;\n+\t\t\t\"HEAD:baz baz\")\n+\t\t\t\tpath_info=\" path=\\\"baz baz\\\"\"\n+\t\t\t\t;;\n+\t\t\t\"HEAD:bat\\\"\")\n+\t\t\t\tpath_info=\" path=\\\"bat\\\\\\\"\\\"\"\n+\t\t\t\t;;\n+\t\t\tesac &&\n+\n+\t\t\t# Before the object is made missing, we use rev-list to\n+\t\t\t# get the expected oids.\n+\t\t\tgit rev-list --objects --no-object-names \\\n+\t\t\t\tHEAD ^\"$obj\" >expect.raw &&\n+\t\t\techo \"?$oid$path_info\" >>expect.raw &&\n+\n+\t\t\tmv \"$path\" \"$path.hidden\" &&\n+\t\t\tgit rev-list --objects --no-object-names \\\n+\t\t\t\t--missing=print-info HEAD >actual.raw &&\n+\n+\t\t\tsort actual.raw >actual &&\n+\t\t\tsort expect.raw >expect &&\n+\t\t\ttest_cmp expect actual\n+\t\t)\n+\t'\n+done\n+\n test_done\n-- \n2.48.1.157.g3b0d05c4a7\n\n"},{"id":"511645","messageId":"20250201201658.11562-5-jltobler@gmail.com","threadId":"62767","inReplyTo":"20250201201658.11562-1-jltobler@gmail.com","subject":"[PATCH v3 4/4] rev-list: extend print-info to print missing object type","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-02-01T20:16:58Z","receivedAt":"2025-02-01T20:20:12Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Additional information about missing objects found in git-rev-list(1)\ncan be printed by specifying the `print-info` missing action for the\n`--missing` option. Extend this action to also print missing object type\ninformation inferred from its containing object. This token follows the\nform `type=<type>` and specifies the expected object type of the missing\nobject.\n\nSigned-off-by: Justin Tobler <jltobler@gmail.com>\n---\n Documentation/rev-list-options.txt |  3 +++\n builtin/rev-list.c                 | 11 ++++++++---\n t/t6022-rev-list-missing.sh        |  3 ++-\n 3 files changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 0bea9d4ad3..f10f78c600 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -1038,6 +1038,9 @@ one of the following:\n * The `path=<path>` shows the path of the missing object inferred from a\n   containing object. A path containing SP or special characters is enclosed in\n   double-quotes in the C style as needed.\n++\n+* The `type=<type>` shows the type of the missing object inferred from a\n+  containing object.\n --\n +\n If some tips passed to the traversal are missing, they will be\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 4a45a4e555..963f96d031 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -79,6 +79,7 @@ static int arg_print_omitted; /* print objects omitted by filter */\n struct missing_objects_map_entry {\n \tstruct oidmap_entry entry;\n \tconst char *path;\n+\tunsigned type;\n };\n static struct oidmap missing_objects;\n enum missing_action {\n@@ -109,7 +110,8 @@ static off_t get_object_disk_usage(struct object *obj)\n \treturn size;\n }\n \n-static void add_missing_object_entry(struct object_id *oid, const char *path)\n+static void add_missing_object_entry(struct object_id *oid, const char *path,\n+\t\t\t\t     unsigned type)\n {\n \tstruct missing_objects_map_entry *entry;\n \n@@ -118,6 +120,7 @@ static void add_missing_object_entry(struct object_id *oid, const char *path)\n \n \tCALLOC_ARRAY(entry, 1);\n \tentry->entry.oid = *oid;\n+\tentry->type = type;\n \tif (path)\n \t\tentry->path = xstrdup(path);\n \toidmap_put(&missing_objects, entry);\n@@ -143,6 +146,8 @@ static void print_missing_object(struct missing_objects_map_entry *entry,\n \n \t\tstrbuf_release(&path);\n \t}\n+\tif (entry->type)\n+\t\tstrbuf_addf(&sb, \" type=%s\", type_name(entry->type));\n \n \tprintf(\"?%s%s\\n\", oid_to_hex(&entry->entry.oid), sb.buf);\n \tstrbuf_release(&sb);\n@@ -167,7 +172,7 @@ static inline void finish_object__ma(struct object *obj, const char *name)\n \n \tcase MA_PRINT:\n \tcase MA_PRINT_INFO:\n-\t\tadd_missing_object_entry(&obj->oid, name);\n+\t\tadd_missing_object_entry(&obj->oid, name, obj->type);\n \t\treturn;\n \n \tcase MA_ALLOW_PROMISOR:\n@@ -844,7 +849,7 @@ int cmd_rev_list(int argc,\n \n \t\t/* Add missing tips */\n \t\twhile ((oid = oidset_iter_next(&iter)))\n-\t\t\tadd_missing_object_entry(oid, NULL);\n+\t\t\tadd_missing_object_entry(oid, NULL, 0);\n \n \t\toidset_clear(&revs.missing_commits);\n \t}\ndiff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh\nindex 38afca6f09..3e2790d4c8 100755\n--- a/t/t6022-rev-list-missing.sh\n+++ b/t/t6022-rev-list-missing.sh\n@@ -164,6 +164,7 @@ do\n \n \t\t\toid=\"$(git rev-parse \"$obj\")\" &&\n \t\t\tpath=\".git/objects/$(test_oid_to_path $oid)\" &&\n+\t\t\ttype_info=\" type=$(git cat-file -t $oid)\" &&\n \n \t\t\tcase $obj in\n \t\t\tHEAD:foo)\n@@ -184,7 +185,7 @@ do\n \t\t\t# get the expected oids.\n \t\t\tgit rev-list --objects --no-object-names \\\n \t\t\t\tHEAD ^\"$obj\" >expect.raw &&\n-\t\t\techo \"?$oid$path_info\" >>expect.raw &&\n+\t\t\techo \"?$oid$path_info$type_info\" >>expect.raw &&\n \n \t\t\tmv \"$path\" \"$path.hidden\" &&\n \t\t\tgit rev-list --objects --no-object-names \\\n-- \n2.48.1.157.g3b0d05c4a7\n\n"},{"id":"511654","messageId":"76390e3b-e749-4d28-98a5-05db7c5fbcd3@gmail.com","threadId":"62767","inReplyTo":"20250201201658.11562-3-jltobler@gmail.com","subject":"Re: [PATCH v3 2/4] quote: add quote_path() flag to ignore config","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-02T10:52:41Z","receivedAt":"2025-02-02T10:52:46Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Justin\n\nOn 01/02/2025 20:16, Justin Tobler wrote:\n> The `quote_path()` function invokes `quote_c_style_counted()` to handle\n> quoting. This means the output `quote_path()` is ultimately affected by\n> `core.quotePath` configuration. In a subsequent commit, `quote_path()`\n> will be used in a scenario where the output should remain consistent\n> regardless of the current configuration.\n> \n> Introduce the `QUOTE_PATH_IGNORE_CONFIG` flag for `quote_path()`which\n> when set instructs the underlying `quote_c_style_counted()` to also\n> ignore the `core.quotePath` configuration when executed.\n\nI'm confused as to why this is necessary. All of our existing plumbing \ncommands that print paths respect core.quotePath so why is rev-list \ndifferent? The config setting only affects the representation used for \nbytes 0x80 and above, control characters, backslash and double-quote are \nalways quoted.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Justin Tobler <jltobler@gmail.com>\n> ---\n>   quote.c | 13 ++++++++++---\n>   quote.h |  3 ++-\n>   2 files changed, 12 insertions(+), 4 deletions(-)\n> \n> diff --git a/quote.c b/quote.c\n> index d129c1de70..baec34ca94 100644\n> --- a/quote.c\n> +++ b/quote.c\n> @@ -370,10 +370,18 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne\n>   {\n>   \tstruct strbuf sb = STRBUF_INIT;\n>   \tconst char *rel = relative_path(in, prefix, &sb);\n> -\tint force_dq = ((flags & QUOTE_PATH_QUOTE_SP) && strchr(rel, ' '));\n> +\tunsigned cquote_flags = 0;\n> +\tint force_dq = 0;\n>   \n>   \tstrbuf_reset(out);\n>   \n> +\tif ((flags & QUOTE_PATH_QUOTE_SP) && strchr(rel, ' ')) {\n> +\t\tforce_dq = 1;\n> +\t\tcquote_flags &= CQUOTE_NODQ;\n> +\t}\n> +\tif (flags & QUOTE_PATH_IGNORE_CONFIG)\n> +\t\tcquote_flags &= CQUOTE_IGNORE_CONFIG;\n> +\n>   \t/*\n>   \t * If the caller wants us to enclose the output in a dq-pair\n>   \t * whether quote_c_style_counted() needs to, we do it ourselves\n> @@ -381,8 +389,7 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne\n>   \t */\n>   \tif (force_dq)\n>   \t\tstrbuf_addch(out, '\"');\n> -\tquote_c_style_counted(rel, strlen(rel), out, NULL,\n> -\t\t\t      force_dq ? CQUOTE_NODQ : 0);\n> +\tquote_c_style_counted(rel, strlen(rel), out, NULL, cquote_flags);\n>   \tif (force_dq)\n>   \t\tstrbuf_addch(out, '\"');\n>   \tstrbuf_release(&sb);\n> diff --git a/quote.h b/quote.h\n> index 2a793fbef6..84903951ef 100644\n> --- a/quote.h\n> +++ b/quote.h\n> @@ -94,7 +94,8 @@ void write_name_quoted_relative(const char *name, const char *prefix,\n>   \n>   /* quote path as relative to the given prefix */\n>   char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags);\n> -#define QUOTE_PATH_QUOTE_SP 01\n> +#define QUOTE_PATH_QUOTE_SP\t (1u << 0)\n> +#define QUOTE_PATH_IGNORE_CONFIG (1u << 1)\n>   \n>   /* quoting as a string literal for other languages */\n>   void perl_quote_buf(struct strbuf *sb, const char *src);\n\n"},{"id":"511724","messageId":"CAP8UFD2y=di9iKF+-nm90bg6zVnQ10OW+bvAyxTPM+xKpMYfLA@mail.gmail.com","threadId":"62767","inReplyTo":"20250201201658.11562-2-jltobler@gmail.com","subject":"Re: [PATCH v3 1/4] quote: add c quote flag to ignore core.quotePath","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-02-03T09:51:13Z","receivedAt":"2025-02-03T09:51:27Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, Feb 1, 2025 at 9:20 PM Justin Tobler <jltobler@gmail.com> wrote:\n\n\n> -static inline int cq_must_quote(char c)\n> +static inline int cq_must_quote(char c, int ignore_config)\n\nI think it's a bit better to use 'unsigned int' instead of just 'int'\nfor such flags, but it's fine here to use an 'int' because both\n`quote_path_fully` and `no_dq` below already use that type.\n\n> -               for (len = 0; len < maxlen && !cq_must_quote(s[len]); len++);\n> +               for (len = 0;\n> +                    len < maxlen && !cq_must_quote(s[len], ignore_config); len++);\n\nMicronit: If you really want to split the line into many lines, I\nthink it might be better to go all the way like this:\n\n              for (len = 0;\n                   len < maxlen && !cq_must_quote(s[len], ignore_config);\n                   len++);\n\n> @@ -83,7 +83,8 @@ int sq_dequote_to_strvec(char *arg, struct strvec *);\n>  int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);\n>\n>  /* Bits in the flags parameter to quote_c_style() */\n> -#define CQUOTE_NODQ 01\n> +#define CQUOTE_NODQ         (1u << 0)\n\nNice small cleanup which could be mentioned in the commit message if\nyou reroll this series.\n\n> +#define CQUOTE_IGNORE_CONFIG (1u << 1)\n"},{"id":"511726","messageId":"CAP8UFD0bPXkryaBRQCJZfOOPXBRmxs3EMN9mn1SmBBbvmb7YjQ@mail.gmail.com","threadId":"62767","inReplyTo":"20250201201658.11562-3-jltobler@gmail.com","subject":"Re: [PATCH v3 2/4] quote: add quote_path() flag to ignore config","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-02-03T10:07:56Z","receivedAt":"2025-02-03T10:08:09Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, Feb 1, 2025 at 9:20 PM Justin Tobler <jltobler@gmail.com> wrote:\n\n> @@ -370,10 +370,18 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne\n>  {\n>         struct strbuf sb = STRBUF_INIT;\n>         const char *rel = relative_path(in, prefix, &sb);\n> -       int force_dq = ((flags & QUOTE_PATH_QUOTE_SP) && strchr(rel, ' '));\n> +       unsigned cquote_flags = 0;\n> +       int force_dq = 0;\n\nI don't think you needed to change how force_dq was computed.\n\n>         strbuf_reset(out);\n>\n> +       if ((flags & QUOTE_PATH_QUOTE_SP) && strchr(rel, ' ')) {\n> +               force_dq = 1;\n> +               cquote_flags &= CQUOTE_NODQ;\n> +       }\n\nIf you didn't change how force_dq was computed above, then this could be just:\n\n     if (force_dq)\n              cquote_flags &= CQUOTE_NODQ;\n\n>  /* quote path as relative to the given prefix */\n>  char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags);\n> -#define QUOTE_PATH_QUOTE_SP 01\n> +#define QUOTE_PATH_QUOTE_SP     (1u << 0)\n\nNice cleanup that could be mentioned in the commit message if you reroll.\n\n> +#define QUOTE_PATH_IGNORE_CONFIG (1u << 1)\n"},{"id":"511727","messageId":"CAP8UFD0vbv7tRP1cac-q-Qqk7wxo57oP3VLqj7Gn_kHi41y=hQ@mail.gmail.com","threadId":"62767","inReplyTo":"20250201201658.11562-1-jltobler@gmail.com","subject":"Re: [PATCH v3 0/4] rev-list: print additional missing object information","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-02-03T10:45:49Z","receivedAt":"2025-02-03T10:46:04Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Sat, Feb 1, 2025 at 9:20 PM Justin Tobler <jltobler@gmail.com> wrote:\n>\n> Greetings,\n>\n> It is possible to configure git-rev-list(1) to print the OID of missing\n> objects by setting the `--missing=print` option. While it is useful\n> knowing about these objects, it would be nice to have even more context\n> about the objects that are missing. Luckily, from an object containing\n> the missing object, it is possible to infer additional information the\n> missing object. For example, if the tree containing a missing blob still\n> exists, the tree entry for the missing object should contain path and\n> type information.\n>\n> This series aims to provide git-rev-list(1) with a new `print-info`\n> missing action for the `--missing` option that, when set, behaves like\n> the existing `print` action but also prints other potentially\n> interesting information about the missing object.\n\nI took a look and commented a bit on patches 1/4 and 2/4. Not sure my\ncomments are worth a reroll on their own. The other patches look good\nto me.\n\nAnyway I think you might want to address Phillip Wood's concerns too:\n\nhttps://lore.kernel.org/git/76390e3b-e749-4d28-98a5-05db7c5fbcd3@gmail.com/\n\nThanks.\n"},{"id":"511761","messageId":"xmqqcyfy1xr9.fsf@gitster.g","threadId":"62767","inReplyTo":"CAP8UFD2y=di9iKF+-nm90bg6zVnQ10OW+bvAyxTPM+xKpMYfLA@mail.gmail.com","subject":"Re: [PATCH v3 1/4] quote: add c quote flag to ignore core.quotePath","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-03T22:14:50Z","receivedAt":"2025-02-03T22:14:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <christian.couder@gmail.com> writes:\n\n> On Sat, Feb 1, 2025 at 9:20 PM Justin Tobler <jltobler@gmail.com> wrote:\n>\n>\n>> -static inline int cq_must_quote(char c)\n>> +static inline int cq_must_quote(char c, int ignore_config)\n>\n> I think it's a bit better to use 'unsigned int' instead of just 'int'\n> for such flags, but it's fine here to use an 'int' because both\n> `quote_path_fully` and `no_dq` below already use that type.\n\nYup, good forward thinking to suggest using unsigned as these \"this\nis just a single bit, so let's use platform natural int\" tend to\ngrow into \"there is this another bit that is orthogonal, so pass it\nas well in the same flag word\", at which point unsigned would work\nbetter for us.  But until that happens, plain platform natural int\nis fine.\n\n>> -               for (len = 0; len < maxlen && !cq_must_quote(s[len]); len++);\n>> +               for (len = 0;\n>> +                    len < maxlen && !cq_must_quote(s[len], ignore_config); len++);\n>\n> Micronit: If you really want to split the line into many lines, I\n> think it might be better to go all the way like this:\n>\n>               for (len = 0;\n>                    len < maxlen && !cq_must_quote(s[len], ignore_config);\n>                    len++);\n\nGood style suggestion.\n\nThanks.\n\n"},{"id":"511762","messageId":"xmqq4j1a1wvr.fsf@gitster.g","threadId":"62767","inReplyTo":"20250201201658.11562-2-jltobler@gmail.com","subject":"Re: [PATCH v3 1/4] quote: add c quote flag to ignore core.quotePath","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-03T22:33:44Z","receivedAt":"2025-02-03T22:33:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Justin Tobler <jltobler@gmail.com> writes:\n\n> The output of `cq_must_quote()` is affected by `core.quotePath`. This is\n> undesirable for operations that want to ensure consistent output\n> independent of config settings.\n>\n> Introduce the `CQUOTE_IGNORE_CONFIG` flag for the `quote_c_style*`\n> functions which when set makes `cq_must_quote()` always follow the\n> default behavior (core.quotePath=true) regardless of how its set in the\n> config.\n\nHmph.  \n\nI was hoping that we can flip the default for 'core.quotePath' to\n'no' at Git 3.0 boundary, to help folks in non-ASCII locale.  If\nthis is about emitting pipe-able output out of rev-list, unlike a\npatch that is to be e-mailed (and being 8-bit clean was a risky\nassumption to make in 2005) that core.quotePath was originally\ninvented for, it is more so that we would not want to force the\nreceiving end to unquote, no?\n\nSo regardless of what the future default value of core.quotePath\nwould be, I am not convinced that it is a good idea to octal quote\nany and all bytes outside the ASCII range in the rev-list output.\n\nAfter all, \"git rev-list --objects\" would show such a path without\nquoting, no [*]?\n\n    Side note: the path in the output from \"git rev-list --objects\"\n    is a hack to allow receiving end to compute a path hash, and\n    does not have to be strictly reversible, so it emits verbatim\n    bytes but truncates the output at LF to preserve the one-line\n    one-object output format.\n\nWe do need to quote certain bytes (e.g., LF cannot be allowed\nverbatim, when the output is line-oriented, and we use C-quote,\nwhich means literal double-quote needs to be also quoted), so we\ncannot mimic paths emitted by \"git rev-list --objects\", but I do not\nthink it buys us much to quote non-ASCII bytes these days.\n\n"},{"id":"511763","messageId":"xmqqtt9azlnx.fsf@gitster.g","threadId":"62767","inReplyTo":"20250201201658.11562-3-jltobler@gmail.com","subject":"Re: [PATCH v3 2/4] quote: add quote_path() flag to ignore config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-03T22:52:02Z","receivedAt":"2025-02-03T22:52:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Justin Tobler <jltobler@gmail.com> writes:\n\n> The `quote_path()` function invokes `quote_c_style_counted()` to handle\n> quoting. This means the output `quote_path()` is ultimately affected by\n> `core.quotePath` configuration. In a subsequent commit, `quote_path()`\n> will be used in a scenario where the output should remain consistent\n> regardless of the current configuration.\n>\n> Introduce the `QUOTE_PATH_IGNORE_CONFIG` flag for `quote_path()`which\n> when set instructs the underlying `quote_c_style_counted()` to also\n> ignore the `core.quotePath` configuration when executed.\n>\n> Signed-off-by: Justin Tobler <jltobler@gmail.com>\n> ---\n\nI've already read what [3/4] does using this, but I do not quite see\nwhy the scenario the quote_path() function is called requires that\nthe output should remain consistent.\n\nAs it is possible that a path has any byte other than NUL in it, the\nrecipient MUST be prepared to unquote it before being able to use\nit.  So if a run of \"rev-list --info\" in the same repository with\nthe same arguments tells a path as \"abc\" (without quotes) and\nanother one shows the same path as \"abc\" (with quotes) or even as\n\"\\141\\142\\143\", the consumer should be able to cope with the\nvariations and see that they are the same path, no?  At least, I do\nnot quite see why we require consistency across different settings\nof the configuration.\n\nIf we drop the first two patches, as long as we use a known value in\ncore.quotepath in tests in t6022, everything should reliably work,\nno?\n\nWhen we do need to quote SP (i.e. outside the \"diff\" output, for\nwhich originally cquote machinery was invented), the default cquote\nmechanism does not help, so we need to allow _some_ tweak from the\ncaller's side, but I do not know if I agree that \"no, we do not\nallow bytes with 8-bit set without quoting\" is a good idea in this\nage.\n\nThanks.\n\n>  quote.c | 13 ++++++++++---\n>  quote.h |  3 ++-\n>  2 files changed, 12 insertions(+), 4 deletions(-)\n>\n> diff --git a/quote.c b/quote.c\n> index d129c1de70..baec34ca94 100644\n> --- a/quote.c\n> +++ b/quote.c\n> @@ -370,10 +370,18 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne\n>  {\n>  \tstruct strbuf sb = STRBUF_INIT;\n>  \tconst char *rel = relative_path(in, prefix, &sb);\n> -\tint force_dq = ((flags & QUOTE_PATH_QUOTE_SP) && strchr(rel, ' '));\n> +\tunsigned cquote_flags = 0;\n> +\tint force_dq = 0;\n>  \n>  \tstrbuf_reset(out);\n>  \n> +\tif ((flags & QUOTE_PATH_QUOTE_SP) && strchr(rel, ' ')) {\n> +\t\tforce_dq = 1;\n> +\t\tcquote_flags &= CQUOTE_NODQ;\n> +\t}\n> +\tif (flags & QUOTE_PATH_IGNORE_CONFIG)\n> +\t\tcquote_flags &= CQUOTE_IGNORE_CONFIG;\n> +\n>  \t/*\n>  \t * If the caller wants us to enclose the output in a dq-pair\n>  \t * whether quote_c_style_counted() needs to, we do it ourselves\n> @@ -381,8 +389,7 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne\n>  \t */\n>  \tif (force_dq)\n>  \t\tstrbuf_addch(out, '\"');\n> -\tquote_c_style_counted(rel, strlen(rel), out, NULL,\n> -\t\t\t      force_dq ? CQUOTE_NODQ : 0);\n> +\tquote_c_style_counted(rel, strlen(rel), out, NULL, cquote_flags);\n>  \tif (force_dq)\n>  \t\tstrbuf_addch(out, '\"');\n>  \tstrbuf_release(&sb);\n> diff --git a/quote.h b/quote.h\n> index 2a793fbef6..84903951ef 100644\n> --- a/quote.h\n> +++ b/quote.h\n> @@ -94,7 +94,8 @@ void write_name_quoted_relative(const char *name, const char *prefix,\n>  \n>  /* quote path as relative to the given prefix */\n>  char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigned flags);\n> -#define QUOTE_PATH_QUOTE_SP 01\n> +#define QUOTE_PATH_QUOTE_SP\t (1u << 0)\n> +#define QUOTE_PATH_IGNORE_CONFIG (1u << 1)\n>  \n>  /* quoting as a string literal for other languages */\n>  void perl_quote_buf(struct strbuf *sb, const char *src);\n"},{"id":"511817","messageId":"xmqqikppvf2t.fsf@gitster.g","threadId":"62767","inReplyTo":"xmqq4j1a1wvr.fsf@gitster.g","subject":"Re: [PATCH v3 1/4] quote: add c quote flag to ignore core.quotePath","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-04T16:40:10Z","receivedAt":"2025-02-04T16:40:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> So regardless of what the future default value of core.quotePath\n> would be, I am not convinced that it is a good idea to octal quote\n> any and all bytes outside the ASCII range in the rev-list output.\n>\n> After all, \"git rev-list --objects\" would show such a path without\n> quoting, no [*]?\n>\n>     Side note: the path in the output from \"git rev-list --objects\"\n>     is a hack to allow receiving end to compute a path hash, and\n>     does not have to be strictly reversible, so it emits verbatim\n>     bytes but truncates the output at LF to preserve the one-line\n>     one-object output format.\n>\n> We do need to quote certain bytes (e.g., LF cannot be allowed\n> verbatim, when the output is line-oriented, and we use C-quote,\n> which means literal double-quote needs to be also quoted), so we\n> cannot mimic paths emitted by \"git rev-list --objects\", but I do not\n> think it buys us much to quote non-ASCII bytes these days.\n\nRereading this I realize that I was not quite making sense.\n\nA short version of what I wanted to say is:\n\n - The output format need to do some quoting anyway because it is\n   inevitable to make the string stuffed as \"value\" in a space\n   separated list of var=value on a single line.\n\n - It does not really matter if core.quotePath allows us to pass\n   bytes with 8-bit set (e.g. UTF-8 outside ASCII) unquoted or\n   require quoting.  The receiving end must be prepared to unquote,\n   so it is dubious that a new feature to ignore core.quotePath is\n   needed.\n\n - We do need to quote SP that cquote does not require quoting, so\n   some wrapping around quote_c_style() like quote_path() does is\n   needed.\n\nThanks.\n"},{"id":"511835","messageId":"ksta3edbxwkl62u36xif63q4ebux5hdaxkbme6v6li5v3sgndb@nuu5yyp5clbk","threadId":"62767","inReplyTo":"76390e3b-e749-4d28-98a5-05db7c5fbcd3@gmail.com","subject":"Re: [PATCH v3 2/4] quote: add quote_path() flag to ignore config","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-02-04T22:39:36Z","receivedAt":"2025-02-04T22:42:35Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/02/02 10:52AM, Phillip Wood wrote:\n> Hi Justin\n> \n> On 01/02/2025 20:16, Justin Tobler wrote:\n> > The `quote_path()` function invokes `quote_c_style_counted()` to handle\n> > quoting. This means the output `quote_path()` is ultimately affected by\n> > `core.quotePath` configuration. In a subsequent commit, `quote_path()`\n> > will be used in a scenario where the output should remain consistent\n> > regardless of the current configuration.\n> > \n> > Introduce the `QUOTE_PATH_IGNORE_CONFIG` flag for `quote_path()`which\n> > when set instructs the underlying `quote_c_style_counted()` to also\n> > ignore the `core.quotePath` configuration when executed.\n> \n> I'm confused as to why this is necessary. All of our existing plumbing\n> commands that print paths respect core.quotePath so why is rev-list\n> different? The config setting only affects the representation used for bytes\n> 0x80 and above, control characters, backslash and double-quote are always\n> quoted.\n\nYou are correct that it isn't neccesary. From a previous discussion, I\ninitally thought it would preferrable to have consistent output for this\nplumbing operation, but as you pointed out other plumbing commands\nrespect core.quotePath. I'll drop these first two patches in the next\nversion.\n\nThanks for the feedback!\n-Justin\n"},{"id":"511836","messageId":"i4nwrzqjhuvo26kt45aykazut7rgvhh5w474dkqay3fhmnuvtx@czemalalwuwm","threadId":"62767","inReplyTo":"xmqqikppvf2t.fsf@gitster.g","subject":"Re: [PATCH v3 1/4] quote: add c quote flag to ignore core.quotePath","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-02-04T22:50:06Z","receivedAt":"2025-02-04T22:53:04Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/02/04 08:40AM, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > So regardless of what the future default value of core.quotePath\n> > would be, I am not convinced that it is a good idea to octal quote\n> > any and all bytes outside the ASCII range in the rev-list output.\n> >\n> > After all, \"git rev-list --objects\" would show such a path without\n> > quoting, no [*]?\n> >\n> >     Side note: the path in the output from \"git rev-list --objects\"\n> >     is a hack to allow receiving end to compute a path hash, and\n> >     does not have to be strictly reversible, so it emits verbatim\n> >     bytes but truncates the output at LF to preserve the one-line\n> >     one-object output format.\n> >\n> > We do need to quote certain bytes (e.g., LF cannot be allowed\n> > verbatim, when the output is line-oriented, and we use C-quote,\n> > which means literal double-quote needs to be also quoted), so we\n> > cannot mimic paths emitted by \"git rev-list --objects\", but I do not\n> > think it buys us much to quote non-ASCII bytes these days.\n> \n> Rereading this I realize that I was not quite making sense.\n> \n> A short version of what I wanted to say is:\n> \n>  - The output format need to do some quoting anyway because it is\n>    inevitable to make the string stuffed as \"value\" in a space\n>    separated list of var=value on a single line.\n> \n>  - It does not really matter if core.quotePath allows us to pass\n>    bytes with 8-bit set (e.g. UTF-8 outside ASCII) unquoted or\n>    require quoting.  The receiving end must be prepared to unquote,\n>    so it is dubious that a new feature to ignore core.quotePath is\n>    needed.\n\nI agree, thinking about this some more, forcing the value quoting\nbehavior to act as if core.quotePath is always enabled is a step in the\nwrong direction. If anything, we could force it disabled to be more\nfriendly to non-ASCII characters, but that is probably not worth it\neither.\n\n>  - We do need to quote SP that cquote does not require quoting, so\n>    some wrapping around quote_c_style() like quote_path() does is\n>    needed.\n\nI think using quote_path() as-is should be sufficient. I'll drop the\nfirst two patches from this series in the next version I send and adapt\naccordingly.\n\nThanks\n-Justin\n"},{"id":"511837","messageId":"ykyebummbcnkwsk4svzqvt2ofluyn4uyghnykobjatv6mfveef@3dg5hkw56dzi","threadId":"62767","inReplyTo":"CAP8UFD0vbv7tRP1cac-q-Qqk7wxo57oP3VLqj7Gn_kHi41y=hQ@mail.gmail.com","subject":"Re: [PATCH v3 0/4] rev-list: print additional missing object information","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-02-04T22:51:24Z","receivedAt":"2025-02-04T22:54:22Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/02/03 11:45AM, Christian Couder wrote:\n> On Sat, Feb 1, 2025 at 9:20 PM Justin Tobler <jltobler@gmail.com> wrote:\n> >\n> > Greetings,\n> >\n> > It is possible to configure git-rev-list(1) to print the OID of missing\n> > objects by setting the `--missing=print` option. While it is useful\n> > knowing about these objects, it would be nice to have even more context\n> > about the objects that are missing. Luckily, from an object containing\n> > the missing object, it is possible to infer additional information the\n> > missing object. For example, if the tree containing a missing blob still\n> > exists, the tree entry for the missing object should contain path and\n> > type information.\n> >\n> > This series aims to provide git-rev-list(1) with a new `print-info`\n> > missing action for the `--missing` option that, when set, behaves like\n> > the existing `print` action but also prints other potentially\n> > interesting information about the missing object.\n> \n> I took a look and commented a bit on patches 1/4 and 2/4. Not sure my\n> comments are worth a reroll on their own. The other patches look good\n> to me.\n> \n> Anyway I think you might want to address Phillip Wood's concerns too:\n> \n> https://lore.kernel.org/git/76390e3b-e749-4d28-98a5-05db7c5fbcd3@gmail.com/\n> \n> Thanks.\n\nThanks Christian for the review!\n\n-Justin\n"},{"id":"511839","messageId":"20250205004147.887106-1-jltobler@gmail.com","threadId":"62767","inReplyTo":"20250201201658.11562-1-jltobler@gmail.com","subject":"[PATCH v4 0/2] rev-list: print additional missing object information","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-02-05T00:41:45Z","receivedAt":"2025-02-05T00:45:08Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Greetings,\n\nIt is possible to configure git-rev-list(1) to print the OID of missing\nobjects by setting the `--missing=print` option. While it is useful\nknowing about these objects, it would be nice to have even more context\nabout the objects that are missing. Luckily, from an object containing\nthe missing object, it is possible to infer additional information the\nmissing object. For example, if the tree containing a missing blob still\nexists, the tree entry for the missing object should contain path and\ntype information.\n\nThis series aims to provide git-rev-list(1) with a new `print-info`\nmissing action for the `--missing` option that, when set, behaves like\nthe existing `print` action but also prints other potentially\ninteresting information about the missing object.\n\nMissing object info is printed in the form `?<oid> [<token>=<value>]...`\nwhere multiple `<token>=<value>` pairs may be specified each separated\nfrom each other with a SP. Values that contain SP or LF characters are\nexpected to be encoded in a manner such that these problematic bytes are\nhandled. For missing object path information this is handled by quoting\nthe path in the C style if it contains SP or special characters.\n\nOne concern I currently have with this quoting approach is that it is a\nbit more challenging to machine parse compared to something like using a\nnull byte to delimit between missing info. One option is, in a followup\nseries, introduce a git-for-each-ref(1) style format syntax. Maybe\nsomething like: `--missing=print-info:%(path)%00%(type)`. I'm curious if\nanyone may have thoughts around this. My goal is to ensure that there is\nan easy to use machine parsable interface to get this information. I\ncould see something like `?<oid> path=\"foo \\\"bar\" type=blob`, being a\nbit complex.\n\nThe series is set up as follows:\n\n- Patch 1 introduces the `print-info` missing action and supports\n  printing missing object path information.\n\n- Patch 2 extends the `print-info` missing action to also print object\n  type information about the missing object.\n\nChanges in V4:\n\n- The core.quotePath behavior is no longer force enabled for the missing\n  info values. Consequently the first two patches from the previous\n  version are dropped.\n\nThanks,\n-Justin\n\nJustin Tobler (2):\n  rev-list: add print-info action to print missing object path\n  rev-list: extend print-info to print missing object type\n\n Documentation/rev-list-options.txt |  19 ++++++\n builtin/rev-list.c                 | 106 ++++++++++++++++++++++++-----\n t/t6022-rev-list-missing.sh        |  53 +++++++++++++++\n 3 files changed, 161 insertions(+), 17 deletions(-)\n\nRange-diff against v3:\n1:  f628728300 < -:  ---------- quote: add c quote flag to ignore core.quotePath\n2:  53a3811d8f < -:  ---------- quote: add quote_path() flag to ignore config\n3:  fe7a3da8de ! 1:  e3d5295b4d rev-list: add print-info action to print missing object path\n    @@ builtin/rev-list.c: static off_t get_object_disk_usage(struct object *obj)\n     +\t\tstruct strbuf path = STRBUF_INIT;\n     +\n     +\t\tstrbuf_addstr(&sb, \" path=\");\n    -+\t\tquote_path(entry->path, NULL, &path,\n    -+\t\t\t   QUOTE_PATH_QUOTE_SP | QUOTE_PATH_IGNORE_CONFIG);\n    ++\t\tquote_path(entry->path, NULL, &path, QUOTE_PATH_QUOTE_SP);\n     +\t\tstrbuf_addbuf(&sb, &path);\n     +\n     +\t\tstrbuf_release(&path);\n4:  788b497d00 = 2:  6aa71444d3 rev-list: extend print-info to print missing object type\n\nbase-commit: b74ff38af58464688b211140b90ec90598d340c6\n-- \n2.48.1.157.g3b0d05c4a7\n\n"},{"id":"511840","messageId":"20250205004147.887106-2-jltobler@gmail.com","threadId":"62767","inReplyTo":"20250205004147.887106-1-jltobler@gmail.com","subject":"[PATCH v4 1/2] rev-list: add print-info action to print missing object path","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-02-05T00:41:46Z","receivedAt":"2025-02-05T00:45:22Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Missing objects identified through git-rev-list(1) can be printed by\nsetting the `--missing=print` option. Additional information about the\nmissing object, such as its path and type, may be present in its\ncontaining object.\n\nAdd the `print-info` missing action for the `--missing` option that,\nwhen set, prints additional insight about the missing object inferred\nfrom its containing object. Each line of output for a missing object is\nin the form: `?<oid> [<token>=<value>]...`. The `<token>=<value>` pairs\ncontaining additional information are separated from each other by a SP.\nThe value is encoded in a token specific fashion, but SP or LF contained\nin value are always expected to be represented in such a way that the\nresulting encoded value does not have either of these two problematic\nbytes. This format is kept generic so it can be extended in the future\nto support additional information.\n\nFor now, only a missing object path info is implemented. It follows the\nform `path=<path>` and specifies the full path to the object from the\ntop-level tree. A path containing SP or special characters is enclosed\nin double-quotes in the C style as needed. In a subsequent commit,\nmissing object type info will also be added.\n\nSigned-off-by: Justin Tobler <jltobler@gmail.com>\n---\n Documentation/rev-list-options.txt |  16 +++++\n builtin/rev-list.c                 | 101 ++++++++++++++++++++++++-----\n t/t6022-rev-list-missing.sh        |  52 +++++++++++++++\n 3 files changed, 152 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 459e5a02f5..0bea9d4ad3 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -1024,6 +1024,22 @@ Unexpected missing objects will raise an error.\n The form '--missing=print' is like 'allow-any', but will also print a\n list of the missing objects.  Object IDs are prefixed with a ``?'' character.\n +\n+The form '--missing=print-info' is like 'print', but will also print additional\n+information about the missing object inferred from its containing object. The\n+information is all printed on the same line with the missing object ID in the\n+form: `?<oid> [<token>=<value>]...`. The `<token>=<value>` pairs containing\n+additional information are separated from each other by a SP. The value is\n+encoded in a token specific fashion, but SP or LF contained in value are always\n+expected to be represented in such a way that the resulting encoded value does\n+not have either of these two problematic bytes. Each `<token>=<value>` may be\n+one of the following:\n++\n+--\n+* The `path=<path>` shows the path of the missing object inferred from a\n+  containing object. A path containing SP or special characters is enclosed in\n+  double-quotes in the C style as needed.\n+--\n++\n If some tips passed to the traversal are missing, they will be\n considered as missing too, and the traversal will ignore them. In case\n we cannot get their Object ID though, an error will be raised.\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 3196da7b2d..00f157eb68 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -22,7 +22,10 @@\n #include \"progress.h\"\n #include \"reflog-walk.h\"\n #include \"oidset.h\"\n+#include \"oidmap.h\"\n #include \"packfile.h\"\n+#include \"quote.h\"\n+#include \"strbuf.h\"\n \n static const char rev_list_usage[] =\n \"git rev-list [<options>] <commit>... [--] [<path>...]\\n\"\n@@ -73,11 +76,16 @@ static unsigned progress_counter;\n static struct oidset omitted_objects;\n static int arg_print_omitted; /* print objects omitted by filter */\n \n-static struct oidset missing_objects;\n+struct missing_objects_map_entry {\n+\tstruct oidmap_entry entry;\n+\tconst char *path;\n+};\n+static struct oidmap missing_objects;\n enum missing_action {\n \tMA_ERROR = 0,    /* fail if any missing objects are encountered */\n \tMA_ALLOW_ANY,    /* silently allow ALL missing objects */\n \tMA_PRINT,        /* print ALL missing objects in special section */\n+\tMA_PRINT_INFO,   /* same as MA_PRINT but also prints missing object info */\n \tMA_ALLOW_PROMISOR, /* silently allow all missing PROMISOR objects */\n };\n static enum missing_action arg_missing_action;\n@@ -101,7 +109,45 @@ static off_t get_object_disk_usage(struct object *obj)\n \treturn size;\n }\n \n-static inline void finish_object__ma(struct object *obj)\n+static void add_missing_object_entry(struct object_id *oid, const char *path)\n+{\n+\tstruct missing_objects_map_entry *entry;\n+\n+\tif (oidmap_get(&missing_objects, oid))\n+\t\treturn;\n+\n+\tCALLOC_ARRAY(entry, 1);\n+\tentry->entry.oid = *oid;\n+\tif (path)\n+\t\tentry->path = xstrdup(path);\n+\toidmap_put(&missing_objects, entry);\n+}\n+\n+static void print_missing_object(struct missing_objects_map_entry *entry,\n+\t\t\t\t int print_missing_info)\n+{\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tif (!print_missing_info) {\n+\t\tprintf(\"?%s\\n\", oid_to_hex(&entry->entry.oid));\n+\t\treturn;\n+\t}\n+\n+\tif (entry->path && *entry->path) {\n+\t\tstruct strbuf path = STRBUF_INIT;\n+\n+\t\tstrbuf_addstr(&sb, \" path=\");\n+\t\tquote_path(entry->path, NULL, &path, QUOTE_PATH_QUOTE_SP);\n+\t\tstrbuf_addbuf(&sb, &path);\n+\n+\t\tstrbuf_release(&path);\n+\t}\n+\n+\tprintf(\"?%s%s\\n\", oid_to_hex(&entry->entry.oid), sb.buf);\n+\tstrbuf_release(&sb);\n+}\n+\n+static inline void finish_object__ma(struct object *obj, const char *name)\n {\n \t/*\n \t * Whether or not we try to dynamically fetch missing objects\n@@ -119,7 +165,8 @@ static inline void finish_object__ma(struct object *obj)\n \t\treturn;\n \n \tcase MA_PRINT:\n-\t\toidset_insert(&missing_objects, &obj->oid);\n+\tcase MA_PRINT_INFO:\n+\t\tadd_missing_object_entry(&obj->oid, name);\n \t\treturn;\n \n \tcase MA_ALLOW_PROMISOR:\n@@ -152,7 +199,7 @@ static void show_commit(struct commit *commit, void *data)\n \n \tif (revs->do_not_die_on_missing_objects &&\n \t    oidset_contains(&revs->missing_commits, &commit->object.oid)) {\n-\t\tfinish_object__ma(&commit->object);\n+\t\tfinish_object__ma(&commit->object, NULL);\n \t\treturn;\n \t}\n \n@@ -268,12 +315,11 @@ static void show_commit(struct commit *commit, void *data)\n \tfinish_commit(commit);\n }\n \n-static int finish_object(struct object *obj, const char *name UNUSED,\n-\t\t\t void *cb_data)\n+static int finish_object(struct object *obj, const char *name, void *cb_data)\n {\n \tstruct rev_list_info *info = cb_data;\n \tif (oid_object_info_extended(the_repository, &obj->oid, NULL, 0) < 0) {\n-\t\tfinish_object__ma(obj);\n+\t\tfinish_object__ma(obj, name);\n \t\treturn 1;\n \t}\n \tif (info->revs->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)\n@@ -414,6 +460,12 @@ static inline int parse_missing_action_value(const char *value)\n \t\treturn 1;\n \t}\n \n+\tif (!strcmp(value, \"print-info\")) {\n+\t\targ_missing_action = MA_PRINT_INFO;\n+\t\tfetch_if_missing = 0;\n+\t\treturn 1;\n+\t}\n+\n \tif (!strcmp(value, \"allow-promisor\")) {\n \t\targ_missing_action = MA_ALLOW_PROMISOR;\n \t\tfetch_if_missing = 0;\n@@ -781,10 +833,18 @@ int cmd_rev_list(int argc,\n \n \tif (arg_print_omitted)\n \t\toidset_init(&omitted_objects, DEFAULT_OIDSET_SIZE);\n-\tif (arg_missing_action == MA_PRINT) {\n-\t\toidset_init(&missing_objects, DEFAULT_OIDSET_SIZE);\n+\tif (arg_missing_action == MA_PRINT ||\n+\t    arg_missing_action == MA_PRINT_INFO) {\n+\t\tstruct oidset_iter iter;\n+\t\tstruct object_id *oid;\n+\n+\t\toidmap_init(&missing_objects, DEFAULT_OIDSET_SIZE);\n+\t\toidset_iter_init(&revs.missing_commits, &iter);\n+\n \t\t/* Add missing tips */\n-\t\toidset_insert_from_set(&missing_objects, &revs.missing_commits);\n+\t\twhile ((oid = oidset_iter_next(&iter)))\n+\t\t\tadd_missing_object_entry(oid, NULL);\n+\n \t\toidset_clear(&revs.missing_commits);\n \t}\n \n@@ -800,13 +860,20 @@ int cmd_rev_list(int argc,\n \t\t\tprintf(\"~%s\\n\", oid_to_hex(oid));\n \t\toidset_clear(&omitted_objects);\n \t}\n-\tif (arg_missing_action == MA_PRINT) {\n-\t\tstruct oidset_iter iter;\n-\t\tstruct object_id *oid;\n-\t\toidset_iter_init(&missing_objects, &iter);\n-\t\twhile ((oid = oidset_iter_next(&iter)))\n-\t\t\tprintf(\"?%s\\n\", oid_to_hex(oid));\n-\t\toidset_clear(&missing_objects);\n+\tif (arg_missing_action == MA_PRINT ||\n+\t    arg_missing_action == MA_PRINT_INFO) {\n+\t\tstruct missing_objects_map_entry *entry;\n+\t\tstruct oidmap_iter iter;\n+\n+\t\toidmap_iter_init(&missing_objects, &iter);\n+\n+\t\twhile ((entry = oidmap_iter_next(&iter))) {\n+\t\t\tprint_missing_object(entry, arg_missing_action ==\n+\t\t\t\t\t\t\t    MA_PRINT_INFO);\n+\t\t\tfree((void *)entry->path);\n+\t\t}\n+\n+\t\toidmap_free(&missing_objects, true);\n \t}\n \n \tstop_progress(&progress);\ndiff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh\nindex 7553a9cca2..38afca6f09 100755\n--- a/t/t6022-rev-list-missing.sh\n+++ b/t/t6022-rev-list-missing.sh\n@@ -145,4 +145,56 @@ do\n \tdone\n done\n \n+for obj in \"HEAD~1\" \"HEAD^{tree}\" \"HEAD:foo\" \"HEAD:foo/bar\" \"HEAD:baz baz\"\n+do\n+\ttest_expect_success \"--missing=print-info with missing '$obj'\" '\n+\t\ttest_when_finished rm -rf missing-info &&\n+\n+\t\tgit init missing-info &&\n+\t\t(\n+\t\t\tcd missing-info &&\n+\t\t\tgit commit --allow-empty -m first &&\n+\n+\t\t\tmkdir foo &&\n+\t\t\techo bar >foo/bar &&\n+\t\t\techo baz >\"baz baz\" &&\n+\t\t\techo bat >bat\\\" &&\n+\t\t\tgit add -A &&\n+\t\t\tgit commit -m second &&\n+\n+\t\t\toid=\"$(git rev-parse \"$obj\")\" &&\n+\t\t\tpath=\".git/objects/$(test_oid_to_path $oid)\" &&\n+\n+\t\t\tcase $obj in\n+\t\t\tHEAD:foo)\n+\t\t\t\tpath_info=\" path=foo\"\n+\t\t\t\t;;\n+\t\t\tHEAD:foo/bar)\n+\t\t\t\tpath_info=\" path=foo/bar\"\n+\t\t\t\t;;\n+\t\t\t\"HEAD:baz baz\")\n+\t\t\t\tpath_info=\" path=\\\"baz baz\\\"\"\n+\t\t\t\t;;\n+\t\t\t\"HEAD:bat\\\"\")\n+\t\t\t\tpath_info=\" path=\\\"bat\\\\\\\"\\\"\"\n+\t\t\t\t;;\n+\t\t\tesac &&\n+\n+\t\t\t# Before the object is made missing, we use rev-list to\n+\t\t\t# get the expected oids.\n+\t\t\tgit rev-list --objects --no-object-names \\\n+\t\t\t\tHEAD ^\"$obj\" >expect.raw &&\n+\t\t\techo \"?$oid$path_info\" >>expect.raw &&\n+\n+\t\t\tmv \"$path\" \"$path.hidden\" &&\n+\t\t\tgit rev-list --objects --no-object-names \\\n+\t\t\t\t--missing=print-info HEAD >actual.raw &&\n+\n+\t\t\tsort actual.raw >actual &&\n+\t\t\tsort expect.raw >expect &&\n+\t\t\ttest_cmp expect actual\n+\t\t)\n+\t'\n+done\n+\n test_done\n-- \n2.48.1.157.g3b0d05c4a7\n\n"},{"id":"511841","messageId":"20250205004147.887106-3-jltobler@gmail.com","threadId":"62767","inReplyTo":"20250205004147.887106-1-jltobler@gmail.com","subject":"[PATCH v4 2/2] rev-list: extend print-info to print missing object type","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-02-05T00:41:47Z","receivedAt":"2025-02-05T00:45:24Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"Additional information about missing objects found in git-rev-list(1)\ncan be printed by specifying the `print-info` missing action for the\n`--missing` option. Extend this action to also print missing object type\ninformation inferred from its containing object. This token follows the\nform `type=<type>` and specifies the expected object type of the missing\nobject.\n\nSigned-off-by: Justin Tobler <jltobler@gmail.com>\n---\n Documentation/rev-list-options.txt |  3 +++\n builtin/rev-list.c                 | 11 ++++++++---\n t/t6022-rev-list-missing.sh        |  3 ++-\n 3 files changed, 13 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 0bea9d4ad3..f10f78c600 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -1038,6 +1038,9 @@ one of the following:\n * The `path=<path>` shows the path of the missing object inferred from a\n   containing object. A path containing SP or special characters is enclosed in\n   double-quotes in the C style as needed.\n++\n+* The `type=<type>` shows the type of the missing object inferred from a\n+  containing object.\n --\n +\n If some tips passed to the traversal are missing, they will be\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 00f157eb68..ead43a34ef 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -79,6 +79,7 @@ static int arg_print_omitted; /* print objects omitted by filter */\n struct missing_objects_map_entry {\n \tstruct oidmap_entry entry;\n \tconst char *path;\n+\tunsigned type;\n };\n static struct oidmap missing_objects;\n enum missing_action {\n@@ -109,7 +110,8 @@ static off_t get_object_disk_usage(struct object *obj)\n \treturn size;\n }\n \n-static void add_missing_object_entry(struct object_id *oid, const char *path)\n+static void add_missing_object_entry(struct object_id *oid, const char *path,\n+\t\t\t\t     unsigned type)\n {\n \tstruct missing_objects_map_entry *entry;\n \n@@ -118,6 +120,7 @@ static void add_missing_object_entry(struct object_id *oid, const char *path)\n \n \tCALLOC_ARRAY(entry, 1);\n \tentry->entry.oid = *oid;\n+\tentry->type = type;\n \tif (path)\n \t\tentry->path = xstrdup(path);\n \toidmap_put(&missing_objects, entry);\n@@ -142,6 +145,8 @@ static void print_missing_object(struct missing_objects_map_entry *entry,\n \n \t\tstrbuf_release(&path);\n \t}\n+\tif (entry->type)\n+\t\tstrbuf_addf(&sb, \" type=%s\", type_name(entry->type));\n \n \tprintf(\"?%s%s\\n\", oid_to_hex(&entry->entry.oid), sb.buf);\n \tstrbuf_release(&sb);\n@@ -166,7 +171,7 @@ static inline void finish_object__ma(struct object *obj, const char *name)\n \n \tcase MA_PRINT:\n \tcase MA_PRINT_INFO:\n-\t\tadd_missing_object_entry(&obj->oid, name);\n+\t\tadd_missing_object_entry(&obj->oid, name, obj->type);\n \t\treturn;\n \n \tcase MA_ALLOW_PROMISOR:\n@@ -843,7 +848,7 @@ int cmd_rev_list(int argc,\n \n \t\t/* Add missing tips */\n \t\twhile ((oid = oidset_iter_next(&iter)))\n-\t\t\tadd_missing_object_entry(oid, NULL);\n+\t\t\tadd_missing_object_entry(oid, NULL, 0);\n \n \t\toidset_clear(&revs.missing_commits);\n \t}\ndiff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh\nindex 38afca6f09..3e2790d4c8 100755\n--- a/t/t6022-rev-list-missing.sh\n+++ b/t/t6022-rev-list-missing.sh\n@@ -164,6 +164,7 @@ do\n \n \t\t\toid=\"$(git rev-parse \"$obj\")\" &&\n \t\t\tpath=\".git/objects/$(test_oid_to_path $oid)\" &&\n+\t\t\ttype_info=\" type=$(git cat-file -t $oid)\" &&\n \n \t\t\tcase $obj in\n \t\t\tHEAD:foo)\n@@ -184,7 +185,7 @@ do\n \t\t\t# get the expected oids.\n \t\t\tgit rev-list --objects --no-object-names \\\n \t\t\t\tHEAD ^\"$obj\" >expect.raw &&\n-\t\t\techo \"?$oid$path_info\" >>expect.raw &&\n+\t\t\techo \"?$oid$path_info$type_info\" >>expect.raw &&\n \n \t\t\tmv \"$path\" \"$path.hidden\" &&\n \t\t\tgit rev-list --objects --no-object-names \\\n-- \n2.48.1.157.g3b0d05c4a7\n\n"},{"id":"511866","messageId":"CAP8UFD31kbtqXQDp9LyA+x+h+m592=HQHHbskSfar3S2GOfWVg@mail.gmail.com","threadId":"62767","inReplyTo":"20250205004147.887106-1-jltobler@gmail.com","subject":"Re: [PATCH v4 0/2] rev-list: print additional missing object information","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2025-02-05T10:35:56Z","receivedAt":"2025-02-05T10:36:10Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Feb 5, 2025 at 1:45 AM Justin Tobler <jltobler@gmail.com> wrote:\n\n> Changes in V4:\n>\n> - The core.quotePath behavior is no longer force enabled for the missing\n>   info values. Consequently the first two patches from the previous\n>   version are dropped.\n\nThis v4 looks good to me. Ack!\n"},{"id":"511870","messageId":"xmqq5xlor0la.fsf@gitster.g","threadId":"62767","inReplyTo":"20250205004147.887106-1-jltobler@gmail.com","subject":"Re: [PATCH v4 0/2] rev-list: print additional missing object information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-05T13:18:57Z","receivedAt":"2025-02-05T13:19:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Justin Tobler <jltobler@gmail.com> writes:\n\n> One concern I currently have with this quoting approach is that it is a\n> bit more challenging to machine parse compared to something like using a\n> null byte to delimit between missing info. One option is, in a followup\n> series, introduce a git-for-each-ref(1) style format syntax. Maybe\n> something like: `--missing=print-info:%(path)%00%(type)`. I'm curious if\n> anyone may have thoughts around this.\n\nWould it be so bad if we said that in -z mode with --info option,\neach record is terminated with two NUL bytes, and elements on a list\nof var=value pairs have a single NUL in between, or something silly\nlike that?  The point is to get away with just a fixed format,\nwithout any customization.\n\n"},{"id":"511889","messageId":"4peozlz56kmdhziyq6uhsvm4t7atmwtbl7divyuqqb4l64wgom@z3adjdx7v5x2","threadId":"62767","inReplyTo":"xmqq5xlor0la.fsf@gitster.g","subject":"Re: [PATCH v4 0/2] rev-list: print additional missing object information","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-02-05T17:17:32Z","receivedAt":"2025-02-05T17:20:31Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/02/05 05:18AM, Junio C Hamano wrote:\n> Justin Tobler <jltobler@gmail.com> writes:\n> \n> > One concern I currently have with this quoting approach is that it is a\n> > bit more challenging to machine parse compared to something like using a\n> > null byte to delimit between missing info. One option is, in a followup\n> > series, introduce a git-for-each-ref(1) style format syntax. Maybe\n> > something like: `--missing=print-info:%(path)%00%(type)`. I'm curious if\n> > anyone may have thoughts around this.\n> \n> Would it be so bad if we said that in -z mode with --info option,\n> each record is terminated with two NUL bytes, and elements on a list\n> of var=value pairs have a single NUL in between, or something silly\n> like that?  The point is to get away with just a fixed format,\n> without any customization.\n\nI agree that some sort of fixed format would be preferable as it's less\ncomplex while also being simpler to implement. I originally considered\nusing NUL but realized a single NUL byte to delimit between entries\nwouldn't be sufficient to determine where each record would end. Using\ntwo NUL bytes next to each other to mark the end of a record would work\nthough.\n\nSince even a normal rev-list record may have an object name entry in\naddition to its OID when the `--objects` option is set, maybe we could\nintroduce a `-z` option that always terminates a record with two NUL\nbytes?\n\nThe output for `git rev-list -z --objects --missing=print-info` could\nlook something like the following (no LF at EOL):\n\n  6aa71444d3d41315509c3f2cfe2d45d86cea20d7<NUL><NUL>\n  f009994f5d7fc97c1e87b4dc7ad69057a07e85c4<NUL>foo/bar<NUL><NUL>\n  ?f10f78c60046b2be841c9e2403960663439296c3<NUL>path=foo/bar/baz<NUL>type=blob<NUL><NUL>\n  ?ead43a34efd775b58d6b3e86db6bc71bbedd2c1c<NUL>path=foo/bar/baz 2<NUL>type=blob<NUL><NUL>\n\nHaving two NUL bytes to delimit between records might be a bit odd in\nthe common case for git-rev-list(1) without the `--objects` and\n`--missing` options since we would only expect a list of OIDs. Having\nconsistent `-z` option output irrespective of other options might be\npreferable though.\n\nIf this approach seems reasonable, I can do so in a followup series.\n\nThanks\n-Justin\n"},{"id":"511890","messageId":"nvg26kqiyzvzp65aoelxbhl4e7bodo7zsldqq7shgah65dyt7r@ojnp7ggj65ie","threadId":"62767","inReplyTo":"CAP8UFD31kbtqXQDp9LyA+x+h+m592=HQHHbskSfar3S2GOfWVg@mail.gmail.com","subject":"Re: [PATCH v4 0/2] rev-list: print additional missing object information","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-02-05T17:18:28Z","receivedAt":"2025-02-05T17:21:27Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/02/05 11:35AM, Christian Couder wrote:\n> On Wed, Feb 5, 2025 at 1:45 AM Justin Tobler <jltobler@gmail.com> wrote:\n> \n> > Changes in V4:\n> >\n> > - The core.quotePath behavior is no longer force enabled for the missing\n> >   info values. Consequently the first two patches from the previous\n> >   version are dropped.\n> \n> This v4 looks good to me. Ack!\n\nThanks for the review!\n\n-Justin\n"},{"id":"511896","messageId":"xmqqpljwnt26.fsf@gitster.g","threadId":"62767","inReplyTo":"4peozlz56kmdhziyq6uhsvm4t7atmwtbl7divyuqqb4l64wgom@z3adjdx7v5x2","subject":"Re: [PATCH v4 0/2] rev-list: print additional missing object information","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-02-05T18:29:53Z","receivedAt":"2025-02-05T18:29:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Justin Tobler <jltobler@gmail.com> writes:\n\n> wouldn't be sufficient to determine where each record would end. Using\n> two NUL bytes next to each other to mark the end of a record would work\n> though.\n\nI think we already use that convention elsewhere, and that is why I\nbrought it up as a potential approach to take.\n\n"},{"id":"512239","messageId":"1c6e6e0e-5d99-4842-b76d-b57f7c6855a5@gmail.com","threadId":"62767","inReplyTo":"ksta3edbxwkl62u36xif63q4ebux5hdaxkbme6v6li5v3sgndb@nuu5yyp5clbk","subject":"Re: [PATCH v3 2/4] quote: add quote_path() flag to ignore config","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-02-11T16:51:49Z","receivedAt":"2025-02-11T16:51:53Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Justin\n\nOn 04/02/2025 22:39, Justin Tobler wrote:\n> \n> You are correct that it isn't neccesary. From a previous discussion, I\n> initally thought it would preferrable to have consistent output for this\n> plumbing operation, but as you pointed out other plumbing commands\n> respect core.quotePath. I'll drop these first two patches in the next\n> version.\n\nThat's great, I'm afraid I don't know enough about the rev-list \nmachinery to comment on the meat of this series.\n\nBest Wishes\n\nPhillip\n\n"}]}