On 20.12.2021 13:54, Junio C Hamano wrote:
Fabian Stelzer <fs@xxxxxxxxxxxx> writes:
+--crypto-sign[=<keyid>]::
+--no-crypto-sign::
--gpg-sign[=<keyid>]::
--no-gpg-sign::
- GPG-sign commits. The `keyid` argument is optional and
- defaults to the committer identity; if specified, it must be
- stuck to the option without a space. `--no-gpg-sign` is useful to
- countermand both `commit.gpgSign` configuration variable, and
- earlier `--gpg-sign`.
+ Cryptographically sign commits. The `keyid` argument is optional and
+ its default depends on the configured `cryptoSign.format`; if specified,
+ it must be stuck to the option without a space. `--no-crypto-sign` is
+ useful to countermand both `commit.gpgSign` configuration variable, and
+ earlier `--crypto-sign`.
+ `--(no-)gpg-sign` is a compatibility alias and has no effect on which
+ cryptographic format will be used. This is determined by the
+ configuration variable cryptoSign.format (see linkgit:git-config[1]).
I'd make the last three lines into a separate paragraph and nudge
users toward the new spelling if I were doing this change, e.g.
...
earlier `--crypto-sign`.
+
The option was originally called `--[no-]gpg-sign` and is still
supported as a synonym, but it is encouraged to migrate to use
the `--crypto-sign` option.
Will do.
Not the problem with this patch, but can the format be inferred from
<keyid>?
If so, `--crypt-sign=<keyid of format X>` can choose the format X
and specify what exact key to use at the same time without the
cryptosign.format configuration variable. But if not, the interface
leaves us in an awkward place by letting different keys easily
specified from the command line while making it impossible to
switch between GPG and SSH from the command line.
I thought about doing this when we added the key:: prefix to differentiate
between literal keys and file paths for ssh. Something like
ssh-key::/ssh-file::/gpg-key::
I decided against it since I think this could lead to different to
understand behaviour for the user. It is quite clear that a flag takes
precedence over a config var, but what if i set `format=gpg` and then
`user.signingkey=ssh-key::...`?
If we would start this from a green field and did not have the format
setting, then just inferring from the key would be ok. But with the backward
compatibility I think this is too confusing.
If --gpg-sign is not a mere synonym, but also implies GPG is
preferred when cryptoSign.format is not specified, that is a
different story, of course. That makes it unnecessary to deprecate
`--gpg-sign` and in addition we need to add `--ssh-sign` option that
works similarly, which may not scale well but I do not expect we'd
add many more next to GPG and SSH, hopefully? I dunno.
This is indeed a path we could take. Here the flag would have precedence
over the config. However it would not make sense to just specify --ssh-sign
when `user.signingkey` is set to a gpg key id. So the user would always have
to specify the key or we would need to move the signing key config into the
format specific blocks leading to even more compatibility code paths :/
I'm going to make some bold assumptions here, so please correct me if I'm
wrong.
Most users will not need to switch signing methods within repositories, if
at all. Corporate users will probably adopt ssh, if anything at all. I have
never seen gpg consistently used in a corporate setup. Open source
development is heavily leaning on gpg. So the most common case will probably
be users having to switch between work (ssh) & oss (gpg) development which
can either be configured per repository or by using `includeIf "gitdir:"` in
your .gitconfig.
So I think the extra flags or extended key prefixes to infer the format are
not needed and we can choose the simple option of only having the single
cryptosign.format config.
diff --git a/builtin/commit.c b/builtin/commit.c
index 883c16256c..2c789ff6f9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1639,8 +1639,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),
OPT_CLEANUP(&cleanup_arg),
OPT_BOOL(0, "status", &include_status, N_("include status in commit message template")),
- { OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
+ { OPTION_STRING, 'S', "crypto-sign", &sign_commit, N_("key-id"),
+ N_("cryptographically sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
+ { OPTION_STRING, 0, "gpg-sign", &sign_commit, N_("key-id"),
N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
Leaving this explained as "GPG sign commit" contradicts "it is a
mere alias that does not even imply GPG is preferred when no
preference is given by the cryptoSign.format variable".
True, will fix.
+
Unwanted blank line before the "this is the last line of these
options" marker?
/* end commit message options */
OPT_GROUP(N_("Commit contents options")),
