sequencer: use trailer's trailer layout

Make sequencer use trailer.c's trailer layout definition, as opposed to
parsing the footer by itself. This makes "commit -s", "cherry-pick -x",
and "format-patch --signoff" consistent with trailer, allowing
non-trailer lines and multiple-line trailers in trailer blocks under
certain conditions, and therefore suppressing the extra newline in those
cases.

Consistency with trailer extends to respecting trailer configs.  Tests
have been included to show that.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sequencer.c b/sequencer.c
index 5fd75f30dd4455bf71a12fd628b488fc2b17e192..d64c97397b5387eb707f84e44f32b1dcb4e3f7c3 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -16,6 +16,7 @@
 #include "refs.h"
 #include "argv-array.h"
 #include "quote.h"
 #include "refs.h"
 #include "argv-array.h"
 #include "quote.h"

+#include "trailer.h"

 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 
 
 #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
 


@@ -56,30 +57,6 @@ static const char *get_todo_path(const struct replay_opts *opts)
        return git_path_todo_file();
 }
 
        return git_path_todo_file();
 }
 

-static int is_rfc2822_line(const char *buf, int len)
-{
-       int i;
-
-       for (i = 0; i < len; i++) {
-               int ch = buf[i];
-               if (ch == ':')
-                       return 1;
-               if (!isalnum(ch) && ch != '-')
-                       break;
-       }
-
-       return 0;
-}
-
-static int is_cherry_picked_from_line(const char *buf, int len)
-{
-       /*
-        * We only care that it looks roughly like (cherry picked from ...)
-        */
-       return len > strlen(cherry_picked_prefix) + 1 &&
-               starts_with(buf, cherry_picked_prefix) && buf[len - 1] == ')';
-}
-

 /*
  * Returns 0 for non-conforming footer
  * Returns 1 for conforming footer
 /*
  * Returns 0 for non-conforming footer
  * Returns 1 for conforming footer


@@ -89,49 +66,25 @@ static int is_cherry_picked_from_line(const char *buf, int len)
 static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
        int ignore_footer)
 {
 static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
        int ignore_footer)
 {

-       char prev;
-       int i, k;
-       int len = sb->len - ignore_footer;
-       const char *buf = sb->buf;
-       int found_sob = 0;
-
-       /* footer must end with newline */
-       if (!len || buf[len - 1] != '\n')
-               return 0;
+       struct trailer_info info;
+       int i;
+       int found_sob = 0, found_sob_last = 0;

 
 

-       prev = '\0';
-       for (i = len - 1; i > 0; i--) {
-               char ch = buf[i];
-               if (prev == '\n' && ch == '\n') /* paragraph break */
-                       break;
-               prev = ch;
-       }
+       trailer_info_get(&info, sb->buf);

 
 

-       /* require at least one blank line */
-       if (prev != '\n' || buf[i] != '\n')
+       if (info.trailer_start == info.trailer_end)

                return 0;
 
                return 0;
 

-       /* advance to start of last paragraph */
-       while (i < len - 1 && buf[i] == '\n')
-               i++;
-
-       for (; i < len; i = k) {
-               int found_rfc2822;
-
-               for (k = i; k < len && buf[k] != '\n'; k++)
-                       ; /* do nothing */
-               k++;
+       for (i = 0; i < info.trailer_nr; i++)
+               if (sob && !strncmp(info.trailers[i], sob->buf, sob->len)) {
+                       found_sob = 1;
+                       if (i == info.trailer_nr - 1)
+                               found_sob_last = 1;
+               }

 
 

-               found_rfc2822 = is_rfc2822_line(buf + i, k - i - 1);
-               if (found_rfc2822 && sob &&
-                   !strncmp(buf + i, sob->buf, sob->len))
-                       found_sob = k;
+       trailer_info_release(&info);

 
 

-               if (!(found_rfc2822 ||
-                     is_cherry_picked_from_line(buf + i, k - i - 1)))
-                       return 0;
-       }
-       if (found_sob == i)
+       if (found_sob_last)

                return 3;
        if (found_sob)
                return 2;
                return 3;
        if (found_sob)
                return 2;

diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh
index 9cce5ae8815a11e3453f5669b0592f7f7659009b..bf0a5c9887235afb5e336a34815d6bb3d64875d8 100755 (executable)
--- a/t/t3511-cherry-pick-x.sh
+++ b/t/t3511-cherry-pick-x.sh
@@ -25,9 +25,8 @@ Signed-off-by: B.U. Thor <buthor@example.com>"
 
 mesg_broken_footer="$mesg_no_footer
 
 
 mesg_broken_footer="$mesg_no_footer
 

-The signed-off-by string should begin with the words Signed-off-by followed
-by a colon and space, and then the signers name and email address. e.g.
-Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+This is not recognized as a footer because Myfooter is not a recognized token.
+Myfooter: A.U. Thor <author@example.com>"

 
 mesg_with_footer_sob="$mesg_with_footer
 Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
 
 mesg_with_footer_sob="$mesg_with_footer
 Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"


@@ -112,6 +111,17 @@ test_expect_success 'cherry-pick -s inserts blank line after non-conforming foot
        test_cmp expect actual
 '
 
        test_cmp expect actual
 '
 

+test_expect_success 'cherry-pick -s recognizes trailer config' '
+       pristine_detach initial &&
+       git -c "trailer.Myfooter.ifexists=add" cherry-pick -s mesg-broken-footer &&
+       cat <<-EOF >expect &&
+               $mesg_broken_footer
+               Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
+       EOF
+       git log -1 --pretty=format:%B >actual &&
+       test_cmp expect actual
+'
+

 test_expect_success 'cherry-pick -x inserts blank line when conforming footer not found' '
        pristine_detach initial &&
        sha1=$(git rev-parse mesg-no-footer^0) &&
 test_expect_success 'cherry-pick -x inserts blank line when conforming footer not found' '
        pristine_detach initial &&
        sha1=$(git rev-parse mesg-no-footer^0) &&

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index ba4902df2b605f89ec2abcac50abefc2f23fc9bf..482112ca339f05fc31e6f9c2c6168971a1121e87 100755 (executable)
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1294,8 +1294,7 @@ EOF
 4:Subject: [PATCH] subject
 8:
 10:Signed-off-by: example happens to be wrapped here.
 4:Subject: [PATCH] subject
 8:
 10:Signed-off-by: example happens to be wrapped here.

-11:
-12:Signed-off-by: C O Mitter <committer@example.com>
+11:Signed-off-by: C O Mitter <committer@example.com>

 EOF
        test_cmp expected actual
 '
 EOF
        test_cmp expected actual
 '


@@ -1368,7 +1367,7 @@ EOF
        test_cmp expected actual
 '
 
        test_cmp expected actual
 '
 

-test_expect_success 'signoff: detect garbage in non-conforming footer' '
+test_expect_success 'signoff: tolerate garbage in conforming footer' '

        append_signoff <<\EOF >actual &&
 subject
 
        append_signoff <<\EOF >actual &&
 subject
 


@@ -1383,8 +1382,36 @@ EOF
 8:
 10:
 13:Signed-off-by: C O Mitter <committer@example.com>
 8:
 10:
 13:Signed-off-by: C O Mitter <committer@example.com>

-14:
-15:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+       test_cmp expected actual
+'
+
+test_expect_success 'signoff: respect trailer config' '
+       append_signoff <<\EOF >actual &&
+subject
+
+Myfooter: x
+Some Trash
+EOF
+       cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+11:
+12:Signed-off-by: C O Mitter <committer@example.com>
+EOF
+       test_cmp expected actual &&
+
+       test_config trailer.Myfooter.ifexists add &&
+       append_signoff <<\EOF >actual &&
+subject
+
+Myfooter: x
+Some Trash
+EOF
+       cat >expected <<\EOF &&
+4:Subject: [PATCH] subject
+8:
+11:Signed-off-by: C O Mitter <committer@example.com>

 EOF
        test_cmp expected actual
 '
 EOF
        test_cmp expected actual
 '

diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index d84897a67a3c365e280f88b36f43fc49e1ac9d7b..4003a27e6aa117e30446bca2d91f230f144ae8a9 100755 (executable)
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -460,6 +460,42 @@ $alt" &&
        test_cmp expected actual
 '
 
        test_cmp expected actual
 '
 

+test_expect_success 'signoff respects trailer config' '
+
+       echo 5 >positive &&
+       git add positive &&
+       git commit -s -m "subject
+
+non-trailer line
+Myfooter: x" &&
+       git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
+       (
+               echo subject
+               echo
+               echo non-trailer line
+               echo Myfooter: x
+               echo
+               echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+       ) >expected &&
+       test_cmp expected actual &&
+
+       echo 6 >positive &&
+       git add positive &&
+       git -c "trailer.Myfooter.ifexists=add" commit -s -m "subject
+
+non-trailer line
+Myfooter: x" &&
+       git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
+       (
+               echo subject
+               echo
+               echo non-trailer line
+               echo Myfooter: x
+               echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+       ) >expected &&
+       test_cmp expected actual
+'
+

 test_expect_success 'multiple -m' '
 
        >negative &&
 test_expect_success 'multiple -m' '
 
        >negative &&