| Message ID | pull.1041.v9.git.1648231393.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Builtin FSMonitor Part 2 | expand |
On March 25, 2022 2:03 PM, Jeff Hostetler wrote: >Here is V9 of Part 2 of my builtin FSMonitor series. This version addresses bash >style issues in t7527 raised on V8. These changes do not require a new version of >Part 3. > >Here is a range-diff from V8 to V9 relative to 715d08a9e5 (The eighth batch, 2022- >02-25). > > 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: c43009124f = 18: c43009124f fsmonitor--daemon: implement handle_client >callback >19: ed338777b5 = 19: ed338777b5 help: include fsmonitor--daemon feature flag in >version info >20: c99bac29d4 = 20: c99bac29d4 t/helper/fsmonitor-client: create IPC client to >talk to FSMonitor Daemon >21: c8709da945 ! 21: bc94e379b0 t7527: create test for fsmonitor--daemon > @@ Commit message > t7527: create test for fsmonitor--daemon > > Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> > - Signed-off-by: Junio C Hamano <gitster@pobox.com> > > ## t/t7527-builtin-fsmonitor.sh (new) ## > @@ > @@ t/t7527-builtin-fsmonitor.sh (new) > + rm -rf $1 > +} > + > -+is_value () { > -+ test -n "$1" && test "${1::1}" != "-" > -+} > -+ > +start_daemon () { > -+ r= && > -+ tf= && > -+ t2= && > -+ tk= && > ++ r= tf= t2= tk= && > + > + while test "$#" -ne 0 > + do > + case "$1" in > + -C) > -+ shift; > -+ is_value $1 || BUG "error: -C requires value" > -+ r="-C $1" > ++ r="-C ${2?}" > + shift > + ;; > + --tf) > -+ shift; > -+ is_value $1 || BUG "error: --tf requires value" > -+ tf="$1" > ++ tf="${2?}" > + shift > + ;; > + --t2) > -+ shift; > -+ is_value $1 || BUG "error: --t2 requires value" > -+ t2="$1" > ++ t2="${2?}" > + shift > + ;; > + --tk) > -+ shift; > -+ is_value $1 || BUG "error: --tk requires value" > -+ tk="$1" > ++ tk="${2?}" > + shift > + ;; > -+ *) > ++ -*) > + BUG "error: unknown option: '$1'" > + ;; > ++ *) > ++ BUG "error: unbound argument: '$1'" > ++ ;; > + esac > ++ shift > + done && > + > + ( >22: cc39ecf10a = 22: 06d56d3a73 t/perf: avoid copying builtin fsmonitor files into >test repo >23: 2bb3eb8476 = 23: 2dd0215127 t/helper/test-chmtime: skip directories on >Windows >24: bab9a9b080 = 24: bb88cddc13 t/perf/p7519: fix coding style >25: 2dd06ad2f7 = 25: 50c2afaa49 t/perf/p7519: speed up test on Windows >26: 6eaa5765ae = 26: 5b18e3b692 t/perf/p7519: add fsmonitor--daemon test >cases >27: 30957f3930 = 27: 899c23f63c fsmonitor--daemon: periodically truncate list of >modified files >28: c8ca2a1727 = 28: 597a7192f9 fsmonitor--daemon: use a cookie file to sync with >file system >29: 4caf1d89b8 = 29: 68a05fd289 fsmonitor: force update index after large >responses >30: f87a1eba69 = 30: 5eb696daba t7527: test status with untracked-cache and >fsmonitor--daemon > > >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 | 609 ++++++++++ > t/test-lib.sh | 7 + > 38 files changed, 4326 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 >Published-As: https://github.com/gitgitgadget/git/releases/tag/pr- >1041%2Fjeffhostetler%2Fbuiltin-fsmonitor-part2-v9 >Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr- >1041/jeffhostetler/builtin-fsmonitor-part2-v9 >Pull-Request: https://github.com/gitgitgadget/git/pull/1041 > >Range-diff vs v8: > > 1: e98373f997f = 1: e98373f997f fsmonitor: enhance existing comments, clarify >trivial response handling > 2: ab68b944173 = 2: ab68b944173 fsmonitor-ipc: create client routines for git- >fsmonitor--daemon > 3: e04c7301f24 = 3: e04c7301f24 fsmonitor: config settings are repository- >specific > 4: ea02ba25d8f = 4: ea02ba25d8f fsmonitor: use IPC to query the builtin >FSMonitor daemon > 5: 6ab7db9cb76 = 5: 6ab7db9cb76 fsmonitor: document builtin fsmonitor > 6: 0ce8ae3f2cf = 6: 0ce8ae3f2cf fsmonitor--daemon: add a built-in fsmonitor >daemon > 7: 4624ce2fa47 = 7: 4624ce2fa47 fsmonitor--daemon: implement 'stop' and >'status' commands > 8: a29fe7266a4 = 8: a29fe7266a4 compat/fsmonitor/fsm-listen-win32: stub in >backend for Windows > 9: 2f8a42fdb93 = 9: 2f8a42fdb93 compat/fsmonitor/fsm-listen-darwin: stub in >backend for Darwin > 10: f07800690ee = 10: f07800690ee fsmonitor--daemon: implement 'run' >command > 11: a6a39a3306d = 11: a6a39a3306d fsmonitor--daemon: implement 'start' >command > 12: d62e338d008 = 12: d62e338d008 fsmonitor--daemon: add pathname >classification > 13: 53e06b4ae5d = 13: 53e06b4ae5d fsmonitor--daemon: define token-ids > 14: 39f43fabe02 = 14: 39f43fabe02 fsmonitor--daemon: create token-based >changed path cache > 15: 239558e34ff = 15: 239558e34ff compat/fsmonitor/fsm-listen-win32: >implement FSMonitor backend on Windows > 16: 14b775e9d8b = 16: 14b775e9d8b compat/fsmonitor/fsm-listen-darwin: add >MacOS header files for FSEvent > 17: 55bd7aee06c = 17: 55bd7aee06c compat/fsmonitor/fsm-listen-darwin: >implement FSEvent listener on MacOS > 18: c43009124fb = 18: c43009124fb fsmonitor--daemon: implement handle_client >callback > 19: ed338777b56 = 19: ed338777b56 help: include fsmonitor--daemon feature >flag in version info > 20: c99bac29d42 = 20: c99bac29d42 t/helper/fsmonitor-client: create IPC client to >talk to FSMonitor Daemon > 21: c8709da9457 ! 21: bc94e379b03 t7527: create test for fsmonitor--daemon > @@ Commit message > t7527: create test for fsmonitor--daemon > > Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> > - Signed-off-by: Junio C Hamano <gitster@pobox.com> > > ## t/t7527-builtin-fsmonitor.sh (new) ## > @@ > @@ t/t7527-builtin-fsmonitor.sh (new) > + rm -rf $1 > +} > + > -+is_value () { > -+ test -n "$1" && test "${1::1}" != "-" > -+} > -+ > +start_daemon () { > -+ r= && > -+ tf= && > -+ t2= && > -+ tk= && > ++ r= tf= t2= tk= && > + > + while test "$#" -ne 0 > + do > + case "$1" in > + -C) > -+ shift; > -+ is_value $1 || BUG "error: -C requires value" > -+ r="-C $1" > ++ r="-C ${2?}" > + shift > + ;; > + --tf) > -+ shift; > -+ is_value $1 || BUG "error: --tf requires value" > -+ tf="$1" > ++ tf="${2?}" > + shift > + ;; > + --t2) > -+ shift; > -+ is_value $1 || BUG "error: --t2 requires value" > -+ t2="$1" > ++ t2="${2?}" > + shift > + ;; > + --tk) > -+ shift; > -+ is_value $1 || BUG "error: --tk requires value" > -+ tk="$1" > ++ tk="${2?}" > + shift > + ;; > -+ *) > ++ -*) > + BUG "error: unknown option: '$1'" > + ;; > ++ *) > ++ BUG "error: unbound argument: '$1'" > ++ ;; > + esac > ++ shift > + done && > + > + ( > 22: cc39ecf10ae = 22: 06d56d3a733 t/perf: avoid copying builtin fsmonitor files >into test repo > 23: 2bb3eb84767 = 23: 2dd02151278 t/helper/test-chmtime: skip directories on >Windows > 24: bab9a9b0802 = 24: bb88cddc137 t/perf/p7519: fix coding style > 25: 2dd06ad2f71 = 25: 50c2afaa49e t/perf/p7519: speed up test on Windows > 26: 6eaa5765ae1 = 26: 5b18e3b6926 t/perf/p7519: add fsmonitor--daemon test >cases > 27: 30957f3930e = 27: 899c23f63c3 fsmonitor--daemon: periodically truncate list >of modified files > 28: c8ca2a17277 = 28: 597a7192f94 fsmonitor--daemon: use a cookie file to sync >with file system > 29: 4caf1d89b84 = 29: 68a05fd2892 fsmonitor: force update index after large >responses > 30: f87a1eba693 = 30: 5eb696daba2 t7527: test status with untracked-cache and >fsmonitor--daemon So that we can plan for next time, would you mind giving some thought to what would be required for Linux and whether it makes sense to extend this. No urgency, but I would not mind participating - assuming my $DAYJOB lets me. I have a backlog for git that I need to get done first anyway. Kind Regards, Randall
On 3/25/22 3:02 PM, rsbecker@nexbridge.com wrote: > On March 25, 2022 2:03 PM, Jeff Hostetler wrote: [...] > > So that we can plan for next time, would you mind giving some thought to what would be required for Linux and whether it makes sense to extend this. No urgency, but I would not mind participating - assuming my $DAYJOB lets me. I have a backlog for git that I need to get done first anyway. > > Kind Regards, > Randall > A Linux backend would need to: (1) stub in compat/fsmonitor/fsm-listen-linux.c (see commits 8/30 and 9/30 in this series). That gives you enough for the builtin/fsmonitor--daemon.c to link with your new backend. (2) populate those 4 routines. (2a) __ctor() and __dtor() will be called from the main thread before and after the listener thread is created. You can do anything you need there to register/deregister a watch on the FS. See inotify() and/or fanotify(). I haven't looked at those routines for a while, so all I can say is google it. (2b) __stop_async() will be called by "some" thread to request that the listener thread stop listening. This is an async request, so just notify the listener thread and return. (There is an example of this for Unix in compat/simple-ipc/ipc-unix-sockets.c) (2c) __loop() will be called (once) by the "listener" thread-proc to process/service events from the FS until a shutdown event is received. This runs in the body of the "listener" thread. It should probably use poll()/select()/whatever on fd's from inotify()/fsnotify() *and* whatever you set up in (2b) to wait for a shutdown event. (There are examples of this wait-loop in the Windows and Mac backends, but they have *very* different FS event and wait models, so they might not be very helpful here.) When you get a "batch" of one or more paths from the FS, use the fsmonitor_classify_*() routines to classify or discard them and then use fsmonitor_publish() to publish newly changed paths to the other threads. The core code will handle path de-dup and all locking so you don't have to. It is important that you figure out how to get recursive data from the FS. We want to watch the complete worktree. Windows and Mac let you register the root directory of the watch and automatically give me events for anything under it. IIRC, inotify() only gave you a single directory and you had to readdir() and recurse to get fd's to the subdirs. I haven't looked to see if fanotify() solves that or not. So there may be some fd juggling and tree walking required. That could/should all be hidden inside the __ctor() and/or __loop() routines. (3) stub in compat/fsmonitor/fsm-settings-linux.c (see the peers. This is needed to link. (4) fill in any platform-specific reasons why you might want to reject a worktree. for example, when they are remote (NFS/SMB might support it, but do you trust it....) (5) stub in compat/fsmonitor/fsm-health-linux.c (see fsm-health-*.c in part 3). That will give you enough to link the health thread. (6) this part is probably optional (at least for now). we can use this if we want to add platform-specific things like auto-shutdown after idle. The mac version is currently empty, but the Windows version needs to watch the worktree root separately (because the FS watch is limited to what is *within* watched directory root). That should help get you get started. Let me know if you have questions. Jeff
"Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes: > Here is V9 of Part 2 of my builtin FSMonitor series. This version addresses > bash style issues in t7527 raised on V8. These changes do not require a new > version of Part 3. This addresses all the "oops" fix-ups from part 2.5, and good base to build part 3 on top, right? Queued. Thanks.
On Fri, Mar 25 2022, Jeff Hostetler wrote: > On 3/25/22 3:02 PM, rsbecker@nexbridge.com wrote: >> On March 25, 2022 2:03 PM, Jeff Hostetler wrote: > [...] >> So that we can plan for next time, would you mind giving some >> thought to what would be required for Linux and whether it makes >> sense to extend this. No urgency, but I would not mind participating >> - assuming my $DAYJOB lets me. I have a backlog for git that I need >> to get done first anyway. >> Kind Regards, >> Randall >> > > A Linux backend would need to: > > (1) stub in compat/fsmonitor/fsm-listen-linux.c (see commits > 8/30 and 9/30 in this series). That gives you enough for > the builtin/fsmonitor--daemon.c to link with your new > backend. > > (2) populate those 4 routines. > > (2a) __ctor() and __dtor() will be called from the main thread > before and after the listener thread is created. You can > do anything you need there to register/deregister a watch > on the FS. > > See inotify() and/or fanotify(). I haven't looked at those > routines for a while, so all I can say is google it. > > (2b) __stop_async() will be called by "some" thread to request > that the listener thread stop listening. This is an async > request, so just notify the listener thread and return. > (There is an example of this for Unix in > compat/simple-ipc/ipc-unix-sockets.c) > > (2c) __loop() will be called (once) by the "listener" thread-proc > to process/service events from the FS until a shutdown event > is received. This runs in the body of the "listener" thread. > It should probably use poll()/select()/whatever on fd's from > inotify()/fsnotify() *and* whatever you set up in (2b) to > wait for a shutdown event. > > (There are examples of this wait-loop in the Windows and Mac > backends, but they have *very* different FS event and wait > models, so they might not be very helpful here.) > > When you get a "batch" of one or more paths from the FS, > use the fsmonitor_classify_*() routines to classify or > discard them and then use fsmonitor_publish() to publish > newly changed paths to the other threads. > > The core code will handle path de-dup and all locking so > you don't have to. > > It is important that you figure out how to get recursive data > from the FS. We want to watch the complete worktree. Windows > and Mac let you register the root directory of the watch and > automatically give me events for anything under it. IIRC, > inotify() only gave you a single directory and you had to > readdir() and recurse to get fd's to the subdirs. I haven't > looked to see if fanotify() solves that or not. So there may > be some fd juggling and tree walking required. That could/should > all be hidden inside the __ctor() and/or __loop() routines. > > (3) stub in compat/fsmonitor/fsm-settings-linux.c (see the > peers. This is needed to link. > > (4) fill in any platform-specific reasons why you might want > to reject a worktree. for example, when they are remote > (NFS/SMB might support it, but do you trust it....) > > (5) stub in compat/fsmonitor/fsm-health-linux.c (see > fsm-health-*.c in part 3). That will give you enough > to link the health thread. > > (6) this part is probably optional (at least for now). we > can use this if we want to add platform-specific things > like auto-shutdown after idle. The mac version is currently > empty, but the Windows version needs to watch the worktree > root separately (because the FS watch is limited to what > is *within* watched directory root). > > That should help get you get started. > Let me know if you have questions. Wouldn't it be much simpler POC in this case to write "watchman backend"? Then we'd both get a Linux backend, and an alternate backend for the other platforms to validate their implementation. Some past references to that: https://lore.kernel.org/git/871r8c73ej.fsf@evledraar.gmail.com/ & https://lore.kernel.org/git/87h7lgfchm.fsf@evledraar.gmail.com/
On 3/25/22 7:09 PM, Junio C Hamano wrote: > "Jeff Hostetler via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> Here is V9 of Part 2 of my builtin FSMonitor series. This version addresses >> bash style issues in t7527 raised on V8. These changes do not require a new >> version of Part 3. > > This addresses all the "oops" fix-ups from part 2.5, and good base > to build part 3 on top, right? > > Queued. > > Thanks. > Yes, this version of part 2 includes all of the fixups from 2.5. And 2.5 should be dropped. And yes, this part 2 should be the basis for part 3. Thanks again! Jeff
On 3/25/22 8:48 PM, Ævar Arnfjörð Bjarmason wrote: > > On Fri, Mar 25 2022, Jeff Hostetler wrote: > >> On 3/25/22 3:02 PM, rsbecker@nexbridge.com wrote: >>> On March 25, 2022 2:03 PM, Jeff Hostetler wrote: >> [...] [...] > > Wouldn't it be much simpler POC in this case to write "watchman > backend"? Then we'd both get a Linux backend, and an alternate backend > for the other platforms to validate their implementation. > > Some past references to that: > https://lore.kernel.org/git/871r8c73ej.fsf@evledraar.gmail.com/ & > https://lore.kernel.org/git/87h7lgfchm.fsf@evledraar.gmail.com/ > Yes, there are several ways for a client command, such as anyone who calls read_index/refresh_index, to get FS change data from a monitoring service. Let's go thru the options here for the sake of conversation: (option 1): Use the hook-like mechanism that Ben built in 2017 to talk to an interlude program, shell script, perl script, etc. That "script" itself then talks to a long-running service/daemon, such as Watchman, to get the list of changes and relays them back to the client. * This "proxy" has to handle protocol format conversions. * It may also have to start the service on new repos. * And depends upon a third-party service being installed. * We are limited to supporting platforms where the third-party tool is supported. (option 2): Replace the hook with builtin client code to talk directly to the service and bypass the need for the proxy script/executable. * Git client code would need client-side IPC to talk to an established and running service. (Similar to the client side of Simple-IPC but probably not pkt-line based.) * Git client code would now need to handle any protocol format conversions. * Git client code might also have to start the service. * And we'd still be dependent on a third-party service being installed. * And we are still limited to supporting platforms where the third-party tool is supported. * So far we've been assuming that that third-party tool is "Watchman", but technically, you could have other such services available. * So you may need multiple implementations of option 2, one for each third-party tool. * I'm not saying that this is hard, but just yet another detail that would have to be encoded in the Git source to get this "free" feature. (option 3): Git implements a daemon to monitor the file system directly. * Git owns the protocol between client and service. * Git owns the backend, so no third-party tools required. * Git owns service startup. * Unfortunately, we are also responsible for building the backends on each platform we want to support. * In the future, we could augment the service to be more "Git-aware", such as discarding data for ignored files, but that is just speculation at this point. Now, with that context in place: [1] Nothing prohibits us from having all three options be available on a platform. They should all be able to coexist. [2] One of my stated goals was to reduce the dependency on third-party tools -- especially on platforms that don't have a simple package management system. The point here was to make it easier for enterprises to deploy Git to 1k's or 10k's of users (and possibly unattended build machines) and make use of the feature without *also* having to deploy and track updates to yet-another third-party tool or otherwise complicate their ES deployment setups. Only option 3 gets rid of the third-party tool requirement. [3] Option 2 is a valuable suggestion, don't get me wrong. It can/ will/should improve performance over option 1 by eliminating an extra process creation and the overhead of pumping all of that data thru another socket-pair/process and all of the context switches that that requires. [4] Option 2 and option 3 could/should perform relatively equally. And if we wanted to deprecate the hook-like interface, doing an option 2 implementation would allow us to transition the platforms for which I don't currently have a backend. [5] However, option 2 does not eliminate the need for a third-party tool, so it is of limited interest to me at this time. Yes, it would be nice to have it for testing and perf testing purposes and comparisons with option 3, but if I have to budget my time, I would rather spend my efforts on additional backends. I consider the question of doing option 2 and a Linux backend as two completely independent topics -- topics that we can discuss and/or pursue in parallel if there is interest. [6] Randall's question was about doing option 3 and I hope that I provided helpful information should he or anyone else want to pick up that effort before I can. [7] If you want to start a parallel conversation on option 2, let's do that in a new top-level email thread. Cheers, Jeff
Here is V9 of Part 2 of my builtin FSMonitor series. This version addresses bash style issues in t7527 raised on V8. These changes do not require a new version of Part 3. Here is a range-diff from V8 to V9 relative to 715d08a9e5 (The eighth batch, 2022-02-25). 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: c43009124f = 18: c43009124f fsmonitor--daemon: implement handle_client callback 19: ed338777b5 = 19: ed338777b5 help: include fsmonitor--daemon feature flag in version info 20: c99bac29d4 = 20: c99bac29d4 t/helper/fsmonitor-client: create IPC client to talk to FSMonitor Daemon 21: c8709da945 ! 21: bc94e379b0 t7527: create test for fsmonitor--daemon @@ Commit message t7527: create test for fsmonitor--daemon Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> ## t/t7527-builtin-fsmonitor.sh (new) ## @@ @@ t/t7527-builtin-fsmonitor.sh (new) + rm -rf $1 +} + -+is_value () { -+ test -n "$1" && test "${1::1}" != "-" -+} -+ +start_daemon () { -+ r= && -+ tf= && -+ t2= && -+ tk= && ++ r= tf= t2= tk= && + + while test "$#" -ne 0 + do + case "$1" in + -C) -+ shift; -+ is_value $1 || BUG "error: -C requires value" -+ r="-C $1" ++ r="-C ${2?}" + shift + ;; + --tf) -+ shift; -+ is_value $1 || BUG "error: --tf requires value" -+ tf="$1" ++ tf="${2?}" + shift + ;; + --t2) -+ shift; -+ is_value $1 || BUG "error: --t2 requires value" -+ t2="$1" ++ t2="${2?}" + shift + ;; + --tk) -+ shift; -+ is_value $1 || BUG "error: --tk requires value" -+ tk="$1" ++ tk="${2?}" + shift + ;; -+ *) ++ -*) + BUG "error: unknown option: '$1'" + ;; ++ *) ++ BUG "error: unbound argument: '$1'" ++ ;; + esac ++ shift + done && + + ( 22: cc39ecf10a = 22: 06d56d3a73 t/perf: avoid copying builtin fsmonitor files into test repo 23: 2bb3eb8476 = 23: 2dd0215127 t/helper/test-chmtime: skip directories on Windows 24: bab9a9b080 = 24: bb88cddc13 t/perf/p7519: fix coding style 25: 2dd06ad2f7 = 25: 50c2afaa49 t/perf/p7519: speed up test on Windows 26: 6eaa5765ae = 26: 5b18e3b692 t/perf/p7519: add fsmonitor--daemon test cases 27: 30957f3930 = 27: 899c23f63c fsmonitor--daemon: periodically truncate list of modified files 28: c8ca2a1727 = 28: 597a7192f9 fsmonitor--daemon: use a cookie file to sync with file system 29: 4caf1d89b8 = 29: 68a05fd289 fsmonitor: force update index after large responses 30: f87a1eba69 = 30: 5eb696daba t7527: test status with untracked-cache and fsmonitor--daemon 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 | 609 ++++++++++ t/test-lib.sh | 7 + 38 files changed, 4326 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 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1041%2Fjeffhostetler%2Fbuiltin-fsmonitor-part2-v9 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1041/jeffhostetler/builtin-fsmonitor-part2-v9 Pull-Request: https://github.com/gitgitgadget/git/pull/1041 Range-diff vs v8: 1: e98373f997f = 1: e98373f997f fsmonitor: enhance existing comments, clarify trivial response handling 2: ab68b944173 = 2: ab68b944173 fsmonitor-ipc: create client routines for git-fsmonitor--daemon 3: e04c7301f24 = 3: e04c7301f24 fsmonitor: config settings are repository-specific 4: ea02ba25d8f = 4: ea02ba25d8f fsmonitor: use IPC to query the builtin FSMonitor daemon 5: 6ab7db9cb76 = 5: 6ab7db9cb76 fsmonitor: document builtin fsmonitor 6: 0ce8ae3f2cf = 6: 0ce8ae3f2cf fsmonitor--daemon: add a built-in fsmonitor daemon 7: 4624ce2fa47 = 7: 4624ce2fa47 fsmonitor--daemon: implement 'stop' and 'status' commands 8: a29fe7266a4 = 8: a29fe7266a4 compat/fsmonitor/fsm-listen-win32: stub in backend for Windows 9: 2f8a42fdb93 = 9: 2f8a42fdb93 compat/fsmonitor/fsm-listen-darwin: stub in backend for Darwin 10: f07800690ee = 10: f07800690ee fsmonitor--daemon: implement 'run' command 11: a6a39a3306d = 11: a6a39a3306d fsmonitor--daemon: implement 'start' command 12: d62e338d008 = 12: d62e338d008 fsmonitor--daemon: add pathname classification 13: 53e06b4ae5d = 13: 53e06b4ae5d fsmonitor--daemon: define token-ids 14: 39f43fabe02 = 14: 39f43fabe02 fsmonitor--daemon: create token-based changed path cache 15: 239558e34ff = 15: 239558e34ff compat/fsmonitor/fsm-listen-win32: implement FSMonitor backend on Windows 16: 14b775e9d8b = 16: 14b775e9d8b compat/fsmonitor/fsm-listen-darwin: add MacOS header files for FSEvent 17: 55bd7aee06c = 17: 55bd7aee06c compat/fsmonitor/fsm-listen-darwin: implement FSEvent listener on MacOS 18: c43009124fb = 18: c43009124fb fsmonitor--daemon: implement handle_client callback 19: ed338777b56 = 19: ed338777b56 help: include fsmonitor--daemon feature flag in version info 20: c99bac29d42 = 20: c99bac29d42 t/helper/fsmonitor-client: create IPC client to talk to FSMonitor Daemon 21: c8709da9457 ! 21: bc94e379b03 t7527: create test for fsmonitor--daemon @@ Commit message t7527: create test for fsmonitor--daemon Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> ## t/t7527-builtin-fsmonitor.sh (new) ## @@ @@ t/t7527-builtin-fsmonitor.sh (new) + rm -rf $1 +} + -+is_value () { -+ test -n "$1" && test "${1::1}" != "-" -+} -+ +start_daemon () { -+ r= && -+ tf= && -+ t2= && -+ tk= && ++ r= tf= t2= tk= && + + while test "$#" -ne 0 + do + case "$1" in + -C) -+ shift; -+ is_value $1 || BUG "error: -C requires value" -+ r="-C $1" ++ r="-C ${2?}" + shift + ;; + --tf) -+ shift; -+ is_value $1 || BUG "error: --tf requires value" -+ tf="$1" ++ tf="${2?}" + shift + ;; + --t2) -+ shift; -+ is_value $1 || BUG "error: --t2 requires value" -+ t2="$1" ++ t2="${2?}" + shift + ;; + --tk) -+ shift; -+ is_value $1 || BUG "error: --tk requires value" -+ tk="$1" ++ tk="${2?}" + shift + ;; -+ *) ++ -*) + BUG "error: unknown option: '$1'" + ;; ++ *) ++ BUG "error: unbound argument: '$1'" ++ ;; + esac ++ shift + done && + + ( 22: cc39ecf10ae = 22: 06d56d3a733 t/perf: avoid copying builtin fsmonitor files into test repo 23: 2bb3eb84767 = 23: 2dd02151278 t/helper/test-chmtime: skip directories on Windows 24: bab9a9b0802 = 24: bb88cddc137 t/perf/p7519: fix coding style 25: 2dd06ad2f71 = 25: 50c2afaa49e t/perf/p7519: speed up test on Windows 26: 6eaa5765ae1 = 26: 5b18e3b6926 t/perf/p7519: add fsmonitor--daemon test cases 27: 30957f3930e = 27: 899c23f63c3 fsmonitor--daemon: periodically truncate list of modified files 28: c8ca2a17277 = 28: 597a7192f94 fsmonitor--daemon: use a cookie file to sync with file system 29: 4caf1d89b84 = 29: 68a05fd2892 fsmonitor: force update index after large responses 30: f87a1eba693 = 30: 5eb696daba2 t7527: test status with untracked-cache and fsmonitor--daemon