| Message ID | cf252e24b8c4da19ee9f886a1ab9c9c391d89d66.1612216941.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | FSMonitor Preliminary Commits | expand |
"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Jeff Hostetler <jeffhost@microsoft.com> > > The Mac version of xargs does not support the "-d" option. Convert the test > setup to pipe the data set thru `lf_to_nul | xargs -0` instead. "xargs -0" is not all that portable, either, and neither is "touch -h". But since the t/perf stuff already depends on having GNU toolchain anyway, I can be persuaded to believe that it is OK. Do we know that this part runs much later than the staged files are last touched, so that these uses of "touch" actually are effective to make the paths stat-dirty? Otherwise, we may be just "touch"ing them with the timestamp they already have after all. Thanks. > Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> > --- > t/perf/p7519-fsmonitor.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh > index 9b43342806b..7bb37e9a6c1 100755 > --- a/t/perf/p7519-fsmonitor.sh > +++ b/t/perf/p7519-fsmonitor.sh > @@ -165,7 +165,7 @@ test_fsmonitor_suite() { > ' > > test_perf_w_drop_caches "status (dirty) ($DESC)" ' > - git ls-files | head -100000 | xargs -d "\n" touch -h && > + git ls-files | head -100000 | lf_to_nul | xargs -0 touch -h && > git status > '
On 2/1/21 6:25 PM, Junio C Hamano wrote: > "Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> From: Jeff Hostetler <jeffhost@microsoft.com> >> >> The Mac version of xargs does not support the "-d" option. Convert the test >> setup to pipe the data set thru `lf_to_nul | xargs -0` instead. > > "xargs -0" is not all that portable, either, and neither is "touch -h". > > But since the t/perf stuff already depends on having GNU toolchain > anyway, I can be persuaded to believe that it is OK. > > Do we know that this part runs much later than the staged files are > last touched, so that these uses of "touch" actually are effective > to make the paths stat-dirty? Otherwise, we may be just "touch"ing > them with the timestamp they already have after all. I'm not sure now that you mention it. I suppose on modern filesystems that have mtimes with nanosecond fields we could (are) assuming that "touch" is actually doing something. On older filesystems (such as FAT32), you're right it is probably not doing anything at the speed that the test runs. TBH I'm not sure that the test needs the "-h". Symlinks are not that common and it shouldn't affect the timings that much if there are a few. I'm not sure what to do about "-0". Not even "--null" is portable. Let me do a little digging here. > > Thanks. > >> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> >> --- >> t/perf/p7519-fsmonitor.sh | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh >> index 9b43342806b..7bb37e9a6c1 100755 >> --- a/t/perf/p7519-fsmonitor.sh >> +++ b/t/perf/p7519-fsmonitor.sh >> @@ -165,7 +165,7 @@ test_fsmonitor_suite() { >> ' >> >> test_perf_w_drop_caches "status (dirty) ($DESC)" ' >> - git ls-files | head -100000 | xargs -d "\n" touch -h && >> + git ls-files | head -100000 | lf_to_nul | xargs -0 touch -h && >> git status >> '
Jeff Hostetler <git@jeffhostetler.com> writes: > I'm not sure now that you mention it. I suppose on modern filesystems > that have mtimes with nanosecond fields we could (are) assuming that > "touch" is actually doing something. On older filesystems (such as > FAT32), you're right it is probably not doing anything at the speed > that the test runs. That one is probably the most relevant nit among the ones I raised. I do not actually mind if we used test-chmtime to force our own timestamp (e.g. "5 seconds before the filesystem time"), and added the helper the "--stdin" option to read paths to work around the "xargs" issue. > TBH I'm not sure that the test needs the "-h". Symlinks are not that > common and it shouldn't affect the timings that much if there are a few. I agree. > I'm not sure what to do about "-0". Not even "--null" is portable. Correct. I do not think it is worth "digging", though. I do not mind "ls-files -z | test-tool chmtime -600 --stdin -z" to lose xargs, but we already depend on GNU time to run t/perf, and it is not too far a stretch to require GNU xargs that knows "-0" or "-d". Thanks.
diff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh index 9b43342806b..7bb37e9a6c1 100755 --- a/t/perf/p7519-fsmonitor.sh +++ b/t/perf/p7519-fsmonitor.sh @@ -165,7 +165,7 @@ test_fsmonitor_suite() { ' test_perf_w_drop_caches "status (dirty) ($DESC)" ' - git ls-files | head -100000 | xargs -d "\n" touch -h && + git ls-files | head -100000 | lf_to_nul | xargs -0 touch -h && git status '