[PATCH v6 0/3] improvements for git-bundle

From: Jiang Xin <worldhello.net@gmail.com>
Date: Jan 12, 2021, 02:27 UTC
Message-ID: <20210112022703.1884-1-worldhello.net@gmail.com>
In-Reply-To: <xmqq1rer8cbz.fsf@gitster.c.googlers.com>

From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

Introduce two improvements for git-bundle

+ Commit "bundle: lost objects when removing duplicate pendings",
  which fixes command like:

        $ git bundle create <file> 'master^!'

+ Commits "bundle: arguments can be read from stdin",
  which add "--stdin" option support for git-bundle, like:

        $ git bundle create <file> <input

## Changes since v5:

Junio C Hamano <gitster@pobox.com> writes:

Show 23 quoted lines

>
> Jiang Xin <worldhello.net@gmail.com> writes:
>
> > +     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.

range-diff v5...v6 (part-1):

    @@ t/t6020-bundle-misc.sh (new)
     +		esac
     +		shift
     +	done
    -+
    -+	var=$1
    -+	shift
    -+	if test -z "$var"
    ++	if test $# -lt 2
     +	then
    -+		echo >&2 "error: var is not defined"
    ++		echo >&2 "error: test_commit_setvar must have at least 2 arguments"
     +		return 1
     +	fi
    ++	var=$1
    ++	shift
     +	indir=${indir:+"$indir"/}
     +	if test -z "$notick"
     +	then

Show 5 quoted lines

> > +     else
> > +             file=${2:-"$1.t"} &&
> > +             echo "${3-$1}" > "$indir$file" &&
>
> Style?  [*Nit 2*]

range-diff v5...v6 (part-2):

* Add new arg "${2:-HEAD}" for `git tag`.
* Fix [*Nit 2*].

    @@ t/t6020-bundle-misc.sh (new)
     +		oid=$(git ${indir:+ -C "$indir"} rev-parse HEAD)
     +	elif test -n "$tag"
     +	then
    -+		git ${indir:+ -C "$indir"} tag -m "$1" "$1" &&
    ++		git ${indir:+ -C "$indir"} tag -m "$1" "$1" "${2:-HEAD}" &&
     +		oid=$(git ${indir:+ -C "$indir"} rev-parse "$1")
     +	else
     +		file=${2:-"$1.t"} &&
    -+		echo "${3-$1}" > "$indir$file" &&
    ++		echo "${3-$1}" >"$indir$file" &&
     +		git ${indir:+ -C "$indir"} add "$file" &&
     +		git ${indir:+ -C "$indir"} commit $signoff -m "$1" &&
     +		oid=$(git ${indir:+ -C "$indir"} rev-parse HEAD)

Show 9 quoted lines

> > +# 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.

range-diff v5...v6 (part-3):

    @@ t/t6020-bundle-misc.sh (new)
     +	eval $var=$oid
     +}
     +
    -+
     +# 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" \
    -+		-e "s/$(echo $B | cut -c1-7)[0-9a-f]*/<COMMIT-B>/g" \
    -+		-e "s/$(echo $C | cut -c1-7)[0-9a-f]*/<COMMIT-C>/g" \
    -+		-e "s/$(echo $D | cut -c1-7)[0-9a-f]*/<COMMIT-D>/g" \
    -+		-e "s/$(echo $E | cut -c1-7)[0-9a-f]*/<COMMIT-E>/g" \
    -+		-e "s/$(echo $F | cut -c1-7)[0-9a-f]*/<COMMIT-F>/g" \
    -+		-e "s/$(echo $G | cut -c1-7)[0-9a-f]*/<COMMIT-G>/g" \
    -+		-e "s/$(echo $H | cut -c1-7)[0-9a-f]*/<COMMIT-H>/g" \
    -+		-e "s/$(echo $I | cut -c1-7)[0-9a-f]*/<COMMIT-I>/g" \
    -+		-e "s/$(echo $J | cut -c1-7)[0-9a-f]*/<COMMIT-J>/g" \
    -+		-e "s/$(echo $K | cut -c1-7)[0-9a-f]*/<COMMIT-K>/g" \
    -+		-e "s/$(echo $L | cut -c1-7)[0-9a-f]*/<COMMIT-L>/g" \
    -+		-e "s/$(echo $M | cut -c1-7)[0-9a-f]*/<COMMIT-M>/g" \
    -+		-e "s/$(echo $N | cut -c1-7)[0-9a-f]*/<COMMIT-N>/g" \
    -+		-e "s/$(echo $O | cut -c1-7)[0-9a-f]*/<COMMIT-O>/g" \
    -+		-e "s/$(echo $P | cut -c1-7)[0-9a-f]*/<COMMIT-P>/g" \
    -+		-e "s/$(echo $TAG1 | cut -c1-7)[0-9a-f]*/<TAG-1>/g" \
    -+		-e "s/$(echo $TAG2 | cut -c1-7)[0-9a-f]*/<TAG-2>/g" \
    -+		-e "s/$(echo $TAG3 | cut -c1-7)[0-9a-f]*/<TAG-3>/g" \
    ++		-e "s/${A%${A#???????}}[0-9a-f]*/<COMMIT-A>/g" \
    ++		-e "s/${B%${B#???????}}[0-9a-f]*/<COMMIT-B>/g" \
    ++		-e "s/${C%${C#???????}}[0-9a-f]*/<COMMIT-C>/g" \
    ++		-e "s/${D%${D#???????}}[0-9a-f]*/<COMMIT-D>/g" \
    ++		-e "s/${E%${E#???????}}[0-9a-f]*/<COMMIT-E>/g" \
    ++		-e "s/${F%${F#???????}}[0-9a-f]*/<COMMIT-F>/g" \
    ++		-e "s/${G%${G#???????}}[0-9a-f]*/<COMMIT-G>/g" \
    ++		-e "s/${H%${H#???????}}[0-9a-f]*/<COMMIT-H>/g" \
    ++		-e "s/${I%${I#???????}}[0-9a-f]*/<COMMIT-I>/g" \
    ++		-e "s/${J%${J#???????}}[0-9a-f]*/<COMMIT-J>/g" \
    ++		-e "s/${K%${K#???????}}[0-9a-f]*/<COMMIT-K>/g" \
    ++		-e "s/${L%${L#???????}}[0-9a-f]*/<COMMIT-L>/g" \
    ++		-e "s/${M%${M#???????}}[0-9a-f]*/<COMMIT-M>/g" \
    ++		-e "s/${N%${N#???????}}[0-9a-f]*/<COMMIT-N>/g" \
    ++		-e "s/${O%${O#???????}}[0-9a-f]*/<COMMIT-O>/g" \
    ++		-e "s/${P%${P#???????}}[0-9a-f]*/<COMMIT-P>/g" \
    ++		-e "s/${TAG1%${TAG1#???????}}[0-9a-f]*/<TAG-1>/g" \
    ++		-e "s/${TAG2%${TAG2#???????}}[0-9a-f]*/<TAG-2>/g" \
    ++		-e "s/${TAG3%${TAG3#???????}}[0-9a-f]*/<TAG-3>/g" \
     +		-e "s/ *\$//"
     +}
     +

Show 10 quoted lines

> 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.

range-diff v5...v6 (part-4):

1:  fa7516b2ec ! 1:  900bb16178 test: add helper functions for git-bundle
    @@ t/test-bundle-functions.sh (new)
     +		git index-pack --stdin --fix-thin "$pack" <"$bundle.thin.pack"
     +	else
     +		git index-pack "$pack"
    -+	fi
    -+	if test $? -ne 0
    -+	then
    -+		echo >&2 "error: fail to convert $bundle or index-pack"
    -+		return 1
    -+	fi
    ++	fi || return 1
     +	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"
2:  ea543de111 = 2:  1bbf0ab213 bundle: lost objects when removing duplicate pendings
3:  18f0d48814 = 3:  7ac0751821 bundle: arguments can be read from stdin

--

Jiang Xin (3):
  test: add helper functions for git-bundle
  bundle: lost objects when removing duplicate pendings
  bundle: arguments can be read from stdin

 bundle.c                   | 109 +++++----
 object.c                   |  10 +-
 t/t5510-fetch.sh           |  26 +--
 t/t5607-clone-bundle.sh    |   4 +-
 t/t6020-bundle-misc.sh     | 463 +++++++++++++++++++++++++++++++++++++
 t/test-bundle-functions.sh |  42 ++++
 6 files changed, 576 insertions(+), 78 deletions(-)
 create mode 100755 t/t6020-bundle-misc.sh
 create mode 100644 t/test-bundle-functions.sh

-- 
2.28.0.15.gba9e81f0bd