| Message ID | 85831cc2-307f-1be8-9bb3-c44028ad07fa@web.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 100c2da2d3a330366588143d720f09a88926972a |
| Headers | show |
| Series | p3400: stop using tac(1) | expand |
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 }
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é
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
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é
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)
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 ;-)
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 ||
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(-) -- 2.33.0