Re: [PATCH 01/23] fsm-listen-win32: handle shortnames

From: Derrick Stolee <derrickstolee@github.com>
Date: Feb 24, 2022, 14:48 UTC
Message-ID: <e3b854c5-540a-980a-ceac-492d1ad3af90@github.com>
In-Reply-To: <23f38338cecd270c15fd1bba42de79c53e14ca2a.1644940773.git.gitgitgadget@gmail.com>

On 2/15/2022 10:59 AM, Jeff Hostetler via GitGitGadget wrote:

Show 18 quoted lines

> From: Jeff Hostetler <jeffhost@microsoft.com>
> 
> Teach FSMonitor daemon on Windows to recognize shortname paths as
> aliases of normal longname paths.  FSMonitor clients, such as `git
> status`, should receive the longname spelling of changed files (when
> possible).
> 
> Sometimes we receive FS events using the shortname, such as when a CMD
> shell runs "RENAME GIT~1 FOO" or "RMDIR GIT~1".  The FS notification
> arrives using whatever combination of long and shortnames were used by
> the other process.  (Shortnames do seem to be case normalized,
> however.)
> 
> Use Windows GetLongPathNameW() to try to map the pathname spelling in
> the notification event into the normalized longname spelling.  (This
> can fail if the file/directory is deleted, moved, or renamed, because
> we are asking the FS for the mapping in response to the event and
> after it has already happened, but we try.)

I recall when we first discovered that this was important for users. I'm glad we haven't needed to worry about it since this fix.

> Special case the shortname spelling of ".git" to avoid under-reporting
> these events.

This is particularly critical due to the case of deleting the .git directory being really important to catch, but GetLongPathNameW() won't work in that case.

Show 14 quoted lines

> +	if (!GetLongPathNameW(buf_in, buf_out, MAX_PATH)) {
> +		/*
> +		 * The shortname to longname conversion can fail for
> +		 * various reasons, for example if the file has been
> +		 * deleted.  (That is, if we just received a
> +		 * delete-file notification event and the file is
> +		 * already gone, we can't ask the file system to
> +		 * lookup the longname for it.  Likewise, for moves
> +		 * and renames where we are given the old name.)
> +		 *
> +		 * NEEDSWORK: Since deleting or moving a file or
> +		 * directory by shortname is rather obscure, I'm going
> +		 * ignore the failure and ask the caller to report the
> +		 * original relative path.  This seemds kinder than

s/seemds/seems/

> +		 * failing here and forcing a resync.
> +		 */
> +		return GRR_NO_CONVERSION_NEEDED;
> +	}

This NEEDSWORK is something I saw while reading that makes it seem like we _need_ to do something here. I think this is an appropriate short-term solution that we can revisit if necessary. I don't anticipate that actually being the case, though.

Thanks, -Stolee