Hi Patrick, Le 2025-03-31 à 03:27, Patrick Steinhardt a écrit : > On Fri, Mar 28, 2025 at 05:07:48PM +0000, Philippe Blain via GitGitGadget wrote: >> diff --git a/t/perf/p7821-grep-engines-fixed.sh b/t/perf/p7821-grep-engines-fixed.sh >> index 61e41b82cff..1d126c7b039 100755 >> --- a/t/perf/p7821-grep-engines-fixed.sh >> +++ b/t/perf/p7821-grep-engines-fixed.sh >> @@ -33,13 +33,13 @@ do >> fi >> if ! test_have_prereq PERF_GREP_ENGINES_THREADS >> then >> - test_perf $prereq "$engine grep$GIT_PERF_7821_GREP_OPTS $pattern" " >> + test_perf "$engine grep$GIT_PERF_7821_GREP_OPTS $pattern" --prereq "$prereq" " >> git -c grep.patternType=$engine grep$GIT_PERF_7821_GREP_OPTS $pattern >'out.$engine' || : >> " >> else >> for threads in $GIT_PERF_GREP_THREADS >> do >> - test_perf PTHREADS,$prereq "$engine grep$GIT_PERF_7821_GREP_OPTS $pattern with $threads threads" " >> + test_perf "$engine grep$GIT_PERF_7821_GREP_OPTS $pattern with $threads threads" --prereq "PTHREADS,$prereq" " >> git -c grep.patternType=$engine -c grep.threads=$threads grep$GIT_PERF_7821_GREP_OPTS $pattern >'out.$engine.$threads' || : >> " >> done > > "$prereq" can be empty here as it depends on which regexp engine we're > using. The second case you adapt already looked weird before because we > potentially checked for "PTHREADS,", Indeed, the reason why the second case did not fail even when built with PCRE (which would not fail the first 'test_perf' since '$prereq' would be empty) is that this second test (in fact the whole loop) is only reached if GIT_PERF_GREP_THREADS is set in the environment, which sets the PERF_GREP_ENGINES_THREADS prereq. So just running 'make perf' or './p7821-*' would not enter this part of the test. > but the first case was correct > before but is now potentially checking for the empty prerequisite. Does > that actually work as expected? Yes, it was correct when built with PCRE, but not without, as then $prereq would not be empty. I did check that it works correctly before sending the patch, both when built with and without PCRE. Thank you for the review and also checking that the patch works correctly. I just checked 'test_skip' which is the function that checks the prereq and indeed and empty 'test_prereq' is treated as no prereq. Cheers, Philippe.
