Re-fix "builtin-commit: fix --signoff"

An earlier fix to the said commit was incomplete; it mixed up the
meaning of the flag parameter passed to the internal fmt_ident()
function, so this corrects it.

git_author_info() and git_committer_info() can be told to issue a
warning when no usable user information is found, and optionally can be
told to error out.  Operations that actually use the information to
record a new commit or a tag will still error out, but the caller to
leave reflog record will just silently use bogus user information.

Not warning on misconfigured user information while writing a reflog
entry is somewhat debatable, but it is probably nicer to the users to
silently let it pass, because the only information you are losing is who
checked out the branch.

* git_author_info() and git_committer_info() used to take 1 (positive
int) to error out with a warning on misconfiguration; this is now
signalled with a symbolic constant IDENT_ERROR_ON_NO_NAME.

* These functions used to take -1 (negative int) to warn but continue;
this is now signalled with a symbolic constant IDENT_WARN_ON_NO_NAME.

* fmt_ident() function implements the above error reporting behaviour
common to git_author_info() and git_committer_info().  A symbolic
constant IDENT_NO_DATE can be or'ed in to the flag parameter to make
it return only the "Name <email@address.xz>".

* fmt_name() is a thin wrapper around fmt_ident() that always passes
IDENT_ERROR_ON_NO_NAME and IDENT_NO_DATE.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c
index 88b0ab36eba6ded8f1a39e0d4c83122b7e026874..6610d18358bae81ac1f162ca0c71062d36664c2a 100644 (file)
--- a/builtin-commit-tree.c
+++ b/builtin-commit-tree.c
@@ -98,8 +98,8 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
                strbuf_addf(&buffer, "parent %s\n", sha1_to_hex(parent_sha1[i]));
 
        /* Person/date information */
                strbuf_addf(&buffer, "parent %s\n", sha1_to_hex(parent_sha1[i]));
 
        /* Person/date information */

-       strbuf_addf(&buffer, "author %s\n", git_author_info(1));
-       strbuf_addf(&buffer, "committer %s\n", git_committer_info(1));
+       strbuf_addf(&buffer, "author %s\n", git_author_info(IDENT_ERROR_ON_NO_NAME));
+       strbuf_addf(&buffer, "committer %s\n", git_committer_info(IDENT_ERROR_ON_NO_NAME));

        if (!encoding_is_utf8)
                strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding);
        strbuf_addch(&buffer, '\n');
        if (!encoding_is_utf8)
                strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding);
        strbuf_addch(&buffer, '\n');

diff --git a/builtin-commit.c b/builtin-commit.c
index 30a9debcda9fc478571590fe45dc8647cef434fe..b6b81d531d8b560315298353e5b0ae377581c566 100644 (file)
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -497,7 +497,7 @@ static void determine_author_info(struct strbuf *sb)
                email = xstrndup(lb + 2, rb - (lb + 2));
        }
 
                email = xstrndup(lb + 2, rb - (lb + 2));
        }
 

-       strbuf_addf(sb, "author %s\n", fmt_ident(name, email, date, 1));
+       strbuf_addf(sb, "author %s\n", fmt_ident(name, email, date, IDENT_ERROR_ON_NO_NAME));

 }
 
 static int parse_and_validate_options(int argc, const char *argv[],
 }
 
 static int parse_and_validate_options(int argc, const char *argv[],


@@ -776,7 +776,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
        }
 
        determine_author_info(&sb);
        }
 
        determine_author_info(&sb);

-       strbuf_addf(&sb, "committer %s\n", git_committer_info(1));
+       strbuf_addf(&sb, "committer %s\n", git_committer_info(IDENT_ERROR_ON_NO_NAME));

        if (!is_encoding_utf8(git_commit_encoding))
                strbuf_addf(&sb, "encoding %s\n", git_commit_encoding);
        strbuf_addch(&sb, '\n');
        if (!is_encoding_utf8(git_commit_encoding))
                strbuf_addf(&sb, "encoding %s\n", git_commit_encoding);
        strbuf_addch(&sb, '\n');

diff --git a/builtin-log.c b/builtin-log.c
index e1f1cf67143721933010065130adaba8723b272b..b6a11220e258468bddd92181bf18ce55343abf2e 100644 (file)
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -557,7 +557,7 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids, const cha
 
 static void gen_message_id(char *dest, unsigned int length, char *base)
 {
 
 static void gen_message_id(char *dest, unsigned int length, char *base)
 {

-       const char *committer = git_committer_info(-1);
+       const char *committer = git_committer_info(IDENT_WARN_ON_NO_NAME);

        const char *email_start = strrchr(committer, '<');
        const char *email_end = strrchr(committer, '>');
        if(!email_start || !email_end || email_start > email_end - 1)
        const char *email_start = strrchr(committer, '<');
        const char *email_end = strrchr(committer, '>');
        if(!email_start || !email_end || email_start > email_end - 1)


@@ -665,7 +665,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
                         !strcmp(argv[i], "-s")) {
                        const char *committer;
                        const char *endpos;
                         !strcmp(argv[i], "-s")) {
                        const char *committer;
                        const char *endpos;

-                       committer = git_committer_info(1);
+                       committer = git_committer_info(IDENT_ERROR_ON_NO_NAME);

                        endpos = strchr(committer, '>');
                        if (!endpos)
                                die("bogos committer info %s\n", committer);
                        endpos = strchr(committer, '>');
                        if (!endpos)
                                die("bogos committer info %s\n", committer);

diff --git a/builtin-tag.c b/builtin-tag.c
index 9f966fc8837ee84c6d83825f0036e40997fec5f2..517419fd3d885d677e8c2eeb533fc14f3df501f5 100644 (file)
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -188,7 +188,7 @@ static int do_sign(struct strbuf *buffer)
        int len;
 
        if (!*signingkey) {
        int len;
 
        if (!*signingkey) {

-               if (strlcpy(signingkey, git_committer_info(1),
+               if (strlcpy(signingkey, git_committer_info(IDENT_ERROR_ON_NO_NAME),

                                sizeof(signingkey)) > sizeof(signingkey) - 1)
                        return error("committer info too long.");
                bracket = strchr(signingkey, '>');
                                sizeof(signingkey)) > sizeof(signingkey) - 1)
                        return error("committer info too long.");
                bracket = strchr(signingkey, '>');


@@ -298,7 +298,7 @@ static void create_tag(const unsigned char *object, const char *tag,
                          sha1_to_hex(object),
                          typename(type),
                          tag,
                          sha1_to_hex(object),
                          typename(type),
                          tag,

-                         git_committer_info(1));
+                         git_committer_info(IDENT_ERROR_ON_NO_NAME));

 
        if (header_len > sizeof(header_buf) - 1)
                die("tag header too big.");
 
        if (header_len > sizeof(header_buf) - 1)
                die("tag header too big.");

diff --git a/cache.h b/cache.h
index 406befb705a0a99119476bbed3be12dd6059d932..c6142e9b7a8effbe31b5dcca3035d32138de84cf 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -453,6 +453,9 @@ void datestamp(char *buf, int bufsize);
 unsigned long approxidate(const char *);
 enum date_mode parse_date_format(const char *format);
 
 unsigned long approxidate(const char *);
 enum date_mode parse_date_format(const char *format);
 

+#define IDENT_WARN_ON_NO_NAME  1
+#define IDENT_ERROR_ON_NO_NAME 2
+#define IDENT_NO_DATE         4

 extern const char *git_author_info(int);
 extern const char *git_committer_info(int);
 extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);
 extern const char *git_author_info(int);
 extern const char *git_committer_info(int);
 extern const char *fmt_ident(const char *name, const char *email, const char *date_str, int);

diff --git a/ident.c b/ident.c
index 7631698f27a8e1685614c233037eb067eef65168..892d77ac9366978fcb172e0d258410f271631012 100644 (file)
--- a/ident.c
+++ b/ident.c
@@ -182,14 +182,15 @@ static const char *env_hint =
 "Omit --global to set the identity only in this repository.\n"
 "\n";
 
 "Omit --global to set the identity only in this repository.\n"
 "\n";
 

-static const char *fmt_ident_1(const char *name, const char *email,
-                              const char *date_str, int flag)
+const char *fmt_ident(const char *name, const char *email,
+                     const char *date_str, int flag)

 {
        static char buffer[1000];
        char date[50];
        int i;
 {
        static char buffer[1000];
        char date[50];
        int i;

-       int error_on_no_name = !!(flag & 01);
-       int name_addr_only = !!(flag & 02);
+       int error_on_no_name = (flag & IDENT_ERROR_ON_NO_NAME);
+       int warn_on_no_name = (flag & IDENT_WARN_ON_NO_NAME);
+       int name_addr_only = (flag & IDENT_NO_DATE);

 
        setup_ident();
        if (!name)
 
        setup_ident();
        if (!name)


@@ -200,12 +201,12 @@ static const char *fmt_ident_1(const char *name, const char *email,
        if (!*name) {
                struct passwd *pw;
 
        if (!*name) {
                struct passwd *pw;
 

-               if (0 <= error_on_no_name &&
+               if ((warn_on_no_name || error_on_no_name) &&

                    name == git_default_name && env_hint) {
                        fprintf(stderr, env_hint, au_env, co_env);
                        env_hint = NULL; /* warn only once, for "git-var -l" */
                }
                    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 (0 < error_on_no_name)
+               if (error_on_no_name)

                        die("empty ident %s <%s> not allowed", name, email);
                pw = getpwuid(getuid());
                if (!pw)
                        die("empty ident %s <%s> not allowed", name, email);
                pw = getpwuid(getuid());
                if (!pw)


@@ -234,30 +235,23 @@ static const char *fmt_ident_1(const char *name, const char *email,
        return buffer;
 }
 
        return buffer;
 }
 

-const char *fmt_ident(const char *name, const char *email,
-                     const char *date_str, int error_on_no_name)
-{
-       int flag = (error_on_no_name ? 01 : 0);
-       return fmt_ident_1(name, email, date_str, flag);
-}
-

 const char *fmt_name(const char *name, const char *email)
 {
 const char *fmt_name(const char *name, const char *email)
 {

-       return fmt_ident_1(name, email, NULL, 03);
+       return fmt_ident(name, email, NULL, IDENT_ERROR_ON_NO_NAME | IDENT_NO_DATE);

 }
 
 }
 

-const char *git_author_info(int error_on_no_name)
+const char *git_author_info(int flag)

 {
        return fmt_ident(getenv("GIT_AUTHOR_NAME"),
                         getenv("GIT_AUTHOR_EMAIL"),
                         getenv("GIT_AUTHOR_DATE"),
 {
        return fmt_ident(getenv("GIT_AUTHOR_NAME"),
                         getenv("GIT_AUTHOR_EMAIL"),
                         getenv("GIT_AUTHOR_DATE"),

-                        error_on_no_name);
+                        flag);

 }
 
 }
 

-const char *git_committer_info(int error_on_no_name)
+const char *git_committer_info(int flag)

 {
        return fmt_ident(getenv("GIT_COMMITTER_NAME"),
                         getenv("GIT_COMMITTER_EMAIL"),
                         getenv("GIT_COMMITTER_DATE"),
 {
        return fmt_ident(getenv("GIT_COMMITTER_NAME"),
                         getenv("GIT_COMMITTER_EMAIL"),
                         getenv("GIT_COMMITTER_DATE"),

-                        error_on_no_name);
+                        flag);

 }
 }

diff --git a/refs.c b/refs.c
index 3e6e98c5eb20fc7a365d25b3abc275644b4c26f9..759924d0c93621d72918f29145ae0377f34e8e0c 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -1094,7 +1094,7 @@ static int log_ref_write(const char *ref_name, const unsigned char *old_sha1,
        adjust_shared_perm(log_file);
 
        msglen = msg ? strlen(msg) : 0;
        adjust_shared_perm(log_file);
 
        msglen = msg ? strlen(msg) : 0;

-       committer = git_committer_info(-1);
+       committer = git_committer_info(0);

        maxlen = strlen(committer) + msglen + 100;
        logrec = xmalloc(maxlen);
        len = sprintf(logrec, "%s %s %s\n",
        maxlen = strlen(committer) + msglen + 100;
        logrec = xmalloc(maxlen);
        len = sprintf(logrec, "%s %s %s\n",

diff --git a/var.c b/var.c
index 4127031910c83987c8ef394e3711942e2db8a8f9..0de0efa2aa3b216a0bc846135a54ed0dd0549f8b 100644 (file)
--- a/var.c
+++ b/var.c
@@ -21,7 +21,7 @@ static void list_vars(void)
 {
        struct git_var *ptr;
        for(ptr = git_vars; ptr->read; ptr++) {
 {
        struct git_var *ptr;
        for(ptr = git_vars; ptr->read; ptr++) {

-               printf("%s=%s\n", ptr->name, ptr->read(0));
+               printf("%s=%s\n", ptr->name, ptr->read(IDENT_WARN_ON_NO_NAME));

        }
 }
 
        }
 }
 


@@ -32,7 +32,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 = NULL;
        for(ptr = git_vars; ptr->read; ptr++) {
                if (strcmp(var, ptr->name) == 0) {

-                       val = ptr->read(1);
+                       val = ptr->read(IDENT_ERROR_ON_NO_NAME);

                        break;
                }
        }
                        break;
                }
        }