Skip to contentSkip to conversation

t6036: avoid "cp -a"

PATCH In review Carlo Marcelo Arenas Belón 3 participants 4 messages Dec 1, 2018

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 t6036: avoid "cp -a"

b8cd1bb713 ("t6036, t6043: increase code coverage for file collision handling", 2018-11-07) uses this GNU extension that is not available in a POSIX complaint cp; use cp -R instead

---
to be applied on top of en/merge-path-collision for next

 t/t6036-recursive-corner-cases.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
t/t6036-recursive-corner-cases.sh +1 -1
index b7488b00c0..fdb120d0dc 100755
@@ -1631,7 +1631,7 @@ test_expect_success 'check nested conflicts' '
 
 		# Compare m to expected contents
 		>empty &&
-		cp -a m_stage_2 expected_final_m &&
+		cp -R m_stage_2 expected_final_m &&
 		test_must_fail git merge-file --diff3 \
 			-L "HEAD"                     \
 			-L "merged common ancestors"  \
Signature · 1 line
2.20.0.rc1.6.ga1598010f
#1 lore raw

Hi,

Thanks for the patch!

Quoted 2 lines · Carlo Marcelo Arenas Belón

b8cd1bb713 ("t6036, t6043: increase code coverage for file collision handling", 2018-11-07) uses this GNU extension that is not available in a POSIX complaint

This is an extraordinarily long line; can you rewrap at around 72 characters?

Quoted 23 lines

cp; use cp -R instead

---
to be applied on top of en/merge-path-collision for next

 t/t6036-recursive-corner-cases.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
t/t6036-recursive-corner-cases.sh +1 -2
index b7488b00c0..fdb120d0dc 100755
@@ -1631,7 +1631,7 @@ test_expect_success 'check nested conflicts' '
 
                # Compare m to expected contents
                >empty &&
-               cp -a m_stage_2 expected_final_m &&
+               cp -R m_stage_2 expected_final_m &&
                test_must_fail git merge-file --diff3 \
                        -L "HEAD"                     \
                        -L "merged common ancestors"  \
--
Quoted 1 line

2.20.0.rc1.6.ga1598010f

Oops. Thanks for catching. To be honest, we don't even need -a, -R, etc. -- it was just a habit for me to add -a after cp. A simple cp would do, though what you have here is fine too.

#2 lore raw
Quoted 7 lines · Elijah Newren

Thanks for the patch!

On Fri, Nov 30, 2018 at 6:52 PM Carlo Marcelo Arenas Belón
...
Oops. Thanks for catching. To be honest, we don't even need -a, -R, etc. -- it was just a habit for me to add -a after cp. A simple cp would do, though what you have here is fine too.

Thanks, both. I think the topic won't escape 'next' until 2.20 final, and after that we can rewind 'next', so it may be better to squash it in, but anyway...

-- >8 --
From: Carlo Marcelo Arenas Belón <[email protected]>
Date: Fri, 30 Nov 2018 18:52:12 -0800
Subject: [PATCH] t6036: avoid non-portable "cp -a"

b8cd1bb713 ("t6036, t6043: increase code coverage for file collision handling", 2018-11-07) uses this GNU extension that is not available in a POSIX complaint cp. In this particular case, there is no need to use the option, as it is just copying a single file to create another file.

---
 t/t6036-recursive-corner-cases.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
t/t6036-recursive-corner-cases.sh +1 -1
index b7488b00c0..d23b948f27 100755
@@ -1631,7 +1631,7 @@ test_expect_success 'check nested conflicts' '
 
 		# Compare m to expected contents
 		>empty &&
-		cp -a m_stage_2 expected_final_m &&
+		cp m_stage_2 expected_final_m &&
 		test_must_fail git merge-file --diff3 \
 			-L "HEAD"                     \
 			-L "merged common ancestors"  \
Signature · 1 line
2.20.0-rc1-10-g7068cbc4ab
#3 lore raw

Thanks both. Agree with Junio it would be better if squashed; apologize for not catching it earlier, but the following might help to make it visible for anyone that care to run the linter:

  $ make test-lint-shell-syntax

Carlo
-- >8 --
From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <[email protected]>
Subject: [PATCH] tests: add lint for non portable cp -a

cp -a, while a common flag isn't in POSIX and will therefore fail on systems that don't have GNUish tools (like OpenBSD, AIX or Solaris)

---
 t/check-non-portable-shell.pl | 1 +
 1 file changed, 1 insertion(+)
t/check-non-portable-shell.pl +1 -0
index b45bdac688..8037eef777 100755
@@ -35,6 +35,7 @@ sub err {
 		chomp;
 	}
 
+	/\bcp\s+-a/ and err 'cp -a is not portable';
 	/\bsed\s+-i/ and err 'sed -i is not portable';
 	/\becho\s+-[neE]/ and err 'echo with option is not portable (use printf)';
 	/^\s*declare\s+/ and err 'arrays/declare not portable';
Signature · 1 line
2.20.0.rc2