Merge branch 'jh/builtin-fsmonitor-part-2plus' into seen

Various small fixes and cleanups on part-2 of the same topic.

* jh/builtin-fsmonitor-part-2plus:
  fsmonitor-settings: simplify initialization of settings data
  fsmonitor--daemon: add _() to calls to error()
  fsmonitor--daemon: add _() to calls to die()
  compat/fsmonitor/fsm-listen-win32: add _() to calls to error()
  compat/fsmonitor/fsm-listen-darwin: add _() to calls to error()
  fsmonitor-ipc: add _() to calls to die()
  t7527: delete unused verify_status() function
  t7527: fix && chaining in matrix_try()
  t7527: add parameters to start_daemon to handle args and subshell
  t/perf/p7519: cleanup coding style
  t/perf/p7519: use grep rather than egrep in test
  fsmonitor--daemon: refactor cookie handling for readability
  t/helper/fsmonitor-client: cleanup call to parse_options()
  compat/fsmonitor/fsm-listen-darwin: split out GCC-specific declarations
  update-index: convert advise() messages back to warning()
  t/test-lib: avoid using git on LHS of pipe

builtin/fsmonitor--daemon.c

@@ -109,14 +109,14 @@ static int do_as_client__status(void)
 
 enum fsmonitor_cookie_item_result {
 	FCIR_ERROR = -1, /* could not create cookie file ? */
-	FCIR_INIT = 0,
+	FCIR_INIT,
 	FCIR_SEEN,
 	FCIR_ABORT,
 };
 
 struct fsmonitor_cookie_item {
 	struct hashmap_entry entry;
-	const char *name;
+	char *name;
 	enum fsmonitor_cookie_item_result result;
 };
 
@@ -166,37 +166,44 @@ static enum fsmonitor_cookie_item_result with_lock__wait_for_cookie(
 	 * that the listener thread has seen it.
 	 */
 	fd = open(cookie_pathname.buf, O_WRONLY | O_CREAT | O_EXCL, 0600);
-	if (fd >= 0) {
-		close(fd);
-		unlink(cookie_pathname.buf);
-
-		/*
-		 * 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.
-		 */
-		while (cookie->result == FCIR_INIT)
-			pthread_cond_wait(&state->cookies_cond,
-					  &state->main_lock);
-	} else {
+	if (fd < 0) {
 		error_errno(_("could not create fsmonitor cookie '%s'"),
 			    cookie->name);
 
 		cookie->result = FCIR_ERROR;
+		goto done;
 	}
 
+	/*
+	 * Technically, close() and unlink() can fail, but we don't
+	 * care here.  We only created the file to trigger a watch
+	 * event from the FS to know that when we're up to date.
+	 */
+	close(fd);
+	unlink(cookie_pathname.buf);
+
+	/*
+	 * 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.
+	 */
+	while (cookie->result == FCIR_INIT)
+		pthread_cond_wait(&state->cookies_cond,
+				  &state->main_lock);
+
+done:
 	hashmap_remove(&state->cookies, &cookie->entry, NULL);
 
 	result = cookie->result;
 
-	free((char*)cookie->name);
+	free(cookie->name);
 	free(cookie);
 	strbuf_release(&cookie_pathname);
 
@@ -1319,7 +1326,7 @@ static int try_to_run_foreground_daemon(int detach_console)
 	 * common error case.
 	 */
 	if (fsmonitor_ipc__get_state() == IPC_STATE__LISTENING)
-		die("fsmonitor--daemon is already running '%s'",
+		die(_("fsmonitor--daemon is already running '%s'"),
 		    the_repository->worktree);
 
 	if (fsmonitor__announce_startup) {
@@ -1374,7 +1381,7 @@ static int try_to_start_background_daemon(void)
 	 * immediately exited).
 	 */
 	if (fsmonitor_ipc__get_state() == IPC_STATE__LISTENING)
-		die("fsmonitor--daemon is already running '%s'",
+		die(_("fsmonitor--daemon is already running '%s'"),
 		    the_repository->worktree);
 
 	if (fsmonitor__announce_startup) {
@@ -1404,13 +1411,13 @@ static int try_to_start_background_daemon(void)
 	default:
 	case SBGR_ERROR:
 	case SBGR_CB_ERROR:
-		return error("daemon failed to start");
+		return error(_("daemon failed to start"));
 
 	case SBGR_TIMEOUT:
-		return error("daemon not online yet");
+		return error(_("daemon not online yet"));
 
 	case SBGR_DIED:
-		return error("daemon terminated");
+		return error(_("daemon terminated"));
 	}
 }
 

builtin/update-index.c

@@ -1238,18 +1238,18 @@ 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);
 		if (fsm_mode == FSMONITOR_MODE_DISABLED) {
-			advise(_("core.fsmonitor is unset; "
-				 "set it if you really want to "
-				 "enable fsmonitor"));
+			warning(_("core.fsmonitor is unset; "
+				  "set it if you really want to "
+				  "enable fsmonitor"));
 		}
 		add_fsmonitor(&the_index);
 		report(_("fsmonitor enabled"));
 	} else if (!fsmonitor) {
 		enum fsmonitor_mode fsm_mode = fsm_settings__get_mode(r);
 		if (fsm_mode > FSMONITOR_MODE_DISABLED)
-			advise(_("core.fsmonitor is set; "
-				 "remove it if you really want to "
-				 "disable fsmonitor"));
+			warning(_("core.fsmonitor is set; "
+				  "remove it if you really want to "
+				  "disable fsmonitor"));
 		remove_fsmonitor(&the_index);
 		report(_("fsmonitor disabled"));
 	}

compat/fsmonitor/fsm-darwin-gcc.h

@@ -0,0 +1,92 @@
+#ifndef FSM_DARWIN_GCC_H
+#define FSM_DARWIN_GCC_H
+
+#ifndef __clang__
+/*
+ * It is possible to #include CoreFoundation/CoreFoundation.h when compiling
+ * with clang, but not with GCC as of time of writing.
+ *
+ * See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93082 for details.
+ */
+typedef unsigned int FSEventStreamCreateFlags;
+#define kFSEventStreamEventFlagNone               0x00000000
+#define kFSEventStreamEventFlagMustScanSubDirs    0x00000001
+#define kFSEventStreamEventFlagUserDropped        0x00000002
+#define kFSEventStreamEventFlagKernelDropped      0x00000004
+#define kFSEventStreamEventFlagEventIdsWrapped    0x00000008
+#define kFSEventStreamEventFlagHistoryDone        0x00000010
+#define kFSEventStreamEventFlagRootChanged        0x00000020
+#define kFSEventStreamEventFlagMount              0x00000040
+#define kFSEventStreamEventFlagUnmount            0x00000080
+#define kFSEventStreamEventFlagItemCreated        0x00000100
+#define kFSEventStreamEventFlagItemRemoved        0x00000200
+#define kFSEventStreamEventFlagItemInodeMetaMod   0x00000400
+#define kFSEventStreamEventFlagItemRenamed        0x00000800
+#define kFSEventStreamEventFlagItemModified       0x00001000
+#define kFSEventStreamEventFlagItemFinderInfoMod  0x00002000
+#define kFSEventStreamEventFlagItemChangeOwner    0x00004000
+#define kFSEventStreamEventFlagItemXattrMod       0x00008000
+#define kFSEventStreamEventFlagItemIsFile         0x00010000
+#define kFSEventStreamEventFlagItemIsDir          0x00020000
+#define kFSEventStreamEventFlagItemIsSymlink      0x00040000
+#define kFSEventStreamEventFlagOwnEvent           0x00080000
+#define kFSEventStreamEventFlagItemIsHardlink     0x00100000
+#define kFSEventStreamEventFlagItemIsLastHardlink 0x00200000
+#define kFSEventStreamEventFlagItemCloned         0x00400000
+
+typedef struct __FSEventStream *FSEventStreamRef;
+typedef const FSEventStreamRef ConstFSEventStreamRef;
+
+typedef unsigned int CFStringEncoding;
+#define kCFStringEncodingUTF8 0x08000100
+
+typedef const struct __CFString *CFStringRef;
+typedef const struct __CFArray *CFArrayRef;
+typedef const struct __CFRunLoop *CFRunLoopRef;
+
+struct FSEventStreamContext {
+    long long version;
+    void *cb_data, *retain, *release, *copy_description;
+};
+
+typedef struct FSEventStreamContext FSEventStreamContext;
+typedef unsigned int FSEventStreamEventFlags;
+#define kFSEventStreamCreateFlagNoDefer 0x02
+#define kFSEventStreamCreateFlagWatchRoot 0x04
+#define kFSEventStreamCreateFlagFileEvents 0x10
+
+typedef unsigned long long FSEventStreamEventId;
+#define kFSEventStreamEventIdSinceNow 0xFFFFFFFFFFFFFFFFULL
+
+typedef void (*FSEventStreamCallback)(ConstFSEventStreamRef streamRef,
+				      void *context,
+				      __SIZE_TYPE__ num_of_events,
+				      void *event_paths,
+				      const FSEventStreamEventFlags event_flags[],
+				      const FSEventStreamEventId event_ids[]);
+typedef double CFTimeInterval;
+FSEventStreamRef FSEventStreamCreate(void *allocator,
+				     FSEventStreamCallback callback,
+				     FSEventStreamContext *context,
+				     CFArrayRef paths_to_watch,
+				     FSEventStreamEventId since_when,
+				     CFTimeInterval latency,
+				     FSEventStreamCreateFlags flags);
+CFStringRef CFStringCreateWithCString(void *allocator, const char *string,
+				      CFStringEncoding encoding);
+CFArrayRef CFArrayCreate(void *allocator, const void **items, long long count,
+			 void *callbacks);
+void CFRunLoopRun(void);
+void CFRunLoopStop(CFRunLoopRef run_loop);
+CFRunLoopRef CFRunLoopGetCurrent(void);
+extern CFStringRef kCFRunLoopDefaultMode;
+void FSEventStreamScheduleWithRunLoop(FSEventStreamRef stream,
+				      CFRunLoopRef run_loop,
+				      CFStringRef run_loop_mode);
+unsigned char FSEventStreamStart(FSEventStreamRef stream);
+void FSEventStreamStop(FSEventStreamRef stream);
+void FSEventStreamInvalidate(FSEventStreamRef stream);
+void FSEventStreamRelease(FSEventStreamRef stream);
+
+#endif /* !clang */
+#endif /* FSM_DARWIN_GCC_H */

compat/fsmonitor/fsm-listen-darwin.c

@@ -1,95 +1,6 @@
 #ifndef __clang__
-/*
- * It is possible to #include CoreFoundation/CoreFoundation.h when compiling
- * with clang, but not with GCC as of time of writing.
- *
- * See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93082 for details.
- */
-typedef unsigned int FSEventStreamCreateFlags;
-#define kFSEventStreamEventFlagNone               0x00000000
-#define kFSEventStreamEventFlagMustScanSubDirs    0x00000001
-#define kFSEventStreamEventFlagUserDropped        0x00000002
-#define kFSEventStreamEventFlagKernelDropped      0x00000004
-#define kFSEventStreamEventFlagEventIdsWrapped    0x00000008
-#define kFSEventStreamEventFlagHistoryDone        0x00000010
-#define kFSEventStreamEventFlagRootChanged        0x00000020
-#define kFSEventStreamEventFlagMount              0x00000040
-#define kFSEventStreamEventFlagUnmount            0x00000080
-#define kFSEventStreamEventFlagItemCreated        0x00000100
-#define kFSEventStreamEventFlagItemRemoved        0x00000200
-#define kFSEventStreamEventFlagItemInodeMetaMod   0x00000400
-#define kFSEventStreamEventFlagItemRenamed        0x00000800
-#define kFSEventStreamEventFlagItemModified       0x00001000
-#define kFSEventStreamEventFlagItemFinderInfoMod  0x00002000
-#define kFSEventStreamEventFlagItemChangeOwner    0x00004000
-#define kFSEventStreamEventFlagItemXattrMod       0x00008000
-#define kFSEventStreamEventFlagItemIsFile         0x00010000
-#define kFSEventStreamEventFlagItemIsDir          0x00020000
-#define kFSEventStreamEventFlagItemIsSymlink      0x00040000
-#define kFSEventStreamEventFlagOwnEvent           0x00080000
-#define kFSEventStreamEventFlagItemIsHardlink     0x00100000
-#define kFSEventStreamEventFlagItemIsLastHardlink 0x00200000
-#define kFSEventStreamEventFlagItemCloned         0x00400000
-
-typedef struct __FSEventStream *FSEventStreamRef;
-typedef const FSEventStreamRef ConstFSEventStreamRef;
-
-typedef unsigned int CFStringEncoding;
-#define kCFStringEncodingUTF8 0x08000100
-
-typedef const struct __CFString *CFStringRef;
-typedef const struct __CFArray *CFArrayRef;
-typedef const struct __CFRunLoop *CFRunLoopRef;
-
-struct FSEventStreamContext {
-    long long version;
-    void *cb_data, *retain, *release, *copy_description;
-};
-
-typedef struct FSEventStreamContext FSEventStreamContext;
-typedef unsigned int FSEventStreamEventFlags;
-#define kFSEventStreamCreateFlagNoDefer 0x02
-#define kFSEventStreamCreateFlagWatchRoot 0x04
-#define kFSEventStreamCreateFlagFileEvents 0x10
-
-typedef unsigned long long FSEventStreamEventId;
-#define kFSEventStreamEventIdSinceNow 0xFFFFFFFFFFFFFFFFULL
-
-typedef void (*FSEventStreamCallback)(ConstFSEventStreamRef streamRef,
-				      void *context,
-				      __SIZE_TYPE__ num_of_events,
-				      void *event_paths,
-				      const FSEventStreamEventFlags event_flags[],
-				      const FSEventStreamEventId event_ids[]);
-typedef double CFTimeInterval;
-FSEventStreamRef FSEventStreamCreate(void *allocator,
-				     FSEventStreamCallback callback,
-				     FSEventStreamContext *context,
-				     CFArrayRef paths_to_watch,
-				     FSEventStreamEventId since_when,
-				     CFTimeInterval latency,
-				     FSEventStreamCreateFlags flags);
-CFStringRef CFStringCreateWithCString(void *allocator, const char *string,
-				      CFStringEncoding encoding);
-CFArrayRef CFArrayCreate(void *allocator, const void **items, long long count,
-			 void *callbacks);
-void CFRunLoopRun(void);
-void CFRunLoopStop(CFRunLoopRef run_loop);
-CFRunLoopRef CFRunLoopGetCurrent(void);
-extern CFStringRef kCFRunLoopDefaultMode;
-void FSEventStreamScheduleWithRunLoop(FSEventStreamRef stream,
-				      CFRunLoopRef run_loop,
-				      CFStringRef run_loop_mode);
-unsigned char FSEventStreamStart(FSEventStreamRef stream);
-void FSEventStreamStop(FSEventStreamRef stream);
-void FSEventStreamInvalidate(FSEventStreamRef stream);
-void FSEventStreamRelease(FSEventStreamRef stream);
+#include "fsm-darwin-gcc.h"
 #else
-/*
- * Let Apple's headers declare `isalnum()` first, before
- * Git's headers override it via a constant
- */
-#include <string.h>
 #include <CoreFoundation/CoreFoundation.h>
 #include <CoreServices/CoreServices.h>
 #endif
@@ -424,7 +335,7 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state)
 	return 0;
 
 failed:
-	error("Unable to create FSEventStream.");
+	error(_("Unable to create FSEventStream."));
 
 	FREE_AND_NULL(state->backend_data);
 	return -1;
@@ -472,7 +383,7 @@ void fsm_listen__loop(struct fsmonitor_daemon_state *state)
 	data->stream_scheduled = 1;
 
 	if (!FSEventStreamStart(data->stream)) {
-		error("Failed to start the FSEventStream");
+		error(_("Failed to start the FSEventStream"));
 		goto force_error_stop_without_loop;
 	}
 	data->stream_started = 1;

compat/fsmonitor/fsm-listen-win32.c

@@ -82,7 +82,7 @@ static int normalize_path_in_utf8(FILE_NOTIFY_INFORMATION *info,
 		if (len > 0)
 			goto normalize;
 		if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) {
-			error("[GLE %ld] could not convert path to UTF-8: '%.*ls'",
+			error(_("[GLE %ld] could not convert path to UTF-8: '%.*ls'"),
 			      GetLastError(),
 			      (int)(info->FileNameLength / sizeof(WCHAR)),
 			      info->FileName);
@@ -185,7 +185,7 @@ static int start_rdcw_watch(struct fsmonitor_daemon_backend_data *data,
 	if (watch->is_active)
 		return 0;
 
-	error("ReadDirectoryChangedW failed on '%s' [GLE %ld]",
+	error(_("ReadDirectoryChangedW failed on '%s' [GLE %ld]"),
 	      watch->path.buf, GetLastError());
 	return -1;
 }
@@ -228,7 +228,7 @@ static int recv_rdcw_watch(struct one_watch *watch)
 	 * sure it is worth it.
 	 */
 
-	error("GetOverlappedResult failed on '%s' [GLE %ld]",
+	error(_("GetOverlappedResult failed on '%s' [GLE %ld]"),
 	      watch->path.buf, gle);
 	return -1;
 }

fsmonitor-ipc.c

@@ -152,7 +152,7 @@ int fsmonitor_ipc__send_command(const char *command,
 	state = ipc_client_try_connect(fsmonitor_ipc__get_path(), &options,
 				       &connection);
 	if (state != IPC_STATE__LISTENING) {
-		die("fsmonitor--daemon is not running");
+		die(_("fsmonitor--daemon is not running"));
 		return -1;
 	}
 
@@ -161,7 +161,7 @@ int fsmonitor_ipc__send_command(const char *command,
 	ipc_client_close_connection(connection);
 
 	if (ret == -1) {
-		die("could not send '%s' command to fsmonitor--daemon", c);
+		die(_("could not send '%s' command to fsmonitor--daemon"), c);
 		return -1;
 	}