Re: [PATCH 2/5] t/lib-commit-graph.sh: avoid directory change in `graph_git_behavior()`

From
Eric Sunshine <hidden>
Date
2023-07-21 18:02:15
Reply to
Taylor Blau, 2023-07-21
On Fri, Jul 21, 2023 at 1:32 PM Taylor Blau [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
---
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@sigill.intra.peff.net/ (local)
[2]: https://lore.kernel.org/git/e3bfc53363b14826d828e1adffbbeea@74d39fa044aa309eaea14b9f57fe79c/ (local)
[3]: https://lore.kernel.org/git/20160518010609.Horde.sM8QUFek6WMAAwho56DDob8@webmail.informatik.kit.edu/ (local)
[4]: https://lore.kernel.org/git/1240044459-57227-1-git-send-email-ben@ben.com/ (local)
Thread (18 messages) 18 messages, 2 authors, last reply 2023-07-24
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help