Thread (6 messages) flat view 6 messages, 3 authors, 2024-09-13
STALE701d

[PATCH 0/2] fix bare repositories with Git.pm

From: Jeff King <hidden>
Date: 2024-09-12 22:34:15

On Thu, Sep 12, 2024 at 06:32:00PM +0200, Rodrigo wrote:
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.
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...
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.
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help