diff options
28 files changed, 2439 insertions, 149 deletions
@@ -477,8 +477,14 @@ include shared.mak # # If your platform supports a built-in fsmonitor backend, set # FSMONITOR_DAEMON_BACKEND to the "<name>" of the corresponding -# `compat/fsmonitor/fsm-listen-<name>.c` that implements the -# `fsm_listen__*()` routines. +# `compat/fsmonitor/fsm-listen-<name>.c` and +# `compat/fsmonitor/fsm-health-<name>.c` files +# that implement the `fsm_listen__*()` and `fsm_health__*()` routines. +# +# If your platform has OS-specific ways to tell if a repo is incompatible with +# fsmonitor (whether the hook or IPC daemon version), set FSMONITOR_OS_SETTINGS +# to the "<name>" of the corresponding `compat/fsmonitor/fsm-settings-<name>.c` +# that implements the `fsm_os_settings__*()` routines. # # Define DEVELOPER to enable more compiler warnings. Compiler version # and family are auto detected, but could be overridden by defining @@ -730,6 +736,7 @@ TEST_BUILTINS_OBJS += test-getcwd.o TEST_BUILTINS_OBJS += test-hash-speed.o TEST_BUILTINS_OBJS += test-hash.o TEST_BUILTINS_OBJS += test-hashmap.o +TEST_BUILTINS_OBJS += test-hexdump.o TEST_BUILTINS_OBJS += test-index-version.o TEST_BUILTINS_OBJS += test-json-writer.o TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o @@ -2012,6 +2019,12 @@ endif 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 +endif + +ifdef FSMONITOR_OS_SETTINGS + COMPAT_CFLAGS += -DHAVE_FSMONITOR_OS_SETTINGS + COMPAT_OBJS += compat/fsmonitor/fsm-settings-$(FSMONITOR_OS_SETTINGS).o endif ifeq ($(TCLTK_PATH),) @@ -2966,6 +2979,9 @@ GIT-BUILD-OPTIONS: FORCE ifdef FSMONITOR_DAEMON_BACKEND @echo FSMONITOR_DAEMON_BACKEND=\''$(subst ','\'',$(subst ','\'',$(FSMONITOR_DAEMON_BACKEND)))'\' >>$@+ endif +ifdef FSMONITOR_OS_SETTINGS + @echo FSMONITOR_OS_SETTINGS=\''$(subst ','\'',$(subst ','\'',$(FSMONITOR_OS_SETTINGS)))'\' >>$@+ +endif ifdef TEST_OUTPUT_DIRECTORY @echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst ','\'',$(TEST_OUTPUT_DIRECTORY)))'\' >>$@+ endif diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index 46be55a..2c109cf 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -3,6 +3,7 @@ #include "parse-options.h" #include "fsmonitor.h" #include "fsmonitor-ipc.h" +#include "compat/fsmonitor/fsm-health.h" #include "compat/fsmonitor/fsm-listen.h" #include "fsmonitor--daemon.h" #include "simple-ipc.h" @@ -1136,6 +1137,18 @@ void fsmonitor_publish(struct fsmonitor_daemon_state *state, pthread_mutex_unlock(&state->main_lock); } +static void *fsm_health__thread_proc(void *_state) +{ + struct fsmonitor_daemon_state *state = _state; + + trace2_thread_start("fsm-health"); + + fsm_health__loop(state); + + trace2_thread_exit(); + return NULL; +} + static void *fsm_listen__thread_proc(void *_state) { struct fsmonitor_daemon_state *state = _state; @@ -1174,6 +1187,9 @@ static int fsmonitor_run_daemon_1(struct fsmonitor_daemon_state *state) */ .uds_disallow_chdir = 0 }; + int health_started = 0; + int listener_started = 0; + int err = 0; /* * Start the IPC thread pool before the we've started the file @@ -1181,11 +1197,11 @@ static int fsmonitor_run_daemon_1(struct fsmonitor_daemon_state *state) * before we need it. */ if (ipc_server_run_async(&state->ipc_server_data, - fsmonitor_ipc__get_path(), &ipc_opts, + state->path_ipc.buf, &ipc_opts, handle_client, state)) return error_errno( _("could not start IPC thread pool on '%s'"), - fsmonitor_ipc__get_path()); + state->path_ipc.buf); /* * Start the fsmonitor listener thread to collect filesystem @@ -1194,15 +1210,31 @@ static int fsmonitor_run_daemon_1(struct fsmonitor_daemon_state *state) if (pthread_create(&state->listener_thread, NULL, fsm_listen__thread_proc, state) < 0) { ipc_server_stop_async(state->ipc_server_data); - ipc_server_await(state->ipc_server_data); + err = error(_("could not start fsmonitor listener thread")); + goto cleanup; + } + listener_started = 1; - return error(_("could not start fsmonitor listener thread")); + /* + * Start the health thread to watch over our process. + */ + if (pthread_create(&state->health_thread, NULL, + fsm_health__thread_proc, state) < 0) { + ipc_server_stop_async(state->ipc_server_data); + err = error(_("could not start fsmonitor health thread")); + goto cleanup; } + health_started = 1; /* * The daemon is now fully functional in background threads. + * Our primary thread should now just wait while the threads + * do all the work. + */ +cleanup: + /* * Wait for the IPC thread pool to shutdown (whether by client - * request or from filesystem activity). + * request, from filesystem activity, or an error). */ ipc_server_await(state->ipc_server_data); @@ -1211,15 +1243,29 @@ static int fsmonitor_run_daemon_1(struct fsmonitor_daemon_state *state) * event from the IPC thread pool, but it doesn't hurt to tell * it again. And wait for it to shutdown. */ - fsm_listen__stop_async(state); - pthread_join(state->listener_thread, NULL); + if (listener_started) { + fsm_listen__stop_async(state); + pthread_join(state->listener_thread, NULL); + } - return state->error_code; + if (health_started) { + fsm_health__stop_async(state); + pthread_join(state->health_thread, NULL); + } + + if (err) + return err; + if (state->listen_error_code) + return state->listen_error_code; + if (state->health_error_code) + return state->health_error_code; + return 0; } static int fsmonitor_run_daemon(void) { struct fsmonitor_daemon_state state; + const char *home; int err; memset(&state, 0, sizeof(state)); @@ -1227,7 +1273,8 @@ static int fsmonitor_run_daemon(void) hashmap_init(&state.cookies, cookies_cmp, NULL, 0); pthread_mutex_init(&state.main_lock, NULL); pthread_cond_init(&state.cookies_cond, NULL); - state.error_code = 0; + state.listen_error_code = 0; + state.health_error_code = 0; state.current_token_data = fsmonitor_new_token_data(); /* Prepare to (recursively) watch the <worktree-root> directory. */ @@ -1290,6 +1337,15 @@ static int fsmonitor_run_daemon(void) strbuf_addch(&state.path_cookie_prefix, '/'); /* + * We create a named-pipe or unix domain socket inside of the + * ".git" directory. (Well, on Windows, we base our named + * pipe in the NPFS on the absolute path of the git + * directory.) + */ + strbuf_init(&state.path_ipc, 0); + strbuf_addstr(&state.path_ipc, absolute_path(fsmonitor_ipc__get_path())); + + /* * Confirm that we can create platform-specific resources for the * filesystem listener before we bother starting all the threads. */ @@ -1298,18 +1354,42 @@ static int fsmonitor_run_daemon(void) goto done; } + if (fsm_health__ctor(&state)) { + err = error(_("could not initialize health thread")); + goto done; + } + + /* + * CD out of the worktree root directory. + * + * The common Git startup mechanism causes our CWD to be the + * root of the worktree. On Windows, this causes our process + * to hold a locked handle on the CWD. This prevents the + * worktree from being moved or deleted while the daemon is + * running. + * + * We assume that our FS and IPC listener threads have either + * opened all of the handles that they need or will do + * everything using absolute paths. + */ + home = getenv("HOME"); + if (home && *home && chdir(home)) + die_errno(_("could not cd home '%s'"), home); + err = fsmonitor_run_daemon_1(&state); done: pthread_cond_destroy(&state.cookies_cond); pthread_mutex_destroy(&state.main_lock); fsm_listen__dtor(&state); + fsm_health__dtor(&state); ipc_server_free(state.ipc_server_data); strbuf_release(&state.path_worktree_watch); strbuf_release(&state.path_gitdir_watch); strbuf_release(&state.path_cookie_prefix); + strbuf_release(&state.path_ipc); return err; } @@ -1423,6 +1503,7 @@ static int try_to_start_background_daemon(void) int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix) { const char *subcmd; + enum fsmonitor_reason reason; int detach_console = 0; struct option options[] = { @@ -1449,6 +1530,23 @@ int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix) die(_("invalid 'ipc-threads' value (%d)"), fsmonitor__ipc_threads); + prepare_repo_settings(the_repository); + /* + * If the repo is fsmonitor-compatible, explicitly set IPC-mode + * (without bothering to load the `core.fsmonitor` config settings). + * + * If the repo is not compatible, the repo-settings will be set to + * incompatible rather than IPC, so we can use one of the __get + * routines to detect the discrepancy. + */ + fsm_settings__set_ipc(the_repository); + + reason = fsm_settings__get_reason(the_repository); + if (reason > FSMONITOR_REASON_OK) + die("%s", + fsm_settings__get_incompatible_msg(the_repository, + reason)); + if (!strcmp(subcmd, "start")) return !!try_to_start_background_daemon(); diff --git a/builtin/update-index.c b/builtin/update-index.c index 43c713c..b622499 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -1257,6 +1257,22 @@ int cmd_update_index(int argc, const char **argv, const char *prefix) if (fsmonitor > 0) { enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r); + enum fsmonitor_reason reason = fsm_settings__get_reason(r); + + /* + * The user wants to turn on FSMonitor using the command + * line argument. (We don't know (or care) whether that + * is the IPC or HOOK version.) + * + * Use one of the __get routines to force load the FSMonitor + * config settings into the repo-settings. That will detect + * whether the file system is compatible so that we can stop + * here with a nice error message. + */ + if (reason > FSMONITOR_REASON_OK) + die("%s", + fsm_settings__get_incompatible_msg(r, reason)); + if (fsm_mode == FSMONITOR_MODE_DISABLED) { warning(_("core.fsmonitor is unset; " "set it if you really want to " diff --git a/compat/fsmonitor/fsm-health-darwin.c b/compat/fsmonitor/fsm-health-darwin.c new file mode 100644 index 0000000..b9f709e --- /dev/null +++ b/compat/fsmonitor/fsm-health-darwin.c @@ -0,0 +1,24 @@ +#include "cache.h" +#include "config.h" +#include "fsmonitor.h" +#include "fsm-health.h" +#include "fsmonitor--daemon.h" + +int fsm_health__ctor(struct fsmonitor_daemon_state *state) +{ + return 0; +} + +void fsm_health__dtor(struct fsmonitor_daemon_state *state) +{ + return; +} + +void fsm_health__loop(struct fsmonitor_daemon_state *state) +{ + return; +} + +void fsm_health__stop_async(struct fsmonitor_daemon_state *state) +{ +} diff --git a/compat/fsmonitor/fsm-health-win32.c b/compat/fsmonitor/fsm-health-win32.c new file mode 100644 index 0000000..2ea08c1 --- /dev/null +++ b/compat/fsmonitor/fsm-health-win32.c @@ -0,0 +1,278 @@ +#include "cache.h" +#include "config.h" +#include "fsmonitor.h" +#include "fsm-health.h" +#include "fsmonitor--daemon.h" + +/* + * Every minute wake up and test our health. + */ +#define WAIT_FREQ_MS (60 * 1000) + +/* + * State machine states for each of the interval functions + * used for polling our health. + */ +enum interval_fn_ctx { + CTX_INIT = 0, + CTX_TERM, + CTX_TIMER +}; + +typedef int (interval_fn)(struct fsmonitor_daemon_state *state, + enum interval_fn_ctx ctx); + +struct fsm_health_data +{ + HANDLE hEventShutdown; + + HANDLE hHandles[1]; /* the array does not own these handles */ +#define HEALTH_SHUTDOWN 0 + int nr_handles; /* number of active event handles */ + + struct wt_moved + { + wchar_t wpath[MAX_PATH + 1]; + BY_HANDLE_FILE_INFORMATION bhfi; + } wt_moved; +}; + +/* + * Lookup the system unique ID for the path. This is as close as + * we get to an inode number, but this also contains volume info, + * so it is a little stronger. + */ +static int lookup_bhfi(wchar_t *wpath, + BY_HANDLE_FILE_INFORMATION *bhfi) +{ + DWORD desired_access = FILE_LIST_DIRECTORY; + DWORD share_mode = + FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE; + HANDLE hDir; + + hDir = CreateFileW(wpath, desired_access, share_mode, NULL, + OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); + if (hDir == INVALID_HANDLE_VALUE) { + error(_("[GLE %ld] health thread could not open '%ls'"), + GetLastError(), wpath); + return -1; + } + + if (!GetFileInformationByHandle(hDir, bhfi)) { + error(_("[GLE %ld] health thread getting BHFI for '%ls'"), + GetLastError(), wpath); + CloseHandle(hDir); + return -1; + } + + CloseHandle(hDir); + return 0; +} + +/* + * Compare the relevant fields from two system unique IDs. + * We use this to see if two different handles to the same + * path actually refer to the same *instance* of the file + * or directory. + */ +static int bhfi_eq(const BY_HANDLE_FILE_INFORMATION *bhfi_1, + const BY_HANDLE_FILE_INFORMATION *bhfi_2) +{ + return (bhfi_1->dwVolumeSerialNumber == bhfi_2->dwVolumeSerialNumber && + bhfi_1->nFileIndexHigh == bhfi_2->nFileIndexHigh && + bhfi_1->nFileIndexLow == bhfi_2->nFileIndexLow); +} + +/* + * Shutdown if the original worktree root directory been deleted, + * moved, or renamed? + * + * Since the main thread did a "chdir(getenv($HOME))" and our CWD + * is not in the worktree root directory and because the listener + * thread added FILE_SHARE_DELETE to the watch handle, it is possible + * for the root directory to be moved or deleted while we are still + * watching it. We want to detect that here and force a shutdown. + * + * Granted, a delete MAY cause some operations to fail, such as + * GetOverlappedResult(), but it is not guaranteed. And because + * ReadDirectoryChangesW() only reports on changes *WITHIN* the + * directory, not changes *ON* the directory, our watch will not + * receive a delete event for it. + * + * A move/rename of the worktree root will also not generate an event. + * And since the listener thread already has an open handle, it may + * continue to receive events for events within the directory. + * However, the pathname of the named-pipe was constructed using the + * original location of the worktree root. (Remember named-pipes are + * stored in the NPFS and not in the actual file system.) Clients + * trying to talk to the worktree after the move/rename will not + * reach our daemon process, since we're still listening on the + * pipe with original path. + * + * Furthermore, if the user does something like: + * + * $ mv repo repo.old + * $ git init repo + * + * A new daemon cannot be started in the new instance of "repo" + * because the named-pipe is still being used by the daemon on + * the original instance. + * + * So, detect move/rename/delete and shutdown. This should also + * handle unsafe drive removal. + * + * We use the file system unique ID to distinguish the original + * directory instance from a new instance and force a shutdown + * if the unique ID changes. + * + * Since a worktree move/rename/delete/unmount doesn't happen + * that often (and we can't get an immediate event anyway), we + * use a timeout and periodically poll it. + */ +static int has_worktree_moved(struct fsmonitor_daemon_state *state, + enum interval_fn_ctx ctx) +{ + struct fsm_health_data *data = state->health_data; + BY_HANDLE_FILE_INFORMATION bhfi; + int r; + + switch (ctx) { + case CTX_TERM: + return 0; + + case CTX_INIT: + if (xutftowcs_path(data->wt_moved.wpath, + state->path_worktree_watch.buf) < 0) { + error(_("could not convert to wide characters: '%s'"), + state->path_worktree_watch.buf); + return -1; + } + + /* + * On the first call we lookup the unique sequence ID for + * the worktree root directory. + */ + return lookup_bhfi(data->wt_moved.wpath, &data->wt_moved.bhfi); + + case CTX_TIMER: + r = lookup_bhfi(data->wt_moved.wpath, &bhfi); + if (r) + return r; + if (!bhfi_eq(&data->wt_moved.bhfi, &bhfi)) { + error(_("BHFI changed '%ls'"), data->wt_moved.wpath); + return -1; + } + return 0; + + default: + die(_("unhandled case in 'has_worktree_moved': %d"), + (int)ctx); + } + + return 0; +} + + +int fsm_health__ctor(struct fsmonitor_daemon_state *state) +{ + struct fsm_health_data *data; + + CALLOC_ARRAY(data, 1); + + data->hEventShutdown = CreateEvent(NULL, TRUE, FALSE, NULL); + + data->hHandles[HEALTH_SHUTDOWN] = data->hEventShutdown; + data->nr_handles++; + + state->health_data = data; + return 0; +} + +void fsm_health__dtor(struct fsmonitor_daemon_state *state) +{ + struct fsm_health_data *data; + + if (!state || !state->health_data) + return; + + data = state->health_data; + + CloseHandle(data->hEventShutdown); + + FREE_AND_NULL(state->health_data); +} + +/* + * A table of the polling functions. + */ +static interval_fn *table[] = { + has_worktree_moved, + NULL, /* must be last */ +}; + +/* + * Call all of the polling functions in the table. + * Shortcut and return first error. + * + * Return 0 if all succeeded. + */ +static int call_all(struct fsmonitor_daemon_state *state, + enum interval_fn_ctx ctx) +{ + int k; + + for (k = 0; table[k]; k++) { + int r = table[k](state, ctx); + if (r) + return r; + } + + return 0; +} + +void fsm_health__loop(struct fsmonitor_daemon_state *state) +{ + struct fsm_health_data *data = state->health_data; + int r; + + r = call_all(state, CTX_INIT); + if (r < 0) + goto force_error_stop; + if (r > 0) + goto force_shutdown; + + for (;;) { + DWORD dwWait = WaitForMultipleObjects(data->nr_handles, + data->hHandles, + FALSE, WAIT_FREQ_MS); + + if (dwWait == WAIT_OBJECT_0 + HEALTH_SHUTDOWN) + goto clean_shutdown; + + if (dwWait == WAIT_TIMEOUT) { + r = call_all(state, CTX_TIMER); + if (r < 0) + goto force_error_stop; + if (r > 0) + goto force_shutdown; + continue; + } + + error(_("health thread wait failed [GLE %ld]"), + GetLastError()); + goto force_error_stop; + } + +force_error_stop: + state->health_error_code = -1; +force_shutdown: + ipc_server_stop_async(state->ipc_server_data); +clean_shutdown: + call_all(state, CTX_TERM); + return; +} + +void fsm_health__stop_async(struct fsmonitor_daemon_state *state) +{ + SetEvent(state->health_data->hHandles[HEALTH_SHUTDOWN]); +} diff --git a/compat/fsmonitor/fsm-health.h b/compat/fsmonitor/fsm-health.h new file mode 100644 index 0000000..45547ba --- /dev/null +++ b/compat/fsmonitor/fsm-health.h @@ -0,0 +1,47 @@ +#ifndef FSM_HEALTH_H +#define FSM_HEALTH_H + +/* This needs to be implemented by each backend */ + +#ifdef HAVE_FSMONITOR_DAEMON_BACKEND + +struct fsmonitor_daemon_state; + +/* + * Initialize platform-specific data for the fsmonitor health thread. + * This will be called from the main thread PRIOR to staring the + * thread. + * + * Returns 0 if successful. + * Returns -1 otherwise. + */ +int fsm_health__ctor(struct fsmonitor_daemon_state *state); + +/* + * Cleanup platform-specific data for the health thread. + * This will be called from the main thread AFTER joining the thread. + */ +void fsm_health__dtor(struct fsmonitor_daemon_state *state); + +/* + * The main body of the platform-specific event loop to monitor the + * health of the daemon process. This will run in the health thread. + * + * The health thread should call `ipc_server_stop_async()` if it needs + * to cause a shutdown. (It should NOT do so if it receives a shutdown + * shutdown signal.) + * + * It should set `state->health_error_code` to -1 if the daemon should exit + * with an error. + */ +void fsm_health__loop(struct fsmonitor_daemon_state *state); + +/* + * Gently request that the health thread shutdown. + * It does not wait for it to stop. The caller should do a JOIN + * to wait for it. + */ +void fsm_health__stop_async(struct fsmonitor_daemon_state *state); + +#endif /* HAVE_FSMONITOR_DAEMON_BACKEND */ +#endif /* FSM_HEALTH_H */ diff --git a/compat/fsmonitor/fsm-listen-darwin.c b/compat/fsmonitor/fsm-listen-darwin.c index dc8a331..8e208e8 100644 --- a/compat/fsmonitor/fsm-listen-darwin.c +++ b/compat/fsmonitor/fsm-listen-darwin.c @@ -27,7 +27,7 @@ #include "fsm-listen.h" #include "fsmonitor--daemon.h" -struct fsmonitor_daemon_backend_data +struct fsm_listen_data { CFStringRef cfsr_worktree_path; CFStringRef cfsr_gitdir_path; @@ -100,12 +100,17 @@ static void log_flags_set(const char *path, const FSEventStreamEventFlags flag) if (flag & kFSEventStreamEventFlagItemCloned) strbuf_addstr(&msg, "ItemCloned|"); - trace_printf_key(&trace_fsmonitor, "fsevent: '%s', flags=%u %s", + trace_printf_key(&trace_fsmonitor, "fsevent: '%s', flags=0x%x %s", path, flag, msg.buf); strbuf_release(&msg); } +static int ef_is_root_changed(const FSEventStreamEventFlags ef) +{ + return (ef & kFSEventStreamEventFlagRootChanged); +} + static int ef_is_root_delete(const FSEventStreamEventFlags ef) { return (ef & kFSEventStreamEventFlagItemIsDir && @@ -125,6 +130,60 @@ static int ef_is_dropped(const FSEventStreamEventFlags ef) ef & kFSEventStreamEventFlagUserDropped); } +/* + * If an `xattr` change is the only reason we received this event, + * then silently ignore it. Git doesn't care about xattr's. We + * have to be careful here because the kernel can combine multiple + * events for a single path. And because events always have certain + * bits set, such as `ItemIsFile` or `ItemIsDir`. + * + * Return 1 if we should ignore it. + */ +static int ef_ignore_xattr(const FSEventStreamEventFlags ef) +{ + static const FSEventStreamEventFlags mask = + kFSEventStreamEventFlagItemChangeOwner | + kFSEventStreamEventFlagItemCreated | + kFSEventStreamEventFlagItemFinderInfoMod | + kFSEventStreamEventFlagItemInodeMetaMod | + kFSEventStreamEventFlagItemModified | + kFSEventStreamEventFlagItemRemoved | + kFSEventStreamEventFlagItemRenamed | + kFSEventStreamEventFlagItemXattrMod | + kFSEventStreamEventFlagItemCloned; + + return ((ef & mask) == kFSEventStreamEventFlagItemXattrMod); +} + +/* + * On MacOS we have to adjust for Unicode composition insensitivity + * (where NFC and NFD spellings are not respected). The different + * spellings are essentially aliases regardless of how the path is + * actually stored on the disk. + * + * This is related to "core.precomposeUnicode" (which wants to try + * to hide NFD completely and treat everything as NFC). Here, we + * don't know what the value the client has (or will have) for this + * config setting when they make a query, so assume the worst and + * emit both when the OS gives us an NFD path. + */ +static void my_add_path(struct fsmonitor_batch *batch, const char *path) +{ + char *composed; + + /* add the NFC or NFD path as received from the OS */ + fsmonitor_batch__add_path(batch, path); + + /* if NFD, also add the corresponding NFC spelling */ + composed = (char *)precompose_string_if_needed(path); + if (!composed || composed == path) + return; + + fsmonitor_batch__add_path(batch, composed); + free(composed); +} + + static void fsevent_callback(ConstFSEventStreamRef streamRef, void *ctx, size_t num_of_events, @@ -133,7 +192,7 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef, const FSEventStreamEventId event_ids[]) { struct fsmonitor_daemon_state *state = ctx; - struct fsmonitor_daemon_backend_data *data = state->backend_data; + struct fsm_listen_data *data = state->listen_data; char **paths = (char **)event_paths; struct fsmonitor_batch *batch = NULL; struct string_list cookie_list = STRING_LIST_INIT_DUP; @@ -190,6 +249,33 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef, continue; } + if (ef_is_root_changed(event_flags[k])) { + /* + * The spelling of the pathname of the root directory + * has changed. This includes the name of the root + * directory itself or of any parent directory in the + * path. + * + * (There may be other conditions that throw this, + * but I couldn't find any information on it.) + * + * Force a shutdown now and avoid things getting + * out of sync. The Unix domain socket is inside + * the .git directory and a spelling change will make + * it hard for clients to rendezvous with us. + */ + trace_printf_key(&trace_fsmonitor, + "event: root changed"); + goto force_shutdown; + } + + if (ef_ignore_xattr(event_flags[k])) { + trace_printf_key(&trace_fsmonitor, + "ignore-xattr: '%s', flags=0x%x", + path_k, event_flags[k]); + continue; + } + switch (fsmonitor_classify_path_absolute(state, path_k)) { case IS_INSIDE_DOT_GIT_WITH_COOKIE_PREFIX: @@ -248,7 +334,7 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef, if (!batch) batch = fsmonitor_batch__new(); - fsmonitor_batch__add_path(batch, rel); + my_add_path(batch, rel); } if (event_flags[k] & kFSEventStreamEventFlagItemIsDir) { @@ -261,7 +347,7 @@ static void fsevent_callback(ConstFSEventStreamRef streamRef, if (!batch) batch = fsmonitor_batch__new(); - fsmonitor_batch__add_path(batch, tmp.buf); + my_add_path(batch, tmp.buf); } break; @@ -318,11 +404,11 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state) NULL, NULL }; - struct fsmonitor_daemon_backend_data *data; + struct fsm_listen_data *data; const void *dir_array[2]; CALLOC_ARRAY(data, 1); - state->backend_data = data; + state->listen_data = data; data->cfsr_worktree_path = CFStringCreateWithCString( NULL, state->path_worktree_watch.buf, kCFStringEncodingUTF8); @@ -354,18 +440,18 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state) failed: error(_("Unable to create FSEventStream.")); - FREE_AND_NULL(state->backend_data); + FREE_AND_NULL(state->listen_data); return -1; } void fsm_listen__dtor(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data; + struct fsm_listen_data *data; - if (!state || !state->backend_data) + if (!state || !state->listen_data) return; - data = state->backend_data; + data = state->listen_data; if (data->stream) { if (data->stream_started) @@ -375,14 +461,14 @@ void fsm_listen__dtor(struct fsmonitor_daemon_state *state) FSEventStreamRelease(data->stream); } - FREE_AND_NULL(state->backend_data); + FREE_AND_NULL(state->listen_data); } void fsm_listen__stop_async(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data; + struct fsm_listen_data *data; - data = state->backend_data; + data = state->listen_data; data->shutdown_style = SHUTDOWN_EVENT; CFRunLoopStop(data->rl); @@ -390,9 +476,9 @@ void fsm_listen__stop_async(struct fsmonitor_daemon_state *state) void fsm_listen__loop(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data; + struct fsm_listen_data *data; - data = state->backend_data; + data = state->listen_data; data->rl = CFRunLoopGetCurrent(); @@ -409,7 +495,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) switch (data->shutdown_style) { case FORCE_ERROR_STOP: - state->error_code = -1; + state->listen_error_code = -1; /* fall thru */ case FORCE_SHUTDOWN: ipc_server_stop_async(state->ipc_server_data); @@ -421,7 +507,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) return; force_error_stop_without_loop: - state->error_code = -1; + state->listen_error_code = -1; ipc_server_stop_async(state->ipc_server_data); return; } diff --git a/compat/fsmonitor/fsm-listen-win32.c b/compat/fsmonitor/fsm-listen-win32.c index 5b928ab..03df8d9 100644 --- a/compat/fsmonitor/fsm-listen-win32.c +++ b/compat/fsmonitor/fsm-listen-win32.c @@ -25,6 +25,9 @@ struct one_watch DWORD count; struct strbuf path; + wchar_t wpath_longname[MAX_PATH + 1]; + DWORD wpath_longname_len; + HANDLE hDir; HANDLE hEvent; OVERLAPPED overlapped; @@ -34,9 +37,24 @@ struct one_watch * need to later call GetOverlappedResult() and possibly CancelIoEx(). */ BOOL is_active; + + /* + * Are shortnames enabled on the containing drive? This is + * always true for "C:/" drives and usually never true for + * other drives. + * + * We only set this for the worktree because we only need to + * convert shortname paths to longname paths for items we send + * to clients. (We don't care about shortname expansion for + * paths inside a GITDIR because we never send them to + * clients.) + */ + BOOL has_shortnames; + BOOL has_tilde; + wchar_t dotgit_shortname[16]; /* for 8.3 name */ }; -struct fsmonitor_daemon_backend_data +struct fsm_listen_data { struct one_watch *watch_worktree; struct one_watch *watch_gitdir; @@ -51,17 +69,18 @@ struct fsmonitor_daemon_backend_data }; /* - * Convert the WCHAR path from the notification into UTF8 and - * then normalize it. + * Convert the WCHAR path from the event into UTF8 and normalize it. + * + * `wpath_len` is in WCHARS not bytes. */ -static int normalize_path_in_utf8(FILE_NOTIFY_INFORMATION *info, +static int normalize_path_in_utf8(wchar_t *wpath, DWORD wpath_len, struct strbuf *normalized_path) { int reserve; int len = 0; strbuf_reset(normalized_path); - if (!info->FileNameLength) + if (!wpath_len) goto normalize; /* @@ -70,12 +89,12 @@ static int normalize_path_in_utf8(FILE_NOTIFY_INFORMATION *info, * sequence of 2 UTF8 characters. That should let us * avoid ERROR_INSUFFICIENT_BUFFER 99.9+% of the time. */ - reserve = info->FileNameLength + 1; + reserve = 2 * wpath_len + 1; strbuf_grow(normalized_path, reserve); for (;;) { - len = WideCharToMultiByte(CP_UTF8, 0, info->FileName, - info->FileNameLength / sizeof(WCHAR), + len = WideCharToMultiByte(CP_UTF8, 0, + wpath, wpath_len, normalized_path->buf, strbuf_avail(normalized_path) - 1, NULL, NULL); @@ -83,9 +102,7 @@ static int normalize_path_in_utf8(FILE_NOTIFY_INFORMATION *info, goto normalize; if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) { error(_("[GLE %ld] could not convert path to UTF-8: '%.*ls'"), - GetLastError(), - (int)(info->FileNameLength / sizeof(WCHAR)), - info->FileName); + GetLastError(), (int)wpath_len, wpath); return -1; } @@ -98,9 +115,176 @@ normalize: return strbuf_normalize_path(normalized_path); } +/* + * See if the worktree root directory has shortnames enabled. + * This will help us decide if we need to do an expensive shortname + * to longname conversion on every notification event. + * + * We do not want to create a file to test this, so we assume that the + * root directory contains a ".git" file or directory. (Our caller + * only calls us for the worktree root, so this should be fine.) + * + * Remember the spelling of the shortname for ".git" if it exists. + */ +static void check_for_shortnames(struct one_watch *watch) +{ + wchar_t buf_in[MAX_PATH + 1]; + wchar_t buf_out[MAX_PATH + 1]; + wchar_t *last; + wchar_t *p; + + /* build L"<wt-root-path>/.git" */ + swprintf(buf_in, ARRAY_SIZE(buf_in) - 1, L"%ls.git", + watch->wpath_longname); + + if (!GetShortPathNameW(buf_in, buf_out, ARRAY_SIZE(buf_out))) + return; + + /* + * Get the final filename component of the shortpath. + * We know that the path does not have a final slash. + */ + for (last = p = buf_out; *p; p++) + if (*p == L'/' || *p == '\\') + last = p + 1; + + if (!wcscmp(last, L".git")) + return; + + watch->has_shortnames = 1; + wcsncpy(watch->dotgit_shortname, last, + ARRAY_SIZE(watch->dotgit_shortname)); + + /* + * The shortname for ".git" is usually of the form "GIT~1", so + * we should be able to avoid shortname to longname mapping on + * every notification event if the source string does not + * contain a "~". + * + * However, the documentation for GetLongPathNameW() says + * that there are filesystems that don't follow that pattern + * and warns against this optimization. + * + * Lets test this. + */ + if (wcschr(watch->dotgit_shortname, L'~')) + watch->has_tilde = 1; +} + +enum get_relative_result { + GRR_NO_CONVERSION_NEEDED, + GRR_HAVE_CONVERSION, + GRR_SHUTDOWN, +}; + +/* + * Info notification paths are relative to the root of the watch. + * If our CWD is still at the root, then we can use relative paths + * to convert from shortnames to longnames. If our process has a + * different CWD, then we need to construct an absolute path, do + * the conversion, and then return the root-relative portion. + * + * We use the longname form of the root as our basis and assume that + * it already has a trailing slash. + * + * `wpath_len` is in WCHARS not bytes. + */ +static enum get_relative_result get_relative_longname( + struct one_watch *watch, + const wchar_t *wpath, DWORD wpath_len, + wchar_t *wpath_longname, size_t bufsize_wpath_longname) +{ + wchar_t buf_in[2 * MAX_PATH + 1]; + wchar_t buf_out[MAX_PATH + 1]; + DWORD root_len; + DWORD out_len; + + /* + * Build L"<wt-root-path>/<event-rel-path>" + * Note that the <event-rel-path> might not be null terminated + * so we avoid swprintf() constructions. + */ + root_len = watch->wpath_longname_len; + if (root_len + wpath_len >= ARRAY_SIZE(buf_in)) { + /* + * This should not happen. We cannot append the observed + * relative path onto the end of the worktree root path + * without overflowing the buffer. Just give up. + */ + return GRR_SHUTDOWN; + } + wcsncpy(buf_in, watch->wpath_longname, root_len); + wcsncpy(buf_in + root_len, wpath, wpath_len); + buf_in[root_len + wpath_len] = 0; + + /* + * We don't actually know if the source pathname is a + * shortname or a longname. This Windows routine allows + * either to be given as input. + */ + out_len = GetLongPathNameW(buf_in, buf_out, ARRAY_SIZE(buf_out)); + if (!out_len) { + /* + * The shortname to longname conversion can fail for + * various reasons, for example if the file has been + * deleted. (That is, if we just received a + * delete-file notification event and the file is + * already gone, we can't ask the file system to + * lookup the longname for it. Likewise, for moves + * and renames where we are given the old name.) + * + * Since deleting or moving a file or directory by its + * shortname is rather obscure, I'm going ignore the + * failure and ask the caller to report the original + * relative path. This seems kinder than failing here + * and forcing a resync. Besides, forcing a resync on + * every file/directory delete would effectively + * cripple monitoring. + * + * We might revisit this in the future. + */ + return GRR_NO_CONVERSION_NEEDED; + } + + if (!wcscmp(buf_in, buf_out)) { + /* + * The path does not have a shortname alias. + */ + return GRR_NO_CONVERSION_NEEDED; + } + + if (wcsncmp(buf_in, buf_out, root_len)) { + /* + * The spelling of the root directory portion of the computed + * longname has changed. This should not happen. Basically, + * it means that we don't know where (without recomputing the + * longname of just the root directory) to split out the + * relative path. Since this should not happen, I'm just + * going to let this fail and force a shutdown (because all + * subsequent events are probably going to see the same + * mismatch). + */ + return GRR_SHUTDOWN; + } + + if (out_len - root_len >= bufsize_wpath_longname) { + /* + * This should not happen. We cannot copy the root-relative + * portion of the path into the provided buffer without an + * overrun. Just give up. + */ + return GRR_SHUTDOWN; + } + + /* Return the worktree root-relative portion of the longname. */ + + wcscpy(wpath_longname, buf_out + root_len); + return GRR_HAVE_CONVERSION; +} + void fsm_listen__stop_async(struct fsmonitor_daemon_state *state) { - SetEvent(state->backend_data->hListener[LISTENER_SHUTDOWN]); + SetEvent(state->listen_data->hListener[LISTENER_SHUTDOWN]); } static struct one_watch *create_watch(struct fsmonitor_daemon_state *state, @@ -111,7 +295,9 @@ static struct one_watch *create_watch(struct fsmonitor_daemon_state *state, DWORD share_mode = FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE; HANDLE hDir; - wchar_t wpath[MAX_PATH]; + DWORD len_longname; + wchar_t wpath[MAX_PATH + 1]; + wchar_t wpath_longname[MAX_PATH + 1]; if (xutftowcs_path(wpath, path) < 0) { error(_("could not convert to wide characters: '%s'"), path); @@ -128,6 +314,21 @@ static struct one_watch *create_watch(struct fsmonitor_daemon_state *state, return NULL; } + len_longname = GetLongPathNameW(wpath, wpath_longname, + ARRAY_SIZE(wpath_longname)); + if (!len_longname) { + error(_("[GLE %ld] could not get longname of '%s'"), + GetLastError(), path); + CloseHandle(hDir); + return NULL; + } + + if (wpath_longname[len_longname - 1] != L'/' && + wpath_longname[len_longname - 1] != L'\\') { + wpath_longname[len_longname++] = L'/'; + wpath_longname[len_longname] = 0; + } + CALLOC_ARRAY(watch, 1); watch->buf_len = sizeof(watch->buffer); /* assume full MAX_RDCW_BUF */ @@ -135,6 +336,9 @@ static struct one_watch *create_watch(struct fsmonitor_daemon_state *state, strbuf_init(&watch->path, 0); strbuf_addstr(&watch->path, path); + wcscpy(watch->wpath_longname, wpath_longname); + watch->wpath_longname_len = len_longname; + watch->hDir = hDir; watch->hEvent = CreateEvent(NULL, TRUE, FALSE, NULL); @@ -155,7 +359,7 @@ static void destroy_watch(struct one_watch *watch) free(watch); } -static int start_rdcw_watch(struct fsmonitor_daemon_backend_data *data, +static int start_rdcw_watch(struct fsm_listen_data *data, struct one_watch *watch) { DWORD dwNotifyFilter = @@ -220,12 +424,22 @@ static int recv_rdcw_watch(struct one_watch *watch) } /* - * NEEDSWORK: If an external <gitdir> is deleted, the above - * returns an error. I'm not sure that there's anything that - * we can do here other than failing -- the <worktree>/.git - * link file would be broken anyway. We might try to check - * for that and return a better error message, but I'm not - * sure it is worth it. + * GetOverlappedResult() fails if the watched directory is + * deleted while we were waiting for an overlapped IO to + * complete. The documentation did not list specific errors, + * but I observed ERROR_ACCESS_DENIED (0x05) errors during + * testing. + * + * Note that we only get notificaiton events for events + * *within* the directory, not *on* the directory itself. + * (These might be properies of the parent directory, for + * example). + * + * NEEDSWORK: We might try to check for the deleted directory + * case and return a better error message, but I'm not sure it + * is worth it. + * + * Shutdown if we get any error. */ error(_("GetOverlappedResult failed on '%s' [GLE %ld]"), @@ -259,6 +473,62 @@ static void cancel_rdcw_watch(struct one_watch *watch) } /* + * Process a single relative pathname event. + * Return 1 if we should shutdown. + */ +static int process_1_worktree_event( + struct string_list *cookie_list, + struct fsmonitor_batch **batch, + const struct strbuf *path, + enum fsmonitor_path_type t, + DWORD info_action) +{ + const char *slash; + + switch (t) { + case IS_INSIDE_DOT_GIT_WITH_COOKIE_PREFIX: + /* special case cookie files within .git */ + + /* Use just the filename of the cookie file. */ + slash = find_last_dir_sep(path->buf); + string_list_append(cookie_list, + slash ? slash + 1 : path->buf); + break; + + case IS_INSIDE_DOT_GIT: + /* ignore everything inside of "<worktree>/.git/" */ + break; + + case IS_DOT_GIT: + /* "<worktree>/.git" was deleted (or renamed away) */ + if ((info_action == FILE_ACTION_REMOVED) || + (info_action == FILE_ACTION_RENAMED_OLD_NAME)) { + trace2_data_string("fsmonitor", NULL, + "fsm-listen/dotgit", + "removed"); + return 1; + } + break; + + case IS_WORKDIR_PATH: + /* queue normal pathname */ + if (!*batch) + *batch = fsmonitor_batch__new(); + fsmonitor_batch__add_path(*batch, path->buf); + break; + + case IS_GITDIR: + case IS_INSIDE_GITDIR: + case IS_INSIDE_GITDIR_WITH_COOKIE_PREFIX: + default: + BUG("unexpected path classification '%d' for '%s'", + t, path->buf); + } + + return 0; +} + +/* * Process filesystem events that happen anywhere (recursively) under the * <worktree> root directory. For a normal working directory, this includes * both version controlled files and the contents of the .git/ directory. @@ -268,12 +538,13 @@ static void cancel_rdcw_watch(struct one_watch *watch) */ static int process_worktree_events(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data = state->backend_data; + struct fsm_listen_data *data = state->listen_data; struct one_watch *watch = data->watch_worktree; struct strbuf path = STRBUF_INIT; struct string_list cookie_list = STRING_LIST_INIT_DUP; struct fsmonitor_batch *batch = NULL; const char *p = watch->buffer; + wchar_t wpath_longname[MAX_PATH + 1]; /* * If the kernel gets more events than will fit in the kernel @@ -306,54 +577,64 @@ static int process_worktree_events(struct fsmonitor_daemon_state *state) */ for (;;) { FILE_NOTIFY_INFORMATION *info = (void *)p; - const char *slash; + wchar_t *wpath = info->FileName; + DWORD wpath_len = info->FileNameLength / sizeof(WCHAR); enum fsmonitor_path_type t; + enum get_relative_result grr; + + if (watch->has_shortnames) { + if (!wcscmp(wpath, watch->dotgit_shortname)) { + /* + * This event exactly matches the + * spelling of the shortname of + * ".git", so we can skip some steps. + * + * (This case is odd because the user + * can "rm -rf GIT~1" and we cannot + * use the filesystem to map it back + * to ".git".) + */ + strbuf_reset(&path); + strbuf_addstr(&path, ".git"); + t = IS_DOT_GIT; + goto process_it; + } - strbuf_reset(&path); - if (normalize_path_in_utf8(info, &path) == -1) - goto skip_this_path; - - t = fsmonitor_classify_path_workdir_relative(path.buf); - - switch (t) { - case IS_INSIDE_DOT_GIT_WITH_COOKIE_PREFIX: - /* special case cookie files within .git */ - - /* Use just the filename of the cookie file. */ - slash = find_last_dir_sep(path.buf); - string_list_append(&cookie_list, - slash ? slash + 1 : path.buf); - break; - - case IS_INSIDE_DOT_GIT: - /* ignore everything inside of "<worktree>/.git/" */ - break; + if (watch->has_tilde && !wcschr(wpath, L'~')) { + /* + * Shortnames on this filesystem have tildes + * and the notification path does not have + * one, so we assume that it is a longname. + */ + goto normalize_it; + } - case IS_DOT_GIT: - /* "<worktree>/.git" was deleted (or renamed away) */ - if ((info->Action == FILE_ACTION_REMOVED) || - (info->Action == FILE_ACTION_RENAMED_OLD_NAME)) { - trace2_data_string("fsmonitor", NULL, - "fsm-listen/dotgit", - "removed"); + grr = get_relative_longname(watch, wpath, wpath_len, + wpath_longname, + ARRAY_SIZE(wpath_longname)); + switch (grr) { + case GRR_NO_CONVERSION_NEEDED: /* use info buffer as is */ + break; + case GRR_HAVE_CONVERSION: + wpath = wpath_longname; + wpath_len = wcslen(wpath); + break; + default: + case GRR_SHUTDOWN: goto force_shutdown; } - break; + } - case IS_WORKDIR_PATH: - /* queue normal pathname */ - if (!batch) - batch = fsmonitor_batch__new(); - fsmonitor_batch__add_path(batch, path.buf); - break; +normalize_it: + if (normalize_path_in_utf8(wpath, wpath_len, &path) == -1) + goto skip_this_path; - case IS_GITDIR: - case IS_INSIDE_GITDIR: - case IS_INSIDE_GITDIR_WITH_COOKIE_PREFIX: - default: - BUG("unexpected path classification '%d' for '%s'", - t, path.buf); - } + t = fsmonitor_classify_path_workdir_relative(path.buf); + +process_it: + if (process_1_worktree_event(&cookie_list, &batch, &path, t, + info->Action)) + goto force_shutdown; skip_this_path: if (!info->NextEntryOffset) @@ -382,10 +663,13 @@ force_shutdown: * Note that we DO NOT get filesystem events on the external <gitdir> * itself (it is not inside something that we are watching). In particular, * we do not get an event if the external <gitdir> is deleted. + * + * Also, we do not care about shortnames within the external <gitdir>, since + * we never send these paths to clients. */ static int process_gitdir_events(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data = state->backend_data; + struct fsm_listen_data *data = state->listen_data; struct one_watch *watch = data->watch_gitdir; struct strbuf path = STRBUF_INIT; struct string_list cookie_list = STRING_LIST_INIT_DUP; @@ -403,8 +687,10 @@ static int process_gitdir_events(struct fsmonitor_daemon_state *state) const char *slash; enum fsmonitor_path_type t; - strbuf_reset(&path); - if (normalize_path_in_utf8(info, &path) == -1) + if (normalize_path_in_utf8( + info->FileName, + info->FileNameLength / sizeof(WCHAR), + &path) == -1) goto skip_this_path; t = fsmonitor_classify_path_gitdir_relative(path.buf); @@ -441,11 +727,11 @@ skip_this_path: void fsm_listen__loop(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data = state->backend_data; + struct fsm_listen_data *data = state->listen_data; DWORD dwWait; int result; - state->error_code = 0; + state->listen_error_code = 0; if (start_rdcw_watch(data, data->watch_worktree) == -1) goto force_error_stop; @@ -510,7 +796,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state) } force_error_stop: - state->error_code = -1; + state->listen_error_code = -1; force_shutdown: /* @@ -527,7 +813,7 @@ clean_shutdown: int fsm_listen__ctor(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data; + struct fsm_listen_data *data; CALLOC_ARRAY(data, 1); @@ -538,6 +824,8 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state) if (!data->watch_worktree) goto failed; + check_for_shortnames(data->watch_worktree); + if (state->nr_paths_watching > 1) { data->watch_gitdir = create_watch(state, state->path_gitdir_watch.buf); @@ -558,7 +846,7 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state) data->nr_listener_handles++; } - state->backend_data = data; + state->listen_data = data; return 0; failed: @@ -571,16 +859,16 @@ failed: void fsm_listen__dtor(struct fsmonitor_daemon_state *state) { - struct fsmonitor_daemon_backend_data *data; + struct fsm_listen_data *data; - if (!state || !state->backend_data) + if (!state || !state->listen_data) return; - data = state->backend_data; + data = state->listen_data; CloseHandle(data->hEventShutdown); destroy_watch(data->watch_worktree); destroy_watch(data->watch_gitdir); - FREE_AND_NULL(state->backend_data); + FREE_AND_NULL(state->listen_data); } diff --git a/compat/fsmonitor/fsm-listen.h b/compat/fsmonitor/fsm-listen.h index f053934..41650bf 100644 --- a/compat/fsmonitor/fsm-listen.h +++ b/compat/fsmonitor/fsm-listen.h @@ -33,7 +33,7 @@ void fsm_listen__dtor(struct fsmonitor_daemon_state *state); * do so if the listener thread receives a normal shutdown signal from * the IPC layer.) * - * It should set `state->error_code` to -1 if the daemon should exit + * It should set `state->listen_error_code` to -1 if the daemon should exit * with an error. */ void fsm_listen__loop(struct fsmonitor_daemon_state *state); diff --git a/compat/fsmonitor/fsm-settings-darwin.c b/compat/fsmonitor/fsm-settings-darwin.c new file mode 100644 index 0000000..efc732c --- /dev/null +++ b/compat/fsmonitor/fsm-settings-darwin.c @@ -0,0 +1,89 @@ +#include "cache.h" +#include "config.h" +#include "repository.h" +#include "fsmonitor-settings.h" +#include "fsmonitor.h" +#include <sys/param.h> +#include <sys/mount.h> + +/* + * [1] Remote working directories are problematic for FSMonitor. + * + * The underlying file system on the server machine and/or the remote + * mount type (NFS, SAMBA, etc.) dictates whether notification events + * are available at all to remote client machines. + * + * Kernel differences between the server and client machines also + * dictate the how (buffering, frequency, de-dup) the events are + * delivered to client machine processes. + * + * A client machine (such as a laptop) may choose to suspend/resume + * and it is unclear (without lots of testing) whether the watcher can + * resync after a resume. We might be able to treat this as a normal + * "events were dropped by the kernel" event and do our normal "flush + * and resync" --or-- we might need to close the existing (zombie?) + * notification fd and create a new one. + * + * In theory, the above issues need to be addressed whether we are + * using the Hook or IPC API. + * + * For the builtin FSMonitor, we create the Unix domain socket for the + * IPC in the .git directory. If the working directory is remote, + * then the socket will be created on the remote file system. This + * can fail if the remote file system does not support UDS file types + * (e.g. smbfs to a Windows server) or if the remote kernel does not + * allow a non-local process to bind() the socket. (These problems + * could be fixed by moving the UDS out of the .git directory and to a + * well-known local directory on the client machine, but care should + * be taken to ensure that $HOME is actually local and not a managed + * file share.) + * + * So (for now at least), mark remote working directories as + * incompatible. + * + * + * [2] FAT32 and NTFS working directories are problematic too. + * + * The builtin FSMonitor uses a Unix domain socket in the .git + * directory for IPC. These Windows drive formats do not support + * Unix domain sockets, so mark them as incompatible for the daemon. + * + */ +static enum fsmonitor_reason check_volume(struct repository *r) +{ + struct statfs fs; + + if (statfs(r->worktree, &fs) == -1) { + int saved_errno = errno; + trace_printf_key(&trace_fsmonitor, "statfs('%s') failed: %s", + r->worktree, strerror(saved_errno)); + errno = saved_errno; + return FSMONITOR_REASON_ERROR; + } + + 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); + + if (!(fs.f_flags & MNT_LOCAL)) + return FSMONITOR_REASON_REMOTE; + + if (!strcmp(fs.f_fstypename, "msdos")) /* aka FAT32 */ + return FSMONITOR_REASON_NOSOCKETS; + + if (!strcmp(fs.f_fstypename, "ntfs")) + return FSMONITOR_REASON_NOSOCKETS; + + return FSMONITOR_REASON_OK; +} + +enum fsmonitor_reason fsm_os__incompatible(struct repository *r) +{ + enum fsmonitor_reason reason; + + reason = check_volume(r); + if (reason != FSMONITOR_REASON_OK) + return reason; + + return FSMONITOR_REASON_OK; +} diff --git a/compat/fsmonitor/fsm-settings-win32.c b/compat/fsmonitor/fsm-settings-win32.c new file mode 100644 index 0000000..9076557 --- /dev/null +++ b/compat/fsmonitor/fsm-settings-win32.c @@ -0,0 +1,137 @@ +#include "cache.h" +#include "config.h" +#include "repository.h" +#include "fsmonitor-settings.h" +#include "fsmonitor.h" + +/* + * VFS for Git is incompatible with FSMonitor. + * + * Granted, core Git does not know anything about VFS for Git and we + * shouldn't make assumptions about a downstream feature, but users + * can install both versions. And this can lead to incorrect results + * from core Git commands. So, without bringing in any of the VFS for + * Git code, do a simple config test for a published config setting. + * (We do not look at the various *_TEST_* environment variables.) + */ +static enum fsmonitor_reason check_vfs4git(struct repository *r) +{ + const char *const_str; + + if (!repo_config_get_value(r, "core.virtualfilesystem", &const_str)) + return FSMONITOR_REASON_VFS4GIT; + + return FSMONITOR_REASON_OK; +} + +/* + * Remote working directories are problematic for FSMonitor. + * + * The underlying file system on the server machine and/or the remote + * mount type dictates whether notification events are available at + * all to remote client machines. + * + * Kernel differences between the server and client machines also + * dictate the how (buffering, frequency, de-dup) the events are + * delivered to client machine processes. + * + * A client machine (such as a laptop) may choose to suspend/resume + * and it is unclear (without lots of testing) whether the watcher can + * resync after a resume. We might be able to treat this as a normal + * "events were dropped by the kernel" event and do our normal "flush + * and resync" --or-- we might need to close the existing (zombie?) + * notification fd and create a new one. + * + * In theory, the above issues need to be addressed whether we are + * using the Hook or IPC API. + * + * So (for now at least), mark remote working directories as + * incompatible. + * + * Notes for testing: + * + * (a) Windows allows a network share to be mapped to a drive letter. + * (This is the normal method to access it.) + * + * $ NET USE Z: \\server\share + * $ git -C Z:/repo status + * + * (b) Windows allows a network share to be referenced WITHOUT mapping + * it to drive letter. + * + * $ NET USE \\server\share\dir + * $ git -C //server/share/repo status + * + * (c) Windows allows "SUBST" to create a fake drive mapping to an + * arbitrary path (which may be remote) + * + * $ SUBST Q: Z:\repo + * $ git -C Q:/ status + * + * (d) Windows allows a directory symlink to be created on a local + * file system that points to a remote repo. + * + * $ mklink /d ./link //server/share/repo + * $ git -C ./link status + */ +static enum fsmonitor_reason check_remote(struct repository *r) +{ + wchar_t wpath[MAX_PATH]; + wchar_t wfullpath[MAX_PATH]; + size_t wlen; + UINT driveType; + + /* + * Do everything in wide chars because the drive letter might be + * a multi-byte sequence. See win32_has_dos_drive_prefix(). + */ + if (xutftowcs_path(wpath, r->worktree) < 0) + return FSMONITOR_REASON_ERROR; + + /* + * 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; + } + + /* + * 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 FSMONITOR_REASON_ERROR; + + driveType = GetDriveTypeW(wfullpath); + trace_printf_key(&trace_fsmonitor, + "DriveType '%s' L'%ls' (%u)", + r->worktree, wfullpath, driveType); + + if (driveType == DRIVE_REMOTE) { + trace_printf_key(&trace_fsmonitor, + "check_remote('%s') true", + r->worktree); + return FSMONITOR_REASON_REMOTE; + } + + return FSMONITOR_REASON_OK; +} + +enum fsmonitor_reason fsm_os__incompatible(struct repository *r) +{ + enum fsmonitor_reason reason; + + reason = check_vfs4git(r); + if (reason != FSMONITOR_REASON_OK) + return reason; + + reason = check_remote(r); + if (reason != FSMONITOR_REASON_OK) + return reason; + + return FSMONITOR_REASON_OK; +} diff --git a/config.mak.uname b/config.mak.uname index 259d151..ce83cad 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -164,6 +164,7 @@ ifeq ($(uname_S),Darwin) ifndef NO_PTHREADS ifndef NO_UNIX_SOCKETS FSMONITOR_DAEMON_BACKEND = darwin + FSMONITOR_OS_SETTINGS = darwin endif endif @@ -451,6 +452,8 @@ ifeq ($(uname_S),Windows) # These are always available, so we do not have to conditionally # support it. FSMONITOR_DAEMON_BACKEND = win32 + FSMONITOR_OS_SETTINGS = win32 + NO_SVN_TESTS = YesPlease RUNTIME_PREFIX = YesPlease HAVE_WPGMPTR = YesWeDo @@ -641,6 +644,8 @@ ifeq ($(uname_S),MINGW) # These are always available, so we do not have to conditionally # support it. FSMONITOR_DAEMON_BACKEND = win32 + FSMONITOR_OS_SETTINGS = win32 + RUNTIME_PREFIX = YesPlease HAVE_WPGMPTR = YesWeDo NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 29acc4f..1b23f24 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -307,9 +307,17 @@ if(SUPPORTS_SIMPLE_IPC) if(CMAKE_SYSTEM_NAME STREQUAL "Windows") 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) + + add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-win32.c) elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin") 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) + + add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-darwin.c) endif() endif() diff --git a/fsmonitor--daemon.h b/fsmonitor--daemon.h index bd09fff..2102a5c 100644 --- a/fsmonitor--daemon.h +++ b/fsmonitor--daemon.h @@ -33,10 +33,12 @@ void fsmonitor_batch__free_list(struct fsmonitor_batch *batch); */ void fsmonitor_batch__add_path(struct fsmonitor_batch *batch, const char *path); -struct fsmonitor_daemon_backend_data; /* opaque platform-specific data */ +struct fsm_listen_data; /* opaque platform-specific data for listener thread */ +struct fsm_health_data; /* opaque platform-specific data for health thread */ struct fsmonitor_daemon_state { pthread_t listener_thread; + pthread_t health_thread; pthread_mutex_t main_lock; struct strbuf path_worktree_watch; @@ -50,10 +52,13 @@ struct fsmonitor_daemon_state { int cookie_seq; struct hashmap cookies; - int error_code; - struct fsmonitor_daemon_backend_data *backend_data; + int listen_error_code; + int health_error_code; + struct fsm_listen_data *listen_data; + struct fsm_health_data *health_data; struct ipc_server_data *ipc_server_data; + struct strbuf path_ipc; }; /* diff --git a/fsmonitor-settings.c b/fsmonitor-settings.c index 757d230..658cb79 100644 --- a/fsmonitor-settings.c +++ b/fsmonitor-settings.c @@ -9,23 +9,52 @@ */ struct fsmonitor_settings { enum fsmonitor_mode mode; + enum fsmonitor_reason reason; char *hook_path; }; -static void lookup_fsmonitor_settings(struct repository *r) +static enum fsmonitor_reason check_for_incompatible(struct repository *r) +{ + if (!r->worktree) { + /* + * Bare repositories don't have a working directory and + * therefore have nothing to watch. + */ + return FSMONITOR_REASON_BARE; + } + +#ifdef HAVE_FSMONITOR_OS_SETTINGS + { + enum fsmonitor_reason reason; + + reason = fsm_os__incompatible(r); + if (reason != FSMONITOR_REASON_OK) + return reason; + } +#endif + + return FSMONITOR_REASON_OK; +} + +static struct fsmonitor_settings *alloc_settings(void) { struct fsmonitor_settings *s; + + CALLOC_ARRAY(s, 1); + s->mode = FSMONITOR_MODE_DISABLED; + s->reason = FSMONITOR_REASON_UNTESTED; + + return s; +} + +static void lookup_fsmonitor_settings(struct repository *r) +{ const char *const_str; int bool_value; if (r->settings.fsmonitor) return; - CALLOC_ARRAY(s, 1); - s->mode = FSMONITOR_MODE_DISABLED; - - r->settings.fsmonitor = s; - /* * Overload the existing "core.fsmonitor" config setting (which * has historically been either unset or a hook pathname) to @@ -38,6 +67,8 @@ static void lookup_fsmonitor_settings(struct repository *r) case 0: /* config value was set to <bool> */ if (bool_value) fsm_settings__set_ipc(r); + else + fsm_settings__set_disabled(r); return; case 1: /* config value was unset */ @@ -53,18 +84,18 @@ static void lookup_fsmonitor_settings(struct repository *r) return; } - if (!const_str || !*const_str) - return; - - fsm_settings__set_hook(r, const_str); + if (const_str && *const_str) + fsm_settings__set_hook(r, const_str); + else + fsm_settings__set_disabled(r); } enum fsmonitor_mode fsm_settings__get_mode(struct repository *r) { if (!r) r = the_repository; - - lookup_fsmonitor_settings(r); + if (!r->settings.fsmonitor) + lookup_fsmonitor_settings(r); return r->settings.fsmonitor->mode; } @@ -73,31 +104,55 @@ const char *fsm_settings__get_hook_path(struct repository *r) { if (!r) r = the_repository; - - lookup_fsmonitor_settings(r); + if (!r->settings.fsmonitor) + lookup_fsmonitor_settings(r); return r->settings.fsmonitor->hook_path; } void fsm_settings__set_ipc(struct repository *r) { + enum fsmonitor_reason reason = check_for_incompatible(r); + + if (reason != FSMONITOR_REASON_OK) { + fsm_settings__set_incompatible(r, reason); + return; + } + + /* + * Caller requested IPC explicitly, so avoid (possibly + * recursive) config lookup. + */ if (!r) r = the_repository; - - lookup_fsmonitor_settings(r); + if (!r->settings.fsmonitor) + r->settings.fsmonitor = alloc_settings(); r->settings.fsmonitor->mode = FSMONITOR_MODE_IPC; + r->settings.fsmonitor->reason = reason; FREE_AND_NULL(r->settings.fsmonitor->hook_path); } void fsm_settings__set_hook(struct repository *r, const char *path) { + enum fsmonitor_reason reason = check_for_incompatible(r); + + if (reason != FSMONITOR_REASON_OK) { + fsm_settings__set_incompatible(r, reason); + return; + } + + /* + * Caller requested hook explicitly, so avoid (possibly + * recursive) config lookup. + */ if (!r) r = the_repository; - - lookup_fsmonitor_settings(r); + if (!r->settings.fsmonitor) + r->settings.fsmonitor = alloc_settings(); r->settings.fsmonitor->mode = FSMONITOR_MODE_HOOK; + r->settings.fsmonitor->reason = reason; FREE_AND_NULL(r->settings.fsmonitor->hook_path); r->settings.fsmonitor->hook_path = strdup(path); } @@ -106,9 +161,81 @@ void fsm_settings__set_disabled(struct repository *r) { if (!r) r = the_repository; - - lookup_fsmonitor_settings(r); + if (!r->settings.fsmonitor) + r->settings.fsmonitor = alloc_settings(); r->settings.fsmonitor->mode = FSMONITOR_MODE_DISABLED; + r->settings.fsmonitor->reason = FSMONITOR_REASON_OK; FREE_AND_NULL(r->settings.fsmonitor->hook_path); } + +void fsm_settings__set_incompatible(struct repository *r, + enum fsmonitor_reason reason) +{ + if (!r) + r = the_repository; + if (!r->settings.fsmonitor) + r->settings.fsmonitor = alloc_settings(); + + r->settings.fsmonitor->mode = FSMONITOR_MODE_INCOMPATIBLE; + r->settings.fsmonitor->reason = reason; + FREE_AND_NULL(r->settings.fsmonitor->hook_path); +} + +enum fsmonitor_reason fsm_settings__get_reason(struct repository *r) +{ + if (!r) + r = the_repository; + if (!r->settings.fsmonitor) + lookup_fsmonitor_settings(r); + + return r->settings.fsmonitor->reason; +} + +char *fsm_settings__get_incompatible_msg(const struct repository *r, + enum fsmonitor_reason reason) +{ + struct strbuf msg = STRBUF_INIT; + + switch (reason) { + case FSMONITOR_REASON_UNTESTED: + case FSMONITOR_REASON_OK: + goto done; + + case FSMONITOR_REASON_BARE: + strbuf_addf(&msg, + _("bare repository '%s' is incompatible with fsmonitor"), + xgetcwd()); + goto done; + + case FSMONITOR_REASON_ERROR: + strbuf_addf(&msg, + _("repository '%s' is incompatible with fsmonitor due to errors"), + r->worktree); + goto done; + + case FSMONITOR_REASON_REMOTE: + strbuf_addf(&msg, + _("remote repository '%s' is incompatible with fsmonitor"), + r->worktree); + goto done; + + case FSMONITOR_REASON_VFS4GIT: + strbuf_addf(&msg, + _("virtual repository '%s' is incompatible with fsmonitor"), + r->worktree); + goto done; + + case FSMONITOR_REASON_NOSOCKETS: + strbuf_addf(&msg, + _("repository '%s' is incompatible with fsmonitor due to lack of Unix sockets"), + r->worktree); + goto done; + } + + BUG("Unhandled case in fsm_settings__get_incompatible_msg: '%d'", + reason); + +done: + return strbuf_detach(&msg, NULL); +} diff --git a/fsmonitor-settings.h b/fsmonitor-settings.h index a4c5d7b..d9c2605 100644 --- a/fsmonitor-settings.h +++ b/fsmonitor-settings.h @@ -4,18 +4,51 @@ struct repository; enum fsmonitor_mode { + FSMONITOR_MODE_INCOMPATIBLE = -1, /* see _reason */ FSMONITOR_MODE_DISABLED = 0, FSMONITOR_MODE_HOOK = 1, /* core.fsmonitor=<hook_path> */ FSMONITOR_MODE_IPC = 2, /* core.fsmonitor=<true> */ }; +/* + * Incompatibility reasons. + */ +enum fsmonitor_reason { + FSMONITOR_REASON_UNTESTED = 0, + FSMONITOR_REASON_OK, /* no incompatibility or when disabled */ + FSMONITOR_REASON_BARE, + FSMONITOR_REASON_ERROR, /* FS error probing for compatibility */ + FSMONITOR_REASON_REMOTE, + FSMONITOR_REASON_VFS4GIT, /* VFS for Git virtualization */ + FSMONITOR_REASON_NOSOCKETS, /* NTFS,FAT32 do not support Unix sockets */ +}; + void fsm_settings__set_ipc(struct repository *r); void fsm_settings__set_hook(struct repository *r, const char *path); void fsm_settings__set_disabled(struct repository *r); +void fsm_settings__set_incompatible(struct repository *r, + enum fsmonitor_reason reason); 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, + enum fsmonitor_reason reason); + struct fsmonitor_settings; +#ifdef HAVE_FSMONITOR_OS_SETTINGS +/* + * Ask platform-specific code whether the repository is incompatible + * with fsmonitor (both hook and ipc modes). For example, if the working + * directory is on a remote volume and mounted via a technology that does + * not support notification events, then we should not pretend to watch it. + * + * fsm_os__* routines should considered private to fsm_settings__ + * routines. + */ +enum fsmonitor_reason fsm_os__incompatible(struct repository *r); +#endif /* HAVE_FSMONITOR_OS_SETTINGS */ + #endif /* FSMONITOR_SETTINGS_H */ diff --git a/fsmonitor.c b/fsmonitor.c index 292a674..57d6a48 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -184,30 +184,68 @@ static int query_fsmonitor_hook(struct repository *r, static void fsmonitor_refresh_callback(struct index_state *istate, char *name) { int i, len = strlen(name); - if (name[len - 1] == '/') { + int pos = index_name_pos(istate, name, len); + + trace_printf_key(&trace_fsmonitor, + "fsmonitor_refresh_callback '%s' (pos %d)", + name, pos); + if (name[len - 1] == '/') { /* - * TODO We should binary search to find the first path with - * TODO this directory prefix. Then linearly update entries - * TODO while the prefix matches. Taking care to search without - * TODO the trailing slash -- because '/' sorts after a few - * TODO interesting special chars, like '.' and ' '. + * The daemon can decorate directory events, such as + * moves or renames, with a trailing slash if the OS + * FS Event contains sufficient information, such as + * MacOS. + * + * Use this to invalidate the entire cone under that + * directory. + * + * We do not expect an exact match because the index + * does not normally contain directory entries, so we + * start at the insertion point and scan. */ + if (pos < 0) + pos = -pos - 1; /* Mark all entries for the folder invalid */ - for (i = 0; i < istate->cache_nr; i++) { - if (istate->cache[i]->ce_flags & CE_FSMONITOR_VALID && - starts_with(istate->cache[i]->name, name)) - istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID; + for (i = pos; i < istate->cache_nr; i++) { + if (!starts_with(istate->cache[i]->name, name)) + break; + istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID; } - /* Need to remove the / from the path for the untracked cache */ + + /* + * We need to remove the traling "/" from the path + * for the untracked cache. + */ name[len - 1] = '\0'; + } else if (pos >= 0) { + /* + * We have an exact match for this path and can just + * invalidate it. + */ + istate->cache[pos]->ce_flags &= ~CE_FSMONITOR_VALID; } else { - int pos = index_name_pos(istate, name, strlen(name)); - - if (pos >= 0) { - struct cache_entry *ce = istate->cache[pos]; - ce->ce_flags &= ~CE_FSMONITOR_VALID; + /* + * The path is not a tracked file -or- it is a + * directory event on a platform that cannot + * distinguish between file and directory events in + * the event handler, such as Windows. + * + * Scan as if it is a directory and invalidate the + * cone under it. (But remember to ignore items + * between "name" and "name/", such as "name-" and + * "name.". + */ + pos = -pos - 1; + + for (i = pos; i < istate->cache_nr; i++) { + if (!starts_with(istate->cache[i]->name, name)) + break; + if ((unsigned char)istate->cache[i]->name[len] > '/') + break; + if (istate->cache[i]->name[len] == '/') + istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID; } } @@ -215,7 +253,6 @@ static void fsmonitor_refresh_callback(struct index_state *istate, char *name) * Mark the untracked cache dirty even if it wasn't found in the index * as it could be a new untracked file. */ - trace_printf_key(&trace_fsmonitor, "fsmonitor_refresh_callback '%s'", name); untracked_cache_invalidate_path(istate, name, 0); } @@ -543,6 +580,8 @@ void tweak_fsmonitor(struct index_state *istate) if (fsmonitor_enabled) { /* Mark all entries valid */ for (i = 0; i < istate->cache_nr; i++) { + if (S_ISGITLINK(istate->cache[i]->ce_mode)) + continue; istate->cache[i]->ce_flags |= CE_FSMONITOR_VALID; } diff --git a/fsmonitor.h b/fsmonitor.h index 3f41f65..edf7ce5 100644 --- a/fsmonitor.h +++ b/fsmonitor.h @@ -68,6 +68,15 @@ static inline int is_fsmonitor_refreshed(const struct index_state *istate) * Set the given cache entries CE_FSMONITOR_VALID bit. This should be * called any time the cache entry has been updated to reflect the * current state of the file on disk. + * + * However, never mark submodules as valid. When commands like "git + * status" run they might need to recurse into the submodule (using a + * child process) to get a summary of the submodule state. We don't + * have (and don't want to create) the facility to translate every + * FS event that we receive and that happens to be deep inside of a + * submodule back to the submodule root, so we cannot correctly keep + * track of this bit on the gitlink directory. Therefore, we never + * set it on submodules. */ static inline void mark_fsmonitor_valid(struct index_state *istate, struct cache_entry *ce) { @@ -75,6 +84,8 @@ static inline void mark_fsmonitor_valid(struct index_state *istate, struct cache if (fsm_mode > FSMONITOR_MODE_DISABLED && !(ce->ce_flags & CE_FSMONITOR_VALID)) { + if (S_ISGITLINK(ce->ce_mode)) + return; istate->cache_changed = 1; ce->ce_flags |= CE_FSMONITOR_VALID; trace_printf_key(&trace_fsmonitor, "mark_fsmonitor_clean '%s'", ce->name); @@ -538,7 +538,7 @@ static struct cmd_struct commands[] = { { "format-patch", cmd_format_patch, RUN_SETUP }, { "fsck", cmd_fsck, RUN_SETUP }, { "fsck-objects", cmd_fsck, RUN_SETUP }, - { "fsmonitor--daemon", cmd_fsmonitor__daemon, RUN_SETUP }, + { "fsmonitor--daemon", cmd_fsmonitor__daemon, SUPPORT_SUPER_PREFIX | RUN_SETUP }, { "gc", cmd_gc, RUN_SETUP }, { "get-tar-commit-id", cmd_get_tar_commit_id, NO_PARSEOPT }, { "grep", cmd_grep, RUN_SETUP_GENTLY }, diff --git a/t/helper/test-fsmonitor-client.c b/t/helper/test-fsmonitor-client.c index 3062c8a..54a4856 100644 --- a/t/helper/test-fsmonitor-client.c +++ b/t/helper/test-fsmonitor-client.c @@ -7,6 +7,8 @@ #include "cache.h" #include "parse-options.h" #include "fsmonitor-ipc.h" +#include "thread-utils.h" +#include "trace2.h" #ifndef HAVE_FSMONITOR_DAEMON_BACKEND int cmd__fsmonitor_client(int argc, const char **argv) @@ -79,20 +81,121 @@ static int do_send_flush(void) return 0; } +struct hammer_thread_data +{ + pthread_t pthread_id; + int thread_nr; + + int nr_requests; + const char *token; + + int sum_successful; + int sum_errors; +}; + +static void *hammer_thread_proc(void *_hammer_thread_data) +{ + struct hammer_thread_data *data = _hammer_thread_data; + struct strbuf answer = STRBUF_INIT; + int k; + int ret; + + trace2_thread_start("hammer"); + + for (k = 0; k < data->nr_requests; k++) { + strbuf_reset(&answer); + + ret = fsmonitor_ipc__send_query(data->token, &answer); + if (ret < 0) + data->sum_errors++; + else + data->sum_successful++; + } + + strbuf_release(&answer); + trace2_thread_exit(); + return NULL; +} + +/* + * Start a pool of client threads that will each send a series of + * commands to the daemon. + * + * The goal is to overload the daemon with a sustained series of + * concurrent requests. + */ +static int do_hammer(const char *token, int nr_threads, int nr_requests) +{ + struct hammer_thread_data *data = NULL; + int k; + int sum_join_errors = 0; + int sum_commands = 0; + int sum_errors = 0; + + if (!token || !*token) + token = get_token_from_index(); + if (nr_threads < 1) + nr_threads = 1; + if (nr_requests < 1) + nr_requests = 1; + + CALLOC_ARRAY(data, nr_threads); + + for (k = 0; k < nr_threads; k++) { + struct hammer_thread_data *p = &data[k]; + p->thread_nr = k; + p->nr_requests = nr_requests; + p->token = token; + + if (pthread_create(&p->pthread_id, NULL, hammer_thread_proc, p)) { + warning("failed to create thread[%d] skipping remainder", k); + nr_threads = k; + break; + } + } + + for (k = 0; k < nr_threads; k++) { + struct hammer_thread_data *p = &data[k]; + + if (pthread_join(p->pthread_id, NULL)) + sum_join_errors++; + sum_commands += p->sum_successful; + sum_errors += p->sum_errors; + } + + fprintf(stderr, "HAMMER: [threads %d][requests %d] [ok %d][err %d][join %d]\n", + nr_threads, nr_requests, sum_commands, sum_errors, sum_join_errors); + + free(data); + + /* + * Return an error if any of the _send_query requests failed. + * We don't care about thread create/join errors. + */ + return sum_errors > 0; +} + int cmd__fsmonitor_client(int argc, const char **argv) { const char *subcmd; const char *token = NULL; + int nr_threads = 1; + int nr_requests = 1; const char * const fsmonitor_client_usage[] = { "test-tool fsmonitor-client query [<token>]", "test-tool fsmonitor-client flush", + "test-tool fsmonitor-client hammer [<token>] [<threads>] [<requests>]", NULL, }; struct option options[] = { OPT_STRING(0, "token", &token, "token", "command token to send to the server"), + + OPT_INTEGER(0, "threads", &nr_threads, "number of client threads"), + OPT_INTEGER(0, "requests", &nr_requests, "number of requests per thread"), + OPT_END() }; @@ -111,6 +214,9 @@ int cmd__fsmonitor_client(int argc, const char **argv) if (!strcmp(subcmd, "flush")) return !!do_send_flush(); + if (!strcmp(subcmd, "hammer")) + return !!do_hammer(token, nr_threads, nr_requests); + die("Unhandled subcommand: '%s'", subcmd); } #endif diff --git a/t/helper/test-hexdump.c b/t/helper/test-hexdump.c new file mode 100644 index 0000000..811e89c --- /dev/null +++ b/t/helper/test-hexdump.c @@ -0,0 +1,30 @@ +#include "test-tool.h" +#include "git-compat-util.h" + +/* + * Read stdin and print a hexdump to stdout. + */ +int cmd__hexdump(int argc, const char **argv) +{ + char buf[1024]; + ssize_t i, len; + int have_data = 0; + + for (;;) { + len = xread(0, buf, sizeof(buf)); + if (len < 0) + die_errno("failure reading stdin"); + if (!len) + break; + + have_data = 1; + + for (i = 0; i < len; i++) + printf("%02x ", (unsigned char)buf[i]); + } + + if (have_data) + putchar('\n'); + + return 0; +} diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c index d2eacd3..318fdba 100644 --- a/t/helper/test-tool.c +++ b/t/helper/test-tool.c @@ -38,6 +38,7 @@ static struct test_cmd cmds[] = { { "getcwd", cmd__getcwd }, { "hashmap", cmd__hashmap }, { "hash-speed", cmd__hash_speed }, + { "hexdump", cmd__hexdump }, { "index-version", cmd__index_version }, { "json-writer", cmd__json_writer }, { "lazy-init-name-hash", cmd__lazy_init_name_hash }, diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h index 960cc27..bb79927 100644 --- a/t/helper/test-tool.h +++ b/t/helper/test-tool.h @@ -29,6 +29,7 @@ int cmd__genzeros(int argc, const char **argv); int cmd__getcwd(int argc, const char **argv); int cmd__hashmap(int argc, const char **argv); int cmd__hash_speed(int argc, const char **argv); +int cmd__hexdump(int argc, const char **argv); int cmd__index_version(int argc, const char **argv); int cmd__json_writer(int argc, const char **argv); int cmd__lazy_init_name_hash(int argc, const char **argv); diff --git a/t/lib-unicode-nfc-nfd.sh b/t/lib-unicode-nfc-nfd.sh new file mode 100755 index 0000000..2223224 --- /dev/null +++ b/t/lib-unicode-nfc-nfd.sh @@ -0,0 +1,162 @@ +# Help detect how Unicode NFC and NFD are handled on the filesystem. + +# A simple character that has a NFD form. +# +# NFC: U+00e9 LATIN SMALL LETTER E WITH ACUTE +# UTF8(NFC): \xc3 \xa9 +# +# NFD: U+0065 LATIN SMALL LETTER E +# U+0301 COMBINING ACUTE ACCENT +# UTF8(NFD): \x65 + \xcc \x81 +# +utf8_nfc=$(printf "\xc3\xa9") +utf8_nfd=$(printf "\x65\xcc\x81") + +# Is the OS or the filesystem "Unicode composition sensitive"? +# +# That is, does the OS or the filesystem allow files to exist with +# both the NFC and NFD spellings? Or, does the OS/FS lie to us and +# tell us that the NFC and NFD forms are equivalent. +# +# This is or may be independent of what type of filesystem we have, +# since it might be handled by the OS at a layer above the FS. +# Testing shows on MacOS using APFS, HFS+, and FAT32 reports a +# collision, for example. +# +# This does not tell us how the Unicode pathname will be spelled +# on disk, but rather only that the two spelling "collide". We +# will examine the actual on disk spelling in a later prereq. +# +test_lazy_prereq UNICODE_COMPOSITION_SENSITIVE ' + mkdir trial_${utf8_nfc} && + mkdir trial_${utf8_nfd} +' + +# Is the spelling of an NFC pathname preserved on disk? +# +# On MacOS with HFS+ and FAT32, NFC paths are converted into NFD +# and on APFS, NFC paths are preserved. As we have established +# above, this is independent of "composition sensitivity". +# +test_lazy_prereq UNICODE_NFC_PRESERVED ' + mkdir c_${utf8_nfc} && + ls | test-tool hexdump >dump && + grep "63 5f c3 a9" dump +' + +# Is the spelling of an NFD pathname preserved on disk? +# +test_lazy_prereq UNICODE_NFD_PRESERVED ' + mkdir d_${utf8_nfd} && + ls | test-tool hexdump >dump && + grep "64 5f 65 cc 81" dump +' + +# The following _DOUBLE_ forms are more for my curiosity, +# but there may be quirks lurking when there are multiple +# combining characters in non-canonical order. + +# Unicode also allows multiple combining characters +# that can be decomposed in pieces. +# +# NFC: U+1f67 GREEK SMALL LETTER OMEGA WITH DASIA AND PERISPOMENI +# UTF8(NFC): \xe1 \xbd \xa7 +# +# NFD1: U+1f61 GREEK SMALL LETTER OMEGA WITH DASIA +# U+0342 COMBINING GREEK PERISPOMENI +# UTF8(NFD1): \xe1 \xbd \xa1 + \xcd \x82 +# +# But U+1f61 decomposes into +# NFD2: U+03c9 GREEK SMALL LETTER OMEGA +# U+0314 COMBINING REVERSED COMMA ABOVE +# UTF8(NFD2): \xcf \x89 + \xcc \x94 +# +# Yielding: \xcf \x89 + \xcc \x94 + \xcd \x82 +# +# Note that I've used the canonical ordering of the +# combinining characters. It is also possible to +# swap them. My testing shows that that non-standard +# ordering also causes a collision in mkdir. However, +# the resulting names don't draw correctly on the +# terminal (implying that the on-disk format also has +# them out of order). +# +greek_nfc=$(printf "\xe1\xbd\xa7") +greek_nfd1=$(printf "\xe1\xbd\xa1\xcd\x82") +greek_nfd2=$(printf "\xcf\x89\xcc\x94\xcd\x82") + +# See if a double decomposition also collides. +# +test_lazy_prereq UNICODE_DOUBLE_COMPOSITION_SENSITIVE ' + mkdir trial_${greek_nfc} && + mkdir trial_${greek_nfd2} +' + +# See if the NFC spelling appears on the disk. +# +test_lazy_prereq UNICODE_DOUBLE_NFC_PRESERVED ' + mkdir c_${greek_nfc} && + ls | test-tool hexdump >dump && + grep "63 5f e1 bd a7" dump +' + +# See if the NFD spelling appears on the disk. +# +test_lazy_prereq UNICODE_DOUBLE_NFD_PRESERVED ' + mkdir d_${greek_nfd2} && + ls | test-tool hexdump >dump && + grep "64 5f cf 89 cc 94 cd 82" dump +' + +# The following is for debugging. I found it useful when +# trying to understand the various (OS, FS) quirks WRT +# Unicode and how composition/decomposition is handled. +# For example, when trying to understand how (macOS, APFS) +# and (macOS, HFS) and (macOS, FAT32) compare. +# +# It is rather noisy, so it is disabled by default. +# +if test "$unicode_debug" = "true" +then + if test_have_prereq UNICODE_COMPOSITION_SENSITIVE + then + echo NFC and NFD are distinct on this OS/filesystem. + else + echo NFC and NFD are aliases on this OS/filesystem. + fi + + if test_have_prereq UNICODE_NFC_PRESERVED + then + echo NFC maintains original spelling. + else + echo NFC is modified. + fi + + if test_have_prereq UNICODE_NFD_PRESERVED + then + echo NFD maintains original spelling. + else + echo NFD is modified. + fi + + if test_have_prereq UNICODE_DOUBLE_COMPOSITION_SENSITIVE + then + echo DOUBLE NFC and NFD are distinct on this OS/filesystem. + else + echo DOUBLE NFC and NFD are aliases on this OS/filesystem. + fi + + if test_have_prereq UNICODE_DOUBLE_NFC_PRESERVED + then + echo Double NFC maintains original spelling. + else + echo Double NFC is modified. + fi + + if test_have_prereq UNICODE_DOUBLE_NFD_PRESERVED + then + echo Double NFD maintains original spelling. + else + echo Double NFD is modified. + fi +fi diff --git a/t/perf/p7527-builtin-fsmonitor.sh b/t/perf/p7527-builtin-fsmonitor.sh new file mode 100755 index 0000000..9338b9e --- /dev/null +++ b/t/perf/p7527-builtin-fsmonitor.sh @@ -0,0 +1,257 @@ +#!/bin/sh + +test_description="Perf test for the builtin FSMonitor" + +. ./perf-lib.sh + +if ! test_have_prereq FSMONITOR_DAEMON +then + skip_all="fsmonitor--daemon is not supported on this platform" + test_done +fi + +test_lazy_prereq UNTRACKED_CACHE ' + { git update-index --test-untracked-cache; ret=$?; } && + test $ret -ne 1 +' + +# Lie to perf-lib and ask for a new empty repo and avoid +# the complaints about GIT_PERF_REPO not being big enough +# the perf hit when GIT_PERF_LARGE_REPO is copied into +# the trash directory. +# +# NEEDSWORK: It would be nice if perf-lib had an option to +# "borrow" an existing large repo (especially for gigantic +# monorepos) and use it in-place. For now, fake it here. +# +test_perf_fresh_repo + + +# Use a generated synthetic monorepo. If it doesn't exist, we will +# generate it. If it does exist, we will put it in a known state +# before we start our timings. +# +PARAM_D=5 +PARAM_W=10 +PARAM_F=9 + +PARAMS="$PARAM_D"."$PARAM_W"."$PARAM_F" + +BALLAST_BR=p0006-ballast +export BALLAST_BR + +TMP_BR=tmp_br +export TMP_BR + +REPO=../repos/gen-many-files-"$PARAMS".git +export REPO + +if ! test -d $REPO +then + (cd ../repos; ./many-files.sh -d $PARAM_D -w $PARAM_W -f $PARAM_F) +fi + + +enable_uc () { + git -C $REPO config core.untrackedcache true + git -C $REPO update-index --untracked-cache + git -C $REPO status >/dev/null 2>&1 +} + +disable_uc () { + git -C $REPO config core.untrackedcache false + git -C $REPO update-index --no-untracked-cache + git -C $REPO status >/dev/null 2>&1 +} + +start_fsm () { + git -C $REPO fsmonitor--daemon start + git -C $REPO fsmonitor--daemon status + git -C $REPO config core.fsmonitor true + git -C $REPO update-index --fsmonitor + git -C $REPO status >/dev/null 2>&1 +} + +stop_fsm () { + git -C $REPO config --unset core.fsmonitor + git -C $REPO update-index --no-fsmonitor + test_might_fail git -C $REPO fsmonitor--daemon stop 2>/dev/null + git -C $REPO status >/dev/null 2>&1 +} + + +# Ensure that FSMonitor is turned off on the borrowed repo. +# +test_expect_success "Setup borrowed repo (fsm+uc)" " + stop_fsm && + disable_uc +" + +# Also ensure that it starts in a known state. +# +# Because we assume that $GIT_PERF_REPEAT_COUNT > 1, we are not going to time +# the ballast checkout, since only the first invocation does any work and the +# subsequent ones just print "already on branch" and quit, so the reported +# time is not useful. +# +# Create a temp branch and do all work relative to it so that we don't +# accidentially alter the real ballast branch. +# +test_expect_success "Setup borrowed repo (temp ballast branch)" " + test_might_fail git -C $REPO checkout $BALLAST_BR && + test_might_fail git -C $REPO reset --hard && + git -C $REPO clean -d -f && + test_might_fail git -C $REPO branch -D $TMP_BR && + git -C $REPO branch $TMP_BR $BALLAST_BR && + git -C $REPO checkout $TMP_BR +" + + +echo Data >data.txt + +# NEEDSWORK: We assume that $GIT_PERF_REPEAT_COUNT > 1. With +# FSMonitor enabled, we can get a skewed view of status times, since +# the index MAY (or may not) be updated after the first invocation +# which will update the FSMonitor Token, so the subsequent invocations +# may get a smaller response from the daemon. +# +do_status () { + msg=$1 + + test_perf "$msg" " + git -C $REPO status >/dev/null 2>&1 + " +} + +do_matrix () { + uc=$1 + fsm=$2 + + t="[uc $uc][fsm $fsm]" + MATRIX_BR="$TMP_BR-$uc-$fsm" + + test_expect_success "$t Setup matrix branch" " + git -C $REPO clean -d -f && + git -C $REPO checkout $TMP_BR && + test_might_fail git -C $REPO branch -D $MATRIX_BR && + git -C $REPO branch $MATRIX_BR $TMP_BR && + git -C $REPO checkout $MATRIX_BR + " + + if test $uc = true + then + enable_uc + else + disable_uc + fi + + if test $fsm = true + then + start_fsm + else + stop_fsm + fi + + do_status "$t status after checkout" + + # Modify many files in the matrix branch. + # Stage them. + # Commit them. + # Rollback. + # + test_expect_success "$t modify tracked files" " + find $REPO -name file1 -exec cp data.txt {} \\; + " + + do_status "$t status after big change" + + # Don't bother timing the "add" because _REPEAT_COUNT + # issue described above. + # + test_expect_success "$t add all" " + git -C $REPO add -A + " + + do_status "$t status after add all" + + test_expect_success "$t add dot" " + git -C $REPO add . + " + + do_status "$t status after add dot" + + test_expect_success "$t commit staged" " + git -C $REPO commit -a -m data + " + + do_status "$t status after commit" + + test_expect_success "$t reset HEAD~1 hard" " + git -C $REPO reset --hard HEAD~1 >/dev/null 2>&1 + " + + do_status "$t status after reset hard" + + # Create some untracked files. + # + test_expect_success "$t create untracked files" " + cp -R $REPO/ballast/dir1 $REPO/ballast/xxx1 + " + + do_status "$t status after create untracked files" + + # Remove the new untracked files. + # + test_expect_success "$t clean -df" " + git -C $REPO clean -d -f + " + + do_status "$t status after clean" + + if test $fsm = true + then + stop_fsm + fi +} + +# Begin testing each case in the matrix that we care about. +# +uc_values="false" +test_have_prereq UNTRACKED_CACHE && uc_values="false true" + +fsm_values="false true" + +for uc_val in $uc_values +do + for fsm_val in $fsm_values + do + do_matrix $uc_val $fsm_val + done +done + +cleanup () { + uc=$1 + fsm=$2 + + MATRIX_BR="$TMP_BR-$uc-$fsm" + + test_might_fail git -C $REPO branch -D $MATRIX_BR +} + + +# We're borrowing this repo. We should leave it in a clean state. +# +test_expect_success "Cleanup temp and matrix branches" " + git -C $REPO clean -d -f && + test_might_fail git -C $REPO checkout $BALLAST_BR && + test_might_fail git -C $REPO branch -D $TMP_BR && + for uc_val in $uc_values + do + for fsm_val in $fsm_values + do + cleanup $uc_val $fsm_val + done + done +" + +test_done diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh index d4f9c6a..8348e3a 100755 --- a/t/t7519-status-fsmonitor.sh +++ b/t/t7519-status-fsmonitor.sh @@ -55,6 +55,38 @@ test_lazy_prereq UNTRACKED_CACHE ' test $ret -ne 1 ' +# Test that we detect and disallow repos that are incompatible with FSMonitor. +test_expect_success 'incompatible bare repo' ' + test_when_finished "rm -rf ./bare-clone actual expect" && + git init --bare bare-clone && + + test_must_fail \ + git -C ./bare-clone -c core.fsmonitor=foo \ + update-index --fsmonitor 2>actual && + grep "bare repository .* is incompatible with fsmonitor" actual && + + test_must_fail \ + git -C ./bare-clone -c core.fsmonitor=true \ + update-index --fsmonitor 2>actual && + grep "bare repository .* is incompatible with fsmonitor" actual +' + +test_expect_success FSMONITOR_DAEMON 'run fsmonitor-daemon in bare repo' ' + test_when_finished "rm -rf ./bare-clone actual" && + git init --bare bare-clone && + test_must_fail git -C ./bare-clone fsmonitor--daemon run 2>actual && + grep "bare repository .* is incompatible with fsmonitor" actual +' + +test_expect_success MINGW,FSMONITOR_DAEMON 'run fsmonitor-daemon in virtual repo' ' + test_when_finished "rm -rf ./fake-virtual-clone actual" && + git init fake-virtual-clone && + test_must_fail git -C ./fake-virtual-clone \ + -c core.virtualfilesystem=true \ + fsmonitor--daemon run 2>actual && + grep "virtual repository .* is incompatible with fsmonitor" actual +' + test_expect_success 'setup' ' : >tracked && : >modified && diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh index bd0c952..56c0dff 100755 --- a/t/t7527-builtin-fsmonitor.sh +++ b/t/t7527-builtin-fsmonitor.sh @@ -124,6 +124,36 @@ test_expect_success 'implicit daemon start' ' test_must_fail git -C test_implicit fsmonitor--daemon status ' +# Verify that the daemon has shutdown. Spin a few seconds to +# make the test a little more robust during CI testing. +# +# We're looking for an implicit shutdown, such as when we delete or +# rename the ".git" directory. Our delete/rename will cause a file +# system event that the daemon will see and the daemon will +# auto-shutdown as soon as it sees it. But this is racy with our `git +# fsmonitor--daemon status` commands (and we cannot use a cookie file +# here to help us). So spin a little and give the daemon a chance to +# see the event. (This is primarily for underpowered CI build/test +# machines (where it might take a moment to wake and reschedule the +# daemon process) to avoid false alarms during test runs.) +# +IMPLICIT_TIMEOUT=5 + +verify_implicit_shutdown () { + r=$1 && + + k=0 && + while test "$k" -lt $IMPLICIT_TIMEOUT + do + git -C $r fsmonitor--daemon status || return 0 + + sleep 1 + k=$(( $k + 1 )) + done && + + return 1 +} + test_expect_success 'implicit daemon stop (delete .git)' ' test_when_finished "stop_daemon_delete_repo test_implicit_1" && @@ -142,10 +172,9 @@ test_expect_success 'implicit daemon stop (delete .git)' ' # This would make the test result dependent upon whether we # were using fsmonitor on our development worktree. # - sleep 1 && mkdir test_implicit_1/.git && - test_must_fail git -C test_implicit_1 fsmonitor--daemon status + verify_implicit_shutdown test_implicit_1 ' test_expect_success 'implicit daemon stop (rename .git)' ' @@ -160,10 +189,70 @@ test_expect_success 'implicit daemon stop (rename .git)' ' # See [1] above. # - sleep 1 && mkdir test_implicit_2/.git && - test_must_fail git -C test_implicit_2 fsmonitor--daemon status + verify_implicit_shutdown test_implicit_2 +' + +# File systems on Windows may or may not have shortnames. +# This is a volume-specific setting on modern systems. +# "C:/" drives are required to have them enabled. Other +# hard drives default to disabled. +# +# This is a crude test to see if shortnames are enabled +# on the volume containing the test directory. It is +# crude, but it does not require elevation like `fsutil`. +# +test_lazy_prereq SHORTNAMES ' + mkdir .foo && + test -d "FOO~1" +' + +# Here we assume that the shortname of ".git" is "GIT~1". +test_expect_success MINGW,SHORTNAMES 'implicit daemon stop (rename GIT~1)' ' + test_when_finished "stop_daemon_delete_repo test_implicit_1s" && + + git init test_implicit_1s && + + start_daemon -C test_implicit_1s && + + # renaming the .git directory will implicitly stop the daemon. + # this moves {.git, GIT~1} to {.gitxyz, GITXYZ~1}. + # the rename-from FS Event will contain the shortname. + # + mv test_implicit_1s/GIT~1 test_implicit_1s/.gitxyz && + + # See [1] above. + # this moves {.gitxyz, GITXYZ~1} to {.git, GIT~1}. + mv test_implicit_1s/.gitxyz test_implicit_1s/.git && + + verify_implicit_shutdown test_implicit_1s +' + +# Here we first create a file with LONGNAME of "GIT~1" before +# we create the repo. This will cause the shortname of ".git" +# to be "GIT~2". +test_expect_success MINGW,SHORTNAMES 'implicit daemon stop (rename GIT~2)' ' + test_when_finished "stop_daemon_delete_repo test_implicit_1s2" && + + mkdir test_implicit_1s2 && + echo HELLO >test_implicit_1s2/GIT~1 && + git init test_implicit_1s2 && + + test_path_is_file test_implicit_1s2/GIT~1 && + test_path_is_dir test_implicit_1s2/GIT~2 && + + start_daemon -C test_implicit_1s2 && + + # renaming the .git directory will implicitly stop the daemon. + # the rename-from FS Event will contain the shortname. + # + mv test_implicit_1s2/GIT~2 test_implicit_1s2/.gitxyz && + + # See [1] above. + mv test_implicit_1s2/.gitxyz test_implicit_1s2/.git && + + verify_implicit_shutdown test_implicit_1s2 ' test_expect_success 'cannot start multiple daemons' ' @@ -209,6 +298,16 @@ test_expect_success 'setup' ' trace* EOF + mkdir -p T1/T2/T3/T4 && + echo 1 >T1/F1 && + echo 1 >T1/T2/F1 && + echo 1 >T1/T2/T3/F1 && + echo 1 >T1/T2/T3/T4/F1 && + echo 2 >T1/F2 && + echo 2 >T1/T2/F2 && + echo 2 >T1/T2/T3/F2 && + echo 2 >T1/T2/T3/T4/F2 && + git -c core.fsmonitor=false add . && test_tick && git -c core.fsmonitor=false commit -m initial && @@ -291,6 +390,19 @@ directory_to_file () { echo 1 >dir1 } +move_directory_contents_deeper() { + mkdir T1/_new_ && + mv T1/[A-Z]* T1/_new_ +} + +move_directory_up() { + mv T1/T2/T3 T1 +} + +move_directory() { + mv T1/T2/T3 T1/T2/NewT3 +} + # The next few test cases confirm that our fsmonitor daemon sees each type # of OS filesystem notification that we care about. At this layer we just # ensure we are getting the OS notifications and do not try to confirm what @@ -595,6 +707,10 @@ do matrix_try $uc_val $fsm_val file_to_directory matrix_try $uc_val $fsm_val directory_to_file + matrix_try $uc_val $fsm_val move_directory_contents_deeper + matrix_try $uc_val $fsm_val move_directory_up + matrix_try $uc_val $fsm_val move_directory + if test $fsm_val = true then test_expect_success "Matrix[uc:$uc_val][fsm:$fsm_val] disable fsmonitor at end" ' @@ -606,4 +722,281 @@ do done done +# Test Unicode UTF-8 characters in the pathname of the working +# directory root. Use of "*A()" routines rather than "*W()" routines +# on Windows can sometimes lead to odd failures. +# +u1=$(printf "u_c3_a6__\xC3\xA6") +u2=$(printf "u_e2_99_ab__\xE2\x99\xAB") +u_values="$u1 $u2" +for u in $u_values +do + test_expect_success "unicode in repo root path: $u" ' + test_when_finished "stop_daemon_delete_repo $u" && + + git init "$u" && + echo 1 >"$u"/file1 && + git -C "$u" add file1 && + git -C "$u" config core.fsmonitor true && + + start_daemon -C "$u" && + git -C "$u" status >actual && + grep "new file: file1" actual + ' +done + +# Test fsmonitor interaction with submodules. +# +# If we start the daemon in the super, it will see FS events for +# everything in the working directory cone and this includes any +# files/directories contained *within* the submodules. +# +# A `git status` at top level will get events for items within the +# submodule and ignore them, since they aren't named in the index +# of the super repo. This makes the fsmonitor response a little +# noisy, but it doesn't alter the correctness of the state of the +# super-proper. +# +# When we have submodules, `git status` normally does a recursive +# status on each of the submodules and adds a summary row for any +# dirty submodules. (See the "S..." bits in porcelain V2 output.) +# +# It is therefore important that the top level status not be tricked +# by the FSMonitor response to skip those recursive calls. That is, +# even if FSMonitor says that the mtime of the submodule directory +# hasn't changed and it could be implicitly marked valid, we must +# not take that shortcut. We need to force the recusion into the +# submodule so that we get a summary of the status *within* the +# submodule. + +create_super () { + super="$1" && + + git init "$super" && + echo x >"$super/file_1" && + echo y >"$super/file_2" && + echo z >"$super/file_3" && + mkdir "$super/dir_1" && + echo a >"$super/dir_1/file_11" && + echo b >"$super/dir_1/file_12" && + mkdir "$super/dir_1/dir_2" && + echo a >"$super/dir_1/dir_2/file_21" && + echo b >"$super/dir_1/dir_2/file_22" && + git -C "$super" add . && + git -C "$super" commit -m "initial $super commit" +} + +create_sub () { + sub="$1" && + + git init "$sub" && + echo x >"$sub/file_x" && + echo y >"$sub/file_y" && + echo z >"$sub/file_z" && + mkdir "$sub/dir_x" && + echo a >"$sub/dir_x/file_a" && + echo b >"$sub/dir_x/file_b" && + mkdir "$sub/dir_x/dir_y" && + echo a >"$sub/dir_x/dir_y/file_a" && + echo b >"$sub/dir_x/dir_y/file_b" && + git -C "$sub" add . && + git -C "$sub" commit -m "initial $sub commit" +} + +my_match_and_clean () { + git -C super --no-optional-locks status --porcelain=v2 >actual.with && + git -C super --no-optional-locks -c core.fsmonitor=false \ + status --porcelain=v2 >actual.without && + test_cmp actual.with actual.without && + + git -C super/dir_1/dir_2/sub reset --hard && + git -C super/dir_1/dir_2/sub clean -d -f +} + +test_expect_success 'submodule always visited' ' + test_when_finished "git -C super fsmonitor--daemon stop; \ + rm -rf super; \ + rm -rf sub" && + + create_super super && + create_sub sub && + + git -C super submodule add ../sub ./dir_1/dir_2/sub && + git -C super commit -m "add sub" && + + start_daemon -C super && + git -C super config core.fsmonitor true && + git -C super update-index --fsmonitor && + git -C super status && + + # Now run pairs of commands w/ and w/o FSMonitor while we make + # some dirt in the submodule and confirm matching output. + + # Completely clean status. + my_match_and_clean && + + # .M S..U + echo z >super/dir_1/dir_2/sub/dir_x/dir_y/foobar_u && + my_match_and_clean && + + # .M S.M. + echo z >super/dir_1/dir_2/sub/dir_x/dir_y/foobar_m && + git -C super/dir_1/dir_2/sub add . && + my_match_and_clean && + + # .M S.M. + echo z >>super/dir_1/dir_2/sub/dir_x/dir_y/file_a && + git -C super/dir_1/dir_2/sub add . && + my_match_and_clean && + + # .M SC.. + echo z >>super/dir_1/dir_2/sub/dir_x/dir_y/file_a && + git -C super/dir_1/dir_2/sub add . && + git -C super/dir_1/dir_2/sub commit -m "SC.." && + my_match_and_clean +' + +# If a submodule has a `sub/.git/` directory (rather than a file +# pointing to the super's `.git/modules/sub`) and `core.fsmonitor` +# turned on in the submodule and the daemon is not yet started in +# the submodule, and someone does a `git submodule absorbgitdirs` +# in the super, Git will recursively invoke `git submodule--helper` +# to do the work and this may try to read the index. This will +# try to start the daemon in the submodule *and* pass (either +# directly or via inheritance) the `--super-prefix` arg to the +# `git fsmonitor--daemon start` command inside the submodule. +# This causes a warning because fsmonitor--daemon does take that +# global arg (see the table in git.c) +# +# This causes a warning when trying to start the daemon that is +# somewhat confusing. It does not seem to hurt anything because +# the fsmonitor code maps the query failure into a trivial response +# and does the work anyway. +# +# It would be nice to silence the warning, however. + +have_t2_error_event () { + log=$1 + msg="fsmonitor--daemon doesnQt support --super-prefix" && + + tr '\047' Q <$1 | grep -e "$msg" +} + +test_expect_success "stray submodule super-prefix warning" ' + test_when_finished "rm -rf super; \ + rm -rf sub; \ + rm super-sub.trace" && + + create_super super && + create_sub sub && + + # Copy rather than submodule add so that we get a .git dir. + cp -R ./sub ./super/dir_1/dir_2/sub && + + git -C super/dir_1/dir_2/sub config core.fsmonitor true && + + git -C super submodule add ../sub ./dir_1/dir_2/sub && + git -C super commit -m "add sub" && + + test_path_is_dir super/dir_1/dir_2/sub/.git && + + GIT_TRACE2_EVENT="$PWD/super-sub.trace" \ + git -C super submodule absorbgitdirs && + + ! have_t2_error_event super-sub.trace +' + +# On a case-insensitive file system, confirm that the daemon +# notices when the .git directory is moved/renamed/deleted +# regardless of how it is spelled in the the FS event. +# That is, does the FS event receive the spelling of the +# operation or does it receive the spelling preserved with +# the file/directory. +# +test_expect_success CASE_INSENSITIVE_FS 'case insensitive+preserving' ' +# test_when_finished "stop_daemon_delete_repo test_insensitive" && + + git init test_insensitive && + + start_daemon -C test_insensitive --tf "$PWD/insensitive.trace" && + + mkdir -p test_insensitive/abc/def && + echo xyz >test_insensitive/ABC/DEF/xyz && + + test_path_is_dir test_insensitive/.git && + test_path_is_dir test_insensitive/.GIT && + + # Rename .git using an alternate spelling to verify that that + # daemon detects it and automatically shuts down. + mv test_insensitive/.GIT test_insensitive/.FOO && + + # See [1] above. + mv test_insensitive/.FOO test_insensitive/.git && + + verify_implicit_shutdown test_insensitive && + + # Verify that events were reported using on-disk spellings of the + # directories and files that we touched. We may or may not get a + # trailing slash on modified directories. + # + egrep "^event: abc/?$" ./insensitive.trace && + egrep "^event: abc/def/?$" ./insensitive.trace && + egrep "^event: abc/def/xyz$" ./insensitive.trace +' + +# The variable "unicode_debug" is defined in the following library +# script to dump information about how the (OS, FS) handles Unicode +# composition. Uncomment the following line if you want to enable it. +# +# unicode_debug=true + +. "$TEST_DIRECTORY/lib-unicode-nfc-nfd.sh" + +# See if the OS or filesystem does NFC/NFD aliasing/munging. +# +# The daemon should err on the side of caution and send BOTH the +# NFC and NFD forms. It does not know the original spelling of +# the pathname (how the user thinks it should be spelled), so +# emit both and let the client decide (when necessary). This is +# similar to "core.precomposeUnicode". +# +test_expect_success !UNICODE_COMPOSITION_SENSITIVE 'Unicode nfc/nfd' ' + test_when_finished "stop_daemon_delete_repo test_unicode" && + + git init test_unicode && + + start_daemon -C test_unicode --tf "$PWD/unicode.trace" && + + # Create a directory using an NFC spelling. + # + mkdir test_unicode/nfc && + mkdir test_unicode/nfc/c_${utf8_nfc} && + + # Create a directory using an NFD spelling. + # + mkdir test_unicode/nfd && + mkdir test_unicode/nfd/d_${utf8_nfd} && + + git -C test_unicode fsmonitor--daemon stop && + + if test_have_prereq UNICODE_NFC_PRESERVED + then + # We should have seen NFC event from OS. + # We should not have synthesized an NFD event. + egrep "^event: nfc/c_${utf8_nfc}/?$" ./unicode.trace && + egrep -v "^event: nfc/c_${utf8_nfd}/?$" ./unicode.trace + else + # We should have seen NFD event from OS. + # We should have synthesized an NFC event. + egrep "^event: nfc/c_${utf8_nfd}/?$" ./unicode.trace && + egrep "^event: nfc/c_${utf8_nfc}/?$" ./unicode.trace + fi && + + # We assume UNICODE_NFD_PRESERVED. + # We should have seen explicit NFD from OS. + # We should have synthesized an NFC event. + egrep "^event: nfd/d_${utf8_nfd}/?$" ./unicode.trace && + egrep "^event: nfd/d_${utf8_nfc}/?$" ./unicode.trace +' + test_done diff --git a/unpack-trees.c b/unpack-trees.c index 7d73f62..d561ca0 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1840,6 +1840,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options o->result.fsmonitor_last_update = xstrdup_or_null(o->src_index->fsmonitor_last_update); + o->result.fsmonitor_has_run_once = o->src_index->fsmonitor_has_run_once; if (!o->src_index->initialized && !repo->settings.command_requires_full_index && |