pr-1041/jeffhostetler/builtin-fsmonitor-part2-v8
tagged this
24 Mar 16:49
Here is V8 of Part 2 of my builtin FSMonitor series. This contains last
minute comments on V7. I'll also send an updated version of Part 3 that
builds upon this version.
Here is a range-diff from V7 to V8 relative to 715d08a9e5 (The eighth batch,
2022-02-25). Changes since V7 can be summarized as:
* [] Improved arg checking in start_daemon() in t7527.
* [] Switch start_daemon() args to use double-dash keywords.
* [] Split out coding style changes in p7519 into separate commit from
speed up changes.
* [] Disabled some of the matrix tests in t7527 concerning the untracked
cache.
* [] Other minor style cleanups.
1: e98373f997 = 1: e98373f997 fsmonitor: enhance existing comments, clarify trivial response handling
2: ab68b94417 = 2: ab68b94417 fsmonitor-ipc: create client routines for git-fsmonitor--daemon
3: e04c7301f2 = 3: e04c7301f2 fsmonitor: config settings are repository-specific
4: ea02ba25d8 = 4: ea02ba25d8 fsmonitor: use IPC to query the builtin FSMonitor daemon
5: 6ab7db9cb7 = 5: 6ab7db9cb7 fsmonitor: document builtin fsmonitor
6: 0ce8ae3f2c = 6: 0ce8ae3f2c fsmonitor--daemon: add a built-in fsmonitor daemon
7: 4624ce2fa4 = 7: 4624ce2fa4 fsmonitor--daemon: implement 'stop' and 'status' commands
8: a29fe7266a = 8: a29fe7266a compat/fsmonitor/fsm-listen-win32: stub in backend for Windows
9: 2f8a42fdb9 = 9: 2f8a42fdb9 compat/fsmonitor/fsm-listen-darwin: stub in backend for Darwin
10: f07800690e = 10: f07800690e fsmonitor--daemon: implement 'run' command
11: a6a39a3306 = 11: a6a39a3306 fsmonitor--daemon: implement 'start' command
12: d62e338d00 = 12: d62e338d00 fsmonitor--daemon: add pathname classification
13: 53e06b4ae5 = 13: 53e06b4ae5 fsmonitor--daemon: define token-ids
14: 39f43fabe0 = 14: 39f43fabe0 fsmonitor--daemon: create token-based changed path cache
15: 239558e34f = 15: 239558e34f compat/fsmonitor/fsm-listen-win32: implement FSMonitor backend on Windows
16: 14b775e9d8 = 16: 14b775e9d8 compat/fsmonitor/fsm-listen-darwin: add MacOS header files for FSEvent
17: 55bd7aee06 = 17: 55bd7aee06 compat/fsmonitor/fsm-listen-darwin: implement FSEvent listener on MacOS
18: 1f4b5209bf ! 18: c43009124f fsmonitor--daemon: implement handle_client callback
@@ builtin/fsmonitor--daemon.c: void fsmonitor_force_resync(struct fsmonitor_daemon
+ trace2_data_intmax("fsmonitor", the_repository,
+ "response/trivial", 1);
+
-+ strbuf_release(&response_token);
-+ strbuf_release(&requested_token_id);
-+ return 0;
++ goto cleanup;
+ }
+
+ /*
@@ builtin/fsmonitor--daemon.c: void fsmonitor_force_resync(struct fsmonitor_daemon
+ trace2_data_intmax("fsmonitor", the_repository, "response/count/files", count);
+ trace2_data_intmax("fsmonitor", the_repository, "response/count/duplicates", duplicates);
+
++cleanup:
+ strbuf_release(&response_token);
+ strbuf_release(&requested_token_id);
+ strbuf_release(&payload);
19: 8cf62c9fc6 = 19: ed338777b5 help: include fsmonitor--daemon feature flag in version info
20: 1bd74a8159 = 20: c99bac29d4 t/helper/fsmonitor-client: create IPC client to talk to FSMonitor Daemon
21: 4a920d0b54 ! 21: c8709da945 t7527: create test for fsmonitor--daemon
@@ t/t7527-builtin-fsmonitor.sh (new)
+ rm -rf $1
+}
+
++is_value () {
++ test -n "$1" && test "${1::1}" != "-"
++}
++
+start_daemon () {
+ r= &&
+ tf= &&
@@ t/t7527-builtin-fsmonitor.sh (new)
+ case "$1" in
+ -C)
+ shift;
-+ test "$#" -ne 0 || BUG "error: -C requires arg"
++ is_value $1 || BUG "error: -C requires value"
+ r="-C $1"
+ shift
+ ;;
-+ -tf)
++ --tf)
+ shift;
-+ test "$#" -ne 0 || BUG "error: -tf requires arg"
++ is_value $1 || BUG "error: --tf requires value"
+ tf="$1"
+ shift
+ ;;
-+ -t2)
++ --t2)
+ shift;
-+ test "$#" -ne 0 || BUG "error: -t2 requires arg"
++ is_value $1 || BUG "error: --t2 requires value"
+ t2="$1"
+ shift
+ ;;
-+ -tk)
++ --tk)
+ shift;
-+ test "$#" -ne 0 || BUG "error: -tk requires arg"
++ is_value $1 || BUG "error: --tk requires value"
+ tk="$1"
+ shift
+ ;;
@@ t/t7527-builtin-fsmonitor.sh (new)
+test_expect_success 'edit some files' '
+ test_when_finished clean_up_repo_and_stop_daemon &&
+
-+ start_daemon -tf "$PWD/.git/trace" &&
++ start_daemon --tf "$PWD/.git/trace" &&
+
+ edit_files &&
+
@@ t/t7527-builtin-fsmonitor.sh (new)
+test_expect_success 'create some files' '
+ test_when_finished clean_up_repo_and_stop_daemon &&
+
-+ start_daemon -tf "$PWD/.git/trace" &&
++ start_daemon --tf "$PWD/.git/trace" &&
+
+ create_files &&
+
@@ t/t7527-builtin-fsmonitor.sh (new)
+test_expect_success 'delete some files' '
+ test_when_finished clean_up_repo_and_stop_daemon &&
+
-+ start_daemon -tf "$PWD/.git/trace" &&
++ start_daemon --tf "$PWD/.git/trace" &&
+
+ delete_files &&
+
@@ t/t7527-builtin-fsmonitor.sh (new)
+test_expect_success 'rename some files' '
+ test_when_finished clean_up_repo_and_stop_daemon &&
+
-+ start_daemon -tf "$PWD/.git/trace" &&
++ start_daemon --tf "$PWD/.git/trace" &&
+
+ rename_files &&
+
@@ t/t7527-builtin-fsmonitor.sh (new)
+test_expect_success 'rename directory' '
+ test_when_finished clean_up_repo_and_stop_daemon &&
+
-+ start_daemon -tf "$PWD/.git/trace" &&
++ start_daemon --tf "$PWD/.git/trace" &&
+
+ mv dirtorename dirrenamed &&
+
@@ t/t7527-builtin-fsmonitor.sh (new)
+test_expect_success 'file changes to directory' '
+ test_when_finished clean_up_repo_and_stop_daemon &&
+
-+ start_daemon -tf "$PWD/.git/trace" &&
++ start_daemon --tf "$PWD/.git/trace" &&
+
+ file_to_directory &&
+
@@ t/t7527-builtin-fsmonitor.sh (new)
+test_expect_success 'directory changes to a file' '
+ test_when_finished clean_up_repo_and_stop_daemon &&
+
-+ start_daemon -tf "$PWD/.git/trace" &&
++ start_daemon --tf "$PWD/.git/trace" &&
+
+ directory_to_file &&
+
@@ t/t7527-builtin-fsmonitor.sh (new)
+
+ git init test_flush &&
+
-+ start_daemon -C test_flush -tf "$PWD/.git/trace_daemon" -tk true &&
++ start_daemon -C test_flush --tf "$PWD/.git/trace_daemon" --tk true &&
+
+ # The daemon should have an initial token with no events in _0 and
+ # then a few (probably platform-specific number of) events in _1.
@@ t/t7527-builtin-fsmonitor.sh (new)
+ git -C wt-base worktree add ../wt-secondary &&
+
+ start_daemon -C wt-secondary \
-+ -tf "$PWD/trace_wt_secondary" \
-+ -t2 "$PWD/trace2_wt_secondary" &&
++ --tf "$PWD/trace_wt_secondary" \
++ --t2 "$PWD/trace2_wt_secondary" &&
+
+ git -C wt-secondary fsmonitor--daemon stop &&
+ test_must_fail git -C wt-secondary fsmonitor--daemon status
22: c925a9a745 = 22: cc39ecf10a t/perf: avoid copying builtin fsmonitor files into test repo
23: 5b3381c223 = 23: 2bb3eb8476 t/helper/test-chmtime: skip directories on Windows
-: ---------- > 24: bab9a9b080 t/perf/p7519: fix coding style
24: 803a540cc0 ! 25: 2dd06ad2f7 t/perf/p7519: speed up test on Windows
@@ Commit message
Signed-off-by: Junio C Hamano <gitster@pobox.com>
## t/perf/p7519-fsmonitor.sh ##
-@@ t/perf/p7519-fsmonitor.sh: then
- fi
- fi
-
--trace_start() {
-+trace_start () {
- if test -n "$GIT_PERF_7519_TRACE"
- then
- name="$1"
-@@ t/perf/p7519-fsmonitor.sh: trace_start() {
- fi
- }
-
--trace_stop() {
-+trace_stop () {
- if test -n "$GIT_PERF_7519_TRACE"
- then
- unset GIT_TRACE2_PERF
+@@ t/perf/p7519-fsmonitor.sh: trace_stop () {
fi
}
+touch_files () {
-+ n=$1
-+ d="$n"_files
++ n=$1 &&
++ d="$n"_files &&
+
-+ (cd $d ; test_seq 1 $n | xargs touch )
++ (cd $d && test_seq 1 $n | xargs touch )
+}
+
test_expect_success "one time repo setup" '
@@ t/perf/p7519-fsmonitor.sh: test_expect_success "one time repo setup" '
git add 1_file 10_files 100_files 1000_files 10000_files &&
git commit -qm "Add files" &&
-@@ t/perf/p7519-fsmonitor.sh: test_expect_success "one time repo setup" '
- fi
- '
-
--setup_for_fsmonitor() {
-+setup_for_fsmonitor () {
- # set INTEGRATION_SCRIPT depending on the environment
- if test -n "$INTEGRATION_PATH"
- then
-@@ t/perf/p7519-fsmonitor.sh: test_perf_w_drop_caches () {
- test_perf "$@"
- }
-
--test_fsmonitor_suite() {
-+test_fsmonitor_suite () {
- if test -n "$INTEGRATION_SCRIPT"; then
- DESC="fsmonitor=$(basename $INTEGRATION_SCRIPT)"
- else
-@@ t/perf/p7519-fsmonitor.sh: test_fsmonitor_suite() {
+@@ t/perf/p7519-fsmonitor.sh: test_fsmonitor_suite () {
# Update the mtimes on upto 100k files to make status think
# that they are dirty. For simplicity, omit any files with
25: d5ca2df31c = 26: 6eaa5765ae t/perf/p7519: add fsmonitor--daemon test cases
26: 42631259e8 = 27: 30957f3930 fsmonitor--daemon: periodically truncate list of modified files
27: f256c3cbe8 = 28: c8ca2a1727 fsmonitor--daemon: use a cookie file to sync with file system
28: 08af8296f9 = 29: 4caf1d89b8 fsmonitor: force update index after large responses
29: e6cf84dc8e ! 30: f87a1eba69 t7527: test status with untracked-cache and fsmonitor--daemon
@@ t/t7527-builtin-fsmonitor.sh: test_expect_success 'cleanup worktrees' '
+ fsm=$2 &&
+ fn=$3 &&
+
++ if test $uc = true && test $fsm = false
++ then
++ # The untracked-cache is buggy when FSMonitor is
++ # DISABLED, so skip the tests for this matrix
++ # combination.
++ #
++ # We've observed random, occasional test failures on
++ # Windows and MacOS when the UC is turned on and FSM
++ # is turned off. These are rare, but they do happen
++ # indicating that it is probably a race condition within
++ # the untracked cache itself.
++ #
++ # It usually happens when a test does F/D trickery and
++ # then the NEXT test fails because of extra status
++ # output from stale UC data from the previous test.
++ #
++ # Since FSMonitor is not involved in the error, skip
++ # the tests for this matrix combination.
++ #
++ return 0
++ fi &&
++
+ test_expect_success "Matrix[uc:$uc][fsm:$fsm] $fn" '
+ matrix_clean_up_repo &&
+ $fn &&
Jeff Hostetler (30):
fsmonitor: enhance existing comments, clarify trivial response
handling
fsmonitor-ipc: create client routines for git-fsmonitor--daemon
fsmonitor: config settings are repository-specific
fsmonitor: use IPC to query the builtin FSMonitor daemon
fsmonitor: document builtin fsmonitor
fsmonitor--daemon: add a built-in fsmonitor daemon
fsmonitor--daemon: implement 'stop' and 'status' commands
compat/fsmonitor/fsm-listen-win32: stub in backend for Windows
compat/fsmonitor/fsm-listen-darwin: stub in backend for Darwin
fsmonitor--daemon: implement 'run' command
fsmonitor--daemon: implement 'start' command
fsmonitor--daemon: add pathname classification
fsmonitor--daemon: define token-ids
fsmonitor--daemon: create token-based changed path cache
compat/fsmonitor/fsm-listen-win32: implement FSMonitor backend on
Windows
compat/fsmonitor/fsm-listen-darwin: add MacOS header files for FSEvent
compat/fsmonitor/fsm-listen-darwin: implement FSEvent listener on
MacOS
fsmonitor--daemon: implement handle_client callback
help: include fsmonitor--daemon feature flag in version info
t/helper/fsmonitor-client: create IPC client to talk to FSMonitor
Daemon
t7527: create test for fsmonitor--daemon
t/perf: avoid copying builtin fsmonitor files into test repo
t/helper/test-chmtime: skip directories on Windows
t/perf/p7519: fix coding style
t/perf/p7519: speed up test on Windows
t/perf/p7519: add fsmonitor--daemon test cases
fsmonitor--daemon: periodically truncate list of modified files
fsmonitor--daemon: use a cookie file to sync with file system
fsmonitor: force update index after large responses
t7527: test status with untracked-cache and fsmonitor--daemon
.gitignore | 1 +
Documentation/config/core.txt | 60 +-
Documentation/git-fsmonitor--daemon.txt | 75 ++
Documentation/git-update-index.txt | 8 +-
Makefile | 17 +
builtin.h | 1 +
builtin/fsmonitor--daemon.c | 1479 +++++++++++++++++++++++
builtin/update-index.c | 7 +-
cache.h | 1 -
compat/fsmonitor/fsm-darwin-gcc.h | 92 ++
compat/fsmonitor/fsm-listen-darwin.c | 427 +++++++
compat/fsmonitor/fsm-listen-win32.c | 586 +++++++++
compat/fsmonitor/fsm-listen.h | 49 +
config.c | 14 -
config.h | 1 -
config.mak.uname | 20 +
contrib/buildsystems/CMakeLists.txt | 10 +
environment.c | 1 -
fsmonitor--daemon.h | 166 +++
fsmonitor-ipc.c | 171 +++
fsmonitor-ipc.h | 48 +
fsmonitor-settings.c | 114 ++
fsmonitor-settings.h | 21 +
fsmonitor.c | 216 +++-
fsmonitor.h | 15 +-
git.c | 1 +
help.c | 4 +
repo-settings.c | 1 +
repository.h | 3 +
t/README | 4 +-
t/helper/test-chmtime.c | 15 +
t/helper/test-fsmonitor-client.c | 116 ++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/perf/p7519-fsmonitor.sh | 68 +-
t/perf/perf-lib.sh | 2 +-
t/t7527-builtin-fsmonitor.sh | 620 ++++++++++
t/test-lib.sh | 7 +
38 files changed, 4337 insertions(+), 106 deletions(-)
create mode 100644 Documentation/git-fsmonitor--daemon.txt
create mode 100644 builtin/fsmonitor--daemon.c
create mode 100644 compat/fsmonitor/fsm-darwin-gcc.h
create mode 100644 compat/fsmonitor/fsm-listen-darwin.c
create mode 100644 compat/fsmonitor/fsm-listen-win32.c
create mode 100644 compat/fsmonitor/fsm-listen.h
create mode 100644 fsmonitor--daemon.h
create mode 100644 fsmonitor-ipc.c
create mode 100644 fsmonitor-ipc.h
create mode 100644 fsmonitor-settings.c
create mode 100644 fsmonitor-settings.h
create mode 100644 t/helper/test-fsmonitor-client.c
create mode 100755 t/t7527-builtin-fsmonitor.sh
base-commit: 715d08a9e51251ad8290b181b6ac3b9e1f9719d7
Submitted-As: https://lore.kernel.org/git/pull.1041.v8.git.1648140586.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1041.git.1631822063.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1041.v2.git.1633614772.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1041.v3.git.1634157107.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1041.v4.git.1634826309.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1041.v5.git.1644612979.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1041.v6.git.1646160212.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.1041.v7.git.1647972010.gitgitgadget@gmail.com
Assets 2
-
2022-03-24T16:49:46Z -
2022-03-24T16:49:46Z -