fsmonitor: option to allow fsmonitor to run against network-mounted repos
Subsystems (2)
PATCH v11 2/6 fsmonitor: relocate socket file if .git directory is remote
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
If the .git directory is on a remote file system, create the socket file in 'fsmonitor.socketDir' if it is defined, else create it in $HOME.
--- Makefile | 1 + builtin/fsmonitor--daemon.c | 3 +- compat/fsmonitor/fsm-ipc-darwin.c | 51 ++++++++++++++++++++++++++ compat/fsmonitor/fsm-ipc-win32.c | 9 +++++ compat/fsmonitor/fsm-settings-darwin.c | 2 +- contrib/buildsystems/CMakeLists.txt | 2 + fsmonitor-ipc.c | 18 ++++----- fsmonitor-ipc.h | 4 +- 8 files changed, 77 insertions(+), 13 deletions(-) create mode 100644 compat/fsmonitor/fsm-ipc-darwin.c create mode 100644 compat/fsmonitor/fsm-ipc-win32.c
Makefile +1 -0
@@ -2034,6 +2034,7 @@ ifdef FSMONITOR_DAEMON_BACKEND COMPAT_CFLAGS += -DHAVE_FSMONITOR_DAEMON_BACKEND COMPAT_OBJS += compat/fsmonitor/fsm-listen-$(FSMONITOR_DAEMON_BACKEND).o COMPAT_OBJS += compat/fsmonitor/fsm-health-$(FSMONITOR_DAEMON_BACKEND).o + COMPAT_OBJS += compat/fsmonitor/fsm-ipc-$(FSMONITOR_DAEMON_BACKEND).o endif ifdef FSMONITOR_OS_SETTINGS
builtin/fsmonitor--daemon.c +2 -1
@@ -1343,7 +1343,8 @@ static int fsmonitor_run_daemon(void)* directory.) */ strbuf_init (&state.path_ipc,0 ); -strbuf_addstr (&state.path_ipc,absolute_path (fsmonitor_ipc__get_path ())); +strbuf_addstr (&state.path_ipc, +absolute_path (fsmonitor_ipc__get_path (the_repository)));/* * Confirm that we can create platform-specific resources for the
compat/fsmonitor/fsm-ipc-darwin.c new file +51 -0
51 lines not on this page · raw message
compat/fsmonitor/fsm-ipc-win32.c new file +9 -0
@@ -0,0 +1,9 @@ +#include "config.h" +#include "fsmonitor-ipc.h" + +const char *fsmonitor_ipc__get_path (struct repository *r) { +static char *ret; +if (!ret) + ret =git_pathdup ("fsmonitor--daemon.ipc" ); +return ret; +}
compat/fsmonitor/fsm-settings-darwin.c +1 -1
@@ -26,7 +26,7 @@static enum fsmonitor_reason check_uds_volume (struct repository *r) {struct fs_info fs; -const char *ipc_path =fsmonitor_ipc__get_path (); +const char *ipc_path =fsmonitor_ipc__get_path (r);struct strbuf path =STRBUF_INIT ;strbuf_add (&path, ipc_path,strlen (ipc_path));
contrib/buildsystems/CMakeLists.txt +2 -0
@@ -308,6 +308,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-win32.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-win32.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-win32.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-win32.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS)
@@ -316,6 +317,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-darwin.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-darwin.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-darwin.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-darwin.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS)
fsmonitor-ipc.c +8 -10
@@ -18,7 +18,7 @@ int fsmonitor_ipc__is_supported(void)return 0 ; } -const char *fsmonitor_ipc__get_path (void ) +const char *fsmonitor_ipc__get_path (struct repository *r) {return NULL ; }
@@ -47,11 +47,9 @@ int fsmonitor_ipc__is_supported(void)return 1 ; } -GIT_PATH_FUNC(fsmonitor_ipc__get_path, "fsmonitor--daemon.ipc") -enum ipc_active_state fsmonitor_ipc__get_state (void ) { -return ipc_get_active_state (fsmonitor_ipc__get_path ()); +return ipc_get_active_state (fsmonitor_ipc__get_path (the_repository)); }static int spawn_daemon (void )
@@ -81,8 +79,8 @@ int fsmonitor_ipc__send_query(const char *since_token,trace2_data_string ("fsm_client" ,NULL ,"query/command" , tok); try_again: - state =ipc_client_try_connect (fsmonitor_ipc__get_path (), &options, - &connection); + state =ipc_client_try_connect (fsmonitor_ipc__get_path (the_repository), + &options, &connection);switch (state) {case IPC_STATE__LISTENING :
@@ -117,13 +115,13 @@ try_again:case IPC_STATE__INVALID_PATH : ret =error (_ ("fsmonitor_ipc__send_query: invalid path '%s'" ), -fsmonitor_ipc__get_path ()); +fsmonitor_ipc__get_path (the_repository)); goto done;case IPC_STATE__OTHER_ERROR :default : ret =error (_ ("fsmonitor_ipc__send_query: unspecified error on '%s'" ), -fsmonitor_ipc__get_path ()); +fsmonitor_ipc__get_path (the_repository)); goto done; }
@@ -149,8 +147,8 @@ int fsmonitor_ipc__send_command(const char *command, options.wait_if_busy =1 ; options.wait_if_not_found =0 ; - state =ipc_client_try_connect (fsmonitor_ipc__get_path (), &options, - &connection); + state =ipc_client_try_connect (fsmonitor_ipc__get_path (the_repository), + &options, &connection);if (state !=IPC_STATE__LISTENING ) {die (_ ("fsmonitor--daemon is not running" ));return -1 ;
fsmonitor-ipc.h +3 -1
@@ -3,6 +3,8 @@#include "simple-ipc.h" +struct repository ; +/* * Returns true if built-in file system monitor daemon is defined * for this platform.
@@ -16,7 +18,7 @@ int fsmonitor_ipc__is_supported(void);* * Returns NULL if the daemon is not supported on this platform. */ -const char *fsmonitor_ipc__get_path (void ); +const char *fsmonitor_ipc__get_path (struct repository *r);/* * Try to determine whether there is a `git-fsmonitor--daemon` process
Signature · 1 line
gitgitgadget
PATCH v11 3/6 fsmonitor: avoid socket location check if using hook
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
If monitoring is done via fsmonitor hook rather than IPC there is no need to check if the location of the Unix Domain socket (UDS) file is on a remote filesystem.
--- compat/fsmonitor/fsm-settings-darwin.c | 10 ++++++---- compat/fsmonitor/fsm-settings-win32.c | 2 +- fsmonitor-settings.c | 8 ++++---- fsmonitor-settings.h | 2 +- 4 files changed, 12 insertions(+), 10 deletions(-)
compat/fsmonitor/fsm-settings-darwin.c +6 -4
@@ -49,13 +49,15 @@ static enum fsmonitor_reason check_uds_volume(struct repository *r)return FSMONITOR_REASON_OK ; } -enum fsmonitor_reason fsm_os__incompatible (struct repository *r) +enum fsmonitor_reason fsm_os__incompatible (struct repository *r,int ipc) {enum fsmonitor_reason reason; - reason =check_uds_volume (r); -if (reason !=FSMONITOR_REASON_OK ) -return reason; +if (ipc) { + reason =check_uds_volume (r); +if (reason !=FSMONITOR_REASON_OK ) +return reason; + }return FSMONITOR_REASON_OK ; }
compat/fsmonitor/fsm-settings-win32.c +1 -1
@@ -25,7 +25,7 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)return FSMONITOR_REASON_OK ; } -enum fsmonitor_reason fsm_os__incompatible (struct repository *r) +enum fsmonitor_reason fsm_os__incompatible (struct repository *r,int ipc) {enum fsmonitor_reason reason;
fsmonitor-settings.c +4 -4
@@ -60,7 +60,7 @@ static enum fsmonitor_reason check_remote(struct repository *r) }#endif -static enum fsmonitor_reason check_for_incompatible (struct repository *r) +static enum fsmonitor_reason check_for_incompatible (struct repository *r,int ipc) {if (!r->worktree) {/*
@@ -77,7 +77,7 @@ static enum fsmonitor_reason check_for_incompatible(struct repository *r) reason =check_remote (r);if (reason !=FSMONITOR_REASON_OK )return reason; - reason =fsm_os__incompatible (r); + reason =fsm_os__incompatible (r, ipc);if (reason !=FSMONITOR_REASON_OK )return reason; }
@@ -162,7 +162,7 @@ const char *fsm_settings__get_hook_path(struct repository *r)void fsm_settings__set_ipc (struct repository *r) { -enum fsmonitor_reason reason =check_for_incompatible (r); +enum fsmonitor_reason reason =check_for_incompatible (r,1 );if (reason !=FSMONITOR_REASON_OK ) {fsm_settings__set_incompatible (r, reason);
@@ -185,7 +185,7 @@ void fsm_settings__set_ipc(struct repository *r)void fsm_settings__set_hook (struct repository *r,const char *path) { -enum fsmonitor_reason reason =check_for_incompatible (r); +enum fsmonitor_reason reason =check_for_incompatible (r,0 );if (reason !=FSMONITOR_REASON_OK ) {fsm_settings__set_incompatible (r, reason);
fsmonitor-settings.h +1 -1
@@ -48,7 +48,7 @@ struct fsmonitor_settings;* fsm_os__* routines should considered private to fsm_settings__ * routines. */ -enum fsmonitor_reason fsm_os__incompatible (struct repository *r); +enum fsmonitor_reason fsm_os__incompatible (struct repository *r,int ipc);#endif /* HAVE_FSMONITOR_OS_SETTINGS */ #endif /* FSMONITOR_SETTINGS_H */
Signature · 1 line
gitgitgadget
PATCH v11 4/6 fsmonitor: deal with synthetic firmlinks on macOS
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
Starting with macOS 10.15 (Catalina), Apple introduced a new feature called 'firmlinks' in order to separate the boot volume into two volumes, one read-only and one writable but still present them to the user as a single volume. Along with this change, Apple removed the ability to create symlinks in the root directory and replaced them with 'synthetic firmlinks'. See 'man synthetic.conf'
When FSEevents reports the path of changed files, if the path involves a synthetic firmlink, the path is reported from the point of the synthetic firmlink and not the real path. For example:
Real path:
/System/Volumes/Data/network/working/directory/foo.txt
Synthetic firmlink:
/network -> /System/Volumes/Data/network
FSEvents path:
/network/working/directory/foo.txt
This causes the FSEvents path to not match against the worktree directory.
There are several ways in which synthetic firmlinks can be created:
they can be defined in /etc/synthetic.conf, the automounter can create them, and there may be other means. Simply reading /etc/synthetic.conf is insufficient. No matter what process creates synthetic firmlinks, they all get created in the root directory.
Therefore, in order to deal with synthetic firmlinks, the root directory is scanned and the first possible synthetic firmink that, when resolved, is a prefix of the worktree is used to map FSEvents paths to worktree paths.
--- builtin/fsmonitor--daemon.c | 8 +++ compat/fsmonitor/fsm-listen-darwin.c | 14 +++- compat/fsmonitor/fsm-path-utils-darwin.c | 92 ++++++++++++++++++++++++ compat/fsmonitor/fsm-path-utils-win32.c | 17 +++++ fsmonitor--daemon.h | 3 + fsmonitor-path-utils.h | 36 ++++++++++ 6 files changed, 168 insertions(+), 2 deletions(-)
builtin/fsmonitor--daemon.c +8 -0
@@ -3,6 +3,7 @@#include "parse-options.h" #include "fsmonitor.h" #include "fsmonitor-ipc.h" +#include "fsmonitor-path-utils.h" #include "compat/fsmonitor/fsm-health.h" #include "compat/fsmonitor/fsm-listen.h" #include "fsmonitor--daemon.h"
@@ -1282,6 +1283,13 @@ static int fsmonitor_run_daemon(void)strbuf_addstr (&state.path_worktree_watch,absolute_path (get_git_work_tree ())); state.nr_paths_watching =1 ; + state.alias.alias =NULL ; + state.alias.points_to =NULL ; +if (fsmonitor__get_alias (state.path_worktree_watch.buf, &state.alias)) { + err =error (_ ("could not get worktree alias" )); + goto done; + } +/* * We create and delete cookie files somewhere inside the .git * directory to help us keep sync with the file system. If
compat/fsmonitor/fsm-listen-darwin.c +12 -2
@@ -26,6 +26,7 @@#include "fsmonitor.h" #include "fsm-listen.h" #include "fsmonitor--daemon.h" +#include "fsmonitor-path-utils.h" struct fsm_listen_data {
@@ -198,8 +199,9 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,struct string_list cookie_list =STRING_LIST_INIT_DUP ;const char *path_k;const char *slash; -int k; +char *resolved =NULL ;struct strbuf tmp =STRBUF_INIT ; +int k;/* * Build a list of all filesystem changes into a private/local
@@ -209,7 +211,12 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,/* * On Mac, we receive an array of absolute paths. */ - path_k = paths[k]; +free (resolved); + resolved =fsmonitor__resolve_alias (paths[k], &state->alias); +if (resolved) + path_k = resolved; +else + path_k = paths[k];/* * If you want to debug FSEvents, log them to GIT_TRACE_FSMONITOR.
@@ -238,6 +245,7 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,fsmonitor_force_resync (state);fsmonitor_batch__free_list (batch);string_list_clear (&cookie_list,0 ); + batch =NULL ;/* * We assume that any events that we received
@@ -360,12 +368,14 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef, } } +free (resolved);fsmonitor_publish (state, batch, &cookie_list);string_list_clear (&cookie_list,0 );strbuf_release (&tmp);return ; force_shutdown: +free (resolved);fsmonitor_batch__free_list (batch);string_list_clear (&cookie_list,0 );
compat/fsmonitor/fsm-path-utils-darwin.c +92 -0
100 lines not on this page · raw message
compat/fsmonitor/fsm-path-utils-win32.c +17 -0
@@ -126,3 +126,20 @@ int fsmonitor__is_fs_remote(const char *path)return -1 ;return fs.is_remote; } + +/* +* No-op for now. +*/ +int fsmonitor__get_alias (const char *path,struct alias_info *info) +{ +return 0 ; +} + +/* +* No-op for now. +*/ +char *fsmonitor__resolve_alias (const char *path, +const struct alias_info *info) +{ +return NULL ; +}
fsmonitor--daemon.h +3 -0
@@ -8,6 +8,7 @@#include "run-command.h" #include "simple-ipc.h" #include "thread-utils.h" +#include "fsmonitor-path-utils.h" struct fsmonitor_batch ;struct fsmonitor_token_data ;
@@ -43,6 +44,7 @@ struct fsmonitor_daemon_state {struct strbuf path_worktree_watch;struct strbuf path_gitdir_watch; +struct alias_info alias;int nr_paths_watching;struct fsmonitor_token_data *current_token_data;
@@ -59,6 +61,7 @@ struct fsmonitor_daemon_state {struct ipc_server_data *ipc_server_data;struct strbuf path_ipc; + };/*
fsmonitor-path-utils.h +36 -0
@@ -1,6 +1,14 @@ #ifndef FSM_PATH_UTILS_H #define FSM_PATH_UTILS_H +#include "strbuf.h" + +struct alias_info +{ + char *alias; + char *points_to; +}; + struct fs_info { int is_remote; char *typename;
@@ -20,4 +28,32 @@ int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info);*/ int fsmonitor__is_fs_remote (const char *path); +/* +* Get the alias in given path, if any. +* +* Sets alias to the first alias that matches any part of the path. +* +* If an alias is found, info.alias and info.points_to are set to the +* found mapping. +* +* Returns -1 on error, 0 otherwise. +* +* The caller owns the storage that is occupied by set info.alias and +* info.points_to and is responsible for releasing it with `free(3)` +* when done. +*/ +int fsmonitor__get_alias (const char *path,struct alias_info *info); + +/* +* Resolve the path against the given alias. +* +* Returns the resolved path if there is one, NULL otherwise. +* +* The caller owns the storage that the returned string occupies and +* is responsible for releasing it with `free(3)` when done. +*/ +char *fsmonitor__resolve_alias (const char *path, +const struct alias_info *info); + +#endif
Signature · 1 line
gitgitgadget
PATCH v11 5/6 fsmonitor: check for compatability before communicating with fsmonitor
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
If fsmonitor is not in a compatible state, die with an appropriate error messge.
--- compat/fsmonitor/fsm-settings-darwin.c | 2 +- fsmonitor-settings.c | 10 +++++++--- fsmonitor-settings.h | 2 +- fsmonitor.c | 4 ++++ 4 files changed, 13 insertions(+), 5 deletions(-)
compat/fsmonitor/fsm-settings-darwin.c +1 -1
@@ -38,7 +38,7 @@ static enum fsmonitor_reason check_uds_volume(struct repository *r)strbuf_release (&path);if (fs.is_remote) -return FSMONITOR_REASON_REMOTE ; +return FSMONITOR_REASON_NOSOCKETS ;if (!strcmp (fs.typename,"msdos" ))/* aka FAT32 */ return FSMONITOR_REASON_NOSOCKETS ;
fsmonitor-settings.c +7 -3
@@ -1,6 +1,7 @@#include "cache.h" #include "config.h" #include "repository.h" +#include "fsmonitor-ipc.h" #include "fsmonitor-settings.h" #include "fsmonitor-path-utils.h"
@@ -242,10 +243,11 @@ enum fsmonitor_reason fsm_settings__get_reason(struct repository *r)return r->settings.fsmonitor->reason; } -char *fsm_settings__get_incompatible_msg (const struct repository *r, +char *fsm_settings__get_incompatible_msg (struct repository *r,enum fsmonitor_reason reason) {struct strbuf msg =STRBUF_INIT ; +const char *socket_dir;switch (reason) {case FSMONITOR_REASON_UNTESTED :
@@ -281,9 +283,11 @@ char *fsm_settings__get_incompatible_msg(const struct repository *r, goto done;case FSMONITOR_REASON_NOSOCKETS : + socket_dir =dirname ((char *)fsmonitor_ipc__get_path (r));strbuf_addf (&msg, -_ ("repository '%s' is incompatible with fsmonitor due to lack of Unix sockets" ), - r->worktree); +_ ("socket directory '%s' is incompatible with fsmonitor due" +" to lack of Unix sockets support" ), + socket_dir); goto done; }
fsmonitor-settings.h +1 -1
@@ -33,7 +33,7 @@ enum fsmonitor_mode fsm_settings__get_mode(struct repository *r);const char *fsm_settings__get_hook_path (struct repository *r);enum fsmonitor_reason fsm_settings__get_reason (struct repository *r); -char *fsm_settings__get_incompatible_msg (const struct repository *r, +char *fsm_settings__get_incompatible_msg (struct repository *r,enum fsmonitor_reason reason);struct fsmonitor_settings ;
fsmonitor.c +4 -0
@@ -305,6 +305,10 @@ void refresh_fsmonitor(struct index_state *istate)int is_trivial =0 ;struct repository *r = istate->repo ? istate->repo : the_repository;enum fsmonitor_mode fsm_mode =fsm_settings__get_mode (r); +enum fsmonitor_reason reason =fsm_settings__get_reason (r); + +if (reason >FSMONITOR_REASON_OK ) +warning ("%s" ,fsm_settings__get_incompatible_msg (r, reason));if (fsm_mode <=FSMONITOR_MODE_DISABLED || istate->fsmonitor_has_run_once)
Signature · 1 line
gitgitgadget
PATCH v11 6/6 fsmonitor: add documentation for allowRemote and socketDir options
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
Add documentation for 'fsmonitor.allowRemote' and 'fsmonitor.socketDir'. Call-out experimental nature of 'fsmonitor.allowRemote' and limited file system support for 'fsmonitor.socketDir'.
--- Documentation/git-fsmonitor--daemon.txt | 48 +++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-)
Documentation/git-fsmonitor--daemon.txt +45 -3
@@ -3,7 +3,7 @@ git-fsmonitor{litdd}daemon(1) NAME ---- -git-fsmonitor--daemon - A Built-in File System Monitor +git-fsmonitor--daemon - A Built-in Filesystem Monitor SYNOPSIS --------
@@ -17,7 +17,7 @@ DESCRIPTION ----------- A daemon to watch the working directory for file and directory -changes using platform-specific file system notification facilities. +changes using platform-specific filesystem notification facilities. This daemon communicates directly with commands like `git status` using the link:technical/api-simple-ipc.html[simple IPC] interface
@@ -63,13 +63,55 @@ CAVEATS ------- The fsmonitor daemon does not currently know about submodules and does -not know to filter out file system events that happen within a +not know to filter out filesystem events that happen within a submodule. If fsmonitor daemon is watching a super repo and a file is modified within the working directory of a submodule, it will report the change (as happening against the super repo). However, the client will properly ignore these extra events, so performance may be affected but it will not cause an incorrect result. +By default, the fsmonitor daemon refuses to work against network-mounted +repositories; this may be overridden by setting `fsmonitor.allowRemote` to +`true`. Note, however, that the fsmonitor daemon is not guaranteed to work +correctly with all network-mounted repositories and such use is considered +experimental. + +On Mac OS, the inter-process communication (IPC) between various Git +commands and the fsmonitor daemon is done via a Unix domain socket (UDS) -- a +special type of file -- which is supported by native Mac OS filesystems, +but not on network-mounted filesystems, NTFS, or FAT32. Other filesystems +may or may not have the needed support; the fsmonitor daemon is not guaranteed +to work with these filesystems and such use is considered experimental. + +By default, the socket is created in the `.git` directory, however, if the +`.git` directory is on a network-mounted filesystem, it will be instead be +created at `$HOME/.git-fsmonitor-*` unless `$HOME` itself is on a +network-mounted filesystem in which case you must set the configuration +variable `fsmonitor.socketDir` to the path of a directory on a Mac OS native +filesystem in which to create the socket file. + +If none of the above directories (`.git`, `$HOME`, or `fsmonitor.socketDir`) +is on a native Mac OS file filesystem the fsmonitor daemon will report an +error that will cause the daemon and the currently running command to exit. + +CONFIGURATION +------------- + +When `core.fsmonitor` is set to `true` (see linkgit:git-config[1]) +the fsmonitor daemon will pay attention to the following configuration +variables: + +`fsmonitor.allowRemote`:: + By default, the daemon refuses to work against network-mounted + repositories. Setting `fsmonitor.allowRemote` to `true` overrides + this behavior. + +`fsmonitor.socketDir`:: + This Mac OS-specific option, if set, specifies the directory in + which to create the Unix domain socket used for communication + between fsmonitor and various Git commands. The directory must + reside on a native Mac OS filesystem as discussed above. + GIT --- Part of the linkgit:git[1] suite
Signature · 1 line
gitgitgadget
v12 cover fsmonitor: option to allow fsmonitor to run against network-mounted repos
In reply to Eric DeCosta via GitGitGadgetFollow-on to the work done to allow Windows to work against network-mounted repos for macOS.
Have macOS take advantage of the same configuration option, 'fsmonitor.allowRemote' that was introduced for Windows. Setting this option to true will override the default behavior (erroring-out) when a network-mounted repo is detected by fsmonitor.
The added wrinkle being that the Unix domain socket (UDS) file used for IPC cannot be created in a network location; instead $HOME is used if the default location is on the network. The user may, optionally, set the 'fsmonitor.socketDir' configuration option to a valid, local directory if $HOME itself is on the network or is simply not the desired location for the UDS file.
An additional issue is that for mount points in the root directory, FSEvents does not report a path that matches the worktree directory due to the introduction of 'synthetic firmlinks'. fsmonitor must map the FSEvents paths to the worktree directory by interrogating the root filesystem for synthetic firmlinks and using that information to translate the path.
v12 differs from v11:
* bug fixes
v11 differs from v10:
* incorporates code review feedback
* fix memory leak in fsm-listen-darwin.c
v10 differs from v9:
* incorporates code review feedback
* improves error messaging for incompatible socket directory
v9 differs from v8:
* incorporates code review feedback
* check for incompatibility before communicating with fsmonitor
v8 differs from v7:
* incorporates code review feedback
* gets the rebase right
v7 differs from v6:
* incorporates code review feedback
v6 differs from v5:
* incorporates earlier, Windows-specific changes that have not made it back yet to the master branch
* incorporates code review feedback
* adds documentation for 'fsmonitor.allowRemote' and 'fsmonitor.socketDir'
v5 differs significantly from earlier versions:
* redesign of handling 'fsmonitor.allowRemote' and 'fsmonitor.socketDir' such that these options are no longer added to the settings data structure but are rather read from config at point of use
* refactoring of code for handling platform-specific file system checks via a common interface to avoid platform #ifdef in IPC code and be in-model with other platform-specific fsmonitor code
* dealing with 'synthetic firmlinks' on macOS
Eric DeCosta (6):
fsmonitor: refactor filesystem checks to common interface
fsmonitor: relocate socket file if .git directory is remote
fsmonitor: avoid socket location check if using hook
fsmonitor: deal with synthetic firmlinks on macOS
fsmonitor: check for compatability before communicating with fsmonitor
fsmonitor: add documentation for allowRemote and socketDir options
Documentation/git-fsmonitor--daemon.txt | 48 ++++++- Makefile | 2 + builtin/fsmonitor--daemon.c | 11 +- compat/fsmonitor/fsm-ipc-darwin.c | 52 +++++++ compat/fsmonitor/fsm-ipc-win32.c | 9 ++ compat/fsmonitor/fsm-listen-darwin.c | 14 +- compat/fsmonitor/fsm-path-utils-darwin.c | 132 +++++++++++++++++ compat/fsmonitor/fsm-path-utils-win32.c | 145 +++++++++++++++++++ compat/fsmonitor/fsm-settings-darwin.c | 72 +++------- compat/fsmonitor/fsm-settings-win32.c | 174 +---------------------- contrib/buildsystems/CMakeLists.txt | 4 + fsmonitor--daemon.h | 3 + fsmonitor-ipc.c | 18 ++- fsmonitor-ipc.h | 4 +- fsmonitor-path-utils.h | 59 ++++++++ fsmonitor-settings.c | 68 ++++++++- fsmonitor-settings.h | 4 +- fsmonitor.c | 7 + 18 files changed, 580 insertions(+), 246 deletions(-) create mode 100644 compat/fsmonitor/fsm-ipc-darwin.c create mode 100644 compat/fsmonitor/fsm-ipc-win32.c create mode 100644 compat/fsmonitor/fsm-path-utils-darwin.c create mode 100644 compat/fsmonitor/fsm-path-utils-win32.c create mode 100644 fsmonitor-path-utils.h
base-commit: 4b79ee4b0cd1130ba8907029cdc5f6a1632aca26
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1326%2Fedecosta-mw%2Ffsmonitor_macos-v12
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1326/edecosta-mw/fsmonitor_macos-v12
Pull-Request: https://github.com/gitgitgadget/git/pull/1326
Range-diff vs v11:
1: 155a6890806 = 1: 5958dab0163 fsmonitor: refactor filesystem checks to common interface
2: dbe113abb87 ! 2: 20220b08edb fsmonitor: relocate socket file if .git directory is remote
@@ compat/fsmonitor/fsm-ipc-darwin.c (new)
+
+const char *fsmonitor_ipc__get_path(struct repository *r)
+{
-+ static const char *ipc_path;
++ static const char *ipc_path = NULL;
+ SHA_CTX sha1ctx;
+ char *sock_dir = NULL;
+ struct strbuf ipc_file = STRBUF_INIT;
@@ compat/fsmonitor/fsm-ipc-darwin.c (new)
+ if (ipc_path)
+ return ipc_path;
+
-+ ipc_path = fsmonitor_ipc__get_default_path();
+
+ /* By default the socket file is created in the .git directory */
-+ if (fsmonitor__is_fs_remote(ipc_path) < 1)
++ if (fsmonitor__is_fs_remote(r->gitdir) < 1) {
++ ipc_path = fsmonitor_ipc__get_default_path();
+ return ipc_path;
++ }
+
+ SHA1_Init(&sha1ctx);
+ SHA1_Update(&sha1ctx, r->worktree, strlen(r->worktree));
3: 86c15299ae8 = 3: e9921550a67 fsmonitor: avoid socket location check if using hook
4: 1a516fd9214 = 4: 6efdc6ed74e fsmonitor: deal with synthetic firmlinks on macOS
5: e0fe05dabef ! 5: 421d77775dc fsmonitor: check for compatability before communicating with fsmonitor
@@ fsmonitor-settings.h: enum fsmonitor_mode fsm_settings__get_mode(struct reposito
struct fsmonitor_settings;
## fsmonitor.c ##
+@@ fsmonitor.c: static int fsmonitor_force_update_threshold = 100;
+
+ void refresh_fsmonitor(struct index_state *istate)
+ {
++ static int warn_once = 0;
+ struct strbuf query_result = STRBUF_INIT;
+ int query_success = 0, hook_version = -1;
+ size_t bol = 0; /* beginning of line */
@@ fsmonitor.c: void refresh_fsmonitor(struct index_state *istate)
int is_trivial = 0;
struct repository *r = istate->repo ? istate->repo : the_repository;
enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r);
+ enum fsmonitor_reason reason = fsm_settings__get_reason(r);
+
-+ if (reason > FSMONITOR_REASON_OK)
++ if (!warn_once && reason > FSMONITOR_REASON_OK) {
++ warn_once = 1;
+ warning("%s", fsm_settings__get_incompatible_msg(r, reason));
++ }
if (fsm_mode <= FSMONITOR_MODE_DISABLED ||
istate->fsmonitor_has_run_once) 6: 3200b505988 = 6: b375b0ac798 fsmonitor: add documentation for allowRemote and socketDir options
Signature · 1 line
gitgitgadget
PATCH v12 1/6 fsmonitor: refactor filesystem checks to common interface
From: Eric DeCosta <[email protected]>
Provide a common interface for getting basic filesystem information including filesystem type and whether the filesystem is remote.
Refactor existing code for getting basic filesystem info and detecting remote file systems to the new interface.
Refactor filesystem checks to leverage new interface. For macOS, error-out if the Unix Domain socket (UDS) file is on a remote filesystem.
--- Makefile | 1 + compat/fsmonitor/fsm-path-utils-darwin.c | 40 ++++++ compat/fsmonitor/fsm-path-utils-win32.c | 128 +++++++++++++++++ compat/fsmonitor/fsm-settings-darwin.c | 62 +++----- compat/fsmonitor/fsm-settings-win32.c | 172 +---------------------- contrib/buildsystems/CMakeLists.txt | 2 + fsmonitor-path-utils.h | 23 +++ fsmonitor-settings.c | 50 +++++++ 8 files changed, 263 insertions(+), 215 deletions(-) create mode 100644 compat/fsmonitor/fsm-path-utils-darwin.c create mode 100644 compat/fsmonitor/fsm-path-utils-win32.c create mode 100644 fsmonitor-path-utils.h
Makefile +1 -0
@@ -2044,6 +2044,7 @@ endif ifdef FSMONITOR_OS_SETTINGS COMPAT_CFLAGS += -DHAVE_FSMONITOR_OS_SETTINGS COMPAT_OBJS += compat/fsmonitor/fsm-settings-$(FSMONITOR_OS_SETTINGS).o + COMPAT_OBJS += compat/fsmonitor/fsm-path-utils-$(FSMONITOR_OS_SETTINGS).o endif ifeq ($(TCLTK_PATH),)
compat/fsmonitor/fsm-path-utils-darwin.c new file +40 -0
@@ -0,0 +1,40 @@ +#include "fsmonitor.h" +#include "fsmonitor-path-utils.h" +#include <sys/param.h> +#include <sys/mount.h> + +int fsmonitor__get_fs_info (const char *path,struct fs_info *fs_info) +{ +struct statfs fs; +if (statfs (path, &fs) ==-1 ) { +int saved_errno = errno; +trace_printf_key (&trace_fsmonitor,"statfs('%s') failed: %s" , + path,strerror (saved_errno)); + errno = saved_errno; +return -1 ; + } + +trace_printf_key (&trace_fsmonitor, +"statfs('%s') [type 0x%08x][flags 0x%08x] '%s'" , + path, fs.f_type, fs.f_flags, fs.f_fstypename); + +if (!(fs.f_flags &MNT_LOCAL )) + fs_info->is_remote =1 ; +else + fs_info->is_remote =0 ; + + fs_info->typename = fs.f_fstypename; + +trace_printf_key (&trace_fsmonitor, +"'%s' is_remote: %d" , + path, fs_info->is_remote); +return 0 ; +} + +int fsmonitor__is_fs_remote (const char *path) +{ +struct fs_info fs; +if (fsmonitor__get_fs_info (path, &fs)) +return -1 ; +return fs.is_remote; +}
compat/fsmonitor/fsm-path-utils-win32.c new file +128 -0
128 lines not on this page · raw message
compat/fsmonitor/fsm-settings-darwin.c +17 -45
96 lines not on this page · raw message
compat/fsmonitor/fsm-settings-win32.c +2 -170
190 lines not on this page · raw message
contrib/buildsystems/CMakeLists.txt +2 -0
@@ -308,6 +308,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-win32.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-win32.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-win32.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-win32.c)
@@ -315,6 +316,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-darwin.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-darwin.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-darwin.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-darwin.c)
fsmonitor-path-utils.h new file +23 -0
@@ -0,0 +1,23 @@ +#ifndef FSM_PATH_UTILS_H +#define FSM_PATH_UTILS_H + +struct fs_info { +int is_remote; +char *typename; +}; + +/* +* Get some basic filesystem informtion for the given path +* +* Returns -1 on error, zero otherwise. +*/ +int fsmonitor__get_fs_info (const char *path,struct fs_info *fs_info); + +/* +* Determines if the filesystem that path resides on is remote. +* +* Returns -1 on error, 0 if not remote, 1 if remote. +*/ +int fsmonitor__is_fs_remote (const char *path); + +#endif
fsmonitor-settings.c +50 -0
68 lines not on this page · raw message
Signature · 1 line
gitgitgadget
PATCH v12 2/6 fsmonitor: relocate socket file if .git directory is remote
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
If the .git directory is on a remote file system, create the socket file in 'fsmonitor.socketDir' if it is defined, else create it in $HOME.
--- Makefile | 1 + builtin/fsmonitor--daemon.c | 3 +- compat/fsmonitor/fsm-ipc-darwin.c | 52 ++++++++++++++++++++++++++ compat/fsmonitor/fsm-ipc-win32.c | 9 +++++ compat/fsmonitor/fsm-settings-darwin.c | 2 +- contrib/buildsystems/CMakeLists.txt | 2 + fsmonitor-ipc.c | 18 ++++----- fsmonitor-ipc.h | 4 +- 8 files changed, 78 insertions(+), 13 deletions(-) create mode 100644 compat/fsmonitor/fsm-ipc-darwin.c create mode 100644 compat/fsmonitor/fsm-ipc-win32.c
Makefile +1 -0
@@ -2039,6 +2039,7 @@ ifdef FSMONITOR_DAEMON_BACKEND COMPAT_CFLAGS += -DHAVE_FSMONITOR_DAEMON_BACKEND COMPAT_OBJS += compat/fsmonitor/fsm-listen-$(FSMONITOR_DAEMON_BACKEND).o COMPAT_OBJS += compat/fsmonitor/fsm-health-$(FSMONITOR_DAEMON_BACKEND).o + COMPAT_OBJS += compat/fsmonitor/fsm-ipc-$(FSMONITOR_DAEMON_BACKEND).o endif ifdef FSMONITOR_OS_SETTINGS
builtin/fsmonitor--daemon.c +2 -1
@@ -1343,7 +1343,8 @@ static int fsmonitor_run_daemon(void)* directory.) */ strbuf_init (&state.path_ipc,0 ); -strbuf_addstr (&state.path_ipc,absolute_path (fsmonitor_ipc__get_path ())); +strbuf_addstr (&state.path_ipc, +absolute_path (fsmonitor_ipc__get_path (the_repository)));/* * Confirm that we can create platform-specific resources for the
compat/fsmonitor/fsm-ipc-darwin.c new file +52 -0
52 lines not on this page · raw message
compat/fsmonitor/fsm-ipc-win32.c new file +9 -0
@@ -0,0 +1,9 @@ +#include "config.h" +#include "fsmonitor-ipc.h" + +const char *fsmonitor_ipc__get_path (struct repository *r) { +static char *ret; +if (!ret) + ret =git_pathdup ("fsmonitor--daemon.ipc" ); +return ret; +}
compat/fsmonitor/fsm-settings-darwin.c +1 -1
@@ -26,7 +26,7 @@static enum fsmonitor_reason check_uds_volume (struct repository *r) {struct fs_info fs; -const char *ipc_path =fsmonitor_ipc__get_path (); +const char *ipc_path =fsmonitor_ipc__get_path (r);struct strbuf path =STRBUF_INIT ;strbuf_add (&path, ipc_path,strlen (ipc_path));
contrib/buildsystems/CMakeLists.txt +2 -0
@@ -308,6 +308,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-win32.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-win32.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-win32.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-win32.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS)
@@ -316,6 +317,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-darwin.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-darwin.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-darwin.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-darwin.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS)
fsmonitor-ipc.c +8 -10
@@ -18,7 +18,7 @@ int fsmonitor_ipc__is_supported(void)return 0 ; } -const char *fsmonitor_ipc__get_path (void ) +const char *fsmonitor_ipc__get_path (struct repository *r) {return NULL ; }
@@ -47,11 +47,9 @@ int fsmonitor_ipc__is_supported(void)return 1 ; } -GIT_PATH_FUNC(fsmonitor_ipc__get_path, "fsmonitor--daemon.ipc") -enum ipc_active_state fsmonitor_ipc__get_state (void ) { -return ipc_get_active_state (fsmonitor_ipc__get_path ()); +return ipc_get_active_state (fsmonitor_ipc__get_path (the_repository)); }static int spawn_daemon (void )
@@ -81,8 +79,8 @@ int fsmonitor_ipc__send_query(const char *since_token,trace2_data_string ("fsm_client" ,NULL ,"query/command" , tok); try_again: - state =ipc_client_try_connect (fsmonitor_ipc__get_path (), &options, - &connection); + state =ipc_client_try_connect (fsmonitor_ipc__get_path (the_repository), + &options, &connection);switch (state) {case IPC_STATE__LISTENING :
@@ -117,13 +115,13 @@ try_again:case IPC_STATE__INVALID_PATH : ret =error (_ ("fsmonitor_ipc__send_query: invalid path '%s'" ), -fsmonitor_ipc__get_path ()); +fsmonitor_ipc__get_path (the_repository)); goto done;case IPC_STATE__OTHER_ERROR :default : ret =error (_ ("fsmonitor_ipc__send_query: unspecified error on '%s'" ), -fsmonitor_ipc__get_path ()); +fsmonitor_ipc__get_path (the_repository)); goto done; }
@@ -149,8 +147,8 @@ int fsmonitor_ipc__send_command(const char *command, options.wait_if_busy =1 ; options.wait_if_not_found =0 ; - state =ipc_client_try_connect (fsmonitor_ipc__get_path (), &options, - &connection); + state =ipc_client_try_connect (fsmonitor_ipc__get_path (the_repository), + &options, &connection);if (state !=IPC_STATE__LISTENING ) {die (_ ("fsmonitor--daemon is not running" ));return -1 ;
fsmonitor-ipc.h +3 -1
@@ -3,6 +3,8 @@#include "simple-ipc.h" +struct repository ; +/* * Returns true if built-in file system monitor daemon is defined * for this platform.
@@ -16,7 +18,7 @@ int fsmonitor_ipc__is_supported(void);* * Returns NULL if the daemon is not supported on this platform. */ -const char *fsmonitor_ipc__get_path (void ); +const char *fsmonitor_ipc__get_path (struct repository *r);/* * Try to determine whether there is a `git-fsmonitor--daemon` process
Signature · 1 line
gitgitgadget
PATCH v12 3/6 fsmonitor: avoid socket location check if using hook
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
If monitoring is done via fsmonitor hook rather than IPC there is no need to check if the location of the Unix Domain socket (UDS) file is on a remote filesystem.
--- compat/fsmonitor/fsm-settings-darwin.c | 10 ++++++---- compat/fsmonitor/fsm-settings-win32.c | 2 +- fsmonitor-settings.c | 8 ++++---- fsmonitor-settings.h | 2 +- 4 files changed, 12 insertions(+), 10 deletions(-)
compat/fsmonitor/fsm-settings-darwin.c +6 -4
@@ -49,13 +49,15 @@ static enum fsmonitor_reason check_uds_volume(struct repository *r)return FSMONITOR_REASON_OK ; } -enum fsmonitor_reason fsm_os__incompatible (struct repository *r) +enum fsmonitor_reason fsm_os__incompatible (struct repository *r,int ipc) {enum fsmonitor_reason reason; - reason =check_uds_volume (r); -if (reason !=FSMONITOR_REASON_OK ) -return reason; +if (ipc) { + reason =check_uds_volume (r); +if (reason !=FSMONITOR_REASON_OK ) +return reason; + }return FSMONITOR_REASON_OK ; }
compat/fsmonitor/fsm-settings-win32.c +1 -1
@@ -25,7 +25,7 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)return FSMONITOR_REASON_OK ; } -enum fsmonitor_reason fsm_os__incompatible (struct repository *r) +enum fsmonitor_reason fsm_os__incompatible (struct repository *r,int ipc) {enum fsmonitor_reason reason;
fsmonitor-settings.c +4 -4
@@ -60,7 +60,7 @@ static enum fsmonitor_reason check_remote(struct repository *r) }#endif -static enum fsmonitor_reason check_for_incompatible (struct repository *r) +static enum fsmonitor_reason check_for_incompatible (struct repository *r,int ipc) {if (!r->worktree) {/*
@@ -77,7 +77,7 @@ static enum fsmonitor_reason check_for_incompatible(struct repository *r) reason =check_remote (r);if (reason !=FSMONITOR_REASON_OK )return reason; - reason =fsm_os__incompatible (r); + reason =fsm_os__incompatible (r, ipc);if (reason !=FSMONITOR_REASON_OK )return reason; }
@@ -162,7 +162,7 @@ const char *fsm_settings__get_hook_path(struct repository *r)void fsm_settings__set_ipc (struct repository *r) { -enum fsmonitor_reason reason =check_for_incompatible (r); +enum fsmonitor_reason reason =check_for_incompatible (r,1 );if (reason !=FSMONITOR_REASON_OK ) {fsm_settings__set_incompatible (r, reason);
@@ -185,7 +185,7 @@ void fsm_settings__set_ipc(struct repository *r)void fsm_settings__set_hook (struct repository *r,const char *path) { -enum fsmonitor_reason reason =check_for_incompatible (r); +enum fsmonitor_reason reason =check_for_incompatible (r,0 );if (reason !=FSMONITOR_REASON_OK ) {fsm_settings__set_incompatible (r, reason);
fsmonitor-settings.h +1 -1
@@ -48,7 +48,7 @@ struct fsmonitor_settings;* fsm_os__* routines should considered private to fsm_settings__ * routines. */ -enum fsmonitor_reason fsm_os__incompatible (struct repository *r); +enum fsmonitor_reason fsm_os__incompatible (struct repository *r,int ipc);#endif /* HAVE_FSMONITOR_OS_SETTINGS */ #endif /* FSMONITOR_SETTINGS_H */
Signature · 1 line
gitgitgadget
PATCH v12 4/6 fsmonitor: deal with synthetic firmlinks on macOS
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
Starting with macOS 10.15 (Catalina), Apple introduced a new feature called 'firmlinks' in order to separate the boot volume into two volumes, one read-only and one writable but still present them to the user as a single volume. Along with this change, Apple removed the ability to create symlinks in the root directory and replaced them with 'synthetic firmlinks'. See 'man synthetic.conf'
When FSEevents reports the path of changed files, if the path involves a synthetic firmlink, the path is reported from the point of the synthetic firmlink and not the real path. For example:
Real path:
/System/Volumes/Data/network/working/directory/foo.txt
Synthetic firmlink:
/network -> /System/Volumes/Data/network
FSEvents path:
/network/working/directory/foo.txt
This causes the FSEvents path to not match against the worktree directory.
There are several ways in which synthetic firmlinks can be created:
they can be defined in /etc/synthetic.conf, the automounter can create them, and there may be other means. Simply reading /etc/synthetic.conf is insufficient. No matter what process creates synthetic firmlinks, they all get created in the root directory.
Therefore, in order to deal with synthetic firmlinks, the root directory is scanned and the first possible synthetic firmink that, when resolved, is a prefix of the worktree is used to map FSEvents paths to worktree paths.
--- builtin/fsmonitor--daemon.c | 8 +++ compat/fsmonitor/fsm-listen-darwin.c | 14 +++- compat/fsmonitor/fsm-path-utils-darwin.c | 92 ++++++++++++++++++++++++ compat/fsmonitor/fsm-path-utils-win32.c | 17 +++++ fsmonitor--daemon.h | 3 + fsmonitor-path-utils.h | 36 ++++++++++ 6 files changed, 168 insertions(+), 2 deletions(-)
builtin/fsmonitor--daemon.c +8 -0
@@ -3,6 +3,7 @@#include "parse-options.h" #include "fsmonitor.h" #include "fsmonitor-ipc.h" +#include "fsmonitor-path-utils.h" #include "compat/fsmonitor/fsm-health.h" #include "compat/fsmonitor/fsm-listen.h" #include "fsmonitor--daemon.h"
@@ -1282,6 +1283,13 @@ static int fsmonitor_run_daemon(void)strbuf_addstr (&state.path_worktree_watch,absolute_path (get_git_work_tree ())); state.nr_paths_watching =1 ; + state.alias.alias =NULL ; + state.alias.points_to =NULL ; +if (fsmonitor__get_alias (state.path_worktree_watch.buf, &state.alias)) { + err =error (_ ("could not get worktree alias" )); + goto done; + } +/* * We create and delete cookie files somewhere inside the .git * directory to help us keep sync with the file system. If
compat/fsmonitor/fsm-listen-darwin.c +12 -2
@@ -26,6 +26,7 @@#include "fsmonitor.h" #include "fsm-listen.h" #include "fsmonitor--daemon.h" +#include "fsmonitor-path-utils.h" struct fsm_listen_data {
@@ -198,8 +199,9 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,struct string_list cookie_list =STRING_LIST_INIT_DUP ;const char *path_k;const char *slash; -int k; +char *resolved =NULL ;struct strbuf tmp =STRBUF_INIT ; +int k;/* * Build a list of all filesystem changes into a private/local
@@ -209,7 +211,12 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,/* * On Mac, we receive an array of absolute paths. */ - path_k = paths[k]; +free (resolved); + resolved =fsmonitor__resolve_alias (paths[k], &state->alias); +if (resolved) + path_k = resolved; +else + path_k = paths[k];/* * If you want to debug FSEvents, log them to GIT_TRACE_FSMONITOR.
@@ -238,6 +245,7 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,fsmonitor_force_resync (state);fsmonitor_batch__free_list (batch);string_list_clear (&cookie_list,0 ); + batch =NULL ;/* * We assume that any events that we received
@@ -360,12 +368,14 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef, } } +free (resolved);fsmonitor_publish (state, batch, &cookie_list);string_list_clear (&cookie_list,0 );strbuf_release (&tmp);return ; force_shutdown: +free (resolved);fsmonitor_batch__free_list (batch);string_list_clear (&cookie_list,0 );
compat/fsmonitor/fsm-path-utils-darwin.c +92 -0
100 lines not on this page · raw message
compat/fsmonitor/fsm-path-utils-win32.c +17 -0
@@ -126,3 +126,20 @@ int fsmonitor__is_fs_remote(const char *path)return -1 ;return fs.is_remote; } + +/* +* No-op for now. +*/ +int fsmonitor__get_alias (const char *path,struct alias_info *info) +{ +return 0 ; +} + +/* +* No-op for now. +*/ +char *fsmonitor__resolve_alias (const char *path, +const struct alias_info *info) +{ +return NULL ; +}
fsmonitor--daemon.h +3 -0
@@ -8,6 +8,7 @@#include "run-command.h" #include "simple-ipc.h" #include "thread-utils.h" +#include "fsmonitor-path-utils.h" struct fsmonitor_batch ;struct fsmonitor_token_data ;
@@ -43,6 +44,7 @@ struct fsmonitor_daemon_state {struct strbuf path_worktree_watch;struct strbuf path_gitdir_watch; +struct alias_info alias;int nr_paths_watching;struct fsmonitor_token_data *current_token_data;
@@ -59,6 +61,7 @@ struct fsmonitor_daemon_state {struct ipc_server_data *ipc_server_data;struct strbuf path_ipc; + };/*
fsmonitor-path-utils.h +36 -0
@@ -1,6 +1,14 @@ #ifndef FSM_PATH_UTILS_H #define FSM_PATH_UTILS_H +#include "strbuf.h" + +struct alias_info +{ + char *alias; + char *points_to; +}; + struct fs_info { int is_remote; char *typename;
@@ -20,4 +28,32 @@ int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info);*/ int fsmonitor__is_fs_remote (const char *path); +/* +* Get the alias in given path, if any. +* +* Sets alias to the first alias that matches any part of the path. +* +* If an alias is found, info.alias and info.points_to are set to the +* found mapping. +* +* Returns -1 on error, 0 otherwise. +* +* The caller owns the storage that is occupied by set info.alias and +* info.points_to and is responsible for releasing it with `free(3)` +* when done. +*/ +int fsmonitor__get_alias (const char *path,struct alias_info *info); + +/* +* Resolve the path against the given alias. +* +* Returns the resolved path if there is one, NULL otherwise. +* +* The caller owns the storage that the returned string occupies and +* is responsible for releasing it with `free(3)` when done. +*/ +char *fsmonitor__resolve_alias (const char *path, +const struct alias_info *info); + +#endif
Signature · 1 line
gitgitgadget
Re: [PATCH v12 4/6] fsmonitor: deal with synthetic firmlinks on macOS
builtin/fsmonitor--daemon.c static int fsmonitor_run_daemon(void) · line 1291
Quoted 8 lines · Eric DeCosta via GitGitGadget
From: Eric DeCosta <[email protected]>
[...]builtin/fsmonitor--daemon.c +6
-0+ state.alias.alias =NULL ; + state.alias.points_to =NULL ; +if (fsmonitor__get_alias (state.path_worktree_watch.buf, &state.alias)) { + err =error (_ ("could not get worktree alias" )); + goto done; + }
As we can see here this is in the one-off setup code...
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 68
Quoted 8 lines
compat/fsmonitor/fsm-path-utils-darwin.c +8
-0+int fsmonitor__get_alias (const char *path,struct alias_info *info) +{ +DIR * dir; +int read; +int retval; +struct dirent *de; +struct strbuf alias; +struct strbuf points_to;
...more of a code clarity comment than anything, else, but...
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 76
Quoted 8 lines
compat/fsmonitor/fsm-path-utils-darwin.c +8
-0+ + retval =0 ; + dir =opendir ("/" ); +if (!dir) +return -1 ; + +strbuf_init (&alias,256 ); +strbuf_init (&points_to,MAXPATHLEN );
...can't we just use the STRBUF_INIT macro here instead? most paths are nowhere near MAXPATHLEN, but more importantly we try to avoid these sorts of memory micro-managements except for hot codepaths.
In this case it's just the one-off setup of fsmonitor, isn't it? So just using the default allocation pattern seems worthwhile, and will save e.g. anyone grepping for MAXPATHLEN looking for bugs (the MAXPATHLEN is sometimes not the actual maximum pathlen).
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 86
Quoted 10 lines
compat/fsmonitor/fsm-path-utils-darwin.c +10
-0+ +while ((de =readdir (dir)) !=NULL ) { +strbuf_reset (&alias); +strbuf_addch (&alias,'/' ); +strbuf_add (&alias, de->d_name,strlen (de->d_name)); + + read =readlink (alias.buf, points_to.buf,MAXPATHLEN ); +if (read >0 ) { +strbuf_setlen (&points_to, read); + if ((strncmp(points_to.buf, path, points_to.len) == 0)
We usually do (!strcmp()), not strcmp() == 0, ditto strncmp. See CodingGuidelines.
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 103
Quoted 1 line
compat/fsmonitor/fsm-path-utils-darwin.c +1
-0+ done:
Nit: labels shouldn't be indented.
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 104
Quoted 1 line
compat/fsmonitor/fsm-path-utils-darwin.c +1
-0+closedir (dir);
We checked the opendir() return value, why not closedir() too?
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 118
Quoted 1 line
compat/fsmonitor/fsm-path-utils-darwin.c +1
-0+ if ((strncmp(info->alias, path, len) == 0)
ditto !foo() v.s. foo() == 0.
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 123
Quoted 5 lines
compat/fsmonitor/fsm-path-utils-darwin.c +5
-0+ && path[len] == '/') { +struct strbuf tmp; +const char *remainder = path + len; +int ptr_len =strlen (info->points_to); +int rem_len =strlen (remainder);
Make these s/int/size_t/.
Review
Quoted 2 lines
(quoted diff) +2
-0+ + strbuf_init(&tmp, ptr_len + rem_len);
And use st_add() here instead of " + ". I don't think it'll overflow, but it's good to guard overflows out of habit...
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 126
Quoted 1 line
compat/fsmonitor/fsm-path-utils-darwin.c +1
-0+strbuf_add (&tmp, info->points_to, ptr_len);
Earlier you constructed a strbuf, and then strbuf_detached() it into this new "struct alias_info" you made. And now we're having to strlen() that to get the lenght that we knew earlier?
Can't we just make the member a "struct strbuf" instead? Maybe not, I have not reviewed that aspect carefully...
RE: [PATCH v12 4/6] fsmonitor: deal with synthetic firmlinks on macOS
Quoted 51 lines
-----Original Message-----
From: Ævar Arnfjörð Bjarmason <[email protected]>
Sent: Monday, September 26, 2022 11:16 AM
To: Eric DeCosta via GitGitGadget <[email protected]>
Cc: [email protected]; Jeff Hostetler <[email protected]>; Eric Sunshine <[email protected]>; Torsten Bögershausen <[email protected]>; Ramsay Jones <[email protected]>; Johannes Schindelin <[email protected]>; Eric DeCosta <[email protected]>
Subject: Re: [PATCH v12 4/6] fsmonitor: deal with synthetic firmlinks on macOSbuiltin/fsmonitor--daemon.c static int fsmonitor_run_daemon(void)
Quoted 4 lines · Eric DeCosta via GitGitGadget
From: Eric DeCosta <[email protected]> [...]
builtin/fsmonitor--daemon.c +3
-0+ state.alias.alias =NULL ; + state.alias.points_to =NULL ; + if (fsmonitor__get_alias(state.path_worktree_watch.buf,&state.alias)) {
builtin/fsmonitor--daemon.c static int fsmonitor_run_daemon(void) · line 1291
Quoted 3 lines
builtin/fsmonitor--daemon.c +3
-0+ err =error (_ ("could not get worktree alias" )); + goto done; + }As we can see here this is in the one-off setup code...
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 68
Quoted 7 lines
compat/fsmonitor/fsm-path-utils-darwin.c +7
-0+int fsmonitor__get_alias (const char *path,struct alias_info *info) { +DIR * dir; +int read; +int retval; +struct dirent *de; +struct strbuf alias; +struct strbuf points_to;...more of a code clarity comment than anything, else, but...
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 76
Quoted 8 lines
compat/fsmonitor/fsm-path-utils-darwin.c +8
-0+ + retval =0 ; + dir =opendir ("/" ); +if (!dir) +return -1 ; + +strbuf_init (&alias,256 ); +strbuf_init (&points_to,MAXPATHLEN );...can't we just use the
STRBUF_INITmacro here instead? most paths are nowhere near MAXPATHLEN, but more importantly we try to avoid these sorts of memory micro-managements except for hot codepaths.
Quoted 5 lines
In this case it's just the one-off setup of fsmonitor, isn't it? So just using the default allocation pattern seems worthwhile, and will save e.g. anyone grepping for MAXPATHLEN looking for bugs (the MAXPATHLEN is sometimes not the actual maximum pathlen).
OK, makes sense.
Quoted 13 lines
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 86
Quoted 10 lines
compat/fsmonitor/fsm-path-utils-darwin.c +10
-0+ +while ((de =readdir (dir)) !=NULL ) { +strbuf_reset (&alias); +strbuf_addch (&alias,'/' ); +strbuf_add (&alias, de->d_name,strlen (de->d_name)); + + read =readlink (alias.buf, points_to.buf,MAXPATHLEN ); +if (read >0 ) { +strbuf_setlen (&points_to, read); + if ((strncmp(points_to.buf, path, points_to.len) == 0)We usually do (!strcmp()), not
strcmp()== 0, ditto strncmp. See CodingGuidelines.
Quoted 2 lines
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 103
Quoted 1 line
compat/fsmonitor/fsm-path-utils-darwin.c +1
-0+ done:
Fixed.
Quoted 2 lines
Nit: labels shouldn't be indented.
Fixed
Quoted 4 lines
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 104
Quoted 1 line
compat/fsmonitor/fsm-path-utils-darwin.c +1
-0+closedir (dir);We checked the
opendir()return value, why notclosedir()too?
OK, will do.
Quoted 12 lines
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 118
Quoted 1 line
compat/fsmonitor/fsm-path-utils-darwin.c +1
-0+ if ((strncmp(info->alias, path, len) == 0)ditto !foo() v.s.
foo()== 0.compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 123
Quoted 5 lines
compat/fsmonitor/fsm-path-utils-darwin.c +5
-0+ && path[len] == '/') { +struct strbuf tmp; +const char *remainder = path + len; +int ptr_len =strlen (info->points_to); +int rem_len =strlen (remainder);Make these s/int/size_t/.
Quoted 6 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ + strbuf_init(&tmp, ptr_len + rem_len);And use
st_add()here instead of " + ". I don't think it'll overflow, but it's good to guard overflows out of habit...
Sure, just strbuf_add(). Then I don't need ptr_len or rem_len either.
Quoted 9 lines
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 126
Quoted 1 line
compat/fsmonitor/fsm-path-utils-darwin.c +1
-0+strbuf_add (&tmp, info->points_to, ptr_len);Earlier you constructed a strbuf, and then
strbuf_detached()it into this new "structalias_info" you made. And now we're having tostrlen()that to get the lenght that we knew earlier?Can't we just make the member a "struct strbuf" instead? Maybe not, I have not reviewed that aspect carefully...
Certainly could do that.
-Eric
Re: [PATCH v12 4/6] fsmonitor: deal with synthetic firmlinks on macOS
In reply to Eric DeCosta via GitGitGadgetQuoted 1 line · Eric DeCosta via GitGitGadget
From: Eric DeCosta <[email protected]>
..one aspect I missed...
builtin/fsmonitor--daemon.c static int fsmonitor_run_daemon(void) · line 1290
Quoted 5 lines
builtin/fsmonitor--daemon.c +5
-0+ state.alias.alias =NULL ; + state.alias.points_to =NULL ; +if (fsmonitor__get_alias (state.path_worktree_watch.buf, &state.alias)) { + err =error (_ ("could not get worktree alias" )); + goto done;
Okey, it errored and we call error() to say it didn't work, good so far, but...
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 65
Quoted 5 lines
compat/fsmonitor/fsm-path-utils-darwin.c +5
-0+int fsmonitor__get_alias (const char *path,struct alias_info *info) +{ +DIR * dir; +int read; +int retval;
...we could just...
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 70
Quoted 5 lines
compat/fsmonitor/fsm-path-utils-darwin.c +5
-0+struct dirent *de; +struct strbuf alias; +struct strbuf points_to; + + retval =0 ;
...have initialized that above if we do it unconditionally, but more on this below...
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 73
Quoted 3 lines
compat/fsmonitor/fsm-path-utils-darwin.c +3
-0+ dir =opendir ("/" ); +if (!dir) +return -1 ;
Here in the actual implementation, which looking at the end-state we *only* end up calling from that one caller we could have called error_errno() to get a better message, but didn't.
I think much better would be to skip that above entirely, or keep it you want two errors, but then just have the more meaningful error_errno() here, where we're closer to the error, and can report a better one.
Of course we might sometimes have a good error, and sometimes a bad one, but...(continued below)
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 83
Quoted 10 lines
compat/fsmonitor/fsm-path-utils-darwin.c +10
-0+ +strbuf_init (&alias,256 ); +strbuf_init (&points_to,MAXPATHLEN ); + +while ((de =readdir (dir)) !=NULL ) { +strbuf_reset (&alias); +strbuf_addch (&alias,'/' ); +strbuf_add (&alias, de->d_name,strlen (de->d_name)); + + read =readlink (alias.buf, points_to.buf,MAXPATHLEN );
I think a:
if (!read)
BUG("got 0 from readlink?");Or something would be a good paranoia addition, as you're technically relying on...
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 96
Quoted 13 lines
compat/fsmonitor/fsm-path-utils-darwin.c +13
-0+if (read >0 ) { +strbuf_setlen (&points_to, read); +if ((strncmp (points_to.buf, path, points_to.len) ==0 ) + && path[points_to.len] =='/' ) { + info->alias =strbuf_detach (&alias,NULL ); + info->points_to =strbuf_detach (&points_to,NULL ); +trace_printf_key (&trace_fsmonitor, +"Found alias for '%s' : '%s' -> '%s'" , + path, info->alias, info->points_to); + retval =0 ; + goto done; + } + }else if (errno !=EINVAL ) {/* Something other than not a link */
...the possibility that we return 0 but a stale errno happens to be set, I don't think it'll happen in practice and that it always returned -1 if we get here, but being strict with calling syscalls is generally good.
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 97
Quoted 1 line
compat/fsmonitor/fsm-path-utils-darwin.c +1
-0+trace_printf_key (&trace_fsmonitor,"Error %s" ,strerror (errno));
(continued from above)..here we see the only codepath that sets retval != 0,
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 98
Quoted 1 line
compat/fsmonitor/fsm-path-utils-darwin.c +1
-0+ retval =-1 ;
Here we could have just called error_errno() instead.
Quoted 2 lines
+ * The caller owns the storage that the returned string occupies and
+ * is responsible for releasing it withfree(3)when done.
nit: we could just put a full stop after "it" and skip the rest. I.e. trust that the reader knows that allocated memory is freed with free().
fsmonitor-path-utils.h int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info); · line 58
Quoted 5 lines
+ */
fsmonitor-path-utils.h +4
-0+char *fsmonitor__resolve_alias (const char *path, +const struct alias_info *info); + +
nit: extra whitespace at end of file.
Quoted 1 line
#endif
PATCH v12 5/6 fsmonitor: check for compatability before communicating with fsmonitor
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
If fsmonitor is not in a compatible state, die with an appropriate error messge.
--- compat/fsmonitor/fsm-settings-darwin.c | 2 +- fsmonitor-settings.c | 10 +++++++--- fsmonitor-settings.h | 2 +- fsmonitor.c | 7 +++++++ 4 files changed, 16 insertions(+), 5 deletions(-)
compat/fsmonitor/fsm-settings-darwin.c +1 -1
@@ -38,7 +38,7 @@ static enum fsmonitor_reason check_uds_volume(struct repository *r)strbuf_release (&path);if (fs.is_remote) -return FSMONITOR_REASON_REMOTE ; +return FSMONITOR_REASON_NOSOCKETS ;if (!strcmp (fs.typename,"msdos" ))/* aka FAT32 */ return FSMONITOR_REASON_NOSOCKETS ;
fsmonitor-settings.c +7 -3
@@ -1,6 +1,7 @@#include "cache.h" #include "config.h" #include "repository.h" +#include "fsmonitor-ipc.h" #include "fsmonitor-settings.h" #include "fsmonitor-path-utils.h"
@@ -242,10 +243,11 @@ enum fsmonitor_reason fsm_settings__get_reason(struct repository *r)return r->settings.fsmonitor->reason; } -char *fsm_settings__get_incompatible_msg (const struct repository *r, +char *fsm_settings__get_incompatible_msg (struct repository *r,enum fsmonitor_reason reason) {struct strbuf msg =STRBUF_INIT ; +const char *socket_dir;switch (reason) {case FSMONITOR_REASON_UNTESTED :
@@ -281,9 +283,11 @@ char *fsm_settings__get_incompatible_msg(const struct repository *r, goto done;case FSMONITOR_REASON_NOSOCKETS : + socket_dir =dirname ((char *)fsmonitor_ipc__get_path (r));strbuf_addf (&msg, -_ ("repository '%s' is incompatible with fsmonitor due to lack of Unix sockets" ), - r->worktree); +_ ("socket directory '%s' is incompatible with fsmonitor due" +" to lack of Unix sockets support" ), + socket_dir); goto done; }
fsmonitor-settings.h +1 -1
@@ -33,7 +33,7 @@ enum fsmonitor_mode fsm_settings__get_mode(struct repository *r);const char *fsm_settings__get_hook_path (struct repository *r);enum fsmonitor_reason fsm_settings__get_reason (struct repository *r); -char *fsm_settings__get_incompatible_msg (const struct repository *r, +char *fsm_settings__get_incompatible_msg (struct repository *r,enum fsmonitor_reason reason);struct fsmonitor_settings ;
fsmonitor.c +7 -0
@@ -295,6 +295,7 @@ static int fsmonitor_force_update_threshold = 100;void refresh_fsmonitor (struct index_state *istate) { +static int warn_once =0 ;struct strbuf query_result =STRBUF_INIT ;int query_success =0 , hook_version =-1 ;size_t bol =0 ;/* beginning of line */
@@ -305,6 +306,12 @@ void refresh_fsmonitor(struct index_state *istate)int is_trivial =0 ;struct repository *r = istate->repo ? istate->repo : the_repository;enum fsmonitor_mode fsm_mode =fsm_settings__get_mode (r); +enum fsmonitor_reason reason =fsm_settings__get_reason (r); + +if (!warn_once && reason >FSMONITOR_REASON_OK ) { + warn_once =1 ; +warning ("%s" ,fsm_settings__get_incompatible_msg (r, reason)); + }if (fsm_mode <=FSMONITOR_MODE_DISABLED || istate->fsmonitor_has_run_once)
Signature · 1 line
gitgitgadget
RE: [PATCH v12 5/6] fsmonitor: check for compatability before communicating with fsmonitor
Quoted 15 lines
-----Original Message-----
From: Eric DeCosta via GitGitGadget <[email protected]>
Sent: Saturday, September 24, 2022 3:46 PM
To: [email protected]
Cc: Jeff Hostetler <[email protected]>; Eric Sunshine <[email protected]>; Torsten Bögershausen <[email protected]>; Ævar Arnfjörð Bjarmason <[email protected]>; Ramsay Jones <[email protected]>; Johannes Schindelin <[email protected]>; Eric DeCosta <[email protected]>; Eric DeCosta <[email protected]>
Subject: [PATCH v12 5/6] fsmonitor: check for compatability before communicating with fsmonitor
Quoted 7 lines
From: Eric DeCosta <[email protected]>
If fsmonitor is not in a compatible state, die with an appropriate error messge.
Signed-off-by Eric DeCosta <[email protected]>---
Grr. Should be "warn with an appropriate error message".
-Eric
Quoted 105 lines
compat/fsmonitor/fsm-settings-darwin.c | 2 +- fsmonitor-settings.c | 10 +++++++--- fsmonitor-settings.h | 2 +- fsmonitor.c | 7 +++++++ 4 files changed, 16 insertions(+), 5 deletions(-)compat/fsmonitor/fsm-settings-darwin.c +1
-1@@ -38,7 +38,7 @@ static enum fsmonitor_reason check_uds_volume(struct repository *r) strbuf_release(&path);if (fs.is_remote) -return FSMONITOR_REASON_REMOTE ; +return FSMONITOR_REASON_NOSOCKETS ;if (!strcmp (fs.typename,"msdos" ))/* aka FAT32 */ compat/fsmonitor/fsm-settings-darwin.c +0
-0return FSMONITOR_REASON_NOSOCKETS ;fsmonitor-settings.c index renamed from fsmonitor-settings.c +0
-0
531a1b6f956..8592a4d9bad100644fsmonitor-settings.c +3
-1@@ -1,6 +1,7 @@#include "cache.h" #include "config.h" #include "repository.h" +#include "fsmonitor-ipc.h" #include "fsmonitor-settings.h" #include "fsmonitor-path-utils.h" @@ -242,10 +243,11 @@ enum fsmonitor_reason fsm_settings__get_reason(struct repository *r) return r->settings.fsmonitor->reason; } -char *fsm_settings__get_incompatible_msg (const struct repository *r, +char *fsm_settings__get_incompatible_msg (struct repository *r,enum fsmonitor_reason reason) {struct strbuf msg =STRBUF_INIT ; +const char *socket_dir;switch (reason) {fsmonitor-settings.c +2
-2case FSMONITOR_REASON_UNTESTED :@@ -281,9 +283,11 @@ char *fsm_settings__get_incompatible_msg(conststruct repository *r, goto done;case FSMONITOR_REASON_NOSOCKETS : + socket_dir =dirname ((char *)fsmonitor_ipc__get_path (r)); strbuf_addf(&msg, - _("repository '%s' is incompatible with fsmonitor due to lack of Unix sockets"), - r->worktree); + _("socket directory '%s' is incompatible withfsmonitor due"
fsmonitor-settings.c +2
-0+ " to lack of Unix sockets support"), + socket_dir); goto done; }fsmonitor-settings.h index renamed from fsmonitor-settings.h +0
-0
0721617b95a..ab02e3995ee100644fsmonitor-settings.h +0
-0@@ -33,7 +33,7 @@ enum fsmonitor_mode fsm_settings__get_mode(structrepository *r); const char *fsm_settings__get_hook_path(struct repository *r);
enum
fsmonitor_reasonfsm_settings__get_reason(struct repository *r); -
char *fsm_settings__get_incompatible_msg(const struct repository *r,fsmonitor-settings.h +1
-0+char *fsm_settings__get_incompatible_msg (struct repository *r,enum fsmonitor_reason reason);struct fsmonitor_settings ;fsmonitor.c +1
-0@@ -295,6 +295,7 @@ static int fsmonitor_force_update_threshold = 100;void refresh_fsmonitor (struct index_state *istate) { +static int warn_once =0 ;struct strbuf query_result =STRBUF_INIT ;int query_success =0 , hook_version =-1 ; size_t bol = 0; /* beginning of line */ @@ -305,6 +306,12 @@ voidrefresh_fsmonitor (struct index_state *istate)fsmonitor.c +5
-0int is_trivial =0 ;struct repository *r = istate->repo ? istate->repo : the_repository;enum fsmonitor_mode fsm_mode =fsm_settings__get_mode (r); +enum fsmonitor_reason reason =fsm_settings__get_reason (r); + +if (!warn_once && reason >FSMONITOR_REASON_OK ) { + warn_once =1 ; + warning("%s", fsm_settings__get_incompatible_msg(r,reason));
fsmonitor.c +1
-1+ }if (fsm_mode <=FSMONITOR_MODE_DISABLED || istate->fsmonitor_has_run_once) --gitgitgadget
Re: [PATCH v12 5/6] fsmonitor: check for compatability before communicating with fsmonitor
In reply to Eric DeCosta via GitGitGadgetfsmonitor-settings.c char *fsm_settings__get_incompatible_msg(const struct repository *r, · line 290
Quoted 13 lines · Eric DeCosta via GitGitGadget
From: Eric DeCosta <[email protected]>
[...]fsmonitor-settings.c +4
-2@@ -281,9 +283,11 @@ char *fsm_settings__get_incompatible_msg(const struct repository *r, goto done;case FSMONITOR_REASON_NOSOCKETS : + socket_dir =dirname ((char *)fsmonitor_ipc__get_path (r));strbuf_addf (&msg, -_ ("repository '%s' is incompatible with fsmonitor due to lack of Unix sockets" ), - r->worktree); +_ ("socket directory '%s' is incompatible with fsmonitor due" +" to lack of Unix sockets support" ), + socket_dir);
Could do with less "while at it" here. We are:
* Wrapping the string, making the functional change(s) harder to spot.
* replacing r->worktree with socket_dir
* Adding " support" to the end of the string, and replacing "repository" with "socket directory"
AFAICT the continuation of the string isn't indented in the way we usually do, i.e. to align with the opening ".
RE: [PATCH v12 5/6] fsmonitor: check for compatability before communicating with fsmonitor
Quoted 34 lines
-----Original Message-----
From: Ævar Arnfjörð Bjarmason <[email protected]>
Sent: Monday, September 26, 2022 11:24 AM
To: Eric DeCosta via GitGitGadget <[email protected]>
Cc: [email protected]; Jeff Hostetler <[email protected]>; Eric Sunshine <[email protected]>; Torsten Bögershausen <[email protected]>; Ramsay Jones <[email protected]>; Johannes Schindelin <[email protected]>; Eric DeCosta <[email protected]>
Subject: Re: [PATCH v12 5/6] fsmonitor: check for compatability before communicating with fsmonitorQuoted 1 line · Eric DeCosta via GitGitGadget
From: Eric DeCosta <[email protected]> [...] @@ -281,9 +283,11
@@
fsmonitor-settings.c char *fsm_settings__get_incompatible_msg(const struct repository *r,
Quoted 7 lines
char *fsm_settings__get_incompatible_msg(const struct repository *r,
fsmonitor-settings.c +1
-1goto done;case FSMONITOR_REASON_NOSOCKETS : + socket_dir =dirname ((char *)fsmonitor_ipc__get_path (r)); strbuf_addf(&msg, - _("repository '%s' is incompatible with fsmonitordue to lack of Unix sockets"),
Review
Quoted 2 lines
(quoted diff) +1
-1- r->worktree); + _("socket directory '%s' is incompatible withfsmonitor due"
fsmonitor-settings.c char *fsm_settings__get_incompatible_msg(const struct repository *r, · line 290
Quoted 2 lines
fsmonitor-settings.c +2
-0+ " to lack of Unix sockets support"), + socket_dir);Could do with less "while at it" here. We are:
Quoted 7 lines
* Wrapping the string, making the functional change(s) harder to spot.
* replacingr->worktreewithsocket_dir
* Adding " support" to the end of the string, and replacing "repository" with
"socket directory"AFAICT the continuation of the string isn't indented in the way we usually do, i.e. to align with the opening ".
The string, when properly indented, exceeds an 80 character line length. I'll fix the indentation, but I don't think there's a much better alternative to the wrapping.
The worktree could be in a perfectly fine location whereas the socket_dir may not . Crafting the error message the way I did reflects where the problem is rather than reporting a potentially misleading error about the repository.
-Eric
PATCH v12 6/6 fsmonitor: add documentation for allowRemote and socketDir options
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
Add documentation for 'fsmonitor.allowRemote' and 'fsmonitor.socketDir'. Call-out experimental nature of 'fsmonitor.allowRemote' and limited file system support for 'fsmonitor.socketDir'.
--- Documentation/git-fsmonitor--daemon.txt | 48 +++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-)
Documentation/git-fsmonitor--daemon.txt +45 -3
@@ -3,7 +3,7 @@ git-fsmonitor{litdd}daemon(1) NAME ---- -git-fsmonitor--daemon - A Built-in File System Monitor +git-fsmonitor--daemon - A Built-in Filesystem Monitor SYNOPSIS --------
@@ -17,7 +17,7 @@ DESCRIPTION ----------- A daemon to watch the working directory for file and directory -changes using platform-specific file system notification facilities. +changes using platform-specific filesystem notification facilities. This daemon communicates directly with commands like `git status` using the link:technical/api-simple-ipc.html[simple IPC] interface
@@ -63,13 +63,55 @@ CAVEATS ------- The fsmonitor daemon does not currently know about submodules and does -not know to filter out file system events that happen within a +not know to filter out filesystem events that happen within a submodule. If fsmonitor daemon is watching a super repo and a file is modified within the working directory of a submodule, it will report the change (as happening against the super repo). However, the client will properly ignore these extra events, so performance may be affected but it will not cause an incorrect result. +By default, the fsmonitor daemon refuses to work against network-mounted +repositories; this may be overridden by setting `fsmonitor.allowRemote` to +`true`. Note, however, that the fsmonitor daemon is not guaranteed to work +correctly with all network-mounted repositories and such use is considered +experimental. + +On Mac OS, the inter-process communication (IPC) between various Git +commands and the fsmonitor daemon is done via a Unix domain socket (UDS) -- a +special type of file -- which is supported by native Mac OS filesystems, +but not on network-mounted filesystems, NTFS, or FAT32. Other filesystems +may or may not have the needed support; the fsmonitor daemon is not guaranteed +to work with these filesystems and such use is considered experimental. + +By default, the socket is created in the `.git` directory, however, if the +`.git` directory is on a network-mounted filesystem, it will be instead be +created at `$HOME/.git-fsmonitor-*` unless `$HOME` itself is on a +network-mounted filesystem in which case you must set the configuration +variable `fsmonitor.socketDir` to the path of a directory on a Mac OS native +filesystem in which to create the socket file. + +If none of the above directories (`.git`, `$HOME`, or `fsmonitor.socketDir`) +is on a native Mac OS file filesystem the fsmonitor daemon will report an +error that will cause the daemon and the currently running command to exit. + +CONFIGURATION +------------- + +When `core.fsmonitor` is set to `true` (see linkgit:git-config[1]) +the fsmonitor daemon will pay attention to the following configuration +variables: + +`fsmonitor.allowRemote`:: + By default, the daemon refuses to work against network-mounted + repositories. Setting `fsmonitor.allowRemote` to `true` overrides + this behavior. + +`fsmonitor.socketDir`:: + This Mac OS-specific option, if set, specifies the directory in + which to create the Unix domain socket used for communication + between fsmonitor and various Git commands. The directory must + reside on a native Mac OS filesystem as discussed above. + GIT --- Part of the linkgit:git[1] suite
Signature · 1 line
gitgitgadget
Re: [PATCH v12 6/6] fsmonitor: add documentation for allowRemote and socketDir options
Quoted 16 lines · Eric DeCosta via GitGitGadget
From: Eric DeCosta <[email protected]>
Add documentation for 'fsmonitor.allowRemote' and 'fsmonitor.socketDir'. Call-out experimental nature of 'fsmonitor.allowRemote' and limited file system support for 'fsmonitor.socketDir'.
Signed-off-by Eric DeCosta <[email protected]>--- Documentation/git-fsmonitor--daemon.txt | 48 +++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-)Documentation/git-fsmonitor--daemon.txt +0
-0
Documentation/git-fsmonitor--daemon.txt git-fsmonitor{litdd}daemon(1) · line 6
Quoted 5 lines
Documentation/git-fsmonitor--daemon.txt +1
-1@@ -3,7 +3,7 @@ git-fsmonitor{litdd}daemon(1) NAME ---- -git-fsmonitor--daemon - A Built-in File System Monitor +git-fsmonitor--daemon - A Built-in Filesystem Monitor
We have ~400 uses of "filesystem" in-tree, but ~100 for "file system". I don't mind the change per-se, but this looks like an odd "while at it" change.
Skmming your series your 1/6 even uses "file system" in the commit message :)
Documentation/git-fsmonitor--daemon.txt DESCRIPTION · line 20
Quoted 3 lines
Documentation/git-fsmonitor--daemon.txt +1
-1A daemon to watch the working directory for file and directory -changes using platform-specific file system notification facilities. +changes using platform-specific filesystem notification facilities.
More while-at-it...
Documentation/git-fsmonitor--daemon.txt CAVEATS · line 66
Quoted 9 lines
Documentation/git-fsmonitor--daemon.txt +0
-0This daemon communicates directly with commands like `git status`using the link:technical/api-simple-ipc.html[simple IPC] interface
Documentation/git-fsmonitor--daemon.txt +1
-1@@ -63,13 +63,55 @@ CAVEATS ------- The fsmonitor daemon does not currently know about submodules and does -not know to filter out file system events that happen within a +not know to filter out filesystem events that happen within a
...and here...
Documentation/git-fsmonitor--daemon.txt CAVEATS · line 71
Quoted 5 lines
Documentation/git-fsmonitor--daemon.txt +0
-0submodule. If fsmonitor daemon is watching a super repo and a file is modified within the working directory of a submodule, it will report the change (as happening against the super repo). However, the client will properly ignore these extra events, so performance may be affected but it will not cause an incorrect result.
But here we get to the real meat of this change...
Quoted 13 lines
Documentation/git-fsmonitor--daemon.txt +13
-0+By default, the fsmonitor daemon refuses to work against network-mounted +repositories; this may be overridden by setting `fsmonitor.allowRemote` to +`true`. Note, however, that the fsmonitor daemon is not guaranteed to work +correctly with all network-mounted repositories and such use is considered +experimental. + +On Mac OS, the inter-process communication (IPC) between various Git +commands and the fsmonitor daemon is done via a Unix domain socket (UDS) -- a +special type of file -- which is supported by native Mac OS filesystems, +but not on network-mounted filesystems, NTFS, or FAT32. Other filesystems +may or may not have the needed support; the fsmonitor daemon is not guaranteed +to work with these filesystems and such use is considered experimental. +
Documentation/git-fsmonitor--daemon.txt CAVEATS · line 95
Quoted 10 lines
Documentation/git-fsmonitor--daemon.txt +10
-0+By default, the socket is created in the `.git` directory, however, if the +`.git` directory is on a network-mounted filesystem, it will be instead be +created at `$HOME/.git-fsmonitor-*` unless `$HOME` itself is on a +network-mounted filesystem in which case you must set the configuration +variable `fsmonitor.socketDir` to the path of a directory on a Mac OS native +filesystem in which to create the socket file. + +If none of the above directories (`.git`, `$HOME`, or `fsmonitor.socketDir`) +is on a native Mac OS file filesystem the fsmonitor daemon will report an +error that will cause the daemon and the currently running command to exit.
From skimming this looks reasonable.
Quoted 8 lines
Documentation/git-fsmonitor--daemon.txt +8
-0+CONFIGURATION +------------- + +When `core.fsmonitor` is set to `true` (see linkgit:git-config[1]) +the fsmonitor daemon will pay attention to the following configuration +variables: + +`fsmonitor.allowRemote`::
Documentation/git-fsmonitor--daemon.txt CAVEATS · line 114
Quoted 10 lines
Documentation/git-fsmonitor--daemon.txt +10
-0+ By default, the daemon refuses to work against network-mounted + repositories. Setting `fsmonitor.allowRemote` to `true` overrides + this behavior. + +`fsmonitor.socketDir`:: + This Mac OS-specific option, if set, specifies the directory in + which to create the Unix domain socket used for communication + between fsmonitor and various Git commands. The directory must + reside on a native Mac OS filesystem as discussed above. +
But here we should instead create Documentation/config/fsmonitor.txt, and include it here. See my recent 7a54d740451 (Merge branch 'ab/dedup-config-and-command-docs', 2022-09-14) which made it so for many commands.
By doing it like this "git config" is no longer the canonical reference for all config, which it mostly was pre-7a54d740451, then was again post-7a54d740451, and now we'd have this exception...
Re: [PATCH v12 6/6] fsmonitor: add documentation for allowRemote and socketDir options
Quoted 14 lines · Ævar Arnfjörð Bjarmason
Documentation/git-fsmonitor--daemon.txt git-fsmonitor{litdd}daemon(1) · line 6
Quoted 8 lines · Eric DeCosta via GitGitGadget
Add documentation for 'fsmonitor.allowRemote' and 'fsmonitor.socketDir'. Call-out experimental nature of 'fsmonitor.allowRemote' and limited file system support for 'fsmonitor.socketDir'.
Signed-off-by Eric DeCosta <[email protected]>---
Documentation/git-fsmonitor--daemon.txt +1
-1-git-fsmonitor--daemon - A Built-in File System Monitor +git-fsmonitor--daemon - A Built-in Filesystem MonitorWe have ~400 uses of "filesystem" in-tree, but ~100 for "file system". I don't mind the change per-se, but this looks like an odd "while at it" change.
Quoted 5 lines
Documentation/git-fsmonitor--daemon.txt DESCRIPTION · line 20
Quoted 3 lines
A daemon to watch the working directory for file and directory
Documentation/git-fsmonitor--daemon.txt +1
-1-changes using platform-specific file system notification facilities. +changes using platform-specific filesystem notification facilities.More while-at-it...
These changes may have been in response to my review[1], though I did say that such a change was outside the scope of this series. Perhaps I need to choose my wording more carefully?
[1]: https://lore.kernel.org/git/CAPig+cQ5SMw+0Cwtw47LQM59-mQjJaOPe_LTybAC2j=3F9OywA@mail.gmail.com/
RE: [PATCH v12 6/6] fsmonitor: add documentation for allowRemote and socketDir options
Quoted 37 lines
-----Original Message-----
From: Eric Sunshine <[email protected]>
Sent: Monday, September 26, 2022 10:16 PM
To: Ævar Arnfjörð Bjarmason <[email protected]>
Cc: Eric DeCosta via GitGitGadget <[email protected]>; Git List <[email protected]>; Jeff Hostetler <[email protected]>; Torsten Bögershausen <[email protected]>; Ramsay Jones <[email protected]>; Johannes Schindelin <[email protected]>; Eric DeCosta <[email protected]>
Subject: Re: [PATCH v12 6/6] fsmonitor: add documentation for allowRemote and socketDir optionsQuoted 2 lines · Ævar Arnfjörð Bjarmason
Quoted 1 line · Eric DeCosta via GitGitGadget
Add documentation for 'fsmonitor.allowRemote' and
'fsmonitor.socketDir'.
Quoted 12 lines
Documentation/git-fsmonitor--daemon.txt git-fsmonitor{litdd}daemon(1) · line 6
Quoted 7 lines
Call-out experimental nature of 'fsmonitor.allowRemote' and limited file system support for 'fsmonitor.socketDir'.
Signed-off-by Eric DeCosta <[email protected]>---
Documentation/git-fsmonitor--daemon.txt +1
-1-git-fsmonitor--daemon - A Built-in File System Monitor +git-fsmonitor--daemon - A Built-in Filesystem MonitorWe have ~400 uses of "filesystem" in-tree, but ~100 for "file system". I don't mind the change per-se, but this looks like an odd "while at it" change.
Quoted 5 lines
Documentation/git-fsmonitor--daemon.txt DESCRIPTION · line 20
Quoted 3 lines
A daemon to watch the working directory for file and directory
Documentation/git-fsmonitor--daemon.txt +1
-1-changes using platform-specific file system notification facilities. +changes using platform-specific filesystem notification facilities.More while-at-it...
Quoted 8 lines
These changes may have been in response to my review[1], though I did say that such a change was outside the scope of this series. Perhaps I need to choose my wording more carefully?
[1]: https://lore.kernel.org/git/CAPig+cQ5SMw+0Cwtw47LQM59-
[email protected]/ <https://protect-
us.mimecast.com/s/STwOCG6A7DIYgAqYc7Wdth?domain=lore.kernel.org>
Your wording was fine. Next time I'll know better and leave it for another patch set.
-Eric
Quoted 79 lines
-----Original Message-----
From: Eric DeCosta via GitGitGadget <[email protected]>
Sent: Saturday, September 24, 2022 3:46 PM
To: [email protected]
Cc: Jeff Hostetler <[email protected]>; Eric Sunshine <[email protected]>; Torsten Bögershausen <[email protected]>; Ævar Arnfjörð Bjarmason <[email protected]>; Ramsay Jones <[email protected]>; Johannes Schindelin <[email protected]>; Eric DeCosta <[email protected]>
Subject: [PATCH v12 0/6] fsmonitor: option to allow fsmonitor to run against network-mounted reposFollow-on to the work done to allow Windows to work against network- mounted repos for macOS.
Have macOS take advantage of the same configuration option, 'fsmonitor.allowRemote' that was introduced for Windows. Setting this option to true will override the default behavior (erroring-out) when a network-mounted repo is detected by fsmonitor.
The added wrinkle being that the Unix domain socket (UDS) file used for IPC cannot be created in a network location; instead $HOME is used if the default location is on the network. The user may, optionally, set the 'fsmonitor.socketDir' configuration option to a valid, local directory if $HOME itself is on the network or is simply not the desired location for the UDS file.
An additional issue is that for mount points in the root directory, FSEvents does not report a path that matches the worktree directory due to the introduction of 'synthetic firmlinks'. fsmonitor must map the FSEvents paths to the worktree directory by interrogating the root filesystem for synthetic firmlinks and using that information to translate the path.
v12 differs from v11:
* bug fixes
v11 differs from v10:
* incorporates code review feedback
* fix memory leak infsm-listen-darwin.cv10 differs from v9:
* incorporates code review feedback
* improves error messaging for incompatible socket directoryv9 differs from v8:
* incorporates code review feedback
* check for incompatibility before communicating with fsmonitorv8 differs from v7:
* incorporates code review feedback
* gets the rebase rightv7 differs from v6:
* incorporates code review feedback
v6 differs from v5:
* incorporates earlier, Windows-specific changes that have not made it back yet to the master branch
* incorporates code review feedback
* adds documentation for 'fsmonitor.allowRemote' and 'fsmonitor.socketDir'v5 differs significantly from earlier versions:
* redesign of handling 'fsmonitor.allowRemote' and 'fsmonitor.socketDir' such that these options are no longer added to the settings data structure but are rather read from config at point of use
* refactoring of code for handling platform-specific file system checks via a common interface to avoid platform #ifdef in IPC code and be in-model with other platform-specific fsmonitor code
* dealing with 'synthetic firmlinks' on macOS
Quoted 8 lines
Eric DeCosta (6):
fsmonitor: refactor filesystem checks to common interface
fsmonitor: relocate socket file if .git directory is remote
fsmonitor: avoid socket location check if using hook
fsmonitor: deal with synthetic firmlinks on macOS
fsmonitor: check for compatability before communicating with fsmonitor
fsmonitor: add documentation for allowRemote and socketDir options
Any further thoughts? If not I think this patch set is good to merge.
-Eric
Quoted 91 lines
Documentation/git-fsmonitor--daemon.txt| 48 ++++++-Makefile| 2 +builtin/fsmonitor--daemon.c| 11 +-compat/fsmonitor/fsm-ipc-darwin.c| 52 +++++++compat/fsmonitor/fsm-ipc-win32.c| 9 ++ compat/fsmonitor/fsm-listen-darwin.c| 14 +-compat/fsmonitor/fsm-path-utils-darwin.c| 132 +++++++++++++++++compat/fsmonitor/fsm-path-utils-win32.c| 145 +++++++++++++++++++compat/fsmonitor/fsm-settings-darwin.c| 72 +++----- --compat/fsmonitor/fsm-settings-win32.c| 174 +----------------------contrib/buildsystems/CMakeLists.txt| 4 +fsmonitor--daemon.h| 3 +fsmonitor-ipc.c| 18 ++-fsmonitor-ipc.h| 4 +-fsmonitor-path-utils.h| 59 ++++++++fsmonitor-settings.c| 68 ++++++++-fsmonitor-settings.h| 4 +-fsmonitor.c| 7 +18 files changed, 580 insertions(+), 246 deletions(-) create mode 100644
compat/fsmonitor/fsm-ipc-darwin.ccreate mode 100644compat/fsmonitor/fsm-ipc-win32.ccreate mode 100644compat/fsmonitor/fsm-path-utils-darwin.c
create mode 100644compat/fsmonitor/fsm-path-utils-win32.ccreate mode 100644fsmonitor-path-utils.hbase-commit:
4b79ee4b0cd1130ba8907029cdc5f6a1632aca26
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-
1326%2Fedecosta-mw%2Ffsmonitor_macos-v12 <https://protect-
us.mimecast.com/s/8B-_CDkx7zSvvOYvhZsW8c?domain=github.com>
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git <https://protect-
us.mimecast.com/s/G1p4CERy7Ah99649sZioql?domain=github.com> pr- 1326/edecosta-mw/fsmonitor_macos-v12
Pull-Request: https://github.com/gitgitgadget/git/pull/1326 <https://protect-
us.mimecast.com/s/tTa3CG6A7DIYYL3Ys0on8b?domain=github.com>Range-diff vs v11:
1:
155a6890806= 1:5958dab0163fsmonitor: refactor filesystem checks to common interface
2:dbe113abb87! 2:20220b08edbfsmonitor: relocate socket file if .git directory is remote @@compat/fsmonitor/fsm-ipc-darwin.c(new)(quoted diff) +6
-1+ +const char *fsmonitor_ipc__get_path(struct repository *r) { -+ static const char *ipc_path; ++ static const char *ipc_path = NULL; + SHA_CTX sha1ctx; + char *sock_dir = NULL; + struct strbuf ipc_file = STRBUF_INIT;@@
compat/fsmonitor/fsm-ipc-darwin.c(new)
+ if (ipc_path)
+ returnipc_path;(quoted diff) +10
-2+ -+ ipc_path = fsmonitor_ipc__get_default_path(); + + /* By default the socket file is created in the .git directory */ -+ if (fsmonitor__is_fs_remote(ipc_path) < 1) ++ if (fsmonitor__is_fs_remote(r->gitdir) < 1) { ipc_path = ++ fsmonitor_ipc__get_default_path(); + return ipc_path; ++ } + + SHA1_Init(&sha1ctx); + SHA1_Update(&sha1ctx, r->worktree, strlen(r->worktree));3:
86c15299ae8= 3:e9921550a67fsmonitor: avoid socket location check if using hook
4:1a516fd9214= 4:6efdc6ed74efsmonitor: deal with synthetic firmlinks on macOS
5:e0fe05dabef! 5:421d77775dcfsmonitor: check for compatability before communicating with fsmonitor @@fsmonitor-settings.h: enumfsmonitor_modefsm_settings__get_mode(struct reposito structfsmonitor_settings;##
fsmonitor.c##
+@@fsmonitor.c: static intfsmonitor_force_update_threshold= 100; +
+ voidrefresh_fsmonitor(struct index_state *istate){
++ static intwarn_once= 0;
+ struct strbufquery_result=STRBUF_INIT; intquery_success= 0,
+hook_version= -1;size_tbol = 0; /* beginning of line */ @@fsmonitor.c: voidrefresh_fsmonitor(struct index_state *istate)intis_trivial= 0; struct repository *r =istate->repo?istate->repo:the_repository; enumfsmonitor_modefsm_mode=fsm_settings__get_mode(r);
+ enumfsmonitor_reasonreason =fsm_settings__get_reason(r); +
-+ if (reason >FSMONITOR_REASON_OK)
++ if (!warn_once && reason >FSMONITOR_REASON_OK) {warn_once= 1;
+warning("%s", fsm_settings__get_incompatible_msg(r, reason)); ++ }if (
fsm_mode<=FSMONITOR_MODE_DISABLED||istate->fsmonitor_has_run_once)
6:3200b505988= 6:b375b0ac798fsmonitor: add documentation for allowRemote and socketDir options--
gitgitgadget
v13 cover fsmonitor: option to allow fsmonitor to run against network-mounted repos
In reply to Eric DeCosta via GitGitGadgetFollow-on to the work done to allow Windows to work against network-mounted repos for macOS.
Have macOS take advantage of the same configuration option, 'fsmonitor.allowRemote' that was introduced for Windows. Setting this option to true will override the default behavior (erroring-out) when a network-mounted repo is detected by fsmonitor.
The added wrinkle being that the Unix domain socket (UDS) file used for IPC cannot be created in a network location; instead $HOME is used if the default location is on the network. The user may, optionally, set the 'fsmonitor.socketDir' configuration option to a valid, local directory if $HOME itself is on the network or is simply not the desired location for the UDS file.
An additional issue is that for mount points in the root directory, FSEvents does not report a path that matches the worktree directory due to the introduction of 'synthetic firmlinks'. fsmonitor must map the FSEvents paths to the worktree directory by interrogating the root filesystem for synthetic firmlinks and using that information to translate the path.
v13 differs from v12:
* code review feedback
v12 differs from v11:
* bug fixes
v11 differs from v10:
* incorporates code review feedback
* fix memory leak in fsm-listen-darwin.c
v10 differs from v9:
* incorporates code review feedback
* improves error messaging for incompatible socket directory
v9 differs from v8:
* incorporates code review feedback
* check for incompatibility before communicating with fsmonitor
v8 differs from v7:
* incorporates code review feedback
* gets the rebase right
v7 differs from v6:
* incorporates code review feedback
v6 differs from v5:
* incorporates earlier, Windows-specific changes that have not made it back yet to the master branch
* incorporates code review feedback
* adds documentation for 'fsmonitor.allowRemote' and 'fsmonitor.socketDir'
v5 differs significantly from earlier versions:
* redesign of handling 'fsmonitor.allowRemote' and 'fsmonitor.socketDir' such that these options are no longer added to the settings data structure but are rather read from config at point of use
* refactoring of code for handling platform-specific file system checks via a common interface to avoid platform #ifdef in IPC code and be in-model with other platform-specific fsmonitor code
* dealing with 'synthetic firmlinks' on macOS
Eric DeCosta (6):
fsmonitor: refactor filesystem checks to common interface
fsmonitor: relocate socket file if .git directory is remote
fsmonitor: avoid socket location check if using hook
fsmonitor: deal with synthetic firmlinks on macOS
fsmonitor: check for compatability before communicating with fsmonitor
fsmonitor: add documentation for allowRemote and socketDir options
Documentation/config.txt | 2 + Documentation/config/fsmonitor--daemon.txt | 11 ++ Documentation/git-fsmonitor--daemon.txt | 37 ++++- Makefile | 2 + builtin/fsmonitor--daemon.c | 13 +- compat/fsmonitor/fsm-ipc-darwin.c | 52 ++++++ compat/fsmonitor/fsm-ipc-win32.c | 9 ++ compat/fsmonitor/fsm-listen-darwin.c | 14 +- compat/fsmonitor/fsm-path-utils-darwin.c | 133 ++++++++++++++++ compat/fsmonitor/fsm-path-utils-win32.c | 145 +++++++++++++++++ compat/fsmonitor/fsm-settings-darwin.c | 72 +++------ compat/fsmonitor/fsm-settings-win32.c | 174 +-------------------- contrib/buildsystems/CMakeLists.txt | 4 + fsmonitor--daemon.h | 3 + fsmonitor-ipc.c | 18 +-- fsmonitor-ipc.h | 4 +- fsmonitor-path-utils.h | 57 +++++++ fsmonitor-settings.c | 68 +++++++- fsmonitor-settings.h | 4 +- fsmonitor.c | 7 + 20 files changed, 583 insertions(+), 246 deletions(-) create mode 100644 Documentation/config/fsmonitor--daemon.txt create mode 100644 compat/fsmonitor/fsm-ipc-darwin.c create mode 100644 compat/fsmonitor/fsm-ipc-win32.c create mode 100644 compat/fsmonitor/fsm-path-utils-darwin.c create mode 100644 compat/fsmonitor/fsm-path-utils-win32.c create mode 100644 fsmonitor-path-utils.h
base-commit: 2a7d63a2453e2c30353342a2c9385fa22a846987
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1326%2Fedecosta-mw%2Ffsmonitor_macos-v13
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1326/edecosta-mw/fsmonitor_macos-v13
Pull-Request: https://github.com/gitgitgadget/git/pull/1326
Range-diff vs v12:
1: 5958dab0163 = 1: 0b9b64428c5 fsmonitor: refactor filesystem checks to common interface 2: 20220b08edb ! 2: 680d4c83f99 fsmonitor: relocate socket file if .git directory is remote
@@ Metadata
## Commit message ##
fsmonitor: relocate socket file if .git directory is remote
- If the .git directory is on a remote file system, create the socket
+ If the .git directory is on a remote filesystem, create the socket
file in 'fsmonitor.socketDir' if it is defined, else create it in $HOME.
3: e9921550a67 = 3: 7987d0c1f33 fsmonitor: avoid socket location check if using hook
4: 6efdc6ed74e ! 4: 324eb5acd85 fsmonitor: deal with synthetic firmlinks on macOS
@@ builtin/fsmonitor--daemon.c: static int fsmonitor_run_daemon(void)
strbuf_addstr(&state.path_worktree_watch, absolute_path(get_git_work_tree()));
state.nr_paths_watching = 1;
-+ state.alias.alias = NULL;
-+ state.alias.points_to = NULL;
++ strbuf_init(&state.alias.alias, 0);
++ strbuf_init(&state.alias.points_to, 0);
+ if (fsmonitor__get_alias(state.path_worktree_watch.buf, &state.alias)) {
+ err = error(_("could not get worktree alias"));
+ goto done;
@@ builtin/fsmonitor--daemon.c: static int fsmonitor_run_daemon(void)
/*
* We create and delete cookie files somewhere inside the .git
* directory to help us keep sync with the file system. If
+@@ builtin/fsmonitor--daemon.c: done:
+ strbuf_release(&state.path_gitdir_watch);
+ strbuf_release(&state.path_cookie_prefix);
+ strbuf_release(&state.path_ipc);
++ strbuf_release(&state.alias.alias);
++ strbuf_release(&state.alias.points_to);
+
+ return err;
+ }
## compat/fsmonitor/fsm-listen-darwin.c ##
@@
@@ compat/fsmonitor/fsm-path-utils-darwin.c: int fsmonitor__is_fs_remote(const char
+ */
+int fsmonitor__get_alias(const char *path, struct alias_info *info)
+{
-+ DIR * dir;
++ DIR *dir;
+ int read;
-+ int retval;
++ int retval = -1;
+ struct dirent *de;
+ struct strbuf alias;
+ struct strbuf points_to;
+
-+ retval = 0;
+ dir = opendir("/");
-+ if (!dir)
++ if (!dir) {
++ error_errno("opendir('/') failed");
+ return -1;
++ }
+
+ strbuf_init(&alias, 256);
++
++ /* no way of knowing what the link will resolve to, so MAXPATHLEN */
+ strbuf_init(&points_to, MAXPATHLEN);
+
+ while ((de = readdir(dir)) != NULL) {
@@ compat/fsmonitor/fsm-path-utils-darwin.c: int fsmonitor__is_fs_remote(const char
+ read = readlink(alias.buf, points_to.buf, MAXPATHLEN);
+ if (read > 0) {
+ strbuf_setlen(&points_to, read);
-+ if ((strncmp(points_to.buf, path, points_to.len) == 0)
++ if ((!strncmp(points_to.buf, path, points_to.len))
+ && path[points_to.len] == '/') {
-+ info->alias = strbuf_detach(&alias, NULL);
-+ info->points_to = strbuf_detach(&points_to, NULL);
++ strbuf_addbuf(&info->alias, &alias);
++ strbuf_addbuf(&info->points_to, &points_to);
+ trace_printf_key(&trace_fsmonitor,
+ "Found alias for '%s' : '%s' -> '%s'",
-+ path, info->alias, info->points_to);
++ path, info->alias.buf, info->points_to.buf);
+ retval = 0;
+ goto done;
+ }
++ } else if (!read) {
++ BUG("readlink returned 0");
+ } else if (errno != EINVAL) { /* Something other than not a link */
-+ trace_printf_key(&trace_fsmonitor, "Error %s", strerror(errno));
-+ retval = -1;
++ error_errno("readlink('%s') failed", alias.buf);
+ goto done;
+ }
+ }
++ retval = 0; /* no alias */
+
-+ done:
-+ closedir(dir);
++done:
++ if (closedir(dir) < 0)
++ warning_errno("closedir('/') failed");
+ strbuf_release(&alias);
+ strbuf_release(&points_to);
+ return retval;
@@ compat/fsmonitor/fsm-path-utils-darwin.c: int fsmonitor__is_fs_remote(const char
+char *fsmonitor__resolve_alias(const char *path,
+ const struct alias_info *info)
+{
-+ int len = info->alias ? strlen(info->alias) : 0;
-+
-+ if (!len)
++ if (!info->alias.len)
+ return NULL;
+
-+ if ((strncmp(info->alias, path, len) == 0)
-+ && path[len] == '/') {
-+ struct strbuf tmp;
-+ const char *remainder = path + len;
-+ int ptr_len = strlen(info->points_to);
-+ int rem_len = strlen(remainder);
++ if ((!strncmp(info->alias.buf, path, info->alias.len))
++ && path[info->alias.len] == '/') {
++ struct strbuf tmp = STRBUF_INIT;
++ const char *remainder = path + info->alias.len;
+
-+ strbuf_init(&tmp, ptr_len + rem_len);
-+ strbuf_add(&tmp, info->points_to, ptr_len);
-+ strbuf_add(&tmp, remainder, rem_len);
++ strbuf_addbuf(&tmp, &info->points_to);
++ strbuf_add(&tmp, remainder, strlen(remainder));
+ return strbuf_detach(&tmp, NULL);
+ }
+
@@ fsmonitor-path-utils.h
+
+struct alias_info
+{
-+ char *alias;
-+ char *points_to;
++ struct strbuf alias;
++ struct strbuf points_to;
+};
+
struct fs_info {
int is_remote;
char *typename;
+ };
+
+ /*
+- * Get some basic filesystem informtion for the given path
++ * Get some basic filesystem information for the given path
+ *
+ * Returns -1 on error, zero otherwise.
+ */
@@ fsmonitor-path-utils.h: int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info);
*/
int fsmonitor__is_fs_remote(const char *path);
@@ fsmonitor-path-utils.h: int fsmonitor__get_fs_info(const char *path, struct fs_i
+ *
+ * Returns -1 on error, 0 otherwise.
+ *
-+ * The caller owns the storage that is occupied by set info.alias and
-+ * info.points_to and is responsible for releasing it with `free(3)`
-+ * when done.
++ * The caller owns the storage that is occupied by info.alias and
++ * info.points_to and is responsible for releasing it.
+ */
+int fsmonitor__get_alias(const char *path, struct alias_info *info);
+
@@ fsmonitor-path-utils.h: int fsmonitor__get_fs_info(const char *path, struct fs_i
+ * Returns the resolved path if there is one, NULL otherwise.
+ *
+ * The caller owns the storage that the returned string occupies and
-+ * is responsible for releasing it with `free(3)` when done.
++ * is responsible for releasing it.
+ */
+char *fsmonitor__resolve_alias(const char *path,
+ const struct alias_info *info);
-+
+
#endif 5: 421d77775dc ! 5: b1ea378dff7 fsmonitor: check for compatability before communicating with fsmonitor
@@ Metadata
## Commit message ##
fsmonitor: check for compatability before communicating with fsmonitor
- If fsmonitor is not in a compatible state, die with an appropriate error
- messge.
+ If fsmonitor is not in a compatible state, warn with an appropriate message.
@@ fsmonitor-settings.c: char *fsm_settings__get_incompatible_msg(const struct repo
- _("repository '%s' is incompatible with fsmonitor due to lack of Unix sockets"),
- r->worktree);
+ _("socket directory '%s' is incompatible with fsmonitor due"
-+ " to lack of Unix sockets support"),
++ " to lack of Unix sockets support"),
+ socket_dir);
goto done;
}
6: b375b0ac798 ! 6: 04f607b1f21 fsmonitor: add documentation for allowRemote and socketDir options
@@ Commit message
fsmonitor: add documentation for allowRemote and socketDir options
Add documentation for 'fsmonitor.allowRemote' and 'fsmonitor.socketDir'.
- Call-out experimental nature of 'fsmonitor.allowRemote' and limited file
- system support for 'fsmonitor.socketDir'.
+ Call-out experimental nature of 'fsmonitor.allowRemote' and limited
+ filesystem support for 'fsmonitor.socketDir'.
+ ## Documentation/config.txt ##
+@@ Documentation/config.txt: include::config/filter.txt[]
+
+ include::config/fsck.txt[]
+
++include::config/fsmonitor--daemon.txt[]
++
+ include::config/gc.txt[]
+
+ include::config/gitcvs.txt[]
+
+ ## Documentation/config/fsmonitor--daemon.txt (new) ##
+@@
++fsmonitor.allowRemote::
++ By default, the fsmonitor daemon refuses to work against network-mounted
++ repositories. Setting fsmonitor.allowRemote to true overrides this
++ behavior. Only respected when core.fsmonitor is set to true.
++
++fsmonitor.socketDir::
++ This Mac OS-specific option, if set, specifies the directory in
++ which to create the Unix domain socket used for communication
++ between the fsmonitor daemon and various Git commands. The directory must
++ reside on a native Mac OS filesystem. Only respected when core.fsmonitor
++ is set to true.
+
## Documentation/git-fsmonitor--daemon.txt ##
@@ Documentation/git-fsmonitor--daemon.txt: git-fsmonitor{litdd}daemon(1)
@@ Documentation/git-fsmonitor--daemon.txt: CAVEATS
+CONFIGURATION
+-------------
+
-+When core.fsmonitor is set to true (see linkgit:git-config[1]) -+the fsmonitor daemon will pay attention to the following configuration -+variables:
++include::includes/cmd-config-section-all.txt[]
+
-+fsmonitor.allowRemote::
-+ By default, the daemon refuses to work against network-mounted
-+ repositories. Setting fsmonitor.allowRemote to true overrides
-+ this behavior.
-+
-+fsmonitor.socketDir::
-+ This Mac OS-specific option, if set, specifies the directory in
-+ which to create the Unix domain socket used for communication
-+ between fsmonitor and various Git commands. The directory must
-+ reside on a native Mac OS filesystem as discussed above.
++include::config/fsmonitor--daemon.txt[]
+
GIT
---
Signature · 1 line
gitgitgadget
PATCH v13 1/6 fsmonitor: refactor filesystem checks to common interface
From: Eric DeCosta <[email protected]>
Provide a common interface for getting basic filesystem information including filesystem type and whether the filesystem is remote.
Refactor existing code for getting basic filesystem info and detecting remote file systems to the new interface.
Refactor filesystem checks to leverage new interface. For macOS, error-out if the Unix Domain socket (UDS) file is on a remote filesystem.
--- Makefile | 1 + compat/fsmonitor/fsm-path-utils-darwin.c | 40 ++++++ compat/fsmonitor/fsm-path-utils-win32.c | 128 +++++++++++++++++ compat/fsmonitor/fsm-settings-darwin.c | 62 +++----- compat/fsmonitor/fsm-settings-win32.c | 172 +---------------------- contrib/buildsystems/CMakeLists.txt | 2 + fsmonitor-path-utils.h | 23 +++ fsmonitor-settings.c | 50 +++++++ 8 files changed, 263 insertions(+), 215 deletions(-) create mode 100644 compat/fsmonitor/fsm-path-utils-darwin.c create mode 100644 compat/fsmonitor/fsm-path-utils-win32.c create mode 100644 fsmonitor-path-utils.h
Makefile +1 -0
@@ -2044,6 +2044,7 @@ endif ifdef FSMONITOR_OS_SETTINGS COMPAT_CFLAGS += -DHAVE_FSMONITOR_OS_SETTINGS COMPAT_OBJS += compat/fsmonitor/fsm-settings-$(FSMONITOR_OS_SETTINGS).o + COMPAT_OBJS += compat/fsmonitor/fsm-path-utils-$(FSMONITOR_OS_SETTINGS).o endif ifeq ($(TCLTK_PATH),)
compat/fsmonitor/fsm-path-utils-darwin.c new file +40 -0
@@ -0,0 +1,40 @@ +#include "fsmonitor.h" +#include "fsmonitor-path-utils.h" +#include <sys/param.h> +#include <sys/mount.h> + +int fsmonitor__get_fs_info (const char *path,struct fs_info *fs_info) +{ +struct statfs fs; +if (statfs (path, &fs) ==-1 ) { +int saved_errno = errno; +trace_printf_key (&trace_fsmonitor,"statfs('%s') failed: %s" , + path,strerror (saved_errno)); + errno = saved_errno; +return -1 ; + } + +trace_printf_key (&trace_fsmonitor, +"statfs('%s') [type 0x%08x][flags 0x%08x] '%s'" , + path, fs.f_type, fs.f_flags, fs.f_fstypename); + +if (!(fs.f_flags &MNT_LOCAL )) + fs_info->is_remote =1 ; +else + fs_info->is_remote =0 ; + + fs_info->typename = fs.f_fstypename; + +trace_printf_key (&trace_fsmonitor, +"'%s' is_remote: %d" , + path, fs_info->is_remote); +return 0 ; +} + +int fsmonitor__is_fs_remote (const char *path) +{ +struct fs_info fs; +if (fsmonitor__get_fs_info (path, &fs)) +return -1 ; +return fs.is_remote; +}
compat/fsmonitor/fsm-path-utils-win32.c new file +128 -0
128 lines not on this page · raw message
compat/fsmonitor/fsm-settings-darwin.c +17 -45
96 lines not on this page · raw message
compat/fsmonitor/fsm-settings-win32.c +2 -170
190 lines not on this page · raw message
contrib/buildsystems/CMakeLists.txt +2 -0
@@ -308,6 +308,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-win32.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-win32.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-win32.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-win32.c)
@@ -315,6 +316,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-darwin.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-darwin.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-darwin.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-darwin.c)
fsmonitor-path-utils.h new file +23 -0
@@ -0,0 +1,23 @@ +#ifndef FSM_PATH_UTILS_H +#define FSM_PATH_UTILS_H + +struct fs_info { +int is_remote; +char *typename; +}; + +/* +* Get some basic filesystem informtion for the given path +* +* Returns -1 on error, zero otherwise. +*/ +int fsmonitor__get_fs_info (const char *path,struct fs_info *fs_info); + +/* +* Determines if the filesystem that path resides on is remote. +* +* Returns -1 on error, 0 if not remote, 1 if remote. +*/ +int fsmonitor__is_fs_remote (const char *path); + +#endif
fsmonitor-settings.c +50 -0
68 lines not on this page · raw message
Signature · 1 line
gitgitgadget
PATCH v13 2/6 fsmonitor: relocate socket file if .git directory is remote
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
If the .git directory is on a remote filesystem, create the socket file in 'fsmonitor.socketDir' if it is defined, else create it in $HOME.
--- Makefile | 1 + builtin/fsmonitor--daemon.c | 3 +- compat/fsmonitor/fsm-ipc-darwin.c | 52 ++++++++++++++++++++++++++ compat/fsmonitor/fsm-ipc-win32.c | 9 +++++ compat/fsmonitor/fsm-settings-darwin.c | 2 +- contrib/buildsystems/CMakeLists.txt | 2 + fsmonitor-ipc.c | 18 ++++----- fsmonitor-ipc.h | 4 +- 8 files changed, 78 insertions(+), 13 deletions(-) create mode 100644 compat/fsmonitor/fsm-ipc-darwin.c create mode 100644 compat/fsmonitor/fsm-ipc-win32.c
Makefile +1 -0
@@ -2039,6 +2039,7 @@ ifdef FSMONITOR_DAEMON_BACKEND COMPAT_CFLAGS += -DHAVE_FSMONITOR_DAEMON_BACKEND COMPAT_OBJS += compat/fsmonitor/fsm-listen-$(FSMONITOR_DAEMON_BACKEND).o COMPAT_OBJS += compat/fsmonitor/fsm-health-$(FSMONITOR_DAEMON_BACKEND).o + COMPAT_OBJS += compat/fsmonitor/fsm-ipc-$(FSMONITOR_DAEMON_BACKEND).o endif ifdef FSMONITOR_OS_SETTINGS
builtin/fsmonitor--daemon.c +2 -1
@@ -1343,7 +1343,8 @@ static int fsmonitor_run_daemon(void)* directory.) */ strbuf_init (&state.path_ipc,0 ); -strbuf_addstr (&state.path_ipc,absolute_path (fsmonitor_ipc__get_path ())); +strbuf_addstr (&state.path_ipc, +absolute_path (fsmonitor_ipc__get_path (the_repository)));/* * Confirm that we can create platform-specific resources for the
compat/fsmonitor/fsm-ipc-darwin.c new file +52 -0
52 lines not on this page · raw message
compat/fsmonitor/fsm-ipc-win32.c new file +9 -0
@@ -0,0 +1,9 @@ +#include "config.h" +#include "fsmonitor-ipc.h" + +const char *fsmonitor_ipc__get_path (struct repository *r) { +static char *ret; +if (!ret) + ret =git_pathdup ("fsmonitor--daemon.ipc" ); +return ret; +}
compat/fsmonitor/fsm-settings-darwin.c +1 -1
@@ -26,7 +26,7 @@static enum fsmonitor_reason check_uds_volume (struct repository *r) {struct fs_info fs; -const char *ipc_path =fsmonitor_ipc__get_path (); +const char *ipc_path =fsmonitor_ipc__get_path (r);struct strbuf path =STRBUF_INIT ;strbuf_add (&path, ipc_path,strlen (ipc_path));
contrib/buildsystems/CMakeLists.txt +2 -0
@@ -308,6 +308,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-win32.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-win32.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-win32.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-win32.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS)
@@ -316,6 +317,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-darwin.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-darwin.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-darwin.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-darwin.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS)
fsmonitor-ipc.c +8 -10
@@ -18,7 +18,7 @@ int fsmonitor_ipc__is_supported(void)return 0 ; } -const char *fsmonitor_ipc__get_path (void ) +const char *fsmonitor_ipc__get_path (struct repository *r) {return NULL ; }
@@ -47,11 +47,9 @@ int fsmonitor_ipc__is_supported(void)return 1 ; } -GIT_PATH_FUNC(fsmonitor_ipc__get_path, "fsmonitor--daemon.ipc") -enum ipc_active_state fsmonitor_ipc__get_state (void ) { -return ipc_get_active_state (fsmonitor_ipc__get_path ()); +return ipc_get_active_state (fsmonitor_ipc__get_path (the_repository)); }static int spawn_daemon (void )
@@ -81,8 +79,8 @@ int fsmonitor_ipc__send_query(const char *since_token,trace2_data_string ("fsm_client" ,NULL ,"query/command" , tok); try_again: - state =ipc_client_try_connect (fsmonitor_ipc__get_path (), &options, - &connection); + state =ipc_client_try_connect (fsmonitor_ipc__get_path (the_repository), + &options, &connection);switch (state) {case IPC_STATE__LISTENING :
@@ -117,13 +115,13 @@ try_again:case IPC_STATE__INVALID_PATH : ret =error (_ ("fsmonitor_ipc__send_query: invalid path '%s'" ), -fsmonitor_ipc__get_path ()); +fsmonitor_ipc__get_path (the_repository)); goto done;case IPC_STATE__OTHER_ERROR :default : ret =error (_ ("fsmonitor_ipc__send_query: unspecified error on '%s'" ), -fsmonitor_ipc__get_path ()); +fsmonitor_ipc__get_path (the_repository)); goto done; }
@@ -149,8 +147,8 @@ int fsmonitor_ipc__send_command(const char *command, options.wait_if_busy =1 ; options.wait_if_not_found =0 ; - state =ipc_client_try_connect (fsmonitor_ipc__get_path (), &options, - &connection); + state =ipc_client_try_connect (fsmonitor_ipc__get_path (the_repository), + &options, &connection);if (state !=IPC_STATE__LISTENING ) {die (_ ("fsmonitor--daemon is not running" ));return -1 ;
fsmonitor-ipc.h +3 -1
@@ -3,6 +3,8 @@#include "simple-ipc.h" +struct repository ; +/* * Returns true if built-in file system monitor daemon is defined * for this platform.
@@ -16,7 +18,7 @@ int fsmonitor_ipc__is_supported(void);* * Returns NULL if the daemon is not supported on this platform. */ -const char *fsmonitor_ipc__get_path (void ); +const char *fsmonitor_ipc__get_path (struct repository *r);/* * Try to determine whether there is a `git-fsmonitor--daemon` process
Signature · 1 line
gitgitgadget
PATCH v13 3/6 fsmonitor: avoid socket location check if using hook
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
If monitoring is done via fsmonitor hook rather than IPC there is no need to check if the location of the Unix Domain socket (UDS) file is on a remote filesystem.
--- compat/fsmonitor/fsm-settings-darwin.c | 10 ++++++---- compat/fsmonitor/fsm-settings-win32.c | 2 +- fsmonitor-settings.c | 8 ++++---- fsmonitor-settings.h | 2 +- 4 files changed, 12 insertions(+), 10 deletions(-)
compat/fsmonitor/fsm-settings-darwin.c +6 -4
@@ -49,13 +49,15 @@ static enum fsmonitor_reason check_uds_volume(struct repository *r)return FSMONITOR_REASON_OK ; } -enum fsmonitor_reason fsm_os__incompatible (struct repository *r) +enum fsmonitor_reason fsm_os__incompatible (struct repository *r,int ipc) {enum fsmonitor_reason reason; - reason =check_uds_volume (r); -if (reason !=FSMONITOR_REASON_OK ) -return reason; +if (ipc) { + reason =check_uds_volume (r); +if (reason !=FSMONITOR_REASON_OK ) +return reason; + }return FSMONITOR_REASON_OK ; }
compat/fsmonitor/fsm-settings-win32.c +1 -1
@@ -25,7 +25,7 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)return FSMONITOR_REASON_OK ; } -enum fsmonitor_reason fsm_os__incompatible (struct repository *r) +enum fsmonitor_reason fsm_os__incompatible (struct repository *r,int ipc) {enum fsmonitor_reason reason;
fsmonitor-settings.c +4 -4
@@ -60,7 +60,7 @@ static enum fsmonitor_reason check_remote(struct repository *r) }#endif -static enum fsmonitor_reason check_for_incompatible (struct repository *r) +static enum fsmonitor_reason check_for_incompatible (struct repository *r,int ipc) {if (!r->worktree) {/*
@@ -77,7 +77,7 @@ static enum fsmonitor_reason check_for_incompatible(struct repository *r) reason =check_remote (r);if (reason !=FSMONITOR_REASON_OK )return reason; - reason =fsm_os__incompatible (r); + reason =fsm_os__incompatible (r, ipc);if (reason !=FSMONITOR_REASON_OK )return reason; }
@@ -162,7 +162,7 @@ const char *fsm_settings__get_hook_path(struct repository *r)void fsm_settings__set_ipc (struct repository *r) { -enum fsmonitor_reason reason =check_for_incompatible (r); +enum fsmonitor_reason reason =check_for_incompatible (r,1 );if (reason !=FSMONITOR_REASON_OK ) {fsm_settings__set_incompatible (r, reason);
@@ -185,7 +185,7 @@ void fsm_settings__set_ipc(struct repository *r)void fsm_settings__set_hook (struct repository *r,const char *path) { -enum fsmonitor_reason reason =check_for_incompatible (r); +enum fsmonitor_reason reason =check_for_incompatible (r,0 );if (reason !=FSMONITOR_REASON_OK ) {fsm_settings__set_incompatible (r, reason);
fsmonitor-settings.h +1 -1
@@ -48,7 +48,7 @@ struct fsmonitor_settings;* fsm_os__* routines should considered private to fsm_settings__ * routines. */ -enum fsmonitor_reason fsm_os__incompatible (struct repository *r); +enum fsmonitor_reason fsm_os__incompatible (struct repository *r,int ipc);#endif /* HAVE_FSMONITOR_OS_SETTINGS */ #endif /* FSMONITOR_SETTINGS_H */
Signature · 1 line
gitgitgadget
PATCH v13 4/6 fsmonitor: deal with synthetic firmlinks on macOS
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
Starting with macOS 10.15 (Catalina), Apple introduced a new feature called 'firmlinks' in order to separate the boot volume into two volumes, one read-only and one writable but still present them to the user as a single volume. Along with this change, Apple removed the ability to create symlinks in the root directory and replaced them with 'synthetic firmlinks'. See 'man synthetic.conf'
When FSEevents reports the path of changed files, if the path involves a synthetic firmlink, the path is reported from the point of the synthetic firmlink and not the real path. For example:
Real path:
/System/Volumes/Data/network/working/directory/foo.txt
Synthetic firmlink:
/network -> /System/Volumes/Data/network
FSEvents path:
/network/working/directory/foo.txt
This causes the FSEvents path to not match against the worktree directory.
There are several ways in which synthetic firmlinks can be created:
they can be defined in /etc/synthetic.conf, the automounter can create them, and there may be other means. Simply reading /etc/synthetic.conf is insufficient. No matter what process creates synthetic firmlinks, they all get created in the root directory.
Therefore, in order to deal with synthetic firmlinks, the root directory is scanned and the first possible synthetic firmink that, when resolved, is a prefix of the worktree is used to map FSEvents paths to worktree paths.
--- builtin/fsmonitor--daemon.c | 10 +++ compat/fsmonitor/fsm-listen-darwin.c | 14 +++- compat/fsmonitor/fsm-path-utils-darwin.c | 93 ++++++++++++++++++++++++ compat/fsmonitor/fsm-path-utils-win32.c | 17 +++++ fsmonitor--daemon.h | 3 + fsmonitor-path-utils.h | 36 ++++++++- 6 files changed, 170 insertions(+), 3 deletions(-)
builtin/fsmonitor--daemon.c +10 -0
@@ -3,6 +3,7 @@#include "parse-options.h" #include "fsmonitor.h" #include "fsmonitor-ipc.h" +#include "fsmonitor-path-utils.h" #include "compat/fsmonitor/fsm-health.h" #include "compat/fsmonitor/fsm-listen.h" #include "fsmonitor--daemon.h"
@@ -1282,6 +1283,13 @@ static int fsmonitor_run_daemon(void)strbuf_addstr (&state.path_worktree_watch,absolute_path (get_git_work_tree ())); state.nr_paths_watching =1 ; +strbuf_init (&state.alias.alias,0 ); +strbuf_init (&state.alias.points_to,0 ); +if (fsmonitor__get_alias (state.path_worktree_watch.buf, &state.alias)) { + err =error (_ ("could not get worktree alias" )); + goto done; + } +/* * We create and delete cookie files somewhere inside the .git * directory to help us keep sync with the file system. If
@@ -1391,6 +1399,8 @@ done:strbuf_release (&state.path_gitdir_watch);strbuf_release (&state.path_cookie_prefix);strbuf_release (&state.path_ipc); +strbuf_release (&state.alias.alias); +strbuf_release (&state.alias.points_to);return err; }
compat/fsmonitor/fsm-listen-darwin.c +12 -2
@@ -26,6 +26,7 @@#include "fsmonitor.h" #include "fsm-listen.h" #include "fsmonitor--daemon.h" +#include "fsmonitor-path-utils.h" struct fsm_listen_data {
@@ -198,8 +199,9 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,struct string_list cookie_list =STRING_LIST_INIT_DUP ;const char *path_k;const char *slash; -int k; +char *resolved =NULL ;struct strbuf tmp =STRBUF_INIT ; +int k;/* * Build a list of all filesystem changes into a private/local
@@ -209,7 +211,12 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,/* * On Mac, we receive an array of absolute paths. */ - path_k = paths[k]; +free (resolved); + resolved =fsmonitor__resolve_alias (paths[k], &state->alias); +if (resolved) + path_k = resolved; +else + path_k = paths[k];/* * If you want to debug FSEvents, log them to GIT_TRACE_FSMONITOR.
@@ -238,6 +245,7 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,fsmonitor_force_resync (state);fsmonitor_batch__free_list (batch);string_list_clear (&cookie_list,0 ); + batch =NULL ;/* * We assume that any events that we received
@@ -360,12 +368,14 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef, } } +free (resolved);fsmonitor_publish (state, batch, &cookie_list);string_list_clear (&cookie_list,0 );strbuf_release (&tmp);return ; force_shutdown: +free (resolved);fsmonitor_batch__free_list (batch);string_list_clear (&cookie_list,0 );
compat/fsmonitor/fsm-path-utils-darwin.c +93 -0
101 lines not on this page · raw message
compat/fsmonitor/fsm-path-utils-win32.c +17 -0
@@ -126,3 +126,20 @@ int fsmonitor__is_fs_remote(const char *path)return -1 ;return fs.is_remote; } + +/* +* No-op for now. +*/ +int fsmonitor__get_alias (const char *path,struct alias_info *info) +{ +return 0 ; +} + +/* +* No-op for now. +*/ +char *fsmonitor__resolve_alias (const char *path, +const struct alias_info *info) +{ +return NULL ; +}
fsmonitor--daemon.h +3 -0
@@ -8,6 +8,7 @@#include "run-command.h" #include "simple-ipc.h" #include "thread-utils.h" +#include "fsmonitor-path-utils.h" struct fsmonitor_batch ;struct fsmonitor_token_data ;
@@ -43,6 +44,7 @@ struct fsmonitor_daemon_state {struct strbuf path_worktree_watch;struct strbuf path_gitdir_watch; +struct alias_info alias;int nr_paths_watching;struct fsmonitor_token_data *current_token_data;
@@ -59,6 +61,7 @@ struct fsmonitor_daemon_state {struct ipc_server_data *ipc_server_data;struct strbuf path_ipc; + };/*
fsmonitor-path-utils.h +35 -1
@@ -1,13 +1,21 @@#ifndef FSM_PATH_UTILS_H #define FSM_PATH_UTILS_H +#include "strbuf.h" + +struct alias_info +{ +struct strbuf alias; +struct strbuf points_to; +}; +struct fs_info {int is_remote;char *typename; };/* -* Get some basic filesystem informtion for the given path +* Get some basic filesystem information for the given path * * Returns -1 on error, zero otherwise. */
@@ -20,4 +28,30 @@ int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info);*/ int fsmonitor__is_fs_remote (const char *path); +/* +* Get the alias in given path, if any. +* +* Sets alias to the first alias that matches any part of the path. +* +* If an alias is found, info.alias and info.points_to are set to the +* found mapping. +* +* Returns -1 on error, 0 otherwise. +* +* The caller owns the storage that is occupied by info.alias and +* info.points_to and is responsible for releasing it. +*/ +int fsmonitor__get_alias (const char *path,struct alias_info *info); + +/* +* Resolve the path against the given alias. +* +* Returns the resolved path if there is one, NULL otherwise. +* +* The caller owns the storage that the returned string occupies and +* is responsible for releasing it. +*/ +char *fsmonitor__resolve_alias (const char *path, +const struct alias_info *info); +#endif
Signature · 1 line
gitgitgadget
Re: [PATCH v13 4/6] fsmonitor: deal with synthetic firmlinks on macOS
builtin/fsmonitor--daemon.c static int fsmonitor_run_daemon(void) · line 1289
Quoted 6 lines · Eric DeCosta via GitGitGadget
From: Eric DeCosta <[email protected]>
builtin/fsmonitor--daemon.c +2
-0+if (fsmonitor__get_alias (state.path_worktree_watch.buf, &state.alias)) { + err =error (_ ("could not get worktree alias" ));
We could keep this, but if we'd error() in the function itself we'd just emit two error() lines when one would do.
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 70
Quoted 1 line
compat/fsmonitor/fsm-path-utils-darwin.c +1
-0+ dir =opendir ("/" );
We should have a:
const char *const dir = "/";
So we can pass it to this..
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 72
Quoted 2 lines
compat/fsmonitor/fsm-path-utils-darwin.c +2
-0+if (!dir) { +error_errno ("opendir('/') failed" );
...and as a parameter to a _()-translated string. That way the next translation doesn't need to translate opendir('/home') or whatever.
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 73
Quoted 1 line
compat/fsmonitor/fsm-path-utils-darwin.c +1
-0+return -1 ;
Just skip the braces and do "return error_errno()", these functions return -1 for thath reason.
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 79
Quoted 4 lines
compat/fsmonitor/fsm-path-utils-darwin.c +3
-0+strbuf_init (&alias,256 ); +/* no way of knowing what the link will resolve to, so MAXPATHLEN */ +strbuf_init (&points_to,MAXPATHLEN );
Still need manual memory juggling? Ok.
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 84
Quoted 5 lines
compat/fsmonitor/fsm-path-utils-darwin.c +5
-0+ +while ((de =readdir (dir)) !=NULL ) { +strbuf_reset (&alias); +strbuf_addch (&alias,'/' ); +strbuf_add (&alias, de->d_name,strlen (de->d_name));
strbuf_addf(&alias, "/%s", de->d_name);
Or rather:
strbuf_addf(&alias, "%s%s", root_dir, de->d_name);
If you split that "/" into a variable?
Quoted 16 lines
compat/fsmonitor/fsm-path-utils-darwin.c +16
-0+ + read =readlink (alias.buf, points_to.buf,MAXPATHLEN ); +if (read >0 ) { +strbuf_setlen (&points_to, read); +if ((!strncmp (points_to.buf, path, points_to.len)) + && path[points_to.len] =='/' ) { +strbuf_addbuf (&info->alias, &alias); +strbuf_addbuf (&info->points_to, &points_to); +trace_printf_key (&trace_fsmonitor, +"Found alias for '%s' : '%s' -> '%s'" , + path, info->alias.buf, info->points_to.buf); + retval =0 ; + goto done; + } + }else if (!read) { +BUG ("readlink returned 0" );
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 110
Quoted 10 lines
compat/fsmonitor/fsm-path-utils-darwin.c +10
-0+ }else if (errno !=EINVAL ) {/* Something other than not a link */ +error_errno ("readlink('%s') failed" , alias.buf); + goto done; + } + } + retval =0 ;/* no alias */ + +done: +if (closedir (dir) <0 ) +warning_errno ("closedir('/') failed" );
Why not return an error for this? If you can't close the dir that usually means the write or similar didn't work.
PATCH v13 5/6 fsmonitor: check for compatability before communicating with fsmonitor
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
If fsmonitor is not in a compatible state, warn with an appropriate message.
--- compat/fsmonitor/fsm-settings-darwin.c | 2 +- fsmonitor-settings.c | 10 +++++++--- fsmonitor-settings.h | 2 +- fsmonitor.c | 7 +++++++ 4 files changed, 16 insertions(+), 5 deletions(-)
compat/fsmonitor/fsm-settings-darwin.c +1 -1
@@ -38,7 +38,7 @@ static enum fsmonitor_reason check_uds_volume(struct repository *r)strbuf_release (&path);if (fs.is_remote) -return FSMONITOR_REASON_REMOTE ; +return FSMONITOR_REASON_NOSOCKETS ;if (!strcmp (fs.typename,"msdos" ))/* aka FAT32 */ return FSMONITOR_REASON_NOSOCKETS ;
fsmonitor-settings.c +7 -3
@@ -1,6 +1,7 @@#include "cache.h" #include "config.h" #include "repository.h" +#include "fsmonitor-ipc.h" #include "fsmonitor-settings.h" #include "fsmonitor-path-utils.h"
@@ -242,10 +243,11 @@ enum fsmonitor_reason fsm_settings__get_reason(struct repository *r)return r->settings.fsmonitor->reason; } -char *fsm_settings__get_incompatible_msg (const struct repository *r, +char *fsm_settings__get_incompatible_msg (struct repository *r,enum fsmonitor_reason reason) {struct strbuf msg =STRBUF_INIT ; +const char *socket_dir;switch (reason) {case FSMONITOR_REASON_UNTESTED :
@@ -281,9 +283,11 @@ char *fsm_settings__get_incompatible_msg(const struct repository *r, goto done;case FSMONITOR_REASON_NOSOCKETS : + socket_dir =dirname ((char *)fsmonitor_ipc__get_path (r));strbuf_addf (&msg, -_ ("repository '%s' is incompatible with fsmonitor due to lack of Unix sockets" ), - r->worktree); +_ ("socket directory '%s' is incompatible with fsmonitor due" +" to lack of Unix sockets support" ), + socket_dir); goto done; }
fsmonitor-settings.h +1 -1
@@ -33,7 +33,7 @@ enum fsmonitor_mode fsm_settings__get_mode(struct repository *r);const char *fsm_settings__get_hook_path (struct repository *r);enum fsmonitor_reason fsm_settings__get_reason (struct repository *r); -char *fsm_settings__get_incompatible_msg (const struct repository *r, +char *fsm_settings__get_incompatible_msg (struct repository *r,enum fsmonitor_reason reason);struct fsmonitor_settings ;
fsmonitor.c +7 -0
@@ -295,6 +295,7 @@ static int fsmonitor_force_update_threshold = 100;void refresh_fsmonitor (struct index_state *istate) { +static int warn_once =0 ;struct strbuf query_result =STRBUF_INIT ;int query_success =0 , hook_version =-1 ;size_t bol =0 ;/* beginning of line */
@@ -305,6 +306,12 @@ void refresh_fsmonitor(struct index_state *istate)int is_trivial =0 ;struct repository *r = istate->repo ? istate->repo : the_repository;enum fsmonitor_mode fsm_mode =fsm_settings__get_mode (r); +enum fsmonitor_reason reason =fsm_settings__get_reason (r); + +if (!warn_once && reason >FSMONITOR_REASON_OK ) { + warn_once =1 ; +warning ("%s" ,fsm_settings__get_incompatible_msg (r, reason)); + }if (fsm_mode <=FSMONITOR_MODE_DISABLED || istate->fsmonitor_has_run_once)
Signature · 1 line
gitgitgadget
PATCH v13 6/6 fsmonitor: add documentation for allowRemote and socketDir options
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
Add documentation for 'fsmonitor.allowRemote' and 'fsmonitor.socketDir'. Call-out experimental nature of 'fsmonitor.allowRemote' and limited filesystem support for 'fsmonitor.socketDir'.
--- Documentation/config.txt | 2 ++ Documentation/config/fsmonitor--daemon.txt | 11 +++++++ Documentation/git-fsmonitor--daemon.txt | 37 ++++++++++++++++++++-- 3 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 Documentation/config/fsmonitor--daemon.txt
Documentation/config.txt +2 -0
@@ -423,6 +423,8 @@ include::config/filter.txt[] include::config/fsck.txt[] +include::config/fsmonitor--daemon.txt[] + include::config/gc.txt[] include::config/gitcvs.txt[]
Documentation/config/fsmonitor--daemon.txt new file +11 -0
@@ -0,0 +1,11 @@ +fsmonitor.allowRemote:: + By default, the fsmonitor daemon refuses to work against network-mounted + repositories. Setting `fsmonitor.allowRemote` to `true` overrides this + behavior. Only respected when `core.fsmonitor` is set to `true`. + +fsmonitor.socketDir:: + This Mac OS-specific option, if set, specifies the directory in + which to create the Unix domain socket used for communication + between the fsmonitor daemon and various Git commands. The directory must + reside on a native Mac OS filesystem. Only respected when `core.fsmonitor` + is set to `true`.
Documentation/git-fsmonitor--daemon.txt +34 -3
@@ -3,7 +3,7 @@ git-fsmonitor{litdd}daemon(1) NAME ---- -git-fsmonitor--daemon - A Built-in File System Monitor +git-fsmonitor--daemon - A Built-in Filesystem Monitor SYNOPSIS --------
@@ -17,7 +17,7 @@ DESCRIPTION ----------- A daemon to watch the working directory for file and directory -changes using platform-specific file system notification facilities. +changes using platform-specific filesystem notification facilities. This daemon communicates directly with commands like `git status` using the link:technical/api-simple-ipc.html[simple IPC] interface
@@ -63,13 +63,44 @@ CAVEATS ------- The fsmonitor daemon does not currently know about submodules and does -not know to filter out file system events that happen within a +not know to filter out filesystem events that happen within a submodule. If fsmonitor daemon is watching a super repo and a file is modified within the working directory of a submodule, it will report the change (as happening against the super repo). However, the client will properly ignore these extra events, so performance may be affected but it will not cause an incorrect result. +By default, the fsmonitor daemon refuses to work against network-mounted +repositories; this may be overridden by setting `fsmonitor.allowRemote` to +`true`. Note, however, that the fsmonitor daemon is not guaranteed to work +correctly with all network-mounted repositories and such use is considered +experimental. + +On Mac OS, the inter-process communication (IPC) between various Git +commands and the fsmonitor daemon is done via a Unix domain socket (UDS) -- a +special type of file -- which is supported by native Mac OS filesystems, +but not on network-mounted filesystems, NTFS, or FAT32. Other filesystems +may or may not have the needed support; the fsmonitor daemon is not guaranteed +to work with these filesystems and such use is considered experimental. + +By default, the socket is created in the `.git` directory, however, if the +`.git` directory is on a network-mounted filesystem, it will be instead be +created at `$HOME/.git-fsmonitor-*` unless `$HOME` itself is on a +network-mounted filesystem in which case you must set the configuration +variable `fsmonitor.socketDir` to the path of a directory on a Mac OS native +filesystem in which to create the socket file. + +If none of the above directories (`.git`, `$HOME`, or `fsmonitor.socketDir`) +is on a native Mac OS file filesystem the fsmonitor daemon will report an +error that will cause the daemon and the currently running command to exit. + +CONFIGURATION +------------- + +include::includes/cmd-config-section-all.txt[] + +include::config/fsmonitor--daemon.txt[] + GIT --- Part of the linkgit:git[1] suite
Signature · 1 line
gitgitgadget
v14 cover fsmonitor: option to allow fsmonitor to run against network-mounted repos
In reply to Eric DeCosta via GitGitGadgetFollow-on to the work done to allow Windows to work against network-mounted repos for macOS.
Have macOS take advantage of the same configuration option, 'fsmonitor.allowRemote' that was introduced for Windows. Setting this option to true will override the default behavior (erroring-out) when a network-mounted repo is detected by fsmonitor.
The added wrinkle being that the Unix domain socket (UDS) file used for IPC cannot be created in a network location; instead $HOME is used if the default location is on the network. The user may, optionally, set the 'fsmonitor.socketDir' configuration option to a valid, local directory if $HOME itself is on the network or is simply not the desired location for the UDS file.
An additional issue is that for mount points in the root directory, FSEvents does not report a path that matches the worktree directory due to the introduction of 'synthetic firmlinks'. fsmonitor must map the FSEvents paths to the worktree directory by interrogating the root filesystem for synthetic firmlinks and using that information to translate the path.
v14 differs from v13:
* code review feedback
v13 differs from v12:
* code review feedback
v12 differs from v11:
* bug fixes
v11 differs from v10:
* incorporates code review feedback
* fix memory leak in fsm-listen-darwin.c
v10 differs from v9:
* incorporates code review feedback
* improves error messaging for incompatible socket directory
v9 differs from v8:
* incorporates code review feedback
* check for incompatibility before communicating with fsmonitor
v8 differs from v7:
* incorporates code review feedback
* gets the rebase right
v7 differs from v6:
* incorporates code review feedback
v6 differs from v5:
* incorporates earlier, Windows-specific changes that have not made it back yet to the master branch
* incorporates code review feedback
* adds documentation for 'fsmonitor.allowRemote' and 'fsmonitor.socketDir'
v5 differs significantly from earlier versions:
* redesign of handling 'fsmonitor.allowRemote' and 'fsmonitor.socketDir' such that these options are no longer added to the settings data structure but are rather read from config at point of use
* refactoring of code for handling platform-specific file system checks via a common interface to avoid platform #ifdef in IPC code and be in-model with other platform-specific fsmonitor code
* dealing with 'synthetic firmlinks' on macOS
Eric DeCosta (6):
fsmonitor: refactor filesystem checks to common interface
fsmonitor: relocate socket file if .git directory is remote
fsmonitor: avoid socket location check if using hook
fsmonitor: deal with synthetic firmlinks on macOS
fsmonitor: check for compatability before communicating with fsmonitor
fsmonitor: add documentation for allowRemote and socketDir options
Documentation/config.txt | 2 + Documentation/config/fsmonitor--daemon.txt | 11 ++ Documentation/git-fsmonitor--daemon.txt | 37 ++++- Makefile | 2 + builtin/fsmonitor--daemon.c | 11 +- compat/fsmonitor/fsm-ipc-darwin.c | 52 ++++++ compat/fsmonitor/fsm-ipc-win32.c | 9 ++ compat/fsmonitor/fsm-listen-darwin.c | 14 +- compat/fsmonitor/fsm-path-utils-darwin.c | 132 ++++++++++++++++ compat/fsmonitor/fsm-path-utils-win32.c | 145 +++++++++++++++++ compat/fsmonitor/fsm-settings-darwin.c | 72 +++------ compat/fsmonitor/fsm-settings-win32.c | 174 +-------------------- contrib/buildsystems/CMakeLists.txt | 4 + fsmonitor--daemon.h | 3 + fsmonitor-ipc.c | 18 +-- fsmonitor-ipc.h | 4 +- fsmonitor-path-utils.h | 57 +++++++ fsmonitor-settings.c | 68 +++++++- fsmonitor-settings.h | 4 +- fsmonitor.c | 7 + 20 files changed, 580 insertions(+), 246 deletions(-) create mode 100644 Documentation/config/fsmonitor--daemon.txt create mode 100644 compat/fsmonitor/fsm-ipc-darwin.c create mode 100644 compat/fsmonitor/fsm-ipc-win32.c create mode 100644 compat/fsmonitor/fsm-path-utils-darwin.c create mode 100644 compat/fsmonitor/fsm-path-utils-win32.c create mode 100644 fsmonitor-path-utils.h
base-commit: 2a7d63a2453e2c30353342a2c9385fa22a846987
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1326%2Fedecosta-mw%2Ffsmonitor_macos-v14
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1326/edecosta-mw/fsmonitor_macos-v14
Pull-Request: https://github.com/gitgitgadget/git/pull/1326
Range-diff vs v13:
1: 0b9b64428c5 = 1: 0b9b64428c5 fsmonitor: refactor filesystem checks to common interface
2: 680d4c83f99 = 2: 680d4c83f99 fsmonitor: relocate socket file if .git directory is remote
3: 7987d0c1f33 = 3: 7987d0c1f33 fsmonitor: avoid socket location check if using hook
4: 324eb5acd85 ! 4: 241043b7c15 fsmonitor: deal with synthetic firmlinks on macOS
@@ builtin/fsmonitor--daemon.c: static int fsmonitor_run_daemon(void)
+ strbuf_init(&state.alias.alias, 0);
+ strbuf_init(&state.alias.points_to, 0);
-+ if (fsmonitor__get_alias(state.path_worktree_watch.buf, &state.alias)) {
-+ err = error(_("could not get worktree alias"));
++ if ((err = fsmonitor__get_alias(state.path_worktree_watch.buf, &state.alias)))
+ goto done;
-+ }
+
/*
* We create and delete cookie files somewhere inside the .git
@@ compat/fsmonitor/fsm-path-utils-darwin.c: int fsmonitor__is_fs_remote(const char
+int fsmonitor__get_alias(const char *path, struct alias_info *info)
+{
+ DIR *dir;
-+ int read;
+ int retval = -1;
++ const char *const root = "/";
++ struct stat st;
+ struct dirent *de;
+ struct strbuf alias;
-+ struct strbuf points_to;
++ struct strbuf points_to = STRBUF_INIT;
+
-+ dir = opendir("/");
-+ if (!dir) {
-+ error_errno("opendir('/') failed");
-+ return -1;
-+ }
++ dir = opendir(root);
++ if (!dir)
++ return error_errno(_("opendir('%s') failed"), root);
+
+ strbuf_init(&alias, 256);
+
-+ /* no way of knowing what the link will resolve to, so MAXPATHLEN */
-+ strbuf_init(&points_to, MAXPATHLEN);
-+
+ while ((de = readdir(dir)) != NULL) {
+ strbuf_reset(&alias);
-+ strbuf_addch(&alias, '/');
-+ strbuf_add(&alias, de->d_name, strlen(de->d_name));
++ strbuf_addf(&alias, "%s%s", root, de->d_name);
+
-+ read = readlink(alias.buf, points_to.buf, MAXPATHLEN);
-+ if (read > 0) {
-+ strbuf_setlen(&points_to, read);
-+ if ((!strncmp(points_to.buf, path, points_to.len))
-+ && path[points_to.len] == '/') {
-+ strbuf_addbuf(&info->alias, &alias);
-+ strbuf_addbuf(&info->points_to, &points_to);
-+ trace_printf_key(&trace_fsmonitor,
-+ "Found alias for '%s' : '%s' -> '%s'",
-+ path, info->alias.buf, info->points_to.buf);
-+ retval = 0;
-+ goto done;
-+ }
-+ } else if (!read) {
-+ BUG("readlink returned 0");
-+ } else if (errno != EINVAL) { /* Something other than not a link */
-+ error_errno("readlink('%s') failed", alias.buf);
++ if (lstat(alias.buf, &st) < 0) {
++ error_errno(_("lstat('%s') failed"), alias.buf);
++ goto done;
++ }
++
++ if (!S_ISLNK(st.st_mode))
++ continue;
++
++ if (strbuf_readlink(&points_to, alias.buf, st.st_size) < 0) {
++ error_errno(_("strbuf_readlink('%s') failed"), alias.buf);
++ goto done;
++ }
++
++ if (!strncmp(points_to.buf, path, points_to.len) &&
++ (path[points_to.len] == '/')) {
++ strbuf_addbuf(&info->alias, &alias);
++ strbuf_addbuf(&info->points_to, &points_to);
++ trace_printf_key(&trace_fsmonitor,
++ "Found alias for '%s' : '%s' -> '%s'",
++ path, info->alias.buf, info->points_to.buf);
++ retval = 0;
+ goto done;
+ }
+ }
+ retval = 0; /* no alias */
+
+done:
-+ if (closedir(dir) < 0)
-+ warning_errno("closedir('/') failed");
+ strbuf_release(&alias);
+ strbuf_release(&points_to);
++ if (closedir(dir) < 0)
++ return error_errno(_("closedir('%s') failed"), root);
+ return retval;
+}
+
5: b1ea378dff7 = 5: d906debba5e fsmonitor: check for compatability before communicating with fsmonitor
6: 04f607b1f21 = 6: ed14fbd009e fsmonitor: add documentation for allowRemote and socketDir optionsSignature · 1 line
gitgitgadget
PATCH v14 1/6 fsmonitor: refactor filesystem checks to common interface
From: Eric DeCosta <[email protected]>
Provide a common interface for getting basic filesystem information including filesystem type and whether the filesystem is remote.
Refactor existing code for getting basic filesystem info and detecting remote file systems to the new interface.
Refactor filesystem checks to leverage new interface. For macOS, error-out if the Unix Domain socket (UDS) file is on a remote filesystem.
--- Makefile | 1 + compat/fsmonitor/fsm-path-utils-darwin.c | 40 ++++++ compat/fsmonitor/fsm-path-utils-win32.c | 128 +++++++++++++++++ compat/fsmonitor/fsm-settings-darwin.c | 62 +++----- compat/fsmonitor/fsm-settings-win32.c | 172 +---------------------- contrib/buildsystems/CMakeLists.txt | 2 + fsmonitor-path-utils.h | 23 +++ fsmonitor-settings.c | 50 +++++++ 8 files changed, 263 insertions(+), 215 deletions(-) create mode 100644 compat/fsmonitor/fsm-path-utils-darwin.c create mode 100644 compat/fsmonitor/fsm-path-utils-win32.c create mode 100644 fsmonitor-path-utils.h
Makefile +1 -0
@@ -2044,6 +2044,7 @@ endif ifdef FSMONITOR_OS_SETTINGS COMPAT_CFLAGS += -DHAVE_FSMONITOR_OS_SETTINGS COMPAT_OBJS += compat/fsmonitor/fsm-settings-$(FSMONITOR_OS_SETTINGS).o + COMPAT_OBJS += compat/fsmonitor/fsm-path-utils-$(FSMONITOR_OS_SETTINGS).o endif ifeq ($(TCLTK_PATH),)
compat/fsmonitor/fsm-path-utils-darwin.c new file +40 -0
@@ -0,0 +1,40 @@ +#include "fsmonitor.h" +#include "fsmonitor-path-utils.h" +#include <sys/param.h> +#include <sys/mount.h> + +int fsmonitor__get_fs_info (const char *path,struct fs_info *fs_info) +{ +struct statfs fs; +if (statfs (path, &fs) ==-1 ) { +int saved_errno = errno; +trace_printf_key (&trace_fsmonitor,"statfs('%s') failed: %s" , + path,strerror (saved_errno)); + errno = saved_errno; +return -1 ; + } + +trace_printf_key (&trace_fsmonitor, +"statfs('%s') [type 0x%08x][flags 0x%08x] '%s'" , + path, fs.f_type, fs.f_flags, fs.f_fstypename); + +if (!(fs.f_flags &MNT_LOCAL )) + fs_info->is_remote =1 ; +else + fs_info->is_remote =0 ; + + fs_info->typename = fs.f_fstypename; + +trace_printf_key (&trace_fsmonitor, +"'%s' is_remote: %d" , + path, fs_info->is_remote); +return 0 ; +} + +int fsmonitor__is_fs_remote (const char *path) +{ +struct fs_info fs; +if (fsmonitor__get_fs_info (path, &fs)) +return -1 ; +return fs.is_remote; +}
compat/fsmonitor/fsm-path-utils-win32.c new file +128 -0
128 lines not on this page · raw message
compat/fsmonitor/fsm-settings-darwin.c +17 -45
96 lines not on this page · raw message
compat/fsmonitor/fsm-settings-win32.c +2 -170
190 lines not on this page · raw message
contrib/buildsystems/CMakeLists.txt +2 -0
@@ -308,6 +308,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-win32.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-win32.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-win32.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-win32.c)
@@ -315,6 +316,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-darwin.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-darwin.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-darwin.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-darwin.c)
fsmonitor-path-utils.h new file +23 -0
@@ -0,0 +1,23 @@ +#ifndef FSM_PATH_UTILS_H +#define FSM_PATH_UTILS_H + +struct fs_info { +int is_remote; +char *typename; +}; + +/* +* Get some basic filesystem informtion for the given path +* +* Returns -1 on error, zero otherwise. +*/ +int fsmonitor__get_fs_info (const char *path,struct fs_info *fs_info); + +/* +* Determines if the filesystem that path resides on is remote. +* +* Returns -1 on error, 0 if not remote, 1 if remote. +*/ +int fsmonitor__is_fs_remote (const char *path); + +#endif
fsmonitor-settings.c +50 -0
68 lines not on this page · raw message
Signature · 1 line
gitgitgadget
PATCH v14 2/6 fsmonitor: relocate socket file if .git directory is remote
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
If the .git directory is on a remote filesystem, create the socket file in 'fsmonitor.socketDir' if it is defined, else create it in $HOME.
--- Makefile | 1 + builtin/fsmonitor--daemon.c | 3 +- compat/fsmonitor/fsm-ipc-darwin.c | 52 ++++++++++++++++++++++++++ compat/fsmonitor/fsm-ipc-win32.c | 9 +++++ compat/fsmonitor/fsm-settings-darwin.c | 2 +- contrib/buildsystems/CMakeLists.txt | 2 + fsmonitor-ipc.c | 18 ++++----- fsmonitor-ipc.h | 4 +- 8 files changed, 78 insertions(+), 13 deletions(-) create mode 100644 compat/fsmonitor/fsm-ipc-darwin.c create mode 100644 compat/fsmonitor/fsm-ipc-win32.c
Makefile +1 -0
@@ -2039,6 +2039,7 @@ ifdef FSMONITOR_DAEMON_BACKEND COMPAT_CFLAGS += -DHAVE_FSMONITOR_DAEMON_BACKEND COMPAT_OBJS += compat/fsmonitor/fsm-listen-$(FSMONITOR_DAEMON_BACKEND).o COMPAT_OBJS += compat/fsmonitor/fsm-health-$(FSMONITOR_DAEMON_BACKEND).o + COMPAT_OBJS += compat/fsmonitor/fsm-ipc-$(FSMONITOR_DAEMON_BACKEND).o endif ifdef FSMONITOR_OS_SETTINGS
builtin/fsmonitor--daemon.c +2 -1
@@ -1343,7 +1343,8 @@ static int fsmonitor_run_daemon(void)* directory.) */ strbuf_init (&state.path_ipc,0 ); -strbuf_addstr (&state.path_ipc,absolute_path (fsmonitor_ipc__get_path ())); +strbuf_addstr (&state.path_ipc, +absolute_path (fsmonitor_ipc__get_path (the_repository)));/* * Confirm that we can create platform-specific resources for the
compat/fsmonitor/fsm-ipc-darwin.c new file +52 -0
52 lines not on this page · raw message
compat/fsmonitor/fsm-ipc-win32.c new file +9 -0
@@ -0,0 +1,9 @@ +#include "config.h" +#include "fsmonitor-ipc.h" + +const char *fsmonitor_ipc__get_path (struct repository *r) { +static char *ret; +if (!ret) + ret =git_pathdup ("fsmonitor--daemon.ipc" ); +return ret; +}
compat/fsmonitor/fsm-settings-darwin.c +1 -1
@@ -26,7 +26,7 @@static enum fsmonitor_reason check_uds_volume (struct repository *r) {struct fs_info fs; -const char *ipc_path =fsmonitor_ipc__get_path (); +const char *ipc_path =fsmonitor_ipc__get_path (r);struct strbuf path =STRBUF_INIT ;strbuf_add (&path, ipc_path,strlen (ipc_path));
contrib/buildsystems/CMakeLists.txt +2 -0
@@ -308,6 +308,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-win32.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-win32.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-win32.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-win32.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS)
@@ -316,6 +317,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-darwin.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-darwin.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-darwin.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-darwin.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS)
fsmonitor-ipc.c +8 -10
@@ -18,7 +18,7 @@ int fsmonitor_ipc__is_supported(void)return 0 ; } -const char *fsmonitor_ipc__get_path (void ) +const char *fsmonitor_ipc__get_path (struct repository *r) {return NULL ; }
@@ -47,11 +47,9 @@ int fsmonitor_ipc__is_supported(void)return 1 ; } -GIT_PATH_FUNC(fsmonitor_ipc__get_path, "fsmonitor--daemon.ipc") -enum ipc_active_state fsmonitor_ipc__get_state (void ) { -return ipc_get_active_state (fsmonitor_ipc__get_path ()); +return ipc_get_active_state (fsmonitor_ipc__get_path (the_repository)); }static int spawn_daemon (void )
@@ -81,8 +79,8 @@ int fsmonitor_ipc__send_query(const char *since_token,trace2_data_string ("fsm_client" ,NULL ,"query/command" , tok); try_again: - state =ipc_client_try_connect (fsmonitor_ipc__get_path (), &options, - &connection); + state =ipc_client_try_connect (fsmonitor_ipc__get_path (the_repository), + &options, &connection);switch (state) {case IPC_STATE__LISTENING :
@@ -117,13 +115,13 @@ try_again:case IPC_STATE__INVALID_PATH : ret =error (_ ("fsmonitor_ipc__send_query: invalid path '%s'" ), -fsmonitor_ipc__get_path ()); +fsmonitor_ipc__get_path (the_repository)); goto done;case IPC_STATE__OTHER_ERROR :default : ret =error (_ ("fsmonitor_ipc__send_query: unspecified error on '%s'" ), -fsmonitor_ipc__get_path ()); +fsmonitor_ipc__get_path (the_repository)); goto done; }
@@ -149,8 +147,8 @@ int fsmonitor_ipc__send_command(const char *command, options.wait_if_busy =1 ; options.wait_if_not_found =0 ; - state =ipc_client_try_connect (fsmonitor_ipc__get_path (), &options, - &connection); + state =ipc_client_try_connect (fsmonitor_ipc__get_path (the_repository), + &options, &connection);if (state !=IPC_STATE__LISTENING ) {die (_ ("fsmonitor--daemon is not running" ));return -1 ;
fsmonitor-ipc.h +3 -1
@@ -3,6 +3,8 @@#include "simple-ipc.h" +struct repository ; +/* * Returns true if built-in file system monitor daemon is defined * for this platform.
@@ -16,7 +18,7 @@ int fsmonitor_ipc__is_supported(void);* * Returns NULL if the daemon is not supported on this platform. */ -const char *fsmonitor_ipc__get_path (void ); +const char *fsmonitor_ipc__get_path (struct repository *r);/* * Try to determine whether there is a `git-fsmonitor--daemon` process
Signature · 1 line
gitgitgadget
PATCH v14 3/6 fsmonitor: avoid socket location check if using hook
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
If monitoring is done via fsmonitor hook rather than IPC there is no need to check if the location of the Unix Domain socket (UDS) file is on a remote filesystem.
--- compat/fsmonitor/fsm-settings-darwin.c | 10 ++++++---- compat/fsmonitor/fsm-settings-win32.c | 2 +- fsmonitor-settings.c | 8 ++++---- fsmonitor-settings.h | 2 +- 4 files changed, 12 insertions(+), 10 deletions(-)
compat/fsmonitor/fsm-settings-darwin.c +6 -4
@@ -49,13 +49,15 @@ static enum fsmonitor_reason check_uds_volume(struct repository *r)return FSMONITOR_REASON_OK ; } -enum fsmonitor_reason fsm_os__incompatible (struct repository *r) +enum fsmonitor_reason fsm_os__incompatible (struct repository *r,int ipc) {enum fsmonitor_reason reason; - reason =check_uds_volume (r); -if (reason !=FSMONITOR_REASON_OK ) -return reason; +if (ipc) { + reason =check_uds_volume (r); +if (reason !=FSMONITOR_REASON_OK ) +return reason; + }return FSMONITOR_REASON_OK ; }
compat/fsmonitor/fsm-settings-win32.c +1 -1
@@ -25,7 +25,7 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)return FSMONITOR_REASON_OK ; } -enum fsmonitor_reason fsm_os__incompatible (struct repository *r) +enum fsmonitor_reason fsm_os__incompatible (struct repository *r,int ipc) {enum fsmonitor_reason reason;
fsmonitor-settings.c +4 -4
@@ -60,7 +60,7 @@ static enum fsmonitor_reason check_remote(struct repository *r) }#endif -static enum fsmonitor_reason check_for_incompatible (struct repository *r) +static enum fsmonitor_reason check_for_incompatible (struct repository *r,int ipc) {if (!r->worktree) {/*
@@ -77,7 +77,7 @@ static enum fsmonitor_reason check_for_incompatible(struct repository *r) reason =check_remote (r);if (reason !=FSMONITOR_REASON_OK )return reason; - reason =fsm_os__incompatible (r); + reason =fsm_os__incompatible (r, ipc);if (reason !=FSMONITOR_REASON_OK )return reason; }
@@ -162,7 +162,7 @@ const char *fsm_settings__get_hook_path(struct repository *r)void fsm_settings__set_ipc (struct repository *r) { -enum fsmonitor_reason reason =check_for_incompatible (r); +enum fsmonitor_reason reason =check_for_incompatible (r,1 );if (reason !=FSMONITOR_REASON_OK ) {fsm_settings__set_incompatible (r, reason);
@@ -185,7 +185,7 @@ void fsm_settings__set_ipc(struct repository *r)void fsm_settings__set_hook (struct repository *r,const char *path) { -enum fsmonitor_reason reason =check_for_incompatible (r); +enum fsmonitor_reason reason =check_for_incompatible (r,0 );if (reason !=FSMONITOR_REASON_OK ) {fsm_settings__set_incompatible (r, reason);
fsmonitor-settings.h +1 -1
@@ -48,7 +48,7 @@ struct fsmonitor_settings;* fsm_os__* routines should considered private to fsm_settings__ * routines. */ -enum fsmonitor_reason fsm_os__incompatible (struct repository *r); +enum fsmonitor_reason fsm_os__incompatible (struct repository *r,int ipc);#endif /* HAVE_FSMONITOR_OS_SETTINGS */ #endif /* FSMONITOR_SETTINGS_H */
Signature · 1 line
gitgitgadget
PATCH v14 4/6 fsmonitor: deal with synthetic firmlinks on macOS
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
Starting with macOS 10.15 (Catalina), Apple introduced a new feature called 'firmlinks' in order to separate the boot volume into two volumes, one read-only and one writable but still present them to the user as a single volume. Along with this change, Apple removed the ability to create symlinks in the root directory and replaced them with 'synthetic firmlinks'. See 'man synthetic.conf'
When FSEevents reports the path of changed files, if the path involves a synthetic firmlink, the path is reported from the point of the synthetic firmlink and not the real path. For example:
Real path:
/System/Volumes/Data/network/working/directory/foo.txt
Synthetic firmlink:
/network -> /System/Volumes/Data/network
FSEvents path:
/network/working/directory/foo.txt
This causes the FSEvents path to not match against the worktree directory.
There are several ways in which synthetic firmlinks can be created:
they can be defined in /etc/synthetic.conf, the automounter can create them, and there may be other means. Simply reading /etc/synthetic.conf is insufficient. No matter what process creates synthetic firmlinks, they all get created in the root directory.
Therefore, in order to deal with synthetic firmlinks, the root directory is scanned and the first possible synthetic firmink that, when resolved, is a prefix of the worktree is used to map FSEvents paths to worktree paths.
--- builtin/fsmonitor--daemon.c | 8 +++ compat/fsmonitor/fsm-listen-darwin.c | 14 +++- compat/fsmonitor/fsm-path-utils-darwin.c | 92 ++++++++++++++++++++++++ compat/fsmonitor/fsm-path-utils-win32.c | 17 +++++ fsmonitor--daemon.h | 3 + fsmonitor-path-utils.h | 36 +++++++++- 6 files changed, 167 insertions(+), 3 deletions(-)
builtin/fsmonitor--daemon.c +8 -0
@@ -3,6 +3,7 @@#include "parse-options.h" #include "fsmonitor.h" #include "fsmonitor-ipc.h" +#include "fsmonitor-path-utils.h" #include "compat/fsmonitor/fsm-health.h" #include "compat/fsmonitor/fsm-listen.h" #include "fsmonitor--daemon.h"
@@ -1282,6 +1283,11 @@ static int fsmonitor_run_daemon(void)strbuf_addstr (&state.path_worktree_watch,absolute_path (get_git_work_tree ())); state.nr_paths_watching =1 ; +strbuf_init (&state.alias.alias,0 ); +strbuf_init (&state.alias.points_to,0 ); +if ((err =fsmonitor__get_alias (state.path_worktree_watch.buf, &state.alias))) + goto done; +/* * We create and delete cookie files somewhere inside the .git * directory to help us keep sync with the file system. If
@@ -1391,6 +1397,8 @@ done:strbuf_release (&state.path_gitdir_watch);strbuf_release (&state.path_cookie_prefix);strbuf_release (&state.path_ipc); +strbuf_release (&state.alias.alias); +strbuf_release (&state.alias.points_to);return err; }
compat/fsmonitor/fsm-listen-darwin.c +12 -2
@@ -26,6 +26,7 @@#include "fsmonitor.h" #include "fsm-listen.h" #include "fsmonitor--daemon.h" +#include "fsmonitor-path-utils.h" struct fsm_listen_data {
@@ -198,8 +199,9 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,struct string_list cookie_list =STRING_LIST_INIT_DUP ;const char *path_k;const char *slash; -int k; +char *resolved =NULL ;struct strbuf tmp =STRBUF_INIT ; +int k;/* * Build a list of all filesystem changes into a private/local
@@ -209,7 +211,12 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,/* * On Mac, we receive an array of absolute paths. */ - path_k = paths[k]; +free (resolved); + resolved =fsmonitor__resolve_alias (paths[k], &state->alias); +if (resolved) + path_k = resolved; +else + path_k = paths[k];/* * If you want to debug FSEvents, log them to GIT_TRACE_FSMONITOR.
@@ -238,6 +245,7 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,fsmonitor_force_resync (state);fsmonitor_batch__free_list (batch);string_list_clear (&cookie_list,0 ); + batch =NULL ;/* * We assume that any events that we received
@@ -360,12 +368,14 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef, } } +free (resolved);fsmonitor_publish (state, batch, &cookie_list);string_list_clear (&cookie_list,0 );strbuf_release (&tmp);return ; force_shutdown: +free (resolved);fsmonitor_batch__free_list (batch);string_list_clear (&cookie_list,0 );
compat/fsmonitor/fsm-path-utils-darwin.c +92 -0
100 lines not on this page · raw message
compat/fsmonitor/fsm-path-utils-win32.c +17 -0
@@ -126,3 +126,20 @@ int fsmonitor__is_fs_remote(const char *path)return -1 ;return fs.is_remote; } + +/* +* No-op for now. +*/ +int fsmonitor__get_alias (const char *path,struct alias_info *info) +{ +return 0 ; +} + +/* +* No-op for now. +*/ +char *fsmonitor__resolve_alias (const char *path, +const struct alias_info *info) +{ +return NULL ; +}
fsmonitor--daemon.h +3 -0
@@ -8,6 +8,7 @@#include "run-command.h" #include "simple-ipc.h" #include "thread-utils.h" +#include "fsmonitor-path-utils.h" struct fsmonitor_batch ;struct fsmonitor_token_data ;
@@ -43,6 +44,7 @@ struct fsmonitor_daemon_state {struct strbuf path_worktree_watch;struct strbuf path_gitdir_watch; +struct alias_info alias;int nr_paths_watching;struct fsmonitor_token_data *current_token_data;
@@ -59,6 +61,7 @@ struct fsmonitor_daemon_state {struct ipc_server_data *ipc_server_data;struct strbuf path_ipc; + };/*
fsmonitor-path-utils.h +35 -1
@@ -1,13 +1,21 @@#ifndef FSM_PATH_UTILS_H #define FSM_PATH_UTILS_H +#include "strbuf.h" + +struct alias_info +{ +struct strbuf alias; +struct strbuf points_to; +}; +struct fs_info {int is_remote;char *typename; };/* -* Get some basic filesystem informtion for the given path +* Get some basic filesystem information for the given path * * Returns -1 on error, zero otherwise. */
@@ -20,4 +28,30 @@ int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info);*/ int fsmonitor__is_fs_remote (const char *path); +/* +* Get the alias in given path, if any. +* +* Sets alias to the first alias that matches any part of the path. +* +* If an alias is found, info.alias and info.points_to are set to the +* found mapping. +* +* Returns -1 on error, 0 otherwise. +* +* The caller owns the storage that is occupied by info.alias and +* info.points_to and is responsible for releasing it. +*/ +int fsmonitor__get_alias (const char *path,struct alias_info *info); + +/* +* Resolve the path against the given alias. +* +* Returns the resolved path if there is one, NULL otherwise. +* +* The caller owns the storage that the returned string occupies and +* is responsible for releasing it. +*/ +char *fsmonitor__resolve_alias (const char *path, +const struct alias_info *info); +#endif
Signature · 1 line
gitgitgadget
PATCH v14 5/6 fsmonitor: check for compatability before communicating with fsmonitor
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
If fsmonitor is not in a compatible state, warn with an appropriate message.
--- compat/fsmonitor/fsm-settings-darwin.c | 2 +- fsmonitor-settings.c | 10 +++++++--- fsmonitor-settings.h | 2 +- fsmonitor.c | 7 +++++++ 4 files changed, 16 insertions(+), 5 deletions(-)
compat/fsmonitor/fsm-settings-darwin.c +1 -1
@@ -38,7 +38,7 @@ static enum fsmonitor_reason check_uds_volume(struct repository *r)strbuf_release (&path);if (fs.is_remote) -return FSMONITOR_REASON_REMOTE ; +return FSMONITOR_REASON_NOSOCKETS ;if (!strcmp (fs.typename,"msdos" ))/* aka FAT32 */ return FSMONITOR_REASON_NOSOCKETS ;
fsmonitor-settings.c +7 -3
@@ -1,6 +1,7 @@#include "cache.h" #include "config.h" #include "repository.h" +#include "fsmonitor-ipc.h" #include "fsmonitor-settings.h" #include "fsmonitor-path-utils.h"
@@ -242,10 +243,11 @@ enum fsmonitor_reason fsm_settings__get_reason(struct repository *r)return r->settings.fsmonitor->reason; } -char *fsm_settings__get_incompatible_msg (const struct repository *r, +char *fsm_settings__get_incompatible_msg (struct repository *r,enum fsmonitor_reason reason) {struct strbuf msg =STRBUF_INIT ; +const char *socket_dir;switch (reason) {case FSMONITOR_REASON_UNTESTED :
@@ -281,9 +283,11 @@ char *fsm_settings__get_incompatible_msg(const struct repository *r, goto done;case FSMONITOR_REASON_NOSOCKETS : + socket_dir =dirname ((char *)fsmonitor_ipc__get_path (r));strbuf_addf (&msg, -_ ("repository '%s' is incompatible with fsmonitor due to lack of Unix sockets" ), - r->worktree); +_ ("socket directory '%s' is incompatible with fsmonitor due" +" to lack of Unix sockets support" ), + socket_dir); goto done; }
fsmonitor-settings.h +1 -1
@@ -33,7 +33,7 @@ enum fsmonitor_mode fsm_settings__get_mode(struct repository *r);const char *fsm_settings__get_hook_path (struct repository *r);enum fsmonitor_reason fsm_settings__get_reason (struct repository *r); -char *fsm_settings__get_incompatible_msg (const struct repository *r, +char *fsm_settings__get_incompatible_msg (struct repository *r,enum fsmonitor_reason reason);struct fsmonitor_settings ;
fsmonitor.c +7 -0
@@ -295,6 +295,7 @@ static int fsmonitor_force_update_threshold = 100;void refresh_fsmonitor (struct index_state *istate) { +static int warn_once =0 ;struct strbuf query_result =STRBUF_INIT ;int query_success =0 , hook_version =-1 ;size_t bol =0 ;/* beginning of line */
@@ -305,6 +306,12 @@ void refresh_fsmonitor(struct index_state *istate)int is_trivial =0 ;struct repository *r = istate->repo ? istate->repo : the_repository;enum fsmonitor_mode fsm_mode =fsm_settings__get_mode (r); +enum fsmonitor_reason reason =fsm_settings__get_reason (r); + +if (!warn_once && reason >FSMONITOR_REASON_OK ) { + warn_once =1 ; +warning ("%s" ,fsm_settings__get_incompatible_msg (r, reason)); + }if (fsm_mode <=FSMONITOR_MODE_DISABLED || istate->fsmonitor_has_run_once)
Signature · 1 line
gitgitgadget
PATCH v14 6/6 fsmonitor: add documentation for allowRemote and socketDir options
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
Add documentation for 'fsmonitor.allowRemote' and 'fsmonitor.socketDir'. Call-out experimental nature of 'fsmonitor.allowRemote' and limited filesystem support for 'fsmonitor.socketDir'.
--- Documentation/config.txt | 2 ++ Documentation/config/fsmonitor--daemon.txt | 11 +++++++ Documentation/git-fsmonitor--daemon.txt | 37 ++++++++++++++++++++-- 3 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 Documentation/config/fsmonitor--daemon.txt
Documentation/config.txt +2 -0
@@ -423,6 +423,8 @@ include::config/filter.txt[] include::config/fsck.txt[] +include::config/fsmonitor--daemon.txt[] + include::config/gc.txt[] include::config/gitcvs.txt[]
Documentation/config/fsmonitor--daemon.txt new file +11 -0
@@ -0,0 +1,11 @@ +fsmonitor.allowRemote:: + By default, the fsmonitor daemon refuses to work against network-mounted + repositories. Setting `fsmonitor.allowRemote` to `true` overrides this + behavior. Only respected when `core.fsmonitor` is set to `true`. + +fsmonitor.socketDir:: + This Mac OS-specific option, if set, specifies the directory in + which to create the Unix domain socket used for communication + between the fsmonitor daemon and various Git commands. The directory must + reside on a native Mac OS filesystem. Only respected when `core.fsmonitor` + is set to `true`.
Documentation/git-fsmonitor--daemon.txt +34 -3
@@ -3,7 +3,7 @@ git-fsmonitor{litdd}daemon(1) NAME ---- -git-fsmonitor--daemon - A Built-in File System Monitor +git-fsmonitor--daemon - A Built-in Filesystem Monitor SYNOPSIS --------
@@ -17,7 +17,7 @@ DESCRIPTION ----------- A daemon to watch the working directory for file and directory -changes using platform-specific file system notification facilities. +changes using platform-specific filesystem notification facilities. This daemon communicates directly with commands like `git status` using the link:technical/api-simple-ipc.html[simple IPC] interface
@@ -63,13 +63,44 @@ CAVEATS ------- The fsmonitor daemon does not currently know about submodules and does -not know to filter out file system events that happen within a +not know to filter out filesystem events that happen within a submodule. If fsmonitor daemon is watching a super repo and a file is modified within the working directory of a submodule, it will report the change (as happening against the super repo). However, the client will properly ignore these extra events, so performance may be affected but it will not cause an incorrect result. +By default, the fsmonitor daemon refuses to work against network-mounted +repositories; this may be overridden by setting `fsmonitor.allowRemote` to +`true`. Note, however, that the fsmonitor daemon is not guaranteed to work +correctly with all network-mounted repositories and such use is considered +experimental. + +On Mac OS, the inter-process communication (IPC) between various Git +commands and the fsmonitor daemon is done via a Unix domain socket (UDS) -- a +special type of file -- which is supported by native Mac OS filesystems, +but not on network-mounted filesystems, NTFS, or FAT32. Other filesystems +may or may not have the needed support; the fsmonitor daemon is not guaranteed +to work with these filesystems and such use is considered experimental. + +By default, the socket is created in the `.git` directory, however, if the +`.git` directory is on a network-mounted filesystem, it will be instead be +created at `$HOME/.git-fsmonitor-*` unless `$HOME` itself is on a +network-mounted filesystem in which case you must set the configuration +variable `fsmonitor.socketDir` to the path of a directory on a Mac OS native +filesystem in which to create the socket file. + +If none of the above directories (`.git`, `$HOME`, or `fsmonitor.socketDir`) +is on a native Mac OS file filesystem the fsmonitor daemon will report an +error that will cause the daemon and the currently running command to exit. + +CONFIGURATION +------------- + +include::includes/cmd-config-section-all.txt[] + +include::config/fsmonitor--daemon.txt[] + GIT --- Part of the linkgit:git[1] suite
Signature · 1 line
gitgitgadget
v15 cover fsmonitor: option to allow fsmonitor to run against network-mounted repos
In reply to Eric DeCosta via GitGitGadgetFollow-on to the work done to allow Windows to work against network-mounted repos for macOS.
Have macOS take advantage of the same configuration option, 'fsmonitor.allowRemote' that was introduced for Windows. Setting this option to true will override the default behavior (erroring-out) when a network-mounted repo is detected by fsmonitor.
The added wrinkle being that the Unix domain socket (UDS) file used for IPC cannot be created in a network location; instead $HOME is used if the default location is on the network. The user may, optionally, set the 'fsmonitor.socketDir' configuration option to a valid, local directory if $HOME itself is on the network or is simply not the desired location for the UDS file.
An additional issue is that for mount points in the root directory, FSEvents does not report a path that matches the worktree directory due to the introduction of 'synthetic firmlinks'. fsmonitor must map the FSEvents paths to the worktree directory by interrogating the root filesystem for synthetic firmlinks and using that information to translate the path.
v15 differs from v14:
* fix memory leak
v14 differs from v13:
* code review feedback
v13 differs from v12:
* code review feedback
v12 differs from v11:
* bug fixes
v11 differs from v10:
* incorporates code review feedback
* fix memory leak in fsm-listen-darwin.c
v10 differs from v9:
* incorporates code review feedback
* improves error messaging for incompatible socket directory
v9 differs from v8:
* incorporates code review feedback
* check for incompatibility before communicating with fsmonitor
v8 differs from v7:
* incorporates code review feedback
* gets the rebase right
v7 differs from v6:
* incorporates code review feedback
v6 differs from v5:
* incorporates earlier, Windows-specific changes that have not made it back yet to the master branch
* incorporates code review feedback
* adds documentation for 'fsmonitor.allowRemote' and 'fsmonitor.socketDir'
v5 differs significantly from earlier versions:
* redesign of handling 'fsmonitor.allowRemote' and 'fsmonitor.socketDir' such that these options are no longer added to the settings data structure but are rather read from config at point of use
* refactoring of code for handling platform-specific file system checks via a common interface to avoid platform #ifdef in IPC code and be in-model with other platform-specific fsmonitor code
* dealing with 'synthetic firmlinks' on macOS
Eric DeCosta (6):
fsmonitor: refactor filesystem checks to common interface
fsmonitor: relocate socket file if .git directory is remote
fsmonitor: avoid socket location check if using hook
fsmonitor: deal with synthetic firmlinks on macOS
fsmonitor: check for compatability before communicating with fsmonitor
fsmonitor: add documentation for allowRemote and socketDir options
Documentation/config.txt | 2 + Documentation/config/fsmonitor--daemon.txt | 11 ++ Documentation/git-fsmonitor--daemon.txt | 37 ++++- Makefile | 2 + builtin/fsmonitor--daemon.c | 11 +- compat/fsmonitor/fsm-ipc-darwin.c | 52 ++++++ compat/fsmonitor/fsm-ipc-win32.c | 9 ++ compat/fsmonitor/fsm-listen-darwin.c | 14 +- compat/fsmonitor/fsm-path-utils-darwin.c | 135 ++++++++++++++++ compat/fsmonitor/fsm-path-utils-win32.c | 145 +++++++++++++++++ compat/fsmonitor/fsm-settings-darwin.c | 77 +++------ compat/fsmonitor/fsm-settings-win32.c | 174 +-------------------- contrib/buildsystems/CMakeLists.txt | 4 + fsmonitor--daemon.h | 3 + fsmonitor-ipc.c | 18 +-- fsmonitor-ipc.h | 4 +- fsmonitor-path-utils.h | 60 +++++++ fsmonitor-settings.c | 68 +++++++- fsmonitor-settings.h | 4 +- fsmonitor.c | 7 + 20 files changed, 588 insertions(+), 249 deletions(-) create mode 100644 Documentation/config/fsmonitor--daemon.txt create mode 100644 compat/fsmonitor/fsm-ipc-darwin.c create mode 100644 compat/fsmonitor/fsm-ipc-win32.c create mode 100644 compat/fsmonitor/fsm-path-utils-darwin.c create mode 100644 compat/fsmonitor/fsm-path-utils-win32.c create mode 100644 fsmonitor-path-utils.h
base-commit: 3dcec76d9df911ed8321007b1d197c1a206dc164
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1326%2Fedecosta-mw%2Ffsmonitor_macos-v15
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1326/edecosta-mw/fsmonitor_macos-v15
Pull-Request: https://github.com/gitgitgadget/git/pull/1326
Range-diff vs v14:
1: 0b9b64428c5 ! 1: ec49a74086d fsmonitor: refactor filesystem checks to common interface
@@ compat/fsmonitor/fsm-path-utils-darwin.c (new)
+ else
+ fs_info->is_remote = 0;
+
-+ fs_info->typename = fs.f_fstypename;
++ fs_info->typename = xstrdup(fs.f_fstypename);
+
+ trace_printf_key(&trace_fsmonitor,
+ "'%s' is_remote: %d",
@@ compat/fsmonitor/fsm-path-utils-darwin.c (new)
+ struct fs_info fs;
+ if (fsmonitor__get_fs_info(path, &fs))
+ return -1;
++
++ free(fs.typename);
++
+ return fs.is_remote;
+}
@@ compat/fsmonitor/fsm-settings-darwin.c
- trace_printf_key(&trace_fsmonitor,
- "statfs('%s') [type 0x%08x][flags 0x%08x] '%s'",
- r->worktree, fs.f_type, fs.f_flags, fs.f_fstypename);
-+ strbuf_release(&path);
-
+-
- if (!(fs.f_flags & MNT_LOCAL))
-+ if (fs.is_remote)
- return FSMONITOR_REASON_REMOTE;
+- return FSMONITOR_REASON_REMOTE;
++ strbuf_release(&path);
- if (!strcmp(fs.f_fstypename, "msdos")) /* aka FAT32 */
-+ if (!strcmp(fs.typename, "msdos")) /* aka FAT32 */
- return FSMONITOR_REASON_NOSOCKETS;
-
+- return FSMONITOR_REASON_NOSOCKETS;
+-
- if (!strcmp(fs.f_fstypename, "ntfs"))
-+ if (!strcmp(fs.typename, "ntfs"))
++ if (fs.is_remote ||
++ !strcmp(fs.typename, "msdos") ||
++ !strcmp(fs.typename, "ntfs")) {
++ free(fs.typename);
return FSMONITOR_REASON_NOSOCKETS;
++ }
++ free(fs.typename);
return FSMONITOR_REASON_OK;
+ }
+
@@ compat/fsmonitor/fsm-settings-darwin.c: enum fsmonitor_reason fsm_os__incompatible(struct repository *r)
{
enum fsmonitor_reason reason;
@@ fsmonitor-path-utils.h (new)
+/*
+ * Get some basic filesystem informtion for the given path
+ *
++ * The caller owns the storage that is occupied by fs_info and
++ * is responsible for releasing it.
++ *
+ * Returns -1 on error, zero otherwise.
+ */
+int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info);
2: 680d4c83f99 = 2: 7bf1cdfe3b2 fsmonitor: relocate socket file if .git directory is remote
3: 7987d0c1f33 = 3: c5e8b6cfe5d fsmonitor: avoid socket location check if using hook
4: 241043b7c15 ! 4: 863063aefee fsmonitor: deal with synthetic firmlinks on macOS
@@ compat/fsmonitor/fsm-path-utils-darwin.c
#include <sys/mount.h>
@@ compat/fsmonitor/fsm-path-utils-darwin.c: int fsmonitor__is_fs_remote(const char *path)
- return -1;
+
return fs.is_remote;
}
+
@@ fsmonitor-path-utils.h
- * Get some basic filesystem informtion for the given path
+ * Get some basic filesystem information for the given path
*
- * Returns -1 on error, zero otherwise.
- */
+ * The caller owns the storage that is occupied by fs_info and
+ * is responsible for releasing it.
@@ fsmonitor-path-utils.h: int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info);
*/
int fsmonitor__is_fs_remote(const char *path); 5: d906debba5e ! 5: fa974bfd5ef fsmonitor: check for compatability before communicating with fsmonitor
@@ Commit message
- ## compat/fsmonitor/fsm-settings-darwin.c ##
-@@ compat/fsmonitor/fsm-settings-darwin.c: static enum fsmonitor_reason check_uds_volume(struct repository *r)
- strbuf_release(&path);
-
- if (fs.is_remote)
-- return FSMONITOR_REASON_REMOTE;
-+ return FSMONITOR_REASON_NOSOCKETS;
-
- if (!strcmp(fs.typename, "msdos")) /* aka FAT32 */
- return FSMONITOR_REASON_NOSOCKETS;
-
## fsmonitor-settings.c ##
@@
#include "cache.h"
6: ed14fbd009e = 6: af7309745f7 fsmonitor: add documentation for allowRemote and socketDir options
Signature · 1 line
gitgitgadget
PATCH v15 1/6 fsmonitor: refactor filesystem checks to common interface
From: Eric DeCosta <[email protected]>
Provide a common interface for getting basic filesystem information including filesystem type and whether the filesystem is remote.
Refactor existing code for getting basic filesystem info and detecting remote file systems to the new interface.
Refactor filesystem checks to leverage new interface. For macOS, error-out if the Unix Domain socket (UDS) file is on a remote filesystem.
--- Makefile | 1 + compat/fsmonitor/fsm-path-utils-darwin.c | 43 ++++++ compat/fsmonitor/fsm-path-utils-win32.c | 128 +++++++++++++++++ compat/fsmonitor/fsm-settings-darwin.c | 69 +++------ compat/fsmonitor/fsm-settings-win32.c | 172 +---------------------- contrib/buildsystems/CMakeLists.txt | 2 + fsmonitor-path-utils.h | 26 ++++ fsmonitor-settings.c | 50 +++++++ 8 files changed, 272 insertions(+), 219 deletions(-) create mode 100644 compat/fsmonitor/fsm-path-utils-darwin.c create mode 100644 compat/fsmonitor/fsm-path-utils-win32.c create mode 100644 fsmonitor-path-utils.h
Makefile +1 -0
@@ -2044,6 +2044,7 @@ endif ifdef FSMONITOR_OS_SETTINGS COMPAT_CFLAGS += -DHAVE_FSMONITOR_OS_SETTINGS COMPAT_OBJS += compat/fsmonitor/fsm-settings-$(FSMONITOR_OS_SETTINGS).o + COMPAT_OBJS += compat/fsmonitor/fsm-path-utils-$(FSMONITOR_OS_SETTINGS).o endif ifeq ($(TCLTK_PATH),)
compat/fsmonitor/fsm-path-utils-darwin.c new file +43 -0
43 lines not on this page · raw message
compat/fsmonitor/fsm-path-utils-win32.c new file +128 -0
128 lines not on this page · raw message
compat/fsmonitor/fsm-settings-darwin.c +20 -49
101 lines not on this page · raw message
compat/fsmonitor/fsm-settings-win32.c +2 -170
190 lines not on this page · raw message
contrib/buildsystems/CMakeLists.txt +2 -0
@@ -308,6 +308,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-win32.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-win32.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-win32.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-win32.c)
@@ -315,6 +316,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-darwin.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-darwin.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-darwin.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-darwin.c)
fsmonitor-path-utils.h new file +26 -0
@@ -0,0 +1,26 @@ +#ifndef FSM_PATH_UTILS_H +#define FSM_PATH_UTILS_H + +struct fs_info { +int is_remote; +char *typename; +}; + +/* +* Get some basic filesystem informtion for the given path +* +* The caller owns the storage that is occupied by fs_info and +* is responsible for releasing it. +* +* Returns -1 on error, zero otherwise. +*/ +int fsmonitor__get_fs_info (const char *path,struct fs_info *fs_info); + +/* +* Determines if the filesystem that path resides on is remote. +* +* Returns -1 on error, 0 if not remote, 1 if remote. +*/ +int fsmonitor__is_fs_remote (const char *path); + +#endif
fsmonitor-settings.c +50 -0
68 lines not on this page · raw message
Signature · 1 line
gitgitgadget
Re: [PATCH v15 1/6] fsmonitor: refactor filesystem checks to common interface
Quoted 45 lines · Eric DeCosta via GitGitGadget
From: Eric DeCosta <[email protected]>
Provide a common interface for getting basic filesystem information including filesystem type and whether the filesystem is remote.
Refactor existing code for getting basic filesystem info and detecting remote file systems to the new interface.
Refactor filesystem checks to leverage new interface. For macOS, error-out if the Unix Domain socket (UDS) file is on a remote filesystem.
Signed-off-by Eric DeCosta <[email protected]>--- Makefile | 1 + compat/fsmonitor/fsm-path-utils-darwin.c | 43 ++++++ compat/fsmonitor/fsm-path-utils-win32.c | 128 +++++++++++++++++ compat/fsmonitor/fsm-settings-darwin.c | 69 +++------ compat/fsmonitor/fsm-settings-win32.c | 172 +---------------------- contrib/buildsystems/CMakeLists.txt | 2 + fsmonitor-path-utils.h | 26 ++++ fsmonitor-settings.c | 50 +++++++ 8 files changed, 272 insertions(+), 219 deletions(-) create mode 100644 compat/fsmonitor/fsm-path-utils-darwin.c create mode 100644 compat/fsmonitor/fsm-path-utils-win32.c create mode 100644 fsmonitor-path-utils.hMakefile +1
-0@@ -2044,6 +2044,7 @@ endif ifdef FSMONITOR_OS_SETTINGS COMPAT_CFLAGS += -DHAVE_FSMONITOR_OS_SETTINGS COMPAT_OBJS += compat/fsmonitor/fsm-settings-$(FSMONITOR_OS_SETTINGS).o + COMPAT_OBJS += compat/fsmonitor/fsm-path-utils-$(FSMONITOR_OS_SETTINGS).o endif ifeq ($(TCLTK_PATH),)compat/fsmonitor/fsm-path-utils-darwin.c new file +0
-0
compat/fsmonitor/fsm-path-utils-darwin.c line 8
Quoted 8 lines
compat/fsmonitor/fsm-path-utils-darwin.c new file +8
-0@@ -0,0 +1,43 @@ +#include "fsmonitor.h" +#include "fsmonitor-path-utils.h" +#include <sys/param.h> +#include <sys/mount.h> + +int fsmonitor__get_fs_info (const char *path,struct fs_info *fs_info) +{ +struct statfs fs;
Should have a \n here after variable decls.
Quoted 6 lines
compat/fsmonitor/fsm-path-utils-darwin.c new file +6
-0+if (statfs (path, &fs) ==-1 ) { +int saved_errno = errno; +trace_printf_key (&trace_fsmonitor,"statfs('%s') failed: %s" , + path,strerror (saved_errno)); + errno = saved_errno; +return -1 ;
compat/fsmonitor/fsm-path-utils-darwin.c line 24
Quoted 10 lines
compat/fsmonitor/fsm-path-utils-darwin.c new file +10
-0+ } + +trace_printf_key (&trace_fsmonitor, +"statfs('%s') [type 0x%08x][flags 0x%08x] '%s'" , + path, fs.f_type, fs.f_flags, fs.f_fstypename); + +if (!(fs.f_flags &MNT_LOCAL )) + fs_info->is_remote =1 ; +else + fs_info->is_remote =0 ;
Instead:
fs_info->is_remote = !(fs.f_flags & MNT_LOCAL)
?
compat/fsmonitor/fsm-path-utils-darwin.c line 36
Quoted 3 lines
compat/fsmonitor/fsm-path-utils-darwin.c new file +3
-0+int fsmonitor__is_fs_remote (const char *path) +{ +struct fs_info fs;
Same style issue as above.
compat/fsmonitor/fsm-path-utils-darwin.c line 40
Quoted 4 lines
compat/fsmonitor/fsm-path-utils-darwin.c new file +4
-0+if (fsmonitor__get_fs_info (path, &fs)) +return -1 ; + +free (fs.typename);
Can we get a "free_fs_info()" function or something instead, this is one of N codepaths where we now peek into that struct. If we ever add another field that needs malloc'ing altering all callers will be painful.
compat/fsmonitor/fsm-path-utils-darwin.c line 34
Quoted 3 lines
compat/fsmonitor/fsm-path-utils-darwin.c new file +3
-0+if (xutftowcs_path (wpath, path) <0 ) { +return -1 ; + }
Maybe drop the braces here as this code is being modified-while-moved anyway?
Quoted 9 lines
compat/fsmonitor/fsm-path-utils-darwin.c new file +9
-0+ +/* +* GetDriveTypeW() requires a final slash. We assume that the +* worktree pathname points to an actual directory. +*/ + wlen =wcslen (wpath); +if (wpath[wlen -1 ] !=L'\\' && wpath[wlen -1 ] !=L'/' ) { + wpath[wlen++] =L'\\' ; + wpath[wlen] =0 ;
compat/fsmonitor/fsm-path-utils-darwin.c
Quoted 10 lines
compat/fsmonitor/fsm-path-utils-darwin.c new file +10
-0+ } + +/* +* Normalize the path. If nothing else, this converts forward +* slashes to backslashes. This is essential to get GetDriveTypeW() +* correctly handle some UNC "\\server\share\..." paths. +*/ +if (!GetFullPathNameW (wpath,MAX_PATH , wfullpath,NULL )) { +return -1 ; + }
Here you have added needless braces while moving this, let's not do that.
compat/fsmonitor/fsm-path-utils-darwin.c
Quoted 10 lines
compat/fsmonitor/fsm-path-utils-darwin.c new file +10
-0+ + driveType =GetDriveTypeW (wfullpath); +trace_printf_key (&trace_fsmonitor, +"DriveType '%s' L'%ls' (%u)" , + path, wfullpath, driveType); + +if (driveType ==DRIVE_REMOTE ) { + fs_info->is_remote =1 ; +if (check_remote_protocol (wfullpath) <0 ) +return -1 ;
Maybe this code should be more careful not to modify the passed-in struct if we're returning an error?
I.e. do this "return -1" before assigning "is_remote = 1"?
compat/fsmonitor/fsm-path-utils-darwin.c
Quoted 3 lines
compat/fsmonitor/fsm-path-utils-darwin.c new file +3
-0+ }else { + fs_info->is_remote =0 ; + }
Maybe just at the start: "struct fs_info = { 0 }" and skip this "else" ?
Quoted 4 lines
compat/fsmonitor/fsm-path-utils-darwin.c new file +4
-0+ +trace_printf_key (&trace_fsmonitor, +"'%s' is_remote: %d" , + path, fs_info->is_remote);
compat/fsmonitor/fsm-path-utils-darwin.c
Quoted 10 lines
compat/fsmonitor/fsm-path-utils-darwin.c new file +10
-0+ +return 0 ; +} + +int fsmonitor__is_fs_remote (const char *path) +{ +struct fs_info fs; +if (fsmonitor__get_fs_info (path, &fs)) +return -1 ; +return fs.is_remote;
I find this and the resulting "switch/case" caller rather un-idiomatic, i.e. you end up checking 1/0/default.
Why not in your check_remote() instead:
int is_remote; if (fsmonitor__check_fs_remote(r->worktree, &is_remote)) return FSMONITOR_REASON_ERROR; else if (!is_remote) return FSMONITOR_REASON_OK; ...
Where the "..." is the "repo_config_get_bool()" etc. code I suggest below.
I.e. having an "is" function return non-boolean is somewhat confusing, better to write to a variable (which the config API you're using does).
compat/fsmonitor/fsm-path-utils-darwin.c
Quoted 4 lines
compat/fsmonitor/fsm-path-utils-darwin.c new file +4
-0+#ifdef HAVE_FSMONITOR_OS_SETTINGS +static enum fsmonitor_reason check_remote (struct repository *r) +{ +int allow_remote =-1 ;/* -1 unset, 0 not allowed, 1 allowed */
Let's not init this to -1, and instead...
compat/fsmonitor/fsm-path-utils-darwin.c
Quoted 4 lines
compat/fsmonitor/fsm-path-utils-darwin.c new file +4
-0+int is_remote =fsmonitor__is_fs_remote (r->worktree); + +switch (is_remote) { +case 0 :
The usual coding style is to not indent the "case" further than the "switch".
compat/fsmonitor/fsm-path-utils-darwin.c
Quoted 4 lines
compat/fsmonitor/fsm-path-utils-darwin.c new file +4
-0+return FSMONITOR_REASON_OK ; +case 1 : +repo_config_get_bool (r,"fsmonitor.allowremote" , &allow_remote); +if (allow_remote <1 )
...continued from above: We can scope this variable to this case statement, as "case 1: { int v; ... }", but furthermore you don't need to init it to -1 and check this -1 case if you just check the return value of repo_config_get_bool(), which IMO is also more obvious:
int v; if (!repo...(..., &v)) return v ? FSMONITOR_REASON_OK : FSMONITOR_REASON_REMOTE: else return FSMONITOR_REASON_REMOTE;
I.e. you clearly separate the cases where it's un-init'd by checking the return value.
Maybe better (but would result in more churn later if you ever want to change the default):
int v; return !repo...(..., &v) && v ? FSMONITOR_REASON_OK : FSMONITOR_REASON_REMOTE:
PATCH v15 2/6 fsmonitor: relocate socket file if .git directory is remote
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
If the .git directory is on a remote filesystem, create the socket file in 'fsmonitor.socketDir' if it is defined, else create it in $HOME.
--- Makefile | 1 + builtin/fsmonitor--daemon.c | 3 +- compat/fsmonitor/fsm-ipc-darwin.c | 52 ++++++++++++++++++++++++++ compat/fsmonitor/fsm-ipc-win32.c | 9 +++++ compat/fsmonitor/fsm-settings-darwin.c | 2 +- contrib/buildsystems/CMakeLists.txt | 2 + fsmonitor-ipc.c | 18 ++++----- fsmonitor-ipc.h | 4 +- 8 files changed, 78 insertions(+), 13 deletions(-) create mode 100644 compat/fsmonitor/fsm-ipc-darwin.c create mode 100644 compat/fsmonitor/fsm-ipc-win32.c
Makefile +1 -0
@@ -2039,6 +2039,7 @@ ifdef FSMONITOR_DAEMON_BACKEND COMPAT_CFLAGS += -DHAVE_FSMONITOR_DAEMON_BACKEND COMPAT_OBJS += compat/fsmonitor/fsm-listen-$(FSMONITOR_DAEMON_BACKEND).o COMPAT_OBJS += compat/fsmonitor/fsm-health-$(FSMONITOR_DAEMON_BACKEND).o + COMPAT_OBJS += compat/fsmonitor/fsm-ipc-$(FSMONITOR_DAEMON_BACKEND).o endif ifdef FSMONITOR_OS_SETTINGS
builtin/fsmonitor--daemon.c +2 -1
@@ -1343,7 +1343,8 @@ static int fsmonitor_run_daemon(void)* directory.) */ strbuf_init (&state.path_ipc,0 ); -strbuf_addstr (&state.path_ipc,absolute_path (fsmonitor_ipc__get_path ())); +strbuf_addstr (&state.path_ipc, +absolute_path (fsmonitor_ipc__get_path (the_repository)));/* * Confirm that we can create platform-specific resources for the
compat/fsmonitor/fsm-ipc-darwin.c new file +52 -0
52 lines not on this page · raw message
compat/fsmonitor/fsm-ipc-win32.c new file +9 -0
@@ -0,0 +1,9 @@ +#include "config.h" +#include "fsmonitor-ipc.h" + +const char *fsmonitor_ipc__get_path (struct repository *r) { +static char *ret; +if (!ret) + ret =git_pathdup ("fsmonitor--daemon.ipc" ); +return ret; +}
compat/fsmonitor/fsm-settings-darwin.c +1 -1
@@ -26,7 +26,7 @@static enum fsmonitor_reason check_uds_volume (struct repository *r) {struct fs_info fs; -const char *ipc_path =fsmonitor_ipc__get_path (); +const char *ipc_path =fsmonitor_ipc__get_path (r);struct strbuf path =STRBUF_INIT ;strbuf_add (&path, ipc_path,strlen (ipc_path));
contrib/buildsystems/CMakeLists.txt +2 -0
@@ -308,6 +308,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-win32.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-win32.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-win32.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-win32.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS)
@@ -316,6 +317,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-darwin.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-darwin.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-darwin.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-darwin.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS)
fsmonitor-ipc.c +8 -10
@@ -18,7 +18,7 @@ int fsmonitor_ipc__is_supported(void)return 0 ; } -const char *fsmonitor_ipc__get_path (void ) +const char *fsmonitor_ipc__get_path (struct repository *r) {return NULL ; }
@@ -47,11 +47,9 @@ int fsmonitor_ipc__is_supported(void)return 1 ; } -GIT_PATH_FUNC(fsmonitor_ipc__get_path, "fsmonitor--daemon.ipc") -enum ipc_active_state fsmonitor_ipc__get_state (void ) { -return ipc_get_active_state (fsmonitor_ipc__get_path ()); +return ipc_get_active_state (fsmonitor_ipc__get_path (the_repository)); }static int spawn_daemon (void )
@@ -81,8 +79,8 @@ int fsmonitor_ipc__send_query(const char *since_token,trace2_data_string ("fsm_client" ,NULL ,"query/command" , tok); try_again: - state =ipc_client_try_connect (fsmonitor_ipc__get_path (), &options, - &connection); + state =ipc_client_try_connect (fsmonitor_ipc__get_path (the_repository), + &options, &connection);switch (state) {case IPC_STATE__LISTENING :
@@ -117,13 +115,13 @@ try_again:case IPC_STATE__INVALID_PATH : ret =error (_ ("fsmonitor_ipc__send_query: invalid path '%s'" ), -fsmonitor_ipc__get_path ()); +fsmonitor_ipc__get_path (the_repository)); goto done;case IPC_STATE__OTHER_ERROR :default : ret =error (_ ("fsmonitor_ipc__send_query: unspecified error on '%s'" ), -fsmonitor_ipc__get_path ()); +fsmonitor_ipc__get_path (the_repository)); goto done; }
@@ -149,8 +147,8 @@ int fsmonitor_ipc__send_command(const char *command, options.wait_if_busy =1 ; options.wait_if_not_found =0 ; - state =ipc_client_try_connect (fsmonitor_ipc__get_path (), &options, - &connection); + state =ipc_client_try_connect (fsmonitor_ipc__get_path (the_repository), + &options, &connection);if (state !=IPC_STATE__LISTENING ) {die (_ ("fsmonitor--daemon is not running" ));return -1 ;
fsmonitor-ipc.h +3 -1
@@ -3,6 +3,8 @@#include "simple-ipc.h" +struct repository ; +/* * Returns true if built-in file system monitor daemon is defined * for this platform.
@@ -16,7 +18,7 @@ int fsmonitor_ipc__is_supported(void);* * Returns NULL if the daemon is not supported on this platform. */ -const char *fsmonitor_ipc__get_path (void ); +const char *fsmonitor_ipc__get_path (struct repository *r);/* * Try to determine whether there is a `git-fsmonitor--daemon` process
Signature · 1 line
gitgitgadget
Re: [PATCH v15 2/6] fsmonitor: relocate socket file if .git directory is remote
Quoted 55 lines · Eric DeCosta via GitGitGadget
From: Eric DeCosta <[email protected]>
If the .git directory is on a remote filesystem, create the socket file in 'fsmonitor.socketDir' if it is defined, else create it in $HOME.
Signed-off-by Eric DeCosta <[email protected]>--- Makefile | 1 + builtin/fsmonitor--daemon.c | 3 +- compat/fsmonitor/fsm-ipc-darwin.c | 52 ++++++++++++++++++++++++++ compat/fsmonitor/fsm-ipc-win32.c | 9 +++++ compat/fsmonitor/fsm-settings-darwin.c | 2 +- contrib/buildsystems/CMakeLists.txt | 2 + fsmonitor-ipc.c | 18 ++++----- fsmonitor-ipc.h | 4 +- 8 files changed, 78 insertions(+), 13 deletions(-) create mode 100644 compat/fsmonitor/fsm-ipc-darwin.c create mode 100644 compat/fsmonitor/fsm-ipc-win32.cMakefile +1
-0@@ -2039,6 +2039,7 @@ ifdef FSMONITOR_DAEMON_BACKEND COMPAT_CFLAGS += -DHAVE_FSMONITOR_DAEMON_BACKEND COMPAT_OBJS += compat/fsmonitor/fsm-listen-$(FSMONITOR_DAEMON_BACKEND).o COMPAT_OBJS += compat/fsmonitor/fsm-health-$(FSMONITOR_DAEMON_BACKEND).o + COMPAT_OBJS += compat/fsmonitor/fsm-ipc-$(FSMONITOR_DAEMON_BACKEND).o endif ifdef FSMONITOR_OS_SETTINGSbuiltin/fsmonitor--daemon.c +2
-1@@ -1343,7 +1343,8 @@ static int fsmonitor_run_daemon(void)* directory.) */ strbuf_init (&state.path_ipc,0 ); -strbuf_addstr (&state.path_ipc,absolute_path (fsmonitor_ipc__get_path ())); +strbuf_addstr (&state.path_ipc, +absolute_path (fsmonitor_ipc__get_path (the_repository)));/* * Confirm that we can create platform-specific resources for the compat/fsmonitor/fsm-ipc-darwin.c new file +4
-0@@ -0,0 +1,52 @@ +#include "cache.h" +#include "config.h" +#include "strbuf.h" +#include "fsmonitor.h"
compat/fsmonitor/fsm-ipc-darwin.c line 14
Quoted 10 lines
compat/fsmonitor/fsm-ipc-darwin.c new file +10
-0+#include "fsmonitor-ipc.h" +#include "fsmonitor-path-utils.h" + +static GIT_PATH_FUNC(fsmonitor_ipc__get_default_path, "fsmonitor--daemon.ipc") + +const char *fsmonitor_ipc__get_path (struct repository *r) +{ +static const char *ipc_path =NULL ; +SHA_CTX sha1ctx; +char *sock_dir =NULL ;
...don't init this to NULL...
Quoted 11 lines
compat/fsmonitor/fsm-ipc-darwin.c new file +11
-0+struct strbuf ipc_file =STRBUF_INIT ; +unsigned char hash[SHA_DIGEST_LENGTH ]; + +if (!r) +BUG ("No repository passed into fsmonitor_ipc__get_path" ); + +if (ipc_path) +return ipc_path; + + +/* By default the socket file is created in the .git directory */
compat/fsmonitor/fsm-ipc-darwin.c line 35
Quoted 10 lines
compat/fsmonitor/fsm-ipc-darwin.c new file +10
-0+if (fsmonitor__is_fs_remote (r->gitdir) <1 ) { + ipc_path =fsmonitor_ipc__get_default_path (); +return ipc_path; + } + +SHA1_Init (&sha1ctx); +SHA1_Update (&sha1ctx, r->worktree,strlen (r->worktree)); +SHA1_Final (hash, &sha1ctx); + +repo_config_get_string (r,"fsmonitor.socketdir" , &sock_dir);
...instead check the return value here. So:
if (!repo_config_get_string(..., &sock_dir))
...compat/fsmonitor/fsm-ipc-darwin.c line 40
Quoted 4 lines
compat/fsmonitor/fsm-ipc-darwin.c new file +4
-0+/* Create the socket file in either socketDir or $HOME */ +if (sock_dir && *sock_dir) { +strbuf_addf (&ipc_file,"%s/.git-fsmonitor-%s" , + sock_dir,hash_to_hex (hash));
^ Add the body of this branch to the "..." above.
compat/fsmonitor/fsm-ipc-darwin.c line 42
Quoted 2 lines
compat/fsmonitor/fsm-ipc-darwin.c new file +2
-0+ }else { +strbuf_addf (&ipc_file,"~/.git-fsmonitor-%s" ,hash_to_hex (hash));
...and keep this in the "else".
compat/fsmonitor/fsm-ipc-darwin.c line 43
Quoted 2 lines
compat/fsmonitor/fsm-ipc-darwin.c new file +2
-0+ } +free (sock_dir);
You'd then add this free() to the first branch, but better yet in this case avoid this allocation, use "const char *" and use repo_config_get_string_tmp(). It's made for exactly this sort of use-case.
Quoted 14 lines
compat/fsmonitor/fsm-ipc-darwin.c new file +8
-0+ + ipc_path =interpolate_path (ipc_file.buf,1 ); +if (!ipc_path) +die (_ ("Invalid path: %s" ), ipc_file.buf); + +strbuf_release (&ipc_file); +return ipc_path; +}compat/fsmonitor/fsm-ipc-win32.c new file +0
-0
compat/fsmonitor/fsm-ipc-win32.c line 5
Quoted 5 lines
compat/fsmonitor/fsm-ipc-win32.c new file +5
-0@@ -0,0 +1,9 @@ +#include "config.h" +#include "fsmonitor-ipc.h" + +const char *fsmonitor_ipc__get_path (struct repository *r) { +static char *ret;
Missing \n here.
compat/fsmonitor/fsm-ipc-win32.c
Quoted 5 lines
compat/fsmonitor/fsm-ipc-win32.c new file +2
-2try_again: - state =ipc_client_try_connect (fsmonitor_ipc__get_path (), &options, - &connection); + state =ipc_client_try_connect (fsmonitor_ipc__get_path (the_repository), + &options, &connection);
This post-image is mis-indented.
compat/fsmonitor/fsm-ipc-win32.c
Quoted 4 lines
#include "
simple-ipc.h"compat/fsmonitor/fsm-ipc-win32.c new file +2
-0+struct repository ; +
I think we'd usually forard-declare such things, if needed...
compat/fsmonitor/fsm-ipc-win32.c int fsmonitor_ipc__is_supported(void); · line 19
Quoted 8 lines
/*
* Returns true if built-in file system monitor daemon is defined
* for this platform.compat/fsmonitor/fsm-ipc-win32.c new file +0
-1@@ -16,7 +18,7 @@ int fsmonitor_ipc__is_supported(void);* * Returns NULL if the daemon is not supported on this platform. */ -const char *fsmonitor_ipc__get_path (void );
..right before they're needed, so before this line?
Quoted 4 lines
compat/fsmonitor/fsm-ipc-win32.c new file +1
-0+const char *fsmonitor_ipc__get_path (struct repository *r);/* * Try to determine whether there is a `git-fsmonitor--daemon` process
PATCH v15 3/6 fsmonitor: avoid socket location check if using hook
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
If monitoring is done via fsmonitor hook rather than IPC there is no need to check if the location of the Unix Domain socket (UDS) file is on a remote filesystem.
--- compat/fsmonitor/fsm-settings-darwin.c | 10 ++++++---- compat/fsmonitor/fsm-settings-win32.c | 2 +- fsmonitor-settings.c | 8 ++++---- fsmonitor-settings.h | 2 +- 4 files changed, 12 insertions(+), 10 deletions(-)
compat/fsmonitor/fsm-settings-darwin.c +6 -4
@@ -48,13 +48,15 @@ static enum fsmonitor_reason check_uds_volume(struct repository *r)return FSMONITOR_REASON_OK ; } -enum fsmonitor_reason fsm_os__incompatible (struct repository *r) +enum fsmonitor_reason fsm_os__incompatible (struct repository *r,int ipc) {enum fsmonitor_reason reason; - reason =check_uds_volume (r); -if (reason !=FSMONITOR_REASON_OK ) -return reason; +if (ipc) { + reason =check_uds_volume (r); +if (reason !=FSMONITOR_REASON_OK ) +return reason; + }return FSMONITOR_REASON_OK ; }
compat/fsmonitor/fsm-settings-win32.c +1 -1
@@ -25,7 +25,7 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)return FSMONITOR_REASON_OK ; } -enum fsmonitor_reason fsm_os__incompatible (struct repository *r) +enum fsmonitor_reason fsm_os__incompatible (struct repository *r,int ipc) {enum fsmonitor_reason reason;
fsmonitor-settings.c +4 -4
@@ -60,7 +60,7 @@ static enum fsmonitor_reason check_remote(struct repository *r) }#endif -static enum fsmonitor_reason check_for_incompatible (struct repository *r) +static enum fsmonitor_reason check_for_incompatible (struct repository *r,int ipc) {if (!r->worktree) {/*
@@ -77,7 +77,7 @@ static enum fsmonitor_reason check_for_incompatible(struct repository *r) reason =check_remote (r);if (reason !=FSMONITOR_REASON_OK )return reason; - reason =fsm_os__incompatible (r); + reason =fsm_os__incompatible (r, ipc);if (reason !=FSMONITOR_REASON_OK )return reason; }
@@ -162,7 +162,7 @@ const char *fsm_settings__get_hook_path(struct repository *r)void fsm_settings__set_ipc (struct repository *r) { -enum fsmonitor_reason reason =check_for_incompatible (r); +enum fsmonitor_reason reason =check_for_incompatible (r,1 );if (reason !=FSMONITOR_REASON_OK ) {fsm_settings__set_incompatible (r, reason);
@@ -185,7 +185,7 @@ void fsm_settings__set_ipc(struct repository *r)void fsm_settings__set_hook (struct repository *r,const char *path) { -enum fsmonitor_reason reason =check_for_incompatible (r); +enum fsmonitor_reason reason =check_for_incompatible (r,0 );if (reason !=FSMONITOR_REASON_OK ) {fsm_settings__set_incompatible (r, reason);
fsmonitor-settings.h +1 -1
@@ -48,7 +48,7 @@ struct fsmonitor_settings;* fsm_os__* routines should considered private to fsm_settings__ * routines. */ -enum fsmonitor_reason fsm_os__incompatible (struct repository *r); +enum fsmonitor_reason fsm_os__incompatible (struct repository *r,int ipc);#endif /* HAVE_FSMONITOR_OS_SETTINGS */ #endif /* FSMONITOR_SETTINGS_H */
Signature · 1 line
gitgitgadget
PATCH v15 4/6 fsmonitor: deal with synthetic firmlinks on macOS
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
Starting with macOS 10.15 (Catalina), Apple introduced a new feature called 'firmlinks' in order to separate the boot volume into two volumes, one read-only and one writable but still present them to the user as a single volume. Along with this change, Apple removed the ability to create symlinks in the root directory and replaced them with 'synthetic firmlinks'. See 'man synthetic.conf'
When FSEevents reports the path of changed files, if the path involves a synthetic firmlink, the path is reported from the point of the synthetic firmlink and not the real path. For example:
Real path:
/System/Volumes/Data/network/working/directory/foo.txt
Synthetic firmlink:
/network -> /System/Volumes/Data/network
FSEvents path:
/network/working/directory/foo.txt
This causes the FSEvents path to not match against the worktree directory.
There are several ways in which synthetic firmlinks can be created:
they can be defined in /etc/synthetic.conf, the automounter can create them, and there may be other means. Simply reading /etc/synthetic.conf is insufficient. No matter what process creates synthetic firmlinks, they all get created in the root directory.
Therefore, in order to deal with synthetic firmlinks, the root directory is scanned and the first possible synthetic firmink that, when resolved, is a prefix of the worktree is used to map FSEvents paths to worktree paths.
--- builtin/fsmonitor--daemon.c | 8 +++ compat/fsmonitor/fsm-listen-darwin.c | 14 +++- compat/fsmonitor/fsm-path-utils-darwin.c | 92 ++++++++++++++++++++++++ compat/fsmonitor/fsm-path-utils-win32.c | 17 +++++ fsmonitor--daemon.h | 3 + fsmonitor-path-utils.h | 36 +++++++++- 6 files changed, 167 insertions(+), 3 deletions(-)
builtin/fsmonitor--daemon.c +8 -0
@@ -3,6 +3,7 @@#include "parse-options.h" #include "fsmonitor.h" #include "fsmonitor-ipc.h" +#include "fsmonitor-path-utils.h" #include "compat/fsmonitor/fsm-health.h" #include "compat/fsmonitor/fsm-listen.h" #include "fsmonitor--daemon.h"
@@ -1282,6 +1283,11 @@ static int fsmonitor_run_daemon(void)strbuf_addstr (&state.path_worktree_watch,absolute_path (get_git_work_tree ())); state.nr_paths_watching =1 ; +strbuf_init (&state.alias.alias,0 ); +strbuf_init (&state.alias.points_to,0 ); +if ((err =fsmonitor__get_alias (state.path_worktree_watch.buf, &state.alias))) + goto done; +/* * We create and delete cookie files somewhere inside the .git * directory to help us keep sync with the file system. If
@@ -1391,6 +1397,8 @@ done:strbuf_release (&state.path_gitdir_watch);strbuf_release (&state.path_cookie_prefix);strbuf_release (&state.path_ipc); +strbuf_release (&state.alias.alias); +strbuf_release (&state.alias.points_to);return err; }
compat/fsmonitor/fsm-listen-darwin.c +12 -2
@@ -26,6 +26,7 @@#include "fsmonitor.h" #include "fsm-listen.h" #include "fsmonitor--daemon.h" +#include "fsmonitor-path-utils.h" struct fsm_listen_data {
@@ -198,8 +199,9 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,struct string_list cookie_list =STRING_LIST_INIT_DUP ;const char *path_k;const char *slash; -int k; +char *resolved =NULL ;struct strbuf tmp =STRBUF_INIT ; +int k;/* * Build a list of all filesystem changes into a private/local
@@ -209,7 +211,12 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,/* * On Mac, we receive an array of absolute paths. */ - path_k = paths[k]; +free (resolved); + resolved =fsmonitor__resolve_alias (paths[k], &state->alias); +if (resolved) + path_k = resolved; +else + path_k = paths[k];/* * If you want to debug FSEvents, log them to GIT_TRACE_FSMONITOR.
@@ -238,6 +245,7 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef,fsmonitor_force_resync (state);fsmonitor_batch__free_list (batch);string_list_clear (&cookie_list,0 ); + batch =NULL ;/* * We assume that any events that we received
@@ -360,12 +368,14 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef, } } +free (resolved);fsmonitor_publish (state, batch, &cookie_list);string_list_clear (&cookie_list,0 );strbuf_release (&tmp);return ; force_shutdown: +free (resolved);fsmonitor_batch__free_list (batch);string_list_clear (&cookie_list,0 );
compat/fsmonitor/fsm-path-utils-darwin.c +92 -0
100 lines not on this page · raw message
compat/fsmonitor/fsm-path-utils-win32.c +17 -0
@@ -126,3 +126,20 @@ int fsmonitor__is_fs_remote(const char *path)return -1 ;return fs.is_remote; } + +/* +* No-op for now. +*/ +int fsmonitor__get_alias (const char *path,struct alias_info *info) +{ +return 0 ; +} + +/* +* No-op for now. +*/ +char *fsmonitor__resolve_alias (const char *path, +const struct alias_info *info) +{ +return NULL ; +}
fsmonitor--daemon.h +3 -0
@@ -8,6 +8,7 @@#include "run-command.h" #include "simple-ipc.h" #include "thread-utils.h" +#include "fsmonitor-path-utils.h" struct fsmonitor_batch ;struct fsmonitor_token_data ;
@@ -43,6 +44,7 @@ struct fsmonitor_daemon_state {struct strbuf path_worktree_watch;struct strbuf path_gitdir_watch; +struct alias_info alias;int nr_paths_watching;struct fsmonitor_token_data *current_token_data;
@@ -59,6 +61,7 @@ struct fsmonitor_daemon_state {struct ipc_server_data *ipc_server_data;struct strbuf path_ipc; + };/*
fsmonitor-path-utils.h +35 -1
@@ -1,13 +1,21 @@#ifndef FSM_PATH_UTILS_H #define FSM_PATH_UTILS_H +#include "strbuf.h" + +struct alias_info +{ +struct strbuf alias; +struct strbuf points_to; +}; +struct fs_info {int is_remote;char *typename; };/* -* Get some basic filesystem informtion for the given path +* Get some basic filesystem information for the given path * * The caller owns the storage that is occupied by fs_info and * is responsible for releasing it.
@@ -23,4 +31,30 @@ int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info);*/ int fsmonitor__is_fs_remote (const char *path); +/* +* Get the alias in given path, if any. +* +* Sets alias to the first alias that matches any part of the path. +* +* If an alias is found, info.alias and info.points_to are set to the +* found mapping. +* +* Returns -1 on error, 0 otherwise. +* +* The caller owns the storage that is occupied by info.alias and +* info.points_to and is responsible for releasing it. +*/ +int fsmonitor__get_alias (const char *path,struct alias_info *info); + +/* +* Resolve the path against the given alias. +* +* Returns the resolved path if there is one, NULL otherwise. +* +* The caller owns the storage that the returned string occupies and +* is responsible for releasing it. +*/ +char *fsmonitor__resolve_alias (const char *path, +const struct alias_info *info); +#endif
Signature · 1 line
gitgitgadget
Re: [PATCH v15 4/6] fsmonitor: deal with synthetic firmlinks on macOS
Review
Quoted 3 lines · Eric DeCosta via GitGitGadget
From: Eric DeCosta <[email protected]>
(quoted diff) +1
-0+ struct strbuf alias;
Rather than init-ing here...
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 78
Quoted 7 lines
compat/fsmonitor/fsm-path-utils-darwin.c +7
-0+struct strbuf points_to =STRBUF_INIT ; + + dir =opendir (root); +if (!dir) +return error_errno (_ ("opendir('%s') failed" ), root); + +strbuf_init (&alias,256 );
...we pre-grow here...
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 80
Quoted 1 line
compat/fsmonitor/fsm-path-utils-darwin.c +1
-0+while ((de =readdir (dir)) !=NULL ) {
...but as shown here, we may not even use this at all, but even then is this micro-optimization worth it? If it is a reader would be helped with an explanation of what the 256 is, is this meant to be some OSX-specific PATH_MAX, but hardcoded?
Quoted 6 lines
compat/fsmonitor/fsm-path-utils-darwin.c +6
-0+strbuf_reset (&alias); +strbuf_addf (&alias,"%s%s" , root, de->d_name); + +if (lstat (alias.buf, &st) <0 ) { +error_errno (_ ("lstat('%s') failed" ), alias.buf); + goto done;
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 96
Quoted 10 lines
compat/fsmonitor/fsm-path-utils-darwin.c +10
-0+ } + +if (!S_ISLNK (st.st_mode)) +continue ; + +if (strbuf_readlink (&points_to, alias.buf, st.st_size) <0 ) { +error_errno (_ ("strbuf_readlink('%s') failed" ), alias.buf); + goto done; + } +
Maybe this code would be simpler if you split it into a trivial static function, so you could pass in the "alias" and "points_to", and just do "return error...(...)" here and in the other places.
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 100
Quoted 4 lines
compat/fsmonitor/fsm-path-utils-darwin.c +4
-0+if (!strncmp (points_to.buf, path, points_to.len) && + (path[points_to.len] =='/' )) { +strbuf_addbuf (&info->alias, &alias); +strbuf_addbuf (&info->points_to, &points_to);
Earlier you use strbuf_addf() for a "append two strings", maybe we could save ourselves a line and do the same here...
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 103
Quoted 3 lines
compat/fsmonitor/fsm-path-utils-darwin.c +3
-0+trace_printf_key (&trace_fsmonitor, +"Found alias for '%s' : '%s' -> '%s'" , + path, info->alias.buf, info->points_to.buf);
...except we're only doing this to emit this, and then we'll free() it? Can't we just use %s%s here instead of %s, and e.g. pass "info->alias.buf, alias" instead of the now-appende-to "info->alias.buf"?
Review
Quoted 2 lines
(quoted diff) +2
-0+char *fsmonitor__resolve_alias(const char *path, + const struct alias_info *info)
I commented on this in a few other places, and I'll stop noting these now, but you're mis-indenting function decls consistently, also in a *.h change later in this commit.
Please look through those for this series.
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 122
Quoted 3 lines
compat/fsmonitor/fsm-path-utils-darwin.c +3
-0+{ +if (!info->alias.len) +return NULL ;
Maybe "check if we have a zero-length string" should belong in the caller, as "resolve it as an alias" for "\0" is nonsense?
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 130
Quoted 8 lines
compat/fsmonitor/fsm-path-utils-darwin.c +8
-0+ +if ((!strncmp (info->alias.buf, path, info->alias.len)) + && path[info->alias.len] =='/' ) { +struct strbuf tmp =STRBUF_INIT ; +const char *remainder = path + info->alias.len; + +strbuf_addbuf (&tmp, &info->points_to); +strbuf_add (&tmp, remainder,strlen (remainder));
There's no point in strbuf_add() if you have to dynamically use strlen() over just using strbuf_addstr() (which inline resolves to the same), let's just use that.
Or just a single strbuf_addf()...
compat/fsmonitor/fsm-path-utils-darwin.c int fsmonitor__is_fs_remote(const char *path) · line 131
Quoted 1 line
compat/fsmonitor/fsm-path-utils-darwin.c +1
-0+return strbuf_detach (&tmp,NULL );
...Or actually, these last 3 lines can be replaced by a mere:
return xstrfmt("%s%s", info->points_to.buf, remainder);Please just use that. It's not exactly the same due to the pre-sizing with "addbuf", but again (I commented on a similar case in an earlier commit), is such micro-optimization worth it here over brevity?
Quoted 3 lines
+/*
+ * No-op for now.
+ */
Please just drop this comment, it doesn't add any information. It would be useful to say why we're seemingly faking "no aliase on win32", or to say that it does have them, but implementing them is a "TODO".
But we can see it's a "no-op for now" from the code....
compat/fsmonitor/fsm-path-utils-win32.c int fsmonitor__is_fs_remote(const char *path) · line 140
Quoted 8 lines
compat/fsmonitor/fsm-path-utils-win32.c +8
-0+int fsmonitor__get_alias (const char *path,struct alias_info *info) +{ +return 0 ; +} + +/* +* No-op for now. +*/
...ditto.
Quoted 4 lines
fsmonitor-path-utils.h +4
-0+/* +* Get the alias in given path, if any. +* +* Sets alias to the first alias that matches any part of the path.
fsmonitor-path-utils.h int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info); · line 47
Quoted 10 lines
fsmonitor-path-utils.h +10
-0+* +* If an alias is found, info.alias and info.points_to are set to the +* found mapping. +* +* Returns -1 on error, 0 otherwise. +* +* The caller owns the storage that is occupied by info.alias and +* info.points_to and is responsible for releasing it. +*/ +int fsmonitor__get_alias (const char *path,struct alias_info *info);
I have not looked carefully at this interface, but instead of all of this explanation & the caller needing to carefully reason about what parts of this struct it can and can't peek into couldn't we just make it take two "char **" arguments, one for "alias" and another for "points_to".
It would then be obvious what the semantics are, and who owns the memory.
But maybe retaining the strbuf-ness is important (but even then, we could pass a "struct strbuf *" to populate.
fsmonitor-path-utils.h int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info); · line 59
Quoted 12 lines
fsmonitor-path-utils.h +12
-0+ +/* +* Resolve the path against the given alias. +* +* Returns the resolved path if there is one, NULL otherwise. +* +* The caller owns the storage that the returned string occupies and +* is responsible for releasing it. +*/ +char *fsmonitor__resolve_alias (const char *path, +const struct alias_info *info); +
Here we don't say anything about the ownership & freeing of "info", does the same apply? But the API design comment above also applies.
PATCH v15 5/6 fsmonitor: check for compatability before communicating with fsmonitor
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
If fsmonitor is not in a compatible state, warn with an appropriate message.
--- fsmonitor-settings.c | 10 +++++++--- fsmonitor-settings.h | 2 +- fsmonitor.c | 7 +++++++ 3 files changed, 15 insertions(+), 4 deletions(-)
fsmonitor-settings.c +7 -3
@@ -1,6 +1,7 @@#include "cache.h" #include "config.h" #include "repository.h" +#include "fsmonitor-ipc.h" #include "fsmonitor-settings.h" #include "fsmonitor-path-utils.h"
@@ -242,10 +243,11 @@ enum fsmonitor_reason fsm_settings__get_reason(struct repository *r)return r->settings.fsmonitor->reason; } -char *fsm_settings__get_incompatible_msg (const struct repository *r, +char *fsm_settings__get_incompatible_msg (struct repository *r,enum fsmonitor_reason reason) {struct strbuf msg =STRBUF_INIT ; +const char *socket_dir;switch (reason) {case FSMONITOR_REASON_UNTESTED :
@@ -281,9 +283,11 @@ char *fsm_settings__get_incompatible_msg(const struct repository *r, goto done;case FSMONITOR_REASON_NOSOCKETS : + socket_dir =dirname ((char *)fsmonitor_ipc__get_path (r));strbuf_addf (&msg, -_ ("repository '%s' is incompatible with fsmonitor due to lack of Unix sockets" ), - r->worktree); +_ ("socket directory '%s' is incompatible with fsmonitor due" +" to lack of Unix sockets support" ), + socket_dir); goto done; }
fsmonitor-settings.h +1 -1
@@ -33,7 +33,7 @@ enum fsmonitor_mode fsm_settings__get_mode(struct repository *r);const char *fsm_settings__get_hook_path (struct repository *r);enum fsmonitor_reason fsm_settings__get_reason (struct repository *r); -char *fsm_settings__get_incompatible_msg (const struct repository *r, +char *fsm_settings__get_incompatible_msg (struct repository *r,enum fsmonitor_reason reason);struct fsmonitor_settings ;
fsmonitor.c +7 -0
@@ -295,6 +295,7 @@ static int fsmonitor_force_update_threshold = 100;void refresh_fsmonitor (struct index_state *istate) { +static int warn_once =0 ;struct strbuf query_result =STRBUF_INIT ;int query_success =0 , hook_version =-1 ;size_t bol =0 ;/* beginning of line */
@@ -305,6 +306,12 @@ void refresh_fsmonitor(struct index_state *istate)int is_trivial =0 ;struct repository *r = istate->repo ? istate->repo : the_repository;enum fsmonitor_mode fsm_mode =fsm_settings__get_mode (r); +enum fsmonitor_reason reason =fsm_settings__get_reason (r); + +if (!warn_once && reason >FSMONITOR_REASON_OK ) { + warn_once =1 ; +warning ("%s" ,fsm_settings__get_incompatible_msg (r, reason)); + }if (fsm_mode <=FSMONITOR_MODE_DISABLED || istate->fsmonitor_has_run_once)
Signature · 1 line
gitgitgadget
PATCH v15 6/6 fsmonitor: add documentation for allowRemote and socketDir options
In reply to Eric DeCosta via GitGitGadgetFrom: Eric DeCosta <[email protected]>
Add documentation for 'fsmonitor.allowRemote' and 'fsmonitor.socketDir'. Call-out experimental nature of 'fsmonitor.allowRemote' and limited filesystem support for 'fsmonitor.socketDir'.
--- Documentation/config.txt | 2 ++ Documentation/config/fsmonitor--daemon.txt | 11 +++++++ Documentation/git-fsmonitor--daemon.txt | 37 ++++++++++++++++++++-- 3 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 Documentation/config/fsmonitor--daemon.txt
Documentation/config.txt +2 -0
@@ -423,6 +423,8 @@ include::config/filter.txt[] include::config/fsck.txt[] +include::config/fsmonitor--daemon.txt[] + include::config/gc.txt[] include::config/gitcvs.txt[]
Documentation/config/fsmonitor--daemon.txt new file +11 -0
@@ -0,0 +1,11 @@ +fsmonitor.allowRemote:: + By default, the fsmonitor daemon refuses to work against network-mounted + repositories. Setting `fsmonitor.allowRemote` to `true` overrides this + behavior. Only respected when `core.fsmonitor` is set to `true`. + +fsmonitor.socketDir:: + This Mac OS-specific option, if set, specifies the directory in + which to create the Unix domain socket used for communication + between the fsmonitor daemon and various Git commands. The directory must + reside on a native Mac OS filesystem. Only respected when `core.fsmonitor` + is set to `true`.
Documentation/git-fsmonitor--daemon.txt +34 -3
@@ -3,7 +3,7 @@ git-fsmonitor{litdd}daemon(1) NAME ---- -git-fsmonitor--daemon - A Built-in File System Monitor +git-fsmonitor--daemon - A Built-in Filesystem Monitor SYNOPSIS --------
@@ -17,7 +17,7 @@ DESCRIPTION ----------- A daemon to watch the working directory for file and directory -changes using platform-specific file system notification facilities. +changes using platform-specific filesystem notification facilities. This daemon communicates directly with commands like `git status` using the link:technical/api-simple-ipc.html[simple IPC] interface
@@ -63,13 +63,44 @@ CAVEATS ------- The fsmonitor daemon does not currently know about submodules and does -not know to filter out file system events that happen within a +not know to filter out filesystem events that happen within a submodule. If fsmonitor daemon is watching a super repo and a file is modified within the working directory of a submodule, it will report the change (as happening against the super repo). However, the client will properly ignore these extra events, so performance may be affected but it will not cause an incorrect result. +By default, the fsmonitor daemon refuses to work against network-mounted +repositories; this may be overridden by setting `fsmonitor.allowRemote` to +`true`. Note, however, that the fsmonitor daemon is not guaranteed to work +correctly with all network-mounted repositories and such use is considered +experimental. + +On Mac OS, the inter-process communication (IPC) between various Git +commands and the fsmonitor daemon is done via a Unix domain socket (UDS) -- a +special type of file -- which is supported by native Mac OS filesystems, +but not on network-mounted filesystems, NTFS, or FAT32. Other filesystems +may or may not have the needed support; the fsmonitor daemon is not guaranteed +to work with these filesystems and such use is considered experimental. + +By default, the socket is created in the `.git` directory, however, if the +`.git` directory is on a network-mounted filesystem, it will be instead be +created at `$HOME/.git-fsmonitor-*` unless `$HOME` itself is on a +network-mounted filesystem in which case you must set the configuration +variable `fsmonitor.socketDir` to the path of a directory on a Mac OS native +filesystem in which to create the socket file. + +If none of the above directories (`.git`, `$HOME`, or `fsmonitor.socketDir`) +is on a native Mac OS file filesystem the fsmonitor daemon will report an +error that will cause the daemon and the currently running command to exit. + +CONFIGURATION +------------- + +include::includes/cmd-config-section-all.txt[] + +include::config/fsmonitor--daemon.txt[] + GIT --- Part of the linkgit:git[1] suite
Signature · 1 line
gitgitgadget
Re: [PATCH v15 6/6] fsmonitor: add documentation for allowRemote and socketDir options
Quoted 3 lines · Eric DeCosta via GitGitGadget
From: Eric DeCosta <[email protected]>
[...]
+include::config/fsmonitor--daemon.txt[]
We tend to name these files after the config namespace, not the name of the built-in that's (mostly?) using it.
Documentation/config/fsmonitor--daemon.txt
Quoted 10 lines
Documentation/config.txt +1
-0+ include::config/gc.txt[] include::config/gitcvs.txt[]Documentation/config/fsmonitor--daemon.txt new file +0
-0@@ -0,0 +1,11 @@
So this should be just .../config/fsmonitor.txt
Quoted 9 lines · Eric DeCosta via GitGitGadget
An additional issue is that for mount points in the root directory, FSEvents does not report a path that matches the worktree directory due to the introduction of 'synthetic firmlinks'. fsmonitor must map the FSEvents paths to the worktree directory by interrogating the root filesystem for synthetic firmlinks and using that information to translate the path.
v15 differs from v14:
* fix memory leak
Thanks. I thought "what, another iteration? didn't we see this enough times already?" and a note like this does help reminding reviewers why we need one.
Very much appreciated.
Quoted 25 lines
-----Original Message-----
From: Junio C Hamano <[email protected]> On Behalf Of Junio C Hamano
Sent: Wednesday, October 5, 2022 2:05 PM
To: Eric DeCosta via GitGitGadget <[email protected]>
Cc: [email protected]; Jeff Hostetler <[email protected]>; Eric Sunshine <[email protected]>; Torsten Bögershausen <[email protected]>; Ævar Arnfjörð Bjarmason <[email protected]>; Ramsay Jones <[email protected]>; Johannes Schindelin <[email protected]>; Eric DeCosta <[email protected]>
Subject: Re: [PATCH v15 0/6] fsmonitor: option to allow fsmonitor to run against network-mounted reposQuoted 5 lines · Eric DeCosta via GitGitGadget
An additional issue is that for mount points in the root directory, FSEvents does not report a path that matches the worktree directory due to the introduction of 'synthetic firmlinks'. fsmonitor must map the FSEvents paths to the worktree directory by interrogating the root filesystem for synthetic firmlinks and using that information to translate the
path.
Quoted 3 lines
v15 differs from v14:
Quoted 7 lines
Quoted 1 line
* fix memory leak
Thanks. I thought "what, another iteration? didn't we see this enough times already?" and a note like this does help reminding reviewers why we need one.
Very much appreciated.
Thanks. Just getting better at doing the right thing as I climb up the learning curve :)
Hopefully it is in a state where it is now GTM.
-Eric