On 9/13/2018 2:54 PM, Ævar Arnfjörð Bjarmason wrote:
On Thu, Sep 13 2018, Ben Peart wrote:diff --git a/config.c b/config.c index 3461993f0a..3555c63f28 100644 --- a/config.c +++ b/config.c @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void) int git_config_get_fsmonitor(void) { if (git_config_get_pathname("core.fsmonitor", &core_fsmonitor)) - core_fsmonitor = getenv("GIT_FSMONITOR_TEST"); + core_fsmonitor = getenv("GIT_TEST_FSMONITOR"); if (core_fsmonitor && !*core_fsmonitor) core_fsmonitor = NULL; diff --git a/t/README b/t/README index 9028b47d92..545438c820 100644 --- a/t/README +++ b/t/README @@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. +GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor +code path for utilizing a file system monitor to speed up detecting +new or changed files. + Naming Tests ------------I've seen this & will watch out for it, but still, when I'm updating to "next" in a couple of months I may not be tracking the exact state of the integration of this patch, and then running with GIT_FSMONITOR_TEST=... will suddenly be a noop. So maybe something like this to test-lib.sh as well (or directly in config.c): if test -n "$GIT_FSMONITOR_TEST" then echo "The GIT_FSMONITOR_TEST variable has been renamed to GIT_TEST_FSMONITOR" exit 1 fi Maybe I'm being too nitpicky and there's only two of us who run the test with this anyway, and we can deal with it.
I agree that there are probably only 2 people in the world who ever used this but I'm happy to add the additional test to make it obvious it has been renamed.
It just rubs me the wrong way that we have a test mode that silently stops being picked up because a command-line option or env variable got renamed, especially since we've had it for 4 stable releases, especially since it's so easy for us to avoid that confusion (just die), v.s. potential time wasted downstream (wondering why fsmonitor stuff broke on $SOME_OS even though we're testing for it during package build...).
