From 9245ddd515d0fb5da52da4fd4dfc71460e98db90 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 12 Nov 2008 11:59:02 -0600 Subject: [PATCH 01/10] t7700: demonstrate mishandling of objects in packs with a .keep file Objects residing in pack files that have an associated .keep file are not supposed to be repacked into new pack files, but they are. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- t/t7700-repack.sh | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100755 t/t7700-repack.sh diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh new file mode 100755 index 0000000000..7aaff0bbe0 --- /dev/null +++ b/t/t7700-repack.sh @@ -0,0 +1,38 @@ +#!/bin/sh + +test_description='git repack works correctly' + +. ./test-lib.sh + +test_expect_failure 'objects in packs marked .keep are not repacked' ' + echo content1 > file1 && + echo content2 > file2 && + git add . && + git commit -m initial_commit && + # Create two packs + # The first pack will contain all of the objects except one + git rev-list --objects --all | grep -v file2 | + git pack-objects pack > /dev/null && + # The second pack will contain the excluded object + packsha1=$(git rev-list --objects --all | grep file2 | + git pack-objects pack) && + touch -r pack-$packsha1.pack pack-$packsha1.keep && + objsha1=$(git verify-pack -v pack-$packsha1.idx | head -n 1 | + sed -e "s/^\([0-9a-f]\{40\}\).*/\1/") && + mv pack-* .git/objects/pack/ && + git repack -A -d -l && + git prune-packed && + for p in .git/objects/pack/*.idx; do + idx=$(basename $p) + test "pack-$packsha1.idx" = "$idx" && continue + if git verify-pack -v $p | egrep "^$objsha1"; then + found_duplicate_object=1 + echo "DUPLICATE OBJECT FOUND" + break + fi + done && + test -z "$found_duplicate_object" +' + +test_done + From 8d25931d6ff47a7fb06512d767d1d416d9bc7733 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 12 Nov 2008 11:59:03 -0600 Subject: [PATCH 02/10] packed_git: convert pack_local flag into a bitfield and add pack_keep pack_keep will be set when a pack file has an associated .keep file. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- cache.h | 3 ++- sha1_file.c | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/cache.h b/cache.h index a1e4982cd4..1a5740f589 100644 --- a/cache.h +++ b/cache.h @@ -671,7 +671,8 @@ extern struct packed_git { int index_version; time_t mtime; int pack_fd; - int pack_local; + unsigned pack_local:1, + pack_keep:1; unsigned char sha1[20]; /* something like ".git/objects/pack/xxxxx.pack" */ char pack_name[FLEX_ARRAY]; /* more */ diff --git a/sha1_file.c b/sha1_file.c index 12fc767ee5..adb116350b 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -828,6 +828,11 @@ struct packed_git *add_packed_git(const char *path, int path_len, int local) return NULL; } memcpy(p->pack_name, path, path_len); + + strcpy(p->pack_name + path_len, ".keep"); + if (!access(p->pack_name, F_OK)) + p->pack_keep = 1; + strcpy(p->pack_name + path_len, ".pack"); if (stat(p->pack_name, &st) || !S_ISREG(st.st_mode)) { free(p); From e96fb9b8f90f907d720ea6b71c92e30c2b071f4a Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 12 Nov 2008 11:59:04 -0600 Subject: [PATCH 03/10] pack-objects: new option --honor-pack-keep This adds a new option to pack-objects which will cause it to ignore an object which appears in a local pack which has a .keep file, even if it was specified for packing. This option will be used by the porcelain repack. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- Documentation/git-pack-objects.txt | 5 +++++ builtin-pack-objects.c | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt index 8c354bd470..f9fac2ccf9 100644 --- a/Documentation/git-pack-objects.txt +++ b/Documentation/git-pack-objects.txt @@ -109,6 +109,11 @@ base-name:: The default is unlimited, unless the config variable `pack.packSizeLimit` is set. +--honor-pack-keep:: + This flag causes an object already in a local pack that + has a .keep file to be ignored, even if it appears in the + standard input. + --incremental:: This flag causes an object already in a pack ignored even if it appears in the standard input. diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index b0dddbee4f..29c00474d6 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -71,6 +71,7 @@ static int reuse_delta = 1, reuse_object = 1; static int keep_unreachable, unpack_unreachable, include_tag; static int local; static int incremental; +static int ignore_packed_keep; static int allow_ofs_delta; static const char *base_name; static int progress = 1; @@ -703,6 +704,8 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type, return 0; if (local && !p->pack_local) return 0; + if (ignore_packed_keep && p->pack_local && p->pack_keep) + return 0; } } @@ -2042,6 +2045,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) incremental = 1; continue; } + if (!strcmp("--honor-pack-keep", arg)) { + ignore_packed_keep = 1; + continue; + } if (!prefixcmp(arg, "--compression=")) { char *end; int level = strtoul(arg+14, &end, 0); From dd718365cccfddd7d5992a40296de50e7cabdfc8 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 12 Nov 2008 11:59:05 -0600 Subject: [PATCH 04/10] repack: don't repack local objects in packs with .keep file If the user created a .keep file for a local pack, then it can be inferred that the user does not want those objects repacked. This fixes the repack bug tested by t7700. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- git-repack.sh | 2 +- t/t7700-repack.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/git-repack.sh b/git-repack.sh index d39eb6cea6..8bb22014b9 100755 --- a/git-repack.sh +++ b/git-repack.sh @@ -83,7 +83,7 @@ case ",$all_into_one," in esac args="$args $local $quiet $no_reuse$extra" -names=$(git pack-objects --non-empty --all --reflog $args file1 && echo content2 > file2 && git add . && From f7991d1ed37502c0e98dfa95866dfc1a8b08d834 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 12 Nov 2008 11:59:06 -0600 Subject: [PATCH 05/10] repack: do not fall back to incremental repacking with [-a|-A] When repack is called with either the -a or -A option, the user has requested to repack all objects including those referenced by the alternates mechanism. Currently, if there are no local packs without .keep files, then repack will call pack-objects with the '--unpacked --incremental' options which causes it to exclude alternate packed objects. So, remove this fallback. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- git-repack.sh | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/git-repack.sh b/git-repack.sh index 8bb22014b9..4d313d136e 100755 --- a/git-repack.sh +++ b/git-repack.sh @@ -71,13 +71,10 @@ case ",$all_into_one," in existing="$existing $e" fi done - fi - if test -z "$args" - then - args='--unpacked --incremental' - elif test -n "$unpack_unreachable" - then - args="$args $unpack_unreachable" + if test -n "$args" -a -n "$unpack_unreachable" + then + args="$args $unpack_unreachable" + fi fi ;; esac From 01af249fa15ce63ea69e89e3520022420ecb409c Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 12 Nov 2008 11:59:07 -0600 Subject: [PATCH 06/10] builtin-gc.c: use new pack_keep bitfield to detect .keep file existence Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- builtin-gc.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/builtin-gc.c b/builtin-gc.c index fac200e0b0..53a0d43b67 100644 --- a/builtin-gc.c +++ b/builtin-gc.c @@ -134,19 +134,9 @@ static int too_many_packs(void) prepare_packed_git(); for (cnt = 0, p = packed_git; p; p = p->next) { - char path[PATH_MAX]; - size_t len; - int keep; - if (!p->pack_local) continue; - len = strlen(p->pack_name); - if (PATH_MAX <= len + 1) - continue; /* oops, give up */ - memcpy(path, p->pack_name, len-5); - memcpy(path + len - 5, ".keep", 6); - keep = access(p->pack_name, F_OK) && (errno == ENOENT); - if (keep) + if (p->pack_keep) continue; /* * Perhaps check the size of the pack and count only From 3c3df429106770966397086b6d2bced452ec7383 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Sun, 9 Nov 2008 23:59:56 -0600 Subject: [PATCH 07/10] t7700: demonstrate mishandling of loose objects in an alternate ODB Loose objects residing in an alternate object database should not be packed when the -l option to repack is used. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- t/t7700-repack.sh | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh index 356afe371b..43c9cf9139 100755 --- a/t/t7700-repack.sh +++ b/t/t7700-repack.sh @@ -34,5 +34,24 @@ test_expect_success 'objects in packs marked .keep are not repacked' ' test -z "$found_duplicate_object" ' +test_expect_failure 'loose objects in alternate ODB are not repacked' ' + mkdir alt_objects && + echo `pwd`/alt_objects > .git/objects/info/alternates && + echo content3 > file3 && + objsha1=$(GIT_OBJECT_DIRECTORY=alt_objects git hash-object -w file3) && + git add file3 && + git commit -m commit_file3 && + git repack -a -d -l && + git prune-packed && + for p in .git/objects/pack/*.idx; do + if git verify-pack -v $p | egrep "^$objsha1"; then + found_duplicate_object=1 + echo "DUPLICATE OBJECT FOUND" + break + fi + done && + test -z "$found_duplicate_object" +' + test_done From 0f4dc14ac4945448f7309964afeb879d20408e07 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Sun, 9 Nov 2008 23:59:57 -0600 Subject: [PATCH 08/10] sha1_file.c: split has_loose_object() into local and non-local counterparts Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- cache.h | 1 + sha1_file.c | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/cache.h b/cache.h index 1a5740f589..7595c149ea 100644 --- a/cache.h +++ b/cache.h @@ -565,6 +565,7 @@ extern int move_temp_to_file(const char *tmpfile, const char *filename); extern int has_sha1_pack(const unsigned char *sha1, const char **ignore); extern int has_sha1_file(const unsigned char *sha1); +extern int has_loose_object_nonlocal(const unsigned char *sha1); extern int has_pack_file(const unsigned char *sha1); extern int has_pack_index(const unsigned char *sha1); diff --git a/sha1_file.c b/sha1_file.c index adb116350b..0203de5855 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -410,23 +410,30 @@ void prepare_alt_odb(void) read_info_alternates(get_object_directory(), 0); } -static int has_loose_object(const unsigned char *sha1) +static int has_loose_object_local(const unsigned char *sha1) { char *name = sha1_file_name(sha1); - struct alternate_object_database *alt; + return !access(name, F_OK); +} - if (!access(name, F_OK)) - return 1; +int has_loose_object_nonlocal(const unsigned char *sha1) +{ + struct alternate_object_database *alt; prepare_alt_odb(); for (alt = alt_odb_list; alt; alt = alt->next) { - name = alt->name; - fill_sha1_path(name, sha1); + fill_sha1_path(alt->name, sha1); if (!access(alt->base, F_OK)) return 1; } return 0; } +static int has_loose_object(const unsigned char *sha1) +{ + return has_loose_object_local(sha1) || + has_loose_object_nonlocal(sha1); +} + static unsigned int pack_used_ctr; static unsigned int pack_mmap_calls; static unsigned int peak_pack_open_windows; From daae06259556246959963947752bde4ee2df7b44 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Sun, 9 Nov 2008 23:59:58 -0600 Subject: [PATCH 09/10] pack-objects: extend --local to mean ignore non-local loose objects too With this patch, --local means pack only local objects that are not already packed. Additionally, this fixes t7700 testing whether loose objects in an alternate object database are repacked. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- Documentation/git-pack-objects.txt | 2 +- builtin-pack-objects.c | 3 +++ t/t7700-repack.sh | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt index f9fac2ccf9..7d4c1a7556 100644 --- a/Documentation/git-pack-objects.txt +++ b/Documentation/git-pack-objects.txt @@ -121,7 +121,7 @@ base-name:: --local:: This flag is similar to `--incremental`; instead of ignoring all packed objects, it only ignores objects - that are packed and not in the local object store + that are packed and/or not in the local object store (i.e. borrowed from an alternate). --non-empty:: diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 29c00474d6..85bd795d3b 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -691,6 +691,9 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type, return 0; } + if (!exclude && local && has_loose_object_nonlocal(sha1)) + return 0; + for (p = packed_git; p; p = p->next) { off_t offset = find_pack_entry_one(sha1, p); if (offset) { diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh index 43c9cf9139..960bff47fa 100755 --- a/t/t7700-repack.sh +++ b/t/t7700-repack.sh @@ -34,7 +34,7 @@ test_expect_success 'objects in packs marked .keep are not repacked' ' test -z "$found_duplicate_object" ' -test_expect_failure 'loose objects in alternate ODB are not repacked' ' +test_expect_success 'loose objects in alternate ODB are not repacked' ' mkdir alt_objects && echo `pwd`/alt_objects > .git/objects/info/alternates && echo content3 > file3 && From 3289b9dec56d34fe05f90c262d11adc0a61e16e7 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 12 Nov 2008 18:50:26 -0600 Subject: [PATCH 10/10] t7700: test that 'repack -a' packs alternate packed objects Previously, when 'repack -a' was called and there were no packs in the local repository without a .keep file, the repack would fall back to calling pack-objects with '--unpacked --incremental'. This resulted in the created pack file, if any, to be missing the packed objects in the alternate object store. Test that this specific case has been fixed. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- t/t7700-repack.sh | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh index 960bff47fa..3f602ea7de 100755 --- a/t/t7700-repack.sh +++ b/t/t7700-repack.sh @@ -53,5 +53,21 @@ test_expect_success 'loose objects in alternate ODB are not repacked' ' test -z "$found_duplicate_object" ' +test_expect_success 'packed obs in alt ODB are repacked even when local repo is packless' ' + mkdir alt_objects/pack + mv .git/objects/pack/* alt_objects/pack && + git repack -a && + myidx=$(ls -1 .git/objects/pack/*.idx) && + test -f "$myidx" && + for p in alt_objects/pack/*.idx; do + git verify-pack -v $p | sed -n -e "/^[0-9a-f]\{40\}/p" + done | while read sha1 rest; do + if ! ( git verify-pack -v $myidx | grep "^$sha1" ); then + echo "Missing object in local pack: $sha1" + return 1 + fi + done +' + test_done