threads / patch / 51722

patch, 3 partsHandle worktrees at the top of a network drive

Subject: [PATCH 0/3] Handle worktrees at the top of a network drive

## tl;dr

5 messages between Aug 24, 2019 and Aug 26, 2019. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Johannes Schindelin via GitGitGadget· Aug 24, 2019, 22:10 UTC · lore

Windows' network drive concept is a quite useful and versatile one. Once authenticated, one can even change the working directory to a network drive (cd \\server\share, works in PowerShell and Git Bash).

Some users want to have their Git repositories there, and with these patches, that works, even.

This is yet another patch series in the seemingly endless stream of Git for Windows patches.

Johannes Schindelin (3):
  setup_git_directory(): handle UNC paths correctly
  Fix .git/ discovery at the root of UNC shares
  setup_git_directory(): handle UNC root paths correctly
 setup.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
base-commit: 8104ec994ea3849a968b4667d072fedd1e688642
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-141%2Fdscho%2Fgitdir-at-unc-root-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-141/dscho/gitdir-at-unc-root-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/141
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Aug 24, 2019, 22:10 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 1/3] setup_git_directory(): handle UNC paths correctly

From: Johannes Schindelin <johannes.schindelin@gmx.de>
The first offset in a UNC path is not the host name, but the folder name after that.
This fixes https://github.com/git-for-windows/git/issues/1181
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to setup.c +1 −1
diff --git a/setup.c b/setup.c
index ca9e8a949e..a803b3ade3 100644
--- a/setup.c
+++ b/setup.c
@@ -906,7 +906,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
 	const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);
 	struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
 	const char *gitdirenv;
-	int ceil_offset = -1, min_offset = has_dos_drive_prefix(dir->buf) ? 3 : 1;
+	int ceil_offset = -1, min_offset = offset_1st_component(dir->buf);
 	dev_t current_device = 0;
 	int one_filesystem = 1;
 
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Aug 24, 2019, 22:10 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 2/3] Fix .git/ discovery at the root of UNC shares

From: Johannes Schindelin <johannes.schindelin@gmx.de>

A very common assumption in Git's source code base is that offset_1st_component() returns either 0 for relative paths, or 1 for absolute paths that start with a slash. In other words, the return value is either 0 or points just after the dir separator.

This assumption is not fulfilled when calling offset_1st_component() e.g. on UNC paths on Windows, e.g. "//my-server/my-share". In this case, offset_1st_component() returns the length of the entire string (which is correct, because stripping the last "component" would not result in a valid directory), yet the return value still does not point just after a dir separator.

This assumption is most prominently seen in the setup_git_directory_gently_1() function, where we want to append a ".git" component and simply assume that there is already a dir separator. In the UNC example given above, this assumption is incorrect.

As a consequence, Git will fail to handle a worktree at the top of a UNC share correctly.

Let's fix this by adding a dir separator specifically for that case: we found that there is no first component in the path and it does not end in a dir separator? Then add it.

This fixes https://github.com/git-for-windows/git/issues/1320
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 setup.c | 6 ++++++
 1 file changed, 6 insertions(+)
Show changes to setup.c +6 −0
diff --git a/setup.c b/setup.c
index a803b3ade3..3d24de15dd 100644
--- a/setup.c
+++ b/setup.c
@@ -934,6 +934,12 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
 	if (ceil_offset < 0)
 		ceil_offset = min_offset - 2;
 
+	if (min_offset && min_offset == dir->len &&
+	    !is_dir_sep(dir->buf[min_offset - 1])) {
+		strbuf_addch(dir, '/');
+		min_offset++;
+	}
+
 	/*
 	 * Test in the following order (relative to the dir):
 	 * - .git (file containing "gitdir: <path>")
-- 
gitgitgadget
Johannes Schindelin via GitGitGadget· Aug 24, 2019, 22:10 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 3/3] setup_git_directory(): handle UNC root paths correctly

From: Johannes Schindelin <johannes.schindelin@gmx.de>

When working in the root directory of a file share (this is only possible in Git Bash and Powershell, but not in CMD), the current directory is reported without a trailing slash.

This is different from Unix and standard Windows directories: both / and C:\ are reported with a trailing slash as current directories.

If a Git worktree is located there, Git is not quite prepared for that: while it does manage to find the .git directory/file, it returns as length of the top-level directory's path *one more* than the length of the current directory, and setup_git_directory_gently() would then return an undefined string as prefix.

In practice, this undefined string usually points to NUL bytes, and does not cause much harm. Under rare circumstances that are really involved to reproduce (and not reliably so), the reported prefix could be a suffix string of Git's exec path, though.

A careful analysis determined that this bug is unlikely to be exploitable, therefore we mark this as a regular bug fix.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to setup.c +1 −1
diff --git a/setup.c b/setup.c
index 3d24de15dd..c8c6bf3f49 100644
--- a/setup.c
+++ b/setup.c
@@ -784,7 +784,7 @@ static const char *setup_discovered_git_dir(const char *gitdir,
 		set_git_dir(gitdir);
 	inside_git_dir = 0;
 	inside_work_tree = 1;
-	if (offset == cwd->len)
+	if (offset >= cwd->len)
 		return NULL;
 
 	/* Make "offset" point past the '/' (already the case for root dirs) */
-- 
gitgitgadget
Junio C Hamano· Aug 26, 2019, 17:08 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH 0/3] Handle worktrees at the top of a network drive

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 6 quoted lines
> Windows' network drive concept is a quite useful and versatile one. Once
> authenticated, one can even change the working directory to a network drive
> (cd \\server\share, works in PowerShell and Git Bash).
>
> Some users want to have their Git repositories there, and with these
> patches, that works, even.

OK. I see too many 'even' (iow, if it walks and quacks like any other directory, it should be able to house a Git repository, so to me it does not even deserve 'it even works'), but that's probably just me.

Queued.  Thanks.
Show 16 quoted lines
> This is yet another patch series in the seemingly endless stream of Git for
> Windows patches.
>
> Johannes Schindelin (3):
>   setup_git_directory(): handle UNC paths correctly
>   Fix .git/ discovery at the root of UNC shares
>   setup_git_directory(): handle UNC root paths correctly
>
>  setup.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
>
> base-commit: 8104ec994ea3849a968b4667d072fedd1e688642
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-141%2Fdscho%2Fgitdir-at-unc-root-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-141/dscho/gitdir-at-unc-root-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/141

← back to recent threads