threads / patch / 22138

patch, 2 partsTest update-index for a gitlink to a .git file

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

## tl;dr

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

replies: 8people: 3as markdown or json

Brad King· Jan 8, 2010, 22:36 UTC · lore

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

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
Brad King· Jan 8, 2010, 22:36 UTC · re: Brad King · lore

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
Show changes to t/t2104-update-index-gitfile.sh +38 −0
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
Brad King· Jan 8, 2010, 22:36 UTC · re: Brad King · lore

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

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(-)
Show changes to 2 files +18 −1

setup.c, t/t2104-update-index-gitfile.sh

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
Junio C Hamano· Jan 8, 2010, 23:09 UTC · re: Brad King · lore

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

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.

Avery Pennarun· Jan 8, 2010, 23:24 UTC · re: Junio C Hamano · lore

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

On Fri, Jan 8, 2010 at 6:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 10 quoted lines
> 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
Brad King· Jan 9, 2010, 03:36 UTC · re: Avery Pennarun · lore

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

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
Junio C Hamano· Jan 9, 2010, 17:20 UTC · re: Brad King · lore

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

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.

Brad King· Jan 9, 2010, 03:36 UTC · re: Avery Pennarun · lore

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

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
Show changes to t/t2104-update-index-gitfile.sh +38 −0
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
Brad King· Jan 9, 2010, 03:36 UTC · re: Avery Pennarun · lore

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

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(-)
Show changes to 2 files +20 −4

setup.c, t/t2104-update-index-gitfile.sh

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

← back to recent threads