{"thread":{"id":"54930","subject":"Re: [PATCH] p7519: allow running without watchman prereq","startedAt":"2021-01-04T23:57:24Z","lastAt":"2021-01-05T00:12:02Z","messageCount":2,"participants":["Nipunn Koorapati","Taylor Blau"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"413410","messageId":"CABVhGpUJhS_uq14p1SuhTt9x5c0rkmB_0sw9uSmce8+4WKATRw@mail.gmail.com","threadId":"54930","inReplyTo":"46b2e5af59c451b816973ba3f2c32a68639298b3.1609796006.git.me@ttaylorr.com","subject":"Re: [PATCH] p7519: allow running without watchman prereq","fromName":"Nipunn Koorapati","fromEmail":"nipunn@dropbox.com","sentAt":"2021-01-04T23:56:27Z","receivedAt":"2021-01-04T23:57:24Z","isPatch":true,"sender":{"key":"nipunn@dropbox.com","avatar":null},"body":"> I could do without the 'else' block here, but it seems straightforward\n> enough. Likewise, the \"ensure no silent error\" comment could certainly\n> be removed (since 'test_must_be_empty error' makes what is happening\n> quite clear), but the fix pursued here is minimal.\n\nThis looks great to me. Thanks for fixing - and appreciate the else\nclause here for the reader.\n\n--Nipunn\n"},{"id":"413413","messageId":"46b2e5af59c451b816973ba3f2c32a68639298b3.1609796006.git.me@ttaylorr.com","threadId":"54930","inReplyTo":null,"subject":"[PATCH] p7519: allow running without watchman prereq","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-01-04T21:35:37Z","receivedAt":"2021-01-05T00:12:02Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"p7519 measures the performance of the fsmonitor code. To do this, it\nuses the installed copy of Watchman. If Watchman isn't installed, a noop\nintegration script is installed in its place.\n\nWhen in the latter mode, it is expected that the script should not write\na \"last update token\": in fact, it doesn't write anything at all since\nthe script is blank.\n\nCommit 33226af42b (t/perf/fsmonitor: improve error message if typoing\nhook name, 2020-10-26) made sure that running 'git update-index\n--fsmonitor' did not write anything to stderr, but this is not the case\nwhen using the empty Watchman script, since Git will complain that:\n\n    $ which watchman\n    watchman not found\n    $ cat .git/hooks/fsmonitor-empty\n    $ git -c core.fsmonitor=.git/hooks/fsmonitor-empty update-index --fsmonitor\n    warning: Empty last update token.\n\nPrior to 33226af42b, the output wasn't checked at all, which allowed\nthis noop mode to work. But, 33226af42b breaks p7519 when running it\nwithout a 'watchman(1)' on your system.\n\nHandle this by only checking that the stderr is empty only when running\nwith a real watchman executable. Otherwise, assert that the error\nmessage is the expected one when running in the noop mode.\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\nNoticed while running the perf tests on v2.30.0 on a system that happens\nto not have Watchman installed.\n\nI could do without the 'else' block here, but it seems straightforward\nenough. Likewise, the \"ensure no silent error\" comment could certainly\nbe removed (since 'test_must_be_empty error' makes what is happening\nquite clear), but the fix pursued here is minimal.\n\n t/perf/p7519-fsmonitor.sh | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/t/perf/p7519-fsmonitor.sh b/t/perf/p7519-fsmonitor.sh\nindex 9b43342806..1e20a184c7 100755\n--- a/t/perf/p7519-fsmonitor.sh\n+++ b/t/perf/p7519-fsmonitor.sh\n@@ -129,7 +129,12 @@ setup_for_fsmonitor() {\n\n \tgit config core.fsmonitor \"$INTEGRATION_SCRIPT\" &&\n \tgit update-index --fsmonitor 2>error &&\n-\ttest_must_be_empty error  # ensure no silent error\n+\tif test_have_prereq WATCHMAN\n+\tthen\n+\t\ttest_must_be_empty error  # ensure no silent error\n+\telse\n+\t\tgrep \"Empty last update token\" error\n+\tfi\n }\n\n test_perf_w_drop_caches () {\n--\n2.29.2.533.g07db1f5344\n"}]}