mergetool(vimdiff): allow paths to contain spaces again

In 0041797449d (vimdiff: new implementation with layout support, 2022-03-30), we introduced a completely new implementation of the `vimdiff` backend for `git mergetool`. In this implementation, we no longer call `vim` directly but we accumulate in the variable `FINAL_CMD` an arbitrary number of commands for `vim` to execute, which necessitates the use of `eval` to split the commands properly into multiple command-line arguments. That same `eval` command also needs to pass the paths to `vim`, and while it looks as if they are quoted correctly, that quoting only reaches the `eval` instruction and is lost after that, therefore paths that contain whitespace characters (or other characters that are interpreted by the POSIX shell) are handled incorrectly. This is a simple reproducer: git init -b main bam-merge-fail cd bam-merge-fail echo a>"a file.txt" git add "a file.txt" git commit -m "added 'a file.txt'" echo b>"a file.txt" git add "a file.txt" git commit -m "diverged b 'a file.txt'" git checkout -b c HEAD~ echo c>"a file.txt" git add "a file.txt" git commit -m "diverged c 'a file.txt'" git checkout main git merge c git mergetool --tool=vimdiff With Git v2.37.0/v2.37.1, this will open 7 buffers, not four, and not display the correct contents at all. To fix this, let's not expand the variables containing the path parameters before passing them to the `eval` command, but let that command expand the variables instead. This fixes https://github.com/git-for-windows/git/issues/3945 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johannes Schindelin committed Jul 14, 2022 at 14:31 UTC ccc7b5148bdd9deb33f3cfa5a872a74634105021
1 file changed +35 -4
mergetools/vimdiff
+35 -4
@@ -414,8 +414,8 @@ merge_cmd () {
414
415 if $base_present
416 then
417 - eval "$merge_tool_path" \
418 - -f "$FINAL_CMD" "$LOCAL" "$BASE" "$REMOTE" "$MERGED"
417 + eval '"$merge_tool_path"' \
418 + -f "$FINAL_CMD" '"$LOCAL"' '"$BASE"' '"$REMOTE"' '"$MERGED"'
419 else
420 # If there is no BASE (example: a merge conflict in a new file
421 # with the same name created in both braches which didn't exist
@@ -424,8 +424,8 @@ merge_cmd () {
424 FINAL_CMD=$(echo "$FINAL_CMD" | \
425 sed -e 's:2b:quit:g' -e 's:3b:2b:g' -e 's:4b:3b:g')
426
427 - eval "$merge_tool_path" \
428 - -f "$FINAL_CMD" "$LOCAL" "$REMOTE" "$MERGED"
427 + eval '"$merge_tool_path"' \
428 + -f "$FINAL_CMD" '"$LOCAL"' '"$REMOTE"' '"$MERGED"'
429 fi
430
431 ret="$?"
@@ -614,6 +614,37 @@ run_unit_tests () {
614 fi
615 done
616
617 + # verify that `merge_cmd` handles paths with spaces
618 + record_parameters () {
619 + >actual
620 + for arg
621 + do
622 + echo "$arg" >>actual
623 + done
624 + }
625 +
626 + base_present=false
627 + LOCAL='lo cal'
628 + BASE='ba se'
629 + REMOTE="' '"
630 + MERGED='mer ged'
631 + merge_tool_path=record_parameters
632 +
633 + merge_cmd vimdiff || at_least_one_ko=true
634 +
635 + cat >expect <<-\EOF
636 + -f
637 + -c
638 + echo | split | vertical split | 1b | wincmd l | vertical split | quit | wincmd l | 2b | wincmd j | 3b | tabdo windo diffthis
639 + -c
640 + tabfirst
641 + lo cal
642 + ' '
643 + mer ged
644 + EOF
645 +
646 + diff -u expect actual || at_least_one_ko=true
647 +
648 if test "$at_least_one_ko" = "true"
649 then
650 return 255