Re: [PATCH v5 1/3] test: add helper functions for git-bundle
flat view
From: Junio C Hamano <hidden>
Date: 2021-01-11 20:10:49
Possibly related (same subject, not in this thread)
- 2021-01-10 · [PATCH v5 1/3] test: add helper functions for git-bundle · Jiang Xin <hidden>
Jiang Xin [off-list ref] writes:
quoted hunk
+# Create a commit or tag and set the variable with the object ID. +test_commit_setvar () { + notick= + signoff= + indir= + merge= + tag= + var= + + while test $# != 0 + do + case "$1" in + --merge) + merge=t + ;; + --tag) + tag=t + ;; + --notick) + notick=t + ;; + --signoff) + signoff="$1" + ;; + -C) + shift + indir="$1" + ;; + -*) + echo >&2 "error: unknown option $1" + return 1 + ;; + *) + break + ;; + esac + shift + done + + var=$1 + shift + if test -z "$var" + then + echo >&2 "error: var is not defined" + return 1 + fi
We need to check $# immediately after the loop to ensure that we can
carve out $var and at least another arg. [*Nit 1*]
The previous round required the command line to have at least one
after the loop (including parsing of $var) parsed it, but now we
fall through from here when a command line were:
test_commit_setvar --merge -C there VAR
and because "$1" does not exist, such an error is propagated down to
"git merge" not getting the side branch, "git tag" not getting the
object to tag, etc.
quoted hunk
+ indir=${indir:+"$indir"/} + if test -z "$notick" + then + test_tick + fi && + if test -n "$merge" + then + git ${indir:+ -C "$indir"} merge --no-edit --no-ff \ + ${2:+-m "$2"} "$1" && + oid=$(git ${indir:+ -C "$indir"} rev-parse HEAD) + elif test -n "$tag" + then + git ${indir:+ -C "$indir"} tag -m "$1" "$1" && + oid=$(git ${indir:+ -C "$indir"} rev-parse "$1") + else + file=${2:-"$1.t"} && + echo "${3-$1}" > "$indir$file" &&
Style? [*Nit 2*]
quoted hunk
+ git ${indir:+ -C "$indir"} add "$file" && + git ${indir:+ -C "$indir"} commit $signoff -m "$1" && + oid=$(git ${indir:+ -C "$indir"} rev-parse HEAD) + fi && + eval $var=$oid +}
quoted hunk
+ + +# Format the output of git commands to make a user-friendly and stable +# text. We can easily prepare the expect text without having to worry +# about future changes of the commit ID and spaces of the output. +make_user_friendly_and_stable_output () { + sed \ + -e "s/$(echo $A | cut -c1-7)[0-9a-f]*/<COMMIT-A>/g" \
Is "$(echo $A | cut -c1-7)" the same as "${A%${A#???????}}"? If so,
the latter may be a bit shorter.
quoted hunk
diff --git a/t/test-bundle-functions.sh b/t/test-bundle-functions.sh new file mode 100644 index 0000000000..0853eb1eca --- /dev/null +++ b/t/test-bundle-functions.sh@@ -0,0 +1,47 @@ +# Library of git-bundle related functions. + +# Display the pack data contained in the bundle file, bypassing the +# header that contains the signature, prerequisites and references. +convert_bundle_to_pack () { + while read x && test -n "$x" + do + :; + done + cat +} + +# Check count of objects in a bundle file. +# We can use "--thin" opiton to check thin pack, which must be fixed by +# command `git-index-pack --fix-thin --stdin`. +test_bundle_object_count () { + thin= + if test "$1" = "--thin" + then + thin=t + shift + fi + if test $# -ne 2 + then + echo >&2 "args should be: <bundle> <count>" + return 1 + fi + bundle=$1 + pack=$bundle.pack + convert_bundle_to_pack <"$bundle" >"$pack" && + if test -n "$thin" + then + mv "$pack" "$bundle.thin.pack" && + git index-pack --stdin --fix-thin "$pack" <"$bundle.thin.pack" + else + git index-pack "$pack" + fi
I wonder why we shouldn't always do "--fix-thin", so that the caller does not even have to bother passing the "--thin" option. Is this to protect us from "git bundle" creating a bundle that contains a thin pack when it should not? A caller that knows it is storing a fully connected history can deliberately omit "--thin" from the command line and make sure "index-pack" that is not asked to do "--fix-thin" indeed finds the pack data fully self-contained, so it may be a good idea to have these two separate codepaths after all. OK.
quoted hunk
+ if test $? -ne 0 + then + echo >&2 "error: fail to convert $bundle or index-pack" + return 1 + fi
Do we even need the "error" message? "git index-pack" would have already given some error message to its standard error stream, no? If so if test -n "$thin" then ... fi || return 1 would be sufficient, I guess.
quoted hunk
+ count=$(git show-index <"${pack%pack}idx" | wc -l) && + test $2 = $count && return 0 + echo >&2 "error: object count for $bundle is $count, not $2" + return 1 +}
Looking good except for a few minor nits I mentioned above. Thanks.