From 79bb4c8e7d74c9c85cd6e6616811fc9a2e7c0afa Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 5 Nov 2019 13:32:31 -0500 Subject: [PATCH 1/8] fsmonitor: disable in a bare repo The fsmonitor feature allows an external tool such as watchman to monitor the working directory. The direct test t7619-status-fsmonitor.sh provides some coverage, but it would be better to run the entire test suite with watchman enabled. This would provide more confidence that the feature is working as intended. If the repository is bare, then there is no working directory to watch. Disable the core_fsmonitor global in this case. Before this change, the test t0003-attributes.sh would fail with GIT_TEST_FSMONITOR pointing to t/t7519/fsmonitor-watchman. Signed-off-by: Derrick Stolee --- config.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/config.c b/config.c index 3900e4947be92b..f6d4e2fae39d5e 100644 --- a/config.c +++ b/config.c @@ -2339,6 +2339,11 @@ int git_config_get_max_percent_split_change(void) int git_config_get_fsmonitor(void) { + if (!the_repository->worktree) { + core_fsmonitor = 0; + return 0; + } + if (git_config_get_pathname("core.fsmonitor", &core_fsmonitor)) core_fsmonitor = getenv("GIT_TEST_FSMONITOR"); From dd492091e329fbc75571dd5d64bfdfd845daec80 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 5 Nov 2019 13:33:28 -0500 Subject: [PATCH 2/8] fsmonitor: do not output to stderr for tests The fsmonitor feature allows an external tool such as watchman to monitor the working directory. The direct test t7619-status-fsmonitor.sh provides some coverage, but it would be better to run the entire test suite with watchman enabled. This would provide more confidence that the feature is working as intended. The test t0003-attributes.sh and others would fail when GIT_TEST_FSMONITOR is pointing at t/t7519/fsmonitor-watchman because it sends a message over stderr when registering the repo with watchman for the first time. Remove this stderr message for the test script to avoid this noise. Signed-off-by: Derrick Stolee --- t/t7519/fsmonitor-watchman | 1 - 1 file changed, 1 deletion(-) diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman index d8e7a1e5ba85c0..06312876aa44bf 100755 --- a/t/t7519/fsmonitor-watchman +++ b/t/t7519/fsmonitor-watchman @@ -94,7 +94,6 @@ sub launch_watchman { my $o = $json_pkg->new->utf8->decode($response); if ($retry > 0 and $o->{error} and $o->{error} =~ m/unable to resolve root .* directory (.*) is not watched/) { - print STDERR "Adding '$git_work_tree' to watchman's watch list.\n"; $retry--; qx/watchman watch "$git_work_tree"/; die "Failed to make watchman watch '$git_work_tree'.\n" . From f9db0c3416ff50587bffe46bb00268d8094c1ad7 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 5 Nov 2019 13:34:42 -0500 Subject: [PATCH 3/8] t1301-shared-repo.sh: disable FSMONITOR The fsmonitor feature allows an external tool such as watchman to monitor the working directory. The direct test t7619-status-fsmonitor.sh provides some coverage, but it would be better to run the entire test suite with watchman enabled. This would provide more confidence that the feature is working as intended. The test t1301-shared-repo.sh would fail when GIT_TEST_FSMONITOR is set to t/t7519/fsmonitor-watchman because it changes permissions in an incompatible way. Signed-off-by: Derrick Stolee --- t/t1301-shared-repo.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh index 2dc853d1be5f0f..665ade0cf2b7a3 100755 --- a/t/t1301-shared-repo.sh +++ b/t/t1301-shared-repo.sh @@ -128,6 +128,7 @@ test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' ' ' test_expect_success POSIXPERM 'forced modes' ' + GIT_TEST_FSMONITOR="" && mkdir -p templates/hooks && echo update-server-info >templates/hooks/post-update && chmod +x templates/hooks/post-update && From efc16962ee2595db50bf051fc84632b8c70036b3 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 5 Nov 2019 13:44:28 -0500 Subject: [PATCH 4/8] t3030-merge-recursive.sh: disable fsmonitor when tweaking GIT_WORK_TREE The fsmonitor feature allows an external tool such as watchman to monitor the working directory. The direct test t7619-status-fsmonitor.sh provides some coverage, but it would be better to run the entire test suite with watchman enabled. This would provide more confidence that the feature is working as intended. Worktrees use a ".git" _file_ instead of a folder to point to the base repo's .git directory and the proper worktree HEAD. The fsmonitor hook tries to create a JSON file inside the ".git" folder which violates the expectation here. It would be better to properly find a safe folder for storing this JSON file. This is also a problem when a test script uses GIT_WORK_TREE. Signed-off-by: Derrick Stolee --- t/t1510-repo-setup.sh | 1 + t/t2400-worktree-add.sh | 2 ++ t/t3030-merge-recursive.sh | 2 ++ 3 files changed, 5 insertions(+) diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh index 9974457f5615c9..28dce0c26f66dd 100755 --- a/t/t1510-repo-setup.sh +++ b/t/t1510-repo-setup.sh @@ -775,6 +775,7 @@ test_expect_success '#29: setup' ' setup_repo 29 non-existent gitfile true && mkdir -p 29/sub/sub 29/wt/sub && ( + GIT_TEST_FSMONITOR="" && cd 29 && GIT_WORK_TREE="$here/29" && export GIT_WORK_TREE && diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh index e819ba741ec960..d4d3cbae0fe971 100755 --- a/t/t2400-worktree-add.sh +++ b/t/t2400-worktree-add.sh @@ -1,5 +1,7 @@ #!/bin/sh +GIT_TEST_FSMONITOR="" + test_description='test git worktree add' . ./test-lib.sh diff --git a/t/t3030-merge-recursive.sh b/t/t3030-merge-recursive.sh index ff641b348a1bac..62f645d6392919 100755 --- a/t/t3030-merge-recursive.sh +++ b/t/t3030-merge-recursive.sh @@ -520,6 +520,7 @@ test_expect_success 'reset and bind merge' ' test_expect_success 'merge-recursive w/ empty work tree - ours has rename' ' ( + GIT_TEST_FSMONITOR="" && GIT_WORK_TREE="$PWD/ours-has-rename-work" && export GIT_WORK_TREE && GIT_INDEX_FILE="$PWD/ours-has-rename-index" && @@ -545,6 +546,7 @@ test_expect_success 'merge-recursive w/ empty work tree - ours has rename' ' test_expect_success 'merge-recursive w/ empty work tree - theirs has rename' ' ( + GIT_TEST_FSMONITOR="" && GIT_WORK_TREE="$PWD/theirs-has-rename-work" && export GIT_WORK_TREE && GIT_INDEX_FILE="$PWD/theirs-has-rename-index" && From a5b0bf6ac7cd28fa47e26ab481f781d74c656f6a Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 5 Nov 2019 14:15:01 -0500 Subject: [PATCH 5/8] tests: disable fsmonitor in submodule tests The fsmonitor feature allows an external tool such as watchman to monitor the working directory. The direct test t7619-status-fsmonitor.sh provides some coverage, but it would be better to run the entire test suite with watchman enabled. This would provide more confidence that the feature is working as intended. The fsmonitor feature struggles with submodules. Disable the GIT_TEST_FSMONITOR environment variable before running tests with a lot of submodule interactions. Signed-off-by: Derrick Stolee --- t/t3404-rebase-interactive.sh | 1 + t/t3600-rm.sh | 1 + t/t4060-diff-submodule-option-diff-format.sh | 3 +++ t/t5526-fetch-submodules.sh | 2 ++ t/t7402-submodule-rebase.sh | 3 +++ t/t7406-submodule-update.sh | 2 ++ t/t7506-status-submodule.sh | 3 +++ t/t7508-status.sh | 3 +++ 8 files changed, 18 insertions(+) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 461dd539ffd480..9dc7d1aefb6576 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -697,6 +697,7 @@ test_expect_success 'do "noop" when there is nothing to cherry-pick' ' ' test_expect_success 'submodule rebase setup' ' + GIT_TEST_FSMONITOR="" && git checkout A && mkdir sub && ( diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index 66282a720ee853..64269bd89d5077 100755 --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -355,6 +355,7 @@ test_expect_success 'rm succeeds when given a directory with a trailing /' ' ' test_expect_success 'rm of a populated submodule with different HEAD fails unless forced' ' + GIT_TEST_FSMONITOR="" && git reset --hard && git submodule update && git -C submod checkout HEAD^ && diff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh index 9dcb69df5c3684..017417790ecb51 100755 --- a/t/t4060-diff-submodule-option-diff-format.sh +++ b/t/t4060-diff-submodule-option-diff-format.sh @@ -15,6 +15,9 @@ This test tries to verify the sanity of --submodule=diff option of git diff. # Tested non-UTF-8 encoding test_encoding="ISO8859-1" +# fsmonitor does not work well with submodules +GIT_TEST_FSMONITOR="" + # String "added" in German (translated with Google Translate), encoded in UTF-8, # used in sample commit log messages in add_file() function below. added=$(printf "hinzugef\303\274gt") diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 63205dfdf962dc..fb346bff058fc9 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -1,6 +1,8 @@ #!/bin/sh # Copyright (c) 2010, Jens Lehmann +GIT_TEST_FSMONITOR="" + test_description='Recursive "git fetch" for submodules' . ./test-lib.sh diff --git a/t/t7402-submodule-rebase.sh b/t/t7402-submodule-rebase.sh index 8e32f190077474..c78e9009cfab89 100755 --- a/t/t7402-submodule-rebase.sh +++ b/t/t7402-submodule-rebase.sh @@ -7,6 +7,9 @@ test_description='Test rebasing, stashing, etc. with submodules' . ./test-lib.sh +# fsmonitor does not work well with submodules +GIT_TEST_FSMONITOR="" + test_expect_success setup ' echo file > file && diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index c973278300a5c9..8d93aaef5f8eca 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -11,6 +11,8 @@ submodule and "git submodule update --rebase/--merge" does not detach the HEAD. . ./test-lib.sh +# fsmonitor does not work well with submodules +GIT_TEST_FSMONITOR="" compare_head() { diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh index 08629a6e702999..1a716f2c2ae49e 100755 --- a/t/t7506-status-submodule.sh +++ b/t/t7506-status-submodule.sh @@ -4,6 +4,9 @@ test_description='git status for submodule' . ./test-lib.sh +# fsmonitor does not work well with submodules +GIT_TEST_FSMONITOR="" + test_create_repo_with_commit () { test_create_repo "$1" && ( diff --git a/t/t7508-status.sh b/t/t7508-status.sh index 4e676cdce8d621..bf0487632d1108 100755 --- a/t/t7508-status.sh +++ b/t/t7508-status.sh @@ -846,6 +846,9 @@ test_expect_success 'status refreshes the index' ' test_cmp expect output ' +# fsmonitor does not work well with submodules +GIT_TEST_FSMONITOR="" + test_expect_success 'setup status submodule summary' ' test_create_repo sm && ( cd sm && From 9cd4a08d82521082f0ee13ebea353708fada3403 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 18 Nov 2019 15:38:15 -0500 Subject: [PATCH 6/8] t7063: disable fsmonitor with status cache The fsmonitor feature allows an external tool such as watchman to monitor the working directory. The direct test t7619-status-fsmonitor.sh provides some coverage, but it would be better to run the entire test suite with watchman enabled. This would provide more confidence that the feature is working as intended. The status cache tests use GIT_TRACE_UNTRACKED_STATS to check very detailed statistics related to how much Git actually checked for untracked files. The fsmonitor feature changes the expected behavior here, so disable the GIT_TEST_FSMONITOR environment variable. Signed-off-by: Derrick Stolee --- t/t7063-status-untracked-cache.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh index 190ae149cf3cb6..c433738a3a216a 100755 --- a/t/t7063-status-untracked-cache.sh +++ b/t/t7063-status-untracked-cache.sh @@ -4,6 +4,9 @@ test_description='test untracked cache' . ./test-lib.sh +# fsmonitor changes the expected behvaior of GIT_TRACE_UNTRACKED_STATS +GIT_TEST_FSMONITOR="" + # On some filesystems (e.g. FreeBSD's ext2 and ufs) directory mtime # is updated lazily after contents in the directory changes, which # forces the untracked cache code to take the slow path. A test From 215ec8688e60c594d50628caa03258010e4d6606 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 18 Nov 2019 15:43:49 -0500 Subject: [PATCH 7/8] t7519: disable external GIT_TEST_FSMONITOR variable The GIT_TEST_FSMONITOR variable was created specifically so the t7519-status-fsmonitor.sh test script could tweak the expected behavior depending on its value. However, if we set it externally to use the Watchman integration, then it breaks the initial tests that demonstrate behavior _without_ the fsmonitor feature. Disable this variable at the start of the script. Signed-off-by: Derrick Stolee --- t/t7519-status-fsmonitor.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index 81a375fa0ff984..443d2e653b7e9f 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -8,6 +8,9 @@ test_description='git status with file system watcher' # "git update-index --fsmonitor" can be used to get the extension written # before testing the results. +# Disable an external value, as we will set it directly as needed. +GIT_TEST_FSMONITOR="" + clean_repo () { git reset --hard HEAD && git clean -fd From e51165f260d564ccb7a9b8e696691eccb184c01a Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 18 Nov 2019 16:16:20 -0500 Subject: [PATCH 8/8] test-lib: clear watchman watches at test completion The fsmonitor feature allows an external tool such as watchman to monitor the working directory. The direct test t7619-status-fsmonitor.sh provides some coverage, but it would be better to run the entire test suite with watchman enabled. This would provide more confidence that the feature is working as intended. When running the test suite in parallel with 'prove -j ', many repos are created and deleted in parallel. When GIT_TEST_FSMONITOR points to t/t7519/fsmonitor-watchman, this can lead to watchman tracking many different folders, overloading its watch queue. As a test script completes, we can tell watchman to stop watching the directories inside the TRASH_DIRECTORY. This is particularly important on Windows where watchman keeps an open handle on the directories it watches, preventing them from being deleted. There is currently a bug in watchman [1] where this handle still is not closed, but if that is updated then these tests can be run on Windows as well. [1] https://github.com/facebook/watchman/issues/764 Signed-off-by: Derrick Stolee --- t/test-lib-functions.sh | 15 +++++++++++++++ t/test-lib.sh | 4 ++++ 2 files changed, 19 insertions(+) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index e0b3f28d3a96e1..ef840ce0975b38 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -1475,3 +1475,18 @@ test_set_port () { port=$(($port + ${GIT_TEST_STRESS_JOB_NR:-0})) eval $var=$port } + +test_clear_watchman () { + if test -n "$GIT_TEST_FSMONITOR" + then + watchman watch-list | + grep "$TRASH_DIRECTORY" | + sed "s/\",//g" | + sed "s/\"//g" >repo-list + + while read repo + do + watchman watch-del "$repo" + done