All of lore.kernel.org
 help / color / mirror / Atom feed
* t6044 broken on pu
@ 2016-05-07 12:00 Torsten Bögershausen
  2016-05-07 12:19 ` Andreas Schwab
  0 siblings, 1 reply; 20+ messages in thread
From: Torsten Bögershausen @ 2016-05-07 12:00 UTC (permalink / raw)
  To: newren, Git Mailing List

The "seq" is not understood by all shells,
using printf fixes this,

index 20a3ffe..48d964e 100755
--- a/t/t6044-merge-unrelated-index-changes.sh
+++ b/t/t6044-merge-unrelated-index-changes.sh
@@ -20,7 +20,7 @@ test_description="merges with unrelated index changes"
 #   Commit E: renames a->subdir/a, adds subdir/e

 test_expect_success 'setup trivial merges' '
-       seq 1 10 >a &&
+       printf 1 2 3 4 5 7 8 9 10 >a &&
        git add a &&
        test_tick && git commit -m A &&

@@ -42,7 +42,7 @@ test_expect_success 'setup trivial merges' '
        test_tick && git commit -m C &&

        git checkout D &&
-       seq 2 10 >a &&
+       printf 2 3 4 5 7 8 9 10 >a &&

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

* Re: t6044 broken on pu
  2016-05-07 12:00 t6044 broken on pu Torsten Bögershausen
@ 2016-05-07 12:19 ` Andreas Schwab
  2016-05-07 13:15   ` Ramsay Jones
  2016-05-07 16:18   ` Torsten Bögershausen
  0 siblings, 2 replies; 20+ messages in thread
From: Andreas Schwab @ 2016-05-07 12:19 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: newren, Git Mailing List

Torsten Bögershausen <tboegi@web.de> writes:

> The "seq" is not understood by all shells,
> using printf fixes this,
>
> index 20a3ffe..48d964e 100755
> --- a/t/t6044-merge-unrelated-index-changes.sh
> +++ b/t/t6044-merge-unrelated-index-changes.sh
> @@ -20,7 +20,7 @@ test_description="merges with unrelated index changes"
>  #   Commit E: renames a->subdir/a, adds subdir/e
>
>  test_expect_success 'setup trivial merges' '
> -       seq 1 10 >a &&
> +       printf 1 2 3 4 5 7 8 9 10 >a &&

$ printf 1 2 3 4 5 7 8 9 10
1

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

* Re: t6044 broken on pu
  2016-05-07 12:19 ` Andreas Schwab
@ 2016-05-07 13:15   ` Ramsay Jones
  2016-05-07 13:43     ` Ramsay Jones
  2016-05-07 16:18   ` Torsten Bögershausen
  1 sibling, 1 reply; 20+ messages in thread
From: Ramsay Jones @ 2016-05-07 13:15 UTC (permalink / raw)
  To: Andreas Schwab, Torsten Bögershausen; +Cc: newren, Git Mailing List



On 07/05/16 13:19, Andreas Schwab wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
> 
>> The "seq" is not understood by all shells,
>> using printf fixes this,
>>
>> index 20a3ffe..48d964e 100755
>> --- a/t/t6044-merge-unrelated-index-changes.sh
>> +++ b/t/t6044-merge-unrelated-index-changes.sh
>> @@ -20,7 +20,7 @@ test_description="merges with unrelated index changes"
>>  #   Commit E: renames a->subdir/a, adds subdir/e
>>
>>  test_expect_success 'setup trivial merges' '
>> -       seq 1 10 >a &&
>> +       printf 1 2 3 4 5 7 8 9 10 >a &&
> 
> $ printf 1 2 3 4 5 7 8 9 10
> 1

yep, I think:

    printf "%d\n" 1 2 3 4 5 6 7 8 9 10 >a &&

would be equivalent.

ATB,
Ramsay Jones

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

* Re: t6044 broken on pu
  2016-05-07 13:15   ` Ramsay Jones
@ 2016-05-07 13:43     ` Ramsay Jones
  0 siblings, 0 replies; 20+ messages in thread
From: Ramsay Jones @ 2016-05-07 13:43 UTC (permalink / raw)
  To: Andreas Schwab, Torsten Bögershausen; +Cc: newren, Git Mailing List



On 07/05/16 14:15, Ramsay Jones wrote:
> 
> 
> On 07/05/16 13:19, Andreas Schwab wrote:
>> Torsten Bögershausen <tboegi@web.de> writes:
>>
>>> The "seq" is not understood by all shells,
>>> using printf fixes this,
>>>
>>> index 20a3ffe..48d964e 100755
>>> --- a/t/t6044-merge-unrelated-index-changes.sh
>>> +++ b/t/t6044-merge-unrelated-index-changes.sh
>>> @@ -20,7 +20,7 @@ test_description="merges with unrelated index changes"
>>>  #   Commit E: renames a->subdir/a, adds subdir/e
>>>
>>>  test_expect_success 'setup trivial merges' '
>>> -       seq 1 10 >a &&
>>> +       printf 1 2 3 4 5 7 8 9 10 >a &&
>>
>> $ printf 1 2 3 4 5 7 8 9 10
>> 1
> 
> yep, I think:
> 
>     printf "%d\n" 1 2 3 4 5 6 7 8 9 10 >a &&
> 
> would be equivalent.
> 

Having said that, there is also 'test_seq' which you can use
to avoid portability problems (although it uses perl, so could
be viewed as a bit heavyweight):

    test_seq 1 10 >a &&

ATB,
Ramsay Jones

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

* Re: t6044 broken on pu
  2016-05-07 12:19 ` Andreas Schwab
  2016-05-07 13:15   ` Ramsay Jones
@ 2016-05-07 16:18   ` Torsten Bögershausen
  2016-05-08  2:21     ` Junio C Hamano
  1 sibling, 1 reply; 20+ messages in thread
From: Torsten Bögershausen @ 2016-05-07 16:18 UTC (permalink / raw)
  To: Andreas Schwab, Torsten Bögershausen; +Cc: newren, Git Mailing List

On 2016-05-07 14.19, Andreas Schwab wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
> 
>> The "seq" is not understood by all shells,
>> using printf fixes this,
>>
>> index 20a3ffe..48d964e 100755
>> --- a/t/t6044-merge-unrelated-index-changes.sh
>> +++ b/t/t6044-merge-unrelated-index-changes.sh
>> @@ -20,7 +20,7 @@ test_description="merges with unrelated index changes"
>>  #   Commit E: renames a->subdir/a, adds subdir/e
>>
>>  test_expect_success 'setup trivial merges' '
>> -       seq 1 10 >a &&
>> +       printf 1 2 3 4 5 7 8 9 10 >a &&
> 
> $ printf 1 2 3 4 5 7 8 9 10
> 1

That's true, but the test passes anyway.
So do we need the sequence ?
Is there something that can be improved in the test ?

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

* Re: t6044 broken on pu
  2016-05-07 16:18   ` Torsten Bögershausen
@ 2016-05-08  2:21     ` Junio C Hamano
  2016-05-08  6:54       ` Torsten Bögershausen
  0 siblings, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2016-05-08  2:21 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Andreas Schwab, newren, Git Mailing List

Torsten Bögershausen <tboegi@web.de> writes:

> That's true, but the test passes anyway.

You can also remove the body of the test and replace it with "true"
and say "the test passes anyway".  Changing the test to use a file
with only one line is irresponsible, if you do not know the nature
of expected future bug that requires 10 lines to be there to
manifest that the test wants to try.

test_seq was invented exactly for the purpose of accomodating
platforms that lack seq, so using it would probably be the best
first step.  Updating implementation of test_seq to avoid $PERL
would be a separate step, if desired (I personally do not think
that is worth it).

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

* Re: t6044 broken on pu
  2016-05-08  2:21     ` Junio C Hamano
@ 2016-05-08  6:54       ` Torsten Bögershausen
  2016-05-08 18:20         ` Junio C Hamano
  0 siblings, 1 reply; 20+ messages in thread
From: Torsten Bögershausen @ 2016-05-08  6:54 UTC (permalink / raw)
  To: Junio C Hamano, Torsten Bögershausen
  Cc: Andreas Schwab, newren, Git Mailing List

On 08.05.16 04:21, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> That's true, but the test passes anyway.
> You can also remove the body of the test and replace it with "true"
> and say "the test passes anyway".  Changing the test to use a file
> with only one line is irresponsible, if you do not know the nature
> of expected future bug that requires 10 lines to be there to
> manifest that the test wants to try.
>
> test_seq was invented exactly for the purpose of accomodating
> platforms that lack seq, so using it would probably be the best
> first step.  Updating implementation of test_seq to avoid $PERL
> would be a separate step, if desired (I personally do not think
> that is worth it).
We don't need to invoke perl, when the shell can do that internally ?

May a  simple
 printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n"

be an option ?

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

* Re: t6044 broken on pu
  2016-05-08  6:54       ` Torsten Bögershausen
@ 2016-05-08 18:20         ` Junio C Hamano
  2016-05-09  4:43           ` Torsten Bögershausen
  2016-05-09  6:30           ` demerphq
  0 siblings, 2 replies; 20+ messages in thread
From: Junio C Hamano @ 2016-05-08 18:20 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Andreas Schwab, newren, Git Mailing List

Torsten Bögershausen <tboegi@web.de> writes:

> May a  simple
>  printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n"
>
> be an option ?

If you were to do that, at least have the decency to make it more
readable by doing something like:

	printf "%s\n" 1 2 3 4 5 6 7 8 9 10

;-)

But as I said, as a response to "t6044 broken on pu" bug report,
s/seq/test_seq/ is the only sensible change.

Improving "test_seq, the alternative to seq" is a separate topic.

If you have aversion to $PERL, perhaps do them without using
anything what is not expected to be built-in in modern shells,
perhaps like this?

 t/test-lib-functions.sh | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8d99eb3..4edddac 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -739,7 +739,12 @@ test_seq () {
 	2)	;;
 	*)	error "bug in the test script: not 1 or 2 parameters to test_seq" ;;
 	esac
-	perl -le 'print for $ARGV[0]..$ARGV[1]' -- "$@"
+	test_seq_counter__=$1
+	while test "$test_seq_counter__" -le $2
+	do
+		echo "$test_seq_counter__"
+		test_seq_counter__=$((test_seq_counter__ + 1))
+	done
 }
 
 # This function can be used to schedule some commands to be run

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

* Re: t6044 broken on pu
  2016-05-08 18:20         ` Junio C Hamano
@ 2016-05-09  4:43           ` Torsten Bögershausen
  2016-05-09 18:22             ` Junio C Hamano
  2016-05-09  6:30           ` demerphq
  1 sibling, 1 reply; 20+ messages in thread
From: Torsten Bögershausen @ 2016-05-09  4:43 UTC (permalink / raw)
  To: Junio C Hamano, Torsten Bögershausen
  Cc: Andreas Schwab, newren, Git Mailing List



On 08.05.16 20:20, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> May a  simple
>>  printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n"
>>
>> be an option ?
> If you were to do that, at least have the decency to make it more
> readable by doing something like:
>
> 	printf "%s\n" 1 2 3 4 5 6 7 8 9 10
>
> ;-)
>
> But as I said, as a response to "t6044 broken on pu" bug report,
> s/seq/test_seq/ is the only sensible change.
>
> Improving "test_seq, the alternative to seq" is a separate topic.
>
> If you have aversion to $PERL, perhaps do them without using
> anything what is not expected to be built-in in modern shells,
> perhaps like this?
Please don't get me wrong -
I wasn't really clear why:
I think that the invocation of an external program
to produce a 10 line test program is a waste of CPU cycles  - in this very use case.

We can try to use the ideal tool to do the job, in this case
the fork() that needs to be done to invoke perl seems rather expensive in relation
to what we get in terms of functionality - a file with 10 lines of content.

I recently read a message why "grep | sed" is not ideal, when sed can do everything
that grep needs to do, and only 1 external program needs to be invoked - not 2.

I try to apply the same pattern to this TC: Stay in the shell, as long as possible.
But if you really need perl for e.g. regexp, then use it.

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

* Re: t6044 broken on pu
  2016-05-08 18:20         ` Junio C Hamano
  2016-05-09  4:43           ` Torsten Bögershausen
@ 2016-05-09  6:30           ` demerphq
  2016-05-09  8:33             ` Jeff King
  1 sibling, 1 reply; 20+ messages in thread
From: demerphq @ 2016-05-09  6:30 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Torsten Bögershausen, Andreas Schwab, newren, Git Mailing List

On 8 May 2016 at 20:20, Junio C Hamano <gitster@pobox.com> wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> May a  simple
>>  printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n"
>>
>> be an option ?
>
> If you were to do that, at least have the decency to make it more
> readable by doing something like:
>
>         printf "%s\n" 1 2 3 4 5 6 7 8 9 10
>
> ;-)
>
> But as I said, as a response to "t6044 broken on pu" bug report,
> s/seq/test_seq/ is the only sensible change.
>
> Improving "test_seq, the alternative to seq" is a separate topic.
>
> If you have aversion to $PERL, perhaps do them without using
> anything what is not expected to be built-in in modern shells,
> perhaps like this?
>
>  t/test-lib-functions.sh | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 8d99eb3..4edddac 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -739,7 +739,12 @@ test_seq () {
>         2)      ;;
>         *)      error "bug in the test script: not 1 or 2 parameters to test_seq" ;;
>         esac
> -       perl -le 'print for $ARGV[0]..$ARGV[1]' -- "$@"
> +       test_seq_counter__=$1
> +       while test "$test_seq_counter__" -le $2
> +       do
> +               echo "$test_seq_counter__"
> +               test_seq_counter__=$((test_seq_counter__ + 1))
> +       done
>  }

Is that perl snippet ever called with non-numeric output?

perl -le 'print for $ARGV[0]..$ARGV[1]' -- A E
A
B
C
D
E

Yves



-- 
perl -Mre=debug -e "/just|another|perl|hacker/"

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

* Re: t6044 broken on pu
  2016-05-09  6:30           ` demerphq
@ 2016-05-09  8:33             ` Jeff King
  2016-05-09 16:02               ` Eric Sunshine
  0 siblings, 1 reply; 20+ messages in thread
From: Jeff King @ 2016-05-09  8:33 UTC (permalink / raw)
  To: demerphq
  Cc: Junio C Hamano, Torsten Bögershausen, Andreas Schwab,
	newren, Git Mailing List

On Mon, May 09, 2016 at 08:30:51AM +0200, demerphq wrote:

> > -       perl -le 'print for $ARGV[0]..$ARGV[1]' -- "$@"
> > +       test_seq_counter__=$1
> > +       while test "$test_seq_counter__" -le $2
> > +       do
> > +               echo "$test_seq_counter__"
> > +               test_seq_counter__=$((test_seq_counter__ + 1))
> > +       done
> >  }
> 
> Is that perl snippet ever called with non-numeric output?
> 
> perl -le 'print for $ARGV[0]..$ARGV[1]' -- A E
> A
> B
> C
> D
> E

I had that thought, too, but I think it would be an error to do so.
test_seq is supposed to be a replacement for "seq", which does not
understand non-numeric sequences.

A quick "git grep test_seq" seems to back that up.

-Peff

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

* Re: t6044 broken on pu
  2016-05-09  8:33             ` Jeff King
@ 2016-05-09 16:02               ` Eric Sunshine
  2016-05-09 16:12                 ` Jeff King
  0 siblings, 1 reply; 20+ messages in thread
From: Eric Sunshine @ 2016-05-09 16:02 UTC (permalink / raw)
  To: Jeff King
  Cc: demerphq, Junio C Hamano, Torsten Bögershausen,
	Andreas Schwab, newren, Git Mailing List

On Mon, May 9, 2016 at 4:33 AM, Jeff King <peff@peff.net> wrote:
> On Mon, May 09, 2016 at 08:30:51AM +0200, demerphq wrote:
>> > -       perl -le 'print for $ARGV[0]..$ARGV[1]' -- "$@"
>> > +       test_seq_counter__=$1
>> > +       while test "$test_seq_counter__" -le $2
>> > +       do
>> > +               echo "$test_seq_counter__"
>> > +               test_seq_counter__=$((test_seq_counter__ + 1))
>> > +       done
>> >  }
>>
>> Is that perl snippet ever called with non-numeric output?
>>
>> perl -le 'print for $ARGV[0]..$ARGV[1]' -- A E
>> A
>> B
>> C
>> D
>> E
>
> I had that thought, too, but I think it would be an error to do so.
> test_seq is supposed to be a replacement for "seq", which does not
> understand non-numeric sequences.

Although, the comment block just above test_seq() in
test-lib-functions.sh says otherwise:

    Print a sequence of numbers or letters in increasing order.  This
    is similar to GNU seq(1), but the latter might not be available
    everywhere (and does not do letters).  It may be used like:

    for i in $(test_seq 100)
    do
        for j in $(test_seq 10 20)
        do
            for k in $(test_seq a z)
            do
                echo $i-$j-$k
            done
        done
    done

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

* Re: t6044 broken on pu
  2016-05-09 16:02               ` Eric Sunshine
@ 2016-05-09 16:12                 ` Jeff King
  2016-05-09 18:26                   ` Junio C Hamano
  0 siblings, 1 reply; 20+ messages in thread
From: Jeff King @ 2016-05-09 16:12 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: demerphq, Junio C Hamano, Torsten Bögershausen,
	Andreas Schwab, newren, Git Mailing List

On Mon, May 09, 2016 at 12:02:45PM -0400, Eric Sunshine wrote:

> > I had that thought, too, but I think it would be an error to do so.
> > test_seq is supposed to be a replacement for "seq", which does not
> > understand non-numeric sequences.
> 
> Although, the comment block just above test_seq() in
> test-lib-functions.sh says otherwise:
> 
>     Print a sequence of numbers or letters in increasing order.  This
>     is similar to GNU seq(1), but the latter might not be available
>     everywhere (and does not do letters).  It may be used like:
> 
>     for i in $(test_seq 100)
>     do
>         for j in $(test_seq 10 20)
>         do
>             for k in $(test_seq a z)
>             do
>                 echo $i-$j-$k
>             done
>         done
>     done

Oh, indeed. I apparently even Acked that documentation once upon a time. :-/

Anyway, I double-checked my earlier "grep" and I do not think anybody is
using that functionality. So I think we'd be OK to change it as long as
we updated the documentation to match.

I don't really care either way whether it is replaced or not (at one
point there were some people really interested in NO_PERL not even using
one-liners in the test suite, but I am not one of them).

-Peff

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

* Re: t6044 broken on pu
  2016-05-09  4:43           ` Torsten Bögershausen
@ 2016-05-09 18:22             ` Junio C Hamano
  0 siblings, 0 replies; 20+ messages in thread
From: Junio C Hamano @ 2016-05-09 18:22 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: Andreas Schwab, newren, Git Mailing List

Torsten Bögershausen <tboegi@web.de> writes:

> On 08.05.16 20:20, Junio C Hamano wrote:
>> Torsten Bögershausen <tboegi@web.de> writes:
>>
>>> May a  simple
>>>  printf "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n"
>>>
>>> be an option ?
>> If you were to do that, at least have the decency to make it more
>> readable by doing something like:
>>
>> 	printf "%s\n" 1 2 3 4 5 6 7 8 9 10
>>
>> ;-)
>>
>> But as I said, as a response to "t6044 broken on pu" bug report,
>> s/seq/test_seq/ is the only sensible change.
>>
>> Improving "test_seq, the alternative to seq" is a separate topic.
>>
>> If you have aversion to $PERL, perhaps do them without using
>> anything what is not expected to be built-in in modern shells,
>> perhaps like this?
> Please don't get me wrong -

I don't.

> I wasn't really clear why:

So is it now clear?

The title of the bug report is "t6044 broken on pu". The analysis of
the issue is "We use 'test_seq 1 10' when we want one to ten on each
line in the output to use in test to make sure it is portable, but a
new commit forgot the portability issue and used seq instead".

The only sensible fix is "Use test_seq like everybody else".

Improving the implementation of test_seq is a totally separate
issue.  It may be a good thing to do independently to save external
program, and the effect of such change will not be limited to t6044
but all other existing users of test_seq.

And that is why it must be done as a separate topic.

Why you think it is a good idea to update t6044 with printf
"1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n" is beyond me--don't you want to
make sure that existing users of test_seq benefits the same way?

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

* Re: t6044 broken on pu
  2016-05-09 16:12                 ` Jeff King
@ 2016-05-09 18:26                   ` Junio C Hamano
  2016-05-09 18:36                     ` Junio C Hamano
  0 siblings, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2016-05-09 18:26 UTC (permalink / raw)
  To: Jeff King
  Cc: Eric Sunshine, demerphq, Torsten Bögershausen,
	Andreas Schwab, newren, Git Mailing List

Jeff King <peff@peff.net> writes:

> On Mon, May 09, 2016 at 12:02:45PM -0400, Eric Sunshine wrote:
>
>> > I had that thought, too, but I think it would be an error to do so.
>> > test_seq is supposed to be a replacement for "seq", which does not
>> > understand non-numeric sequences.
>> 
>> Although, the comment block just above test_seq() in
>> test-lib-functions.sh says otherwise:
>> 
>>     Print a sequence of numbers or letters in increasing order.  This
>>     is similar to GNU seq(1), but the latter might not be available
>>     everywhere (and does not do letters).  It may be used like:
>> 
>>     for i in $(test_seq 100)
>>     do
>>         for j in $(test_seq 10 20)
>>         do
>>             for k in $(test_seq a z)
>>             do
>>                 echo $i-$j-$k
>>             done
>>         done
>>     done
>
> Oh, indeed. I apparently even Acked that documentation once upon a time. :-/
>
> Anyway, I double-checked my earlier "grep" and I do not think anybody is
> using that functionality. So I think we'd be OK to change it as long as
> we updated the documentation to match.

Yes, I think the comment should just go.  Nobody used that alphabet
form since it was written in d17cf5f3 (tests: Introduce test_seq,
2012-08-04).

> I don't really care either way whether it is replaced or not (at one
> point there were some people really interested in NO_PERL not even using
> one-liners in the test suite, but I am not one of them).

Neither am I, but I think it is prudent to drop that "letters".  The
comment even says the letter form is not portable already, so the
mention of GNU seq(1) is not helping at all.

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

* Re: t6044 broken on pu
  2016-05-09 18:26                   ` Junio C Hamano
@ 2016-05-09 18:36                     ` Junio C Hamano
  2016-05-09 20:10                       ` Eric Sunshine
                                         ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Junio C Hamano @ 2016-05-09 18:36 UTC (permalink / raw)
  To: Jeff King
  Cc: Eric Sunshine, demerphq, Torsten Bögershausen,
	Andreas Schwab, newren, Git Mailing List

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

> Yes, I think the comment should just go.  Nobody used that alphabet
> form since it was written in d17cf5f3 (tests: Introduce test_seq,
> 2012-08-04).
>
>> I don't really care either way whether it is replaced or not (at one
>> point there were some people really interested in NO_PERL not even using
>> one-liners in the test suite, but I am not one of them).
>
> Neither am I, but I think it is prudent to drop that "letters".  The
> comment even says the letter form is not portable already, so the
> mention of GNU seq(1) is not helping at all.

-- >8 --
Subject: test-lib-functions.sh: remove misleading comment on test_seq

We never used the "letters" form since we came up with "test_seq" to
replace use of non-portable "seq" in our test script, which we
introduced it at d17cf5f3 (tests: Introduce test_seq, 2012-08-04).

We use this helper to either iterate for N times (i.e. the values on
the lines do not even matter), or just to get N distinct strings
(i.e. the values on the lines themselves do not really matter, but
we care that they are different from each other and reproducible).

Stop promising that we may allow using "letters"; this would open an
easier reimplementation that does not rely on $PERL, if somebody
later wants to.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/test-lib-functions.sh | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8d99eb3..cc9f61d 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -718,20 +718,13 @@ test_cmp_rev () {
 	test_cmp expect.rev actual.rev
 }
 
-# Print a sequence of numbers or letters in increasing order.  This is
-# similar to GNU seq(1), but the latter might not be available
-# everywhere (and does not do letters).  It may be used like:
-#
-#	for i in $(test_seq 100)
-#	do
-#		for j in $(test_seq 10 20)
-#		do
-#			for k in $(test_seq a z)
-#			do
-#				echo $i-$j-$k
-#			done
-#		done
-#	done
+# Print a sequence of integers in increasing order, either with
+# two arguments (start and end):
+#
+#     test_seq 1 5 -- outputs 1 2 3 4 5 one line at a time
+#
+# or with one argument (end), in which case it starts counting 
+# from 1.
 
 test_seq () {
 	case $# in

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

* Re: t6044 broken on pu
  2016-05-09 18:36                     ` Junio C Hamano
@ 2016-05-09 20:10                       ` Eric Sunshine
  2016-05-09 21:08                       ` Junio C Hamano
  2016-05-10  2:41                       ` Jeff King
  2 siblings, 0 replies; 20+ messages in thread
From: Eric Sunshine @ 2016-05-09 20:10 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, demerphq, Torsten Bögershausen, Andreas Schwab,
	Elijah Newren, Git Mailing List

On Mon, May 9, 2016 at 2:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Subject: test-lib-functions.sh: remove misleading comment on test_seq
>
> We never used the "letters" form since we came up with "test_seq" to
> replace use of non-portable "seq" in our test script, which we
> introduced it at d17cf5f3 (tests: Introduce test_seq, 2012-08-04).
>
> We use this helper to either iterate for N times (i.e. the values on
> the lines do not even matter), or just to get N distinct strings
> (i.e. the values on the lines themselves do not really matter, but
> we care that they are different from each other and reproducible).
>
> Stop promising that we may allow using "letters"; this would open an
> easier reimplementation that does not rely on $PERL, if somebody
> later wants to.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> @@ -718,20 +718,13 @@ test_cmp_rev () {
> -# Print a sequence of numbers or letters in increasing order.  This is
> -# similar to GNU seq(1), but the latter might not be available
> -# everywhere (and does not do letters).  It may be used like:
> -#
> -#      for i in $(test_seq 100)
> -#      do
> -#              for j in $(test_seq 10 20)
> -#              do
> -#                      for k in $(test_seq a z)
> -#                      do
> -#                              echo $i-$j-$k
> -#                      done
> -#              done
> -#      done
> +# Print a sequence of integers in increasing order, either with
> +# two arguments (start and end):
> +#
> +#     test_seq 1 5 -- outputs 1 2 3 4 5 one line at a time
> +#
> +# or with one argument (end), in which case it starts counting
> +# from 1.

This new documentation is quite readable. Thanks.

>  test_seq () {
>         case $# in

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

* Re: t6044 broken on pu
  2016-05-09 18:36                     ` Junio C Hamano
  2016-05-09 20:10                       ` Eric Sunshine
@ 2016-05-09 21:08                       ` Junio C Hamano
  2016-05-09 21:27                         ` Eric Sunshine
  2016-05-10  2:41                       ` Jeff King
  2 siblings, 1 reply; 20+ messages in thread
From: Junio C Hamano @ 2016-05-09 21:08 UTC (permalink / raw)
  To: Jeff King
  Cc: Eric Sunshine, demerphq, Torsten Bögershausen,
	Andreas Schwab, newren, Git Mailing List

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

> Stop promising that we may allow using "letters"; this would open an
> easier reimplementation that does not rely on $PERL, if somebody
> later wants to.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

And I am not the one who particularly wants to, but here is the
previous patch sent elsewhere in the thread.

-- >8 --
Subject: [PATCH] test-lib-functions.sh: rewrite test_seq without Perl

Rewrite the 'seq' imitation only with commands and features
that are typically found as built-in in modern POSIX shells,
instead of relying on Perl to run a single-liner.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/test-lib-functions.sh | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 39b8151..9734e32 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -679,7 +679,12 @@ test_seq () {
 	2)	;;
 	*)	error "bug in the test script: not 1 or 2 parameters to test_seq" ;;
 	esac
-	perl -le 'print for $ARGV[0]..$ARGV[1]' -- "$@"
+	test_seq_counter__=$1
+	while test "$test_seq_counter__" -le "$2"
+	do
+		echo "$test_seq_counter__"
+		test_seq_counter__=$(( $test_seq_counter__ + 1 ))
+	done
 }
 
 # This function can be used to schedule some commands to be run
-- 
2.8.2-557-gee41d5e

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

* Re: t6044 broken on pu
  2016-05-09 21:08                       ` Junio C Hamano
@ 2016-05-09 21:27                         ` Eric Sunshine
  0 siblings, 0 replies; 20+ messages in thread
From: Eric Sunshine @ 2016-05-09 21:27 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jeff King, demerphq, Torsten Bögershausen, Andreas Schwab,
	Elijah Newren, Git Mailing List

On Mon, May 9, 2016 at 5:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Subject: [PATCH] test-lib-functions.sh: rewrite test_seq without Perl
>
> Rewrite the 'seq' imitation only with commands and features
> that are typically found as built-in in modern POSIX shells,
> instead of relying on Perl to run a single-liner.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> @@ -679,7 +679,12 @@ test_seq () {
>         2)      ;;
>         *)      error "bug in the test script: not 1 or 2 parameters to test_seq" ;;
>         esac
> -       perl -le 'print for $ARGV[0]..$ARGV[1]' -- "$@"
> +       test_seq_counter__=$1
> +       while test "$test_seq_counter__" -le "$2"
> +       do
> +               echo "$test_seq_counter__"
> +               test_seq_counter__=$(( $test_seq_counter__ + 1 ))
> +       done
>  }

Looks (obviously) correct and works as expected on Mac and BSD.

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

* Re: t6044 broken on pu
  2016-05-09 18:36                     ` Junio C Hamano
  2016-05-09 20:10                       ` Eric Sunshine
  2016-05-09 21:08                       ` Junio C Hamano
@ 2016-05-10  2:41                       ` Jeff King
  2 siblings, 0 replies; 20+ messages in thread
From: Jeff King @ 2016-05-10  2:41 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Eric Sunshine, demerphq, Torsten Bögershausen,
	Andreas Schwab, newren, Git Mailing List

On Mon, May 09, 2016 at 11:36:09AM -0700, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Yes, I think the comment should just go.  Nobody used that alphabet
> > form since it was written in d17cf5f3 (tests: Introduce test_seq,
> > 2012-08-04).
> >
> >> I don't really care either way whether it is replaced or not (at one
> >> point there were some people really interested in NO_PERL not even using
> >> one-liners in the test suite, but I am not one of them).
> >
> > Neither am I, but I think it is prudent to drop that "letters".  The
> > comment even says the letter form is not portable already, so the
> > mention of GNU seq(1) is not helping at all.
> 
> -- >8 --
> Subject: test-lib-functions.sh: remove misleading comment on test_seq

Thanks, this (and the actual implementation change) both look good to
me, and I'm happy with either or both being applied.

-Peff

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

end of thread, other threads:[~2016-05-10  2:41 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-07 12:00 t6044 broken on pu Torsten Bögershausen
2016-05-07 12:19 ` Andreas Schwab
2016-05-07 13:15   ` Ramsay Jones
2016-05-07 13:43     ` Ramsay Jones
2016-05-07 16:18   ` Torsten Bögershausen
2016-05-08  2:21     ` Junio C Hamano
2016-05-08  6:54       ` Torsten Bögershausen
2016-05-08 18:20         ` Junio C Hamano
2016-05-09  4:43           ` Torsten Bögershausen
2016-05-09 18:22             ` Junio C Hamano
2016-05-09  6:30           ` demerphq
2016-05-09  8:33             ` Jeff King
2016-05-09 16:02               ` Eric Sunshine
2016-05-09 16:12                 ` Jeff King
2016-05-09 18:26                   ` Junio C Hamano
2016-05-09 18:36                     ` Junio C Hamano
2016-05-09 20:10                       ` Eric Sunshine
2016-05-09 21:08                       ` Junio C Hamano
2016-05-09 21:27                         ` Eric Sunshine
2016-05-10  2:41                       ` Jeff King

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.