threads / patch / 38226

patchUse wc instead of awk to count subtrees in t0090-cache-tree

Subject: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree

## tl;dr

10 messages between Dec 22, 2014 and Dec 23, 2014. Diffs are folded; open one to read it.

replies: 9people: 3as markdown or json

Ben Walton· Dec 22, 2014, 17:52 UTC · lore

The awk statements previously used in this test weren't compatible with the native versions of awk on Solaris:

echo "dir" | /bin/awk -v c=0 '$1 {++c} END {print c}' awk: syntax error near line 1 awk: bailing out near line 1

echo "dir" | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}' 0

And with GNU awk for comparison: echo "dir" | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}' 1

Instead of modifying the awk code to work, use wc -w instead as that is both adequate and simpler.

Signed-off-by: Ben Walton <bdwalton@gmail.com>
---
 t/t0090-cache-tree.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to t/t0090-cache-tree.sh +1 −1
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 067f4c6..f2b1c9c 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -22,7 +22,7 @@ generate_expected_cache_tree_rec () {
 	# ls-files might have foo/bar, foo/bar/baz, and foo/bar/quux
 	# We want to count only foo because it's the only direct child
 	subtrees=$(git ls-files|grep /|cut -d / -f 1|uniq) &&
-	subtree_count=$(echo "$subtrees"|awk -v c=0 '$1 {++c} END {print c}') &&
+	subtree_count=$(echo "$subtrees"|wc -w) &&
 	entries=$(git ls-files|wc -l) &&
 	printf "SHA $dir (%d entries, %d subtrees)\n" "$entries" "$subtree_count" &&
 	for subtree in $subtrees

-- 
1.9.1
Junio C Hamano· Dec 22, 2014, 21:45 UTC · re: Ben Walton · lore

Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree

Ben Walton <bdwalton@gmail.com> writes:
Show 16 quoted lines
> The awk statements previously used in this test weren't compatible
> with the native versions of awk on Solaris:
>
> echo "dir" | /bin/awk -v c=0 '$1 {++c} END {print c}'
> awk: syntax error near line 1
> awk: bailing out near line 1
>
> echo "dir" | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
> 0
>
> And with GNU awk for comparison:
> echo "dir" | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}'
> 1
>
> Instead of modifying the awk code to work, use wc -w instead as that
> is both adequate and simpler.

Hmm, why "wc -w" not "wc -l", though? Is somebody squashing a one-elem-per-line output from ls-files onto a single line?

Jonathan Nieder· Dec 22, 2014, 22:02 UTC · re: Junio C Hamano · lore

Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree

Junio C Hamano wrote:
> Ben Walton <bdwalton@gmail.com> writes:
Show 12 quoted lines
>> echo "dir" | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
>> 0
>>
>> And with GNU awk for comparison:
>> echo "dir" | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}'
>> 1
>>
>> Instead of modifying the awk code to work, use wc -w instead as that
>> is both adequate and simpler.
>
> Hmm, why "wc -w" not "wc -l", though?  Is somebody squashing a
> one-elem-per-line output from ls-files onto a single line?
The old code was trying to skip empty lines.
Junio C Hamano· Dec 22, 2014, 22:26 UTC · re: Jonathan Nieder · lore

Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 17 quoted lines
> Junio C Hamano wrote:
>> Ben Walton <bdwalton@gmail.com> writes:
>
>>> echo "dir" | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
>>> 0
>>>
>>> And with GNU awk for comparison:
>>> echo "dir" | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}'
>>> 1
>>>
>>> Instead of modifying the awk code to work, use wc -w instead as that
>>> is both adequate and simpler.
>>
>> Hmm, why "wc -w" not "wc -l", though?  Is somebody squashing a
>> one-elem-per-line output from ls-files onto a single line?
>
> The old code was trying to skip empty lines.
Ahh, I misread the original.
Your suggestion to explicitly check $1 != "" makes sense to me now.

To be blunt, I do not have much sympathy to those who insist using /usr/bin versions of various tools on Solaris that are overriden by xpg variants, but it is somewhat disturbing that the one from xpg4 does not work.

Jonathan Nieder· Dec 22, 2014, 22:01 UTC · re: Ben Walton · lore

Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree

Ben Walton wrote:
> echo "dir" | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
> 0
Thanks.  Weird.  Does
	awk -v c=0 '$1 != "" {++c} END {print c}'
work better?
[...]
Show 10 quoted lines
> --- a/t/t0090-cache-tree.sh
> +++ b/t/t0090-cache-tree.sh
> @@ -22,7 +22,7 @@ generate_expected_cache_tree_rec () {
>  	# ls-files might have foo/bar, foo/bar/baz, and foo/bar/quux
>  	# We want to count only foo because it's the only direct child
>  	subtrees=$(git ls-files|grep /|cut -d / -f 1|uniq) &&
> -	subtree_count=$(echo "$subtrees"|awk -v c=0 '$1 {++c} END {print c}') &&
> +	subtree_count=$(echo "$subtrees"|wc -w) &&
>  	entries=$(git ls-files|wc -l) &&
>  	printf "SHA $dir (%d entries, %d subtrees)\n" "$entries" "$subtree_count" &&
Some implementations of wc add a trailing space, causing
	printf: 1 : invalid number
Using
	printf "SHA $dir (%d entries, %d subtrees)\n" "$entries" $subtree_count &&

(with no quotes around $subtree_count) would avoid trouble, though that's a little subtle.

Hope that helps, Jonathan

Junio C Hamano· Dec 22, 2014, 23:25 UTC · re: Ben Walton · lore

Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree

From: Ben Walton <bdwalton@gmail.com>

The awk statements previously used in this test weren't compatible with the native versions of awk on Solaris:

    echo "dir" | /bin/awk -v c=0 '$1 {++c} END {print c}'
    awk: syntax error near line 1
    awk: bailing out near line 1
    echo "dir" | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
    0
And with GNU awk for comparison:
    echo "dir" | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}'
    1

Work it around by using $1 != "" to state more explicitly that we are skipping empty lines.

Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ben Walton <bdwalton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * Then let's queue this, perhaps?
 t/t0090-cache-tree.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to t/t0090-cache-tree.sh +1 −1
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 067f4c6..601d02d 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -22,7 +22,7 @@ generate_expected_cache_tree_rec () {
 	# ls-files might have foo/bar, foo/bar/baz, and foo/bar/quux
 	# We want to count only foo because it's the only direct child
 	subtrees=$(git ls-files|grep /|cut -d / -f 1|uniq) &&
-	subtree_count=$(echo "$subtrees"|awk -v c=0 '$1 {++c} END {print c}') &&
+	subtree_count=$(echo "$subtrees"|awk -v c=0 '$1 != "" {++c} END {print c}') &&
 	entries=$(git ls-files|wc -l) &&
 	printf "SHA $dir (%d entries, %d subtrees)\n" "$entries" "$subtree_count" &&
 	for subtree in $subtrees
Junio C Hamano· Dec 22, 2014, 23:27 UTC · re: Junio C Hamano · lore

Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree

Junio C Hamano <gitster@pobox.com> writes:
Show 26 quoted lines
> From: Ben Walton <bdwalton@gmail.com>
>
> The awk statements previously used in this test weren't compatible
> with the native versions of awk on Solaris:
>
>     echo "dir" | /bin/awk -v c=0 '$1 {++c} END {print c}'
>     awk: syntax error near line 1
>     awk: bailing out near line 1
>
>     echo "dir" | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
>     0
>
> And with GNU awk for comparison:
>
>     echo "dir" | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}'
>     1
>
> Work it around by using $1 != "" to state more explicitly that we
> are skipping empty lines.
>
> Helped-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Ben Walton <bdwalton@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>
>  * Then let's queue this, perhaps?
heh, not like that without updating the subject, perhaps like this:
Subject: t0090: tweak awk statement for Solaris /usr/xpg4/bin/awk
Sorry for the noise.
Show 16 quoted lines
>  t/t0090-cache-tree.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
> index 067f4c6..601d02d 100755
> --- a/t/t0090-cache-tree.sh
> +++ b/t/t0090-cache-tree.sh
> @@ -22,7 +22,7 @@ generate_expected_cache_tree_rec () {
>  	# ls-files might have foo/bar, foo/bar/baz, and foo/bar/quux
>  	# We want to count only foo because it's the only direct child
>  	subtrees=$(git ls-files|grep /|cut -d / -f 1|uniq) &&
> -	subtree_count=$(echo "$subtrees"|awk -v c=0 '$1 {++c} END {print c}') &&
> +	subtree_count=$(echo "$subtrees"|awk -v c=0 '$1 != "" {++c} END {print c}') &&
>  	entries=$(git ls-files|wc -l) &&
>  	printf "SHA $dir (%d entries, %d subtrees)\n" "$entries" "$subtree_count" &&
>  	for subtree in $subtrees
Jonathan Nieder· Dec 22, 2014, 23:38 UTC · re: Junio C Hamano · lore

Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree

Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
Show 8 quoted lines
>> From: Ben Walton <bdwalton@gmail.com>
>>
>> The awk statements previously used in this test weren't compatible
>> with the native versions of awk on Solaris:
>>
>>     echo "dir" | /bin/awk -v c=0 '$1 {++c} END {print c}'
>>     awk: syntax error near line 1
>>     awk: bailing out near line 1

If I were doing it, I'd leave the above four lines out --- they are describing an unrelated problem. I wonder if we should make the test harness respect SANE_TOOL_PATH to avoid that kind of problem in the future.

[...]
> heh, not like that without updating the subject, perhaps like this:
>
> Subject: t0090: tweak awk statement for Solaris /usr/xpg4/bin/awk
With the updated subject,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.
Junio C Hamano· Dec 23, 2014, 18:26 UTC · re: Jonathan Nieder · lore

Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree

Jonathan Nieder <jrnieder@gmail.com> writes:
> With the updated subject,
>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.  Here is what I tentatively queued for today's pushout.
-- >8 --
From: Ben Walton <bdwalton@gmail.com>
Date: Mon, 22 Dec 2014 15:25:44 -0800
Subject: [PATCH] t0090: tweak awk statement for Solaris /usr/xpg4/bin/awk

The awk statements previously used in this test weren't compatible with the native versions of awk on Solaris:

    echo "dir" | /bin/awk -v c=0 '$1 {++c} END {print c}'
    awk: syntax error near line 1
    awk: bailing out near line 1
    echo "dir" | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
    0

Even though we do not cater to tools in /usr/bin on Solaris that are overridden by corresponding ones in /usr/xpg?/bin, in this case, even the XPG version does not work correctly.

With GNU awk for comparison:
    echo "dir" | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}'
    1

which is what this test expects (and is in line with POSIX; non-empty string is true and an empty string is false).

Work this issue around by using $1 != "" to state more explicitly that we are skipping empty lines.

Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ben Walton <bdwalton@gmail.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t0090-cache-tree.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to t/t0090-cache-tree.sh +1 −1
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 067f4c6..601d02d 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -22,7 +22,7 @@ generate_expected_cache_tree_rec () {
 	# ls-files might have foo/bar, foo/bar/baz, and foo/bar/quux
 	# We want to count only foo because it's the only direct child
 	subtrees=$(git ls-files|grep /|cut -d / -f 1|uniq) &&
-	subtree_count=$(echo "$subtrees"|awk -v c=0 '$1 {++c} END {print c}') &&
+	subtree_count=$(echo "$subtrees"|awk -v c=0 '$1 != "" {++c} END {print c}') &&
 	entries=$(git ls-files|wc -l) &&
 	printf "SHA $dir (%d entries, %d subtrees)\n" "$entries" "$subtree_count" &&
 	for subtree in $subtrees

-- 
2.2.1-321-gd161b79
Junio C Hamano· Dec 22, 2014, 23:34 UTC · re: Junio C Hamano · lore

Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree

Junio C Hamano <gitster@pobox.com> writes:
Show 19 quoted lines
> From: Ben Walton <bdwalton@gmail.com>
>
> The awk statements previously used in this test weren't compatible
> with the native versions of awk on Solaris:
>
>     echo "dir" | /bin/awk -v c=0 '$1 {++c} END {print c}'
>     awk: syntax error near line 1
>     awk: bailing out near line 1
>
>     echo "dir" | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
>     0
>
> And with GNU awk for comparison:
>
>     echo "dir" | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}'
>     1
>
> Work it around by using $1 != "" to state more explicitly that we
> are skipping empty lines.

By the way, I was hoping (eh, what kind of hope is that???) that $1 alone is not a kosher POSIX way but a GNUism, but that does not seem to be the case. POSIX has this [*1*]

    When an expression is used in a Boolean context, if it has a
    numeric value, a value of zero shall be treated as false and any
    other value shall be treated as true. Otherwise, a string value
    of the null string shall be treated as false and any other value
    shall be treated as true. A Boolean context shall be one of the
    following:
and among the "Boolean context" listed is:
    * An expression used as a pattern (as in Overall Program Structure)

So the example with /usr/xpg4/bin/awk does not seem to be a behaviour from a conformant implementationd, and it seems to be correct to label this as "work it around by ..." (not "avoid using GNUism").

We learn new things every day (not that I really wanted to learn glitches in various implementations of awk) ;-).

Thanks.
[Reference]
*1* http://pubs.opengroup.org/onlinepubs/9699919799/utilities/awk.html

← back to recent threads