* [PATCH nd/threaded-index-pack] index-pack: disable threading if NO_PREAD is defined
@ 2012-04-19 14:05 Nguyễn Thái Ngọc Duy
2012-04-19 20:44 ` Junio C Hamano
2012-04-20 6:25 ` Johannes Sixt
0 siblings, 2 replies; 4+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2012-04-19 14:05 UTC (permalink / raw)
To: git
Cc: kusmabite, Johannes Sixt, Ramsay Jones, Junio C Hamano,
Nguyễn Thái Ngọc Duy
NO_PREAD simulates pread() as a sequence of seek, read, seek in
compat/pread.c. The simulation is not thread-safe because another
thread could move the file offset away in the middle of pread
operation. Do not allow threading in that case.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/index-pack.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 847dbb3..c1c3c81 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -39,6 +39,11 @@ struct base_data {
int ofs_first, ofs_last;
};
+#if !defined(NO_PTHREADS) && defined(NO_PREAD)
+/* NO_PREAD uses compat/pread.c, which is not thread-safe. Disable threading. */
+#define NO_PTHREADS
+#endif
+
struct thread_local {
#ifndef NO_PTHREADS
pthread_t thread;
--
1.7.8.36.g69ee2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH nd/threaded-index-pack] index-pack: disable threading if NO_PREAD is defined
2012-04-19 14:05 [PATCH nd/threaded-index-pack] index-pack: disable threading if NO_PREAD is defined Nguyễn Thái Ngọc Duy
@ 2012-04-19 20:44 ` Junio C Hamano
2012-04-20 6:25 ` Johannes Sixt
1 sibling, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2012-04-19 20:44 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, kusmabite, Johannes Sixt, Ramsay Jones, Junio C Hamano
Thanks; will queue.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nd/threaded-index-pack] index-pack: disable threading if NO_PREAD is defined
2012-04-19 14:05 [PATCH nd/threaded-index-pack] index-pack: disable threading if NO_PREAD is defined Nguyễn Thái Ngọc Duy
2012-04-19 20:44 ` Junio C Hamano
@ 2012-04-20 6:25 ` Johannes Sixt
2012-04-20 19:49 ` Junio C Hamano
1 sibling, 1 reply; 4+ messages in thread
From: Johannes Sixt @ 2012-04-20 6:25 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, kusmabite, Ramsay Jones, Junio C Hamano
Am 4/19/2012 16:05, schrieb Nguyễn Thái Ngọc Duy:
> NO_PREAD simulates pread() as a sequence of seek, read, seek in
> compat/pread.c. The simulation is not thread-safe because another
> thread could move the file offset away in the middle of pread
> operation. Do not allow threading in that case.
Unsurprisingly, this fixes the breakage for me.
I used the attached patch to keep t9300 running when the breakage
was detected.
--- 8< ---
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] t9300-fast-import: avoid 'exit' in test_expect_success snippets
Exiting from a for-loop early using '|| break' does not propagate the
failure code, and for this reason, the tests used just 'exit'. But this
ends the test script with 'FATAL: Unexpected exit code 1' in the case of
a failed test.
Fix this by moving the loop into a shell function, from which we can
simply return early.
While at it, modernize the style of the affected test cases.
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
t/t9300-fast-import.sh | 88 +++++++++++++++++++++++++++++-------------------
1 file changed, 54 insertions(+), 34 deletions(-)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 0f5b5e5..8d7be67 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -24,6 +24,13 @@ head_c () {
' - "$1"
}
+verify_packs () {
+ for p in .git/objects/pack/*.pack
+ do
+ git verify-pack "$@" "$p" || return
+ done
+}
+
file2_data='file2
second line of EOF'
@@ -105,9 +112,10 @@ test_expect_success \
'A: create pack from stdin' \
'git fast-import --export-marks=marks.out <input &&
git whatchanged master'
-test_expect_success \
- 'A: verify pack' \
- 'for p in .git/objects/pack/*.pack;do git verify-pack $p||exit;done'
+
+test_expect_success 'A: verify pack' '
+ verify_packs
+'
cat >expect <<EOF
author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
@@ -252,9 +260,11 @@ test_expect_success \
'A: verify marks import does not crash' \
'git fast-import --import-marks=marks.out <input &&
git whatchanged verify--import-marks'
-test_expect_success \
- 'A: verify pack' \
- 'for p in .git/objects/pack/*.pack;do git verify-pack $p||exit;done'
+
+test_expect_success 'A: verify pack' '
+ verify_packs
+'
+
cat >expect <<EOF
:000000 100755 0000000000000000000000000000000000000000 7123f7f44e39be127c5eb701e5968176ee9d78b1 A copy-of-file2
EOF
@@ -514,9 +524,11 @@ test_expect_success \
'C: incremental import create pack from stdin' \
'git fast-import <input &&
git whatchanged branch'
-test_expect_success \
- 'C: verify pack' \
- 'for p in .git/objects/pack/*.pack;do git verify-pack $p||exit;done'
+
+test_expect_success 'C: verify pack' '
+ verify_packs
+'
+
test_expect_success \
'C: validate reuse existing blob' \
'test $newf = `git rev-parse --verify branch:file2/newf` &&
@@ -572,9 +584,10 @@ test_expect_success \
'D: inline data in commit' \
'git fast-import <input &&
git whatchanged branch'
-test_expect_success \
- 'D: verify pack' \
- 'for p in .git/objects/pack/*.pack;do git verify-pack $p||exit;done'
+
+test_expect_success 'D: verify pack' '
+ verify_packs
+'
cat >expect <<EOF
:000000 100755 0000000000000000000000000000000000000000 35a59026a33beac1569b1c7f66f3090ce9c09afc A newdir/exec.sh
@@ -618,9 +631,10 @@ test_expect_success 'E: rfc2822 date, --date-format=raw' '
test_expect_success \
'E: rfc2822 date, --date-format=rfc2822' \
'git fast-import --date-format=rfc2822 <input'
-test_expect_success \
- 'E: verify pack' \
- 'for p in .git/objects/pack/*.pack;do git verify-pack $p||exit;done'
+
+test_expect_success 'E: verify pack' '
+ verify_packs
+'
cat >expect <<EOF
author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> 1170778938 -0500
@@ -669,9 +683,10 @@ test_expect_success \
fi
fi
'
-test_expect_success \
- 'F: verify pack' \
- 'for p in .git/objects/pack/*.pack;do git verify-pack $p||exit;done'
+
+test_expect_success 'F: verify pack' '
+ verify_packs
+'
cat >expect <<EOF
tree `git rev-parse branch~1^{tree}`
@@ -705,9 +720,11 @@ INPUT_END
test_expect_success \
'G: non-fast-forward update forced' \
'git fast-import --force <input'
-test_expect_success \
- 'G: verify pack' \
- 'for p in .git/objects/pack/*.pack;do git verify-pack $p||exit;done'
+
+test_expect_success 'G: verify pack' '
+ verify_packs
+'
+
test_expect_success \
'G: branch changed, but logged' \
'test $old_branch != `git rev-parse --verify branch^0` &&
@@ -742,9 +759,10 @@ test_expect_success \
'H: deletall, add 1' \
'git fast-import <input &&
git whatchanged H'
-test_expect_success \
- 'H: verify pack' \
- 'for p in .git/objects/pack/*.pack;do git verify-pack $p||exit;done'
+
+test_expect_success 'H: verify pack' '
+ verify_packs
+'
cat >expect <<EOF
:100755 000000 f1fb5da718392694d0076d677d6d0e364c79b0bc 0000000000000000000000000000000000000000 D file2/newf
@@ -1857,9 +1875,10 @@ test_expect_success \
'Q: commit notes' \
'git fast-import <input &&
git whatchanged notes-test'
-test_expect_success \
- 'Q: verify pack' \
- 'for p in .git/objects/pack/*.pack;do git verify-pack $p||exit;done'
+
+test_expect_success 'Q: verify pack' '
+ verify_packs
+'
commit1=$(git rev-parse notes-test~2)
commit2=$(git rev-parse notes-test^)
@@ -2616,13 +2635,14 @@ test_expect_success \
'R: blob bigger than threshold' \
'test_create_repo R &&
git --git-dir=R/.git fast-import --big-file-threshold=1 <input'
-test_expect_success \
- 'R: verify created pack' \
- ': >verify &&
- for p in R/.git/objects/pack/*.pack;
- do
- git verify-pack -v $p >>verify || exit;
- done'
+
+test_expect_success 'R: verify created pack' '
+ (
+ cd R &&
+ verify_packs -v > ../verify
+ )
+'
+
test_expect_success \
'R: verify written objects' \
'git --git-dir=R/.git cat-file blob big-file:big1 >actual &&
--
1.7.10.1386.g7bd022
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH nd/threaded-index-pack] index-pack: disable threading if NO_PREAD is defined
2012-04-20 6:25 ` Johannes Sixt
@ 2012-04-20 19:49 ` Junio C Hamano
0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2012-04-20 19:49 UTC (permalink / raw)
To: Johannes Sixt
Cc: Nguyễn Thái Ngọc Duy, git, kusmabite, Ramsay Jones
Johannes Sixt <j.sixt@viscovery.net> writes:
> Am 4/19/2012 16:05, schrieb Nguyễn Thái Ngọc Duy:
>> NO_PREAD simulates pread() as a sequence of seek, read, seek in
>> compat/pread.c. The simulation is not thread-safe because another
>> thread could move the file offset away in the middle of pread
>> operation. Do not allow threading in that case.
>
> Unsurprisingly, this fixes the breakage for me.
>
> I used the attached patch to keep t9300 running when the breakage
> was detected.
>
> --- 8< ---
> From: Johannes Sixt <j6t@kdbg.org>
> Subject: [PATCH] t9300-fast-import: avoid 'exit' in test_expect_success snippets
>
> Exiting from a for-loop early using '|| break' does not propagate the
> failure code, and for this reason, the tests used just 'exit'. But this
> ends the test script with 'FATAL: Unexpected exit code 1' in the case of
> a failed test.
>
> Fix this by moving the loop into a shell function, from which we can
> simply return early.
Makes sense. If the original were written more readably, I may have
suggested to run the entire for loop in a subshell, but a helper
function is equally readable and with many identical checks, it is the
right way to do this.
Thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-04-20 19:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-19 14:05 [PATCH nd/threaded-index-pack] index-pack: disable threading if NO_PREAD is defined Nguyễn Thái Ngọc Duy
2012-04-19 20:44 ` Junio C Hamano
2012-04-20 6:25 ` Johannes Sixt
2012-04-20 19:49 ` Junio C Hamano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.