fsmonitor: option to allow fsmonitor to run against network-mounted repos

From: Eric DeCosta <[email protected]>

Though perhaps not common, there are uses cases where users have large, network-mounted repos. Having the ability to run fsmonitor against network paths would benefit those users.

Most modern Samba-based filers have the necessary support to enable fsmonitor on network-mounted repos. As a first step towards enabling fsmonitor to work against network-mounted repos, introduce a configuration option, 'fsmonitor.allowRemote'. Setting this option to true will override the default behavior (erroring-out) when a network-mounted repo is detected by fsmonitor.

Additionally, as part of this first step, monitoring of network-mounted repos will be restricted to those mounted over SMB regardless of the value of 'fsmonitor.allowRemote' until more extensive testing can be performed.

---
Option to allow fsmonitor to run against repos on network file systems

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1317%2Fedecosta-mw%2Fmaster-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1317/edecosta-mw/master-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1317

 compat/fsmonitor/fsm-settings-win32.c | 59 ++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

index 907655720bb..d120e4710cf 100644
@@ -24,6 +24,58 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)
 	return FSMONITOR_REASON_OK;
 }
 
+/*
+ * Check if monitoring remote working directories is allowed.
+ *
+ * By default monitoring remote working directories is not allowed,
+ * but users may override this behavior in enviroments where they
+ * have proper support.
+*/
+static enum fsmonitor_reason check_allow_remote(struct repository *r)
+{
+	int allow;
+
+	if (repo_config_get_bool(r, "fsmonitor.allowremote", &allow) || !allow)
+		return FSMONITOR_REASON_REMOTE;
+
+	return FSMONITOR_REASON_OK;
+}
+
+/*
+ * Check if the remote working directory is mounted via SMB
+ *
+ * For now, remote working directories are only supported via SMB mounts
+*/
+static enum fsmonitor_reason check_smb(wchar_t *wpath)
+{
+	HANDLE h;
+	FILE_REMOTE_PROTOCOL_INFO proto_info;
+
+	h = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,
+					FILE_FLAG_BACKUP_SEMANTICS, NULL);
+
+	if (h == INVALID_HANDLE_VALUE) {
+		error(_("[GLE %ld] unable to open for read '%ls'"),
+		      GetLastError(), wpath);
+		return FSMONITOR_REASON_ERROR;
+	}
+
+	if (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,
+									&proto_info, sizeof(proto_info))) {
+		error(_("[GLE %ld] unable to get protocol information for '%ls'"),
+		      GetLastError(), wpath);
+		CloseHandle(h);
+		return FSMONITOR_REASON_ERROR;
+	}
+
+	CloseHandle(h);
+
+	if (proto_info.Protocol == WNNC_NET_SMB)
+		return FSMONITOR_REASON_OK;
+
+	return FSMONITOR_REASON_ERROR;
+}
+
 /*
  * Remote working directories are problematic for FSMonitor.
  *

@@ -76,6 +128,7 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)
  */
 static enum fsmonitor_reason check_remote(struct repository *r)
 {
+	enum fsmonitor_reason reason;
 	wchar_t wpath[MAX_PATH];
 	wchar_t wfullpath[MAX_PATH];
 	size_t wlen;

@@ -115,7 +168,11 @@ static enum fsmonitor_reason check_remote(struct repository *r)
 		trace_printf_key(&trace_fsmonitor,
 				 "check_remote('%s') true",
 				 r->worktree);
-		return FSMONITOR_REASON_REMOTE;
+
+		reason = check_smb(wfullpath);
+		if (reason != FSMONITOR_REASON_OK)
+			return reason;
+		return check_allow_remote(r);
 	}
 
 	return FSMONITOR_REASON_OK;

base-commit: c50926e1f48891e2671e1830dbcd2912a4563450

gitgitgadget

My impression from the earlier discussion [*] was that having the necessary support and the support working well are two different things, and the network mounted directory being served via SMB was not enough sign of the latter.

https://lore.kernel.org/git/[email protected]/

I do not do Windows, so I'll leave it up to the experts, but if everybody who talks SMB behaves well, then the updated code that enables fsmonitor for SMB talkers and disables it for all other remotely mounted directories, with a configuration override, does sound like a good way to go. I think the new code in check_remote() is broken, though (see below).

	if (!repo_config_get_bool(..., &allow) && allow)
		return FSMONITOR_REASON_OK;

	return FSMONITOR_REASON_REMOTE;

	if (!repo_config_get_bool(..., &allow))
		return allow ? FSMONITOR_REASON_OK : FSMONITOR_REASON_REMOTE;

	return FSMONITOR_REASON_UNDECIDED; /* invented... */

	reason = check_allow_remote(r);
        if (reason == FSMONITOR_REASON_OK || reason == FSMONITOR_REASON_REMOTE)
		return reason;
	/*
	 * check_allow_remote() did not decide, so use the default
	 * based on the network filesystem.
	 */
	return check_smb(wfullpath);

I'll clean them up, thanks.

That reads much better, thanks.

Makes sense. How about FSMONITOR_OVERRIDE_REQUIRED ? The error message could then indicate that remote repos are normally unsupported but that setting the fsmonitor.allowRemote flag overrides this behavior.

Got it. I'll check my editor settings too :-)

Yeah, I noticed that myself this morning.

If we do as you suggest above, then fsmonitor.allowRemote=true would override regardless of the protocol being used. The more conservative path is to only allow the override if SMB is being used.

It could perhaps be better written as:

reason = check_allow_remote(r);
if (reason != FSMONITOR_REASON_OK)

        return reason;

return check_smb(wbfullpath);

I actually think check_allow_remote() should be renamed to have "config" somewhere in its name, and return -1, 0 or 1 and not "enum fsmonitor_reason".

	static int check_config_allowremote(...)
	{
		int allow;

		if (repor_config_get_bool(..., &allow))
			return allow;
		return -1; /* undecided */
	}

then caller can do

	switch (check_config_allowremote(...)) {
	case 0: /* config overrides and disables */
		return FSMONITOR_REASON_REMOTE;
	case 1: /* config overrides and enables */
		return FSMONITOR_REASON_OK;
	default:
		break; /* config has no opinion */
	}
        return check_smb(...);

Exactly. The code should not try to outsmart the user.

If the user says they wants to use it on a particular remote, even if you do not know if that particular remote system works, just let them try and see if it works. If it does not, they can easily disable, because the enabiling was a deliberate act by them in the first place. They know how to fix it.

Thanks.

100% agree with you, thanks.

The only thing that is somewhat gnawing at me is that just because the remote worktree is mounted via SMB is no guarantee that fsmonitor will work correctly. In many (most?) cases it should, but who knows what support the filer server has.

I think we should allow the user to override regardless - as you said let the user try it. But, conservatively, just because SMB is there may not be enough to let the monitor start without the explicit user override. Being able to report on which protocol is being used could provide useful diagnostics, but that's about it.

I do not think anybody minds if the initial/first step would be to add "option to allow" and do nothing else. No "are we talking SMB?" check, and just a simple "by default we refuse going remote, but since the user has an explicit opt-in configuration set, we allow".

That is sufficient to let people gain experience using fsmonitor on Windows mounting from various filer implementations. And then the next step, in one or two major releases down the road, may try to check what kind of filer we are on and see if it is one of the "good" ones to flip the default selectively.

Eric DeCosta (2):
fsmonitor: option to allow fsmonitor to run against network-mounted
repos
fsmonitor.allowRemote now overrides default behavior

 compat/fsmonitor/fsm-settings-win32.c | 66 +++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

base-commit: c50926e1f48891e2671e1830dbcd2912a4563450
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1317%2Fedecosta-mw%2Fmaster-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1317/edecosta-mw/master-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1317

Range-diff vs v1:

 1:  7e67ce8c944 = 1:  7e67ce8c944 fsmonitor: option to allow fsmonitor to run against network-mounted repos
 -:  ----------- > 2:  7a071c9e6be fsmonitor.allowRemote now overrides default behavior

gitgitgadget

From: Eric DeCosta <[email protected]>

Though perhaps not common, there are uses cases where users have large, network-mounted repos. Having the ability to run fsmonitor against network paths would benefit those users.

Most modern Samba-based filers have the necessary support to enable fsmonitor on network-mounted repos. As a first step towards enabling fsmonitor to work against network-mounted repos, introduce a configuration option, 'fsmonitor.allowRemote'. Setting this option to true will override the default behavior (erroring-out) when a network-mounted repo is detected by fsmonitor.

Additionally, as part of this first step, monitoring of network-mounted repos will be restricted to those mounted over SMB regardless of the value of 'fsmonitor.allowRemote' until more extensive testing can be performed.

---
 compat/fsmonitor/fsm-settings-win32.c | 59 ++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

index 907655720bb..d120e4710cf 100644
@@ -24,6 +24,58 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)
 	return FSMONITOR_REASON_OK;
 }
 
+/*
+ * Check if monitoring remote working directories is allowed.
+ *
+ * By default monitoring remote working directories is not allowed,
+ * but users may override this behavior in enviroments where they
+ * have proper support.
+*/
+static enum fsmonitor_reason check_allow_remote(struct repository *r)
+{
+	int allow;
+
+	if (repo_config_get_bool(r, "fsmonitor.allowremote", &allow) || !allow)
+		return FSMONITOR_REASON_REMOTE;
+
+	return FSMONITOR_REASON_OK;
+}
+
+/*
+ * Check if the remote working directory is mounted via SMB
+ *
+ * For now, remote working directories are only supported via SMB mounts
+*/
+static enum fsmonitor_reason check_smb(wchar_t *wpath)
+{
+	HANDLE h;
+	FILE_REMOTE_PROTOCOL_INFO proto_info;
+
+	h = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,
+					FILE_FLAG_BACKUP_SEMANTICS, NULL);
+
+	if (h == INVALID_HANDLE_VALUE) {
+		error(_("[GLE %ld] unable to open for read '%ls'"),
+		      GetLastError(), wpath);
+		return FSMONITOR_REASON_ERROR;
+	}
+
+	if (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,
+									&proto_info, sizeof(proto_info))) {
+		error(_("[GLE %ld] unable to get protocol information for '%ls'"),
+		      GetLastError(), wpath);
+		CloseHandle(h);
+		return FSMONITOR_REASON_ERROR;
+	}
+
+	CloseHandle(h);
+
+	if (proto_info.Protocol == WNNC_NET_SMB)
+		return FSMONITOR_REASON_OK;
+
+	return FSMONITOR_REASON_ERROR;
+}
+
 /*
  * Remote working directories are problematic for FSMonitor.
  *

@@ -76,6 +128,7 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)
  */
 static enum fsmonitor_reason check_remote(struct repository *r)
 {
+	enum fsmonitor_reason reason;
 	wchar_t wpath[MAX_PATH];
 	wchar_t wfullpath[MAX_PATH];
 	size_t wlen;

@@ -115,7 +168,11 @@ static enum fsmonitor_reason check_remote(struct repository *r)
 		trace_printf_key(&trace_fsmonitor,
 				 "check_remote('%s') true",
 				 r->worktree);
-		return FSMONITOR_REASON_REMOTE;
+
+		reason = check_smb(wfullpath);
+		if (reason != FSMONITOR_REASON_OK)
+			return reason;
+		return check_allow_remote(r);
 	}
 
 	return FSMONITOR_REASON_OK;

gitgitgadget

From: Eric DeCosta <[email protected]>

Reworked the logic around fsmonitor.allowRemote such that if allowRemote is set it will determine if monitoring the remote worktree is allowed.

Get remote protocoal information; if this fails report an error else print it if tracing is enabled.

Fixed fomratting issues.

---
 compat/fsmonitor/fsm-settings-win32.c | 57 ++++++++++++++++-----------
 1 file changed, 33 insertions(+), 24 deletions(-)

index d120e4710cf..32c0695c6c1 100644
@@ -27,53 +27,55 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)
 /*
  * Check if monitoring remote working directories is allowed.
  *
- * By default monitoring remote working directories is not allowed,
- * but users may override this behavior in enviroments where they
- * have proper support.
-*/
-static enum fsmonitor_reason check_allow_remote(struct repository *r)
+ * By default, monitoring remote working directories is
+ * disabled unless on a network filesystem that is known to
+ * behave well.  Users may override this behavior in enviroments where
+ * they have proper support.
+ */
+static int check_config_allowremote(struct repository *r)
 {
 	int allow;
 
-	if (repo_config_get_bool(r, "fsmonitor.allowremote", &allow) || !allow)
-		return FSMONITOR_REASON_REMOTE;
+	if (!repo_config_get_bool(r, "fsmonitor.allowremote", &allow))
+		return allow;
 
-	return FSMONITOR_REASON_OK;
+	return -1; /* fsmonitor.allowremote not set */
 }
 
 /*
- * Check if the remote working directory is mounted via SMB
+ * Check remote working directory protocol.
  *
- * For now, remote working directories are only supported via SMB mounts
-*/
-static enum fsmonitor_reason check_smb(wchar_t *wpath)
+ * Error if client machine cannot get remote protocol information.
+ */
+static void check_remote_protocol(wchar_t *wpath)
 {
 	HANDLE h;
 	FILE_REMOTE_PROTOCOL_INFO proto_info;
 
 	h = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,
-					FILE_FLAG_BACKUP_SEMANTICS, NULL);
+			FILE_FLAG_BACKUP_SEMANTICS, NULL);
 
 	if (h == INVALID_HANDLE_VALUE) {
 		error(_("[GLE %ld] unable to open for read '%ls'"),
 		      GetLastError(), wpath);
-		return FSMONITOR_REASON_ERROR;
+		return;
 	}
 
 	if (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,
-									&proto_info, sizeof(proto_info))) {
+		&proto_info, sizeof(proto_info))) {
 		error(_("[GLE %ld] unable to get protocol information for '%ls'"),
 		      GetLastError(), wpath);
 		CloseHandle(h);
-		return FSMONITOR_REASON_ERROR;
+		return;
 	}
 
 	CloseHandle(h);
 
-	if (proto_info.Protocol == WNNC_NET_SMB)
-		return FSMONITOR_REASON_OK;
+	trace_printf_key(&trace_fsmonitor,
+				"check_remote_protocol('%ls') remote protocol %#8.8lx",
+				wpath, proto_info.Protocol);
 
-	return FSMONITOR_REASON_ERROR;
+	return;
 }
 
 /*

@@ -128,7 +130,6 @@ static enum fsmonitor_reason check_smb(wchar_t *wpath)
  */
 static enum fsmonitor_reason check_remote(struct repository *r)
 {
-	enum fsmonitor_reason reason;
 	wchar_t wpath[MAX_PATH];
 	wchar_t wfullpath[MAX_PATH];
 	size_t wlen;

@@ -169,10 +170,18 @@ static enum fsmonitor_reason check_remote(struct repository *r)
 				 "check_remote('%s') true",
 				 r->worktree);
 
-		reason = check_smb(wfullpath);
-		if (reason != FSMONITOR_REASON_OK)
-			return reason;
-		return check_allow_remote(r);
+		check_remote_protocol(wfullpath);
+
+		switch (check_config_allowremote(r)) {
+		case 0: /* config overrides and disables */
+			return FSMONITOR_REASON_REMOTE;
+		case 1: /* config overrides and enables */
+			return FSMONITOR_REASON_OK;
+		default:
+			break; /* config has no opinion */
+		}
+
+		return FSMONITOR_REASON_REMOTE;
 	}
 
 	return FSMONITOR_REASON_OK;

gitgitgadget

The end result (i.e. HEAD^{tree} of the branch you developed these two patches on) may be good (I haven't checked), but it is not how we fix problems in an earlier attempt in this project by keeping the faulty commit(s) on the bottom and piling "oops, that was wrong, and here is a fix-up" commit(s) on top.

Once you are happy with the end result, use "rebase -i" to clean-up the history leading to that end result. The goal is to pretend as if you were a perfect human, more perfect than your actual self, who came up with an ideal patch without making mistakes that need to be corrected with "fix-up" commits. In this particular case, you'd most likely want to end up with a single commit, so squashing them together and fixing up the log message might be all you need to do. When you work on a more elaborate topic, you may also want to split or reorder original commits to present a logical progression towards the end result. "rebase -i" is a good tool to help you do so.

I am not a user of GitGitGadget myself, but if I recall correctly, you should be able to force-push the result of such a clean-up to update the pull-request, to trigger a new iteration to be sent to the list.

Thanks.

Well, needless to say I wasn't expecting GitGitGadget to do what it did.I had squashed things down to just two commits and forced-pushed the second commit thinking that just the relevant stuff from the second commit would show up in the next patch. Obviously that didn't happen. Sorry about that.

I can certainly squash it down to just one commit and force-push that.

Oh, sorry to hear that. If your ideal "logical progression" needs two commits, then please do present the series that way. What GGG sent out was apparently not that (i.e. the same one from v1 with full of fix-ups for it in 2/2).

Given that, in the end, the change is rather small and involves just one file, having it be just one commit is fine. Perhaps my next lesson to learn is to generate and send the patch sets myself, but that will be for another time.

Thank you for all your patience, it makes a total noob like me feel welcome.

From: Eric DeCosta <[email protected]>

Though perhaps not common, there are uses cases where users have large, network-mounted repos. Having the ability to run fsmonitor against network paths would benefit those users.

Most modern Samba-based filers have the necessary support to enable fsmonitor on network-mounted repos. As a first step towards enabling fsmonitor to work against network-mounted repos, introduce a configuration option, 'fsmonitor.allowRemote'. Setting this option to true will override the default behavior (erroring-out) when a network-mounted repo is detected by fsmonitor.

---
Option to allow fsmonitor to run against repos on network file systems

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1317%2Fedecosta-mw%2Fmaster-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1317/edecosta-mw/master-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1317

Range-diff vs v2:

1: 7e67ce8c944 ! 1: 6c5f176cbee fsmonitor: option to allow fsmonitor to run against network-mounted repos

     @@ Commit message
          true will override the default behavior (erroring-out) when a
          network-mounted repo is detected by fsmonitor.
      
     -    Additionally, as part of this first step, monitoring of network-mounted
     -    repos will be restricted to those mounted over SMB regardless of the
     -    value of 'fsmonitor.allowRemote' until more extensive testing can be
     -    performed.
     -
          Signed-off-by: Eric DeCosta <[email protected]>
      
       ## compat/fsmonitor/fsm-settings-win32.c ##
     @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4gi
      +/*
      + * Check if monitoring remote working directories is allowed.
      + *
     -+ * By default monitoring remote working directories is not allowed,
     -+ * but users may override this behavior in enviroments where they
     -+ * have proper support.
     -+*/
     -+static enum fsmonitor_reason check_allow_remote(struct repository *r)
     ++ * By default, monitoring remote working directories is
     ++ * disabled unless on a network filesystem that is known to
     ++ * behave well.  Users may override this behavior in enviroments where
     ++ * they have proper support.
     ++ */
     ++static int check_config_allowremote(struct repository *r)
      +{
      +	int allow;
      +
     -+	if (repo_config_get_bool(r, "fsmonitor.allowremote", &allow) || !allow)
     -+		return FSMONITOR_REASON_REMOTE;
     ++	if (!repo_config_get_bool(r, "fsmonitor.allowremote", &allow))
     ++		return allow;
      +
     -+	return FSMONITOR_REASON_OK;
     ++	return -1; /* fsmonitor.allowremote not set */
      +}
      +
      +/*
     -+ * Check if the remote working directory is mounted via SMB
     ++ * Check remote working directory protocol.
      + *
     -+ * For now, remote working directories are only supported via SMB mounts
     -+*/
     -+static enum fsmonitor_reason check_smb(wchar_t *wpath)
     ++ * Error if client machine cannot get remote protocol information.
     ++ */
     ++static void check_remote_protocol(wchar_t *wpath)
      +{
      +	HANDLE h;
      +	FILE_REMOTE_PROTOCOL_INFO proto_info;
      +
      +	h = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,
     -+					FILE_FLAG_BACKUP_SEMANTICS, NULL);
     ++			FILE_FLAG_BACKUP_SEMANTICS, NULL);
      +
      +	if (h == INVALID_HANDLE_VALUE) {
      +		error(_("[GLE %ld] unable to open for read '%ls'"),
      +		      GetLastError(), wpath);
     -+		return FSMONITOR_REASON_ERROR;
     ++		return;
      +	}
      +
      +	if (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,
     -+									&proto_info, sizeof(proto_info))) {
     ++		&proto_info, sizeof(proto_info))) {
      +		error(_("[GLE %ld] unable to get protocol information for '%ls'"),
      +		      GetLastError(), wpath);
      +		CloseHandle(h);
     -+		return FSMONITOR_REASON_ERROR;
     ++		return;
      +	}
      +
      +	CloseHandle(h);
      +
     -+	if (proto_info.Protocol == WNNC_NET_SMB)
     -+		return FSMONITOR_REASON_OK;
     ++	trace_printf_key(&trace_fsmonitor,
     ++				"check_remote_protocol('%ls') remote protocol %#8.8lx",
     ++				wpath, proto_info.Protocol);
      +
     -+	return FSMONITOR_REASON_ERROR;
     ++	return;
      +}
      +
       /*
        * Remote working directories are problematic for FSMonitor.
        *
     -@@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4git(struct repository *r)
     -  */
     - static enum fsmonitor_reason check_remote(struct repository *r)
     - {
     -+	enum fsmonitor_reason reason;
     - 	wchar_t wpath[MAX_PATH];
     - 	wchar_t wfullpath[MAX_PATH];
     - 	size_t wlen;
      @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_remote(struct repository *r)
       		trace_printf_key(&trace_fsmonitor,
       				 "check_remote('%s') true",
       				 r->worktree);
     --		return FSMONITOR_REASON_REMOTE;
      +
     -+		reason = check_smb(wfullpath);
     -+		if (reason != FSMONITOR_REASON_OK)
     -+			return reason;
     -+		return check_allow_remote(r);
     ++		check_remote_protocol(wfullpath);
     ++
     ++		switch (check_config_allowremote(r)) {
     ++		case 0: /* config overrides and disables */
     ++			return FSMONITOR_REASON_REMOTE;
     ++		case 1: /* config overrides and enables */
     ++			return FSMONITOR_REASON_OK;
     ++		default:
     ++			break; /* config has no opinion */
     ++		}
     ++
     + 		return FSMONITOR_REASON_REMOTE;
       	}
       
     - 	return FSMONITOR_REASON_OK;

2: 7a071c9e6be < -: ----------- fsmonitor.allowRemote now overrides default behavior

 compat/fsmonitor/fsm-settings-win32.c | 66 +++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

index 907655720bb..32c0695c6c1 100644
@@ -24,6 +24,60 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)
 	return FSMONITOR_REASON_OK;
 }
 
+/*
+ * Check if monitoring remote working directories is allowed.
+ *
+ * By default, monitoring remote working directories is
+ * disabled unless on a network filesystem that is known to
+ * behave well.  Users may override this behavior in enviroments where
+ * they have proper support.
+ */
+static int check_config_allowremote(struct repository *r)
+{
+	int allow;
+
+	if (!repo_config_get_bool(r, "fsmonitor.allowremote", &allow))
+		return allow;
+
+	return -1; /* fsmonitor.allowremote not set */
+}
+
+/*
+ * Check remote working directory protocol.
+ *
+ * Error if client machine cannot get remote protocol information.
+ */
+static void check_remote_protocol(wchar_t *wpath)
+{
+	HANDLE h;
+	FILE_REMOTE_PROTOCOL_INFO proto_info;
+
+	h = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,
+			FILE_FLAG_BACKUP_SEMANTICS, NULL);
+
+	if (h == INVALID_HANDLE_VALUE) {
+		error(_("[GLE %ld] unable to open for read '%ls'"),
+		      GetLastError(), wpath);
+		return;
+	}
+
+	if (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,
+		&proto_info, sizeof(proto_info))) {
+		error(_("[GLE %ld] unable to get protocol information for '%ls'"),
+		      GetLastError(), wpath);
+		CloseHandle(h);
+		return;
+	}
+
+	CloseHandle(h);
+
+	trace_printf_key(&trace_fsmonitor,
+				"check_remote_protocol('%ls') remote protocol %#8.8lx",
+				wpath, proto_info.Protocol);
+
+	return;
+}
+
 /*
  * Remote working directories are problematic for FSMonitor.
  *

@@ -115,6 +169,18 @@ static enum fsmonitor_reason check_remote(struct repository *r)
 		trace_printf_key(&trace_fsmonitor,
 				 "check_remote('%s') true",
 				 r->worktree);
+
+		check_remote_protocol(wfullpath);
+
+		switch (check_config_allowremote(r)) {
+		case 0: /* config overrides and disables */
+			return FSMONITOR_REASON_REMOTE;
+		case 1: /* config overrides and enables */
+			return FSMONITOR_REASON_OK;
+		default:
+			break; /* config has no opinion */
+		}
+
 		return FSMONITOR_REASON_REMOTE;
 	}
 

base-commit: c50926e1f48891e2671e1830dbcd2912a4563450

gitgitgadget

From: Eric DeCosta <[email protected]>

Though perhaps not common, there are uses cases where users have large, network-mounted repos. Having the ability to run fsmonitor against network paths would benefit those users.

Most modern Samba-based filers have the necessary support to enable fsmonitor on network-mounted repos. As a first step towards enabling fsmonitor to work against network-mounted repos, introduce a configuration option, 'fsmonitor.allowRemote'. Setting this option to true will override the default behavior (erroring-out) when a network-mounted repo is detected by fsmonitor.

---
Option to allow fsmonitor to run against repos on network file systems

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1317%2Fedecosta-mw%2Fmaster-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1317/edecosta-mw/master-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1317

Range-diff vs v3:

1: 6c5f176cbee ! 1: 058dc400c8a fsmonitor: option to allow fsmonitor to run against network-mounted repos

     @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4gi
      + * Check if monitoring remote working directories is allowed.
      + *
      + * By default, monitoring remote working directories is
     -+ * disabled unless on a network filesystem that is known to
     -+ * behave well.  Users may override this behavior in enviroments where
     ++ * disabled.  Users may override this behavior in enviroments where
      + * they have proper support.
      + */
      +static int check_config_allowremote(struct repository *r)
     @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4gi
      + *
      + * Error if client machine cannot get remote protocol information.
      + */
     -+static void check_remote_protocol(wchar_t *wpath)
     ++static int check_remote_protocol(wchar_t *wpath)
      +{
      +	HANDLE h;
      +	FILE_REMOTE_PROTOCOL_INFO proto_info;
     @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4gi
      +	if (h == INVALID_HANDLE_VALUE) {
      +		error(_("[GLE %ld] unable to open for read '%ls'"),
      +		      GetLastError(), wpath);
     -+		return;
     ++		return -1;
      +	}
      +
      +	if (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,
     @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4gi
      +		error(_("[GLE %ld] unable to get protocol information for '%ls'"),
      +		      GetLastError(), wpath);
      +		CloseHandle(h);
     -+		return;
     ++		return -1;
      +	}
      +
      +	CloseHandle(h);
     @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4gi
      +				"check_remote_protocol('%ls') remote protocol %#8.8lx",
      +				wpath, proto_info.Protocol);
      +
     -+	return;
     ++	return 0;
      +}
      +
       /*
        * Remote working directories are problematic for FSMonitor.
        *
     +@@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_vfs4git(struct repository *r)
     +  */
     + static enum fsmonitor_reason check_remote(struct repository *r)
     + {
     ++	int ret;
     + 	wchar_t wpath[MAX_PATH];
     + 	wchar_t wfullpath[MAX_PATH];
     + 	size_t wlen;
      @@ compat/fsmonitor/fsm-settings-win32.c: static enum fsmonitor_reason check_remote(struct repository *r)
       		trace_printf_key(&trace_fsmonitor,
       				 "check_remote('%s') true",
       				 r->worktree);
      +
     -+		check_remote_protocol(wfullpath);
     ++		ret = check_remote_protocol(wfullpath);
     ++		if (ret < 0)
     ++			return FSMONITOR_REASON_ERROR;
      +
      +		switch (check_config_allowremote(r)) {
      +		case 0: /* config overrides and disables */

 compat/fsmonitor/fsm-settings-win32.c | 68 +++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

index 907655720bb..e5ec5b0a9f7 100644
@@ -24,6 +24,59 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)
 	return FSMONITOR_REASON_OK;
 }
 
+/*
+ * Check if monitoring remote working directories is allowed.
+ *
+ * By default, monitoring remote working directories is
+ * disabled.  Users may override this behavior in enviroments where
+ * they have proper support.
+ */
+static int check_config_allowremote(struct repository *r)
+{
+	int allow;
+
+	if (!repo_config_get_bool(r, "fsmonitor.allowremote", &allow))
+		return allow;
+
+	return -1; /* fsmonitor.allowremote not set */
+}
+
+/*
+ * Check remote working directory protocol.
+ *
+ * Error if client machine cannot get remote protocol information.
+ */
+static int check_remote_protocol(wchar_t *wpath)
+{
+	HANDLE h;
+	FILE_REMOTE_PROTOCOL_INFO proto_info;
+
+	h = CreateFileW(wpath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING,
+			FILE_FLAG_BACKUP_SEMANTICS, NULL);
+
+	if (h == INVALID_HANDLE_VALUE) {
+		error(_("[GLE %ld] unable to open for read '%ls'"),
+		      GetLastError(), wpath);
+		return -1;
+	}
+
+	if (!GetFileInformationByHandleEx(h, FileRemoteProtocolInfo,
+		&proto_info, sizeof(proto_info))) {
+		error(_("[GLE %ld] unable to get protocol information for '%ls'"),
+		      GetLastError(), wpath);
+		CloseHandle(h);
+		return -1;
+	}
+
+	CloseHandle(h);
+
+	trace_printf_key(&trace_fsmonitor,
+				"check_remote_protocol('%ls') remote protocol %#8.8lx",
+				wpath, proto_info.Protocol);
+
+	return 0;
+}
+
 /*
  * Remote working directories are problematic for FSMonitor.
  *

@@ -76,6 +129,7 @@ static enum fsmonitor_reason check_vfs4git(struct repository *r)
  */
 static enum fsmonitor_reason check_remote(struct repository *r)
 {
+	int ret;
 	wchar_t wpath[MAX_PATH];
 	wchar_t wfullpath[MAX_PATH];
 	size_t wlen;

@@ -115,6 +169,20 @@ static enum fsmonitor_reason check_remote(struct repository *r)
 		trace_printf_key(&trace_fsmonitor,
 				 "check_remote('%s') true",
 				 r->worktree);
+
+		ret = check_remote_protocol(wfullpath);
+		if (ret < 0)
+			return FSMONITOR_REASON_ERROR;
+
+		switch (check_config_allowremote(r)) {
+		case 0: /* config overrides and disables */
+			return FSMONITOR_REASON_REMOTE;
+		case 1: /* config overrides and enables */
+			return FSMONITOR_REASON_OK;
+		default:
+			break; /* config has no opinion */
+		}
+
 		return FSMONITOR_REASON_REMOTE;
 	}
 

base-commit: c50926e1f48891e2671e1830dbcd2912a4563450

gitgitgadget

"uses cases" -> "use cases", probably.

Will make the above typofix while queuing.

Thanks.

V4 LGTM. Thanks for your persistence and attention to detail here.

Jeff

Thanks, both. Queued.