From 749be728d469e9a0acfdc020feff17c2da510083 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 18 Feb 2006 20:31:05 -0800 Subject: [PATCH 1/2] Delay "empty ident" errors until they really matter. 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 --- cache.h | 4 ++-- commit-tree.c | 4 ++-- ident.c | 47 +++++++++++++++++++++++++---------------------- var.c | 6 +++--- 4 files changed, 32 insertions(+), 29 deletions(-) diff --git a/cache.h b/cache.h index b5db01f28..da73fb37c 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 b1c8dca48..88871b022 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 09d4d716c..7c81fe8d8 100644 --- a/ident.c +++ b/ident.c @@ -46,15 +46,6 @@ static void copy_gecos(struct passwd *w, char *name, int sz) } -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 offset, const char *src) 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 *name, const char *email, 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 *name, const char *email, 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 59da56da0..a57a33b81 100644 --- a/var.c +++ b/var.c @@ -12,7 +12,7 @@ static const char var_usage[] = "git-var [-l | ]"; 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 *var) val = NULL; for(ptr = git_vars; ptr->read; ptr++) { if (strcmp(var, ptr->name) == 0) { - val = ptr->read(); + val = ptr->read(1); break; } } From e3b59a44f6705896db80965427a7cf9e2112634b Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 18 Feb 2006 20:51:26 -0800 Subject: [PATCH 2/2] Keep Porcelainish from failing by broken ident after making changes. "empty ident not allowed" error makes commit-tree fail, so we are already safer in that we would not end up with commit objects that have bogus names on the author or committer fields. However, before commit-tree is called there are already changes made to the index file and the working tree. The operation can be resumed after fixing the environment problem, but when this triggers to a newcomer with unusable gecos, the first question becomes "what did I lose and how would I recover". This patch modifies some Porcelainish commands to verify GIT_COMMITTER_IDENT as soon as we know we are going to make some commits before doing much damage to prevent confusion. Signed-off-by: Junio C Hamano --- git-am.sh | 4 +++- git-applymbox.sh | 2 ++ git-merge.sh | 5 +++++ git-resolve.sh | 3 +++ 4 files changed, 13 insertions(+), 1 deletion(-) diff --git a/git-am.sh b/git-am.sh index 98b9215f7..85ecada65 100755 --- a/git-am.sh +++ b/git-am.sh @@ -1,11 +1,13 @@ #!/bin/sh # -# +# Copyright (c) 2005, 2006 Junio C Hamano USAGE='[--signoff] [--dotest=] [--utf8] [--binary] [--3way] or, when resuming [--skip | --resolved]' . git-sh-setup +git var GIT_COMMITTER_IDENT >/dev/null || exit + stop_here () { echo "$1" >"$dotest/next" exit 1 diff --git a/git-applymbox.sh b/git-applymbox.sh index 61c8c024c..5569fdcc3 100755 --- a/git-applymbox.sh +++ b/git-applymbox.sh @@ -21,6 +21,8 @@ USAGE='[-u] [-k] [-q] [-m] (-c .dotest/ | mbox) [signoff]' . git-sh-setup +git var GIT_COMMITTER_IDENT >/dev/null || exit + keep_subject= query_apply= continue= utf8= resume=t while case "$#" in 0) break ;; esac do diff --git a/git-merge.sh b/git-merge.sh index 74f07610f..2b4a603df 100755 --- a/git-merge.sh +++ b/git-merge.sh @@ -142,6 +142,8 @@ case "$#,$common,$no_commit" in 1,*,) # We are not doing octopus, not fast forward, and have only # one common. See if it is really trivial. + git var GIT_COMMITTER_IDENT >/dev/null || exit + echo "Trying really trivial in-index merge..." git-update-index --refresh 2>/dev/null if git-read-tree --trivial -m -u $common $head "$1" && @@ -179,6 +181,9 @@ case "$#,$common,$no_commit" in ;; esac +# We are going to make a new commit. +git var GIT_COMMITTER_IDENT >/dev/null || exit + case "$use_strategies" in '') case "$#" in diff --git a/git-resolve.sh b/git-resolve.sh index 926307005..b53ede8d8 100755 --- a/git-resolve.sh +++ b/git-resolve.sh @@ -50,6 +50,9 @@ case "$common" in ;; esac +# We are going to make a new commit. +git var GIT_COMMITTER_IDENT >/dev/null || exit + # Find an optimum merge base if there are more than one candidates. LF=' '