git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/4] format-patch: fix a bug in option exclusivity and add a test to t4014

From
Patrick Steinhardt <ps@pks.im>
Date
Apr 17, 2024, 06:27 UTC
Message-ID
<Zh9r1K2_T5wvVJVC@tanuki>
In-Reply-To
<c975f961779b4a7b10c0743b4b8b3ad8c89cb617.1713324598.git.dsimic@manjaro.org>
On Wed, Apr 17, 2024 at 05:32:42AM +0200, Dragan Simic wrote:
Show 29 quoted lines
> Fix a bug that allows --rfc and -k options to be specified together when
> executing "git format-patch".  This bug was introduced back in the commit
> e0d7db7423a9 ("format-patch: --rfc honors what --subject-prefix sets"),
> about eight months ago, but it has remained undetected so far, presumably
> because of no associated test coverage.
> 
> Add a new test to the t4014 that covers the mutual exclusivity of the --rfc
> and -k command-line options for "git format-patch", for future coverage.
> 
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---
>  builtin/log.c           | 5 ++++-
>  t/t4014-format-patch.sh | 4 ++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/log.c b/builtin/log.c
> index c0a8bb95e983..e5a238f1cf2c 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -2050,8 +2050,11 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  	if (cover_from_description_arg)
>  		cover_from_description_mode = parse_cover_from_description(cover_from_description_arg);
>  
> -	if (rfc)
> +	/* Also mark the subject prefix as modified, for later checks */
> +	if (rfc) {
>  		strbuf_insertstr(&sprefix, 0, "RFC ");
> +		subject_prefix = 1;
> +	}

As an alternative fix, can we drop `subject_prefix` and replace it with `sprefix.len` instead? It seems to merely be a proxy value for that anyway, and if we didn't have that variable then the bug would not have been possible to begin with.

Patrick
Show 18 quoted lines
>  	if (reroll_count) {
>  		strbuf_addf(&sprefix, " v%s", reroll_count);
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index e37a1411ee24..e22c4ac34e6e 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1397,6 +1397,10 @@ test_expect_success '--rfc is argument order independent' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success '--rfc and -k cannot be used together' '
> +	test_must_fail git format-patch -1 --stdout --rfc -k >patch
> +'
> +
>  test_expect_success '--from=ident notices bogus ident' '
>  	test_must_fail git format-patch -1 --stdout --from=foo >patch
>  '
> 
Previous: Dragan SimicNext: Dragan Simic
Message 6 of 51 in “format-patch: fix an option coexistence bug and add new --resend option”
  1. 0/4 format-patch: fix an option coexistence bug and add new --resend optionDragan Simic, Apr 17, 2024
  2. 1/4 format-patch docs: avoid use of parentheses to improve readabilityDragan Simic, Apr 17, 2024
  3. 2/4 format-patch: fix a bug in option exclusivity and add a test to t4014Dragan Simic, Apr 17, 2024
  4. Eric SunshineApr 17, 2024
  5. Dragan SimicApr 17, 2024
  6. Patrick SteinhardtApr 17, 2024
  7. Dragan SimicApr 17, 2024
  8. Dragan SimicApr 18, 2024
  9. Kristoffer HaugsbakkApr 17, 2024
  10. Eric SunshineApr 17, 2024
  11. Dragan SimicApr 17, 2024
  12. Kristoffer HaugsbakkApr 17, 2024
  13. Dragan SimicApr 17, 2024
  14. Dragan SimicApr 17, 2024
  15. Dragan SimicApr 17, 2024
  16. 4/4 t4014: add tests to cover --resend option and its exclusivityDragan Simic, Apr 17, 2024
  17. Eric SunshineApr 17, 2024
  18. Dragan SimicApr 17, 2024
  19. 3/4 format-patch: new --resend option for adding "RESEND" to patch subjectsDragan Simic, Apr 17, 2024
  20. Kristoffer HaugsbakkApr 17, 2024
  21. Dragan SimicApr 17, 2024
  22. Kristoffer HaugsbakkApr 17, 2024
  23. Dragan SimicApr 17, 2024
  24. Eric SunshineApr 17, 2024
  25. Dragan SimicApr 17, 2024
  26. Eric SunshineApr 17, 2024
  27. Dragan SimicApr 17, 2024
  28. Dragan SimicApr 18, 2024
  29. Phillip WoodApr 17, 2024
  30. Dragan SimicApr 17, 2024
  31. Kristoffer HaugsbakkApr 17, 2024
  32. Dragan SimicApr 17, 2024
  33. Kristoffer HaugsbakkApr 17, 2024
  34. Dragan SimicApr 17, 2024
  35. Junio C HamanoApr 17, 2024
  36. Dragan SimicApr 17, 2024
  37. Junio C HamanoApr 17, 2024
  38. Dragan SimicApr 17, 2024
  39. Dragan SimicApr 18, 2024
  40. Junio C HamanoApr 18, 2024
  41. Dragan SimicApr 19, 2024
  42. Eric SunshineApr 19, 2024
  43. Dragan SimicApr 19, 2024
  44. Eric SunshineApr 19, 2024
  45. Junio C HamanoApr 19, 2024
  46. Eric SunshineApr 19, 2024
  47. Junio C HamanoApr 19, 2024
  48. Eric SunshineApr 17, 2024
  49. Dragan SimicApr 17, 2024
  50. Eric SunshineApr 17, 2024
  51. Dragan SimicApr 17, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.