[RFC] empty ident?

[RFC] empty ident?

[Junio C Hamano]

It appears that some people who did not care about having bogus
names in their own commit messages are bitten by the recent
change to require a sane environment [*1*].

While it was a good idea to prevent people from using bogus
names to create commits and doing sign-offs, the error message
is not very informative.  This patch attempts to warn things
upfront and hint people how to fix their environments.

Likes, dislikes, don't cares?

[Footnote]

*1* The thread is this one.

    http://marc.theaimsgroup.com/?t=113868084800004

    Especially this message.

    http://marc.theaimsgroup.com/?m=113932830015032

---
diff --git a/ident.c b/ident.c
index 23b8cfc..09d4d71 100644
--- a/ident.c
+++ b/ident.c
@@ -46,6 +46,15 @@ static void copy_gecos(struct passwd *w,
 
 }
 
+static const char au_env[] = "GIT_AUTHOR_NAME";
+static const char co_env[] = "GIT_COMMITTER_NAME";
+static const char env_hint[] =
+"\n*** Environment problem:\n"
+"*** Your name cannot be determined from your system services (gecos).\n"
+"*** You would need to set %s and %s\n"
+"*** environment variables; otherwise you won't be able to perform\n"
+"*** certain operations because of \"empty ident\" errors.\n\n";
+
 int setup_ident(void)
 {
 	int len;
@@ -57,6 +66,11 @@ int setup_ident(void)
 	/* Get the name ("gecos") */
 	copy_gecos(pw, git_default_name, sizeof(git_default_name));
 
+	if (!*git_default_name) {
+		if (!getenv(au_env) || !getenv(co_env))
+			fprintf(stderr, env_hint, au_env, co_env);
+	}
+
 	/* Make up a fake email address (name + '@' + hostname [+ '.' + domainname]) */
 	len = strlen(pw->pw_name);
 	if (len > sizeof(git_default_email)/2)

Re: [RFC] empty ident?

[Nicolas Vilz 'niv']

[-- Attachment #1: Type: text/plain, Size: 790 bytes --]

Junio C Hamano wrote:
> It appears that some people who did not care about having bogus
> names in their own commit messages are bitten by the recent
> change to require a sane environment [*1*].
> 
> While it was a good idea to prevent people from using bogus
> names to create commits and doing sign-offs, the error message
> is not very informative.  This patch attempts to warn things
> upfront and hint people how to fix their environments.

Mh... it made me not using cg-init, but git-init, and after that editing
.git/config, before the first commit.

This doesn't apply to works with git-svn, because here imported commits
are tailored from the base-uuid of the SVN repository and the commit-user.

niv <niv@$uuid>

after that i can setup my .git/config, as well.

Sincerly
Nicolas

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

[PATCH] Nice error message for empty idents

[Petr Baudis]

Dear diary, on Sat, Feb 18, 2006 at 09:44:14AM CET, I got a letter
where Junio C Hamano <junkio@cox.net> said that...
> It appears that some people who did not care about having bogus
> names in their own commit messages are bitten by the recent
> change to require a sane environment [*1*].
> 
> While it was a good idea to prevent people from using bogus
> names to create commits and doing sign-offs, the error message
> is not very informative.  This patch attempts to warn things
> upfront and hint people how to fix their environments.
> 
> Likes, dislikes, don't cares?

One dislike from me - you shouldn't care if you ain't gonna do it - it
makes no sense to require valid gecos if you are never committing
anything. Note that this "premature requirement" plague has already hit
GIT - come on:

	$ git-rev-list --help
	fatal: Not a git repository

That's worse than CVS (which allows cvs --help cmd but not cvs cmd
--help outside of a repository).

I agree that the error message itself is bad.

---

[PATCH] Nice error message for empty idents

Signed-off-by: Petr Baudis <pasky@suse.cz>

---
commit 19b5bda8f0301ed4b856c5cd9137637b89d95d30
tree 8f91bfd101c32f258c3d8cca467eb7be26d62b91
parent 8dcc626cd144b2c6eae2a299242bbbe905cb0059
author Petr Baudis <pasky@suse.cz> Sat, 18 Feb 2006 18:26:32 +0100
committer Petr Baudis <xpasky@machine.or.cz> Sat, 18 Feb 2006 18:26:32 +0100

 ident.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/ident.c b/ident.c
index 23b8cfc..c57ba5c 100644
--- a/ident.c
+++ b/ident.c
@@ -168,9 +168,10 @@ static const char *get_ident(const char 
 	if (!email)
 		email = git_default_email;
 
-	if (!*name || !*email)
-		die("empty ident %s <%s> not allowed",
-		    name, email);
+	if (!*name)
+		die("empty realname not allowed - please set a realname for your login or use GIT_AUTHOR_NAME");
+	if (!*email)
+		die("empty email not allowed - please set your GIT_AUTHOR_EMAIL to a non-empty value");
 
 	strcpy(date, git_default_date);
 	if (date_str)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Of the 3 great composers Mozart tells us what it's like to be human,
Beethoven tells us what it's like to be Beethoven and Bach tells us
what it's like to be the universe.  -- Douglas Adams

Re: [PATCH] Nice error message for empty idents

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> 	$ git-rev-list --help
> 	fatal: Not a git repository

Hmph, true.  Ideas?

> I agree that the error message itself is bad.

> -	if (!*name || !*email)
> -		die("empty ident %s <%s> not allowed",
> -		    name, email);
> +	if (!*name)
> +		die("empty realname not allowed - please set a realname for your login or use GIT_AUTHOR_NAME");
> +	if (!*email)
> +		die("empty email not allowed - please set your GIT_AUTHOR_EMAIL to a non-empty value");

I've considered doing this, but didn't do it that way, because I
could not figure out what to do with GIT_COMMITTER_NAME.  Also
it might make sense to suggest user.name configuration item in
the error/help message.  Ideas?

[PATCH] Delay "empty ident" errors until they really matter.

[Junio C Hamano]

Previous one warned people upfront to encourage fixing their
environment early, but some people just use repositories and git
tools read-only without making any changes, and in such a case
there is not much point insisting on them having a usable ident.

This round attempts to move the error until either "git-var"
asks for the ident explicitly or "commit-tree" wants to use it.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---
  Petr Baudis <pasky@suse.cz> writes:

  > One dislike from me - you shouldn't care if you ain't gonna do it - it
  > makes no sense to require valid gecos if you are never committing
  > anything.

  You are absolutely right.  This is to fix the previous one.

 cache.h       |    4 ++--
 commit-tree.c |    4 ++--
 ident.c       |   47 +++++++++++++++++++++++++----------------------
 var.c         |    6 +++---
 4 files changed, 32 insertions(+), 29 deletions(-)

749be728d469e9a0acfdc020feff17c2da510083
diff --git a/cache.h b/cache.h
index b5db01f..da73fb3 100644
--- a/cache.h
+++ b/cache.h
@@ -246,8 +246,8 @@ void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
 
 extern int setup_ident(void);
-extern const char *git_author_info(void);
-extern const char *git_committer_info(void);
+extern const char *git_author_info(int);
+extern const char *git_committer_info(int);
 
 struct checkout {
 	const char *base_dir;
diff --git a/commit-tree.c b/commit-tree.c
index b1c8dca..88871b0 100644
--- a/commit-tree.c
+++ b/commit-tree.c
@@ -118,8 +118,8 @@ int main(int argc, char **argv)
 		add_buffer(&buffer, &size, "parent %s\n", sha1_to_hex(parent_sha1[i]));
 
 	/* Person/date information */
-	add_buffer(&buffer, &size, "author %s\n", git_author_info());
-	add_buffer(&buffer, &size, "committer %s\n\n", git_committer_info());
+	add_buffer(&buffer, &size, "author %s\n", git_author_info(1));
+	add_buffer(&buffer, &size, "committer %s\n\n", git_committer_info(1));
 
 	/* And add the comment */
 	while (fgets(comment, sizeof(comment), stdin) != NULL)
diff --git a/ident.c b/ident.c
index 09d4d71..7c81fe8 100644
--- a/ident.c
+++ b/ident.c
@@ -46,15 +46,6 @@ static void copy_gecos(struct passwd *w,
 
 }
 
-static const char au_env[] = "GIT_AUTHOR_NAME";
-static const char co_env[] = "GIT_COMMITTER_NAME";
-static const char env_hint[] =
-"\n*** Environment problem:\n"
-"*** Your name cannot be determined from your system services (gecos).\n"
-"*** You would need to set %s and %s\n"
-"*** environment variables; otherwise you won't be able to perform\n"
-"*** certain operations because of \"empty ident\" errors.\n\n";
-
 int setup_ident(void)
 {
 	int len;
@@ -66,11 +57,6 @@ int setup_ident(void)
 	/* Get the name ("gecos") */
 	copy_gecos(pw, git_default_name, sizeof(git_default_name));
 
-	if (!*git_default_name) {
-		if (!getenv(au_env) || !getenv(co_env))
-			fprintf(stderr, env_hint, au_env, co_env);
-	}
-
 	/* Make up a fake email address (name + '@' + hostname [+ '.' + domainname]) */
 	len = strlen(pw->pw_name);
 	if (len > sizeof(git_default_email)/2)
@@ -170,8 +156,18 @@ static int copy(char *buf, int size, int
 	return offset;
 }
 
+static const char au_env[] = "GIT_AUTHOR_NAME";
+static const char co_env[] = "GIT_COMMITTER_NAME";
+static const char *env_hint =
+"\n*** Environment problem:\n"
+"*** Your name cannot be determined from your system services (gecos).\n"
+"*** You would need to set %s and %s\n"
+"*** environment variables; otherwise you won't be able to perform\n"
+"*** certain operations because of \"empty ident\" errors.\n"
+"*** Alternatively, you can use user.name configuration variable.\n\n";
+
 static const char *get_ident(const char *name, const char *email,
-			     const char *date_str)
+			     const char *date_str, int error_on_no_name)
 {
 	static char buffer[1000];
 	char date[50];
@@ -182,9 +178,14 @@ static const char *get_ident(const char 
 	if (!email)
 		email = git_default_email;
 
-	if (!*name || !*email)
-		die("empty ident %s <%s> not allowed",
-		    name, email);
+	if (!*name) {
+		if (name == git_default_name && env_hint) {
+			fprintf(stderr, env_hint, au_env, co_env);
+			env_hint = NULL; /* warn only once, for "git-var -l" */
+		}
+		if (error_on_no_name)
+			die("empty ident %s <%s> not allowed", name, email);
+	}
 
 	strcpy(date, git_default_date);
 	if (date_str)
@@ -201,16 +202,18 @@ static const char *get_ident(const char 
 	return buffer;
 }
 
-const char *git_author_info(void)
+const char *git_author_info(int error_on_no_name)
 {
 	return get_ident(getenv("GIT_AUTHOR_NAME"),
 			 getenv("GIT_AUTHOR_EMAIL"),
-			 getenv("GIT_AUTHOR_DATE"));
+			 getenv("GIT_AUTHOR_DATE"),
+			 error_on_no_name);
 }
 
-const char *git_committer_info(void)
+const char *git_committer_info(int error_on_no_name)
 {
 	return get_ident(getenv("GIT_COMMITTER_NAME"),
 			 getenv("GIT_COMMITTER_EMAIL"),
-			 getenv("GIT_COMMITTER_DATE"));
+			 getenv("GIT_COMMITTER_DATE"),
+			 error_on_no_name);
 }
diff --git a/var.c b/var.c
index 59da56d..a57a33b 100644
--- a/var.c
+++ b/var.c
@@ -12,7 +12,7 @@ static const char var_usage[] = "git-var
 
 struct git_var {
 	const char *name;
-	const char *(*read)(void);
+	const char *(*read)(int);
 };
 static struct git_var git_vars[] = {
 	{ "GIT_COMMITTER_IDENT", git_committer_info },
@@ -24,7 +24,7 @@ static void list_vars(void)
 {
 	struct git_var *ptr;
 	for(ptr = git_vars; ptr->read; ptr++) {
-		printf("%s=%s\n", ptr->name, ptr->read());
+		printf("%s=%s\n", ptr->name, ptr->read(0));
 	}
 }
 
@@ -35,7 +35,7 @@ static const char *read_var(const char *
 	val = NULL;
 	for(ptr = git_vars; ptr->read; ptr++) {
 		if (strcmp(var, ptr->name) == 0) {
-			val = ptr->read();
+			val = ptr->read(1);
 			break;
 		}
 	}
-- 
1.2.1.g2902