From: Eric Sunshine <sunshine@sunshineco.com>
Date: Fri, 01 Feb 2019 19:42:53 GMT
Subject: Re: [PATCH v9 1/6] t5323: test cases for git-pack-redundant
Message-ID: <CAPig+cRX=ttHWe1C6m1u9-02oT-TpBtHWb48N=-wX4jSYaFuew@mail.gmail.com>
In-Reply-To: <20190201162152.31136-2-worldhello.net@gmail.com>

On Fri, Feb 1, 2019 at 11:22 AM Jiang Xin <worldhello.net@gmail.com> wrote:
> Add test cases for git pack-redundant to validate new algorithm for git
> pack-redundant.
>
> Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> ---
> diff --git a/t/t5323-pack-redundant.sh b/t/t5323-pack-redundant.sh
> @@ -0,0 +1,510 @@
> +# Note: DO NOT run it in a subshell, otherwise the variables will not be set

Which variables won't be set? It's not clear what this restriction is about.

> +# Usage: create_commits_in <repo> A B C ...
> +create_commits_in () {
> +       repo="$1" &&
> +       parent=$(git -C "$repo" rev-parse HEAD^{} 2>/dev/null) || parent=

Broken &&-chain. Instead, perhaps:

    if ! parent=$(git -C "$repo" rev-parse HEAD^{} 2>/dev/null)
    then
        parent=
    fi &&

or something simpler.

> +       T=$(git -C "$repo" write-tree) &&
> +       shift &&
> +       while test $# -gt 0
> +       do
> +               name=$1 &&
> +               test_tick &&
> +               if test -z "$parent"
> +               then
> +                       oid=$(echo $name | git -C "$repo" commit-tree $T)
> +               else
> +                       oid=$(echo $name | git -C "$repo" commit-tree -p $parent $T)
> +               fi &&
> +               eval $name=$oid &&
> +               parent=$oid &&
> +               shift ||
> +               return 1
> +       done

Broken &&-chain. Use:

    done &&

> +       git -C "$repo" update-ref refs/heads/master $oid
> +}
> +
> +# Note: DO NOT run it in a subshell, otherwise the variables will not be set
> +create_pack_1 () {
> +       P1=$(git -C "$master_repo/objects/pack" pack-objects -q pack <<-EOF

Which variables? Note that you can capture output of a subshell into a
variable, if necessary.