757c6ee7a3
When an end-user runs "git push" with an empty string for the remote
repository name, e.g.
$ git push '' main
"git push" fails with a BUG(). Even though this is a nonsense request
that we want to fail, we shouldn't hit a BUG(). Instead we want to give
a sensible error message, e.g., 'bad repository'".
This is because since 9badf97c42 (remote: allow resetting url list,
2024-06-14), we reset the remote URL if the provided URL is empty. When
a user of 'remotes_remote_get' tries to fetch a remote with an empty
repo name, the function initializes the remote via 'make_remote'. But
the remote is still not a valid remote, since the URL is empty, so it
tries to add the URL alias using 'add_url_alias'. This in-turn will call
'add_url', but since the URL is empty we call 'strvec_clear' on the
`remote->url`. Back in 'remotes_remote_get', we again check if the
remote is valid, which fails, so we return 'NULL' for the 'struct
remote *' value.
The 'builtin/push.c' code, calls 'set_refspecs' before validating the
remote. This worked with empty repo names earlier since we would get a
remote, albeit with an empty URL. With the new changes, we get a 'NULL'
remote value, this causes the check for remote to fail and raises the
BUG in 'set_refspecs'.
Do a simple fix by doing remote validation first. Also add a test to
validate the bug fix. With this, we can also now directly pass remote to
'set_refspecs' instead of it trying to lazily obtain it.
Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
68 lines
1.7 KiB
Bash
Executable File
68 lines
1.7 KiB
Bash
Executable File
#!/bin/sh
|
|
|
|
test_description='detect some push errors early (before contacting remote)'
|
|
|
|
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
|
|
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
|
|
|
|
TEST_PASSES_SANITIZE_LEAK=true
|
|
. ./test-lib.sh
|
|
|
|
test_expect_success 'setup commits' '
|
|
test_commit one
|
|
'
|
|
|
|
test_expect_success 'setup remote' '
|
|
git init --bare remote.git &&
|
|
git remote add origin remote.git
|
|
'
|
|
|
|
test_expect_success 'setup fake receive-pack' '
|
|
FAKE_RP_ROOT=$(pwd) &&
|
|
export FAKE_RP_ROOT &&
|
|
write_script fake-rp <<-\EOF &&
|
|
echo yes >"$FAKE_RP_ROOT"/rp-ran
|
|
exit 1
|
|
EOF
|
|
git config remote.origin.receivepack "\"\$FAKE_RP_ROOT/fake-rp\""
|
|
'
|
|
|
|
test_expect_success 'detect missing branches early' '
|
|
echo no >rp-ran &&
|
|
echo no >expect &&
|
|
test_must_fail git push origin missing &&
|
|
test_cmp expect rp-ran
|
|
'
|
|
|
|
test_expect_success 'detect missing sha1 expressions early' '
|
|
echo no >rp-ran &&
|
|
echo no >expect &&
|
|
test_must_fail git push origin main~2:main &&
|
|
test_cmp expect rp-ran
|
|
'
|
|
|
|
# We use an existing local_ref, since it follows a different flow in
|
|
# 'builtin/push.c:set_refspecs()' and we want to test that regression.
|
|
test_expect_success 'detect empty remote with existing local ref' '
|
|
test_must_fail git push "" main 2> stderr &&
|
|
grep "fatal: bad repository ${SQ}${SQ}" stderr
|
|
'
|
|
|
|
# While similar to the previous test, here we want to ensure that
|
|
# even targeted refspecs are handled.
|
|
test_expect_success 'detect empty remote with targeted refspec' '
|
|
test_must_fail git push "" HEAD:refs/heads/main 2> stderr &&
|
|
grep "fatal: bad repository ${SQ}${SQ}" stderr
|
|
'
|
|
|
|
test_expect_success 'detect ambiguous refs early' '
|
|
git branch foo &&
|
|
git tag foo &&
|
|
echo no >rp-ran &&
|
|
echo no >expect &&
|
|
test_must_fail git push origin foo &&
|
|
test_cmp expect rp-ran
|
|
'
|
|
|
|
test_done
|