# Intro A recent patch series, kn/rev-list-missing-fix [1] extended the `--missing` option in git-rev-list(1) to support commit objects. Unfortunately, git-rev-list(1) with `--missing` set to something other than 'error' still fails, usually with a "fatal: bad object <oid>" error message, when a missing object is passed as an argument. This patch series removes this limitation and when using `--missing=print` allows all missing objects to be printed including those that are passed as arguments. [1] https://lore.kernel.org/git/20231026101109.43110-1-karthik.188@xxxxxxxxx/ # Patch overview Patch 1/5 (t9210: do not rely on lazy fetching to fail) is a test fix suggested by Junio, so that a mostly unrelated test will not wrongly fail when this series is merged. Patches 2/5 (revision: clarify a 'return NULL' in get_reference()), 3/5 (oidset: refactor oidset_insert_from_set()) and 4/5 (t6022: fix 'test' style and 'even though' typo) are very small preparatory cleanups. Patch 5/5 (rev-list: allow missing tips with --missing=[print|allow*]) allows git-rev-list(1) with `--missing=<arg>` when <arg> is not 'error' to not fail if some tips it is passed are missing. # Changes since V2 Thanks to Linus Arver, and Junio who commented on V2! The changes since V2 are the following: - Patch 1/5 (t9210: do not rely on lazy fetching to fail) was added to fix a test that wrongly failed when this series was applied, thanks to Junio who authored it. - In patch 5/5 (rev-list: allow missing tips with --missing=[print|allow*]), some grammos and typos were fixed in the commit message, thanks to Junio and Linus. - In patch 5/5 (rev-list: allow missing tips with --missing=[print|allow*]), the NEEDSWORK comment was improved, thanks to Junio. In particular, it doesn't use "ugly" and "dumb" anymore and gives an example of what's broken. - In patch 5/5 (rev-list: allow missing tips with --missing=[print|allow*]), a code comment as been clarified by removing "already" from it. # Range-diff since V2 -: ---------- > 1: 6e6f2cc26b t9210: do not rely on lazy fetching to fail 1: 5233a83181 = 2: 733c78144e revision: clarify a 'return NULL' in get_reference() 2: cfa72c8cf1 = 3: 4c9e032456 oidset: refactor oidset_insert_from_set() 3: 5668340516 = 4: 35ca6e7c3d t6022: fix 'test' style and 'even though' typo 4: 55792110ca ! 5: da36843b44 rev-list: allow missing tips with --missing=[print|allow*] @@ Commit message We could introduce a new option to make it work like this, but most users are likely to prefer the command to have this behavior as the default one. Introducing a new option would require another dumb loop - to look for that options early, which isn't nice. + to look for that option early, which isn't nice. Also we made `git rev-list` work with missing commits very recently and the command is most often passed commits as arguments. So let's - consider this as a bug fix related to these previous change. + consider this as a bug fix related to these recent changes. While at it let's add a NEEDSWORK comment to say that we should get rid of the existing ugly dumb loops that parse the @@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *pr * Let "--missing" to conditionally set fetch_if_missing. */ + /* -+ * NEEDSWORK: These dump loops to look for some options early -+ * are ugly. We really need setup_revisions() to have a -+ * mechanism to allow and disallow some sets of options for -+ * different commands (like rev-list, replay, etc). Such -+ * mechanism should do an early parsing of option and be able -+ * to manage the `--exclude-promisor-objects` and `--missing=...` -+ * options below. ++ * NEEDSWORK: These loops that attempt to find presence of ++ * options without understanding that the options they are ++ * skipping are broken (e.g., it would not know "--grep ++ * --exclude-promisor-objects" is not triggering ++ * "--exclude-promisor-objects" option). We really need ++ * setup_revisions() to have a mechanism to allow and disallow ++ * some sets of options for different commands (like rev-list, ++ * replay, etc). Such a mechanism should do an early parsing ++ * of options and be able to manage the `--missing=...` and ++ * `--exclude-promisor-objects` options below. + */ for (i = 1; i < argc; i++) { const char *arg = argv[i]; @@ builtin/rev-list.c: int cmd_rev_list(int argc, const char **argv, const char *pr - if (arg_missing_action == MA_PRINT) + if (arg_missing_action == MA_PRINT) { oidset_init(&missing_objects, DEFAULT_OIDSET_SIZE); -+ /* Already add missing tips */ ++ /* Add missing tips */ + oidset_insert_from_set(&missing_objects, &revs.missing_commits); + oidset_clear(&revs.missing_commits); + } Christian Couder (4): revision: clarify a 'return NULL' in get_reference() oidset: refactor oidset_insert_from_set() t6022: fix 'test' style and 'even though' typo rev-list: allow missing tips with --missing=[print|allow*] Junio C Hamano (1): t9210: do not rely on lazy fetching to fail Documentation/rev-list-options.txt | 4 ++ builtin/rev-list.c | 18 ++++++++- list-objects-filter.c | 11 +----- oidset.c | 10 +++++ oidset.h | 6 +++ revision.c | 16 ++++++-- t/t6022-rev-list-missing.sh | 61 +++++++++++++++++++++++++++++- t/t9210-scalar.sh | 9 ++++- 8 files changed, 117 insertions(+), 18 deletions(-) -- 2.44.0.rc0.51.gda36843b44
