Merge branch 'ks/pack-objects-bitmap'
[git/git.git] / builtin / pack-objects.c
index 0954375..166e52c 100644 (file)
@@ -67,7 +67,8 @@ static struct packed_git *reuse_packfile;
 static uint32_t reuse_packfile_objects;
 static off_t reuse_packfile_offset;
 
 static uint32_t reuse_packfile_objects;
 static off_t reuse_packfile_offset;
 
-static int use_bitmap_index = 1;
+static int use_bitmap_index_default = 1;
+static int use_bitmap_index = -1;
 static int write_bitmap_index;
 static uint16_t write_bitmap_options;
 
 static int write_bitmap_index;
 static uint16_t write_bitmap_options;
 
@@ -945,13 +946,48 @@ static int have_duplicate_entry(const unsigned char *sha1,
        return 1;
 }
 
        return 1;
 }
 
+static int want_found_object(int exclude, struct packed_git *p)
+{
+       if (exclude)
+               return 1;
+       if (incremental)
+               return 0;
+
+       /*
+        * When asked to do --local (do not include an object that appears in a
+        * pack we borrow from elsewhere) or --honor-pack-keep (do not include
+        * an object that appears in a pack marked with .keep), finding a pack
+        * that matches the criteria is sufficient for us to decide to omit it.
+        * However, even if this pack does not satisfy the criteria, we need to
+        * make sure no copy of this object appears in _any_ pack that makes us
+        * to omit the object, so we need to check all the packs.
+        *
+        * We can however first check whether these options can possible matter;
+        * if they do not matter we know we want the object in generated pack.
+        * Otherwise, we signal "-1" at the end to tell the caller that we do
+        * not know either way, and it needs to check more packs.
+        */
+       if (!ignore_packed_keep &&
+           (!local || !have_non_local_packs))
+               return 1;
+
+       if (local && !p->pack_local)
+               return 0;
+       if (ignore_packed_keep && p->pack_local && p->pack_keep)
+               return 0;
+
+       /* we don't know yet; keep looking for more packs */
+       return -1;
+}
+
 /*
  * Check whether we want the object in the pack (e.g., we do not want
  * objects found in non-local stores if the "--local" option was used).
  *
 /*
  * Check whether we want the object in the pack (e.g., we do not want
  * objects found in non-local stores if the "--local" option was used).
  *
- * As a side effect of this check, we will find the packed version of this
- * object, if any. We therefore pass out the pack information to avoid having
- * to look it up again later.
+ * If the caller already knows an existing pack it wants to take the object
+ * from, that is passed in *found_pack and *found_offset; otherwise this
+ * function finds if there is any pack that has the object and returns the pack
+ * and its offset in these variables.
  */
 static int want_object_in_pack(const unsigned char *sha1,
                               int exclude,
  */
 static int want_object_in_pack(const unsigned char *sha1,
                               int exclude,
@@ -959,15 +995,30 @@ static int want_object_in_pack(const unsigned char *sha1,
                               off_t *found_offset)
 {
        struct packed_git *p;
                               off_t *found_offset)
 {
        struct packed_git *p;
+       int want;
 
        if (!exclude && local && has_loose_object_nonlocal(sha1))
                return 0;
 
 
        if (!exclude && local && has_loose_object_nonlocal(sha1))
                return 0;
 
-       *found_pack = NULL;
-       *found_offset = 0;
+       /*
+        * If we already know the pack object lives in, start checks from that
+        * pack - in the usual case when neither --local was given nor .keep files
+        * are present we will determine the answer right now.
+        */
+       if (*found_pack) {
+               want = want_found_object(exclude, *found_pack);
+               if (want != -1)
+                       return want;
+       }
 
        for (p = packed_git; p; p = p->next) {
 
        for (p = packed_git; p; p = p->next) {
-               off_t offset = find_pack_entry_one(sha1, p);
+               off_t offset;
+
+               if (p == *found_pack)
+                       offset = *found_offset;
+               else
+                       offset = find_pack_entry_one(sha1, p);
+
                if (offset) {
                        if (!*found_pack) {
                                if (!is_pack_valid(p))
                if (offset) {
                        if (!*found_pack) {
                                if (!is_pack_valid(p))
@@ -975,31 +1026,9 @@ static int want_object_in_pack(const unsigned char *sha1,
                                *found_offset = offset;
                                *found_pack = p;
                        }
                                *found_offset = offset;
                                *found_pack = p;
                        }
-                       if (exclude)
-                               return 1;
-                       if (incremental)
-                               return 0;
-
-                       /*
-                        * When asked to do --local (do not include an
-                        * object that appears in a pack we borrow
-                        * from elsewhere) or --honor-pack-keep (do not
-                        * include an object that appears in a pack marked
-                        * with .keep), we need to make sure no copy of this
-                        * object come from in _any_ pack that causes us to
-                        * omit it, and need to complete this loop.  When
-                        * neither option is in effect, we know the object
-                        * we just found is going to be packed, so break
-                        * out of the loop to return 1 now.
-                        */
-                       if (!ignore_packed_keep &&
-                           (!local || !have_non_local_packs))
-                               break;
-
-                       if (local && !p->pack_local)
-                               return 0;
-                       if (ignore_packed_keep && p->pack_local && p->pack_keep)
-                               return 0;
+                       want = want_found_object(exclude, p);
+                       if (want != -1)
+                               return want;
                }
        }
 
                }
        }
 
@@ -1040,8 +1069,8 @@ static const char no_closure_warning[] = N_(
 static int add_object_entry(const unsigned char *sha1, enum object_type type,
                            const char *name, int exclude)
 {
 static int add_object_entry(const unsigned char *sha1, enum object_type type,
                            const char *name, int exclude)
 {
-       struct packed_git *found_pack;
-       off_t found_offset;
+       struct packed_git *found_pack = NULL;
+       off_t found_offset = 0;
        uint32_t index_pos;
 
        if (have_duplicate_entry(sha1, exclude, &index_pos))
        uint32_t index_pos;
 
        if (have_duplicate_entry(sha1, exclude, &index_pos))
@@ -1074,6 +1103,9 @@ static int add_object_entry_from_bitmap(const unsigned char *sha1,
        if (have_duplicate_entry(sha1, 0, &index_pos))
                return 0;
 
        if (have_duplicate_entry(sha1, 0, &index_pos))
                return 0;
 
+       if (!want_object_in_pack(sha1, 0, &pack, &offset))
+               return 0;
+
        create_object_entry(sha1, type, name_hash, 0, 0, index_pos, pack, offset);
 
        display_progress(progress_state, nr_result);
        create_object_entry(sha1, type, name_hash, 0, 0, index_pos, pack, offset);
 
        display_progress(progress_state, nr_result);
@@ -2273,7 +2305,7 @@ static int git_pack_config(const char *k, const char *v, void *cb)
                        write_bitmap_options &= ~BITMAP_OPT_HASH_CACHE;
        }
        if (!strcmp(k, "pack.usebitmaps")) {
                        write_bitmap_options &= ~BITMAP_OPT_HASH_CACHE;
        }
        if (!strcmp(k, "pack.usebitmaps")) {
-               use_bitmap_index = git_config_bool(k, v);
+               use_bitmap_index_default = git_config_bool(k, v);
                return 0;
        }
        if (!strcmp(k, "pack.threads")) {
                return 0;
        }
        if (!strcmp(k, "pack.threads")) {
@@ -2522,13 +2554,13 @@ static void loosen_unused_packed_objects(struct rev_info *revs)
 }
 
 /*
 }
 
 /*
- * This tracks any options which a reader of the pack might
- * not understand, and which would therefore prevent blind reuse
- * of what we have on disk.
+ * This tracks any options which pack-reuse code expects to be on, or which a
+ * reader of the pack might not understand, and which would therefore prevent
+ * blind reuse of what we have on disk.
  */
 static int pack_options_allow_reuse(void)
 {
  */
 static int pack_options_allow_reuse(void)
 {
-       return allow_ofs_delta;
+       return pack_to_stdout && allow_ofs_delta;
 }
 
 static int get_object_list_from_bitmap(struct rev_info *revs)
 }
 
 static int get_object_list_from_bitmap(struct rev_info *revs)
@@ -2821,7 +2853,23 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
        if (!rev_list_all || !rev_list_reflog || !rev_list_index)
                unpack_unreachable_expiration = 0;
 
        if (!rev_list_all || !rev_list_reflog || !rev_list_index)
                unpack_unreachable_expiration = 0;
 
-       if (!use_internal_rev_list || !pack_to_stdout || is_repository_shallow())
+       /*
+        * "soft" reasons not to use bitmaps - for on-disk repack by default we want
+        *
+        * - to produce good pack (with bitmap index not-yet-packed objects are
+        *   packed in suboptimal order).
+        *
+        * - to use more robust pack-generation codepath (avoiding possible
+        *   bugs in bitmap code and possible bitmap index corruption).
+        */
+       if (!pack_to_stdout)
+               use_bitmap_index_default = 0;
+
+       if (use_bitmap_index < 0)
+               use_bitmap_index = use_bitmap_index_default;
+
+       /* "hard" reasons not to use bitmaps; these just won't work at all */
+       if (!use_internal_rev_list || (!pack_to_stdout && write_bitmap_index) || is_repository_shallow())
                use_bitmap_index = 0;
 
        if (pack_to_stdout || !rev_list_all)
                use_bitmap_index = 0;
 
        if (pack_to_stdout || !rev_list_all)