Merge branch 'jc/gpg-lazy-init'

gitster · gitster · commit af5388d2ddb0 · 2023-03-17T14:03:10.000-07:00
Instead of forcing each command to choose to honor GPG related
configuration variables, make the subsystem lazily initialize
itself.

* jc/gpg-lazy-init:
  drop pure pass-through config callbacks
  gpg-interface: lazily initialize and read the configuration
diff --git a/builtin/am.c b/builtin/am.c
@@ -2301,17 +2301,6 @@ static int parse_opt_show_current_patch(const struct option *opt, const char *ar
 	return 0;
 }
 
-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);
-}
-
 int cmd_am(int argc, const char **argv, const char *prefix)
 {
 	struct am_state state;
@@ -2435,7 +2424,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage_with_options(usage, options);
 
-	git_config(git_am_config, NULL);
+	git_config(git_default_config, NULL);
 
 	am_state_init(&state);
 
diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
@@ -38,14 +38,6 @@ static void new_parent(struct commit *parent, struct commit_list **parents_p)
 	commit_list_insert(parent, 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);
-}
-
 static int parse_parent_arg_callback(const struct option *opt,
 		const char *arg, int unset)
 {
@@ -122,7 +114,7 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	git_config(commit_tree_config, NULL);
+	git_config(git_default_config, NULL);
 
 	if (argc < 2 || !strcmp(argv[1], "-h"))
 		usage_with_options(commit_tree_usage, options);
diff --git 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
@@ -603,8 +603,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
@@ -661,9 +661,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
@@ -360,8 +360,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;
@@ -373,10 +371,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
@@ -508,11 +508,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
@@ -134,10 +134,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
@@ -131,8 +131,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
@@ -181,8 +181,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;
@@ -195,9 +193,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
@@ -52,14 +52,6 @@ static int verify_commit(const char *name, unsigned flags)
 	return run_gpg_verify((struct commit *)obj, 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);
-}
-
 int cmd_verify_commit(int argc, const char **argv, const char *prefix)
 {
 	int i = 1, verbose = 0, had_error = 0;
@@ -70,7 +62,7 @@ int cmd_verify_commit(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	git_config(git_verify_commit_config, NULL);
+	git_config(git_default_config, NULL);
 
 	argc = parse_options(argc, argv, prefix, verify_commit_options,
 			     verify_commit_usage, PARSE_OPT_KEEP_ARGV0);
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
@@ -19,14 +19,6 @@ static const char * const verify_tag_usage[] = {
 		NULL
 };
 
-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);
-}
-
 int cmd_verify_tag(int argc, const char **argv, const char *prefix)
 {
 	int i = 1, verbose = 0, had_error = 0;
@@ -39,7 +31,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	git_config(git_verify_tag_config, NULL);
+	git_config(git_default_config, NULL);
 
 	argc = parse_options(argc, argv, prefix, verify_tag_options,
 			     verify_tag_usage, PARSE_OPT_KEEP_ARGV0);
diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
@@ -20,8 +20,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);
@@ -40,9 +38,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
@@ -10,6 +10,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;
@@ -633,6 +645,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;
 
@@ -696,11 +710,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;
@@ -889,6 +905,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();
 	}
@@ -899,6 +917,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) {
@@ -924,6 +944,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
@@ -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
@@ -265,10 +265,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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -1600,7 +1600,6 @@ int cmd_status(int argc, const char *argv, const char prefix)`
`1600`	`1600`	`static int git_commit_config(const char k, const char v, void *cb)`
`1601`	`1601`	`{`
`1602`	`1602`	`struct wt_status *s = cb;`
`1603`		`- int status;`
`1604`	`1603`
`1605`	`1604`	`if (!strcmp(k, "commit.template"))`
`1606`	`1605`	`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)`
`1620`	`1619`	`return 0;`
`1621`	`1620`	`}`
`1622`	`1621`
`1623`		`- status = git_gpg_config(k, v, NULL);`
`1624`		`- if (status)`
`1625`		`- return status;`
`1626`	`1622`	`return git_status_config(k, v, s);`
`1627`	`1623`	`}`
`1628`	`1624`
Original file line number	Diff line number	Diff line change
`@@ -603,8 +603,6 @@ static int git_log_config(const char var, const char value, void *cb)`
`603`	`603`	`return 0;`
`604`	`604`	`}`
`605`	`605`
`606`		`- if (git_gpg_config(var, value, cb) < 0)`
`607`		`- return -1;`
`608`	`606`	`return git_diff_ui_config(var, value, cb);`
`609`	`607`	`}`
`610`	`608`
Original file line number	Diff line number	Diff line change
`@@ -661,9 +661,6 @@ static int git_merge_config(const char k, const char v, void *cb)`
`661`	`661`	`}`
`662`	`662`
`663`	`663`	`status = fmt_merge_msg_config(k, v, cb);`
`664`		`- if (status)`
`665`		`- return status;`
`666`		`- status = git_gpg_config(k, v, NULL);`
`667`	`664`	`if (status)`
`668`	`665`	`return status;`
`669`	`666`	`return git_diff_ui_config(k, v, cb);`
Original file line number	Diff line number	Diff line change
`@@ -360,8 +360,6 @@ static enum rebase_type config_get_rebase(int *rebase_unspecified)`
`360`	`360`	`*/`
`361`	`361`	`static int git_pull_config(const char var, const char value, void *cb)`
`362`	`362`	`{`
`363`		`- int status;`
`364`		`-`
`365`	`363`	`if (!strcmp(var, "rebase.autostash")) {`
`366`	`364`	`config_autostash = git_config_bool(var, value);`
`367`	`365`	`return 0;`
`@@ -373,10 +371,6 @@ static int git_pull_config(const char var, const char value, void *cb)`
`373`	`371`	`check_trust_level = 0;`
`374`	`372`	`}`
`375`	`373`
`376`		`- status = git_gpg_config(var, value, cb);`
`377`		`- if (status)`
`378`		`- return status;`
`379`		`-`
`380`	`374`	`return git_default_config(var, value, cb);`
`381`	`375`	`}`
`382`	`376`
Original file line number	Diff line number	Diff line change
`@@ -134,10 +134,6 @@ static int receive_pack_config(const char var, const char value, void *cb)`
`134`	`134`	`{`
`135`	`135`	`int status = parse_hide_refs_config(var, value, "receive", &hidden_refs);`
`136`	`136`
`137`		`- if (status)`
`138`		`- return status;`
`139`		`-`
`140`		`- status = git_gpg_config(var, value, NULL);`
`141`	`137`	`if (status)`
`142`	`138`	`return status;`
`143`	`139`
Original file line number	Diff line number	Diff line change
`@@ -131,8 +131,6 @@ static void print_helper_status(struct ref *ref)`
`131`	`131`
`132`	`132`	`static int send_pack_config(const char k, const char v, void *cb)`
`133`	`133`	`{`
`134`		`- git_gpg_config(k, v, NULL);`
`135`		`-`
`136`	`134`	`if (!strcmp(k, "push.gpgsign")) {`
`137`	`135`	`const char *value;`
`138`	`136`	`if (!git_config_get_value("push.gpgsign", &value)) {`