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>
This commit is contained in:
		 Jonathan Tan
					Jonathan Tan
				
			
				
					committed by
					
						 Junio C Hamano
						Junio C Hamano
					
				
			
			
				
	
			
			
			 Junio C Hamano
						Junio C Hamano
					
				
			
						parent
						
							e8c352c316
						
					
				
				
					commit
					967dfd4d56
				
			
							
								
								
									
										77
									
								
								sequencer.c
									
									
									
									
									
								
							
							
						
						
									
										77
									
								
								sequencer.c
									
									
									
									
									
								
							| @ -16,6 +16,7 @@ | ||||
| #include "refs.h" | ||||
| #include "argv-array.h" | ||||
| #include "quote.h" | ||||
| #include "trailer.h" | ||||
|  | ||||
| #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(); | ||||
| } | ||||
|  | ||||
| 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 | ||||
| @ -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) | ||||
| { | ||||
| 	char prev; | ||||
| 	int i, k; | ||||
| 	int len = sb->len - ignore_footer; | ||||
| 	const char *buf = sb->buf; | ||||
| 	int found_sob = 0; | ||||
| 	struct trailer_info info; | ||||
| 	int i; | ||||
| 	int found_sob = 0, found_sob_last = 0; | ||||
|  | ||||
| 	/* footer must end with newline */ | ||||
| 	if (!len || buf[len - 1] != '\n') | ||||
| 	trailer_info_get(&info, sb->buf); | ||||
|  | ||||
| 	if (info.trailer_start == info.trailer_end) | ||||
| 		return 0; | ||||
|  | ||||
| 	prev = '\0'; | ||||
| 	for (i = len - 1; i > 0; i--) { | ||||
| 		char ch = buf[i]; | ||||
| 		if (prev == '\n' && ch == '\n') /* paragraph break */ | ||||
| 			break; | ||||
| 		prev = ch; | ||||
| 	} | ||||
| 	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; | ||||
| 		} | ||||
|  | ||||
| 	/* require at least one blank line */ | ||||
| 	if (prev != '\n' || buf[i] != '\n') | ||||
| 		return 0; | ||||
| 	trailer_info_release(&info); | ||||
|  | ||||
| 	/* 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++; | ||||
|  | ||||
| 		found_rfc2822 = is_rfc2822_line(buf + i, k - i - 1); | ||||
| 		if (found_rfc2822 && sob && | ||||
| 		    !strncmp(buf + i, sob->buf, sob->len)) | ||||
| 			found_sob = k; | ||||
|  | ||||
| 		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; | ||||
|  | ||||
| @ -25,9 +25,8 @@ Signed-off-by: B.U. Thor <buthor@example.com>" | ||||
|  | ||||
| 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>" | ||||
| @ -112,6 +111,17 @@ test_expect_success 'cherry-pick -s inserts blank line after non-conforming foot | ||||
| 	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) && | ||||
|  | ||||
| @ -1294,8 +1294,7 @@ EOF | ||||
| 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 | ||||
| ' | ||||
| @ -1368,7 +1367,7 @@ EOF | ||||
| 	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 | ||||
|  | ||||
| @ -1383,8 +1382,36 @@ EOF | ||||
| 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 | ||||
| ' | ||||
|  | ||||
| @ -460,6 +460,42 @@ $alt" && | ||||
| 	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 && | ||||
|  | ||||
		Reference in New Issue
	
	Block a user