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

[PATCH v5 0/3] Trailer readability cleanups

From
LGLinus Arver via GitGitGadget <gitgitgadget@gmail.com>
Date
Oct 20, 2023, 19:01 UTC
Message-ID
<pull.1563.v5.git.1697828495.gitgitgadget@gmail.com>
In-Reply-To
<pull.1563.v4.git.1695709372.gitgitgadget@gmail.com>

These patches were created while digging into the trailer code to better understand how it works, in preparation for making the trailer.{c,h} files as small as possible to make them available as a library for external users. This series was originally created as part of [1], but are sent here separately because the changes here are arguably more subjective in nature.

These patches do not add or change any features. Instead, their goal is to make the code easier to understand for new contributors (like myself), by making various cleanups and improvements. Ultimately, my hope is that with such cleanups, we are better positioned to make larger changes (especially the broader libification effort, as in "Introduce Git Standard Library" [2]).

Updates in v5 =============

 * Patch 4 ("trailer: only use trailer_block_* variables if trailers were
   found") has been dropped.
 * Patch 2 returns early if "--no-divider" is true, avoiding unnecessary
   loop iterations (thanks Jonathan).
 * Added missing Reported-by trailer for Patch 3 (it was originally Glen's
   idea to use offsets).
 * Patch 3: Fixed typo in "trailer.h" that referred to an obsolete function
   name ("find_true_end_of_input()", instead of
   "find_end_of_log_message()").

Updates in v4 =============

 * The first 3 patches in v3 were merged into 'master'. Necessarily, those 3
   patches have been dropped.
 * Patch 4 in v3 ("trailer: rename *_DEFAULT enums to *_UNSPECIFIED") has
   been dropped, as well as Patch 9 in v3 ("trailer: make stack variable
   names match field names"). These were dropped to simplify this series for
   what I think is the more immediate, important change (see next bullet
   point).
 * Patches 5-8 in v3 are the only ones remaining in this series. They still
   solely deal with --no-divider and trailer block start/end cleanups.

Updates in v3 =============

 * Patches 4 and 6 (--no-divider and trailer block start/end cleanups) have
   been reorganized to Patches 5-8. This ended up touching commit.c in a
   minor way, but otherwise all of the changes here are cleanups and do not
   change any behavior.

Updates in v2 =============

 * Patch 1: Drop the use of a #define. Instead just use an anonymous struct
   named internal.
 * Patch 2: Don't free info out parameter inside parse_trailers(). Instead
   free it from the caller, process_trailers(). Update comment in
   parse_trailers().
 * Patch 3: Reword commit message.
 * Patch 4: Mention be3d654343 (commit: pass --no-divider to
   interpret-trailers, 2023-06-17) in commit message.
 * Added Patch 6 to make trailer_info use offsets for trailer_start and
   trailer_end (thanks to Glen Choo for the suggestion).

[1] https://lore.kernel.org/git/pull.1564.git.1691210737.gitgitgadget@gmail.com/T/#mb044012670663d8eb7a548924bbcc933bef116de [2] https://lore.kernel.org/git/20230627195251.1973421-1-calvinwan@google.com/ [3] https://lore.kernel.org/git/pull.1149.git.1677143700.gitgitgadget@gmail.com/ [4] https://lore.kernel.org/git/6b4cb31b17077181a311ca87e82464a1e2ad67dd.1686797630.git.gitgitgadget@gmail.com/ [5] https://lore.kernel.org/git/pull.1563.git.1691211879.gitgitgadget@gmail.com/T/#m0131f9829c35d8e0103ffa88f07d8e0e43dd732c

Linus Arver (3):
  commit: ignore_non_trailer computes number of bytes to ignore
  trailer: find the end of the log message
  trailer: use offsets for trailer_start/trailer_end
 builtin/commit.c |  2 +-
 builtin/merge.c  |  2 +-
 commit.c         |  2 +-
 commit.h         |  4 +--
 sequencer.c      |  2 +-
 trailer.c        | 85 +++++++++++++++++++++++++++++-------------------
 trailer.h        | 10 +++---
 7 files changed, 62 insertions(+), 45 deletions(-)
base-commit: bcb6cae2966cc407ca1afc77413b3ef11103c175
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1563%2Flistx%2Ftrailer-libification-prep-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1563/listx/trailer-libification-prep-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/1563
Range-diff vs v4:
 1:  4ce5cf77005 = 1:  4ce5cf77005 commit: ignore_non_trailer computes number of bytes to ignore
 2:  c904caba7e1 ! 2:  ce25420db29 trailer: find the end of the log message
     @@ Commit message
          the starting point which find_trailer_start() needs to start searching
          backward to parse individual trailers (if any).
      
     +    Helped-by: Jonathan Tan <jonathantanmy@google.com>
          Helped-by: Junio C Hamano <gitster@pobox.com>
          Signed-off-by: Linus Arver <linusa@google.com>
      
     @@ trailer.c: static ssize_t last_line(const char *buf, size_t len)
      +static size_t find_end_of_log_message(const char *input, int no_divider)
       {
      +	size_t end;
     -+
       	const char *s;
       
      -	for (s = str; *s; s = next_line(s)) {
      +	/* Assume the naive end of the input is already what we want. */
      +	end = strlen(input);
      +
     ++	if (no_divider) {
     ++		return end;
     ++	}
     ++
      +	/* Optionally skip over any patch part ("---" line and below). */
      +	for (s = input; *s; s = next_line(s)) {
       		const char *v;
       
      -		if (skip_prefix(s, "---", &v) && isspace(*v))
      -			return s - str;
     -+		if (!no_divider && skip_prefix(s, "---", &v) && isspace(*v)) {
     ++		if (skip_prefix(s, "---", &v) && isspace(*v)) {
      +			end = s - input;
      +			break;
      +		}
 3:  796e47c1e5f ! 3:  e3a7b150241 trailer: use offsets for trailer_start/trailer_end
     @@ Commit message
          more explicit about these offsets (that they are for the entire trailer
          block including other trailers). Ditto for trailer_end.
      
     +    Reported-by: Glen Choo <glencbz@gmail.com>
          Signed-off-by: Linus Arver <linusa@google.com>
      
       ## sequencer.c ##
     @@ trailer.h: int trailer_set_if_missing(enum trailer_if_missing *item, const char
      -	 * input string.
      +	 * Offsets to the trailer block start and end positions in the input
      +	 * string. If no trailer block is found, these are both set to the
     -+	 * "true" end of the input, per find_true_end_of_input().
     -+	 *
     -+	 * NOTE: This will be changed so that these point to 0 in the next
     -+	 * patch if no trailers are found.
     ++	 * "true" end of the input (find_end_of_log_message()).
       	 */
      -	const char *trailer_start, *trailer_end;
      +	size_t trailer_block_start, trailer_block_end;
 4:  64e1bd4e4be < -:  ----------- trailer: only use trailer_block_* variables if trailers were found
-- 
gitgitgadget
Previous: Linus Arver via GitGitGadgetNext: Linus Arver via GitGitGadget
Message 66 of 72 in “Trailer readability cleanups”
  1. 0/5 Trailer readability cleanupsLinus Arver via GitGitGadget, Aug 5, 2023
  2. 1/5 trailer: separate public from internal portion of trailer_iteratorLinus Arver via GitGitGadget, Aug 5, 2023
  3. Glen ChooAug 7, 2023
  4. Phillip WoodAug 8, 2023
  5. Linus ArverAug 10, 2023
  6. Linus ArverAug 10, 2023
  7. 2/5 trailer: split process_input_file into separate piecesLinus Arver via GitGitGadget, Aug 5, 2023
  8. Glen ChooAug 7, 2023
  9. Linus ArverAug 11, 2023
  10. 4/5 trailer: teach find_patch_start about --no-dividerLinus Arver via GitGitGadget, Aug 5, 2023
  11. Glen ChooAug 7, 2023
  12. Linus ArverAug 11, 2023
  13. Glen ChooAug 11, 2023
  14. 3/5 trailer: split process_command_line_args into separate functionsLinus Arver via GitGitGadget, Aug 5, 2023
  15. Glen ChooAug 7, 2023
  16. Linus ArverAug 11, 2023
  17. Linus ArverAug 11, 2023
  18. Glen ChooAug 11, 2023
  19. 5/5 trailer: rename *_DEFAULT enums to *_UNSPECIFIEDLinus Arver via GitGitGadget, Aug 5, 2023
  20. Glen ChooAug 7, 2023
  21. Linus ArverAug 11, 2023
  22. 0/6 Trailer readability cleanupsLinus Arver via GitGitGadget, Sep 9, 2023
  23. 1/6 trailer: separate public from internal portion of trailer_iteratorLinus Arver via GitGitGadget, Sep 9, 2023
  24. Junio C HamanoSep 11, 2023
  25. 2/6 trailer: split process_input_file into separate piecesLinus Arver via GitGitGadget, Sep 9, 2023
  26. Junio C HamanoSep 11, 2023
  27. 3/6 trailer: split process_command_line_args into separate functionsLinus Arver via GitGitGadget, Sep 9, 2023
  28. 4/6 trailer: teach find_patch_start about --no-dividerLinus Arver via GitGitGadget, Sep 9, 2023
  29. Junio C HamanoSep 11, 2023
  30. Linus ArverSep 14, 2023
  31. Junio C HamanoSep 14, 2023
  32. Linus ArverSep 14, 2023
  33. 6/6 trailer: use offsets for trailer_start/trailer_endLinus Arver via GitGitGadget, Sep 9, 2023
  34. Junio C HamanoSep 11, 2023
  35. Linus ArverSep 14, 2023
  36. Linus ArverSep 14, 2023
  37. 5/6 trailer: rename *_DEFAULT enums to *_UNSPECIFIEDLinus Arver via GitGitGadget, Sep 9, 2023
  38. Junio C HamanoSep 11, 2023
  39. Linus ArverSep 14, 2023
  40. Junio C HamanoSep 14, 2023
  41. Linus ArverSep 22, 2023
  42. Junio C HamanoSep 22, 2023
  43. Linus ArverSep 26, 2023
  44. 0/9 Trailer readability cleanupsLinus Arver via GitGitGadget, Sep 22, 2023
  45. 1/9 trailer: separate public from internal portion of trailer_iteratorLinus Arver via GitGitGadget, Sep 22, 2023
  46. 2/9 trailer: split process_input_file into separate piecesLinus Arver via GitGitGadget, Sep 22, 2023
  47. 3/9 trailer: split process_command_line_args into separate functionsLinus Arver via GitGitGadget, Sep 22, 2023
  48. 4/9 trailer: rename *_DEFAULT enums to *_UNSPECIFIEDLinus Arver via GitGitGadget, Sep 22, 2023
  49. 5/9 commit: ignore_non_trailer computes number of bytes to ignoreLinus Arver via GitGitGadget, Sep 22, 2023
  50. 6/9 trailer: find the end of the log messageLinus Arver via GitGitGadget, Sep 22, 2023
  51. 9/9 trailer: make stack variable names match field namesLinus Arver via GitGitGadget, Sep 22, 2023
  52. 7/9 trailer: use offsets for trailer_start/trailer_endLinus Arver via GitGitGadget, Sep 22, 2023
  53. 8/9 trailer: only use trailer_block_* variables if trailers were foundLinus Arver via GitGitGadget, Sep 22, 2023
  54. Junio C HamanoSep 22, 2023
  55. Linus ArverSep 22, 2023
  56. Junio C HamanoSep 23, 2023
  57. Linus ArverSep 26, 2023
  58. 0/4 Trailer readability cleanupsLinus Arver via GitGitGadget, Sep 26, 2023
  59. 1/4 commit: ignore_non_trailer computes number of bytes to ignoreLinus Arver via GitGitGadget, Sep 26, 2023
  60. 2/4 trailer: find the end of the log messageLinus Arver via GitGitGadget, Sep 26, 2023
  61. Jonathan TanSep 28, 2023
  62. Linus ArverOct 20, 2023
  63. Junio C HamanoOct 20, 2023
  64. 3/4 trailer: use offsets for trailer_start/trailer_endLinus Arver via GitGitGadget, Sep 26, 2023
  65. 4/4 trailer: only use trailer_block_* variables if trailers were foundLinus Arver via GitGitGadget, Sep 26, 2023
  66. 0/3 Trailer readability cleanupsLinus Arver via GitGitGadget, Oct 20, 2023
  67. 1/3 commit: ignore_non_trailer computes number of bytes to ignoreLinus Arver via GitGitGadget, Oct 20, 2023
  68. 2/3 trailer: find the end of the log messageLinus Arver via GitGitGadget, Oct 20, 2023
  69. Junio C HamanoOct 20, 2023
  70. Linus ArverDec 29, 2023
  71. Linus ArverDec 29, 2023
  72. 3/3 trailer: use offsets for trailer_start/trailer_endLinus Arver via GitGitGadget, Oct 20, 2023

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.