[PATCH 1/4] git-init-db should error out with a message

[PATCH 1/4] git-init-db should error out with a message

[Johannes Schindelin]

When the HEAD symref could not be created, it is helpful for the user to 
know that.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

 init-db.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

applies-to: 51f96562f1ef47cd9a09731e3f27445efaddbbe7
159c632ef3cc7371aaa495c41afd5fd41e2d3f3f
diff --git a/init-db.c b/init-db.c
index aabc09f..2c27e18 100644
--- a/init-db.c
+++ b/init-db.c
@@ -192,7 +192,7 @@ static void create_default_files(const c
 	strcpy(path + len, "HEAD");
 	if (read_ref(path, sha1) < 0) {
 		if (create_symref(path, "refs/heads/master") < 0)
-			exit(1);
+			die("Could not create HEAD symref!");
 	}
 	path[len] = 0;
 	copy_templates(path, len, template_path);
---
0.99.8.GIT

Re: [PATCH 1/4] git-init-db should error out with a message

[Alex Riesen]

Johannes Schindelin, Wed, Oct 26, 2005 01:39:24 +0200:
> When the HEAD symref could not be created, it is helpful for the user to 
> know that.
> 

Not just that. It would be interesting to give the user an option to
use the file references ("ref: refs/heads/master").
Something like that:

Add --no-symref (make init-db use file references)

---

 cache.h   |    1 +
 init-db.c |   11 +++++++++--
 refs.c    |    7 ++++++-
 3 files changed, 16 insertions(+), 3 deletions(-)

applies-to: dba443573167bb9b0023613428e6d1a69477fac6
097ca1bf9b21d19d425e8151986eb36f82cbeff3
diff --git a/cache.h b/cache.h
index d776016..e410ce2 100644
--- a/cache.h
+++ b/cache.h
@@ -239,6 +239,7 @@ extern char *sha1_to_hex(const unsigned 
 extern int read_ref(const char *filename, unsigned char *sha1);
 extern const char *resolve_ref(const char *path, unsigned char *sha1, int);
 extern int create_symref(const char *git_HEAD, const char *refs_heads_master);
+extern int create_file_symref(const char *git_HEAD, const char *refs_heads_master);
 extern int validate_symref(const char *git_HEAD);
 
 /* General helper functions */
diff --git a/init-db.c b/init-db.c
index aabc09f..2d2b705 100644
--- a/init-db.c
+++ b/init-db.c
@@ -161,6 +161,8 @@ static void copy_templates(const char *g
 	closedir(dir);
 }
 
+static int try_symref = 1;
+
 static void create_default_files(const char *git_dir,
 				 char *template_path)
 {
@@ -191,8 +193,11 @@ static void create_default_files(const c
 	 */
 	strcpy(path + len, "HEAD");
 	if (read_ref(path, sha1) < 0) {
-		if (create_symref(path, "refs/heads/master") < 0)
-			exit(1);
+		int err = 0;
+		if ( try_symref )
+			err = create_symref(path, "refs/heads/master");
+		if ( !err && create_file_symref(path, "refs/heads/master") < 0 )
+			die("cannot create %s", path);
 	}
 	path[len] = 0;
 	copy_templates(path, len, template_path);
@@ -220,6 +225,8 @@ int main(int argc, char **argv)
 			break;
 		else if (!strncmp(arg, "--template=", 11))
 			template_dir = arg+11;
+		else if (!strcmp(arg, "--no-symref"))
+			try_symref = 0;
 		else
 			die(init_db_usage);
 	}
diff --git a/refs.c b/refs.c
index 97506a4..8029667 100644
--- a/refs.c
+++ b/refs.c
@@ -120,6 +120,12 @@ int create_symref(const char *git_HEAD, 
 	unlink(git_HEAD);
 	return symlink(refs_heads_master, git_HEAD);
 #else
+	return create_file_symref(git_HEAD, refs_heads_master);
+#endif
+}
+
+int create_file_symref(const char *git_HEAD, const char *refs_heads_master)
+{
 	const char *lockpath;
 	char ref[1000];
 	int fd, len, written;
@@ -144,7 +150,6 @@ int create_symref(const char *git_HEAD, 
 		return -3;
 	}
 	return 0;
-#endif
 }
 
 int read_ref(const char *filename, unsigned char *sha1)
---
0.99.8.GIT

Re: [PATCH 1/4] git-init-db should error out with a message

[Junio C Hamano]

Alex Riesen <fork0@users.sourceforge.net> writes:

> Not just that. It would be interesting to give the user an option to
> use the file references ("ref: refs/heads/master").

Actually, the users should not have to care how HEAD reference
is implemented.  It might make sense to use regular file symref
regardless of platforms (i.e. never define USE_SYMLINK_HEAD on
any platform).

We support reading from either kind of symref, so if we did
that, the only case that *could* matter form compatibility point
of view is that repositories touched by the updated git is
unusable for an ancient git that does not understand regular
file symref.  From performance and simplicity point of view,
however, using symlink when possible is better, and that is what
Johannes' patch does.

HOWEVER, I think "falling back" (both in Johannes' patch which
is in the "master" branch, and your version) has a funny failure
mode.  What happens when two processes try redirecting .git/HEAD
simultaneously, possibly to different branch heads?  Both of
them unlink(), one successfully does symlink(), and the other
gets EEXIST and falls back to create regular file symref.

Which is probably not so wrong; if this race matters, then you
have bigger problem -- the user is doing 'git checkout' of
different branches at the same time, or something silly like
that.  But it does not feel quite right, either.

Re: [PATCH 1/4] git-init-db should error out with a message

[Alex Riesen]

Junio C Hamano, Wed, Oct 26, 2005 22:27:00 +0200:
> > Not just that. It would be interesting to give the user an option to
> > use the file references ("ref: refs/heads/master").
> 
> Actually, the users should not have to care how HEAD reference
> is implemented.  It might make sense to use regular file symref
> regardless of platforms (i.e. never define USE_SYMLINK_HEAD on
> any platform).

This my idea too. All the time I was doing that patch :)

> HOWEVER, I think "falling back" (both in Johannes' patch which
> is in the "master" branch, and your version) has a funny failure
> mode.  What happens when two processes try redirecting .git/HEAD
> simultaneously, possibly to different branch heads?  Both of
> them unlink(), one successfully does symlink(), and the other
> gets EEXIST and falls back to create regular file symref.

I think the file ref version uses rename of HEAD.lock into HEAD, doesn't it?
Rename(2) should just remove the symlink, right?

Re: [PATCH 1/4] git-init-db should error out with a message

[Junio C Hamano]

Alex Riesen <fork0@users.sourceforge.net> writes:

> I think the file ref version uses rename of HEAD.lock into HEAD, doesn't it?
> Rename(2) should just remove the symlink, right?

If everybody used symlink or if everybody used regular file
symref, we would catch this race and the second one will be
stopped.  My point was that by falling back we are introducing
this unnecessary race, which might be unimportant but still it
is a new race.

To avoid that, I think symlink version needs to honor the
HEAD.lock convention, which would slow down normal cases.