{"thread":{"id":"58283","subject":"[PATCH] fsmonitor: option to allow fsmonitor to run against network-mounted repos","startedAt":"2022-08-09T17:44:17Z","lastAt":"2022-08-15T17:33:27Z","messageCount":20,"participants":["Eric DeCosta via GitGitGadget","Junio C Hamano","Eric D","Jeff Hostetler"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"460928","messageId":"pull.1317.git.1660067049965.gitgitgadget@gmail.com","threadId":"58283","inReplyTo":null,"subject":"[PATCH] fsmonitor: option to allow fsmonitor to run against network-mounted repos","fromName":"Eric DeCosta via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-09T17:44:09Z","receivedAt":"2022-08-09T17:44:17Z","isPatch":true,"sender":{"key":"edecosta@mathworks.com","avatar":"https://avatars.githubusercontent.com/u/67609563?v=4"},"body":"From: Eric DeCosta \n\nThough perhaps not common, there are uses cases where users have large,\nnetwork-mounted repos. Having the ability to run fsmonitor against\nnetwork paths would benefit those users.\n\nMost modern Samba-based filers have the necessary support to enable\nfsmonitor on network-mounted repos. As a first step towards enabling\nfsmonitor to work against network-mounted repos, introduce a\nconfiguration option, 'fsmonitor.allowRemote'. Setting this option to\ntrue will override the default behavior (erroring-out) when a\nnetwork-mounted repo is detected by fsmonitor.\n\nAdditionally, as part of this first step, monitoring of network-mounted\nrepos will be restricted to those mounted over SMB regardless of the\nvalue of 'fsmonitor.allowRemote' until more extensive testing can be\nperformed.\n\nSigned-off-by: Eric DeCosta \n---\n Option to allow fsmonitor to run against repos on network file systems\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1317%2Fedecosta-mw%2Fmaster-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1317/edecosta-mw/master-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1317\n\n compat/fsmonitor/fsm-settings-win32.c | 59 ++++++++++++++++++++++++++-\n 1 file changed, 58 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/fsmonitor/fsm-settings-win32.c b/compat/fsmonitor/fsm-settings-win32.c\nindex 907655720bb..d120e4710cf 100644\n--- a/compat/fsmonitor/fsm-settings-win32.c\n+++ b/compat/fsmonitor/fsm-settings-win32.c\n@@ -24,6 +24,58 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)\n \treturn FSMONITOR_REASON_OK;\n }\n \n+/*\n+ * Check if monitoring remote working directories is allowed.\n+ *\n+ * By default monitoring remote working directories is not allowed,\n+ * but users may override this behavior in enviroments where they\n+ * have proper support.\n+*/\n+static enum fsmonitor_reason check_allow_remote(struct repository *r)\n+{\n+\tint allow;\n+\n+\tif (repo_config_get_bool(r, \"fsmonitor.allowremote\", &allow) || !allow)\n+\t\treturn FSMONITOR_REASON_REMOTE;\n+\n+\treturn FSMONITOR_REASON_OK;\n+}\n+\n+/*\n+ * Check if the remote working directory is mounted via SMB\n+ *\n+ * For now, remote working directories are only supported via SMB mounts\n+*/\n+static enum fsmonitor_reason check_smb(wchar_t *wpath)\n+{\n+\tHANDLE h;\n+\tFILE_REMOTE_PROTOCOL_INFO proto_info;\n+\n+\th = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,\n+\t\t\t\t\tFILE_FLAG_BACKUP_SEMANTICS, NULL);\n+\n+\tif (h == INVALID_HANDLE_VALUE) {\n+\t\terror(_(\"[GLE %ld] unable to open for read '%ls'\"),\n+\t\t GetLastError(), wpath);\n+\t\treturn FSMONITOR_REASON_ERROR;\n+\t}\n+\n+\tif (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,\n+\t\t\t\t\t\t\t\t\t&proto_info, sizeof(proto_info))) {\n+\t\terror(_(\"[GLE %ld] unable to get protocol information for '%ls'\"),\n+\t\t GetLastError(), wpath);\n+\t\tCloseHandle(h);\n+\t\treturn FSMONITOR_REASON_ERROR;\n+\t}\n+\n+\tCloseHandle(h);\n+\n+\tif (proto_info.Protocol == WNNC_NET_SMB)\n+\t\treturn FSMONITOR_REASON_OK;\n+\n+\treturn FSMONITOR_REASON_ERROR;\n+}\n+\n /*\n * Remote working directories are problematic for FSMonitor.\n *\n@@ -76,6 +128,7 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)\n */\n static enum fsmonitor_reason check_remote(struct repository *r)\n {\n+\tenum fsmonitor_reason reason;\n \twchar_t wpath[MAX_PATH];\n \twchar_t wfullpath[MAX_PATH];\n \tsize_t wlen;\n@@ -115,7 +168,11 @@ static enum fsmonitor_reason check_remote(struct repository *r)\n \t\ttrace_printf_key(&trace_fsmonitor,\n \t\t\t\t \"check_remote('%s') true\",\n \t\t\t\t r->worktree);\n-\t\treturn FSMONITOR_REASON_REMOTE;\n+\n+\t\treason = check_smb(wfullpath);\n+\t\tif (reason != FSMONITOR_REASON_OK)\n+\t\t\treturn reason;\n+\t\treturn check_allow_remote(r);\n \t}\n \n \treturn FSMONITOR_REASON_OK;\n\nbase-commit: c50926e1f48891e2671e1830dbcd2912a4563450\n-- \ngitgitgadget\n"},{"id":"461004","messageId":"xmqqmtccniw4.fsf@gitster.g","threadId":"58283","inReplyTo":"pull.1317.git.1660067049965.gitgitgadget@gmail.com","subject":"Re: [PATCH] fsmonitor: option to allow fsmonitor to run against network-mounted repos","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-10T16:49:31Z","receivedAt":"2022-08-10T16:49:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Eric DeCosta via GitGitGadget\" writes:\n\n> Most modern Samba-based filers have the necessary support to enable\n> fsmonitor on network-mounted repos.\n\nMy impression from the earlier discussion [*] was that having the\nnecessary support and the support working well are two different\nthings, and the network mounted directory being served via SMB was\nnot enough sign of the latter.\n\nhttps://lore.kernel.org/git/16832f8a-c582-23bb-dda9-b7b2597a42eb@jeffhostetler.com/\n\nI do not do Windows, so I'll leave it up to the experts, but if\neverybody who talks SMB behaves well, then the updated code that\nenables fsmonitor for SMB talkers and disables it for all other\nremotely mounted directories, with a configuration override, does\nsound like a good way to go. I think the new code in check_remote()\nis broken, though (see below).\n\n> +/*\n> + * Check if monitoring remote working directories is allowed.\n> + *\n> + * By default monitoring remote working directories is not allowed,\n> + * but users may override this behavior in enviroments where they\n> + * have proper support.\n> +*/\n\nAll existing multi-line comments in this file (and properly\nformatted ones for this project) ends with \"*/\" where the asterisk\naligns with the asterisk on the previous line. \n\nThe three-line design goal is well written, but as you are special\ncasing SMB, perhaps\n\n\tBy default, monitoring remote working directories is\n\tdisabled unless on a network filesystem that is known to\n\tbehave well. Users may override ...\n\nor something like that may be warranted.\n\n> +static enum fsmonitor_reason check_allow_remote(struct repository *r)\n> +{\n> +\tint allow;\n> +\n> +\tif (repo_config_get_bool(r, \"fsmonitor.allowremote\", &allow) || !allow)\n> +\t\treturn FSMONITOR_REASON_REMOTE;\n> +\n> +\treturn FSMONITOR_REASON_OK;\n> +}\n\nThis is not wrong per se, but it probably is easier to follow if you\nwrite them the other way around.\n\n\tif (!repo_config_get_bool(..., &allow) && allow)\n\t\treturn FSMONITOR_REASON_OK;\n\n\treturn FSMONITOR_REASON_REMOTE;\n\nAfter all, as the big comment before the function says, by default\nwe deny and only on exceptions we allow.\n\nActually, the above is not quite right. You'd probably want a \"not\nset\" or \"undecided\" bit. I.e. something like\n\n\tif (!repo_config_get_bool(..., &allow))\n\t\treturn allow ? FSMONITOR_REASON_OK : FSMONITOR_REASON_REMOTE;\n\n\treturn FSMONITOR_REASON_UNDECIDED; /* invented... */\n\n> +/*\n> + * Check if the remote working directory is mounted via SMB\n> + *\n> + * For now, remote working directories are only supported via SMB mounts\n> +*/\n\n\"*/\" -> \" */\".\n\n> +static enum fsmonitor_reason check_smb(wchar_t *wpath)\n> +{\n> +\tHANDLE h;\n> +\tFILE_REMOTE_PROTOCOL_INFO proto_info;\n> +\n> +\th = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,\n> +\t\t\t\t\tFILE_FLAG_BACKUP_SEMANTICS, NULL);\n> +\n> +\tif (h == INVALID_HANDLE_VALUE) {\n> +\t\terror(_(\"[GLE %ld] unable to open for read '%ls'\"),\n> +\t\t GetLastError(), wpath);\n> +\t\treturn FSMONITOR_REASON_ERROR;\n> +\t}\n> +\n> +\tif (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,\n> +\t\t\t\t\t\t\t\t\t&proto_info, sizeof(proto_info))) {\n\nOverly deep indentation that unnecessarily causes an overly long\nline. \"&\" in \"&proto_info\" should align with \"h\" in\n\"...HandleEx(h,\", we use fixed-width font, and our tab-width is 8.\n\nThe second line of CreateFileW() call is also overly indented and\nmay want to be fixed, but it does not overly extend to the right end\nof the display (we aim to fit in 80-columns, as CodingGuideline says)\nso it would be nice if it gets fixed, but it may be tolerated. This\none is not.\n\n> +\t\terror(_(\"[GLE %ld] unable to get protocol information for '%ls'\"),\n> +\t\t GetLastError(), wpath);\n\nThis line gets it right ;-)\n\n> +\t\tCloseHandle(h);\n> +\t\treturn FSMONITOR_REASON_ERROR;\n> +\t}\n> +\n> +\tCloseHandle(h);\n> +\n> +\tif (proto_info.Protocol == WNNC_NET_SMB)\n> +\t\treturn FSMONITOR_REASON_OK;\n> +\n> +\treturn FSMONITOR_REASON_ERROR;\n\nIs it? Shouldn't it be REASON_REMOTE, not ERROR? Unlike the\nearlier case where you couldn't figure out the protocol information,\nat this point you know you learned what protocol is in use, and it\nis just that the protocol was not what you liked.\n\n> +}\n> +\n> /*\n> * Remote working directories are problematic for FSMonitor.\n> *\n> @@ -76,6 +128,7 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)\n> */\n> static enum fsmonitor_reason check_remote(struct repository *r)\n> {\n> +\tenum fsmonitor_reason reason;\n> \twchar_t wpath[MAX_PATH];\n> \twchar_t wfullpath[MAX_PATH];\n> \tsize_t wlen;\n> @@ -115,7 +168,11 @@ static enum fsmonitor_reason check_remote(struct repository *r)\n> \t\ttrace_printf_key(&trace_fsmonitor,\n> \t\t\t\t \"check_remote('%s') true\",\n> \t\t\t\t r->worktree);\n> -\t\treturn FSMONITOR_REASON_REMOTE;\n> +\n> +\t\treason = check_smb(wfullpath);\n> +\t\tif (reason != FSMONITOR_REASON_OK)\n> +\t\t\treturn reason;\n> +\t\treturn check_allow_remote(r);\n\nThis does not fulfill the promise you made in front of\ncheck_allow_remote(). As we saw, check_smb() returns something\nother than REASON_OK when it is talking to a remote that is not SMB,\nand when that happens, you are not giving check_allow_remote() a\nchance to intervene and override. It should be more like\n\n\treason = check_allow_remote(r);\n if (reason == FSMONITOR_REASON_OK || reason == FSMONITOR_REASON_REMOTE)\n\t\treturn reason;\n\t/*\n\t * check_allow_remote() did not decide, so use the default\n\t * based on the network filesystem.\n\t */\n\treturn check_smb(wfullpath);\n\nI would think.\n\n> \t}\n> \n> \treturn FSMONITOR_REASON_OK;\n>\n> base-commit: c50926e1f48891e2671e1830dbcd2912a4563450\n"},{"id":"461018","messageId":"CAMxJVdH3B2An7La9knM=QJojQ334O+Z2-tqNvqRZz2Eu6CV+-w@mail.gmail.com","threadId":"58283","inReplyTo":"xmqqmtccniw4.fsf@gitster.g","subject":"Re: [PATCH] fsmonitor: option to allow fsmonitor to run against network-mounted repos","fromName":"Eric D","fromEmail":"eric.decosta@gmail.com","sentAt":"2022-08-10T18:49:28Z","receivedAt":"2022-08-10T18:49:47Z","isPatch":true,"sender":{"key":"eric.decosta@gmail.com","avatar":null},"body":"On Wed, Aug 10, 2022 at 12:56 PM Junio C Hamano wrote:\n>\n> \"Eric DeCosta via GitGitGadget\" writes:\n>\n> > Most modern Samba-based filers have the necessary support to enable\n> > fsmonitor on network-mounted repos.\n>\n> My impression from the earlier discussion [*] was that having the\n> necessary support and the support working well are two different\n> things, and the network mounted directory being served via SMB was\n> not enough sign of the latter.\n>\n> https://lore.kernel.org/git/16832f8a-c582-23bb-dda9-b7b2597a42eb@jeffhostetler.com/\n>\n> I do not do Windows, so I'll leave it up to the experts, but if\n> everybody who talks SMB behaves well, then the updated code that\n> enables fsmonitor for SMB talkers and disables it for all other\n> remotely mounted directories, with a configuration override, does\n> sound like a good way to go. I think the new code in check_remote()\n> is broken, though (see below).\n\n> > +/*\n> > + * Check if monitoring remote working directories is allowed.\n> > + *\n> > + * By default monitoring remote working directories is not allowed,\n> > + * but users may override this behavior in enviroments where they\n> > + * have proper support.\n> > +*/\n> All existing multi-line comments in this file (and properly\n> formatted ones for this project) ends with \"*/\" where the asterisk\n> aligns with the asterisk on the previous line.\n\nI'll clean them up, thanks.\n\n> The three-line design goal is well written, but as you are special\n> casing SMB, perhaps\n>\n> By default, monitoring remote working directories is\n> disabled unless on a network filesystem that is known to\n> behave well. Users may override ...\n>\n> or something like that may be warranted.\n\nThat reads much better, thanks.\n\n> > +static enum fsmonitor_reason check_allow_remote(struct repository *r)\n> > +{\n> > + int allow;\n> > +\n> > + if (repo_config_get_bool(r, \"fsmonitor.allowremote\", &allow) || !allow)\n> > + return FSMONITOR_REASON_REMOTE;\n> > +\n> > + return FSMONITOR_REASON_OK;\n> > +}\n>\n> This is not wrong per se, but it probably is easier to follow if you\n> write them the other way around.\n>\n> if (!repo_config_get_bool(..., &allow) && allow)\n> return FSMONITOR_REASON_OK;\n>\n> return FSMONITOR_REASON_REMOTE;\n>\n> After all, as the big comment before the function says, by default\n> we deny and only on exceptions we allow.\n>\n> Actually, the above is not quite right. You'd probably want a \"not\n> set\" or \"undecided\" bit. I.e. something like\n>\n> if (!repo_config_get_bool(..., &allow))\n> return allow ? FSMONITOR_REASON_OK : FSMONITOR_REASON_REMOTE;\n>\n> return FSMONITOR_REASON_UNDECIDED; /* invented... */\n\nMakes sense. How about FSMONITOR_OVERRIDE_REQUIRED ? The error message\ncould then indicate that remote repos are normally unsupported but\nthat setting the fsmonitor.allowRemote flag overrides this behavior.\n\n> > +/*\n> > + * Check if the remote working directory is mounted via SMB\n> > + *\n> > + * For now, remote working directories are only supported via SMB mounts\n> > +*/\n>\n> \"*/\" -> \" */\".\n>\n> > +static enum fsmonitor_reason check_smb(wchar_t *wpath)\n> > +{\n> > + HANDLE h;\n> > + FILE_REMOTE_PROTOCOL_INFO proto_info;\n> > +\n> > + h = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,\n> > + FILE_FLAG_BACKUP_SEMANTICS, NULL);\n> > +\n> > + if (h == INVALID_HANDLE_VALUE) {\n> > + error(_(\"[GLE %ld] unable to open for read '%ls'\"),\n> > + GetLastError(), wpath);\n> > + return FSMONITOR_REASON_ERROR;\n> > + }\n> > +\n> > + if (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,\n> > + &proto_info, sizeof(proto_info))) {\n>\n> Overly deep indentation that unnecessarily causes an overly long\n> line. \"&\" in \"&proto_info\" should align with \"h\" in\n> \"...HandleEx(h,\", we use fixed-width font, and our tab-width is 8.\n>\n> The second line of CreateFileW() call is also overly indented and\n> may want to be fixed, but it does not overly extend to the right end\n> of the display (we aim to fit in 80-columns, as CodingGuideline says)\n> so it would be nice if it gets fixed, but it may be tolerated. This\n> one is not.\n>\n> > + error(_(\"[GLE %ld] unable to get protocol information for '%ls'\"),\n> > + GetLastError(), wpath);\n>\n> This line gets it right ;-)\n\nGot it. I'll check my editor settings too :-)\n\n> > + CloseHandle(h);\n> > + return FSMONITOR_REASON_ERROR;\n> > + }\n> > +\n> > + CloseHandle(h);\n> > +\n> > + if (proto_info.Protocol == WNNC_NET_SMB)\n> > + return FSMONITOR_REASON_OK;\n> > +\n> > + return FSMONITOR_REASON_ERROR;\n>\n> Is it? Shouldn't it be REASON_REMOTE, not ERROR? Unlike the\n> earlier case where you couldn't figure out the protocol information,\n> at this point you know you learned what protocol is in use, and it\n> is just that the protocol was not what you liked.\n\nYeah, I noticed that myself this morning.\n\n> > +}\n> > +\n> > /*\n> > * Remote working directories are problematic for FSMonitor.\n> > *\n> > @@ -76,6 +128,7 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)\n> > */\n> > static enum fsmonitor_reason check_remote(struct repository *r)\n> > {\n> > + enum fsmonitor_reason reason;\n> > wchar_t wpath[MAX_PATH];\n> > wchar_t wfullpath[MAX_PATH];\n> > size_t wlen;\n> > @@ -115,7 +168,11 @@ static enum fsmonitor_reason check_remote(struct repository *r)\n> > trace_printf_key(&trace_fsmonitor,\n> > \"check_remote('%s') true\",\n> > r->worktree);\n> > - return FSMONITOR_REASON_REMOTE;\n> > +\n> > + reason = check_smb(wfullpath);\n> > + if (reason != FSMONITOR_REASON_OK)\n> > + return reason;\n> > + return check_allow_remote(r);\n>\n> This does not fulfill the promise you made in front of\n> check_allow_remote(). As we saw, check_smb() returns something\n> other than REASON_OK when it is talking to a remote that is not SMB,\n> and when that happens, you are not giving check_allow_remote() a\n> chance to intervene and override. It should be more like\n>\n> reason = check_allow_remote(r);\n> if (reason == FSMONITOR_REASON_OK || reason == FSMONITOR_REASON_REMOTE)\n> return reason;\n> /*\n> * check_allow_remote() did not decide, so use the default\n> * based on the network filesystem.\n> */\n> return check_smb(wfullpath);\n>\n> I would think.\n\nIf we do as you suggest above, then fsmonitor.allowRemote=true would\noverride regardless of the protocol being used. The more conservative\npath is to only allow the override if SMB is being used.\n\nIt could perhaps be better written as:\n\nreason = check_allow_remote(r);\nif (reason != FSMONITOR_REASON_OK)\n return reason;\n\nreturn check_smb(wbfullpath);\n\n\n> > }\n> >\n> > return FSMONITOR_REASON_OK;\n> >\n> > base-commit: c50926e1f48891e2671e1830dbcd2912a4563450\n"},{"id":"461021","messageId":"xmqqbksrlvyb.fsf@gitster.g","threadId":"58283","inReplyTo":"CAMxJVdH3B2An7La9knM=QJojQ334O+Z2-tqNvqRZz2Eu6CV+-w@mail.gmail.com","subject":"Re: [PATCH] fsmonitor: option to allow fsmonitor to run against network-mounted repos","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-10T19:50:20Z","receivedAt":"2022-08-10T19:50:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric D writes:\n\n> Makes sense. How about FSMONITOR_OVERRIDE_REQUIRED ? The error message\n> could then indicate that remote repos are normally unsupported but\n> that setting the fsmonitor.allowRemote flag overrides this behavior.\n\nI actually think check_allow_remote() should be renamed to have\n\"config\" somewhere in its name, and return -1, 0 or 1 and not \"enum\nfsmonitor_reason\".\n\n\tstatic int check_config_allowremote(...)\n\t{\n\t\tint allow;\n\n\t\tif (repor_config_get_bool(..., &allow))\n\t\t\treturn allow;\n\t\treturn -1; /* undecided */\n\t}\n\nthen caller can do\n\n\tswitch (check_config_allowremote(...)) {\n\tcase 0: /* config overrides and disables */\n\t\treturn FSMONITOR_REASON_REMOTE;\n\tcase 1: /* config overrides and enables */\n\t\treturn FSMONITOR_REASON_OK;\n\tdefault:\n\t\tbreak; /* config has no opinion */\n\t}\n return check_smb(...);\n\n> If we do as you suggest above, then fsmonitor.allowRemote=true would\n> override regardless of the protocol being used.\n\nExactly. The code should not try to outsmart the user.\n\nIf the user says they wants to use it on a particular remote, even\nif you do not know if that particular remote system works, just let\nthem try and see if it works. If it does not, they can easily\ndisable, because the enabiling was a deliberate act by them in the\nfirst place. They know how to fix it.\n\nThanks.\n\n"},{"id":"461026","messageId":"CAMxJVdH-UbCZYp1hcvy4efEYd33jk0bFosJOtag5S4F9-m3-sg@mail.gmail.com","threadId":"58283","inReplyTo":"xmqqbksrlvyb.fsf@gitster.g","subject":"Re: [PATCH] fsmonitor: option to allow fsmonitor to run against network-mounted repos","fromName":"Eric D","fromEmail":"eric.decosta@gmail.com","sentAt":"2022-08-10T20:36:57Z","receivedAt":"2022-08-10T20:37:17Z","isPatch":true,"sender":{"key":"eric.decosta@gmail.com","avatar":null},"body":"On Wed, Aug 10, 2022 at 3:50 PM Junio C Hamano wrote:\n>\n> Eric D writes:\n>\n> > Makes sense. How about FSMONITOR_OVERRIDE_REQUIRED ? The error message\n> > could then indicate that remote repos are normally unsupported but\n> > that setting the fsmonitor.allowRemote flag overrides this behavior.\n>\n> I actually think check_allow_remote() should be renamed to have\n> \"config\" somewhere in its name, and return -1, 0 or 1 and not \"enum\n> fsmonitor_reason\".\n>\n> static int check_config_allowremote(...)\n> {\n> int allow;\n>\n> if (repor_config_get_bool(..., &allow))\n> return allow;\n> return -1; /* undecided */\n> }\n>\n> then caller can do\n>\n> switch (check_config_allowremote(...)) {\n> case 0: /* config overrides and disables */\n> return FSMONITOR_REASON_REMOTE;\n> case 1: /* config overrides and enables */\n> return FSMONITOR_REASON_OK;\n> default:\n> break; /* config has no opinion */\n> }\n> return check_smb(...);\n>\n> > If we do as you suggest above, then fsmonitor.allowRemote=true would\n> > override regardless of the protocol being used.\n>\n> Exactly. The code should not try to outsmart the user.\n>\n> If the user says they wants to use it on a particular remote, even\n> if you do not know if that particular remote system works, just let\n> them try and see if it works. If it does not, they can easily\n> disable, because the enabiling was a deliberate act by them in the\n> first place. They know how to fix it.\n>\n> Thanks.\n>\n\n100% agree with you, thanks.\n"},{"id":"461035","messageId":"CAMxJVdEvNKzwbHkRHcbd=_8yEuYZiNsNABVieU1tV5kna_nJ_g@mail.gmail.com","threadId":"58283","inReplyTo":"CAMxJVdH-UbCZYp1hcvy4efEYd33jk0bFosJOtag5S4F9-m3-sg@mail.gmail.com","subject":"Re: [PATCH] fsmonitor: option to allow fsmonitor to run against network-mounted repos","fromName":"Eric D","fromEmail":"eric.decosta@gmail.com","sentAt":"2022-08-10T21:30:00Z","receivedAt":"2022-08-10T21:30:18Z","isPatch":true,"sender":{"key":"eric.decosta@gmail.com","avatar":null},"body":"On Wed, Aug 10, 2022 at 4:36 PM Eric D wrote:\n>\n> On Wed, Aug 10, 2022 at 3:50 PM Junio C Hamano wrote:\n> >\n> > Eric D writes:\n> >\n> > > Makes sense. How about FSMONITOR_OVERRIDE_REQUIRED ? The error message\n> > > could then indicate that remote repos are normally unsupported but\n> > > that setting the fsmonitor.allowRemote flag overrides this behavior.\n> >\n> > I actually think check_allow_remote() should be renamed to have\n> > \"config\" somewhere in its name, and return -1, 0 or 1 and not \"enum\n> > fsmonitor_reason\".\n> >\n> > static int check_config_allowremote(...)\n> > {\n> > int allow;\n> >\n> > if (repor_config_get_bool(..., &allow))\n> > return allow;\n> > return -1; /* undecided */\n> > }\n> >\n> > then caller can do\n> >\n> > switch (check_config_allowremote(...)) {\n> > case 0: /* config overrides and disables */\n> > return FSMONITOR_REASON_REMOTE;\n> > case 1: /* config overrides and enables */\n> > return FSMONITOR_REASON_OK;\n> > default:\n> > break; /* config has no opinion */\n> > }\n> > return check_smb(...);\n> >\n> > > If we do as you suggest above, then fsmonitor.allowRemote=true would\n> > > override regardless of the protocol being used.\n> >\n> > Exactly. The code should not try to outsmart the user.\n> >\n> > If the user says they wants to use it on a particular remote, even\n> > if you do not know if that particular remote system works, just let\n> > them try and see if it works. If it does not, they can easily\n> > disable, because the enabiling was a deliberate act by them in the\n> > first place. They know how to fix it.\n> >\n> > Thanks.\n> >\n>\n> 100% agree with you, thanks.\n\nThe only thing that is somewhat gnawing at me is that just because the\nremote worktree is mounted via SMB is no guarantee that fsmonitor will\nwork correctly. In many (most?) cases it should, but who knows what\nsupport the filer server has.\n\nI think we should allow the user to override regardless - as you said\nlet the user try it. But, conservatively, just because SMB is there\nmay not be enough to let the monitor start without the explicit user\noverride. Being able to report on which protocol is being used could\nprovide useful diagnostics, but that's about it.\n"},{"id":"461038","messageId":"xmqqwnbfixnd.fsf@gitster.g","threadId":"58283","inReplyTo":"CAMxJVdEvNKzwbHkRHcbd=_8yEuYZiNsNABVieU1tV5kna_nJ_g@mail.gmail.com","subject":"Re: [PATCH] fsmonitor: option to allow fsmonitor to run against network-mounted repos","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-10T21:41:58Z","receivedAt":"2022-08-10T21:42:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric D writes:\n\n> The only thing that is somewhat gnawing at me is that just because the\n> remote worktree is mounted via SMB is no guarantee that fsmonitor will\n> work correctly. In many (most?) cases it should, but who knows what\n> support the filer server has.\n>\n> I think we should allow the user to override regardless - as you said\n> let the user try it. But, conservatively, just because SMB is there\n> may not be enough to let the monitor start without the explicit user\n> override. Being able to report on which protocol is being used could\n> provide useful diagnostics, but that's about it.\n\nI do not think anybody minds if the initial/first step would be to\nadd \"option to allow\" and do nothing else. No \"are we talking SMB?\"\ncheck, and just a simple \"by default we refuse going remote, but\nsince the user has an explicit opt-in configuration set, we allow\".\n\nThat is sufficient to let people gain experience using fsmonitor on\nWindows mounting from various filer implementations. And then the\nnext step, in one or two major releases down the road, may try to\ncheck what kind of filer we are on and see if it is one of the\n\"good\" ones to flip the default selectively.\n\n"},{"id":"461081","messageId":"7e67ce8c9449a2b5997cfe36e662898e3e00dbf0.1660233432.git.gitgitgadget@gmail.com","threadId":"58283","inReplyTo":"pull.1317.v2.git.1660233432.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] fsmonitor: option to allow fsmonitor to run against network-mounted repos","fromName":"Eric DeCosta via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-11T15:57:11Z","receivedAt":"2022-08-11T16:12:36Z","isPatch":true,"sender":{"key":"edecosta@mathworks.com","avatar":"https://avatars.githubusercontent.com/u/67609563?v=4"},"body":"From: Eric DeCosta \n\nThough perhaps not common, there are uses cases where users have large,\nnetwork-mounted repos. Having the ability to run fsmonitor against\nnetwork paths would benefit those users.\n\nMost modern Samba-based filers have the necessary support to enable\nfsmonitor on network-mounted repos. As a first step towards enabling\nfsmonitor to work against network-mounted repos, introduce a\nconfiguration option, 'fsmonitor.allowRemote'. Setting this option to\ntrue will override the default behavior (erroring-out) when a\nnetwork-mounted repo is detected by fsmonitor.\n\nAdditionally, as part of this first step, monitoring of network-mounted\nrepos will be restricted to those mounted over SMB regardless of the\nvalue of 'fsmonitor.allowRemote' until more extensive testing can be\nperformed.\n\nSigned-off-by: Eric DeCosta \n---\n compat/fsmonitor/fsm-settings-win32.c | 59 ++++++++++++++++++++++++++-\n 1 file changed, 58 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/fsmonitor/fsm-settings-win32.c b/compat/fsmonitor/fsm-settings-win32.c\nindex 907655720bb..d120e4710cf 100644\n--- a/compat/fsmonitor/fsm-settings-win32.c\n+++ b/compat/fsmonitor/fsm-settings-win32.c\n@@ -24,6 +24,58 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)\n \treturn FSMONITOR_REASON_OK;\n }\n \n+/*\n+ * Check if monitoring remote working directories is allowed.\n+ *\n+ * By default monitoring remote working directories is not allowed,\n+ * but users may override this behavior in enviroments where they\n+ * have proper support.\n+*/\n+static enum fsmonitor_reason check_allow_remote(struct repository *r)\n+{\n+\tint allow;\n+\n+\tif (repo_config_get_bool(r, \"fsmonitor.allowremote\", &allow) || !allow)\n+\t\treturn FSMONITOR_REASON_REMOTE;\n+\n+\treturn FSMONITOR_REASON_OK;\n+}\n+\n+/*\n+ * Check if the remote working directory is mounted via SMB\n+ *\n+ * For now, remote working directories are only supported via SMB mounts\n+*/\n+static enum fsmonitor_reason check_smb(wchar_t *wpath)\n+{\n+\tHANDLE h;\n+\tFILE_REMOTE_PROTOCOL_INFO proto_info;\n+\n+\th = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,\n+\t\t\t\t\tFILE_FLAG_BACKUP_SEMANTICS, NULL);\n+\n+\tif (h == INVALID_HANDLE_VALUE) {\n+\t\terror(_(\"[GLE %ld] unable to open for read '%ls'\"),\n+\t\t GetLastError(), wpath);\n+\t\treturn FSMONITOR_REASON_ERROR;\n+\t}\n+\n+\tif (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,\n+\t\t\t\t\t\t\t\t\t&proto_info, sizeof(proto_info))) {\n+\t\terror(_(\"[GLE %ld] unable to get protocol information for '%ls'\"),\n+\t\t GetLastError(), wpath);\n+\t\tCloseHandle(h);\n+\t\treturn FSMONITOR_REASON_ERROR;\n+\t}\n+\n+\tCloseHandle(h);\n+\n+\tif (proto_info.Protocol == WNNC_NET_SMB)\n+\t\treturn FSMONITOR_REASON_OK;\n+\n+\treturn FSMONITOR_REASON_ERROR;\n+}\n+\n /*\n * Remote working directories are problematic for FSMonitor.\n *\n@@ -76,6 +128,7 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)\n */\n static enum fsmonitor_reason check_remote(struct repository *r)\n {\n+\tenum fsmonitor_reason reason;\n \twchar_t wpath[MAX_PATH];\n \twchar_t wfullpath[MAX_PATH];\n \tsize_t wlen;\n@@ -115,7 +168,11 @@ static enum fsmonitor_reason check_remote(struct repository *r)\n \t\ttrace_printf_key(&trace_fsmonitor,\n \t\t\t\t \"check_remote('%s') true\",\n \t\t\t\t r->worktree);\n-\t\treturn FSMONITOR_REASON_REMOTE;\n+\n+\t\treason = check_smb(wfullpath);\n+\t\tif (reason != FSMONITOR_REASON_OK)\n+\t\t\treturn reason;\n+\t\treturn check_allow_remote(r);\n \t}\n \n \treturn FSMONITOR_REASON_OK;\n-- \ngitgitgadget\n\n"},{"id":"461082","messageId":"7a071c9e6be68b58306582dbac5952a5b1bcbc6a.1660233432.git.gitgitgadget@gmail.com","threadId":"58283","inReplyTo":"pull.1317.v2.git.1660233432.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] fsmonitor.allowRemote now overrides default behavior","fromName":"Eric DeCosta via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-11T15:57:12Z","receivedAt":"2022-08-11T16:12:38Z","isPatch":true,"sender":{"key":"edecosta@mathworks.com","avatar":"https://avatars.githubusercontent.com/u/67609563?v=4"},"body":"From: Eric DeCosta \n\nReworked the logic around fsmonitor.allowRemote such that if\nallowRemote is set it will determine if monitoring the remote\nworktree is allowed.\n\nGet remote protocoal information; if this fails report an error else\nprint it if tracing is enabled.\n\nFixed fomratting issues.\n\nSigned-off-by: Eric DeCosta \n---\n compat/fsmonitor/fsm-settings-win32.c | 57 ++++++++++++++++-----------\n 1 file changed, 33 insertions(+), 24 deletions(-)\n\ndiff --git a/compat/fsmonitor/fsm-settings-win32.c b/compat/fsmonitor/fsm-settings-win32.c\nindex d120e4710cf..32c0695c6c1 100644\n--- a/compat/fsmonitor/fsm-settings-win32.c\n+++ b/compat/fsmonitor/fsm-settings-win32.c\n@@ -27,53 +27,55 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)\n /*\n * Check if monitoring remote working directories is allowed.\n *\n- * By default monitoring remote working directories is not allowed,\n- * but users may override this behavior in enviroments where they\n- * have proper support.\n-*/\n-static enum fsmonitor_reason check_allow_remote(struct repository *r)\n+ * By default, monitoring remote working directories is\n+ * disabled unless on a network filesystem that is known to\n+ * behave well. Users may override this behavior in enviroments where\n+ * they have proper support.\n+ */\n+static int check_config_allowremote(struct repository *r)\n {\n \tint allow;\n \n-\tif (repo_config_get_bool(r, \"fsmonitor.allowremote\", &allow) || !allow)\n-\t\treturn FSMONITOR_REASON_REMOTE;\n+\tif (!repo_config_get_bool(r, \"fsmonitor.allowremote\", &allow))\n+\t\treturn allow;\n \n-\treturn FSMONITOR_REASON_OK;\n+\treturn -1; /* fsmonitor.allowremote not set */\n }\n \n /*\n- * Check if the remote working directory is mounted via SMB\n+ * Check remote working directory protocol.\n *\n- * For now, remote working directories are only supported via SMB mounts\n-*/\n-static enum fsmonitor_reason check_smb(wchar_t *wpath)\n+ * Error if client machine cannot get remote protocol information.\n+ */\n+static void check_remote_protocol(wchar_t *wpath)\n {\n \tHANDLE h;\n \tFILE_REMOTE_PROTOCOL_INFO proto_info;\n \n \th = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,\n-\t\t\t\t\tFILE_FLAG_BACKUP_SEMANTICS, NULL);\n+\t\t\tFILE_FLAG_BACKUP_SEMANTICS, NULL);\n \n \tif (h == INVALID_HANDLE_VALUE) {\n \t\terror(_(\"[GLE %ld] unable to open for read '%ls'\"),\n \t\t GetLastError(), wpath);\n-\t\treturn FSMONITOR_REASON_ERROR;\n+\t\treturn;\n \t}\n \n \tif (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,\n-\t\t\t\t\t\t\t\t\t&proto_info, sizeof(proto_info))) {\n+\t\t&proto_info, sizeof(proto_info))) {\n \t\terror(_(\"[GLE %ld] unable to get protocol information for '%ls'\"),\n \t\t GetLastError(), wpath);\n \t\tCloseHandle(h);\n-\t\treturn FSMONITOR_REASON_ERROR;\n+\t\treturn;\n \t}\n \n \tCloseHandle(h);\n \n-\tif (proto_info.Protocol == WNNC_NET_SMB)\n-\t\treturn FSMONITOR_REASON_OK;\n+\ttrace_printf_key(&trace_fsmonitor,\n+\t\t\t\t\"check_remote_protocol('%ls') remote protocol %#8.8lx\",\n+\t\t\t\twpath, proto_info.Protocol);\n \n-\treturn FSMONITOR_REASON_ERROR;\n+\treturn;\n }\n \n /*\n@@ -128,7 +130,6 @@ static enum fsmonitor_reason check_smb(wchar_t *wpath)\n */\n static enum fsmonitor_reason check_remote(struct repository *r)\n {\n-\tenum fsmonitor_reason reason;\n \twchar_t wpath[MAX_PATH];\n \twchar_t wfullpath[MAX_PATH];\n \tsize_t wlen;\n@@ -169,10 +170,18 @@ static enum fsmonitor_reason check_remote(struct repository *r)\n \t\t\t\t \"check_remote('%s') true\",\n \t\t\t\t r->worktree);\n \n-\t\treason = check_smb(wfullpath);\n-\t\tif (reason != FSMONITOR_REASON_OK)\n-\t\t\treturn reason;\n-\t\treturn check_allow_remote(r);\n+\t\tcheck_remote_protocol(wfullpath);\n+\n+\t\tswitch (check_config_allowremote(r)) {\n+\t\tcase 0: /* config overrides and disables */\n+\t\t\treturn FSMONITOR_REASON_REMOTE;\n+\t\tcase 1: /* config overrides and enables */\n+\t\t\treturn FSMONITOR_REASON_OK;\n+\t\tdefault:\n+\t\t\tbreak; /* config has no opinion */\n+\t\t}\n+\n+\t\treturn FSMONITOR_REASON_REMOTE;\n \t}\n \n \treturn FSMONITOR_REASON_OK;\n-- \ngitgitgadget\n"},{"id":"461083","messageId":"pull.1317.v2.git.1660233432.gitgitgadget@gmail.com","threadId":"58283","inReplyTo":"pull.1317.git.1660067049965.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] Option to allow fsmonitor to run against repos on network file systems","fromName":"Eric DeCosta via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-11T15:57:10Z","receivedAt":"2022-08-11T16:12:41Z","isPatch":true,"sender":{"key":"edecosta@mathworks.com","avatar":"https://avatars.githubusercontent.com/u/67609563?v=4"},"body":"cc: Eric D eric.decosta@gmail.com\n\nEric DeCosta (2):\n fsmonitor: option to allow fsmonitor to run against network-mounted\n repos\n fsmonitor.allowRemote now overrides default behavior\n\n compat/fsmonitor/fsm-settings-win32.c | 66 +++++++++++++++++++++++++++\n 1 file changed, 66 insertions(+)\n\n\nbase-commit: c50926e1f48891e2671e1830dbcd2912a4563450\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1317%2Fedecosta-mw%2Fmaster-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1317/edecosta-mw/master-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1317\n\nRange-diff vs v1:\n\n 1: 7e67ce8c944 = 1: 7e67ce8c944 fsmonitor: option to allow fsmonitor to run against network-mounted repos\n -: ----------- > 2: 7a071c9e6be fsmonitor.allowRemote now overrides default behavior\n\n-- \ngitgitgadget\n"},{"id":"461086","messageId":"xmqqtu6ig1s5.fsf@gitster.g","threadId":"58283","inReplyTo":"7a071c9e6be68b58306582dbac5952a5b1bcbc6a.1660233432.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/2] fsmonitor.allowRemote now overrides default behavior","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-11T16:53:14Z","receivedAt":"2022-08-11T17:13:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Eric DeCosta via GitGitGadget\" writes:\n\n> From: Eric DeCosta \n>\n> Reworked the logic around fsmonitor.allowRemote such that if\n> allowRemote is set it will determine if monitoring the remote\n> worktree is allowed.\n>\n> Get remote protocoal information; if this fails report an error else\n> print it if tracing is enabled.\n>\n> Fixed fomratting issues.\n\nThe end result (i.e. HEAD^{tree} of the branch you developed these\ntwo patches on) may be good (I haven't checked), but it is not how\nwe fix problems in an earlier attempt in this project by keeping the\nfaulty commit(s) on the bottom and piling \"oops, that was wrong, and\nhere is a fix-up\" commit(s) on top.\n\nOnce you are happy with the end result, use \"rebase -i\" to clean-up\nthe history leading to that end result. The goal is to pretend as\nif you were a perfect human, more perfect than your actual self, who\ncame up with an ideal patch without making mistakes that need to be\ncorrected with \"fix-up\" commits. In this particular case, you'd\nmost likely want to end up with a single commit, so squashing them\ntogether and fixing up the log message might be all you need to do.\nWhen you work on a more elaborate topic, you may also want to split\nor reorder original commits to present a logical progression towards\nthe end result. \"rebase -i\" is a good tool to help you do so.\n\nI am not a user of GitGitGadget myself, but if I recall correctly,\nyou should be able to force-push the result of such a clean-up to\nupdate the pull-request, to trigger a new iteration to be sent to\nthe list.\n\nThanks.\n"},{"id":"461089","messageId":"CAMxJVdEV=rSXtM-nagvtMPdArkvQgoNauaQb1sk0CL3sPSvKmw@mail.gmail.com","threadId":"58283","inReplyTo":"xmqqtu6ig1s5.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] fsmonitor.allowRemote now overrides default behavior","fromName":"Eric D","fromEmail":"eric.decosta@gmail.com","sentAt":"2022-08-11T17:49:07Z","receivedAt":"2022-08-11T17:49:22Z","isPatch":true,"sender":{"key":"eric.decosta@gmail.com","avatar":null},"body":"Well, needless to say I wasn't expecting GitGitGadget to do what it\ndid.I had squashed things down to just two commits and forced-pushed\nthe second commit thinking that just the relevant stuff from the\nsecond commit would show up in the next patch. Obviously that didn't\nhappen. Sorry about that.\n\nI can certainly squash it down to just one commit and force-push that.\n\nOn Thu, Aug 11, 2022 at 1:17 PM Junio C Hamano wrote:\n>\n> \"Eric DeCosta via GitGitGadget\" writes:\n>\n> > From: Eric DeCosta \n> >\n> > Reworked the logic around fsmonitor.allowRemote such that if\n> > allowRemote is set it will determine if monitoring the remote\n> > worktree is allowed.\n> >\n> > Get remote protocoal information; if this fails report an error else\n> > print it if tracing is enabled.\n> >\n> > Fixed fomratting issues.\n>\n> The end result (i.e. HEAD^{tree} of the branch you developed these\n> two patches on) may be good (I haven't checked), but it is not how\n> we fix problems in an earlier attempt in this project by keeping the\n> faulty commit(s) on the bottom and piling \"oops, that was wrong, and\n> here is a fix-up\" commit(s) on top.\n>\n> Once you are happy with the end result, use \"rebase -i\" to clean-up\n> the history leading to that end result. The goal is to pretend as\n> if you were a perfect human, more perfect than your actual self, who\n> came up with an ideal patch without making mistakes that need to be\n> corrected with \"fix-up\" commits. In this particular case, you'd\n> most likely want to end up with a single commit, so squashing them\n> together and fixing up the log message might be all you need to do.\n> When you work on a more elaborate topic, you may also want to split\n> or reorder original commits to present a logical progression towards\n> the end result. \"rebase -i\" is a good tool to help you do so.\n>\n> I am not a user of GitGitGadget myself, but if I recall correctly,\n> you should be able to force-push the result of such a clean-up to\n> update the pull-request, to trigger a new iteration to be sent to\n> the list.\n>\n> Thanks.\n"},{"id":"461091","messageId":"xmqqpmh6fyzc.fsf@gitster.g","threadId":"58283","inReplyTo":"CAMxJVdEV=rSXtM-nagvtMPdArkvQgoNauaQb1sk0CL3sPSvKmw@mail.gmail.com","subject":"Re: [PATCH v2 2/2] fsmonitor.allowRemote now overrides default behavior","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-11T17:53:43Z","receivedAt":"2022-08-11T17:53:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric D writes:\n\n> Well, needless to say I wasn't expecting GitGitGadget to do what it\n> did.I had squashed things down to just two commits and forced-pushed\n> the second commit thinking that just the relevant stuff from the\n> second commit would show up in the next patch. Obviously that didn't\n> happen. Sorry about that.\n\nOh, sorry to hear that. If your ideal \"logical progression\" needs\ntwo commits, then please do present the series that way. What GGG\nsent out was apparently not that (i.e. the same one from v1 with\nfull of fix-ups for it in 2/2).\n\n"},{"id":"461092","messageId":"CAMxJVdFgMXf1XM68W_j4XfAfNzxPNc9NbAUaemtg-teqgvJ5mw@mail.gmail.com","threadId":"58283","inReplyTo":"xmqqpmh6fyzc.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] fsmonitor.allowRemote now overrides default behavior","fromName":"Eric D","fromEmail":"eric.decosta@gmail.com","sentAt":"2022-08-11T17:58:12Z","receivedAt":"2022-08-11T17:58:27Z","isPatch":true,"sender":{"key":"eric.decosta@gmail.com","avatar":null},"body":"Given that, in the end, the change is rather small and involves just\none file, having it be just one commit is fine. Perhaps my next lesson\nto learn is to generate and send the patch sets myself, but that will\nbe for another time.\n\nThank you for all your patience, it makes a total noob like me feel welcome.\n\nOn Thu, Aug 11, 2022 at 1:53 PM Junio C Hamano wrote:\n>\n> Eric D writes:\n>\n> > Well, needless to say I wasn't expecting GitGitGadget to do what it\n> > did.I had squashed things down to just two commits and forced-pushed\n> > the second commit thinking that just the relevant stuff from the\n> > second commit would show up in the next patch. Obviously that didn't\n> > happen. Sorry about that.\n>\n> Oh, sorry to hear that. If your ideal \"logical progression\" needs\n> two commits, then please do present the series that way. What GGG\n> sent out was apparently not that (i.e. the same one from v1 with\n> full of fix-ups for it in 2/2).\n>\n"},{"id":"461095","messageId":"pull.1317.v3.git.1660242752495.gitgitgadget@gmail.com","threadId":"58283","inReplyTo":"pull.1317.v2.git.1660233432.gitgitgadget@gmail.com","subject":"[PATCH v3] fsmonitor: option to allow fsmonitor to run against network-mounted repos","fromName":"Eric DeCosta via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-11T18:32:32Z","receivedAt":"2022-08-11T18:32:39Z","isPatch":true,"sender":{"key":"edecosta@mathworks.com","avatar":"https://avatars.githubusercontent.com/u/67609563?v=4"},"body":"From: Eric DeCosta \n\nThough perhaps not common, there are uses cases where users have large,\nnetwork-mounted repos. Having the ability to run fsmonitor against\nnetwork paths would benefit those users.\n\nMost modern Samba-based filers have the necessary support to enable\nfsmonitor on network-mounted repos. As a first step towards enabling\nfsmonitor to work against network-mounted repos, introduce a\nconfiguration option, 'fsmonitor.allowRemote'. Setting this option to\ntrue will override the default behavior (erroring-out) when a\nnetwork-mounted repo is detected by fsmonitor.\n\nSigned-off-by: Eric DeCosta \n---\n Option to allow fsmonitor to run against repos on network file systems\n \n cc: Eric D eric.decosta@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1317%2Fedecosta-mw%2Fmaster-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1317/edecosta-mw/master-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1317\n\nRange-diff vs v2:\n\n 1: 7e67ce8c944 ! 1: 6c5f176cbee fsmonitor: option to allow fsmonitor to run against network-mounted repos\n @@ Commit message\n true will override the default behavior (erroring-out) when a\n network-mounted repo is detected by fsmonitor.\n \n - Additionally, as part of this first step, monitoring of network-mounted\n - repos will be restricted to those mounted over SMB regardless of the\n - value of 'fsmonitor.allowRemote' until more extensive testing can be\n - performed.\n -\n Signed-off-by: Eric DeCosta \n \n ## compat/fsmonitor/fsm-settings-win32.c ##\n @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4gi\n +/*\n + * Check if monitoring remote working directories is allowed.\n + *\n -+ * By default monitoring remote working directories is not allowed,\n -+ * but users may override this behavior in enviroments where they\n -+ * have proper support.\n -+*/\n -+static enum fsmonitor_reason check_allow_remote(struct repository *r)\n ++ * By default, monitoring remote working directories is\n ++ * disabled unless on a network filesystem that is known to\n ++ * behave well. Users may override this behavior in enviroments where\n ++ * they have proper support.\n ++ */\n ++static int check_config_allowremote(struct repository *r)\n +{\n +\tint allow;\n +\n -+\tif (repo_config_get_bool(r, \"fsmonitor.allowremote\", &allow) || !allow)\n -+\t\treturn FSMONITOR_REASON_REMOTE;\n ++\tif (!repo_config_get_bool(r, \"fsmonitor.allowremote\", &allow))\n ++\t\treturn allow;\n +\n -+\treturn FSMONITOR_REASON_OK;\n ++\treturn -1; /* fsmonitor.allowremote not set */\n +}\n +\n +/*\n -+ * Check if the remote working directory is mounted via SMB\n ++ * Check remote working directory protocol.\n + *\n -+ * For now, remote working directories are only supported via SMB mounts\n -+*/\n -+static enum fsmonitor_reason check_smb(wchar_t *wpath)\n ++ * Error if client machine cannot get remote protocol information.\n ++ */\n ++static void check_remote_protocol(wchar_t *wpath)\n +{\n +\tHANDLE h;\n +\tFILE_REMOTE_PROTOCOL_INFO proto_info;\n +\n +\th = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,\n -+\t\t\t\t\tFILE_FLAG_BACKUP_SEMANTICS, NULL);\n ++\t\t\tFILE_FLAG_BACKUP_SEMANTICS, NULL);\n +\n +\tif (h == INVALID_HANDLE_VALUE) {\n +\t\terror(_(\"[GLE %ld] unable to open for read '%ls'\"),\n +\t\t GetLastError(), wpath);\n -+\t\treturn FSMONITOR_REASON_ERROR;\n ++\t\treturn;\n +\t}\n +\n +\tif (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,\n -+\t\t\t\t\t\t\t\t\t&proto_info, sizeof(proto_info))) {\n ++\t\t&proto_info, sizeof(proto_info))) {\n +\t\terror(_(\"[GLE %ld] unable to get protocol information for '%ls'\"),\n +\t\t GetLastError(), wpath);\n +\t\tCloseHandle(h);\n -+\t\treturn FSMONITOR_REASON_ERROR;\n ++\t\treturn;\n +\t}\n +\n +\tCloseHandle(h);\n +\n -+\tif (proto_info.Protocol == WNNC_NET_SMB)\n -+\t\treturn FSMONITOR_REASON_OK;\n ++\ttrace_printf_key(&trace_fsmonitor,\n ++\t\t\t\t\"check_remote_protocol('%ls') remote protocol %#8.8lx\",\n ++\t\t\t\twpath, proto_info.Protocol);\n +\n -+\treturn FSMONITOR_REASON_ERROR;\n ++\treturn;\n +}\n +\n /*\n * Remote working directories are problematic for FSMonitor.\n *\n -@@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4git(struct repository *r)\n - */\n - static enum fsmonitor_reason check_remote(struct repository *r)\n - {\n -+\tenum fsmonitor_reason reason;\n - \twchar_t wpath[MAX_PATH];\n - \twchar_t wfullpath[MAX_PATH];\n - \tsize_t wlen;\n @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_remote(struct repository *r)\n \t\ttrace_printf_key(&trace_fsmonitor,\n \t\t\t\t \"check_remote('%s') true\",\n \t\t\t\t r->worktree);\n --\t\treturn FSMONITOR_REASON_REMOTE;\n +\n -+\t\treason = check_smb(wfullpath);\n -+\t\tif (reason != FSMONITOR_REASON_OK)\n -+\t\t\treturn reason;\n -+\t\treturn check_allow_remote(r);\n ++\t\tcheck_remote_protocol(wfullpath);\n ++\n ++\t\tswitch (check_config_allowremote(r)) {\n ++\t\tcase 0: /* config overrides and disables */\n ++\t\t\treturn FSMONITOR_REASON_REMOTE;\n ++\t\tcase 1: /* config overrides and enables */\n ++\t\t\treturn FSMONITOR_REASON_OK;\n ++\t\tdefault:\n ++\t\t\tbreak; /* config has no opinion */\n ++\t\t}\n ++\n + \t\treturn FSMONITOR_REASON_REMOTE;\n \t}\n \n - \treturn FSMONITOR_REASON_OK;\n 2: 7a071c9e6be < -: ----------- fsmonitor.allowRemote now overrides default behavior\n\n\n compat/fsmonitor/fsm-settings-win32.c | 66 +++++++++++++++++++++++++++\n 1 file changed, 66 insertions(+)\n\ndiff --git a/compat/fsmonitor/fsm-settings-win32.c b/compat/fsmonitor/fsm-settings-win32.c\nindex 907655720bb..32c0695c6c1 100644\n--- a/compat/fsmonitor/fsm-settings-win32.c\n+++ b/compat/fsmonitor/fsm-settings-win32.c\n@@ -24,6 +24,60 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)\n \treturn FSMONITOR_REASON_OK;\n }\n \n+/*\n+ * Check if monitoring remote working directories is allowed.\n+ *\n+ * By default, monitoring remote working directories is\n+ * disabled unless on a network filesystem that is known to\n+ * behave well. Users may override this behavior in enviroments where\n+ * they have proper support.\n+ */\n+static int check_config_allowremote(struct repository *r)\n+{\n+\tint allow;\n+\n+\tif (!repo_config_get_bool(r, \"fsmonitor.allowremote\", &allow))\n+\t\treturn allow;\n+\n+\treturn -1; /* fsmonitor.allowremote not set */\n+}\n+\n+/*\n+ * Check remote working directory protocol.\n+ *\n+ * Error if client machine cannot get remote protocol information.\n+ */\n+static void check_remote_protocol(wchar_t *wpath)\n+{\n+\tHANDLE h;\n+\tFILE_REMOTE_PROTOCOL_INFO proto_info;\n+\n+\th = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,\n+\t\t\tFILE_FLAG_BACKUP_SEMANTICS, NULL);\n+\n+\tif (h == INVALID_HANDLE_VALUE) {\n+\t\terror(_(\"[GLE %ld] unable to open for read '%ls'\"),\n+\t\t GetLastError(), wpath);\n+\t\treturn;\n+\t}\n+\n+\tif (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,\n+\t\t&proto_info, sizeof(proto_info))) {\n+\t\terror(_(\"[GLE %ld] unable to get protocol information for '%ls'\"),\n+\t\t GetLastError(), wpath);\n+\t\tCloseHandle(h);\n+\t\treturn;\n+\t}\n+\n+\tCloseHandle(h);\n+\n+\ttrace_printf_key(&trace_fsmonitor,\n+\t\t\t\t\"check_remote_protocol('%ls') remote protocol %#8.8lx\",\n+\t\t\t\twpath, proto_info.Protocol);\n+\n+\treturn;\n+}\n+\n /*\n * Remote working directories are problematic for FSMonitor.\n *\n@@ -115,6 +169,18 @@ static enum fsmonitor_reason check_remote(struct repository *r)\n \t\ttrace_printf_key(&trace_fsmonitor,\n \t\t\t\t \"check_remote('%s') true\",\n \t\t\t\t r->worktree);\n+\n+\t\tcheck_remote_protocol(wfullpath);\n+\n+\t\tswitch (check_config_allowremote(r)) {\n+\t\tcase 0: /* config overrides and disables */\n+\t\t\treturn FSMONITOR_REASON_REMOTE;\n+\t\tcase 1: /* config overrides and enables */\n+\t\t\treturn FSMONITOR_REASON_OK;\n+\t\tdefault:\n+\t\t\tbreak; /* config has no opinion */\n+\t\t}\n+\n \t\treturn FSMONITOR_REASON_REMOTE;\n \t}\n \n\nbase-commit: c50926e1f48891e2671e1830dbcd2912a4563450\n-- \ngitgitgadget\n"},{"id":"461097","messageId":"xmqqh72iefsn.fsf@gitster.g","threadId":"58283","inReplyTo":"pull.1317.v3.git.1660242752495.gitgitgadget@gmail.com","subject":"Re: [PATCH v3] fsmonitor: option to allow fsmonitor to run against network-mounted repos","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-11T19:33:28Z","receivedAt":"2022-08-11T19:33:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Eric DeCosta via GitGitGadget\" writes:\n\n> From: Eric DeCosta \n>\n> Though perhaps not common, there are uses cases where users have large,\n> network-mounted repos. Having the ability to run fsmonitor against\n> network paths would benefit those users.\n>\n> Most modern Samba-based filers have the necessary support to enable\n> fsmonitor on network-mounted repos. As a first step towards enabling\n> fsmonitor to work against network-mounted repos, introduce a\n> configuration option, 'fsmonitor.allowRemote'. Setting this option to\n> true will override the default behavior (erroring-out) when a\n> network-mounted repo is detected by fsmonitor.\n>\n> Signed-off-by: Eric DeCosta \n> ---\n> ...\n> compat/fsmonitor/fsm-settings-win32.c | 66 +++++++++++++++++++++++++++\n> 1 file changed, 66 insertions(+)\n>\n> diff --git a/compat/fsmonitor/fsm-settings-win32.c b/compat/fsmonitor/fsm-settings-win32.c\n> index 907655720bb..32c0695c6c1 100644\n> --- a/compat/fsmonitor/fsm-settings-win32.c\n> +++ b/compat/fsmonitor/fsm-settings-win32.c\n> @@ -24,6 +24,60 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)\n> \treturn FSMONITOR_REASON_OK;\n> }\n> \n> +/*\n> + * Check if monitoring remote working directories is allowed.\n> + *\n> + * By default, monitoring remote working directories is\n> + * disabled unless on a network filesystem that is known to\n> + * behave well. Users may override this behavior in enviroments where\n> + * they have proper support.\n> + */\n\nAfter applying this patch, \"unless on a network filesystem ...\" part\nis not exactly in effect yet; we could say that we start with no\nknown-to-behave-well network filesystems, but we can then update the\nabove comment when we start to know of at least one good one.\n\n> +/*\n> + * Check remote working directory protocol.\n> + *\n> + * Error if client machine cannot get remote protocol information.\n> + */\n\nGood, but void means that the caller of this function does not know\nwhen we detected an error. Perhaps return -1 on error, return 0 on\n\"not error\", so that we can return 1 when we learn to recognize\n\"known to behave well\" network filesystem to tell the caller?\n\nThat is,\n\n> +static void check_remote_protocol(wchar_t *wpath)\n\n\"void\" -> \"int\"\n\n> +{\n> +\tHANDLE h;\n> +\tFILE_REMOTE_PROTOCOL_INFO proto_info;\n> +\n> +\th = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,\n> +\t\t\tFILE_FLAG_BACKUP_SEMANTICS, NULL);\n> +\n> +\tif (h == INVALID_HANDLE_VALUE) {\n> +\t\terror(_(\"[GLE %ld] unable to open for read '%ls'\"),\n> +\t\t GetLastError(), wpath);\n> +\t\treturn;\n\n\"return\" -> \"return -1\"\n\n> +\t}\n> +\n> +\tif (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,\n> +\t\t&proto_info, sizeof(proto_info))) {\n> +\t\terror(_(\"[GLE %ld] unable to get protocol information for '%ls'\"),\n> +\t\t GetLastError(), wpath);\n> +\t\tCloseHandle(h);\n> +\t\treturn;\n\n\"return\" -> \"return -1\"\n\n> +\t}\n> +\n> +\tCloseHandle(h);\n> +\n> +\ttrace_printf_key(&trace_fsmonitor,\n> +\t\t\t\t\"check_remote_protocol('%ls') remote protocol %#8.8lx\",\n> +\t\t\t\twpath, proto_info.Protocol);\n> +\n> +\treturn;\n\n\"return\" -> \"return 0\" (or \"-1\")\n\n> +}\n> +\n> /*\n> * Remote working directories are problematic for FSMonitor.\n> *\n> @@ -115,6 +169,18 @@ static enum fsmonitor_reason check_remote(struct repository *r)\n> \t\ttrace_printf_key(&trace_fsmonitor,\n> \t\t\t\t \"check_remote('%s') true\",\n> \t\t\t\t r->worktree);\n> +\n> +\t\tcheck_remote_protocol(wfullpath);\n\nAnd here\n\n\t\tret = check_remote_protocol(wfullpath);\n\t\tif (ret < 0)\n\t\t\t/* definitely an error */\n\t\t\treturn FSMONITOR_REASON_ERROR;\n\nand then we can fall thru the non-error case below. We'd of course\nneed to declare \"int ret\" at the beginning of the function.\n\n> +\t\tswitch (check_config_allowremote(r)) {\n> +\t\tcase 0: /* config overrides and disables */\n> +\t\t\treturn FSMONITOR_REASON_REMOTE;\n> +\t\tcase 1: /* config overrides and enables */\n> +\t\t\treturn FSMONITOR_REASON_OK;\n> +\t\tdefault:\n> +\t\t\tbreak; /* config has no opinion */\n> +\t\t}\n> +\n> \t\treturn FSMONITOR_REASON_REMOTE;\n> \t}\n\nIn the future, when this \"first step\" graduates to the upcoming\nrelease, we may want to have a follow-up enhancement patch that\nchanges the code like so:\n\n * we recognize ones like SMB in check_remote_protocol() as \"known\n to be good\", and return 1 from there\n\n * after the \"switch\" above determies that the configuration file\n does not have any opinion, instead of unconditionally returning\n REASON_REMOTE to refuse the request, pay attention to \"ret\", e.g.\n something like\n\n-\treturn FSMONITOR_REASON_REMOTE;\n+\tif (!ret)\n+\t\treturn FSMONITOR_REASON_REMOTE;\n+\telse /* known to be good ones */\n+\t\treturn FSMONITOR_REASON_OK;\n\nWhen we do so, we'd resurrect the \"unless on a network filesystem\nthat is known to behave well\" comment. What this last part does is\nexactly that.\n\nThanks.\n"},{"id":"461106","messageId":"pull.1317.v4.git.1660262231357.gitgitgadget@gmail.com","threadId":"58283","inReplyTo":"pull.1317.v3.git.1660242752495.gitgitgadget@gmail.com","subject":"[PATCH v4] fsmonitor: option to allow fsmonitor to run against network-mounted repos","fromName":"Eric DeCosta via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-11T23:57:11Z","receivedAt":"2022-08-11T23:57:17Z","isPatch":true,"sender":{"key":"edecosta@mathworks.com","avatar":"https://avatars.githubusercontent.com/u/67609563?v=4"},"body":"From: Eric DeCosta \n\nThough perhaps not common, there are uses cases where users have large,\nnetwork-mounted repos. Having the ability to run fsmonitor against\nnetwork paths would benefit those users.\n\nMost modern Samba-based filers have the necessary support to enable\nfsmonitor on network-mounted repos. As a first step towards enabling\nfsmonitor to work against network-mounted repos, introduce a\nconfiguration option, 'fsmonitor.allowRemote'. Setting this option to\ntrue will override the default behavior (erroring-out) when a\nnetwork-mounted repo is detected by fsmonitor.\n\nSigned-off-by: Eric DeCosta \n---\n Option to allow fsmonitor to run against repos on network file systems\n \n cc: Eric D eric.decosta@gmail.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1317%2Fedecosta-mw%2Fmaster-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1317/edecosta-mw/master-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/1317\n\nRange-diff vs v3:\n\n 1: 6c5f176cbee ! 1: 058dc400c8a fsmonitor: option to allow fsmonitor to run against network-mounted repos\n @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4gi\n + * Check if monitoring remote working directories is allowed.\n + *\n + * By default, monitoring remote working directories is\n -+ * disabled unless on a network filesystem that is known to\n -+ * behave well. Users may override this behavior in enviroments where\n ++ * disabled. Users may override this behavior in enviroments where\n + * they have proper support.\n + */\n +static int check_config_allowremote(struct repository *r)\n @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4gi\n + *\n + * Error if client machine cannot get remote protocol information.\n + */\n -+static void check_remote_protocol(wchar_t *wpath)\n ++static int check_remote_protocol(wchar_t *wpath)\n +{\n +\tHANDLE h;\n +\tFILE_REMOTE_PROTOCOL_INFO proto_info;\n @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4gi\n +\tif (h == INVALID_HANDLE_VALUE) {\n +\t\terror(_(\"[GLE %ld] unable to open for read '%ls'\"),\n +\t\t GetLastError(), wpath);\n -+\t\treturn;\n ++\t\treturn -1;\n +\t}\n +\n +\tif (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,\n @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4gi\n +\t\terror(_(\"[GLE %ld] unable to get protocol information for '%ls'\"),\n +\t\t GetLastError(), wpath);\n +\t\tCloseHandle(h);\n -+\t\treturn;\n ++\t\treturn -1;\n +\t}\n +\n +\tCloseHandle(h);\n @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4gi\n +\t\t\t\t\"check_remote_protocol('%ls') remote protocol %#8.8lx\",\n +\t\t\t\twpath, proto_info.Protocol);\n +\n -+\treturn;\n ++\treturn 0;\n +}\n +\n /*\n * Remote working directories are problematic for FSMonitor.\n *\n +@@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4git(struct repository *r)\n + */\n + static enum fsmonitor_reason check_remote(struct repository *r)\n + {\n ++\tint ret;\n + \twchar_t wpath[MAX_PATH];\n + \twchar_t wfullpath[MAX_PATH];\n + \tsize_t wlen;\n @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_remote(struct repository *r)\n \t\ttrace_printf_key(&trace_fsmonitor,\n \t\t\t\t \"check_remote('%s') true\",\n \t\t\t\t r->worktree);\n +\n -+\t\tcheck_remote_protocol(wfullpath);\n ++\t\tret = check_remote_protocol(wfullpath);\n ++\t\tif (ret < 0)\n ++\t\t\treturn FSMONITOR_REASON_ERROR;\n +\n +\t\tswitch (check_config_allowremote(r)) {\n +\t\tcase 0: /* config overrides and disables */\n\n\n compat/fsmonitor/fsm-settings-win32.c | 68 +++++++++++++++++++++++++++\n 1 file changed, 68 insertions(+)\n\ndiff --git a/compat/fsmonitor/fsm-settings-win32.c b/compat/fsmonitor/fsm-settings-win32.c\nindex 907655720bb..e5ec5b0a9f7 100644\n--- a/compat/fsmonitor/fsm-settings-win32.c\n+++ b/compat/fsmonitor/fsm-settings-win32.c\n@@ -24,6 +24,59 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)\n \treturn FSMONITOR_REASON_OK;\n }\n \n+/*\n+ * Check if monitoring remote working directories is allowed.\n+ *\n+ * By default, monitoring remote working directories is\n+ * disabled. Users may override this behavior in enviroments where\n+ * they have proper support.\n+ */\n+static int check_config_allowremote(struct repository *r)\n+{\n+\tint allow;\n+\n+\tif (!repo_config_get_bool(r, \"fsmonitor.allowremote\", &allow))\n+\t\treturn allow;\n+\n+\treturn -1; /* fsmonitor.allowremote not set */\n+}\n+\n+/*\n+ * Check remote working directory protocol.\n+ *\n+ * Error if client machine cannot get remote protocol information.\n+ */\n+static int check_remote_protocol(wchar_t *wpath)\n+{\n+\tHANDLE h;\n+\tFILE_REMOTE_PROTOCOL_INFO proto_info;\n+\n+\th = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,\n+\t\t\tFILE_FLAG_BACKUP_SEMANTICS, NULL);\n+\n+\tif (h == INVALID_HANDLE_VALUE) {\n+\t\terror(_(\"[GLE %ld] unable to open for read '%ls'\"),\n+\t\t GetLastError(), wpath);\n+\t\treturn -1;\n+\t}\n+\n+\tif (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,\n+\t\t&proto_info, sizeof(proto_info))) {\n+\t\terror(_(\"[GLE %ld] unable to get protocol information for '%ls'\"),\n+\t\t GetLastError(), wpath);\n+\t\tCloseHandle(h);\n+\t\treturn -1;\n+\t}\n+\n+\tCloseHandle(h);\n+\n+\ttrace_printf_key(&trace_fsmonitor,\n+\t\t\t\t\"check_remote_protocol('%ls') remote protocol %#8.8lx\",\n+\t\t\t\twpath, proto_info.Protocol);\n+\n+\treturn 0;\n+}\n+\n /*\n * Remote working directories are problematic for FSMonitor.\n *\n@@ -76,6 +129,7 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)\n */\n static enum fsmonitor_reason check_remote(struct repository *r)\n {\n+\tint ret;\n \twchar_t wpath[MAX_PATH];\n \twchar_t wfullpath[MAX_PATH];\n \tsize_t wlen;\n@@ -115,6 +169,20 @@ static enum fsmonitor_reason check_remote(struct repository *r)\n \t\ttrace_printf_key(&trace_fsmonitor,\n \t\t\t\t \"check_remote('%s') true\",\n \t\t\t\t r->worktree);\n+\n+\t\tret = check_remote_protocol(wfullpath);\n+\t\tif (ret < 0)\n+\t\t\treturn FSMONITOR_REASON_ERROR;\n+\n+\t\tswitch (check_config_allowremote(r)) {\n+\t\tcase 0: /* config overrides and disables */\n+\t\t\treturn FSMONITOR_REASON_REMOTE;\n+\t\tcase 1: /* config overrides and enables */\n+\t\t\treturn FSMONITOR_REASON_OK;\n+\t\tdefault:\n+\t\t\tbreak; /* config has no opinion */\n+\t\t}\n+\n \t\treturn FSMONITOR_REASON_REMOTE;\n \t}\n \n\nbase-commit: c50926e1f48891e2671e1830dbcd2912a4563450\n-- \ngitgitgadget\n"},{"id":"461130","messageId":"xmqqh72hb9sv.fsf@gitster.g","threadId":"58283","inReplyTo":"pull.1317.v4.git.1660262231357.gitgitgadget@gmail.com","subject":"Re: [PATCH v4] fsmonitor: option to allow fsmonitor to run against network-mounted repos","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-12T18:23:28Z","receivedAt":"2022-08-12T18:23:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Eric DeCosta via GitGitGadget\" writes:\n\n> From: Eric DeCosta \n>\n> Though perhaps not common, there are uses cases where users have large,\n\n\"uses cases\" -> \"use cases\", probably.\n\n> network-mounted repos. Having the ability to run fsmonitor against\n> network paths would benefit those users.\n>\n> Most modern Samba-based filers have the necessary support to enable\n> fsmonitor on network-mounted repos. As a first step towards enabling\n> fsmonitor to work against network-mounted repos, introduce a\n> configuration option, 'fsmonitor.allowRemote'. Setting this option to\n> true will override the default behavior (erroring-out) when a\n> network-mounted repo is detected by fsmonitor.\n>\n> Signed-off-by: Eric DeCosta \n> ---\n\nWill make the above typofix while queuing.\n\nThanks.\n"},{"id":"461245","messageId":"47e86c2f-ab13-e8e6-8f69-0a79c3f4c343@jeffhostetler.com","threadId":"58283","inReplyTo":"pull.1317.v4.git.1660262231357.gitgitgadget@gmail.com","subject":"Re: [PATCH v4] fsmonitor: option to allow fsmonitor to run against network-mounted repos","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2022-08-15T16:01:50Z","receivedAt":"2022-08-15T16:02:00Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 8/11/22 7:57 PM, Eric DeCosta via GitGitGadget wrote:\n> From: Eric DeCosta \n> \n> Though perhaps not common, there are uses cases where users have large,\n> network-mounted repos. Having the ability to run fsmonitor against\n> network paths would benefit those users.\n> \n> Most modern Samba-based filers have the necessary support to enable\n> fsmonitor on network-mounted repos. As a first step towards enabling\n> fsmonitor to work against network-mounted repos, introduce a\n> configuration option, 'fsmonitor.allowRemote'. Setting this option to\n> true will override the default behavior (erroring-out) when a\n> network-mounted repo is detected by fsmonitor.\n\nV4 LGTM. Thanks for your persistence and attention to detail here.\n\nJeff\n\n"},{"id":"461252","messageId":"xmqq7d392yzh.fsf@gitster.g","threadId":"58283","inReplyTo":"47e86c2f-ab13-e8e6-8f69-0a79c3f4c343@jeffhostetler.com","subject":"Re: [PATCH v4] fsmonitor: option to allow fsmonitor to run against network-mounted repos","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-15T17:33:22Z","receivedAt":"2022-08-15T17:33:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\n\n> On 8/11/22 7:57 PM, Eric DeCosta via GitGitGadget wrote:\n>> From: Eric DeCosta \n>> Though perhaps not common, there are uses cases where users have\n>> large,\n>> network-mounted repos. Having the ability to run fsmonitor against\n>> network paths would benefit those users.\n>> Most modern Samba-based filers have the necessary support to enable\n>> fsmonitor on network-mounted repos. As a first step towards enabling\n>> fsmonitor to work against network-mounted repos, introduce a\n>> configuration option, 'fsmonitor.allowRemote'. Setting this option to\n>> true will override the default behavior (erroring-out) when a\n>> network-mounted repo is detected by fsmonitor.\n>\n> V4 LGTM. Thanks for your persistence and attention to detail here.\n\nThanks, both. Queued.\n"}]}