[PATCH 3/3] pack-objects: honor '.keep' files

[drafnel@gmail.com]

From: Brandon Casey <drafnel@gmail.com>

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.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---


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