mirror of
https://github.com/git-for-windows/git.git
synced 2026-06-21 20:20:30 -05:00
54a441bcea
If a .git file is well-formed but points to a directory that is not
itself a valid repository, then we say:
fatal: not a git repository: <pointed-to-repo>
without mentioning the .git file that pointed us there in the first
place. Doing so could better help the user understand the source of the
problem.
In theory the most helpful thing we could do is mention both paths,
like:
gitfile '<gitfile>' points to invalid repository: <pointed-to-repo>
But there's another catch: when we generate the error, we don't always
know the pointed-to repository! This leads to a potential segfault.
The message comes from read_gitfile_error_die(). Originally we only
called that function from inside read_gitfile_gently(), passing in both
the gitfile path and the pointed-to path. But that changed in 1dd27bfbfd
(setup: improve error diagnosis for invalid .git files, 2026-03-04).
Since then, the caller in setup_git_directory_gently(), even if it wants
to die on error, always passes in the "return_error_code" flag, asking
the function to instead return a numeric error code. And then it calls
read_gitfile_error_die() itself, passing NULL for the pointed-to path.
If we get the READ_GITFILE_ERR_NOT_A_REPO code, we form a message using
that NULL pointer, and either segfault or get garbage like "not a git
repository: (null)", depending on the platform.
We could fix this by having the function pass out both the numeric error
code and the pointed-to path. But that creates a new headache: we have
to allocate that string on the heap and pass ownership back to the
caller. So now every caller has to be aware of it (and either free the
result, or signal that they are not interested by using an extra
parameter).
Instead, let's just drop the pointed-to path from the error message
entirely, and mention only the gitfile. This fixes the NULL dereference
without introducing any more complexity. The user-facing error message
is not as detailed as it could be, but is better than the original.
Since it mentions the gitfile, a user investigating the situation can
look there to find the pointed-to path (whereas you could not go the
other way from the original message).
There's an existing test in t0002 which triggers this case, but we
didn't notice the problem because it checks only that we said "not a
repository", and not the full string. So if we print "(null)" it is
happy. It will probably crash on some non-glibc platforms, but nobody
seems to have reported it yet (the breakage is recent-ish as of v2.54).
I'm also somewhat surprised that building with ASan/UBSan doesn't catch
this, but it doesn't seem to (and I found an open issue with somebody
asking for NULL printf checks to be implemented in the sanitizers).
We'll tweak the test to match the new error, but there's no need to beef
it up further, since we're not showing the pointed-to path at all.
We also racily trigger this in t7450. During parallel cloning we might
see one of several errors, including this one. And so we must update
that message, too (you can otherwise find the failure pretty quickly by
running t7450 with --stress).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
139 lines
3.1 KiB
Bash
Executable File
139 lines
3.1 KiB
Bash
Executable File
#!/bin/sh
|
|
|
|
test_description='.git file
|
|
|
|
Verify that plumbing commands work when .git is a file
|
|
'
|
|
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
|
|
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
|
|
|
|
. ./test-lib.sh
|
|
|
|
objpath() {
|
|
echo "$1" | sed -e 's|\(..\)|\1/|'
|
|
}
|
|
|
|
test_expect_success 'initial setup' '
|
|
REAL="$(pwd)/.real" &&
|
|
mv .git "$REAL"
|
|
'
|
|
|
|
test_expect_success 'bad setup: invalid .git file format' '
|
|
echo "gitdir $REAL" >.git &&
|
|
test_must_fail git rev-parse 2>.err &&
|
|
test_grep "invalid gitfile format" .err
|
|
'
|
|
|
|
test_expect_success 'bad setup: invalid .git file path' '
|
|
echo "gitdir: $REAL.not" >.git &&
|
|
test_must_fail git rev-parse 2>.err &&
|
|
test_grep "gitfile does not point to a valid repository" .err
|
|
'
|
|
|
|
test_expect_success 'final setup + check rev-parse --git-dir' '
|
|
echo "gitdir: $REAL" >.git &&
|
|
echo "$REAL" >expect &&
|
|
git rev-parse --git-dir >actual &&
|
|
test_cmp expect actual
|
|
'
|
|
|
|
test_expect_success 'check hash-object' '
|
|
echo "foo" >bar &&
|
|
SHA=$(git hash-object -w --stdin <bar) &&
|
|
test_path_is_file "$REAL/objects/$(objpath $SHA)"
|
|
'
|
|
|
|
test_expect_success 'check cat-file' '
|
|
git cat-file blob $SHA >actual &&
|
|
test_cmp bar actual
|
|
'
|
|
|
|
test_expect_success 'check update-index' '
|
|
test_path_is_missing "$REAL/index" &&
|
|
rm -f "$REAL/objects/$(objpath $SHA)" &&
|
|
git update-index --add bar &&
|
|
test_path_is_file "$REAL/index" &&
|
|
test_path_is_file "$REAL/objects/$(objpath $SHA)"
|
|
'
|
|
|
|
test_expect_success 'check write-tree' '
|
|
SHA=$(git write-tree) &&
|
|
test_path_is_file "$REAL/objects/$(objpath $SHA)"
|
|
'
|
|
|
|
test_expect_success 'check commit-tree' '
|
|
SHA=$(echo "commit bar" | git commit-tree $SHA) &&
|
|
test_path_is_file "$REAL/objects/$(objpath $SHA)"
|
|
'
|
|
|
|
test_expect_success 'check rev-list' '
|
|
git update-ref "HEAD" "$SHA" &&
|
|
git rev-list HEAD >actual &&
|
|
echo $SHA >expected &&
|
|
test_cmp expected actual
|
|
'
|
|
|
|
test_expect_success 'setup_git_dir twice in subdir' '
|
|
git init sgd &&
|
|
(
|
|
cd sgd &&
|
|
git config alias.lsfi ls-files &&
|
|
mv .git .realgit &&
|
|
echo "gitdir: .realgit" >.git &&
|
|
mkdir subdir &&
|
|
cd subdir &&
|
|
>foo &&
|
|
git add foo &&
|
|
git lsfi >actual &&
|
|
echo foo >expected &&
|
|
test_cmp expected actual
|
|
)
|
|
'
|
|
|
|
test_expect_success 'enter_repo non-strict mode' '
|
|
test_create_repo enter_repo &&
|
|
(
|
|
cd enter_repo &&
|
|
test_tick &&
|
|
test_commit foo &&
|
|
mv .git .realgit &&
|
|
echo "gitdir: .realgit" >.git
|
|
) &&
|
|
head=$(git -C enter_repo rev-parse HEAD) &&
|
|
git ls-remote enter_repo >actual &&
|
|
cat >expected <<-EOF &&
|
|
$head HEAD
|
|
$head refs/heads/main
|
|
$head refs/tags/foo
|
|
EOF
|
|
test_cmp expected actual
|
|
'
|
|
|
|
test_expect_success 'enter_repo linked checkout' '
|
|
(
|
|
cd enter_repo &&
|
|
git worktree add ../foo refs/tags/foo
|
|
) &&
|
|
head=$(git -C enter_repo rev-parse HEAD) &&
|
|
git ls-remote foo >actual &&
|
|
cat >expected <<-EOF &&
|
|
$head HEAD
|
|
$head refs/heads/main
|
|
$head refs/tags/foo
|
|
EOF
|
|
test_cmp expected actual
|
|
'
|
|
|
|
test_expect_success 'enter_repo strict mode' '
|
|
head=$(git -C enter_repo rev-parse HEAD) &&
|
|
git ls-remote --upload-pack="git upload-pack --strict" foo/.git >actual &&
|
|
cat >expected <<-EOF &&
|
|
$head HEAD
|
|
$head refs/heads/main
|
|
$head refs/tags/foo
|
|
EOF
|
|
test_cmp expected actual
|
|
'
|
|
|
|
test_done
|