[PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'

[PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'

[Brandon Casey]

The '--no-thread' option is a Getopt::Long boolean option. The '--no-'
prefix (as in --no-thread) for boolean options is not supported in
Getopt::Long version 2.32 which was released with Perl 5.8.0. This version
only supports '--no' as in '--nothread'.  More recent versions of
Getopt::Long, such as version 2.34, support either prefix. So use the older
form in the tests.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 t/t9001-send-email.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index d9420e0..ce26ea4 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -616,7 +616,7 @@ test_expect_success 'in-reply-to but no threading' '
 		--from="Example <nobody@example.com>" \
 		--to=nobody@example.com \
 		--in-reply-to="<in-reply-id@example.com>" \
-		--no-thread \
+		--nothread \
 		$patches |
 	grep "In-Reply-To: <in-reply-id@example.com>"
 '
-- 
1.6.2.4.24.gde59d2

[PATCH] t7700-repack: repack -a now works properly, expect success from test

[Brandon Casey]

Since the recent rework of the object listing mechanism of
pack-objects/rev-list, git-repack now properly packs objects from alternate
repositories even when the local repository contains packs.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 t/t7700-repack.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 6b29bff..87c9b0e 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 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 &&
-- 
1.6.2.4.24.gde59d2

Re: [PATCH] t7700-repack: repack -a now works properly, expect success from test

[Brandon Casey]

Brandon Casey wrote:
> 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 <casey@nrlssc.navy.mil>
> ---
> 
> 
> 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
> 
> Junio C Hamano wrote:
>> 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.
> 
> 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?

diff --git a/sha1_file.c b/sha1_file.c
index 4563173..a595eac 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -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