threads / patch / 12163

patch, 5 partsTeach resolve_gitlink_ref() about the .git file

Subject: [PATCH 3/5] Teach resolve_gitlink_ref() about the .git file

## tl;dr

17 messages between Feb 17, 2008 and Feb 18, 2008. Diffs are folded; open one to read it.

replies: 16people: 3as markdown or json

Lars Hjemli· Feb 17, 2008, 22:14 UTC · lore

Intoducing the .git file (again)

These patches enables .git to be a textfile containing the path to the git directory proper. It passes all the tests so hopefully there are no regressions, but there may be bugs and omissions lurking when this feature is actually used; I've exercised it in my git, cgit and dayjob repos but git is big and has many codepaths so I wouldn't be suprised if there still are some git commands left which fails to obey the .git file.

PS: These patches could certainly be squashed into a single patch, but I've
left them as is to make each one easier to review.
PPS: If included, the .git file should probably be used by git-submodule to
clone submodule repositories into something like $GIT_DIR/submodules/<name>,
as that would make local submodule changes more resistant to dataloss due to
checkout/reset in the containing repository.
Shortlog:
 Add platform-independent .git "symlink"
 Fix setup of $GIT_DIR in git-sh-setup.sh
 Teach resolve_gitlink_ref() about the .git file
 git-submodule: prepare for the .git-file
 Teach GIT-VERSION-GEN about the .git file
Diffstat:
 Documentation/repository-layout.txt |    5 ++-
 GIT-VERSION-GEN                     |    2 +-
 cache.h                             |    1 +
 environment.c                       |   38 ++++++++++++++++++
 git-sh-setup.sh                     |   12 ++---
 git-submodule.sh                    |    4 +-
 refs.c                              |   17 +++++++-
 setup.c                             |    9 ++++
 t/t0002-gitfile.sh                  |   74 +++++++++++++++++++++++++++++++++++
 9 files changed, 148 insertions(+), 14 deletions(-)
Lars Hjemli· Feb 17, 2008, 22:14 UTC · re: Lars Hjemli · lore

[PATCH 1/5] Add platform-independent .git "symlink"

This patch allows .git to be a regular textfile containing the path of the real git directory (formatted like "gitdir: <path>\n"), which is useful on platforms lacking support for real symlinks.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 Documentation/repository-layout.txt |    5 ++-
 cache.h                             |    1 +
 environment.c                       |   38 ++++++++++++++++++
 setup.c                             |    9 ++++
 t/t0002-gitfile.sh                  |   74 +++++++++++++++++++++++++++++++++++
 5 files changed, 126 insertions(+), 1 deletions(-)
 create mode 100755 t/t0002-gitfile.sh
Show changes to 5 files +126 −1

Documentation/repository-layout.txt, cache.h, environment.c, setup.c, t/t0002-gitfile.sh

diff --git a/Documentation/repository-layout.txt b/Documentation/repository-layout.txt
index 6939130..e9db3a1 100644
--- a/Documentation/repository-layout.txt
+++ b/Documentation/repository-layout.txt
@@ -3,7 +3,10 @@ git repository layout
 
 You may find these things in your git repository (`.git`
 directory for a repository associated with your working tree, or
-`'project'.git` directory for a public 'bare' repository).
+`'project'.git` directory for a public 'bare' repository. It is
+also possible to have a working tree where `.git` is a plain
+ascii file containing `gitdir: <path>\n`, i.e. the path to the
+real git repository).
 
 objects::
 	Object store associated with this repository.  Usually
diff --git a/cache.h b/cache.h
index e1000bc..1ad822a 100644
--- a/cache.h
+++ b/cache.h
@@ -277,6 +277,7 @@ extern char *get_index_file(void);
 extern char *get_graft_file(void);
 extern int set_git_dir(const char *path);
 extern const char *get_git_work_tree(void);
+extern const char *read_gitfile_gently(const char *path);
 
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 
diff --git a/environment.c b/environment.c
index 3527f16..d120e8f 100644
--- a/environment.c
+++ b/environment.c
@@ -45,10 +45,48 @@ static const char *work_tree;
 static const char *git_dir;
 static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
 
+/*
+ * Try to read the location of the git directory from the .git file,
+ * return path to git directory if found.
+ * Format of the .git file is
+ *    gitdir: <path>\n
+ */
+const char *read_gitfile_gently(const char *path)
+{
+	static char buf[PATH_MAX + 9];  /* "gitdir: " + "\n" */
+	struct stat st;
+	int fd;
+	size_t len;
+
+	if (stat(path, &st))
+		return NULL;
+	if (!S_ISREG(st.st_mode) || st.st_size >= sizeof(buf))
+		return NULL;
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return NULL;
+	len = read_in_full(fd, buf, sizeof(buf));
+	close(fd);
+	if (len != st.st_size)
+		return NULL;
+	if (!len || buf[len - 1] != '\n')
+		return NULL;
+	buf[len - 1] = '\0';
+	if (prefixcmp(buf, "gitdir: "))
+		return NULL;
+/*
+	if (!is_git_directory(buf + 8))
+		return NULL;
+*/
+	return make_absolute_path(buf + 8);
+}
+
 static void setup_git_env(void)
 {
 	git_dir = getenv(GIT_DIR_ENVIRONMENT);
 	if (!git_dir)
+		git_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
+	if (!git_dir)
 		git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
 	git_object_dir = getenv(DB_ENVIRONMENT);
 	if (!git_object_dir) {
diff --git a/setup.c b/setup.c
index 4509598..ebdf64b 100644
--- a/setup.c
+++ b/setup.c
@@ -247,6 +247,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);
 	static char cwd[PATH_MAX+1];
 	const char *gitdirenv;
+	const char *gitfile_dir;
 	int len, offset;
 
 	/*
@@ -293,8 +294,10 @@ const char *setup_git_directory_gently(int *nongit_ok)
 
 	/*
 	 * Test in the following order (relative to the cwd):
+	 * - .git (file containing "gitdir: <path>\n")
 	 * - .git/
 	 * - ./ (bare)
+	 * - ../.git (file containing "gitdir: <path>\n")
 	 * - ../.git/
 	 * - ../ (bare)
 	 * - ../../.git/
@@ -302,6 +305,12 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	 */
 	offset = len = strlen(cwd);
 	for (;;) {
+		gitfile_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
+		if (gitfile_dir && is_git_directory(gitfile_dir)) {
+			if (set_git_dir(gitfile_dir))
+				return NULL;
+			break;
+		}
 		if (is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT))
 			break;
 		if (is_git_directory(".")) {
diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
new file mode 100755
index 0000000..d280663
--- /dev/null
+++ b/t/t0002-gitfile.sh
@@ -0,0 +1,74 @@
+#!/bin/sh
+
+test_description='.git file
+
+Verify that plumbing commands work when .git is a file
+'
+. ./test-lib.sh
+
+objpath() {
+    echo "$1" | sed -e 's|\(..\)|\1/|'
+}
+
+objck() {
+	p=$(objpath "$1")
+	if test ! -f "$REAL/objects/$p"
+	then
+		echo "Object not found: $REAL/objects/$p"
+		false
+	fi
+}
+
+test_expect_success 'setup' '
+	REAL="$(pwd)/.real" &&
+	mv .git "$REAL" &&
+	echo "gitdir: $REAL" >.git
+'
+
+test_expect_success 'check rev-parse --git-dir' '
+	test "$REAL" = "$(git rev-parse --git-dir)"
+'
+
+test_expect_success 'check hash-object' '
+	echo "foo" >bar &&
+	SHA=$(cat bar | git hash-object -w --stdin) &&
+	objck $SHA
+'
+
+test_expect_success 'check cat-file' '
+	git cat-file blob $SHA >actual &&
+	diff -u bar actual
+'
+
+test_expect_success 'check update-index' '
+	if test -f "$REAL/index"
+	then
+		echo "Hmm, $REAL/index exists?"
+		false
+	fi &&
+	rm -f "$REAL/objects/$(objpath $SHA)" &&
+	git update-index --add bar &&
+	if ! test -f "$REAL/index"
+	then
+		echo "$REAL/index not found"
+		false
+	fi &&
+	objck $SHA
+'
+
+test_expect_success 'check write-tree' '
+	SHA=$(git write-tree) &&
+	objck $SHA
+'
+
+test_expect_success 'check commit-tree' '
+	SHA=$(echo "commit bar" | git commit-tree $SHA) &&
+	objck $SHA
+'
+
+test_expect_success 'check rev-list' '
+	echo $SHA >"$REAL/HEAD" &&
+	test "$SHA" = "$(git rev-list HEAD)"
+'
+
+test_done

-- 
1.5.4.1.188.gdfa6c
Lars Hjemli· Feb 17, 2008, 22:14 UTC · re: Lars Hjemli · lore

[PATCH 2/5] Fix setup of $GIT_DIR in git-sh-setup.sh

Since .git can be a file refering to the real GIT_DIR, git-sh-setup needs to use 'git rev-parse --git-dir' to obtain the location of the git repository.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 git-sh-setup.sh |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)
Show changes to git-sh-setup.sh +5 −7
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index f388275..a7dbce2 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -127,20 +127,18 @@ get_author_ident_from_commit () {
 # if we require to be in a git repository.
 if test -z "$NONGIT_OK"
 then
+	GIT_DIR=$(git rev-parse --git-dir) || {
+		exit=$?
+		echo >&2 "Failed to find a valid git directory."
+		exit $exit
+	}
 	if [ -z "$SUBDIRECTORY_OK" ]
 	then
-		: ${GIT_DIR=.git}
 		test -z "$(git rev-parse --show-cdup)" || {
 			exit=$?
 			echo >&2 "You need to run this command from the toplevel of the working tree."
 			exit $exit
 		}
-	else
-		GIT_DIR=$(git rev-parse --git-dir) || {
-		    exit=$?
-		    echo >&2 "Failed to find a valid git directory."
-		    exit $exit
-		}
 	fi
 	test -n "$GIT_DIR" && GIT_DIR=$(cd "$GIT_DIR" && pwd) || {
 		echo >&2 "Unable to determine absolute path of git directory"

-- 
1.5.4.1.188.gdfa6c
Lars Hjemli· Feb 17, 2008, 22:14 UTC · re: Lars Hjemli · lore

When .git in a submodule is a file, resolve_gitlink_ref() needs to pick up the real GIT_DIR of the submodule from that file.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 refs.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)
Show changes to refs.c +14 −3
diff --git a/refs.c b/refs.c
index 67d2a50..56de5cf 100644
--- a/refs.c
+++ b/refs.c
@@ -351,6 +351,7 @@ int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *re
 {
 	int len = strlen(path), retval;
 	char *gitdir;
+	const char *tmp;
 
 	while (len && path[len-1] == '/')
 		len--;
@@ -358,9 +359,19 @@ int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *re
 		return -1;
 	gitdir = xmalloc(len + MAXREFLEN + 8);
 	memcpy(gitdir, path, len);
-	memcpy(gitdir + len, "/.git/", 7);
-
-	retval = resolve_gitlink_ref_recursive(gitdir, len+6, refname, result, 0);
+	memcpy(gitdir + len, "/.git", 6);
+	len += 5;
+
+	tmp = read_gitfile_gently(gitdir);
+	if (tmp) {
+		free(gitdir);
+		len = strlen(tmp);
+		gitdir = xmalloc(len + MAXREFLEN + 3);
+		memcpy(gitdir, tmp, len);
+	}
+	gitdir[len] = '/';
+	gitdir[++len] = '\0';
+	retval = resolve_gitlink_ref_recursive(gitdir, len, refname, result, 0);
 	free(gitdir);
 	return retval;
 }

-- 
1.5.4.1.188.gdfa6c
Lars Hjemli· Feb 17, 2008, 22:14 UTC · re: Lars Hjemli · lore

[PATCH 4/5] git-submodule: prepare for the .git-file

When git-submodule tries to detect 'active' submodules, it checks for the existence of a directory named '.git'. This isn't good enough now that .git can be a file pointing to the real $GIT_DIR so the tests are changed to reflect this.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 git-submodule.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Show changes to git-submodule.sh +2 −2
diff --git a/git-submodule.sh b/git-submodule.sh
index a6aaf40..e7c08b5 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -288,7 +288,7 @@ cmd_update()
 			continue
 		fi
 
-		if ! test -d "$path"/.git
+		if ! test -d "$path"/.git -o -f "$path"/.git
 		then
 			module_clone "$path" "$url" || exit
 			subsha1=
@@ -362,7 +362,7 @@ cmd_status()
 	do
 		name=$(module_name "$path") || exit
 		url=$(git config submodule."$name".url)
-		if test -z "url" || ! test -d "$path"/.git
+		if test -z "url" || ! test -d "$path"/.git -o -f "$path"/.git
 		then
 			say "-$sha1 $path"
 			continue;

-- 
1.5.4.1.188.gdfa6c
Lars Hjemli· Feb 17, 2008, 22:14 UTC · re: Lars Hjemli · lore

[PATCH 5/5] Teach GIT-VERSION-GEN about the .git file

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 GIT-VERSION-GEN |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to GIT-VERSION-GEN +1 −1
diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
index 38a3273..2432a4f 100755
--- a/GIT-VERSION-GEN
+++ b/GIT-VERSION-GEN
@@ -11,7 +11,7 @@ LF='
 if test -f version
 then
 	VN=$(cat version) || VN="$DEF_VER"
-elif test -d .git &&
+elif test -d .git -o -f .git &&
 	VN=$(git describe --abbrev=4 HEAD 2>/dev/null) &&
 	case "$VN" in
 	*$LF*) (exit 1) ;;

-- 
1.5.4.1.188.gdfa6c
Junio C Hamano· Feb 18, 2008, 05:44 UTC · re: Lars Hjemli · lore

Re: [PATCH 2/5] Fix setup of $GIT_DIR in git-sh-setup.sh

Lars Hjemli <hjemli@gmail.com> writes:
> Since .git can be a file refering to the real GIT_DIR, git-sh-setup needs
> to use 'git rev-parse --git-dir' to obtain the location of the git
> repository.

I wonder if this depend on your [1/5]. Isn't this actually a simplification (removing 7 adding 5 lines) that applies to the mainline already? IOW, is there a downside of doing this without any of the rest of the series?

Show 8 quoted lines
> @@ -127,20 +127,18 @@ get_author_ident_from_commit () {
>  # if we require to be in a git repository.
>  if test -z "$NONGIT_OK"
>  then
> +	GIT_DIR=$(git rev-parse --git-dir) || {
> +		exit=$?
> +		echo >&2 "Failed to find a valid git directory."
> +		exit $exit

rev-parse --git-dir would have said "fatal: Not a git repository" already. Do we still need to say "Failed to find..."?

Lars Hjemli· Feb 18, 2008, 08:41 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/5] Fix setup of $GIT_DIR in git-sh-setup.sh

On Feb 18, 2008 6:44 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> Lars Hjemli <hjemli@gmail.com> writes:
>
> > Since .git can be a file refering to the real GIT_DIR, git-sh-setup needs
> > to use 'git rev-parse --git-dir' to obtain the location of the git
> > repository.
>
> I wonder if this depend on your [1/5].  Isn't this actually a
> simplification (removing 7 adding 5 lines) that applies to the
> mainline already?
True. I'll resend as [1/5] with a fixed up commit message.
Show 13 quoted lines
> > @@ -127,20 +127,18 @@ get_author_ident_from_commit () {
> >  # if we require to be in a git repository.
> >  if test -z "$NONGIT_OK"
> >  then
> > +     GIT_DIR=$(git rev-parse --git-dir) || {
> > +             exit=$?
> > +             echo >&2 "Failed to find a valid git directory."
> > +             exit $exit
>
> rev-parse --git-dir would have said "fatal: Not a git
> repository" already.  Do we still need to say "Failed to
> find..."?
>
Absolutely not. Thanks.
-- 
larsh
Johannes Schindelin· Feb 17, 2008, 22:25 UTC · re: Lars Hjemli · lore

Re: [PATCH 1/5] Add platform-independent .git "symlink"

Hi,
On Sun, 17 Feb 2008, Lars Hjemli wrote:
Show 8 quoted lines
> @@ -302,6 +305,12 @@ const char *setup_git_directory_gently(int *nongit_ok)
>  	 */
>  	offset = len = strlen(cwd);
>  	for (;;) {
> +		gitfile_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
> +		if (gitfile_dir && is_git_directory(gitfile_dir)) {
> +			if (set_git_dir(gitfile_dir))
> +				return NULL;
Should this not complain loudly?  Really loudly, as in die()?

Ciao, Dscho

Lars Hjemli· Feb 17, 2008, 22:37 UTC · re: Johannes Schindelin · lore

Re: [PATCH 1/5] Add platform-independent .git "symlink"

On Feb 17, 2008 11:25 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 12 quoted lines
> On Sun, 17 Feb 2008, Lars Hjemli wrote:
>
> > @@ -302,6 +305,12 @@ const char *setup_git_directory_gently(int *nongit_ok)
> >        */
> >       offset = len = strlen(cwd);
> >       for (;;) {
> > +             gitfile_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
> > +             if (gitfile_dir && is_git_directory(gitfile_dir)) {
> > +                     if (set_git_dir(gitfile_dir))
> > +                             return NULL;
>
> Should this not complain loudly?  Really loudly, as in die()?
Yes, I'll resend. Thanks.

-- larsh

Lars Hjemli· Feb 17, 2008, 22:50 UTC · re: Lars Hjemli · lore

[PATCH 1/5] Add platform-independent .git "symlink"

This patch allows .git to be a regular textfile containing the path of the real git directory (formatted like "gitdir: <path>\n"), which is useful on platforms lacking support for real symlinks.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 Documentation/repository-layout.txt |    5 ++-
 cache.h                             |    1 +
 environment.c                       |   38 ++++++++++++++++++
 setup.c                             |    9 ++++
 t/t0002-gitfile.sh                  |   74 +++++++++++++++++++++++++++++++++++
 5 files changed, 126 insertions(+), 1 deletions(-)
 create mode 100755 t/t0002-gitfile.sh
Show changes to 5 files +126 −1

Documentation/repository-layout.txt, cache.h, environment.c, setup.c, t/t0002-gitfile.sh

diff --git a/Documentation/repository-layout.txt b/Documentation/repository-layout.txt
index 6939130..e9db3a1 100644
--- a/Documentation/repository-layout.txt
+++ b/Documentation/repository-layout.txt
@@ -3,7 +3,10 @@ git repository layout
 
 You may find these things in your git repository (`.git`
 directory for a repository associated with your working tree, or
-`'project'.git` directory for a public 'bare' repository).
+`'project'.git` directory for a public 'bare' repository. It is
+also possible to have a working tree where `.git` is a plain
+ascii file containing `gitdir: <path>\n`, i.e. the path to the
+real git repository).
 
 objects::
 	Object store associated with this repository.  Usually
diff --git a/cache.h b/cache.h
index e1000bc..1ad822a 100644
--- a/cache.h
+++ b/cache.h
@@ -277,6 +277,7 @@ extern char *get_index_file(void);
 extern char *get_graft_file(void);
 extern int set_git_dir(const char *path);
 extern const char *get_git_work_tree(void);
+extern const char *read_gitfile_gently(const char *path);
 
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 
diff --git a/environment.c b/environment.c
index 3527f16..d120e8f 100644
--- a/environment.c
+++ b/environment.c
@@ -45,10 +45,48 @@ static const char *work_tree;
 static const char *git_dir;
 static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
 
+/*
+ * Try to read the location of the git directory from the .git file,
+ * return path to git directory if found.
+ * Format of the .git file is
+ *    gitdir: <path>\n
+ */
+const char *read_gitfile_gently(const char *path)
+{
+	static char buf[PATH_MAX + 9];  /* "gitdir: " + "\n" */
+	struct stat st;
+	int fd;
+	size_t len;
+
+	if (stat(path, &st))
+		return NULL;
+	if (!S_ISREG(st.st_mode) || st.st_size >= sizeof(buf))
+		return NULL;
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return NULL;
+	len = read_in_full(fd, buf, sizeof(buf));
+	close(fd);
+	if (len != st.st_size)
+		return NULL;
+	if (!len || buf[len - 1] != '\n')
+		return NULL;
+	buf[len - 1] = '\0';
+	if (prefixcmp(buf, "gitdir: "))
+		return NULL;
+/*
+	if (!is_git_directory(buf + 8))
+		return NULL;
+*/
+	return make_absolute_path(buf + 8);
+}
+
 static void setup_git_env(void)
 {
 	git_dir = getenv(GIT_DIR_ENVIRONMENT);
 	if (!git_dir)
+		git_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
+	if (!git_dir)
 		git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
 	git_object_dir = getenv(DB_ENVIRONMENT);
 	if (!git_object_dir) {
diff --git a/setup.c b/setup.c
index 4509598..f8ebaf5 100644
--- a/setup.c
+++ b/setup.c
@@ -247,6 +247,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);
 	static char cwd[PATH_MAX+1];
 	const char *gitdirenv;
+	const char *gitfile_dir;
 	int len, offset;
 
 	/*
@@ -293,8 +294,10 @@ const char *setup_git_directory_gently(int *nongit_ok)
 
 	/*
 	 * Test in the following order (relative to the cwd):
+	 * - .git (file containing "gitdir: <path>\n")
 	 * - .git/
 	 * - ./ (bare)
+	 * - ../.git (file containing "gitdir: <path>\n")
 	 * - ../.git/
 	 * - ../ (bare)
 	 * - ../../.git/
@@ -302,6 +305,12 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	 */
 	offset = len = strlen(cwd);
 	for (;;) {
+		gitfile_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
+		if (gitfile_dir && is_git_directory(gitfile_dir)) {
+			if (set_git_dir(gitfile_dir))
+				die("Repository setup failed");
+			break;
+		}
 		if (is_git_directory(DEFAULT_GIT_DIR_ENVIRONMENT))
 			break;
 		if (is_git_directory(".")) {
diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
new file mode 100755
index 0000000..d280663
--- /dev/null
+++ b/t/t0002-gitfile.sh
@@ -0,0 +1,74 @@
+#!/bin/sh
+
+test_description='.git file
+
+Verify that plumbing commands work when .git is a file
+'
+. ./test-lib.sh
+
+objpath() {
+    echo "$1" | sed -e 's|\(..\)|\1/|'
+}
+
+objck() {
+	p=$(objpath "$1")
+	if test ! -f "$REAL/objects/$p"
+	then
+		echo "Object not found: $REAL/objects/$p"
+		false
+	fi
+}
+
+test_expect_success 'setup' '
+	REAL="$(pwd)/.real" &&
+	mv .git "$REAL" &&
+	echo "gitdir: $REAL" >.git
+'
+
+test_expect_success 'check rev-parse --git-dir' '
+	test "$REAL" = "$(git rev-parse --git-dir)"
+'
+
+test_expect_success 'check hash-object' '
+	echo "foo" >bar &&
+	SHA=$(cat bar | git hash-object -w --stdin) &&
+	objck $SHA
+'
+
+test_expect_success 'check cat-file' '
+	git cat-file blob $SHA >actual &&
+	diff -u bar actual
+'
+
+test_expect_success 'check update-index' '
+	if test -f "$REAL/index"
+	then
+		echo "Hmm, $REAL/index exists?"
+		false
+	fi &&
+	rm -f "$REAL/objects/$(objpath $SHA)" &&
+	git update-index --add bar &&
+	if ! test -f "$REAL/index"
+	then
+		echo "$REAL/index not found"
+		false
+	fi &&
+	objck $SHA
+'
+
+test_expect_success 'check write-tree' '
+	SHA=$(git write-tree) &&
+	objck $SHA
+'
+
+test_expect_success 'check commit-tree' '
+	SHA=$(echo "commit bar" | git commit-tree $SHA) &&
+	objck $SHA
+'
+
+test_expect_success 'check rev-list' '
+	echo $SHA >"$REAL/HEAD" &&
+	test "$SHA" = "$(git rev-list HEAD)"
+'
+
+test_done

-- 
1.5.4.1.188.gfde78
Junio C Hamano· Feb 18, 2008, 05:43 UTC · re: Lars Hjemli · lore

Re: [PATCH 1/5] Add platform-independent .git "symlink"

Lars Hjemli <hjemli@gmail.com> writes:
> This patch allows .git to be a regular textfile containing the path of
> the real git directory (formatted like "gitdir: <path>\n"), which is
> useful on platforms lacking support for real symlinks.

Hmmmm. I have suspected all along that these "platforms lacking support for real symlinks" are the ones whose native line termination convention is CRLF. How do you envision this file is initialized? By users editing the file by hand? Or some git specific tool? If the former, it might make sense to define the format as "a single line, terminated with platform line terminator" and open and read it in text mode. If the latter, we do not have to care, and "terminated with LF" is good enough.

I think a sane simplification is to allow the file to have any number of optional \r or \n at the end. We may care about allowing arbitrary and possibly crazy filenames in the tracked contents, but we can say "Sorry, you cannot create a directory 'ab\nc\r' and use it as the .git directory substitute."

Show 14 quoted lines
> +const char *read_gitfile_gently(const char *path)
> +{
> +	static char buf[PATH_MAX + 9];  /* "gitdir: " + "\n" */
> +	struct stat st;
> +	int fd;
> +	size_t len;
> +
> +	if (stat(path, &st))
> +		return NULL;
> +	if (!S_ISREG(st.st_mode) || st.st_size >= sizeof(buf))
> +		return NULL;
> +	fd = open(path, O_RDONLY);
> +	if (fd < 0)
> +		return NULL;
Up to this point it is fine "gently" semantics.
Show 9 quoted lines
> +	len = read_in_full(fd, buf, sizeof(buf));
> +	close(fd);
> +	if (len != st.st_size)
> +		return NULL;
> +	if (!len || buf[len - 1] != '\n')
> +		return NULL;
> +	buf[len - 1] = '\0';
> +	if (prefixcmp(buf, "gitdir: "))
> +		return NULL;

But I am not sure about this part. We found what claims to be the ".git" fake symlink but it is ill-formed. Don't we want to diagnose the possible breakage for the user?

> +/*
> +	if (!is_git_directory(buf + 8))
> +		return NULL;
> +*/
Likewise.
Lars Hjemli· Feb 18, 2008, 08:35 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/5] Add platform-independent .git "symlink"

On Feb 18, 2008 6:43 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 8 quoted lines
> Lars Hjemli <hjemli@gmail.com> writes:
>
> > This patch allows .git to be a regular textfile containing the path of
> > the real git directory (formatted like "gitdir: <path>\n"), which is
> > useful on platforms lacking support for real symlinks.
>
> I think a sane simplification is to allow the file to have
> any number of optional \r or \n at the end.
Agreed.
Show 13 quoted lines
> > +     len = read_in_full(fd, buf, sizeof(buf));
> > +     close(fd);
> > +     if (len != st.st_size)
> > +             return NULL;
> > +     if (!len || buf[len - 1] != '\n')
> > +             return NULL;
> > +     buf[len - 1] = '\0';
> > +     if (prefixcmp(buf, "gitdir: "))
> > +             return NULL;
>
> But I am not sure about this part.  We found what claims to be
> the ".git" fake symlink but it is ill-formed.  Don't we want to
> diagnose the possible breakage for the user?

Yes, I think I got to eager in my 'gentleness'. It's probably better to die() with an appropriate errormessage.

Show 7 quoted lines
>
> > +/*
> > +     if (!is_git_directory(buf + 8))
> > +             return NULL;
> > +*/
>
> Likewise.
True, I'll uncomment and die().
Thanks for the review.

-- larsh

Johannes Schindelin· Feb 18, 2008, 11:45 UTC · re: Lars Hjemli · lore

Re: [PATCH 1/5] Add platform-independent .git "symlink"

Hi,
On Mon, 18 Feb 2008, Lars Hjemli wrote:
Show 11 quoted lines
> On Feb 18, 2008 6:43 AM, Junio C Hamano <gitster@pobox.com> wrote:
> > Lars Hjemli <hjemli@gmail.com> writes:
> >
> > > +/*
> > > +     if (!is_git_directory(buf + 8))
> > > +             return NULL;
> > > +*/
> >
> > Likewise.
> 
> True, I'll uncomment and die().
Hmm.  From check_repository_format_gently():
		if (!nongit_ok)
                        die ("...")
		warning("Expected git repo version <= %d...");
I think we want that, too.  (die() when !nongit_ok, but warn otherwise)

Ciao, Dscho

Johannes Schindelin· Feb 17, 2008, 22:20 UTC · re: Lars Hjemli · lore

Re: Intoducing the .git file (again)

Hi,
On Sun, 17 Feb 2008, Lars Hjemli wrote:
Show 5 quoted lines
> PPS: If included, the .git file should probably be used by git-submodule 
> to clone submodule repositories into something like 
> $GIT_DIR/submodules/<name>, as that would make local submodule changes 
> more resistant to dataloss due to checkout/reset in the containing 
> repository.

I don't buy that argument. For the moment, the submodules are self-contained repositories. The superproject does not even have to have a single object contained in a submodule. I'd try to keep that separation.

As for data loss, again, as a submodule is self-contained, the same rules apply to it as for any repository.

Ciao, Dscho

Lars Hjemli· Feb 17, 2008, 22:29 UTC · re: Johannes Schindelin · lore

Re: Intoducing the .git file (again)

On Feb 17, 2008 11:20 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 14 quoted lines
> Hi,
>
> On Sun, 17 Feb 2008, Lars Hjemli wrote:
>
> > PPS: If included, the .git file should probably be used by git-submodule
> > to clone submodule repositories into something like
> > $GIT_DIR/submodules/<name>, as that would make local submodule changes
> > more resistant to dataloss due to checkout/reset in the containing
> > repository.
>
> I don't buy that argument.  For the moment, the submodules are
> self-contained repositories.  The superproject does not even have to have
> a single object contained in a submodule.  I'd try to keep that
> separation.

They would still be separated and self-contained repositories, but with my suggestion the submodule repository would be moved out of the working tree of the containing repository. Which I believe is a good thing, especially when you switch from one branch in the containing repository which had the submodule in './foo' to another branch where the submodule is located in './lib/foo'.

-- larsh

Johannes Schindelin· Feb 17, 2008, 23:27 UTC · re: Lars Hjemli · lore

Re: Intoducing the .git file (again)

Hi,
On Sun, 17 Feb 2008, Lars Hjemli wrote:
Show 22 quoted lines
> On Feb 17, 2008 11:20 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
> > On Sun, 17 Feb 2008, Lars Hjemli wrote:
> >
> > > PPS: If included, the .git file should probably be used by 
> > > git-submodule to clone submodule repositories into something like 
> > > $GIT_DIR/submodules/<name>, as that would make local submodule 
> > > changes more resistant to dataloss due to checkout/reset in the 
> > > containing repository.
> >
> > I don't buy that argument.  For the moment, the submodules are 
> > self-contained repositories.  The superproject does not even have to 
> > have a single object contained in a submodule.  I'd try to keep that 
> > separation.
> 
> They would still be separated and self-contained repositories, but with 
> my suggestion the submodule repository would be moved out of the working 
> tree of the containing repository. Which I believe is a good thing, 
> especially when you switch from one branch in the containing repository 
> which had the submodule in './foo' to another branch where the submodule 
> is located in './lib/foo'.
That is a good argument.

But what about the argument when you stop being interested in the superproject, and move the submodule out of it (then deleting the superproject)?

However, when a submodule is deleted, wasn't the plan all along to put the submodule's .git/ into the superproject's .git/submodules-deleted/ or some such?

Ciao, Dscho

← back to recent threads