[PATCH] p3400: stop using tac(1)

[PATCH] p3400: stop using tac(1)

[René Scharfe]

b3dfeebb92 (rebase: avoid computing unnecessary patch IDs, 2016-07-29)
added a perf test that calls tac(1) from GNU core utilities.  Support
systems without it by reversing the generated list using sort -nr
instead.  sort(1) with options -n and -r is already used in other tests.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 t/perf/p3400-rebase.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/perf/p3400-rebase.sh b/t/perf/p3400-rebase.sh
index 7a0bb29448..43d5a34e8c 100755
--- a/t/perf/p3400-rebase.sh
+++ b/t/perf/p3400-rebase.sh
@@ -18,7 +18,7 @@ test_expect_success 'setup rebasing on top of a lot of changes' '
 		test_tick &&
 		git commit -m commit$i unrelated-file$i &&
 		echo change$i >unrelated-file$i &&
-		test_seq 1000 | tac >>unrelated-file$i &&
+		test_seq 1000 | sort -nr >>unrelated-file$i &&
 		git add unrelated-file$i &&
 		test_tick &&
 		git commit -m commit$i-reverse unrelated-file$i ||
--
2.33.0

Re: [PATCH] p3400: stop using tac(1)

[Jeff King]

On Sat, Oct 02, 2021 at 07:44:14PM +0200, René Scharfe wrote:

> b3dfeebb92 (rebase: avoid computing unnecessary patch IDs, 2016-07-29)
> added a perf test that calls tac(1) from GNU core utilities.  Support
> systems without it by reversing the generated list using sort -nr
> instead.  sort(1) with options -n and -r is already used in other tests.

Cute fix. With regular seq(1), this whole thing can become:

  seq 1000 -1 1

without the extra process, but our test_seq doesn't understand non-1
increments (nor comparisons besides -le). It wouldn't be that hard to
teach it, but given that this is the first time we've wanted it, it may
not be worth the effort.

-Peff

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 6348e8d733..76c8c0f2f6 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1149,15 +1149,22 @@ test_cmp_fspath () {
 
 test_seq () {
 	case $# in
-	1)	set 1 "$@" ;;
-	2)	;;
-	*)	BUG "not 1 or 2 parameters to test_seq" ;;
+	1)	set 1 1 "$1" ;;
+	2)	set "$1" 1 "$2" ;;
+	3)	;;
+	*)	BUG "not 1, 2, or 3 parameters to test_seq" ;;
 	esac
+	if test "$1" -lt "$3"
+	then
+		test_seq_cmp__=-le
+	else
+		test_seq_cmp__=-ge
+	fi
 	test_seq_counter__=$1
-	while test "$test_seq_counter__" -le "$2"
+	while test "$test_seq_counter__" $test_seq_cmp__ "$3"
 	do
 		echo "$test_seq_counter__"
-		test_seq_counter__=$(( $test_seq_counter__ + 1 ))
+		test_seq_counter__=$(( $test_seq_counter__ + $2 ))
 	done
 }
 

Re: [PATCH] p3400: stop using tac(1)

[René Scharfe]

Am 04.10.21 um 10:31 schrieb Jeff King:
> On Sat, Oct 02, 2021 at 07:44:14PM +0200, René Scharfe wrote:
>
>> b3dfeebb92 (rebase: avoid computing unnecessary patch IDs, 2016-07-29)
>> added a perf test that calls tac(1) from GNU core utilities.  Support
>> systems without it by reversing the generated list using sort -nr
>> instead.  sort(1) with options -n and -r is already used in other tests.
>
> Cute fix. With regular seq(1), this whole thing can become:
>
>   seq 1000 -1 1
>
> without the extra process, but our test_seq doesn't understand non-1
> increments (nor comparisons besides -le). It wouldn't be that hard to
> teach it, but given that this is the first time we've wanted it, it may
> not be worth the effort.

Right.  The original also allows "seq 1000 1", by the way.  Not sure we
need that either.

But when you say "without the extra process", I think:

test_seq () {
	set_step='END {step = first < last ? 1 : -1}'
	loop='END {for (; first <= last && step > 0 || first >= last && step < 0; first += step) print first}'
	case $# in
	1)      awk -v first=1    -v last="$1"    "$set_step $loop" ;;
	2)      awk -v first="$1" -v last="$2"    "$set_step $loop" ;;
	3)      awk -v first="$1" -v last="$3" -v step="$2" "$loop" ;;
	*)      BUG "not 1, 2, or 3 parameters to test_seq" ;;
	esac </dev/null
}

;-)

René

Re: [PATCH] p3400: stop using tac(1)

[Jeff King]

On Tue, Oct 05, 2021 at 08:45:38PM +0200, René Scharfe wrote:

> Am 04.10.21 um 10:31 schrieb Jeff King:
> > On Sat, Oct 02, 2021 at 07:44:14PM +0200, René Scharfe wrote:
> >
> >> b3dfeebb92 (rebase: avoid computing unnecessary patch IDs, 2016-07-29)
> >> added a perf test that calls tac(1) from GNU core utilities.  Support
> >> systems without it by reversing the generated list using sort -nr
> >> instead.  sort(1) with options -n and -r is already used in other tests.
> >
> > Cute fix. With regular seq(1), this whole thing can become:
> >
> >   seq 1000 -1 1
> >
> > without the extra process, but our test_seq doesn't understand non-1
> > increments (nor comparisons besides -le). It wouldn't be that hard to
> > teach it, but given that this is the first time we've wanted it, it may
> > not be worth the effort.
> 
> Right.  The original also allows "seq 1000 1", by the way.  Not sure we
> need that either.

I'm not sure what you mean by "original" here. "seq 1000 1" produces no
output for me (nor does it work with test_seq).

> But when you say "without the extra process", I think:

I meant without the extra tac/sort process.

> test_seq () {
> 	set_step='END {step = first < last ? 1 : -1}'
> 	loop='END {for (; first <= last && step > 0 || first >= last && step < 0; first += step) print first}'
> 	case $# in
> 	1)      awk -v first=1    -v last="$1"    "$set_step $loop" ;;
> 	2)      awk -v first="$1" -v last="$2"    "$set_step $loop" ;;
> 	3)      awk -v first="$1" -v last="$3" -v step="$2" "$loop" ;;
> 	*)      BUG "not 1, 2, or 3 parameters to test_seq" ;;
> 	esac </dev/null
> }

Right, that works. It does introduce an extra awk process per
invocation, though I doubt that really matters all that much. The diff I
showed in my earlier response works totally in shell, like the current
test_seq().

-Peff

Re: [PATCH] p3400: stop using tac(1)

[René Scharfe]

Am 05.10.21 um 21:38 schrieb Jeff King:
> On Tue, Oct 05, 2021 at 08:45:38PM +0200, René Scharfe wrote:
>
>> Am 04.10.21 um 10:31 schrieb Jeff King:
>>> On Sat, Oct 02, 2021 at 07:44:14PM +0200, René Scharfe wrote:
>>>
>>>> b3dfeebb92 (rebase: avoid computing unnecessary patch IDs, 2016-07-29)
>>>> added a perf test that calls tac(1) from GNU core utilities.  Support
>>>> systems without it by reversing the generated list using sort -nr
>>>> instead.  sort(1) with options -n and -r is already used in other tests.
>>>
>>> Cute fix. With regular seq(1), this whole thing can become:
>>>
>>>   seq 1000 -1 1
>>>
>>> without the extra process, but our test_seq doesn't understand non-1
>>> increments (nor comparisons besides -le). It wouldn't be that hard to
>>> teach it, but given that this is the first time we've wanted it, it may
>>> not be worth the effort.
>>
>> Right.  The original also allows "seq 1000 1", by the way.  Not sure we
>> need that either.
>
> I'm not sure what you mean by "original" here. "seq 1000 1" produces no
> output for me (nor does it work with test_seq).

I meant the non-shell one, i.e. seq(1).  I somehow expected everyone to use
the same, but of course there is GNU seq, which has "an omitted INCREMENT
defaults to 1 even when LAST is smaller than FIRST" [1] and BSD seq with
"When first is larger than last the default incr is -1" [2].

[1] https://www.man7.org/linux/man-pages/man1/seq.1.html
[2] https://man.netbsd.org/seq.1

René

Re: [PATCH] p3400: stop using tac(1)

[René Scharfe]

Am 05.10.21 um 21:38 schrieb Jeff King:
> Right, that works. It does introduce an extra awk process per
> invocation, though I doubt that really matters all that much. The diff I
> showed in my earlier response works totally in shell, like the current
> test_seq().

In my mind, test is an external command.  Wikipedia [1] says it has been
a builtin since the early 80ies, and I couldn't find a shell without it.
I wonder where I picked up that outdated assumption -- I'm not actually
_that_ old.  Time for an update..

René


[1] https://en.wikipedia.org/wiki/Test_(Unix)

Re: [PATCH] p3400: stop using tac(1)

[Junio C Hamano]

René Scharfe <l.s.r@web.de> writes:

> In my mind, test is an external command.  Wikipedia [1] says it has been
> a builtin since the early 80ies, and I couldn't find a shell without it.
> I wonder where I picked up that outdated assumption -- I'm not actually
> _that_ old.  Time for an update..

Funny, as I was wondering about the same thing the other day in a
totally different context.

Our older shell script (like tests) tends to strongly prefer "case
... esac" over "test" for this exact reason, e.g option parsing loop
often uses

    while case $#,$1 in 0,*) break ;; *,-*) ;; *) break ;; esac
    do
	...

instead of

    while test $# -gt 0 && ...

to avoid "test".  I'm not actually _that_ old, either, but my
current suspicion is that those who traind us are old enough ;-)