* [PATCH 13/15] t3030 (merge-recursive): use test_expect_code
From: Ramkumar Ramachandra @ 2011-12-07 19:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <1323286611-4806-1-git-send-email-artagnon@gmail.com>
Use test_expect_code in preference to repeatedly checking exit codes
by hand.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t3030-merge-recursive.sh | 72 ++++----------------------------------------
1 files changed, 6 insertions(+), 66 deletions(-)
diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh
index 55ef189..a5e3da7 100755
--- a/t/t3030-merge-recursive.sh
+++ b/t/t3030-merge-recursive.sh
@@ -285,17 +285,7 @@ test_expect_success 'merge-recursive simple' '
rm -fr [abcd] &&
git checkout -f "$c2" &&
- git merge-recursive "$c0" -- "$c2" "$c1"
- status=$?
- case "$status" in
- 1)
- : happy
- ;;
- *)
- echo >&2 "why status $status!!!"
- false
- ;;
- esac
+ test_expect_code 1 git merge-recursive "$c0" -- "$c2" "$c1"
'
test_expect_success 'merge-recursive result' '
@@ -334,17 +324,7 @@ test_expect_success 'merge-recursive remove conflict' '
rm -fr [abcd] &&
git checkout -f "$c1" &&
- git merge-recursive "$c0" -- "$c1" "$c5"
- status=$?
- case "$status" in
- 1)
- : happy
- ;;
- *)
- echo >&2 "why status $status!!!"
- false
- ;;
- esac
+ test_expect_code 1 git merge-recursive "$c0" -- "$c1" "$c5"
'
test_expect_success 'merge-recursive remove conflict' '
@@ -388,17 +368,7 @@ test_expect_success 'merge-recursive d/f conflict' '
git reset --hard &&
git checkout -f "$c1" &&
- git merge-recursive "$c0" -- "$c1" "$c4"
- status=$?
- case "$status" in
- 1)
- : happy
- ;;
- *)
- echo >&2 "why status $status!!!"
- false
- ;;
- esac
+ test_expect_code 1 git merge-recursive "$c0" -- "$c1" "$c4"
'
test_expect_success 'merge-recursive d/f conflict result' '
@@ -422,17 +392,7 @@ test_expect_success 'merge-recursive d/f conflict the other way' '
git reset --hard &&
git checkout -f "$c4" &&
- git merge-recursive "$c0" -- "$c4" "$c1"
- status=$?
- case "$status" in
- 1)
- : happy
- ;;
- *)
- echo >&2 "why status $status!!!"
- false
- ;;
- esac
+ test_expect_code 1 git merge-recursive "$c0" -- "$c4" "$c1"
'
test_expect_success 'merge-recursive d/f conflict result the other way' '
@@ -456,17 +416,7 @@ test_expect_success 'merge-recursive d/f conflict' '
git reset --hard &&
git checkout -f "$c1" &&
- git merge-recursive "$c0" -- "$c1" "$c6"
- status=$?
- case "$status" in
- 1)
- : happy
- ;;
- *)
- echo >&2 "why status $status!!!"
- false
- ;;
- esac
+ test_expect_code 1 git merge-recursive "$c0" -- "$c1" "$c6"
'
test_expect_success 'merge-recursive d/f conflict result' '
@@ -490,17 +440,7 @@ test_expect_success 'merge-recursive d/f conflict' '
git reset --hard &&
git checkout -f "$c6" &&
- git merge-recursive "$c0" -- "$c6" "$c1"
- status=$?
- case "$status" in
- 1)
- : happy
- ;;
- *)
- echo >&2 "why status $status!!!"
- false
- ;;
- esac
+ test_expect_code 1 git merge-recursive "$c0" -- "$c6" "$c1"
'
test_expect_success 'merge-recursive d/f conflict result' '
--
1.7.7.3
^ permalink raw reply related
* [PATCH 12/15] t3419 (rebase-patch-id): fix && chaining
From: Ramkumar Ramachandra @ 2011-12-07 19:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <1323286611-4806-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t3419-rebase-patch-id.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh
index bd8efaf..e70ac10 100755
--- a/t/t3419-rebase-patch-id.sh
+++ b/t/t3419-rebase-patch-id.sh
@@ -39,7 +39,7 @@ run()
}
test_expect_success 'setup' '
- git commit --allow-empty -m initial
+ git commit --allow-empty -m initial &&
git tag root
'
--
1.7.7.3
^ permalink raw reply related
* [PATCH 11/15] t3310 (notes-merge-manual-resolve): fix && chaining
From: Ramkumar Ramachandra @ 2011-12-07 19:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <1323286611-4806-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t3310-notes-merge-manual-resolve.sh | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/t/t3310-notes-merge-manual-resolve.sh b/t/t3310-notes-merge-manual-resolve.sh
index 4ec4d11..4367197 100755
--- a/t/t3310-notes-merge-manual-resolve.sh
+++ b/t/t3310-notes-merge-manual-resolve.sh
@@ -389,7 +389,7 @@ test_expect_success 'abort notes merge' '
test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
test_cmp /dev/null output &&
# m has not moved (still == y)
- test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)"
+ test "$(git rev-parse refs/notes/m)" = "$(cat pre_merge_y)" &&
# Verify that other notes refs has not changed (w, x, y and z)
verify_notes w &&
verify_notes x &&
@@ -525,9 +525,9 @@ EOF
test -f .git/NOTES_MERGE_WORKTREE/$commit_sha3 &&
test -f .git/NOTES_MERGE_WORKTREE/$commit_sha4 &&
# Refs are unchanged
- test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)"
- test "$(git rev-parse refs/notes/y)" = "$(git rev-parse NOTES_MERGE_PARTIAL^1)"
- test "$(git rev-parse refs/notes/m)" != "$(git rev-parse NOTES_MERGE_PARTIAL^1)"
+ test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
+ test "$(git rev-parse refs/notes/y)" = "$(git rev-parse NOTES_MERGE_PARTIAL^1)" &&
+ test "$(git rev-parse refs/notes/m)" != "$(git rev-parse NOTES_MERGE_PARTIAL^1)" &&
# Mention refs/notes/m, and its current and expected value in output
grep -q "refs/notes/m" output &&
grep -q "$(git rev-parse refs/notes/m)" output &&
@@ -545,7 +545,7 @@ test_expect_success 'resolve situation by aborting the notes merge' '
test_must_fail ls .git/NOTES_MERGE_* >output 2>/dev/null &&
test_cmp /dev/null output &&
# m has not moved (still == w)
- test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)"
+ test "$(git rev-parse refs/notes/m)" = "$(git rev-parse refs/notes/w)" &&
# Verify that other notes refs has not changed (w, x, y and z)
verify_notes w &&
verify_notes x &&
--
1.7.7.3
^ permalink raw reply related
* [PATCH 10/15] t3400 (rebase): fix && chaining
From: Ramkumar Ramachandra @ 2011-12-07 19:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <1323286611-4806-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t3400-rebase.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 6eaecec..c355533 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -172,8 +172,8 @@ test_expect_success 'fail when upstream arg is missing and not configured' '
test_expect_success 'default to @{upstream} when upstream arg is missing' '
git checkout -b default topic &&
- git config branch.default.remote .
- git config branch.default.merge refs/heads/master
+ git config branch.default.remote . &&
+ git config branch.default.merge refs/heads/master &&
git rebase &&
test "$(git rev-parse default~1)" = "$(git rev-parse master)"
'
--
1.7.7.3
^ permalink raw reply related
* [PATCH 09/15] t3418 (rebase-continue): fix && chaining
From: Ramkumar Ramachandra @ 2011-12-07 19:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <1323286611-4806-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t3418-rebase-continue.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 1e855cd..2680375 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -51,7 +51,7 @@ test_expect_success 'rebase --continue remembers merge strategy and options' '
test_commit "commit-new-file-F3-on-topic-branch" F3 32 &&
test_when_finished "rm -fr test-bin funny.was.run" &&
mkdir test-bin &&
- cat >test-bin/git-merge-funny <<-EOF
+ cat >test-bin/git-merge-funny <<-EOF &&
#!$SHELL_PATH
case "\$1" in --opt) ;; *) exit 2 ;; esac
shift &&
@@ -77,7 +77,7 @@ test_expect_success 'rebase --continue remembers merge strategy and options' '
test_expect_success 'rebase --continue remembers --rerere-autoupdate' '
rm -fr .git/rebase-* &&
git reset --hard commit-new-file-F3-on-topic-branch &&
- git checkout master
+ git checkout master &&
test_commit "commit-new-file-F3" F3 3 &&
git config rerere.enabled true &&
test_must_fail git rebase -m master topic &&
--
1.7.7.3
^ permalink raw reply related
* [PATCH 08/15] t3200 (branch): fix && chaining
From: Ramkumar Ramachandra @ 2011-12-07 19:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <1323286611-4806-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t3200-branch.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index bc73c20..7877290 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -22,7 +22,7 @@ test_expect_success \
test_expect_success \
'git branch --help should not have created a bogus branch' '
- git branch --help </dev/null >/dev/null 2>/dev/null;
+ git branch --help </dev/null >/dev/null 2>/dev/null &&
test_path_is_missing .git/refs/heads/--help
'
@@ -88,7 +88,7 @@ test_expect_success \
test_expect_success \
'git branch -m n/n n should work' \
'git branch -l n/n &&
- git branch -m n/n n
+ git branch -m n/n n &&
test_path_is_file .git/logs/refs/heads/n'
test_expect_success 'git branch -m o/o o should fail when o/p exists' '
--
1.7.7.3
^ permalink raw reply related
* [PATCH 07/15] t1510 (worktree): fix && chaining
From: Ramkumar Ramachandra @ 2011-12-07 19:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <1323286611-4806-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t1501-worktree.sh | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index 6384983..d9761bd 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -48,7 +48,7 @@ test_expect_success 'setup: helper for testing rev-parse' '
'
test_expect_success 'setup: core.worktree = relative path' '
- unset GIT_WORK_TREE;
+ unset GIT_WORK_TREE &&
GIT_DIR=repo.git &&
GIT_CONFIG="$(pwd)"/$GIT_DIR/config &&
export GIT_DIR GIT_CONFIG &&
@@ -89,7 +89,7 @@ test_expect_success 'subdir of work tree' '
'
test_expect_success 'setup: core.worktree = absolute path' '
- unset GIT_WORK_TREE;
+ unset GIT_WORK_TREE &&
GIT_DIR=$(pwd)/repo.git &&
GIT_CONFIG=$GIT_DIR/config &&
export GIT_DIR GIT_CONFIG &&
@@ -334,7 +334,7 @@ test_expect_success 'absolute pathspec should fail gracefully' '
'
test_expect_success 'make_relative_path handles double slashes in GIT_DIR' '
- >dummy_file
+ >dummy_file &&
echo git --git-dir="$(pwd)//repo.git" --work-tree="$(pwd)" add dummy_file &&
git --git-dir="$(pwd)//repo.git" --work-tree="$(pwd)" add dummy_file
'
--
1.7.7.3
^ permalink raw reply related
* [PATCH 06/15] t1511 (rev-parse-caret): fix && chaining
From: Ramkumar Ramachandra @ 2011-12-07 19:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <1323286611-4806-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t1511-rev-parse-caret.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t1511-rev-parse-caret.sh b/t/t1511-rev-parse-caret.sh
index e043cb7..eaefc77 100755
--- a/t/t1511-rev-parse-caret.sh
+++ b/t/t1511-rev-parse-caret.sh
@@ -6,7 +6,7 @@ test_description='tests for ref^{stuff}'
test_expect_success 'setup' '
echo blob >a-blob &&
- git tag -a -m blob blob-tag `git hash-object -w a-blob`
+ git tag -a -m blob blob-tag `git hash-object -w a-blob` &&
mkdir a-tree &&
echo moreblobs >a-tree/another-blob &&
git add . &&
--
1.7.7.3
^ permalink raw reply related
* [PATCH 05/15] t1510 (repo-setup): fix && chaining
From: Ramkumar Ramachandra @ 2011-12-07 19:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <1323286611-4806-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t1510-repo-setup.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh
index ec50a9a..80aedfc 100755
--- a/t/t1510-repo-setup.sh
+++ b/t/t1510-repo-setup.sh
@@ -603,7 +603,7 @@ test_expect_success '#22a: core.worktree = GIT_DIR = .git dir' '
# like case #6.
setup_repo 22a "$here/22a/.git" "" unset &&
- setup_repo 22ab . "" unset
+ setup_repo 22ab . "" unset &&
mkdir -p 22a/.git/sub 22a/sub &&
mkdir -p 22ab/.git/sub 22ab/sub &&
try_case 22a/.git unset . \
@@ -742,7 +742,7 @@ test_expect_success '#28: core.worktree and core.bare conflict (gitfile case)' '
# Case #29: GIT_WORK_TREE(+core.worktree) overrides core.bare (gitfile case).
test_expect_success '#29: setup' '
setup_repo 29 non-existent gitfile true &&
- mkdir -p 29/sub/sub 29/wt/sub
+ mkdir -p 29/sub/sub 29/wt/sub &&
(
cd 29 &&
GIT_WORK_TREE="$here/29" &&
--
1.7.7.3
^ permalink raw reply related
* [PATCH 04/15] t1007 (hash-object): fix && chaining
From: Ramkumar Ramachandra @ 2011-12-07 19:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <1323286611-4806-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t1007-hash-object.sh | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
index 6d52b82..316c60a 100755
--- a/t/t1007-hash-object.sh
+++ b/t/t1007-hash-object.sh
@@ -154,13 +154,13 @@ test_expect_success 'check that --no-filters option works with --stdin-paths' '
pop_repo
for args in "-w --stdin" "--stdin -w"; do
- push_repo
+ push_repo &&
test_expect_success "hash from stdin and write to database ($args)" '
test $example_sha1 = $(git hash-object $args < example)
- '
+ ' &&
- test_blob_exists $example_sha1
+ test_blob_exists $example_sha1 &&
pop_repo
done
@@ -176,20 +176,20 @@ test_expect_success "hash two files with names on stdin" '
'
for args in "-w --stdin-paths" "--stdin-paths -w"; do
- push_repo
+ push_repo &&
test_expect_success "hash two files with names on stdin and write to database ($args)" '
test "$sha1s" = "$(echo_without_newline "$filenames" | git hash-object $args)"
- '
+ ' &&
- test_blob_exists $hello_sha1
- test_blob_exists $example_sha1
+ test_blob_exists $hello_sha1 &&
+ test_blob_exists $example_sha1 &&
pop_repo
done
test_expect_success 'corrupt tree' '
- echo abc >malformed-tree
+ echo abc >malformed-tree &&
test_must_fail git hash-object -t tree malformed-tree
'
--
1.7.7.3
^ permalink raw reply related
* [PATCH 03/15] t1412 (reflog-loop): fix && chaining
From: Ramkumar Ramachandra @ 2011-12-07 19:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <1323286611-4806-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t1412-reflog-loop.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t1412-reflog-loop.sh b/t/t1412-reflog-loop.sh
index 647d888..3acd895 100755
--- a/t/t1412-reflog-loop.sh
+++ b/t/t1412-reflog-loop.sh
@@ -20,7 +20,7 @@ test_expect_success 'setup reflog with alternating commits' '
'
test_expect_success 'reflog shows all entries' '
- cat >expect <<-\EOF
+ cat >expect <<-\EOF &&
topic@{0} reset: moving to two
topic@{1} reset: moving to one
topic@{2} reset: moving to two
--
1.7.7.3
^ permalink raw reply related
* [PATCH 02/15] t1300 (repo-config): fix && chaining
From: Ramkumar Ramachandra @ 2011-12-07 19:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <1323286611-4806-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t1300-repo-config.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 51caff0..0690e0e 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -38,7 +38,7 @@ cat > expect << EOF
WhatEver = Second
EOF
test_expect_success 'similar section' '
- git config Cores.WhatEver Second
+ git config Cores.WhatEver Second &&
test_cmp expect .git/config
'
--
1.7.7.3
^ permalink raw reply related
* [PATCH 01/15] t1013 (loose-object-format): fix && chaining
From: Ramkumar Ramachandra @ 2011-12-07 19:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <1323286611-4806-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t1013-loose-object-format.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t1013-loose-object-format.sh b/t/t1013-loose-object-format.sh
index 0a9cedd..fbf5f2f 100755
--- a/t/t1013-loose-object-format.sh
+++ b/t/t1013-loose-object-format.sh
@@ -34,7 +34,7 @@ assert_blob_equals() {
}
test_expect_success setup '
- cp -R "$TEST_DIRECTORY/t1013/objects" .git/
+ cp -R "$TEST_DIRECTORY/t1013/objects" .git/ &&
git --version
'
--
1.7.7.3
^ permalink raw reply related
* Re: &&-chaining tester
From: Ramkumar Ramachandra @ 2011-12-07 19:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Git List
In-Reply-To: <20111207100858.GB13374@elie.hsd1.il.comcast.net>
Hi,
Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
>
>> Missing the chaining '&&' seems to be quite a common
>> error: I vaguely remember seeing a patch to detect this sometime ago.
>> Jonathan?
>
> [...]
> - fix all cases of broken &&-chaining. For extra bonus points,
> send out those patches, respond to reviews, and gently usher
> them into Junio's tree
Hm, involves a huge amount of janitorial work. Anyway, thanks for
pointing me in the right direction- here's a small start.
[What I used; for reference]
--
t/test-lib.sh | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index bdd9513..3bf75ac 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -472,7 +472,17 @@ test_eval_ () {
eval >&3 2>&4 "$*"
}
+check_command_chaining_ () {
+ eval >&3 2>&4 "(exit 189) && $1"
+ eval_chain_ret=$?
+ if test "$eval_chain_ret" != 189
+ then
+ echo '[CHAIN_BUG] in the test script: missing "&&" in test commands'
+ fi
+}
+
test_run_ () {
+ check_command_chaining_ "$1"
test_cleanup=:
expecting_failure=$2
test_eval_ "$1"
--
Thanks.
-- Ram
Ramkumar Ramachandra (15):
t1013 (loose-object-format): fix && chaining
t1300 (repo-config): fix && chaining
t1412 (reflog-loop): fix && chaining
t1007 (hash-object): fix && chaining
t1510 (repo-setup): fix && chaining
t1511 (rev-parse-caret): fix && chaining
t1510 (worktree): fix && chaining
t3200 (branch): fix && chaining
t3418 (rebase-continue): fix && chaining
t3400 (rebase): fix && chaining
t3310 (notes-merge-manual-resolve): fix && chaining
t3419 (rebase-patch-id): fix && chaining
t3030 (merge-recursive): use test_expect_code
t1006 (cat-file): use test_cmp
t3040 (subprojects-basic): modernize style
t/t1006-cat-file.sh | 53 +++---------
t/t1007-hash-object.sh | 16 ++--
t/t1013-loose-object-format.sh | 2 +-
t/t1300-repo-config.sh | 2 +-
t/t1412-reflog-loop.sh | 2 +-
t/t1501-worktree.sh | 6 +-
t/t1510-repo-setup.sh | 4 +-
t/t1511-rev-parse-caret.sh | 2 +-
t/t3030-merge-recursive.sh | 72 ++---------------
t/t3040-subprojects-basic.sh | 144 ++++++++++++++++----------------
t/t3200-branch.sh | 4 +-
t/t3310-notes-merge-manual-resolve.sh | 10 +-
t/t3400-rebase.sh | 4 +-
t/t3418-rebase-continue.sh | 4 +-
t/t3419-rebase-patch-id.sh | 2 +-
15 files changed, 119 insertions(+), 208 deletions(-)
--
1.7.7.3
^ permalink raw reply related
* Re: [PATCH 4/2] grep: turn off threading for non-worktree
From: Jeff King @ 2011-12-07 18:28 UTC (permalink / raw)
To: René Scharfe; +Cc: Thomas Rast, git, Eric Herman, Junio C Hamano
In-Reply-To: <4EDF9E53.7090702@lsrfire.ath.cx>
On Wed, Dec 07, 2011 at 06:11:47PM +0100, René Scharfe wrote:
> > $ time git grep --threads=8 'a.*b' HEAD >/dev/null
> > real 0m8.655s
> > user 0m23.817s
> > sys 0m0.480s
> [...]
>
> Ugh, right, Turbo Boost complicates matters.
>
> I don't understand the multiplied user time in the threaded case,
> though. Is it caused by busy-waiting? Thomas reported similar numbers
> earlier.
I think it's mostly the clock speed. This processor runs at 1.86GHz but
boosts to 3.2GHz. So we'd expect just the actual work to take close to
twice as long. Plus it's a quad-core with hyperthreading, so 8 threads
is going to mean two threads sharing each core, including cache (i.e.,
hyperthreading a core does not let you double performance, even though
it presents itself as an extra core).
And then you have context switching and lock overhead. So I can believe
that it takes 3x the CPU time to accomplish the task. In an ideal world,
it would be mitigated by having 8x the threads, but in this case, lock
contention brings us down to less than 3x, and it's a slight net loss.
-Peff
^ permalink raw reply
* Re: Odd issue - The Diffs That WILL NOT DIE.
From: Chris Patti @ 2011-12-07 18:16 UTC (permalink / raw)
To: Philippe Vaucher; +Cc: Carlos Martín Nieto, git
In-Reply-To: <CAGK7Mr7z6hqoda=pr3syzC5mO1+ZExMeD7sReeyRaYL5OMhemw@mail.gmail.com>
On Wed, Dec 7, 2011 at 12:20 PM, Philippe Vaucher
<philippe.vaucher@gmail.com> wrote:
>> 11:35][admin@Hiram-Abiff-2:~/src]$ rm -rf framework/
>> [11:44][admin@Hiram-Abiff-2:~/src]$ git clone
> ...snip...
>> [11:51][admin@Hiram-Abiff-2:~/src]$ cd framework/
>> [11:51][admin@Hiram-Abiff-2:~/src/framework(master)]$ git diff
> ...snip...
>
> Can you paste the output of "git status" right after the "cd framework"?
>
> Looks like you have some external process that goes and touches your
> file after the git clone, or that git fails to check out the files
> after cloning but isn't able to work out it failed to checkout those
> files.
>
> Philippe
FYI this behavior persists across reboots so this rogue process is
also restarting itself :)
Thanks,
-Chris
--
Christopher Patti - Geek At Large | GTalk: cpatti@gmail.com | AIM:
chrisfeohpatti | P: (260) 54PATTI
"Technology challenges art, art inspires technology." - John Lasseter, Pixar
^ permalink raw reply
* Re: [PATCH v2 3/3] grep: disable threading in all but worktree case
From: Jeff King @ 2011-12-07 18:10 UTC (permalink / raw)
To: René Scharfe; +Cc: git, Eric Herman, Junio C Hamano
In-Reply-To: <4EDF99BA.3040006@lsrfire.ath.cx>
On Wed, Dec 07, 2011 at 05:52:10PM +0100, René Scharfe wrote:
> > As a user, I would do:
> >
> > git grep --threads=1 ...
> >
> > if I wanted a single-threaded process. Instead, we actually spawn a
> > sub-thread and do all of the locking, which has a measurable cost:
>
> Yes, the difference is measurable, and that's exactly how I like it to
> be. :) A user can turn off threading with --threads=0 or (more
> intuitively) --no-threads. And we can quantify the overhead.
That seems acceptable to me if --threads is for speed-testing, but a
horrible interface if it is meant for end users who just want git to be
fast.
-Peff
^ permalink raw reply
* Re: Odd issue - The Diffs That WILL NOT DIE.
From: Chris Patti @ 2011-12-07 18:05 UTC (permalink / raw)
To: Philippe Vaucher; +Cc: Carlos Martín Nieto, git
In-Reply-To: <CAGK7Mr7z6hqoda=pr3syzC5mO1+ZExMeD7sReeyRaYL5OMhemw@mail.gmail.com>
On Wed, Dec 7, 2011 at 12:20 PM, Philippe Vaucher
<philippe.vaucher@gmail.com> wrote:
>> 11:35][admin@Hiram-Abiff-2:~/src]$ rm -rf framework/
>> [11:44][admin@Hiram-Abiff-2:~/src]$ git clone
> ...snip...
>> [11:51][admin@Hiram-Abiff-2:~/src]$ cd framework/
>> [11:51][admin@Hiram-Abiff-2:~/src/framework(master)]$ git diff
> ...snip...
>
> Can you paste the output of "git status" right after the "cd framework"?
>
> Looks like you have some external process that goes and touches your
> file after the git clone, or that git fails to check out the files
> after cloning but isn't able to work out it failed to checkout those
> files.
>
> Philippe
Sure.
--
[13:04][admin@Hiram-Abiff-2:~/src/framework(master)]$ git status
# On branch master
# Changes not staged for commit:
# (use "git add <file>..." to update what will be committed)
# (use "git checkout -- <file>..." to discard changes in working directory)
#
# modified: app/modules/Core/controllers/CloudSponge.php
#
no changes added to commit (use "git add" and/or "git commit -a")
[13:04][admin@Hiram-Abiff-2:~/src/framework(master)]$
I'll try rebooting the Mac to see if there's some rogue process afoot :)
-Chris
--
Christopher Patti - Geek At Large | GTalk: cpatti@gmail.com | AIM:
chrisfeohpatti | P: (260) 54PATTI
"Technology challenges art, art inspires technology." - John Lasseter, Pixar
^ permalink raw reply
* [PATCH] index-pack: eliminate unlimited recursion in get_delta_base()
From: Nguyễn Thái Ngọc Duy @ 2011-12-07 17:50 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <7vvcpthh97.fsf@alter.siamese.dyndns.org>
Revert the order of delta applying so that by the time a delta is
applied, its base is either non-delta or already inflated.
get_delta_base() is still recursive, but because base's data is always
ready, the inner get_delta_base() call never has any chance to call
itself again.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/index-pack.c | 30 +++++++++++++++++++++---------
1 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 0945adb..103e19c 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -508,10 +508,25 @@ static void *get_base_data(struct base_data *c)
{
if (!c->data) {
struct object_entry *obj = c->obj;
+ struct base_data **delta = NULL;
+ int delta_nr = 0, delta_alloc = 0;
- if (is_delta_type(obj->type)) {
- void *base = get_base_data(c->base);
- void *raw = get_data_from_pack(obj);
+ for (; is_delta_type(c->obj->type); c = c->base) {
+ ALLOC_GROW(delta, delta_nr + 1, delta_alloc);
+ delta[delta_nr++] = c;
+ }
+ if (!delta_nr) {
+ c->data = get_data_from_pack(obj);
+ c->size = obj->size;
+ base_cache_used += c->size;
+ prune_base_data(c);
+ }
+ for (; delta_nr > 0; delta_nr--) {
+ void *base, *raw;
+ c = delta[delta_nr - 1];
+ obj = c->obj;
+ base = get_base_data(c->base);
+ raw = get_data_from_pack(obj);
c->data = patch_delta(
base, c->base->size,
raw, obj->size,
@@ -519,13 +534,10 @@ static void *get_base_data(struct base_data *c)
free(raw);
if (!c->data)
bad_object(obj->idx.offset, "failed to apply delta");
- } else {
- c->data = get_data_from_pack(obj);
- c->size = obj->size;
+ base_cache_used += c->size;
+ prune_base_data(c);
}
-
- base_cache_used += c->size;
- prune_base_data(c);
+ free(delta);
}
return c->data;
}
--
1.7.8.36.g69ee2
^ permalink raw reply related
* Re: Odd issue - The Diffs That WILL NOT DIE.
From: Philippe Vaucher @ 2011-12-07 17:20 UTC (permalink / raw)
To: Chris Patti; +Cc: Carlos Martín Nieto, git
In-Reply-To: <CAJ8P3RCPt9Kwi1F7_TEkZQhkm1mwR_TFKhYszS5LL50kXU8oNQ@mail.gmail.com>
> 11:35][admin@Hiram-Abiff-2:~/src]$ rm -rf framework/
> [11:44][admin@Hiram-Abiff-2:~/src]$ git clone
...snip...
> [11:51][admin@Hiram-Abiff-2:~/src]$ cd framework/
> [11:51][admin@Hiram-Abiff-2:~/src/framework(master)]$ git diff
...snip...
Can you paste the output of "git status" right after the "cd framework"?
Looks like you have some external process that goes and touches your
file after the git clone, or that git fails to check out the files
after cloning but isn't able to work out it failed to checkout those
files.
Philippe
^ permalink raw reply
* Re: [PATCH 4/2] grep: turn off threading for non-worktree
From: René Scharfe @ 2011-12-07 17:11 UTC (permalink / raw)
To: Jeff King; +Cc: Thomas Rast, git, Eric Herman, Junio C Hamano
In-Reply-To: <20111207044242.GB10765@sigill.intra.peff.net>
Am 07.12.2011 05:42, schrieb Jeff King:
> On Wed, Dec 07, 2011 at 12:01:37AM +0100, René Scharfe wrote:
>
>> Reading of git objects needs to be protected by an exclusive lock
>> and cannot be parallelized. Searching the read buffers can be done
>> in parallel, but for simple expressions threading is a net loss due
>> to its overhead, as measured by Thomas. Turn it off unless we're
>> searching in the worktree.
>
> Based on my earlier numbers, I was going to complain that we should
> also be checking the "simple expressions" assumption here, as time spent
> in the actual regex might be important.
>
> However, after trying to repeat my experiment, I think the numbers I
> posted earlier were misleading. For example, using my "more complex"
> regex of 'a.*b':
>
> $ time git grep --threads=8 'a.*b' HEAD >/dev/null
> real 0m8.655s
> user 0m23.817s
> sys 0m0.480s
>
> Look at that sweet, sweet parallelism. It's a quad-core with
> hyperthreading, so we're not getting the 8x speedup we might hope for
> (presumably due to lock contention on extracting blobs), but hey, 3x
> isn't bad. Except, wait:
>
> $ time git grep --threads=0 'a.*b' HEAD >/dev/null
> real 0m7.651s
> user 0m7.600s
> sys 0m0.048s
>
> We can get 1x on a single core, but the total time is lower! This
> processor is an i7 with "turbo boost", which means it clocks higher in
> single-core mode than when multiple cores are active. So the numbers I
> posted earlier were misleading. Yes, we got parallelism, but at the cost
> of knocking the clock speed down for a net loss.
Ugh, right, Turbo Boost complicates matters.
I don't understand the multiplied user time in the threaded case,
though. Is it caused by busy-waiting? Thomas reported similar numbers
earlier.
> The sweet spot for me seems to be:
>
> $ time git grep --threads=2 'a.*b' HEAD >/dev/null
> real 0m6.303s
> user 0m11.129s
> sys 0m0.220s
>
> I'd be curious to see results from somebody with a quad-core (or more)
> without turbo boost; I suspect that threading may have more benefit
> there, even though we have some lock contention for blobs.
It would be nice if we could come up with simple rules to calculate
defaults for the number of threads on a given run. Users shouldn't have
to specify this option normally. And it would be good if these rules
didn't require a list of all CPUs known to git. :)
>> --- a/builtin/grep.c
>> +++ b/builtin/grep.c
>> @@ -1048,7 +1048,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>> nr_threads = 0;
>> #else
>> if (nr_threads == -1)
>> - nr_threads = (online_cpus() > 1) ? THREADS : 0;
>> + nr_threads = (online_cpus() > 1 && !list.nr) ? THREADS : 0;
>>
>> if (nr_threads > 0) {
>> opt.use_threads = 1;
>
> This doesn't kick in for "--cached", which has the same performance
> characteristics as grepping a tree. I think you want to add "&& !cached" to
> the conditional.
Oh, yes.
René
^ permalink raw reply
* Re: [PATCH 4/2] grep: turn off threading for non-worktree
From: René Scharfe @ 2011-12-07 17:00 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Eric Herman, Junio C Hamano
In-Reply-To: <201112070912.54766.trast@student.ethz.ch>
Am 07.12.2011 09:12, schrieb Thomas Rast:
> René Scharfe wrote:
>> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
>> index 47ac188..e981a9b 100644
>> --- a/Documentation/git-grep.txt
>> +++ b/Documentation/git-grep.txt
>> @@ -228,8 +228,9 @@ OPTIONS
>> there is a match and with non-zero status when there isn't.
>>
>> --threads <n>::
>> + Run <n> search threads in parallel. Default is 8 when searching
>> + the worktree and 0 otherwise. This option is ignored if git was
>> + built without support for POSIX threads.
> [...]
>> - nr_threads = (online_cpus() > 1) ? THREADS : 0;
>> + nr_threads = (online_cpus() > 1 && !list.nr) ? THREADS : 0;
>
> It would be more consistent to stick to the pack.threads convention
> where 0 means "all of my cores", so to disable threading the user
> would set the number of threads to 1. Or were you trying to measure
> the contention between the worker thread and the add_work() thread?
Yes, indeed, the cost for the threading overhead does interest me. The
documentation should perhaps mention --no-threads explicitly to avoid
confusion.
Currently there is no way to specify "as many threads as there are
cores" here. Previous measurements indicated that it wasn't too useful,
however, because I/O parallelism was beneficial even for machines with
less than eight cores and more threads didn't pay off.
René
^ permalink raw reply
* Re: [PATCH v2 3/3] grep: disable threading in all but worktree case
From: René Scharfe @ 2011-12-07 16:54 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Eric Herman, Junio C Hamano
In-Reply-To: <201112070911.08079.trast@student.ethz.ch>
Am 07.12.2011 09:11, schrieb Thomas Rast:
> René Scharfe wrote:
>> Am 02.12.2011 17:15, schrieb René Scharfe:
>>> How about adding a parameter to control the number of threads
>>> (--threads?) instead that defaults to eight (or five) for the worktree
>>> and one for the rest? That would also make benchmarking easier.
>>
>> Like this:
>>
>> -- >8 --
>> Subject: grep: add parameter --threads
>>
>> Allow the number of threads to be specified by the user. This makes
>> benchmarking the performance impact of different numbers of threads
>> much easier.
>
> Sounds good, though in the end we would also want to have a config
> variable for the poor OS X users who have to tune their threads
> *down*... :-)
We could set different defaults for different platforms..
René
^ permalink raw reply
* Re: Odd issue - The Diffs That WILL NOT DIE.
From: Chris Patti @ 2011-12-07 16:54 UTC (permalink / raw)
To: Carlos Martín Nieto, Chris Patti, git
In-Reply-To: <20111206215102.GA3654@centaur.lab.cmartin.tk>
On Tue, Dec 6, 2011 at 4:51 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
> On Tue, Dec 06, 2011 at 04:43:50PM -0500, Chris Patti wrote:
>> I have a Homebrew installed version if Git 1.7.8 running on OSX Lion.
>>
>> I'm seeing a very odd issue where these diffs I didn't create keep
>> recurring in a particular repository.
>
> Which diffs? You haven't given us any? What files does this happen
> with? Do they have any peculiarities?
>
> If these are files with non-ASCII filenames, then you're hitting a
> misfeature of the HFS+ filesystem (it lies when git asks it about
> files).
>
>>
>> I've tried:
>>
>> * Nuking the repo and re-cloning, cloning into a totally different
>> containing directory
>> * git reset --hard, git checkout -- of the offending file supposedly
>> containing the diffs
>>
>> Is there some sort of uber persistent local cache that's bound to the
>> remote repository?
>
> The remote repository shouldn't have anything to do with this.
>
> cmn
OK. Let me give you a very specific series of commands, sorry about
the poor question / report (Not convinced it's a bug, probably pilot
error?)
If my understanding of the way Git works is correct, there should be
NO pending diffs in a freshly cloned repository, yes?
Note that I tried git diff and git diff -w below with identical results:
---
11:35][admin@Hiram-Abiff-2:~/src]$ rm -rf framework/
[11:37][admin@Hiram-Abiff-2:~/src]$
[11:44][admin@Hiram-Abiff-2:~/src]$ git clone
ssh://git.bluestatedigital.com/home/git/framework.git
Cloning into 'framework'...
remote: Counting objects: 378540, done.
remote: Compressing objects: 100% (100469/100469), done.
remote: Total 378540 (delta 261046), reused 374685 (delta 258447)
Receiving objects: 100% (378540/378540), 148.33 MiB | 2.08 MiB/s, done.
Resolving deltas: 100% (261046/261046), done.
[11:51][admin@Hiram-Abiff-2:~/src]$ cd framework/
[11:51][admin@Hiram-Abiff-2:~/src/framework(master)]$ git diff
diff --git a/app/modules/Core/controllers/CloudSponge.php b/app/modules/Core/con
index 615a7b3..911d456 100644
--- a/app/modules/Core/controllers/CloudSponge.php
+++ b/app/modules/Core/controllers/CloudSponge.php
@@ -1,229 +1,71 @@
-<?php
-
-require_once "utils/blue_mailer/queue_mailer.class.php";
-
-class Core_CloudSponge_Controller extends Core_PageBase_Controller
-{
- private $_config;
- private $_tsFactory;
- private $_emailValidator;
- private $_formFactory;
- private $_rowBuilderFactory;
-
- public function __construct
- (
- Blue_Config $config,
- Blue_SecureTimestamp_Factory $tsFactory,
- Blue_Email_Validator $emailValidator,
- Framework_Ui_Form_Factory $formFactory,
[11:51][admin@Hiram-Abiff-2:~/src/framework(master)]$ git diff -w
diff --git a/app/modules/Core/controllers/CloudSponge.php b/app/modules/Core/con
index 615a7b3..911d456 100644
--- a/app/modules/Core/controllers/CloudSponge.php
+++ b/app/modules/Core/controllers/CloudSponge.php
@@ -1,229 +1,71 @@
-<?php
-
-require_once "utils/blue_mailer/queue_mailer.class.php";
-
-class Core_CloudSponge_Controller extends Core_PageBase_Controller
-{
- private $_config;
- private $_tsFactory;
- private $_emailValidator;
- private $_formFactory;
- private $_rowBuilderFactory;
-
- public function __construct
- (
- Blue_Config $config,
- Blue_SecureTimestamp_Factory $tsFactory,
- Blue_Email_Validator $emailValidator,
- Framework_Ui_Form_Factory $formFactory,
[11:52][admin@Hiram-Abiff-2:~/src/framework(master)]$
--
Christopher Patti - Geek At Large | GTalk: cpatti@gmail.com | AIM:
chrisfeohpatti | P: (260) 54PATTI
"Technology challenges art, art inspires technology." - John Lasseter, Pixar
^ permalink raw reply related
* Re: [PATCH v2 3/3] grep: disable threading in all but worktree case
From: René Scharfe @ 2011-12-07 16:52 UTC (permalink / raw)
To: Jeff King; +Cc: git, Eric Herman, Junio C Hamano
In-Reply-To: <20111207042431.GA10765@sigill.intra.peff.net>
Am 07.12.2011 05:24, schrieb Jeff King:
> On Tue, Dec 06, 2011 at 11:48:26PM +0100, René Scharfe wrote:
>
>> #ifndef NO_PTHREADS
>> - if (use_threads) {
>> + if (nr_threads > 0) {
>> grep_sha1_async(opt, name, sha1);
>> return 0;
>> } else
>
> Should this be "if (nr_threads > 1)"?
>
> As a user, I would do:
>
> git grep --threads=1 ...
>
> if I wanted a single-threaded process. Instead, we actually spawn a
> sub-thread and do all of the locking, which has a measurable cost:
Yes, the difference is measurable, and that's exactly how I like it to
be. :) A user can turn off threading with --threads=0 or (more
intuitively) --no-threads. And we can quantify the overhead.
> $ time git grep --threads=0 SIMPLE HEAD >/dev/null
> real 0m2.994s
> user 0m2.932s
> sys 0m0.060s
>
> $ time git grep --threads=1 SIMPLE HEAD >/dev/null
> real 0m3.407s
> user 0m3.392s
> sys 0m0.140s
>
> Should --threads=1 be equivalent to --threads=0?
We can do that if there's another way to calculate this difference, or
if it is not useful to know. I find your results interesting at least,
though. :)
René
^ 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