t6044 broken on pu

classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

t6044 broken on pu

Torsten Bögershausen-2
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 &&
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Andreas Schwab-3
Torsten Bögershausen <[hidden email]> 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, [hidden email]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Ramsay Jones-2


On 07/05/16 13:19, Andreas Schwab wrote:

> Torsten Bögershausen <[hidden email]> 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


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Ramsay Jones-2


On 07/05/16 14:15, Ramsay Jones wrote:

>
>
> On 07/05/16 13:19, Andreas Schwab wrote:
>> Torsten Bögershausen <[hidden email]> 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


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Torsten Bögershausen-2
In reply to this post by Andreas Schwab-3
On 2016-05-07 14.19, Andreas Schwab wrote:

> Torsten Bögershausen <[hidden email]> 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 ?


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Junio C Hamano
Torsten Bögershausen <[hidden email]> 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).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Torsten Bögershausen-2
On 08.05.16 04:21, Junio C Hamano wrote:

> Torsten Bögershausen <[hidden email]> 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 ?


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Junio C Hamano
Torsten Bögershausen <[hidden email]> 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Torsten Bögershausen-2


On 08.05.16 20:20, Junio C Hamano wrote:

> Torsten Bögershausen <[hidden email]> 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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

demerphq
In reply to this post by Junio C Hamano
On 8 May 2016 at 20:20, Junio C Hamano <[hidden email]> wrote:

> Torsten Bögershausen <[hidden email]> 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/"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Jeff King
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Eric Sunshine
On Mon, May 9, 2016 at 4:33 AM, Jeff King <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Jeff King
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Junio C Hamano
In reply to this post by Torsten Bögershausen-2
Torsten Bögershausen <[hidden email]> writes:

> On 08.05.16 20:20, Junio C Hamano wrote:
>> Torsten Bögershausen <[hidden email]> 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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Junio C Hamano
In reply to this post by Jeff King
Jeff King <[hidden email]> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Junio C Hamano
Junio C Hamano <[hidden email]> 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 <[hidden email]>
---
 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Eric Sunshine
On Mon, May 9, 2016 at 2:36 PM, Junio C Hamano <[hidden email]> 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 <[hidden email]>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Junio C Hamano
In reply to this post by Junio C Hamano
Junio C Hamano <[hidden email]> 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 <[hidden email]>

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 <[hidden email]>
---
 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Eric Sunshine
On Mon, May 9, 2016 at 5:08 PM, Junio C Hamano <[hidden email]> 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 <[hidden email]>
> ---
> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: t6044 broken on pu

Jeff King
In reply to this post by Junio C Hamano
On Mon, May 09, 2016 at 11:36:09AM -0700, Junio C Hamano wrote:

> Junio C Hamano <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html