git-apply tests need to be portable

PATCH New Don Zickus 3 participants 3 messages Jun 15, 2016

Share & tools

Don Zickus

author

2016-06-15 22:44 UTC #0 lore raw

PATCH 1/1 git-apply tests need to be portable

When I created the tests for my git-apply patch, I accidently used the '-i' flag for sed. Not all versions of sed handle this flag, so I converted those instances to output to a temp file and move that temp file back to the original file.

Thanks to Jim Meyering for noticing this!

---

Junio, I am not sure if you want me to resend the whole patch again or is a small update like this preferred. I am indifferent either way.

Cheers,
Don

---
 t/t4127-apply-same-fn.sh |   33 ++++++++++++++++++++++-----------
 1 files changed, 22 insertions(+), 11 deletions(-)

t/t4127-apply-same-fn.sh +22 ~~-11~~

index 2a6ed77..2726a29 100755
@@ -14,10 +14,12 @@ test_expect_success setup '
 	git commit -m initial
 '
 test_expect_success 'apply same filename with independent changes' '
-	sed -i -e "s/^d/z/" same_fn &&
+	sed -e "s/^d/z/" same_fn > f &&
+	mv f same_fn &&
 	git diff > patch0 &&
 	git add same_fn &&
-	sed -i -e "s/^i/y/" same_fn &&
+	sed -e "s/^i/y/" same_fn > f &&
+	mv f same_fn &&
 	git diff >> patch0 &&
 	cp same_fn same_fn2 &&
 	git reset --hard &&

@@ -27,10 +29,12 @@ test_expect_success 'apply same filename with independent changes' '
 
 test_expect_success 'apply same filename with overlapping changes' '
 	git reset --hard
-	sed -i -e "s/^d/z/" same_fn &&
+	sed -e "s/^d/z/" same_fn > f &&
+	mv f same_fn &&
 	git diff > patch0 &&
 	git add same_fn &&
-	sed -i -e "s/^e/y/" same_fn &&
+	sed -e "s/^e/y/" same_fn > f &&
+	mv f same_fn &&
 	git diff >> patch0 &&
 	cp same_fn same_fn2 &&
 	git reset --hard &&

@@ -41,10 +45,12 @@ test_expect_success 'apply same filename with overlapping changes' '
 test_expect_success 'apply same new filename after rename' '
 	git reset --hard
 	git mv same_fn new_fn
-	sed -i -e "s/^d/z/" new_fn &&
+	sed -e "s/^d/z/" new_fn > f &&
+	mv f new_fn &&
 	git add new_fn &&
 	git diff -M --cached > patch1 &&
-	sed -i -e "s/^e/y/" new_fn &&
+	sed -e "s/^e/y/" new_fn > f &&
+	mv f new_fn &&
 	git diff >> patch1 &&
 	cp new_fn new_fn2 &&
 	git reset --hard &&

@@ -55,11 +61,13 @@ test_expect_success 'apply same new filename after rename' '
 test_expect_success 'apply same old filename after rename -- should fail.' '
 	git reset --hard
 	git mv same_fn new_fn
-	sed -i -e "s/^d/z/" new_fn &&
+	sed -e "s/^d/z/" new_fn > f &&
+	mv f new_fn &&
 	git add new_fn &&
 	git diff -M --cached > patch1 &&
 	git mv new_fn same_fn
-	sed -i -e "s/^e/y/" same_fn &&
+	sed -e "s/^e/y/" same_fn > f &&
+	mv f same_fn &&
 	git diff >> patch1 &&
 	git reset --hard &&
 	test_must_fail git apply patch1

@@ -68,15 +76,18 @@ test_expect_success 'apply same old filename after rename -- should fail.' '
 test_expect_success 'apply A->B (rename), C->A (rename), A->A -- should pass.' '
 	git reset --hard
 	git mv same_fn new_fn
-	sed -i -e "s/^d/z/" new_fn &&
+	sed -e "s/^d/z/" new_fn > f &&
+	mv f new_fn &&
 	git add new_fn &&
 	git diff -M --cached > patch1 &&
 	git commit -m "a rename" &&
 	git mv other_fn same_fn
-	sed -i -e "s/^e/y/" same_fn &&
+	sed -e "s/^e/y/" same_fn > f &&
+	mv f same_fn &&
 	git add same_fn &&
 	git diff -M --cached >> patch1 &&
-	sed -i -e "s/^g/x/" same_fn &&
+	sed -e "s/^g/x/" same_fn > f &&
+	mv f same_fn &&
 	git diff >> patch1 &&
 	git reset --hard HEAD^ &&
 	git apply patch1

Signature · 1 line

1.5.6.rc2.48.g13da

Johannes Sixt

2016-06-15 22:44 UTC #1 lore raw

PATCH 1/1 t4127-apply-same-fn: Avoid sed -i

From: Johannes Sixt <[email protected]>

Signed-off-by Johannes Sixt <[email protected]>

---

Quoted 4 lines · Don Zickus

When I created the tests for my git-apply patch, I accidently used the '-i' flag for sed. Not all versions of sed handle this flag, so I converted those instances to output to a temp file and move that temp file back to the original file.

I had this alternate patch in my tree, but forgot to submit it.

-- Hannes

 t/t4127-apply-same-fn.sh |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)

t/t4127-apply-same-fn.sh +16 ~~-11~~

index 2a6ed77..1f859dd 100755
@@ -4,6 +4,11 @@ test_description='apply same filename'
 
 . ./test-lib.sh
 
+modify () {
+	sed -e "$1" < "$2" > "$2".x &&
+	mv "$2".x "$2"
+}
+
 test_expect_success setup '
 	for i in a b c d e f g h i j k l m
 	do

@@ -14,10 +19,10 @@ test_expect_success setup '
 	git commit -m initial
 '
 test_expect_success 'apply same filename with independent changes' '
-	sed -i -e "s/^d/z/" same_fn &&
+	modify "s/^d/z/" same_fn &&
 	git diff > patch0 &&
 	git add same_fn &&
-	sed -i -e "s/^i/y/" same_fn &&
+	modify "s/^i/y/" same_fn &&
 	git diff >> patch0 &&
 	cp same_fn same_fn2 &&
 	git reset --hard &&

@@ -27,10 +32,10 @@ test_expect_success 'apply same filename with independent changes' '
 
 test_expect_success 'apply same filename with overlapping changes' '
 	git reset --hard
-	sed -i -e "s/^d/z/" same_fn &&
+	modify "s/^d/z/" same_fn &&
 	git diff > patch0 &&
 	git add same_fn &&
-	sed -i -e "s/^e/y/" same_fn &&
+	modify "s/^e/y/" same_fn &&
 	git diff >> patch0 &&
 	cp same_fn same_fn2 &&
 	git reset --hard &&

@@ -41,10 +46,10 @@ test_expect_success 'apply same filename with overlapping changes' '
 test_expect_success 'apply same new filename after rename' '
 	git reset --hard
 	git mv same_fn new_fn
-	sed -i -e "s/^d/z/" new_fn &&
+	modify "s/^d/z/" new_fn &&
 	git add new_fn &&
 	git diff -M --cached > patch1 &&
-	sed -i -e "s/^e/y/" new_fn &&
+	modify "s/^e/y/" new_fn &&
 	git diff >> patch1 &&
 	cp new_fn new_fn2 &&
 	git reset --hard &&

@@ -55,11 +60,11 @@ test_expect_success 'apply same new filename after rename' '
 test_expect_success 'apply same old filename after rename -- should fail.' '
 	git reset --hard
 	git mv same_fn new_fn
-	sed -i -e "s/^d/z/" new_fn &&
+	modify "s/^d/z/" new_fn &&
 	git add new_fn &&
 	git diff -M --cached > patch1 &&
 	git mv new_fn same_fn
-	sed -i -e "s/^e/y/" same_fn &&
+	modify "s/^e/y/" same_fn &&
 	git diff >> patch1 &&
 	git reset --hard &&
 	test_must_fail git apply patch1

@@ -68,15 +73,15 @@ test_expect_success 'apply same old filename after rename -- should fail.' '
 test_expect_success 'apply A->B (rename), C->A (rename), A->A -- should pass.' '
 	git reset --hard
 	git mv same_fn new_fn
-	sed -i -e "s/^d/z/" new_fn &&
+	modify "s/^d/z/" new_fn &&
 	git add new_fn &&
 	git diff -M --cached > patch1 &&
 	git commit -m "a rename" &&
 	git mv other_fn same_fn
-	sed -i -e "s/^e/y/" same_fn &&
+	modify "s/^e/y/" same_fn &&
 	git add same_fn &&
 	git diff -M --cached >> patch1 &&
-	sed -i -e "s/^g/x/" same_fn &&
+	modify "s/^g/x/" same_fn &&
 	git diff >> patch1 &&
 	git reset --hard HEAD^ &&
 	git apply patch1

Signature · 1 line

1.5.6.1.1099.ge791.dirty

Johannes Schindelin

2016-06-15 22:44 UTC #2 lore raw

Re: [PATCH] t4127-apply-same-fn: Avoid sed -i

Hi,

Quoted 2 lines · Johannes Sixt

Quoted 2 lines
When I created the tests for my git-apply patch, I accidently used the '-i' flag for sed.

Maybe we should modify our hooks so that this is warned about? While at it, we could warn about quoted "wc -l", too...

Ciao,
Dscho