From: Koji Nakamaru <koji.nakamaru@xxxxxxxx>
fsmonitor_classify_path_absolute() expects state->path_gitdir_watch.buf
has no trailing '/' or '.' For a submodule, fsmonitor_run_daemon() sets
the value with trailing "/." (as repo_get_git_dir(the_repository) on
Darwin returns ".") so that fsmonitor_classify_path_absolute() returns
IS_OUTSIDE_CONE.
In this case, fsevent_callback() doesn't update cookie_list so that
fsmonitor_publish() does nothing and with_lock__mark_cookies_seen() is
not invoked.
As with_lock__wait_for_cookie() infinitely waits for state->cookies_cond
that with_lock__mark_cookies_seen() should unlock, the whole daemon
hangs.
Remove trailing "/." from state->path_gitdir_watch.buf for submodules
and add a corresponding test in t7527-builtin-fsmonitor.sh.
Suggested-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
Suggested-by: Junio C Hamano <gitster@xxxxxxxxx>
Signed-off-by: Koji Nakamaru <koji.nakamaru@xxxxxxxx>
---
fsmonitor/darwin: fix hangs for submodules
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1802%2FKojiNakamaru%2Ffix%2Ffsmonitor-darwin-hangs-for-submodules-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1802/KojiNakamaru/fix/fsmonitor-darwin-hangs-for-submodules-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1802
Range-diff vs v1:
1: 1889cbb193d ! 1: decf68499f7 fsmonitor OSX: fix hangs for submodules
@@ Commit message
Remove trailing "/." from state->path_gitdir_watch.buf for submodules
and add a corresponding test in t7527-builtin-fsmonitor.sh.
- Helped-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
+ Suggested-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
+ Suggested-by: Junio C Hamano <gitster@xxxxxxxxx>
Signed-off-by: Koji Nakamaru <koji.nakamaru@xxxxxxxx>
## builtin/fsmonitor--daemon.c ##
@@ builtin/fsmonitor--daemon.c: static int fsmonitor_run_daemon(void)
## t/t7527-builtin-fsmonitor.sh ##
-@@ t/t7527-builtin-fsmonitor.sh: have_t2_data_event () {
- grep -e '"event":"data".*"category":"'"$c"'".*"key":"'"$k"'"'
- }
+@@ t/t7527-builtin-fsmonitor.sh: test_expect_success "submodule absorbgitdirs implicitly starts daemon" '
+ test_subcommand git fsmonitor--daemon start <super-sub.trace
+ '
+start_git_in_background () {
+ git "$@" &
+ git_pid=$!
++ git_pgid=$(ps -o pgid= -p $git_pid)
+ nr_tries_left=10
+ while true
+ do
+ if test $nr_tries_left -eq 0
+ then
-+ kill $git_pid
++ kill -- -$git_pgid
+ exit 1
+ fi
+ sleep 1
+ nr_tries_left=$(($nr_tries_left - 1))
-+ done > /dev/null 2>&1 &
++ done >/dev/null 2>&1 &
+ watchdog_pid=$!
+ wait $git_pid
+}
+
-+stop_git_and_watchdog () {
-+ kill $git_pid $watchdog_pid
++stop_git () {
++ while kill -0 -- -$git_pgid
++ do
++ kill -- -$git_pgid
++ sleep 1
++ done
++}
++
++stop_watchdog () {
++ while kill -0 $watchdog_pid
++ do
++ kill $watchdog_pid
++ sleep 1
++ done
+}
+
- test_expect_success 'explicit daemon start and stop' '
- test_when_finished "stop_daemon_delete_repo test_explicit" &&
-
-@@ t/t7527-builtin-fsmonitor.sh: test_expect_success "submodule absorbgitdirs implicitly starts daemon" '
- test_subcommand git fsmonitor--daemon start <super-sub.trace
- '
-
+test_expect_success "submodule implicitly starts daemon by pull" '
-+ test_atexit "stop_git_and_watchdog" &&
-+ test_when_finished "rm -rf cloned; \
-+ rm -rf super; \
-+ rm -rf sub" &&
++ test_atexit "stop_watchdog" &&
++ test_when_finished "stop_git && rm -rf cloned super sub" &&
+
+ create_super super &&
+ create_sub sub &&
@@ t/t7527-builtin-fsmonitor.sh: test_expect_success "submodule absorbgitdirs impli
+ git clone --recurse-submodules super cloned &&
+
+ git -C cloned/dir_1/dir_2/sub config core.fsmonitor true &&
++ set -m &&
+ start_git_in_background -C cloned pull --recurse-submodules
+'
+
builtin/fsmonitor--daemon.c | 1 +
t/t7527-builtin-fsmonitor.sh | 51 ++++++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+)
diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c
index dce8a3b2482..e1e6b96d09e 100644
--- a/builtin/fsmonitor--daemon.c
+++ b/builtin/fsmonitor--daemon.c
@@ -1314,6 +1314,7 @@ static int fsmonitor_run_daemon(void)
strbuf_reset(&state.path_gitdir_watch);
strbuf_addstr(&state.path_gitdir_watch,
absolute_path(repo_get_git_dir(the_repository)));
+ strbuf_strip_suffix(&state.path_gitdir_watch, "/.");
state.nr_paths_watching = 2;
}
diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
index 730f3c7f810..2dd1ca1a7b7 100755
--- a/t/t7527-builtin-fsmonitor.sh
+++ b/t/t7527-builtin-fsmonitor.sh
@@ -907,6 +907,57 @@ test_expect_success "submodule absorbgitdirs implicitly starts daemon" '
test_subcommand git fsmonitor--daemon start <super-sub.trace
'
+start_git_in_background () {
+ git "$@" &
+ git_pid=$!
+ git_pgid=$(ps -o pgid= -p $git_pid)
+ nr_tries_left=10
+ while true
+ do
+ if test $nr_tries_left -eq 0
+ then
+ kill -- -$git_pgid
+ exit 1
+ fi
+ sleep 1
+ nr_tries_left=$(($nr_tries_left - 1))
+ done >/dev/null 2>&1 &
+ watchdog_pid=$!
+ wait $git_pid
+}
+
+stop_git () {
+ while kill -0 -- -$git_pgid
+ do
+ kill -- -$git_pgid
+ sleep 1
+ done
+}
+
+stop_watchdog () {
+ while kill -0 $watchdog_pid
+ do
+ kill $watchdog_pid
+ sleep 1
+ done
+}
+
+test_expect_success "submodule implicitly starts daemon by pull" '
+ test_atexit "stop_watchdog" &&
+ test_when_finished "stop_git && rm -rf cloned super sub" &&
+
+ create_super super &&
+ create_sub sub &&
+
+ git -C super submodule add ../sub ./dir_1/dir_2/sub &&
+ git -C super commit -m "add sub" &&
+ git clone --recurse-submodules super cloned &&
+
+ git -C cloned/dir_1/dir_2/sub config core.fsmonitor true &&
+ set -m &&
+ start_git_in_background -C cloned pull --recurse-submodules
+'
+
# On a case-insensitive file system, confirm that the daemon
# notices when the .git directory is moved/renamed/deleted
# regardless of how it is spelled in the FS event.
base-commit: 3857aae53f3633b7de63ad640737c657387ae0c6
--
gitgitgadget
