From 9245ddd515d0fb5da52da4fd4dfc71460e98db90 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 12 Nov 2008 11:59:02 -0600 Subject: 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 diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh new file mode 100755 index 0000000..7aaff0b --- /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 + -- cgit v0.10.2-6-g49f6 From 8d25931d6ff47a7fb06512d767d1d416d9bc7733 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 12 Nov 2008 11:59:03 -0600 Subject: 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 diff --git a/cache.h b/cache.h index a1e4982..1a5740f 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 12fc767..adb1163 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); -- cgit v0.10.2-6-g49f6 From e96fb9b8f90f907d720ea6b71c92e30c2b071f4a Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 12 Nov 2008 11:59:04 -0600 Subject: 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 diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt index 8c354bd..f9fac2c 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 b0dddbe..29c0047 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); -- cgit v0.10.2-6-g49f6 From dd718365cccfddd7d5992a40296de50e7cabdfc8 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 12 Nov 2008 11:59:05 -0600 Subject: 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 diff --git a/git-repack.sh b/git-repack.sh index d39eb6c..8bb2201 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 . && -- cgit v0.10.2-6-g49f6 From f7991d1ed37502c0e98dfa95866dfc1a8b08d834 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 12 Nov 2008 11:59:06 -0600 Subject: 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 diff --git a/git-repack.sh b/git-repack.sh index 8bb2201..4d313d1 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 -- cgit v0.10.2-6-g49f6 From 01af249fa15ce63ea69e89e3520022420ecb409c Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 12 Nov 2008 11:59:07 -0600 Subject: builtin-gc.c: use new pack_keep bitfield to detect .keep file existence Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano diff --git a/builtin-gc.c b/builtin-gc.c index fac200e..53a0d43 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 -- cgit v0.10.2-6-g49f6 From 3c3df429106770966397086b6d2bced452ec7383 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Sun, 9 Nov 2008 23:59:56 -0600 Subject: 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 diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh index 356afe3..43c9cf9 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 -- cgit v0.10.2-6-g49f6 From 0f4dc14ac4945448f7309964afeb879d20408e07 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Sun, 9 Nov 2008 23:59:57 -0600 Subject: sha1_file.c: split has_loose_object() into local and non-local counterparts Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano diff --git a/cache.h b/cache.h index 1a5740f..7595c14 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 adb1163..0203de5 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; -- cgit v0.10.2-6-g49f6 From daae06259556246959963947752bde4ee2df7b44 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Sun, 9 Nov 2008 23:59:58 -0600 Subject: 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 diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt index f9fac2c..7d4c1a7 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 29c0047..85bd795 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 43c9cf9..960bff4 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 && -- cgit v0.10.2-6-g49f6 From 3289b9dec56d34fe05f90c262d11adc0a61e16e7 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Wed, 12 Nov 2008 18:50:26 -0600 Subject: 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 diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh index 960bff4..3f602ea 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 -- cgit v0.10.2-6-g49f6 From 83d0289df6fb4deae100ee3fc37b90683c2e8c9f Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Thu, 13 Nov 2008 14:11:46 -0600 Subject: 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 diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt index bbe1485..aaa8852 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 4d313d1..458a497 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 531dac0..9813f11 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 -- cgit v0.10.2-6-g49f6