sequencer: use read_author_script()

Use the new function added in the last commit to read the author script, updating read_env_script() and read_author_ident(). We now have a single code path that reads the author script for am and all flavors of rebase. This changes the behavior of read_env_script() as previously it would set any environment variables that were in the author-script file. Now it is an error if the file contains other variables or any of GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL and GIT_AUTHOR_DATE are missing. This is what am and the non interactive version of rebase have been doing for several years so hopefully it will not cause a problem for interactive rebase users. The advantage is that we are reusing existing code from am which uses sq_dequote() to properly dequote variables. This fixes potential problems with user edited scripts as read_env_script() which did not track quotes properly. This commit also removes the fallback code for checking for a broken author script after git is upgraded when a rebase is stopped. Now that the parsing uses sq_dequote() it will reliably return an error if the quoting is broken and the user will have to abort the rebase and restart. This isn't ideal but it's a corner case and the detection of the broken quoting could be confused by user edited author scripts. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Phillip Wood committed Oct 31, 2018 at 10:15 UTC 4d010a757c40d6e6e478354991bd052ef30cb853
1 file changed +21 -76
sequencer.c
+21 -76
@@ -771,53 +771,24 @@ finish:
771 }
772
773 /*
774 - * write_author_script() used to fail to terminate the last line with a "'" and
775 - * also escaped "'" incorrectly as "'\\\\''" rather than "'\\''". We check for
776 - * the terminating "'" on the last line to see how "'" has been escaped in case
777 - * git was upgraded while rebase was stopped.
778 - */
779 -static int quoting_is_broken(const char *s, size_t n)
780 -{
781 - /* Skip any empty lines in case the file was hand edited */
782 - while (n > 0 && s[--n] == '\n')
783 - ; /* empty */
784 - if (n > 0 && s[n] != '\'')
785 - return 1;
786 -
787 - return 0;
788 -}
789 -
790 -/*
791 - * Read a list of environment variable assignments (such as the author-script
792 - * file) into an environment block. Returns -1 on error, 0 otherwise.
774 + * Read a GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL AND GIT_AUTHOR_DATE from a
775 + * file with shell quoting into struct argv_array. Returns -1 on
776 + * error, 0 otherwise.
777 */
778 static int read_env_script(struct argv_array *env)
779 {
796 - struct strbuf script = STRBUF_INIT;
797 - int i, count = 0, sq_bug;
798 - const char *p2;
799 - char *p;
780 + char *name, *email, *date;
781
801 - if (strbuf_read_file(&script, rebase_path_author_script(), 256) <= 0)
782 + if (read_author_script(rebase_path_author_script(),
783 + &name, &email, &date, 0))
784 return -1;
803 - /* write_author_script() used to quote incorrectly */
804 - sq_bug = quoting_is_broken(script.buf, script.len);
805 - for (p = script.buf; *p; p++)
806 - if (sq_bug && skip_prefix(p, "'\\\\''", &p2))
807 - strbuf_splice(&script, p - script.buf, p2 - p, "'", 1);
808 - else if (skip_prefix(p, "'\\''", &p2))
809 - strbuf_splice(&script, p - script.buf, p2 - p, "'", 1);
810 - else if (*p == '\'')
811 - strbuf_splice(&script, p-- - script.buf, 1, "", 0);
812 - else if (*p == '\n') {
813 - *p = '\0';
814 - count++;
815 - }
785
817 - for (i = 0, p = script.buf; i < count; i++) {
818 - argv_array_push(env, p);
819 - p += strlen(p) + 1;
820 - }
786 + argv_array_pushf(env, "GIT_AUTHOR_NAME=%s", name);
787 + argv_array_pushf(env, "GIT_AUTHOR_EMAIL=%s", email);
788 + argv_array_pushf(env, "GIT_AUTHOR_DATE=%s", date);
789 + free(name);
790 + free(email);
791 + free(date);
792
793 return 0;
794 }
@@ -837,54 +808,28 @@ static char *get_author(const char *message)
808 /* Read author-script and return an ident line (author <email> timestamp) */
809 static const char *read_author_ident(struct strbuf *buf)
810 {
840 - const char *keys[] = {
841 - "GIT_AUTHOR_NAME=", "GIT_AUTHOR_EMAIL=", "GIT_AUTHOR_DATE="
842 - };
811 struct strbuf out = STRBUF_INIT;
844 - char *in, *eol;
845 - const char *val[3];
846 - int i = 0;
812 + char *name, *email, *date;
813
848 - if (strbuf_read_file(buf, rebase_path_author_script(), 256) <= 0)
814 + if (read_author_script(rebase_path_author_script(),
815 + &name, &email, &date, 0))
816 return NULL;
817
851 - /* dequote values and construct ident line in-place */
852 - for (in = buf->buf; i < 3 && in - buf->buf < buf->len; i++) {
853 - if (!skip_prefix(in, keys[i], (const char **)&in)) {
854 - warning(_("could not parse '%s' (looking for '%s')"),
855 - rebase_path_author_script(), keys[i]);
856 - return NULL;
857 - }
858 -
859 - eol = strchrnul(in, '\n');
860 - *eol = '\0';
861 - if (!sq_dequote(in)) {
862 - warning(_("bad quoting on %s value in '%s'"),
863 - keys[i], rebase_path_author_script());
864 - return NULL;
865 - }
866 - val[i] = in;
867 - in = eol + 1;
868 - }
869 -
870 - if (i < 3) {
871 - warning(_("could not parse '%s' (looking for '%s')"),
872 - rebase_path_author_script(), keys[i]);
873 - return NULL;
874 - }
875 -
818 /* validate date since fmt_ident() will die() on bad value */
877 - if (parse_date(val[2], &out)){
819 + if (parse_date(date, &out)){
820 warning(_("invalid date format '%s' in '%s'"),
879 - val[2], rebase_path_author_script());
821 + date, rebase_path_author_script());
822 strbuf_release(&out);
823 return NULL;
824 }
825
826 strbuf_reset(&out);
885 - strbuf_addstr(&out, fmt_ident(val[0], val[1], val[2], 0));
827 + strbuf_addstr(&out, fmt_ident(name, email, date, 0));
828 strbuf_swap(buf, &out);
829 strbuf_release(&out);
830 + free(name);
831 + free(email);
832 + free(date);
833 return buf->buf;
834 }
835