threads / patch / 2229

patch, 4 partsgit-init-db should error out with a message

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

## tl;dr

5 messages between Oct 25, 2005 and Oct 26, 2005. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Johannes Schindelin· Oct 25, 2005, 23:39 UTC · lore

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

Show changes to init-db.c +1 −1
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
Alex Riesen· Oct 26, 2005, 19:45 UTC · re: Johannes Schindelin · lore

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

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

Show changes to 3 files +16 −3

cache.h, init-db.c, refs.c

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
Junio C Hamano· Oct 26, 2005, 20:27 UTC · re: Alex Riesen · lore

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

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.

Alex Riesen· Oct 26, 2005, 20:47 UTC · re: Junio C Hamano · lore

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

Junio C Hamano, Wed, Oct 26, 2005 22:27:00 +0200:
Show 7 quoted lines
> > 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 :)
Show 6 quoted lines
> 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?

Junio C Hamano· Oct 26, 2005, 23:18 UTC · re: Alex Riesen · lore

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

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.

← back to recent threads