Re: [PATCH] commit-tree: do not pay attention to commit.gpgsign

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 <redacted>
---

 * 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