Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle worktrees at the top of a network drive #141

Closed
wants to merge 3 commits into from

Conversation

dscho
Copy link
Member

@dscho dscho commented Feb 24, 2019

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.

The first offset in a UNC path is not the host name, but the folder name after that.

This fixes git-for-windows#1181

Signed-off-by: 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 git-for-windows#1320

Signed-off-by: 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>
@dscho
Copy link
Member Author

dscho commented Feb 24, 2019

This PR supersedes #65.

@dscho dscho added the ready to submit Has commits that have not been submitted yet label Feb 24, 2019
@dscho
Copy link
Member Author

dscho commented Aug 24, 2019

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 24, 2019

Submitted as pull.141.git.gitgitgadget@gmail.com

@dscho dscho removed the ready to submit Has commits that have not been submitted yet label Aug 24, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Aug 26, 2019

On the Git mailing list, Junio C Hamano wrote (reply to this):

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

> 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.

> 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
Copy link

gitgitgadget bot commented Aug 26, 2019

This branch is now known as js/gitdir-at-unc-root.

@gitgitgadget
Copy link

gitgitgadget bot commented Aug 26, 2019

This patch series was integrated into pu via git@39c3430.

@gitgitgadget gitgitgadget bot added the pu label Aug 26, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Aug 30, 2019

This patch series was integrated into pu via git@a4110bd.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 3, 2019

This patch series was integrated into pu via git@beae6da.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 5, 2019

This patch series was integrated into pu via git@b960f22.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 7, 2019

This patch series was integrated into pu via git@b693753.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 9, 2019

This patch series was integrated into pu via git@e8ced64.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 9, 2019

This patch series was integrated into next via git@70ccc59.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 16, 2019

This patch series was integrated into pu via git@535c5ae.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 17, 2019

This patch series was integrated into pu via git@e79ced2.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 18, 2019

This patch series was integrated into pu via git@7df2cf2.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 18, 2019

This patch series was integrated into pu via git@49c6612.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 30, 2019

This patch series was integrated into pu via git@b57a88a.

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 30, 2019

This patch series was integrated into master via git@b57a88a.

@gitgitgadget gitgitgadget bot added the master label Sep 30, 2019
@gitgitgadget gitgitgadget bot closed this Sep 30, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Sep 30, 2019

Closed via b57a88a.

@dscho dscho deleted the gitdir-at-unc-root branch September 30, 2019 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant