Re: [PATCH v3 13/13] mergetool: fix running in subdir when rerere enabled
From: Junio C Hamano <hidden>
Date: 2017-01-09 18:12:55
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2017-01-09 · Re: [PATCH v3 13/13] mergetool: fix running in subdir when rerere enabled · Richard Hansen <hidden>
- 2017-01-09 · Re: [PATCH v3 13/13] mergetool: fix running in subdir when rerere enabled · Junio C Hamano <hidden>
- 2017-01-09 · Re: [PATCH v3 13/13] mergetool: fix running in subdir when rerere enabled · Junio C Hamano <hidden>
- 2017-01-09 · Re: [PATCH v3 13/13] mergetool: fix running in subdir when rerere enabled · Richard Hansen <hidden>
- 2017-01-09 · Re: [PATCH v3 13/13] mergetool: fix running in subdir when rerere enabled · Johannes Sixt <hidden>
Richard Hansen [off-list ref] writes:
quoted hunk
If rerere is enabled and no pathnames are given, run cd_to_toplevel before running 'git diff --name-only' so that 'git diff --name-only' sees the files named by 'git rerere remaining', which outputs pathnames relative to the top-level directory. The cd_to_toplevel command could be run after 'git rerere remaining', but it is run before just in case 'git rerere remaining' is ever changed to print pathnames relative to the current working directory rather than relative to the top-level directory. An alternative approach would be to unconditionally convert all relative pathnames (including the orderfile pathname) to be relative to the top-level directory and then run cd_to_toplevel before 'git diff --name-only', but unfortunately 'git rev-parse --prefix' requires valid pathnames, which would break some valid use cases. This fixes a regression introduced in 57937f70a09c12ef484c290865dac4066d207c9c (v2.11.0). Signed-off-by: Richard Hansen <redacted> --- git-mergetool.sh | 32 ++++++++++++++++++++++++++++++++ t/t7610-mergetool.sh | 2 +- 2 files changed, 33 insertions(+), 1 deletion(-)diff --git a/git-mergetool.sh b/git-mergetool.sh index b506896dc..22f56c25a 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh@@ -456,6 +456,28 @@ main () {
While doing the extra cd_to_toplevel added by this patch may not break anything that comes after the existing cd_to_toplevel, I find the result of applying this patch unnecessarily confusing. As "if the user didn't give any pathnames from the command line, ask rerere what paths it thinks are necessary to be handled" is merely a laziness fallback, it feels conceptually wrong to use different invocations of -O$orderfile when rerere is and is not in effect. I wonder if it makes more sense to always move to toplevel upfront and consistently use path from the toplevel, perhaps like the patch does. The first hunk is what you wrote but only inside MERGE_RR block, and the second hunk deals with converting end-user supplied paths that are relative to the original relative to the top-level. The tweaking of $orderfile you have in the first hunk may have to be tightened mimicking the way how "eval ... --sq ... ; shift" is used in the second hunk to avoid confusion in case orderfile specified by the end user happens to be the same as a valid revname (e.g. "master").
diff --git a/git-mergetool.sh b/git-mergetool.sh
index b506896dc1..adbbeceb47 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh@@ -454,6 +454,14 @@ main () { merge_keep_backup="$(git config --bool mergetool.keepBackup || echo true)" merge_keep_temporaries="$(git config --bool mergetool.keepTemporaries || echo false)" + prefix=$(git rev-parse --show-prefix) || exit 1 + cd_to_toplevel + + if test -n "$orderfile" + then + orderfile=$(git rev-parse --prefix "$prefix" "$orderfile") || exit 1 + fi + if test $# -eq 0 && test -e "$GIT_DIR/MERGE_RR" then set -- $(git rerere remaining)
@@ -461,14 +469,16 @@ main () { then print_noop_and_exit fi + elif test $# -ge 0 + then + eval "set -- $(git rev-parse --sq --prefix "$prefix" -- "$@")" + shift fi files=$(git -c core.quotePath=false \ diff --name-only --diff-filter=U \ ${orderfile:+"-O$orderfile"} -- "$@") - cd_to_toplevel - if test -z "$files" then print_noop_and_exit