On Mon, May 02, 2016 at 02:58:45PM -0700, Junio C Hamano wrote:
> ba3c69a9 (commit: teach --gpg-sign option, 2011-10-05) introduced a
> "signed commit" by teaching --[no-gpg-sign option and commit.gpgsign
> configuration variable to various commands that create commits.
>
> Teaching these to "git commit" and "git merge", both of which are
> end-user facing Porcelain commands, was perfectly fine. Allowing
> the plumbing "git commit-tree" to suddenly change the behaviour to
> surprise the scripts by paying attention to commit.gpgsign was not.
>
> Among the in-tree scripts, filter-branch, quiltimport, rebase and
> stash are the commands that run "commit-tree". If any of these
> wants to allow users to always sign every single commit, they should
> offer their own configuration (e.g. "filterBranch..gpgsign") with an
> option to disable (e.g. "git filter-branch --no-gpgsign").
>
> Ignoring commit.gpgsign option _obviously_ breaks the backward
> compatibility, but I seriously doubt anybody sane is depending on
> this misfeature that commit-tree blindly follows commit.gpgsign in
> any third-party script that calls it.
>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
>
> * This is an simpler alternative that forces commit-tree callers
> that want to honor commit.gpgsign to do so themselves.
I don't have any such scripts myself (aside from git-stash, whose
signing behavior is moderately annoying), but I think this simpler form
is fine. There is already an escape hatch for scripts, and it is:
if test "$(git config --bool commit.gpgsign)" = "true"; then
sign=-S
else
sign=
fi
git commit-tree $sign ...
That is a few more lines than "--use-commit-gpgsign-config", but it's
simple enough to be acceptable, and matches the same technique that
other config options need when used with plumbing.
So I think the motivation and premise are good, but...
> -static int commit_tree_config(const char *var, const char *value, void *cb)
> -{
> - int status = git_gpg_config(var, value, NULL);
> - if (status)
> - return status;
> - if (!strcmp(var, "commit.gpgsign")) {
> - sign_commit = git_config_bool(var, value) ? "" : NULL;
> - return 0;
> - }
> - return git_default_config(var, value, cb);
> -}
> -
I think this may be going too far. If I do "git commit-tree -S", I'd
expect it to use gpg.program, but here you are dropping the call to
git_gpg_config. Likewise for user.signingkey.
So I think you just want to drop the commit.gpgsign block here, and keep
the rest.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html