{"thread":{"id":"60926","subject":"[PATCH 0/7] fsmonitor: completing a stale patch that Implements fsmonitor for Linux","startedAt":"2024-02-15T10:29:42Z","lastAt":"2025-01-31T03:29:06Z","messageCount":14,"participants":["marzi via GitGitGadget","Eric DeCosta via GitGitGadget","marzi.esipreh via GitGitGadget","Jean-Noël Avila","Patrick Steinhardt","Manoraj K"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"488715","messageId":"pull.1667.git.git.1707992978.gitgitgadget@gmail.com","threadId":"60926","inReplyTo":null,"subject":"[PATCH 0/7] fsmonitor: completing a stale patch that Implements fsmonitor for Linux","fromName":"marzi via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-02-15T10:29:31Z","receivedAt":"2024-02-15T10:29:42Z","isPatch":true,"sender":{"key":"name:marzi","avatar":null},"body":"The goal in the original pull is to deliver fsmonitor for Linux that works\nsimilar to fsmonitor for Windows and Mac OS.\n\nThe original stale patch : https://github.com/git/git/pull/1352\n\nThis patch set builds upon previous work for done for Windows and Mac OS to\nimplement a fsmonitor back-end for Linux based on the Linux inotify API.\ninotify differs significantly from the equivalent Windows and Mac OS APIs in\nthat a watch must be registered for every directory of interest (rather than\na singular watch at the root of the directory tree) and special care must be\ntaken to handle directory renames correctly.\n\nMore information about inotify:\nhttps://man7.org/linux/man-pages/man7/inotify.7.html\n\nEric DeCosta (6):\n fsmonitor: rebase with master\n fsmonitor: determine if filesystem is local or remote\n fsmonitor: implement filesystem change listener for Linux\n fsmonitor: enable fsmonitor for Linux\n fsmonitor: test updates\n fsmonitor: update doc for Linux\n\nmarzi.esipreh (1):\n fsmonitor: addressed comments for patch 1352\n\n Documentation/config/fsmonitor--daemon.txt | 4 +-\n Documentation/git-fsmonitor--daemon.txt | 26 +-\n Makefile | 4 +\n compat/fsmonitor/fsm-health-linux.c | 24 +\n compat/fsmonitor/fsm-ipc-darwin.c | 57 +-\n compat/fsmonitor/fsm-ipc-linux.c | 1 +\n compat/fsmonitor/fsm-ipc-unix.c | 55 ++\n compat/fsmonitor/fsm-listen-linux.c | 678 +++++++++++++++++++++\n compat/fsmonitor/fsm-path-utils-linux.c | 196 ++++++\n compat/fsmonitor/fsm-path-utils-linux.h | 91 +++\n compat/fsmonitor/fsm-settings-darwin.c | 64 +-\n compat/fsmonitor/fsm-settings-linux.c | 1 +\n compat/fsmonitor/fsm-settings-unix.c | 64 ++\n config.mak.uname | 11 +\n contrib/buildsystems/CMakeLists.txt | 11 +-\n t/t7527-builtin-fsmonitor.sh | 138 ++++-\n 16 files changed, 1262 insertions(+), 163 deletions(-)\n create mode 100644 compat/fsmonitor/fsm-health-linux.c\n create mode 100644 compat/fsmonitor/fsm-ipc-linux.c\n create mode 100644 compat/fsmonitor/fsm-ipc-unix.c\n create mode 100644 compat/fsmonitor/fsm-listen-linux.c\n create mode 100644 compat/fsmonitor/fsm-path-utils-linux.c\n create mode 100644 compat/fsmonitor/fsm-path-utils-linux.h\n create mode 100644 compat/fsmonitor/fsm-settings-linux.c\n create mode 100644 compat/fsmonitor/fsm-settings-unix.c\n\n\nbase-commit: 4fc51f00ef18d2c0174ab2fd39d0ee473fd144bd\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1667%2Fmaryis%2Ffsmonitor_linux-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1667/maryis/fsmonitor_linux-v1\nPull-Request: https://github.com/git/git/pull/1667\n-- \ngitgitgadget\n"},{"id":"488716","messageId":"5973bbe18aeecf486d8256cc402285665c45e66a.1707992978.git.gitgitgadget@gmail.com","threadId":"60926","inReplyTo":"pull.1667.git.git.1707992978.gitgitgadget@gmail.com","subject":"[PATCH 1/7] fsmonitor: rebase with master","fromName":"Eric DeCosta via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-02-15T10:29:32Z","receivedAt":"2024-02-15T10:29:43Z","isPatch":true,"sender":{"key":"edecosta@mathworks.com","avatar":"https://avatars.githubusercontent.com/u/67609563?v=4"},"body":"From: Eric DeCosta \n\nrebased with master, and resolved conflicts\nSigned-off-by: Eric DeCosta \n---\n compat/fsmonitor/fsm-health-linux.c | 24 ++++++++++\n compat/fsmonitor/fsm-ipc-darwin.c | 57 +----------------------\n compat/fsmonitor/fsm-ipc-linux.c | 1 +\n compat/fsmonitor/fsm-ipc-unix.c | 53 +++++++++++++++++++++\n compat/fsmonitor/fsm-settings-darwin.c | 64 +-------------------------\n compat/fsmonitor/fsm-settings-linux.c | 1 +\n compat/fsmonitor/fsm-settings-unix.c | 61 ++++++++++++++++++++++++\n 7 files changed, 142 insertions(+), 119 deletions(-)\n create mode 100644 compat/fsmonitor/fsm-health-linux.c\n create mode 100644 compat/fsmonitor/fsm-ipc-linux.c\n create mode 100644 compat/fsmonitor/fsm-ipc-unix.c\n create mode 100644 compat/fsmonitor/fsm-settings-linux.c\n create mode 100644 compat/fsmonitor/fsm-settings-unix.c\n\ndiff --git a/compat/fsmonitor/fsm-health-linux.c b/compat/fsmonitor/fsm-health-linux.c\nnew file mode 100644\nindex 00000000000..b9f709e8548\n--- /dev/null\n+++ b/compat/fsmonitor/fsm-health-linux.c\n@@ -0,0 +1,24 @@\n+#include \"cache.h\"\n+#include \"config.h\"\n+#include \"fsmonitor.h\"\n+#include \"fsm-health.h\"\n+#include \"fsmonitor--daemon.h\"\n+\n+int fsm_health__ctor(struct fsmonitor_daemon_state *state)\n+{\n+\treturn 0;\n+}\n+\n+void fsm_health__dtor(struct fsmonitor_daemon_state *state)\n+{\n+\treturn;\n+}\n+\n+void fsm_health__loop(struct fsmonitor_daemon_state *state)\n+{\n+\treturn;\n+}\n+\n+void fsm_health__stop_async(struct fsmonitor_daemon_state *state)\n+{\n+}\ndiff --git a/compat/fsmonitor/fsm-ipc-darwin.c b/compat/fsmonitor/fsm-ipc-darwin.c\nindex 6f3a95410cc..4c3c92081ee 100644\n--- a/compat/fsmonitor/fsm-ipc-darwin.c\n+++ b/compat/fsmonitor/fsm-ipc-darwin.c\n@@ -1,56 +1 @@\n-#include \"git-compat-util.h\"\n-#include \"config.h\"\n-#include \"gettext.h\"\n-#include \"hex.h\"\n-#include \"path.h\"\n-#include \"repository.h\"\n-#include \"strbuf.h\"\n-#include \"fsmonitor-ll.h\"\n-#include \"fsmonitor-ipc.h\"\n-#include \"fsmonitor-path-utils.h\"\n-\n-static GIT_PATH_FUNC(fsmonitor_ipc__get_default_path, \"fsmonitor--daemon.ipc\")\n-\n-const char *fsmonitor_ipc__get_path(struct repository *r)\n-{\n-\tstatic const char *ipc_path = NULL;\n-\tgit_SHA_CTX sha1ctx;\n-\tchar *sock_dir = NULL;\n-\tstruct strbuf ipc_file = STRBUF_INIT;\n-\tunsigned char hash[GIT_MAX_RAWSZ];\n-\n-\tif (!r)\n-\t\tBUG(\"No repository passed into fsmonitor_ipc__get_path\");\n-\n-\tif (ipc_path)\n-\t\treturn ipc_path;\n-\n-\n-\t/* By default the socket file is created in the .git directory */\n-\tif (fsmonitor__is_fs_remote(r->gitdir) < 1) {\n-\t\tipc_path = fsmonitor_ipc__get_default_path();\n-\t\treturn ipc_path;\n-\t}\n-\n-\tgit_SHA1_Init(&sha1ctx);\n-\tgit_SHA1_Update(&sha1ctx, r->worktree, strlen(r->worktree));\n-\tgit_SHA1_Final(hash, &sha1ctx);\n-\n-\trepo_config_get_string(r, \"fsmonitor.socketdir\", &sock_dir);\n-\n-\t/* Create the socket file in either socketDir or $HOME */\n-\tif (sock_dir && *sock_dir) {\n-\t\tstrbuf_addf(&ipc_file, \"%s/.git-fsmonitor-%s\",\n-\t\t\t\t\tsock_dir, hash_to_hex(hash));\n-\t} else {\n-\t\tstrbuf_addf(&ipc_file, \"~/.git-fsmonitor-%s\", hash_to_hex(hash));\n-\t}\n-\tfree(sock_dir);\n-\n-\tipc_path = interpolate_path(ipc_file.buf, 1);\n-\tif (!ipc_path)\n-\t\tdie(_(\"Invalid path: %s\"), ipc_file.buf);\n-\n-\tstrbuf_release(&ipc_file);\n-\treturn ipc_path;\n-}\n+#include \"fsm-ipc-unix.c\"\ndiff --git a/compat/fsmonitor/fsm-ipc-linux.c b/compat/fsmonitor/fsm-ipc-linux.c\nnew file mode 100644\nindex 00000000000..4c3c92081ee\n--- /dev/null\n+++ b/compat/fsmonitor/fsm-ipc-linux.c\n@@ -0,0 +1 @@\n+#include \"fsm-ipc-unix.c\"\ndiff --git a/compat/fsmonitor/fsm-ipc-unix.c b/compat/fsmonitor/fsm-ipc-unix.c\nnew file mode 100644\nindex 00000000000..eb25123fa12\n--- /dev/null\n+++ b/compat/fsmonitor/fsm-ipc-unix.c\n@@ -0,0 +1,53 @@\n+#include \"cache.h\"\n+#include \"config.h\"\n+#include \"hex.h\"\n+#include \"strbuf.h\"\n+#include \"fsmonitor.h\"\n+#include \"fsmonitor-ipc.h\"\n+#include \"fsmonitor-path-utils.h\"\n+\n+static GIT_PATH_FUNC(fsmonitor_ipc__get_default_path, \"fsmonitor--daemon.ipc\")\n+\n+const char *fsmonitor_ipc__get_path(struct repository *r)\n+{\n+\tstatic const char *ipc_path = NULL;\n+\tgit_SHA_CTX sha1ctx;\n+\tchar *sock_dir = NULL;\n+\tstruct strbuf ipc_file = STRBUF_INIT;\n+\tunsigned char hash[GIT_MAX_RAWSZ];\n+\n+\tif (!r)\n+\t\tBUG(\"No repository passed into fsmonitor_ipc__get_path\");\n+\n+\tif (ipc_path)\n+\t\treturn ipc_path;\n+\n+\n+\t/* By default the socket file is created in the .git directory */\n+\tif (fsmonitor__is_fs_remote(r->gitdir) < 1) {\n+\t\tipc_path = fsmonitor_ipc__get_default_path();\n+\t\treturn ipc_path;\n+\t}\n+\n+\tgit_SHA1_Init(&sha1ctx);\n+\tgit_SHA1_Update(&sha1ctx, r->worktree, strlen(r->worktree));\n+\tgit_SHA1_Final(hash, &sha1ctx);\n+\n+\trepo_config_get_string(r, \"fsmonitor.socketdir\", &sock_dir);\n+\n+\t/* Create the socket file in either socketDir or $HOME */\n+\tif (sock_dir && *sock_dir) {\n+\t\tstrbuf_addf(&ipc_file, \"%s/.git-fsmonitor-%s\",\n+\t\t\t\t\tsock_dir, hash_to_hex(hash));\n+\t} else {\n+\t\tstrbuf_addf(&ipc_file, \"~/.git-fsmonitor-%s\", hash_to_hex(hash));\n+\t}\n+\tfree(sock_dir);\n+\n+\tipc_path = interpolate_path(ipc_file.buf, 1);\n+\tif (!ipc_path)\n+\t\tdie(_(\"Invalid path: %s\"), ipc_file.buf);\n+\n+\tstrbuf_release(&ipc_file);\n+\treturn ipc_path;\n+}\ndiff --git a/compat/fsmonitor/fsm-settings-darwin.c b/compat/fsmonitor/fsm-settings-darwin.c\nindex a3825906351..14baf9f0603 100644\n--- a/compat/fsmonitor/fsm-settings-darwin.c\n+++ b/compat/fsmonitor/fsm-settings-darwin.c\n@@ -1,63 +1 @@\n-#include \"git-compat-util.h\"\n-#include \"config.h\"\n-#include \"fsmonitor-ll.h\"\n-#include \"fsmonitor-ipc.h\"\n-#include \"fsmonitor-settings.h\"\n-#include \"fsmonitor-path-utils.h\"\n-\n- /*\n- * For the builtin FSMonitor, we create the Unix domain socket for the\n- * IPC in the .git directory. If the working directory is remote,\n- * then the socket will be created on the remote file system. This\n- * can fail if the remote file system does not support UDS file types\n- * (e.g. smbfs to a Windows server) or if the remote kernel does not\n- * allow a non-local process to bind() the socket. (These problems\n- * could be fixed by moving the UDS out of the .git directory and to a\n- * well-known local directory on the client machine, but care should\n- * be taken to ensure that $HOME is actually local and not a managed\n- * file share.)\n- *\n- * FAT32 and NTFS working directories are problematic too.\n- *\n- * The builtin FSMonitor uses a Unix domain socket in the .git\n- * directory for IPC. These Windows drive formats do not support\n- * Unix domain sockets, so mark them as incompatible for the daemon.\n- *\n- */\n-static enum fsmonitor_reason check_uds_volume(struct repository *r)\n-{\n-\tstruct fs_info fs;\n-\tconst char *ipc_path = fsmonitor_ipc__get_path(r);\n-\tstruct strbuf path = STRBUF_INIT;\n-\tstrbuf_add(&path, ipc_path, strlen(ipc_path));\n-\n-\tif (fsmonitor__get_fs_info(dirname(path.buf), &fs) == -1) {\n-\t\tstrbuf_release(&path);\n-\t\treturn FSMONITOR_REASON_ERROR;\n-\t}\n-\n-\tstrbuf_release(&path);\n-\n-\tif (fs.is_remote ||\n-\t\t!strcmp(fs.typename, \"msdos\") ||\n-\t\t!strcmp(fs.typename, \"ntfs\")) {\n-\t\tfree(fs.typename);\n-\t\treturn FSMONITOR_REASON_NOSOCKETS;\n-\t}\n-\n-\tfree(fs.typename);\n-\treturn FSMONITOR_REASON_OK;\n-}\n-\n-enum fsmonitor_reason fsm_os__incompatible(struct repository *r, int ipc)\n-{\n-\tenum fsmonitor_reason reason;\n-\n-\tif (ipc) {\n-\t\treason = check_uds_volume(r);\n-\t\tif (reason != FSMONITOR_REASON_OK)\n-\t\t\treturn reason;\n-\t}\n-\n-\treturn FSMONITOR_REASON_OK;\n-}\n+#include \"fsm-settings-unix.c\"\ndiff --git a/compat/fsmonitor/fsm-settings-linux.c b/compat/fsmonitor/fsm-settings-linux.c\nnew file mode 100644\nindex 00000000000..14baf9f0603\n--- /dev/null\n+++ b/compat/fsmonitor/fsm-settings-linux.c\n@@ -0,0 +1 @@\n+#include \"fsm-settings-unix.c\"\ndiff --git a/compat/fsmonitor/fsm-settings-unix.c b/compat/fsmonitor/fsm-settings-unix.c\nnew file mode 100644\nindex 00000000000..d16dca89416\n--- /dev/null\n+++ b/compat/fsmonitor/fsm-settings-unix.c\n@@ -0,0 +1,61 @@\n+#include \"fsmonitor.h\"\n+#include \"fsmonitor-ipc.h\"\n+#include \"fsmonitor-path-utils.h\"\n+\n+ /*\n+ * For the builtin FSMonitor, we create the Unix domain socket for the\n+ * IPC in the .git directory. If the working directory is remote,\n+ * then the socket will be created on the remote file system. This\n+ * can fail if the remote file system does not support UDS file types\n+ * (e.g. smbfs to a Windows server) or if the remote kernel does not\n+ * allow a non-local process to bind() the socket. (These problems\n+ * could be fixed by moving the UDS out of the .git directory and to a\n+ * well-known local directory on the client machine, but care should\n+ * be taken to ensure that $HOME is actually local and not a managed\n+ * file share.)\n+ *\n+ * FAT32 and NTFS working directories are problematic too.\n+ *\n+ * The builtin FSMonitor uses a Unix domain socket in the .git\n+ * directory for IPC. These Windows drive formats do not support\n+ * Unix domain sockets, so mark them as incompatible for the daemon.\n+ *\n+ */\n+static enum fsmonitor_reason check_uds_volume(struct repository *r)\n+{\n+\tstruct fs_info fs;\n+\tconst char *ipc_path = fsmonitor_ipc__get_path(r);\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstrbuf_addstr(&path, ipc_path);\n+\n+\tif (fsmonitor__get_fs_info(dirname(path.buf), &fs) == -1) {\n+\t\tfree(fs.typename);\n+\t\tstrbuf_release(&path);\n+\t\treturn FSMONITOR_REASON_ERROR;\n+\t}\n+\n+\tstrbuf_release(&path);\n+\n+\tif (fs.is_remote ||\n+\t\t!strcmp(fs.typename, \"msdos\") ||\n+\t\t!strcmp(fs.typename, \"ntfs\")) {\n+\t\tfree(fs.typename);\n+\t\treturn FSMONITOR_REASON_NOSOCKETS;\n+\t}\n+\n+\tfree(fs.typename);\n+\treturn FSMONITOR_REASON_OK;\n+}\n+\n+enum fsmonitor_reason fsm_os__incompatible(struct repository *r, int ipc)\n+{\n+\tenum fsmonitor_reason reason;\n+\n+\tif (ipc) {\n+\t\treason = check_uds_volume(r);\n+\t\tif (reason != FSMONITOR_REASON_OK)\n+\t\t\treturn reason;\n+\t}\n+\n+\treturn FSMONITOR_REASON_OK;\n+}\n-- \ngitgitgadget\n\n"},{"id":"488717","messageId":"d26de10866662a5bcd16d562cd1063dedd21cf02.1707992978.git.gitgitgadget@gmail.com","threadId":"60926","inReplyTo":"pull.1667.git.git.1707992978.gitgitgadget@gmail.com","subject":"[PATCH 2/7] fsmonitor: determine if filesystem is local or remote","fromName":"Eric DeCosta via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-02-15T10:29:33Z","receivedAt":"2024-02-15T10:29:44Z","isPatch":true,"sender":{"key":"edecosta@mathworks.com","avatar":"https://avatars.githubusercontent.com/u/67609563?v=4"},"body":"From: Eric DeCosta \n\nCompare the given path to the mounted filesystems. Find the mount that is\nthe longest prefix of the path (if any) and determine if that mount is on a\nlocal or remote filesystem.\n\nSigned-off-by: Eric DeCosta \n---\n Makefile | 4 +\n compat/fsmonitor/fsm-path-utils-linux.c | 195 ++++++++++++++++++++++++\n compat/fsmonitor/fsm-path-utils-linux.h | 91 +++++++++++\n config.mak.uname | 11 ++\n 4 files changed, 301 insertions(+)\n create mode 100644 compat/fsmonitor/fsm-path-utils-linux.c\n create mode 100644 compat/fsmonitor/fsm-path-utils-linux.h\n\ndiff --git a/Makefile b/Makefile\nindex 78e874099d9..0f36a0fd83a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2088,6 +2088,10 @@ ifdef HAVE_CLOCK_GETTIME\n \tBASIC_CFLAGS += -DHAVE_CLOCK_GETTIME\n endif\n \n+ifdef HAVE_LINUX_MAGIC_H\n+\tBASIC_CFLAGS += -DHAVE_LINUX_MAGIC_H\n+endif\n+\n ifdef HAVE_CLOCK_MONOTONIC\n \tBASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC\n endif\ndiff --git a/compat/fsmonitor/fsm-path-utils-linux.c b/compat/fsmonitor/fsm-path-utils-linux.c\nnew file mode 100644\nindex 00000000000..c21d1349532\n--- /dev/null\n+++ b/compat/fsmonitor/fsm-path-utils-linux.c\n@@ -0,0 +1,195 @@\n+#include \"git-compat-util.h\"\n+#include \"abspath.h\"\n+#include \"fsmonitor.h\"\n+#include \"fsmonitor-path-utils.h\"\n+#include \"fsm-path-utils-linux.h\"\n+#include \n+#include \n+#include \n+#include \n+#include \n+\n+static int is_remote_fs(const char *path)\n+{\n+\tstruct statfs fs;\n+\n+\tif (statfs(path, &fs))\n+\t\treturn error_errno(_(\"statfs('%s') failed\"), path);\n+\n+\tswitch (fs.f_type) {\n+\tcase ACFS_SUPER_MAGIC:\n+\tcase AFS_SUPER_MAGIC:\n+\tcase CEPH_SUPER_MAGIC:\n+\tcase CIFS_SUPER_MAGIC:\n+\tcase CODA_SUPER_MAGIC:\n+\tcase FHGFS_SUPER_MAGIC:\n+\tcase GFS_SUPER_MAGIC:\n+\tcase GPFS_SUPER_MAGIC:\n+\tcase IBRIX_SUPER_MAGIC:\n+\tcase KAFS_SUPER_MAGIC:\n+\tcase LUSTRE_SUPER_MAGIC:\n+\tcase NCP_SUPER_MAGIC:\n+\tcase NFS_SUPER_MAGIC:\n+\tcase NFSD_SUPER_MAGIC:\n+\tcase OCFS2_SUPER_MAGIC:\n+\tcase PANFS_SUPER_MAGIC:\n+\tcase SMB_SUPER_MAGIC:\n+\tcase SMB2_SUPER_MAGIC:\n+\tcase SNFS_SUPER_MAGIC:\n+\tcase VMHGFS_SUPER_MAGIC:\n+\tcase VXFS_SUPER_MAGIC:\n+\t\treturn 1;\n+\tdefault:\n+\t\treturn 0;\n+\t}\n+}\n+\n+static int find_mount(const char *path, const struct statvfs *fs,\n+\t\t\tstruct mntent *entry)\n+{\n+\tconst char *const mounts = \"/proc/mounts\";\n+\tchar *rp = real_pathdup(path, 1);\n+\tstruct mntent *ment = NULL;\n+\tstruct statvfs mntfs;\n+\tFILE *fp;\n+\tint found = 0;\n+\tint ret = 0;\n+\tsize_t dlen, plen, flen = 0;\n+\n+\tentry->mnt_fsname = NULL;\n+\tentry->mnt_dir = NULL;\n+\tentry->mnt_type = NULL;\n+\n+\tfp = setmntent(mounts, \"r\");\n+\tif (!fp) {\n+\t\tfree(rp);\n+\t\treturn error_errno(_(\"setmntent('%s') failed\"), mounts);\n+\t}\n+\n+\tplen = strlen(rp);\n+\n+\t/* read all the mount information and compare to path */\n+\twhile ((ment = getmntent(fp))) {\n+\t\tif (statvfs(ment->mnt_dir, &mntfs)) {\n+\t\t\tswitch (errno) {\n+\t\t\tcase EPERM:\n+\t\t\tcase ESRCH:\n+\t\t\tcase EACCES:\n+\t\t\t\tcontinue;\n+\t\t\tdefault:\n+\t\t\t\terror_errno(_(\"statvfs('%s') failed\"), ment->mnt_dir);\n+\t\t\t\tret = -1;\n+\t\t\t\tgoto done;\n+\t\t\t}\n+\t\t}\n+\n+\t\t/* is mount on the same filesystem and is a prefix of the path */\n+\t\tif ((fs->f_fsid == mntfs.f_fsid) &&\n+\t\t\t!strncmp(ment->mnt_dir, rp, strlen(ment->mnt_dir))) {\n+\t\t\tdlen = strlen(ment->mnt_dir);\n+\t\t\tif (dlen > plen)\n+\t\t\t\tcontinue;\n+\t\t\t/*\n+\t\t\t * look for the longest prefix (including root)\n+\t\t\t */\n+\t\t\tif (dlen > flen &&\n+\t\t\t\t((dlen == 1 && ment->mnt_dir[0] == '/') ||\n+\t\t\t\t (!rp[dlen] || rp[dlen] == '/'))) {\n+\t\t\t\tflen = dlen;\n+\t\t\t\tfound = 1;\n+\n+\t\t\t\t/*\n+\t\t\t\t * https://man7.org/linux/man-pages/man3/getmntent.3.html\n+\t\t\t\t *\n+\t\t\t\t * The pointer points to a static area of memory which is\n+\t\t\t\t * overwritten by subsequent calls to getmntent().\n+\t\t\t\t */\n+\t\t\t\tfree(entry->mnt_fsname);\n+\t\t\t\tfree(entry->mnt_dir);\n+\t\t\t\tfree(entry->mnt_type);\n+\t\t\t\tentry->mnt_fsname = xstrdup(ment->mnt_fsname);\n+\t\t\t\tentry->mnt_dir = xstrdup(ment->mnt_dir);\n+\t\t\t\tentry->mnt_type = xstrdup(ment->mnt_type);\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+done:\n+\tfree(rp);\n+\tendmntent(fp);\n+\n+\tif (!found)\n+\t\treturn -1;\n+\n+\treturn ret;\n+}\n+\n+int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info)\n+{\n+\tint ret = 0;\n+\tstruct mntent entry;\n+\tstruct statvfs fs;\n+\n+\tfs_info->is_remote = -1;\n+\tfs_info->typename = NULL;\n+\n+\tif (statvfs(path, &fs))\n+\t\treturn error_errno(_(\"statvfs('%s') failed\"), path);\n+\n+\tif (find_mount(path, &fs, &entry) < 0) {\n+\t\tret = -1;\n+\t\tgoto done;\n+\t}\n+\n+\ttrace_printf_key(&trace_fsmonitor,\n+\t\t\t \"statvfs('%s') [flags 0x%08lx] '%s' '%s'\",\n+\t\t\t path, fs.f_flag, entry.mnt_type, entry.mnt_fsname);\n+\n+\tfs_info->is_remote = is_remote_fs(entry.mnt_dir);\n+\tfs_info->typename = xstrdup(entry.mnt_fsname);\n+\n+\tif (fs_info->is_remote < 0)\n+\t\tret = -1;\n+\n+\ttrace_printf_key(&trace_fsmonitor,\n+\t\t\t\t\"'%s' is_remote: %d\",\n+\t\t\t\tpath, fs_info->is_remote);\n+\n+done:\n+\tfree(entry.mnt_fsname);\n+\tfree(entry.mnt_dir);\n+\tfree(entry.mnt_type);\n+\treturn ret;\n+}\n+\n+int fsmonitor__is_fs_remote(const char *path)\n+{\n+\tint ret = 0;\n+\tstruct fs_info fs;\n+\n+\tif (fsmonitor__get_fs_info(path, &fs))\n+\t\tret = -1;\n+\telse\n+\t\tret = fs.is_remote;\n+\n+\tfree(fs.typename);\n+\n+\treturn ret;\n+}\n+\n+/*\n+ * No-op for now.\n+ */\n+int fsmonitor__get_alias(const char *path, struct alias_info *info)\n+{\n+\treturn 0;\n+}\n+\n+/*\n+ * No-op for now.\n+ */\n+char *fsmonitor__resolve_alias(const char *path,\n+\t\tconst struct alias_info *info)\n+{\n+\treturn NULL;\n+}\ndiff --git a/compat/fsmonitor/fsm-path-utils-linux.h b/compat/fsmonitor/fsm-path-utils-linux.h\nnew file mode 100644\nindex 00000000000..49bdb3c4728\n--- /dev/null\n+++ b/compat/fsmonitor/fsm-path-utils-linux.h\n@@ -0,0 +1,91 @@\n+#ifndef FSM_PATH_UTILS_LINUX_H\n+#define FSM_PATH_UTILS_LINUX_H\n+#endif\n+\n+#ifdef HAVE_LINUX_MAGIC_H\n+#include \n+#endif\n+\n+#ifndef ACFS_SUPER_MAGIC\n+#define ACFS_SUPER_MAGIC 0x61636673\n+#endif\n+\n+#ifndef AFS_SUPER_MAGIC\n+#define AFS_SUPER_MAGIC 0x5346414f\n+#endif\n+\n+#ifndef CEPH_SUPER_MAGIC\n+#define CEPH_SUPER_MAGIC 0x00c36400\n+#endif\n+\n+#ifndef CIFS_SUPER_MAGIC\n+#define CIFS_SUPER_MAGIC 0xff534d42\n+#endif\n+\n+#ifndef CODA_SUPER_MAGIC\n+#define CODA_SUPER_MAGIC 0x73757245\n+#endif\n+\n+#ifndef FHGFS_SUPER_MAGIC\n+#define FHGFS_SUPER_MAGIC 0x19830326\n+#endif\n+\n+#ifndef GFS_SUPER_MAGIC\n+#define GFS_SUPER_MAGIC 0x1161970\n+#endif\n+\n+#ifndef GPFS_SUPER_MAGIC\n+#define GPFS_SUPER_MAGIC 0x47504653\n+#endif\n+\n+#ifndef IBRIX_SUPER_MAGIC\n+#define IBRIX_SUPER_MAGIC 0x013111a8\n+#endif\n+\n+#ifndef KAFS_SUPER_MAGIC\n+#define KAFS_SUPER_MAGIC 0x6b414653\n+#endif\n+\n+#ifndef LUSTRE_SUPER_MAGIC\n+#define LUSTRE_SUPER_MAGIC 0x0bd00bd0\n+#endif\n+\n+#ifndef NCP_SUPER_MAGIC\n+#define NCP_SUPER_MAGIC 0x564c\n+#endif\n+\n+#ifndef NFS_SUPER_MAGIC\n+#define NFS_SUPER_MAGIC 0x6969\n+#endif\n+\n+#ifndef NFSD_SUPER_MAGIC\n+#define NFSD_SUPER_MAGIC 0x6e667364\n+#endif\n+\n+#ifndef OCFS2_SUPER_MAGIC\n+#define OCFS2_SUPER_MAGIC 0x7461636f\n+#endif\n+\n+#ifndef PANFS_SUPER_MAGIC\n+#define PANFS_SUPER_MAGIC 0xaad7aaea\n+#endif\n+\n+#ifndef SMB_SUPER_MAGIC\n+#define SMB_SUPER_MAGIC 0x517b\n+#endif\n+\n+#ifndef SMB2_SUPER_MAGIC\n+#define SMB2_SUPER_MAGIC 0xfe534d42\n+#endif\n+\n+#ifndef SNFS_SUPER_MAGIC\n+#define SNFS_SUPER_MAGIC 0xbeefdead\n+#endif\n+\n+#ifndef VMHGFS_SUPER_MAGIC\n+#define VMHGFS_SUPER_MAGIC 0xbacbacbc\n+#endif\n+\n+#ifndef VXFS_SUPER_MAGIC\n+#define VXFS_SUPER_MAGIC 0xa501fcf5\n+#endif\ndiff --git a/config.mak.uname b/config.mak.uname\nindex dacc95172dc..80d7e2a2e68 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -68,6 +68,17 @@ ifeq ($(uname_S),Linux)\n \tifneq ($(findstring .el7.,$(uname_R)),)\n \t\tBASIC_CFLAGS += -std=c99\n \tendif\n+\tifeq ($(shell test -f /usr/include/linux/magic.h && echo y),y)\n+\t\tHAVE_LINUX_MAGIC_H = YesPlease\n+\tendif\n+\t# The builtin FSMonitor on Linux builds upon Simple-IPC. Both require\n+\t# Unix domain sockets and PThreads.\n+\tifndef NO_PTHREADS\n+\tifndef NO_UNIX_SOCKETS\n+\tFSMONITOR_DAEMON_BACKEND = linux\n+\tFSMONITOR_OS_SETTINGS = linux\n+\tendif\n+\tendif\n endif\n ifeq ($(uname_S),GNU/kFreeBSD)\n \tHAVE_ALLOCA_H = YesPlease\n-- \ngitgitgadget\n\n"},{"id":"488718","messageId":"5fad429b4d53dee4eb509f0db98ef860762436fc.1707992978.git.gitgitgadget@gmail.com","threadId":"60926","inReplyTo":"pull.1667.git.git.1707992978.gitgitgadget@gmail.com","subject":"[PATCH 3/7] fsmonitor: implement filesystem change listener for Linux","fromName":"Eric DeCosta via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-02-15T10:29:34Z","receivedAt":"2024-02-15T10:29:45Z","isPatch":true,"sender":{"key":"edecosta@mathworks.com","avatar":"https://avatars.githubusercontent.com/u/67609563?v=4"},"body":"From: Eric DeCosta \n\nImplement a filesystem change listener for Linux based on the inotify API:\nhttps://man7.org/linux/man-pages/man7/inotify.7.html\n\ninotify requires registering a watch on every directory in the worktree and\nspecial handling of moves/renames.\n\nSigned-off-by: Eric DeCosta \n---\n compat/fsmonitor/fsm-listen-linux.c | 676 ++++++++++++++++++++++++++++\n 1 file changed, 676 insertions(+)\n create mode 100644 compat/fsmonitor/fsm-listen-linux.c\n\ndiff --git a/compat/fsmonitor/fsm-listen-linux.c b/compat/fsmonitor/fsm-listen-linux.c\nnew file mode 100644\nindex 00000000000..e8548e4e009\n--- /dev/null\n+++ b/compat/fsmonitor/fsm-listen-linux.c\n@@ -0,0 +1,676 @@\n+#include \"cache.h\"\n+#include \"fsmonitor.h\"\n+#include \"fsm-listen.h\"\n+#include \"fsmonitor--daemon.h\"\n+#include \n+#include \n+#include \n+#include \n+\n+/*\n+ * Safe value to bitwise OR with rest of mask for\n+ * kernels that do not support IN_MASK_CREATE\n+ */\n+#ifndef IN_MASK_CREATE\n+#define IN_MASK_CREATE 0x00000000\n+#endif\n+\n+enum shutdown_reason {\n+\tSHUTDOWN_CONTINUE = 0,\n+\tSHUTDOWN_STOP,\n+\tSHUTDOWN_ERROR,\n+\tSHUTDOWN_FORCE\n+};\n+\n+struct watch_entry {\n+\tstruct hashmap_entry ent;\n+\tint wd;\n+\tuint32_t cookie;\n+\tconst char *dir;\n+};\n+\n+struct rename_entry {\n+\tstruct hashmap_entry ent;\n+\ttime_t whence;\n+\tuint32_t cookie;\n+\tconst char *dir;\n+};\n+\n+struct fsm_listen_data {\n+\tint fd_inotify;\n+\tenum shutdown_reason shutdown;\n+\tstruct hashmap watches;\n+\tstruct hashmap renames;\n+\tstruct hashmap revwatches;\n+};\n+\n+static int watch_entry_cmp(const void *cmp_data,\n+\t\t\t const struct hashmap_entry *eptr,\n+\t\t\t const struct hashmap_entry *entry_or_key,\n+\t\t\t const void *keydata)\n+{\n+\tconst struct watch_entry *e1, *e2;\n+\n+\te1 = container_of(eptr, const struct watch_entry, ent);\n+\te2 = container_of(eptr, const struct watch_entry, ent);\n+\treturn e1->wd != e2->wd;\n+}\n+\n+static int revwatches_entry_cmp(const void *cmp_data,\n+\t\t\t const struct hashmap_entry *eptr,\n+\t\t\t const struct hashmap_entry *entry_or_key,\n+\t\t\t const void *keydata)\n+{\n+\tconst struct watch_entry *e1, *e2;\n+\n+\te1 = container_of(eptr, const struct watch_entry, ent);\n+\te2 = container_of(eptr, const struct watch_entry, ent);\n+\treturn strcmp(e1->dir, e2->dir);\n+}\n+\n+static int rename_entry_cmp(const void *cmp_data,\n+\t\t\t const struct hashmap_entry *eptr,\n+\t\t\t const struct hashmap_entry *entry_or_key,\n+\t\t\t const void *keydata)\n+{\n+\tconst struct rename_entry *e1, *e2;\n+\n+\te1 = container_of(eptr, const struct rename_entry, ent);\n+\te2 = container_of(eptr, const struct rename_entry, ent);\n+\treturn e1->cookie != e2->cookie;\n+}\n+\n+/*\n+ * Register an inotify watch, add watch descriptor to path mapping\n+ * and the reverse mapping.\n+ */\n+static int add_watch(const char *path, struct fsm_listen_data *data)\n+{\n+\tconst char *interned = strintern(path);\n+\tstruct watch_entry *w1, *w2;\n+\n+\t/* add the inotify watch, don't allow watches to be modified */\n+\tint wd = inotify_add_watch(data->fd_inotify, interned,\n+\t\t\t\t(IN_ALL_EVENTS | IN_ONLYDIR | IN_MASK_CREATE)\n+\t\t\t\t^ IN_ACCESS ^ IN_CLOSE ^ IN_OPEN);\n+\tif (wd < 0)\n+\t\treturn error_errno(\"inotify_add_watch('%s') failed\", interned);\n+\n+\t/* add watch descriptor -> directory mapping */\n+\tCALLOC_ARRAY(w1, 1);\n+\tw1->wd = wd;\n+\tw1->dir = interned;\n+\thashmap_entry_init(&w1->ent, memhash(&w1->wd, sizeof(int)));\n+\thashmap_add(&data->watches, &w1->ent);\n+\n+\t/* add directory -> watch descriptor mapping */\n+\tCALLOC_ARRAY(w2, 1);\n+\tw2->wd = wd;\n+\tw2->dir = interned;\n+\thashmap_entry_init(&w2->ent, memhash(w2->dir, strlen(w2->dir)));\n+\thashmap_add(&data->revwatches, &w2->ent);\n+\n+\treturn 0;\n+}\n+\n+/*\n+ * Remove the inotify watch, the watch descriptor to path mapping\n+ * and the reverse mapping.\n+ */\n+static void remove_watch(struct watch_entry *w,\n+\tstruct fsm_listen_data *data)\n+{\n+\tstruct watch_entry k1, k2, *w1, *w2;\n+\n+\t/* remove watch, ignore error if kernel already did it */\n+\tif (inotify_rm_watch(data->fd_inotify, w->wd) && errno != EINVAL)\n+\t\terror_errno(\"inotify_rm_watch() failed\");\n+\n+\thashmap_entry_init(&k1.ent, memhash(&w->wd, sizeof(int)));\n+\tw1 = hashmap_remove_entry(&data->watches, &k1, ent, NULL);\n+\tif (!w1)\n+\t\tBUG(\"Double remove of watch for '%s'\", w->dir);\n+\n+\tif (w1->cookie)\n+\t\tBUG(\"Removing watch for '%s' which has a pending rename\", w1->dir);\n+\n+\thashmap_entry_init(&k2.ent, memhash(w->dir, strlen(w->dir)));\n+\tw2 = hashmap_remove_entry(&data->revwatches, &k2, ent, NULL);\n+\tif (!w2)\n+\t\tBUG(\"Double remove of reverse watch for '%s'\", w->dir);\n+\n+\t/* w1->dir and w2->dir are interned strings, we don't own them */\n+\tfree(w1);\n+\tfree(w2);\n+}\n+\n+/*\n+ * Check for stale directory renames.\n+ *\n+ * https://man7.org/linux/man-pages/man7/inotify.7.html\n+ *\n+ * Allow for some small timeout to account for the fact that insertion of the\n+ * IN_MOVED_FROM+IN_MOVED_TO event pair is not atomic, and the possibility that\n+ * there may not be any IN_MOVED_TO event.\n+ *\n+ * If the IN_MOVED_TO event is not received within the timeout then events have\n+ * been missed and the monitor is in an inconsistent state with respect to the\n+ * filesystem.\n+ */\n+static int check_stale_dir_renames(struct hashmap *renames, time_t max_age)\n+{\n+\tstruct rename_entry *re;\n+\tstruct hashmap_iter iter;\n+\n+\thashmap_for_each_entry(renames, &iter, re, ent) {\n+\t\tif (re->whence <= max_age)\n+\t\t\treturn -1;\n+\t}\n+\treturn 0;\n+}\n+\n+/*\n+ * Track pending renames.\n+ *\n+ * Tracking is done via a event cookie to watch descriptor mapping.\n+ *\n+ * A rename is not complete until matching a IN_MOVED_TO event is received\n+ * for a corresponding IN_MOVED_FROM event.\n+ */\n+static void add_dir_rename(uint32_t cookie, const char *path,\n+\tstruct fsm_listen_data *data)\n+{\n+\tstruct watch_entry k, *w;\n+\tstruct rename_entry *re;\n+\n+\t/* lookup the watch descriptor for the given path */\n+\thashmap_entry_init(&k.ent, memhash(path, strlen(path)));\n+\tw = hashmap_get_entry(&data->revwatches, &k, ent, NULL);\n+\tif (!w) /* should never happen */\n+\t\tBUG(\"No watch for '%s'\", path);\n+\tw->cookie = cookie;\n+\n+\t/* add the pending rename to match against later */\n+\tCALLOC_ARRAY(re, 1);\n+\tre->dir = w->dir;\n+\tre->cookie = w->cookie;\n+\tre->whence = time(NULL);\n+\thashmap_entry_init(&re->ent, memhash(&re->cookie, sizeof(uint32_t)));\n+\thashmap_add(&data->renames, &re->ent);\n+}\n+\n+/*\n+ * Handle directory renames\n+ *\n+ * Once a IN_MOVED_TO event is received, lookup the rename tracking information\n+ * via the event cookie and use this information to update the watch.\n+ */\n+static void rename_dir(uint32_t cookie, const char *path,\n+\tstruct fsm_listen_data *data)\n+{\n+\tstruct rename_entry rek, *re;\n+\tstruct watch_entry k, *w;\n+\n+\t/* lookup a pending rename to match */\n+\trek.cookie = cookie;\n+\thashmap_entry_init(&rek.ent, memhash(&rek.cookie, sizeof(uint32_t)));\n+\tre = hashmap_get_entry(&data->renames, &rek, ent, NULL);\n+\tif (re) {\n+\t\tk.dir = re->dir;\n+\t\thashmap_entry_init(&k.ent, memhash(k.dir, strlen(k.dir)));\n+\t\tw = hashmap_get_entry(&data->revwatches, &k, ent, NULL);\n+\t\tif (w) {\n+\t\t\tw->cookie = 0; /* rename handled */\n+\t\t\tremove_watch(w, data);\n+\t\t\tadd_watch(path, data);\n+\t\t} else {\n+\t\t\tBUG(\"No matching watch\");\n+\t\t}\n+\t} else {\n+\t\tBUG(\"No matching cookie\");\n+\t}\n+}\n+\n+/*\n+ * Recursively add watches to every directory under path\n+ */\n+static int register_inotify(const char *path,\n+\tstruct fsmonitor_daemon_state *state,\n+\tstruct fsmonitor_batch *batch)\n+{\n+\tDIR *dir;\n+\tconst char *rel;\n+\tstruct strbuf current = STRBUF_INIT;\n+\tstruct dirent *de;\n+\tstruct stat fs;\n+\tint ret = -1;\n+\n+\tdir = opendir(path);\n+\tif (!dir)\n+\t\treturn error_errno(\"opendir('%s') failed\", path);\n+\n+\twhile ((de = readdir_skip_dot_and_dotdot(dir)) != NULL) {\n+\t\tstrbuf_reset(¤t);\n+\t\tstrbuf_addf(¤t, \"%s/%s\", path, de->d_name);\n+\t\tif (lstat(current.buf, &fs)) {\n+\t\t\terror_errno(\"lstat('%s') failed\", current.buf);\n+\t\t\tgoto failed;\n+\t\t}\n+\n+\t\t/* recurse into directory */\n+\t\tif (S_ISDIR(fs.st_mode)) {\n+\t\t\tif (add_watch(current.buf, state->listen_data))\n+\t\t\t\tgoto failed;\n+\t\t\tif (register_inotify(current.buf, state, batch))\n+\t\t\t\tgoto failed;\n+\t\t} else if (batch) {\n+\t\t\trel = current.buf + state->path_worktree_watch.len + 1;\n+\t\t\ttrace_printf_key(&trace_fsmonitor, \"explicitly adding '%s'\", rel);\n+\t\t\tfsmonitor_batch__add_path(batch, rel);\n+\t\t}\n+\t}\n+\tret = 0;\n+\n+failed:\n+\tstrbuf_release(¤t);\n+\tif (closedir(dir) < 0)\n+\t\treturn error_errno(\"closedir('%s') failed\", path);\n+\treturn ret;\n+}\n+\n+static int em_rename_dir_from(u_int32_t mask)\n+{\n+\treturn ((mask & IN_ISDIR) && (mask & IN_MOVED_FROM));\n+}\n+\n+static int em_rename_dir_to(u_int32_t mask)\n+{\n+\treturn ((mask & IN_ISDIR) && (mask & IN_MOVED_TO));\n+}\n+\n+static int em_remove_watch(u_int32_t mask)\n+{\n+\treturn (mask & IN_DELETE_SELF);\n+}\n+\n+static int em_dir_renamed(u_int32_t mask)\n+{\n+\treturn ((mask & IN_ISDIR) && (mask & IN_MOVE));\n+}\n+\n+static int em_dir_created(u_int32_t mask)\n+{\n+\treturn ((mask & IN_ISDIR) && (mask & IN_CREATE));\n+}\n+\n+static int em_dir_deleted(uint32_t mask)\n+{\n+\treturn ((mask & IN_ISDIR) && (mask & IN_DELETE));\n+}\n+\n+static int em_force_shutdown(u_int32_t mask)\n+{\n+\treturn (mask & IN_UNMOUNT) || (mask & IN_Q_OVERFLOW);\n+}\n+\n+static int em_ignore(u_int32_t mask)\n+{\n+\treturn (mask & IN_IGNORED) || (mask & IN_MOVE_SELF);\n+}\n+\n+static void log_mask_set(const char *path, u_int32_t mask)\n+{\n+\tstruct strbuf msg = STRBUF_INIT;\n+\n+\tif (mask & IN_ACCESS)\n+\t\tstrbuf_addstr(&msg, \"IN_ACCESS|\");\n+\tif (mask & IN_MODIFY)\n+\t\tstrbuf_addstr(&msg, \"IN_MODIFY|\");\n+\tif (mask & IN_ATTRIB)\n+\t\tstrbuf_addstr(&msg, \"IN_ATTRIB|\");\n+\tif (mask & IN_CLOSE_WRITE)\n+\t\tstrbuf_addstr(&msg, \"IN_CLOSE_WRITE|\");\n+\tif (mask & IN_CLOSE_NOWRITE)\n+\t\tstrbuf_addstr(&msg, \"IN_CLOSE_NOWRITE|\");\n+\tif (mask & IN_OPEN)\n+\t\tstrbuf_addstr(&msg, \"IN_OPEN|\");\n+\tif (mask & IN_MOVED_FROM)\n+\t\tstrbuf_addstr(&msg, \"IN_MOVED_FROM|\");\n+\tif (mask & IN_MOVED_TO)\n+\t\tstrbuf_addstr(&msg, \"IN_MOVED_TO|\");\n+\tif (mask & IN_CREATE)\n+\t\tstrbuf_addstr(&msg, \"IN_CREATE|\");\n+\tif (mask & IN_DELETE)\n+\t\tstrbuf_addstr(&msg, \"IN_DELETE|\");\n+\tif (mask & IN_DELETE_SELF)\n+\t\tstrbuf_addstr(&msg, \"IN_DELETE_SELF|\");\n+\tif (mask & IN_MOVE_SELF)\n+\t\tstrbuf_addstr(&msg, \"IN_MOVE_SELF|\");\n+\tif (mask & IN_UNMOUNT)\n+\t\tstrbuf_addstr(&msg, \"IN_UNMOUNT|\");\n+\tif (mask & IN_Q_OVERFLOW)\n+\t\tstrbuf_addstr(&msg, \"IN_Q_OVERFLOW|\");\n+\tif (mask & IN_IGNORED)\n+\t\tstrbuf_addstr(&msg, \"IN_IGNORED|\");\n+\tif (mask & IN_ISDIR)\n+\t\tstrbuf_addstr(&msg, \"IN_ISDIR|\");\n+\n+\ttrace_printf_key(&trace_fsmonitor, \"inotify_event: '%s', mask=%#8.8x %s\",\n+\t\t\t\tpath, mask, msg.buf);\n+\n+\tstrbuf_release(&msg);\n+}\n+\n+int fsm_listen__ctor(struct fsmonitor_daemon_state *state)\n+{\n+\tint fd;\n+\tint ret = 0;\n+\tstruct fsm_listen_data *data;\n+\n+\tCALLOC_ARRAY(data, 1);\n+\tstate->listen_data = data;\n+\tstate->listen_error_code = -1;\n+\tdata->shutdown = SHUTDOWN_ERROR;\n+\n+\tfd = inotify_init1(O_NONBLOCK);\n+\tif (fd < 0)\n+\t\treturn error_errno(\"inotify_init1() failed\");\n+\n+\tdata->fd_inotify = fd;\n+\n+\thashmap_init(&data->watches, watch_entry_cmp, NULL, 0);\n+\thashmap_init(&data->renames, rename_entry_cmp, NULL, 0);\n+\thashmap_init(&data->revwatches, revwatches_entry_cmp, NULL, 0);\n+\n+\tif (add_watch(state->path_worktree_watch.buf, data))\n+\t\tret = -1;\n+\telse if (register_inotify(state->path_worktree_watch.buf, state, NULL))\n+\t\tret = -1;\n+\telse if (state->nr_paths_watching > 1) {\n+\t\tif (add_watch(state->path_gitdir_watch.buf, data))\n+\t\t\tret = -1;\n+\t\telse if (register_inotify(state->path_gitdir_watch.buf, state, NULL))\n+\t\t\tret = -1;\n+\t}\n+\n+\tif (!ret) {\n+\t\tstate->listen_error_code = 0;\n+\t\tdata->shutdown = SHUTDOWN_CONTINUE;\n+\t}\n+\n+\treturn ret;\n+}\n+\n+void fsm_listen__dtor(struct fsmonitor_daemon_state *state)\n+{\n+\tstruct fsm_listen_data *data;\n+\tstruct hashmap_iter iter;\n+\tstruct watch_entry *w;\n+\tint fd;\n+\n+\tif (!state || !state->listen_data)\n+\t\treturn;\n+\n+\tdata = state->listen_data;\n+\tfd = data->fd_inotify;\n+\n+\thashmap_for_each_entry(&data->watches, &iter, w, ent) {\n+\t\tw->cookie = 0; /* ignore any pending renames */\n+\t\tremove_watch(w, data);\n+\t}\n+\thashmap_clear(&data->watches);\n+\n+\thashmap_clear(&data->revwatches); /* remove_watch freed the entries */\n+\n+\thashmap_clear_and_free(&data->renames, struct rename_entry, ent);\n+\n+\tFREE_AND_NULL(state->listen_data);\n+\n+\tif (fd && (close(fd) < 0))\n+\t\terror_errno(_(\"closing inotify file descriptor failed\"));\n+}\n+\n+void fsm_listen__stop_async(struct fsmonitor_daemon_state *state)\n+{\n+\tif (!state->listen_data->shutdown)\n+\t\tstate->listen_data->shutdown = SHUTDOWN_STOP;\n+}\n+\n+/*\n+ * Process a single inotify event and queue for publication.\n+ */\n+static int process_event(const char *path,\n+\tconst struct inotify_event *event,\n+\tstruct fsmonitor_batch *batch,\n+\tstruct string_list *cookie_list,\n+\tstruct fsmonitor_daemon_state *state)\n+{\n+\tconst char *rel;\n+\tconst char *last_sep;\n+\n+\tswitch (fsmonitor_classify_path_absolute(state, path)) {\n+\t\tcase IS_INSIDE_DOT_GIT_WITH_COOKIE_PREFIX:\n+\t\tcase IS_INSIDE_GITDIR_WITH_COOKIE_PREFIX:\n+\t\t\t/* Use just the filename of the cookie file. */\n+\t\t\tlast_sep = find_last_dir_sep(path);\n+\t\t\tstring_list_append(cookie_list,\n+\t\t\t\t\tlast_sep ? last_sep + 1 : path);\n+\t\t\tbreak;\n+\t\tcase IS_INSIDE_DOT_GIT:\n+\t\tcase IS_INSIDE_GITDIR:\n+\t\t\tbreak;\n+\t\tcase IS_DOT_GIT:\n+\t\tcase IS_GITDIR:\n+\t\t\t/*\n+\t\t\t* If .git directory is deleted or renamed away,\n+\t\t\t* we have to quit.\n+\t\t\t*/\n+\t\t\tif (em_dir_deleted(event->mask)) {\n+\t\t\t\ttrace_printf_key(&trace_fsmonitor,\n+\t\t\t\t\t\t\"event: gitdir removed\");\n+\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n+\t\t\t\tgoto done;\n+\t\t\t}\n+\n+\t\t\tif (em_dir_renamed(event->mask)) {\n+\t\t\t\ttrace_printf_key(&trace_fsmonitor,\n+\t\t\t\t\t\t\"event: gitdir renamed\");\n+\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n+\t\t\t\tgoto done;\n+\t\t\t}\n+\t\t\tbreak;\n+\t\tcase IS_WORKDIR_PATH:\n+\t\t\t/* normal events in the working directory */\n+\t\t\tif (trace_pass_fl(&trace_fsmonitor))\n+\t\t\t\tlog_mask_set(path, event->mask);\n+\n+\t\t\trel = path + state->path_worktree_watch.len + 1;\n+\t\t\tfsmonitor_batch__add_path(batch, rel);\n+\n+\t\t\tif (em_dir_deleted(event->mask))\n+\t\t\t\tbreak;\n+\n+\t\t\t/* received IN_MOVE_FROM, add tracking for expected IN_MOVE_TO */\n+\t\t\tif (em_rename_dir_from(event->mask))\n+\t\t\t\tadd_dir_rename(event->cookie, path, state->listen_data);\n+\n+\t\t\t/* received IN_MOVE_TO, update watch to reflect new path */\n+\t\t\tif (em_rename_dir_to(event->mask)) {\n+\t\t\t\trename_dir(event->cookie, path, state->listen_data);\n+\t\t\t\tif (register_inotify(path, state, batch)) {\n+\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n+\t\t\t\t\tgoto done;\n+\t\t\t\t}\n+\t\t\t}\n+\n+\t\t\tif (em_dir_created(event->mask)) {\n+\t\t\t\tif (add_watch(path, state->listen_data)) {\n+\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n+\t\t\t\t\tgoto done;\n+\t\t\t\t}\n+\t\t\t\tif (register_inotify(path, state, batch)) {\n+\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n+\t\t\t\t\tgoto done;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tbreak;\n+\t\tcase IS_OUTSIDE_CONE:\n+\t\tdefault:\n+\t\t\ttrace_printf_key(&trace_fsmonitor,\n+\t\t\t\t\t\"ignoring '%s'\", path);\n+\t\t\tbreak;\n+\t}\n+\treturn 0;\n+done:\n+\treturn -1;\n+}\n+\n+/*\n+ * Read the inotify event stream and pre-process events before further\n+ * processing and eventual publishing.\n+ */\n+static void handle_events(struct fsmonitor_daemon_state *state)\n+{\n+\t /* See https://man7.org/linux/man-pages/man7/inotify.7.html */\n+\tchar buf[4096]\n+\t\t__attribute__ ((aligned(__alignof__(struct inotify_event))));\n+\n+\tstruct hashmap watches = state->listen_data->watches;\n+\tstruct fsmonitor_batch *batch = NULL;\n+\tstruct string_list cookie_list = STRING_LIST_INIT_DUP;\n+\tstruct watch_entry k, *w;\n+\tstruct strbuf path;\n+\tconst struct inotify_event *event;\n+\tint fd = state->listen_data->fd_inotify;\n+\tssize_t len;\n+\tchar *ptr, *p;\n+\n+\tstrbuf_init(&path, PATH_MAX);\n+\n+\tfor(;;) {\n+\t\tlen = read(fd, buf, sizeof(buf));\n+\t\tif (len == -1 && errno != EAGAIN) {\n+\t\t\terror_errno(_(\"reading inotify message stream failed\"));\n+\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n+\t\t\tgoto done;\n+\t\t}\n+\n+\t\t/* nothing to read */\n+\t\tif (len <= 0)\n+\t\t\tgoto done;\n+\n+\t\t/* Loop over all events in the buffer. */\n+\t\tfor (ptr = buf; ptr < buf + len;\n+\t\t\t ptr += sizeof(struct inotify_event) + event->len) {\n+\n+\t\t\tevent = (const struct inotify_event *) ptr;\n+\n+\t\t\tif (em_ignore(event->mask))\n+\t\t\t\tcontinue;\n+\n+\t\t\t/* File system was unmounted or event queue overflowed */\n+\t\t\tif (em_force_shutdown(event->mask)) {\n+\t\t\t\tif (trace_pass_fl(&trace_fsmonitor))\n+\t\t\t\t\tlog_mask_set(\"Forcing shutdown\", event->mask);\n+\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n+\t\t\t\tgoto done;\n+\t\t\t}\n+\n+\t\t\thashmap_entry_init(&k.ent, memhash(&event->wd, sizeof(int)));\n+\t\t\tk.wd = event->wd;\n+\n+\t\t\tw = hashmap_get_entry(&watches, &k, ent, NULL);\n+\t\t\tif (!w) /* should never happen */\n+\t\t\t\tBUG(\"No watch for '%s'\", event->name);\n+\n+\t\t\t/* directory watch was removed */\n+\t\t\tif (em_remove_watch(event->mask)) {\n+\t\t\t\tremove_watch(w, state->listen_data);\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\n+\t\t\tstrbuf_reset(&path);\n+\t\t\tstrbuf_add(&path, w->dir, strlen(w->dir));\n+\t\t\tstrbuf_addch(&path, '/');\n+\t\t\tstrbuf_addstr(&path, event->name);\n+\n+\t\t\tp = fsmonitor__resolve_alias(path.buf, &state->alias);\n+\t\t\tif (!p)\n+\t\t\t\tp = strbuf_detach(&path, NULL);\n+\n+\t\t\tif (!batch)\n+\t\t\t\tbatch = fsmonitor_batch__new();\n+\n+\t\t\tif (process_event(p, event, batch, &cookie_list, state)) {\n+\t\t\t\tfree(p);\n+\t\t\t\tgoto done;\n+\t\t\t}\n+\t\t\tfree(p);\n+\t\t}\n+\t\tstrbuf_reset(&path);\n+\t\tfsmonitor_publish(state, batch, &cookie_list);\n+\t\tstring_list_clear(&cookie_list, 0);\n+\t\tbatch = NULL;\n+\t}\n+done:\n+\tstrbuf_release(&path);\n+\tfsmonitor_batch__free_list(batch);\n+\tstring_list_clear(&cookie_list, 0);\n+}\n+\n+/*\n+ * Non-blocking read of the inotify events stream. The inotify fd is polled\n+ * frequently to help minimize the number of queue overflows.\n+ */\n+void fsm_listen__loop(struct fsmonitor_daemon_state *state)\n+{\n+\tint poll_num;\n+\tconst int interval = 1000;\n+\ttime_t checked = time(NULL);\n+\tstruct pollfd fds[1];\n+\tfds[0].fd = state->listen_data->fd_inotify;\n+\tfds[0].events = POLLIN;\n+\n+\tfor(;;) {\n+\t\tswitch (state->listen_data->shutdown) {\n+\t\t\tcase SHUTDOWN_CONTINUE:\n+\t\t\t\tpoll_num = poll(fds, 1, 1);\n+\t\t\t\tif (poll_num == -1) {\n+\t\t\t\t\tif (errno == EINTR)\n+\t\t\t\t\t\tcontinue;\n+\t\t\t\t\terror_errno(_(\"polling inotify message stream failed\"));\n+\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n+\t\t\t\t\tcontinue;\n+\t\t\t\t}\n+\n+\t\t\t\tif ((time(NULL) - checked) >= interval) {\n+\t\t\t\t\tchecked = time(NULL);\n+\t\t\t\t\tif (check_stale_dir_renames(&state->listen_data->renames,\n+\t\t\t\t\t\tchecked - interval)) {\n+\t\t\t\t\t\ttrace_printf_key(&trace_fsmonitor,\n+\t\t\t\t\t\t\t\"Missed IN_MOVED_TO events, forcing shutdown\");\n+\t\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n+\t\t\t\t\t\tcontinue;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\n+\t\t\t\tif (poll_num > 0 && (fds[0].revents & POLLIN))\n+\t\t\t\t\thandle_events(state);\n+\n+\t\t\t\tcontinue;\n+\t\t\tcase SHUTDOWN_ERROR:\n+\t\t\t\tstate->listen_error_code = -1;\n+\t\t\t\tipc_server_stop_async(state->ipc_server_data);\n+\t\t\t\tbreak;\n+\t\t\tcase SHUTDOWN_FORCE:\n+\t\t\t\tstate->listen_error_code = 0;\n+\t\t\t\tipc_server_stop_async(state->ipc_server_data);\n+\t\t\t\tbreak;\n+\t\t\tcase SHUTDOWN_STOP:\n+\t\t\tdefault:\n+\t\t\t\tstate->listen_error_code = 0;\n+\t\t\t\tbreak;\n+\t\t}\n+\t\treturn;\n+\t}\n+}\n-- \ngitgitgadget\n\n"},{"id":"488719","messageId":"8c10bf49b2537ea01035b21ffc1a011a7110d823.1707992978.git.gitgitgadget@gmail.com","threadId":"60926","inReplyTo":"pull.1667.git.git.1707992978.gitgitgadget@gmail.com","subject":"[PATCH 4/7] fsmonitor: enable fsmonitor for Linux","fromName":"Eric DeCosta via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-02-15T10:29:35Z","receivedAt":"2024-02-15T10:29:47Z","isPatch":true,"sender":{"key":"edecosta@mathworks.com","avatar":"https://avatars.githubusercontent.com/u/67609563?v=4"},"body":"From: Eric DeCosta \n\nUpdate build to enable fsmonitor for Linux.\n\nSigned-off-by: Eric DeCosta \n---\n contrib/buildsystems/CMakeLists.txt | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 804629c525b..61ec95c8507 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -304,7 +304,16 @@ else()\n endif()\n \n if(SUPPORTS_SIMPLE_IPC)\n-\tif(CMAKE_SYSTEM_NAME STREQUAL \"Windows\")\n+\tif(CMAKE_SYSTEM_NAME STREQUAL \"Linux\")\n+\t\tadd_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND)\n+\t\tlist(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-linux.c)\n+\t\tlist(APPEND compat_SOURCES compat/fsmonitor/fsm-health-linux.c)\n+\t\tlist(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-linux.c)\n+\t\tlist(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-linux.c)\n+\n+\t\tadd_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS)\n+\t\tlist(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-linux.c)\n+\telseif(CMAKE_SYSTEM_NAME STREQUAL \"Windows\")\n \t\tadd_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND)\n \t\tlist(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-win32.c)\n \t\tlist(APPEND compat_SOURCES compat/fsmonitor/fsm-health-win32.c)\n-- \ngitgitgadget\n\n"},{"id":"488720","messageId":"2550a7824832ab4f570b82ec452bd12e315ea7f5.1707992978.git.gitgitgadget@gmail.com","threadId":"60926","inReplyTo":"pull.1667.git.git.1707992978.gitgitgadget@gmail.com","subject":"[PATCH 5/7] fsmonitor: test updates","fromName":"Eric DeCosta via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-02-15T10:29:36Z","receivedAt":"2024-02-15T10:29:47Z","isPatch":true,"sender":{"key":"edecosta@mathworks.com","avatar":"https://avatars.githubusercontent.com/u/67609563?v=4"},"body":"From: Eric DeCosta \n\nt7527-builtin-fsmonitor was leaking fsmonitor--daemon processes in some\ncases.\n\nAccomodate slight difference in the number of events generated on Linux.\n\nOn lower-powered systems, spin a little to give the daemon time\nto respond to and log filesystem events.\n\nSigned-off-by: Eric DeCosta \n---\n t/t7527-builtin-fsmonitor.sh | 138 +++++++++++++++++++++++++++--------\n 1 file changed, 106 insertions(+), 32 deletions(-)\n\ndiff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh\nindex 363f9dc0e41..1d33e418015 100755\n--- a/t/t7527-builtin-fsmonitor.sh\n+++ b/t/t7527-builtin-fsmonitor.sh\n@@ -13,7 +13,7 @@ fi\n stop_daemon_delete_repo () {\n \tr=$1 &&\n \ttest_might_fail git -C $r fsmonitor--daemon stop &&\n-\trm -rf $1\n+\trm -rf $r\n }\n \n start_daemon () {\n@@ -72,6 +72,34 @@ start_daemon () {\n \t)\n }\n \n+IMPLICIT_TIMEOUT=5\n+\n+wait_for_update () {\n+\tfunc=$1 &&\n+\tfile=$2 &&\n+\tsz=$(wc -c < \"$file\") &&\n+\tlast=0 &&\n+\t$func &&\n+\tk=0 &&\n+\twhile test \"$k\" -lt $IMPLICIT_TIMEOUT\n+\tdo\n+\t\tnsz=$(wc -c < \"$file\")\n+\t\tif test \"$nsz\" -gt \"$sz\"\n+\t\tthen\n+\t\t\tif test \"$last\" -eq \"$nsz\"\n+\t\t\tthen\n+\t\t\t\tcat \"$file\" &&\n+\t\t\t\treturn 0\n+\t\t\tfi\n+\t\t\tlast=$nsz\n+\t\tfi\n+\t\tsleep 1\n+\t\tk=$(( $k + 1 ))\n+\tdone &&\n+\tcat \"$file\" &&\n+\treturn 0\n+}\n+\n # Is a Trace2 data event present with the given catetory and key?\n # We do not care what the value is.\n #\n@@ -137,7 +165,6 @@ test_expect_success 'implicit daemon start' '\n # machines (where it might take a moment to wake and reschedule the\n # daemon process) to avoid false alarms during test runs.)\n #\n-IMPLICIT_TIMEOUT=5\n \n verify_implicit_shutdown () {\n \tr=$1 &&\n@@ -373,6 +400,15 @@ create_files () {\n \techo 3 >dir2/new\n }\n \n+rename_directory () {\n+\tmv dirtorename dirrenamed\n+}\n+\n+rename_directory_file () {\n+\tmv dirtorename dirrenamed &&\n+\techo 1 > dirrenamed/new\n+}\n+\n rename_files () {\n \tmv rename renamed &&\n \tmv dir1/rename dir1/renamed &&\n@@ -427,10 +463,12 @@ test_expect_success 'edit some files' '\n \n \tstart_daemon --tf \"$PWD/.git/trace\" &&\n \n-\tedit_files &&\n+\twait_for_update edit_files \"$PWD/.git/trace\" &&\n \n \ttest-tool fsmonitor-client query --token 0 &&\n \n+\ttest_might_fail git fsmonitor--daemon stop &&\n+\n \tgrep \"^event: dir1/modified$\" .git/trace &&\n \tgrep \"^event: dir2/modified$\" .git/trace &&\n \tgrep \"^event: modified$\" .git/trace &&\n@@ -442,10 +480,12 @@ test_expect_success 'create some files' '\n \n \tstart_daemon --tf \"$PWD/.git/trace\" &&\n \n-\tcreate_files &&\n+\twait_for_update create_files \"$PWD/.git/trace\" &&\n \n \ttest-tool fsmonitor-client query --token 0 &&\n \n+\ttest_might_fail git fsmonitor--daemon stop &&\n+\n \tgrep \"^event: dir1/new$\" .git/trace &&\n \tgrep \"^event: dir2/new$\" .git/trace &&\n \tgrep \"^event: new$\" .git/trace\n@@ -456,10 +496,12 @@ test_expect_success 'delete some files' '\n \n \tstart_daemon --tf \"$PWD/.git/trace\" &&\n \n-\tdelete_files &&\n+\twait_for_update delete_files \"$PWD/.git/trace\" &&\n \n \ttest-tool fsmonitor-client query --token 0 &&\n \n+\ttest_might_fail git fsmonitor--daemon stop &&\n+\n \tgrep \"^event: dir1/delete$\" .git/trace &&\n \tgrep \"^event: dir2/delete$\" .git/trace &&\n \tgrep \"^event: delete$\" .git/trace\n@@ -470,10 +512,12 @@ test_expect_success 'rename some files' '\n \n \tstart_daemon --tf \"$PWD/.git/trace\" &&\n \n-\trename_files &&\n+\twait_for_update rename_files \"$PWD/.git/trace\" &&\n \n \ttest-tool fsmonitor-client query --token 0 &&\n \n+\ttest_might_fail git fsmonitor--daemon stop &&\n+\n \tgrep \"^event: dir1/rename$\" .git/trace &&\n \tgrep \"^event: dir2/rename$\" .git/trace &&\n \tgrep \"^event: rename$\" .git/trace &&\n@@ -487,23 +531,42 @@ test_expect_success 'rename directory' '\n \n \tstart_daemon --tf \"$PWD/.git/trace\" &&\n \n-\tmv dirtorename dirrenamed &&\n+\twait_for_update rename_directory \"$PWD/.git/trace\" &&\n \n \ttest-tool fsmonitor-client query --token 0 &&\n \n+\ttest_might_fail git fsmonitor--daemon stop &&\n+\n \tgrep \"^event: dirtorename/*$\" .git/trace &&\n \tgrep \"^event: dirrenamed/*$\" .git/trace\n '\n \n+test_expect_success 'rename directory file' '\n+\ttest_when_finished clean_up_repo_and_stop_daemon &&\n+\n+\tstart_daemon --tf \"$PWD/.git/trace\" &&\n+\n+\twait_for_update rename_directory_file \"$PWD/.git/trace\" &&\n+\n+\ttest-tool fsmonitor-client query --token 0 &&\n+\n+\ttest_might_fail git fsmonitor--daemon stop &&\n+\n+\tgrep \"^event: dirtorename/*$\" .git/trace &&\n+\tgrep \"^event: dirrenamed/*$\" .git/trace &&\n+\tgrep \"^event: dirrenamed/new$\" .git/trace\n+'\n test_expect_success 'file changes to directory' '\n \ttest_when_finished clean_up_repo_and_stop_daemon &&\n \n \tstart_daemon --tf \"$PWD/.git/trace\" &&\n \n-\tfile_to_directory &&\n+\twait_for_update file_to_directory \"$PWD/.git/trace\" &&\n \n \ttest-tool fsmonitor-client query --token 0 &&\n \n+\ttest_might_fail git fsmonitor--daemon stop &&\n+\n \tgrep \"^event: delete$\" .git/trace &&\n \tgrep \"^event: delete/new$\" .git/trace\n '\n@@ -513,10 +576,12 @@ test_expect_success 'directory changes to a file' '\n \n \tstart_daemon --tf \"$PWD/.git/trace\" &&\n \n-\tdirectory_to_file &&\n+\twait_for_update directory_to_file \"$PWD/.git/trace\" &&\n \n \ttest-tool fsmonitor-client query --token 0 &&\n \n+\ttest_might_fail git fsmonitor--daemon stop &&\n+\n \tgrep \"^event: dir1$\" .git/trace\n '\n \n@@ -561,7 +626,7 @@ test_expect_success 'flush cached data' '\n \ttest-tool -C test_flush fsmonitor-client query --token \"builtin:test_00000002:0\" >actual_2 &&\n \tnul_to_q actual_q2 &&\n \n-\tgrep \"^builtin:test_00000002:0Q$\" actual_q2 &&\n+\tgrep \"^builtin:test_00000002:[0-1]Q$\" actual_q2 &&\n \n \t>test_flush/file_3 &&\n \n@@ -732,7 +797,8 @@ u_values=\"$u1 $u2\"\n for u in $u_values\n do\n \ttest_expect_success \"unicode in repo root path: $u\" '\n-\t\ttest_when_finished \"stop_daemon_delete_repo $u\" &&\n+\t\ttest_when_finished \\\n+\t\t\"stop_daemon_delete_repo `echo \"$u\" | sed 's:x:\\\\\\\\\\\\\\\\\\\\\\\\\\\\x:g'`\" &&\n \n \t\tgit init \"$u\" &&\n \t\techo 1 >\"$u\"/file1 &&\n@@ -823,8 +889,7 @@ test_expect_success 'submodule setup' '\n '\n \n test_expect_success 'submodule always visited' '\n-\ttest_when_finished \"git -C super fsmonitor--daemon stop; \\\n-\t\t\t rm -rf super; \\\n+\ttest_when_finished \"rm -rf super; \\\n \t\t\t rm -rf sub\" &&\n \n \tcreate_super super &&\n@@ -871,10 +936,29 @@ test_expect_success 'submodule always visited' '\n # the submodule, and someone does a `git submodule absorbgitdirs`\n # in the super, Git will recursively invoke `git submodule--helper`\n # to do the work and this may try to read the index. This will\n-# try to start the daemon in the submodule.\n+# try to start the daemon in the submodule *and* pass (either\n+# directly or via inheritance) the `--super-prefix` arg to the\n+# `git fsmonitor--daemon start` command inside the submodule.\n+# This causes a warning because fsmonitor--daemon does take that\n+# global arg (see the table in git.c)\n+#\n+# This causes a warning when trying to start the daemon that is\n+# somewhat confusing. It does not seem to hurt anything because\n+# the fsmonitor code maps the query failure into a trivial response\n+# and does the work anyway.\n+#\n+# It would be nice to silence the warning, however.\n \n-test_expect_success \"submodule absorbgitdirs implicitly starts daemon\" '\n-\ttest_when_finished \"rm -rf super; \\\n+have_t2_error_event () {\n+\tlog=$1\n+\tmsg=\"fsmonitor--daemon doesnQt support --super-prefix\" &&\n+\n+\ttr '\\047' Q <$1 | grep -e \"$msg\"\n+}\n+\n+test_expect_success \"stray submodule super-prefix warning\" '\n+\ttest_when_finished \"git -C super/dir_1/dir_2/sub fsmonitor--daemon stop; \\\n+\t\t\t rm -rf super; \\\n \t\t\t rm -rf sub; \\\n \t\t\t rm super-sub.trace\" &&\n \n@@ -891,31 +975,21 @@ test_expect_success \"submodule absorbgitdirs implicitly starts daemon\" '\n \n \ttest_path_is_dir super/dir_1/dir_2/sub/.git &&\n \n-\tcwd=\"$(cd super && pwd)\" &&\n-\tcat >expect <<-EOF &&\n-\tMigrating git directory of '\\''dir_1/dir_2/sub'\\'' from\n-\t'\\''$cwd/dir_1/dir_2/sub/.git'\\'' to\n-\t'\\''$cwd/.git/modules/dir_1/dir_2/sub'\\''\n-\tEOF\n \tGIT_TRACE2_EVENT=\"$PWD/super-sub.trace\" \\\n-\t\tgit -C super submodule absorbgitdirs >out 2>actual &&\n-\ttest_cmp expect actual &&\n-\ttest_must_be_empty out &&\n+\t\tgit -C super submodule absorbgitdirs &&\n \n-\t# Confirm that the trace2 log contains a record of the\n-\t# daemon starting.\n-\ttest_subcommand git fsmonitor--daemon start \n\nUpdate the documentation for Linux.\n\nSigned-off-by: Eric DeCosta \n---\n Documentation/config/fsmonitor--daemon.txt | 4 ++--\n Documentation/git-fsmonitor--daemon.txt | 26 ++++++++++++++--------\n 2 files changed, 19 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/config/fsmonitor--daemon.txt b/Documentation/config/fsmonitor--daemon.txt\nindex 671f9b94628..4ca92536286 100644\n--- a/Documentation/config/fsmonitor--daemon.txt\n+++ b/Documentation/config/fsmonitor--daemon.txt\n@@ -4,8 +4,8 @@ fsmonitor.allowRemote::\n behavior. Only respected when `core.fsmonitor` is set to `true`.\n \n fsmonitor.socketDir::\n- This Mac OS-specific option, if set, specifies the directory in\n+ Mac OS and Linux-specific option. If set, specifies the directory in\n which to create the Unix domain socket used for communication\n between the fsmonitor daemon and various Git commands. The directory must\n- reside on a native Mac OS filesystem. Only respected when `core.fsmonitor`\n+ reside on a native filesystem. Only respected when `core.fsmonitor`\n is set to `true`.\ndiff --git a/Documentation/git-fsmonitor--daemon.txt b/Documentation/git-fsmonitor--daemon.txt\nindex 8585d19f4d8..ebdc4aec3c0 100644\n--- a/Documentation/git-fsmonitor--daemon.txt\n+++ b/Documentation/git-fsmonitor--daemon.txt\n@@ -76,23 +76,31 @@ repositories; this may be overridden by setting `fsmonitor.allowRemote` to\n correctly with all network-mounted repositories, so such use is considered\n experimental.\n \n-On Mac OS, the inter-process communication (IPC) between various Git\n+On Linux and Mac OS, the inter-process communication (IPC) between various Git\n commands and the fsmonitor daemon is done via a Unix domain socket (UDS) -- a\n-special type of file -- which is supported by native Mac OS filesystems,\n-but not on network-mounted filesystems, NTFS, or FAT32. Other filesystems\n-may or may not have the needed support; the fsmonitor daemon is not guaranteed\n-to work with these filesystems and such use is considered experimental.\n+special type of file -- which is supported by many native Linux and Mac OS\n+filesystems, but not on network-mounted filesystems, NTFS, or FAT32. Other\n+filesystems may or may not have the needed support; the fsmonitor daemon is not\n+guaranteed to work with these filesystems and such use is considered\n+experimental.\n \n By default, the socket is created in the `.git` directory. However, if the\n `.git` directory is on a network-mounted filesystem, it will instead be\n created at `$HOME/.git-fsmonitor-*` unless `$HOME` itself is on a\n-network-mounted filesystem, in which case you must set the configuration\n-variable `fsmonitor.socketDir` to the path of a directory on a Mac OS native\n+network-mounted filesystem in which case you must set the configuration\n+variable `fsmonitor.socketDir` to the path of a directory on a native\n filesystem in which to create the socket file.\n \n If none of the above directories (`.git`, `$HOME`, or `fsmonitor.socketDir`)\n-is on a native Mac OS file filesystem the fsmonitor daemon will report an\n-error that will cause the daemon and the currently running command to exit.\n+is on a native Linux or Mac OS filesystem the fsmonitor daemon will report\n+an error that will cause the daemon to exit and the currently running command\n+to issue a warning.\n+\n+On Linux, the fsmonitor daemon registers a watch for each directory in the\n+repository. The default per-user limit for the number of watches on most Linux\n+systems is 8192. This may not be sufficient for large repositories or if\n+multiple instances of the fsmonitor daemon are running.\n+See https://watchexec.github.io/docs/inotify-limits.html[Linux inotify limits] for more information.\n \n CONFIGURATION\n -------------\n-- \ngitgitgadget\n\n"},{"id":"488721","messageId":"74a4fa335a7c2014a35be8556887170169360b36.1707992978.git.gitgitgadget@gmail.com","threadId":"60926","inReplyTo":"pull.1667.git.git.1707992978.gitgitgadget@gmail.com","subject":"[PATCH 7/7] fsmonitor: addressed comments for patch 1352","fromName":"marzi.esipreh via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-02-15T10:29:38Z","receivedAt":"2024-02-15T10:29:49Z","isPatch":true,"sender":{"key":"name:marzi.esipreh","avatar":null},"body":"From: \"marzi.esipreh\" \n\naddressed comments on 1352, rebased, resolved conflicts\n\nSigned-off-by: Marzieh Esipreh \n---\n compat/fsmonitor/fsm-health-linux.c | 2 +-\n compat/fsmonitor/fsm-ipc-unix.c | 6 +-\n compat/fsmonitor/fsm-listen-linux.c | 170 ++++++++++++------------\n compat/fsmonitor/fsm-path-utils-linux.c | 1 +\n compat/fsmonitor/fsm-settings-unix.c | 3 +\n 5 files changed, 95 insertions(+), 87 deletions(-)\n\ndiff --git a/compat/fsmonitor/fsm-health-linux.c b/compat/fsmonitor/fsm-health-linux.c\nindex b9f709e8548..4c291f8a066 100644\n--- a/compat/fsmonitor/fsm-health-linux.c\n+++ b/compat/fsmonitor/fsm-health-linux.c\n@@ -1,4 +1,4 @@\n-#include \"cache.h\"\n+#include \"git-compat-util.h\"\n #include \"config.h\"\n #include \"fsmonitor.h\"\n #include \"fsm-health.h\"\ndiff --git a/compat/fsmonitor/fsm-ipc-unix.c b/compat/fsmonitor/fsm-ipc-unix.c\nindex eb25123fa12..70afddfd298 100644\n--- a/compat/fsmonitor/fsm-ipc-unix.c\n+++ b/compat/fsmonitor/fsm-ipc-unix.c\n@@ -1,10 +1,12 @@\n-#include \"cache.h\"\n+#include \"git-compat-util.h\"\n #include \"config.h\"\n #include \"hex.h\"\n #include \"strbuf.h\"\n #include \"fsmonitor.h\"\n #include \"fsmonitor-ipc.h\"\n #include \"fsmonitor-path-utils.h\"\n+#include \"gettext.h\"\n+#include \"path.h\"\n \n static GIT_PATH_FUNC(fsmonitor_ipc__get_default_path, \"fsmonitor--daemon.ipc\")\n \n@@ -17,7 +19,7 @@ const char *fsmonitor_ipc__get_path(struct repository *r)\n \tunsigned char hash[GIT_MAX_RAWSZ];\n \n \tif (!r)\n-\t\tBUG(\"No repository passed into fsmonitor_ipc__get_path\");\n+\t\tBUG(\"no repository passed into fsmonitor_ipc__get_path\");\n \n \tif (ipc_path)\n \t\treturn ipc_path;\ndiff --git a/compat/fsmonitor/fsm-listen-linux.c b/compat/fsmonitor/fsm-listen-linux.c\nindex e8548e4e009..84d8fb28d5d 100644\n--- a/compat/fsmonitor/fsm-listen-linux.c\n+++ b/compat/fsmonitor/fsm-listen-linux.c\n@@ -1,7 +1,10 @@\n-#include \"cache.h\"\n+#include \"git-compat-util.h\"\n+#include \"config.h\"\n #include \"fsmonitor.h\"\n #include \"fsm-listen.h\"\n #include \"fsmonitor--daemon.h\"\n+#include \"gettext.h\"\n+#include \"simple-ipc.h\"\n #include \n #include \n #include \n@@ -129,15 +132,15 @@ static void remove_watch(struct watch_entry *w,\n \thashmap_entry_init(&k1.ent, memhash(&w->wd, sizeof(int)));\n \tw1 = hashmap_remove_entry(&data->watches, &k1, ent, NULL);\n \tif (!w1)\n-\t\tBUG(\"Double remove of watch for '%s'\", w->dir);\n+\t\tBUG(\"double remove of watch for '%s'\", w->dir);\n \n \tif (w1->cookie)\n-\t\tBUG(\"Removing watch for '%s' which has a pending rename\", w1->dir);\n+\t\tBUG(\"removing watch for '%s' which has a pending rename\", w1->dir);\n \n \thashmap_entry_init(&k2.ent, memhash(w->dir, strlen(w->dir)));\n \tw2 = hashmap_remove_entry(&data->revwatches, &k2, ent, NULL);\n \tif (!w2)\n-\t\tBUG(\"Double remove of reverse watch for '%s'\", w->dir);\n+\t\tBUG(\"double remove of reverse watch for '%s'\", w->dir);\n \n \t/* w1->dir and w2->dir are interned strings, we don't own them */\n \tfree(w1);\n@@ -187,7 +190,7 @@ static void add_dir_rename(uint32_t cookie, const char *path,\n \thashmap_entry_init(&k.ent, memhash(path, strlen(path)));\n \tw = hashmap_get_entry(&data->revwatches, &k, ent, NULL);\n \tif (!w) /* should never happen */\n-\t\tBUG(\"No watch for '%s'\", path);\n+\t\tBUG(\"no watch for '%s'\", path);\n \tw->cookie = cookie;\n \n \t/* add the pending rename to match against later */\n@@ -224,10 +227,10 @@ static void rename_dir(uint32_t cookie, const char *path,\n \t\t\tremove_watch(w, data);\n \t\t\tadd_watch(path, data);\n \t\t} else {\n-\t\t\tBUG(\"No matching watch\");\n+\t\t\tBUG(\"no matching watch\");\n \t\t}\n \t} else {\n-\t\tBUG(\"No matching cookie\");\n+\t\tBUG(\"no matching cookie\");\n \t}\n }\n \n@@ -249,7 +252,7 @@ static int register_inotify(const char *path,\n \tif (!dir)\n \t\treturn error_errno(\"opendir('%s') failed\", path);\n \n-\twhile ((de = readdir_skip_dot_and_dotdot(dir)) != NULL) {\n+\twhile ((de = readdir_skip_dot_and_dotdot(dir))) {\n \t\tstrbuf_reset(¤t);\n \t\tstrbuf_addf(¤t, \"%s/%s\", path, de->d_name);\n \t\tif (lstat(current.buf, &fs)) {\n@@ -353,7 +356,7 @@ static void log_mask_set(const char *path, u_int32_t mask)\n \tif (mask & IN_IGNORED)\n \t\tstrbuf_addstr(&msg, \"IN_IGNORED|\");\n \tif (mask & IN_ISDIR)\n-\t\tstrbuf_addstr(&msg, \"IN_ISDIR|\");\n+\t\tstrbuf_addstr(&msg, \"IN_ISDIR\");\n \n \ttrace_printf_key(&trace_fsmonitor, \"inotify_event: '%s', mask=%#8.8x %s\",\n \t\t\t\tpath, mask, msg.buf);\n@@ -373,8 +376,10 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state)\n \tdata->shutdown = SHUTDOWN_ERROR;\n \n \tfd = inotify_init1(O_NONBLOCK);\n-\tif (fd < 0)\n+\tif (fd < 0) {\n+\t\tFREE_AND_NULL(data);\n \t\treturn error_errno(\"inotify_init1() failed\");\n+\t}\n \n \tdata->fd_inotify = fd;\n \n@@ -386,12 +391,10 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state)\n \t\tret = -1;\n \telse if (register_inotify(state->path_worktree_watch.buf, state, NULL))\n \t\tret = -1;\n-\telse if (state->nr_paths_watching > 1) {\n-\t\tif (add_watch(state->path_gitdir_watch.buf, data))\n-\t\t\tret = -1;\n-\t\telse if (register_inotify(state->path_gitdir_watch.buf, state, NULL))\n-\t\t\tret = -1;\n-\t}\n+\telse if (state->nr_paths_watching > 1 &&\n+\t\t (add_watch(state->path_gitdir_watch.buf, data) ||\n+\t\t register_inotify(state->path_gitdir_watch.buf, state, NULL)))\n+\t\tret = -1;\n \n \tif (!ret) {\n \t\tstate->listen_error_code = 0;\n@@ -449,80 +452,80 @@ static int process_event(const char *path,\n \tconst char *last_sep;\n \n \tswitch (fsmonitor_classify_path_absolute(state, path)) {\n-\t\tcase IS_INSIDE_DOT_GIT_WITH_COOKIE_PREFIX:\n-\t\tcase IS_INSIDE_GITDIR_WITH_COOKIE_PREFIX:\n-\t\t\t/* Use just the filename of the cookie file. */\n-\t\t\tlast_sep = find_last_dir_sep(path);\n-\t\t\tstring_list_append(cookie_list,\n-\t\t\t\t\tlast_sep ? last_sep + 1 : path);\n-\t\t\tbreak;\n-\t\tcase IS_INSIDE_DOT_GIT:\n-\t\tcase IS_INSIDE_GITDIR:\n-\t\t\tbreak;\n-\t\tcase IS_DOT_GIT:\n-\t\tcase IS_GITDIR:\n-\t\t\t/*\n-\t\t\t* If .git directory is deleted or renamed away,\n-\t\t\t* we have to quit.\n-\t\t\t*/\n-\t\t\tif (em_dir_deleted(event->mask)) {\n-\t\t\t\ttrace_printf_key(&trace_fsmonitor,\n-\t\t\t\t\t\t\"event: gitdir removed\");\n-\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n-\t\t\t\tgoto done;\n-\t\t\t}\n+\tcase IS_INSIDE_DOT_GIT_WITH_COOKIE_PREFIX:\n+\tcase IS_INSIDE_GITDIR_WITH_COOKIE_PREFIX:\n+\t\t/* Use just the filename of the cookie file. */\n+\t\tlast_sep = find_last_dir_sep(path);\n+\t\tstring_list_append(cookie_list,\n+\t\t\t\tlast_sep ? last_sep + 1 : path);\n+\t\tbreak;\n+\tcase IS_INSIDE_DOT_GIT:\n+\tcase IS_INSIDE_GITDIR:\n+\t\tbreak;\n+\tcase IS_DOT_GIT:\n+\tcase IS_GITDIR:\n+\t\t/*\n+\t\t* If .git directory is deleted or renamed away,\n+\t\t* we have to quit.\n+\t\t*/\n+\t\tif (em_dir_deleted(event->mask)) {\n+\t\t\ttrace_printf_key(&trace_fsmonitor,\n+\t\t\t\t\t\"event: gitdir removed\");\n+\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n+\t\t\tgoto done;\n+\t\t}\n \n-\t\t\tif (em_dir_renamed(event->mask)) {\n-\t\t\t\ttrace_printf_key(&trace_fsmonitor,\n-\t\t\t\t\t\t\"event: gitdir renamed\");\n-\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n-\t\t\t\tgoto done;\n-\t\t\t}\n-\t\t\tbreak;\n-\t\tcase IS_WORKDIR_PATH:\n-\t\t\t/* normal events in the working directory */\n-\t\t\tif (trace_pass_fl(&trace_fsmonitor))\n-\t\t\t\tlog_mask_set(path, event->mask);\n+\t\tif (em_dir_renamed(event->mask)) {\n+\t\t\ttrace_printf_key(&trace_fsmonitor,\n+\t\t\t\t\t\"event: gitdir renamed\");\n+\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n+\t\t\tgoto done;\n+\t\t}\n+\t\tbreak;\n+\tcase IS_WORKDIR_PATH:\n+\t\t/* normal events in the working directory */\n+\t\tif (trace_pass_fl(&trace_fsmonitor))\n+\t\t\tlog_mask_set(path, event->mask);\n \n-\t\t\trel = path + state->path_worktree_watch.len + 1;\n-\t\t\tfsmonitor_batch__add_path(batch, rel);\n+\t\trel = path + state->path_worktree_watch.len + 1;\n+\t\tfsmonitor_batch__add_path(batch, rel);\n \n-\t\t\tif (em_dir_deleted(event->mask))\n-\t\t\t\tbreak;\n+\t\tif (em_dir_deleted(event->mask))\n+\t\t\tbreak;\n \n-\t\t\t/* received IN_MOVE_FROM, add tracking for expected IN_MOVE_TO */\n-\t\t\tif (em_rename_dir_from(event->mask))\n-\t\t\t\tadd_dir_rename(event->cookie, path, state->listen_data);\n+\t\t/* received IN_MOVE_FROM, add tracking for expected IN_MOVE_TO */\n+\t\tif (em_rename_dir_from(event->mask))\n+\t\t\tadd_dir_rename(event->cookie, path, state->listen_data);\n \n-\t\t\t/* received IN_MOVE_TO, update watch to reflect new path */\n-\t\t\tif (em_rename_dir_to(event->mask)) {\n-\t\t\t\trename_dir(event->cookie, path, state->listen_data);\n-\t\t\t\tif (register_inotify(path, state, batch)) {\n-\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n-\t\t\t\t\tgoto done;\n-\t\t\t\t}\n+\t\t/* received IN_MOVE_TO, update watch to reflect new path */\n+\t\tif (em_rename_dir_to(event->mask)) {\n+\t\t\trename_dir(event->cookie, path, state->listen_data);\n+\t\t\tif (register_inotify(path, state, batch)) {\n+\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n+\t\t\t\tgoto done;\n \t\t\t}\n+\t\t}\n \n-\t\t\tif (em_dir_created(event->mask)) {\n-\t\t\t\tif (add_watch(path, state->listen_data)) {\n-\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n-\t\t\t\t\tgoto done;\n-\t\t\t\t}\n-\t\t\t\tif (register_inotify(path, state, batch)) {\n-\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n-\t\t\t\t\tgoto done;\n-\t\t\t\t}\n+\t\tif (em_dir_created(event->mask)) {\n+\t\t\tif (add_watch(path, state->listen_data)) {\n+\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n+\t\t\t\tgoto done;\n \t\t\t}\n-\t\t\tbreak;\n-\t\tcase IS_OUTSIDE_CONE:\n-\t\tdefault:\n-\t\t\ttrace_printf_key(&trace_fsmonitor,\n-\t\t\t\t\t\"ignoring '%s'\", path);\n-\t\t\tbreak;\n+\t\t\tif (register_inotify(path, state, batch)) {\n+\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n+\t\t\t\tgoto done;\n+\t\t\t}\n+\t\t}\n+\t\tbreak;\n+\tcase IS_OUTSIDE_CONE:\n+\tdefault:\n+\t\ttrace_printf_key(&trace_fsmonitor,\n+\t\t\t\t\"ignoring '%s'\", path);\n+\t\tbreak;\n \t}\n \treturn 0;\n-done:\n-\treturn -1;\n+\tdone:\n+\t\treturn -1;\n }\n \n /*\n@@ -531,7 +534,7 @@ static int process_event(const char *path,\n */\n static void handle_events(struct fsmonitor_daemon_state *state)\n {\n-\t /* See https://man7.org/linux/man-pages/man7/inotify.7.html */\n+\t/* See https://man7.org/linux/man-pages/man7/inotify.7.html */\n \tchar buf[4096]\n \t\t__attribute__ ((aligned(__alignof__(struct inotify_event))));\n \n@@ -539,13 +542,12 @@ static void handle_events(struct fsmonitor_daemon_state *state)\n \tstruct fsmonitor_batch *batch = NULL;\n \tstruct string_list cookie_list = STRING_LIST_INIT_DUP;\n \tstruct watch_entry k, *w;\n-\tstruct strbuf path;\n \tconst struct inotify_event *event;\n \tint fd = state->listen_data->fd_inotify;\n \tssize_t len;\n \tchar *ptr, *p;\n \n-\tstrbuf_init(&path, PATH_MAX);\n+\tstruct strbuf path = STRBUF_INIT;\n \n \tfor(;;) {\n \t\tlen = read(fd, buf, sizeof(buf));\n@@ -581,7 +583,7 @@ static void handle_events(struct fsmonitor_daemon_state *state)\n \n \t\t\tw = hashmap_get_entry(&watches, &k, ent, NULL);\n \t\t\tif (!w) /* should never happen */\n-\t\t\t\tBUG(\"No watch for '%s'\", event->name);\n+\t\t\t\tBUG(\"no watch for '%s'\", event->name);\n \n \t\t\t/* directory watch was removed */\n \t\t\tif (em_remove_watch(event->mask)) {\ndiff --git a/compat/fsmonitor/fsm-path-utils-linux.c b/compat/fsmonitor/fsm-path-utils-linux.c\nindex c21d1349532..0e3b33ffa48 100644\n--- a/compat/fsmonitor/fsm-path-utils-linux.c\n+++ b/compat/fsmonitor/fsm-path-utils-linux.c\n@@ -3,6 +3,7 @@\n #include \"fsmonitor.h\"\n #include \"fsmonitor-path-utils.h\"\n #include \"fsm-path-utils-linux.h\"\n+#include \"gettext.h\"\n #include \n #include \n #include \ndiff --git a/compat/fsmonitor/fsm-settings-unix.c b/compat/fsmonitor/fsm-settings-unix.c\nindex d16dca89416..c9b75aa44fe 100644\n--- a/compat/fsmonitor/fsm-settings-unix.c\n+++ b/compat/fsmonitor/fsm-settings-unix.c\n@@ -1,6 +1,9 @@\n+#include \"git-compat-util.h\"\n+#include \"config.h\"\n #include \"fsmonitor.h\"\n #include \"fsmonitor-ipc.h\"\n #include \"fsmonitor-path-utils.h\"\n+#include \n \n /*\n * For the builtin FSMonitor, we create the Unix domain socket for the\n-- \ngitgitgadget\n"},{"id":"488731","messageId":"3e3d733f-d7d7-4dee-9ee9-d52ea215b36f@gmail.com","threadId":"60926","inReplyTo":"d26de10866662a5bcd16d562cd1063dedd21cf02.1707992978.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/7] fsmonitor: determine if filesystem is local or remote","fromName":"Jean-Noël Avila","fromEmail":"avila.jn@gmail.com","sentAt":"2024-02-15T11:24:39Z","receivedAt":"2024-02-15T11:24:44Z","isPatch":true,"sender":{"key":"jn.avila@free.fr","avatar":"https://avatars.githubusercontent.com/u/156172?v=4"},"body":"Hello,\n\nLe 15/02/2024 à 11:29, Eric DeCosta via GitGitGadget a écrit :\n> From: Eric DeCosta \n> \n> Compare the given path to the mounted filesystems. Find the mount that is\n> the longest prefix of the path (if any) and determine if that mount is on a\n> local or remote filesystem.\n> \n> Signed-off-by: Eric DeCosta \n> ---\n> Makefile | 4 +\n> compat/fsmonitor/fsm-path-utils-linux.c | 195 ++++++++++++++++++++++++\n> compat/fsmonitor/fsm-path-utils-linux.h | 91 +++++++++++\n> config.mak.uname | 11 ++\n> 4 files changed, 301 insertions(+)\n> create mode 100644 compat/fsmonitor/fsm-path-utils-linux.c\n> create mode 100644 compat/fsmonitor/fsm-path-utils-linux.h\n> \n> diff --git a/Makefile b/Makefile\n> index 78e874099d9..0f36a0fd83a 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -2088,6 +2088,10 @@ ifdef HAVE_CLOCK_GETTIME\n> \tBASIC_CFLAGS += -DHAVE_CLOCK_GETTIME\n> endif\n> \n> +ifdef HAVE_LINUX_MAGIC_H\n> +\tBASIC_CFLAGS += -DHAVE_LINUX_MAGIC_H\n> +endif\n> +\n> ifdef HAVE_CLOCK_MONOTONIC\n> \tBASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC\n> endif\n> diff --git a/compat/fsmonitor/fsm-path-utils-linux.c b/compat/fsmonitor/fsm-path-utils-linux.c\n> new file mode 100644\n> index 00000000000..c21d1349532\n> --- /dev/null\n> +++ b/compat/fsmonitor/fsm-path-utils-linux.c\n> @@ -0,0 +1,195 @@\n> +#include \"git-compat-util.h\"\n> +#include \"abspath.h\"\n> +#include \"fsmonitor.h\"\n> +#include \"fsmonitor-path-utils.h\"\n> +#include \"fsm-path-utils-linux.h\"\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +static int is_remote_fs(const char *path)\n> +{\n> +\tstruct statfs fs;\n> +\n> +\tif (statfs(path, &fs))\n> +\t\treturn error_errno(_(\"statfs('%s') failed\"), path);\n\nFor the sake of simplifying of the work of translators, would it be wise \nto change this to\n\n+\tif (statfs(path, &fs))\n+\t\t/* TRANSLATORS: %s('%s') is a libc function call */\n+\t\treturn error_errno(_(\"%s('%s') failed\"), \"statfs\", +\t\t\t\tpath);\n\nand generalize this to all other messages?\n\nThanks,\n\nJN\n\n"},{"id":"488734","messageId":"Zc4WYrozXIJ41xtW@tanuki","threadId":"60926","inReplyTo":"5973bbe18aeecf486d8256cc402285665c45e66a.1707992978.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/7] fsmonitor: rebase with master","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-02-15T13:49:22Z","receivedAt":"2024-02-15T13:49:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Feb 15, 2024 at 10:29:32AM +0000, Eric DeCosta via GitGitGadget wrote:\n> From: Eric DeCosta \n> \n> rebased with master, and resolved conflicts\n\nIt would be a lot more useful if you adopted the original phrasing of\nthe commit:\n\n```\nfsmonitor: prepare to share code between Mac OS and Linux \n\nLinux and Mac OS can share some of the code originally developed for Mac OS.\n\nMac OS and Linux can share fsm-ipc-unix.c and fsm-settings-unix.c\n\nSigned-off-by: Eric DeCosta \n```\n\nDepending on whether or not you have made significant changes during the\nrebase I'd also convert the trailers to:\n\nPatch-originally-by: Eric DeCoste \nSigned-off-by: Marzieh Esipreh \n\nFurthermore, I think it would be useful if this commit was split up even\nfurther than it already is. Having a preparatory patch that moves around\nshareable code is a different topic than introducing the code skeleton\nfor Linux support, and as far as I can see the shared code does not end\nup requiring anything from the new \"*-linux.c\" files.\n\nPatrick\n\n> Signed-off-by: Eric DeCosta \n> ---\n> compat/fsmonitor/fsm-health-linux.c | 24 ++++++++++\n> compat/fsmonitor/fsm-ipc-darwin.c | 57 +----------------------\n> compat/fsmonitor/fsm-ipc-linux.c | 1 +\n> compat/fsmonitor/fsm-ipc-unix.c | 53 +++++++++++++++++++++\n> compat/fsmonitor/fsm-settings-darwin.c | 64 +-------------------------\n> compat/fsmonitor/fsm-settings-linux.c | 1 +\n> compat/fsmonitor/fsm-settings-unix.c | 61 ++++++++++++++++++++++++\n> 7 files changed, 142 insertions(+), 119 deletions(-)\n> create mode 100644 compat/fsmonitor/fsm-health-linux.c\n> create mode 100644 compat/fsmonitor/fsm-ipc-linux.c\n> create mode 100644 compat/fsmonitor/fsm-ipc-unix.c\n> create mode 100644 compat/fsmonitor/fsm-settings-linux.c\n> create mode 100644 compat/fsmonitor/fsm-settings-unix.c\n> \n> diff --git a/compat/fsmonitor/fsm-health-linux.c b/compat/fsmonitor/fsm-health-linux.c\n> new file mode 100644\n> index 00000000000..b9f709e8548\n> --- /dev/null\n> +++ b/compat/fsmonitor/fsm-health-linux.c\n> @@ -0,0 +1,24 @@\n> +#include \"cache.h\"\n> +#include \"config.h\"\n> +#include \"fsmonitor.h\"\n> +#include \"fsm-health.h\"\n> +#include \"fsmonitor--daemon.h\"\n> +\n> +int fsm_health__ctor(struct fsmonitor_daemon_state *state)\n> +{\n> +\treturn 0;\n> +}\n> +\n> +void fsm_health__dtor(struct fsmonitor_daemon_state *state)\n> +{\n> +\treturn;\n> +}\n> +\n> +void fsm_health__loop(struct fsmonitor_daemon_state *state)\n> +{\n> +\treturn;\n> +}\n> +\n> +void fsm_health__stop_async(struct fsmonitor_daemon_state *state)\n> +{\n> +}\n> diff --git a/compat/fsmonitor/fsm-ipc-darwin.c b/compat/fsmonitor/fsm-ipc-darwin.c\n> index 6f3a95410cc..4c3c92081ee 100644\n> --- a/compat/fsmonitor/fsm-ipc-darwin.c\n> +++ b/compat/fsmonitor/fsm-ipc-darwin.c\n> @@ -1,56 +1 @@\n> -#include \"git-compat-util.h\"\n> -#include \"config.h\"\n> -#include \"gettext.h\"\n> -#include \"hex.h\"\n> -#include \"path.h\"\n> -#include \"repository.h\"\n> -#include \"strbuf.h\"\n> -#include \"fsmonitor-ll.h\"\n> -#include \"fsmonitor-ipc.h\"\n> -#include \"fsmonitor-path-utils.h\"\n> -\n> -static GIT_PATH_FUNC(fsmonitor_ipc__get_default_path, \"fsmonitor--daemon.ipc\")\n> -\n> -const char *fsmonitor_ipc__get_path(struct repository *r)\n> -{\n> -\tstatic const char *ipc_path = NULL;\n> -\tgit_SHA_CTX sha1ctx;\n> -\tchar *sock_dir = NULL;\n> -\tstruct strbuf ipc_file = STRBUF_INIT;\n> -\tunsigned char hash[GIT_MAX_RAWSZ];\n> -\n> -\tif (!r)\n> -\t\tBUG(\"No repository passed into fsmonitor_ipc__get_path\");\n> -\n> -\tif (ipc_path)\n> -\t\treturn ipc_path;\n> -\n> -\n> -\t/* By default the socket file is created in the .git directory */\n> -\tif (fsmonitor__is_fs_remote(r->gitdir) < 1) {\n> -\t\tipc_path = fsmonitor_ipc__get_default_path();\n> -\t\treturn ipc_path;\n> -\t}\n> -\n> -\tgit_SHA1_Init(&sha1ctx);\n> -\tgit_SHA1_Update(&sha1ctx, r->worktree, strlen(r->worktree));\n> -\tgit_SHA1_Final(hash, &sha1ctx);\n> -\n> -\trepo_config_get_string(r, \"fsmonitor.socketdir\", &sock_dir);\n> -\n> -\t/* Create the socket file in either socketDir or $HOME */\n> -\tif (sock_dir && *sock_dir) {\n> -\t\tstrbuf_addf(&ipc_file, \"%s/.git-fsmonitor-%s\",\n> -\t\t\t\t\tsock_dir, hash_to_hex(hash));\n> -\t} else {\n> -\t\tstrbuf_addf(&ipc_file, \"~/.git-fsmonitor-%s\", hash_to_hex(hash));\n> -\t}\n> -\tfree(sock_dir);\n> -\n> -\tipc_path = interpolate_path(ipc_file.buf, 1);\n> -\tif (!ipc_path)\n> -\t\tdie(_(\"Invalid path: %s\"), ipc_file.buf);\n> -\n> -\tstrbuf_release(&ipc_file);\n> -\treturn ipc_path;\n> -}\n> +#include \"fsm-ipc-unix.c\"\n> diff --git a/compat/fsmonitor/fsm-ipc-linux.c b/compat/fsmonitor/fsm-ipc-linux.c\n> new file mode 100644\n> index 00000000000..4c3c92081ee\n> --- /dev/null\n> +++ b/compat/fsmonitor/fsm-ipc-linux.c\n> @@ -0,0 +1 @@\n> +#include \"fsm-ipc-unix.c\"\n> diff --git a/compat/fsmonitor/fsm-ipc-unix.c b/compat/fsmonitor/fsm-ipc-unix.c\n> new file mode 100644\n> index 00000000000..eb25123fa12\n> --- /dev/null\n> +++ b/compat/fsmonitor/fsm-ipc-unix.c\n> @@ -0,0 +1,53 @@\n> +#include \"cache.h\"\n> +#include \"config.h\"\n> +#include \"hex.h\"\n> +#include \"strbuf.h\"\n> +#include \"fsmonitor.h\"\n> +#include \"fsmonitor-ipc.h\"\n> +#include \"fsmonitor-path-utils.h\"\n> +\n> +static GIT_PATH_FUNC(fsmonitor_ipc__get_default_path, \"fsmonitor--daemon.ipc\")\n> +\n> +const char *fsmonitor_ipc__get_path(struct repository *r)\n> +{\n> +\tstatic const char *ipc_path = NULL;\n> +\tgit_SHA_CTX sha1ctx;\n> +\tchar *sock_dir = NULL;\n> +\tstruct strbuf ipc_file = STRBUF_INIT;\n> +\tunsigned char hash[GIT_MAX_RAWSZ];\n> +\n> +\tif (!r)\n> +\t\tBUG(\"No repository passed into fsmonitor_ipc__get_path\");\n> +\n> +\tif (ipc_path)\n> +\t\treturn ipc_path;\n> +\n> +\n> +\t/* By default the socket file is created in the .git directory */\n> +\tif (fsmonitor__is_fs_remote(r->gitdir) < 1) {\n> +\t\tipc_path = fsmonitor_ipc__get_default_path();\n> +\t\treturn ipc_path;\n> +\t}\n> +\n> +\tgit_SHA1_Init(&sha1ctx);\n> +\tgit_SHA1_Update(&sha1ctx, r->worktree, strlen(r->worktree));\n> +\tgit_SHA1_Final(hash, &sha1ctx);\n> +\n> +\trepo_config_get_string(r, \"fsmonitor.socketdir\", &sock_dir);\n> +\n> +\t/* Create the socket file in either socketDir or $HOME */\n> +\tif (sock_dir && *sock_dir) {\n> +\t\tstrbuf_addf(&ipc_file, \"%s/.git-fsmonitor-%s\",\n> +\t\t\t\t\tsock_dir, hash_to_hex(hash));\n> +\t} else {\n> +\t\tstrbuf_addf(&ipc_file, \"~/.git-fsmonitor-%s\", hash_to_hex(hash));\n> +\t}\n> +\tfree(sock_dir);\n> +\n> +\tipc_path = interpolate_path(ipc_file.buf, 1);\n> +\tif (!ipc_path)\n> +\t\tdie(_(\"Invalid path: %s\"), ipc_file.buf);\n> +\n> +\tstrbuf_release(&ipc_file);\n> +\treturn ipc_path;\n> +}\n> diff --git a/compat/fsmonitor/fsm-settings-darwin.c b/compat/fsmonitor/fsm-settings-darwin.c\n> index a3825906351..14baf9f0603 100644\n> --- a/compat/fsmonitor/fsm-settings-darwin.c\n> +++ b/compat/fsmonitor/fsm-settings-darwin.c\n> @@ -1,63 +1 @@\n> -#include \"git-compat-util.h\"\n> -#include \"config.h\"\n> -#include \"fsmonitor-ll.h\"\n> -#include \"fsmonitor-ipc.h\"\n> -#include \"fsmonitor-settings.h\"\n> -#include \"fsmonitor-path-utils.h\"\n> -\n> - /*\n> - * For the builtin FSMonitor, we create the Unix domain socket for the\n> - * IPC in the .git directory. If the working directory is remote,\n> - * then the socket will be created on the remote file system. This\n> - * can fail if the remote file system does not support UDS file types\n> - * (e.g. smbfs to a Windows server) or if the remote kernel does not\n> - * allow a non-local process to bind() the socket. (These problems\n> - * could be fixed by moving the UDS out of the .git directory and to a\n> - * well-known local directory on the client machine, but care should\n> - * be taken to ensure that $HOME is actually local and not a managed\n> - * file share.)\n> - *\n> - * FAT32 and NTFS working directories are problematic too.\n> - *\n> - * The builtin FSMonitor uses a Unix domain socket in the .git\n> - * directory for IPC. These Windows drive formats do not support\n> - * Unix domain sockets, so mark them as incompatible for the daemon.\n> - *\n> - */\n> -static enum fsmonitor_reason check_uds_volume(struct repository *r)\n> -{\n> -\tstruct fs_info fs;\n> -\tconst char *ipc_path = fsmonitor_ipc__get_path(r);\n> -\tstruct strbuf path = STRBUF_INIT;\n> -\tstrbuf_add(&path, ipc_path, strlen(ipc_path));\n> -\n> -\tif (fsmonitor__get_fs_info(dirname(path.buf), &fs) == -1) {\n> -\t\tstrbuf_release(&path);\n> -\t\treturn FSMONITOR_REASON_ERROR;\n> -\t}\n> -\n> -\tstrbuf_release(&path);\n> -\n> -\tif (fs.is_remote ||\n> -\t\t!strcmp(fs.typename, \"msdos\") ||\n> -\t\t!strcmp(fs.typename, \"ntfs\")) {\n> -\t\tfree(fs.typename);\n> -\t\treturn FSMONITOR_REASON_NOSOCKETS;\n> -\t}\n> -\n> -\tfree(fs.typename);\n> -\treturn FSMONITOR_REASON_OK;\n> -}\n> -\n> -enum fsmonitor_reason fsm_os__incompatible(struct repository *r, int ipc)\n> -{\n> -\tenum fsmonitor_reason reason;\n> -\n> -\tif (ipc) {\n> -\t\treason = check_uds_volume(r);\n> -\t\tif (reason != FSMONITOR_REASON_OK)\n> -\t\t\treturn reason;\n> -\t}\n> -\n> -\treturn FSMONITOR_REASON_OK;\n> -}\n> +#include \"fsm-settings-unix.c\"\n> diff --git a/compat/fsmonitor/fsm-settings-linux.c b/compat/fsmonitor/fsm-settings-linux.c\n> new file mode 100644\n> index 00000000000..14baf9f0603\n> --- /dev/null\n> +++ b/compat/fsmonitor/fsm-settings-linux.c\n> @@ -0,0 +1 @@\n> +#include \"fsm-settings-unix.c\"\n> diff --git a/compat/fsmonitor/fsm-settings-unix.c b/compat/fsmonitor/fsm-settings-unix.c\n> new file mode 100644\n> index 00000000000..d16dca89416\n> --- /dev/null\n> +++ b/compat/fsmonitor/fsm-settings-unix.c\n> @@ -0,0 +1,61 @@\n> +#include \"fsmonitor.h\"\n> +#include \"fsmonitor-ipc.h\"\n> +#include \"fsmonitor-path-utils.h\"\n> +\n> + /*\n> + * For the builtin FSMonitor, we create the Unix domain socket for the\n> + * IPC in the .git directory. If the working directory is remote,\n> + * then the socket will be created on the remote file system. This\n> + * can fail if the remote file system does not support UDS file types\n> + * (e.g. smbfs to a Windows server) or if the remote kernel does not\n> + * allow a non-local process to bind() the socket. (These problems\n> + * could be fixed by moving the UDS out of the .git directory and to a\n> + * well-known local directory on the client machine, but care should\n> + * be taken to ensure that $HOME is actually local and not a managed\n> + * file share.)\n> + *\n> + * FAT32 and NTFS working directories are problematic too.\n> + *\n> + * The builtin FSMonitor uses a Unix domain socket in the .git\n> + * directory for IPC. These Windows drive formats do not support\n> + * Unix domain sockets, so mark them as incompatible for the daemon.\n> + *\n> + */\n> +static enum fsmonitor_reason check_uds_volume(struct repository *r)\n> +{\n> +\tstruct fs_info fs;\n> +\tconst char *ipc_path = fsmonitor_ipc__get_path(r);\n> +\tstruct strbuf path = STRBUF_INIT;\n> +\tstrbuf_addstr(&path, ipc_path);\n> +\n> +\tif (fsmonitor__get_fs_info(dirname(path.buf), &fs) == -1) {\n> +\t\tfree(fs.typename);\n> +\t\tstrbuf_release(&path);\n> +\t\treturn FSMONITOR_REASON_ERROR;\n> +\t}\n> +\n> +\tstrbuf_release(&path);\n> +\n> +\tif (fs.is_remote ||\n> +\t\t!strcmp(fs.typename, \"msdos\") ||\n> +\t\t!strcmp(fs.typename, \"ntfs\")) {\n> +\t\tfree(fs.typename);\n> +\t\treturn FSMONITOR_REASON_NOSOCKETS;\n> +\t}\n> +\n> +\tfree(fs.typename);\n> +\treturn FSMONITOR_REASON_OK;\n> +}\n> +\n> +enum fsmonitor_reason fsm_os__incompatible(struct repository *r, int ipc)\n> +{\n> +\tenum fsmonitor_reason reason;\n> +\n> +\tif (ipc) {\n> +\t\treason = check_uds_volume(r);\n> +\t\tif (reason != FSMONITOR_REASON_OK)\n> +\t\t\treturn reason;\n> +\t}\n> +\n> +\treturn FSMONITOR_REASON_OK;\n> +}\n> -- \n> gitgitgadget\n> \n> \n"},{"id":"488735","messageId":"Zc4WZ2EkrVrHzs43@tanuki","threadId":"60926","inReplyTo":"d26de10866662a5bcd16d562cd1063dedd21cf02.1707992978.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/7] fsmonitor: determine if filesystem is local or remote","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-02-15T13:49:27Z","receivedAt":"2024-02-15T13:49:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Feb 15, 2024 at 10:29:33AM +0000, Eric DeCosta via GitGitGadget wrote:\n> From: Eric DeCosta \n> \n> Compare the given path to the mounted filesystems. Find the mount that is\n> the longest prefix of the path (if any) and determine if that mount is on a\n> local or remote filesystem.\n\nIt would be nice to motivate this change in the commit message. Right\nnow it only explains what the commit does, but it does not mention at\nall why that would be a good idea in the first place. Explaining that\nthis is part of the interface that the existing fsmonitor infrastructure\nexpects to exist would help.\n\n> Signed-off-by: Eric DeCosta \n> ---\n> Makefile | 4 +\n> compat/fsmonitor/fsm-path-utils-linux.c | 195 ++++++++++++++++++++++++\n> compat/fsmonitor/fsm-path-utils-linux.h | 91 +++++++++++\n> config.mak.uname | 11 ++\n> 4 files changed, 301 insertions(+)\n> create mode 100644 compat/fsmonitor/fsm-path-utils-linux.c\n> create mode 100644 compat/fsmonitor/fsm-path-utils-linux.h\n> \n> diff --git a/Makefile b/Makefile\n> index 78e874099d9..0f36a0fd83a 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -2088,6 +2088,10 @@ ifdef HAVE_CLOCK_GETTIME\n> \tBASIC_CFLAGS += -DHAVE_CLOCK_GETTIME\n> endif\n> \n> +ifdef HAVE_LINUX_MAGIC_H\n> +\tBASIC_CFLAGS += -DHAVE_LINUX_MAGIC_H\n> +endif\n> +\n> ifdef HAVE_CLOCK_MONOTONIC\n> \tBASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC\n> endif\n> diff --git a/compat/fsmonitor/fsm-path-utils-linux.c b/compat/fsmonitor/fsm-path-utils-linux.c\n> new file mode 100644\n> index 00000000000..c21d1349532\n> --- /dev/null\n> +++ b/compat/fsmonitor/fsm-path-utils-linux.c\n> @@ -0,0 +1,195 @@\n> +#include \"git-compat-util.h\"\n> +#include \"abspath.h\"\n> +#include \"fsmonitor.h\"\n> +#include \"fsmonitor-path-utils.h\"\n> +#include \"fsm-path-utils-linux.h\"\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +static int is_remote_fs(const char *path)\n> +{\n> +\tstruct statfs fs;\n> +\n> +\tif (statfs(path, &fs))\n> +\t\treturn error_errno(_(\"statfs('%s') failed\"), path);\n> +\n> +\tswitch (fs.f_type) {\n> +\tcase ACFS_SUPER_MAGIC:\n> +\tcase AFS_SUPER_MAGIC:\n> +\tcase CEPH_SUPER_MAGIC:\n> +\tcase CIFS_SUPER_MAGIC:\n> +\tcase CODA_SUPER_MAGIC:\n> +\tcase FHGFS_SUPER_MAGIC:\n> +\tcase GFS_SUPER_MAGIC:\n> +\tcase GPFS_SUPER_MAGIC:\n> +\tcase IBRIX_SUPER_MAGIC:\n> +\tcase KAFS_SUPER_MAGIC:\n> +\tcase LUSTRE_SUPER_MAGIC:\n> +\tcase NCP_SUPER_MAGIC:\n> +\tcase NFS_SUPER_MAGIC:\n> +\tcase NFSD_SUPER_MAGIC:\n> +\tcase OCFS2_SUPER_MAGIC:\n> +\tcase PANFS_SUPER_MAGIC:\n> +\tcase SMB_SUPER_MAGIC:\n> +\tcase SMB2_SUPER_MAGIC:\n> +\tcase SNFS_SUPER_MAGIC:\n> +\tcase VMHGFS_SUPER_MAGIC:\n> +\tcase VXFS_SUPER_MAGIC:\n> +\t\treturn 1;\n> +\tdefault:\n> +\t\treturn 0;\n> +\t}\n> +}\n\nThis list doesn't feel all that maintainable to me, but so be it if\nthere is no better interface available.\n\n> +static int find_mount(const char *path, const struct statvfs *fs,\n> +\t\t\tstruct mntent *entry)\n\nI don't quite understand why `find_mount()` is required in the first\nplace. Why can't we statfs(2) the path directly? This syscall provides\nthe fsid and should be sufficient for us to fill in `struct fs_info`.\n\nExplaining details like this in the commit message would help guide the\nreader's expectations.\n\nPatrick\n\n> +{\n> +\tconst char *const mounts = \"/proc/mounts\";\n> +\tchar *rp = real_pathdup(path, 1);\n> +\tstruct mntent *ment = NULL;\n> +\tstruct statvfs mntfs;\n> +\tFILE *fp;\n> +\tint found = 0;\n> +\tint ret = 0;\n> +\tsize_t dlen, plen, flen = 0;\n> +\n> +\tentry->mnt_fsname = NULL;\n> +\tentry->mnt_dir = NULL;\n> +\tentry->mnt_type = NULL;\n> +\n> +\tfp = setmntent(mounts, \"r\");\n> \n> +\tif (!fp) {\n> +\t\tfree(rp);\n> +\t\treturn error_errno(_(\"setmntent('%s') failed\"), mounts);\n> +\t}\n> +\n> +\tplen = strlen(rp);\n> +\n> +\t/* read all the mount information and compare to path */\n> +\twhile ((ment = getmntent(fp))) {\n> +\t\tif (statvfs(ment->mnt_dir, &mntfs)) {\n> +\t\t\tswitch (errno) {\n> +\t\t\tcase EPERM:\n> +\t\t\tcase ESRCH:\n> +\t\t\tcase EACCES:\n> +\t\t\t\tcontinue;\n> +\t\t\tdefault:\n> +\t\t\t\terror_errno(_(\"statvfs('%s') failed\"), ment->mnt_dir);\n> +\t\t\t\tret = -1;\n> +\t\t\t\tgoto done;\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\t/* is mount on the same filesystem and is a prefix of the path */\n> +\t\tif ((fs->f_fsid == mntfs.f_fsid) &&\n> +\t\t\t!strncmp(ment->mnt_dir, rp, strlen(ment->mnt_dir))) {\n> +\t\t\tdlen = strlen(ment->mnt_dir);\n> +\t\t\tif (dlen > plen)\n> +\t\t\t\tcontinue;\n> +\t\t\t/*\n> +\t\t\t * look for the longest prefix (including root)\n> +\t\t\t */\n> +\t\t\tif (dlen > flen &&\n> +\t\t\t\t((dlen == 1 && ment->mnt_dir[0] == '/') ||\n> +\t\t\t\t (!rp[dlen] || rp[dlen] == '/'))) {\n> +\t\t\t\tflen = dlen;\n> +\t\t\t\tfound = 1;\n> +\n> +\t\t\t\t/*\n> +\t\t\t\t * https://man7.org/linux/man-pages/man3/getmntent.3.html\n> +\t\t\t\t *\n> +\t\t\t\t * The pointer points to a static area of memory which is\n> +\t\t\t\t * overwritten by subsequent calls to getmntent().\n> +\t\t\t\t */\n> +\t\t\t\tfree(entry->mnt_fsname);\n> +\t\t\t\tfree(entry->mnt_dir);\n> +\t\t\t\tfree(entry->mnt_type);\n> +\t\t\t\tentry->mnt_fsname = xstrdup(ment->mnt_fsname);\n> +\t\t\t\tentry->mnt_dir = xstrdup(ment->mnt_dir);\n> +\t\t\t\tentry->mnt_type = xstrdup(ment->mnt_type);\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\n> +done:\n> +\tfree(rp);\n> +\tendmntent(fp);\n> +\n> +\tif (!found)\n> +\t\treturn -1;\n> +\n> +\treturn ret;\n> +}\n> +\n> +int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info)\n> +{\n> +\tint ret = 0;\n> +\tstruct mntent entry;\n> +\tstruct statvfs fs;\n> +\n> +\tfs_info->is_remote = -1;\n> +\tfs_info->typename = NULL;\n> +\n> +\tif (statvfs(path, &fs))\n> +\t\treturn error_errno(_(\"statvfs('%s') failed\"), path);\n> +\n> +\tif (find_mount(path, &fs, &entry) < 0) {\n> +\t\tret = -1;\n> +\t\tgoto done;\n> +\t}\n> +\n> +\ttrace_printf_key(&trace_fsmonitor,\n> +\t\t\t \"statvfs('%s') [flags 0x%08lx] '%s' '%s'\",\n> +\t\t\t path, fs.f_flag, entry.mnt_type, entry.mnt_fsname);\n> +\n> +\tfs_info->is_remote = is_remote_fs(entry.mnt_dir);\n> +\tfs_info->typename = xstrdup(entry.mnt_fsname);\n> +\n> +\tif (fs_info->is_remote < 0)\n> +\t\tret = -1;\n> +\n> +\ttrace_printf_key(&trace_fsmonitor,\n> +\t\t\t\t\"'%s' is_remote: %d\",\n> +\t\t\t\tpath, fs_info->is_remote);\n> +\n> +done:\n> +\tfree(entry.mnt_fsname);\n> +\tfree(entry.mnt_dir);\n> +\tfree(entry.mnt_type);\n> +\treturn ret;\n> +}\n> +\n> +int fsmonitor__is_fs_remote(const char *path)\n> +{\n> +\tint ret = 0;\n> +\tstruct fs_info fs;\n> +\n> +\tif (fsmonitor__get_fs_info(path, &fs))\n> +\t\tret = -1;\n> +\telse\n> +\t\tret = fs.is_remote;\n> +\n> +\tfree(fs.typename);\n> +\n> +\treturn ret;\n> +}\n> +\n> +/*\n> + * No-op for now.\n> + */\n> +int fsmonitor__get_alias(const char *path, struct alias_info *info)\n> +{\n> +\treturn 0;\n> +}\n> +\n> +/*\n> + * No-op for now.\n> + */\n> +char *fsmonitor__resolve_alias(const char *path,\n> +\t\tconst struct alias_info *info)\n> +{\n> +\treturn NULL;\n> +}\n> diff --git a/compat/fsmonitor/fsm-path-utils-linux.h b/compat/fsmonitor/fsm-path-utils-linux.h\n> new file mode 100644\n> index 00000000000..49bdb3c4728\n> --- /dev/null\n> +++ b/compat/fsmonitor/fsm-path-utils-linux.h\n> @@ -0,0 +1,91 @@\n> +#ifndef FSM_PATH_UTILS_LINUX_H\n> +#define FSM_PATH_UTILS_LINUX_H\n> +#endif\n> +\n> +#ifdef HAVE_LINUX_MAGIC_H\n> +#include \n> +#endif\n> +\n> +#ifndef ACFS_SUPER_MAGIC\n> +#define ACFS_SUPER_MAGIC 0x61636673\n> +#endif\n> +\n> +#ifndef AFS_SUPER_MAGIC\n> +#define AFS_SUPER_MAGIC 0x5346414f\n> +#endif\n> +\n> +#ifndef CEPH_SUPER_MAGIC\n> +#define CEPH_SUPER_MAGIC 0x00c36400\n> +#endif\n> +\n> +#ifndef CIFS_SUPER_MAGIC\n> +#define CIFS_SUPER_MAGIC 0xff534d42\n> +#endif\n> +\n> +#ifndef CODA_SUPER_MAGIC\n> +#define CODA_SUPER_MAGIC 0x73757245\n> +#endif\n> +\n> +#ifndef FHGFS_SUPER_MAGIC\n> +#define FHGFS_SUPER_MAGIC 0x19830326\n> +#endif\n> +\n> +#ifndef GFS_SUPER_MAGIC\n> +#define GFS_SUPER_MAGIC 0x1161970\n> +#endif\n> +\n> +#ifndef GPFS_SUPER_MAGIC\n> +#define GPFS_SUPER_MAGIC 0x47504653\n> +#endif\n> +\n> +#ifndef IBRIX_SUPER_MAGIC\n> +#define IBRIX_SUPER_MAGIC 0x013111a8\n> +#endif\n> +\n> +#ifndef KAFS_SUPER_MAGIC\n> +#define KAFS_SUPER_MAGIC 0x6b414653\n> +#endif\n> +\n> +#ifndef LUSTRE_SUPER_MAGIC\n> +#define LUSTRE_SUPER_MAGIC 0x0bd00bd0\n> +#endif\n> +\n> +#ifndef NCP_SUPER_MAGIC\n> +#define NCP_SUPER_MAGIC 0x564c\n> +#endif\n> +\n> +#ifndef NFS_SUPER_MAGIC\n> +#define NFS_SUPER_MAGIC 0x6969\n> +#endif\n> +\n> +#ifndef NFSD_SUPER_MAGIC\n> +#define NFSD_SUPER_MAGIC 0x6e667364\n> +#endif\n> +\n> +#ifndef OCFS2_SUPER_MAGIC\n> +#define OCFS2_SUPER_MAGIC 0x7461636f\n> +#endif\n> +\n> +#ifndef PANFS_SUPER_MAGIC\n> +#define PANFS_SUPER_MAGIC 0xaad7aaea\n> +#endif\n> +\n> +#ifndef SMB_SUPER_MAGIC\n> +#define SMB_SUPER_MAGIC 0x517b\n> +#endif\n> +\n> +#ifndef SMB2_SUPER_MAGIC\n> +#define SMB2_SUPER_MAGIC 0xfe534d42\n> +#endif\n> +\n> +#ifndef SNFS_SUPER_MAGIC\n> +#define SNFS_SUPER_MAGIC 0xbeefdead\n> +#endif\n> +\n> +#ifndef VMHGFS_SUPER_MAGIC\n> +#define VMHGFS_SUPER_MAGIC 0xbacbacbc\n> +#endif\n> +\n> +#ifndef VXFS_SUPER_MAGIC\n> +#define VXFS_SUPER_MAGIC 0xa501fcf5\n> +#endif\n> diff --git a/config.mak.uname b/config.mak.uname\n> index dacc95172dc..80d7e2a2e68 100644\n> --- a/config.mak.uname\n> +++ b/config.mak.uname\n> @@ -68,6 +68,17 @@ ifeq ($(uname_S),Linux)\n> \tifneq ($(findstring .el7.,$(uname_R)),)\n> \t\tBASIC_CFLAGS += -std=c99\n> \tendif\n> +\tifeq ($(shell test -f /usr/include/linux/magic.h && echo y),y)\n> +\t\tHAVE_LINUX_MAGIC_H = YesPlease\n> +\tendif\n> +\t# The builtin FSMonitor on Linux builds upon Simple-IPC. Both require\n> +\t# Unix domain sockets and PThreads.\n> +\tifndef NO_PTHREADS\n> +\tifndef NO_UNIX_SOCKETS\n> +\tFSMONITOR_DAEMON_BACKEND = linux\n> +\tFSMONITOR_OS_SETTINGS = linux\n> +\tendif\n> +\tendif\n> endif\n> ifeq ($(uname_S),GNU/kFreeBSD)\n> \tHAVE_ALLOCA_H = YesPlease\n> -- \n> gitgitgadget\n> \n> \n"},{"id":"488736","messageId":"Zc4Wbc1EIbVRCkad@tanuki","threadId":"60926","inReplyTo":"5fad429b4d53dee4eb509f0db98ef860762436fc.1707992978.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/7] fsmonitor: implement filesystem change listener for Linux","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-02-15T13:49:33Z","receivedAt":"2024-02-15T13:49:38Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Feb 15, 2024 at 10:29:34AM +0000, Eric DeCosta via GitGitGadget wrote:\n> From: Eric DeCosta \n> \n> Implement a filesystem change listener for Linux based on the inotify API:\n> https://man7.org/linux/man-pages/man7/inotify.7.html\n> \n> inotify requires registering a watch on every directory in the worktree and\n> special handling of moves/renames.\n\nI assume that fsmonitor is especially important in the context of repos\nwith large trees, and to the best of my knowledge inotify(7) does not\nscale well when installing many watches. I thus have to wonder whether\nfanotify(7) would be a better match to implement this nowadays, and what\nthe considerations were to pick one over the other.\n\n> Signed-off-by: Eric DeCosta \n> ---\n> compat/fsmonitor/fsm-listen-linux.c | 676 ++++++++++++++++++++++++++++\n> 1 file changed, 676 insertions(+)\n> create mode 100644 compat/fsmonitor/fsm-listen-linux.c\n> \n> diff --git a/compat/fsmonitor/fsm-listen-linux.c b/compat/fsmonitor/fsm-listen-linux.c\n> new file mode 100644\n> index 00000000000..e8548e4e009\n> --- /dev/null\n> +++ b/compat/fsmonitor/fsm-listen-linux.c\n> @@ -0,0 +1,676 @@\n> +#include \"cache.h\"\n> +#include \"fsmonitor.h\"\n> +#include \"fsm-listen.h\"\n> +#include \"fsmonitor--daemon.h\"\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +/*\n> + * Safe value to bitwise OR with rest of mask for\n> + * kernels that do not support IN_MASK_CREATE\n> + */\n> +#ifndef IN_MASK_CREATE\n> +#define IN_MASK_CREATE 0x00000000\n> +#endif\n> +\n> +enum shutdown_reason {\n> +\tSHUTDOWN_CONTINUE = 0,\n> +\tSHUTDOWN_STOP,\n> +\tSHUTDOWN_ERROR,\n> +\tSHUTDOWN_FORCE\n> +};\n> +\n> +struct watch_entry {\n> +\tstruct hashmap_entry ent;\n> +\tint wd;\n> +\tuint32_t cookie;\n> +\tconst char *dir;\n> +};\n> +\n> +struct rename_entry {\n> +\tstruct hashmap_entry ent;\n> +\ttime_t whence;\n> +\tuint32_t cookie;\n> +\tconst char *dir;\n> +};\n> +\n> +struct fsm_listen_data {\n> +\tint fd_inotify;\n> +\tenum shutdown_reason shutdown;\n> +\tstruct hashmap watches;\n> +\tstruct hashmap renames;\n> +\tstruct hashmap revwatches;\n> +};\n> +\n> +static int watch_entry_cmp(const void *cmp_data,\n> +\t\t\t const struct hashmap_entry *eptr,\n> +\t\t\t const struct hashmap_entry *entry_or_key,\n> +\t\t\t const void *keydata)\n> +{\n> +\tconst struct watch_entry *e1, *e2;\n> +\n> +\te1 = container_of(eptr, const struct watch_entry, ent);\n> +\te2 = container_of(eptr, const struct watch_entry, ent);\n> +\treturn e1->wd != e2->wd;\n> +}\n> +\n> +static int revwatches_entry_cmp(const void *cmp_data,\n> +\t\t\t const struct hashmap_entry *eptr,\n> +\t\t\t const struct hashmap_entry *entry_or_key,\n> +\t\t\t const void *keydata)\n> +{\n> +\tconst struct watch_entry *e1, *e2;\n> +\n> +\te1 = container_of(eptr, const struct watch_entry, ent);\n> +\te2 = container_of(eptr, const struct watch_entry, ent);\n> +\treturn strcmp(e1->dir, e2->dir);\n> +}\n> +\n> +static int rename_entry_cmp(const void *cmp_data,\n> +\t\t\t const struct hashmap_entry *eptr,\n> +\t\t\t const struct hashmap_entry *entry_or_key,\n> +\t\t\t const void *keydata)\n> +{\n> +\tconst struct rename_entry *e1, *e2;\n> +\n> +\te1 = container_of(eptr, const struct rename_entry, ent);\n> +\te2 = container_of(eptr, const struct rename_entry, ent);\n> +\treturn e1->cookie != e2->cookie;\n> +}\n> +\n> +/*\n> + * Register an inotify watch, add watch descriptor to path mapping\n> + * and the reverse mapping.\n> + */\n> +static int add_watch(const char *path, struct fsm_listen_data *data)\n> +{\n> +\tconst char *interned = strintern(path);\n> +\tstruct watch_entry *w1, *w2;\n> +\n> +\t/* add the inotify watch, don't allow watches to be modified */\n> +\tint wd = inotify_add_watch(data->fd_inotify, interned,\n> +\t\t\t\t(IN_ALL_EVENTS | IN_ONLYDIR | IN_MASK_CREATE)\n> +\t\t\t\t^ IN_ACCESS ^ IN_CLOSE ^ IN_OPEN);\n> +\tif (wd < 0)\n> +\t\treturn error_errno(\"inotify_add_watch('%s') failed\", interned);\n> +\n> +\t/* add watch descriptor -> directory mapping */\n> +\tCALLOC_ARRAY(w1, 1);\n> +\tw1->wd = wd;\n> +\tw1->dir = interned;\n> +\thashmap_entry_init(&w1->ent, memhash(&w1->wd, sizeof(int)));\n> +\thashmap_add(&data->watches, &w1->ent);\n> +\n> +\t/* add directory -> watch descriptor mapping */\n> +\tCALLOC_ARRAY(w2, 1);\n> +\tw2->wd = wd;\n> +\tw2->dir = interned;\n> +\thashmap_entry_init(&w2->ent, memhash(w2->dir, strlen(w2->dir)));\n> +\thashmap_add(&data->revwatches, &w2->ent);\n> +\n> +\treturn 0;\n> +}\n> +\n> +/*\n> + * Remove the inotify watch, the watch descriptor to path mapping\n> + * and the reverse mapping.\n> + */\n> +static void remove_watch(struct watch_entry *w,\n> +\tstruct fsm_listen_data *data)\n> +{\n> +\tstruct watch_entry k1, k2, *w1, *w2;\n> +\n> +\t/* remove watch, ignore error if kernel already did it */\n> +\tif (inotify_rm_watch(data->fd_inotify, w->wd) && errno != EINVAL)\n> +\t\terror_errno(\"inotify_rm_watch() failed\");\n> +\n> +\thashmap_entry_init(&k1.ent, memhash(&w->wd, sizeof(int)));\n> +\tw1 = hashmap_remove_entry(&data->watches, &k1, ent, NULL);\n> +\tif (!w1)\n> +\t\tBUG(\"Double remove of watch for '%s'\", w->dir);\n> +\n> +\tif (w1->cookie)\n> +\t\tBUG(\"Removing watch for '%s' which has a pending rename\", w1->dir);\n> +\n> +\thashmap_entry_init(&k2.ent, memhash(w->dir, strlen(w->dir)));\n> +\tw2 = hashmap_remove_entry(&data->revwatches, &k2, ent, NULL);\n> +\tif (!w2)\n> +\t\tBUG(\"Double remove of reverse watch for '%s'\", w->dir);\n> +\n> +\t/* w1->dir and w2->dir are interned strings, we don't own them */\n> +\tfree(w1);\n> +\tfree(w2);\n> +}\n> +\n> +/*\n> + * Check for stale directory renames.\n> + *\n> + * https://man7.org/linux/man-pages/man7/inotify.7.html\n> + *\n> + * Allow for some small timeout to account for the fact that insertion of the\n> + * IN_MOVED_FROM+IN_MOVED_TO event pair is not atomic, and the possibility that\n> + * there may not be any IN_MOVED_TO event.\n> + *\n> + * If the IN_MOVED_TO event is not received within the timeout then events have\n> + * been missed and the monitor is in an inconsistent state with respect to the\n> + * filesystem.\n> + */\n> +static int check_stale_dir_renames(struct hashmap *renames, time_t max_age)\n> +{\n> +\tstruct rename_entry *re;\n> +\tstruct hashmap_iter iter;\n> +\n> +\thashmap_for_each_entry(renames, &iter, re, ent) {\n> +\t\tif (re->whence <= max_age)\n> +\t\t\treturn -1;\n> +\t}\n> +\treturn 0;\n> +}\n> +\n> +/*\n> + * Track pending renames.\n> + *\n> + * Tracking is done via a event cookie to watch descriptor mapping.\n> + *\n> + * A rename is not complete until matching a IN_MOVED_TO event is received\n> + * for a corresponding IN_MOVED_FROM event.\n> + */\n> +static void add_dir_rename(uint32_t cookie, const char *path,\n> +\tstruct fsm_listen_data *data)\n\nNit: `add_dir_rename()` and the below `rename_dir()` sound as if we\nactually perform the rename ourselves. How about `track_dir_rename()`\nand `finalize_dir_rename()`?\n\n> +{\n> +\tstruct watch_entry k, *w;\n> +\tstruct rename_entry *re;\n> +\n> +\t/* lookup the watch descriptor for the given path */\n> +\thashmap_entry_init(&k.ent, memhash(path, strlen(path)));\n> +\tw = hashmap_get_entry(&data->revwatches, &k, ent, NULL);\n> +\tif (!w) /* should never happen */\n> +\t\tBUG(\"No watch for '%s'\", path);\n\nError message should start with lower-case character.\n\n> +\tw->cookie = cookie;\n> +\n> +\t/* add the pending rename to match against later */\n> +\tCALLOC_ARRAY(re, 1);\n> +\tre->dir = w->dir;\n> +\tre->cookie = w->cookie;\n> +\tre->whence = time(NULL);\n> +\thashmap_entry_init(&re->ent, memhash(&re->cookie, sizeof(uint32_t)));\n> +\thashmap_add(&data->renames, &re->ent);\n> +}\n> +\n> +/*\n> + * Handle directory renames\n> + *\n> + * Once a IN_MOVED_TO event is received, lookup the rename tracking information\n> + * via the event cookie and use this information to update the watch.\n> + */\n> +static void rename_dir(uint32_t cookie, const char *path,\n> +\tstruct fsm_listen_data *data)\n> +{\n> +\tstruct rename_entry rek, *re;\n> +\tstruct watch_entry k, *w;\n> +\n> +\t/* lookup a pending rename to match */\n> +\trek.cookie = cookie;\n> +\thashmap_entry_init(&rek.ent, memhash(&rek.cookie, sizeof(uint32_t)));\n> +\tre = hashmap_get_entry(&data->renames, &rek, ent, NULL);\n> +\tif (re) {\n> +\t\tk.dir = re->dir;\n> +\t\thashmap_entry_init(&k.ent, memhash(k.dir, strlen(k.dir)));\n> +\t\tw = hashmap_get_entry(&data->revwatches, &k, ent, NULL);\n> +\t\tif (w) {\n> +\t\t\tw->cookie = 0; /* rename handled */\n> +\t\t\tremove_watch(w, data);\n> +\t\t\tadd_watch(path, data);\n> +\t\t} else {\n> +\t\t\tBUG(\"No matching watch\");\n> +\t\t}\n> +\t} else {\n> +\t\tBUG(\"No matching cookie\");\n\nThe above two bugs should start with a lower-case letter.\n\n> +\t}\n> +}\n> +\n> +/*\n> + * Recursively add watches to every directory under path\n> + */\n> +static int register_inotify(const char *path,\n> +\tstruct fsmonitor_daemon_state *state,\n> +\tstruct fsmonitor_batch *batch)\n> +{\n> +\tDIR *dir;\n> +\tconst char *rel;\n> +\tstruct strbuf current = STRBUF_INIT;\n> +\tstruct dirent *de;\n> +\tstruct stat fs;\n> +\tint ret = -1;\n> +\n> +\tdir = opendir(path);\n> +\tif (!dir)\n> +\t\treturn error_errno(\"opendir('%s') failed\", path);\n> +\n> +\twhile ((de = readdir_skip_dot_and_dotdot(dir)) != NULL) {\n> +\t\tstrbuf_reset(¤t);\n> +\t\tstrbuf_addf(¤t, \"%s/%s\", path, de->d_name);\n> +\t\tif (lstat(current.buf, &fs)) {\n> +\t\t\terror_errno(\"lstat('%s') failed\", current.buf);\n\nMissing `_()` translation marker.\n\n> +\t\t\tgoto failed;\n> +\t\t}\n> +\n> +\t\t/* recurse into directory */\n> +\t\tif (S_ISDIR(fs.st_mode)) {\n> +\t\t\tif (add_watch(current.buf, state->listen_data))\n> +\t\t\t\tgoto failed;\n> +\t\t\tif (register_inotify(current.buf, state, batch))\n> +\t\t\t\tgoto failed;\n> +\t\t} else if (batch) {\n> +\t\t\trel = current.buf + state->path_worktree_watch.len + 1;\n> +\t\t\ttrace_printf_key(&trace_fsmonitor, \"explicitly adding '%s'\", rel);\n> +\t\t\tfsmonitor_batch__add_path(batch, rel);\n> +\t\t}\n> +\t}\n> +\tret = 0;\n> +\n> +failed:\n> +\tstrbuf_release(¤t);\n> +\tif (closedir(dir) < 0)\n> +\t\treturn error_errno(\"closedir('%s') failed\", path);\n\nMissing `_()` translation marker.\n\n> +\treturn ret;\n> +}\n> +\n> +static int em_rename_dir_from(u_int32_t mask)\n> +{\n> +\treturn ((mask & IN_ISDIR) && (mask & IN_MOVED_FROM));\n> +}\n> +\n> +static int em_rename_dir_to(u_int32_t mask)\n> +{\n> +\treturn ((mask & IN_ISDIR) && (mask & IN_MOVED_TO));\n> +}\n> +\n> +static int em_remove_watch(u_int32_t mask)\n> +{\n> +\treturn (mask & IN_DELETE_SELF);\n> +}\n> +\n> +static int em_dir_renamed(u_int32_t mask)\n> +{\n> +\treturn ((mask & IN_ISDIR) && (mask & IN_MOVE));\n> +}\n> +\n> +static int em_dir_created(u_int32_t mask)\n> +{\n> +\treturn ((mask & IN_ISDIR) && (mask & IN_CREATE));\n> +}\n> +\n> +static int em_dir_deleted(uint32_t mask)\n> +{\n> +\treturn ((mask & IN_ISDIR) && (mask & IN_DELETE));\n> +}\n> +\n> +static int em_force_shutdown(u_int32_t mask)\n> +{\n> +\treturn (mask & IN_UNMOUNT) || (mask & IN_Q_OVERFLOW);\n> +}\n> +\n> +static int em_ignore(u_int32_t mask)\n> +{\n> +\treturn (mask & IN_IGNORED) || (mask & IN_MOVE_SELF);\n> +}\n> +\n> +static void log_mask_set(const char *path, u_int32_t mask)\n> +{\n> +\tstruct strbuf msg = STRBUF_INIT;\n> +\n> +\tif (mask & IN_ACCESS)\n> +\t\tstrbuf_addstr(&msg, \"IN_ACCESS|\");\n> +\tif (mask & IN_MODIFY)\n> +\t\tstrbuf_addstr(&msg, \"IN_MODIFY|\");\n> +\tif (mask & IN_ATTRIB)\n> +\t\tstrbuf_addstr(&msg, \"IN_ATTRIB|\");\n> +\tif (mask & IN_CLOSE_WRITE)\n> +\t\tstrbuf_addstr(&msg, \"IN_CLOSE_WRITE|\");\n> +\tif (mask & IN_CLOSE_NOWRITE)\n> +\t\tstrbuf_addstr(&msg, \"IN_CLOSE_NOWRITE|\");\n> +\tif (mask & IN_OPEN)\n> +\t\tstrbuf_addstr(&msg, \"IN_OPEN|\");\n> +\tif (mask & IN_MOVED_FROM)\n> +\t\tstrbuf_addstr(&msg, \"IN_MOVED_FROM|\");\n> +\tif (mask & IN_MOVED_TO)\n> +\t\tstrbuf_addstr(&msg, \"IN_MOVED_TO|\");\n> +\tif (mask & IN_CREATE)\n> +\t\tstrbuf_addstr(&msg, \"IN_CREATE|\");\n> +\tif (mask & IN_DELETE)\n> +\t\tstrbuf_addstr(&msg, \"IN_DELETE|\");\n> +\tif (mask & IN_DELETE_SELF)\n> +\t\tstrbuf_addstr(&msg, \"IN_DELETE_SELF|\");\n> +\tif (mask & IN_MOVE_SELF)\n> +\t\tstrbuf_addstr(&msg, \"IN_MOVE_SELF|\");\n> +\tif (mask & IN_UNMOUNT)\n> +\t\tstrbuf_addstr(&msg, \"IN_UNMOUNT|\");\n> +\tif (mask & IN_Q_OVERFLOW)\n> +\t\tstrbuf_addstr(&msg, \"IN_Q_OVERFLOW|\");\n> +\tif (mask & IN_IGNORED)\n> +\t\tstrbuf_addstr(&msg, \"IN_IGNORED|\");\n> +\tif (mask & IN_ISDIR)\n> +\t\tstrbuf_addstr(&msg, \"IN_ISDIR|\");\n\nDoesn't this end up with one trailing '|' in `msg`? You could use\n`strbuf_strip_suffix(msg, \"|\")` to drop it.\n\n> +\ttrace_printf_key(&trace_fsmonitor, \"inotify_event: '%s', mask=%#8.8x %s\",\n> +\t\t\t\tpath, mask, msg.buf);\n> +\n> +\tstrbuf_release(&msg);\n> +}\n> +\n> +int fsm_listen__ctor(struct fsmonitor_daemon_state *state)\n> +{\n> +\tint fd;\n> +\tint ret = 0;\n> +\tstruct fsm_listen_data *data;\n> +\n> +\tCALLOC_ARRAY(data, 1);\n> +\tstate->listen_data = data;\n> +\tstate->listen_error_code = -1;\n> +\tdata->shutdown = SHUTDOWN_ERROR;\n> +\n> +\tfd = inotify_init1(O_NONBLOCK);\n> +\tif (fd < 0)\n> +\t\treturn error_errno(\"inotify_init1() failed\");\n> +\n> +\tdata->fd_inotify = fd;\n> +\n> +\thashmap_init(&data->watches, watch_entry_cmp, NULL, 0);\n> +\thashmap_init(&data->renames, rename_entry_cmp, NULL, 0);\n> +\thashmap_init(&data->revwatches, revwatches_entry_cmp, NULL, 0);\n> +\n> +\tif (add_watch(state->path_worktree_watch.buf, data))\n> +\t\tret = -1;\n> +\telse if (register_inotify(state->path_worktree_watch.buf, state, NULL))\n> +\t\tret = -1;\n> +\telse if (state->nr_paths_watching > 1) {\n> +\t\tif (add_watch(state->path_gitdir_watch.buf, data))\n> +\t\t\tret = -1;\n> +\t\telse if (register_inotify(state->path_gitdir_watch.buf, state, NULL))\n> +\t\t\tret = -1;\n> +\t}\n\nStyle: if one of the branches requires braces then all of them should.\n\n> +\n> +\tif (!ret) {\n> +\t\tstate->listen_error_code = 0;\n> +\t\tdata->shutdown = SHUTDOWN_CONTINUE;\n> +\t}\n> +\n> +\treturn ret;\n> +}\n> +\n> +void fsm_listen__dtor(struct fsmonitor_daemon_state *state)\n> +{\n> +\tstruct fsm_listen_data *data;\n> +\tstruct hashmap_iter iter;\n> +\tstruct watch_entry *w;\n> +\tint fd;\n> +\n> +\tif (!state || !state->listen_data)\n> +\t\treturn;\n> +\n> +\tdata = state->listen_data;\n> +\tfd = data->fd_inotify;\n> +\n> +\thashmap_for_each_entry(&data->watches, &iter, w, ent) {\n> +\t\tw->cookie = 0; /* ignore any pending renames */\n> +\t\tremove_watch(w, data);\n> +\t}\n> +\thashmap_clear(&data->watches);\n> +\n> +\thashmap_clear(&data->revwatches); /* remove_watch freed the entries */\n> +\n> +\thashmap_clear_and_free(&data->renames, struct rename_entry, ent);\n> +\n> +\tFREE_AND_NULL(state->listen_data);\n\nThe empty lines between all these cleanups can probably be removed.\n\n> +\n> +\tif (fd && (close(fd) < 0))\n> +\t\terror_errno(_(\"closing inotify file descriptor failed\"));\n> +}\n> +\n> +void fsm_listen__stop_async(struct fsmonitor_daemon_state *state)\n> +{\n> +\tif (!state->listen_data->shutdown)\n> +\t\tstate->listen_data->shutdown = SHUTDOWN_STOP;\n> +}\n> +\n> +/*\n> + * Process a single inotify event and queue for publication.\n> + */\n> +static int process_event(const char *path,\n> +\tconst struct inotify_event *event,\n> +\tstruct fsmonitor_batch *batch,\n> +\tstruct string_list *cookie_list,\n> +\tstruct fsmonitor_daemon_state *state)\n> +{\n> +\tconst char *rel;\n> +\tconst char *last_sep;\n> +\n> +\tswitch (fsmonitor_classify_path_absolute(state, path)) {\n> +\t\tcase IS_INSIDE_DOT_GIT_WITH_COOKIE_PREFIX:\n> +\t\tcase IS_INSIDE_GITDIR_WITH_COOKIE_PREFIX:\n> +\t\t\t/* Use just the filename of the cookie file. */\n> +\t\t\tlast_sep = find_last_dir_sep(path);\n> +\t\t\tstring_list_append(cookie_list,\n> +\t\t\t\t\tlast_sep ? last_sep + 1 : path);\n> +\t\t\tbreak;\n> +\t\tcase IS_INSIDE_DOT_GIT:\n> +\t\tcase IS_INSIDE_GITDIR:\n> +\t\t\tbreak;\n> +\t\tcase IS_DOT_GIT:\n> +\t\tcase IS_GITDIR:\n> +\t\t\t/*\n> +\t\t\t* If .git directory is deleted or renamed away,\n> +\t\t\t* we have to quit.\n> +\t\t\t*/\n> +\t\t\tif (em_dir_deleted(event->mask)) {\n> +\t\t\t\ttrace_printf_key(&trace_fsmonitor,\n> +\t\t\t\t\t\t\"event: gitdir removed\");\n> +\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n> +\t\t\t\tgoto done;\n> +\t\t\t}\n> +\n> +\t\t\tif (em_dir_renamed(event->mask)) {\n> +\t\t\t\ttrace_printf_key(&trace_fsmonitor,\n> +\t\t\t\t\t\t\"event: gitdir renamed\");\n> +\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n> +\t\t\t\tgoto done;\n> +\t\t\t}\n> +\t\t\tbreak;\n> +\t\tcase IS_WORKDIR_PATH:\n> +\t\t\t/* normal events in the working directory */\n> +\t\t\tif (trace_pass_fl(&trace_fsmonitor))\n> +\t\t\t\tlog_mask_set(path, event->mask);\n> +\n> +\t\t\trel = path + state->path_worktree_watch.len + 1;\n> +\t\t\tfsmonitor_batch__add_path(batch, rel);\n> +\n> +\t\t\tif (em_dir_deleted(event->mask))\n> +\t\t\t\tbreak;\n> +\n> +\t\t\t/* received IN_MOVE_FROM, add tracking for expected IN_MOVE_TO */\n> +\t\t\tif (em_rename_dir_from(event->mask))\n> +\t\t\t\tadd_dir_rename(event->cookie, path, state->listen_data);\n> +\n> +\t\t\t/* received IN_MOVE_TO, update watch to reflect new path */\n> +\t\t\tif (em_rename_dir_to(event->mask)) {\n> +\t\t\t\trename_dir(event->cookie, path, state->listen_data);\n> +\t\t\t\tif (register_inotify(path, state, batch)) {\n> +\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n> +\t\t\t\t\tgoto done;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\n> +\t\t\tif (em_dir_created(event->mask)) {\n> +\t\t\t\tif (add_watch(path, state->listen_data)) {\n> +\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n> +\t\t\t\t\tgoto done;\n> +\t\t\t\t}\n> +\t\t\t\tif (register_inotify(path, state, batch)) {\n> +\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n> +\t\t\t\t\tgoto done;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t\tbreak;\n> +\t\tcase IS_OUTSIDE_CONE:\n> +\t\tdefault:\n> +\t\t\ttrace_printf_key(&trace_fsmonitor,\n> +\t\t\t\t\t\"ignoring '%s'\", path);\n> +\t\t\tbreak;\n> +\t}\n> +\treturn 0;\n> +done:\n> +\treturn -1;\n> +}\n> +\n> +/*\n> + * Read the inotify event stream and pre-process events before further\n> + * processing and eventual publishing.\n> + */\n> +static void handle_events(struct fsmonitor_daemon_state *state)\n> +{\n> +\t /* See https://man7.org/linux/man-pages/man7/inotify.7.html */\n> +\tchar buf[4096]\n> +\t\t__attribute__ ((aligned(__alignof__(struct inotify_event))));\n> +\n> +\tstruct hashmap watches = state->listen_data->watches;\n> +\tstruct fsmonitor_batch *batch = NULL;\n> +\tstruct string_list cookie_list = STRING_LIST_INIT_DUP;\n> +\tstruct watch_entry k, *w;\n> +\tstruct strbuf path;\n> +\tconst struct inotify_event *event;\n> +\tint fd = state->listen_data->fd_inotify;\n> +\tssize_t len;\n> +\tchar *ptr, *p;\n\nI think many of the variables could be moved into deeper scopes. Makes\nit easier to assess what is being used where.\n\n> +\n> +\tstrbuf_init(&path, PATH_MAX);\n> +\n> +\tfor(;;) {\n> +\t\tlen = read(fd, buf, sizeof(buf));\n> +\t\tif (len == -1 && errno != EAGAIN) {\n\nWouldn't we also have to handle EINTR here?\n\n> +\t\t\terror_errno(_(\"reading inotify message stream failed\"));\n> +\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n> +\t\t\tgoto done;\n> +\t\t}\n> +\n> +\t\t/* nothing to read */\n> +\t\tif (len <= 0)\n> +\t\t\tgoto done;\n> +\n> +\t\t/* Loop over all events in the buffer. */\n> +\t\tfor (ptr = buf; ptr < buf + len;\n> +\t\t\t ptr += sizeof(struct inotify_event) + event->len) {\n\nNit: there's an additional whitespace here.\n\n> +\n> +\t\t\tevent = (const struct inotify_event *) ptr;\n> +\n> +\t\t\tif (em_ignore(event->mask))\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\t/* File system was unmounted or event queue overflowed */\n> +\t\t\tif (em_force_shutdown(event->mask)) {\n> +\t\t\t\tif (trace_pass_fl(&trace_fsmonitor))\n> +\t\t\t\t\tlog_mask_set(\"Forcing shutdown\", event->mask);\n> +\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n> +\t\t\t\tgoto done;\n> +\t\t\t}\n> +\n> +\t\t\thashmap_entry_init(&k.ent, memhash(&event->wd, sizeof(int)));\n> +\t\t\tk.wd = event->wd;\n> +\n> +\t\t\tw = hashmap_get_entry(&watches, &k, ent, NULL);\n> +\t\t\tif (!w) /* should never happen */\n> +\t\t\t\tBUG(\"No watch for '%s'\", event->name);\n\nError messages should start with a lower-case letter.\n\n> +\n> +\t\t\t/* directory watch was removed */\n> +\t\t\tif (em_remove_watch(event->mask)) {\n> +\t\t\t\tremove_watch(w, state->listen_data);\n> +\t\t\t\tcontinue;\n> +\t\t\t}\n> +\n> +\t\t\tstrbuf_reset(&path);\n> +\t\t\tstrbuf_add(&path, w->dir, strlen(w->dir));\n> +\t\t\tstrbuf_addch(&path, '/');\n> +\t\t\tstrbuf_addstr(&path, event->name);\n\nThe above three lines can be simplified to:\n\n```\nstrbuf_addf(\"%s/%s\", w->dir, event->name);\n```\n\n> +\n> +\t\t\tp = fsmonitor__resolve_alias(path.buf, &state->alias);\n> +\t\t\tif (!p)\n> +\t\t\t\tp = strbuf_detach(&path, NULL);\n> +\n> +\t\t\tif (!batch)\n> +\t\t\t\tbatch = fsmonitor_batch__new();\n> +\n> +\t\t\tif (process_event(p, event, batch, &cookie_list, state)) {\n> +\t\t\t\tfree(p);\n> +\t\t\t\tgoto done;\n> +\t\t\t}\n> +\t\t\tfree(p);\n> +\t\t}\n> +\t\tstrbuf_reset(&path);\n> +\t\tfsmonitor_publish(state, batch, &cookie_list);\n> +\t\tstring_list_clear(&cookie_list, 0);\n> +\t\tbatch = NULL;\n> +\t}\n> +done:\n> +\tstrbuf_release(&path);\n> +\tfsmonitor_batch__free_list(batch);\n> +\tstring_list_clear(&cookie_list, 0);\n> +}\n> +\n> +/*\n> + * Non-blocking read of the inotify events stream. The inotify fd is polled\n> + * frequently to help minimize the number of queue overflows.\n> + */\n> +void fsm_listen__loop(struct fsmonitor_daemon_state *state)\n> +{\n> +\tint poll_num;\n> +\tconst int interval = 1000;\n\nYou could rename the variable to `interval_ms` to clarify its unit.\n\n> +\ttime_t checked = time(NULL);\n> +\tstruct pollfd fds[1];\n> +\tfds[0].fd = state->listen_data->fd_inotify;\n> +\tfds[0].events = POLLIN;\n> +\n> +\tfor(;;) {\n\nNit: `while (1)`\n\n> +\t\tswitch (state->listen_data->shutdown) {\n> +\t\t\tcase SHUTDOWN_CONTINUE:\n> +\t\t\t\tpoll_num = poll(fds, 1, 1);\n\nWhy do you pick a timeout of 1 millisecond here? I'd have expected us to\ninstruct poll(3p) to either not block at all (0) or to block for\n`interval`.\n\n> +\t\t\t\tif (poll_num == -1) {\n> +\t\t\t\t\tif (errno == EINTR)\n> +\t\t\t\t\t\tcontinue;\n\nWouldn't we also have to handle EAGAIN here?\n\nPatrick\n\n> +\t\t\t\t\terror_errno(_(\"polling inotify message stream failed\"));\n> +\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n> +\t\t\t\t\tcontinue;\n> +\t\t\t\t}\n> +\n> +\t\t\t\tif ((time(NULL) - checked) >= interval) {\n> +\t\t\t\t\tchecked = time(NULL);\n> +\t\t\t\t\tif (check_stale_dir_renames(&state->listen_data->renames,\n> +\t\t\t\t\t\tchecked - interval)) {\n> +\t\t\t\t\t\ttrace_printf_key(&trace_fsmonitor,\n> +\t\t\t\t\t\t\t\"Missed IN_MOVED_TO events, forcing shutdown\");\n> +\t\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n> +\t\t\t\t\t\tcontinue;\n> +\t\t\t\t\t}\n> +\t\t\t\t}\n> +\n> +\t\t\t\tif (poll_num > 0 && (fds[0].revents & POLLIN))\n> +\t\t\t\t\thandle_events(state);\n> +\n> +\t\t\t\tcontinue;\n> +\t\t\tcase SHUTDOWN_ERROR:\n> +\t\t\t\tstate->listen_error_code = -1;\n> +\t\t\t\tipc_server_stop_async(state->ipc_server_data);\n> +\t\t\t\tbreak;\n> +\t\t\tcase SHUTDOWN_FORCE:\n> +\t\t\t\tstate->listen_error_code = 0;\n> +\t\t\t\tipc_server_stop_async(state->ipc_server_data);\n> +\t\t\t\tbreak;\n> +\t\t\tcase SHUTDOWN_STOP:\n> +\t\t\tdefault:\n> +\t\t\t\tstate->listen_error_code = 0;\n> +\t\t\t\tbreak;\n> +\t\t}\n> +\t\treturn;\n> +\t}\n> +}\n> -- \n> gitgitgadget\n> \n> \n"},{"id":"488737","messageId":"Zc4WcgJbH73uyRtc@tanuki","threadId":"60926","inReplyTo":"74a4fa335a7c2014a35be8556887170169360b36.1707992978.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 7/7] fsmonitor: addressed comments for patch 1352","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-02-15T13:49:38Z","receivedAt":"2024-02-15T13:49:44Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Feb 15, 2024 at 10:29:38AM +0000, marzi.esipreh via GitGitGadget wrote:\n> From: \"marzi.esipreh\" \n> \n> addressed comments on 1352, rebased, resolved conflicts\n\nPlease squash these changes into the preceding commits whereever\nrequired.\n\nPatrick\n\n> \n> Signed-off-by: Marzieh Esipreh \n> ---\n> compat/fsmonitor/fsm-health-linux.c | 2 +-\n> compat/fsmonitor/fsm-ipc-unix.c | 6 +-\n> compat/fsmonitor/fsm-listen-linux.c | 170 ++++++++++++------------\n> compat/fsmonitor/fsm-path-utils-linux.c | 1 +\n> compat/fsmonitor/fsm-settings-unix.c | 3 +\n> 5 files changed, 95 insertions(+), 87 deletions(-)\n> \n> diff --git a/compat/fsmonitor/fsm-health-linux.c b/compat/fsmonitor/fsm-health-linux.c\n> index b9f709e8548..4c291f8a066 100644\n> --- a/compat/fsmonitor/fsm-health-linux.c\n> +++ b/compat/fsmonitor/fsm-health-linux.c\n> @@ -1,4 +1,4 @@\n> -#include \"cache.h\"\n> +#include \"git-compat-util.h\"\n> #include \"config.h\"\n> #include \"fsmonitor.h\"\n> #include \"fsm-health.h\"\n> diff --git a/compat/fsmonitor/fsm-ipc-unix.c b/compat/fsmonitor/fsm-ipc-unix.c\n> index eb25123fa12..70afddfd298 100644\n> --- a/compat/fsmonitor/fsm-ipc-unix.c\n> +++ b/compat/fsmonitor/fsm-ipc-unix.c\n> @@ -1,10 +1,12 @@\n> -#include \"cache.h\"\n> +#include \"git-compat-util.h\"\n> #include \"config.h\"\n> #include \"hex.h\"\n> #include \"strbuf.h\"\n> #include \"fsmonitor.h\"\n> #include \"fsmonitor-ipc.h\"\n> #include \"fsmonitor-path-utils.h\"\n> +#include \"gettext.h\"\n> +#include \"path.h\"\n> \n> static GIT_PATH_FUNC(fsmonitor_ipc__get_default_path, \"fsmonitor--daemon.ipc\")\n> \n> @@ -17,7 +19,7 @@ const char *fsmonitor_ipc__get_path(struct repository *r)\n> \tunsigned char hash[GIT_MAX_RAWSZ];\n> \n> \tif (!r)\n> -\t\tBUG(\"No repository passed into fsmonitor_ipc__get_path\");\n> +\t\tBUG(\"no repository passed into fsmonitor_ipc__get_path\");\n> \n> \tif (ipc_path)\n> \t\treturn ipc_path;\n> diff --git a/compat/fsmonitor/fsm-listen-linux.c b/compat/fsmonitor/fsm-listen-linux.c\n> index e8548e4e009..84d8fb28d5d 100644\n> --- a/compat/fsmonitor/fsm-listen-linux.c\n> +++ b/compat/fsmonitor/fsm-listen-linux.c\n> @@ -1,7 +1,10 @@\n> -#include \"cache.h\"\n> +#include \"git-compat-util.h\"\n> +#include \"config.h\"\n> #include \"fsmonitor.h\"\n> #include \"fsm-listen.h\"\n> #include \"fsmonitor--daemon.h\"\n> +#include \"gettext.h\"\n> +#include \"simple-ipc.h\"\n> #include \n> #include \n> #include \n> @@ -129,15 +132,15 @@ static void remove_watch(struct watch_entry *w,\n> \thashmap_entry_init(&k1.ent, memhash(&w->wd, sizeof(int)));\n> \tw1 = hashmap_remove_entry(&data->watches, &k1, ent, NULL);\n> \tif (!w1)\n> -\t\tBUG(\"Double remove of watch for '%s'\", w->dir);\n> +\t\tBUG(\"double remove of watch for '%s'\", w->dir);\n> \n> \tif (w1->cookie)\n> -\t\tBUG(\"Removing watch for '%s' which has a pending rename\", w1->dir);\n> +\t\tBUG(\"removing watch for '%s' which has a pending rename\", w1->dir);\n> \n> \thashmap_entry_init(&k2.ent, memhash(w->dir, strlen(w->dir)));\n> \tw2 = hashmap_remove_entry(&data->revwatches, &k2, ent, NULL);\n> \tif (!w2)\n> -\t\tBUG(\"Double remove of reverse watch for '%s'\", w->dir);\n> +\t\tBUG(\"double remove of reverse watch for '%s'\", w->dir);\n> \n> \t/* w1->dir and w2->dir are interned strings, we don't own them */\n> \tfree(w1);\n> @@ -187,7 +190,7 @@ static void add_dir_rename(uint32_t cookie, const char *path,\n> \thashmap_entry_init(&k.ent, memhash(path, strlen(path)));\n> \tw = hashmap_get_entry(&data->revwatches, &k, ent, NULL);\n> \tif (!w) /* should never happen */\n> -\t\tBUG(\"No watch for '%s'\", path);\n> +\t\tBUG(\"no watch for '%s'\", path);\n> \tw->cookie = cookie;\n> \n> \t/* add the pending rename to match against later */\n> @@ -224,10 +227,10 @@ static void rename_dir(uint32_t cookie, const char *path,\n> \t\t\tremove_watch(w, data);\n> \t\t\tadd_watch(path, data);\n> \t\t} else {\n> -\t\t\tBUG(\"No matching watch\");\n> +\t\t\tBUG(\"no matching watch\");\n> \t\t}\n> \t} else {\n> -\t\tBUG(\"No matching cookie\");\n> +\t\tBUG(\"no matching cookie\");\n> \t}\n> }\n> \n> @@ -249,7 +252,7 @@ static int register_inotify(const char *path,\n> \tif (!dir)\n> \t\treturn error_errno(\"opendir('%s') failed\", path);\n> \n> -\twhile ((de = readdir_skip_dot_and_dotdot(dir)) != NULL) {\n> +\twhile ((de = readdir_skip_dot_and_dotdot(dir))) {\n> \t\tstrbuf_reset(¤t);\n> \t\tstrbuf_addf(¤t, \"%s/%s\", path, de->d_name);\n> \t\tif (lstat(current.buf, &fs)) {\n> @@ -353,7 +356,7 @@ static void log_mask_set(const char *path, u_int32_t mask)\n> \tif (mask & IN_IGNORED)\n> \t\tstrbuf_addstr(&msg, \"IN_IGNORED|\");\n> \tif (mask & IN_ISDIR)\n> -\t\tstrbuf_addstr(&msg, \"IN_ISDIR|\");\n> +\t\tstrbuf_addstr(&msg, \"IN_ISDIR\");\n> \n> \ttrace_printf_key(&trace_fsmonitor, \"inotify_event: '%s', mask=%#8.8x %s\",\n> \t\t\t\tpath, mask, msg.buf);\n> @@ -373,8 +376,10 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state)\n> \tdata->shutdown = SHUTDOWN_ERROR;\n> \n> \tfd = inotify_init1(O_NONBLOCK);\n> -\tif (fd < 0)\n> +\tif (fd < 0) {\n> +\t\tFREE_AND_NULL(data);\n> \t\treturn error_errno(\"inotify_init1() failed\");\n> +\t}\n> \n> \tdata->fd_inotify = fd;\n> \n> @@ -386,12 +391,10 @@ int fsm_listen__ctor(struct fsmonitor_daemon_state *state)\n> \t\tret = -1;\n> \telse if (register_inotify(state->path_worktree_watch.buf, state, NULL))\n> \t\tret = -1;\n> -\telse if (state->nr_paths_watching > 1) {\n> -\t\tif (add_watch(state->path_gitdir_watch.buf, data))\n> -\t\t\tret = -1;\n> -\t\telse if (register_inotify(state->path_gitdir_watch.buf, state, NULL))\n> -\t\t\tret = -1;\n> -\t}\n> +\telse if (state->nr_paths_watching > 1 &&\n> +\t\t (add_watch(state->path_gitdir_watch.buf, data) ||\n> +\t\t register_inotify(state->path_gitdir_watch.buf, state, NULL)))\n> +\t\tret = -1;\n> \n> \tif (!ret) {\n> \t\tstate->listen_error_code = 0;\n> @@ -449,80 +452,80 @@ static int process_event(const char *path,\n> \tconst char *last_sep;\n> \n> \tswitch (fsmonitor_classify_path_absolute(state, path)) {\n> -\t\tcase IS_INSIDE_DOT_GIT_WITH_COOKIE_PREFIX:\n> -\t\tcase IS_INSIDE_GITDIR_WITH_COOKIE_PREFIX:\n> -\t\t\t/* Use just the filename of the cookie file. */\n> -\t\t\tlast_sep = find_last_dir_sep(path);\n> -\t\t\tstring_list_append(cookie_list,\n> -\t\t\t\t\tlast_sep ? last_sep + 1 : path);\n> -\t\t\tbreak;\n> -\t\tcase IS_INSIDE_DOT_GIT:\n> -\t\tcase IS_INSIDE_GITDIR:\n> -\t\t\tbreak;\n> -\t\tcase IS_DOT_GIT:\n> -\t\tcase IS_GITDIR:\n> -\t\t\t/*\n> -\t\t\t* If .git directory is deleted or renamed away,\n> -\t\t\t* we have to quit.\n> -\t\t\t*/\n> -\t\t\tif (em_dir_deleted(event->mask)) {\n> -\t\t\t\ttrace_printf_key(&trace_fsmonitor,\n> -\t\t\t\t\t\t\"event: gitdir removed\");\n> -\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n> -\t\t\t\tgoto done;\n> -\t\t\t}\n> +\tcase IS_INSIDE_DOT_GIT_WITH_COOKIE_PREFIX:\n> +\tcase IS_INSIDE_GITDIR_WITH_COOKIE_PREFIX:\n> +\t\t/* Use just the filename of the cookie file. */\n> +\t\tlast_sep = find_last_dir_sep(path);\n> +\t\tstring_list_append(cookie_list,\n> +\t\t\t\tlast_sep ? last_sep + 1 : path);\n> +\t\tbreak;\n> +\tcase IS_INSIDE_DOT_GIT:\n> +\tcase IS_INSIDE_GITDIR:\n> +\t\tbreak;\n> +\tcase IS_DOT_GIT:\n> +\tcase IS_GITDIR:\n> +\t\t/*\n> +\t\t* If .git directory is deleted or renamed away,\n> +\t\t* we have to quit.\n> +\t\t*/\n> +\t\tif (em_dir_deleted(event->mask)) {\n> +\t\t\ttrace_printf_key(&trace_fsmonitor,\n> +\t\t\t\t\t\"event: gitdir removed\");\n> +\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n> +\t\t\tgoto done;\n> +\t\t}\n> \n> -\t\t\tif (em_dir_renamed(event->mask)) {\n> -\t\t\t\ttrace_printf_key(&trace_fsmonitor,\n> -\t\t\t\t\t\t\"event: gitdir renamed\");\n> -\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n> -\t\t\t\tgoto done;\n> -\t\t\t}\n> -\t\t\tbreak;\n> -\t\tcase IS_WORKDIR_PATH:\n> -\t\t\t/* normal events in the working directory */\n> -\t\t\tif (trace_pass_fl(&trace_fsmonitor))\n> -\t\t\t\tlog_mask_set(path, event->mask);\n> +\t\tif (em_dir_renamed(event->mask)) {\n> +\t\t\ttrace_printf_key(&trace_fsmonitor,\n> +\t\t\t\t\t\"event: gitdir renamed\");\n> +\t\t\tstate->listen_data->shutdown = SHUTDOWN_FORCE;\n> +\t\t\tgoto done;\n> +\t\t}\n> +\t\tbreak;\n> +\tcase IS_WORKDIR_PATH:\n> +\t\t/* normal events in the working directory */\n> +\t\tif (trace_pass_fl(&trace_fsmonitor))\n> +\t\t\tlog_mask_set(path, event->mask);\n> \n> -\t\t\trel = path + state->path_worktree_watch.len + 1;\n> -\t\t\tfsmonitor_batch__add_path(batch, rel);\n> +\t\trel = path + state->path_worktree_watch.len + 1;\n> +\t\tfsmonitor_batch__add_path(batch, rel);\n> \n> -\t\t\tif (em_dir_deleted(event->mask))\n> -\t\t\t\tbreak;\n> +\t\tif (em_dir_deleted(event->mask))\n> +\t\t\tbreak;\n> \n> -\t\t\t/* received IN_MOVE_FROM, add tracking for expected IN_MOVE_TO */\n> -\t\t\tif (em_rename_dir_from(event->mask))\n> -\t\t\t\tadd_dir_rename(event->cookie, path, state->listen_data);\n> +\t\t/* received IN_MOVE_FROM, add tracking for expected IN_MOVE_TO */\n> +\t\tif (em_rename_dir_from(event->mask))\n> +\t\t\tadd_dir_rename(event->cookie, path, state->listen_data);\n> \n> -\t\t\t/* received IN_MOVE_TO, update watch to reflect new path */\n> -\t\t\tif (em_rename_dir_to(event->mask)) {\n> -\t\t\t\trename_dir(event->cookie, path, state->listen_data);\n> -\t\t\t\tif (register_inotify(path, state, batch)) {\n> -\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n> -\t\t\t\t\tgoto done;\n> -\t\t\t\t}\n> +\t\t/* received IN_MOVE_TO, update watch to reflect new path */\n> +\t\tif (em_rename_dir_to(event->mask)) {\n> +\t\t\trename_dir(event->cookie, path, state->listen_data);\n> +\t\t\tif (register_inotify(path, state, batch)) {\n> +\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n> +\t\t\t\tgoto done;\n> \t\t\t}\n> +\t\t}\n> \n> -\t\t\tif (em_dir_created(event->mask)) {\n> -\t\t\t\tif (add_watch(path, state->listen_data)) {\n> -\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n> -\t\t\t\t\tgoto done;\n> -\t\t\t\t}\n> -\t\t\t\tif (register_inotify(path, state, batch)) {\n> -\t\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n> -\t\t\t\t\tgoto done;\n> -\t\t\t\t}\n> +\t\tif (em_dir_created(event->mask)) {\n> +\t\t\tif (add_watch(path, state->listen_data)) {\n> +\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n> +\t\t\t\tgoto done;\n> \t\t\t}\n> -\t\t\tbreak;\n> -\t\tcase IS_OUTSIDE_CONE:\n> -\t\tdefault:\n> -\t\t\ttrace_printf_key(&trace_fsmonitor,\n> -\t\t\t\t\t\"ignoring '%s'\", path);\n> -\t\t\tbreak;\n> +\t\t\tif (register_inotify(path, state, batch)) {\n> +\t\t\t\tstate->listen_data->shutdown = SHUTDOWN_ERROR;\n> +\t\t\t\tgoto done;\n> +\t\t\t}\n> +\t\t}\n> +\t\tbreak;\n> +\tcase IS_OUTSIDE_CONE:\n> +\tdefault:\n> +\t\ttrace_printf_key(&trace_fsmonitor,\n> +\t\t\t\t\"ignoring '%s'\", path);\n> +\t\tbreak;\n> \t}\n> \treturn 0;\n> -done:\n> -\treturn -1;\n> +\tdone:\n> +\t\treturn -1;\n> }\n> \n> /*\n> @@ -531,7 +534,7 @@ static int process_event(const char *path,\n> */\n> static void handle_events(struct fsmonitor_daemon_state *state)\n> {\n> -\t /* See https://man7.org/linux/man-pages/man7/inotify.7.html */\n> +\t/* See https://man7.org/linux/man-pages/man7/inotify.7.html */\n> \tchar buf[4096]\n> \t\t__attribute__ ((aligned(__alignof__(struct inotify_event))));\n> \n> @@ -539,13 +542,12 @@ static void handle_events(struct fsmonitor_daemon_state *state)\n> \tstruct fsmonitor_batch *batch = NULL;\n> \tstruct string_list cookie_list = STRING_LIST_INIT_DUP;\n> \tstruct watch_entry k, *w;\n> -\tstruct strbuf path;\n> \tconst struct inotify_event *event;\n> \tint fd = state->listen_data->fd_inotify;\n> \tssize_t len;\n> \tchar *ptr, *p;\n> \n> -\tstrbuf_init(&path, PATH_MAX);\n> +\tstruct strbuf path = STRBUF_INIT;\n> \n> \tfor(;;) {\n> \t\tlen = read(fd, buf, sizeof(buf));\n> @@ -581,7 +583,7 @@ static void handle_events(struct fsmonitor_daemon_state *state)\n> \n> \t\t\tw = hashmap_get_entry(&watches, &k, ent, NULL);\n> \t\t\tif (!w) /* should never happen */\n> -\t\t\t\tBUG(\"No watch for '%s'\", event->name);\n> +\t\t\t\tBUG(\"no watch for '%s'\", event->name);\n> \n> \t\t\t/* directory watch was removed */\n> \t\t\tif (em_remove_watch(event->mask)) {\n> diff --git a/compat/fsmonitor/fsm-path-utils-linux.c b/compat/fsmonitor/fsm-path-utils-linux.c\n> index c21d1349532..0e3b33ffa48 100644\n> --- a/compat/fsmonitor/fsm-path-utils-linux.c\n> +++ b/compat/fsmonitor/fsm-path-utils-linux.c\n> @@ -3,6 +3,7 @@\n> #include \"fsmonitor.h\"\n> #include \"fsmonitor-path-utils.h\"\n> #include \"fsm-path-utils-linux.h\"\n> +#include \"gettext.h\"\n> #include \n> #include \n> #include \n> diff --git a/compat/fsmonitor/fsm-settings-unix.c b/compat/fsmonitor/fsm-settings-unix.c\n> index d16dca89416..c9b75aa44fe 100644\n> --- a/compat/fsmonitor/fsm-settings-unix.c\n> +++ b/compat/fsmonitor/fsm-settings-unix.c\n> @@ -1,6 +1,9 @@\n> +#include \"git-compat-util.h\"\n> +#include \"config.h\"\n> #include \"fsmonitor.h\"\n> #include \"fsmonitor-ipc.h\"\n> #include \"fsmonitor-path-utils.h\"\n> +#include \n> \n> /*\n> * For the builtin FSMonitor, we create the Unix domain socket for the\n> -- \n> gitgitgadget\n> \n"},{"id":"511533","messageId":"20250131032859.37865-1-mkenchugonde@atlassian.com","threadId":"60926","inReplyTo":"pull.1667.git.git.1707992978.gitgitgadget@gmail.com","subject":"[PATCH 0/7] fsmonitor: completing a stale patch that Implements fsmonitor for Linux","fromName":"Manoraj K","fromEmail":"kmr.manu535@gmail.com","sentAt":"2025-01-31T03:28:59Z","receivedAt":"2025-01-31T03:29:06Z","isPatch":true,"sender":{"key":"kmr.manu535@gmail.com","avatar":null},"body":"Hi @maryis, I'm Just checking to see if you are able to make any progress on this PR post the last commit. FYI, I tried the build from this branch on one of the Linux instances, and the git status is ~80% faster.\n\nThere are some merge conflicts with master branch (config.mak.uname and compat/fsmonitor/fsm-ipc-darwin.c) I tried to solve them, but, couldn't make sense of the changes so didn't proceed.\n"}]}