* [PATCH v5 09/14] t7610: always work on a test-specific branch
From: Richard Hansen @ 2017-01-10 20:41 UTC (permalink / raw)
To: git; +Cc: davvid, j6t, hansenr, sbeller, simon, gitster
In-Reply-To: <20170110204202.21779-1-hansenr@google.com>
Create and use a test-specific branch when the test might create a
commit. This is not always necessary for correctness, but it improves
debuggability by ensuring a commit created by test #N shows up on the
testN branch, not the branch for test #N-1.
Signed-off-by: Richard Hansen <hansenr@google.com>
---
t/t7610-mergetool.sh | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 7d5e1df88..efcf5c3f1 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -184,6 +184,7 @@ test_expect_success 'mergetool in subdir' '
test_expect_success 'mergetool on file in parent dir' '
test_when_finished "git reset --hard" &&
+ git checkout -b test$test_count &&
git submodule update -N &&
(
cd subdir &&
@@ -217,6 +218,7 @@ test_expect_success 'mergetool skips autoresolved' '
test_expect_success 'mergetool merges all from subdir' '
test_when_finished "git reset --hard" &&
+ git checkout -b test$test_count &&
test_config rerere.enabled false &&
(
cd subdir &&
@@ -288,7 +290,7 @@ test_expect_success 'mergetool takes partial path' '
test_expect_success 'mergetool delete/delete conflict' '
test_when_finished "git reset --hard HEAD" &&
- git checkout move-to-c &&
+ git checkout -b test$test_count move-to-c &&
test_must_fail git merge move-to-b &&
echo d | git mergetool a/a/file.txt &&
! test -f a/a/file.txt &&
@@ -304,6 +306,7 @@ test_expect_success 'mergetool delete/delete conflict' '
test_expect_success 'mergetool produces no errors when keepBackup is used' '
test_when_finished "git reset --hard HEAD" &&
+ git checkout -b test$test_count &&
test_config mergetool.keepBackup true &&
test_must_fail git merge move-to-b &&
: >expect &&
@@ -314,6 +317,7 @@ test_expect_success 'mergetool produces no errors when keepBackup is used' '
test_expect_success 'mergetool honors tempfile config for deleted files' '
test_when_finished "git reset --hard HEAD" &&
+ git checkout -b test$test_count &&
test_config mergetool.keepTemporaries false &&
test_must_fail git merge move-to-b &&
echo d | git mergetool a/a/file.txt &&
@@ -323,6 +327,7 @@ test_expect_success 'mergetool honors tempfile config for deleted files' '
test_expect_success 'mergetool keeps tempfiles when aborting delete/delete' '
test_when_finished "git reset --hard HEAD" &&
test_when_finished "git clean -fdx" &&
+ git checkout -b test$test_count &&
test_config mergetool.keepTemporaries true &&
test_must_fail git merge move-to-b &&
! (echo a; echo n) | git mergetool a/a/file.txt &&
@@ -640,7 +645,7 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT
test_expect_success 'diff.orderFile configuration is honored' '
test_when_finished "git reset --hard >/dev/null" &&
- git checkout order-file-side2 &&
+ git checkout -b test$test_count order-file-side2 &&
test_config diff.orderFile order-file &&
test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
test_config mergetool.myecho.trustExitCode true &&
@@ -658,6 +663,7 @@ test_expect_success 'diff.orderFile configuration is honored' '
'
test_expect_success 'mergetool -Oorder-file is honored' '
test_when_finished "git reset --hard >/dev/null 2>&1" &&
+ git checkout -b test$test_count &&
test_config diff.orderFile order-file &&
test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
test_config mergetool.myecho.trustExitCode true &&
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH v5 11/14] t7610: spell 'git reset --hard' consistently
From: Richard Hansen @ 2017-01-10 20:41 UTC (permalink / raw)
To: git; +Cc: davvid, j6t, hansenr, sbeller, simon, gitster
In-Reply-To: <20170110204202.21779-1-hansenr@google.com>
Signed-off-by: Richard Hansen <hansenr@google.com>
---
t/t7610-mergetool.sh | 37 +++++++++++++++++++------------------
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 54164a320..c031ecd9e 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -289,23 +289,23 @@ test_expect_success 'mergetool takes partial path' '
'
test_expect_success 'mergetool delete/delete conflict' '
- test_when_finished "git reset --hard HEAD" &&
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count move-to-c &&
test_must_fail git merge move-to-b &&
echo d | git mergetool a/a/file.txt &&
! test -f a/a/file.txt &&
- git reset --hard HEAD &&
+ git reset --hard &&
test_must_fail git merge move-to-b &&
echo m | git mergetool a/a/file.txt &&
test -f b/b/file.txt &&
- git reset --hard HEAD &&
+ git reset --hard &&
test_must_fail git merge move-to-b &&
! echo a | git mergetool a/a/file.txt &&
! test -f a/a/file.txt
'
test_expect_success 'mergetool produces no errors when keepBackup is used' '
- test_when_finished "git reset --hard HEAD" &&
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count move-to-c &&
test_config mergetool.keepBackup true &&
test_must_fail git merge move-to-b &&
@@ -316,7 +316,7 @@ test_expect_success 'mergetool produces no errors when keepBackup is used' '
'
test_expect_success 'mergetool honors tempfile config for deleted files' '
- test_when_finished "git reset --hard HEAD" &&
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count move-to-c &&
test_config mergetool.keepTemporaries false &&
test_must_fail git merge move-to-b &&
@@ -325,7 +325,7 @@ test_expect_success 'mergetool honors tempfile config for deleted files' '
'
test_expect_success 'mergetool keeps tempfiles when aborting delete/delete' '
- test_when_finished "git reset --hard HEAD" &&
+ test_when_finished "git reset --hard" &&
test_when_finished "git clean -fdx" &&
git checkout -b test$test_count move-to-c &&
test_config mergetool.keepTemporaries true &&
@@ -342,7 +342,7 @@ test_expect_success 'mergetool keeps tempfiles when aborting delete/delete' '
'
test_expect_success 'deleted vs modified submodule' '
- test_when_finished "git reset --hard HEAD" &&
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count branch1 &&
git submodule update -N &&
mv submod submod-movedaside &&
@@ -560,7 +560,7 @@ test_expect_success 'directory vs modified submodule' '
test "$(cat submod/file16)" = "not a submodule" &&
rm -rf submod.orig &&
- git reset --hard >/dev/null 2>&1 &&
+ git reset --hard &&
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
test ! -e submod.orig &&
@@ -572,7 +572,8 @@ test_expect_success 'directory vs modified submodule' '
( cd submod && git clean -f && git reset --hard ) &&
git submodule update -N &&
test "$(cat submod/bar)" = "master submodule" &&
- git reset --hard >/dev/null 2>&1 && rm -rf submod-movedaside &&
+ git reset --hard &&
+ rm -rf submod-movedaside &&
git checkout -b test$test_count.c master &&
git submodule update -N &&
@@ -582,7 +583,7 @@ test_expect_success 'directory vs modified submodule' '
git submodule update -N &&
test "$(cat submod/bar)" = "master submodule" &&
- git reset --hard >/dev/null 2>&1 &&
+ git reset --hard &&
git submodule update -N &&
test_must_fail git merge test$test_count &&
test -n "$(git ls-files -u)" &&
@@ -590,13 +591,13 @@ test_expect_success 'directory vs modified submodule' '
( yes "r" | git mergetool submod ) &&
test "$(cat submod/file16)" = "not a submodule" &&
- git reset --hard master >/dev/null 2>&1 &&
+ git reset --hard master &&
( cd submod && git clean -f && git reset --hard ) &&
git submodule update -N
'
test_expect_success 'file with no base' '
- test_when_finished "git reset --hard master >/dev/null 2>&1" &&
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count branch1 &&
test_must_fail git merge master &&
git mergetool --no-prompt --tool mybase -- both &&
@@ -605,7 +606,7 @@ test_expect_success 'file with no base' '
'
test_expect_success 'custom commands override built-ins' '
- test_when_finished "git reset --hard master >/dev/null 2>&1" &&
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count branch1 &&
test_config mergetool.defaults.cmd "cat \"\$REMOTE\" >\"\$MERGED\"" &&
test_config mergetool.defaults.trustExitCode true &&
@@ -616,7 +617,7 @@ test_expect_success 'custom commands override built-ins' '
'
test_expect_success 'filenames seen by tools start with ./' '
- test_when_finished "git reset --hard master >/dev/null 2>&1" &&
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count branch1 &&
test_config mergetool.writeToTemp false &&
test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
@@ -632,7 +633,7 @@ test_lazy_prereq MKTEMP '
'
test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToTemp' '
- test_when_finished "git reset --hard master >/dev/null 2>&1" &&
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count branch1 &&
test_config mergetool.writeToTemp true &&
test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
@@ -644,7 +645,7 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT
'
test_expect_success 'diff.orderFile configuration is honored' '
- test_when_finished "git reset --hard >/dev/null" &&
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count order-file-side2 &&
test_config diff.orderFile order-file &&
test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
@@ -662,7 +663,7 @@ test_expect_success 'diff.orderFile configuration is honored' '
test_cmp expect actual
'
test_expect_success 'mergetool -Oorder-file is honored' '
- test_when_finished "git reset --hard >/dev/null 2>&1" &&
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count order-file-side2 &&
test_config diff.orderFile order-file &&
test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
@@ -678,7 +679,7 @@ test_expect_success 'mergetool -Oorder-file is honored' '
git mergetool -O/dev/null --no-prompt --tool myecho >output &&
git grep --no-index -h -A2 Merging: output >actual &&
test_cmp expect actual &&
- git reset --hard >/dev/null 2>&1 &&
+ git reset --hard &&
git config --unset diff.orderFile &&
test_must_fail git merge order-file-side1 &&
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH v5 10/14] t7610: don't assume the checked-out commit
From: Richard Hansen @ 2017-01-10 20:41 UTC (permalink / raw)
To: git; +Cc: davvid, j6t, hansenr, sbeller, simon, gitster
In-Reply-To: <20170110204202.21779-1-hansenr@google.com>
Always check out the required commit at the beginning of the test so
that a failure in a previous test does not cause the test to work off
of the wrong commit.
This is a step toward making the tests more independent so that if one
test fails it doesn't cause subsequent tests to fail.
Signed-off-by: Richard Hansen <hansenr@google.com>
---
t/t7610-mergetool.sh | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index efcf5c3f1..54164a320 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -184,7 +184,7 @@ test_expect_success 'mergetool in subdir' '
test_expect_success 'mergetool on file in parent dir' '
test_when_finished "git reset --hard" &&
- git checkout -b test$test_count &&
+ git checkout -b test$test_count branch1 &&
git submodule update -N &&
(
cd subdir &&
@@ -218,7 +218,7 @@ test_expect_success 'mergetool skips autoresolved' '
test_expect_success 'mergetool merges all from subdir' '
test_when_finished "git reset --hard" &&
- git checkout -b test$test_count &&
+ git checkout -b test$test_count branch1 &&
test_config rerere.enabled false &&
(
cd subdir &&
@@ -306,7 +306,7 @@ test_expect_success 'mergetool delete/delete conflict' '
test_expect_success 'mergetool produces no errors when keepBackup is used' '
test_when_finished "git reset --hard HEAD" &&
- git checkout -b test$test_count &&
+ git checkout -b test$test_count move-to-c &&
test_config mergetool.keepBackup true &&
test_must_fail git merge move-to-b &&
: >expect &&
@@ -317,7 +317,7 @@ test_expect_success 'mergetool produces no errors when keepBackup is used' '
test_expect_success 'mergetool honors tempfile config for deleted files' '
test_when_finished "git reset --hard HEAD" &&
- git checkout -b test$test_count &&
+ git checkout -b test$test_count move-to-c &&
test_config mergetool.keepTemporaries false &&
test_must_fail git merge move-to-b &&
echo d | git mergetool a/a/file.txt &&
@@ -327,7 +327,7 @@ test_expect_success 'mergetool honors tempfile config for deleted files' '
test_expect_success 'mergetool keeps tempfiles when aborting delete/delete' '
test_when_finished "git reset --hard HEAD" &&
test_when_finished "git clean -fdx" &&
- git checkout -b test$test_count &&
+ git checkout -b test$test_count move-to-c &&
test_config mergetool.keepTemporaries true &&
test_must_fail git merge move-to-b &&
! (echo a; echo n) | git mergetool a/a/file.txt &&
@@ -663,7 +663,7 @@ test_expect_success 'diff.orderFile configuration is honored' '
'
test_expect_success 'mergetool -Oorder-file is honored' '
test_when_finished "git reset --hard >/dev/null 2>&1" &&
- git checkout -b test$test_count &&
+ git checkout -b test$test_count order-file-side2 &&
test_config diff.orderFile order-file &&
test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
test_config mergetool.myecho.trustExitCode true &&
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH v5 12/14] t7610: add test case for rerere+mergetool+subdir bug
From: Richard Hansen @ 2017-01-10 20:42 UTC (permalink / raw)
To: git; +Cc: davvid, j6t, hansenr, sbeller, simon, gitster
In-Reply-To: <20170110204202.21779-1-hansenr@google.com>
If rerere is enabled and mergetool is run from a subdirectory,
mergetool always prints "No files need merging". Add an expected
failure test case for this situation.
Signed-off-by: Richard Hansen <hansenr@google.com>
---
t/t7610-mergetool.sh | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index c031ecd9e..b36fde1c0 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -216,7 +216,7 @@ test_expect_success 'mergetool skips autoresolved' '
test "$output" = "No files need merging"
'
-test_expect_success 'mergetool merges all from subdir' '
+test_expect_success 'mergetool merges all from subdir (rerere disabled)' '
test_when_finished "git reset --hard" &&
git checkout -b test$test_count branch1 &&
test_config rerere.enabled false &&
@@ -234,6 +234,25 @@ test_expect_success 'mergetool merges all from subdir' '
)
'
+test_expect_failure 'mergetool merges all from subdir (rerere enabled)' '
+ test_when_finished "git reset --hard" &&
+ git checkout -b test$test_count branch1 &&
+ test_config rerere.enabled true &&
+ rm -rf .git/rr-cache &&
+ (
+ cd subdir &&
+ test_must_fail git merge master &&
+ ( yes "r" | git mergetool ../submod ) &&
+ ( yes "d" "d" | git mergetool --no-prompt ) &&
+ test "$(cat ../file1)" = "master updated" &&
+ test "$(cat ../file2)" = "master new" &&
+ test "$(cat file3)" = "master new sub" &&
+ ( cd .. && git submodule update -N ) &&
+ test "$(cat ../submod/bar)" = "master submodule" &&
+ git commit -m "branch2 resolved by mergetool from subdir"
+ )
+'
+
test_expect_success 'mergetool skips resolved paths when rerere is active' '
test_when_finished "git reset --hard" &&
test_config rerere.enabled true &&
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH v5 13/14] mergetool: take the "-O" out of $orderfile
From: Richard Hansen @ 2017-01-10 20:42 UTC (permalink / raw)
To: git; +Cc: davvid, j6t, hansenr, sbeller, simon, gitster
In-Reply-To: <20170110204202.21779-1-hansenr@google.com>
This will make it easier for a future commit to convert a relative
orderfile pathname to either absolute or relative to the top-level
directory. It also improves code readability.
Signed-off-by: Richard Hansen <hansenr@google.com>
---
git-mergetool.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-mergetool.sh b/git-mergetool.sh
index e52b4e4f2..b506896dc 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -421,7 +421,7 @@ main () {
prompt=true
;;
-O*)
- orderfile="$1"
+ orderfile="${1#-O}"
;;
--)
shift
@@ -465,7 +465,7 @@ main () {
files=$(git -c core.quotePath=false \
diff --name-only --diff-filter=U \
- ${orderfile:+"$orderfile"} -- "$@")
+ ${orderfile:+"-O$orderfile"} -- "$@")
cd_to_toplevel
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH v5 14/14] mergetool: fix running in subdir when rerere enabled
From: Richard Hansen @ 2017-01-10 20:42 UTC (permalink / raw)
To: git; +Cc: davvid, j6t, hansenr, sbeller, simon, gitster
In-Reply-To: <20170110204202.21779-1-hansenr@google.com>
The pathnames output by the 'git rerere remaining' command are
relative to the top-level directory but the 'git diff --name-only'
command expects its pathname arguments to be relative to the current
working directory. Run cd_to_toplevel before running 'git diff
--name-only' and adjust any relative pathnames so that 'git mergetool'
does not fail when run from a subdirectory with rerere enabled.
This fixes a regression introduced in
57937f70a09c12ef484c290865dac4066d207c9c (v2.11.0).
Based-on-patch-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Richard Hansen <hansenr@google.com>
---
git-mergetool.sh | 17 +++++++++++++++--
t/t7610-mergetool.sh | 7 ++++++-
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/git-mergetool.sh b/git-mergetool.sh
index b506896dc..c062e3de3 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -454,6 +454,17 @@ 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" |
+ sed -e 1d
+ )
+ fi
+
if test $# -eq 0 && test -e "$GIT_DIR/MERGE_RR"
then
set -- $(git rerere remaining)
@@ -461,14 +472,16 @@ main () {
then
print_noop_and_exit
fi
+ elif test $# -ge 0
+ then
+ # rev-parse provides the -- needed for 'set'
+ eval "set $(git rev-parse --sq --prefix "$prefix" -- "$@")"
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
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index b36fde1c0..820fc8597 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -234,7 +234,7 @@ test_expect_success 'mergetool merges all from subdir (rerere disabled)' '
)
'
-test_expect_failure 'mergetool merges all from subdir (rerere enabled)' '
+test_expect_success 'mergetool merges all from subdir (rerere enabled)' '
test_when_finished "git reset --hard" &&
git checkout -b test$test_count branch1 &&
test_config rerere.enabled true &&
@@ -677,6 +677,11 @@ test_expect_success 'diff.orderFile configuration is honored' '
b
a
EOF
+
+ # make sure "order-file" that is ambiguous between
+ # rev and path is understood correctly.
+ git branch order-file HEAD &&
+
git mergetool --no-prompt --tool myecho >output &&
git grep --no-index -h -A2 Merging: output >actual &&
test_cmp expect actual
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH v5 08/14] t7610: delete some now-unnecessary 'git reset --hard' lines
From: Richard Hansen @ 2017-01-10 20:41 UTC (permalink / raw)
To: git; +Cc: davvid, j6t, hansenr, sbeller, simon, gitster
In-Reply-To: <20170110204202.21779-1-hansenr@google.com>
Tests now always run 'git reset --hard' at the end (even if they
fail), so it's no longer necessary to run 'git reset --hard' at the
beginning of a test.
Signed-off-by: Richard Hansen <hansenr@google.com>
---
t/t7610-mergetool.sh | 2 --
1 file changed, 2 deletions(-)
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 55587504e..7d5e1df88 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -184,7 +184,6 @@ test_expect_success 'mergetool in subdir' '
test_expect_success 'mergetool on file in parent dir' '
test_when_finished "git reset --hard" &&
- git reset --hard &&
git submodule update -N &&
(
cd subdir &&
@@ -277,7 +276,6 @@ test_expect_success 'conflicted stash sets up rerere' '
test_expect_success 'mergetool takes partial path' '
test_when_finished "git reset --hard" &&
- git reset --hard &&
test_config rerere.enabled false &&
git checkout -b test$test_count branch1 &&
git submodule update -N &&
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH v5 07/14] t7610: run 'git reset --hard' after each test to clean up
From: Richard Hansen @ 2017-01-10 20:41 UTC (permalink / raw)
To: git; +Cc: davvid, j6t, hansenr, sbeller, simon, gitster
In-Reply-To: <20170110204202.21779-1-hansenr@google.com>
Use test_when_finished to run 'git reset --hard' after each test so
that the repository is left in a saner state for the next test.
This is a step toward making the tests more independent so that if one
test fails it doesn't cause subsequent tests to fail.
Signed-off-by: Richard Hansen <hansenr@google.com>
---
t/t7610-mergetool.sh | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 2d92a2646..55587504e 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -127,6 +127,7 @@ test_expect_success 'setup' '
'
test_expect_success 'custom mergetool' '
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count branch1 &&
git submodule update -N &&
test_must_fail git merge master >/dev/null 2>&1 &&
@@ -170,6 +171,7 @@ test_expect_success 'mergetool crlf' '
'
test_expect_success 'mergetool in subdir' '
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count branch1 &&
git submodule update -N &&
(
@@ -181,6 +183,7 @@ test_expect_success 'mergetool in subdir' '
'
test_expect_success 'mergetool on file in parent dir' '
+ test_when_finished "git reset --hard" &&
git reset --hard &&
git submodule update -N &&
(
@@ -214,6 +217,7 @@ test_expect_success 'mergetool skips autoresolved' '
'
test_expect_success 'mergetool merges all from subdir' '
+ test_when_finished "git reset --hard" &&
test_config rerere.enabled false &&
(
cd subdir &&
@@ -244,6 +248,7 @@ test_expect_success 'mergetool skips resolved paths when rerere is active' '
'
test_expect_success 'conflicted stash sets up rerere' '
+ test_when_finished "git reset --hard" &&
test_config rerere.enabled true &&
git checkout stash1 &&
echo "Conflicting stash content" >file11 &&
@@ -403,6 +408,7 @@ test_expect_success 'deleted vs modified submodule' '
'
test_expect_success 'file vs modified submodule' '
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count branch1 &&
git submodule update -N &&
mv submod submod-movedaside &&
@@ -474,6 +480,7 @@ test_expect_success 'file vs modified submodule' '
'
test_expect_success 'submodule in subdirectory' '
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count branch1 &&
git submodule update -N &&
(
@@ -535,6 +542,7 @@ test_expect_success 'submodule in subdirectory' '
'
test_expect_success 'directory vs modified submodule' '
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count branch1 &&
mv submod submod-movedaside &&
git rm --cached submod &&
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH v5 06/14] t7610: don't rely on state from previous test
From: Richard Hansen @ 2017-01-10 20:41 UTC (permalink / raw)
To: git; +Cc: davvid, j6t, hansenr, sbeller, simon, gitster
In-Reply-To: <20170110204202.21779-1-hansenr@google.com>
If the repository must be in a particular state (beyond what is
already done by the 'setup' test case) before the test can run, make
the necessary repository changes in the test script even if it means
duplicating some lines of code from the previous test case.
This is a step toward making the tests more independent so that if one
test fails it doesn't cause subsequent tests to fail.
Signed-off-by: Richard Hansen <hansenr@google.com>
---
t/t7610-mergetool.sh | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index f62ceffdc..2d92a2646 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -181,8 +181,12 @@ test_expect_success 'mergetool in subdir' '
'
test_expect_success 'mergetool on file in parent dir' '
+ git reset --hard &&
+ git submodule update -N &&
(
cd subdir &&
+ test_must_fail git merge master >/dev/null 2>&1 &&
+ ( yes "" | git mergetool file3 >/dev/null 2>&1 ) &&
( yes "" | git mergetool ../file1 >/dev/null 2>&1 ) &&
( yes "" | git mergetool ../file2 ../spaced\ name >/dev/null 2>&1 ) &&
( yes "" | git mergetool ../both >/dev/null 2>&1 ) &&
@@ -651,6 +655,8 @@ test_expect_success 'mergetool -Oorder-file is honored' '
test_config diff.orderFile order-file &&
test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
test_config mergetool.myecho.trustExitCode true &&
+ echo b >order-file &&
+ echo a >>order-file &&
test_must_fail git merge order-file-side1 &&
cat >expect <<-\EOF &&
Merging:
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH v5 05/14] t7610: use test_when_finished for cleanup tasks
From: Richard Hansen @ 2017-01-10 20:41 UTC (permalink / raw)
To: git; +Cc: davvid, j6t, hansenr, sbeller, simon, gitster
In-Reply-To: <20170110204202.21779-1-hansenr@google.com>
This is a step toward making the tests more independent so that if one
test fails it doesn't cause subsequent tests to fail.
Signed-off-by: Richard Hansen <hansenr@google.com>
---
t/t7610-mergetool.sh | 71 +++++++++++++++++++++++++++-------------------------
1 file changed, 37 insertions(+), 34 deletions(-)
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 550838a1c..f62ceffdc 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -145,6 +145,11 @@ test_expect_success 'custom mergetool' '
'
test_expect_success 'mergetool crlf' '
+ test_when_finished "git reset --hard" &&
+ # This test_config line must go after the above reset line so that
+ # core.autocrlf is unconfigured before reset runs. (The
+ # test_config command uses test_when_finished internally and
+ # test_when_finished is LIFO.)
test_config core.autocrlf true &&
git checkout -b test$test_count branch1 &&
test_must_fail git merge master >/dev/null 2>&1 &&
@@ -161,9 +166,7 @@ test_expect_success 'mergetool crlf' '
test "$(printf x | cat subdir/file3 -)" = "$(printf "master new sub\r\nx")" &&
git submodule update -N &&
test "$(cat submod/bar)" = "master submodule" &&
- git commit -m "branch1 resolved with mergetool - autocrlf" &&
- test_config core.autocrlf false &&
- git reset --hard
+ git commit -m "branch1 resolved with mergetool - autocrlf"
'
test_expect_success 'mergetool in subdir' '
@@ -194,6 +197,7 @@ test_expect_success 'mergetool on file in parent dir' '
'
test_expect_success 'mergetool skips autoresolved' '
+ test_when_finished "git reset --hard" &&
git checkout -b test$test_count branch1 &&
git submodule update -N &&
test_must_fail git merge master &&
@@ -202,8 +206,7 @@ test_expect_success 'mergetool skips autoresolved' '
( yes "d" | git mergetool file12 >/dev/null 2>&1 ) &&
( yes "l" | git mergetool submod >/dev/null 2>&1 ) &&
output="$(git mergetool --no-prompt)" &&
- test "$output" = "No files need merging" &&
- git reset --hard
+ test "$output" = "No files need merging"
'
test_expect_success 'mergetool merges all from subdir' '
@@ -223,6 +226,7 @@ test_expect_success 'mergetool merges all from subdir' '
'
test_expect_success 'mergetool skips resolved paths when rerere is active' '
+ test_when_finished "git reset --hard" &&
test_config rerere.enabled true &&
rm -rf .git/rr-cache &&
git checkout -b test$test_count branch1 &&
@@ -232,8 +236,7 @@ test_expect_success 'mergetool skips resolved paths when rerere is active' '
( yes "d" "d" | git mergetool --no-prompt >/dev/null 2>&1 ) &&
git submodule update -N &&
output="$(yes "n" | git mergetool --no-prompt)" &&
- test "$output" = "No files need merging" &&
- git reset --hard
+ test "$output" = "No files need merging"
'
test_expect_success 'conflicted stash sets up rerere' '
@@ -264,6 +267,7 @@ test_expect_success 'conflicted stash sets up rerere' '
'
test_expect_success 'mergetool takes partial path' '
+ test_when_finished "git reset --hard" &&
git reset --hard &&
test_config rerere.enabled false &&
git checkout -b test$test_count branch1 &&
@@ -272,11 +276,11 @@ test_expect_success 'mergetool takes partial path' '
( yes "" | git mergetool subdir ) &&
- test "$(cat subdir/file3)" = "master new sub" &&
- git reset --hard
+ test "$(cat subdir/file3)" = "master new sub"
'
test_expect_success 'mergetool delete/delete conflict' '
+ test_when_finished "git reset --hard HEAD" &&
git checkout move-to-c &&
test_must_fail git merge move-to-b &&
echo d | git mergetool a/a/file.txt &&
@@ -288,29 +292,30 @@ test_expect_success 'mergetool delete/delete conflict' '
git reset --hard HEAD &&
test_must_fail git merge move-to-b &&
! echo a | git mergetool a/a/file.txt &&
- ! test -f a/a/file.txt &&
- git reset --hard HEAD
+ ! test -f a/a/file.txt
'
test_expect_success 'mergetool produces no errors when keepBackup is used' '
+ test_when_finished "git reset --hard HEAD" &&
test_config mergetool.keepBackup true &&
test_must_fail git merge move-to-b &&
: >expect &&
echo d | git mergetool a/a/file.txt 2>actual &&
test_cmp expect actual &&
- ! test -d a &&
- git reset --hard HEAD
+ ! test -d a
'
test_expect_success 'mergetool honors tempfile config for deleted files' '
+ test_when_finished "git reset --hard HEAD" &&
test_config mergetool.keepTemporaries false &&
test_must_fail git merge move-to-b &&
echo d | git mergetool a/a/file.txt &&
- ! test -d a &&
- git reset --hard HEAD
+ ! test -d a
'
test_expect_success 'mergetool keeps tempfiles when aborting delete/delete' '
+ test_when_finished "git reset --hard HEAD" &&
+ test_when_finished "git clean -fdx" &&
test_config mergetool.keepTemporaries true &&
test_must_fail git merge move-to-b &&
! (echo a; echo n) | git mergetool a/a/file.txt &&
@@ -321,12 +326,11 @@ test_expect_success 'mergetool keeps tempfiles when aborting delete/delete' '
file_REMOTE_.txt
EOF
ls -1 a/a | sed -e "s/[0-9]*//g" >actual &&
- test_cmp expect actual &&
- git clean -fdx &&
- git reset --hard HEAD
+ test_cmp expect actual
'
test_expect_success 'deleted vs modified submodule' '
+ test_when_finished "git reset --hard HEAD" &&
git checkout -b test$test_count branch1 &&
git submodule update -N &&
mv submod submod-movedaside &&
@@ -391,8 +395,7 @@ test_expect_success 'deleted vs modified submodule' '
test "$(cat submod/bar)" = "master submodule" &&
output="$(git mergetool --no-prompt)" &&
test "$output" = "No files need merging" &&
- git commit -m "Merge resolved by keeping module" &&
- git reset --hard HEAD
+ git commit -m "Merge resolved by keeping module"
'
test_expect_success 'file vs modified submodule' '
@@ -479,6 +482,7 @@ test_expect_success 'submodule in subdirectory' '
git commit -m "add initial versions"
)
) &&
+ test_when_finished "rm -rf subdir/subdir_module" &&
git submodule add git://example.com/subsubmodule subdir/subdir_module &&
git add subdir/subdir_module &&
git commit -m "add submodule in subdirectory" &&
@@ -523,8 +527,7 @@ test_expect_success 'submodule in subdirectory' '
test "$(cat subdir/subdir_module/file15)" = "test$test_count.b" &&
git submodule update -N &&
test "$(cat subdir/subdir_module/file15)" = "test$test_count.a" &&
- git commit -m "branch1 resolved with mergetool" &&
- rm -rf subdir/subdir_module
+ git commit -m "branch1 resolved with mergetool"
'
test_expect_success 'directory vs modified submodule' '
@@ -578,34 +581,34 @@ test_expect_success 'directory vs modified submodule' '
'
test_expect_success 'file with no base' '
+ test_when_finished "git reset --hard master >/dev/null 2>&1" &&
git checkout -b test$test_count branch1 &&
test_must_fail git merge master &&
git mergetool --no-prompt --tool mybase -- both &&
>expected &&
- test_cmp both expected &&
- git reset --hard master >/dev/null 2>&1
+ test_cmp both expected
'
test_expect_success 'custom commands override built-ins' '
+ test_when_finished "git reset --hard master >/dev/null 2>&1" &&
git checkout -b test$test_count branch1 &&
test_config mergetool.defaults.cmd "cat \"\$REMOTE\" >\"\$MERGED\"" &&
test_config mergetool.defaults.trustExitCode true &&
test_must_fail git merge master &&
git mergetool --no-prompt --tool defaults -- both &&
echo master both added >expected &&
- test_cmp both expected &&
- git reset --hard master >/dev/null 2>&1
+ test_cmp both expected
'
test_expect_success 'filenames seen by tools start with ./' '
+ test_when_finished "git reset --hard master >/dev/null 2>&1" &&
git checkout -b test$test_count branch1 &&
test_config mergetool.writeToTemp false &&
test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
test_config mergetool.myecho.trustExitCode true &&
test_must_fail git merge master &&
git mergetool --no-prompt --tool myecho -- both >actual &&
- grep ^\./both_LOCAL_ actual >/dev/null &&
- git reset --hard master >/dev/null 2>&1
+ grep ^\./both_LOCAL_ actual >/dev/null
'
test_lazy_prereq MKTEMP '
@@ -614,6 +617,7 @@ test_lazy_prereq MKTEMP '
'
test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToTemp' '
+ test_when_finished "git reset --hard master >/dev/null 2>&1" &&
git checkout -b test$test_count branch1 &&
test_config mergetool.writeToTemp true &&
test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
@@ -621,11 +625,11 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT
test_must_fail git merge master &&
git mergetool --no-prompt --tool myecho -- both >actual &&
test_must_fail grep ^\./both_LOCAL_ actual >/dev/null &&
- grep /both_LOCAL_ actual >/dev/null &&
- git reset --hard master >/dev/null 2>&1
+ grep /both_LOCAL_ actual >/dev/null
'
test_expect_success 'diff.orderFile configuration is honored' '
+ test_when_finished "git reset --hard >/dev/null" &&
git checkout order-file-side2 &&
test_config diff.orderFile order-file &&
test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
@@ -640,10 +644,10 @@ test_expect_success 'diff.orderFile configuration is honored' '
EOF
git mergetool --no-prompt --tool myecho >output &&
git grep --no-index -h -A2 Merging: output >actual &&
- test_cmp expect actual &&
- git reset --hard >/dev/null
+ test_cmp expect actual
'
test_expect_success 'mergetool -Oorder-file is honored' '
+ test_when_finished "git reset --hard >/dev/null 2>&1" &&
test_config diff.orderFile order-file &&
test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
test_config mergetool.myecho.trustExitCode true &&
@@ -667,8 +671,7 @@ test_expect_success 'mergetool -Oorder-file is honored' '
EOF
git mergetool -Oorder-file --no-prompt --tool myecho >output &&
git grep --no-index -h -A2 Merging: output >actual &&
- test_cmp expect actual &&
- git reset --hard >/dev/null 2>&1
+ test_cmp expect actual
'
test_done
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH v5 04/14] t7610: Move setup code to the 'setup' test case.
From: Richard Hansen @ 2017-01-10 20:41 UTC (permalink / raw)
To: git; +Cc: davvid, j6t, hansenr, sbeller, simon, gitster
In-Reply-To: <20170110204202.21779-1-hansenr@google.com>
Multiple test cases depend on these hunks, so move them to the 'setup'
test case. This is a step toward making the tests more independent so
that if one test fails it doesn't cause subsequent tests to fail.
Signed-off-by: Richard Hansen <hansenr@google.com>
---
t/t7610-mergetool.sh | 65 ++++++++++++++++++++++++++++------------------------
1 file changed, 35 insertions(+), 30 deletions(-)
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 14090739f..550838a1c 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -55,6 +55,22 @@ test_expect_success 'setup' '
git rm file12 &&
git commit -m "branch1 changes" &&
+ git checkout -b delete-base branch1 &&
+ mkdir -p a/a &&
+ (echo one; echo two; echo 3; echo 4) >a/a/file.txt &&
+ git add a/a/file.txt &&
+ git commit -m"base file" &&
+ git checkout -b move-to-b delete-base &&
+ mkdir -p b/b &&
+ git mv a/a/file.txt b/b/file.txt &&
+ (echo one; echo two; echo 4) >b/b/file.txt &&
+ git commit -a -m"move to b" &&
+ git checkout -b move-to-c delete-base &&
+ mkdir -p c/c &&
+ git mv a/a/file.txt c/c/file.txt &&
+ (echo one; echo two; echo 3) >c/c/file.txt &&
+ git commit -a -m"move to c" &&
+
git checkout -b stash1 master &&
echo stash1 change file11 >file11 &&
git add file11 &&
@@ -86,6 +102,23 @@ test_expect_success 'setup' '
git rm file11 &&
git commit -m "master updates" &&
+ git clean -fdx &&
+ git checkout -b order-file-start master &&
+ echo start >a &&
+ echo start >b &&
+ git add a b &&
+ git commit -m start &&
+ git checkout -b order-file-side1 order-file-start &&
+ echo side1 >a &&
+ echo side1 >b &&
+ git add a b &&
+ git commit -m side1 &&
+ git checkout -b order-file-side2 order-file-start &&
+ echo side2 >a &&
+ echo side2 >b &&
+ git add a b &&
+ git commit -m side2 &&
+
git config merge.tool mytool &&
git config mergetool.mytool.cmd "cat \"\$REMOTE\" >\"\$MERGED\"" &&
git config mergetool.mytool.trustExitCode true &&
@@ -244,21 +277,7 @@ test_expect_success 'mergetool takes partial path' '
'
test_expect_success 'mergetool delete/delete conflict' '
- git checkout -b delete-base branch1 &&
- mkdir -p a/a &&
- (echo one; echo two; echo 3; echo 4) >a/a/file.txt &&
- git add a/a/file.txt &&
- git commit -m"base file" &&
- git checkout -b move-to-b delete-base &&
- mkdir -p b/b &&
- git mv a/a/file.txt b/b/file.txt &&
- (echo one; echo two; echo 4) >b/b/file.txt &&
- git commit -a -m"move to b" &&
- git checkout -b move-to-c delete-base &&
- mkdir -p c/c &&
- git mv a/a/file.txt c/c/file.txt &&
- (echo one; echo two; echo 3) >c/c/file.txt &&
- git commit -a -m"move to c" &&
+ git checkout move-to-c &&
test_must_fail git merge move-to-b &&
echo d | git mergetool a/a/file.txt &&
! test -f a/a/file.txt &&
@@ -607,26 +626,12 @@ test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToT
'
test_expect_success 'diff.orderFile configuration is honored' '
+ git checkout order-file-side2 &&
test_config diff.orderFile order-file &&
test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
test_config mergetool.myecho.trustExitCode true &&
echo b >order-file &&
echo a >>order-file &&
- git checkout -b order-file-start master &&
- echo start >a &&
- echo start >b &&
- git add a b &&
- git commit -m start &&
- git checkout -b order-file-side1 order-file-start &&
- echo side1 >a &&
- echo side1 >b &&
- git add a b &&
- git commit -m side1 &&
- git checkout -b order-file-side2 order-file-start &&
- echo side2 >a &&
- echo side2 >b &&
- git add a b &&
- git commit -m side2 &&
test_must_fail git merge order-file-side1 &&
cat >expect <<-\EOF &&
Merging:
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH v5 03/14] t7610: update branch names to match test number
From: Richard Hansen @ 2017-01-10 20:41 UTC (permalink / raw)
To: git; +Cc: davvid, j6t, hansenr, sbeller, simon, gitster
In-Reply-To: <20170110204202.21779-1-hansenr@google.com>
Rename the testNN branches so that NN matches the test number. This
should make it easier to troubleshoot test issues. Use $test_count to
keep this future-proof.
Signed-off-by: Richard Hansen <hansenr@google.com>
---
t/t7610-mergetool.sh | 82 ++++++++++++++++++++++++++--------------------------
1 file changed, 41 insertions(+), 41 deletions(-)
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 6d9f21511..14090739f 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -94,7 +94,7 @@ test_expect_success 'setup' '
'
test_expect_success 'custom mergetool' '
- git checkout -b test1 branch1 &&
+ git checkout -b test$test_count branch1 &&
git submodule update -N &&
test_must_fail git merge master >/dev/null 2>&1 &&
( yes "" | git mergetool both >/dev/null 2>&1 ) &&
@@ -113,7 +113,7 @@ test_expect_success 'custom mergetool' '
test_expect_success 'mergetool crlf' '
test_config core.autocrlf true &&
- git checkout -b test2 branch1 &&
+ git checkout -b test$test_count branch1 &&
test_must_fail git merge master >/dev/null 2>&1 &&
( yes "" | git mergetool file1 >/dev/null 2>&1 ) &&
( yes "" | git mergetool file2 >/dev/null 2>&1 ) &&
@@ -134,7 +134,7 @@ test_expect_success 'mergetool crlf' '
'
test_expect_success 'mergetool in subdir' '
- git checkout -b test3 branch1 &&
+ git checkout -b test$test_count branch1 &&
git submodule update -N &&
(
cd subdir &&
@@ -161,7 +161,7 @@ test_expect_success 'mergetool on file in parent dir' '
'
test_expect_success 'mergetool skips autoresolved' '
- git checkout -b test4 branch1 &&
+ git checkout -b test$test_count branch1 &&
git submodule update -N &&
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
@@ -192,7 +192,7 @@ test_expect_success 'mergetool merges all from subdir' '
test_expect_success 'mergetool skips resolved paths when rerere is active' '
test_config rerere.enabled true &&
rm -rf .git/rr-cache &&
- git checkout -b test5 branch1 &&
+ git checkout -b test$test_count branch1 &&
git submodule update -N &&
test_must_fail git merge master >/dev/null 2>&1 &&
( yes "l" | git mergetool --no-prompt submod >/dev/null 2>&1 ) &&
@@ -233,7 +233,7 @@ test_expect_success 'conflicted stash sets up rerere' '
test_expect_success 'mergetool takes partial path' '
git reset --hard &&
test_config rerere.enabled false &&
- git checkout -b test12 branch1 &&
+ git checkout -b test$test_count branch1 &&
git submodule update -N &&
test_must_fail git merge master &&
@@ -308,12 +308,12 @@ test_expect_success 'mergetool keeps tempfiles when aborting delete/delete' '
'
test_expect_success 'deleted vs modified submodule' '
- git checkout -b test6 branch1 &&
+ git checkout -b test$test_count branch1 &&
git submodule update -N &&
mv submod submod-movedaside &&
git rm --cached submod &&
git commit -m "Submodule deleted from branch" &&
- git checkout -b test6.a test6 &&
+ git checkout -b test$test_count.a test$test_count &&
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
@@ -329,7 +329,7 @@ test_expect_success 'deleted vs modified submodule' '
git commit -m "Merge resolved by keeping module" &&
mv submod submod-movedaside &&
- git checkout -b test6.b test6 &&
+ git checkout -b test$test_count.b test$test_count &&
git submodule update -N &&
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
@@ -343,9 +343,9 @@ test_expect_success 'deleted vs modified submodule' '
git commit -m "Merge resolved by deleting module" &&
mv submod-movedaside submod &&
- git checkout -b test6.c master &&
+ git checkout -b test$test_count.c master &&
git submodule update -N &&
- test_must_fail git merge test6 &&
+ test_must_fail git merge test$test_count &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
( yes "" | git mergetool both >/dev/null 2>&1 ) &&
@@ -359,9 +359,9 @@ test_expect_success 'deleted vs modified submodule' '
git commit -m "Merge resolved by deleting module" &&
mv submod.orig submod &&
- git checkout -b test6.d master &&
+ git checkout -b test$test_count.d master &&
git submodule update -N &&
- test_must_fail git merge test6 &&
+ test_must_fail git merge test$test_count &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
( yes "" | git mergetool both >/dev/null 2>&1 ) &&
@@ -377,14 +377,14 @@ test_expect_success 'deleted vs modified submodule' '
'
test_expect_success 'file vs modified submodule' '
- git checkout -b test7 branch1 &&
+ git checkout -b test$test_count branch1 &&
git submodule update -N &&
mv submod submod-movedaside &&
git rm --cached submod &&
echo not a submodule >submod &&
git add submod &&
git commit -m "Submodule path becomes file" &&
- git checkout -b test7.a branch1 &&
+ git checkout -b test$test_count.a branch1 &&
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
@@ -400,7 +400,7 @@ test_expect_success 'file vs modified submodule' '
git commit -m "Merge resolved by keeping module" &&
mv submod submod-movedaside &&
- git checkout -b test7.b test7 &&
+ git checkout -b test$test_count.b test$test_count &&
test_must_fail git merge master &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
@@ -413,11 +413,11 @@ test_expect_success 'file vs modified submodule' '
test "$output" = "No files need merging" &&
git commit -m "Merge resolved by keeping file" &&
- git checkout -b test7.c master &&
+ git checkout -b test$test_count.c master &&
rmdir submod && mv submod-movedaside submod &&
test ! -e submod.orig &&
git submodule update -N &&
- test_must_fail git merge test7 &&
+ test_must_fail git merge test$test_count &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
( yes "" | git mergetool both >/dev/null 2>&1 ) &&
@@ -430,10 +430,10 @@ test_expect_success 'file vs modified submodule' '
test "$output" = "No files need merging" &&
git commit -m "Merge resolved by keeping file" &&
- git checkout -b test7.d master &&
+ git checkout -b test$test_count.d master &&
rmdir submod && mv submod.orig submod &&
git submodule update -N &&
- test_must_fail git merge test7 &&
+ test_must_fail git merge test$test_count &&
test -n "$(git ls-files -u)" &&
( yes "" | git mergetool file1 file2 spaced\ name subdir/file3 >/dev/null 2>&1 ) &&
( yes "" | git mergetool both>/dev/null 2>&1 ) &&
@@ -448,7 +448,7 @@ test_expect_success 'file vs modified submodule' '
'
test_expect_success 'submodule in subdirectory' '
- git checkout -b test10 branch1 &&
+ git checkout -b test$test_count branch1 &&
git submodule update -N &&
(
cd subdir &&
@@ -464,52 +464,52 @@ test_expect_success 'submodule in subdirectory' '
git add subdir/subdir_module &&
git commit -m "add submodule in subdirectory" &&
- git checkout -b test10.a test10 &&
+ git checkout -b test$test_count.a test$test_count &&
git submodule update -N &&
(
cd subdir/subdir_module &&
git checkout -b super10.a &&
- echo test10.a >file15 &&
+ echo test$test_count.a >file15 &&
git add file15 &&
git commit -m "on branch 10.a"
) &&
git add subdir/subdir_module &&
- git commit -m "change submodule in subdirectory on test10.a" &&
+ git commit -m "change submodule in subdirectory on test$test_count.a" &&
- git checkout -b test10.b test10 &&
+ git checkout -b test$test_count.b test$test_count &&
git submodule update -N &&
(
cd subdir/subdir_module &&
git checkout -b super10.b &&
- echo test10.b >file15 &&
+ echo test$test_count.b >file15 &&
git add file15 &&
git commit -m "on branch 10.b"
) &&
git add subdir/subdir_module &&
- git commit -m "change submodule in subdirectory on test10.b" &&
+ git commit -m "change submodule in subdirectory on test$test_count.b" &&
- test_must_fail git merge test10.a >/dev/null 2>&1 &&
+ test_must_fail git merge test$test_count.a >/dev/null 2>&1 &&
(
cd subdir &&
( yes "l" | git mergetool subdir_module )
) &&
- test "$(cat subdir/subdir_module/file15)" = "test10.b" &&
+ test "$(cat subdir/subdir_module/file15)" = "test$test_count.b" &&
git submodule update -N &&
- test "$(cat subdir/subdir_module/file15)" = "test10.b" &&
+ test "$(cat subdir/subdir_module/file15)" = "test$test_count.b" &&
git reset --hard &&
git submodule update -N &&
- test_must_fail git merge test10.a >/dev/null 2>&1 &&
+ test_must_fail git merge test$test_count.a >/dev/null 2>&1 &&
( yes "r" | git mergetool subdir/subdir_module ) &&
- test "$(cat subdir/subdir_module/file15)" = "test10.b" &&
+ test "$(cat subdir/subdir_module/file15)" = "test$test_count.b" &&
git submodule update -N &&
- test "$(cat subdir/subdir_module/file15)" = "test10.a" &&
+ test "$(cat subdir/subdir_module/file15)" = "test$test_count.a" &&
git commit -m "branch1 resolved with mergetool" &&
rm -rf subdir/subdir_module
'
test_expect_success 'directory vs modified submodule' '
- git checkout -b test11 branch1 &&
+ git checkout -b test$test_count branch1 &&
mv submod submod-movedaside &&
git rm --cached submod &&
mkdir submod &&
@@ -537,9 +537,9 @@ test_expect_success 'directory vs modified submodule' '
test "$(cat submod/bar)" = "master submodule" &&
git reset --hard >/dev/null 2>&1 && rm -rf submod-movedaside &&
- git checkout -b test11.c master &&
+ git checkout -b test$test_count.c master &&
git submodule update -N &&
- test_must_fail git merge test11 &&
+ test_must_fail git merge test$test_count &&
test -n "$(git ls-files -u)" &&
( yes "l" | git mergetool submod ) &&
git submodule update -N &&
@@ -547,7 +547,7 @@ test_expect_success 'directory vs modified submodule' '
git reset --hard >/dev/null 2>&1 &&
git submodule update -N &&
- test_must_fail git merge test11 &&
+ test_must_fail git merge test$test_count &&
test -n "$(git ls-files -u)" &&
test ! -e submod.orig &&
( yes "r" | git mergetool submod ) &&
@@ -559,7 +559,7 @@ test_expect_success 'directory vs modified submodule' '
'
test_expect_success 'file with no base' '
- git checkout -b test13 branch1 &&
+ git checkout -b test$test_count branch1 &&
test_must_fail git merge master &&
git mergetool --no-prompt --tool mybase -- both &&
>expected &&
@@ -568,7 +568,7 @@ test_expect_success 'file with no base' '
'
test_expect_success 'custom commands override built-ins' '
- git checkout -b test14 branch1 &&
+ git checkout -b test$test_count branch1 &&
test_config mergetool.defaults.cmd "cat \"\$REMOTE\" >\"\$MERGED\"" &&
test_config mergetool.defaults.trustExitCode true &&
test_must_fail git merge master &&
@@ -579,7 +579,7 @@ test_expect_success 'custom commands override built-ins' '
'
test_expect_success 'filenames seen by tools start with ./' '
- git checkout -b test15 branch1 &&
+ git checkout -b test$test_count branch1 &&
test_config mergetool.writeToTemp false &&
test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
test_config mergetool.myecho.trustExitCode true &&
@@ -595,7 +595,7 @@ test_lazy_prereq MKTEMP '
'
test_expect_success MKTEMP 'temporary filenames are used with mergetool.writeToTemp' '
- git checkout -b test16 branch1 &&
+ git checkout -b test$test_count branch1 &&
test_config mergetool.writeToTemp true &&
test_config mergetool.myecho.cmd "echo \"\$LOCAL\"" &&
test_config mergetool.myecho.trustExitCode true &&
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* [PATCH v5 02/14] rev-parse doc: pass "--" to rev-parse in the --prefix example
From: Richard Hansen @ 2017-01-10 20:41 UTC (permalink / raw)
To: git; +Cc: davvid, j6t, hansenr, sbeller, simon, gitster
In-Reply-To: <20170110204202.21779-1-hansenr@google.com>
The "--" argument avoids "ambiguous argument: unknown revision or
path not in the working tree" errors when a pathname argument refers
to a non-existent file.
The "--" passed explicitly to set was removed because rev-parse
outputs the "--" argument that it is given.
Signed-off-by: Richard Hansen <hansenr@google.com>
---
Documentation/git-rev-parse.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index b6c6326cd..7241e9689 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -91,7 +91,8 @@ repository. For example:
----
prefix=$(git rev-parse --show-prefix)
cd "$(git rev-parse --show-toplevel)"
-eval "set -- $(git rev-parse --sq --prefix "$prefix" "$@")"
+# rev-parse provides the -- needed for 'set'
+eval "set $(git rev-parse --sq --prefix "$prefix" -- "$@")"
----
--verify::
--
2.11.0.390.gc69c2f50cf-goog
^ permalink raw reply related
* Re: [PATCH 2/4] t1000: modernize style
From: Stefan Beller @ 2017-01-10 20:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Brandon Williams, David Turner, git@vger.kernel.org
In-Reply-To: <xmqqr34ar6s1.fsf@gitster.mtv.corp.google.com>
On Tue, Jan 10, 2017 at 12:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> The preferred style in tests seems to be
>
> s/seems to be/is/;
If this is the only nit, mind to fix up the commit message locally?
(I was even unsure if we want to have this patch as part
of a larger series, as it is just refactoring for the sake of refactoring,
i.e. t1000 doesn't see a new test in this series, only t1001 does)
>
>>
>> test_expect_success 'short description, ended by 2 single quotes' '
>> here comes the test &&
>> and chains over many lines &&
>> ended by a quote
>> '
>
> Thanks. This is way overdue. During the time the script has been
> dormant for more than two years, we should have done this.
agreed.
^ permalink raw reply
* Re: [PATCH v10 03/20] ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
From: Junio C Hamano @ 2017-01-10 20:45 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, jacob.keller
In-Reply-To: <20170110084953.15890-4-Karthik.188@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> + if_then_else->condition_satisfied = 1;
> + } else if (if_then_else->cmp_status == COMPARE_UNEQUAL) {
Please, no space before tabs (locally fixed--no need to resend).
^ permalink raw reply
* Re: [PATCHv2 4/5] unpack-trees: factor file removal out of check_updates
From: Stefan Beller @ 2017-01-10 20:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, René Scharfe, git@vger.kernel.org
In-Reply-To: <xmqqd1fusmsu.fsf@gitster.mtv.corp.google.com>
On Tue, Jan 10, 2017 at 12:05 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> This makes check_updates shorter and easier to understand.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>
> I agree that 3/5 made it easier to understand by ejecting a block
> that is not essential to the functionality of the function out of
> it, making the remainder of the fuction about "removing gone files
> and then write out the modified files".
>
> The ejecting of the first half of these two operations, both are
> what this function is about, done by this step feels backwards. If
> anything, the "only do the actual working tree manipulation when not
> doing a dry-run and told to update" logic that must be in both are
> spread in two helper functions after step 5/5, and with the added
> boilerplate for these two helpers, the end result becomes _longer_
> to understand what is really going on when check_updates() is
> called.
>
> Is the original after step 3/5 too long and hard to understand?
It is ok to understand for the current state of the world,
so please drop 4 and 5 for now.
In a future, when
* working tree operations would learn about threading or
* working tree operations were aware of submodules
then steps of 4 and 5 would be longer, such that the check_updates
function may require further splitting up.
As far as I understand the operations now, a working tree operation
is done like this:
* First compute the new index, how the world would look like
(don't touch a thing in the working tree, it is like a complete
dry run, that just checks for potential loss of data, e.g.
untracked files, merge conflicts etc)
* remove all files to be marked for deletion
* add and update all files that are new or change content.
check_updates does the last two things listed here, which in the
grand scheme of things looked odd to me.
When introducing e.g. threaded checkout, then each of the functions
introduced in patch 4&5 would be one compartment, i.e. the function
remove_workingtree_files would need to have all its tasks finished
before we even queue up tasks for updating files, such that we
do not run into D/F conflicts racily.
So yeah, maybe this is too much future-visionary thinking, so please
drop 4/5; I can revive them if needed. I think I mainly did them
for my own clarity and understanding.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH v10 00/20] port branch.c to use ref-filter's printing options
From: Junio C Hamano @ 2017-01-10 20:51 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, jacob.keller
In-Reply-To: <20170110084953.15890-1-Karthik.188@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> index 81db67d74..08be8462c 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -95,13 +95,17 @@ refname::
> The name of the ref (the part after $GIT_DIR/).
> For a non-ambiguous short name of the ref append `:short`.
> The option core.warnAmbiguousRefs is used to select the strict
> + abbreviation mode. If `lstrip=<N>` is appended, strips `<N>`
> + slash-separated path components from the front of the refname
> + (e.g., `%(refname:lstrip=2)` turns `refs/tags/foo` into `foo` and
> + `%(refname:rstrip=2)` turns `refs/tags/foo` into `refs`).
I hiccupped while reading this, as the (e.g.) example talks about rstrip
that is not mentioned in the main text that is enhanced by the
example.
If `lstrip=<N>` (`rstrip=<N>`) is appended, strips `<N>`
slash-separated path components from the front (tail) of the
refname (e.g. `%(refname:lstrip=2)` ...
perhaps?
> + if `<N>` is a negative number, then only `<N>` path components
> + are left behind.
Begin the sentence with cap? I'd rephrase it a bit while at it if I
were doing this:
If `<N>` is a negative number, strip as many path components
as necessary from the specified end to leave `-<N>` path
components.
Other than the above, looks very good to me.
Thanks.
^ permalink raw reply
* Re: [PATCHv2 4/5] unpack-trees: factor file removal out of check_updates
From: Junio C Hamano @ 2017-01-10 21:34 UTC (permalink / raw)
To: Stefan Beller; +Cc: Jeff King, René Scharfe, git@vger.kernel.org
In-Reply-To: <CAGZ79kbXVCvZuj6rTckGwOfFtRSFpx9p611oKfFLAayTgB242Q@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> As far as I understand the operations now, a working tree operation
> is done like this:
>
> * First compute the new index, how the world would look like
> (don't touch a thing in the working tree, it is like a complete
> dry run, that just checks for potential loss of data, e.g.
> untracked files, merge conflicts etc)
> * remove all files to be marked for deletion
> * add and update all files that are new or change content.
>
> check_updates does the last two things listed here, which in the
> grand scheme of things looked odd to me.
In the grand scheme of things, the flow has always been understood
more like this two-step process:
- compute the result in core (rejecting an inconsistent result
before touching the working tree)
- reflect the result to the working tree
and the latter is done by check_updates().
Removing gone entries before instanciating possibly new entries is
done in order to make room where we can create a new path D/F in the
result, after removing an old path D [*1*]. But it is merely an
implementation detail of the latter, i.e. "reflect the result to the
working tree.
It is arguably true that check_updates() is not about "checking" but
is about "doing", hence is misnamed.
[Footnote]
*1* If the result computed in-core wants to create D/F, it must have
removal of D---otherwise the result is inconsistent.
^ permalink raw reply
* Re: [PATCH 1/2] diff: document behavior of relative diff.orderFile
From: Richard Hansen @ 2017-01-10 22:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <xmqqziiyr7e9.fsf@gitster.mtv.corp.google.com>
On 2017-01-10 15:23, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Richard Hansen <hansenr@google.com> writes:
>>
>>> On 2017-01-10 01:58, Jeff King wrote:
>>> ...
>>>> What happens in a bare repository?
>>>>
>>>> I'm guessing it's relative to the top-level of the repository,
>>>
>>> I just tried it out and it's relative to $PWD.
>>
>> That is understandable. When the user says
>>
>> $ cmd -O $file
>>
>> with any option -O that takes a filename, it is most natural if we
>> used $PWD/$file when $file is not absolute path.
>
> Ahh, ignore me in the above. The discussion is about the
> configuration variable, and I agree that being relative to GIT_DIR
> would have made more sense. In fact taking it as relative to PWD
> does not make any sense.
I'll stay silent regarding bare repositories then.
>
> We should have been a lot more careful when we added 6d8940b562
> ("diff: add diff.orderfile configuration variable", 2013-12-18), but
> it is too late to complain now.
>
> A related tangent.
>
> I wonder if anything that uses git_config_pathname() should be
> relative to GIT_DIR when it is not absolute.
I think so. (For bare repositories anyway; non-bare should be relative
to GIT_WORK_TREE.) Perhaps git_config_pathname() itself should convert
relative paths to absolute so that every pathname setting automatically
works without changing any calling code.
-Richard
^ permalink raw reply
* Re: [PATCH 2/4] t1000: modernize style
From: Junio C Hamano @ 2017-01-10 22:01 UTC (permalink / raw)
To: Stefan Beller; +Cc: Brandon Williams, David Turner, git@vger.kernel.org
In-Reply-To: <CAGZ79kbm4GRwNe7J_KP_V3eP8ZyAMEhOy-HL_ytHGtPoe19NPg@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Tue, Jan 10, 2017 at 12:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> The preferred style in tests seems to be
>>
>> s/seems to be/is/;
>
> If this is the only nit, mind to fix up the commit message locally?
Certainly. It wasn't even meant as a "nitpick". I was just
confirming your observation.
^ permalink raw reply
* Re: [PATCH v1] convert: add "status=delayed" to filter process protocol
From: Jakub Narębski @ 2017-01-10 22:11 UTC (permalink / raw)
To: Junio C Hamano, Lars Schneider; +Cc: git, Eric Wong, Jakub Narebski
In-Reply-To: <xmqqa8b115ll.fsf@gitster.mtv.corp.google.com>
W dniu 09.01.2017 o 00:42, Junio C Hamano pisze:
> larsxschneider@gmail.com writes:
>> From: Lars Schneider <larsxschneider@gmail.com>
>>
>> Some `clean` / `smudge` filters might require a significant amount of
>> time to process a single blob. During this process the Git checkout
>> operation is blocked and Git needs to wait until the filter is done to
>> continue with the checkout.
Lars, what is expected use case for this feature; that is when do you
think this problem may happen? Is it something that happened IRL?
>>
>> Teach the filter process protocol (introduced in edcc858) to accept the
>> status "delayed" as response to a filter request. Upon this response Git
>> continues with the checkout operation and asks the filter to process the
>> blob again after all other blobs have been processed.
>
> Hmm, I would have expected that the basic flow would become
>
> for each paths to be processed:
> convert-to-worktree to buf
> if not delayed:
> do the caller's thing to use buf
> else:
> remember path
>
> for each delayed paths:
> ensure filter process finished processing for path
> fetch the thing to buf from the process
> do the caller's thing to use buf
I would expect here to have a kind of event loop, namely
while there are delayed paths:
get path that is ready from filter
fetch the thing to buf (supporting "delayed")
if path done
do the caller's thing to use buf
(e.g. finish checkout path, eof convert, etc.)
We can either trust filter process to tell us when it finished sending
delayed paths, or keep list of paths that are being delayed in Git.
>
> and that would make quite a lot of sense. However, what is actually
> implemented is a bit disappointing from that point of view. While
> its first part is the same as above, the latter part instead does:
>
> for each delayed paths:
> checkout the path
>
> Presumably, checkout_entry() does the "ensure that the process is
> done converting" (otherwise the result is simply buggy), but what
> disappoints me is that this does not allow callers that call
> "convert-to-working-tree", whose interface is obtain the bytestream
> in-core in the working tree representation, given an object in the
> object-db representation in an in-core buffer, to _use_ the result
> of the conversion. The caller does not have a chance to even see
> the result as it is written straight to the filesystem, once it
> calls checkout_delayed_entries().
>
^ permalink raw reply
* Re: [PATCH 1/2] diff: document behavior of relative diff.orderFile
From: Junio C Hamano @ 2017-01-10 22:15 UTC (permalink / raw)
To: Richard Hansen; +Cc: Jeff King, git
In-Reply-To: <9daa70e4-82b0-a82a-67b9-e893546638a7@google.com>
Richard Hansen <hansenr@google.com> writes:
>> A related tangent.
>>
>> I wonder if anything that uses git_config_pathname() should be
>> relative to GIT_DIR when it is not absolute.
>
> I think so. (For bare repositories anyway; non-bare should be
> relative to GIT_WORK_TREE.) Perhaps git_config_pathname() itself
> should convert relative paths to absolute so that every pathname
> setting automatically works without changing any calling code.
Yes, that was what I was alluding to. We might have to wait until
major version boundary to do so, but I think that it is the sensible
way forward in the longer term to convert relative to absolute in
git_config_pathname().
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2] asciidoctor: fix user-manual to be built by `asciidoctor`
From: Junio C Hamano @ 2017-01-10 22:59 UTC (permalink / raw)
To: brian m. carlson
Cc: Jeff King, Lars Schneider, Johannes Schindelin, git,
마누엘
In-Reply-To: <20170107220834.uge5ksdr66asw27q@genre.crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
>> [1] I think we've also traditionally considered asciidoc to be the
>> definitive toolchain, and people using asciidoctor are free to
>> submit patches to make things work correctly in both places. I'm not
>> opposed to changing that attitude, as it seems like asciidoctor is
>> faster and more actively maintained these days. But I suspect our
>> build chain would need some improvements. Last time I tried building
>> with AsciiDoctor it involved a lot manual tweaking of Makefile
>> variables. It sounds like Dscho is doing it regularly, though. It
>> should probably work out of the box (with something like
>> USE_ASCIIDOCTOR=Yes) if we expect people to actually rely on it.
>
> Yes, that would probably be beneficial. I'll see if I can come up with
> some patches based on Dscho's work.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2] asciidoctor: fix user-manual to be built by `asciidoctor`
From: Junio C Hamano @ 2017-01-10 23:04 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git, 마누엘
In-Reply-To: <20170108032709.k43zmej5lxmcoj4o@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sat, Jan 07, 2017 at 02:03:30PM -0800, Junio C Hamano wrote:
>
>> Is that a longer way to say that the claim "... is designed as a
>> book" is false?
>>
>> > So I dunno. I really do think "article" is conceptually the most
>> > appropriate style, but I agree that there are some book-like things
>> > (like appendices).
>>
>> ... Yeah, I should have read forward first before starting to insert
>> my comments.
>
> To be honest, I'm not sure whether "book" versus "article" was really
> considered in the original writing. I think we can call it whichever
> produces the output we find most pleasing. I was mostly just pointing at
> there are some tradeoffs in the end result in flipping the type.
I understand.
And I tend to agree that the silliness you observed (like a t-o-c
for a one-section "chapter") is not quite welcome.
For now I queued only 2/2 which looked good. I won't object if
somebody else rerolls 1/2 to appease AsciiDoctor, but let's take an
obviously good bit first.
Thanks.
^ permalink raw reply
* Re: [PATCH v1] convert: add "status=delayed" to filter process protocol
From: Taylor Blau @ 2017-01-10 23:33 UTC (permalink / raw)
To: Jakub Narębski; +Cc: Junio C Hamano, Lars Schneider, git, Eric Wong
In-Reply-To: <ec8078ef-8ff2-d26f-ef73-5ef612737eee@gmail.com>
On Tue, Jan 10, 2017 at 11:11:01PM +0100, Jakub Narębski wrote:
> W dniu 09.01.2017 o 00:42, Junio C Hamano pisze:
> > larsxschneider@gmail.com writes:
> >> From: Lars Schneider <larsxschneider@gmail.com>
> >>
> >> Some `clean` / `smudge` filters might require a significant amount of
> >> time to process a single blob. During this process the Git checkout
> >> operation is blocked and Git needs to wait until the filter is done to
> >> continue with the checkout.
>
> Lars, what is expected use case for this feature; that is when do you
> think this problem may happen? Is it something that happened IRL?
>
> >>
> >> Teach the filter process protocol (introduced in edcc858) to accept the
> >> status "delayed" as response to a filter request. Upon this response Git
> >> continues with the checkout operation and asks the filter to process the
> >> blob again after all other blobs have been processed.
> >
> > Hmm, I would have expected that the basic flow would become
> >
> > for each paths to be processed:
> > convert-to-worktree to buf
> > if not delayed:
> > do the caller's thing to use buf
> > else:
> > remember path
> >
> > for each delayed paths:
> > ensure filter process finished processing for path
> > fetch the thing to buf from the process
> > do the caller's thing to use buf
>
> I would expect here to have a kind of event loop, namely
>
> while there are delayed paths:
> get path that is ready from filter
> fetch the thing to buf (supporting "delayed")
> if path done
> do the caller's thing to use buf
> (e.g. finish checkout path, eof convert, etc.)
>
> We can either trust filter process to tell us when it finished sending
> delayed paths, or keep list of paths that are being delayed in Git.
This makes a lot of sense to me. The "get path that is ready from filter" should
block until the filter has data that it is ready to send. This way Git isn't
wasting time in a busy-loop asking whether the filter has data ready to be sent.
It also means that if the filter has one large chunk that it's ready to write,
Git can work on that while the filter continues to process more data,
theoretically improving the performance of checkouts with many large delayed
objects.
>
> >
> > and that would make quite a lot of sense. However, what is actually
> > implemented is a bit disappointing from that point of view. While
> > its first part is the same as above, the latter part instead does:
> >
> > for each delayed paths:
> > checkout the path
> >
> > Presumably, checkout_entry() does the "ensure that the process is
> > done converting" (otherwise the result is simply buggy), but what
> > disappoints me is that this does not allow callers that call
> > "convert-to-working-tree", whose interface is obtain the bytestream
> > in-core in the working tree representation, given an object in the
> > object-db representation in an in-core buffer, to _use_ the result
> > of the conversion. The caller does not have a chance to even see
> > the result as it is written straight to the filesystem, once it
> > calls checkout_delayed_entries().
> >
>
In addition to the above, I'd also like to investigate adding a "no more items"
message into the filter protocol. This would be useful for filters that
batch delayed items into groups. In particular, if the batch size is `N`, and Git
sends `2N-1` items, the second batch will be under-filled. The filter on the
other end needs some mechanism to send the second batch, even though it hasn't
hit max capacity.
Specifically, this is how Git LFS implements object transfers for data it does
not have locally, but I'm sure that this sort of functionality would be useful
for other filter implementations as well.
--
Thanks,
Taylor Blau
ttaylorr@github.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox