threads / patch / 22567

patchSupport working directory located at root

Subject: [PATCH] Support working directory located at root

## tl;dr

5 messages between Feb 8, 2010 and Feb 9, 2010. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Nguyễn Thái Ngọc Duy· Feb 8, 2010, 14:53 UTC · lore

Git should work regardless where the working directory is located, even at root. This patch fixes two places where it assumes working directory always have parent directory.

In setup_git_directory_gently(), when Git goes up to root and finds .git there, it happily sets worktree to "".

In prefix_path(), loosen the outside repo check a little bit. Usually when a path XXX is inside worktree /foo, it must be either "/foo", or "/foo/...". When worktree is simply "/", we can safely ignore the check: we have a slash at the beginning already.

Not related to worktree, but also set gitdir correctly if a bare repo is placed (insanely?) at root.

Thanks João Carlos Mendes Luís for pointing out this problem.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Tell me if this patch is rejected, I'll send another one that makes
 setup_git_* die() if worktree/gitdir is to be set empty. I don't think we
 expect gitdir or worktree to be empty anywhere.
 setup.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)
Show changes to setup.c +4 −2
diff --git a/setup.c b/setup.c
index b38cbee..0fcd2fd 100644
--- a/setup.c
+++ b/setup.c
@@ -25,7 +25,7 @@ const char *prefix_path(const char *prefix, int len, const char *path)
 		len = strlen(work_tree);
 		total = strlen(sanitized) + 1;
 		if (strncmp(sanitized, work_tree, len) ||
-		    (sanitized[len] != '\0' && sanitized[len] != '/')) {
+		    (len > 1 && sanitized[len] != '\0' && sanitized[len] != '/')) {
 		error_out:
 			die("'%s' is outside repository", orig);
 		}
@@ -403,7 +403,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			if (!work_tree_env)
 				inside_work_tree = 0;
 			if (offset != len) {
-				cwd[offset] = '\0';
+				cwd[offset ? offset : 1] = '\0';
 				set_git_dir(cwd);
 			} else
 				set_git_dir(".");
@@ -427,6 +427,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	inside_git_dir = 0;
 	if (!work_tree_env)
 		inside_work_tree = 1;
+	if (offset == 0) /* reached root, set worktree to '/' */
+		offset = 1;
 	git_work_tree_cfg = xstrndup(cwd, offset);
 	if (check_repository_format_gently(nongit_ok))
 		return NULL;
-- 
1.7.0.rc0.54.gd33ef
Junio C Hamano· Feb 8, 2010, 17:43 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] Support working directory located at root

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 6 quoted lines
> Git should work regardless where the working directory is located,
> even at root. This patch fixes two places where it assumes working
> directory always have parent directory.
>
> In setup_git_directory_gently(), when Git goes up to root and finds
> .git there, it happily sets worktree to "".

If you mean "instead set it to "/" and things will work much better." I agree with the reasoning (not suggesting to reword---just trying to make sure I understood what you meant).

> In prefix_path(), loosen the outside repo check a little bit. Usually
> when a path XXX is inside worktree /foo, it must be either "/foo", or
> "/foo/...". When worktree is simply "/", we can safely ignore the
> check: we have a slash at the beginning already.

The logic for the "are we inside?" check above sounds correct. When work_tree is at root, have "/" in it, and len inside the "if orig is absolute" block is 1, so memmove() strips out the leading '/' and makes the result relative to the root level. Am I reading the code right?

> Not related to worktree, but also set gitdir correctly if a bare repo
> is placed (insanely?) at root.
Yuck, but looks correct.
Nguyen Thai Ngoc Duy· Feb 9, 2010, 02:18 UTC · re: Junio C Hamano · lore

Re: [PATCH] Support working directory located at root

2010/2/9 Junio C Hamano <gitster@pobox.com>:
Show 12 quoted lines
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> Git should work regardless where the working directory is located,
>> even at root. This patch fixes two places where it assumes working
>> directory always have parent directory.
>>
>> In setup_git_directory_gently(), when Git goes up to root and finds
>> .git there, it happily sets worktree to "".
>
> If you mean "instead set it to "/" and things will work much better."
> I agree with the reasoning (not suggesting to reword---just trying to
> make sure I understood what you meant).
Yes.
Show 10 quoted lines
>
>> In prefix_path(), loosen the outside repo check a little bit. Usually
>> when a path XXX is inside worktree /foo, it must be either "/foo", or
>> "/foo/...". When worktree is simply "/", we can safely ignore the
>> check: we have a slash at the beginning already.
>
> The logic for the "are we inside?" check above sounds correct.  When
> work_tree is at root, have "/" in it, and len inside the "if orig is
> absolute" block is 1, so memmove() strips out the leading '/' and makes
> the result relative to the root level.  Am I reading the code right?

You are. But I suspect my change in this code is not enough and caused the problem (on msys?) for Hannes. If the worktree somehow is '//' and sanitized is '/etc/resolv.conf', then we could end up eating two chars, leading to "'tc/resolv.conf' not match" error.

Hannes, can you put a "printf("%s\n", work_tree);" in prefix_path() to see if it's the case?

-- 
Duy
Nguyen Thai Ngoc Duy· Feb 9, 2010, 02:49 UTC · re: Nguyen Thai Ngoc Duy · lore

Re: [PATCH] Support working directory located at root

2010/2/9 Nguyen Thai Ngoc Duy <pclouds@gmail.com>:
Show 7 quoted lines
> But I suspect my change in this code is not enough and caused
> the problem (on msys?) for Hannes. If the worktree somehow is '//' and
> sanitized is '/etc/resolv.conf', then we could end up eating two
> chars, leading to "'tc/resolv.conf' not match" error.
>
> Hannes, can you put a "printf("%s\n", work_tree);" in prefix_path() to
> see if it's the case?
Silly me. You used relative path, so that code path was untouched.
-- 
Duy
Johannes Sixt· Feb 8, 2010, 21:29 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] Support working directory located at root

On Montag, 8. Februar 2010, Nguyễn Thái Ngọc Duy wrote:
Show 27 quoted lines
> @@ -25,7 +25,7 @@ const char *prefix_path(const char *prefix, int len,
> const char *path) len = strlen(work_tree);
>  		total = strlen(sanitized) + 1;
>  		if (strncmp(sanitized, work_tree, len) ||
> -		    (sanitized[len] != '\0' && sanitized[len] != '/')) {
> +		    (len > 1 && sanitized[len] != '\0' && sanitized[len] != '/')) {
>  		error_out:
>  			die("'%s' is outside repository", orig);
>  		}
> @@ -403,7 +403,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
>  			if (!work_tree_env)
>  				inside_work_tree = 0;
>  			if (offset != len) {
> -				cwd[offset] = '\0';
> +				cwd[offset ? offset : 1] = '\0';
>  				set_git_dir(cwd);
>  			} else
>  				set_git_dir(".");
> @@ -427,6 +427,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
>  	inside_git_dir = 0;
>  	if (!work_tree_env)
>  		inside_work_tree = 1;
> +	if (offset == 0) /* reached root, set worktree to '/' */
> +		offset = 1;
>  	git_work_tree_cfg = xstrndup(cwd, offset);
>  	if (check_repository_format_gently(nongit_ok))
>  		return NULL;
Does not work:

etc@master:1028> ~/Src/git/git/git add resolv.conf fatal: pathspec 'tc/resolv.conf' did not match any files

I wonder how this works on Windows where we do not want to strip the slash from C:/ either.

-- Hannes

← back to recent threads