{"thread":{"id":"61766","subject":"rev-list does not output commit metadata (nor honor --format) when --use-bitmap-index is on","startedAt":"2024-07-11T18:12:31Z","lastAt":"2024-07-17T20:45:40Z","messageCount":4,"participants":["Jullyana Ramos","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"498525","messageId":"DM4PR21MB3537F74639094ECFA909880C85A52@DM4PR21MB3537.namprd21.prod.outlook.com","threadId":"61766","inReplyTo":null,"subject":"rev-list does not output commit metadata (nor honor --format) when --use-bitmap-index is on","fromName":"Jullyana Ramos","fromEmail":"jullyana.ramos@microsoft.com","sentAt":"2024-07-11T18:12:16Z","receivedAt":"2024-07-11T18:12:31Z","isPatch":false,"sender":{"key":"jullyana.ramos@microsoft.com","avatar":null},"body":"What did you do before the bug happened? (Steps to reproduce your issue)\nI ran \"git rev-list --use-bitmap-index HEAD\" to see history while taking advantage of bitmap index.\n\nWhat did you expect to happen? (Expected behavior)\nA list of commits to be output, including their metadata (message, author, etc.)\n\nWhat happened instead? (Actual behavior)\nOnly the commit SHAs are being output, no matter what I pass to --format.\nNote that this only reproduces if bitmap index has been created for the repository.\n\nWhat's different between what you expected and what actually happened?\nThe commit metadata is missing.\n\nAnything else you want to add:\nI work for Microsoft at Visual Studio Git integration and I rely on the output of rev-list to populate commit data into the IDE.\nI do not know if this change was intentional, but I can reproduce the issue with version 2.34.1 (Ubuntu) and 2.45.2 (Windows) and I could not reproduce it with version 2.3.5 (Windows). (I know, huge gap, my apologies) I did make sure this was a Git issue, not a Git for Windows issue.\nI can reproduce it with a brand new repository. Create a commit, run rev-list, commit metadata is there. Generate bitmap index and metadata goes missing.\n\n\n[System Info]\ngit version:\ngit version 2.45.2.windows.1\ncpu: x86_64\nbuilt from commit: 91d03cb2e4fbf6ad961ace739b8a646868cb154d\nsizeof-long: 4\nsizeof-size_t: 8\nshell-path: /bin/sh\nfeature: fsmonitor--daemon\nuname: Windows 10.0 22631 \ncompiler info: gnuc: 14.1\nlibc info: no libc information available\n$SHELL (typically, interactive shell): <unset>\n\n\n[Enabled Hooks]\nnot run from a git repository - no hooks to show"},{"id":"498819","messageId":"20240717074418.GI547635@coredump.intra.peff.net","threadId":"61766","inReplyTo":"DM4PR21MB3537F74639094ECFA909880C85A52@DM4PR21MB3537.namprd21.prod.outlook.com","subject":"Re: rev-list does not output commit metadata (nor honor --format) when --use-bitmap-index is on","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-07-17T07:44:18Z","receivedAt":"2024-07-17T07:44:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 11, 2024 at 06:12:16PM +0000, Jullyana Ramos wrote:\n\n> What did you do before the bug happened? (Steps to reproduce your issue)\n> I ran \"git rev-list --use-bitmap-index HEAD\" to see history while taking advantage of bitmap index.\n> \n> What did you expect to happen? (Expected behavior)\n> A list of commits to be output, including their metadata (message, author, etc.)\n> \n> What happened instead? (Actual behavior)\n> Only the commit SHAs are being output, no matter what I pass to --format.\n> Note that this only reproduces if bitmap index has been created for the repository.\n> \n> What's different between what you expected and what actually happened?\n> The commit metadata is missing.\n\nThis is the expected output, but perhaps not by anybody who isn't\nfamiliar with the rev-list and bitmap internals. ;) The --use-bitmap-index\noption was introduced to produce answers as quickly as possible, and\ntypically for \"--objects\" (where the graph traversal is very expensive).\nSo it uses its own code paths for printing out the bitmap result. And\nnotably it will not show the results in any defined order (though I\nthink in practice it will often follow the order of objects in the pack,\nwhich roughly corresponds with traversal order).\n\nSo I don't think anybody gave much thought to what it would do with\n--format, or many other options. And then to make things doubly\nconfusing, --use-bitmap-index is best-effort: if there are no bitmaps,\nor we can't load them for some reason, we'll fall back to the existing\ncode paths. So you would get the formatted output in that case!\n\nSo I'd mostly consider this to be a documentation bug (which does\nmention one of the caveats, but definitely not the ordering or format\nissues). That could certainly be improved.\n\nAlternatively, or in addition, we could perhaps be more careful about\nrejecting certain options used with --use-bitmap-index (but enumerating\nthe complete set feels like a maintenance nightmare).\n\nAnd finally, we might consider giving more guidance in the documentation\non when to use --use-bitmap-index. If you're going to use --format\nanyway, then bitmaps are not buying you much in the first place; you\nstill have to open every commit object to show it! They might help if\nyour query requires a lot of traversing but only outputs a few commits\n(so \"HEAD~3..HEAD\" or similar). But I'd actually expect commit graphs to\ndo just as good a job (or better) these days.\n\nAll that said, I think you _could_ make it work as you expect with\nsomething like the patch below. I'm hesitant to pursue that, though, as\nit is probably just opening up even more opportunities for confusion\n(e.g., --graph will be nonsense in this case due to the ordering issue).\n\n> I do not know if this change was intentional, but I can reproduce the\n> issue with version 2.34.1 (Ubuntu) and 2.45.2 (Windows) and I could\n> not reproduce it with version 2.3.5 (Windows). (I know, huge gap, my\n> apologies) I did make sure this was a Git issue, not a Git for Windows\n> issue.  I can reproduce it with a brand new repository. Create a\n> commit, run rev-list, commit metadata is there. Generate bitmap index\n> and metadata goes missing.\n\nI was surprised that it would have ever worked. But it is just that\n2.3.5 predates 4eb707ebd6 (rev-list: allow commit-only bitmap\ntraversals, 2020-02-14). Before then we skipped bitmap traversal unless\n--objects was specified.\n\nBut even back then, running \"rev-list --use-bitmap-index --objects\n--format=medium\" would ignore \"--format\".\n\n-Peff\n\n---\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 97d077a994..ec1dfd8722 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -369,15 +369,32 @@ static int show_bisect_vars(struct rev_list_info *info, int reaches, int all)\n \treturn 0;\n }\n \n+/*\n+ * Gross hack. The show_reachable_fn type should really take a\n+ * void pointer through which we'd pass this.\n+ */\n+static struct rev_list_info *fast_info;\n+\n static int show_object_fast(\n \tconst struct object_id *oid,\n-\tenum object_type type UNUSED,\n+\tenum object_type type,\n \tint exclude UNUSED,\n \tuint32_t name_hash UNUSED,\n \tstruct packed_git *found_pack UNUSED,\n \toff_t found_offset UNUSED)\n {\n-\tfprintf(stdout, \"%s\\n\", oid_to_hex(oid));\n+\tif (fast_info->revs->commit_format == CMIT_FMT_UNSPECIFIED) {\n+\t\tfprintf(stdout, \"%s\\n\", oid_to_hex(oid));\n+\t} else if (type == OBJ_COMMIT) {\n+\t\tstruct commit *c = lookup_commit(the_repository, oid);\n+\t\tparse_commit_or_die(c);\n+\t\tshow_commit(c, fast_info);\n+\t\treturn 1;\n+\t} else {\n+\t\tstruct object *obj =\n+\t\t\tlookup_object_by_type(the_repository, oid, type);\n+\t\tshow_object(obj, \"\", fast_info);\n+\t}\n \treturn 1;\n }\n \n@@ -472,9 +489,10 @@ static int try_bitmap_count(struct rev_info *revs,\n \treturn 0;\n }\n \n-static int try_bitmap_traversal(struct rev_info *revs,\n+static int try_bitmap_traversal(struct rev_list_info *info,\n \t\t\t\tint filter_provided_objects)\n {\n+\tstruct rev_info *revs = info->revs;\n \tstruct bitmap_index *bitmap_git;\n \n \t/*\n@@ -488,6 +506,7 @@ static int try_bitmap_traversal(struct rev_info *revs,\n \tif (!bitmap_git)\n \t\treturn -1;\n \n+\tfast_info = info;\n \ttraverse_bitmap_commit_list(bitmap_git, revs, &show_object_fast);\n \tfree_bitmap_index(bitmap_git);\n \treturn 0;\n@@ -729,7 +748,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\t\tgoto cleanup;\n \t\tif (!try_bitmap_disk_usage(&revs, filter_provided_objects))\n \t\t\tgoto cleanup;\n-\t\tif (!try_bitmap_traversal(&revs, filter_provided_objects))\n+\t\tif (!try_bitmap_traversal(&info, filter_provided_objects))\n \t\t\tgoto cleanup;\n \t}\n \n"},{"id":"498835","messageId":"xmqq8qy058dd.fsf@gitster.g","threadId":"61766","inReplyTo":"20240717074418.GI547635@coredump.intra.peff.net","subject":"Re: rev-list does not output commit metadata (nor honor --format) when --use-bitmap-index is on","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-17T15:46:38Z","receivedAt":"2024-07-17T15:46:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> This is the expected output, but perhaps not by anybody who isn't\n> familiar with the rev-list and bitmap internals.\n\nCertainly not by me ;-).\n\nIf this were an end-user facing Porcelain, I would actually say we\nshould warn and disable \"--use-bitmap-index\" when any of these\nincompatible options are given (i.e., request for features override\nrequest for performance tweaks), but at least we need to document\nthis better, both in documentation and probably in command line\nparser with warning().\n"},{"id":"498860","messageId":"DM4PR21MB353707D6B84334AADA816A3B85A32@DM4PR21MB3537.namprd21.prod.outlook.com","threadId":"61766","inReplyTo":"xmqq8qy058dd.fsf@gitster.g","subject":"Re: [EXTERNAL] Re: rev-list does not output commit metadata (nor honor --format) when --use-bitmap-index is on","fromName":"Jullyana Ramos","fromEmail":"jullyana.ramos@microsoft.com","sentAt":"2024-07-17T20:45:26Z","receivedAt":"2024-07-17T20:45:40Z","isPatch":false,"sender":{"key":"jullyana.ramos@microsoft.com","avatar":null},"body":"I appreciate the context provided by Peff and understand now that bitmap index is not compatible with my goal.\n\nExperimenting again today, I might have set up the test repository wrong the first time I tried with 2.3.5. Formatting likely never worked with --use-bitmap-index. Therefore, I can simply remove its usage from the parser I have, as it has been broken from the beginning.\n\nI also agree this is a documentation issue. If rev-list docs mentioned it, I wouldn't have filed it as a bug. It would be extra nice if incompatible options were better handled as Junio says.\n\nThank you both for your response."}]}