[PATCH 1/4] make_absolute_path(): Do not append redundant slash

[PATCH 1/4] make_absolute_path(): Do not append redundant slash

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

make_absolute_path("foo") at root returns "//foo". This patch makes it
return "/foo" correctly.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 .. so that "git init" will show "initialized empty Git in /.git"

 abspath.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/abspath.c b/abspath.c
index b88122c..e72aede 100644
--- a/abspath.c
+++ b/abspath.c
@@ -54,8 +54,9 @@ const char *make_absolute_path(const char *path)
 			if (len + strlen(last_elem) + 2 > PATH_MAX)
 				die ("Too long path name: '%s/%s'",
 						buf, last_elem);
-			buf[len] = '/';
-			strcpy(buf + len + 1, last_elem);
+			if (*buf != '/' || buf[1] != '\0')
+				buf[len++] = '/';
+			strcpy(buf + len, last_elem);
 			free(last_elem);
 			last_elem = NULL;
 		}
-- 
1.7.0.rc2.182.g3adef

[PATCH 2/4] rev-parse: make --git-dir return /.git instead of //.git

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

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin-rev-parse.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index a8c5043..9c22cd1 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -647,7 +647,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				}
 				if (!getcwd(cwd, PATH_MAX))
 					die_errno("unable to get current working directory");
-				printf("%s/.git\n", cwd);
+				printf("%s%s.git\n", cwd, *cwd == '/' && cwd[1] == '\0' ? "" : "/");
 				continue;
 			}
 			if (!strcmp(arg, "--is-inside-git-dir")) {
-- 
1.7.0.rc2.182.g3adef

Re: [PATCH 2/4] rev-parse: make --git-dir return /.git instead of //.git

[Johannes Sixt]

On Dienstag, 9. Februar 2010, Nguyễn Thái Ngọc Duy wrote:
> @@ -647,7 +647,7 @@ int cmd_rev_parse(int argc, const char **argv, const
> char *prefix) }
>  				if (!getcwd(cwd, PATH_MAX))
>  					die_errno("unable to get current working directory");
> -				printf("%s/.git\n", cwd);
> +				printf("%s%s.git\n", cwd, *cwd == '/' && cwd[1] == '\0' ? "" : "/");

On Windows, when you are in the root of a drive, then cwd is "C:/", i.e. there 
is a trailing slash just as in the Unix root directory. But you do not take 
care of this situation. That is, you would print "C://".

How about:

static inline int is_root_path(const char *path)
{
	if (has_dos_drive_prefix(path))
		path += 2;
	while (is_dir_sep(*path))
		path++;
	return !*path;
}

and use it though-out your series?

(Simplify the loop to 'return is_dir_sep(*path) && !path[1];' if you can 
assume that paths are nomalized.)

-- Hannes

Re: [PATCH 2/4] rev-parse: make --git-dir return /.git instead of //.git

[Nguyen Thai Ngoc Duy]

On Wed, Feb 10, 2010 at 2:18 AM, Johannes Sixt <j6t@kdbg.org> wrote:
> On Dienstag, 9. Februar 2010, Nguyễn Thái Ngọc Duy wrote:
>> @@ -647,7 +647,7 @@ int cmd_rev_parse(int argc, const char **argv, const
>> char *prefix) }
>>                               if (!getcwd(cwd, PATH_MAX))
>>                                       die_errno("unable to get current working directory");
>> -                             printf("%s/.git\n", cwd);
>> +                             printf("%s%s.git\n", cwd, *cwd == '/' && cwd[1] == '\0' ? "" : "/");
>
> On Windows, when you are in the root of a drive, then cwd is "C:/", i.e. there
> is a trailing slash just as in the Unix root directory. But you do not take
> care of this situation. That is, you would print "C://".
>
> How about:
>
> static inline int is_root_path(const char *path)
> {
>        if (has_dos_drive_prefix(path))
>                path += 2;
>        while (is_dir_sep(*path))
>                path++;
>        return !*path;
> }
>
> and use it though-out your series?
>
> (Simplify the loop to 'return is_dir_sep(*path) && !path[1];' if you can
> assume that paths are nomalized.)

And return the length of root_path, so that I can use this function in
in setup_git_directory_gently() too. Yeah.
-- 
Duy

[PATCH 3/4] 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 "" instead of "/".

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>
---
 I said I would have code change for DOS drive too. But I take it back.
 Supporting GIT_DIR=C:\.git might be easy, GIT_DIR=C:.git is not.

 setup.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/setup.c b/setup.c
index b38cbee..4d24a74 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,7 +427,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	inside_git_dir = 0;
 	if (!work_tree_env)
 		inside_work_tree = 1;
-	git_work_tree_cfg = xstrndup(cwd, offset);
+	git_work_tree_cfg = xstrndup(cwd, offset ? offset : 1);
 	if (check_repository_format_gently(nongit_ok))
 		return NULL;
 	if (offset == len)
-- 
1.7.0.rc2.182.g3adef

Re: [PATCH 3/4] Support working directory located at root

[Johannes Sixt]

On Dienstag, 9. Februar 2010, Nguyễn Thái Ngọc Duy wrote:
>  I said I would have code change for DOS drive too. But I take it back.
>  Supporting GIT_DIR=C:\.git might be easy, GIT_DIR=C:.git is not.

One does not set GIT_DIR=C:.git; it would be insane because it means ".git in 
an unpredictable directory somewhere on drive C". It would be great to 
support GIT_DIR=C:/.git

-- Hannes

Re: [PATCH 3/4] Support working directory located at root

[Nguyen Thai Ngoc Duy]

On 2/10/10, Johannes Sixt <j6t@kdbg.org> wrote:
> On Dienstag, 9. Februar 2010, Nguyễn Thái Ngọc Duy wrote:
>  >  I said I would have code change for DOS drive too. But I take it back.
>  >  Supporting GIT_DIR=C:\.git might be easy, GIT_DIR=C:.git is not.
>
>
> One does not set GIT_DIR=C:.git; it would be insane because it means ".git in
>  an unpredictable directory somewhere on drive C". It would be great to
>  support GIT_DIR=C:/.git

A bit off topic, but make_relative_path() may need more care for the
Windows port. I thought of making relative path between C:/foo and
D:/bar but it should work well for that case. //machine/share1/foo and
//machine/share2/bar may fail though.
-- 
Duy

[PATCH 4/4] Add test for using Git at root of file system

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

This kind of test requires a throw-away root filesystem so that it can
play on. If you have such a system, go ahead, "chmod 777 /" and run
this test manually ("make test" with root permission won't work).

If you don't but have Linux, static linked busybox, rsync and root
permission, then run "t/t1509/prepare-chroot.sh /tmp/test". You will be
instructed to create /dev/null and given a command to chroot into.
Within chroot, you will be placed at directory "t" of Git source code
Feel free to burn your chroot.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 May have problem committing. Anyway I don't need commit now.

 t/t1509-root-worktree.sh  |  244 +++++++++++++++++++++++++++++++++++++++++++++
 t/t1509/excludes          |   14 +++
 t/t1509/prepare-chroot.sh |   38 +++++++
 3 files changed, 296 insertions(+), 0 deletions(-)
 create mode 100755 t/t1509-root-worktree.sh
 create mode 100644 t/t1509/excludes
 create mode 100755 t/t1509/prepare-chroot.sh

diff --git a/t/t1509-root-worktree.sh b/t/t1509-root-worktree.sh
new file mode 100755
index 0000000..ae01b6d
--- /dev/null
+++ b/t/t1509-root-worktree.sh
@@ -0,0 +1,244 @@
+#!/bin/sh
+
+test_description='Test Git when git repository is located at root
+
+This test requires write access in root. Do not bother if you do not
+have a throwaway chroot or VM.
+
+Script t1509/prepare-chroot.sh may help you setup chroot, then you
+can chroot in and execute this test from there.
+'
+
+. ./test-lib.sh
+
+test_cmp_val() {
+	echo "$1" > expected
+	echo "$2" > result
+	test_cmp expected result
+}
+
+test_vars() {
+	test_expect_success "$1: gitdir" '
+		test_cmp_val "'"$2"'" "$(git rev-parse --git-dir)"
+	'
+
+	test_expect_success "$1: worktree" '
+		test_cmp_val "'"$3"'" "$(git rev-parse --show-toplevel)"
+	'
+
+	test_expect_success "$1: prefix" '
+		test_cmp_val "'"$4"'" "$(git rev-parse --show-prefix)"
+	'
+}
+
+test_foobar_root() {
+	test_expect_success 'add relative' '
+		test -z "$(cd / && git ls-files)" &&
+		git add foo/foome &&
+		git add foo/bar/barme &&
+		git add me &&
+		( cd / && git ls-files --stage ) > result &&
+		test_cmp /ls.expected result &&
+		rm "$(git rev-parse --git-dir)/index"
+	'
+
+	test_expect_success 'add absolute' '
+		test -z "$(cd / && git ls-files)" &&
+		git add /foo/foome &&
+		git add /foo/bar/barme &&
+		git add /me &&
+		( cd / && git ls-files --stage ) > result &&
+		test_cmp /ls.expected result &&
+		rm "$(git rev-parse --git-dir)/index"
+	'
+
+}
+
+test_foobar_foo() {
+	test_expect_success 'add relative' '
+		test -z "$(cd / && git ls-files)" &&
+		git add foome &&
+		git add bar/barme &&
+		git add ../me &&
+		( cd / && git ls-files --stage ) > result &&
+		test_cmp /ls.expected result &&
+		rm "$(git rev-parse --git-dir)/index"
+	'
+
+	test_expect_success 'add absolute' '
+		test -z "$(cd / && git ls-files)" &&
+		git add /foo/foome &&
+		git add /foo/bar/barme &&
+		git add /me &&
+		( cd / && git ls-files --stage ) > result &&
+		test_cmp /ls.expected result &&
+		rm "$(git rev-parse --git-dir)/index"
+	'
+}
+
+test_foobar_foobar() {
+	test_expect_success 'add relative' '
+		test -z "$(cd / && git ls-files)" &&
+		git add ../foome &&
+		git add barme &&
+		git add ../../me &&
+		( cd / && git ls-files --stage ) > result &&
+		test_cmp /ls.expected result &&
+		rm "$(git rev-parse --git-dir)/index"
+	'
+
+	test_expect_success 'add absolute' '
+		test -z "$(cd / && git ls-files)" &&
+		git add /foo/foome &&
+		git add /foo/bar/barme &&
+		git add /me &&
+		( cd / && git ls-files --stage ) > result &&
+		test_cmp /ls.expected result &&
+		rm "$(git rev-parse --git-dir)/index"
+	'
+}
+
+if ! test_have_prereq POSIXPERM || ! [ -w / ]; then
+	say "Dangerous test skipped. Read this test if you want to execute it"
+	test_done
+fi
+
+if [ "$IKNOWWHATIAMDOING" != "YES" ]; then
+	say "You must set env var IKNOWWHATIAMDOING=YES in order to run this test"
+	test_done
+fi
+
+ONE_SHA1=d00491fd7e5bb6fa28c517a0bb32b8b506539d4d
+
+test_expect_success 'setup' '
+	rm -rf /foo
+	mkdir /foo &&
+	mkdir /foo/bar &&
+	echo 1 > /foo/foome &&
+	echo 1 > /foo/bar/barme &&
+	echo 1 > /me
+'
+
+say "GIT_DIR absolute, GIT_WORK_TREE set"
+
+test_expect_success 'go to /' 'cd /'
+
+cat >ls.expected <<EOF
+100644 $ONE_SHA1 0	foo/bar/barme
+100644 $ONE_SHA1 0	foo/foome
+100644 $ONE_SHA1 0	me
+EOF
+
+export GIT_DIR="$TRASH_DIRECTORY/.git"
+export GIT_WORK_TREE=/
+
+test_vars 'abs gitdir, root' "$GIT_DIR" "/" ""
+test_foobar_root
+
+test_expect_success 'go to /foo' 'cd /foo'
+
+test_vars 'abs gitdir, foo' "$GIT_DIR" "/" "foo/"
+test_foobar_foo
+
+test_expect_success 'go to /foo/bar' 'cd /foo/bar'
+
+test_vars 'abs gitdir, foo/bar' "$GIT_DIR" "/" "foo/bar/"
+test_foobar_foobar
+
+say "GIT_DIR relative, GIT_WORK_TREE set"
+
+test_expect_success 'go to /' 'cd /'
+
+export GIT_DIR="$(echo $TRASH_DIRECTORY|sed 's,^/,,')/.git"
+export GIT_WORK_TREE=/
+
+test_vars 'rel gitdir, root' "$GIT_DIR" "/" ""
+test_foobar_root
+
+test_expect_success 'go to /foo' 'cd /foo'
+
+export GIT_DIR="../$TRASH_DIRECTORY/.git"
+export GIT_WORK_TREE=/
+
+test_vars 'rel gitdir, foo' "$TRASH_DIRECTORY/.git" "/" "foo/"
+test_foobar_foo
+
+test_expect_success 'go to /foo/bar' 'cd /foo/bar'
+
+export GIT_DIR="../../$TRASH_DIRECTORY/.git"
+export GIT_WORK_TREE=/
+
+test_vars 'rel gitdir, foo/bar' "$TRASH_DIRECTORY/.git" "/" "foo/bar/"
+test_foobar_foobar
+
+say "GIT_DIR relative, GIT_WORK_TREE relative"
+
+test_expect_success 'go to /' 'cd /'
+
+export GIT_DIR="$(echo $TRASH_DIRECTORY|sed 's,^/,,')/.git"
+export GIT_WORK_TREE=.
+
+test_vars 'rel gitdir, root' "$GIT_DIR" "/" ""
+test_foobar_root
+
+test_expect_success 'go to /' 'cd /foo'
+
+export GIT_DIR="../$TRASH_DIRECTORY/.git"
+export GIT_WORK_TREE=..
+
+test_vars 'rel gitdir, foo' "$TRASH_DIRECTORY/.git" "/" "foo/"
+test_foobar_foo
+
+test_expect_success 'go to /foo/bar' 'cd /foo/bar'
+
+export GIT_DIR="../../$TRASH_DIRECTORY/.git"
+export GIT_WORK_TREE=../..
+
+test_vars 'rel gitdir, foo/bar' "$TRASH_DIRECTORY/.git" "/" "foo/bar/"
+test_foobar_foobar
+
+say ".git at root"
+
+unset GIT_DIR
+unset GIT_WORK_TREE
+
+test_expect_success 'go to /' 'cd /'
+test_expect_success 'setup' '
+	rm -rf /.git
+	echo "Initialized empty Git repository in /.git/" > expected &&
+	git init > result &&
+	test_cmp expected result
+'
+
+test_vars 'auto gitdir, root' ".git" "/" ""
+test_foobar_root
+
+test_expect_success 'go to /foo' 'cd /foo'
+test_vars 'auto gitdir, foo' "/.git" "/" "foo/"
+test_foobar_foo
+
+test_expect_success 'go to /foo/bar' 'cd /foo/bar'
+test_vars 'auto gitdir, foo/bar' "/.git" "/" "foo/bar/"
+test_foobar_foobar
+
+test_expect_success 'cleanup' 'rm -rf /.git'
+
+say "auto bare gitdir"
+
+# DESTROYYYYY!!!!!
+test_expect_failure 'setup' '
+	rm -rf /refs /objects /info /hooks
+	rm /*
+	cd / &&
+	echo "Initialized empty Git repository in /" > expected &&
+	git init --bare > result &&
+	test_cmp expected result
+'
+
+test_vars 'auto gitdir, root' "." "" ""
+
+test_expect_success 'go to /foo' 'cd /foo'
+
+test_vars 'auto gitdir, root' "/" "" ""
+
+test_done
diff --git a/t/t1509/excludes b/t/t1509/excludes
new file mode 100644
index 0000000..d4d21d3
--- /dev/null
+++ b/t/t1509/excludes
@@ -0,0 +1,14 @@
+*.o
+*~
+*.bak
+*.c
+*.h
+.git
+contrib
+Documentation
+git-gui
+gitk-git
+gitweb
+t/t4013
+t/t5100
+t/t5515
diff --git a/t/t1509/prepare-chroot.sh b/t/t1509/prepare-chroot.sh
new file mode 100755
index 0000000..407f2d2
--- /dev/null
+++ b/t/t1509/prepare-chroot.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+
+die() {
+	echo >&2 "$@"
+	exit 1
+}
+
+xmkdir() {
+	while [ -n "$1" ]; do
+		[ -d "$1" ] || mkdir "$1" || die "Unable to mkdir $1"
+		shift
+	done
+}
+
+R="$1"
+
+[ -n "$R" ] || die "Usage: prepare-chroot.sh <root>"
+[ -x git ] || die "This script needs to be executed at git source code's top directory"
+[ -x /bin/busybox ] || die "You need busybox"
+
+xmkdir "$R" "$R/bin" "$R/etc" "$R/lib" "$R/dev"
+[ -c "$R/dev/null" ] || die "/dev/null is missing. Do mknod $R/dev/null c 1 3 && chmod 666 $R/dev/null"
+echo "root:x:0:0:root:/:/bin/sh" > "$R/etc/passwd"
+echo "$(id -nu):x:$(id -u):$(id -g)::$(pwd)/t:/bin/sh" >> "$R/etc/passwd"
+echo "root::0:root" > "$R/etc/group"
+echo "$(id -ng)::$(id -g):$(id -nu)" >> "$R/etc/group"
+
+[ -x "$R/bin/busybox" ] || cp /bin/busybox "$R/bin/busybox"
+[ -x "$R/bin/sh" ] || ln -s /bin/busybox "$R/bin/sh"
+[ -x "$R/bin/su" ] || ln -s /bin/busybox "$R/bin/su"
+
+mkdir -p "$R$(pwd)"
+rsync --exclude-from t/t1509/excludes -Ha . "$R$(pwd)"
+ldd git | grep '=> /' | sed 's,.* => *\([^ ]*\) .*,\1,' | while read i; do
+	mkdir -p "$R$(dirname $i)"
+	cp "$i" "$R/$i"
+done
+echo "Execute this in root: 'chroot $R /bin/su - $(id -nu)'"
-- 
1.7.0.rc2.182.g3adef

Re: [PATCH 4/4] Add test for using Git at root of file system

[João Carlos Mendes Luís]

Nguyễn Thái Ngọc Duy wrote:
> This kind of test requires a throw-away root filesystem so that it can
> play on. If you have such a system, go ahead, "chmod 777 /" and run
> this test manually ("make test" with root permission won't work).
>   

I've seen you have a prepare-chroot.sh file in there.  Is it working or 
not?  I mean, did you create a chrooted environment to test, or there 
was any problem with that?

Re: [PATCH 4/4] Add test for using Git at root of file system

[Nguyen Thai Ngoc Duy]

2010/2/10 João Carlos Mendes Luís <jonny@jonny.eng.br>:
>
>
> Nguyễn Thái Ngọc Duy wrote:
>>
>> This kind of test requires a throw-away root filesystem so that it can
>> play on. If you have such a system, go ahead, "chmod 777 /" and run
>> this test manually ("make test" with root permission won't work).
>>
>
> I've seen you have a prepare-chroot.sh file in there.  Is it working or not?
>  I mean, did you create a chrooted environment to test, or there was any
> problem with that?

That means if you already have a chroot environment, use it. Or you
can use prepare-chroot.sh to create a new chroot environment. Yes I
used prepare-chroot.sh for my testing.
-- 
Duy

Re: [PATCH 1/4] make_absolute_path(): Do not append redundant slash

[Johannes Sixt]

On Dienstag, 9. Februar 2010, Nguyễn Thái Ngọc Duy wrote:
> @@ -54,8 +54,9 @@ const char *make_absolute_path(const char *path)
>  			if (len + strlen(last_elem) + 2 > PATH_MAX)
>  				die ("Too long path name: '%s/%s'",
>  						buf, last_elem);
> -			buf[len] = '/';
> -			strcpy(buf + len + 1, last_elem);
> +			if (*buf != '/' || buf[1] != '\0')
> +				buf[len++] = '/';

Huh? You are adding a slash unless buf is exactly "/". That is, when buf 
is "/foo/" you still add a slash? That's not exactly avoiding redundancy. 
(Disclaimer: I didn't analyze the rest of the function whether my claim is 
true.)

> +			strcpy(buf + len, last_elem);
>  			free(last_elem);
>  			last_elem = NULL;
>  		}

-- Hannes

Re: [PATCH 1/4] make_absolute_path(): Do not append redundant slash

[Nguyen Thai Ngoc Duy]

On Wed, Feb 10, 2010 at 2:10 AM, Johannes Sixt <j6t@kdbg.org> wrote:
> On Dienstag, 9. Februar 2010, Nguyễn Thái Ngọc Duy wrote:
>> @@ -54,8 +54,9 @@ const char *make_absolute_path(const char *path)
>>                       if (len + strlen(last_elem) + 2 > PATH_MAX)
>>                               die ("Too long path name: '%s/%s'",
>>                                               buf, last_elem);
>> -                     buf[len] = '/';
>> -                     strcpy(buf + len + 1, last_elem);
>> +                     if (*buf != '/' || buf[1] != '\0')
>> +                             buf[len++] = '/';
>
> Huh? You are adding a slash unless buf is exactly "/". That is, when buf
> is "/foo/" you still add a slash? That's not exactly avoiding redundancy.
> (Disclaimer: I didn't analyze the rest of the function whether my claim is
> true.)

buf is set by getcwd() so it should never be "/foo/" but doing

if (len && buf[len-1] != '/') buf[len++] = '/';

is probably clearer (and works on Windows too).
-- 
Duy