[PATCH 0/2] Support relative .git file in a submodule

[PATCH 0/2] Support relative .git file in a submodule

[Brad King]

While experimenting with submodules I discovered that if a submodule
has a .git file "symlink" with a relative path to the real submodule
repository then it cannot be added to the superproject:

$ git init
$ mkdir sub
$ cd sub
$ git init
$ mv .git .real
$ echo 'gitdir: .real' > .git
$ echo a > a
$ git add a
$ git commit -m a
$ cd ..
$ git add sub
fatal: Not a git repository: .real

This patch series adds a test demonstrating the problem, and then
fixes it.

Brad King (2):
  Test update-index for a gitlink to a .git file
  Handle relative paths in submodule .git files

 setup.c                         |   17 +++++++++++++++++
 t/t2104-update-index-gitfile.sh |   38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 0 deletions(-)
 create mode 100755 t/t2104-update-index-gitfile.sh

[PATCH 1/2] Test update-index for a gitlink to a .git file

[Brad King]

Check that update-index recognizes a submodule that uses a .git file.
Currently it works when the .git file specifies an absolute path, but
not when it specifies a relative path.

Signed-off-by: Brad King <brad.king@kitware.com>
---
 t/t2104-update-index-gitfile.sh |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)
 create mode 100755 t/t2104-update-index-gitfile.sh

diff --git a/t/t2104-update-index-gitfile.sh b/t/t2104-update-index-gitfile.sh
new file mode 100755
index 0000000..ba71984
--- /dev/null
+++ b/t/t2104-update-index-gitfile.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Brad King
+#
+
+test_description='git update-index for gitlink to .git file.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'submodule with absolute .git file' '
+	mkdir sub1 &&
+	(cd sub1 &&
+	 git init &&
+	 REAL="$(pwd)/.real" &&
+	 mv .git "$REAL"
+	 echo "gitdir: $REAL" >.git &&
+	 test_commit first)
+'
+
+test_expect_success 'add gitlink to absolute .git file' '
+	git update-index --add -- sub1
+'
+
+test_expect_success 'submodule with relative .git file' '
+	mkdir sub2 &&
+	(cd sub2 &&
+	 git init &&
+	 mv .git .real &&
+	 echo "gitdir: .real" >.git &&
+	 test_commit first)
+'
+
+test_expect_failure 'add gitlink to relative .git file' '
+	git update-index --add -- sub2
+'
+
+test_done
-- 
1.6.5

[PATCH 2/2] Handle relative paths in submodule .git files

[Brad King]

Commit 842abf06f36b5b31050db6406265972e3e1cc189 taught
resolve_gitlink_ref() to call read_gitfile_gently() to resolve .git
files.  However, read_gitfile_gently() needs to chdir over to the
directory containing the .git file to resolve relative paths correctly.

Signed-off-by: Brad King <brad.king@kitware.com>
---
 setup.c                         |   17 +++++++++++++++++
 t/t2104-update-index-gitfile.sh |    2 +-
 2 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/setup.c b/setup.c
index 2cf0f19..a233e01 100644
--- a/setup.c
+++ b/setup.c
@@ -255,6 +255,8 @@ const char *read_gitfile_gently(const char *path)
 	struct stat st;
 	int fd;
 	size_t len;
+	char cwd[1024] = "";
+	const char *slash;
 
 	if (stat(path, &st))
 		return NULL;
@@ -276,9 +278,24 @@ const char *read_gitfile_gently(const char *path)
 	if (len < 9)
 		die("No path in gitfile: %s", path);
 	buf[len] = '\0';
+
+	slash = strrchr(path, '/');
+	if (slash) {
+		char *dir = xstrndup(path, slash - path);
+		if (!getcwd(cwd, sizeof(cwd)))
+			die_errno ("Could not get current working directory");
+		if (chdir(dir))
+			die_errno ("Could not switch to '%s'", dir);
+		free(dir);
+	}
+
 	if (!is_git_directory(buf + 8))
 		die("Not a git repository: %s", buf + 8);
 	path = make_absolute_path(buf + 8);
+
+	if (*cwd && chdir(cwd))
+		die_errno ("Could not change back to '%s'", cwd);
+
 	free(buf);
 	return path;
 }
diff --git a/t/t2104-update-index-gitfile.sh b/t/t2104-update-index-gitfile.sh
index ba71984..641607d 100755
--- a/t/t2104-update-index-gitfile.sh
+++ b/t/t2104-update-index-gitfile.sh
@@ -31,7 +31,7 @@ test_expect_success 'submodule with relative .git file' '
 	 test_commit first)
 '
 
-test_expect_failure 'add gitlink to relative .git file' '
+test_expect_success 'add gitlink to relative .git file' '
 	git update-index --add -- sub2
 '
 
-- 
1.6.5

Re: [PATCH 0/2] Support relative .git file in a submodule

[Junio C Hamano]

Brad King <brad.king@kitware.com> writes:

> ... if a submodule
> has a .git file "symlink" with a relative path to the real submodule
> repository then ...

... then I've always thought that is simply a misconfiguration (t0002
seems to use full path for this exact reason).  Is there a reason why
relative path should be used/usable here, other than "being able to is
better than not being able to"???

I don't like my process randomly chdir'ing around assuming they can chdir
back safely very much, and would prefer not to add such codepaths unless
absolutely necessary.

Re: [PATCH 0/2] Support relative .git file in a submodule

[Avery Pennarun]

On Fri, Jan 8, 2010 at 6:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Brad King <brad.king@kitware.com> writes:
>
>> ... if a submodule
>> has a .git file "symlink" with a relative path to the real submodule
>> repository then ...
>
> ... then I've always thought that is simply a misconfiguration (t0002
> seems to use full path for this exact reason).  Is there a reason why
> relative path should be used/usable here, other than "being able to is
> better than not being able to"???

If I have a bunch of git repos in ~/src, and I decide I'd rather
rename it all to ~/source, it seems like it would be nice for all my
links not to be broken.  This sort of thing can also happen if you
have NFS-mounted home directories on a farm of machines, and some of
them automount in /u/username and others use /home/username, for
example.  I think this is the same reason that common sysadmin advice
is to use relative symlinks instead of absolute links.

This problem seems especially true with submodules.  If the
submodule's repo is something like supermodule/.git/submodule.git, a
relative path would almost always be a appropriate, no?

Have fun,

Avery

[PATCH v2 0/2] Support relative .git file in a submodule

[Brad King]

Junio C Hamano wrote:
> then I've always thought that is simply a misconfiguration (t0002
> seems to use full path for this exact reason).

Everything in that test works with REAL=.real except the line

  test "$REAL" = "$(git rev-parse --git-dir)"

because --git-dir returns an absolute path.

> Is there a reason why relative path should be used/usable here, other
> than "being able to is better than not being able to"???

Similar configurations already work:

 - A .git file with a relative path works inside its work tree
 - A .git symlink with a relative path works inside its work tree
 - A submodule whose .git is a real symlink with a relative path works

My patch just fixes an intuitive combination of these cases.

> I don't like my process randomly chdir'ing around assuming they can
> chdir back safely very much, and would prefer not to add such
> codepaths unless absolutely necessary.

Here is a new patch series.  Patch 1/2 is unchanged.  Patch 2/2 has been
re-written to avoid chdir.

Avery Pennarun wrote:
> This problem seems especially true with submodules.  If the
> submodule's repo is something like supermodule/.git/submodule.git, a
> relative path would almost always be a appropriate, no?

Exactly.  In fact the experiment I was doing involved creating submodule
repos inside the main .git and linking to them from the work tree
subdirectories.  I'm looking into combining the approach with that of
git-new-workdir to keep submodules in the same object database.

Brad King (2):
  Test update-index for a gitlink to a .git file
  Handle relative paths in submodule .git files

 setup.c                         |   22 +++++++++++++++++++---
 t/t2104-update-index-gitfile.sh |   38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 3 deletions(-)
 create mode 100755 t/t2104-update-index-gitfile.sh

Re: [PATCH v2 0/2] Support relative .git file in a submodule

[Junio C Hamano]

Brad King <brad.king@kitware.com> writes:

> Exactly.  In fact the experiment I was doing involved creating submodule
> repos inside the main .git and linking to them from the work tree
> subdirectories.  I'm looking into combining the approach with that of
> git-new-workdir to keep submodules in the same object database.

Nice.

I recall we discussed design ideas to allow moving the submodule worktree
and repository more easily using "gitdir:" when we first discussed the
patch series.  I am glad to see finally it is going forward.

[PATCH v2 1/2] Test update-index for a gitlink to a .git file

[Brad King]

Check that update-index recognizes a submodule that uses a .git file.
Currently it works when the .git file specifies an absolute path, but
not when it specifies a relative path.

Signed-off-by: Brad King <brad.king@kitware.com>
---
 t/t2104-update-index-gitfile.sh |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)
 create mode 100755 t/t2104-update-index-gitfile.sh

diff --git a/t/t2104-update-index-gitfile.sh b/t/t2104-update-index-gitfile.sh
new file mode 100755
index 0000000..ba71984
--- /dev/null
+++ b/t/t2104-update-index-gitfile.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Brad King
+#
+
+test_description='git update-index for gitlink to .git file.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'submodule with absolute .git file' '
+	mkdir sub1 &&
+	(cd sub1 &&
+	 git init &&
+	 REAL="$(pwd)/.real" &&
+	 mv .git "$REAL"
+	 echo "gitdir: $REAL" >.git &&
+	 test_commit first)
+'
+
+test_expect_success 'add gitlink to absolute .git file' '
+	git update-index --add -- sub1
+'
+
+test_expect_success 'submodule with relative .git file' '
+	mkdir sub2 &&
+	(cd sub2 &&
+	 git init &&
+	 mv .git .real &&
+	 echo "gitdir: .real" >.git &&
+	 test_commit first)
+'
+
+test_expect_failure 'add gitlink to relative .git file' '
+	git update-index --add -- sub2
+'
+
+test_done
-- 
1.6.5

[PATCH v2 2/2] Handle relative paths in submodule .git files

[Brad King]

Commit 842abf06f36b5b31050db6406265972e3e1cc189 taught
resolve_gitlink_ref() to call read_gitfile_gently() to resolve .git
files.  In this commit teach read_gitfile_gently() to interpret a
relative path in a .git file with respect to the file location.

This change allows update-index to recognize a submodule that uses a
relative path in its .git file.  It previously failed because the
relative path was wrongly interpreted with respect to the superproject
directory.

Signed-off-by: Brad King <brad.king@kitware.com>
---
 setup.c                         |   22 +++++++++++++++++++---
 t/t2104-update-index-gitfile.sh |    2 +-
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/setup.c b/setup.c
index 2cf0f19..f10e2dd 100644
--- a/setup.c
+++ b/setup.c
@@ -252,6 +252,8 @@ static int check_repository_format_gently(int *nongit_ok)
 const char *read_gitfile_gently(const char *path)
 {
 	char *buf;
+	char* dir;
+	const char *slash;
 	struct stat st;
 	int fd;
 	size_t len;
@@ -276,9 +278,23 @@ const char *read_gitfile_gently(const char *path)
 	if (len < 9)
 		die("No path in gitfile: %s", path);
 	buf[len] = '\0';
-	if (!is_git_directory(buf + 8))
-		die("Not a git repository: %s", buf + 8);
-	path = make_absolute_path(buf + 8);
+	dir = buf + 8;
+
+	if (!is_absolute_path(dir) && (slash = strrchr(path, '/'))) {
+		size_t pathlen = slash+1 - path;
+		size_t dirlen = pathlen + len - 8;
+		dir = xmalloc(dirlen + 1);
+		strncpy(dir, path, pathlen);
+		strncpy(dir + pathlen, buf + 8, len - 8);
+		dir[dirlen] = '\0';
+		free(buf);
+		buf = dir;
+	}
+
+	if (!is_git_directory(dir))
+		die("Not a git repository: %s", dir);
+	path = make_absolute_path(dir);
+
 	free(buf);
 	return path;
 }
diff --git a/t/t2104-update-index-gitfile.sh b/t/t2104-update-index-gitfile.sh
index ba71984..641607d 100755
--- a/t/t2104-update-index-gitfile.sh
+++ b/t/t2104-update-index-gitfile.sh
@@ -31,7 +31,7 @@ test_expect_success 'submodule with relative .git file' '
 	 test_commit first)
 '
 
-test_expect_failure 'add gitlink to relative .git file' '
+test_expect_success 'add gitlink to relative .git file' '
 	git update-index --add -- sub2
 '
 
-- 
1.6.5