Re: What's cooking in git.git (Dec 2017, #01; Mon, 4)

Jeff Hostetler [off-list ref] writes:

I'm looking at t5616 now on my mac.
Looks like the MAC doesn't like my line counting in the tests.

Ah, of course, test "$(wc -l)" = number would not work over there
we have "test_line_count" helper exactly for that purose.

From: Torsten Bögershausen <redacted>

wc -l is used to count the number if lines in test scripts.
$ wc -l Makefile
gives a line like this:
105 Makefile
while Mac OS has 4 leading spaces:
     105 Makefile

And this means that shell expressions like
test "$(wc -l <expect)" = "4" don't work under Mac OS,

A portable way to use `wc -l` is to omit the '"':
test $(wc -l <expect) = "4"

Add a check in check-non-portable-shell.pl to find '"' between
`wc -l` and '='

Signed-off-by: Torsten Bögershausen <redacted>
---
 t/check-non-portable-shell.pl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
index 03dc9d2852..9ebf65c26f 100755
--- a/t/check-non-portable-shell.pl
+++ b/t/check-non-portable-shell.pl

@@ -21,6 +21,7 @@ while (<>) {
 	/^\s*declare\s+/ and err 'arrays/declare not portable';
 	/^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
 	/\btest\s+[^=]*==/ and err '"test a == b" is not portable (please use =)';
+	/\bwc -l.*"\s*=/ and err '`"$(wc -l)"` is not portable, please use `$(wc -l)`';
 	/\bexport\s+[A-Za-z0-9_]*=/ and err '"export FOO=bar" is not portable (please use FOO=bar && export FOO)';
 	# this resets our $. for each file
 	close ARGV if eof;

-- 
2.15.1.271.g1a4e40aa5d

Hi Torsten,

On Sun, 10 Dec 2017, tboegi@web.de wrote:

As noted elsewhere, this should suggest `test_line_count` instead. After
all, that function is not only guaranteed to stay portable (even if we
should ever start supporting systems *without* `wc`), but it also has a
semantically-meaningful name worthy of the current century.

Ciao,
Dscho

From: Torsten Bögershausen <redacted>

wc -l was used to count the number if lines in test scripts.
$ wc -l Makefile
gives a line like this:
105 Makefile
while Mac OS has 4 leading spaces:
     105 Makefile

And this means that shell expressions like
test "$(wc -l <expect)" = "4" don't work under Mac OS,

Commit fb3340a6 introduced test_line_count() as a portable solution.

Add a check in check-non-portable-shell.pl to find '"' between
`wc -l` and '=' and hint the user about test_line_count().

Reviewed-by: Johannes Schindelin <redacted>
Signed-off-by: Torsten Bögershausen <redacted>
---

I added Dscho as reviewer, thanks.
If there is anything more, please holler.

 t/check-non-portable-shell.pl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
index 03dc9d2852..fadbb1e5a7 100755
--- a/t/check-non-portable-shell.pl
+++ b/t/check-non-portable-shell.pl

@@ -21,6 +21,7 @@ while (<>) {
 	/^\s*declare\s+/ and err 'arrays/declare not portable';
 	/^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
 	/\btest\s+[^=]*==/ and err '"test a == b" is not portable (please use =)';
+	/\bwc -l.*"\s*=/ and err '`"$(wc -l)"` is not portable, please use test_line_count';
 	/\bexport\s+[A-Za-z0-9_]*=/ and err '"export FOO=bar" is not portable (please use FOO=bar && export FOO)';
 	# this resets our $. for each file
 	close ARGV if eof;

-- 
2.15.1.271.g1a4e40aa5d

On Sat, Dec 16, 2017 at 2:52 PM,  [off-list ref] wrote:

Nit: Every other "please use" suggestion is parenthesized; for
consistency, this probably ought to be, as well.

        /\bexport\s+[A-Za-z0-9_]*=/ and err '"export FOO=bar" is not portable (please use FOO=bar && export FOO)';

From: Torsten Bögershausen <redacted>

wc -l was used to count the number if lines in test scripts.
$ wc -l Makefile
gives a line like this:
105 Makefile
while Mac OS has 4 leading spaces:
     105 Makefile

And this means that shell expressions like
test "$(wc -l <expect)" = "4" don't work under Mac OS,

Commit fb3340a6 introduced test_line_count() as a portable solution.

Add a check in check-non-portable-shell.pl to find '"' between
`wc -l` and '=' and hint the user about test_line_count().

Reviewed-by: Johannes Schindelin <redacted>
Signed-off-by: Torsten Bögershausen <redacted>
---
 t/check-non-portable-shell.pl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl
index 03dc9d285..e07f02843 100755

Use () around the hint.
Thanks to Eric for the sharp eyes.

--- a/t/check-non-portable-shell.pl
+++ b/t/check-non-portable-shell.pl

@@ -21,6 +21,7 @@ while (<>) {
 	/^\s*declare\s+/ and err 'arrays/declare not portable';
 	/^\s*[^#]\s*which\s/ and err 'which is not portable (please use type)';
 	/\btest\s+[^=]*==/ and err '"test a == b" is not portable (please use =)';
+	/\bwc -l.*"\s*=/ and err '`"$(wc -l)"` is not portable (please use test_line_count)';
 	/\bexport\s+[A-Za-z0-9_]*=/ and err '"export FOO=bar" is not portable (please use FOO=bar && export FOO)';
 	# this resets our $. for each file
 	close ARGV if eof;

-- 
2.15.1.271.g1a4e40aa5d