git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j.sixt@viscovery.net>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: git@vger.kernel.org, kusmabite@gmail.com,
	Ramsay Jones <ramsay@ramsay1.demon.co.uk>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH nd/threaded-index-pack] index-pack: disable threading if NO_PREAD is defined
Date: Fri, 20 Apr 2012 08:25:09 +0200	[thread overview]
Message-ID: <4F910145.5030102@viscovery.net> (raw)
In-Reply-To: <1334844329-24557-1-git-send-email-pclouds@gmail.com>

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

  parent reply	other threads:[~2012-04-20  6:25 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2012-04-20 19:49   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F910145.5030102@viscovery.net \
    --to=j.sixt@viscovery.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kusmabite@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=ramsay@ramsay1.demon.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).