1
0
Fork 0
mirror of https://github.com/git/git.git synced 2024-05-10 09:36:28 +02:00
git/t/t5801/git-remote-testgit
Jeff King b5b7b17b2e transport-helper: send "true" value for object-format option
The documentation in gitremote-helpers.txt claims that after a helper
has advertised the "object-format" capability, Git may then send "option
object-format true" to indicate that it would like to hear which object
format the helper is using when it returns refs.

However, the code implementing this has always written just "option
object-format", without the extra "true" value. Nobody noticed in
practice or in the tests because the only two helpers we ship are:

  - remote-curl, which quietly converts missing values into "true". This
    goes all the way back to ef08ef9ea0 (remote-helpers: Support custom
    transport options, 2009-10-30), despite the fact that I don't think
    any other option has ever made use of it.

  - remote-testgit in t5801 does insist on having a "true" value. But
    since it sends the ":object-format" response regardless of whether
    it thinks the caller asked for it (technically breaking protocol),
    everything just works, albeit with an extra shell error:

      .../git/t/t5801/git-remote-testgit: 150: test: =: unexpected operator

    printed to stderr, which you can see running t5801 with --verbose.
    (The problem is that $val is the empty string, and since we don't
    double-quote it in "test $val = true", we invoke "test = true"
    instead).

When the documentation and code do not match, it is often good to fix
the documentation rather than break compatibility. And in this case, we
have had the mis-match since 8b85ee4f47 (transport-helper: implement
object-format extensions, 2020-05-25). However, the sha256 feature was
listed as experimental until 8e42eb0e9a (doc: sha256 is no longer
experimental, 2023-07-31).

It's possible there are some third party helpers that tried to follow
the documentation, and are broken. Changing the code will fix them. It's
also possible that there are ones that follow the code and will be
broken if we change it. I suspect neither is the case given that no
helper authors have brought this up as an issue (I only noticed it
because I was running t5801 in verbose mode for other reasons and
wondered about the weird shell error). That, coupled with the relative
new-ness of sha256, makes me think nobody has really worked on helpers
for it yet, which gives us an opportunity to correct the code before too
much time passes.

And doing so has some value: it brings "object-format" in line with the
syntax of other options, making the protocol more consistent. It also
lets us use set_helper_option(), which has better error reporting.

Note that we don't really need to allow any other values like "false"
here. The point is for Git to tell the helper that it understands
":object-format" lines coming back as part of the ref listing. There's
no point in future versions saying "no, I don't understand that".

To make sure everything works as expected, we can improve the
remote-testgit helper from t5801 to send the ":object-format" line only
if the other side correctly asked for it (which modern Git will always
do). With that test change and without the matching code fix here, t5801
will fail when run with GIT_TEST_DEFAULT_HASH=sha256.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2024-03-20 10:01:30 -07:00

165 lines
3.3 KiB
Bash
Executable File

#!/bin/sh
# Copyright (c) 2012 Felipe Contreras
# The first argument can be a url when the fetch/push command was a url
# instead of a configured remote. In this case, use a generic alias.
if test "$1" = "testgit::$2"; then
alias=_
else
alias=$1
fi
url=$2
dir="$GIT_DIR/testgit/$alias"
if ! git rev-parse --is-inside-git-dir
then
exit 1
fi
h_refspec="refs/heads/*:refs/testgit/$alias/heads/*"
t_refspec="refs/tags/*:refs/testgit/$alias/tags/*"
if test -n "$GIT_REMOTE_TESTGIT_NOREFSPEC"
then
h_refspec=""
t_refspec=""
fi
GIT_DIR="$url/.git"
export GIT_DIR
force=
object_format=
mkdir -p "$dir"
if test -z "$GIT_REMOTE_TESTGIT_NO_MARKS"
then
gitmarks="$dir/git.marks"
testgitmarks="$dir/testgit.marks"
test -e "$gitmarks" || >"$gitmarks"
test -e "$testgitmarks" || >"$testgitmarks"
fi
while read line
do
case $line in
capabilities)
echo 'import'
echo 'export'
test -n "$h_refspec" && echo "refspec $h_refspec"
test -n "$t_refspec" && echo "refspec $t_refspec"
if test -n "$gitmarks"
then
echo "*import-marks $gitmarks"
echo "*export-marks $gitmarks"
fi
test -n "$GIT_REMOTE_TESTGIT_SIGNED_TAGS" && echo "signed-tags"
test -n "$GIT_REMOTE_TESTGIT_NO_PRIVATE_UPDATE" && echo "no-private-update"
echo 'option'
echo 'object-format'
echo
;;
list)
test -n "$object_format" &&
echo ":object-format $(git rev-parse --show-object-format=storage)"
git for-each-ref --format='? %(refname)' 'refs/heads/' 'refs/tags/'
head=$(git symbolic-ref HEAD)
echo "@$head HEAD"
echo
;;
import*)
# read all import lines
while true
do
ref="${line#* }"
refs="$refs $ref"
read line
test "${line%% *}" != "import" && break
done
if test -n "$gitmarks"
then
echo "feature import-marks=$gitmarks"
echo "feature export-marks=$gitmarks"
fi
if test -n "$GIT_REMOTE_TESTGIT_FAILURE"
then
echo "feature done"
exit 1
fi
echo "feature done"
git fast-export \
${h_refspec:+"--refspec=$h_refspec"} \
${t_refspec:+"--refspec=$t_refspec"} \
${testgitmarks:+"--import-marks=$testgitmarks"} \
${testgitmarks:+"--export-marks=$testgitmarks"} \
$refs
echo "done"
;;
export)
if test -n "$GIT_REMOTE_TESTGIT_FAILURE"
then
# consume input so fast-export doesn't get SIGPIPE;
# git would also notice that case, but we want
# to make sure we are exercising the later
# error checks
while read line; do
test "done" = "$line" && break
done
exit 1
fi
before=$(git for-each-ref --format=' %(refname) %(objectname) ')
git fast-import \
${force:+--force} \
${testgitmarks:+"--import-marks=$testgitmarks"} \
${testgitmarks:+"--export-marks=$testgitmarks"} \
--quiet
# figure out which refs were updated
git for-each-ref --format='%(refname) %(objectname)' |
while read ref a
do
case "$before" in
*" $ref $a "*)
continue ;; # unchanged
esac
if test -z "$GIT_REMOTE_TESTGIT_PUSH_ERROR"
then
echo "ok $ref"
else
echo "error $ref $GIT_REMOTE_TESTGIT_PUSH_ERROR"
fi
done
echo
;;
option\ *)
read cmd opt val <<-EOF
$line
EOF
case $opt in
force)
test $val = "true" && force="true" || force=
echo "ok"
;;
object-format)
test $val = "true" && object_format="true" || object_format=
echo "ok"
;;
*)
echo "unsupported"
;;
esac
;;
'')
exit
;;
esac
done