Really slow 'git gc'
Ok, so I was wondering why doing a 'git gc' on my kernel backup on one of the linux-foundation machines was taking so long, and I think I've found a performance problem.
The way I do kernel back-ups is that I just push to two different sites every once in a while (read: multiple times a day when I do lots of merging), and one of them is master.kernel.org that then gets published to others.
The other one is a linux-foundation machine that I have a login on, and that's my "secondary" back-up, in case both kernel.org and my own home machines were to be corrupted somehow. And because it's my secondary, I seldom then log in an gc anything, so it's a mess.
But it was _really_ slow when I finally did so today. The whole "Counting objects" phase was counting by hundreds, which it really shouldn't do on a fast machine.
The reason? Tons and tons of pack-files. But just the existence of the pack-files is not what killed it: things were _much_ faster if I just did a "git pack-objects" by hand.
The real reason _seems_ to be the "--unpacked=pack-....pack" arguments. I literally had 232 pack-files, and it looks like a lot of the time was spent in that silly loop oer 'ignore_packed' in find_pack_entry(), when revision.c does that "has_sha1_pack()" thing. You get a O(n**2) effect in number of pack-files: for each commit we look over every pack-file, and for every pack-file we look at, we look over each ignore_pack entry.
I didn't really analyze this a lot, and now the thing is packed and much faster, but I thought I'd throw this out there..
Linus
Quoted 6 lines · Linus Torvalds
The real reason _seems_ to be the "--unpacked=pack-....pack" arguments. I literally had 232 pack-files, and it looks like a lot of the time was spent in that silly loop oer '
ignore_packed' infind_pack_entry(), whenrevision.cdoes that "has_sha1_pack()" thing. You get aO(n**2)effect in number of pack-files: for each commit we look over every pack-file, and for every pack-file we look at, we look over eachignore_packentry.
I think we can add a single bit to "struct packed_git" and in the middle of setup_revisions() perform the O(N**2) once, so that find_pack_entry() can check the bit without looping.
Quoted 9 lines · Junio C Hamano
Quoted 6 lines · Linus Torvalds
The real reason _seems_ to be the "--unpacked=pack-....pack" arguments. I literally had 232 pack-files, and it looks like a lot of the time was spent in that silly loop oer '
ignore_packed' infind_pack_entry(), whenrevision.cdoes that "has_sha1_pack()" thing. You get aO(n**2)effect in number of pack-files: for each commit we look over every pack-file, and for every pack-file we look at, we look over eachignore_packentry.
Quoted 3 lines
I think we can add a single bit to "struct
packed_git" and in the middle ofsetup_revisions()perform theO(N**2)once, so thatfind_pack_entry()can check the bit without looping.
Roughly like this, although we probably should change the API because most of the callers pass NULL to it. Also we may need a way to say "I am done with ignoring, please clear the pack_ignore bits from all of them" API.
---
cache.h | 4 +++- revision.c | 2 ++ sha1_file.c | 31 +++++++++++++++++++++++-------- 3 files changed, 28 insertions(+), 9 deletions(-)
cache.h +3 -1
@@ -759,7 +759,8 @@ extern struct packed_git {time_t mtime;int pack_fd; unsigned pack_local:1, - pack_keep:1; + pack_keep:1, + pack_ignore:1;unsigned char sha1[20 ];/* something like ".git/objects/pack/xxxxx.pack" */ char pack_name[FLEX_ARRAY ];/* more */
@@ -817,6 +818,7 @@ extern struct packed_git *parse_pack_index(unsigned char *sha1);extern void prepare_packed_git (void );extern void reprepare_packed_git (void );extern void install_packed_git (struct packed_git *pack); +extern void mark_ignore_packed (const char **);extern struct packed_git *find_sha1_pack (const unsigned char *sha1,struct packed_git *packs);
revision.c +2 -0
@@ -1342,6 +1342,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch object =get_reference (revs, revs->def, sha1,0 );add_pending_object_with_mode (revs, object, revs->def, mode); } +if (revs->ignore_packed) +mark_ignore_packed (revs->ignore_packed);/* Did the user ask for any diff output? Run the diff! */ if (revs->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT )
sha1_file.c +23 -8
@@ -1937,6 +1937,27 @@ int matches_pack_name(struct packed_git *p, const char *name)return 0 ; } +void mark_ignore_packed (const char **ignore_packed) +{ +struct packed_git *p; + +if (!ignore_packed || !*ignore_packed) +return ; + +prepare_packed_git (); +if (!packed_git) +return ; + +for (p = packed_git; p; p = p->next) { +const char **ig; +for (ig = ignore_packed; *ig; ig++) +if (matches_pack_name (p, *ig)) { + p->pack_ignore =1 ; +break ; + } + } +} +static int find_pack_entry (const unsigned char *sha1,struct pack_entry *e,const char **ignore_packed) {static struct packed_git *last_found = (void *)1 ;
@@ -1949,14 +1970,8 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e, cons p = (last_found == (void *)1 ) ? packed_git : last_found; do { - if (ignore_packed) { - const char **ig; - for (ig = ignore_packed; *ig; ig++) - if (matches_pack_name(p, *ig)) - break; - if (*ig) - goto next; - } + if (ignore_packed && p->pack_ignore) + goto next; if (p->num_bad_objects) { unsigned i;
Quoted 4 lines · Junio C Hamano
I think we can add a single bit to "struct
packed_git" and in the middle ofsetup_revisions()perform theO(N**2)once, so thatfind_pack_entry()can check the bit without looping.
Yes. However, most users call find_pack_entry() with a NULL ignore_packed, so we'd still have to pass in that flag to say whether we should look at the bit or not.
However, the real issue (?) I think is that the whole "ignore_packed" logic is crazy. It's the wrong way around. The whole thing is broken.
Rather than marking which ones are "unpacked" and should be ignored, it should just look at the ones to keep. That's how the filesystem layout works and that's what "git repack" does anyway.
So I think we should just remove the whole "--unpacked=" thing, and instead replace it with a "--keep" flag - and then only finding things in the keep packs if we have a list of them.
That would (a) make the logic a whole lot easier to follow and (b) get rid of the scalability issue, since you're not really supposed to have more than one or two .keep files anyway (if that).
Nobody uses "--unpacked=xyzzy" by hand anyway. The only thing that generates those things is git-repack.sh, so this is not a compatibility issue, I suspect.
Linus
cover "git repack -a -d" improvements
This is series removes the linear search in "ignore_packed" list when "git repack -a -d" is run to consolidate many packfiles that are not marked to be kept with ".keep". It is oversplit for easier review; in the final form, I think it should be two patch series, 1/6 and the rest.
The current mechanism is to pass the name of packfiles that are not marked to be kept with --unpacked= command line option. This list is used to a few internal functions to pretend as if the objects found in named packs exist in loose form and subject to repacking. This look-up is linear and found to be very inefficient.
[1/6] is a preparatory and an unrelated bugfix to git-repack script.
[2/6] refactors public interface has_sha1_pack() that takes an optional "ignore_packed" list. Most callers pass NULL, so it introduces a new function has_sha1_kept_pack() and migrate the minority caller to this interface while losing the argument from the original function and callers that currently pass NULL.
[3/6] temporarily makes the the extra argument to has_sha1_kept_pack() function to pass "ignore_packed" list from a list of char* to a pointer to struct rev_info, solely to make the refactoring done in [4/6] easier to follow. Most of the effects of this patch will be removed at the end.
[4/6] identifies three places that use "ignore_packed" list to tell if a pack is on the list or not, and introduces a helper function to do so. The helper is conveniently called is_kept_pack(), even though at this stage the list does not necessarily mean a list of "unkept" packs yet.
[5/6] removes --unpacked=<packfile> parameter, and adds --kept-pack-only option. The sole user of --unpacked=<packfile>, git-repack, is updated to pass this option instead of listing the "unkept" packfiles on the command line.
[6/6] reverts most of the effects of [3/6] made solely for reviewability and removes is_kept_pack() helper function, which now is merely a lookup of a structure member "p->pack_keep".
I think we probably could get rid of --honor-pack-keep mechanism after this series, but I didn't look very deeply into it.
Junio C Hamano (6):
git-repack: resist stray environment variable has_sha1_pack(): refactor "pretend these packs do not exist"
interface
has_sha1_kept_pack(): take "struct rev_info" Consolidate ignore_packed logic more
Simplify is_kept_pack()
is_kept_pack(): final clean-up
builtin-count-objects.c | 2 +- builtin-fsck.c | 2 +- builtin-pack-objects.c | 14 ++-------- builtin-prune-packed.c | 2 +- cache.h | 4 +- diff.c | 2 +- git-repack.sh | 6 ++++- revision.c | 25 +++++++------------ revision.h | 6 +--- sha1_file.c | 60 ++++++++++++++++++++-------------------------- 10 files changed, 51 insertions(+), 72 deletions(-)
PATCH 1/6 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.
--- git-repack.sh | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
git-repack.sh +1 -0
@@ -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$//'`
Signature · 1 line
1.6.2.rc2.99.g9f3bb
PATCH 2/6 has_sha1_pack(): refactor "pretend these packs do not exist" interface
In reply to Junio C HamanoMost 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.
--- 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(-)
builtin-count-objects.c +1 -1
@@ -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)++; } }
builtin-fsck.c +1 -1
@@ -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 ;
builtin-prune-packed.c +1 -1
@@ -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 )
cache.h +2 -1
@@ -565,7 +565,8 @@ extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned lextern 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.c +1 -1
@@ -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);
revision.c +2 -1
@@ -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;
sha1_file.c +25 -7
@@ -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, consreturn 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); }
Signature · 1 line
1.6.2.rc2.99.g9f3bb
PATCH 3/6 has_sha1_kept_pack(): take "struct rev_info"
In reply to Junio C HamanoIts "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.
--- cache.h | 1 - revision.c | 2 +- revision.h | 2 ++ sha1_file.c | 16 +++++++++------- 4 files changed, 12 insertions(+), 9 deletions(-)
cache.h +0 -1
@@ -566,7 +566,6 @@ extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned lextern 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);
revision.c +1 -1
@@ -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;
revision.h +2 -0
@@ -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
sha1_file.c +9 -7
@@ -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)
Signature · 1 line
1.6.2.rc2.99.g9f3bb
PATCH 4/6 Consolidate ignore_packed logic more
In reply to Junio C HamanoThis 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".
--- builtin-pack-objects.c | 12 ++---------- cache.h | 1 - revision.h | 1 + sha1_file.c | 24 ++++++++++++++---------- 4 files changed, 17 insertions(+), 21 deletions(-)
builtin-pack-objects.c +2 -10
@@ -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))
cache.h +0 -1
@@ -747,7 +747,6 @@ extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsignextern 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 );
revision.h +1 -0
@@ -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
sha1_file.c +14 -10
@@ -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++)
Signature · 1 line
1.6.2.rc2.99.g9f3bb
Re: [PATCH 0/6] "git repack -a -d" improvements
In reply to Junio C Hamano* Junio C Hamano <[email protected]> writes:
| [2/6] refactors public interface has_sha1_pack() that takes an optional | "ignore_packed" list. Most callers pass NULL, so it introduces a new | function has_sha1_kept_pack() and migrate the minority caller to this | interface while losing the argument from the original function and callers | that currently pass NULL. [...] | [4/6] identifies three places that use "ignore_packed" list to tell if a | pack is on the list or not, and introduces a helper function to do so. | The helper is conveniently called is_kept_pack(), even though at this | stage the list does not necessarily mean a list of "unkept" packs yet.
OK, patch 2/6 failed for me when I was doing 'git am' to import the patch-series, so sorry if do not see all bits of the patch correctly.
Would it be an improvment to change the signature of the currently find_sha1_pack() function to:
struct packed_git *
find_pack_entry(const unsigned char *sha1, off_t *sha1_pack_offset,
struct packed_git *packs)
- The currently existing 'struct pack_entry *e' parameter is only used to retrn the offset, so make it more clear. The struct pack_entry can probably be deleted from the sha1_file.c file.
- When the 'git repack -a -d' command is used, one has to compute the list of allowed pack-files to look into, and give this list to find_pack_entry().
- The currently named find_sha1_pack() function can then be deleted.
- For example, when this function is now used in sha1_object_info() it can be called like this:
found_pack = find_pack_entry(sha1, &offset, packed_git);
-- kjetil
Re: [PATCH 0/6] "git repack -a -d" improvements
Quoted 2 lines · Kjetil Barvik
OK, patch 2/6 failed for me when I was doing 'git am' to import the patch-series, so sorry if do not see all bits of the patch correctly.
Ah, I should have mentioned that the series was meant to apply to v1.6.0.6 (and merged upwards).
I am off to dentist so I won't have time to think about below until I get back.
Quoted 23 lines
Would it be an improvment to change the signature of the currently
find_sha1_pack()function to:struct
packed_git*
find_pack_entry(const unsigned char *sha1,off_t*sha1_pack_offset,
structpacked_git*packs)- The currently existing 'struct
pack_entry*e' parameter is only used to retrn the offset, so make it more clear. The structpack_entrycan probably be deleted from thesha1_file.cfile.- When the 'git repack -a -d' command is used, one has to compute the list of allowed pack-files to look into, and give this list to
find_pack_entry().- The currently named
find_sha1_pack()function can then be deleted.- For example, when this function is now used in
sha1_object_info()it can be called like this:
found_pack=find_pack_entry(sha1, &offset, packed_git);-- kjetil
PATCH 5/6 Simplify is_kept_pack()
In reply to Junio C HamanoThis removes --unpacked=<packfile> 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=<packfile> list" now says "the packfile has corresponding .keep file".
--- 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(-)
builtin-pack-objects.c +3 -3
@@ -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 ;
git-repack.sh +4 -1
@@ -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
revision.c +5 -15
@@ -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=<packfile> no longer supported." ); }else if (!strcmp (arg,"-r" )) { revs->diff =1 ;DIFF_OPT_SET (&revs->diffopt,RECURSIVE );
revision.h +3 -5
@@ -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
sha1_file.c +3 -27
@@ -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;
Signature · 1 line
1.6.2.rc2.99.g9f3bb
PATCH 6/6 is_kept_pack(): final clean-up
In reply to Junio C HamanoNow 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.
--- 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(-)
builtin-pack-objects.c +2 -2
@@ -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))
cache.h +1 -0
@@ -566,6 +566,7 @@ extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned lextern 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);
revision.c +3 -1
@@ -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;
revision.h +0 -3
@@ -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
sha1_file.c +7 -15
@@ -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)
Signature · 1 line
1.6.2.rc2.99.g9f3bb
A message that is not in the archive (sent off-list or not yet received): 7Vazs5mFk91IKAarOd0wrBNmYj7eSJxVIcR0PEQxJl8R0aQmQDEqSJMphMrXhmVu570fijupQ34@cipher.nrlssc.navy.mil
Re: [PATCH] t7700-repack: repack -a now works properly, expect success from test
Quoted 26 lines · Brandon Casey
Since Junio's keep-pack series was merged in at
aec81306, git-repack now properly packs objects from alternate repositories even when the local repository contains packs.Signed-off-by Brandon Casey <[email protected]>---
Well, I finally got some time last week to take a look at this issue, and what do I find, but you've already fixed it. Did you even realize? :)
I was too busy to notice the emails on the subject last month...
http://article.gmane.org/gmane.comp.version-control.git/111758
Quoted 2 lines · Junio C Hamano
I think we probably could get rid of --honor-pack-keep mechanism after this series, but I didn't look very deeply into it.
This makes sense, but doesn't seem to be working. Possibly the way
--kept-pack-only is implemented, it only deals with commits. So if the
commit object is _not_ in a kept pack, but the trees and/or blobs that it references _are_, then both the commit and the trees/blobs will be added to the list of objects to pack even though the trees/blobs should not be.
Quoted 8 lines
The test in t7700 is contrived, and creates two pack files; one marked with .keep which contains a single blob object that is missing from the other. Then the test repacks and checks whether the blob object was placed into the new pack. The --honor-pack-keep mechanism handles this case since it works on the back end and checks whether each object that is added to the objects list exists in a kept pack, and skips it if so.
Disclaimer: I have not taken the time to fully understand
revision.c
Hmm, I think there are a couple of new issues.
1) The --kept-pack-only mechanism does not operate solely on "local" packs now. This means that objects residing in an alternate pack which has a .keep file will not be repacked with repack -a. My current opinion is that .keep files in an alternate object database should not be honored since the user may not have any control over them. This seems to be in line with your statement about this feature affecting locally existing packs from the Release Notes:
git-gc spent excessive amount of time to decide if an object appears in a locally existing pack
2) The 'repack unpacked objects' and 'loosen unpacked objects' mechanisms now do not operate solely on local packs. I think this means that objects residing in alternate not-kept packs will be repacked when '-A -d' is used, and will be loosened when '-a -d' is used.
I have a test for #1 above. I'll think about a test for #2.
I wonder if we'll ever be interested in non-local keep files? If not, then why not set pack_keep _only_ for local packs?
sha1_file.c +5 -3
@@ -881,9 +881,11 @@ struct packed_git *add_packed_git(const char *path, int pat }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 ; +if (local) { +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)) {
I'd also change the name from pack_keep to something that identivies it as local.
The alternative would require that find_pack_ent() differentiate between local kept packs and alternate kept packs.
-brandon
cover repack improvements
As I mentioned in a previous email, the recent changes to pack-objects/rev-list partially fixed one repacking flaw, but introduced 2 others. There were actually two emails, but the first one never made it to the list.
There existed a flaw that prevented repack from properly repacking objects contained in packs from alternate repositories if the local repository contained any non-kept packs. This was fixed by the --kept-pack-only feature, in exchange for not being able to pack objects contained in alternate packs which had a keep file. Since the user may not have control over the referenced object database, I think it is fair not to the honor the keep files in that case.
Additionally, if repack told pack-objects to loosen unreferenced objects, it would even loosen objects residing in alternate packs. They would then be immediately deleted by the call to prune-packed though.
The solution I chose is actually to fall back to the --honor-pack-keep option and to stop using the --kept-pack-only option. pack-objects already iterates over every object added for packing and searches the pack array to check whether it is already packed, so for the case where no keep files are used, there is actually one less iteration through the pack array for each commit. But one more (?) for each object referenced by a commit that is packed in a pack with a .keep file.
So this keeps the local and keep_pack decision making in pack-objects. The other way would be to move the --local option over into rev-list and teach the pack lookup machinery to differentiate between local and non-local packs. Not sure which is better, but this was easier.
I think there is still one case where an object will be loosened unnecessarily. If an unreferenced object exists in a local pack _and_ in an alternate object database, then I believe it will be loosened and then pruned. To fix it, loosen_unused_packed_objects and add_objects_in_unpacked_packs would have to iterate through the non-local packs to check whether they contained the object before loosening/repacking it. Probably won't happen often in practice though.
Also, I think the --kept-pack-only option is now not necessary. The last patch removes the --kept-pack-only feature /ducks and can be dropped if you think this feature may be useful in the future.
Brandon Casey (5):
t7700-repack: add two new tests demonstrating repacking flaws git-repack.sh: don't use --kept-pack-only option to pack-objects
pack-objects: only repack or loosen objects residing in "local" packs
t7700-repack: repack -a now works properly, expect success from test Remove --kept-pack-only option and associated infrastructure
builtin-pack-objects.c | 5 ++--- cache.h | 1 - git-repack.sh | 6 +----- revision.c | 9 +-------- revision.h | 1 - sha1_file.c | 21 +-------------------- t/t7700-repack.sh | 46 +++++++++++++++++++++++++++++++++++++++++++++- 7 files changed, 50 insertions(+), 39 deletions(-)
Re: [PATCH 0/5] repack improvements
Sorry, I forgot the sign-off on these patches. If you decide to apply, please add it or tell me to resend.
thanks,
-brandon
PATCH 1/5 t7700-repack: add two new tests demonstrating repacking flaws
In reply to Brandon Casey1) 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.
--- t/t7700-repack.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 44 insertions(+), 0 deletions(-)
t/t7700-repack.sh +44 -0
@@ -88,5 +88,49 @@ test_expect_failure 'packed obs in alt ODB are repacked when local repo has pack 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 </dev/null pack && + rm -f .git/objects/pack/* && + mv pack-* .git/objects/pack/ && + test 0 = $(git verify-pack -v -- .git/objects/pack/*.idx | + egrep "^$csha1 " | sort | uniq | wc -l) && + echo > .git/objects/info/alternates && + test_must_fail git show $csha1 +' + test_done
Signature · 1 line
1.6.2.16.geb16e
PATCH 2/5 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.
--- git-repack.sh | 6 +----- t/t7700-repack.sh | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-)
git-repack.sh +1 -5
@@ -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"
t/t7700-repack.sh +1 -1
@@ -88,7 +88,7 @@ test_expect_failure 'packed obs in alt ODB are repacked when local repo has pack 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
Signature · 1 line
1.6.2.16.geb16e
PATCH 3/5 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.
--- builtin-pack-objects.c | 4 ++-- t/t7700-repack.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
builtin-pack-objects.c +2 -2
@@ -1966,7 +1966,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" );
@@ -2002,7 +2002,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))
t/t7700-repack.sh +1 -1
@@ -113,7 +113,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}) &&
Signature · 1 line
1.6.2.16.geb16e
PATCH 4/5 t7700-repack: repack -a now works properly, expect success from test
Thanks to Junio's rework of the --unpacked= option of pack-objects/rev-list git-repack now properly packs objects from alternate repositories even when the local repository contains packs.
--- t/t7700-repack.sh | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
t/t7700-repack.sh +1 -1
@@ -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 alt ODB are repacked when local repo has packs' ' +test_expect_success 'packed obs in alt ODB are repacked when local repo has packs' ' rm -f .git/objects/pack/* && echo new_content >> file1 && git add file1 &&
Signature · 1 line
1.6.2.16.geb16e
PATCH 5/5 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.
--- 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(-)
builtin-pack-objects.c +0 -1
@@ -2200,7 +2200,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 ;
cache.h +0 -1
@@ -646,7 +646,6 @@ extern int check_sha1_signature(const unsigned char *sha1, void *buf, unsigned lextern 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);
revision.c +1 -8
@@ -1106,10 +1106,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=<packfile> no longer supported." ); }else if (!strcmp (arg,"-r" )) {
@@ -1679,10 +1675,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;
revision.h +0 -1
@@ -50,7 +50,6 @@ struct rev_info { edge_hint:1, limited:1, unpacked:1, - kept_pack_only:1, boundary:2, left_right:1, rewrite_parents:1,
sha1_file.c +1 -20
@@ -1919,8 +1919,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;
@@ -1932,8 +1931,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++)
@@ -1973,16 +1970,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) {
@@ -2456,12 +2443,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;
Signature · 1 line
1.6.2.16.geb16e