threads / patch / 63453

patchcommit: Add commit.signoff configuration option

Subject: [PATCH] commit: Add commit.signoff configuration option

## tl;dr

14 messages between May 13, 2025 and Jun 9, 2025. Diffs are folded; open one to read it.

replies: 13people: 5as markdown or json

Chris Down· May 13, 2025, 12:20 UTC · lore

Introduce a new `commit.signoff` config variable that mirrors the behavior of the -s/--signoff flag.

We already have prior art in format-patch with `format.signoff`; this brings parity for those who don’t use a patch-based workflow but still rely on signoffs.

Right now people who have to do this regularly often alias commit to `commit --signoff` in their shell, which is less than ideal -- this config option avoids having to do that.

Signed-off-by: Chris Down <chris@chrisdown.name>
---
 Documentation/signoff-option.adoc         |  4 ++++
 builtin/commit.c                          |  4 ++++
 t/t7500-commit-template-squash-signoff.sh | 10 ++++++++++
 3 files changed, 18 insertions(+)
Show changes to 3 files +18 −0

Documentation/signoff-option.adoc, builtin/commit.c, t/t7500-commit-template-squash-signoff.sh

diff --git a/Documentation/signoff-option.adoc b/Documentation/signoff-option.adoc
index cddfb225d1..0055874e84 100644
--- a/Documentation/signoff-option.adoc
+++ b/Documentation/signoff-option.adoc
@@ -13,6 +13,10 @@ endif::git-commit[]
 	Linux kernel and Git projects.)  Consult the documentation or
 	leadership of the project to which you're contributing to
 	understand how the signoffs are used in that project.
+ifdef::git-commit[]
+	The `commit.signoff` configuration variable may also be used to imply
+	`--signoff`.
+endif::git-commit[]
 +
 The `--no-signoff` option can be used to countermand an earlier `--signoff`
 option on the command line.
diff --git a/builtin/commit.c b/builtin/commit.c
index 66bd91fd52..da98895438 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1670,6 +1670,10 @@ static int git_commit_config(const char *k, const char *v,
 							       &is_bool);
 		return 0;
 	}
+	if (!strcmp(k, "commit.signoff")) {
+		signoff = git_config_bool(k, v);
+		return 0;
+	}
 
 	return git_status_config(k, v, ctx, s);
 }
diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh
index 4dca8d97a7..03c20dcb1d 100755
--- a/t/t7500-commit-template-squash-signoff.sh
+++ b/t/t7500-commit-template-squash-signoff.sh
@@ -181,6 +181,16 @@ test_expect_success '--signoff' '
 	test_cmp expect output
 '
 
+test_expect_success 'config commit.signoff implies signoff' '
+	git config commit.signoff true &&
+	echo "871119" >> bar &&
+	git add bar &&
+	echo "zort" | git commit -F - bar &&
+	git cat-file commit HEAD | sed "1,/^\$/d" > output &&
+	test_cmp expect output &&
+	git config --unset commit.signoff
+'
+
 test_expect_success 'commit message from file (1)' '
 	mkdir subdir &&
 	echo "Log in top directory" >log &&
-- 
2.49.0
D. Ben Knoble· May 13, 2025, 21:09 UTC · re: Chris Down · lore

Re: [PATCH] commit: Add commit.signoff configuration option

On Tue, May 13, 2025 at 8:21 AM Chris Down <chris@chrisdown.name> wrote:
Show 11 quoted lines
>
> Introduce a new `commit.signoff` config variable that mirrors the
> behavior of the -s/--signoff flag.
>
> We already have prior art in format-patch with `format.signoff`; this
> brings parity for those who don’t use a patch-based workflow but still
> rely on signoffs.
>
> Right now people who have to do this regularly often alias commit to
> `commit --signoff` in their shell, which is less than ideal -- this
> config option avoids having to do that.

It would probably be nice to say why this makes sense in light of previously-raised objections, too [1].

[1]: https://lore.kernel.org/git/xmqqo6x4p6z2.fsf@gitster.g/
Show 48 quoted lines
>
> Signed-off-by: Chris Down <chris@chrisdown.name>
> ---
>  Documentation/signoff-option.adoc         |  4 ++++
>  builtin/commit.c                          |  4 ++++
>  t/t7500-commit-template-squash-signoff.sh | 10 ++++++++++
>  3 files changed, 18 insertions(+)
>
> diff --git a/Documentation/signoff-option.adoc b/Documentation/signoff-option.adoc
> index cddfb225d1..0055874e84 100644
> --- a/Documentation/signoff-option.adoc
> +++ b/Documentation/signoff-option.adoc
> @@ -13,6 +13,10 @@ endif::git-commit[]
>         Linux kernel and Git projects.)  Consult the documentation or
>         leadership of the project to which you're contributing to
>         understand how the signoffs are used in that project.
> +ifdef::git-commit[]
> +       The `commit.signoff` configuration variable may also be used to imply
> +       `--signoff`.
> +endif::git-commit[]
>  +
>  The `--no-signoff` option can be used to countermand an earlier `--signoff`
>  option on the command line.
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 66bd91fd52..da98895438 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1670,6 +1670,10 @@ static int git_commit_config(const char *k, const char *v,
>                                                                &is_bool);
>                 return 0;
>         }
> +       if (!strcmp(k, "commit.signoff")) {
> +               signoff = git_config_bool(k, v);
> +               return 0;
> +       }
>
>         return git_status_config(k, v, ctx, s);
>  }
> diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh
> index 4dca8d97a7..03c20dcb1d 100755
> --- a/t/t7500-commit-template-squash-signoff.sh
> +++ b/t/t7500-commit-template-squash-signoff.sh
> @@ -181,6 +181,16 @@ test_expect_success '--signoff' '
>         test_cmp expect output
>  '
>
> +test_expect_success 'config commit.signoff implies signoff' '
> +       git config commit.signoff true &&

This should use test_config, I think. Otherwise, the config will stick around if the test fails. You /could/ use test_when_finished, but test_config sets that up for you.

Show 5 quoted lines
> +       echo "871119" >> bar &&
> +       git add bar &&
> +       echo "zort" | git commit -F - bar &&
> +       git cat-file commit HEAD | sed "1,/^\$/d" > output &&
> +       test_cmp expect output &&

Took me a bit to realize (since it doesn't show in the context); "expect" is setup outside the previous signoff test and then (re)used here. Reasonable, but we now have commit_msg_is in this test script. Perhaps worth a quick refactor patch preceding this one to make it use that function? Probably not required, though?

Show 10 quoted lines
> +       git config --unset commit.signoff
> +'
> +
>  test_expect_success 'commit message from file (1)' '
>         mkdir subdir &&
>         echo "Log in top directory" >log &&
> --
> 2.49.0
>
>
-- 
D. Ben Knoble
Chris Down· May 14, 2025, 16:46 UTC · re: D. Ben Knoble · lore

Re: [PATCH] commit: Add commit.signoff configuration option

[Code changes acked, thank you for the review :-)]
D. Ben Knoble writes:
>It would probably be nice to say why this makes sense in light of
>previously-raised objections, too [1].
>
>[1]: https://lore.kernel.org/git/xmqqo6x4p6z2.fsf@gitster.g/

I understand where people are coming from for sure, but I think the conversation has moved on beyond many of those points, right? For example, some of the objections are about format.signoff in 2006, but we merged that into the tree since 2009 in commit 1d1876e9300c ("Add configuration variable for sign-off to format-patch").

 From those threads, the main arguments seem to be:
- Signoff should be a conscious act
- Adding it automatically might dilute its meaning
- The potential for signoffs through inertia without proper intent
However, I think these objections don't hold up well in light of a few things.

First, the objections conflict with precedent from the now merged format.signoff. If we've already determined that configuring automatic signoffs in one context is acceptable (where that will then later become a commit anyway), it creates inconsistency to reject the same capability in another closely related context.

Second, setting a configuration value _is_ a conscious act. When I enable commit.signoff=true in a repository, I'm making a deliberate, repository specific declaration that I have the rights to submit all work in this repository under the project's license. This is arguably even more meaningful than mechanically adding -s to individual commits, either without thought through muscle memory, or through a shell based, context agnostic alias as many users do now.

In general, it feels like an inconsistency in semantics to have format.signoff available but not commit.signoff. From a user's perspective, we're just adding a signoff trailer to something that will eventually become an upstream commit. It doesn't really matter whether that's through a mailing list (format-patch), a pull request (commit), or otherwise.

I'm certainly open to further discussion, of course, but the existing precedent seems to make a compelling case for this feature.

Junio C Hamano· May 15, 2025, 00:17 UTC · re: Chris Down · lore

Re: [PATCH] commit: Add commit.signoff configuration option

Chris Down <chris@chrisdown.name> writes:
> ... For
> example, some of the objections are about format.signoff in 2006, but
> we merged that into the tree since 2009 in commit 1d1876e9300c ("Add
> configuration variable for sign-off to format-patch").

But an old mistake is never an excuse that we can pile more mistakes of the same kind on top. Otherwise we wouldn't have learned anything.

Chris Down· May 15, 2025, 13:22 UTC · re: Junio C Hamano · lore

Re: [PATCH] commit: Add commit.signoff configuration option

Junio C Hamano writes:
>But an old mistake is never an excuse that we can pile more mistakes
>of the same kind on top.  Otherwise we wouldn't have learned anything.

I'm curious about what specific issues format.signoff has caused that make you consider it a mistake. Has there been evidence over the past 16 years of it undermining the significance of signoffs or creating other problems?

Junio C Hamano· May 16, 2025, 13:09 UTC · re: Chris Down · lore

Re: [PATCH] commit: Add commit.signoff configuration option

Chris Down <chris@chrisdown.name> writes:
Show 8 quoted lines
> Junio C Hamano writes:
>>But an old mistake is never an excuse that we can pile more mistakes
>>of the same kind on top.  Otherwise we wouldn't have learned anything.
>
> I'm curious about what specific issues format.signoff has caused that
> make you consider it a mistake. Has there been evidence over the past
> 16 years of it undermining the significance of signoffs or creating
> other problems?

Regarding other problems, the fact that we are having this discussion is indication enough, isn't it? If it didn't exist, we wouldn't have had folks who used it as an excuse to promote commit.signoff in the first place.

Chris Down· May 16, 2025, 15:04 UTC · re: Junio C Hamano · lore

Re: [PATCH] commit: Add commit.signoff configuration option

Junio C Hamano writes:
>Regarding other problems, the fact that we are having this
>discussion is indication enough, isn't it?  If it didn't exist, we
>wouldn't have had folks who used it as an excuse to promote
>commit.signoff in the first place.

I definitely understand your concern about diluting the meaning of signoffs. What separates this patch is that it's not suggesting a config option out of feature parity, but because enabling commit.signoff is itself a deliberate, repository specific certification, which is arguably more meaningful than habitually typing -s for each commit or using a global shell alias.

The repository specificity is particularly important here. Currently users work around this with shell aliases, which are global rather than scoped to a repository, making commit.signoff in practice a more precise mechanism for expressing their intent about what to certify.

Would strengthening the documentation to emphasise that setting commit.signoff is a deliberate, repository-specific certification address your concern about diluting the meaning of signoffs?

Chris Down· Jun 3, 2025, 06:54 UTC · re: Chris Down · lore

Re: [PATCH] commit: Add commit.signoff configuration option

Hi Junio,

I wanted to follow up on the point around repository specificity, since that seems pretty key to the discussion here.

Looking at public dotfiles, many users already work around this with global shell aliases for "commit --signoff". This creates the exact problem (I believe?) you're concerned about: casual, low intent signoffs without deliberate per-repository intent to certify anything.

A repository specific commit.signoff would be more precise than the current workarounds, since it would require explicit configuration per repository rather than blanket global behavior or muscle memory.

Would only allowing using this flag in a repository context (and not the global git config) allay your concerns?

Thanks,
Chris
Junio C Hamano· Jun 4, 2025, 13:32 UTC · re: Chris Down · lore

Re: [PATCH] commit: Add commit.signoff configuration option

Chris Down <chris@chrisdown.name> writes:
> Looking at public dotfiles, many users already work around this with
> global shell aliases for "commit --signoff". This creates the exact
> problem

Users conciously using a general customization mechanism to express specific intent like the above is one thing. Project giving users a tool that is specifically designed to casually set and forget before even understanding the implications is another.

So no, anything that specifically targets commit.signoff would not fall into the same category as end-users creating aliases for themselves to use, I would have to say. And we do not want to give an impression that we give tools specifically designed to encourage users making casual sign-offs.

Collin Funk· Jun 5, 2025, 01:46 UTC · re: Junio C Hamano · lore

Re: [PATCH] commit: Add commit.signoff configuration option

Junio C Hamano <gitster@pobox.com> writes:
Show 16 quoted lines
> Chris Down <chris@chrisdown.name> writes:
>
>> Looking at public dotfiles, many users already work around this with
>> global shell aliases for "commit --signoff". This creates the exact
>> problem
>
> Users conciously using a general customization mechanism to express
> specific intent like the above is one thing.  Project giving users a
> tool that is specifically designed to casually set and forget before
> even understanding the implications is another.
>
> So no, anything that specifically targets commit.signoff would not
> fall into the same category as end-users creating aliases for
> themselves to use, I would have to say.  And we do not want to give
> an impression that we give tools specifically designed to encourage
> users making casual sign-offs.
I agree.

Although the FSF copyright assignment needed to contribute to some GNU projects is time-consuming, it forces the contributor to consider "Do I own the rights to the code I create?" This isn't the case in many US states if you are an exempt employee, which most Software Developers are.

Similarly, when you sign-off to the DCO you also acknowledge that you have the rights to license your work under the projects license. By adding this option, I worry that contributors will just configure it globally as a prerequisite to their work being accepted, and not consider whether they are even allowed to make the contribution in the first place.

Collin
Ben Knoble· May 16, 2025, 11:50 UTC · re: Chris Down · lore

Re: [PATCH] commit: Add commit.signoff configuration option

> Le 14 mai 2025 à 12:46, Chris Down <chris@chrisdown.name> a écrit :
> 
> I understand where people are coming from for sure, but I think the conversation has moved on beyond many of those points, right? For example, some of the objections are about format.signoff in 2006, but we merged that into the tree since 2009 in commit 1d1876e9300c ("Add configuration variable for sign-off to format-patch").
Just in case it wasn’t clear: I think the patch is reasonable ;) I only meant that Junio had provided some background material that should probably be addressed, and now you have. Thanks!
Chris Down· May 16, 2025, 14:28 UTC · re: Ben Knoble · lore

Re: [PATCH] commit: Add commit.signoff configuration option

Ben Knoble writes:
>Just in case it wasn’t clear: I think the patch is reasonable ;) I only meant 
>that Junio had provided some background material that should probably be 
>addressed, and now you have. Thanks!

I didn't think that at all :-) I thought it was really helpful that you pointed these out, thank you.

← back to recent threads