[PATCH v2 0/4] rev-list: allow missing tips with --missing

[Christian Couder <christian.couder@gmail.com>]

# 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@gmail.com/

# Patch overview

Patches 1/4 (revision: clarify a 'return NULL' in get_reference()),
2/4 (oidset: refactor oidset_insert_from_set()) and 
3/4 (t6022: fix 'test' style and 'even though' typo) are very small
preparatory cleanups.

Patch 4/4 (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 V1

Thanks to Linus Arver, Eric Sunshine and Junio who commented on V1!
The changes since V1 are the following:

  - In patch 1/4 (revision: clarify a 'return NULL' in
    get_reference()), some 's/explicitely/explicitly/' typos were fixed
    in the commit message. Thanks to a suggestion from Eric. 

  - Patch 2/4 (oidset: refactor oidset_insert_from_set()) is new. It
    refactors some code into "oidset.{c,h}" to avoid duplicating code
    to copy elements from one oidset into another one. Thanks to a
    suggestion from Linus.

  - Patch 3/4 (t6022: fix 'test' style and 'even though' typo) used to
    fix only an 'even though' typo, but while at it I made it use
    `if test ...` instead of `if [ ... ]` too. Thanks to a suggestion
    from Junio.

  - Patch 4/4 (rev-list: allow missing tips with
    --missing=[print|allow*]) was changed so that missing tips are
    always allowed when `--missing=[print|allow*]` is used, as
    suggested by Junio. So:
      - no new `--allow-missing-tips` option is implemented,
      - no ugly early parsing loop is added,
      - no new 'do_not_die_on_missing_tips' flag is added into
        'struct rev_info',
      - the 'do_not_die_on_missing_objects' is used more instead,
      - the commit message as been changed accordingly.

  - In patch 4/4 (rev-list: allow missing tips with
    --missing=[print|allow*]), a code comment has been added before
    `if (get_oid_with_context(...))` in "revision.c::get_reference()"
    as suggested by Linus.

  - In patch 4/4 (rev-list: allow missing tips with
    --missing=[print|allow*]), a big NEEDSWORK comment has been added
    before the ugly early parsing loops for the
    `--exclude-promisor-objects` and `--missing=...` options in
    "builtin/rev-list.c".

  - In patch 4/4 (rev-list: allow missing tips with
    --missing=[print|allow*]), a code comment now uses "missing tips"
    instead of "missing commits" in "builtin/rev-list.c", as
    suggested by Linus.
    
  - In patch 4/4 (rev-list: allow missing tips with
    --missing=[print|allow*]), the added tests in t6022 have the
    following changes:
      - variables 'obj' and 'tip' have been renamed to 'missing_tip'
        and 'existing_tip' respectively as suggested by Linus,
      - a comment explaining how the 'existing_tip' variable is useful
        has been added as suggested by Linus,
      - `if test ...` is used instead of `if [ ... ]`, as suggested by
        Junio.

  - Also in patch 4/4 (rev-list: allow missing tips with
    --missing=[print|allow*]), the documentation of the `--missing=...`
    option has been improved to talk about missing tips.

# Range-diff since V1

Unfortunately there has been many changes in patch 4/4, so the
range-diff shows different patches:

1:  b8abbc1d42 ! 1:  5233a83181 revision: clarify a 'return NULL' in get_reference()
    @@ Commit message
         revision: clarify a 'return NULL' in get_reference()
     
         In general when we know a pointer variable is NULL, it's clearer to
    -    explicitely return NULL than to return that variable.
    +    explicitly return NULL than to return that variable.
     
         In get_reference() when 'object' is NULL, we already return NULL
         when 'revs->exclude_promisor_objects && is_promisor_object(oid)' is
         true, but we return 'object' when 'revs->ignore_missing' is true.
     
    -    Let's make the code clearer and more uniform by also explicitely
    +    Let's make the code clearer and more uniform by also explicitly
         returning NULL when 'revs->ignore_missing' is true.
     
    +    Helped-by: Eric Sunshine <sunshine@sunshineco.com>
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
     
      ## revision.c ##
-:  ---------- > 2:  cfa72c8cf1 oidset: refactor oidset_insert_from_set()
2:  208d43eb81 ! 3:  5668340516 t6022: fix 'even though' typo in comment
    @@ Metadata
     Author: Christian Couder <chriscool@tuxfamily.org>
     
      ## Commit message ##
    -    t6022: fix 'even though' typo in comment
    +    t6022: fix 'test' style and 'even though' typo
     
         Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
     
    @@ t/t6022-rev-list-missing.sh: do
     -                  # Blobs are shared by all commits, so evethough a commit/tree
     +                  # Blobs are shared by all commits, so even though a commit/tree
                        # might be skipped, its blob must be accounted for.
    -                   if [ $obj != "HEAD:1.t" ]; then
    +-                  if [ $obj != "HEAD:1.t" ]; then
    ++                  if test $obj != "HEAD:1.t"
    ++                  then
                                echo $(git rev-parse HEAD:1.t) >>expect.raw &&
    +                           echo $(git rev-parse HEAD:2.t) >>expect.raw
    +                   fi &&
3:  8be34ce359 < -:  ---------- rev-list: add --allow-missing-tips to be used with --missing=...
-:  ---------- > 4:  55792110ca rev-list: allow missing tips with --missing=[print|allow*]


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*]

 Documentation/rev-list-options.txt |  4 ++
 builtin/rev-list.c                 | 15 +++++++-
 list-objects-filter.c              | 11 +-----
 oidset.c                           | 10 +++++
 oidset.h                           |  6 +++
 revision.c                         | 16 ++++++--
 t/t6022-rev-list-missing.sh        | 61 +++++++++++++++++++++++++++++-
 7 files changed, 106 insertions(+), 17 deletions(-)

-- 
2.43.0.565.g97b5fd12a3.dirty