for_each_abbrev: drop duplicate objects

If an object appears multiple times in the object database
(e.g., in both loose and packed form, or in two separate
packs), the disambiguation machinery may see it more than
once. The get_short_sha1() function handles this already,
but for_each_abbrev() blindly fires the callback for each
instance it finds.

We can fix this by collecting the output in a sha1 array and
de-duplicating it.  As a bonus, the sort done for the
de-duplication means that our output will be stable,
regardless of the order in which the objects are found.

Note that the old code normalized the callback's output to
0/1 to store in the 1-bit ds->ambiguous flag (which both
halted the iteration and was returned from the
for_each_abbrev function). Now that we are using sha1_array,
we can return the real value. In practice, it doesn't matter
as the sole caller only ever returns 0.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sha1_name.c b/sha1_name.c
index acea241139d29f75bd2494e7bff3c2e6fb3976a2..f7e388490a3647172ddb8f1bb21db56d67c61e66 100644 (file)
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -7,6 +7,7 @@
 #include "refs.h"
 #include "remote.h"
 #include "dir.h"
 #include "refs.h"
 #include "remote.h"
 #include "dir.h"

+#include "sha1-array.h"

 
 static int get_sha1_oneline(const char *, unsigned char *, struct commit_list *);
 
 
 static int get_sha1_oneline(const char *, unsigned char *, struct commit_list *);
 


@@ -350,20 +351,30 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,
        return status;
 }
 
        return status;
 }
 

+static int collect_ambiguous(const unsigned char *sha1, void *data)
+{
+       sha1_array_append(data, sha1);
+       return 0;
+}
+

 int for_each_abbrev(const char *prefix, each_abbrev_fn fn, void *cb_data)
 {
 int for_each_abbrev(const char *prefix, each_abbrev_fn fn, void *cb_data)
 {

+       struct sha1_array collect = SHA1_ARRAY_INIT;

        struct disambiguate_state ds;
        struct disambiguate_state ds;

+       int ret;

 
        if (init_object_disambiguation(prefix, strlen(prefix), &ds) < 0)
                return -1;
 
        ds.always_call_fn = 1;
 
        if (init_object_disambiguation(prefix, strlen(prefix), &ds) < 0)
                return -1;
 
        ds.always_call_fn = 1;

-       ds.cb_data = cb_data;
-       ds.fn = fn;
-
+       ds.fn = collect_ambiguous;
+       ds.cb_data = &collect;

        find_short_object_filename(&ds);
        find_short_packed_object(&ds);
        find_short_object_filename(&ds);
        find_short_packed_object(&ds);

-       return ds.ambiguous;
+
+       ret = sha1_array_for_each_unique(&collect, fn, cb_data);
+       sha1_array_clear(&collect);
+       return ret;

 }
 
 int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len)
 }
 
 int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len)

diff --git a/t/t1512-rev-parse-disambiguation.sh b/t/t1512-rev-parse-disambiguation.sh
index dfd356721eeb55b13c5c51075b9bd28b8e1997e7..1d8f550996caf0207a25fcf7d0901aa51dc41f0d 100755 (executable)
--- a/t/t1512-rev-parse-disambiguation.sh
+++ b/t/t1512-rev-parse-disambiguation.sh
@@ -280,6 +280,13 @@ test_expect_success 'rev-parse --disambiguate' '
        test "$(sed -e "s/^\(.........\).*/\1/" actual | sort -u)" = 000000000
 '
 
        test "$(sed -e "s/^\(.........\).*/\1/" actual | sort -u)" = 000000000
 '
 

+test_expect_success 'rev-parse --disambiguate drops duplicates' '
+       git rev-parse --disambiguate=000000000 >expect &&
+       git pack-objects .git/objects/pack/pack <expect &&
+       git rev-parse --disambiguate=000000000 >actual &&
+       test_cmp expect actual
+'
+

 test_expect_success 'ambiguous 40-hex ref' '
        TREE=$(git mktree </dev/null) &&
        REF=$(git rev-parse HEAD) &&
 test_expect_success 'ambiguous 40-hex ref' '
        TREE=$(git mktree </dev/null) &&
        REF=$(git rev-parse HEAD) &&