Re: [PATCH 1/1] Git.pm: add semicolon after catch statement

[Michael McClimon <michael@mcclimon.org>]

> Yeah, this test is particularly confusing because unlike most of our
> suite, it drives the test harness using a separate perl script. So you
> have setup in one file and tests in another.

Oh good, I'm glad it wasn't just me; this was very helpful, thanks.

> 
> But curiously this still does not pass after your patch, because we seem
> to actually open the repository! I think this is because the C code
> allows an explicit GIT_DIR to override the safe-directory checks. But in
> this case that GIT_DIR is set by Git.pm searching for the directory, not
> because the user desires it.

Aha; this gets to what I was saying in the cover letter, which is that my
patch only fixes the runtime error from perl, and Git.pm happily carries on in
an unsafe directory. Fixing _that_ is probably beyond my knowledge!

> So your patch is definitely still the right thing to do, but it feels
> like a hole in the safe-directory mechanism, at least when called via
> Git.pm. +cc folks who worked on that.

If we wanted a test for just the runtime error, something like the following
(including your earlier suggestion to set up the bare repo) demonstrates the
bug and my fix. It doesn't seem like a particularly valuable test on its own,
IMO, but I'm happy to reroll if we want it. Thanks!

diff --git a/t/t9700/test.pl b/t/t9700/test.pl
index e046f7db..72681849 100755
--- a/t/t9700/test.pl
+++ b/t/t9700/test.pl
@@ -30,6 +30,11 @@ sub adjust_dirsep {
 # set up
 our $abs_repo_dir = cwd();
 ok(our $r = Git->repository(Directory => "."), "open repository");
+{
+       local $ENV{GIT_TEST_ASSUME_DIFFERENT_OWNER} = 1;
+       eval { Git->repository(Directory => "$abs_repo_dir/bare.git") };
+       unlike($@, qr/as a HASH ref/i, "no error from perl");
+}

 # config
 is($r->config("test.string"), "value", "config scalar: string");

-- 
Michael McClimon
michael@mcclimon.org