On Fri, Jul 21, 2023 at 1:32 PM Taylor Blau <me@xxxxxxxxxxxx> wrote:
> The `graph_git_behavior()` helper asserts that a number of common Git
> operations (such as `git log --oneline`, `git log --topo-order`, etc.)
> produce identical output regardless of whether or not a commit-graph is
> in use.
>
> This helper takes as its second argument the location (relative to the
> `$TRASH_DIRECTORY`) of the Git repostiory under test. In order to run
> each of its commands within that repository, it first changes into that
> directory, without the use of a sub-shell.
>
> This pollutes future tests which expect to be run in the top-level
> `$TRASH_DIRECTORY` as usual. We could wrap `graph_git_behavior()` in a
> sub-shell, like:
>
> graph_git_behavior() {
> # ...
> (
> cd "$TRASH_DIRECTORY/$DIR" &&
> graph_git_two_modesl
> )
> }
>
> , but since we're invoking git directly, we can pass along a "-C $DIR"
> when "$DIR" is non-empty.
>
> Note, however, that until the remaining callers are cleaned up to avoid
> changing working directories outside of a sub-shell, that we need to
> ensure that we are operating in the top-level $TRASH_DIRECTORY. The
> inner-subshell will go away in a future commit once it is no longer
> necessary.
>
> Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
> ---
> diff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh
> @@ -20,12 +20,14 @@ graph_git_behavior() {
> test_expect_success "check normal git operations: $MSG" '
> - cd "$TRASH_DIRECTORY/$DIR" &&
> - graph_git_two_modes "log --oneline $BRANCH" &&
> - graph_git_two_modes "log --topo-order $BRANCH" &&
> - graph_git_two_modes "log --graph $COMPARE..$BRANCH" &&
> - graph_git_two_modes "branch -vv" &&
> - graph_git_two_modes "merge-base -a $BRANCH $COMPARE"
> + (
> + cd "$TRASH_DIRECTORY" &&
> + graph_git_two_modes "${DIR:+-C $DIR} log --oneline $BRANCH" &&
> + graph_git_two_modes "${DIR:+-C $DIR} log --topo-order $BRANCH" &&
> + graph_git_two_modes "${DIR:+-C $DIR} log --graph $COMPARE..$BRANCH" &&
> + graph_git_two_modes "${DIR:+-C $DIR} branch -vv" &&
> + graph_git_two_modes "${DIR:+-C $DIR} merge-base -a $BRANCH $COMPARE"
> + )
> '
> }
As mentioned in my review of patch [1/5], for safety, you'd probably
want to quote the expansion of DIR in case it ever contains whitespace
(or other weird characters). The obvious POSIX-correct way to do this
would be:
graph_git_two_modes "${DIR:+-C \"$DIR\"} log ..." &&
Unfortunately, however, some older broken shells incorrectly expand
this to a single argument ("-C <dir>") rather than the expected two
arguments (-C and "<dir>")[1,2,3,4]. The workaround is unsightly but
doable:
graph_git_two_modes "${DIR:+-C} ${DIR:+\"$DIR\"} log ..." &&
[1]: https://lore.kernel.org/git/20160517215214.GA16905@xxxxxxxxxxxxxxxxxxxxx/
[2]: https://lore.kernel.org/git/e3bfc53363b14826d828e1adffbbeea@74d39fa044aa309eaea14b9f57fe79c/
[3]: https://lore.kernel.org/git/20160518010609.Horde.sM8QUFek6WMAAwho56DDob8@xxxxxxxxxxxxxxxxxxxxxxxxxx/
[4]: https://lore.kernel.org/git/1240044459-57227-1-git-send-email-ben@xxxxxxx/
