Merge branch 'pw/am-rebase-read-author-script'
authorJunio C Hamano <gitster@pobox.com>
Tue, 13 Nov 2018 13:37:23 +0000 (22:37 +0900)
committerJunio C Hamano <gitster@pobox.com>
Tue, 13 Nov 2018 13:37:23 +0000 (22:37 +0900)
Unify code to read the author-script used in "git am" and the
commands that use the sequencer machinery, e.g. "git rebase -i".

* pw/am-rebase-read-author-script:
sequencer: use read_author_script()
add read_author_script() to libgit
am: rename read_author_script()
am: improve author-script error reporting
am: don't die in read_author_script()

builtin/am.c
sequencer.c
sequencer.h
index 3ee9a9d2a92aaa0e9716719640deaec7e260c811..dc576a33728f5c2264acf66fcbe3243284144781 100644 (file)
@@ -260,32 +260,6 @@ static int read_state_file(struct strbuf *sb, const struct am_state *state,
        die_errno(_("could not read '%s'"), am_path(state, file));
 }
 
-/**
- * Take a series of KEY='VALUE' lines where VALUE part is
- * sq-quoted, and append <KEY, VALUE> at the end of the string list
- */
-static int parse_key_value_squoted(char *buf, struct string_list *list)
-{
-       while (*buf) {
-               struct string_list_item *item;
-               char *np;
-               char *cp = strchr(buf, '=');
-               if (!cp)
-                       return -1;
-               np = strchrnul(cp, '\n');
-               *cp++ = '\0';
-               item = string_list_append(list, buf);
-
-               buf = np + (*np == '\n');
-               *np = '\0';
-               cp = sq_dequote(cp);
-               if (!cp)
-                       return -1;
-               item->util = xstrdup(cp);
-       }
-       return 0;
-}
-
 /**
  * Reads and parses the state directory's "author-script" file, and sets
  * state->author_name, state->author_email and state->author_date accordingly.
@@ -302,42 +276,16 @@ static int parse_key_value_squoted(char *buf, struct string_list *list)
  * script, and thus if the file differs from what this function expects, it is
  * better to bail out than to do something that the user does not expect.
  */
-static int read_author_script(struct am_state *state)
+static int read_am_author_script(struct am_state *state)
 {
        const char *filename = am_path(state, "author-script");
-       struct strbuf buf = STRBUF_INIT;
-       struct string_list kv = STRING_LIST_INIT_DUP;
-       int retval = -1; /* assume failure */
-       int fd;
 
        assert(!state->author_name);
        assert(!state->author_email);
        assert(!state->author_date);
 
-       fd = open(filename, O_RDONLY);
-       if (fd < 0) {
-               if (errno == ENOENT)
-                       return 0;
-               die_errno(_("could not open '%s' for reading"), filename);
-       }
-       strbuf_read(&buf, fd, 0);
-       close(fd);
-       if (parse_key_value_squoted(buf.buf, &kv))
-               goto finish;
-
-       if (kv.nr != 3 ||
-           strcmp(kv.items[0].string, "GIT_AUTHOR_NAME") ||
-           strcmp(kv.items[1].string, "GIT_AUTHOR_EMAIL") ||
-           strcmp(kv.items[2].string, "GIT_AUTHOR_DATE"))
-               goto finish;
-       state->author_name = kv.items[0].util;
-       state->author_email = kv.items[1].util;
-       state->author_date = kv.items[2].util;
-       retval = 0;
-finish:
-       string_list_clear(&kv, !!retval);
-       strbuf_release(&buf);
-       return retval;
+       return read_author_script(filename, &state->author_name,
+                                 &state->author_email, &state->author_date, 1);
 }
 
 /**
@@ -411,7 +359,7 @@ static void am_load(struct am_state *state)
                BUG("state file 'last' does not exist");
        state->last = strtol(sb.buf, NULL, 10);
 
-       if (read_author_script(state) < 0)
+       if (read_am_author_script(state) < 0)
                die(_("could not parse author script"));
 
        read_commit_msg(state);
index 405d5ef86bfd1bfa5e01c63d7f1afe88e7132feb..1869435a721ae6eee502b393357b41a58f4e7582 100644 (file)
@@ -669,55 +669,131 @@ static int write_author_script(const char *message)
        return res;
 }
 
+/**
+ * Take a series of KEY='VALUE' lines where VALUE part is
+ * sq-quoted, and append <KEY, VALUE> at the end of the string list
+ */
+static int parse_key_value_squoted(char *buf, struct string_list *list)
+{
+       while (*buf) {
+               struct string_list_item *item;
+               char *np;
+               char *cp = strchr(buf, '=');
+               if (!cp) {
+                       np = strchrnul(buf, '\n');
+                       return error(_("no key present in '%.*s'"),
+                                    (int) (np - buf), buf);
+               }
+               np = strchrnul(cp, '\n');
+               *cp++ = '\0';
+               item = string_list_append(list, buf);
+
+               buf = np + (*np == '\n');
+               *np = '\0';
+               cp = sq_dequote(cp);
+               if (!cp)
+                       return error(_("unable to dequote value of '%s'"),
+                                    item->string);
+               item->util = xstrdup(cp);
+       }
+       return 0;
+}
 
-/*
- * write_author_script() used to fail to terminate the last line with a "'" and
- * also escaped "'" incorrectly as "'\\\\''" rather than "'\\''". We check for
- * the terminating "'" on the last line to see how "'" has been escaped in case
- * git was upgraded while rebase was stopped.
+/**
+ * Reads and parses the state directory's "author-script" file, and sets name,
+ * email and date accordingly.
+ * Returns 0 on success, -1 if the file could not be parsed.
+ *
+ * The author script is of the format:
+ *
+ *     GIT_AUTHOR_NAME='$author_name'
+ *     GIT_AUTHOR_EMAIL='$author_email'
+ *     GIT_AUTHOR_DATE='$author_date'
+ *
+ * where $author_name, $author_email and $author_date are quoted. We are strict
+ * with our parsing, as the file was meant to be eval'd in the old
+ * git-am.sh/git-rebase--interactive.sh scripts, and thus if the file differs
+ * from what this function expects, it is better to bail out than to do
+ * something that the user does not expect.
  */
-static int quoting_is_broken(const char *s, size_t n)
+int read_author_script(const char *path, char **name, char **email, char **date,
+                      int allow_missing)
 {
-       /* Skip any empty lines in case the file was hand edited */
-       while (n > 0 && s[--n] == '\n')
-               ; /* empty */
-       if (n > 0 && s[n] != '\'')
-               return 1;
+       struct strbuf buf = STRBUF_INIT;
+       struct string_list kv = STRING_LIST_INIT_DUP;
+       int retval = -1; /* assume failure */
+       int i, name_i = -2, email_i = -2, date_i = -2, err = 0;
 
-       return 0;
+       if (strbuf_read_file(&buf, path, 256) <= 0) {
+               strbuf_release(&buf);
+               if (errno == ENOENT && allow_missing)
+                       return 0;
+               else
+                       return error_errno(_("could not open '%s' for reading"),
+                                          path);
+       }
+
+       if (parse_key_value_squoted(buf.buf, &kv))
+               goto finish;
+
+       for (i = 0; i < kv.nr; i++) {
+               if (!strcmp(kv.items[i].string, "GIT_AUTHOR_NAME")) {
+                       if (name_i != -2)
+                               name_i = error(_("'GIT_AUTHOR_NAME' already given"));
+                       else
+                               name_i = i;
+               } else if (!strcmp(kv.items[i].string, "GIT_AUTHOR_EMAIL")) {
+                       if (email_i != -2)
+                               email_i = error(_("'GIT_AUTHOR_EMAIL' already given"));
+                       else
+                               email_i = i;
+               } else if (!strcmp(kv.items[i].string, "GIT_AUTHOR_DATE")) {
+                       if (date_i != -2)
+                               date_i = error(_("'GIT_AUTHOR_DATE' already given"));
+                       else
+                               date_i = i;
+               } else {
+                       err = error(_("unknown variable '%s'"),
+                                   kv.items[i].string);
+               }
+       }
+       if (name_i == -2)
+               error(_("missing 'GIT_AUTHOR_NAME'"));
+       if (email_i == -2)
+               error(_("missing 'GIT_AUTHOR_EMAIL'"));
+       if (date_i == -2)
+               error(_("missing 'GIT_AUTHOR_DATE'"));
+       if (date_i < 0 || email_i < 0 || date_i < 0 || err)
+               goto finish;
+       *name = kv.items[name_i].util;
+       *email = kv.items[email_i].util;
+       *date = kv.items[date_i].util;
+       retval = 0;
+finish:
+       string_list_clear(&kv, !!retval);
+       strbuf_release(&buf);
+       return retval;
 }
 
 /*
- * Read a list of environment variable assignments (such as the author-script
- * file) into an environment block. Returns -1 on error, 0 otherwise.
+ * Read a GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL AND GIT_AUTHOR_DATE from a
+ * file with shell quoting into struct argv_array. Returns -1 on
+ * error, 0 otherwise.
  */
 static int read_env_script(struct argv_array *env)
 {
-       struct strbuf script = STRBUF_INIT;
-       int i, count = 0, sq_bug;
-       const char *p2;
-       char *p;
+       char *name, *email, *date;
 
-       if (strbuf_read_file(&script, rebase_path_author_script(), 256) <= 0)
+       if (read_author_script(rebase_path_author_script(),
+                              &name, &email, &date, 0))
                return -1;
-       /* write_author_script() used to quote incorrectly */
-       sq_bug = quoting_is_broken(script.buf, script.len);
-       for (p = script.buf; *p; p++)
-               if (sq_bug && skip_prefix(p, "'\\\\''", &p2))
-                       strbuf_splice(&script, p - script.buf, p2 - p, "'", 1);
-               else if (skip_prefix(p, "'\\''", &p2))
-                       strbuf_splice(&script, p - script.buf, p2 - p, "'", 1);
-               else if (*p == '\'')
-                       strbuf_splice(&script, p-- - script.buf, 1, "", 0);
-               else if (*p == '\n') {
-                       *p = '\0';
-                       count++;
-               }
 
-       for (i = 0, p = script.buf; i < count; i++) {
-               argv_array_push(env, p);
-               p += strlen(p) + 1;
-       }
+       argv_array_pushf(env, "GIT_AUTHOR_NAME=%s", name);
+       argv_array_pushf(env, "GIT_AUTHOR_EMAIL=%s", email);
+       argv_array_pushf(env, "GIT_AUTHOR_DATE=%s", date);
+       free(name);
+       free(email);
+       free(date);
 
        return 0;
 }
@@ -737,54 +813,28 @@ static char *get_author(const char *message)
 /* Read author-script and return an ident line (author <email> timestamp) */
 static const char *read_author_ident(struct strbuf *buf)
 {
-       const char *keys[] = {
-               "GIT_AUTHOR_NAME=", "GIT_AUTHOR_EMAIL=", "GIT_AUTHOR_DATE="
-       };
        struct strbuf out = STRBUF_INIT;
-       char *in, *eol;
-       const char *val[3];
-       int i = 0;
+       char *name, *email, *date;
 
-       if (strbuf_read_file(buf, rebase_path_author_script(), 256) <= 0)
+       if (read_author_script(rebase_path_author_script(),
+                              &name, &email, &date, 0))
                return NULL;
 
-       /* dequote values and construct ident line in-place */
-       for (in = buf->buf; i < 3 && in - buf->buf < buf->len; i++) {
-               if (!skip_prefix(in, keys[i], (const char **)&in)) {
-                       warning(_("could not parse '%s' (looking for '%s')"),
-                               rebase_path_author_script(), keys[i]);
-                       return NULL;
-               }
-
-               eol = strchrnul(in, '\n');
-               *eol = '\0';
-               if (!sq_dequote(in)) {
-                       warning(_("bad quoting on %s value in '%s'"),
-                               keys[i], rebase_path_author_script());
-                       return NULL;
-               }
-               val[i] = in;
-               in = eol + 1;
-       }
-
-       if (i < 3) {
-               warning(_("could not parse '%s' (looking for '%s')"),
-                       rebase_path_author_script(), keys[i]);
-               return NULL;
-       }
-
        /* validate date since fmt_ident() will die() on bad value */
-       if (parse_date(val[2], &out)){
+       if (parse_date(date, &out)){
                warning(_("invalid date format '%s' in '%s'"),
-                       val[2], rebase_path_author_script());
+                       date, rebase_path_author_script());
                strbuf_release(&out);
                return NULL;
        }
 
        strbuf_reset(&out);
-       strbuf_addstr(&out, fmt_ident(val[0], val[1], val[2], 0));
+       strbuf_addstr(&out, fmt_ident(name, email, date, 0));
        strbuf_swap(buf, &out);
        strbuf_release(&out);
+       free(name);
+       free(email);
+       free(date);
        return buf->buf;
 }
 
index 660cff5050b39e38e721182861ada83e95e8378b..5071a73563f1cdfc35060603d1fef4e085497798 100644 (file)
@@ -130,6 +130,9 @@ int prepare_branch_to_be_rebased(struct replay_opts *opts, const char *commit);
 #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)
 void print_commit_summary(const char *prefix, const struct object_id *oid,
                          unsigned int flags);
+
+int read_author_script(const char *path, char **name, char **email, char **date,
+                      int allow_missing);
 #endif
 
 void parse_strategy_opts(struct replay_opts *opts, char *raw_opts);