gpg-interface: lazily initialize and read the configuration

Instead of forcing the porcelain commands to always read the
configuration variables related to the signing and verifying
signatures, lazily initialize the necessary subsystem on demand upon
the first use.

This hopefully would make it more future-proof as we do not have to
think and decide whether we should call git_gpg_config() in the
git_config() callback for each command.

A few git_config() callback functions that used to be custom
callbacks are now just a thin wrapper around git_default_config().
We could further remove, git_FOO_config and replace calls to
git_config(git_FOO_config) with git_config(git_default_config), but
to make it clear which ones are affected and the effect is only the
removal of git_gpg_config(), it is vastly preferred not to do such a
change in this step (they can be done on top once the dust settled).

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/am.c b/builtin/am.c
index 82a41cbfc4e9ce2538644b9247dfc3a3bbe60888..40126b59c541710435e6c56179e5168d7576f88b 100644 (file)
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2314,12 +2314,6 @@ static int parse_opt_show_current_patch(const struct option *opt, const char *ar
 
 static int git_am_config(const char *k, const char *v, void *cb UNUSED)
 {
-       int status;
-
-       status = git_gpg_config(k, v, NULL);
-       if (status)
-               return status;
-
        return git_default_config(k, v, NULL);
 }
 

diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index cc8d584be2f4a9d3b0c950a4b8cbc6c140904d8c..f6a099d601c4a0c4984757129081c551d1a01f4c 100644 (file)
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -39,9 +39,6 @@ static void new_parent(struct commit *parent, struct commit_list **parents_p)
 
 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;
        return git_default_config(var, value, cb);
 }
 

diff --git a/builtin/commit.c b/builtin/commit.c
index 44b763d7cd025db45581d60902502ae2a2a9fe11..794500dc9e4a68d292ed875ac7dec1d8ad6b1b63 100644 (file)
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1600,7 +1600,6 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 static int git_commit_config(const char *k, const char *v, void *cb)
 {
        struct wt_status *s = cb;
-       int status;
 
        if (!strcmp(k, "commit.template"))
                return git_config_pathname(&template_file, k, v);
@@ -1620,9 +1619,6 @@ static int git_commit_config(const char *k, const char *v, void *cb)
                return 0;
        }
 
-       status = git_gpg_config(k, v, NULL);
-       if (status)
-               return status;
        return git_status_config(k, v, s);
 }
 

diff --git a/builtin/log.c b/builtin/log.c
index 04412dd9c936569506c1ca932a7804059a57904f..56741c6d37e3f9154d61d83486a73540f5cd6bc0 100644 (file)
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -601,8 +601,6 @@ static int git_log_config(const char *var, const char *value, void *cb)
                return 0;
        }
 
-       if (git_gpg_config(var, value, cb) < 0)
-               return -1;
        return git_diff_ui_config(var, value, cb);
 }
 

diff --git a/builtin/merge.c b/builtin/merge.c
index 74de2ebd2b33f967ff85988779bbf85819fe9a3c..289c13656c13a49fa1f7c0c818046957bb47dd28 100644 (file)
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -659,9 +659,6 @@ static int git_merge_config(const char *k, const char *v, void *cb)
        }
 
        status = fmt_merge_msg_config(k, v, cb);
-       if (status)
-               return status;
-       status = git_gpg_config(k, v, NULL);
        if (status)
                return status;
        return git_diff_ui_config(k, v, cb);

diff --git a/builtin/pull.c b/builtin/pull.c
index 1ab4de0005da3f6d2aa4b6a93d61544818af8700..4367727db62836982884a44b149668a5dd5a19e4 100644 (file)
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -359,8 +359,6 @@ static enum rebase_type config_get_rebase(int *rebase_unspecified)
  */
 static int git_pull_config(const char *var, const char *value, void *cb)
 {
-       int status;
-
        if (!strcmp(var, "rebase.autostash")) {
                config_autostash = git_config_bool(var, value);
                return 0;
@@ -372,10 +370,6 @@ static int git_pull_config(const char *var, const char *value, void *cb)
                check_trust_level = 0;
        }
 
-       status = git_gpg_config(var, value, cb);
-       if (status)
-               return status;
-
        return git_default_config(var, value, cb);
 }
 

diff --git a/builtin/push.c b/builtin/push.c
index 60ac8017e521f0c7c9192df89c797e1898ec60fd..8f8904dd08050b450be3125070f7431befd7c1d6 100644 (file)
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -502,11 +502,6 @@ static int git_push_config(const char *k, const char *v, void *cb)
 {
        const char *slot_name;
        int *flags = cb;
-       int status;
-
-       status = git_gpg_config(k, v, NULL);
-       if (status)
-               return status;
 
        if (!strcmp(k, "push.followtags")) {
                if (git_config_bool(k, v))

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index a90af3036303f1c6d8261559c7309fe9934874c7..9894dbdc7910f45a2e070ec8953776bde8d92460 100644 (file)
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -133,10 +133,6 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
 {
        int status = parse_hide_refs_config(var, value, "receive", &hidden_refs);
 
-       if (status)
-               return status;
-
-       status = git_gpg_config(var, value, NULL);
        if (status)
                return status;
 

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 4c5d125fa0a1cb4a01deac04637bd4e71763c619..c31e27346bec2ef2dfb0de046b7a6667d9451631 100644 (file)
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -130,8 +130,6 @@ static void print_helper_status(struct ref *ref)
 
 static int send_pack_config(const char *k, const char *v, void *cb)
 {
-       git_gpg_config(k, v, NULL);
-
        if (!strcmp(k, "push.gpgsign")) {
                const char *value;
                if (!git_config_get_value("push.gpgsign", &value)) {

diff --git a/builtin/tag.c b/builtin/tag.c
index d428c45dc8d812255336ed5da20804981d3be4ab..725cfcd62b980f90d3ac4c173867f85291422478 100644 (file)
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -180,8 +180,6 @@ static const char tag_template_nocleanup[] =
 
 static int git_tag_config(const char *var, const char *value, void *cb)
 {
-       int status;
-
        if (!strcmp(var, "tag.gpgsign")) {
                config_sign_tag = git_config_bool(var, value);
                return 0;
@@ -194,9 +192,6 @@ static int git_tag_config(const char *var, const char *value, void *cb)
                return 0;
        }
 
-       status = git_gpg_config(var, value, cb);
-       if (status)
-               return status;
        if (!strcmp(var, "tag.forcesignannotated")) {
                force_sign_annotate = git_config_bool(var, value);
                return 0;

diff --git a/builtin/verify-commit.c b/builtin/verify-commit.c
index 3ebad32b0f14fffb5cfafce87411a534bccdf724..3c5d0b024c9a58a27ef317e767c43177dc980f69 100644 (file)
--- a/builtin/verify-commit.c
+++ b/builtin/verify-commit.c
@@ -54,9 +54,6 @@ static int verify_commit(const char *name, unsigned flags)
 
 static int git_verify_commit_config(const char *var, const char *value, void *cb)
 {
-       int status = git_gpg_config(var, value, cb);
-       if (status)
-               return status;
        return git_default_config(var, value, cb);
 }
 

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index 217566952d83ce0c657128d6d5e253bebc72f3bb..ecffb069bf15f75e49d3f3d205f6fc18d0b42346 100644 (file)
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -21,9 +21,6 @@ static const char * const verify_tag_usage[] = {
 
 static int git_verify_tag_config(const char *var, const char *value, void *cb)
 {
-       int status = git_gpg_config(var, value, cb);
-       if (status)
-               return status;
        return git_default_config(var, value, cb);
 }
 

diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
index f48f44f9cd1dbdd3aeafbe5a4bd3598e783d0e25..9b83c95a08c7cefed61d04e46dd72773e6cfe927 100644 (file)
--- a/fmt-merge-msg.c
+++ b/fmt-merge-msg.c
@@ -17,8 +17,6 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;
 
 int fmt_merge_msg_config(const char *key, const char *value, void *cb)
 {
-       int status = 0;
-
        if (!strcmp(key, "merge.log") || !strcmp(key, "merge.summary")) {
                int is_bool;
                merge_log_config = git_config_bool_or_int(key, value, &is_bool);
@@ -37,9 +35,6 @@ int fmt_merge_msg_config(const char *key, const char *value, void *cb)
                        string_list_append(&suppress_dest_patterns, value);
                suppress_dest_pattern_seen = 1;
        } else {
-               status = git_gpg_config(key, value, NULL);
-               if (status)
-                       return status;
                return git_default_config(key, value, cb);
        }
        return 0;

diff --git a/gpg-interface.c b/gpg-interface.c
index 687236430bf21c7c64d9a13128af016cf579d3c3..404d4cccf34ed39d4feb17e434e9ca4535867749 100644 (file)
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -9,6 +9,18 @@
 #include "tempfile.h"
 #include "alias.h"
 
+static int git_gpg_config(const char *, const char *, void *);
+
+static void gpg_interface_lazy_init(void)
+{
+       static int done;
+
+       if (done)
+               return;
+       done = 1;
+       git_config(git_gpg_config, NULL);
+}
+
 static char *configured_signing_key;
 static const char *ssh_default_key_command, *ssh_allowed_signers, *ssh_revocation_file;
 static enum signature_trust_level configured_min_trust_level = TRUST_UNDEFINED;
@@ -632,6 +644,8 @@ int check_signature(struct signature_check *sigc,
        struct gpg_format *fmt;
        int status;
 
+       gpg_interface_lazy_init();
+
        sigc->result = 'N';
        sigc->trust_level = -1;
 
@@ -695,11 +709,13 @@ int parse_signature(const char *buf, size_t size, struct strbuf *payload, struct
 
 void set_signing_key(const char *key)
 {
+       gpg_interface_lazy_init();
+
        free(configured_signing_key);
        configured_signing_key = xstrdup(key);
 }
 
-int git_gpg_config(const char *var, const char *value, void *cb UNUSED)
+static int git_gpg_config(const char *var, const char *value, void *cb UNUSED)
 {
        struct gpg_format *fmt = NULL;
        char *fmtname = NULL;
@@ -888,6 +904,8 @@ static const char *get_ssh_key_id(void) {
 /* Returns a textual but unique representation of the signing key */
 const char *get_signing_key_id(void)
 {
+       gpg_interface_lazy_init();
+
        if (use_format->get_key_id) {
                return use_format->get_key_id();
        }
@@ -898,6 +916,8 @@ const char *get_signing_key_id(void)
 
 const char *get_signing_key(void)
 {
+       gpg_interface_lazy_init();
+
        if (configured_signing_key)
                return configured_signing_key;
        if (use_format->get_default_key) {
@@ -923,6 +943,8 @@ const char *gpg_trust_level_to_str(enum signature_trust_level level)
 
 int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)
 {
+       gpg_interface_lazy_init();
+
        return use_format->sign_buffer(buffer, signature, signing_key);
 }
 

diff --git a/gpg-interface.h b/gpg-interface.h
index 8a9ef41779e2fe91305dcb1b13cf66a282446cf6..143cdc1c02d4b31fa08f92fbcae1691c80d2c2d6 100644 (file)
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -79,7 +79,6 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
  */
 const char *gpg_trust_level_to_str(enum signature_trust_level level);
 
-int git_gpg_config(const char *, const char *, void *);
 void set_signing_key(const char *);
 const char *get_signing_key(void);
 

diff --git a/sequencer.c b/sequencer.c
index 3e4a197289714197b6ba391aa1bb26652eaaf836..a234b1ff88926d3ccf044caa6360e9b4c89cae72 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -263,10 +263,6 @@ static int git_sequencer_config(const char *k, const char *v, void *cb)
        if (opts->action == REPLAY_REVERT && !strcmp(k, "revert.reference"))
                opts->commit_use_reference = git_config_bool(k, v);
 
-       status = git_gpg_config(k, v, NULL);
-       if (status)
-               return status;
-
        return git_diff_basic_config(k, v, NULL);
 }
 

diff --git a/t/t7031-verify-tag-signed-ssh.sh b/t/t7031-verify-tag-signed-ssh.sh
index 36eb86a4b19f3c758b2830fe5157a1667069321a..20913b371344264a6dc9c84b58c3b6de3b46e226 100755 (executable)
--- a/t/t7031-verify-tag-signed-ssh.sh
+++ b/t/t7031-verify-tag-signed-ssh.sh
@@ -200,4 +200,14 @@ test_expect_success GPGSSH 'verifying a forged tag with --format should fail sil
        test_must_be_empty actual-forged
 '
 
+test_expect_success GPGSSH 'rev-list --format=%G' '
+       test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+       git rev-list -1 --format="%G? %H" sixth-signed >actual &&
+       cat >expect <<-EOF &&
+       commit $(git rev-parse sixth-signed^0)
+       G $(git rev-parse sixth-signed^0)
+       EOF
+       test_cmp expect actual
+'
+
 test_done