From 9245ddd515d0fb5da52da4fd4dfc71460e98db90 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 12 Nov 2008 11:59:02 -0600 Subject: [PATCH 01/11] 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 000000000..7aaff0bbe --- /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/11] 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 a1e4982cd..1a5740f58 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 12fc767ee..adb116350 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/11] 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 8c354bd47..f9fac2ccf 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 b0dddbee4..29c00474d 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/11] 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 d39eb6cea..8bb22014b 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/11] 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 8bb22014b..4d313d136 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/11] 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 fac200e0b..53a0d43b6 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/11] 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 356afe371..43c9cf913 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/11] 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 1a5740f58..7595c149e 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 adb116350..0203de585 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/11] 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 f9fac2ccf..7d4c1a755 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 29c00474d..85bd795d3 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 43c9cf913..960bff47f 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/11] 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 960bff47f..3f602ea7d 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 From 83d0289df6fb4deae100ee3fc37b90683c2e8c9f Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Thu, 13 Nov 2008 14:11:46 -0600 Subject: [PATCH 11/11] repack: only unpack-unreachable if we are deleting redundant packs The -A option calls pack-objects with the --unpack-unreachable option so that the unreachable objects in local packs are left in the local object store loose. But if the -d option to repack was _not_ used, then these unpacked loose objects are redundant and unnecessary. Update tests in t7701. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- Documentation/git-repack.txt | 11 +++++------ git-repack.sh | 3 ++- t/t7701-repack-unpack-unreachable.sh | 18 +++++++++++++++--- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt index bbe1485a9..aaa885262 100644 --- a/Documentation/git-repack.txt +++ b/Documentation/git-repack.txt @@ -38,12 +38,11 @@ OPTIONS dangling. -A:: - Same as `-a`, but any unreachable objects in a previous - pack become loose, unpacked objects, instead of being - left in the old pack. Unreachable objects are never - intentionally added to a pack, even when repacking. - When used with '-d', this option - prevents unreachable objects from being immediately + Same as `-a`, unless '-d' is used. Then any unreachable + objects in a previous pack become loose, unpacked objects, + instead of being left in the old pack. Unreachable objects + are never intentionally added to a pack, even when repacking. + This option prevents unreachable objects from being immediately deleted by way of being left in the old pack and then removed. Instead, the loose unreachable objects will be pruned according to normal expiry rules diff --git a/git-repack.sh b/git-repack.sh index 4d313d136..458a497af 100755 --- a/git-repack.sh +++ b/git-repack.sh @@ -71,7 +71,8 @@ case ",$all_into_one," in existing="$existing $e" fi done - if test -n "$args" -a -n "$unpack_unreachable" + if test -n "$args" -a -n "$unpack_unreachable" -a \ + -n "$remove_redundant" then args="$args $unpack_unreachable" fi diff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh index 531dac060..9813f113a 100755 --- a/t/t7701-repack-unpack-unreachable.sh +++ b/t/t7701-repack-unpack-unreachable.sh @@ -8,7 +8,7 @@ fsha1= csha1= tsha1= -test_expect_success '-A option leaves unreachable objects unpacked' ' +test_expect_success '-A with -d option leaves unreachable objects unpacked' ' echo content > file1 && git add . && git commit -m initial_commit && @@ -58,7 +58,7 @@ compare_mtimes () ' -- "$@" } -test_expect_success 'unpacked objects receive timestamp of pack file' ' +test_expect_success '-A without -d option leaves unreachable objects packed' ' fsha1path=$(echo "$fsha1" | sed -e "s|\(..\)|\1/|") && fsha1path=".git/objects/$fsha1path" && csha1path=$(echo "$csha1" | sed -e "s|\(..\)|\1/|") && @@ -75,7 +75,19 @@ test_expect_success 'unpacked objects receive timestamp of pack file' ' git branch -D transient_branch && sleep 1 && git repack -A -l && - compare_mtimes "$packfile" "$fsha1path" "$csha1path" "$tsha1path" + test ! -f "$fsha1path" && + test ! -f "$csha1path" && + test ! -f "$tsha1path" && + git show $fsha1 && + git show $csha1 && + git show $tsha1 +' + +test_expect_success 'unpacked objects receive timestamp of pack file' ' + tmppack=".git/objects/pack/tmp_pack" && + ln "$packfile" "$tmppack" && + git repack -A -l -d && + compare_mtimes "$tmppack" "$fsha1path" "$csha1path" "$tsha1path" ' test_done