All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
@ 2014-12-22 17:52 Ben Walton
  2014-12-22 21:45 ` Junio C Hamano
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Ben Walton @ 2014-12-22 17:52 UTC (permalink / raw)
  To: gitster, dturner; +Cc: git, Ben Walton

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(-)

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
  2014-12-22 17:52 [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree Ben Walton
@ 2014-12-22 21:45 ` Junio C Hamano
  2014-12-22 22:02   ` Jonathan Nieder
  2014-12-22 22:01 ` Jonathan Nieder
  2014-12-22 23:25 ` Junio C Hamano
  2 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2014-12-22 21:45 UTC (permalink / raw)
  To: Ben Walton; +Cc: dturner, git

Ben Walton <bdwalton@gmail.com> writes:

> 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?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
  2014-12-22 17:52 [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree Ben Walton
  2014-12-22 21:45 ` Junio C Hamano
@ 2014-12-22 22:01 ` Jonathan Nieder
  2014-12-22 23:25 ` Junio C Hamano
  2 siblings, 0 replies; 10+ messages in thread
From: Jonathan Nieder @ 2014-12-22 22:01 UTC (permalink / raw)
  To: Ben Walton; +Cc: gitster, dturner, git

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?

[...]
> --- 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
  2014-12-22 21:45 ` Junio C Hamano
@ 2014-12-22 22:02   ` Jonathan Nieder
  2014-12-22 22:26     ` Junio C Hamano
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Nieder @ 2014-12-22 22:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ben Walton, dturner, git

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
  2014-12-22 22:02   ` Jonathan Nieder
@ 2014-12-22 22:26     ` Junio C Hamano
  0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2014-12-22 22:26 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Ben Walton, dturner, git

Jonathan Nieder <jrnieder@gmail.com> writes:

> 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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
  2014-12-22 17:52 [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree Ben Walton
  2014-12-22 21:45 ` Junio C Hamano
  2014-12-22 22:01 ` Jonathan Nieder
@ 2014-12-22 23:25 ` Junio C Hamano
  2014-12-22 23:27   ` Junio C Hamano
  2014-12-22 23:34   ` Junio C Hamano
  2 siblings, 2 replies; 10+ messages in thread
From: Junio C Hamano @ 2014-12-22 23:25 UTC (permalink / raw)
  To: Ben Walton; +Cc: dturner, git, Jonathan Nieder

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(-)

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
  2014-12-22 23:25 ` Junio C Hamano
@ 2014-12-22 23:27   ` Junio C Hamano
  2014-12-22 23:38     ` Jonathan Nieder
  2014-12-22 23:34   ` Junio C Hamano
  1 sibling, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2014-12-22 23:27 UTC (permalink / raw)
  To: Ben Walton; +Cc: dturner, git, Jonathan Nieder

Junio C Hamano <gitster@pobox.com> writes:

> 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.



>  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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
  2014-12-22 23:25 ` Junio C Hamano
  2014-12-22 23:27   ` Junio C Hamano
@ 2014-12-22 23:34   ` Junio C Hamano
  1 sibling, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2014-12-22 23:34 UTC (permalink / raw)
  To: Ben Walton; +Cc: dturner, git, Jonathan Nieder

Junio C Hamano <gitster@pobox.com> writes:

> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
  2014-12-22 23:27   ` Junio C Hamano
@ 2014-12-22 23:38     ` Jonathan Nieder
  2014-12-23 18:26       ` Junio C Hamano
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Nieder @ 2014-12-22 23:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ben Walton, dturner, git

Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:

>> 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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
  2014-12-22 23:38     ` Jonathan Nieder
@ 2014-12-23 18:26       ` Junio C Hamano
  0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2014-12-23 18:26 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Ben Walton, dturner, git

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(-)

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-12-23 18:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-22 17:52 [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree Ben Walton
2014-12-22 21:45 ` Junio C Hamano
2014-12-22 22:02   ` Jonathan Nieder
2014-12-22 22:26     ` Junio C Hamano
2014-12-22 22:01 ` Jonathan Nieder
2014-12-22 23:25 ` Junio C Hamano
2014-12-22 23:27   ` Junio C Hamano
2014-12-22 23:38     ` Jonathan Nieder
2014-12-23 18:26       ` Junio C Hamano
2014-12-22 23:34   ` Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.