Re: [PATCH] Support working directory located at root

Re: [PATCH] Support working directory located at root

[Johannes Sixt]

Nguyen Thai Ngoc Duy schrieb:
> But I suspect my change in this code is not enough and caused
> the problem (on msys?) for Hannes.

My test was on Linux, btw, roughly:

cd /
git init
cd etc
git add resolv.conf

-- Hannes

Re: [PATCH] Support working directory located at root

[Nguyen Thai Ngoc Duy]

On 2/9/10, Johannes Sixt <j6t@kdbg.org> wrote:
> Nguyen Thai Ngoc Duy schrieb:
>
> > But I suspect my change in this code is not enough and caused
>  > the problem (on msys?) for Hannes.
>
>
> My test was on Linux, btw, roughly:
>
>  cd /
>  git init
>  cd etc
>  git add resolv.conf

OK. Just a wild guess since you're mingw port guy :-)

I changed offset near the end of setup_git_*_gently, and in turn
screwed up prefix calculation. I probably forgot to unset
GIT_WORK_TREE while testing. Will work on a test script first before
getting back to the patch. I think I can solve the DOS drive case too.
Just checking, UNC path is not affected, right?
-- 
Duy

[PATCH] Support working directory located at root

[Nguyễn Thái Ngọc Duy]

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(-)

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

Re: [PATCH] Support working directory located at root

[Junio C Hamano]

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

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

Re: [PATCH] Support working directory located at root

[Nguyen Thai Ngoc Duy]

2010/2/9 Junio C Hamano <gitster@pobox.com>:
> 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.

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

Re: [PATCH] Support working directory located at root

[Nguyen Thai Ngoc Duy]

2010/2/9 Nguyen Thai Ngoc Duy <pclouds@gmail.com>:
> 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

Re: [PATCH] Support working directory located at root

[Johannes Sixt]

On Montag, 8. Februar 2010, Nguyễn Thái Ngọc Duy wrote:
> @@ -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