pack-bitmap: allow aborting iteration of bitmapped objects

In a subsequent commit we'll lift iteration of bitmapped objects into
the "packed" backend and make it accessible via `odb_for_each_object()`.
The calling convention for that function is that the callback may return
a non-zero exit code, and if so we'll abort iteration. This is currently
impossible to realize though, as `for_each_bitmapped_object()` will
ignore any return value and just churn through all objects completely.

This doesn't matter to the callers of `for_each_bitmapped_object()`, as
there's only one of them in git-cat-file(1), and the callbacks we pass
always return zero. But once we move the logic into the generic
infrastructure it becomes a latent bug waiting to happen.

Refactor the code so that the return value of the `show_reach` callback
is not ignored anymore. Instead, returning a non-zero value will cause
us to abort iteration in both `show_objects_for_type()` and in
`for_each_bitmapped_object()`.

Note though that there's a second user of `show_objects_for_type()` with
`traverse_bitmap_commit_list()`, and that function does indeed invoke
callbacks that may return non-zero. This non-zero return value never had
any effect at all though, and the callbacks that return non-zero values
are only ever invoked via `traverse_bitmap_commit_list()`. Consequently,
we adapt them to always return 0.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 188c4f6d4bd7568590f58c2b3abb31897da1c720..3673b14b89b275e8292f01826e72c2d4ce383fe3 100644 (file)
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1908,7 +1908,7 @@ static int add_object_entry_from_bitmap(const struct object_id *oid,
                return 0;
 
        create_object_entry(oid, type, name_hash, 0, 0, pack, offset);
-       return 1;
+       return 0;
 }
 
 struct pbase_tree_cache {

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 8f63003709242efbe6ece1ec5d2a2fe2c585e371..02818b81c63fd745f88c1287654f3af1d2cd0668 100644 (file)
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -486,7 +486,7 @@ static int show_object_fast(
        void *payload UNUSED)
 {
        fprintf(stdout, "%s\n", oid_to_hex(oid));
-       return 1;
+       return 0;
 }
 
 static void print_disk_usage(off_t size)

diff --git a/pack-bitmap.c b/pack-bitmap.c
index a47c231632405494465468c3fb908052b5768f10..eda38a54337395e392df34c7df5ab951b76c3deb 100644 (file)
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1695,7 +1695,7 @@ static void init_type_iterator(struct ewah_or_iterator *it,
        }
 }
 
-static void show_objects_for_type(
+static int show_objects_for_type(
        struct bitmap_index *bitmap_git,
        struct bitmap *objects,
        enum object_type object_type,
@@ -1704,6 +1704,7 @@ static void show_objects_for_type(
 {
        size_t i = 0;
        uint32_t offset;
+       int ret;
 
        struct ewah_or_iterator it;
        eword_t filter;
@@ -1749,11 +1750,17 @@ static void show_objects_for_type(
 
                        hash = bitmap_name_hash(bitmap_git, index_pos);
 
-                       show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
+                       ret = show_reach(&oid, object_type, 0, hash, pack, ofs, payload);
+                       if (ret)
+                               goto out;
                }
        }
 
+       ret = 0;
+
+out:
        ewah_or_iterator_release(&it);
+       return ret;
 }
 
 static int in_bitmapped_pack(struct bitmap_index *bitmap_git,
@@ -2062,6 +2069,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
                              show_reachable_fn show_reach,
                              void *payload)
 {
+       const enum object_type types[] = {
+               OBJ_COMMIT,
+               OBJ_TREE,
+               OBJ_BLOB,
+               OBJ_TAG,
+       };
        struct bitmap *filtered_bitmap = NULL;
        uint32_t objects_nr;
        size_t full_word_count;
@@ -2086,14 +2099,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
                goto out;
        }
 
-       show_objects_for_type(bitmap_git, filtered_bitmap,
-                             OBJ_COMMIT, show_reach, payload);
-       show_objects_for_type(bitmap_git, filtered_bitmap,
-                             OBJ_TREE, show_reach, payload);
-       show_objects_for_type(bitmap_git, filtered_bitmap,
-                             OBJ_BLOB, show_reach, payload);
-       show_objects_for_type(bitmap_git, filtered_bitmap,
-                             OBJ_TAG, show_reach, payload);
+       for (size_t i = 0; i < ARRAY_SIZE(types); i++) {
+               ret = show_objects_for_type(bitmap_git, filtered_bitmap,
+                                           types[i], show_reach, payload);
+               if (ret)
+                       goto out;
+       }
 
        ret = 0;
 out:

diff --git a/pack-bitmap.h b/pack-bitmap.h
index 47935eb24e29ddd145fad36307f8c960d967fe54..ae8dc491acbbbfd7dc0706bbf5fb0bc13aa5aa41 100644 (file)
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -93,7 +93,8 @@ struct list_objects_filter_options;
 /*
  * Filter bitmapped objects and iterate through all resulting objects,
  * executing `show_reach` for each of them. Returns `-1` in case the filter is
- * not supported, `0` otherwise.
+ * not supported, `0` otherwise. Aborts iteration and bubbles up the return
+ * value in case `show_reach()` returns non-zero.
  */
 int for_each_bitmapped_object(struct bitmap_index *bitmap_git,
                              const struct list_objects_filter_options *filter,