> 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@xxxxxxxxxxxx
