# [PATCH 2/5] init-db, rev-parse --git-dir: do not append redundant slash 10 messages from 2010-02-11 to 2010-02-12. Participants: Nguyễn Thái Ngọc Duy, Johannes Sixt, João Carlos Mendes Luís. Thread: https://gitlist.dev/t/22614 ## Nguyễn Thái Ngọc Duy, 2010-02-11 14:43 Subject: [PATCH 0/5 v2] Worktree/Gitdir at root directory Message-ID: <1265899403-15904-1-git-send-email-pclouds@gmail.com> URL: https://gitlist.dev/e/1265899403-15904-1-git-send-email-pclouds%40gmail.com ``` So here we go again. Changes are: - Support DOS drives as root path (Thanks Hannes) - Fix one "broken" test that I was too lazy to do last time - t1509-root-worktree.sh will refuse to run under root permission I did not test it on Windows, so an Ack from someone who actually tests it on Windows is really appreciated. Nguyễn Thái Ngọc Duy (5): make_absolute_path(): Do not append redundant slash init-db, rev-parse --git-dir: do not append redundant slash Add is_root_path() Support working directory located at root Add test for using Git at root directory abspath.c | 5 +- builtin-init-db.c | 9 +- builtin-rev-parse.c | 4 +- git-compat-util.h | 11 ++ setup.c | 13 ++- t/t1509-root-worktree.sh | 249 +++++++++++++++++++++++++++++++++++++++++++++ t/t1509/excludes | 14 +++ t/t1509/prepare-chroot.sh | 38 +++++++ 8 files changed, 332 insertions(+), 11 deletions(-) create mode 100755 t/t1509-root-worktree.sh create mode 100644 t/t1509/excludes create mode 100755 t/t1509/prepare-chroot.sh ``` ## Nguyễn Thái Ngọc Duy, 2010-02-11 14:43 Subject: [PATCH 1/5] make_absolute_path(): Do not append redundant slash Message-ID: <1265899403-15904-2-git-send-email-pclouds@gmail.com> URL: https://gitlist.dev/e/1265899403-15904-2-git-send-email-pclouds%40gmail.com In-Reply-To: <1265899403-15904-1-git-send-email-pclouds@gmail.com> ``` When concatenating two paths, if the first one already have '/', do not put another '/' in between the two paths. Usually this is not the case as getcwd() won't return '/foo/bar/', except when you are standing at root, then it will return '/'. Signed-off-by: Nguyễn Thái Ngọc Duy --- abspath.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/abspath.c b/abspath.c index b88122c..c91a29c 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 (len && buf[len-1] != '/') + buf[len++] = '/'; + strcpy(buf + len, last_elem); free(last_elem); last_elem = NULL; } -- 1.7.0.rc2.182.g3adef ``` ## Nguyễn Thái Ngọc Duy, 2010-02-11 14:43 Subject: [PATCH 2/5] init-db, rev-parse --git-dir: do not append redundant slash Message-ID: <1265899403-15904-3-git-send-email-pclouds@gmail.com> URL: https://gitlist.dev/e/1265899403-15904-3-git-send-email-pclouds%40gmail.com In-Reply-To: <1265899403-15904-1-git-send-email-pclouds@gmail.com> ``` If git_dir already has the trailing slash, don't put another one before .git. This only happens when git_dir is '/' or 'C:/' Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin-init-db.c | 9 ++++++--- builtin-rev-parse.c | 4 +++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/builtin-init-db.c b/builtin-init-db.c index dd84cae..aae7a4d 100644 --- a/builtin-init-db.c +++ b/builtin-init-db.c @@ -331,11 +331,14 @@ int init_db(const char *template_dir, unsigned int flags) git_config_set("receive.denyNonFastforwards", "true"); } - if (!(flags & INIT_DB_QUIET)) - printf("%s%s Git repository in %s/\n", + if (!(flags & INIT_DB_QUIET)) { + const char *git_dir = get_git_dir(); + int len = strlen(git_dir); + printf("%s%s Git repository in %s%s\n", reinit ? "Reinitialized existing" : "Initialized empty", shared_repository ? " shared" : "", - get_git_dir()); + git_dir, len && git_dir[len-1] != '/' ? "/" : ""); + } return 0; } diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c index a8c5043..88bad9a 100644 --- a/builtin-rev-parse.c +++ b/builtin-rev-parse.c @@ -637,6 +637,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) if (!strcmp(arg, "--git-dir")) { const char *gitdir = getenv(GIT_DIR_ENVIRONMENT); static char cwd[PATH_MAX]; + int len; if (gitdir) { puts(gitdir); continue; @@ -647,7 +648,8 @@ 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); + len = strlen(cwd); + printf("%s%s.git\n", cwd, len && cwd[len-1] != '/' ? "/" : ""); continue; } if (!strcmp(arg, "--is-inside-git-dir")) { -- 1.7.0.rc2.182.g3adef ``` ## Nguyễn Thái Ngọc Duy, 2010-02-11 14:43 Subject: [PATCH 3/5] Add is_root_path() Message-ID: <1265899403-15904-4-git-send-email-pclouds@gmail.com> URL: https://gitlist.dev/e/1265899403-15904-4-git-send-email-pclouds%40gmail.com In-Reply-To: <1265899403-15904-1-git-send-email-pclouds@gmail.com> ``` This function returns the length of the root part of a path, or zero if there is no root. "The root part" is the leading slash on Linux/Unix, or 'C:/' on Windows. Signed-off-by: Nguyễn Thái Ngọc Duy --- ... and can be extended to recognize "//machine/share/" as a root part? git-compat-util.h | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index a3c4537..4d0398d 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -210,6 +210,17 @@ static inline const char *skip_prefix(const char *str, const char *prefix) return strncmp(str, prefix, len) ? NULL : str + len; } +/* path must be canonical */ +static inline int is_root_path(const char *path) +{ + int len = 0; + if (has_dos_drive_prefix(path)) + len += 2; + if (is_dir_sep(path[len])) + len++; + return len; +} + #if defined(NO_MMAP) || defined(USE_WIN32_MMAP) #ifndef PROT_READ -- 1.7.0.rc2.182.g3adef ``` ## Nguyễn Thái Ngọc Duy, 2010-02-11 14:43 Subject: [PATCH 4/5] Support working directory located at root Message-ID: <1265899403-15904-5-git-send-email-pclouds@gmail.com> URL: https://gitlist.dev/e/1265899403-15904-5-git-send-email-pclouds%40gmail.com In-Reply-To: <1265899403-15904-1-git-send-email-pclouds@gmail.com> ``` 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 --- setup.c | 13 ++++++++----- 1 files changed, 8 insertions(+), 5 deletions(-) diff --git a/setup.c b/setup.c index b38cbee..cdad176 100644 --- a/setup.c +++ b/setup.c @@ -18,14 +18,15 @@ const char *prefix_path(const char *prefix, int len, const char *path) if (normalize_path_copy(sanitized, sanitized)) goto error_out; if (is_absolute_path(orig)) { - size_t len, total; + size_t root_len, len, total; const char *work_tree = get_git_work_tree(); if (!work_tree) goto error_out; len = strlen(work_tree); + root_len = is_root_path(work_tree); total = strlen(sanitized) + 1; if (strncmp(sanitized, work_tree, len) || - (sanitized[len] != '\0' && sanitized[len] != '/')) { + (len > root_len && sanitized[len] != '\0' && sanitized[len] != '/')) { error_out: die("'%s' is outside repository", orig); } @@ -321,7 +322,7 @@ const char *setup_git_directory_gently(int *nongit_ok) static char cwd[PATH_MAX+1]; const char *gitdirenv; const char *gitfile_dir; - int len, offset, ceil_offset; + int len, offset, ceil_offset, root_len; /* * Let's assume that we are in a git repository. @@ -403,7 +404,8 @@ const char *setup_git_directory_gently(int *nongit_ok) if (!work_tree_env) inside_work_tree = 0; if (offset != len) { - cwd[offset] = '\0'; + root_len = is_root_path(cwd); + cwd[offset > root_len ? offset : root_len] = '\0'; set_git_dir(cwd); } else set_git_dir("."); @@ -427,7 +429,8 @@ 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); + root_len = is_root_path(cwd); + git_work_tree_cfg = xstrndup(cwd, offset > root_len ? offset : root_len); if (check_repository_format_gently(nongit_ok)) return NULL; if (offset == len) -- 1.7.0.rc2.182.g3adef ``` ## Nguyễn Thái Ngọc Duy, 2010-02-11 14:43 Subject: [PATCH 5/5] Add test for using Git at root directory Message-ID: <1265899403-15904-6-git-send-email-pclouds@gmail.com> URL: https://gitlist.dev/e/1265899403-15904-6-git-send-email-pclouds%40gmail.com In-Reply-To: <1265899403-15904-1-git-send-email-pclouds@gmail.com> ``` 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. Because this is a dangerous test, you are required to set an env variable, and not to use root to run it. Script prepare-root.sh may help you set up a chroot environment with Git test suite inside. You will need Linux, static linked busybox, rsync and root permission to use it. Signed-off-by: Nguyễn Thái Ngọc Duy --- t/t1509-root-worktree.sh | 249 +++++++++++++++++++++++++++++++++++++++++++++ t/t1509/excludes | 14 +++ t/t1509/prepare-chroot.sh | 38 +++++++ 3 files changed, 301 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..5322a3b --- /dev/null +++ b/t/t1509-root-worktree.sh @@ -0,0 +1,249 @@ +#!/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 + +if [ "$UID" = 0 ]; then + say "No you can't run this with root" + 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 < 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_success '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 " +[ -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 ``` ## Johannes Sixt, 2010-02-12 10:11 Subject: Re: [PATCH 3/5] Add is_root_path() Message-ID: <4B75294B.8080205@kdbg.org> URL: https://gitlist.dev/e/4B75294B.8080205%40kdbg.org In-Reply-To: <1265899403-15904-4-git-send-email-pclouds@gmail.com> ``` Nguyễn Thái Ngọc Duy schrieb: > This function returns the length of the root part of a path, or zero if > there is no root. That is, the function primarily doesn't act as a predicate, and in fact you never use it as such. I just noticed that this function is identical to offset_1st_component() in sha1_file.c. > --- a/git-compat-util.h > +++ b/git-compat-util.h > @@ -210,6 +210,17 @@ static inline const char *skip_prefix(const char *str, const char *prefix) > return strncmp(str, prefix, len) ? NULL : str + len; > } > > +/* path must be canonical */ > +static inline int is_root_path(const char *path) > +{ > + int len = 0; > + if (has_dos_drive_prefix(path)) > + len += 2; > + if (is_dir_sep(path[len])) > + len++; > + return len; > +} > + > #if defined(NO_MMAP) || defined(USE_WIN32_MMAP) This is not a good location for this function because on Windows we get warnings about implicitly defined function isalpha() (which is hidden in the macro has_dos_drive_prefix). I suggest to move offset_1st_component() to path.c, which would not be inlined anymore. -- Hannes ``` ## Johannes Sixt, 2010-02-12 10:21 Subject: Re: [PATCH 0/5 v2] Worktree/Gitdir at root directory Message-ID: <4B752BA1.9080105@kdbg.org> URL: https://gitlist.dev/e/4B752BA1.9080105%40kdbg.org In-Reply-To: <1265899403-15904-1-git-send-email-pclouds@gmail.com> ``` Nguyễn Thái Ngọc Duy schrieb: > So here we go again. Changes are: > > - Support DOS drives as root path (Thanks Hannes) > - Fix one "broken" test that I was too lazy to do last time > - t1509-root-worktree.sh will refuse to run under root permission > > I did not test it on Windows, so an Ack from someone who actually tests it on Windows > is really appreciated. I don't see any regressions in may tests on Windows, but I actually don't know what to test (the chroot test doesn't work on Windows). I did test on the root of a partition T:\> git init T:\> cd foo T:\foo> git add bar and it works as expected. You should address my comments to 3/5, please. -- Hannes ``` ## João Carlos Mendes Luís, 2010-02-12 13:05 Subject: Re: [PATCH 0/5 v2] Worktree/Gitdir at root directory Message-ID: <4B755229.6000502@jonny.eng.br> URL: https://gitlist.dev/e/4B755229.6000502%40jonny.eng.br In-Reply-To: <4B752BA1.9080105@kdbg.org> ``` Johannes Sixt wrote: > Nguyễn Thái Ngọc Duy schrieb: >> So here we go again. Changes are: >> >> - Support DOS drives as root path (Thanks Hannes) >> - Fix one "broken" test that I was too lazy to do last time >> - t1509-root-worktree.sh will refuse to run under root permission >> >> I did not test it on Windows, so an Ack from someone who actually >> tests it on Windows >> is really appreciated. > > I don't see any regressions in may tests on Windows, but I actually > don't know what to test (the chroot test doesn't work on Windows). I > did test on the root of a partition > > T:\> git init > T:\> cd foo > T:\foo> git add bar > > and it works as expected. Beware: My first proposed patch "worked" at this level, but failed completely when I wanted to commit. Try more operations just to be sure. The auto test routines include a ram-disk creation, for windows case, but I'm not sure how portable this would be. ``` ## Johannes Sixt, 2010-02-12 16:00 Subject: Re: [PATCH 0/5 v2] Worktree/Gitdir at root directory Message-ID: <4B757B0E.4010402@kdbg.org> URL: https://gitlist.dev/e/4B757B0E.4010402%40kdbg.org In-Reply-To: <4B755229.6000502@jonny.eng.br> ``` João Carlos Mendes Luís schrieb: > Johannes Sixt wrote: >> I don't see any regressions in may tests on Windows, but I actually >> don't know what to test (the chroot test doesn't work on Windows). I >> did test on the root of a partition >> >> T:\> git init >> T:\> cd foo >> T:\foo> git add bar >> >> and it works as expected. > > Beware: My first proposed patch "worked" at this level, but failed > completely when I wanted to commit. Try more operations just to be sure. I tried commit, fetch, checkout, and they work as expected. -- Hannes ```