* [PATCH v3 0/2] index-pack: fsck honor checks
From: John Cai via GitGitGadget @ 2024-01-26 20:59 UTC (permalink / raw)
To: git; +Cc: John Cai
In-Reply-To: <pull.1658.v2.git.git.1706289180.gitgitgadget@gmail.com>
git-index-pack has a --strict mode that can take an optional argument to
provide a list of fsck issues to change their severity. --fsck-objects does
not have such a utility, which would be useful if one would like to be more
lenient or strict on data integrity in a repository.
Like --strict, Allow --fsck-objects to also take a list of fsck msgs to
change the severity.
Changes since V2:
* fixed some typos in the documentation
* added commit trailers
Change since V1:
* edited commit messages
* clarified formatting in documentation for --strict= and --fsck-objects=
John Cai (2):
index-pack: test and document --strict=<msg-id>=<severity>...
index-pack: --fsck-objects to take an optional argument for fsck msgs
Documentation/git-index-pack.txt | 26 +++++++++++++-------
builtin/index-pack.c | 5 ++--
t/t5300-pack-object.sh | 41 ++++++++++++++++++++++++++++++++
3 files changed, 62 insertions(+), 10 deletions(-)
base-commit: 186b115d3062e6230ee296d1ddaa0c4b72a464b5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1658%2Fjohn-cai%2Fjc%2Findex-pack-fsck-honor-checks-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1658/john-cai/jc/index-pack-fsck-honor-checks-v3
Pull-Request: https://github.com/git/git/pull/1658
Range-diff vs v2:
1: b3b3e8bd0bf ! 1: cdf7fc7fe8a index-pack: test and document --strict=<msg>
@@ Metadata
Author: John Cai <johncai86@gmail.com>
## Commit message ##
- index-pack: test and document --strict=<msg>
+ index-pack: test and document --strict=<msg-id>=<severity>...
5d477a334a (fsck (receive-pack): allow demoting errors to warnings,
2015-06-22) allowed a list of fsck msg to downgrade to be passed to
@@ Commit message
directly, (nor use index-pack for that matter) it is still useful to
document and test this feature.
+ Reviewed-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: John Cai <johncai86@gmail.com>
## Documentation/git-index-pack.txt ##
@@ Documentation/git-index-pack.txt: OPTIONS
---strict::
- Die, if the pack contains broken objects or links.
+--strict[=<msg-id>=<severity>...]::
-+ Die, if the pack contains broken objects or links. If `<msg-ids>` is passed,
-+ it should be a comma-separated list of `<msg-id>=<severity>` elements where
-+ `<msg-id>` and `<severity>` are used to change the severity of some possible
-+ issues, e.g., `--strict="missingEmail=ignore,badTagName=error"`. See the entry
-+ for the `fsck.<msg-id>` configuration options in `linkgit:git-fsck[1] for
-+ more information on the possible values of `<msg-id>` and `<severity>`.
++ Die, if the pack contains broken objects or links. An optional
++ comma-separated list of `<msg-id>=<severity>` can be passed to change
++ the severity of some possible issues, e.g.,
++ `--strict="missingEmail=ignore,badTagName=error"`. See the entry for the
++ `fsck.<msg-id>` configuration options in linkgit:git-fsck[1] for more
++ information on the possible values of `<msg-id>` and `<severity>`.
--progress-title::
For internal use only.
@@ builtin/index-pack.c
static const char index_pack_usage[] =
-"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
-+"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-ids>]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
++"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-id>=<severity>...]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
struct object_entry {
struct pack_idx_entry idx;
2: cce63c6465f ! 2: a2b9adb93d8 index-pack: --fsck-objects to take an optional argument for fsck msgs
@@ Commit message
the option. This won't often be used by the normal end user, but it
turns out it is useful for Git forges like GitLab.
+ Reviewed-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: John Cai <johncai86@gmail.com>
## Documentation/git-index-pack.txt ##
@@ Documentation/git-index-pack.txt: default and "Indexing objects" when `--stdin`
---fsck-objects::
- For internal use only.
-+--fsck-objects[=<msg-ids>=<severity>...]::
-+ Instructs index-pack to check for broken objects, but unlike `--strict`,
-+ does not choke on broken links. If `<msg-ids>` is passed, it should be
-+ a comma-separated list of `<msg-id>=<severity>` where `<msg-id>` and
-+ `<severity>` are used to change the severity of `fsck` errors e.g.,
-+ `--strict="missingEmail=ignore,badTagName=ignore"`. See the entry for
-+ the `fsck.<msg-id>` configuration options in `linkgit:git-fsck[1] for
-+ more information on the possible values of `<msg-id>` and `<severity>`.
++--fsck-objects[=<msg-id>=<severity>...]::
++ Die if the pack contains broken objects. If the pack contains a tree
++ pointing to a .gitmodules blob that does not exist, prints the hash of
++ that blob (for the caller to check) after the hash that goes into the
++ name of the pack/idx file (see "Notes").
+
- Die if the pack contains broken objects. If the pack contains a tree
- pointing to a .gitmodules blob that does not exist, prints the hash of
+-Die if the pack contains broken objects. If the pack contains a tree
+-pointing to a .gitmodules blob that does not exist, prints the hash of
+-that blob (for the caller to check) after the hash that goes into the
+-name of the pack/idx file (see "Notes").
++Unlike `--strict` however, don't choke on broken links. An optional
++comma-separated list of `<msg-id>=<severity>` can be passed to change the
++severity of some possible issues, e.g.,
++`--fsck-objects="missingEmail=ignore,badTagName=ignore"`. See the entry for the
++`fsck.<msg-id>` configuration options in linkgit:git-fsck[1] for more
++information on the possible values of `<msg-id>` and `<severity>`.
+
+ --threads=<n>::
+ Specifies the number of threads to spawn when resolving
## builtin/index-pack.c ##
@@
#include "setup.h"
static const char index_pack_usage[] =
--"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-ids>]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
-+"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-ids>]] [--fsck-objects[=<msg-ids>]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
+-"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-id>=<severity>...]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
++"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-id>=<severity>...]] [--fsck-objects[=<msg-id>=<severity>...]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
struct object_entry {
struct pack_idx_entry idx;
--
gitgitgadget
^ permalink raw reply
* [PATCH v3 1/2] index-pack: test and document --strict=<msg-id>=<severity>...
From: John Cai via GitGitGadget @ 2024-01-26 20:59 UTC (permalink / raw)
To: git; +Cc: John Cai, John Cai
In-Reply-To: <pull.1658.v3.git.git.1706302749.gitgitgadget@gmail.com>
From: John Cai <johncai86@gmail.com>
5d477a334a (fsck (receive-pack): allow demoting errors to warnings,
2015-06-22) allowed a list of fsck msg to downgrade to be passed to
--strict. However this is a hidden argument that was not documented nor
tested. Though it is true that most users would not call this option
directly, (nor use index-pack for that matter) it is still useful to
document and test this feature.
Reviewed-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: John Cai <johncai86@gmail.com>
---
Documentation/git-index-pack.txt | 9 +++++++--
builtin/index-pack.c | 2 +-
t/t5300-pack-object.sh | 22 ++++++++++++++++++++++
3 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt
index 6486620c3d8..694bb9409bf 100644
--- a/Documentation/git-index-pack.txt
+++ b/Documentation/git-index-pack.txt
@@ -79,8 +79,13 @@ OPTIONS
to force the version for the generated pack index, and to force
64-bit index entries on objects located above the given offset.
---strict::
- Die, if the pack contains broken objects or links.
+--strict[=<msg-id>=<severity>...]::
+ Die, if the pack contains broken objects or links. An optional
+ comma-separated list of `<msg-id>=<severity>` can be passed to change
+ the severity of some possible issues, e.g.,
+ `--strict="missingEmail=ignore,badTagName=error"`. See the entry for the
+ `fsck.<msg-id>` configuration options in linkgit:git-fsck[1] for more
+ information on the possible values of `<msg-id>` and `<severity>`.
--progress-title::
For internal use only.
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 1ea87e01f29..240c7021168 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -24,7 +24,7 @@
#include "setup.h"
static const char index_pack_usage[] =
-"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
+"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-id>=<severity>...]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
struct object_entry {
struct pack_idx_entry idx;
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index d402ec18b79..496fffa0f8a 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -441,6 +441,28 @@ test_expect_success 'index-pack with --strict' '
)
'
+test_expect_success 'index-pack with --strict downgrading fsck msgs' '
+ test_when_finished rm -rf strict &&
+ git init strict &&
+ (
+ cd strict &&
+ test_commit first hello &&
+ cat >commit <<-EOF &&
+ tree $(git rev-parse HEAD^{tree})
+ parent $(git rev-parse HEAD)
+ author A U Thor
+ committer A U Thor
+
+ commit: this is a commit with bad emails
+
+ EOF
+ git hash-object --literally -t commit -w --stdin <commit >commit_list &&
+ PACK=$(git pack-objects test <commit_list) &&
+ test_must_fail git index-pack --strict "test-$PACK.pack" &&
+ git index-pack --strict="missingEmail=ignore" "test-$PACK.pack"
+ )
+'
+
test_expect_success 'honor pack.packSizeLimit' '
git config pack.packSizeLimit 3m &&
packname_10=$(git pack-objects test-10 <obj-list) &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 2/2] index-pack: --fsck-objects to take an optional argument for fsck msgs
From: John Cai via GitGitGadget @ 2024-01-26 20:59 UTC (permalink / raw)
To: git; +Cc: John Cai, John Cai
In-Reply-To: <pull.1658.v3.git.git.1706302749.gitgitgadget@gmail.com>
From: John Cai <johncai86@gmail.com>
git-index-pack has a --strict option that can take an optional argument
to provide a list of fsck issues to change their severity.
--fsck-objects does not have such a utility, which would be useful if
one would like to be more lenient or strict on data integrity in a
repository.
Like --strict, allow --fsck-objects to also take a list of fsck msgs to
change the severity.
Remove the "For internal use only" note for --fsck-objects, and document
the option. This won't often be used by the normal end user, but it
turns out it is useful for Git forges like GitLab.
Reviewed-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: John Cai <johncai86@gmail.com>
---
Documentation/git-index-pack.txt | 17 +++++++++++------
builtin/index-pack.c | 5 +++--
t/t5300-pack-object.sh | 29 ++++++++++++++++++++++++-----
3 files changed, 38 insertions(+), 13 deletions(-)
diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt
index 694bb9409bf..3db1062d1e4 100644
--- a/Documentation/git-index-pack.txt
+++ b/Documentation/git-index-pack.txt
@@ -96,13 +96,18 @@ default and "Indexing objects" when `--stdin` is specified.
--check-self-contained-and-connected::
Die if the pack contains broken links. For internal use only.
---fsck-objects::
- For internal use only.
+--fsck-objects[=<msg-id>=<severity>...]::
+ Die if the pack contains broken objects. If the pack contains a tree
+ pointing to a .gitmodules blob that does not exist, prints the hash of
+ that blob (for the caller to check) after the hash that goes into the
+ name of the pack/idx file (see "Notes").
+
-Die if the pack contains broken objects. If the pack contains a tree
-pointing to a .gitmodules blob that does not exist, prints the hash of
-that blob (for the caller to check) after the hash that goes into the
-name of the pack/idx file (see "Notes").
+Unlike `--strict` however, don't choke on broken links. An optional
+comma-separated list of `<msg-id>=<severity>` can be passed to change the
+severity of some possible issues, e.g.,
+`--fsck-objects="missingEmail=ignore,badTagName=ignore"`. See the entry for the
+`fsck.<msg-id>` configuration options in linkgit:git-fsck[1] for more
+information on the possible values of `<msg-id>` and `<severity>`.
--threads=<n>::
Specifies the number of threads to spawn when resolving
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 240c7021168..a3a37bd215d 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -24,7 +24,7 @@
#include "setup.h"
static const char index_pack_usage[] =
-"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-id>=<severity>...]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
+"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-id>=<severity>...]] [--fsck-objects[=<msg-id>=<severity>...]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
struct object_entry {
struct pack_idx_entry idx;
@@ -1785,8 +1785,9 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
} else if (!strcmp(arg, "--check-self-contained-and-connected")) {
strict = 1;
check_self_contained_and_connected = 1;
- } else if (!strcmp(arg, "--fsck-objects")) {
+ } else if (skip_to_optional_arg(arg, "--fsck-objects", &arg)) {
do_fsck_object = 1;
+ fsck_set_msg_types(&fsck_options, arg);
} else if (!strcmp(arg, "--verify")) {
verify = 1;
} else if (!strcmp(arg, "--verify-stat")) {
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 496fffa0f8a..a58f91035d1 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -441,8 +441,7 @@ test_expect_success 'index-pack with --strict' '
)
'
-test_expect_success 'index-pack with --strict downgrading fsck msgs' '
- test_when_finished rm -rf strict &&
+test_expect_success 'setup for --strict and --fsck-objects downgrading fsck msgs' '
git init strict &&
(
cd strict &&
@@ -457,12 +456,32 @@ test_expect_success 'index-pack with --strict downgrading fsck msgs' '
EOF
git hash-object --literally -t commit -w --stdin <commit >commit_list &&
- PACK=$(git pack-objects test <commit_list) &&
- test_must_fail git index-pack --strict "test-$PACK.pack" &&
- git index-pack --strict="missingEmail=ignore" "test-$PACK.pack"
+ git pack-objects test <commit_list >pack-name
)
'
+test_with_bad_commit () {
+ must_fail_arg="$1" &&
+ must_pass_arg="$2" &&
+ (
+ cd strict &&
+ test_expect_fail git index-pack "$must_fail_arg" "test-$(cat pack-name).pack"
+ git index-pack "$must_pass_arg" "test-$(cat pack-name).pack"
+ )
+}
+
+test_expect_success 'index-pack with --strict downgrading fsck msgs' '
+ test_with_bad_commit --strict --strict="missingEmail=ignore"
+'
+
+test_expect_success 'index-pack with --fsck-objects downgrading fsck msgs' '
+ test_with_bad_commit --fsck-objects --fsck-objects="missingEmail=ignore"
+'
+
+test_expect_success 'cleanup for --strict and --fsck-objects downgrading fsck msgs' '
+ rm -rf strict
+'
+
test_expect_success 'honor pack.packSizeLimit' '
git config pack.packSizeLimit 3m &&
packname_10=$(git pack-objects test-10 <obj-list) &&
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v3 0/2] index-pack: fsck honor checks
From: Junio C Hamano @ 2024-01-26 21:18 UTC (permalink / raw)
To: John Cai via GitGitGadget, Jonathan Tan; +Cc: git, John Cai
In-Reply-To: <pull.1658.v3.git.git.1706302749.gitgitgadget@gmail.com>
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 1: b3b3e8bd0bf ! 1: cdf7fc7fe8a index-pack: test and document --strict=<msg>
> @@ Metadata
> Author: John Cai <johncai86@gmail.com>
>
> ## Commit message ##
> - index-pack: test and document --strict=<msg>
> + index-pack: test and document --strict=<msg-id>=<severity>...
Ah, I missed this one. Nice spotting.
> 5d477a334a (fsck (receive-pack): allow demoting errors to warnings,
> 2015-06-22) allowed a list of fsck msg to downgrade to be passed to
> @@ Commit message
> directly, (nor use index-pack for that matter) it is still useful to
> document and test this feature.
>
> + Reviewed-by: Christian Couder <christian.couder@gmail.com>
> Signed-off-by: John Cai <johncai86@gmail.com>
I haven't seen Christian involved (by getting Cc'ed these patches,
sending out review comments, or giving his Reviewed-by:) during
these three rounds of this topic. I'll wait until I hear from him
before queuing this, just to be safe.
> ++ Die, if the pack contains broken objects or links. An optional
> ++ comma-separated list of `<msg-id>=<severity>` can be passed to change
> ++ the severity of some possible issues, e.g.,
> ++ `--strict="missingEmail=ignore,badTagName=error"`. See the entry for the
> ++ `fsck.<msg-id>` configuration options in linkgit:git-fsck[1] for more
> ++ information on the possible values of `<msg-id>` and `<severity>`.
This is much better than the tentative text I tweaked. Nice.
> ++--fsck-objects[=<msg-id>=<severity>...]::
> ++ Die if the pack contains broken objects. If the pack contains a tree
> ++ pointing to a .gitmodules blob that does not exist, prints the hash of
> ++ that blob (for the caller to check) after the hash that goes into the
> ++ name of the pack/idx file (see "Notes").
Not a new problem bit I have to wonder what happens if the pack
contains many trees that point at different blobs for ".gitmodules"
path and many of these blobs are not included in the packfile? Will
the caller receive all of these blob object names so that they can
be verified? The reference to the "Notes" only refer to the fact
that usually a single hash value that is used in constructing the
name of the packfile "pack-<Hashvalue>.pack" is emitted to the
standard output, which is not wrong per se, but does not help
readers very much wrt to understanding this.
[jc: dragging JTan into the thread, as this comes from his 5476e1ef
(fetch-pack: print and use dangling .gitmodules, 2021-02-22)].
> +
> ++Unlike `--strict` however, don't choke on broken links. An optional
You'd need a comma on both sides of "however" used like this, I
think.
In any case, I thought your original construction to have this
"unlike" immediately after "die on broken objects" was far easier to
follow.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 0/2] index-pack: fsck honor checks
From: John Cai @ 2024-01-26 22:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: John Cai via GitGitGadget, Jonathan Tan, git
In-Reply-To: <xmqqfryjn686.fsf@gitster.g>
Hi Junio,
On 26 Jan 2024, at 16:18, Junio C Hamano wrote:
> "John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> 1: b3b3e8bd0bf ! 1: cdf7fc7fe8a index-pack: test and document --strict=<msg>
>> @@ Metadata
>> Author: John Cai <johncai86@gmail.com>
>>
>> ## Commit message ##
>> - index-pack: test and document --strict=<msg>
>> + index-pack: test and document --strict=<msg-id>=<severity>...
>
> Ah, I missed this one. Nice spotting.
>
>> 5d477a334a (fsck (receive-pack): allow demoting errors to warnings,
>> 2015-06-22) allowed a list of fsck msg to downgrade to be passed to
>> @@ Commit message
>> directly, (nor use index-pack for that matter) it is still useful to
>> document and test this feature.
>>
>> + Reviewed-by: Christian Couder <christian.couder@gmail.com>
>> Signed-off-by: John Cai <johncai86@gmail.com>
>
> I haven't seen Christian involved (by getting Cc'ed these patches,
> sending out review comments, or giving his Reviewed-by:) during
> these three rounds of this topic. I'll wait until I hear from him
> before queuing this, just to be safe.
Christian was involved on an off-list review of this patch series. You can see
it in [1].
1. https://gitlab.com/gitlab-org/git/-/merge_requests/88
>
>> ++ Die, if the pack contains broken objects or links. An optional
>> ++ comma-separated list of `<msg-id>=<severity>` can be passed to change
>> ++ the severity of some possible issues, e.g.,
>> ++ `--strict="missingEmail=ignore,badTagName=error"`. See the entry for the
>> ++ `fsck.<msg-id>` configuration options in linkgit:git-fsck[1] for more
>> ++ information on the possible values of `<msg-id>` and `<severity>`.
>
> This is much better than the tentative text I tweaked. Nice.
>
>> ++--fsck-objects[=<msg-id>=<severity>...]::
>> ++ Die if the pack contains broken objects. If the pack contains a tree
>> ++ pointing to a .gitmodules blob that does not exist, prints the hash of
>> ++ that blob (for the caller to check) after the hash that goes into the
>> ++ name of the pack/idx file (see "Notes").
>
> Not a new problem bit I have to wonder what happens if the pack
> contains many trees that point at different blobs for ".gitmodules"
> path and many of these blobs are not included in the packfile? Will
> the caller receive all of these blob object names so that they can
> be verified? The reference to the "Notes" only refer to the fact
> that usually a single hash value that is used in constructing the
> name of the packfile "pack-<Hashvalue>.pack" is emitted to the
> standard output, which is not wrong per se, but does not help
> readers very much wrt to understanding this.
>
> [jc: dragging JTan into the thread, as this comes from his 5476e1ef
> (fetch-pack: print and use dangling .gitmodules, 2021-02-22)].
sounds good, will wait for some clarification here
>
>> +
>> ++Unlike `--strict` however, don't choke on broken links. An optional
>
> You'd need a comma on both sides of "however" used like this, I
> think.
good catch
>
> In any case, I thought your original construction to have this
> "unlike" immediately after "die on broken objects" was far easier to
> follow.
I'll reformulate this to be clearer. From the previous version I realized I
didn't take into account the pre-existing "Die if the pack contains broken
objects" block so I put it at the beginning. But now I think you're right in
that the "Unlike..." comes too late.
>
> Thanks.
^ permalink raw reply
* Re: [PATCH v3 0/2] index-pack: fsck honor checks
From: Jonathan Tan @ 2024-01-26 22:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Tan, John Cai via GitGitGadget, git, John Cai
In-Reply-To: <xmqqfryjn686.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> > ++--fsck-objects[=<msg-id>=<severity>...]::
> > ++ Die if the pack contains broken objects. If the pack contains a tree
> > ++ pointing to a .gitmodules blob that does not exist, prints the hash of
> > ++ that blob (for the caller to check) after the hash that goes into the
> > ++ name of the pack/idx file (see "Notes").
>
> Not a new problem bit I have to wonder what happens if the pack
> contains many trees that point at different blobs for ".gitmodules"
> path and many of these blobs are not included in the packfile? Will
> the caller receive all of these blob object names so that they can
> be verified? The reference to the "Notes" only refer to the fact
> that usually a single hash value that is used in constructing the
> name of the packfile "pack-<Hashvalue>.pack" is emitted to the
> standard output, which is not wrong per se, but does not help
> readers very much wrt to understanding this.
>
> [jc: dragging JTan into the thread, as this comes from his 5476e1ef
> (fetch-pack: print and use dangling .gitmodules, 2021-02-22)].
Ah...I can see how that documentation isn't clear. The intention of that
commit is to check every link to a .gitmodules blob. The tests perhaps
should have been written with 2 .gitmodules blobs (in separate commits),
but I think the production code works: I tried changing the test to have
2 commits each with their own .gitmodules blob, and error messages were
printed for both blobs.
(If someone changes that test, e.g. to have 2 blobs, the ">h" in the
"configure_exclusion" invocations look superfluous and is perhaps a
copy-and-paste error from other tests that needed the hash later.)
^ permalink raw reply
* [PATCH v2 00/10] Enrich Trailer API
From: Linus Arver via GitGitGadget @ 2024-01-26 22:38 UTC (permalink / raw)
To: git
Cc: Christian Couder, Junio C Hamano, Emily Shaffer, Josh Steadmon,
Randall S. Becker, Linus Arver
In-Reply-To: <pull.1632.git.1704869487.gitgitgadget@gmail.com>
This patch series is the first 10 patches of a larger cleanup/bugfix series
(henceforth "larger series") I've been working on. The main goal of this
series is to begin the process of "libifying" the trailer API. By "API" I
mean the interface exposed in trailer.h. The larger series brings a number
of additional cleanups (exposing and fixing some bugs along the way), and
builds on top of this series.
When the larger series is merged, we will be in a good state to additionally
pursue the following goals:
1. "API reuse inside Git": make the API expressive enough to eliminate any
need by other parts of Git to use the interpret-trailers builtin as a
subprocess (instead they could just use the API directly);
2. "API stability": add unit tests to codify the expected behavior of API
functions; and
3. "API documentation": create developer-focused documentation to explain
how to use the API effectively, noting any API limitations or
anti-patterns.
The reason why the larger series itself doesn't tackle these goals directly
is because I believe that API code should be thought from the ground up with
a libification-focused perspective. Some structs and functions exposed in
the API today should probably not be libified (read: kept in trailer.h) as
is. For example, the "trailer_iterator" struct has a "private" member and it
feels wrong to allow API users to peek inside here (and take at face value
our future API users' pinky promise that they won't depend on those private
internals not meant for public consumption).
One pattern we could use here to cleanly separate "what is the API"
(publicly exposed) and "what is the implementation" (private) is the
pointer-to-implementation ("pimpl") idiom. There may be other appropriate
patterns, but I've chosen this one because it's a simple, low-level concept
(put structs in foo.c instead of foo.h), which has far-reaching high-level
consequences (API users must rely exclusively on the API to make use of such
private structs, via opaque pointers). The pimpl idiom for C comes from the
book "C Interfaces and Implementations" (see patch "trailer: make
trailer_info struct private").
The idea of turning a public struct into a private one is a fundamental
question of libification because it forces us to reconsider all of the data
structures we have and how they're actually used by already existing users.
For the trailer API, those existing users are the "interpret-trailers"
builtin command, and anything else that includes the "trailer.h" header file
(e.g., sequencer.c). One advantage of this idiom is that even the compiler
understands it --- the compiler will loudly complain if you try to access
the innards of a private struct through an opaque pointer.
Another advantage of this idiom is that it helps to reduce the probability
of breaking changes in the API. Because a private struct's members are out
of view from our users (they only know about opaque pointers to the private
struct, not its members), we are free to modify the members of the struct at
any time, as much as we like, as long as we don't break the semantics of the
exposed API functions (which is why unit-testing these API functions will be
crucial long-term).
If this pimpl idiom turns out to be a mistake, undoing it is easy --- just
move the relevant struct definition from foo.c to the header file. So it's a
great way to try things out without digging ourselves into a pit of despair
that will be difficult to get out of.
With the libification-focused goals out of the way, let's turn to this patch
series in more detail.
Currently, we have "process_trailers()" in trailer.h which does many
different things (parse command-line arguments, create temporary files, etc)
that are independent of the concept of "trailers". Keeping this function as
an API function would make unit-testing it difficult. While there is no
technical reason why we couldn't write unit tests for the smaller functions
that are called within process_trailers(), doing so would involve testing
private ("static" in trailer.c) functions instead of API functions, which
defeats the goal of "API stability" mentioned earlier above.
As an alternative to how things are done in this patch series, we could keep
trailer.h intact and decide to unit-test the existing "trailer_info_get()"
function which does most of the trailer parsing work (and is used by
sequencer.c). However this function wouldn't be easy to test either, because
the resulting "trailer_info" struct merely contains the unparsed "trailers"
lines. So the unit test (if it wants to inspect the result of parsing these
lines) would have to invoke additional parsing functions itself. And at that
point it would no longer be a unit test in the traditional sense, because it
would be invoking multiple functions at once.
In summary this series breaks up "process_trailers()" into smaller pieces,
exposing many of the parts relevant to trailer-related processing in
trailer.h. This will force us to eventually introduce unit tests for these
API functions, but that is a good thing for API stability.
In the future after libification is "complete", users external to Git will
be able to use the same trailer processing API used by the
interpret-trailers builtin. For example, a web server may want to parse
trailers the same way that Git would parse them, without having to call
interpret-trailers as a subprocess. This use case was the original
motivation behind my work in this area.
Thanks to the aggressive refactoring in this series, I've been able to
identify and fix several bugs in our existing implementation. Those fixes
build on top of this series but were not included here, in order to keep
this series small. Below is a "shortlog" of those fixes I have locally:
* "trailer: trailer replacement should not change its position" (If we
found a trailer we'd like to replace, preserve its position relative to
the other trailers found in the trailer block, instead of always moving
it to the beginning or end of the entire trailer block.)
* "interpret-trailers: preserve trailers coming from the input" (Sometimes,
the parsed trailers from the input will be formatted differently
depending on whether we provide --only-trailers or not. Make the trailers
that were not modified and which are coming directly from the input get
formatted the same way, regardless of this flag.)
* "interpret-trailers: do not modify the input if NOP" (Refrain from
subtracting or adding a newline around the patch divider "---" if we are
not adding new trailers.)
* "trailer formatter: split up format_trailer() monolith" (Fix a bug in
git-log where we still printed a blank newline even if we didn't want to
format anything.)
* "interpret-trailers: fail if given unrecognized arguments" (E.g., for
"--where", only accept recognized WHERE_* enum values. If we get
something unrecognized, fail with an error instead of silently doing
nothing. Ditto for "--if-exists" and "--if-missing".)
Notable changes in v2
=====================
* Reorder function parameters to have trailer options at the beginning (and
out parameters toward the end)
* "sequencer: use the trailer iterator": prefer C string instead of strbuf
for new "raw" field
* Patch 1 (was Patch 2) also renames ensure_configured() to
trailer_config_init() (forgot to rename this one previously)
Linus Arver (10):
trailer: prepare to expose functions as part of API
trailer: move interpret_trailers() to interpret-trailers.c
trailer: unify trailer formatting machinery
trailer: delete obsolete formatting functions
sequencer: use the trailer iterator
trailer: make trailer_info struct private
trailer: spread usage of "trailer_block" language
trailer: prepare to move parse_trailers_from_command_line_args() to
builtin
trailer: move arg handling to interpret-trailers.c
trailer: delete obsolete argument handling code from API
builtin/interpret-trailers.c | 169 ++++++++--
builtin/shortlog.c | 7 +-
pretty.c | 2 +-
ref-filter.c | 2 +-
sequencer.c | 35 +--
trailer.c | 576 ++++++++++++++++-------------------
trailer.h | 105 ++++---
7 files changed, 478 insertions(+), 418 deletions(-)
base-commit: a54a84b333adbecf7bc4483c0e36ed5878cac17b
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1632%2Flistx%2Ftrailer-api-refactor-part-1-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1632/listx/trailer-api-refactor-part-1-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1632
Range-diff vs v1:
2: 5f64718abfc ! 1: e2d3ed9b5b6 trailer: include "trailer" term in API functions
@@ Metadata
Author: Linus Arver <linusa@google.com>
## Commit message ##
- trailer: include "trailer" term in API functions
+ trailer: prepare to expose functions as part of API
- These functions are exposed to clients and so they should include
- "trailer" in their names for easier identification, just like all the
- other functions already exposed by trailer.h.
+ In the next patch, we will move "process_trailers" from trailer.c to
+ builtin/interpret-trailers.c. That move will necessitate the growth of
+ the trailer.h API, forcing us to expose some additional functions in
+ trailer.h.
+ Rename relevant functions so that they include the term "trailer" in
+ their name, so that clients of the API will be able to easily identify
+ them by their "trailer" moniker, just like all the other functions
+ already exposed by trailer.h.
+
+ The the opportunity to start putting trailer processions options (opts)
+ as the first parameter. This will be the pattern going forward in this
+ series.
+
+ Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Linus Arver <linusa@google.com>
## builtin/interpret-trailers.c ##
-@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
- struct trailer_info info;
- FILE *outfile = stdout;
-
-- ensure_configured();
-+ trailer_config_init();
-
- read_input_file(&sb, file);
-
-@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
- process_trailers_lists(&head, &arg_head);
+@@ builtin/interpret-trailers.c: int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
+ if (argc) {
+ int i;
+ for (i = 0; i < argc; i++)
+- process_trailers(argv[i], &opts, &trailers);
++ interpret_trailers(&opts, &trailers, argv[i]);
+ } else {
+ if (opts.in_place)
+ die(_("no input file given for in-place editing"));
+- process_trailers(NULL, &opts, &trailers);
++ interpret_trailers(&opts, &trailers, NULL);
}
-- print_all(outfile, &head, opts);
-+ /* Print trailer block. */
-+ format_trailers(outfile, &head, opts);
-
-- free_all(&head);
-+ free_trailers(&head);
- trailer_info_release(&info);
-
- /* Print the lines after the trailers as is */
+ new_trailers_clear(&trailers);
## trailer.c ##
@@ trailer.c: static void print_tok_val(FILE *outfile, const char *tok, const char *val)
fprintf(outfile, "%s%c %s\n", tok, separators[0], val);
}
--void print_all(FILE *outfile, struct list_head *head,
-- const struct process_trailer_options *opts)
-+void format_trailers(FILE *outfile, struct list_head *head,
-+ const struct process_trailer_options *opts)
+-static void print_all(FILE *outfile, struct list_head *head,
+- const struct process_trailer_options *opts)
++static void format_trailers(const struct process_trailer_options *opts,
++ struct list_head *trailers, FILE *outfile)
{
struct list_head *pos;
struct trailer_item *item;
+- list_for_each(pos, head) {
++ list_for_each(pos, trailers) {
+ item = list_entry(pos, struct trailer_item, list);
+ if ((!opts->trim_empty || strlen(item->value) > 0) &&
+ (!opts->only_trailers || item->token))
@@ trailer.c: static int git_trailer_config(const char *conf_key, const char *value,
return 0;
}
--void ensure_configured(void)
-+void trailer_config_init(void)
+-static void ensure_configured(void)
++static void trailer_config_init(void)
{
if (configured)
return;
-@@ trailer.c: void parse_trailers(struct trailer_info *info,
+@@ trailer.c: static void parse_trailers(struct trailer_info *info,
}
}
--void free_all(struct list_head *head)
-+void free_trailers(struct list_head *head)
+-static void free_all(struct list_head *head)
++static void free_trailers(struct list_head *trailers)
{
struct list_head *pos, *p;
- list_for_each_safe(pos, p, head) {
+- list_for_each_safe(pos, p, head) {
++ list_for_each_safe(pos, p, trailers) {
+ list_del(pos);
+ free_trailer_item(list_entry(pos, struct trailer_item, list));
+ }
+@@ trailer.c: static FILE *create_in_place_tempfile(const char *file)
+ return outfile;
+ }
+
+-void process_trailers(const char *file,
+- const struct process_trailer_options *opts,
+- struct list_head *new_trailer_head)
++void interpret_trailers(const struct process_trailer_options *opts,
++ struct list_head *new_trailer_head,
++ const char *file)
+ {
+ LIST_HEAD(head);
+ struct strbuf sb = STRBUF_INIT;
+ struct trailer_info info;
+ FILE *outfile = stdout;
+
+- ensure_configured();
++ trailer_config_init();
+
+ read_input_file(&sb, file);
+
+@@ trailer.c: void process_trailers(const char *file,
+ process_trailers_lists(&head, &arg_head);
+ }
+
+- print_all(outfile, &head, opts);
++ format_trailers(opts, &head, outfile);
+
+- free_all(&head);
++ free_trailers(&head);
+ trailer_info_release(&info);
+
+ /* Print the lines after the trailers as is */
@@ trailer.c: void trailer_info_get(struct trailer_info *info, const char *str,
size_t nr = 0, alloc = 0;
char **last = NULL;
@@ trailer.c: void trailer_info_get(struct trailer_info *info, const char *str,
trailer_block_start = find_trailer_block_start(str, end_of_log_message);
## trailer.h ##
-@@ trailer.h: void trailer_info_get(struct trailer_info *info, const char *str,
-
- void trailer_info_release(struct trailer_info *info);
-
--void ensure_configured(void);
--void print_all(FILE *outfile, struct list_head *head,
-- const struct process_trailer_options *opts);
--void free_all(struct list_head *head);
-+void trailer_config_init(void);
-+void free_trailers(struct list_head *trailers);
-
-+void format_trailers(FILE *outfile, struct list_head *head,
-+ const struct process_trailer_options *opts);
- /*
- * Format the trailers from the commit msg "msg" into the strbuf "out".
- * Note two caveats about "opts":
-@@ trailer.h: void free_all(struct list_head *head);
- * - this is primarily a helper for pretty.c, and not
- * all of the flags are supported.
- *
-- * - this differs from process_trailers slightly in that we always format
-+ * - this differs from format_trailers slightly in that we always format
- * only the trailer block itself, even if the "only_trailers" option is not
- * set.
- */
+@@ trailer.h: struct process_trailer_options {
+
+ #define PROCESS_TRAILER_OPTIONS_INIT {0}
+
+-void process_trailers(const char *file,
+- const struct process_trailer_options *opts,
+- struct list_head *new_trailer_head);
++void interpret_trailers(const struct process_trailer_options *opts,
++ struct list_head *new_trailer_head,
++ const char *file);
+
+ void trailer_info_get(struct trailer_info *info, const char *str,
+ const struct process_trailer_options *opts);
1: 2dc3279b37f ! 2: eaca39fd7ea trailer: move process_trailers() to interpret-trailers.c
@@ Metadata
Author: Linus Arver <linusa@google.com>
## Commit message ##
- trailer: move process_trailers() to interpret-trailers.c
+ trailer: move interpret_trailers() to interpret-trailers.c
The interpret-trailers.c builtin is the only place we need to call
- process_trailers(). As it stands, process_trailers() is inherently tied
- to how the builtin behaves, so move its definition there.
+ interpret_trailers(), so move its definition there.
Delete the corresponding declaration from trailer.h, which then forces
us to expose the working innards of that function. This enriches
- trailer.h to include a more granular API, which can then be unit-tested
- in the future (because process_trailers() by itself does too many things
- to be able to be easily unit-tested).
+ trailer.h with a more granular API, which can then be unit-tested in the
+ future (because interpret_trailers() by itself does too many things to
+ be able to be easily unit-tested).
Take this opportunity to demote some file-handling functions out of the
trailer API implementation, as these have nothing to do with trailers.
- While we're at it, rename process_trailers() to interpret_trailers() in
- the builtin for consistency with the existing cmd_interpret_trailers(),
- which wraps around this function.
-
Signed-off-by: Linus Arver <linusa@google.com>
## builtin/interpret-trailers.c ##
@@ builtin/interpret-trailers.c: static int parse_opt_parse(const struct option *op
+ }
+}
+
-+static void interpret_trailers(const char *file,
-+ const struct process_trailer_options *opts,
-+ struct list_head *new_trailer_head)
++static void interpret_trailers(const struct process_trailer_options *opts,
++ struct list_head *new_trailer_head,
++ const char *file)
+{
+ LIST_HEAD(head);
+ struct strbuf sb = STRBUF_INIT;
+ struct trailer_info info;
+ FILE *outfile = stdout;
+
-+ ensure_configured();
++ trailer_config_init();
+
+ read_input_file(&sb, file);
+
+ if (opts->in_place)
+ outfile = create_in_place_tempfile(file);
+
-+ parse_trailers(&info, sb.buf, &head, opts);
++ parse_trailers(opts, &info, sb.buf, &head);
+
+ /* Print the lines before the trailers */
+ if (!opts->only_trailers)
@@ builtin/interpret-trailers.c: static int parse_opt_parse(const struct option *op
+ process_trailers_lists(&head, &arg_head);
+ }
+
-+ print_all(outfile, &head, opts);
++ format_trailers(opts, &head, outfile);
+
-+ free_all(&head);
++ free_trailers(&head);
+ trailer_info_release(&info);
+
+ /* Print the lines after the trailers as is */
@@ builtin/interpret-trailers.c: static int parse_opt_parse(const struct option *op
int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
{
struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
-@@ builtin/interpret-trailers.c: int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
- if (argc) {
- int i;
- for (i = 0; i < argc; i++)
-- process_trailers(argv[i], &opts, &trailers);
-+ interpret_trailers(argv[i], &opts, &trailers);
- } else {
- if (opts.in_place)
- die(_("no input file given for in-place editing"));
-- process_trailers(NULL, &opts, &trailers);
-+ interpret_trailers(NULL, &opts, &trailers);
- }
-
- new_trailers_clear(&trailers);
## trailer.c ##
@@
@@ trailer.c: static void print_tok_val(FILE *outfile, const char *tok, const char
fprintf(outfile, "%s%c %s\n", tok, separators[0], val);
}
--static void print_all(FILE *outfile, struct list_head *head,
-- const struct process_trailer_options *opts)
-+void print_all(FILE *outfile, struct list_head *head,
-+ const struct process_trailer_options *opts)
+-static void format_trailers(const struct process_trailer_options *opts,
+- struct list_head *trailers, FILE *outfile)
++void format_trailers(const struct process_trailer_options *opts,
++ struct list_head *trailers, FILE *outfile)
{
struct list_head *pos;
struct trailer_item *item;
@@ trailer.c: static int git_trailer_config(const char *conf_key, const char *value
return 0;
}
--static void ensure_configured(void)
-+void ensure_configured(void)
+-static void trailer_config_init(void)
++void trailer_config_init(void)
{
if (configured)
return;
@@ trailer.c: static void unfold_value(struct strbuf *val)
- const char *str,
- struct list_head *head,
- const struct process_trailer_options *opts)
-+void parse_trailers(struct trailer_info *info,
++void parse_trailers(const struct process_trailer_options *opts,
++ struct trailer_info *info,
+ const char *str,
-+ struct list_head *head,
-+ const struct process_trailer_options *opts)
++ struct list_head *head)
{
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
@@ trailer.c: static void parse_trailers(struct trailer_info *info,
}
}
--static void free_all(struct list_head *head)
-+void free_all(struct list_head *head)
+-static void free_trailers(struct list_head *trailers)
++void free_trailers(struct list_head *trailers)
{
struct list_head *pos, *p;
- list_for_each_safe(pos, p, head) {
-@@ trailer.c: static void free_all(struct list_head *head)
+ list_for_each_safe(pos, p, trailers) {
+@@ trailer.c: static void free_trailers(struct list_head *trailers)
}
}
@@ trailer.c: static void free_all(struct list_head *head)
- return outfile;
-}
-
--void process_trailers(const char *file,
-- const struct process_trailer_options *opts,
-- struct list_head *new_trailer_head)
+-void interpret_trailers(const struct process_trailer_options *opts,
+- struct list_head *new_trailer_head,
+- const char *file)
-{
- LIST_HEAD(head);
- struct strbuf sb = STRBUF_INIT;
- struct trailer_info info;
- FILE *outfile = stdout;
-
-- ensure_configured();
+- trailer_config_init();
-
- read_input_file(&sb, file);
-
@@ trailer.c: static void free_all(struct list_head *head)
- process_trailers_lists(&head, &arg_head);
- }
-
-- print_all(outfile, &head, opts);
+- format_trailers(opts, &head, outfile);
-
-- free_all(&head);
+- free_trailers(&head);
- trailer_info_release(&info);
-
- /* Print the lines after the trailers as is */
@@ trailer.h: struct process_trailer_options {
#define PROCESS_TRAILER_OPTIONS_INIT {0}
--void process_trailers(const char *file,
-- const struct process_trailer_options *opts,
-- struct list_head *new_trailer_head);
+-void interpret_trailers(const struct process_trailer_options *opts,
+- struct list_head *new_trailer_head,
+- const char *file);
+void parse_trailers_from_config(struct list_head *config_head);
+
+void parse_trailers_from_command_line_args(struct list_head *arg_head,
@@ trailer.h: struct process_trailer_options {
+void process_trailers_lists(struct list_head *head,
+ struct list_head *arg_head);
+
-+void parse_trailers(struct trailer_info *info,
++void parse_trailers(const struct process_trailer_options *opts,
++ struct trailer_info *info,
+ const char *str,
-+ struct list_head *head,
-+ const struct process_trailer_options *opts);
++ struct list_head *head);
void trailer_info_get(struct trailer_info *info, const char *str,
const struct process_trailer_options *opts);
void trailer_info_release(struct trailer_info *info);
-+void ensure_configured(void);
-+void print_all(FILE *outfile, struct list_head *head,
-+ const struct process_trailer_options *opts);
-+void free_all(struct list_head *head);
++void trailer_config_init(void);
++void format_trailers(const struct process_trailer_options *opts,
++ struct list_head *trailers, FILE *outfile);
++void free_trailers(struct list_head *trailers);
+
/*
* Format the trailers from the commit msg "msg" into the strbuf "out".
3: d3326021fb6 ! 3: 9b7747d550e trailer: unify trailer formatting machinery
@@ Commit message
format_trailers() to process trailers with the additional
process_trailer_options fields like opts->key_only which is only used by
format_trailers_from_commit() and not builtin/interpret-trailers.c.
+ While we're at it, reorder parameters to put the trailer processing
+ options first, and the out parameter (strbuf we write into) at the end.
- This will allow us to delete the format_trailer_info() and
+ This unification will allow us to delete the format_trailer_info() and
print_tok_val() functions in the next patch. They are not deleted here
in order to keep the diff small.
+ Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Linus Arver <linusa@google.com>
## builtin/interpret-trailers.c ##
-@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
+@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
{
LIST_HEAD(head);
struct strbuf sb = STRBUF_INIT;
@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
struct trailer_info info;
FILE *outfile = stdout;
-@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
+@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
+ process_trailers_lists(&head, &arg_head);
}
- /* Print trailer block. */
-- format_trailers(outfile, &head, opts);
-+ format_trailers(&head, opts, &trailer_block);
+- format_trailers(opts, &head, outfile);
++ /* Print trailer block. */
++ format_trailers(opts, &head, &trailer_block);
+ fwrite(trailer_block.buf, 1, trailer_block.len, outfile);
+ strbuf_release(&trailer_block);
@@ pretty.c: static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
}
if (*arg == ')') {
- format_trailers_from_commit(sb, msg + c->subject_off, &opts);
-+ format_trailers_from_commit(msg + c->subject_off, &opts, sb);
++ format_trailers_from_commit(&opts, msg + c->subject_off, sb);
ret = arg - placeholder + 1;
}
trailer_out:
@@ ref-filter.c: static void grab_sub_body_contents(struct atom_value *val, int der
/* Format the trailer info according to the trailer_opts given */
- format_trailers_from_commit(&s, subpos, &atom->u.contents.trailer_opts);
-+ format_trailers_from_commit(subpos, &atom->u.contents.trailer_opts, &s);
++ format_trailers_from_commit(&atom->u.contents.trailer_opts, subpos, &s);
v->s = strbuf_detach(&s, NULL);
} else if (atom->u.contents.option == C_BARE)
@@ trailer.c: static void print_tok_val(FILE *outfile, const char *tok, const char
fprintf(outfile, "%s%c %s\n", tok, separators[0], val);
}
--void format_trailers(FILE *outfile, struct list_head *head,
-- const struct process_trailer_options *opts)
+-void format_trailers(const struct process_trailer_options *opts,
+- struct list_head *trailers, FILE *outfile)
-{
- struct list_head *pos;
- struct trailer_item *item;
-- list_for_each(pos, head) {
+- list_for_each(pos, trailers) {
- item = list_entry(pos, struct trailer_item, list);
- if ((!opts->trim_empty || strlen(item->value) > 0) &&
- (!opts->only_trailers || item->token))
@@ trailer.c: static void unfold_value(struct strbuf *val)
strbuf_release(&out);
}
-+void format_trailers(struct list_head *head,
-+ const struct process_trailer_options *opts,
++void format_trailers(const struct process_trailer_options *opts,
++ struct list_head *trailers,
+ struct strbuf *out)
+{
+ struct list_head *pos;
+ struct trailer_item *item;
+ int need_separator = 0;
+
-+ list_for_each(pos, head) {
++ list_for_each(pos, trailers) {
+ item = list_entry(pos, struct trailer_item, list);
+ if (item->token) {
+ char c;
@@ trailer.c: static void format_trailer_info(struct strbuf *out,
-void format_trailers_from_commit(struct strbuf *out, const char *msg,
- const struct process_trailer_options *opts)
-+void format_trailers_from_commit(const char *msg,
-+ const struct process_trailer_options *opts,
++void format_trailers_from_commit(const struct process_trailer_options *opts,
++ const char *msg,
+ struct strbuf *out)
{
+ LIST_HEAD(head);
@@ trailer.c: static void format_trailer_info(struct strbuf *out,
- trailer_info_get(&info, msg, opts);
- format_trailer_info(out, &info, msg, opts);
-+ parse_trailers(&info, msg, &head, opts);
++ parse_trailers(opts, &info, msg, &head);
+
+ /* If we want the whole block untouched, we can take the fast path. */
+ if (!opts->only_trailers && !opts->unfold && !opts->filter &&
@@ trailer.c: static void format_trailer_info(struct strbuf *out,
+ strbuf_add(out, msg + info.trailer_block_start,
+ info.trailer_block_end - info.trailer_block_start);
+ } else
-+ format_trailers(&head, opts, out);
++ format_trailers(opts, &head, out);
+
+ free_trailers(&head);
trailer_info_release(&info);
@@ trailer.c: static void format_trailer_info(struct strbuf *out,
## trailer.h ##
@@ trailer.h: void trailer_info_release(struct trailer_info *info);
+
void trailer_config_init(void);
+ void format_trailers(const struct process_trailer_options *opts,
+- struct list_head *trailers, FILE *outfile);
++ struct list_head *trailers,
++ struct strbuf *out);
void free_trailers(struct list_head *trailers);
--void format_trailers(FILE *outfile, struct list_head *head,
-- const struct process_trailer_options *opts);
-+void format_trailers(struct list_head *head,
-+ const struct process_trailer_options *opts,
-+ struct strbuf *out);
/*
- * Format the trailers from the commit msg "msg" into the strbuf "out".
- * Note two caveats about "opts":
@@ trailer.h: void trailer_info_release(struct trailer_info *info);
- * - this is primarily a helper for pretty.c, and not
- * all of the flags are supported.
- *
-- * - this differs from format_trailers slightly in that we always format
+- * - this differs from process_trailers slightly in that we always format
- * only the trailer block itself, even if the "only_trailers" option is not
- * set.
+ * Convenience function to format the trailers from the commit msg "msg" into
@@ trailer.h: void trailer_info_release(struct trailer_info *info);
*/
-void format_trailers_from_commit(struct strbuf *out, const char *msg,
- const struct process_trailer_options *opts);
-+void format_trailers_from_commit(const char *msg,
-+ const struct process_trailer_options *opts,
++void format_trailers_from_commit(const struct process_trailer_options *opts,
++ const char *msg,
+ struct strbuf *out);
/*
4: 8d864614757 ! 4: f1171f5202f trailer: delete obsolete formatting functions
@@ trailer.c: void trailer_info_release(struct trailer_info *info)
-
-}
-
- void format_trailers_from_commit(const char *msg,
- const struct process_trailer_options *opts,
+ void format_trailers_from_commit(const struct process_trailer_options *opts,
+ const char *msg,
struct strbuf *out)
5: fd4a9d54d95 ! 5: 5ba842b5005 sequencer: use the trailer iterator
@@ sequencer.c: static const char *get_todo_path(const struct replay_opts *opts)
+ i++;
+ if (sob &&
+ iter.is_trailer &&
-+ !strncmp(iter.raw.buf, sob->buf, sob->len)) {
++ !strncmp(iter.raw, sob->buf, sob->len)) {
+ found_sob = i;
+ }
+ }
@@ sequencer.c: static const char *get_todo_path(const struct replay_opts *opts)
## trailer.c ##
@@ trailer.c: void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
- struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
- strbuf_init(&iter->key, 0);
- strbuf_init(&iter->val, 0);
-+ strbuf_init(&iter->raw, 0);
- opts.no_divider = 1;
- trailer_info_get(&iter->internal.info, msg, &opts);
- iter->internal.cur = 0;
-@@ trailer.c: void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
int trailer_iterator_advance(struct trailer_iterator *iter)
{
@@ trailer.c: void trailer_iterator_init(struct trailer_iterator *iter, const char
+ separator_pos = find_separator(line, separators);
+ iter->is_trailer = (separator_pos > 0);
+
-+ strbuf_reset(&iter->raw);
-+ strbuf_addstr(&iter->raw, line);
++ iter->raw = line;
strbuf_reset(&iter->key);
strbuf_reset(&iter->val);
parse_trailer(&iter->key, &iter->val, NULL,
@@ trailer.c: void trailer_iterator_init(struct trailer_iterator *iter, const char
unfold_value(&iter->val);
return 1;
}
-@@ trailer.c: void trailer_iterator_release(struct trailer_iterator *iter)
- trailer_info_release(&iter->internal.info);
- strbuf_release(&iter->val);
- strbuf_release(&iter->key);
-+ strbuf_release(&iter->raw);
- }
## trailer.h ##
@@ trailer.h: struct trailer_iterator {
@@ trailer.h: struct trailer_iterator {
+ /*
+ * Raw line (e.g., "foo: bar baz") before being parsed as a trailer
-+ * key/val pair. This field can contain non-trailer lines because it's
-+ * valid for a trailer block to contain such lines (i.e., we only
-+ * require 25% of the lines in a trailer block to be trailer lines).
++ * 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).
+ */
-+ struct strbuf raw;
++ const char *raw;
+
+ /*
-+ * 1 if the raw line was parsed as a separate key/val pair.
++ * 1 if the raw line was parsed as a trailer line (key/val pair).
+ */
+ int is_trailer;
+
6: 0cbe96421c7 ! 6: f0ac2f6c4b9 trailer: make trailer_info struct private
@@ Commit message
Make this struct private by putting its definition inside trailer.c.
This has two benefits:
- (1) it makes the surface area of the public facing interface (trailer.h)
- smaller, and
+ (1) it makes the surface area of the public facing
+ interface (trailer.h) smaller, and
- (2) external API users are unable to peer inside this struct (because it
- is only ever exposed as an opaque pointer).
+ (2) external API users are unable to peer inside this struct (because
+ it is only ever exposed as an opaque pointer).
This change exposes some deficiencies in the API, mainly with regard to
information about the location of the trailer block that was parsed.
@@ Commit message
Signed-off-by: Linus Arver <linusa@google.com>
## builtin/interpret-trailers.c ##
-@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
+@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
LIST_HEAD(head);
struct strbuf sb = STRBUF_INIT;
struct strbuf trailer_block = STRBUF_INIT;
@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
FILE *outfile = stdout;
trailer_config_init();
-@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
+@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
if (opts->in_place)
outfile = create_in_place_tempfile(file);
-- parse_trailers(&info, sb.buf, &head, opts);
-+ info = parse_trailers(sb.buf, &head, opts);
+- parse_trailers(opts, &info, sb.buf, &head);
++ info = parse_trailers(opts, sb.buf, &head);
/* Print the lines before the trailers */
if (!opts->only_trailers)
@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
fprintf(outfile, "\n");
-@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
+@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
strbuf_release(&trailer_block);
free_trailers(&head);
@@ trailer.c
struct conf_info {
char *name;
char *key;
-@@ trailer.c: void format_trailers(struct list_head *head,
+@@ trailer.c: void format_trailers(const struct process_trailer_options *opts,
}
}
@@ trailer.c: void format_trailers(struct list_head *head,
+ return info;
+}
+
-+static struct trailer_info *trailer_info_get(const char *str,
-+ const struct process_trailer_options *opts)
++static struct trailer_info *trailer_info_get(const struct process_trailer_options *opts,
++ const char *str)
+{
+ struct trailer_info *info = trailer_info_new();
+ size_t end_of_log_message = 0, trailer_block_start = 0;
@@ trailer.c: void format_trailers(struct list_head *head,
* Parse trailers in "str", populating the trailer info and "head"
* linked list structure.
*/
--void parse_trailers(struct trailer_info *info,
+-void parse_trailers(const struct process_trailer_options *opts,
+- struct trailer_info *info,
- const char *str,
-- struct list_head *head,
-- const struct process_trailer_options *opts)
-+struct trailer_info *parse_trailers(const char *str,
-+ struct list_head *head,
-+ const struct process_trailer_options *opts)
+- struct list_head *head)
++struct trailer_info *parse_trailers(const struct process_trailer_options *opts,
++ const char *str,
++ struct list_head *head)
{
+ struct trailer_info *info;
struct strbuf tok = STRBUF_INIT;
@@ trailer.c: void format_trailers(struct list_head *head,
size_t i;
- trailer_info_get(info, str, opts);
-+ info = trailer_info_get(str, opts);
++ info = trailer_info_get(opts, str);
for (i = 0; i < info->trailer_nr; i++) {
int separator_pos;
-@@ trailer.c: void parse_trailers(struct trailer_info *info,
+@@ trailer.c: void parse_trailers(const struct process_trailer_options *opts,
strbuf_detach(&val, NULL));
}
}
@@ trailer.c: void parse_trailers(struct trailer_info *info,
+ return info;
}
- void free_trailers(struct list_head *head)
-@@ trailer.c: void free_trailers(struct list_head *head)
+ void free_trailers(struct list_head *trailers)
+@@ trailer.c: void free_trailers(struct list_head *trailers)
}
}
@@ trailer.c: void trailer_info_release(struct trailer_info *info)
+ free(info);
}
- void format_trailers_from_commit(const char *msg,
-@@ trailer.c: void format_trailers_from_commit(const char *msg,
+ void format_trailers_from_commit(const struct process_trailer_options *opts,
+@@ trailer.c: void format_trailers_from_commit(const struct process_trailer_options *opts,
struct strbuf *out)
{
LIST_HEAD(head);
- struct trailer_info info;
-
-- parse_trailers(&info, msg, &head, opts);
-+ struct trailer_info *info = parse_trailers(msg, &head, opts);
+- parse_trailers(opts, &info, msg, &head);
++ struct trailer_info *info = parse_trailers(opts, msg, &head);
/* If we want the whole block untouched, we can take the fast path. */
if (!opts->only_trailers && !opts->unfold && !opts->filter &&
@@ trailer.c: void format_trailers_from_commit(const char *msg,
+ strbuf_add(out, msg + info->trailer_block_start,
+ info->trailer_block_end - info->trailer_block_start);
} else
- format_trailers(&head, opts, out);
+ format_trailers(opts, &head, out);
free_trailers(&head);
- trailer_info_release(&info);
@@ trailer.c: void format_trailers_from_commit(const char *msg,
+ struct trailer_info *internal = trailer_info_new();
strbuf_init(&iter->key, 0);
strbuf_init(&iter->val, 0);
- strbuf_init(&iter->raw, 0);
opts.no_divider = 1;
- trailer_info_get(&iter->internal.info, msg, &opts);
+ iter->internal.info = internal;
-+ iter->internal.info = trailer_info_get(msg, &opts);
++ iter->internal.info = trailer_info_get(&opts, msg);
iter->internal.cur = 0;
}
@@ trailer.c: int trailer_iterator_advance(struct trailer_iterator *iter)
+ trailer_info_release(iter->internal.info);
strbuf_release(&iter->val);
strbuf_release(&iter->key);
- strbuf_release(&iter->raw);
+ }
## trailer.h ##
@@
@@ trailer.h: void parse_trailers_from_command_line_args(struct list_head *arg_head
void process_trailers_lists(struct list_head *head,
struct list_head *arg_head);
--void parse_trailers(struct trailer_info *info,
+-void parse_trailers(const struct process_trailer_options *opts,
+- struct trailer_info *info,
- const char *str,
-- struct list_head *head,
-- const struct process_trailer_options *opts);
-+struct trailer_info *parse_trailers(const char *str,
-+ struct list_head *head,
-+ const struct process_trailer_options *opts);
+- struct list_head *head);
++struct trailer_info *parse_trailers(const struct process_trailer_options *opts,
++ const char *str,
++ struct list_head *head);
-void trailer_info_get(struct trailer_info *info, const char *str,
- const struct process_trailer_options *opts);
7: 9183990583f ! 7: 291aa83af55 trailer: spread usage of "trailer_block" language
@@ Commit message
Signed-off-by: Linus Arver <linusa@google.com>
## builtin/interpret-trailers.c ##
-@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
+@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
{
LIST_HEAD(head);
struct strbuf sb = STRBUF_INIT;
@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
FILE *outfile = stdout;
trailer_config_init();
-@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
+@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
if (opts->in_place)
outfile = create_in_place_tempfile(file);
-- info = parse_trailers(sb.buf, &head, opts);
-+ trailer_block = parse_trailers(sb.buf, opts, &head);
+- info = parse_trailers(opts, sb.buf, &head);
++ trailer_block = parse_trailers(opts, sb.buf, &head);
- /* Print the lines before the trailers */
+ /* Print the lines before the trailer block */
@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
fprintf(outfile, "\n");
-@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
+@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
}
/* Print trailer block. */
-- format_trailers(&head, opts, &trailer_block);
+- format_trailers(opts, &head, &trailer_block);
- fwrite(trailer_block.buf, 1, trailer_block.len, outfile);
- strbuf_release(&trailer_block);
-+ format_trailers(&head, opts, &tb);
++ format_trailers(opts, &head, &tb);
+ fwrite(tb.buf, 1, tb.len, outfile);
+ strbuf_release(&tb);
@@ trailer.c
/*
* Array of trailers found.
-@@ trailer.c: void format_trailers(struct list_head *head,
+@@ trailer.c: void format_trailers(const struct process_trailer_options *opts,
}
}
@@ trailer.c: void format_trailers(struct list_head *head,
+ return trailer_block;
}
--static struct trailer_info *trailer_info_get(const char *str,
-- const struct process_trailer_options *opts)
-+static struct trailer_block *trailer_block_get(const char *str,
-+ const struct process_trailer_options *opts)
+-static struct trailer_info *trailer_info_get(const struct process_trailer_options *opts,
+- const char *str)
++static struct trailer_block *trailer_block_get(const struct process_trailer_options *opts,
++ const char *str)
{
- struct trailer_info *info = trailer_info_new();
+ struct trailer_block *trailer_block = trailer_block_new();
size_t end_of_log_message = 0, trailer_block_start = 0;
struct strbuf **trailer_lines, **ptr;
char **trailer_strings = NULL;
-@@ trailer.c: static struct trailer_info *trailer_info_get(const char *str,
+@@ trailer.c: static struct trailer_info *trailer_info_get(const struct process_trailer_option
}
strbuf_list_free(trailer_lines);
@@ trailer.c: static struct trailer_info *trailer_info_get(const char *str,
+ * Parse trailers in "str", populating the trailer_block info and "head" linked
+ * list structure.
*/
--struct trailer_info *parse_trailers(const char *str,
-- struct list_head *head,
-- const struct process_trailer_options *opts)
-+struct trailer_block *parse_trailers(const char *str,
-+ const struct process_trailer_options *opts,
+-struct trailer_info *parse_trailers(const struct process_trailer_options *opts,
+- const char *str,
+- struct list_head *head)
++struct trailer_block *parse_trailers(const struct process_trailer_options *opts,
++ const char *str,
+ struct list_head *head)
{
- struct trailer_info *info;
@@ trailer.c: static struct trailer_info *trailer_info_get(const char *str,
struct strbuf val = STRBUF_INIT;
size_t i;
-- info = trailer_info_get(str, opts);
-+ trailer_block = trailer_block_get(str, opts);
+- info = trailer_info_get(opts, str);
++ trailer_block = trailer_block_get(opts, str);
- for (i = 0; i < info->trailer_nr; i++) {
+ for (i = 0; i < trailer_block->trailer_nr; i++) {
@@ trailer.c: static struct trailer_info *trailer_info_get(const char *str,
if (trailer[0] == comment_line_char)
continue;
separator_pos = find_separator(trailer, separators);
-@@ trailer.c: struct trailer_info *parse_trailers(const char *str,
+@@ trailer.c: struct trailer_info *parse_trailers(const struct process_trailer_options *opts,
}
}
@@ trailer.c: struct trailer_info *parse_trailers(const char *str,
+ return trailer_block;
}
- void free_trailers(struct list_head *head)
-@@ trailer.c: void free_trailers(struct list_head *head)
+ void free_trailers(struct list_head *trailers)
+@@ trailer.c: void free_trailers(struct list_head *trailers)
}
}
@@ trailer.c: void free_trailers(struct list_head *head)
+ free(trailer_block);
}
- void format_trailers_from_commit(const char *msg,
-@@ trailer.c: void format_trailers_from_commit(const char *msg,
+ void format_trailers_from_commit(const struct process_trailer_options *opts,
+@@ trailer.c: void format_trailers_from_commit(const struct process_trailer_options *opts,
struct strbuf *out)
{
LIST_HEAD(head);
-- struct trailer_info *info = parse_trailers(msg, &head, opts);
-+ struct trailer_block *trailer_block = parse_trailers(msg, opts, &head);
+- struct trailer_info *info = parse_trailers(opts, msg, &head);
++ struct trailer_block *trailer_block = parse_trailers(opts, msg, &head);
/* If we want the whole block untouched, we can take the fast path. */
if (!opts->only_trailers && !opts->unfold && !opts->filter &&
@@ trailer.c: void format_trailers_from_commit(const char *msg,
+ strbuf_add(out, msg + trailer_block->start,
+ trailer_block->end - trailer_block->start);
} else
- format_trailers(&head, opts, out);
+ format_trailers(opts, &head, out);
free_trailers(&head);
- trailer_info_release(info);
@@ trailer.c: void format_trailers_from_commit(const char *msg,
- struct trailer_info *internal = trailer_info_new();
strbuf_init(&iter->key, 0);
strbuf_init(&iter->val, 0);
- strbuf_init(&iter->raw, 0);
opts.no_divider = 1;
- iter->internal.info = internal;
-- iter->internal.info = trailer_info_get(msg, &opts);
-+ iter->internal.trailer_block = trailer_block_get(msg, &opts);
+- iter->internal.info = trailer_info_get(&opts, msg);
++ iter->internal.trailer_block = trailer_block_get(&opts, msg);
iter->internal.cur = 0;
}
@@ trailer.c: int trailer_iterator_advance(struct trailer_iterator *iter)
+ trailer_block_release(iter->internal.trailer_block);
strbuf_release(&iter->val);
strbuf_release(&iter->key);
- strbuf_release(&iter->raw);
+ }
## trailer.h ##
@@
@@ trailer.h: void parse_trailers_from_command_line_args(struct list_head *arg_head
void process_trailers_lists(struct list_head *head,
struct list_head *arg_head);
--struct trailer_info *parse_trailers(const char *str,
-- struct list_head *head,
-- const struct process_trailer_options *opts);
-+struct trailer_block *parse_trailers(const char *str,
-+ const struct process_trailer_options *opts,
+-struct trailer_info *parse_trailers(const struct process_trailer_options *opts,
+- const char *str,
+- struct list_head *head);
++struct trailer_block *parse_trailers(const struct process_trailer_options *opts,
++ const char *str,
+ struct list_head *head);
-size_t trailer_block_start(struct trailer_info *info);
@@ trailer.h: void parse_trailers_from_command_line_args(struct list_head *arg_head
+void trailer_block_release(struct trailer_block *trailer_block);
void trailer_config_init(void);
- void free_trailers(struct list_head *trailers);
+ void format_trailers(const struct process_trailer_options *opts,
@@ trailer.h: struct trailer_iterator {
/* private */
8: 406725df46a ! 8: 64ee07d0b53 trailer: prepare to move parse_trailers_from_command_line_args() to builtin
@@ Commit message
parse_trailers_from_command_line_args()
to interpret-trailer.c, because the trailer API should not be concerned
- with command line arguments (as it has nothing to do with trailers
- themselves). The interpret-trailers builtin is the only user of the
- above function.
+ with command line arguments (as they have nothing to do with trailers
+ themselves). The interpret-trailers builtin is the only caller of this
+ function.
+ Also rename "conf_info" to "trailer_conf" for readability, dropping the
+ low-value "_info" suffix as we did earlier in this series for
+ "trailer_info" to "trailer_block".
+
+ Helped-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Linus Arver <linusa@google.com>
## trailer.c ##
@@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head
add_arg_item(arg_head,
strbuf_detach(&tok, NULL),
strbuf_detach(&val, NULL),
-@@ trailer.c: struct trailer_block *parse_trailers(const char *str,
+@@ trailer.c: struct trailer_block *parse_trailers(const struct process_trailer_options *opts,
for (i = 0; i < trailer_block->trailer_nr; i++) {
int separator_pos;
@@ trailer.c: struct trailer_block *parse_trailers(const char *str,
add_trailer_item(head,
NULL,
@@ trailer.c: int trailer_iterator_advance(struct trailer_iterator *iter)
- strbuf_addstr(&iter->raw, line);
+ iter->raw = line;
strbuf_reset(&iter->key);
strbuf_reset(&iter->val);
- parse_trailer(&iter->key, &iter->val, NULL,
@@ trailer.h: void parse_trailers_from_command_line_args(struct list_head *arg_head
+ struct strbuf *tok, struct strbuf *val,
+ const struct trailer_conf **conf);
+
- struct trailer_block *parse_trailers(const char *str,
- const struct process_trailer_options *opts,
+ struct trailer_block *parse_trailers(const struct process_trailer_options *opts,
+ const char *str,
struct list_head *head);
9: 8a99d0fca21 ! 9: 1b4bdde65bc trailer: move arg handling to interpret-trailers.c
@@ builtin/interpret-trailers.c: static int option_parse_trailer(const struct optio
}
@@ builtin/interpret-trailers.c: static void read_input_file(struct strbuf *sb, const char *file)
+ }
- static void interpret_trailers(const char *file,
- const struct process_trailer_options *opts,
-- struct list_head *new_trailer_head)
-+ struct list_head *arg_trailers)
+ static void interpret_trailers(const struct process_trailer_options *opts,
+- struct list_head *new_trailer_head,
++ struct list_head *arg_trailers,
+ const char *file)
{
LIST_HEAD(head);
- struct strbuf sb = STRBUF_INIT;
-@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
+@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
struct trailer_block *trailer_block;
FILE *outfile = stdout;
@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
read_input_file(&sb, file);
if (opts->in_place)
-@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
+@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
if (!opts->only_input) {
@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
}
/* Print trailer block. */
-@@ builtin/interpret-trailers.c: static void interpret_trailers(const char *file,
+@@ builtin/interpret-trailers.c: static void interpret_trailers(const struct process_trailer_options *opts,
int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
{
struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
@@ builtin/interpret-trailers.c: int cmd_interpret_trailers(int argc, const char **
if (argc) {
int i;
for (i = 0; i < argc; i++)
-- interpret_trailers(argv[i], &opts, &trailers);
-+ interpret_trailers(argv[i], &opts, &arg_trailers);
+- interpret_trailers(&opts, &trailers, argv[i]);
++ interpret_trailers(&opts, &arg_trailers, argv[i]);
} else {
if (opts.in_place)
die(_("no input file given for in-place editing"));
-- interpret_trailers(NULL, &opts, &trailers);
-+ interpret_trailers(NULL, &opts, &arg_trailers);
+- interpret_trailers(&opts, &trailers, NULL);
++ interpret_trailers(&opts, &arg_trailers, NULL);
}
- new_trailers_clear(&trailers);
@@ trailer.c: void parse_trailers_from_command_line_args(struct list_head *arg_head
}
}
-@@ trailer.c: void free_trailers(struct list_head *head)
+@@ trailer.c: void free_trailers(struct list_head *trailers)
}
}
@@ trailer.h: struct new_trailer_item {
struct process_trailer_options {
int in_place;
int trim_empty;
-@@ trailer.h: void trailer_block_release(struct trailer_block *trailer_block);
-
- void trailer_config_init(void);
+@@ trailer.h: void format_trailers(const struct process_trailer_options *opts,
+ struct list_head *trailers,
+ struct strbuf *out);
void free_trailers(struct list_head *trailers);
-+void new_trailers_clear(struct list_head *trailers);
-
- void format_trailers(struct list_head *head,
- const struct process_trailer_options *opts,
+-
++void new_trailers_clear(struct list_head *new_trailers);
+ /*
+ * Convenience function to format the trailers from the commit msg "msg" into
+ * the strbuf "out". Reuses format_trailers internally.
10: 243eee730e4 = 10: ed67ebf8647 trailer: delete obsolete argument handling code from API
--
gitgitgadget
^ permalink raw reply
* [PATCH v2 01/10] trailer: prepare to expose functions as part of API
From: Linus Arver via GitGitGadget @ 2024-01-26 22:38 UTC (permalink / raw)
To: git
Cc: Christian Couder, Junio C Hamano, Emily Shaffer, Josh Steadmon,
Randall S. Becker, Linus Arver, Linus Arver
In-Reply-To: <pull.1632.v2.git.1706308737.gitgitgadget@gmail.com>
From: Linus Arver <linusa@google.com>
In the next patch, we will move "process_trailers" from trailer.c to
builtin/interpret-trailers.c. That move will necessitate the growth of
the trailer.h API, forcing us to expose some additional functions in
trailer.h.
Rename relevant functions so that they include the term "trailer" in
their name, so that clients of the API will be able to easily identify
them by their "trailer" moniker, just like all the other functions
already exposed by trailer.h.
The the opportunity to start putting trailer processions options (opts)
as the first parameter. This will be the pattern going forward in this
series.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Linus Arver <linusa@google.com>
---
builtin/interpret-trailers.c | 4 ++--
trailer.c | 26 +++++++++++++-------------
trailer.h | 6 +++---
3 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 033bd1556cf..85a3413baf5 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -132,11 +132,11 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
if (argc) {
int i;
for (i = 0; i < argc; i++)
- process_trailers(argv[i], &opts, &trailers);
+ interpret_trailers(&opts, &trailers, argv[i]);
} else {
if (opts.in_place)
die(_("no input file given for in-place editing"));
- process_trailers(NULL, &opts, &trailers);
+ interpret_trailers(&opts, &trailers, NULL);
}
new_trailers_clear(&trailers);
diff --git a/trailer.c b/trailer.c
index 3a0710a4583..66b6660f5a4 100644
--- a/trailer.c
+++ b/trailer.c
@@ -163,12 +163,12 @@ static void print_tok_val(FILE *outfile, const char *tok, const char *val)
fprintf(outfile, "%s%c %s\n", tok, separators[0], val);
}
-static void print_all(FILE *outfile, struct list_head *head,
- const struct process_trailer_options *opts)
+static void format_trailers(const struct process_trailer_options *opts,
+ struct list_head *trailers, FILE *outfile)
{
struct list_head *pos;
struct trailer_item *item;
- list_for_each(pos, head) {
+ list_for_each(pos, trailers) {
item = list_entry(pos, struct trailer_item, list);
if ((!opts->trim_empty || strlen(item->value) > 0) &&
(!opts->only_trailers || item->token))
@@ -589,7 +589,7 @@ static int git_trailer_config(const char *conf_key, const char *value,
return 0;
}
-static void ensure_configured(void)
+static void trailer_config_init(void)
{
if (configured)
return;
@@ -1035,10 +1035,10 @@ static void parse_trailers(struct trailer_info *info,
}
}
-static void free_all(struct list_head *head)
+static void free_trailers(struct list_head *trailers)
{
struct list_head *pos, *p;
- list_for_each_safe(pos, p, head) {
+ list_for_each_safe(pos, p, trailers) {
list_del(pos);
free_trailer_item(list_entry(pos, struct trailer_item, list));
}
@@ -1075,16 +1075,16 @@ static FILE *create_in_place_tempfile(const char *file)
return outfile;
}
-void process_trailers(const char *file,
- const struct process_trailer_options *opts,
- struct list_head *new_trailer_head)
+void interpret_trailers(const struct process_trailer_options *opts,
+ struct list_head *new_trailer_head,
+ const char *file)
{
LIST_HEAD(head);
struct strbuf sb = STRBUF_INIT;
struct trailer_info info;
FILE *outfile = stdout;
- ensure_configured();
+ trailer_config_init();
read_input_file(&sb, file);
@@ -1110,9 +1110,9 @@ void process_trailers(const char *file,
process_trailers_lists(&head, &arg_head);
}
- print_all(outfile, &head, opts);
+ format_trailers(opts, &head, outfile);
- free_all(&head);
+ free_trailers(&head);
trailer_info_release(&info);
/* Print the lines after the trailers as is */
@@ -1135,7 +1135,7 @@ void trailer_info_get(struct trailer_info *info, const char *str,
size_t nr = 0, alloc = 0;
char **last = NULL;
- ensure_configured();
+ trailer_config_init();
end_of_log_message = find_end_of_log_message(str, opts->no_divider);
trailer_block_start = find_trailer_block_start(str, end_of_log_message);
diff --git a/trailer.h b/trailer.h
index 1644cd05f60..37033e631a1 100644
--- a/trailer.h
+++ b/trailer.h
@@ -81,9 +81,9 @@ struct process_trailer_options {
#define PROCESS_TRAILER_OPTIONS_INIT {0}
-void process_trailers(const char *file,
- const struct process_trailer_options *opts,
- struct list_head *new_trailer_head);
+void interpret_trailers(const struct process_trailer_options *opts,
+ struct list_head *new_trailer_head,
+ const char *file);
void trailer_info_get(struct trailer_info *info, const char *str,
const struct process_trailer_options *opts);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 02/10] trailer: move interpret_trailers() to interpret-trailers.c
From: Linus Arver via GitGitGadget @ 2024-01-26 22:38 UTC (permalink / raw)
To: git
Cc: Christian Couder, Junio C Hamano, Emily Shaffer, Josh Steadmon,
Randall S. Becker, Linus Arver, Linus Arver
In-Reply-To: <pull.1632.v2.git.1706308737.gitgitgadget@gmail.com>
From: Linus Arver <linusa@google.com>
The interpret-trailers.c builtin is the only place we need to call
interpret_trailers(), so move its definition there.
Delete the corresponding declaration from trailer.h, which then forces
us to expose the working innards of that function. This enriches
trailer.h with a more granular API, which can then be unit-tested in the
future (because interpret_trailers() by itself does too many things to
be able to be easily unit-tested).
Take this opportunity to demote some file-handling functions out of the
trailer API implementation, as these have nothing to do with trailers.
Signed-off-by: Linus Arver <linusa@google.com>
---
builtin/interpret-trailers.c | 94 +++++++++++++++++++++++++++
trailer.c | 120 ++++-------------------------------
trailer.h | 20 +++++-
3 files changed, 124 insertions(+), 110 deletions(-)
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 85a3413baf5..8556acde4aa 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -9,6 +9,7 @@
#include "gettext.h"
#include "parse-options.h"
#include "string-list.h"
+#include "tempfile.h"
#include "trailer.h"
#include "config.h"
@@ -91,6 +92,99 @@ static int parse_opt_parse(const struct option *opt, const char *arg,
return 0;
}
+static struct tempfile *trailers_tempfile;
+
+static FILE *create_in_place_tempfile(const char *file)
+{
+ struct stat st;
+ struct strbuf filename_template = STRBUF_INIT;
+ const char *tail;
+ FILE *outfile;
+
+ if (stat(file, &st))
+ die_errno(_("could not stat %s"), file);
+ if (!S_ISREG(st.st_mode))
+ die(_("file %s is not a regular file"), file);
+ if (!(st.st_mode & S_IWUSR))
+ die(_("file %s is not writable by user"), file);
+
+ /* Create temporary file in the same directory as the original */
+ tail = strrchr(file, '/');
+ if (tail)
+ strbuf_add(&filename_template, file, tail - file + 1);
+ strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
+
+ trailers_tempfile = xmks_tempfile_m(filename_template.buf, st.st_mode);
+ strbuf_release(&filename_template);
+ outfile = fdopen_tempfile(trailers_tempfile, "w");
+ if (!outfile)
+ die_errno(_("could not open temporary file"));
+
+ return outfile;
+}
+
+static void read_input_file(struct strbuf *sb, const char *file)
+{
+ if (file) {
+ if (strbuf_read_file(sb, file, 0) < 0)
+ die_errno(_("could not read input file '%s'"), file);
+ } else {
+ if (strbuf_read(sb, fileno(stdin), 0) < 0)
+ die_errno(_("could not read from stdin"));
+ }
+}
+
+static void interpret_trailers(const struct process_trailer_options *opts,
+ struct list_head *new_trailer_head,
+ const char *file)
+{
+ LIST_HEAD(head);
+ struct strbuf sb = STRBUF_INIT;
+ struct trailer_info info;
+ FILE *outfile = stdout;
+
+ trailer_config_init();
+
+ read_input_file(&sb, file);
+
+ if (opts->in_place)
+ outfile = create_in_place_tempfile(file);
+
+ parse_trailers(opts, &info, sb.buf, &head);
+
+ /* Print the lines before the trailers */
+ if (!opts->only_trailers)
+ fwrite(sb.buf, 1, info.trailer_block_start, outfile);
+
+ if (!opts->only_trailers && !info.blank_line_before_trailer)
+ fprintf(outfile, "\n");
+
+
+ if (!opts->only_input) {
+ LIST_HEAD(config_head);
+ LIST_HEAD(arg_head);
+ parse_trailers_from_config(&config_head);
+ parse_trailers_from_command_line_args(&arg_head, new_trailer_head);
+ list_splice(&config_head, &arg_head);
+ process_trailers_lists(&head, &arg_head);
+ }
+
+ format_trailers(opts, &head, outfile);
+
+ free_trailers(&head);
+ trailer_info_release(&info);
+
+ /* Print the lines after the trailers as is */
+ if (!opts->only_trailers)
+ fwrite(sb.buf + info.trailer_block_end, 1, sb.len - info.trailer_block_end, outfile);
+
+ if (opts->in_place)
+ if (rename_tempfile(&trailers_tempfile, file))
+ die_errno(_("could not rename temporary file to %s"), file);
+
+ strbuf_release(&sb);
+}
+
int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
{
struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
diff --git a/trailer.c b/trailer.c
index 66b6660f5a4..d3899195876 100644
--- a/trailer.c
+++ b/trailer.c
@@ -5,7 +5,6 @@
#include "string-list.h"
#include "run-command.h"
#include "commit.h"
-#include "tempfile.h"
#include "trailer.h"
#include "list.h"
/*
@@ -163,8 +162,8 @@ static void print_tok_val(FILE *outfile, const char *tok, const char *val)
fprintf(outfile, "%s%c %s\n", tok, separators[0], val);
}
-static void format_trailers(const struct process_trailer_options *opts,
- struct list_head *trailers, FILE *outfile)
+void format_trailers(const struct process_trailer_options *opts,
+ struct list_head *trailers, FILE *outfile)
{
struct list_head *pos;
struct trailer_item *item;
@@ -366,8 +365,8 @@ static int find_same_and_apply_arg(struct list_head *head,
return 0;
}
-static void process_trailers_lists(struct list_head *head,
- struct list_head *arg_head)
+void process_trailers_lists(struct list_head *head,
+ struct list_head *arg_head)
{
struct list_head *pos, *p;
struct arg_item *arg_tok;
@@ -589,7 +588,7 @@ static int git_trailer_config(const char *conf_key, const char *value,
return 0;
}
-static void trailer_config_init(void)
+void trailer_config_init(void)
{
if (configured)
return;
@@ -719,7 +718,7 @@ static void add_arg_item(struct list_head *arg_head, char *tok, char *val,
list_add_tail(&new_item->list, arg_head);
}
-static void parse_trailers_from_config(struct list_head *config_head)
+void parse_trailers_from_config(struct list_head *config_head)
{
struct arg_item *item;
struct list_head *pos;
@@ -735,8 +734,8 @@ static void parse_trailers_from_config(struct list_head *config_head)
}
}
-static void parse_trailers_from_command_line_args(struct list_head *arg_head,
- struct list_head *new_trailer_head)
+void parse_trailers_from_command_line_args(struct list_head *arg_head,
+ struct list_head *new_trailer_head)
{
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
@@ -775,17 +774,6 @@ static void parse_trailers_from_command_line_args(struct list_head *arg_head,
free(cl_separators);
}
-static void read_input_file(struct strbuf *sb, const char *file)
-{
- if (file) {
- if (strbuf_read_file(sb, file, 0) < 0)
- die_errno(_("could not read input file '%s'"), file);
- } else {
- if (strbuf_read(sb, fileno(stdin), 0) < 0)
- die_errno(_("could not read from stdin"));
- }
-}
-
static const char *next_line(const char *str)
{
const char *nl = strchrnul(str, '\n');
@@ -1000,10 +988,10 @@ static void unfold_value(struct strbuf *val)
* Parse trailers in "str", populating the trailer info and "head"
* linked list structure.
*/
-static void parse_trailers(struct trailer_info *info,
- const char *str,
- struct list_head *head,
- const struct process_trailer_options *opts)
+void parse_trailers(const struct process_trailer_options *opts,
+ struct trailer_info *info,
+ const char *str,
+ struct list_head *head)
{
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
@@ -1035,7 +1023,7 @@ static void parse_trailers(struct trailer_info *info,
}
}
-static void free_trailers(struct list_head *trailers)
+void free_trailers(struct list_head *trailers)
{
struct list_head *pos, *p;
list_for_each_safe(pos, p, trailers) {
@@ -1044,88 +1032,6 @@ static void free_trailers(struct list_head *trailers)
}
}
-static struct tempfile *trailers_tempfile;
-
-static FILE *create_in_place_tempfile(const char *file)
-{
- struct stat st;
- struct strbuf filename_template = STRBUF_INIT;
- const char *tail;
- FILE *outfile;
-
- if (stat(file, &st))
- die_errno(_("could not stat %s"), file);
- if (!S_ISREG(st.st_mode))
- die(_("file %s is not a regular file"), file);
- if (!(st.st_mode & S_IWUSR))
- die(_("file %s is not writable by user"), file);
-
- /* Create temporary file in the same directory as the original */
- tail = strrchr(file, '/');
- if (tail)
- strbuf_add(&filename_template, file, tail - file + 1);
- strbuf_addstr(&filename_template, "git-interpret-trailers-XXXXXX");
-
- trailers_tempfile = xmks_tempfile_m(filename_template.buf, st.st_mode);
- strbuf_release(&filename_template);
- outfile = fdopen_tempfile(trailers_tempfile, "w");
- if (!outfile)
- die_errno(_("could not open temporary file"));
-
- return outfile;
-}
-
-void interpret_trailers(const struct process_trailer_options *opts,
- struct list_head *new_trailer_head,
- const char *file)
-{
- LIST_HEAD(head);
- struct strbuf sb = STRBUF_INIT;
- struct trailer_info info;
- FILE *outfile = stdout;
-
- trailer_config_init();
-
- read_input_file(&sb, file);
-
- if (opts->in_place)
- outfile = create_in_place_tempfile(file);
-
- parse_trailers(&info, sb.buf, &head, opts);
-
- /* Print the lines before the trailers */
- if (!opts->only_trailers)
- fwrite(sb.buf, 1, info.trailer_block_start, outfile);
-
- if (!opts->only_trailers && !info.blank_line_before_trailer)
- fprintf(outfile, "\n");
-
-
- if (!opts->only_input) {
- LIST_HEAD(config_head);
- LIST_HEAD(arg_head);
- parse_trailers_from_config(&config_head);
- parse_trailers_from_command_line_args(&arg_head, new_trailer_head);
- list_splice(&config_head, &arg_head);
- process_trailers_lists(&head, &arg_head);
- }
-
- format_trailers(opts, &head, outfile);
-
- free_trailers(&head);
- trailer_info_release(&info);
-
- /* Print the lines after the trailers as is */
- if (!opts->only_trailers)
- fwrite(sb.buf + info.trailer_block_end, 1, sb.len - info.trailer_block_end, outfile);
-
- if (opts->in_place)
- if (rename_tempfile(&trailers_tempfile, file))
- die_errno(_("could not rename temporary file to %s"), file);
-
- strbuf_release(&sb);
-}
-
void trailer_info_get(struct trailer_info *info, const char *str,
const struct process_trailer_options *opts)
{
diff --git a/trailer.h b/trailer.h
index 37033e631a1..4f603e03ceb 100644
--- a/trailer.h
+++ b/trailer.h
@@ -81,15 +81,29 @@ struct process_trailer_options {
#define PROCESS_TRAILER_OPTIONS_INIT {0}
-void interpret_trailers(const struct process_trailer_options *opts,
- struct list_head *new_trailer_head,
- const char *file);
+void parse_trailers_from_config(struct list_head *config_head);
+
+void parse_trailers_from_command_line_args(struct list_head *arg_head,
+ struct list_head *new_trailer_head);
+
+void process_trailers_lists(struct list_head *head,
+ struct list_head *arg_head);
+
+void parse_trailers(const struct process_trailer_options *opts,
+ struct trailer_info *info,
+ const char *str,
+ struct list_head *head);
void trailer_info_get(struct trailer_info *info, const char *str,
const struct process_trailer_options *opts);
void trailer_info_release(struct trailer_info *info);
+void trailer_config_init(void);
+void format_trailers(const struct process_trailer_options *opts,
+ struct list_head *trailers, FILE *outfile);
+void free_trailers(struct list_head *trailers);
+
/*
* Format the trailers from the commit msg "msg" into the strbuf "out".
* Note two caveats about "opts":
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 03/10] trailer: unify trailer formatting machinery
From: Linus Arver via GitGitGadget @ 2024-01-26 22:38 UTC (permalink / raw)
To: git
Cc: Christian Couder, Junio C Hamano, Emily Shaffer, Josh Steadmon,
Randall S. Becker, Linus Arver, Linus Arver
In-Reply-To: <pull.1632.v2.git.1706308737.gitgitgadget@gmail.com>
From: Linus Arver <linusa@google.com>
Currently have two functions for formatting trailers exposed in
trailer.h:
void format_trailers(FILE *outfile, struct list_head *head,
const struct process_trailer_options *opts);
void format_trailers_from_commit(struct strbuf *out, const char *msg,
const struct process_trailer_options *opts);
and previously these functions, although similar enough (even taking the
same process_trailer_options struct pointer), did not build on each
other.
Make format_trailers_from_commit() rely on format_trailers(). Teach
format_trailers() to process trailers with the additional
process_trailer_options fields like opts->key_only which is only used by
format_trailers_from_commit() and not builtin/interpret-trailers.c.
While we're at it, reorder parameters to put the trailer processing
options first, and the out parameter (strbuf we write into) at the end.
This unification will allow us to delete the format_trailer_info() and
print_tok_val() functions in the next patch. They are not deleted here
in order to keep the diff small.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Linus Arver <linusa@google.com>
---
builtin/interpret-trailers.c | 6 +-
pretty.c | 2 +-
ref-filter.c | 2 +-
trailer.c | 105 +++++++++++++++++++++++++++++------
trailer.h | 19 +++----
5 files changed, 102 insertions(+), 32 deletions(-)
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 8556acde4aa..5352ee65bd1 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -140,6 +140,7 @@ static void interpret_trailers(const struct process_trailer_options *opts,
{
LIST_HEAD(head);
struct strbuf sb = STRBUF_INIT;
+ struct strbuf trailer_block = STRBUF_INIT;
struct trailer_info info;
FILE *outfile = stdout;
@@ -169,7 +170,10 @@ static void interpret_trailers(const struct process_trailer_options *opts,
process_trailers_lists(&head, &arg_head);
}
- format_trailers(opts, &head, outfile);
+ /* Print trailer block. */
+ format_trailers(opts, &head, &trailer_block);
+ fwrite(trailer_block.buf, 1, trailer_block.len, outfile);
+ strbuf_release(&trailer_block);
free_trailers(&head);
trailer_info_release(&info);
diff --git a/pretty.c b/pretty.c
index cf964b060cd..bdbed4295aa 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1759,7 +1759,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
goto trailer_out;
}
if (*arg == ')') {
- format_trailers_from_commit(sb, msg + c->subject_off, &opts);
+ format_trailers_from_commit(&opts, msg + c->subject_off, sb);
ret = arg - placeholder + 1;
}
trailer_out:
diff --git a/ref-filter.c b/ref-filter.c
index 35b989e1dfe..d358953b0ce 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1985,7 +1985,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp
struct strbuf s = STRBUF_INIT;
/* Format the trailer info according to the trailer_opts given */
- format_trailers_from_commit(&s, subpos, &atom->u.contents.trailer_opts);
+ format_trailers_from_commit(&atom->u.contents.trailer_opts, subpos, &s);
v->s = strbuf_detach(&s, NULL);
} else if (atom->u.contents.option == C_BARE)
diff --git a/trailer.c b/trailer.c
index d3899195876..7692bf9eb40 100644
--- a/trailer.c
+++ b/trailer.c
@@ -162,19 +162,6 @@ static void print_tok_val(FILE *outfile, const char *tok, const char *val)
fprintf(outfile, "%s%c %s\n", tok, separators[0], val);
}
-void format_trailers(const struct process_trailer_options *opts,
- struct list_head *trailers, FILE *outfile)
-{
- struct list_head *pos;
- struct trailer_item *item;
- list_for_each(pos, trailers) {
- item = list_entry(pos, struct trailer_item, list);
- if ((!opts->trim_empty || strlen(item->value) > 0) &&
- (!opts->only_trailers || item->token))
- print_tok_val(outfile, item->token, item->value);
- }
-}
-
static struct trailer_item *trailer_from_arg(struct arg_item *arg_tok)
{
struct trailer_item *new_item = xcalloc(1, sizeof(*new_item));
@@ -984,6 +971,78 @@ static void unfold_value(struct strbuf *val)
strbuf_release(&out);
}
+void format_trailers(const struct process_trailer_options *opts,
+ struct list_head *trailers,
+ struct strbuf *out)
+{
+ struct list_head *pos;
+ struct trailer_item *item;
+ int need_separator = 0;
+
+ list_for_each(pos, trailers) {
+ item = list_entry(pos, struct trailer_item, list);
+ if (item->token) {
+ char c;
+
+ struct strbuf tok = STRBUF_INIT;
+ struct strbuf val = STRBUF_INIT;
+ strbuf_addstr(&tok, item->token);
+ strbuf_addstr(&val, item->value);
+
+ /*
+ * Skip key/value pairs where the value was empty. This
+ * can happen from trailers specified without a
+ * separator, like `--trailer "Reviewed-by"` (no
+ * corresponding value).
+ */
+ if (opts->trim_empty && !strlen(item->value))
+ continue;
+
+ if (!opts->filter || opts->filter(&tok, opts->filter_data)) {
+ if (opts->unfold)
+ unfold_value(&val);
+
+ if (opts->separator && need_separator)
+ strbuf_addbuf(out, opts->separator);
+ if (!opts->value_only)
+ strbuf_addbuf(out, &tok);
+ if (!opts->key_only && !opts->value_only) {
+ if (opts->key_value_separator)
+ strbuf_addbuf(out, opts->key_value_separator);
+ else {
+ c = last_non_space_char(tok.buf);
+ if (c) {
+ if (!strchr(separators, c))
+ strbuf_addf(out, "%c ", separators[0]);
+ }
+ }
+ }
+ if (!opts->key_only)
+ strbuf_addbuf(out, &val);
+ if (!opts->separator)
+ strbuf_addch(out, '\n');
+
+ need_separator = 1;
+ }
+
+ strbuf_release(&tok);
+ strbuf_release(&val);
+ } else if (!opts->only_trailers) {
+ if (opts->separator && need_separator) {
+ strbuf_addbuf(out, opts->separator);
+ }
+ strbuf_addstr(out, item->value);
+ if (opts->separator)
+ strbuf_rtrim(out);
+ else
+ strbuf_addch(out, '\n');
+
+ need_separator = 1;
+ }
+
+ }
+}
+
/*
* Parse trailers in "str", populating the trailer info and "head"
* linked list structure.
@@ -1144,13 +1203,25 @@ static void format_trailer_info(struct strbuf *out,
}
-void format_trailers_from_commit(struct strbuf *out, const char *msg,
- const struct process_trailer_options *opts)
+void format_trailers_from_commit(const struct process_trailer_options *opts,
+ const char *msg,
+ struct strbuf *out)
{
+ LIST_HEAD(head);
struct trailer_info info;
- trailer_info_get(&info, msg, opts);
- format_trailer_info(out, &info, msg, opts);
+ parse_trailers(opts, &info, msg, &head);
+
+ /* If we want the whole block untouched, we can take the fast path. */
+ if (!opts->only_trailers && !opts->unfold && !opts->filter &&
+ !opts->separator && !opts->key_only && !opts->value_only &&
+ !opts->key_value_separator) {
+ strbuf_add(out, msg + info.trailer_block_start,
+ info.trailer_block_end - info.trailer_block_start);
+ } else
+ format_trailers(opts, &head, out);
+
+ free_trailers(&head);
trailer_info_release(&info);
}
diff --git a/trailer.h b/trailer.h
index 4f603e03ceb..c309b01323d 100644
--- a/trailer.h
+++ b/trailer.h
@@ -101,22 +101,17 @@ void trailer_info_release(struct trailer_info *info);
void trailer_config_init(void);
void format_trailers(const struct process_trailer_options *opts,
- struct list_head *trailers, FILE *outfile);
+ struct list_head *trailers,
+ struct strbuf *out);
void free_trailers(struct list_head *trailers);
/*
- * Format the trailers from the commit msg "msg" into the strbuf "out".
- * Note two caveats about "opts":
- *
- * - this is primarily a helper for pretty.c, and not
- * all of the flags are supported.
- *
- * - this differs from process_trailers slightly in that we always format
- * only the trailer block itself, even if the "only_trailers" option is not
- * set.
+ * Convenience function to format the trailers from the commit msg "msg" into
+ * the strbuf "out". Reuses format_trailers internally.
*/
-void format_trailers_from_commit(struct strbuf *out, const char *msg,
- const struct process_trailer_options *opts);
+void format_trailers_from_commit(const struct process_trailer_options *opts,
+ const char *msg,
+ struct strbuf *out);
/*
* An interface for iterating over the trailers found in a particular commit
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 04/10] trailer: delete obsolete formatting functions
From: Linus Arver via GitGitGadget @ 2024-01-26 22:38 UTC (permalink / raw)
To: git
Cc: Christian Couder, Junio C Hamano, Emily Shaffer, Josh Steadmon,
Randall S. Becker, Linus Arver, Linus Arver
In-Reply-To: <pull.1632.v2.git.1706308737.gitgitgadget@gmail.com>
From: Linus Arver <linusa@google.com>
Signed-off-by: Linus Arver <linusa@google.com>
---
trailer.c | 79 -------------------------------------------------------
1 file changed, 79 deletions(-)
diff --git a/trailer.c b/trailer.c
index 7692bf9eb40..71ea2bb67f8 100644
--- a/trailer.c
+++ b/trailer.c
@@ -144,24 +144,6 @@ static char last_non_space_char(const char *s)
return '\0';
}
-static void print_tok_val(FILE *outfile, const char *tok, const char *val)
-{
- char c;
-
- if (!tok) {
- fprintf(outfile, "%s\n", val);
- return;
- }
-
- c = last_non_space_char(tok);
- if (!c)
- return;
- if (strchr(separators, c))
- fprintf(outfile, "%s%s\n", tok, val);
- else
- fprintf(outfile, "%s%c %s\n", tok, separators[0], val);
-}
-
static struct trailer_item *trailer_from_arg(struct arg_item *arg_tok)
{
struct trailer_item *new_item = xcalloc(1, sizeof(*new_item));
@@ -1142,67 +1124,6 @@ void trailer_info_release(struct trailer_info *info)
free(info->trailers);
}
-static void format_trailer_info(struct strbuf *out,
- const struct trailer_info *info,
- const char *msg,
- const struct process_trailer_options *opts)
-{
- size_t origlen = out->len;
- size_t i;
-
- /* If we want the whole block untouched, we can take the fast path. */
- if (!opts->only_trailers && !opts->unfold && !opts->filter &&
- !opts->separator && !opts->key_only && !opts->value_only &&
- !opts->key_value_separator) {
- strbuf_add(out, msg + info->trailer_block_start,
- info->trailer_block_end - info->trailer_block_start);
- return;
- }
-
- for (i = 0; i < info->trailer_nr; i++) {
- char *trailer = info->trailers[i];
- ssize_t separator_pos = find_separator(trailer, separators);
-
- if (separator_pos >= 1) {
- struct strbuf tok = STRBUF_INIT;
- struct strbuf val = STRBUF_INIT;
-
- parse_trailer(&tok, &val, NULL, trailer, separator_pos);
- if (!opts->filter || opts->filter(&tok, opts->filter_data)) {
- if (opts->unfold)
- unfold_value(&val);
-
- if (opts->separator && out->len != origlen)
- strbuf_addbuf(out, opts->separator);
- if (!opts->value_only)
- strbuf_addbuf(out, &tok);
- if (!opts->key_only && !opts->value_only) {
- if (opts->key_value_separator)
- strbuf_addbuf(out, opts->key_value_separator);
- else
- strbuf_addstr(out, ": ");
- }
- if (!opts->key_only)
- strbuf_addbuf(out, &val);
- if (!opts->separator)
- strbuf_addch(out, '\n');
- }
- strbuf_release(&tok);
- strbuf_release(&val);
-
- } else if (!opts->only_trailers) {
- if (opts->separator && out->len != origlen) {
- strbuf_addbuf(out, opts->separator);
- }
- strbuf_addstr(out, trailer);
- if (opts->separator) {
- strbuf_rtrim(out);
- }
- }
- }
-
-}
-
void format_trailers_from_commit(const struct process_trailer_options *opts,
const char *msg,
struct strbuf *out)
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 05/10] sequencer: use the trailer iterator
From: Linus Arver via GitGitGadget @ 2024-01-26 22:38 UTC (permalink / raw)
To: git
Cc: Christian Couder, Junio C Hamano, Emily Shaffer, Josh Steadmon,
Randall S. Becker, Linus Arver, Linus Arver
In-Reply-To: <pull.1632.v2.git.1706308737.gitgitgadget@gmail.com>
From: Linus Arver <linusa@google.com>
This patch allows for the removal of "trailer_info_get()" from the
trailer.h API, which will be in the next patch.
Instead of calling "trailer_info_get()", which is a low-level function
in the trailers implementation (trailer.c), call
trailer_iterator_advance(), which was specifically designed for public
consumption in f0939a0eb1 (trailer: add interface for iterating over
commit trailers, 2020-09-27).
Avoiding "trailer_info_get()" means we don't have to worry about options
like "no_divider" (relevant for parsing trailers). We also don't have to
check for things like "info.trailer_start == info.trailer_end" to see
whether there were any trailers (instead we can just check to see
whether the iterator advanced at all).
Also, teach the iterator about non-trailer lines, by adding a new field
called "raw" to hold both trailer and non-trailer lines. This is
necessary because a "trailer block" is a list of trailer lines of at
least 25% trailers (see 146245063e (trailer: allow non-trailers in
trailer block, 2016-10-21)), such that it may hold non-trailer lines.
Signed-off-by: Linus Arver <linusa@google.com>
---
builtin/shortlog.c | 7 +++++--
sequencer.c | 35 +++++++++++++++--------------------
trailer.c | 17 +++++++++--------
trailer.h | 13 +++++++++++++
4 files changed, 42 insertions(+), 30 deletions(-)
diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 1307ed2b88a..dc8fd5a5532 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -172,7 +172,7 @@ static void insert_records_from_trailers(struct shortlog *log,
const char *oneline)
{
struct trailer_iterator iter;
- const char *commit_buffer, *body;
+ const char *commit_buffer, *body, *value;
struct strbuf ident = STRBUF_INIT;
if (!log->trailers.nr)
@@ -190,7 +190,10 @@ static void insert_records_from_trailers(struct shortlog *log,
trailer_iterator_init(&iter, body);
while (trailer_iterator_advance(&iter)) {
- const char *value = iter.val.buf;
+ if (!iter.is_trailer)
+ continue;
+
+ value = iter.val.buf;
if (!string_list_has_string(&log->trailers, iter.key.buf))
continue;
diff --git a/sequencer.c b/sequencer.c
index 3cc88d8a800..bc7c82c5271 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -319,37 +319,32 @@ static const char *get_todo_path(const struct replay_opts *opts)
static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
size_t ignore_footer)
{
- struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
- struct trailer_info info;
- size_t i;
- int found_sob = 0, found_sob_last = 0;
- char saved_char;
-
- opts.no_divider = 1;
+ struct trailer_iterator iter;
+ size_t i = 0, found_sob = 0;
+ char saved_char = sb->buf[sb->len - ignore_footer];
if (ignore_footer) {
- saved_char = sb->buf[sb->len - ignore_footer];
sb->buf[sb->len - ignore_footer] = '\0';
}
- trailer_info_get(&info, sb->buf, &opts);
+ trailer_iterator_init(&iter, sb->buf);
+ while (trailer_iterator_advance(&iter)) {
+ i++;
+ if (sob &&
+ iter.is_trailer &&
+ !strncmp(iter.raw, sob->buf, sob->len)) {
+ found_sob = i;
+ }
+ }
+ trailer_iterator_release(&iter);
if (ignore_footer)
sb->buf[sb->len - ignore_footer] = saved_char;
- if (info.trailer_block_start == info.trailer_block_end)
+ if (!i)
return 0;
- 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;
- }
-
- trailer_info_release(&info);
-
- if (found_sob_last)
+ if (found_sob == i)
return 3;
if (found_sob)
return 2;
diff --git a/trailer.c b/trailer.c
index 71ea2bb67f8..5bcc9b0006c 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1158,17 +1158,18 @@ void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
int trailer_iterator_advance(struct trailer_iterator *iter)
{
- while (iter->internal.cur < iter->internal.info.trailer_nr) {
- char *trailer = iter->internal.info.trailers[iter->internal.cur++];
- int separator_pos = find_separator(trailer, separators);
-
- if (separator_pos < 1)
- continue; /* not a real trailer */
-
+ char *line;
+ int separator_pos;
+ if (iter->internal.cur < iter->internal.info.trailer_nr) {
+ line = iter->internal.info.trailers[iter->internal.cur++];
+ separator_pos = find_separator(line, separators);
+ iter->is_trailer = (separator_pos > 0);
+
+ iter->raw = line;
strbuf_reset(&iter->key);
strbuf_reset(&iter->val);
parse_trailer(&iter->key, &iter->val, NULL,
- trailer, separator_pos);
+ line, separator_pos);
unfold_value(&iter->val);
return 1;
}
diff --git a/trailer.h b/trailer.h
index c309b01323d..a47f18b1525 100644
--- a/trailer.h
+++ b/trailer.h
@@ -127,6 +127,19 @@ struct trailer_iterator {
struct strbuf key;
struct strbuf val;
+ /*
+ * 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).
+ */
+ const char *raw;
+
+ /*
+ * 1 if the raw line was parsed as a trailer line (key/val pair).
+ */
+ int is_trailer;
+
/* private */
struct {
struct trailer_info info;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 06/10] trailer: make trailer_info struct private
From: Linus Arver via GitGitGadget @ 2024-01-26 22:38 UTC (permalink / raw)
To: git
Cc: Christian Couder, Junio C Hamano, Emily Shaffer, Josh Steadmon,
Randall S. Becker, Linus Arver, Linus Arver
In-Reply-To: <pull.1632.v2.git.1706308737.gitgitgadget@gmail.com>
From: Linus Arver <linusa@google.com>
In 13211ae23f (trailer: separate public from internal portion of
trailer_iterator, 2023-09-09) we moved trailer_info behind an anonymous
struct to discourage use by trailer.h API users. However it still left
open the possibility of external use of trailer_info itself. Now that
there are no external users of trailer_info, we can make this struct
private.
Make this struct private by putting its definition inside trailer.c.
This has two benefits:
(1) it makes the surface area of the public facing
interface (trailer.h) smaller, and
(2) external API users are unable to peer inside this struct (because
it is only ever exposed as an opaque pointer).
This change exposes some deficiencies in the API, mainly with regard to
information about the location of the trailer block that was parsed.
Expose new API functions to access this information (needed by
builtin/interpret-trailers.c).
The idea in this patch to hide implementation details behind an "opaque
pointer" is also known as the "pimpl" (pointer to implementation) idiom
in C++ and is a common pattern in that language (where, for example,
abstract classes only have pointers to concrete classes).
However, the original inspiration to use this idiom does not come from
C++, but instead the book "C Interfaces and Implementations: Techniques
for Creating Reusable Software" [1]. This book recommends opaque
pointers as a good design principle for designing C libraries, using the
term "interface" as the functions defined in *.h (header) files and
"implementation" as the corresponding *.c file which define the
interfaces.
The book says this about opaque pointers:
... clients can manipulate such pointers freely, but they can’t
dereference them; that is, they can’t look at the innards of the
structure pointed to by them. Only the implementation has that
privilege. Opaque pointers hide representation details and help
catch errors.
In our case, "struct trailer_info" is now hidden from clients, and the
ways in which this opaque pointer can be used is limited to the richness
of the trailer.h file. In other words, trailer.h exclusively controls
exactly how "trailer_info" pointers are to be used.
[1] Hanson, David R. "C Interfaces and Implementations: Techniques for
Creating Reusable Software". Addison Wesley, 1997. p. 22
Signed-off-by: Linus Arver <linusa@google.com>
---
builtin/interpret-trailers.c | 13 +--
trailer.c | 154 +++++++++++++++++++++++------------
trailer.h | 37 ++-------
3 files changed, 117 insertions(+), 87 deletions(-)
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 5352ee65bd1..9e6ed6b65e2 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -141,7 +141,7 @@ static void interpret_trailers(const struct process_trailer_options *opts,
LIST_HEAD(head);
struct strbuf sb = STRBUF_INIT;
struct strbuf trailer_block = STRBUF_INIT;
- struct trailer_info info;
+ struct trailer_info *info;
FILE *outfile = stdout;
trailer_config_init();
@@ -151,13 +151,13 @@ static void interpret_trailers(const struct process_trailer_options *opts,
if (opts->in_place)
outfile = create_in_place_tempfile(file);
- parse_trailers(opts, &info, sb.buf, &head);
+ info = parse_trailers(opts, sb.buf, &head);
/* Print the lines before the trailers */
if (!opts->only_trailers)
- fwrite(sb.buf, 1, info.trailer_block_start, outfile);
+ fwrite(sb.buf, 1, trailer_block_start(info), outfile);
- if (!opts->only_trailers && !info.blank_line_before_trailer)
+ if (!opts->only_trailers && !blank_line_before_trailer_block(info))
fprintf(outfile, "\n");
@@ -176,11 +176,12 @@ static void interpret_trailers(const struct process_trailer_options *opts,
strbuf_release(&trailer_block);
free_trailers(&head);
- trailer_info_release(&info);
/* Print the lines after the trailers as is */
if (!opts->only_trailers)
- fwrite(sb.buf + info.trailer_block_end, 1, sb.len - info.trailer_block_end, outfile);
+ fwrite(sb.buf + trailer_block_end(info), 1, sb.len - trailer_block_end(info), outfile);
+
+ trailer_info_release(info);
if (opts->in_place)
if (rename_tempfile(&trailers_tempfile, file))
diff --git a/trailer.c b/trailer.c
index 5bcc9b0006c..63774cd068d 100644
--- a/trailer.c
+++ b/trailer.c
@@ -11,6 +11,27 @@
* Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
*/
+struct trailer_info {
+ /*
+ * True if there is a blank line before the location pointed to by
+ * trailer_block_start.
+ */
+ int blank_line_before_trailer;
+
+ /*
+ * 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 (find_end_of_log_message()).
+ */
+ size_t trailer_block_start, trailer_block_end;
+
+ /*
+ * Array of trailers found.
+ */
+ char **trailers;
+ size_t trailer_nr;
+};
+
struct conf_info {
char *name;
char *key;
@@ -1025,20 +1046,72 @@ void format_trailers(const struct process_trailer_options *opts,
}
}
+static struct trailer_info *trailer_info_new(void)
+{
+ struct trailer_info *info = xcalloc(1, sizeof(*info));
+ return info;
+}
+
+static struct trailer_info *trailer_info_get(const struct process_trailer_options *opts,
+ const char *str)
+{
+ struct trailer_info *info = trailer_info_new();
+ size_t end_of_log_message = 0, trailer_block_start = 0;
+ struct strbuf **trailer_lines, **ptr;
+ char **trailer_strings = NULL;
+ size_t nr = 0, alloc = 0;
+ char **last = NULL;
+
+ trailer_config_init();
+
+ end_of_log_message = find_end_of_log_message(str, opts->no_divider);
+ trailer_block_start = find_trailer_block_start(str, end_of_log_message);
+
+ trailer_lines = strbuf_split_buf(str + trailer_block_start,
+ end_of_log_message - trailer_block_start,
+ '\n',
+ 0);
+ for (ptr = trailer_lines; *ptr; ptr++) {
+ if (last && isspace((*ptr)->buf[0])) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_attach(&sb, *last, strlen(*last), strlen(*last));
+ strbuf_addbuf(&sb, *ptr);
+ *last = strbuf_detach(&sb, NULL);
+ continue;
+ }
+ ALLOC_GROW(trailer_strings, nr + 1, alloc);
+ trailer_strings[nr] = strbuf_detach(*ptr, NULL);
+ last = find_separator(trailer_strings[nr], separators) >= 1
+ ? &trailer_strings[nr]
+ : NULL;
+ nr++;
+ }
+ strbuf_list_free(trailer_lines);
+
+ info->blank_line_before_trailer = ends_with_blank_line(str,
+ trailer_block_start);
+ info->trailer_block_start = trailer_block_start;
+ info->trailer_block_end = end_of_log_message;
+ info->trailers = trailer_strings;
+ info->trailer_nr = nr;
+
+ return info;
+}
+
/*
* Parse trailers in "str", populating the trailer info and "head"
* linked list structure.
*/
-void parse_trailers(const struct process_trailer_options *opts,
- struct trailer_info *info,
- const char *str,
- struct list_head *head)
+struct trailer_info *parse_trailers(const struct process_trailer_options *opts,
+ const char *str,
+ struct list_head *head)
{
+ struct trailer_info *info;
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
size_t i;
- trailer_info_get(info, str, opts);
+ info = trailer_info_get(opts, str);
for (i = 0; i < info->trailer_nr; i++) {
int separator_pos;
@@ -1062,6 +1135,8 @@ void parse_trailers(const struct process_trailer_options *opts,
strbuf_detach(&val, NULL));
}
}
+
+ return info;
}
void free_trailers(struct list_head *trailers)
@@ -1073,47 +1148,19 @@ void free_trailers(struct list_head *trailers)
}
}
-void trailer_info_get(struct trailer_info *info, const char *str,
- const struct process_trailer_options *opts)
+size_t trailer_block_start(struct trailer_info *info)
{
- size_t end_of_log_message = 0, trailer_block_start = 0;
- struct strbuf **trailer_lines, **ptr;
- char **trailer_strings = NULL;
- size_t nr = 0, alloc = 0;
- char **last = NULL;
-
- trailer_config_init();
-
- end_of_log_message = find_end_of_log_message(str, opts->no_divider);
- trailer_block_start = find_trailer_block_start(str, end_of_log_message);
+ return info->trailer_block_start;
+}
- trailer_lines = strbuf_split_buf(str + trailer_block_start,
- end_of_log_message - trailer_block_start,
- '\n',
- 0);
- for (ptr = trailer_lines; *ptr; ptr++) {
- if (last && isspace((*ptr)->buf[0])) {
- struct strbuf sb = STRBUF_INIT;
- strbuf_attach(&sb, *last, strlen(*last), strlen(*last));
- strbuf_addbuf(&sb, *ptr);
- *last = strbuf_detach(&sb, NULL);
- continue;
- }
- ALLOC_GROW(trailer_strings, nr + 1, alloc);
- trailer_strings[nr] = strbuf_detach(*ptr, NULL);
- last = find_separator(trailer_strings[nr], separators) >= 1
- ? &trailer_strings[nr]
- : NULL;
- nr++;
- }
- strbuf_list_free(trailer_lines);
+size_t trailer_block_end(struct trailer_info *info)
+{
+ return info->trailer_block_end;
+}
- info->blank_line_before_trailer = ends_with_blank_line(str,
- trailer_block_start);
- info->trailer_block_start = trailer_block_start;
- info->trailer_block_end = end_of_log_message;
- info->trailers = trailer_strings;
- info->trailer_nr = nr;
+int blank_line_before_trailer_block(struct trailer_info *info)
+{
+ return info->blank_line_before_trailer;
}
void trailer_info_release(struct trailer_info *info)
@@ -1122,6 +1169,7 @@ void trailer_info_release(struct trailer_info *info)
for (i = 0; i < info->trailer_nr; i++)
free(info->trailers[i]);
free(info->trailers);
+ free(info);
}
void format_trailers_from_commit(const struct process_trailer_options *opts,
@@ -1129,30 +1177,30 @@ void format_trailers_from_commit(const struct process_trailer_options *opts,
struct strbuf *out)
{
LIST_HEAD(head);
- struct trailer_info info;
-
- parse_trailers(opts, &info, msg, &head);
+ struct trailer_info *info = parse_trailers(opts, msg, &head);
/* If we want the whole block untouched, we can take the fast path. */
if (!opts->only_trailers && !opts->unfold && !opts->filter &&
!opts->separator && !opts->key_only && !opts->value_only &&
!opts->key_value_separator) {
- strbuf_add(out, msg + info.trailer_block_start,
- info.trailer_block_end - info.trailer_block_start);
+ strbuf_add(out, msg + info->trailer_block_start,
+ info->trailer_block_end - info->trailer_block_start);
} else
format_trailers(opts, &head, out);
free_trailers(&head);
- trailer_info_release(&info);
+ trailer_info_release(info);
}
void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
{
struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
+ struct trailer_info *internal = trailer_info_new();
strbuf_init(&iter->key, 0);
strbuf_init(&iter->val, 0);
opts.no_divider = 1;
- trailer_info_get(&iter->internal.info, msg, &opts);
+ iter->internal.info = internal;
+ iter->internal.info = trailer_info_get(&opts, msg);
iter->internal.cur = 0;
}
@@ -1160,8 +1208,8 @@ int trailer_iterator_advance(struct trailer_iterator *iter)
{
char *line;
int separator_pos;
- if (iter->internal.cur < iter->internal.info.trailer_nr) {
- line = iter->internal.info.trailers[iter->internal.cur++];
+ if (iter->internal.cur < iter->internal.info->trailer_nr) {
+ line = iter->internal.info->trailers[iter->internal.cur++];
separator_pos = find_separator(line, separators);
iter->is_trailer = (separator_pos > 0);
@@ -1178,7 +1226,7 @@ int trailer_iterator_advance(struct trailer_iterator *iter)
void trailer_iterator_release(struct trailer_iterator *iter)
{
- trailer_info_release(&iter->internal.info);
+ trailer_info_release(iter->internal.info);
strbuf_release(&iter->val);
strbuf_release(&iter->key);
}
diff --git a/trailer.h b/trailer.h
index a47f18b1525..fff16a29a55 100644
--- a/trailer.h
+++ b/trailer.h
@@ -4,6 +4,8 @@
#include "list.h"
#include "strbuf.h"
+struct trailer_info;
+
enum trailer_where {
WHERE_DEFAULT,
WHERE_END,
@@ -29,27 +31,6 @@ int trailer_set_where(enum trailer_where *item, const char *value);
int trailer_set_if_exists(enum trailer_if_exists *item, const char *value);
int trailer_set_if_missing(enum trailer_if_missing *item, const char *value);
-struct trailer_info {
- /*
- * True if there is a blank line before the location pointed to by
- * trailer_block_start.
- */
- int blank_line_before_trailer;
-
- /*
- * 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 (find_end_of_log_message()).
- */
- size_t trailer_block_start, trailer_block_end;
-
- /*
- * Array of trailers found.
- */
- char **trailers;
- size_t trailer_nr;
-};
-
/*
* A list that represents newly-added trailers, such as those provided
* with the --trailer command line option of git-interpret-trailers.
@@ -89,13 +70,13 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head,
void process_trailers_lists(struct list_head *head,
struct list_head *arg_head);
-void parse_trailers(const struct process_trailer_options *opts,
- struct trailer_info *info,
- const char *str,
- struct list_head *head);
+struct trailer_info *parse_trailers(const struct process_trailer_options *opts,
+ const char *str,
+ struct list_head *head);
-void trailer_info_get(struct trailer_info *info, const char *str,
- const struct process_trailer_options *opts);
+size_t trailer_block_start(struct trailer_info *info);
+size_t trailer_block_end(struct trailer_info *info);
+int blank_line_before_trailer_block(struct trailer_info *info);
void trailer_info_release(struct trailer_info *info);
@@ -142,7 +123,7 @@ struct trailer_iterator {
/* private */
struct {
- struct trailer_info info;
+ struct trailer_info *info;
size_t cur;
} internal;
};
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 07/10] trailer: spread usage of "trailer_block" language
From: Linus Arver via GitGitGadget @ 2024-01-26 22:38 UTC (permalink / raw)
To: git
Cc: Christian Couder, Junio C Hamano, Emily Shaffer, Josh Steadmon,
Randall S. Becker, Linus Arver, Linus Arver
In-Reply-To: <pull.1632.v2.git.1706308737.gitgitgadget@gmail.com>
From: Linus Arver <linusa@google.com>
Deprecate the "trailer_info" struct name and replace it with
"trailer_block". The main reason is to help readability, because
"trailer_info" on the surface sounds like it's about a single trailer
when in reality it is a collection of contiguous lines, at least 25% of
which are trailers.
Signed-off-by: Linus Arver <linusa@google.com>
---
builtin/interpret-trailers.c | 26 +++++-----
trailer.c | 99 ++++++++++++++++++------------------
trailer.h | 18 +++----
3 files changed, 71 insertions(+), 72 deletions(-)
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 9e6ed6b65e2..9e41fa20b5f 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -140,8 +140,8 @@ static void interpret_trailers(const struct process_trailer_options *opts,
{
LIST_HEAD(head);
struct strbuf sb = STRBUF_INIT;
- struct strbuf trailer_block = STRBUF_INIT;
- struct trailer_info *info;
+ struct strbuf tb = STRBUF_INIT;
+ struct trailer_block *trailer_block;
FILE *outfile = stdout;
trailer_config_init();
@@ -151,13 +151,13 @@ static void interpret_trailers(const struct process_trailer_options *opts,
if (opts->in_place)
outfile = create_in_place_tempfile(file);
- info = parse_trailers(opts, sb.buf, &head);
+ trailer_block = parse_trailers(opts, sb.buf, &head);
- /* Print the lines before the trailers */
+ /* Print the lines before the trailer block */
if (!opts->only_trailers)
- fwrite(sb.buf, 1, trailer_block_start(info), outfile);
+ fwrite(sb.buf, 1, trailer_block_start(trailer_block), outfile);
- if (!opts->only_trailers && !blank_line_before_trailer_block(info))
+ if (!opts->only_trailers && !blank_line_before_trailer_block(trailer_block))
fprintf(outfile, "\n");
@@ -171,17 +171,17 @@ static void interpret_trailers(const struct process_trailer_options *opts,
}
/* Print trailer block. */
- format_trailers(opts, &head, &trailer_block);
- fwrite(trailer_block.buf, 1, trailer_block.len, outfile);
- strbuf_release(&trailer_block);
+ format_trailers(opts, &head, &tb);
+ fwrite(tb.buf, 1, tb.len, outfile);
+ strbuf_release(&tb);
free_trailers(&head);
- /* Print the lines after the trailers as is */
+ /* Print the lines after the trailer block as is */
if (!opts->only_trailers)
- fwrite(sb.buf + trailer_block_end(info), 1, sb.len - trailer_block_end(info), outfile);
-
- trailer_info_release(info);
+ fwrite(sb.buf + trailer_block_end(trailer_block),
+ 1, sb.len - trailer_block_end(trailer_block), outfile);
+ trailer_block_release(trailer_block);
if (opts->in_place)
if (rename_tempfile(&trailers_tempfile, file))
diff --git a/trailer.c b/trailer.c
index 63774cd068d..e2a48bea0ae 100644
--- a/trailer.c
+++ b/trailer.c
@@ -11,19 +11,20 @@
* Copyright (c) 2013, 2014 Christian Couder <chriscool@tuxfamily.org>
*/
-struct trailer_info {
+struct trailer_block {
/*
* True if there is a blank line before the location pointed to by
- * trailer_block_start.
+ * "start".
*/
int blank_line_before_trailer;
/*
- * 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 (find_end_of_log_message()).
+ * The locations of the start and end positions of the trailer block
+ * found, as offsets from the beginning of the source text from which
+ * this trailer block was parsed. If no trailer block is found, these
+ * are both set to 0.
*/
- size_t trailer_block_start, trailer_block_end;
+ size_t start, end;
/*
* Array of trailers found.
@@ -1046,16 +1047,16 @@ void format_trailers(const struct process_trailer_options *opts,
}
}
-static struct trailer_info *trailer_info_new(void)
+static struct trailer_block *trailer_block_new(void)
{
- struct trailer_info *info = xcalloc(1, sizeof(*info));
- return info;
+ struct trailer_block *trailer_block = xcalloc(1, sizeof(*trailer_block));
+ return trailer_block;
}
-static struct trailer_info *trailer_info_get(const struct process_trailer_options *opts,
- const char *str)
+static struct trailer_block *trailer_block_get(const struct process_trailer_options *opts,
+ const char *str)
{
- struct trailer_info *info = trailer_info_new();
+ struct trailer_block *trailer_block = trailer_block_new();
size_t end_of_log_message = 0, trailer_block_start = 0;
struct strbuf **trailer_lines, **ptr;
char **trailer_strings = NULL;
@@ -1088,34 +1089,34 @@ static struct trailer_info *trailer_info_get(const struct process_trailer_option
}
strbuf_list_free(trailer_lines);
- info->blank_line_before_trailer = ends_with_blank_line(str,
- trailer_block_start);
- info->trailer_block_start = trailer_block_start;
- info->trailer_block_end = end_of_log_message;
- info->trailers = trailer_strings;
- info->trailer_nr = nr;
+ trailer_block->blank_line_before_trailer = ends_with_blank_line(str,
+ trailer_block_start);
+ trailer_block->start = trailer_block_start;
+ trailer_block->end = end_of_log_message;
+ trailer_block->trailers = trailer_strings;
+ trailer_block->trailer_nr = nr;
- return info;
+ return trailer_block;
}
/*
- * Parse trailers in "str", populating the trailer info and "head"
- * linked list structure.
+ * Parse trailers in "str", populating the trailer_block info and "head" linked
+ * list structure.
*/
-struct trailer_info *parse_trailers(const struct process_trailer_options *opts,
- const char *str,
- struct list_head *head)
+struct trailer_block *parse_trailers(const struct process_trailer_options *opts,
+ const char *str,
+ struct list_head *head)
{
- struct trailer_info *info;
+ struct trailer_block *trailer_block;
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
size_t i;
- info = trailer_info_get(opts, str);
+ trailer_block = trailer_block_get(opts, str);
- for (i = 0; i < info->trailer_nr; i++) {
+ for (i = 0; i < trailer_block->trailer_nr; i++) {
int separator_pos;
- char *trailer = info->trailers[i];
+ char *trailer = trailer_block->trailers[i];
if (trailer[0] == comment_line_char)
continue;
separator_pos = find_separator(trailer, separators);
@@ -1136,7 +1137,7 @@ struct trailer_info *parse_trailers(const struct process_trailer_options *opts,
}
}
- return info;
+ return trailer_block;
}
void free_trailers(struct list_head *trailers)
@@ -1148,28 +1149,28 @@ void free_trailers(struct list_head *trailers)
}
}
-size_t trailer_block_start(struct trailer_info *info)
+size_t trailer_block_start(struct trailer_block *trailer_block)
{
- return info->trailer_block_start;
+ return trailer_block->start;
}
-size_t trailer_block_end(struct trailer_info *info)
+size_t trailer_block_end(struct trailer_block *trailer_block)
{
- return info->trailer_block_end;
+ return trailer_block->end;
}
-int blank_line_before_trailer_block(struct trailer_info *info)
+int blank_line_before_trailer_block(struct trailer_block *trailer_block)
{
- return info->blank_line_before_trailer;
+ return trailer_block->blank_line_before_trailer;
}
-void trailer_info_release(struct trailer_info *info)
+void trailer_block_release(struct trailer_block *trailer_block)
{
size_t i;
- for (i = 0; i < info->trailer_nr; i++)
- free(info->trailers[i]);
- free(info->trailers);
- free(info);
+ for (i = 0; i < trailer_block->trailer_nr; i++)
+ free(trailer_block->trailers[i]);
+ free(trailer_block->trailers);
+ free(trailer_block);
}
void format_trailers_from_commit(const struct process_trailer_options *opts,
@@ -1177,30 +1178,28 @@ void format_trailers_from_commit(const struct process_trailer_options *opts,
struct strbuf *out)
{
LIST_HEAD(head);
- struct trailer_info *info = parse_trailers(opts, msg, &head);
+ struct trailer_block *trailer_block = parse_trailers(opts, msg, &head);
/* If we want the whole block untouched, we can take the fast path. */
if (!opts->only_trailers && !opts->unfold && !opts->filter &&
!opts->separator && !opts->key_only && !opts->value_only &&
!opts->key_value_separator) {
- strbuf_add(out, msg + info->trailer_block_start,
- info->trailer_block_end - info->trailer_block_start);
+ strbuf_add(out, msg + trailer_block->start,
+ trailer_block->end - trailer_block->start);
} else
format_trailers(opts, &head, out);
free_trailers(&head);
- trailer_info_release(info);
+ trailer_block_release(trailer_block);
}
void trailer_iterator_init(struct trailer_iterator *iter, const char *msg)
{
struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
- struct trailer_info *internal = trailer_info_new();
strbuf_init(&iter->key, 0);
strbuf_init(&iter->val, 0);
opts.no_divider = 1;
- iter->internal.info = internal;
- iter->internal.info = trailer_info_get(&opts, msg);
+ iter->internal.trailer_block = trailer_block_get(&opts, msg);
iter->internal.cur = 0;
}
@@ -1208,8 +1207,8 @@ int trailer_iterator_advance(struct trailer_iterator *iter)
{
char *line;
int separator_pos;
- if (iter->internal.cur < iter->internal.info->trailer_nr) {
- line = iter->internal.info->trailers[iter->internal.cur++];
+ if (iter->internal.cur < iter->internal.trailer_block->trailer_nr) {
+ line = iter->internal.trailer_block->trailers[iter->internal.cur++];
separator_pos = find_separator(line, separators);
iter->is_trailer = (separator_pos > 0);
@@ -1226,7 +1225,7 @@ int trailer_iterator_advance(struct trailer_iterator *iter)
void trailer_iterator_release(struct trailer_iterator *iter)
{
- trailer_info_release(iter->internal.info);
+ trailer_block_release(iter->internal.trailer_block);
strbuf_release(&iter->val);
strbuf_release(&iter->key);
}
diff --git a/trailer.h b/trailer.h
index fff16a29a55..be14dd17185 100644
--- a/trailer.h
+++ b/trailer.h
@@ -4,7 +4,7 @@
#include "list.h"
#include "strbuf.h"
-struct trailer_info;
+struct trailer_block;
enum trailer_where {
WHERE_DEFAULT,
@@ -70,15 +70,15 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head,
void process_trailers_lists(struct list_head *head,
struct list_head *arg_head);
-struct trailer_info *parse_trailers(const struct process_trailer_options *opts,
- const char *str,
- struct list_head *head);
+struct trailer_block *parse_trailers(const struct process_trailer_options *opts,
+ const char *str,
+ struct list_head *head);
-size_t trailer_block_start(struct trailer_info *info);
-size_t trailer_block_end(struct trailer_info *info);
-int blank_line_before_trailer_block(struct trailer_info *info);
+size_t trailer_block_start(struct trailer_block *trailer_block);
+size_t trailer_block_end(struct trailer_block *trailer_block);
+int blank_line_before_trailer_block(struct trailer_block *trailer_block);
-void trailer_info_release(struct trailer_info *info);
+void trailer_block_release(struct trailer_block *trailer_block);
void trailer_config_init(void);
void format_trailers(const struct process_trailer_options *opts,
@@ -123,7 +123,7 @@ struct trailer_iterator {
/* private */
struct {
- struct trailer_info *info;
+ struct trailer_block *trailer_block;
size_t cur;
} internal;
};
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 08/10] trailer: prepare to move parse_trailers_from_command_line_args() to builtin
From: Linus Arver via GitGitGadget @ 2024-01-26 22:38 UTC (permalink / raw)
To: git
Cc: Christian Couder, Junio C Hamano, Emily Shaffer, Josh Steadmon,
Randall S. Becker, Linus Arver, Linus Arver
In-Reply-To: <pull.1632.v2.git.1706308737.gitgitgadget@gmail.com>
From: Linus Arver <linusa@google.com>
Expose more functions in the trailer.h API, in preparation for moving
out
parse_trailers_from_command_line_args()
to interpret-trailer.c, because the trailer API should not be concerned
with command line arguments (as they have nothing to do with trailers
themselves). The interpret-trailers builtin is the only caller of this
function.
Also rename "conf_info" to "trailer_conf" for readability, dropping the
low-value "_info" suffix as we did earlier in this series for
"trailer_info" to "trailer_block".
Helped-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Linus Arver <linusa@google.com>
---
trailer.c | 66 +++++++++++++++++++++++++++----------------------------
trailer.h | 10 +++++++++
2 files changed, 42 insertions(+), 34 deletions(-)
diff --git a/trailer.c b/trailer.c
index e2a48bea0ae..d362b9a604e 100644
--- a/trailer.c
+++ b/trailer.c
@@ -33,7 +33,7 @@ struct trailer_block {
size_t trailer_nr;
};
-struct conf_info {
+struct trailer_conf {
char *name;
char *key;
char *command;
@@ -43,7 +43,7 @@ struct conf_info {
enum trailer_if_missing if_missing;
};
-static struct conf_info default_conf_info;
+static struct trailer_conf default_trailer_conf;
struct trailer_item {
struct list_head list;
@@ -59,7 +59,7 @@ struct arg_item {
struct list_head list;
char *token;
char *value;
- struct conf_info conf;
+ struct trailer_conf conf;
};
static LIST_HEAD(conf_head);
@@ -210,7 +210,7 @@ static int check_if_different(struct trailer_item *in_tok,
return 1;
}
-static char *apply_command(struct conf_info *conf, const char *arg)
+static char *apply_command(struct trailer_conf *conf, const char *arg)
{
struct strbuf cmd = STRBUF_INIT;
struct strbuf buf = STRBUF_INIT;
@@ -424,7 +424,8 @@ int trailer_set_if_missing(enum trailer_if_missing *item, const char *value)
return 0;
}
-static void duplicate_conf(struct conf_info *dst, const struct conf_info *src)
+void duplicate_trailer_conf(struct trailer_conf *dst,
+ const struct trailer_conf *src)
{
*dst = *src;
dst->name = xstrdup_or_null(src->name);
@@ -447,7 +448,7 @@ static struct arg_item *get_conf_item(const char *name)
/* Item does not already exists, create it */
CALLOC_ARRAY(item, 1);
- duplicate_conf(&item->conf, &default_conf_info);
+ duplicate_trailer_conf(&item->conf, &default_trailer_conf);
item->conf.name = xstrdup(name);
list_add_tail(&item->list, &conf_head);
@@ -482,17 +483,17 @@ static int git_trailer_default_config(const char *conf_key, const char *value,
variable_name = strrchr(trailer_item, '.');
if (!variable_name) {
if (!strcmp(trailer_item, "where")) {
- if (trailer_set_where(&default_conf_info.where,
+ if (trailer_set_where(&default_trailer_conf.where,
value) < 0)
warning(_("unknown value '%s' for key '%s'"),
value, conf_key);
} else if (!strcmp(trailer_item, "ifexists")) {
- if (trailer_set_if_exists(&default_conf_info.if_exists,
+ if (trailer_set_if_exists(&default_trailer_conf.if_exists,
value) < 0)
warning(_("unknown value '%s' for key '%s'"),
value, conf_key);
} else if (!strcmp(trailer_item, "ifmissing")) {
- if (trailer_set_if_missing(&default_conf_info.if_missing,
+ if (trailer_set_if_missing(&default_trailer_conf.if_missing,
value) < 0)
warning(_("unknown value '%s' for key '%s'"),
value, conf_key);
@@ -511,7 +512,7 @@ static int git_trailer_config(const char *conf_key, const char *value,
{
const char *trailer_item, *variable_name;
struct arg_item *item;
- struct conf_info *conf;
+ struct trailer_conf *conf;
char *name = NULL;
enum trailer_info_type type;
int i;
@@ -585,9 +586,9 @@ void trailer_config_init(void)
return;
/* Default config must be setup first */
- default_conf_info.where = WHERE_END;
- default_conf_info.if_exists = EXISTS_ADD_IF_DIFFERENT_NEIGHBOR;
- default_conf_info.if_missing = MISSING_ADD;
+ default_trailer_conf.where = WHERE_END;
+ default_trailer_conf.if_exists = EXISTS_ADD_IF_DIFFERENT_NEIGHBOR;
+ default_trailer_conf.if_missing = MISSING_ADD;
git_config(git_trailer_default_config, NULL);
git_config(git_trailer_config, NULL);
configured = 1;
@@ -620,7 +621,7 @@ static int token_matches_item(const char *tok, struct arg_item *item, size_t tok
* distinguished from the non-well-formed-line case (in which this function
* returns -1) because some callers of this function need such a distinction.
*/
-static ssize_t find_separator(const char *line, const char *separators)
+ssize_t find_separator(const char *line, const char *separators)
{
int whitespace_found = 0;
const char *c;
@@ -645,28 +646,28 @@ static ssize_t find_separator(const char *line, const char *separators)
*
* If separator_pos is -1, interpret the whole trailer as a token.
*/
-static void parse_trailer(struct strbuf *tok, struct strbuf *val,
- const struct conf_info **conf, const char *trailer,
- ssize_t separator_pos)
+void parse_trailer(const char *line, ssize_t separator_pos,
+ struct strbuf *tok, struct strbuf *val,
+ const struct trailer_conf **conf)
{
struct arg_item *item;
size_t tok_len;
struct list_head *pos;
if (separator_pos != -1) {
- strbuf_add(tok, trailer, separator_pos);
+ strbuf_add(tok, line, separator_pos);
strbuf_trim(tok);
- strbuf_addstr(val, trailer + separator_pos + 1);
+ strbuf_addstr(val, line + separator_pos + 1);
strbuf_trim(val);
} else {
- strbuf_addstr(tok, trailer);
+ strbuf_addstr(tok, line);
strbuf_trim(tok);
}
/* Lookup if the token matches something in the config */
tok_len = token_len_without_separator(tok->buf, tok->len);
if (conf)
- *conf = &default_conf_info;
+ *conf = &default_trailer_conf;
list_for_each(pos, &conf_head) {
item = list_entry(pos, struct arg_item, list);
if (token_matches_item(tok->buf, item, tok_len)) {
@@ -691,13 +692,13 @@ static struct trailer_item *add_trailer_item(struct list_head *head, char *tok,
}
static void add_arg_item(struct list_head *arg_head, char *tok, char *val,
- const struct conf_info *conf,
+ const struct trailer_conf *conf,
const struct new_trailer_item *new_trailer_item)
{
struct arg_item *new_item = xcalloc(1, sizeof(*new_item));
new_item->token = tok;
new_item->value = val;
- duplicate_conf(&new_item->conf, conf);
+ duplicate_trailer_conf(&new_item->conf, conf);
if (new_trailer_item) {
if (new_trailer_item->where != WHERE_DEFAULT)
new_item->conf.where = new_trailer_item->where;
@@ -730,7 +731,7 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head,
{
struct strbuf tok = STRBUF_INIT;
struct strbuf val = STRBUF_INIT;
- const struct conf_info *conf;
+ const struct trailer_conf *conf;
struct list_head *pos;
/*
@@ -753,8 +754,7 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head,
(int) sb.len, sb.buf);
strbuf_release(&sb);
} else {
- parse_trailer(&tok, &val, &conf, tr->text,
- separator_pos);
+ parse_trailer(tr->text, separator_pos, &tok, &val, &conf);
add_arg_item(arg_head,
strbuf_detach(&tok, NULL),
strbuf_detach(&val, NULL),
@@ -1116,20 +1116,19 @@ struct trailer_block *parse_trailers(const struct process_trailer_options *opts,
for (i = 0; i < trailer_block->trailer_nr; i++) {
int separator_pos;
- char *trailer = trailer_block->trailers[i];
- if (trailer[0] == comment_line_char)
+ char *line = trailer_block->trailers[i];
+ if (line[0] == comment_line_char)
continue;
- separator_pos = find_separator(trailer, separators);
+ separator_pos = find_separator(line, separators);
if (separator_pos >= 1) {
- parse_trailer(&tok, &val, NULL, trailer,
- separator_pos);
+ parse_trailer(line, separator_pos, &tok, &val, NULL);
if (opts->unfold)
unfold_value(&val);
add_trailer_item(head,
strbuf_detach(&tok, NULL),
strbuf_detach(&val, NULL));
} else if (!opts->only_trailers) {
- strbuf_addstr(&val, trailer);
+ strbuf_addstr(&val, line);
strbuf_strip_suffix(&val, "\n");
add_trailer_item(head,
NULL,
@@ -1215,8 +1214,7 @@ int trailer_iterator_advance(struct trailer_iterator *iter)
iter->raw = line;
strbuf_reset(&iter->key);
strbuf_reset(&iter->val);
- parse_trailer(&iter->key, &iter->val, NULL,
- line, separator_pos);
+ parse_trailer(line, separator_pos, &iter->key, &iter->val, NULL);
unfold_value(&iter->val);
return 1;
}
diff --git a/trailer.h b/trailer.h
index be14dd17185..c6aa96dd6be 100644
--- a/trailer.h
+++ b/trailer.h
@@ -5,6 +5,7 @@
#include "strbuf.h"
struct trailer_block;
+struct trailer_conf;
enum trailer_where {
WHERE_DEFAULT,
@@ -45,6 +46,9 @@ struct new_trailer_item {
enum trailer_if_missing if_missing;
};
+void duplicate_trailer_conf(struct trailer_conf *dst,
+ const struct trailer_conf *src);
+
struct process_trailer_options {
int in_place;
int trim_empty;
@@ -70,6 +74,12 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head,
void process_trailers_lists(struct list_head *head,
struct list_head *arg_head);
+ssize_t find_separator(const char *line, const char *separators);
+
+void parse_trailer(const char *line, ssize_t separator_pos,
+ struct strbuf *tok, struct strbuf *val,
+ const struct trailer_conf **conf);
+
struct trailer_block *parse_trailers(const struct process_trailer_options *opts,
const char *str,
struct list_head *head);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 09/10] trailer: move arg handling to interpret-trailers.c
From: Linus Arver via GitGitGadget @ 2024-01-26 22:38 UTC (permalink / raw)
To: git
Cc: Christian Couder, Junio C Hamano, Emily Shaffer, Josh Steadmon,
Randall S. Becker, Linus Arver, Linus Arver
In-Reply-To: <pull.1632.v2.git.1706308737.gitgitgadget@gmail.com>
From: Linus Arver <linusa@google.com>
We don't move the "arg_item" struct to interpret-trailers.c, because it
is now a struct that contains information about trailers that should be
injected into the input text's own trailers. We will rename this
language as such in a follow-up patch to keep the diff here small.
Signed-off-by: Linus Arver <linusa@google.com>
---
builtin/interpret-trailers.c | 88 ++++++++++++++++++++++--------------
trailer.c | 63 +++++++++++++++++++-------
trailer.h | 13 +++++-
3 files changed, 113 insertions(+), 51 deletions(-)
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 9e41fa20b5f..ad9b9a9f9ef 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -45,23 +45,17 @@ static int option_parse_if_missing(const struct option *opt,
return trailer_set_if_missing(opt->value, arg);
}
-static void new_trailers_clear(struct list_head *trailers)
-{
- struct list_head *pos, *tmp;
- struct new_trailer_item *item;
-
- list_for_each_safe(pos, tmp, trailers) {
- item = list_entry(pos, struct new_trailer_item, list);
- list_del(pos);
- free(item);
- }
-}
+static char *cl_separators;
static int option_parse_trailer(const struct option *opt,
const char *arg, int unset)
{
struct list_head *trailers = opt->value;
- struct new_trailer_item *item;
+ struct strbuf tok = STRBUF_INIT;
+ struct strbuf val = STRBUF_INIT;
+ const struct trailer_conf *conf;
+ struct trailer_conf *conf_current = new_trailer_conf();
+ ssize_t separator_pos;
if (unset) {
new_trailers_clear(trailers);
@@ -71,12 +65,31 @@ static int option_parse_trailer(const struct option *opt,
if (!arg)
return -1;
- item = xmalloc(sizeof(*item));
- item->text = arg;
- item->where = where;
- item->if_exists = if_exists;
- item->if_missing = if_missing;
- list_add_tail(&item->list, trailers);
+ separator_pos = find_separator(arg, cl_separators);
+ if (separator_pos) {
+ parse_trailer(arg, separator_pos, &tok, &val, &conf);
+ duplicate_trailer_conf(conf_current, conf);
+
+ /*
+ * Override conf_current with settings specified via CLI flags.
+ */
+ trailer_conf_set(where, if_exists, if_missing, conf_current);
+
+ add_arg_item(strbuf_detach(&tok, NULL),
+ strbuf_detach(&val, NULL),
+ conf_current,
+ trailers);
+ } else {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_addstr(&sb, arg);
+ strbuf_trim(&sb);
+ error(_("empty trailer token in trailer '%.*s'"),
+ (int) sb.len, sb.buf);
+ strbuf_release(&sb);
+ }
+
+ free(conf_current);
+
return 0;
}
@@ -135,7 +148,7 @@ static void read_input_file(struct strbuf *sb, const char *file)
}
static void interpret_trailers(const struct process_trailer_options *opts,
- struct list_head *new_trailer_head,
+ struct list_head *arg_trailers,
const char *file)
{
LIST_HEAD(head);
@@ -144,8 +157,6 @@ static void interpret_trailers(const struct process_trailer_options *opts,
struct trailer_block *trailer_block;
FILE *outfile = stdout;
- trailer_config_init();
-
read_input_file(&sb, file);
if (opts->in_place)
@@ -162,12 +173,7 @@ static void interpret_trailers(const struct process_trailer_options *opts,
if (!opts->only_input) {
- LIST_HEAD(config_head);
- LIST_HEAD(arg_head);
- parse_trailers_from_config(&config_head);
- parse_trailers_from_command_line_args(&arg_head, new_trailer_head);
- list_splice(&config_head, &arg_head);
- process_trailers_lists(&head, &arg_head);
+ process_trailers_lists(&head, arg_trailers);
}
/* Print trailer block. */
@@ -193,7 +199,8 @@ static void interpret_trailers(const struct process_trailer_options *opts,
int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
{
struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
- LIST_HEAD(trailers);
+ LIST_HEAD(configured_trailers);
+ LIST_HEAD(arg_trailers);
struct option options[] = {
OPT_BOOL(0, "in-place", &opts.in_place, N_("edit files in place")),
@@ -212,33 +219,48 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
OPT_CALLBACK_F(0, "parse", &opts, NULL, N_("alias for --only-trailers --only-input --unfold"),
PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_parse),
OPT_BOOL(0, "no-divider", &opts.no_divider, N_("do not treat \"---\" as the end of input")),
- OPT_CALLBACK(0, "trailer", &trailers, N_("trailer"),
+ OPT_CALLBACK(0, "trailer", &arg_trailers, N_("trailer"),
N_("trailer(s) to add"), option_parse_trailer),
OPT_END()
};
git_config(git_default_config, NULL);
+ trailer_config_init();
+
+ if (!opts.only_input) {
+ parse_trailers_from_config(&configured_trailers);
+ }
+
+ /*
+ * In command-line arguments, '=' is accepted (in addition to the
+ * separators that are defined).
+ */
+ cl_separators = xstrfmt("=%s", default_separators());
argc = parse_options(argc, argv, prefix, options,
git_interpret_trailers_usage, 0);
- if (opts.only_input && !list_empty(&trailers))
+ free(cl_separators);
+
+ if (opts.only_input && !list_empty(&arg_trailers))
usage_msg_opt(
_("--trailer with --only-input does not make sense"),
git_interpret_trailers_usage,
options);
+ list_splice(&configured_trailers, &arg_trailers);
+
if (argc) {
int i;
for (i = 0; i < argc; i++)
- interpret_trailers(&opts, &trailers, argv[i]);
+ interpret_trailers(&opts, &arg_trailers, argv[i]);
} else {
if (opts.in_place)
die(_("no input file given for in-place editing"));
- interpret_trailers(&opts, &trailers, NULL);
+ interpret_trailers(&opts, &arg_trailers, NULL);
}
- new_trailers_clear(&trailers);
+ new_trailers_clear(&arg_trailers);
return 0;
}
diff --git a/trailer.c b/trailer.c
index d362b9a604e..1865d04bdf2 100644
--- a/trailer.c
+++ b/trailer.c
@@ -66,6 +66,11 @@ static LIST_HEAD(conf_head);
static char *separators = ":";
+const char *default_separators(void)
+{
+ return separators;
+}
+
static int configured;
#define TRAILER_ARG_STRING "$ARG"
@@ -424,6 +429,25 @@ int trailer_set_if_missing(enum trailer_if_missing *item, const char *value)
return 0;
}
+void trailer_conf_set(enum trailer_where where,
+ enum trailer_if_exists if_exists,
+ enum trailer_if_missing if_missing,
+ struct trailer_conf *conf)
+{
+ if (where != WHERE_DEFAULT)
+ conf->where = where;
+ if (if_exists != EXISTS_DEFAULT)
+ conf->if_exists = if_exists;
+ if (if_missing != MISSING_DEFAULT)
+ conf->if_missing = if_missing;
+}
+
+struct trailer_conf *new_trailer_conf(void)
+{
+ struct trailer_conf *new = xcalloc(1, sizeof(*new));
+ return new;
+}
+
void duplicate_trailer_conf(struct trailer_conf *dst,
const struct trailer_conf *src)
{
@@ -642,6 +666,9 @@ ssize_t find_separator(const char *line, const char *separators)
/*
* Obtain the token, value, and conf from the given trailer.
*
+ * The conf needs special handling. We first read hardcoded defaults, and
+ * override them if we find a matching trailer configuration in the config.
+ *
* separator_pos must not be 0, since the token cannot be an empty string.
*
* If separator_pos is -1, interpret the whole trailer as a token.
@@ -691,22 +718,14 @@ static struct trailer_item *add_trailer_item(struct list_head *head, char *tok,
return new_item;
}
-static void add_arg_item(struct list_head *arg_head, char *tok, char *val,
- const struct trailer_conf *conf,
- const struct new_trailer_item *new_trailer_item)
+void add_arg_item(char *tok, char *val, const struct trailer_conf *conf,
+ struct list_head *arg_head)
+
{
struct arg_item *new_item = xcalloc(1, sizeof(*new_item));
new_item->token = tok;
new_item->value = val;
duplicate_trailer_conf(&new_item->conf, conf);
- if (new_trailer_item) {
- if (new_trailer_item->where != WHERE_DEFAULT)
- new_item->conf.where = new_trailer_item->where;
- if (new_trailer_item->if_exists != EXISTS_DEFAULT)
- new_item->conf.if_exists = new_trailer_item->if_exists;
- if (new_trailer_item->if_missing != MISSING_DEFAULT)
- new_item->conf.if_missing = new_trailer_item->if_missing;
- }
list_add_tail(&new_item->list, arg_head);
}
@@ -719,10 +738,10 @@ void parse_trailers_from_config(struct list_head *config_head)
list_for_each(pos, &conf_head) {
item = list_entry(pos, struct arg_item, list);
if (item->conf.command)
- add_arg_item(config_head,
- xstrdup(token_from_item(item, NULL)),
+ add_arg_item(xstrdup(token_from_item(item, NULL)),
xstrdup(""),
- &item->conf, NULL);
+ &item->conf,
+ config_head);
}
}
@@ -755,10 +774,10 @@ void parse_trailers_from_command_line_args(struct list_head *arg_head,
strbuf_release(&sb);
} else {
parse_trailer(tr->text, separator_pos, &tok, &val, &conf);
- add_arg_item(arg_head,
- strbuf_detach(&tok, NULL),
+ add_arg_item(strbuf_detach(&tok, NULL),
strbuf_detach(&val, NULL),
- conf, tr);
+ conf,
+ arg_head);
}
}
@@ -1148,6 +1167,16 @@ void free_trailers(struct list_head *trailers)
}
}
+void new_trailers_clear(struct list_head *trailers)
+{
+ struct list_head *pos, *p;
+
+ list_for_each_safe(pos, p, trailers) {
+ list_del(pos);
+ free_arg_item(list_entry(pos, struct arg_item, list));
+ }
+}
+
size_t trailer_block_start(struct trailer_block *trailer_block)
{
return trailer_block->start;
diff --git a/trailer.h b/trailer.h
index c6aa96dd6be..e01437160cf 100644
--- a/trailer.h
+++ b/trailer.h
@@ -46,9 +46,20 @@ struct new_trailer_item {
enum trailer_if_missing if_missing;
};
+void trailer_conf_set(enum trailer_where where,
+ enum trailer_if_exists if_exists,
+ enum trailer_if_missing if_missing,
+ struct trailer_conf *conf);
+
+struct trailer_conf *new_trailer_conf(void);
void duplicate_trailer_conf(struct trailer_conf *dst,
const struct trailer_conf *src);
+const char *default_separators(void);
+
+void add_arg_item(char *tok, char *val, const struct trailer_conf *conf,
+ struct list_head *arg_head);
+
struct process_trailer_options {
int in_place;
int trim_empty;
@@ -95,7 +106,7 @@ void format_trailers(const struct process_trailer_options *opts,
struct list_head *trailers,
struct strbuf *out);
void free_trailers(struct list_head *trailers);
-
+void new_trailers_clear(struct list_head *new_trailers);
/*
* Convenience function to format the trailers from the commit msg "msg" into
* the strbuf "out". Reuses format_trailers internally.
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 10/10] trailer: delete obsolete argument handling code from API
From: Linus Arver via GitGitGadget @ 2024-01-26 22:38 UTC (permalink / raw)
To: git
Cc: Christian Couder, Junio C Hamano, Emily Shaffer, Josh Steadmon,
Randall S. Becker, Linus Arver, Linus Arver
In-Reply-To: <pull.1632.v2.git.1706308737.gitgitgadget@gmail.com>
From: Linus Arver <linusa@google.com>
This commit was not squashed with its parent in order to keep the diff
separate (to make the additive changes in the parent easier to read).
Signed-off-by: Linus Arver <linusa@google.com>
---
trailer.c | 39 ---------------------------------------
trailer.h | 17 -----------------
2 files changed, 56 deletions(-)
diff --git a/trailer.c b/trailer.c
index 1865d04bdf2..8016908039f 100644
--- a/trailer.c
+++ b/trailer.c
@@ -745,45 +745,6 @@ void parse_trailers_from_config(struct list_head *config_head)
}
}
-void parse_trailers_from_command_line_args(struct list_head *arg_head,
- struct list_head *new_trailer_head)
-{
- struct strbuf tok = STRBUF_INIT;
- struct strbuf val = STRBUF_INIT;
- const struct trailer_conf *conf;
- struct list_head *pos;
-
- /*
- * In command-line arguments, '=' is accepted (in addition to the
- * separators that are defined).
- */
- char *cl_separators = xstrfmt("=%s", separators);
-
- /* Add an arg item for each trailer on the command line */
- list_for_each(pos, new_trailer_head) {
- struct new_trailer_item *tr =
- list_entry(pos, struct new_trailer_item, list);
- ssize_t separator_pos = find_separator(tr->text, cl_separators);
-
- if (separator_pos == 0) {
- struct strbuf sb = STRBUF_INIT;
- strbuf_addstr(&sb, tr->text);
- strbuf_trim(&sb);
- error(_("empty trailer token in trailer '%.*s'"),
- (int) sb.len, sb.buf);
- strbuf_release(&sb);
- } else {
- parse_trailer(tr->text, separator_pos, &tok, &val, &conf);
- add_arg_item(strbuf_detach(&tok, NULL),
- strbuf_detach(&val, NULL),
- conf,
- arg_head);
- }
- }
-
- free(cl_separators);
-}
-
static const char *next_line(const char *str)
{
const char *nl = strchrnul(str, '\n');
diff --git a/trailer.h b/trailer.h
index e01437160cf..a71b770c564 100644
--- a/trailer.h
+++ b/trailer.h
@@ -32,20 +32,6 @@ int trailer_set_where(enum trailer_where *item, const char *value);
int trailer_set_if_exists(enum trailer_if_exists *item, const char *value);
int trailer_set_if_missing(enum trailer_if_missing *item, const char *value);
-/*
- * A list that represents newly-added trailers, such as those provided
- * with the --trailer command line option of git-interpret-trailers.
- */
-struct new_trailer_item {
- struct list_head list;
-
- const char *text;
-
- enum trailer_where where;
- enum trailer_if_exists if_exists;
- enum trailer_if_missing if_missing;
-};
-
void trailer_conf_set(enum trailer_where where,
enum trailer_if_exists if_exists,
enum trailer_if_missing if_missing,
@@ -79,9 +65,6 @@ struct process_trailer_options {
void parse_trailers_from_config(struct list_head *config_head);
-void parse_trailers_from_command_line_args(struct list_head *arg_head,
- struct list_head *new_trailer_head);
-
void process_trailers_lists(struct list_head *head,
struct list_head *arg_head);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2] git-p4: use raw string literals for regular expressions
From: James Touton via GitGitGadget @ 2024-01-26 23:41 UTC (permalink / raw)
To: git; +Cc: James Touton, James Touton
In-Reply-To: <pull.1639.git.1705775149642.gitgitgadget@gmail.com>
From: James Touton <bekenn@gmail.com>
Fixes several Python diagnostics about invalid escape sequences. The
diagnostics appear for me in Python 3.12, and may appear in earlier
versions. The fix is to use raw string literals so that backslashes are
not interpreted as introducing escape sequences. Raw string literals
are already in use in this file, so adding more does not impact
toolchain compatibility.
Signed-off-by: James Touton <bekenn@gmail.com>
---
git-p4: use raw string literals for regular expressions
Changes since v1:
* Updated commit message to include the Python version where the
diagnostics were observed.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1639%2FBekenn%2Fp4-raw-strings-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1639/Bekenn/p4-raw-strings-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1639
Range-diff vs v1:
1: 1ea38dc4643 ! 1: 122ff28ffbd git-p4: use raw string literals for regular expressions
@@ Metadata
## Commit message ##
git-p4: use raw string literals for regular expressions
- Fixes several Python diagnostics about invalid escape sequences.
+ Fixes several Python diagnostics about invalid escape sequences. The
+ diagnostics appear for me in Python 3.12, and may appear in earlier
+ versions. The fix is to use raw string literals so that backslashes are
+ not interpreted as introducing escape sequences. Raw string literals
+ are already in use in this file, so adding more does not impact
+ toolchain compatibility.
Signed-off-by: James Touton <bekenn@gmail.com>
git-p4.py | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 0eb3bb4c47d..156597adb59 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -689,8 +689,8 @@ def setP4ExecBit(file, mode):
if not isModeExec(mode):
p4Type = getP4OpenedType(file)
- p4Type = re.sub('^([cku]?)x(.*)', '\\1\\2', p4Type)
- p4Type = re.sub('(.*?\+.*?)x(.*?)', '\\1\\2', p4Type)
+ p4Type = re.sub(r'^([cku]?)x(.*)', r'\1\2', p4Type)
+ p4Type = re.sub(r'(.*?\+.*?)x(.*?)', r'\1\2', p4Type)
if p4Type[-1] == "+":
p4Type = p4Type[0:-1]
@@ -701,7 +701,7 @@ def getP4OpenedType(file):
"""Returns the perforce file type for the given file."""
result = p4_read_pipe(["opened", wildcard_encode(file)])
- match = re.match(".*\((.+)\)( \*exclusive\*)?\r?$", result)
+ match = re.match(r".*\((.+)\)( \*exclusive\*)?\r?$", result)
if match:
return match.group(1)
else:
@@ -757,7 +757,7 @@ def parseDiffTreeEntry(entry):
global _diff_tree_pattern
if not _diff_tree_pattern:
- _diff_tree_pattern = re.compile(':(\d+) (\d+) (\w+) (\w+) ([A-Z])(\d+)?\t(.*?)((\t(.*))|$)')
+ _diff_tree_pattern = re.compile(r':(\d+) (\d+) (\w+) (\w+) ([A-Z])(\d+)?\t(.*?)((\t(.*))|$)')
match = _diff_tree_pattern.match(entry)
if match:
@@ -918,9 +918,9 @@ def p4CmdList(cmd, stdin=None, stdin_mode='w+b', cb=None, skip_info=False,
if len(result) > 0:
data = result[0].get('data')
if data:
- m = re.search('Too many rows scanned \(over (\d+)\)', data)
+ m = re.search(r'Too many rows scanned \(over (\d+)\)', data)
if not m:
- m = re.search('Request too large \(over (\d+)\)', data)
+ m = re.search(r'Request too large \(over (\d+)\)', data)
if m:
limit = int(m.group(1))
@@ -1452,7 +1452,7 @@ def wildcard_encode(path):
def wildcard_present(path):
- m = re.search("[*#@%]", path)
+ m = re.search(r"[*#@%]", path)
return m is not None
@@ -3048,7 +3048,7 @@ def stripRepoPath(self, path, prefixes):
# Preserve everything in relative path name except leading
# //depot/; just look at first prefix as they all should
# be in the same depot.
- depot = re.sub("^(//[^/]+/).*", r'\1', prefixes[0])
+ depot = re.sub(r"^(//[^/]+/).*", r'\1', prefixes[0])
if p4PathStartsWith(path, depot):
path = path[len(depot):]
@@ -3603,7 +3603,7 @@ def importP4Labels(self, stream, p4Labels):
commitFound = True
else:
gitCommit = read_pipe(["git", "rev-list", "--max-count=1",
- "--reverse", ":/\[git-p4:.*change = %d\]" % changelist], ignore_error=True)
+ "--reverse", r":/\[git-p4:.*change = %d\]" % changelist], ignore_error=True)
if len(gitCommit) == 0:
print("importing label %s: could not find git commit for changelist %d" % (name, changelist))
else:
@@ -4182,7 +4182,7 @@ def run(self, args):
if len(self.changesFile) == 0:
revision = "#head"
- p = re.sub("\.\.\.$", "", p)
+ p = re.sub(r"\.\.\.$", "", p)
if not p.endswith("/"):
p += "/"
@@ -4291,7 +4291,7 @@ def rebase(self):
die("Cannot find upstream branchpoint for rebase")
# the branchpoint may be p4/foo~3, so strip off the parent
- upstream = re.sub("~[0-9]+$", "", upstream)
+ upstream = re.sub(r"~[0-9]+$", "", upstream)
print("Rebasing the current branch onto %s" % upstream)
oldHead = read_pipe(["git", "rev-parse", "HEAD"]).strip()
@@ -4320,8 +4320,8 @@ def __init__(self):
def defaultDestination(self, args):
# TODO: use common prefix of args?
depotPath = args[0]
- depotDir = re.sub("(@[^@]*)$", "", depotPath)
- depotDir = re.sub("(#[^#]*)$", "", depotDir)
+ depotDir = re.sub(r"(@[^@]*)$", "", depotPath)
+ depotDir = re.sub(r"(#[^#]*)$", "", depotDir)
depotDir = re.sub(r"\.\.\.$", "", depotDir)
depotDir = re.sub(r"/$", "", depotDir)
return os.path.split(depotDir)[1]
base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d
--
gitgitgadget
^ permalink raw reply related
* What's cooking in git.git (Jan 2024, #08; Fri, 26)
From: Junio C Hamano @ 2024-01-27 1:00 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release). Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive. A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[Graduated to 'master']
* al/unit-test-ctype (2024-01-16) 1 commit
(merged to 'next' on 2024-01-19 at fcdad0d06c)
+ unit-tests: rewrite t/helper/test-ctype.c as a unit test
Move test-ctype helper to the unit-test framework.
source: <20240112102743.1440-1-ach.lumap@gmail.com>
* es/some-up-to-date-messages-must-stay (2024-01-12) 1 commit
(merged to 'next' on 2024-01-16 at 2b598f7de2)
+ messages: mark some strings with "up-to-date" not to touch
Comment updates to help developers not to attempt to modify
messages from plumbing commands that must stay constant.
It might make sense to reassess the plumbing needs every few years,
but that should be done as a separate effort.
source: <20240112171910.11131-1-ericsunshine@charter.net>
* gt/test-commit-o-i-options (2024-01-17) 2 commits
(merged to 'next' on 2024-01-19 at 0377e2c148)
+ t7501: add tests for --amend --signoff
+ t7501: add tests for --include and --only
A few tests to "git commit -o <pathspec>" and "git commit -i
<pathspec>" has been added.
source: <20240117161421.17333-1-shyamthakkar001@gmail.com>
* jt/tests-with-reftable (2024-01-12) 2 commits
(merged to 'next' on 2024-01-19 at 498d203a57)
+ t5541: remove lockfile creation
+ t1401: remove lockfile creation
Tweak a few tests not to manually modify the reference database
(hence easier to work with other backends like reftable).
source: <pull.1634.v2.git.1705004670.gitgitgadget@gmail.com>
* kh/maintenance-use-xdg-when-it-should (2024-01-18) 4 commits
(merged to 'next' on 2024-01-19 at 9c8c7b2e9d)
+ maintenance: use XDG config if it exists
+ config: factor out global config file retrieval
+ config: rename global config function
+ config: format newlines
When $HOME/.gitignore is missing but XDG config file available, we
should write into the latter, not former. "git gc" and "git
maintenance" wrote into a wrong "global config" file, which have
been corrected.
source: <cover.1705593810.git.code@khaugsbakk.name>
* la/strvec-comment-fix (2024-01-12) 1 commit
(merged to 'next' on 2024-01-19 at 120ef16493)
+ strvec: use correct member name in comments
Comment fix.
source: <pull.1640.git.1705043195997.gitgitgadget@gmail.com>
* mj/gitweb-unreadable-config-error (2024-01-10) 1 commit
(merged to 'next' on 2024-01-19 at 790b7a7855)
+ gitweb: die when a configuration file cannot be read
When given an existing but unreadable file as a configuration file,
gitweb behaved as if the file did not exist at all, but now it
errors out. This is a change that may break backward compatibility.
source: <20240110225709.30168-1-marcelo.jimenez@gmail.com>
* ne/doc-filter-blob-limit-fix (2024-01-16) 1 commit
(merged to 'next' on 2024-01-19 at 4f78975728)
+ rev-list-options: fix off-by-one in '--filter=blob:limit=<n>' explainer
Docfix.
source: <pull.1645.git.git.1705261850650.gitgitgadget@gmail.com>
* ps/commit-graph-write-leakfix (2024-01-15) 1 commit
(merged to 'next' on 2024-01-19 at df537fac39)
+ commit-graph: fix memory leak when not writing graph
Leakfix.
source: <0feab5e7d5bc6275e2c7671cd8f6786ea86fd610.1702891190.git.ps@pks.im>
* ps/completion-with-reftable-fix (2024-01-16) 5 commits
(merged to 'next' on 2024-01-19 at 8854a7d903)
+ completion: treat dangling symrefs as existing pseudorefs
+ completion: silence pseudoref existence check
+ completion: improve existence check for pseudo-refs
+ t9902: verify that completion does not print anything
+ completion: discover repo path in `__git_pseudoref_exists ()`
Completion update to prepare for reftable
source: <cover.1705314554.git.ps@pks.im>
* ps/gitlab-ci-macos (2024-01-18) 6 commits
(merged to 'next' on 2024-01-19 at a239dc8140)
+ ci: add macOS jobs to GitLab CI
+ ci: make p4 setup on macOS more robust
+ ci: handle TEST_OUTPUT_DIRECTORY when printing test failures
+ Makefile: detect new Homebrew location for ARM-based Macs
+ t7527: decrease likelihood of racing with fsmonitor daemon
+ Merge branch 'ps/gitlab-ci-static-analysis' into ps/gitlab-ci-macos
CI for GitLab learned to drive macOS jobs.
source: <cover.1705573336.git.ps@pks.im>
* ps/worktree-refdb-initialization (2024-01-08) 7 commits
(merged to 'next' on 2024-01-19 at e8c649a3ac)
+ builtin/worktree: create refdb via ref backend
+ worktree: expose interface to look up worktree by name
+ builtin/worktree: move setup of commondir file earlier
+ refs/files: skip creation of "refs/{heads,tags}" for worktrees
+ setup: move creation of "refs/" into the files backend
+ refs: prepare `refs_init_db()` for initializing worktree refs
+ Merge branch 'ps/refstorage-extension' into ps/worktree-refdb-initialization
Instead of manually creating refs/ hierarchy on disk upon a
creation of a secondary worktree, which is only usable via the
files backend, use the refs API to populate it.
source: <cover.1704705733.git.ps@pks.im>
* rj/advice-delete-branch-not-fully-merged (2024-01-11) 3 commits
(merged to 'next' on 2024-01-19 at 7102eb6b79)
+ branch: make the advice to force-deleting a conditional one
+ advice: fix an unexpected leading space
+ advice: sort the advice related lists
(this branch is used by rj/advice-disable-how-to-disable.)
The error message given when "git branch -d branch" fails due to
commits unique to the branch has been split into an error and a new
conditional advice message.
source: <4aedc15c-4b3f-4f5e-abea-581b501600f8@gmail.com>
* vd/fsck-submodule-url-test (2024-01-19) 4 commits
(merged to 'next' on 2024-01-19 at dad35ae82c)
+ submodule-config.c: strengthen URL fsck check
+ t7450: test submodule urls
+ test-submodule: remove command line handling for check-name
+ submodule-config.h: move check_submodule_url
Tighten URL checks fsck makes in a URL recorded for submodules.
source: <pull.1635.v2.git.1705542918.gitgitgadget@gmail.com>
--------------------------------------------------
[New Topics]
* jc/coc-whitespace-fix (2024-01-23) 1 commit
(merged to 'next' on 2024-01-26 at 6fb290ad59)
+ CoC: whitespace fix
Docfix.
Will merge to 'master'.
source: <xmqqmssvnb8d.fsf_-_@gitster.g>
* jc/ls-files-doc-update (2024-01-25) 1 commit
(merged to 'next' on 2024-01-26 at a71aeec3d3)
+ ls-files: avoid the verb "deprecate" for individual options
The documentation for the --exclude-per-directory option marked it
as deprecated, which confused readers into thinking there may be a
plan to remove it in the future, which was not our intention.
Will merge to 'master'.
source: <xmqqjznybfp4.fsf@gitster.g>
* jk/fetch-auto-tag-following-fix (2024-01-24) 1 commit
(merged to 'next' on 2024-01-26 at d058f1511b)
+ transport-helper: re-examine object dir after fetching
Fetching via protocol v0 over Smart HTTP transport sometimes failed
to correctly auto-follow tags.
Will merge to 'master'.
source: <20240124010056.GA2603087@coredump.intra.peff.net>
* ps/reftable-compacted-tables-permission-fix (2024-01-26) 1 commit
- reftable/stack: adjust permissions of compacted tables
Reftable bugfix.
Will merge to 'next'.
source: <a211818108053754aca002726d0206623a347952.1706263589.git.ps@pks.im>
* jc/index-pack-fsck-levels (2024-01-26) 2 commits
- index-pack: --fsck-objects to take an optional argument for fsck msgs
- index-pack: test and document --strict=<msg-id>=<severity>...
The "--fsck-objects" option of "git index-pack" now can take the
optional parameter to tweak severity of different fsck errors.
Expecting a reroll.
cf. <BF772E83-2BFE-4652-A742-67FADF3D8FE2@gmail.com>
source: <pull.1658.v3.git.git.1706302749.gitgitgadget@gmail.com>
* zf/subtree-split-fix (2024-01-25) 1 commit
(merged to 'next' on 2024-01-26 at a09e02f208)
+ subtree: fix split processing with multiple subtrees present
"git subtree" (in contrib/) update.
Will merge to 'master'.
source: <pull.1587.v6.git.1701442494319.gitgitgadget@gmail.com>
* ps/reftable-multi-level-indices-fix (2024-01-26) 6 commits
- reftable: document reading and writing indices
- reftable/writer: fix writing multi-level indices
- reftable/writer: simplify writing index records
- reftable/writer: use correct type to iterate through index entries
- reftable/reader: be more careful about errors in indexed seeks
- Merge branch 'jc/reftable-core-fsync' into ps/reftable-multi-level-indices-fix
(this branch uses jc/reftable-core-fsync.)
Write multi-level indices for reftable has been corrected.
source: <cover.1706263918.git.ps@pks.im>
--------------------------------------------------
[Cooking]
* al/t2400-depipe (2024-01-20) 1 commit
(merged to 'next' on 2024-01-22 at a20d4a9a7f)
+ t2400: avoid losing exit status to pipes
Coding style fix.
Will merge to 'master'.
source: <20240120021547.199-1-ach.lumap@gmail.com>
* kl/allow-working-in-dot-git-in-non-bare-repository (2024-01-20) 1 commit
(merged to 'next' on 2024-01-24 at e77b796e11)
+ setup: allow cwd=.git w/ bareRepository=explicit
Loosen "disable repository discovery of a bare repository" check,
triggered by setting safe.bareRepository configuration variable to
'explicit', to exclude the ".git/" directory inside a non-bare
repository from the check.
Will merge to 'master'.
source: <pull.1645.git.1705709303098.gitgitgadget@gmail.com>
* rs/parse-options-with-keep-unknown-abbrev-fix (2024-01-22) 2 commits
(merged to 'next' on 2024-01-23 at a216b482cd)
+ parse-options: simplify positivation handling
+ parse-options: fully disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN
"git diff --no-rename A B" did not disable rename detection but did
not trigger an error from the command line parser.
Will merge to 'master'.
source: <579fd5bc-3bfd-427f-b22d-dab5e7e56eb2@web.de>
source: <fb3c679a-5f00-4934-b028-6b2d081cd5b2@web.de>
* pb/ci-github-skip-logs-for-broken-tests (2024-01-22) 1 commit
(merged to 'next' on 2024-01-23 at f5e3ab2092)
+ ci(github): also skip logs of broken test cases
GitHub CI update.
Will merge to 'master'.
cf. <dbe25fff-e1d4-41f2-8f8f-c538e8c2a77e@github.com>
source: <pull.1649.git.git.1705808313306.gitgitgadget@gmail.com>
* pb/complete-log-more (2024-01-22) 4 commits
(merged to 'next' on 2024-01-24 at 081d2a92fa)
+ completion: complete missing 'git log' options
+ completion: complete --encoding
+ completion: complete --patch-with-raw
+ completion: complete missing rev-list options
The completion script (in contrib/) learned more options that can
be used with "git log".
Will merge to 'master'.
source: <pull.1650.git.git.1705810071.gitgitgadget@gmail.com>
* jc/reftable-core-fsync (2024-01-23) 1 commit
(merged to 'next' on 2024-01-24 at cea12beddb)
+ reftable: honor core.fsync
(this branch is used by ps/reftable-multi-level-indices-fix.)
The write codepath for the reftable data learned to honor
core.fsync configuration.
Will merge to 'master'.
source: <pull.1654.git.git.1706035870956.gitgitgadget@gmail.com>
* ad/custom-merge-placeholder-for-symbolic-pathnames (2024-01-24) 1 commit
(merged to 'next' on 2024-01-24 at d9cf4e227d)
+ merge-ll: expose revision names to custom drivers
The labels on conflict markers for the common ancestor, our version,
and the other version are available to custom 3-way merge driver
via %S, %X, and %Y placeholders.
Will merge to 'master'.
source: <pull.1648.v4.git.git.1706126951879.gitgitgadget@gmail.com>
* cp/unit-test-prio-queue (2024-01-22) 1 commit
- tests: move t0009-prio-queue.sh to the new unit testing framework
Migrate priority queue test to unit testing framework.
source: <pull.1642.v4.git.1705865326185.gitgitgadget@gmail.com>
* jc/reffiles-tests (2024-01-22) 12 commits
(merged to 'next' on 2024-01-24 at 0d1aaa6807)
+ t5312: move reffiles specific tests to t0601
+ t4202: move reffiles specific tests to t0600
+ t3903: make drop stash test ref backend agnostic
+ t1503: move reffiles specific tests to t0600
+ t1415: move reffiles specific tests to t0601
+ t1410: move reffiles specific tests to t0600
+ t1406: move reffiles specific tests to t0600
+ t1405: move reffiles specific tests to t0601
+ t1404: move reffiles specific tests to t0600
+ t1414: convert test to use Git commands instead of writing refs manually
+ remove REFFILES prerequisite for some tests in t1405 and t2017
+ t3210: move to t0601
Tests on ref API are moved around to prepare for reftable.
Will merge to 'master'.
cf. <Za5TW-q4cKS8pNNc@tanuki>
source: <pull.1647.v2.git.git.1705695540.gitgitgadget@gmail.com>
* ml/log-merge-with-cherry-pick-and-other-pseudo-heads (2024-01-17) 2 commits
- revision: implement `git log --merge` also for rebase/cherry_pick/revert
- revision: ensure MERGE_HEAD is a ref in prepare_show_merge
"git log --merge" learned to pay attention to CHERRY_PICK_HEAD and
other kinds of *_HEAD pseudorefs.
Comments?
source: <xmqqzfxa9usx.fsf@gitster.g>
* nb/rebase-x-shell-docfix (2024-01-17) 1 commit
(merged to 'next' on 2024-01-22 at db49e10354)
+ rebase: fix documentation about used shell in -x
Doc update.
Will merge to 'master'.
source: <20240117085347.948960-1-nik.borisov@suse.com>
* tc/show-ref-exists-fix (2024-01-18) 1 commit
(merged to 'next' on 2024-01-22 at 831452f2dd)
+ builtin/show-ref: treat directory as non-existing in --exists
Update to a new feature recently added, "git show-ref --exists".
Will merge to 'master'.
source: <20240110141559.387815-2-toon@iotcl.com>
* gt/t0024-style-fixes (2024-01-20) 2 commits
(merged to 'next' on 2024-01-22 at 36b46efbd0)
+ t0024: style fix
+ t0024: avoid losing exit status to pipes
Coding style fix.
Will merge to 'master'.
source: <20240118215407.8609-1-shyamthakkar001@gmail.com>
* jc/majordomo-to-subspace (2024-01-20) 1 commit
(merged to 'next' on 2024-01-22 at 6a95f43de4)
+ Docs: majordomo@vger.kernel.org has been decomissioned
Doc update.
Will merge to 'master'.
source: <xmqqmst1hsd6.fsf@gitster.g>
* js/oss-fuzz-build-in-ci (2024-01-19) 2 commits
(merged to 'next' on 2024-01-22 at 2954da5a39)
+ ci: build and run minimal fuzzers in GitHub CI
+ fuzz: fix fuzz test build rules
oss-fuzz tests are built and run in CI.
Will merge to 'master'.
source: <cover.1705700054.git.steadmon@google.com>
* kn/for-all-refs (2024-01-24) 4 commits
- for-each-ref: avoid filtering on empty pattern
- refs: introduce `refs_for_each_all_refs()`
- refs: extract out `loose_fill_ref_dir_regular_file()`
- refs: introduce `is_pseudoref()` and `is_headref()`
"git for-each-ref" filters its output with prefixes given from the
command line, but it did not honor an empty string to mean "pass
everything", which has been corrected.
Expecting a reroll?
cf. <xmqqfrymeega.fsf@gitster.g>
source: <20240124152726.124873-1-karthik.188@gmail.com>
* ps/not-so-many-refs-are-special (2024-01-19) 7 commits
(merged to 'next' on 2024-01-22 at f70f463847)
+ Documentation: add "special refs" to the glossary
+ refs: redefine special refs
+ refs: convert MERGE_AUTOSTASH to become a normal pseudo-ref
+ sequencer: introduce functions to handle autostashes via refs
+ refs: convert AUTO_MERGE to become a normal pseudo-ref
+ sequencer: delete REBASE_HEAD in correct repo when picking commits
+ sequencer: clean up pseudo refs with REF_NO_DEREF
Define "special ref" as a very narrow set that consists of
FETCH_HEAD and MERGE_HEAD, and clarify everything else that used to
be classified as such are actually just pseudorefs.
Will merge to 'master'.
source: <cover.1705659748.git.ps@pks.im>
* bk/complete-bisect (2024-01-18) 5 commits
- completion: git-bisect view recognized but not completed
- completion: custom git-bisect terms
- completion: move to maintain define-before-use
- completion: git-log opts to bisect visualize
- completion: complete new old actions, start opts
Command line completion support (in contrib/) has been
updated for "git bisect".
Expecting a reroll.
cf. <ZaofJhHsFjRxx7a3@tanuki>
source: <20240118204323.1113859-1-britton.kerin@gmail.com>
* bk/complete-dirname-for-am-and-format-patch (2024-01-12) 1 commit
- completion: dir-type optargs for am, format-patch
Command line completion support (in contrib/) has been
updated for a few commands to complete directory names where a
directory name is expected.
Needs review.
source: <d37781c3-6af2-409b-95a8-660a9b92d20b@smtp-relay.sendinblue.com>
* bk/complete-send-email (2024-01-12) 1 commit
- completion: don't complete revs when --no-format-patch
Command line completion support (in contrib/) has been taught to
avoid offering revision names as candidates to "git send-email" when
the command is used to send pre-generated files.
Needs review.
source: <a718b5ee-afb0-44bd-a299-3208fac43506@smtp-relay.sendinblue.com>
* la/trailer-api (2024-01-12) 10 commits
- trailer: delete obsolete argument handling code from API
- trailer: move arg handling to interpret-trailers.c
- trailer: prepare to move parse_trailers_from_command_line_args() to builtin
- trailer: spread usage of "trailer_block" language
- trailer: make trailer_info struct private
- sequencer: use the trailer iterator
- trailer: delete obsolete formatting functions
- trailer: unify trailer formatting machinery
- trailer: include "trailer" term in API functions
- trailer: move process_trailers() to interpret-trailers.c
Code clean-up.
Expecting a (hopefully final and small) reroll.
cf. <owlysf2l2bnj.fsf@fine.c.googlers.com>
source: <pull.1632.git.1704869487.gitgitgadget@gmail.com>
* ps/tests-with-ref-files-backend (2024-01-24) 6 commits
- t: mark tests regarding git-pack-refs(1) to be backend specific
- t5526: break test submodule differently
- t1419: mark test suite as files-backend specific
- t1302: make tests more robust with new extensions
- t1301: mark test for `core.sharedRepository` as reffiles specific
- t1300: make tests more robust with non-default ref backends
Prepare existing tests on refs to work better with non-default
backends.
Needs review.
source: <cover.1706085756.git.ps@pks.im>
* en/diffcore-delta-final-line-fix (2024-01-18) 1 commit
(merged to 'next' on 2024-01-22 at 7141d202cb)
+ diffcore-delta: avoid ignoring final 'line' of file
Rename detection logic ignored the final line of a file if it is an
incomplete line.
Will merge to 'master'.
source: <pull.1637.v2.git.1705119973690.gitgitgadget@gmail.com>
* ps/reftable-optimize-io (2024-01-18) 7 commits
(merged to 'next' on 2024-01-22 at b867e8b9a8)
+ reftable/stack: fix race in up-to-date check
+ reftable/stack: unconditionally reload stack after commit
(merged to 'next' on 2024-01-12 at 4096e880e0)
+ reftable/blocksource: use mmap to read tables
+ reftable/blocksource: refactor code to match our coding style
+ reftable/stack: use stat info to avoid re-reading stack list
+ reftable/stack: refactor reloading to use file descriptor
+ reftable/stack: refactor stack reloading to have common exit path
Low-level I/O optimization for reftable.
Will merge to 'master'.
source: <cover.1704966670.git.ps@pks.im>
source: <cover.1705585037.git.ps@pks.im>
* rj/advice-disable-how-to-disable (2024-01-16) 2 commits
(merged to 'next' on 2024-01-23 at f456f4937d)
+ advice: allow disabling the automatic hint in advise_if_enabled()
+ Merge branch 'rj/advice-delete-branch-not-fully-merged' into rj/advice-disable-how-to-disable
All conditional "advice" messages show how to turn them off, which
becomes repetitive. Add a configuration variable to omit the
instruction.
Will merge to 'master'.
source: <6a842ef8-b390-4739-9eef-e867d55ed5ea@gmail.com>
* sd/negotiate-trace-fix (2024-01-03) 1 commit
(merged to 'next' on 2024-01-24 at 6305853ab2)
+ push: region_leave trace for negotiate_using_fetch
Tracing fix.
Will merge to 'master'.
source: <20240103224054.1940209-1-delmerico@google.com>
* cp/apply-core-filemode (2023-12-26) 3 commits
- apply: code simplification
- apply: correctly reverse patch's pre- and post-image mode bits
- apply: ignore working tree filemode when !core.filemode
"git apply" on a filesystem without filemode support have learned
to take a hint from what is in the index for the path, even when
not working with the "--index" or "--cached" option, when checking
the executable bit match what is required by the preimage in the
patch.
Needs review.
source: <20231226233218.472054-1-gitster@pobox.com>
* ja/doc-placeholders-fix (2023-12-26) 2 commits
- doc: enforce placeholders in documentation
- doc: enforce dashes in placeholders
Docfix.
Needs review.
source: <pull.1626.git.1703539287.gitgitgadget@gmail.com>
* jc/bisect-doc (2023-12-09) 1 commit
- bisect: document "terms" subcommand more fully
Doc update.
Needs review.
source: <xmqqzfyjmk02.fsf@gitster.g>
* tb/pair-chunk-expect (2023-11-10) 8 commits
- midx: read `OOFF` chunk with `pair_chunk_expect()`
- midx: read `OIDL` chunk with `pair_chunk_expect()`
- commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
- commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
- commit-graph: read `OIDL` chunk with `pair_chunk_expect()`
- chunk-format: introduce `pair_chunk_expect()` helper
- Merge branch 'jk/chunk-bounds-more' into HEAD
Further code clean-up.
Needs review.
source: <cover.1699569246.git.me@ttaylorr.com>
* tb/path-filter-fix (2024-01-16) 17 commits
- bloom: introduce `deinit_bloom_filters()`
- commit-graph: reuse existing Bloom filters where possible
- object.h: fix mis-aligned flag bits table
- commit-graph: drop unnecessary `graph_read_bloom_data_context`
- commit-graph.c: unconditionally load Bloom filters
- bloom: prepare to discard incompatible Bloom filters
- bloom: annotate filters with hash version
- commit-graph: new Bloom filter version that fixes murmur3
- repo-settings: introduce commitgraph.changedPathsVersion
- t4216: test changed path filters with high bit paths
- t/helper/test-read-graph: implement `bloom-filters` mode
- bloom.h: make `load_bloom_filter_from_graph()` public
- t/helper/test-read-graph.c: extract `dump_graph_info()`
- gitformat-commit-graph: describe version 2 of BDAT
- commit-graph: ensure Bloom filters are read with consistent settings
- revision.c: consult Bloom filters for root commits
- t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`
The Bloom filter used for path limited history traversal was broken
on systems whose "char" is unsigned; update the implementation and
bump the format version to 2.
Will merge to 'next'.
source: <cover.1705442923.git.me@ttaylorr.com>
* ak/color-decorate-symbols (2023-10-23) 7 commits
- log: add color.decorate.pseudoref config variable
- refs: exempt pseudorefs from pattern prefixing
- refs: add pseudorefs array and iteration functions
- log: add color.decorate.ref config variable
- log: add color.decorate.symbol config variable
- log: use designated inits for decoration_colors
- config: restructure color.decorate documentation
A new config for coloring.
Needs review.
source: <20231023221143.72489-1-andy.koppe@gmail.com>
* eb/hash-transition (2023-10-02) 30 commits
- t1016-compatObjectFormat: add tests to verify the conversion between objects
- t1006: test oid compatibility with cat-file
- t1006: rename sha1 to oid
- test-lib: compute the compatibility hash so tests may use it
- builtin/ls-tree: let the oid determine the output algorithm
- object-file: handle compat objects in check_object_signature
- tree-walk: init_tree_desc take an oid to get the hash algorithm
- builtin/cat-file: let the oid determine the output algorithm
- rev-parse: add an --output-object-format parameter
- repository: implement extensions.compatObjectFormat
- object-file: update object_info_extended to reencode objects
- object-file-convert: convert commits that embed signed tags
- object-file-convert: convert commit objects when writing
- object-file-convert: don't leak when converting tag objects
- object-file-convert: convert tag objects when writing
- object-file-convert: add a function to convert trees between algorithms
- object: factor out parse_mode out of fast-import and tree-walk into in object.h
- cache: add a function to read an OID of a specific algorithm
- tag: sign both hashes
- commit: export add_header_signature to support handling signatures on tags
- commit: convert mergetag before computing the signature of a commit
- commit: write commits for both hashes
- object-file: add a compat_oid_in parameter to write_object_file_flags
- object-file: update the loose object map when writing loose objects
- loose: compatibilty short name support
- loose: add a mapping between SHA-1 and SHA-256 for loose objects
- repository: add a compatibility hash algorithm
- object-names: support input of oids in any supported hash
- oid-array: teach oid-array to handle multiple kinds of oids
- object-file-convert: stubs for converting from one object format to another
Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.
Needs review.
source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>
* jx/remote-archive-over-smart-http (2024-01-22) 6 commits
(merged to 'next' on 2024-01-23 at 5fa4633015)
+ transport-helper: call do_take_over() in process_connect
+ transport-helper: call do_take_over() in connect_helper
+ http-backend: new rpc-service for git-upload-archive
+ transport-helper: protocol v2 supports upload-archive
+ remote-curl: supports git-upload-archive service
+ transport-helper: no connection restriction in connect_helper
"git archive --remote=<remote>" learned to talk over the smart
http (aka stateless) transport.
Will merge to 'master'.
source: <cover.1705841443.git.zhiyou.jx@alibaba-inc.com>
* jc/rerere-cleanup (2023-08-25) 4 commits
- rerere: modernize use of empty strbuf
- rerere: try_merge() should use LL_MERGE_ERROR when it means an error
- rerere: fix comment on handle_file() helper
- rerere: simplify check_one_conflict() helper function
Code clean-up.
Not ready to be reviewed yet.
source: <20230824205456.1231371-1-gitster@pobox.com>
^ permalink raw reply
* Re: [PATCH v3 0/2] index-pack: fsck honor checks
From: John Cai @ 2024-01-27 2:31 UTC (permalink / raw)
To: Jonathan Tan; +Cc: Junio C Hamano, John Cai via GitGitGadget, git
In-Reply-To: <20240126221357.2940676-1-jonathantanmy@google.com>
Hi Jonathan,
On 26 Jan 2024, at 17:13, Jonathan Tan wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>>> ++--fsck-objects[=<msg-id>=<severity>...]::
>>> ++ Die if the pack contains broken objects. If the pack contains a tree
>>> ++ pointing to a .gitmodules blob that does not exist, prints the hash of
>>> ++ that blob (for the caller to check) after the hash that goes into the
>>> ++ name of the pack/idx file (see "Notes").
>>
>> Not a new problem bit I have to wonder what happens if the pack
>> contains many trees that point at different blobs for ".gitmodules"
>> path and many of these blobs are not included in the packfile? Will
>> the caller receive all of these blob object names so that they can
>> be verified? The reference to the "Notes" only refer to the fact
>> that usually a single hash value that is used in constructing the
>> name of the packfile "pack-<Hashvalue>.pack" is emitted to the
>> standard output, which is not wrong per se, but does not help
>> readers very much wrt to understanding this.
>>
>> [jc: dragging JTan into the thread, as this comes from his 5476e1ef
>> (fetch-pack: print and use dangling .gitmodules, 2021-02-22)].
>
> Ah...I can see how that documentation isn't clear. The intention of that
> commit is to check every link to a .gitmodules blob. The tests perhaps
> should have been written with 2 .gitmodules blobs (in separate commits),
> but I think the production code works: I tried changing the test to have
> 2 commits each with their own .gitmodules blob, and error messages were
> printed for both blobs.
Thanks for clarifying! Would you mind providing a patch to revise the wording
here to make it clearer? I would try but I feel like I might get the wording
wrong.
>
> (If someone changes that test, e.g. to have 2 blobs, the ">h" in the
> "configure_exclusion" invocations look superfluous and is perhaps a
> copy-and-paste error from other tests that needed the hash later.)
thanks
John
^ permalink raw reply
* [PATCH] pack-bitmap: drop unused `reuse_objects`
From: Taylor Blau @ 2024-01-27 4:00 UTC (permalink / raw)
To: git; +Cc: Jeff King, Junio C Hamano
This variable is no longer used for doing verbatim pack-reuse (or
anywhere within pack-bitmap.c) since d2ea031046 (pack-bitmap: don't rely
on bitmap_git->reuse_objects, 2019-12-18).
Remove it to avoid an unused struct member.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
I noticed this while reading code in this area working on a new topic to
store "pseudo-merge" bitmaps, which can be used to accelerate bitmap
traversals when all commits in a given pseudo-merge appear on either
side of a traversal.
pack-bitmap.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 229a11fb00..2baeabacee 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -51,13 +51,6 @@ struct bitmap_index {
struct packed_git *pack;
struct multi_pack_index *midx;
- /*
- * Mark the first `reuse_objects` in the packfile as reused:
- * they will be sent as-is without using them for repacking
- * calculations
- */
- uint32_t reuse_objects;
-
/* mmapped buffer of the whole bitmap index */
unsigned char *map;
size_t map_size; /* size of the mmaped buffer */
--
2.43.0.386.ge02ecfcc53.dirty
^ permalink raw reply related
* add two steps
From: Johannes Kingma @ 2024-01-27 9:43 UTC (permalink / raw)
To: git
For git to work at least a user.name and user.email globals are needed.
would it make sense to include this in the installation process?
^ permalink raw reply
* Re: what should "git clean -n -f [-d] [-x] <pattern>" do?
From: Junio C Hamano @ 2024-01-27 10:00 UTC (permalink / raw)
To: Sergey Organov; +Cc: Elijah Newren, git
In-Reply-To: <87ede4fg8s.fsf@osv.gnss.ru>
Sergey Organov <sorganov@gmail.com> writes:
> I'm still arguing in favor of fixing "-n", and I believe a fix is needed
> independently from decision about "-f -f".
Even though I do not personally like it, I do not think "which
between do-it (f) and do-not-do-it (n) do you want to use?" is
broken. It sometimes irritates me to find "git clean" (without "-f"
or "-n", and with clean.requireForce not disabled) complain, and I
personally think "git clean" when clean.requireForce is in effect
and no "-n" or "-f" were given should pretend as if "-n" were given.
I wish if it were "without -n or -f, we pretend as if -n were given,
possibly with a warning that says 'you need -f if you actually want
to carry out these operations'".
But that is a separate usability issue.
What I find broken is that giving one 'f' and one 'n' in different
order, i.e. "-f -n" and "-n -f", does not do what I expect. If you
are choosing between do-it (f) and do-not-do-it (n), you ought to be
able to rely on the usual last-one-wins rule. That I find broken.
The mistake[*] of "-f -f" is rather obvious, given that the other
"normal" ways to tweak what is affected by the command are done as
"what else do we clean? directories (d)? ignored (x)?..." options.
When we add the upcoming "precious" bit support, we should make sure
that the way to trigger "oh, by the way, please clobber those paths
that are marked precious, too" is not by giving three '-f'. It
would make it impossible to ask for that without also removing
nested repositories, which takes two '-f'.
[Footnote]
* To a lessor extent, the -v (verbose) option shares the same
problem as "-f -f" here, in that its worldview is to assume that
a single "verbosity level" is sufficient. Unlike the severity
level thing, however, the user who wanted to see only messages
about X but have to also see messages about Y and Z that are at
the same or lessor verbosity level as X can filter out unwanted
messages without causing a real harm.
^ permalink raw reply
* Re: add two steps
From: Beat Bolli @ 2024-01-27 12:23 UTC (permalink / raw)
To: Johannes Kingma, git
In-Reply-To: <66e4baf4-439a-4a1b-ba95-1f9ed222d42a@king.ma>
On 27.01.24 10:43, Johannes Kingma wrote:
> For git to work at least a user.name and user.email globals are needed.
> would it make sense to include this in the installation process?
>
The Git projects only distributes source code, so there's no
installation process as such that could be improved.
--
Cheers, Beat
^ permalink raw reply
* Re: [PATCH 2/4] completion: introduce __git_find_subcommand
From: Rubén Justo @ 2024-01-27 13:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <xmqqh6j0ngsb.fsf@gitster.g>
On 26-ene-2024 09:30:44, Junio C Hamano wrote:
> Rubén Justo <rjusto@gmail.com> writes:
>
> > Let's have a function to get the current subcommand when completing
> > commands that follow the syntax:
> >
> > git <command> <subcommand>
> >
> > As a convenience, let's allow an optional "default subcommand" to be
> > returned if none is found.
> >
> > Signed-off-by: Rubén Justo <rjusto@gmail.com>
> > ---
> > contrib/completion/git-completion.bash | 20 ++++++++++++++++++++
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> > index 916e137021..5f2e904b56 100644
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -575,6 +575,26 @@ __gitcomp_subcommand ()
> > fi
> > }
> >
> > +# Find the current subcommand for commands that follow the syntax:
> > +#
> > +# git <command> <subcommand>
> > +#
> > +# 1: List of possible subcommands.
> > +# 2: Optional subcommand to return when none is found.
> > +__git_find_subcommand ()
> > +{
> > + local subcommand subcommands="$1" default_subcommand="$2"
>
> Are the callers expected to form "$1" by concatenating known tokens
> with a space?
>
> I am just wondering if we can avoid looping, e.g.
>
> local nextword=${words[__git_cmd_idx+1]}
> case " $subcommands " in
> *" $nextword "*)
> echo "$nextword"
> return
> ;;
> esac
>
I like the idea; and it works:
$ words=("a" "b"); __git_cmd_idx=0; __git_find_subcommand "a b" "test"
b
$ : simulate that the user moves the cursor backwards
$ words=("a" "" "b"); __git_cmd_idx=0; __git_find_subcommand "a b" "test"
test
$ : simulate that the user moves the cursor backwards
$ words=("a" " " "b"); __git_cmd_idx=0; __git_find_subcommand "a b" "test"
test
But functions like __git_find_on_cmdline or __git-find_last_on_cmdline
are already iterating. I feel we should keep the loop.
Thank you.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox