drafnel@xxxxxxxxx wrote: > From: Brandon Casey <drafnel@xxxxxxxxx> > > By default, pack-objects creates a pack file with every object specified by > the user. There are two options which can be used to exclude objects which > are accessible by the repository. > > 1) --incremental > This excludes any object which already exists in an accessible pack. > > 2) --local > This excludes any object which exists in a non-local pack. > > With this patch, both arguments also cause objects which exist in packs > marked with a .keep file to be excluded. Only the --local option requires > an explicit check for the .keep file. If the user doesn't want the objects > in a pack marked with .keep to be exclude, then the .keep file should be > removed. > > Additionally, this fixes the repack bug which allowed porcelain repack to > create packs which contained objects already contained in existing packs > marked with a .keep file. This looks sane to me, but I'm holding off on the ack because I don't like the way the flags are managed in struct packed_git. (See my reply to 1/3 in the series.) > Signed-off-by: Brandon Casey <drafnel@xxxxxxxxx> > --- > > > This seems to be the correct fix. > > I thought about two alternatives: > > 1) New option to pack-objects which causes it to honor .keep files > > I didn't think this was necessary since that is why we have .keep > files in the first place. > > So, now there are three variants: > > Neither --incremental or --local is used) > Ignore repo packs, pack is created using all objects specified by > user. > --incremental is used) > Look at repo packs, exclude already packed objects (including those > marked with .keep file) > --local is used) > Look at repo packs, exclude objects in non-local packs (including local > objects marked with .keep file) > > 2) Always honor .keep files, even when --incremental or --local is > not specified. > > I think this may be counter-intuitive, since if you specify all of > the objects explicitly, without using any other options, you expect > all of those objects to be placed into the created pack file. If > .keep is always honored, then a new option would be required to > indicate ignoring .keep files, or we would adopt the platform that > "the user must just remove the .keep file". > > > So, honoring .keep files only with --incremental (which is a side effect) or > --local seems to be the most appropriate. > > Comments welcome. > > -brandon > > > builtin-pack-objects.c | 2 +- > t/t7700-repack.sh | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c > index 6a8b9bf..5199e54 100644 > --- a/builtin-pack-objects.c > +++ b/builtin-pack-objects.c > @@ -701,7 +701,7 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type, > break; > if (incremental) > return 0; > - if (local && !ispacklocal(p)) > + if (local && (!ispacklocal(p) || haspackkeep(p))) > return 0; > } > } > diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh > index 1489e68..b6b02da 100755 > --- a/t/t7700-repack.sh > +++ b/t/t7700-repack.sh > @@ -4,7 +4,7 @@ test_description='git repack works correctly' > > . ./test-lib.sh > > -test_expect_failure 'objects in packs marked .keep are not repacked' ' > +test_expect_success 'objects in packs marked .keep are not repacked' ' > echo content1 > file1 && > echo content2 > file2 && > git add . && > -- > 1.6.0.2.588.g3102 > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Shawn. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
