From: "Linus Arver via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Christian Couder <chriscool@tuxfamily.org>,
Junio C Hamano <gitster@pobox.com>,
Emily Shaffer <nasamuffin@google.com>,
Josh Steadmon <steadmon@google.com>,
"Randall S. Becker" <rsbecker@nexbridge.com>,
Christian Couder <christian.couder@gmail.com>,
Kristoffer Haugsbakk <code@khaugsbakk.name>,
Linus Arver <linus@ucla.edu>, Linus Arver <linusa@google.com>,
Linus Arver <linus@ucla.edu>
Subject: [PATCH v4 00/10] Make trailer_info struct private (plus sequencer cleanup)
Date: Thu, 02 May 2024 04:54:17 +0000 [thread overview]
Message-ID: <pull.1696.v4.git.1714625667.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1696.v3.git.1714091170.gitgitgadget@gmail.com>
NOTE: This series is based on the la/format-trailer-info topic branch (see
its discussion at [1]).
This series is based on the initial series [2], notably the v4 version of
patches 17-20 as suggested by Christian [3]. This version addresses the
review comments for those patches, namely the splitting up of Patch 19 there
into 3 separate patches [4] (as Patches 05-07 here) .
The central idea is to make the trailer_info struct private (that is, move
its definition from trailer.h to trailer.c) --- aka the "pimpl" idiom. See
the detailed commit message for Patch 07 for the motivation behind the
change.
Patch 04 makes sequencer.c a well-behaved trailer API consumer, by making
use of the trailer iterator. Patch 03 prepares us for Patch 04. Patch 08
slightly reduces the weight of the API by removing (from the API surface) an
unused function.
Notable changes in v4
=====================
* Drop "25%" language in Patch 03
* Rename some variables
* Update patch emails to personal (linus@ucla.edu) email
Notable changes in v3
=====================
* (NEW Patch 10) Expand test coverage to check the contents of each
iteration (raw, key, val fields), not just the total number of iterations
* (NEW Patch 09) Add documentation in <trailer.h> for using
parse_trailers()
* (unrelated) I will lose access to my linusa@google.com email address
tomorrow (I'm switching jobs!) and so future emails from me will come
from linus@ucla.edu [5]. I've added the latter email to the CC list here
so things should just work. Cheers
Notable changes in v2
=====================
* Add unit tests at the beginning of the series (Patches 01 and 02) and use
it to verify that the other edge cases remain unchanged when we add the
"raw" member (Patch 03)
[1]
https://lore.kernel.org/git/pull.1694.git.1710485706.gitgitgadget@gmail.com/
[2]
https://lore.kernel.org/git/pull.1632.v4.git.1707196348.gitgitgadget@gmail.com/
[3]
https://lore.kernel.org/git/CAP8UFD08F0V13X0+CJ1uhMPzPWVMs2okGVMJch0DkQg5M3BWLA@mail.gmail.com/
[4]
https://lore.kernel.org/git/CAP8UFD1twELGKvvesxgCrZrypKZpgSt04ira3mvurG1UbpDfxQ@mail.gmail.com/
[5]
https://lore.kernel.org/git/pull.1720.git.1713309711217.gitgitgadget@gmail.com/
Linus Arver (10):
Makefile: sort UNIT_TEST_PROGRAMS
trailer: add unit tests for trailer iterator
trailer: teach iterator about non-trailer lines
sequencer: use the trailer iterator
interpret-trailers: access trailer_info with new helpers
trailer: make parse_trailers() return trailer_info pointer
trailer: make trailer_info struct private
trailer: retire trailer_info_get() from API
trailer: document parse_trailers() usage
trailer unit tests: inspect iterator contents
Makefile | 5 +-
builtin/interpret-trailers.c | 12 +-
sequencer.c | 27 ++-
t/unit-tests/t-trailer.c | 315 +++++++++++++++++++++++++++++++++++
trailer.c | 167 ++++++++++++-------
trailer.h | 94 +++++++----
6 files changed, 506 insertions(+), 114 deletions(-)
create mode 100644 t/unit-tests/t-trailer.c
base-commit: 3452d173241c8b87ecdd67f91f594cb14327e394
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1696%2Flistx%2Ftrailer-api-part-3-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1696/listx/trailer-api-part-3-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1696
Range-diff vs v3:
1: b6a1304f8ae ! 1: 8a9f71442d8 Makefile: sort UNIT_TEST_PROGRAMS
@@
## Metadata ##
-Author: Linus Arver <linusa@google.com>
+Author: Linus Arver <linus@ucla.edu>
## Commit message ##
Makefile: sort UNIT_TEST_PROGRAMS
- Signed-off-by: Linus Arver <linusa@google.com>
+ Signed-off-by: Linus Arver <linus@ucla.edu>
## Makefile ##
@@ Makefile: THIRD_PARTY_SOURCES += sha1collisiondetection/%
2: 4ad0fbbb33c ! 2: b503b539c6f trailer: add unit tests for trailer iterator
@@
## Metadata ##
-Author: Linus Arver <linusa@google.com>
+Author: Linus Arver <linus@ucla.edu>
## Commit message ##
trailer: add unit tests for trailer iterator
@@ Commit message
implementation details which are, unlike the API, subject to drastic
changes).
- Signed-off-by: Linus Arver <linusa@google.com>
+ Signed-off-by: Linus Arver <linus@ucla.edu>
## Makefile ##
@@ Makefile: UNIT_TEST_PROGRAMS += t-ctype
3: 9077d5a315d ! 3: 4aeb48050b1 trailer: teach iterator about non-trailer lines
@@
## Metadata ##
-Author: Linus Arver <linusa@google.com>
+Author: Linus Arver <linus@ucla.edu>
## Commit message ##
trailer: teach iterator about non-trailer lines
@@ Commit message
for non-trailer lines, making the comparison still work even with this
commit).
- Rename "num_expected_trailers" to "num_expected_objects" in
+ Rename "num_expected_trailers" to "num_expected" in
t/unit-tests/t-trailer.c because the items we iterate over now include
non-trailer lines.
- Signed-off-by: Linus Arver <linusa@google.com>
+ Signed-off-by: Linus Arver <linus@ucla.edu>
## t/unit-tests/t-trailer.c ##
@@
@@ t/unit-tests/t-trailer.c
#include "trailer.h"
-static void t_trailer_iterator(const char *msg, size_t num_expected_trailers)
-+static void t_trailer_iterator(const char *msg, size_t num_expected_objects)
++static void t_trailer_iterator(const char *msg, size_t num_expected)
{
struct trailer_iterator iter;
size_t i = 0;
@@ t/unit-tests/t-trailer.c: static void t_trailer_iterator(const char *msg, size_t
trailer_iterator_release(&iter);
- check_uint(i, ==, num_expected_trailers);
-+ check_uint(i, ==, num_expected_objects);
++ check_uint(i, ==, num_expected);
}
static void run_t_trailer_iterator(void)
@@ t/unit-tests/t-trailer.c: static void run_t_trailer_iterator(void)
const char *name;
const char *msg;
- size_t num_expected_trailers;
-+ size_t num_expected_objects;
++ size_t num_expected;
} tc[] = {
{
"empty input",
@@ t/unit-tests/t-trailer.c: static void run_t_trailer_iterator(void)
for (int i = 0; i < sizeof(tc) / sizeof(tc[0]); i++) {
TEST(t_trailer_iterator(tc[i].msg,
- tc[i].num_expected_trailers),
-+ tc[i].num_expected_objects),
++ tc[i].num_expected),
"%s", tc[i].name);
}
}
@@ trailer.h: void format_trailers_from_commit(const struct process_trailer_options
struct trailer_iterator {
+ /*
+ * Raw line (e.g., "foo: bar baz") before being parsed as a trailer
-+ * key/val pair as part of a trailer block. A trailer block can be
-+ * either 100% trailer lines, or mixed in with non-trailer lines (in
-+ * which case at least 25% must be trailer lines).
++ * key/val pair as part of a trailer block (as the "key" and "val"
++ * fields below). If a line fails to parse as a trailer, then the "key"
++ * will be the entire line and "val" will be the empty string.
+ */
+ const char *raw;
-+
struct strbuf key;
struct strbuf val;
4: 4a1d18da574 ! 4: a3d080d4d6c sequencer: use the trailer iterator
@@
## Metadata ##
-Author: Linus Arver <linusa@google.com>
+Author: Linus Arver <linus@ucla.edu>
## Commit message ##
sequencer: use the trailer iterator
@@ Commit message
before when we iterated over the unparsed string array (char **trailers)
in trailer_info.
- Signed-off-by: Linus Arver <linusa@google.com>
+ Signed-off-by: Linus Arver <linus@ucla.edu>
## sequencer.c ##
@@ sequencer.c: static const char *get_todo_path(const struct replay_opts *opts)
5: 460979ba964 ! 5: 44df42ca503 interpret-trailers: access trailer_info with new helpers
@@
## Metadata ##
-Author: Linus Arver <linusa@google.com>
+Author: Linus Arver <linus@ucla.edu>
## Commit message ##
interpret-trailers: access trailer_info with new helpers
@@ Commit message
implementation (and thus hidden from the API).
Helped-by: Christian Couder <chriscool@tuxfamily.org>
- Signed-off-by: Linus Arver <linusa@google.com>
+ Signed-off-by: Linus Arver <linus@ucla.edu>
## builtin/interpret-trailers.c ##
@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
6: d217858c637 ! 6: 9ed7cef9d29 trailer: make parse_trailers() return trailer_info pointer
@@
## Metadata ##
-Author: Linus Arver <linusa@google.com>
+Author: Linus Arver <linus@ucla.edu>
## Commit message ##
trailer: make parse_trailers() return trailer_info pointer
@@ Commit message
format_trailers_from_commit() and trailer_iterator_init() accordingly.
Helped-by: Christian Couder <chriscool@tuxfamily.org>
- Signed-off-by: Linus Arver <linusa@google.com>
+ Signed-off-by: Linus Arver <linus@ucla.edu>
## builtin/interpret-trailers.c ##
@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
7: 49c66c48cc1 ! 7: 246ac9a5d07 trailer: make trailer_info struct private
@@
## Metadata ##
-Author: Linus Arver <linusa@google.com>
+Author: Linus Arver <linus@ucla.edu>
## Commit message ##
trailer: make trailer_info struct private
@@ Commit message
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Christian Couder <chriscool@tuxfamily.org>
- Signed-off-by: Linus Arver <linusa@google.com>
+ Signed-off-by: Linus Arver <linus@ucla.edu>
## trailer.c ##
@@
8: 56e1cca4b7b ! 8: ca6f0c4208c trailer: retire trailer_info_get() from API
@@
## Metadata ##
-Author: Linus Arver <linusa@google.com>
+Author: Linus Arver <linus@ucla.edu>
## Commit message ##
trailer: retire trailer_info_get() from API
@@ Commit message
We have to also reposition it to be above parse_trailers(), which
depends on it.
- Signed-off-by: Linus Arver <linusa@google.com>
+ Signed-off-by: Linus Arver <linus@ucla.edu>
## trailer.c ##
@@ trailer.c: static struct trailer_info *trailer_info_new(void)
9: 35304837e08 ! 9: c1a0f1bed04 trailer: document parse_trailers() usage
@@
## Metadata ##
-Author: Linus Arver <linusa@google.com>
+Author: Linus Arver <linus@ucla.edu>
## Commit message ##
trailer: document parse_trailers() usage
@@ Commit message
comments a bit easier to read (because "head" itself doesn't really have
any domain-specific meaning here).
- Signed-off-by: Linus Arver <linusa@google.com>
+ Signed-off-by: Linus Arver <linus@ucla.edu>
## trailer.c ##
@@ trailer.c: static struct trailer_info *trailer_info_get(const struct process_trailer_option
10: 4d53707f836 ! 10: 310b632ddfd trailer unit tests: inspect iterator contents
@@
## Metadata ##
-Author: Linus Arver <linusa@google.com>
+Author: Linus Arver <linus@ucla.edu>
## Commit message ##
trailer unit tests: inspect iterator contents
@@ Commit message
iteration.
Helped-by: Junio C Hamano <gitster@pobox.com>
- Signed-off-by: Linus Arver <linusa@google.com>
+ Signed-off-by: Linus Arver <linus@ucla.edu>
## t/unit-tests/t-trailer.c ##
@@
#include "test-lib.h"
#include "trailer.h"
--static void t_trailer_iterator(const char *msg, size_t num_expected_objects)
-+struct trailer_assertions {
+-static void t_trailer_iterator(const char *msg, size_t num_expected)
++struct contents {
+ const char *raw;
+ const char *key;
+ const char *val;
+};
+
-+static void t_trailer_iterator(const char *msg, size_t num_expected_objects,
-+ struct trailer_assertions *trailer_assertions)
++static void t_trailer_iterator(const char *msg, size_t num_expected,
++ struct contents *contents)
{
struct trailer_iterator iter;
size_t i = 0;
@@ t/unit-tests/t-trailer.c
trailer_iterator_init(&iter, msg);
- while (trailer_iterator_advance(&iter))
+ while (trailer_iterator_advance(&iter)) {
-+ if (num_expected_objects) {
-+ check_str(iter.raw, trailer_assertions[i].raw);
-+ check_str(iter.key.buf, trailer_assertions[i].key);
-+ check_str(iter.val.buf, trailer_assertions[i].val);
++ if (num_expected) {
++ check_str(iter.raw, contents[i].raw);
++ check_str(iter.key.buf, contents[i].key);
++ check_str(iter.val.buf, contents[i].val);
+ }
i++;
+ }
trailer_iterator_release(&iter);
- check_uint(i, ==, num_expected_objects);
-@@ t/unit-tests/t-trailer.c: static void t_trailer_iterator(const char *msg, size_t num_expected_objects)
+ check_uint(i, ==, num_expected);
+@@ t/unit-tests/t-trailer.c: static void t_trailer_iterator(const char *msg, size_t num_expected)
static void run_t_trailer_iterator(void)
{
@@ t/unit-tests/t-trailer.c: static void t_trailer_iterator(const char *msg, size_t
static struct test_cases {
const char *name;
const char *msg;
- size_t num_expected_objects;
-+ struct trailer_assertions trailer_assertions[10];
+ size_t num_expected;
++ struct contents contents[10];
} tc[] = {
{
"empty input",
@@ t/unit-tests/t-trailer.c: static void run_t_trailer_iterator(void)
for (int i = 0; i < sizeof(tc) / sizeof(tc[0]); i++) {
TEST(t_trailer_iterator(tc[i].msg,
-- tc[i].num_expected_objects),
-+ tc[i].num_expected_objects,
-+ tc[i].trailer_assertions),
+- tc[i].num_expected),
++ tc[i].num_expected,
++ tc[i].contents),
"%s", tc[i].name);
}
}
--
gitgitgadget
next prev parent reply other threads:[~2024-05-02 4:54 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-16 6:27 [PATCH 0/6] Make trailer_info struct private (plus sequencer cleanup) Linus Arver via GitGitGadget
2024-03-16 6:27 ` [PATCH 1/6] trailer: teach iterator about non-trailer lines Linus Arver via GitGitGadget
2024-03-16 6:27 ` [PATCH 2/6] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-03-16 6:27 ` [PATCH 3/6] interpret-trailers: access trailer_info with new helpers Linus Arver via GitGitGadget
2024-03-16 6:27 ` [PATCH 4/6] trailer: make parse_trailers() return trailer_info pointer Linus Arver via GitGitGadget
2024-03-16 6:27 ` [PATCH 5/6] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-03-16 6:27 ` [PATCH 6/6] trailer: retire trailer_info_get() from API Linus Arver via GitGitGadget
2024-03-16 17:06 ` [PATCH 0/6] Make trailer_info struct private (plus sequencer cleanup) Junio C Hamano
2024-03-26 22:00 ` Junio C Hamano
2024-04-19 5:36 ` Linus Arver
2024-04-19 5:22 ` [PATCH v2 0/8] " Linus Arver via GitGitGadget
2024-04-19 5:22 ` [PATCH v2 1/8] Makefile: sort UNIT_TEST_PROGRAMS Linus Arver via GitGitGadget
2024-04-19 5:22 ` [PATCH v2 2/8] trailer: add unit tests for trailer iterator Linus Arver via GitGitGadget
2024-04-19 5:33 ` Linus Arver
2024-04-19 18:46 ` Linus Arver
2024-04-19 21:52 ` Junio C Hamano
2024-04-20 0:14 ` Linus Arver
2024-04-19 5:22 ` [PATCH v2 3/8] trailer: teach iterator about non-trailer lines Linus Arver via GitGitGadget
2024-04-19 5:22 ` [PATCH v2 4/8] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-04-23 21:19 ` Junio C Hamano
2024-04-19 5:22 ` [PATCH v2 5/8] interpret-trailers: access trailer_info with new helpers Linus Arver via GitGitGadget
2024-04-19 5:22 ` [PATCH v2 6/8] trailer: make parse_trailers() return trailer_info pointer Linus Arver via GitGitGadget
2024-04-23 23:17 ` Junio C Hamano
2024-04-19 5:22 ` [PATCH v2 7/8] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-04-23 23:27 ` Junio C Hamano
2024-04-25 3:17 ` Linus Arver
2024-04-19 5:22 ` [PATCH v2 8/8] trailer: retire trailer_info_get() from API Linus Arver via GitGitGadget
2024-04-23 23:27 ` Junio C Hamano
2024-04-24 0:27 ` [PATCH v2 0/8] Make trailer_info struct private (plus sequencer cleanup) Junio C Hamano
2024-04-26 0:26 ` [PATCH v3 00/10] " Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 01/10] Makefile: sort UNIT_TEST_PROGRAMS Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 02/10] trailer: add unit tests for trailer iterator Linus Arver via GitGitGadget
2024-04-26 14:51 ` Christian Couder
2024-04-26 16:20 ` Junio C Hamano
2024-04-26 16:25 ` Linus Arver
2024-04-26 0:26 ` [PATCH v3 03/10] trailer: teach iterator about non-trailer lines Linus Arver via GitGitGadget
2024-04-27 12:50 ` Christian Couder
2024-04-30 4:42 ` Linus Arver
2024-04-30 4:55 ` Linus Arver
2024-04-26 0:26 ` [PATCH v3 04/10] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 05/10] interpret-trailers: access trailer_info with new helpers Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 06/10] trailer: make parse_trailers() return trailer_info pointer Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 07/10] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 08/10] trailer: retire trailer_info_get() from API Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 09/10] trailer: document parse_trailers() usage Linus Arver via GitGitGadget
2024-04-26 0:26 ` [PATCH v3 10/10] trailer unit tests: inspect iterator contents Linus Arver via GitGitGadget
2024-04-27 12:51 ` [PATCH v3 00/10] Make trailer_info struct private (plus sequencer cleanup) Christian Couder
2024-05-02 4:54 ` Linus Arver via GitGitGadget [this message]
2024-05-02 4:54 ` [PATCH v4 01/10] Makefile: sort UNIT_TEST_PROGRAMS Linus Arver via GitGitGadget
2024-05-02 4:54 ` [PATCH v4 02/10] trailer: add unit tests for trailer iterator Linus Arver via GitGitGadget
2024-05-02 16:54 ` Junio C Hamano
2024-05-02 4:54 ` [PATCH v4 03/10] trailer: teach iterator about non-trailer lines Linus Arver via GitGitGadget
2024-05-04 15:33 ` Phillip Wood
2024-05-05 1:37 ` Linus Arver
2024-05-05 14:09 ` Phillip Wood
2024-05-09 7:11 ` Linus Arver
2024-05-13 15:11 ` Phillip Wood
2024-05-13 15:13 ` Phillip Wood
2024-05-02 4:54 ` [PATCH v4 04/10] sequencer: use the trailer iterator Linus Arver via GitGitGadget
2024-05-02 4:54 ` [PATCH v4 05/10] interpret-trailers: access trailer_info with new helpers Linus Arver via GitGitGadget
2024-05-02 4:54 ` [PATCH v4 06/10] trailer: make parse_trailers() return trailer_info pointer Linus Arver via GitGitGadget
2024-05-02 4:54 ` [PATCH v4 07/10] trailer: make trailer_info struct private Linus Arver via GitGitGadget
2024-05-02 4:54 ` [PATCH v4 08/10] trailer: retire trailer_info_get() from API Linus Arver via GitGitGadget
2024-05-02 4:54 ` [PATCH v4 09/10] trailer: document parse_trailers() usage Linus Arver via GitGitGadget
2024-05-02 4:54 ` [PATCH v4 10/10] trailer unit tests: inspect iterator contents Linus Arver via GitGitGadget
2024-05-02 17:15 ` [PATCH v4 00/10] Make trailer_info struct private (plus sequencer cleanup) Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.1696.v4.git.1714625667.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=chriscool@tuxfamily.org \
--cc=christian.couder@gmail.com \
--cc=code@khaugsbakk.name \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=linus@ucla.edu \
--cc=linusa@google.com \
--cc=nasamuffin@google.com \
--cc=rsbecker@nexbridge.com \
--cc=steadmon@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).