Revert "Merge branch 'pt/fsmonitor-linux' into next"

This reverts commit 37fa478,
reversing changes made to 289fcba,
as the tests in the topic was pointed out to be seriously broken.

cf. <ad6hovxCkwMTG11U@szeder.dev>

Author: gitster

Documentation/config/fsmonitor--daemon.adoc

@@ -4,8 +4,8 @@ fsmonitor.allowRemote::
     behavior.  Only respected when `core.fsmonitor` is set to `true`.
 
 fsmonitor.socketDir::
-    This Mac OS and Linux-specific option, if set, specifies the directory in
+    This Mac OS-specific option, if set, specifies the directory in
     which to create the Unix domain socket used for communication
     between the fsmonitor daemon and various Git commands. The directory must
-    reside on a native filesystem.  Only respected when `core.fsmonitor`
+    reside on a native Mac OS filesystem.  Only respected when `core.fsmonitor`
     is set to `true`.

Documentation/git-fsmonitor--daemon.adoc

@@ -76,9 +76,9 @@ repositories; this may be overridden by setting `fsmonitor.allowRemote` to
 correctly with all network-mounted repositories, so such use is considered
 experimental.
 
-On Mac OS and Linux, the inter-process communication (IPC) between various Git
+On Mac OS, the inter-process communication (IPC) between various Git
 commands and the fsmonitor daemon is done via a Unix domain socket (UDS) -- a
-special type of file -- which is supported by native Mac OS and Linux filesystems,
+special type of file -- which is supported by native Mac OS filesystems,
 but not on network-mounted filesystems, NTFS, or FAT32.  Other filesystems
 may or may not have the needed support; the fsmonitor daemon is not guaranteed
 to work with these filesystems and such use is considered experimental.
@@ -87,33 +87,13 @@ By default, the socket is created in the `.git` directory.  However, if the
 `.git` directory is on a network-mounted filesystem, it will instead be
 created at `$HOME/.git-fsmonitor-*` unless `$HOME` itself is on a
 network-mounted filesystem, in which case you must set the configuration
-variable `fsmonitor.socketDir` to the path of a directory on a native
+variable `fsmonitor.socketDir` to the path of a directory on a Mac OS native
 filesystem in which to create the socket file.
 
 If none of the above directories (`.git`, `$HOME`, or `fsmonitor.socketDir`)
-is on a native filesystem the fsmonitor daemon will report an
+is on a native Mac OS file filesystem the fsmonitor daemon will report an
 error that will cause the daemon and the currently running command to exit.
 
-LINUX CAVEATS
-~~~~~~~~~~~~~
-
-On Linux, the fsmonitor daemon uses inotify to monitor filesystem events.
-The inotify system has per-user limits on the number of watches that can
-be created.  The default limit is typically 8192 watches per user.
-
-For large repositories with many directories, you may need to increase
-this limit.  Check the current limit with:
-
-    cat /proc/sys/fs/inotify/max_user_watches
-
-To temporarily increase the limit:
-
-    sudo sysctl fs.inotify.max_user_watches=65536
-
-To make the change permanent, add to `/etc/sysctl.conf`:
-
-    fs.inotify.max_user_watches=65536
-
 CONFIGURATION
 -------------
 

Makefile

@@ -420,7 +420,7 @@ include shared.mak
 # 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`
-# and `compat/fsmonitor/fsm-ipc-<name>.c` files.
+# that implements the `fsm_os_settings__*()` routines.
 #
 # Define LINK_FUZZ_PROGRAMS if you want `make all` to also build the fuzz test
 # programs in oss-fuzz/.
@@ -2379,13 +2379,13 @@ ifdef FSMONITOR_DAEMON_BACKEND
 	COMPAT_CFLAGS += -DHAVE_FSMONITOR_DAEMON_BACKEND
 	COMPAT_OBJS += compat/fsmonitor/fsm-listen-$(FSMONITOR_DAEMON_BACKEND).o
 	COMPAT_OBJS += compat/fsmonitor/fsm-health-$(FSMONITOR_DAEMON_BACKEND).o
+	COMPAT_OBJS += compat/fsmonitor/fsm-ipc-$(FSMONITOR_DAEMON_BACKEND).o
 endif
 
 ifdef FSMONITOR_OS_SETTINGS
 	COMPAT_CFLAGS += -DHAVE_FSMONITOR_OS_SETTINGS
-	COMPAT_OBJS += compat/fsmonitor/fsm-ipc-$(FSMONITOR_OS_SETTINGS).o
 	COMPAT_OBJS += compat/fsmonitor/fsm-settings-$(FSMONITOR_OS_SETTINGS).o
-	COMPAT_OBJS += compat/fsmonitor/fsm-path-utils-$(FSMONITOR_DAEMON_BACKEND).o
+	COMPAT_OBJS += compat/fsmonitor/fsm-path-utils-$(FSMONITOR_OS_SETTINGS).o
 endif
 
 ifdef WITH_BREAKING_CHANGES

builtin/fsmonitor--daemon.c

@@ -16,7 +16,7 @@
 #include "fsmonitor--daemon.h"
 
 #include "simple-ipc.h"
-#include "strmap.h"
+#include "khash.h"
 #include "run-command.h"
 #include "trace.h"
 #include "trace2.h"
@@ -86,8 +86,6 @@ static int do_as_client__send_stop(void)
 {
 	struct strbuf answer = STRBUF_INIT;
 	int ret;
-	int max_wait_ms = 30000;
-	int elapsed_ms = 0;
 
 	ret = fsmonitor_ipc__send_command("quit", &answer);
 
@@ -98,16 +96,8 @@ static int do_as_client__send_stop(void)
 		return ret;
 
 	trace2_region_enter("fsm_client", "polling-for-daemon-exit", NULL);
-	while (fsmonitor_ipc__get_state() == IPC_STATE__LISTENING) {
-		if (elapsed_ms >= max_wait_ms) {
-			trace2_region_leave("fsm_client",
-					    "polling-for-daemon-exit", NULL);
-			return error(_("daemon did not stop within %d seconds"),
-				     max_wait_ms / 1000);
-		}
+	while (fsmonitor_ipc__get_state() == IPC_STATE__LISTENING)
 		sleep_millisec(50);
-		elapsed_ms += 50;
-	}
 	trace2_region_leave("fsm_client", "polling-for-daemon-exit", NULL);
 
 	return 0;
@@ -207,31 +197,20 @@ static enum fsmonitor_cookie_item_result with_lock__wait_for_cookie(
 	unlink(cookie_pathname.buf);
 
 	/*
-	 * Wait for the listener thread to observe the cookie file.
-	 * Time out after a short interval so that the client
-	 * does not hang forever if the filesystem does not deliver
-	 * events (e.g., on certain container/overlay filesystems
-	 * where inotify watches succeed but events never arrive).
+	 * Technically, this is an infinite wait (well, unless another
+	 * thread sends us an abort).  I'd like to change this to
+	 * use `pthread_cond_timedwait()` and return an error/timeout
+	 * and let the caller do the trivial response thing, but we
+	 * don't have that routine in our thread-utils.
+	 *
+	 * After extensive beta testing I'm not really worried about
+	 * this.  Also note that the above open() and unlink() calls
+	 * will cause at least two FS events on that path, so the odds
+	 * of getting stuck are pretty slim.
 	 */
-	{
-		struct timeval now;
-		struct timespec ts;
-		int err = 0;
-
-		gettimeofday(&now, NULL);
-		ts.tv_sec = now.tv_sec + 1;
-		ts.tv_nsec = now.tv_usec * 1000;
-
-		while (cookie->result == FCIR_INIT && !err)
-			err = pthread_cond_timedwait(&state->cookies_cond,
-						     &state->main_lock,
-						     &ts);
-		if (err == ETIMEDOUT && cookie->result == FCIR_INIT) {
-			trace_printf_key(&trace_fsmonitor,
-					 "cookie_wait timed out");
-			cookie->result = FCIR_ERROR;
-		}
-	}
+	while (cookie->result == FCIR_INIT)
+		pthread_cond_wait(&state->cookies_cond,
+				  &state->main_lock);
 
 done:
 	hashmap_remove(&state->cookies, &cookie->entry, NULL);
@@ -674,6 +653,8 @@ static int fsmonitor_parse_client_token(const char *buf_token,
 	return 0;
 }
 
+KHASH_INIT(str, const char *, int, 0, kh_str_hash_func, kh_str_hash_equal)
+
 static int do_handle_client(struct fsmonitor_daemon_state *state,
 			    const char *command,
 			    ipc_server_reply_cb *reply,
@@ -690,7 +671,8 @@ static int do_handle_client(struct fsmonitor_daemon_state *state,
 	const struct fsmonitor_batch *batch;
 	struct fsmonitor_batch *remainder = NULL;
 	intmax_t count = 0, duplicates = 0;
-	struct strset shown = STRSET_INIT;
+	kh_str_t *shown;
+	int hash_ret;
 	int do_trivial = 0;
 	int do_flush = 0;
 	int do_cookie = 0;
@@ -879,14 +861,14 @@ static int do_handle_client(struct fsmonitor_daemon_state *state,
 	 * so walk the batch list backwards from the current head back
 	 * to the batch (sequence number) they named.
 	 *
-	 * We use a strset to de-dup the list of pathnames.
+	 * We use khash to de-dup the list of pathnames.
 	 *
 	 * NEEDSWORK: each batch contains a list of interned strings,
 	 * so we only need to do pointer comparisons here to build the
 	 * hash table.  Currently, we're still comparing the string
 	 * values.
 	 */
-	strset_init_with_options(&shown, NULL, 0);
+	shown = kh_init_str();
 	for (batch = batch_head;
 	     batch && batch->batch_seq_nr > requested_oldest_seq_nr;
 	     batch = batch->next) {
@@ -896,9 +878,11 @@ static int do_handle_client(struct fsmonitor_daemon_state *state,
 			const char *s = batch->interned_paths[k];
 			size_t s_len;
 
-			if (!strset_add(&shown, s))
+			if (kh_get_str(shown, s) != kh_end(shown))
 				duplicates++;
 			else {
+				kh_put_str(shown, s, &hash_ret);
+
 				trace_printf_key(&trace_fsmonitor,
 						 "send[%"PRIuMAX"]: %s",
 						 count, s);
@@ -925,6 +909,8 @@ static int do_handle_client(struct fsmonitor_daemon_state *state,
 		total_response_len += payload.len;
 	}
 
+	kh_release_str(shown);
+
 	pthread_mutex_lock(&state->main_lock);
 
 	if (token_data->client_ref_count > 0)
@@ -968,7 +954,6 @@ static int do_handle_client(struct fsmonitor_daemon_state *state,
 	trace2_data_intmax("fsmonitor", the_repository, "response/count/duplicates", duplicates);
 
 cleanup:
-	strset_clear(&shown);
 	strbuf_release(&response_token);
 	strbuf_release(&requested_token_id);
 	strbuf_release(&payload);
@@ -1420,15 +1405,6 @@ static int fsmonitor_run_daemon(void)
 done:
 	pthread_cond_destroy(&state.cookies_cond);
 	pthread_mutex_destroy(&state.main_lock);
-	{
-		struct hashmap_iter iter;
-		struct fsmonitor_cookie_item *cookie;
-
-		hashmap_for_each_entry(&state.cookies, &iter, cookie, entry)
-			free(cookie->name);
-		hashmap_clear_and_free(&state.cookies,
-				       struct fsmonitor_cookie_item, entry);
-	}
 	fsm_listen__dtor(&state);
 	fsm_health__dtor(&state);
 
@@ -1444,7 +1420,7 @@ static int fsmonitor_run_daemon(void)
 	return err;
 }
 
-static int try_to_run_foreground_daemon(int detach_console)
+static int try_to_run_foreground_daemon(int detach_console MAYBE_UNUSED)
 {
 	/*
 	 * Technically, we don't need to probe for an existing daemon
@@ -1464,21 +1440,10 @@ static int try_to_run_foreground_daemon(int detach_console)
 		fflush(stderr);
 	}
 
-	if (detach_console) {
 #ifdef GIT_WINDOWS_NATIVE
+	if (detach_console)
 		FreeConsole();
-#else
-		/*
-		 * Create a new session so that the daemon is detached
-		 * from the parent's process group.  This prevents
-		 * shells with job control (e.g. bash with "set -m")
-		 * from waiting on the daemon when they wait for a
-		 * foreground command that implicitly spawned it.
-		 */
-		if (setsid() == -1)
-			warning_errno(_("setsid failed"));
 #endif
-	}
 
 	return !!fsmonitor_run_daemon();
 }
@@ -1541,7 +1506,6 @@ static int try_to_start_background_daemon(void)
 	cp.no_stdin = 1;
 	cp.no_stdout = 1;
 	cp.no_stderr = 1;
-	cp.close_fd_above_stderr = 1;
 
 	sbgr = start_bg_command(&cp, bg_wait_cb, NULL,
 				fsmonitor__start_timeout_sec);