On Fri, Jul 21, 2023 at 02:01:58PM -0400, Eric Sunshine wrote:
> > 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 ..." &&
Hmm. I get what you're saying, but I think in this case we're OK, since
this all goes to `graph_git_two_modes`, whose implementation looks like:
graph_git_two_modes() {
git -c core.commitGraph=true $1 >output
}
So I think we really do want everything smashed together into a single
argument.
(Just to sure, here's the diff of what I applied on top of [2/5] before
replying to your message):
--- 8< ---
diff --git a/t/lib-commit-graph.sh b/t/lib-commit-graph.sh
index c93969ae74..d60e64cb0b 100755
--- a/t/lib-commit-graph.sh
+++ b/t/lib-commit-graph.sh
@@ -22,11 +22,11 @@ graph_git_behavior() {
test_expect_success "check normal git operations: $MSG" '
(
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"
+ graph_git_two_modes "${DIR:+-C} ${DIR:+\"$DIR\"} log --oneline $BRANCH" &&
+ graph_git_two_modes "${DIR:+-C} ${DIR:+\"$DIR\"} log --topo-order $BRANCH" &&
+ graph_git_two_modes "${DIR:+-C} ${DIR:+\"$DIR\"} log --graph $COMPARE..$BRANCH" &&
+ graph_git_two_modes "${DIR:+-C} ${DIR:+\"$DIR\"} branch -vv" &&
+ graph_git_two_modes "${DIR:+-C} ${DIR:+\"$DIR\"} merge-base -a $BRANCH $COMPARE"
)
'
}
--- >8 ---
> [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/
Thanks,
Taylor
