[PATCH 0/2] fix bare repositories with Git.pm
- From
Jeff King <peff@peff.net>
- Date
- Sep 12, 2024, 22:34 UTC
- Message-ID
- <20240912223413.GA649897@coredump.intra.peff.net>
- In-Reply-To
- <CAGUZU_JZd_+8y19=kGif6u1+4n_+iOcVWV4p-kC0Uo=8Ev=aBA@mail.gmail.com>
On Thu, Sep 12, 2024 at 06:32:00PM +0200, Rodrigo wrote:
Show 18 quoted lines
> We're having an issue migrating from 2.31.1 to 2.46.0. The following
> Perl code does not work in 2.46.0 as it did in 2.31.1 (tested linux
> and darwin, did not check Windows):
>
> # test.pl
> use Git;
> my $repo = Git->repository( Directory => '/repo/bare.git' );
> my ($fh, $ctx) = $repo->command_output_pipe('rev-list', "2acf3456");
> print do { local $/; <$fh> };
>
> Run:
>
> $ cd /home/rodrigo
> $ perl test.pl
>
> Fails with the error:
>
> fatal: not a git repository: '/home/rodrigo'Yikes, good catch. That's a pretty bad bug. I'm surprised we didn't cover this in the tests, but it's specific to bare repositories.
Show 5 quoted lines
> Bug hunting through the Git.pm code and skimming through the Git SCM > repo, there's a significant change (commit 20da61f25) that makes the > recent Git.pm rely on: > > git rev-parse --is-bare-repository --git-dir
Yep, I confirmed via bisection that that commit is the culprit. Your analysis is mostly good, though...
Show 6 quoted lines
> for locating the correct (maybe a parent) repo directory. The change > was understandably done for security (and other many) reasons. It > started using --is-bare-repository to detect if it's a bare repository > we're dealing with, instead of relying on the old Git.pm redundant > code for bare repo detection, which was a sound decision. But some > crucial code was taken out.
...the problem is actually that not enough code was taken out. ;) I left the code in the bare-only code to turn the relative path to absolute, but it used the wrong path (the one returned by rev-parse, rather than the original Directory option that was passed in). That bare-only path should just go away entirely, and both should use the same (correct) code to get the absolute path.
Show 8 quoted lines
> SOLUTION: I propose using "--absolute-git-dir" instead of "--git-dir": > > git rev-parse --is-bare-repository --absolute-git-dir > > Which will replace the `.` rev-parse response with a full path > resolved by git itself (and not Perl). This means the change to the > Perl code is minimal. I don't know if this will resolve all possible > cases, but it does fix our issue.
It does fix all cases, but it leaves some redundant code in place. Here are two patches. The first does the minimal fix within the code (what 20da61f25 should have done!) and corrects the problem. The second switches to --absolute-git-dir and drops the now-redundant code.
Thank you for a very thorough bug report!
[1/2]: Git.pm: fix bare repository search with Directory option [2/2]: Git.pm: use "rev-parse --absolute-git-dir" rather than perl code
perl/Git.pm | 14 ++++++-------- t/t9700-perl-git.sh | 3 ++- t/t9700/test.pl | 5 +++++ 3 files changed, 13 insertions(+), 9 deletions(-)
-Peff