From 2478dc84b5fb2617bfab3a8f65f1270de578b94a Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 27 Feb 2009 23:58:50 -0800 Subject: [PATCH 01/12] git-repack: resist stray environment variable The script used $args and $existing without initializing it to empty. It would have been confused by an environment variable the end user had before running it. Signed-off-by: Junio C Hamano --- git-repack.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/git-repack.sh b/git-repack.sh index 458a497af8..86000151cc 100755 --- a/git-repack.sh +++ b/git-repack.sh @@ -60,6 +60,7 @@ case ",$all_into_one," in args='--unpacked --incremental' ;; ,t,) + args= existing= if [ -d "$PACKDIR" ]; then for e in `cd "$PACKDIR" && find . -type f -name '*.pack' \ | sed -e 's/^\.\///' -e 's/\.pack$//'` From cd673c1f17228d272c4b7f81fbb28bc31cf0cac6 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 27 Feb 2009 23:15:53 -0800 Subject: [PATCH 02/12] has_sha1_pack(): refactor "pretend these packs do not exist" interface Most of the callers of this function except only one pass NULL to its last parameter, ignore_packed. Introduce has_sha1_kept_pack() function that has the function signature and the semantics of this function, and convert the sole caller that does not pass NULL to call this new function. All other callers and has_sha1_pack() lose the ignore_packed parameter. Signed-off-by: Junio C Hamano --- builtin-count-objects.c | 2 +- builtin-fsck.c | 2 +- builtin-prune-packed.c | 2 +- cache.h | 3 ++- diff.c | 2 +- revision.c | 3 ++- sha1_file.c | 32 +++++++++++++++++++++++++------- 7 files changed, 33 insertions(+), 13 deletions(-) diff --git a/builtin-count-objects.c b/builtin-count-objects.c index 91b5487478..c095e8dd2b 100644 --- a/builtin-count-objects.c +++ b/builtin-count-objects.c @@ -61,7 +61,7 @@ static void count_objects(DIR *d, char *path, int len, int verbose, hex[40] = 0; if (get_sha1_hex(hex, sha1)) die("internal error"); - if (has_sha1_pack(sha1, NULL)) + if (has_sha1_pack(sha1)) (*packed_loose)++; } } diff --git a/builtin-fsck.c b/builtin-fsck.c index 30971ce0ad..491375dc59 100644 --- a/builtin-fsck.c +++ b/builtin-fsck.c @@ -158,7 +158,7 @@ static void check_reachable_object(struct object *obj) * do a full fsck */ if (!obj->parsed) { - if (has_sha1_pack(obj->sha1, NULL)) + if (has_sha1_pack(obj->sha1)) return; /* it is in pack - forget about it */ printf("missing %s %s\n", typename(obj->type), sha1_to_hex(obj->sha1)); errors_found |= ERROR_REACHABLE; diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c index 10cb8df845..2d5b2cd353 100644 --- a/builtin-prune-packed.c +++ b/builtin-prune-packed.c @@ -23,7 +23,7 @@ static void prune_dir(int i, DIR *dir, char *pathname, int len, int opts) memcpy(hex+2, de->d_name, 38); if (get_sha1_hex(hex, sha1)) continue; - if (!has_sha1_pack(sha1, NULL)) + if (!has_sha1_pack(sha1)) continue; memcpy(pathname + len, de->d_name, 38); if (opts & DRY_RUN) diff --git a/cache.h b/cache.h index 42f2f2754b..c1539bf89a 100644 --- a/cache.h +++ b/cache.h @@ -565,7 +565,8 @@ extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned l 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_pack(const unsigned char *sha1); +extern int has_sha1_kept_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); diff --git a/diff.c b/diff.c index f91f256c56..a34d26c23f 100644 --- a/diff.c +++ b/diff.c @@ -1743,7 +1743,7 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int * objects however would tend to be slower as they need * to be individually opened and inflated. */ - if (!FAST_WORKING_DIRECTORY && !want_file && has_sha1_pack(sha1, NULL)) + if (!FAST_WORKING_DIRECTORY && !want_file && has_sha1_pack(sha1)) return 0; len = strlen(name); diff --git a/revision.c b/revision.c index 45fd7a3660..746eeed972 100644 --- a/revision.c +++ b/revision.c @@ -1485,7 +1485,8 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit) { if (commit->object.flags & SHOWN) return commit_ignore; - if (revs->unpacked && has_sha1_pack(commit->object.sha1, revs->ignore_packed)) + if (revs->unpacked && + has_sha1_kept_pack(commit->object.sha1, revs->ignore_packed)) return commit_ignore; if (revs->show_all) return commit_show; diff --git a/sha1_file.c b/sha1_file.c index 88035a0cd1..ac4375d298 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1874,7 +1874,8 @@ int matches_pack_name(struct packed_git *p, const char *name) return 0; } -static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e, const char **ignore_packed) +static int find_pack_ent(const unsigned char *sha1, struct pack_entry *e, + const char **ignore_packed) { static struct packed_git *last_found = (void *)1; struct packed_git *p; @@ -1934,6 +1935,17 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e, cons return 0; } +static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e) +{ + return find_pack_ent(sha1, e, NULL); +} + +static int find_kept_pack_entry(const unsigned char *sha1, struct pack_entry *e, + const char **ignore_packed) +{ + return find_pack_ent(sha1, e, ignore_packed); +} + struct packed_git *find_sha1_pack(const unsigned char *sha1, struct packed_git *packs) { @@ -1975,7 +1987,7 @@ int sha1_object_info(const unsigned char *sha1, unsigned long *sizep) struct pack_entry e; int status; - if (!find_pack_entry(sha1, &e, NULL)) { + if (!find_pack_entry(sha1, &e)) { /* Most likely it's a loose object. */ status = sha1_loose_object_info(sha1, sizep); if (status >= 0) @@ -1983,7 +1995,7 @@ int sha1_object_info(const unsigned char *sha1, unsigned long *sizep) /* Not a loose object; someone else may have just packed it. */ reprepare_packed_git(); - if (!find_pack_entry(sha1, &e, NULL)) + if (!find_pack_entry(sha1, &e)) return status; } return packed_object_info(e.p, e.offset, sizep); @@ -1995,7 +2007,7 @@ static void *read_packed_sha1(const unsigned char *sha1, struct pack_entry e; void *data; - if (!find_pack_entry(sha1, &e, NULL)) + if (!find_pack_entry(sha1, &e)) return NULL; data = cache_or_unpack_entry(e.p, e.offset, size, type, 1); if (!data) { @@ -2395,17 +2407,23 @@ int has_pack_file(const unsigned char *sha1) return 1; } -int has_sha1_pack(const unsigned char *sha1, const char **ignore_packed) +int has_sha1_pack(const unsigned char *sha1) +{ + struct pack_entry e; + return find_pack_entry(sha1, &e); +} + +int has_sha1_kept_pack(const unsigned char *sha1, const char **ignore_packed) { struct pack_entry e; - return find_pack_entry(sha1, &e, ignore_packed); + return find_kept_pack_entry(sha1, &e, ignore_packed); } int has_sha1_file(const unsigned char *sha1) { struct pack_entry e; - if (find_pack_entry(sha1, &e, NULL)) + if (find_pack_entry(sha1, &e)) return 1; return has_loose_object(sha1); } From b8431b033f9e60f87a75b864612873307a3e5966 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 27 Feb 2009 23:30:38 -0800 Subject: [PATCH 03/12] has_sha1_kept_pack(): take "struct rev_info" Its "ignore_packed" parameter always comes from struct rev_info. This patch makes the function take a pointer to the surrounding structure, so that the refactoring in the next patch becomes easier to review. There is an unfortunate header file dependency and the easiest workaround is to temporarily move the function declaration from cache.h to revision.h; this will be moved back to cache.h once the function loses this "ignore_packed" parameter altogether in the later part of the series. Signed-off-by: Junio C Hamano --- cache.h | 1 - revision.c | 2 +- revision.h | 2 ++ sha1_file.c | 16 +++++++++------- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/cache.h b/cache.h index c1539bf89a..8e43f382e8 100644 --- a/cache.h +++ b/cache.h @@ -566,7 +566,6 @@ extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned l extern int move_temp_to_file(const char *tmpfile, const char *filename); extern int has_sha1_pack(const unsigned char *sha1); -extern int has_sha1_kept_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); diff --git a/revision.c b/revision.c index 746eeed972..795e0c03fe 100644 --- a/revision.c +++ b/revision.c @@ -1486,7 +1486,7 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit) if (commit->object.flags & SHOWN) return commit_ignore; if (revs->unpacked && - has_sha1_kept_pack(commit->object.sha1, revs->ignore_packed)) + has_sha1_kept_pack(commit->object.sha1, revs)) return commit_ignore; if (revs->show_all) return commit_show; diff --git a/revision.h b/revision.h index 91f194478b..930429625f 100644 --- a/revision.h +++ b/revision.h @@ -158,4 +158,6 @@ enum commit_action { extern enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit); +extern int has_sha1_kept_pack(const unsigned char *sha1, const struct rev_info *); + #endif diff --git a/sha1_file.c b/sha1_file.c index ac4375d298..f963c3cadb 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -16,6 +16,8 @@ #include "refs.h" #include "pack-revindex.h" #include "sha1-lookup.h" +#include "diff.h" +#include "revision.h" #ifndef O_NOATIME #if defined(__linux__) && (defined(__i386__) || defined(__PPC__)) @@ -1875,7 +1877,7 @@ int matches_pack_name(struct packed_git *p, const char *name) } static int find_pack_ent(const unsigned char *sha1, struct pack_entry *e, - const char **ignore_packed) + const struct rev_info *revs) { static struct packed_git *last_found = (void *)1; struct packed_git *p; @@ -1887,9 +1889,9 @@ static int find_pack_ent(const unsigned char *sha1, struct pack_entry *e, p = (last_found == (void *)1) ? packed_git : last_found; do { - if (ignore_packed) { + if (revs->ignore_packed) { const char **ig; - for (ig = ignore_packed; *ig; ig++) + for (ig = revs->ignore_packed; *ig; ig++) if (matches_pack_name(p, *ig)) break; if (*ig) @@ -1941,9 +1943,9 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e) } static int find_kept_pack_entry(const unsigned char *sha1, struct pack_entry *e, - const char **ignore_packed) + const struct rev_info *revs) { - return find_pack_ent(sha1, e, ignore_packed); + return find_pack_ent(sha1, e, revs); } struct packed_git *find_sha1_pack(const unsigned char *sha1, @@ -2413,10 +2415,10 @@ int has_sha1_pack(const unsigned char *sha1) return find_pack_entry(sha1, &e); } -int has_sha1_kept_pack(const unsigned char *sha1, const char **ignore_packed) +int has_sha1_kept_pack(const unsigned char *sha1, const struct rev_info *revs) { struct pack_entry e; - return find_kept_pack_entry(sha1, &e, ignore_packed); + return find_kept_pack_entry(sha1, &e, revs); } int has_sha1_file(const unsigned char *sha1) From 386cb77210cdb09cd808698d21d0e796cd77f26f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 27 Feb 2009 23:43:37 -0800 Subject: [PATCH 04/12] Consolidate ignore_packed logic more This refactors three loops that check if a given packfile is on the ignore_packed list into a function is_kept_pack(). The function returns false for a pack on the list, and true for a pack not on the list, because this list is solely used by "git repack" to pass list of packfiles that do not have corresponding .keep files, i.e. a packfile not on the list is "kept". Signed-off-by: Junio C Hamano --- builtin-pack-objects.c | 12 ++---------- cache.h | 1 - revision.h | 1 + sha1_file.c | 24 ++++++++++++++---------- 4 files changed, 17 insertions(+), 21 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index fb5e14d56e..7e7719b832 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -1915,11 +1915,7 @@ static void add_objects_in_unpacked_packs(struct rev_info *revs) const unsigned char *sha1; struct object *o; - for (i = 0; i < revs->num_ignore_packed; i++) { - if (matches_pack_name(p, revs->ignore_packed[i])) - break; - } - if (revs->num_ignore_packed <= i) + if (is_kept_pack(p, revs)) continue; if (open_pack_index(p)) die("cannot open pack index"); @@ -1955,11 +1951,7 @@ static void loosen_unused_packed_objects(struct rev_info *revs) const unsigned char *sha1; for (p = packed_git; p; p = p->next) { - for (i = 0; i < revs->num_ignore_packed; i++) { - if (matches_pack_name(p, revs->ignore_packed[i])) - break; - } - if (revs->num_ignore_packed <= i) + if (is_kept_pack(p, revs)) continue; if (open_pack_index(p)) diff --git a/cache.h b/cache.h index 8e43f382e8..23c16d0d99 100644 --- a/cache.h +++ b/cache.h @@ -747,7 +747,6 @@ extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsign extern unsigned long unpack_object_header_gently(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep); extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t); extern const char *packed_object_info_detail(struct packed_git *, off_t, unsigned long *, unsigned long *, unsigned int *, unsigned char *); -extern int matches_pack_name(struct packed_git *p, const char *name); /* Dumb servers support */ extern int update_server_info(int); diff --git a/revision.h b/revision.h index 930429625f..af414e5d94 100644 --- a/revision.h +++ b/revision.h @@ -159,5 +159,6 @@ enum commit_action { extern enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit); extern int has_sha1_kept_pack(const unsigned char *sha1, const struct rev_info *); +extern int is_kept_pack(const struct packed_git *, const struct rev_info *); #endif diff --git a/sha1_file.c b/sha1_file.c index f963c3cadb..6e0a462f10 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1858,7 +1858,7 @@ off_t find_pack_entry_one(const unsigned char *sha1, return 0; } -int matches_pack_name(struct packed_git *p, const char *name) +static int matches_pack_name(const struct packed_git *p, const char *name) { const char *last_c, *c; @@ -1876,6 +1876,17 @@ int matches_pack_name(struct packed_git *p, const char *name) return 0; } +int is_kept_pack(const struct packed_git *p, const struct rev_info *revs) +{ + int i; + + for (i = 0; i < revs->num_ignore_packed; i++) { + if (matches_pack_name(p, revs->ignore_packed[i])) + return 0; + } + return 1; +} + static int find_pack_ent(const unsigned char *sha1, struct pack_entry *e, const struct rev_info *revs) { @@ -1889,15 +1900,8 @@ static int find_pack_ent(const unsigned char *sha1, struct pack_entry *e, p = (last_found == (void *)1) ? packed_git : last_found; do { - if (revs->ignore_packed) { - const char **ig; - for (ig = revs->ignore_packed; *ig; ig++) - if (matches_pack_name(p, *ig)) - break; - if (*ig) - goto next; - } - + if (revs->ignore_packed && !is_kept_pack(p, revs)) + goto next; if (p->num_bad_objects) { unsigned i; for (i = 0; i < p->num_bad_objects; i++) From 03a9683d22eca52bc2b2b9b09258a143e76416f6 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 28 Feb 2009 00:00:21 -0800 Subject: [PATCH 05/12] Simplify is_kept_pack() This removes --unpacked= parameter from the revision parser, and rewrites its use in git-repack to pass a single --kept-pack-only option instead. The new --kept-pack-only option means just that. When this option is given, is_kept_pack() that used to say "not on the --unpacked= list" now says "the packfile has corresponding .keep file". Signed-off-by: Junio C Hamano --- builtin-pack-objects.c | 6 +++--- git-repack.sh | 5 ++++- revision.c | 20 +++++--------------- revision.h | 8 +++----- sha1_file.c | 30 +++--------------------------- 5 files changed, 18 insertions(+), 51 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 7e7719b832..150258b629 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -1915,7 +1915,7 @@ static void add_objects_in_unpacked_packs(struct rev_info *revs) const unsigned char *sha1; struct object *o; - if (is_kept_pack(p, revs)) + if (is_kept_pack(p)) continue; if (open_pack_index(p)) die("cannot open pack index"); @@ -1951,7 +1951,7 @@ static void loosen_unused_packed_objects(struct rev_info *revs) const unsigned char *sha1; for (p = packed_git; p; p = p->next) { - if (is_kept_pack(p, revs)) + if (is_kept_pack(p)) continue; if (open_pack_index(p)) @@ -2149,7 +2149,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) continue; } if (!strcmp("--unpacked", arg) || - !prefixcmp(arg, "--unpacked=") || + !strcmp("--kept-pack-only", arg) || !strcmp("--reflog", arg) || !strcmp("--all", arg)) { use_internal_rev_list = 1; diff --git a/git-repack.sh b/git-repack.sh index 86000151cc..a736009c67 100755 --- a/git-repack.sh +++ b/git-repack.sh @@ -68,10 +68,13 @@ case ",$all_into_one," in if [ -e "$PACKDIR/$e.keep" ]; then : keep else - args="$args --unpacked=$e.pack" existing="$existing $e" fi done + if test -n "$existing" + then + args="--kept-pack-only" + fi if test -n "$args" -a -n "$unpack_unreachable" -a \ -n "$remove_redundant" then diff --git a/revision.c b/revision.c index 795e0c03fe..3cfd6539ab 100644 --- a/revision.c +++ b/revision.c @@ -963,16 +963,6 @@ static void add_message_grep(struct rev_info *revs, const char *pattern) add_grep(revs, pattern, GREP_PATTERN_BODY); } -static void add_ignore_packed(struct rev_info *revs, const char *name) -{ - int num = ++revs->num_ignore_packed; - - revs->ignore_packed = xrealloc(revs->ignore_packed, - sizeof(const char *) * (num + 1)); - revs->ignore_packed[num-1] = name; - revs->ignore_packed[num] = NULL; -} - static int handle_revision_opt(struct rev_info *revs, int argc, const char **argv, int *unkc, const char **unkv) { @@ -1072,12 +1062,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg revs->edge_hint = 1; } else if (!strcmp(arg, "--unpacked")) { revs->unpacked = 1; - free(revs->ignore_packed); - revs->ignore_packed = NULL; - revs->num_ignore_packed = 0; - } else if (!prefixcmp(arg, "--unpacked=")) { + revs->kept_pack_only = 0; + } else if (!strcmp(arg, "--kept-pack-only")) { revs->unpacked = 1; - add_ignore_packed(revs, arg+11); + revs->kept_pack_only = 1; + } else if (!prefixcmp(arg, "--unpacked=")) { + die("--unpacked= no longer supported."); } else if (!strcmp(arg, "-r")) { revs->diff = 1; DIFF_OPT_SET(&revs->diffopt, RECURSIVE); diff --git a/revision.h b/revision.h index af414e5d94..f63596ff45 100644 --- a/revision.h +++ b/revision.h @@ -47,7 +47,8 @@ struct rev_info { blob_objects:1, edge_hint:1, limited:1, - unpacked:1, /* see also ignore_packed below */ + unpacked:1, + kept_pack_only:1, boundary:2, left_right:1, rewrite_parents:1, @@ -75,9 +76,6 @@ struct rev_info { missing_newline:1; enum date_mode date_mode; - const char **ignore_packed; /* pretend objects in these are unpacked */ - int num_ignore_packed; - unsigned int abbrev; enum cmit_fmt commit_format; struct log_info *loginfo; @@ -159,6 +157,6 @@ enum commit_action { extern enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit); extern int has_sha1_kept_pack(const unsigned char *sha1, const struct rev_info *); -extern int is_kept_pack(const struct packed_git *, const struct rev_info *); +extern int is_kept_pack(const struct packed_git *); #endif diff --git a/sha1_file.c b/sha1_file.c index 6e0a462f10..e8a9517d01 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1858,33 +1858,9 @@ off_t find_pack_entry_one(const unsigned char *sha1, return 0; } -static int matches_pack_name(const struct packed_git *p, const char *name) +int is_kept_pack(const struct packed_git *p) { - const char *last_c, *c; - - if (!strcmp(p->pack_name, name)) - return 1; - - for (c = p->pack_name, last_c = c; *c;) - if (*c == '/') - last_c = ++c; - else - ++c; - if (!strcmp(last_c, name)) - return 1; - - return 0; -} - -int is_kept_pack(const struct packed_git *p, const struct rev_info *revs) -{ - int i; - - for (i = 0; i < revs->num_ignore_packed; i++) { - if (matches_pack_name(p, revs->ignore_packed[i])) - return 0; - } - return 1; + return p->pack_keep; } static int find_pack_ent(const unsigned char *sha1, struct pack_entry *e, @@ -1900,7 +1876,7 @@ static int find_pack_ent(const unsigned char *sha1, struct pack_entry *e, p = (last_found == (void *)1) ? packed_git : last_found; do { - if (revs->ignore_packed && !is_kept_pack(p, revs)) + if (revs->kept_pack_only && !is_kept_pack(p)) goto next; if (p->num_bad_objects) { unsigned i; From 69e020ae00ebd3f7ae3c2f35acb139361417ef64 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 28 Feb 2009 00:37:19 -0800 Subject: [PATCH 06/12] is_kept_pack(): final clean-up Now is_kept_pack() is just a member lookup into a structure, we can write it as such. Also rewrite the sole caller of has_sha1_kept_pack() to switch on the criteria the callee uses (namely, revs->kept_pack_only) between calling has_sha1_kept_pack() and has_sha1_pack(), so that these two callees do not have to take a pointer to struct rev_info as an argument. This removes the header file dependency issue temporarily introduced by the earlier commit, so we revert changes associated to that as well. Signed-off-by: Junio C Hamano --- builtin-pack-objects.c | 4 ++-- cache.h | 1 + revision.c | 4 +++- revision.h | 3 --- sha1_file.c | 22 +++++++--------------- 5 files changed, 13 insertions(+), 21 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 150258b629..b2e46264ee 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -1915,7 +1915,7 @@ static void add_objects_in_unpacked_packs(struct rev_info *revs) const unsigned char *sha1; struct object *o; - if (is_kept_pack(p)) + if (p->pack_keep) continue; if (open_pack_index(p)) die("cannot open pack index"); @@ -1951,7 +1951,7 @@ static void loosen_unused_packed_objects(struct rev_info *revs) const unsigned char *sha1; for (p = packed_git; p; p = p->next) { - if (is_kept_pack(p)) + if (p->pack_keep) continue; if (open_pack_index(p)) diff --git a/cache.h b/cache.h index 23c16d0d99..0a3d523d26 100644 --- a/cache.h +++ b/cache.h @@ -566,6 +566,7 @@ extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned l extern int move_temp_to_file(const char *tmpfile, const char *filename); extern int has_sha1_pack(const unsigned char *sha1); +extern int has_sha1_kept_pack(const unsigned char *sha1); extern int has_sha1_file(const unsigned char *sha1); extern int has_loose_object_nonlocal(const unsigned char *sha1); diff --git a/revision.c b/revision.c index 3cfd6539ab..6d8ac46081 100644 --- a/revision.c +++ b/revision.c @@ -1476,7 +1476,9 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit) if (commit->object.flags & SHOWN) return commit_ignore; if (revs->unpacked && - has_sha1_kept_pack(commit->object.sha1, revs)) + (revs->kept_pack_only + ? has_sha1_kept_pack(commit->object.sha1) + : has_sha1_pack(commit->object.sha1))) return commit_ignore; if (revs->show_all) return commit_show; diff --git a/revision.h b/revision.h index f63596ff45..b9fa9c2a67 100644 --- a/revision.h +++ b/revision.h @@ -156,7 +156,4 @@ enum commit_action { extern enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit); -extern int has_sha1_kept_pack(const unsigned char *sha1, const struct rev_info *); -extern int is_kept_pack(const struct packed_git *); - #endif diff --git a/sha1_file.c b/sha1_file.c index e8a9517d01..7ead56cc3e 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -16,8 +16,6 @@ #include "refs.h" #include "pack-revindex.h" #include "sha1-lookup.h" -#include "diff.h" -#include "revision.h" #ifndef O_NOATIME #if defined(__linux__) && (defined(__i386__) || defined(__PPC__)) @@ -1858,13 +1856,8 @@ off_t find_pack_entry_one(const unsigned char *sha1, return 0; } -int is_kept_pack(const struct packed_git *p) -{ - return p->pack_keep; -} - static int find_pack_ent(const unsigned char *sha1, struct pack_entry *e, - const struct rev_info *revs) + int kept_pack_only) { static struct packed_git *last_found = (void *)1; struct packed_git *p; @@ -1876,7 +1869,7 @@ static int find_pack_ent(const unsigned char *sha1, struct pack_entry *e, p = (last_found == (void *)1) ? packed_git : last_found; do { - if (revs->kept_pack_only && !is_kept_pack(p)) + if (kept_pack_only && !p->pack_keep) goto next; if (p->num_bad_objects) { unsigned i; @@ -1919,13 +1912,12 @@ static int find_pack_ent(const unsigned char *sha1, struct pack_entry *e, static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e) { - return find_pack_ent(sha1, e, NULL); + return find_pack_ent(sha1, e, 0); } -static int find_kept_pack_entry(const unsigned char *sha1, struct pack_entry *e, - const struct rev_info *revs) +static int find_kept_pack_entry(const unsigned char *sha1, struct pack_entry *e) { - return find_pack_ent(sha1, e, revs); + return find_pack_ent(sha1, e, 1); } struct packed_git *find_sha1_pack(const unsigned char *sha1, @@ -2395,10 +2387,10 @@ int has_sha1_pack(const unsigned char *sha1) return find_pack_entry(sha1, &e); } -int has_sha1_kept_pack(const unsigned char *sha1, const struct rev_info *revs) +int has_sha1_kept_pack(const unsigned char *sha1) { struct pack_entry e; - return find_kept_pack_entry(sha1, &e, revs); + return find_kept_pack_entry(sha1, &e); } int has_sha1_file(const unsigned char *sha1) From 92cd872202241a0b80e88dadac5a4db071c8d1fa Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Thu, 19 Mar 2009 22:47:50 -0500 Subject: [PATCH 07/12] t7700-repack: add two new tests demonstrating repacking flaws 1) The new --kept-pack-only mechansim of rev-list/pack-objects has replaced --unpacked=. This new mechansim does not operate solely on "local" packs now. The result is that objects residing in an alternate pack which has a .keep file will not be repacked with repack -a. This flaw is only apparent when a commit object is the one residing in an alternate kept pack. 2) The 'repack unpacked objects' and 'loosen unpacked objects' mechanisms of pack-objects, i.e. --keep-unreachable and --unpack-unreachable, now do not operate solely on local packs. The --keep-unreachable option no longer has any callers, but --unpack-unreachable is used when repack is called with '-A -d' and the local repo has existing packs. In this case, objects residing in alternate, not-kept packs will be loosened, and then immediately deleted by repack's call to prune-packed. The test must manually call pack-objects to avoid the call to prune-packed that is made by repack when -d is used. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- t/t7700-repack.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh index 3f602ea7de..fa4772101f 100755 --- a/t/t7700-repack.sh +++ b/t/t7700-repack.sh @@ -69,5 +69,49 @@ test_expect_success 'packed obs in alt ODB are repacked even when local repo is done ' +test_expect_failure 'packed obs in alternate ODB kept pack are repacked' ' + # swap the .keep so the commit object is in the pack with .keep + for p in alt_objects/pack/*.pack + do + base_name=$(basename $p .pack) + if test -f alt_objects/pack/$base_name.keep + then + rm alt_objects/pack/$base_name.keep + else + touch alt_objects/pack/$base_name.keep + fi + done + git repack -a -d && + 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_expect_failure 'packed unreachable obs in alternate ODB are not loosened' ' + rm -f alt_objects/pack/*.keep && + mv .git/objects/pack/* alt_objects/pack/ && + csha1=$(git rev-parse HEAD^{commit}) && + git reset --hard HEAD^ && + sleep 1 && + git reflog expire --expire=now --expire-unreachable=now --all && + # The pack-objects call on the next line is equivalent to + # git repack -A -d without the call to prune-packed + git pack-objects --honor-pack-keep --non-empty --all --reflog \ + --unpack-unreachable .git/objects/info/alternates && + test_must_fail git show $csha1 +' + test_done From 171110a4a67f04c28d2ac89385ab88ba051fc780 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Thu, 19 Mar 2009 22:47:51 -0500 Subject: [PATCH 08/12] git-repack.sh: don't use --kept-pack-only option to pack-objects The --kept-pack-only option to pack-objects treats all kept packs as equal. This results in objects that reside in an alternate pack that has a .keep file, not being packed into a newly created pack when the user specifies the -a option to repack. Since the user may not have any control over the alternate database, git should not refrain from repacking those objects even though they are in a pack with a .keep file. This fixes the 'packed obs in alternate ODB kept pack are repacked' test in t7700. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- git-repack.sh | 6 +----- t/t7700-repack.sh | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/git-repack.sh b/git-repack.sh index a736009c67..e02bf27aa6 100755 --- a/git-repack.sh +++ b/git-repack.sh @@ -71,11 +71,7 @@ case ",$all_into_one," in existing="$existing $e" fi done - if test -n "$existing" - then - args="--kept-pack-only" - fi - if test -n "$args" -a -n "$unpack_unreachable" -a \ + if test -n "$existing" -a -n "$unpack_unreachable" -a \ -n "$remove_redundant" then args="$args $unpack_unreachable" diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh index fa4772101f..adba8a1c65 100755 --- a/t/t7700-repack.sh +++ b/t/t7700-repack.sh @@ -69,7 +69,7 @@ test_expect_success 'packed obs in alt ODB are repacked even when local repo is done ' -test_expect_failure 'packed obs in alternate ODB kept pack are repacked' ' +test_expect_success 'packed obs in alternate ODB kept pack are repacked' ' # swap the .keep so the commit object is in the pack with .keep for p in alt_objects/pack/*.pack do From 79bc4c715516fdb393d107359327c1e7fbb8bf04 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Thu, 19 Mar 2009 22:47:52 -0500 Subject: [PATCH 09/12] pack-objects: only repack or loosen objects residing in "local" packs These two features were invented for use by repack when repack will delete the local packs that have been made redundant. The packs accessible through alternates are not deleted by repack, so the objects contained in them are still accessible after the local packs are deleted. They do not need to be repacked into the new pack or loosened. For the case of loosening they would immediately be deleted by the subsequent prune-packed that is called by repack anyway. This fixes the test 'packed unreachable obs in alternate ODB are not loosened' in t7700. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- builtin-pack-objects.c | 4 ++-- t/t7700-repack.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index b2e46264ee..aae4d243b3 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -1915,7 +1915,7 @@ static void add_objects_in_unpacked_packs(struct rev_info *revs) const unsigned char *sha1; struct object *o; - if (p->pack_keep) + if (!p->pack_local || p->pack_keep) continue; if (open_pack_index(p)) die("cannot open pack index"); @@ -1951,7 +1951,7 @@ static void loosen_unused_packed_objects(struct rev_info *revs) const unsigned char *sha1; for (p = packed_git; p; p = p->next) { - if (p->pack_keep) + if (!p->pack_local || p->pack_keep) continue; if (open_pack_index(p)) diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh index adba8a1c65..1ef3892f92 100755 --- a/t/t7700-repack.sh +++ b/t/t7700-repack.sh @@ -94,7 +94,7 @@ test_expect_success 'packed obs in alternate ODB kept pack are repacked' ' done ' -test_expect_failure 'packed unreachable obs in alternate ODB are not loosened' ' +test_expect_success 'packed unreachable obs in alternate ODB are not loosened' ' rm -f alt_objects/pack/*.keep && mv .git/objects/pack/* alt_objects/pack/ && csha1=$(git rev-parse HEAD^{commit}) && From 4d6acb70411cd4fe69610cf1b22f186fa01614f7 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Thu, 19 Mar 2009 22:47:54 -0500 Subject: [PATCH 10/12] Remove --kept-pack-only option and associated infrastructure This option to pack-objects/rev-list was created to improve the -A and -a options of repack. It was found to be lacking in that it did not provide the ability to differentiate between local and non-local kept packs, and found to be unnecessary since objects residing in local kept packs can be filtered out by the --honor-pack-keep option. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- builtin-pack-objects.c | 1 - cache.h | 1 - revision.c | 9 +-------- revision.h | 1 - sha1_file.c | 21 +-------------------- 5 files changed, 2 insertions(+), 31 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index aae4d243b3..6222f19c78 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -2149,7 +2149,6 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) continue; } if (!strcmp("--unpacked", arg) || - !strcmp("--kept-pack-only", arg) || !strcmp("--reflog", arg) || !strcmp("--all", arg)) { use_internal_rev_list = 1; diff --git a/cache.h b/cache.h index 0a3d523d26..23c16d0d99 100644 --- a/cache.h +++ b/cache.h @@ -566,7 +566,6 @@ extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned l extern int move_temp_to_file(const char *tmpfile, const char *filename); extern int has_sha1_pack(const unsigned char *sha1); -extern int has_sha1_kept_pack(const unsigned char *sha1); extern int has_sha1_file(const unsigned char *sha1); extern int has_loose_object_nonlocal(const unsigned char *sha1); diff --git a/revision.c b/revision.c index 6d8ac46081..50a5b5f394 100644 --- a/revision.c +++ b/revision.c @@ -1062,10 +1062,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg revs->edge_hint = 1; } else if (!strcmp(arg, "--unpacked")) { revs->unpacked = 1; - revs->kept_pack_only = 0; - } else if (!strcmp(arg, "--kept-pack-only")) { - revs->unpacked = 1; - revs->kept_pack_only = 1; } else if (!prefixcmp(arg, "--unpacked=")) { die("--unpacked= no longer supported."); } else if (!strcmp(arg, "-r")) { @@ -1475,10 +1471,7 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit) { if (commit->object.flags & SHOWN) return commit_ignore; - if (revs->unpacked && - (revs->kept_pack_only - ? has_sha1_kept_pack(commit->object.sha1) - : has_sha1_pack(commit->object.sha1))) + if (revs->unpacked && has_sha1_pack(commit->object.sha1)) return commit_ignore; if (revs->show_all) return commit_show; diff --git a/revision.h b/revision.h index b9fa9c2a67..1d322759aa 100644 --- a/revision.h +++ b/revision.h @@ -48,7 +48,6 @@ struct rev_info { edge_hint:1, limited:1, unpacked:1, - kept_pack_only:1, boundary:2, left_right:1, rewrite_parents:1, diff --git a/sha1_file.c b/sha1_file.c index 7ead56cc3e..500fd93127 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1856,8 +1856,7 @@ off_t find_pack_entry_one(const unsigned char *sha1, return 0; } -static int find_pack_ent(const unsigned char *sha1, struct pack_entry *e, - int kept_pack_only) +static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e) { static struct packed_git *last_found = (void *)1; struct packed_git *p; @@ -1869,8 +1868,6 @@ static int find_pack_ent(const unsigned char *sha1, struct pack_entry *e, p = (last_found == (void *)1) ? packed_git : last_found; do { - if (kept_pack_only && !p->pack_keep) - goto next; if (p->num_bad_objects) { unsigned i; for (i = 0; i < p->num_bad_objects; i++) @@ -1910,16 +1907,6 @@ static int find_pack_ent(const unsigned char *sha1, struct pack_entry *e, return 0; } -static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e) -{ - return find_pack_ent(sha1, e, 0); -} - -static int find_kept_pack_entry(const unsigned char *sha1, struct pack_entry *e) -{ - return find_pack_ent(sha1, e, 1); -} - struct packed_git *find_sha1_pack(const unsigned char *sha1, struct packed_git *packs) { @@ -2387,12 +2374,6 @@ int has_sha1_pack(const unsigned char *sha1) return find_pack_entry(sha1, &e); } -int has_sha1_kept_pack(const unsigned char *sha1) -{ - struct pack_entry e; - return find_kept_pack_entry(sha1, &e); -} - int has_sha1_file(const unsigned char *sha1) { struct pack_entry e; From 869a3d34c1aea92a10bc8eaa994bd55f4b0b04f2 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Sat, 21 Mar 2009 17:25:30 -0500 Subject: [PATCH 11/12] t7700: demonstrate repack flaw which may loosen objects unnecessarily If an unreferenced object exists in both a local pack and in either a pack residing in an alternate object database or a local kept pack, then the pack-objects call made by repack will loosen that object only to have it immediately pruned by repack's call to prune-packed. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- t/t7700-repack.sh | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh index 1ef3892f92..013e488bdd 100755 --- a/t/t7700-repack.sh +++ b/t/t7700-repack.sh @@ -113,5 +113,22 @@ test_expect_success 'packed unreachable obs in alternate ODB are not loosened' ' test_must_fail git show $csha1 ' +test_expect_failure 'local packed unreachable obs that exist in alternate ODB are not loosened' ' + echo `pwd`/alt_objects > .git/objects/info/alternates && + echo "$csha1" | git pack-objects --non-empty --all --reflog pack && + rm -f .git/objects/pack/* && + mv pack-* .git/objects/pack/ && + # The pack-objects call on the next line is equivalent to + # git repack -A -d without the call to prune-packed + git pack-objects --honor-pack-keep --non-empty --all --reflog \ + --unpack-unreachable .git/objects/info/alternates && + test_must_fail git show $csha1 +' + test_done From 094085e3362c592c932b41525ed37152ec171192 Mon Sep 17 00:00:00 2001 From: Brandon Casey Date: Sat, 21 Mar 2009 17:26:11 -0500 Subject: [PATCH 12/12] pack-objects: don't loosen objects available in alternate or kept packs If pack-objects is called with the --unpack-unreachable option then it will unpack (i.e. loosen) all unreferenced objects from local not-kept packs, including those that also exist in packs residing in an alternate object database or a locally kept pack. The only user of this option is git-repack. In this case, repack will follow the call to pack-objects with a call to prune-packed, which will delete these newly loosened objects, making the act of loosening a waste of time. The unnecessary loosening can be avoided by checking whether an object exists in a non-local pack or a locally kept pack before loosening it. This fixes the 'local packed unreachable obs that exist in alternate ODB are not loosened' test in t7700. Signed-off-by: Brandon Casey Signed-off-by: Junio C Hamano --- builtin-pack-objects.c | 26 +++++++++++++++++++++++++- t/t7700-repack.sh | 2 +- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 6222f19c78..ad3f8e7751 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -1944,6 +1944,29 @@ static void add_objects_in_unpacked_packs(struct rev_info *revs) free(in_pack.array); } +static int has_sha1_pack_kept_or_nonlocal(const unsigned char *sha1) +{ + static struct packed_git *last_found = (void *)1; + struct packed_git *p; + + p = (last_found != (void *)1) ? last_found : packed_git; + + while (p) { + if ((!p->pack_local || p->pack_keep) && + find_pack_entry_one(sha1, p)) { + last_found = p; + return 1; + } + if (p == last_found) + p = packed_git; + else + p = p->next; + if (p == last_found) + p = p->next; + } + return 0; +} + static void loosen_unused_packed_objects(struct rev_info *revs) { struct packed_git *p; @@ -1959,7 +1982,8 @@ static void loosen_unused_packed_objects(struct rev_info *revs) for (i = 0; i < p->num_objects; i++) { sha1 = nth_packed_object_sha1(p, i); - if (!locate_object_entry(sha1)) + if (!locate_object_entry(sha1) && + !has_sha1_pack_kept_or_nonlocal(sha1)) if (force_object_loose(sha1, p->mtime)) die("unable to force loose object"); } diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh index 013e488bdd..9ce546e3b2 100755 --- a/t/t7700-repack.sh +++ b/t/t7700-repack.sh @@ -113,7 +113,7 @@ test_expect_success 'packed unreachable obs in alternate ODB are not loosened' ' test_must_fail git show $csha1 ' -test_expect_failure 'local packed unreachable obs that exist in alternate ODB are not loosened' ' +test_expect_success 'local packed unreachable obs that exist in alternate ODB are not loosened' ' echo `pwd`/alt_objects > .git/objects/info/alternates && echo "$csha1" | git pack-objects --non-empty --all --reflog pack && rm -f .git/objects/pack/* &&