{"thread":{"id":"63872","subject":"[GSOC PATCH 0/2] builtin/fmt-merge-msg: remove dependency on global variables and 'the_repository'","startedAt":"2025-07-29T16:20:13Z","lastAt":"2025-08-11T18:25:48Z","messageCount":19,"participants":["Ayush Chandekar","Junio C Hamano","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"522971","messageId":"cover.1753804956.git.ayu.chandekar@gmail.com","threadId":"63872","inReplyTo":null,"subject":"[GSOC PATCH 0/2] builtin/fmt-merge-msg: remove dependency on global variables and 'the_repository'","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-29T16:19:33Z","receivedAt":"2025-07-29T16:20:13Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"The aim of this patch series is to remove the definition '#define USE_THE_REPOSITORY_VARIABLE'\nfrom \"builtin/fmt-merge-msg.c\" by removing global variable 'merge_log_config' and the global \n'the_repository'\n\nThis patch series contains two patches:\n\n1 - Remove the global varaible 'merge_log_config' and add a function 'adjust_shortlog_len()'\n in fmt-merge-msg.{c,h} to replicate the variable's usage.\n\n2 - Remove the dependency of 'the_repository' in \"builtin/fmt-merge-msg.c\", allowing the removal\n of the definition '#define USE_THE_REPOSITORY_VARIABLE'. Also add a test to make sure that\n \"git fmt-merge-msg -h\" can be called with repository being NULL.\n\nAyush Chandekar (2):\n environment: remove the global variable 'merge_log_config'\n builtin/fmt-merge-msg: stop depending on 'the_repository'\n\n builtin/fmt-merge-msg.c | 9 ++++-----\n builtin/merge.c | 4 ++--\n environment.c | 2 --\n fmt-merge-msg.c | 30 ++++++++++++++++++++++--------\n fmt-merge-msg.h | 3 ++-\n t/t1517-outside-repo.sh | 7 +++++++\n 6 files changed, 37 insertions(+), 18 deletions(-)\n\n-- \n2.49.0\n\n"},{"id":"522972","messageId":"c82620a1f54ea6760bff204fd2b5fe5c2df1896c.1753804956.git.ayu.chandekar@gmail.com","threadId":"63872","inReplyTo":"cover.1753804956.git.ayu.chandekar@gmail.com","subject":"[GSOC PATCH 1/2] environment: remove the global variable 'merge_log_config'","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-29T16:19:34Z","receivedAt":"2025-07-29T16:20:23Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"The global variable 'merge_log_config', set via the \"merge.log\" or\n\"merge.summary\" settings, is only used in 'cmd_fmt_merge_msg()' and\n'cmd_merge()' to adjust the 'shortlog_len' variable.\n\nRemove 'merge_log_config' and introduce a function\n'adjust_shortlog_len()' in fmt-merge-msg.c to handle the 'shortlog_len'\nvariable.\n\nThis change is part of an ongoing effort to eliminate global variables,\nimprove modularity and help libify the codebase.\n\nMentored-by: Christian Couder \nMentored-by: Ghanshyam Thakkar \nSigned-off-by: Ayush Chandekar \n---\n builtin/fmt-merge-msg.c | 4 ++--\n builtin/merge.c | 4 ++--\n environment.c | 2 --\n fmt-merge-msg.c | 30 ++++++++++++++++++++++--------\n fmt-merge-msg.h | 3 ++-\n 5 files changed, 28 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 3b6aac2cf7..fed8163825 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -58,8 +58,8 @@ int cmd_fmt_merge_msg(int argc,\n \t\t\t 0);\n \tif (argc > 0)\n \t\tusage_with_options(fmt_merge_msg_usage, options);\n-\tif (shortlog_len < 0)\n-\t\tshortlog_len = (merge_log_config > 0) ? merge_log_config : 0;\n+\n+\tadjust_shortlog_len(the_repository, &shortlog_len);\n \n \tif (inpath && strcmp(inpath, \"-\")) {\n \t\tin = fopen(inpath, \"r\");\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 18b22c0a26..e1cf2a6d63 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1403,8 +1403,8 @@ int cmd_merge(int argc,\n \t\tparse_branch_merge_options(branch_mergeoptions);\n \targc = parse_options(argc, argv, prefix, builtin_merge_options,\n \t\t\tbuiltin_merge_usage, 0);\n-\tif (shortlog_len < 0)\n-\t\tshortlog_len = (merge_log_config > 0) ? merge_log_config : 0;\n+\n+\tadjust_shortlog_len(the_repository, &shortlog_len);\n \n \tif (verbosity < 0 && show_progress == -1)\n \t\tshow_progress = 0;\ndiff --git a/environment.c b/environment.c\nindex 7c2480b22e..74c9838b31 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -20,7 +20,6 @@\n #include \"repository.h\"\n #include \"config.h\"\n #include \"refs.h\"\n-#include \"fmt-merge-msg.h\"\n #include \"commit.h\"\n #include \"strvec.h\"\n #include \"path.h\"\n@@ -66,7 +65,6 @@ int grafts_keep_true_parents;\n int core_apply_sparse_checkout;\n int core_sparse_checkout_cone;\n int sparse_expect_files_outside_of_patterns;\n-int merge_log_config = -1;\n int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */\n unsigned long pack_size_limit_cfg;\n int max_allowed_tree_depth =\ndiff --git a/fmt-merge-msg.c b/fmt-merge-msg.c\nindex 40174efa3d..2ceaebb0ce 100644\n--- a/fmt-merge-msg.c\n+++ b/fmt-merge-msg.c\n@@ -26,14 +26,7 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;\n int fmt_merge_msg_config(const char *key, const char *value,\n \t\t\t const struct config_context *ctx, void *cb)\n {\n-\tif (!strcmp(key, \"merge.log\") || !strcmp(key, \"merge.summary\")) {\n-\t\tint is_bool;\n-\t\tmerge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);\n-\t\tif (!is_bool && merge_log_config < 0)\n-\t\t\treturn error(\"%s: negative length %s\", key, value);\n-\t\tif (is_bool && merge_log_config)\n-\t\t\tmerge_log_config = DEFAULT_MERGE_LOG_LEN;\n-\t} else if (!strcmp(key, \"merge.branchdesc\")) {\n+\tif (!strcmp(key, \"merge.branchdesc\")) {\n \t\tuse_branch_desc = git_config_bool(key, value);\n \t} else if (!strcmp(key, \"merge.suppressdest\")) {\n \t\tif (!value)\n@@ -645,6 +638,27 @@ static void find_merge_parents(struct merge_parents *result,\n \tresult->nr = j;\n }\n \n+void adjust_shortlog_len(struct repository *r, int *shortlog_len)\n+{\n+\tconst char *keys[] = { \"merge.log\", \"merge.summary\", NULL};\n+\t\n+\tif (*shortlog_len >= 0)\n+\t\treturn;\n+\n+\tfor (const char **key = keys; *key; ++key) {\n+\t\tint is_bool, value;\n+\t\tif (!repo_config_get_bool_or_int(r, *key, &is_bool, &value)) {\n+\t\t\tif (!is_bool && value < 0) {\n+\t\t\t\terror(\"%s: negative length %d\", *key, value);\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t\t*shortlog_len = (is_bool && value) ? DEFAULT_MERGE_LOG_LEN : value;\n+\t\t\treturn;\n+\t\t}\n+\t}\n+\n+\t*shortlog_len = 0;\n+}\n \n int fmt_merge_msg(struct strbuf *in, struct strbuf *out,\n \t\t struct fmt_merge_msg_opts *opts)\ndiff --git a/fmt-merge-msg.h b/fmt-merge-msg.h\nindex 73ca3e4465..f54f00d26f 100644\n--- a/fmt-merge-msg.h\n+++ b/fmt-merge-msg.h\n@@ -2,6 +2,7 @@\n #define FMT_MERGE_MSG_H\n \n #include \"strbuf.h\"\n+#include \"repository.h\"\n \n #define DEFAULT_MERGE_LOG_LEN 20\n \n@@ -12,9 +13,9 @@ struct fmt_merge_msg_opts {\n \tconst char *into_name;\n };\n \n-extern int merge_log_config;\n int fmt_merge_msg_config(const char *key, const char *value,\n \t\t\t const struct config_context *ctx, void *cb);\n+void adjust_shortlog_len(struct repository *r, int *shortlog_len);\n int fmt_merge_msg(struct strbuf *in, struct strbuf *out,\n \t\t struct fmt_merge_msg_opts *);\n \n-- \n2.49.0\n\n"},{"id":"522973","messageId":"04d6f682a6b2257e14682e809a2fd01ccfcf0d08.1753804956.git.ayu.chandekar@gmail.com","threadId":"63872","inReplyTo":"cover.1753804956.git.ayu.chandekar@gmail.com","subject":"[GSOC PATCH 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-29T16:19:35Z","receivedAt":"2025-07-29T16:20:27Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global\n'the_repository'. Replace all the occurrences of 'the_repository' with\n'repo', where 'repo' is a pointer to 'struct repository' passed to the\nfunction 'cmd_fmt_merge_msg()' and thus remove the definition '#define\nUSE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that \"git\nfmt-merge-msg -h\" can be called outside a repository.\n\nMentored-by: Christian Couder \nMentored-by: Ghanshyam Thakkar \nSigned-off-by: Ayush Chandekar \n---\n builtin/fmt-merge-msg.c | 7 +++----\n t/t1517-outside-repo.sh | 7 +++++++\n 2 files changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex fed8163825..848498b8e6 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -1,4 +1,3 @@\n-#define USE_THE_REPOSITORY_VARIABLE\n #include \"builtin.h\"\n #include \"config.h\"\n #include \"fmt-merge-msg.h\"\n@@ -13,7 +12,7 @@ static const char * const fmt_merge_msg_usage[] = {\n int cmd_fmt_merge_msg(int argc,\n \t\t const char **argv,\n \t\t const char *prefix,\n-\t\t struct repository *repo UNUSED)\n+\t\t struct repository *repo)\n {\n \tchar *inpath = NULL;\n \tconst char *message = NULL;\n@@ -53,13 +52,13 @@ int cmd_fmt_merge_msg(int argc,\n \tint ret;\n \tstruct fmt_merge_msg_opts opts;\n \n-\tgit_config(fmt_merge_msg_config, NULL);\n \targc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,\n \t\t\t 0);\n \tif (argc > 0)\n \t\tusage_with_options(fmt_merge_msg_usage, options);\n+\trepo_config(repo, fmt_merge_msg_config, NULL);\n \n-\tadjust_shortlog_len(the_repository, &shortlog_len);\n+\tadjust_shortlog_len(repo, &shortlog_len);\n \n \tif (inpath && strcmp(inpath, \"-\")) {\n \t\tin = fopen(inpath, \"r\");\ndiff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh\nindex 8f59b867f2..4b4e645860 100755\n--- a/t/t1517-outside-repo.sh\n+++ b/t/t1517-outside-repo.sh\n@@ -121,4 +121,11 @@ test_expect_success 'prune does not crash with -h' '\n \ttest_grep \"[Uu]sage: git prune \" usage\n '\n \n+test_expect_success 'fmt-merge-msg does not crash with -h' '\n+\ttest_expect_code 129 git fmt-merge-msg -h >usage &&\n+\ttest_grep \"[Uu]sage: git fmt-merge-msg \" usage &&\n+\ttest_expect_code 129 nongit git fmt-merge-msg -h >usage &&\n+\ttest_grep \"[Uu]sage: git fmt-merge-msg \" usage\n+'\n+\n test_done\n-- \n2.49.0\n\n"},{"id":"522974","messageId":"xmqqjz3rospl.fsf@gitster.g","threadId":"63872","inReplyTo":"04d6f682a6b2257e14682e809a2fd01ccfcf0d08.1753804956.git.ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-29T16:41:10Z","receivedAt":"2025-07-29T16:41:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ayush Chandekar writes:\n\n> Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global\n> 'the_repository'. Replace all the occurrences of 'the_repository' with\n> 'repo', where 'repo' is a pointer to 'struct repository' passed to the\n> function 'cmd_fmt_merge_msg()' and thus remove the definition '#define\n> USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that \"git\n> fmt-merge-msg -h\" can be called outside a repository.\n\nThis also moves the call to git_config()/repo_config() after\nparse_options().\n\nIt generally is a bad idea to read command line options first and\nthen read the configuration (it is a bug if such a flow causes\nvalues from configuration to overwrite values from command line).\nTHe current set of options and configuration variables may not\noverlap, in which case such a questionable arrangement happen to be\nwithout bug right now, but it would prevent future developers from\nadding new options and configuration variables and make them\ninteract with each other in the most natural way.\n\nIn any case, the reason for this change of the order between config\nand parse-options is not explained at all in the proposed log\nmessage.\n\n> Mentored-by: Christian Couder \n> Mentored-by: Ghanshyam Thakkar \n> Signed-off-by: Ayush Chandekar \n> ---\n> builtin/fmt-merge-msg.c | 7 +++----\n> t/t1517-outside-repo.sh | 7 +++++++\n> 2 files changed, 10 insertions(+), 4 deletions(-)\n>\n> diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\n> index fed8163825..848498b8e6 100644\n> --- a/builtin/fmt-merge-msg.c\n> +++ b/builtin/fmt-merge-msg.c\n> @@ -1,4 +1,3 @@\n> -#define USE_THE_REPOSITORY_VARIABLE\n> #include \"builtin.h\"\n> #include \"config.h\"\n> #include \"fmt-merge-msg.h\"\n> @@ -13,7 +12,7 @@ static const char * const fmt_merge_msg_usage[] = {\n> int cmd_fmt_merge_msg(int argc,\n> \t\t const char **argv,\n> \t\t const char *prefix,\n> -\t\t struct repository *repo UNUSED)\n> +\t\t struct repository *repo)\n> {\n> \tchar *inpath = NULL;\n> \tconst char *message = NULL;\n> @@ -53,13 +52,13 @@ int cmd_fmt_merge_msg(int argc,\n> \tint ret;\n> \tstruct fmt_merge_msg_opts opts;\n> \n> -\tgit_config(fmt_merge_msg_config, NULL);\n> \targc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,\n> \t\t\t 0);\n> \tif (argc > 0)\n> \t\tusage_with_options(fmt_merge_msg_usage, options);\n> +\trepo_config(repo, fmt_merge_msg_config, NULL);\n> \n> -\tadjust_shortlog_len(the_repository, &shortlog_len);\n> +\tadjust_shortlog_len(repo, &shortlog_len);\n> \n> \tif (inpath && strcmp(inpath, \"-\")) {\n> \t\tin = fopen(inpath, \"r\");\n> diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh\n> index 8f59b867f2..4b4e645860 100755\n> --- a/t/t1517-outside-repo.sh\n> +++ b/t/t1517-outside-repo.sh\n> @@ -121,4 +121,11 @@ test_expect_success 'prune does not crash with -h' '\n> \ttest_grep \"[Uu]sage: git prune \" usage\n> '\n> \n> +test_expect_success 'fmt-merge-msg does not crash with -h' '\n> +\ttest_expect_code 129 git fmt-merge-msg -h >usage &&\n> +\ttest_grep \"[Uu]sage: git fmt-merge-msg \" usage &&\n> +\ttest_expect_code 129 nongit git fmt-merge-msg -h >usage &&\n> +\ttest_grep \"[Uu]sage: git fmt-merge-msg \" usage\n> +'\n> +\n> test_done\n"},{"id":"522975","messageId":"xmqqfrefosdj.fsf@gitster.g","threadId":"63872","inReplyTo":"c82620a1f54ea6760bff204fd2b5fe5c2df1896c.1753804956.git.ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH 1/2] environment: remove the global variable 'merge_log_config'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-29T16:48:24Z","receivedAt":"2025-07-29T16:48:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ayush Chandekar writes:\n\n> The global variable 'merge_log_config', set via the \"merge.log\" or\n> \"merge.summary\" settings, is only used in 'cmd_fmt_merge_msg()' and\n> 'cmd_merge()' to adjust the 'shortlog_len' variable.\n>\n> Remove 'merge_log_config' and introduce a function\n> 'adjust_shortlog_len()' in fmt-merge-msg.c to handle the 'shortlog_len'\n> variable.\n>\n> This change is part of an ongoing effort to eliminate global variables,\n> improve modularity and help libify the codebase.\n\nAnd the downsides of this change are...?\n\nOne obvious behaviour change I can see can happen when you have an\ninvalid value set to merge.summary and run the command with command\nline override with the \"--log\" option. In the current code, the\nconfig callback barfs when it notices an invalid merge.summary\nsetting, even though it won't be used because the valid value given\nvia the \"--log\" option would override it. In the updated code,\nadjust_shortlog_len() would short-circuit and does not even bother\nreading from the configuration, so the user will not be notified of\na broken configuration.\n\nIt is not immediately obvious if this particular behaviour change is\na regression or an improvement, but it probably deserves to be noted\nsomewhere to help future developers what our thinking was.\n\n> @@ -26,14 +26,7 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;\n> int fmt_merge_msg_config(const char *key, const char *value,\n> \t\t\t const struct config_context *ctx, void *cb)\n> {\n> -\tif (!strcmp(key, \"merge.log\") || !strcmp(key, \"merge.summary\")) {\n> -\t\tint is_bool;\n> -\t\tmerge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);\n> -\t\tif (!is_bool && merge_log_config < 0)\n> -\t\t\treturn error(\"%s: negative length %s\", key, value);\n> -\t\tif (is_bool && merge_log_config)\n> -\t\t\tmerge_log_config = DEFAULT_MERGE_LOG_LEN;\n> -\t} else if (!strcmp(key, \"merge.branchdesc\")) {\n> +\tif (!strcmp(key, \"merge.branchdesc\")) {\n> \t\tuse_branch_desc = git_config_bool(key, value);\n> \t} else if (!strcmp(key, \"merge.suppressdest\")) {\n> \t\tif (!value)\n> @@ -645,6 +638,27 @@ static void find_merge_parents(struct merge_parents *result,\n> \tresult->nr = j;\n> }\n> \n> +void adjust_shortlog_len(struct repository *r, int *shortlog_len)\n> +{\n> +\tconst char *keys[] = { \"merge.log\", \"merge.summary\", NULL};\n> +\t\n> +\tif (*shortlog_len >= 0)\n> +\t\treturn;\n> +\n> +\tfor (const char **key = keys; *key; ++key) {\n> +\t\tint is_bool, value;\n> +\t\tif (!repo_config_get_bool_or_int(r, *key, &is_bool, &value)) {\n> +\t\t\tif (!is_bool && value < 0) {\n> +\t\t\t\terror(\"%s: negative length %d\", *key, value);\n> +\t\t\t\treturn;\n> +\t\t\t}\n> +\t\t\t*shortlog_len = (is_bool && value) ? DEFAULT_MERGE_LOG_LEN : value;\n> +\t\t\treturn;\n> +\t\t}\n> +\t}\n> +\n> +\t*shortlog_len = 0;\n> +}\n> \n> int fmt_merge_msg(struct strbuf *in, struct strbuf *out,\n> \t\t struct fmt_merge_msg_opts *opts)\n> diff --git a/fmt-merge-msg.h b/fmt-merge-msg.h\n> index 73ca3e4465..f54f00d26f 100644\n> --- a/fmt-merge-msg.h\n> +++ b/fmt-merge-msg.h\n> @@ -2,6 +2,7 @@\n> #define FMT_MERGE_MSG_H\n> \n> #include \"strbuf.h\"\n> +#include \"repository.h\"\n> \n> #define DEFAULT_MERGE_LOG_LEN 20\n> \n> @@ -12,9 +13,9 @@ struct fmt_merge_msg_opts {\n> \tconst char *into_name;\n> };\n> \n> -extern int merge_log_config;\n> int fmt_merge_msg_config(const char *key, const char *value,\n> \t\t\t const struct config_context *ctx, void *cb);\n> +void adjust_shortlog_len(struct repository *r, int *shortlog_len);\n> int fmt_merge_msg(struct strbuf *in, struct strbuf *out,\n> \t\t struct fmt_merge_msg_opts *);\n"},{"id":"522977","messageId":"CAE7as+ZwiMENJDd6rjnF6w9tt_mJ=Kzf-t9U6VxAKmCdacOgbg@mail.gmail.com","threadId":"63872","inReplyTo":"xmqqfrefosdj.fsf@gitster.g","subject":"Re: [GSOC PATCH 1/2] environment: remove the global variable 'merge_log_config'","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-29T17:30:39Z","receivedAt":"2025-07-29T17:30:52Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"Hi Junio,\n\nOn Tue, Jul 29, 2025 at 10:18 PM Junio C Hamano wrote:\n>\n> Ayush Chandekar writes:\n>\n> > The global variable 'merge_log_config', set via the \"merge.log\" or\n> > \"merge.summary\" settings, is only used in 'cmd_fmt_merge_msg()' and\n> > 'cmd_merge()' to adjust the 'shortlog_len' variable.\n> >\n> > Remove 'merge_log_config' and introduce a function\n> > 'adjust_shortlog_len()' in fmt-merge-msg.c to handle the 'shortlog_len'\n> > variable.\n> >\n> > This change is part of an ongoing effort to eliminate global variables,\n> > improve modularity and help libify the codebase.\n>\n> And the downsides of this change are...?\n>\n> One obvious behaviour change I can see can happen when you have an\n> invalid value set to merge.summary and run the command with command\n> line override with the \"--log\" option. In the current code, the\n> config callback barfs when it notices an invalid merge.summary\n> setting, even though it won't be used because the valid value given\n> via the \"--log\" option would override it. In the updated code,\n> adjust_shortlog_len() would short-circuit and does not even bother\n> reading from the configuration, so the user will not be notified of\n> a broken configuration.\n>\n> It is not immediately obvious if this particular behaviour change is\n> a regression or an improvement, but it probably deserves to be noted\n> somewhere to help future developers what our thinking was.\n\nOh right, I did not mention this in the commit message. I am not sure\nif this behaviour is good or not.\n\nTechnically, if the user wants to use the \"--log\" option, they would\nnot care about the config. Whereas, if the user wants to use the\nconfig, they would be notified in case of an invalid one.\n\nI will mention this in the commit message, but do you think this\nbehaviour is fine?\n\nThanks\nAyush\n"},{"id":"522978","messageId":"xmqqbjp2q3wo.fsf@gitster.g","threadId":"63872","inReplyTo":"CAE7as+ZwiMENJDd6rjnF6w9tt_mJ=Kzf-t9U6VxAKmCdacOgbg@mail.gmail.com","subject":"Re: [GSOC PATCH 1/2] environment: remove the global variable 'merge_log_config'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-29T17:53:59Z","receivedAt":"2025-07-29T17:54:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ayush Chandekar writes:\n\n> I will mention this in the commit message, but do you think this\n> behaviour is fine?\n\nI cannot answer that one immediately after saying that I do not know\nif this is a regression or an improvement. \n\nIf you still pushed me to answer it immediately, you'd get my\ndefault position, a conservative \"any changes in behaviour caused by\nan internal code clean-up is bad and is a serious regression\" ;-).\n"},{"id":"522984","messageId":"23428022-ab13-4a3e-90ed-ff91ef93f051@gmail.com","threadId":"63872","inReplyTo":"c82620a1f54ea6760bff204fd2b5fe5c2df1896c.1753804956.git.ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH 1/2] environment: remove the global variable 'merge_log_config'","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-07-29T19:07:15Z","receivedAt":"2025-07-29T19:07:21Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ayush\n\nOn 29/07/2025 17:19, Ayush Chandekar wrote:\n> \n> @@ -26,14 +26,7 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;\n> int fmt_merge_msg_config(const char *key, const char *value,\n> \t\t\t const struct config_context *ctx, void *cb)\n> {\n> -\tif (!strcmp(key, \"merge.log\") || !strcmp(key, \"merge.summary\")) {\n> -\t\tint is_bool;\n> -\t\tmerge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);\n> -\t\tif (!is_bool && merge_log_config < 0)\n> -\t\t\treturn error(\"%s: negative length %s\", key, value);\n> -\t\tif (is_bool && merge_log_config)\n> -\t\t\tmerge_log_config = DEFAULT_MERGE_LOG_LEN;\n> -\t} else if (!strcmp(key, \"merge.branchdesc\")) {\n\nIn the old code if both \"merge.log\" and \"merge.summary\" are set in the \nconfig file the last one wins\n\n> +void adjust_shortlog_len(struct repository *r, int *shortlog_len)\n> +{\n> +\tconst char *keys[] = { \"merge.log\", \"merge.summary\", NULL};\n> +\t\n> +\tif (*shortlog_len >= 0)\n> +\t\treturn;\n> +\n> +\tfor (const char **key = keys; *key; ++key) {\n> +\t\tint is_bool, value;\n> +\t\tif (!repo_config_get_bool_or_int(r, *key, &is_bool, &value)) {\n> +\t\t\tif (!is_bool && value < 0) {\n> +\t\t\t\terror(\"%s: negative length %d\", *key, value);\n> +\t\t\t\treturn;\n> +\t\t\t}\n> +\t\t\t*shortlog_len = (is_bool && value) ? DEFAULT_MERGE_LOG_LEN : value;\n> +\t\t\treturn;\n\nIn the new code \"merge.log\" is always used in preference to \n\"merge.summary\" even if \"merge.summary\" appears later in the config \nfile. When you have two keys setting the same variable I think the only \nway to preserve the last one wins behavior is to keep using a callback \nthat updates the value as the config files are parsed.\n\nThanks\n\nPhillip\n\n"},{"id":"522987","messageId":"CAE7as+Y_S=J8D4xrV75w2KJCKzpHamYt4Ug_iGD068i3Kdq5JA@mail.gmail.com","threadId":"63872","inReplyTo":"23428022-ab13-4a3e-90ed-ff91ef93f051@gmail.com","subject":"Re: [GSOC PATCH 1/2] environment: remove the global variable 'merge_log_config'","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-29T21:16:56Z","receivedAt":"2025-07-29T21:17:08Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"Hi Phillip,\n\nOn Wed, Jul 30, 2025 at 12:37 AM Phillip Wood wrote:\n>\n> Hi Ayush\n>\n> On 29/07/2025 17:19, Ayush Chandekar wrote:\n> >\n> > @@ -26,14 +26,7 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;\n> > int fmt_merge_msg_config(const char *key, const char *value,\n> > const struct config_context *ctx, void *cb)\n> > {\n> > - if (!strcmp(key, \"merge.log\") || !strcmp(key, \"merge.summary\")) {\n> > - int is_bool;\n> > - merge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);\n> > - if (!is_bool && merge_log_config < 0)\n> > - return error(\"%s: negative length %s\", key, value);\n> > - if (is_bool && merge_log_config)\n> > - merge_log_config = DEFAULT_MERGE_LOG_LEN;\n> > - } else if (!strcmp(key, \"merge.branchdesc\")) {\n>\n> In the old code if both \"merge.log\" and \"merge.summary\" are set in the\n> config file the last one wins\n>\n> > +void adjust_shortlog_len(struct repository *r, int *shortlog_len)\n> > +{\n> > + const char *keys[] = { \"merge.log\", \"merge.summary\", NULL};\n> > +\n> > + if (*shortlog_len >= 0)\n> > + return;\n> > +\n> > + for (const char **key = keys; *key; ++key) {\n> > + int is_bool, value;\n> > + if (!repo_config_get_bool_or_int(r, *key, &is_bool, &value)) {\n> > + if (!is_bool && value < 0) {\n> > + error(\"%s: negative length %d\", *key, value);\n> > + return;\n> > + }\n> > + *shortlog_len = (is_bool && value) ? DEFAULT_MERGE_LOG_LEN : value;\n> > + return;\n>\n> In the new code \"merge.log\" is always used in preference to\n> \"merge.summary\" even if \"merge.summary\" appears later in the config\n> file. When you have two keys setting the same variable I think the only\n> way to preserve the last one wins behavior is to keep using a callback\n> that updates the value as the config files are parsed.\n>\n\nSorry for not mentioning this in the commit message.\n\nI had looked at the documentation which says:\n\nDocumentation/git-fmt-merge-msg.adoc\nmerge.summary::\nSynonym to `merge.log`; this is deprecated and will be removed in\nthe future.\n\nSo I thought that I should give precedence to \"merge.log\" as\n\"merge.summary\" is deprecated.\n\n> Thanks\n>\n> Phillip\n>\n\nThanks\nAyush\n"},{"id":"522988","messageId":"CAE7as+ZUcqRbnOC11DQ7=b+YB+9HTfjfqCvxzmz+mpSH6DxkGQ@mail.gmail.com","threadId":"63872","inReplyTo":"xmqqjz3rospl.fsf@gitster.g","subject":"Re: [GSOC PATCH 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-07-29T21:49:01Z","receivedAt":"2025-07-29T21:49:13Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"On Tue, Jul 29, 2025 at 10:11 PM Junio C Hamano wrote:\n>\n> Ayush Chandekar writes:\n>\n> > Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global\n> > 'the_repository'. Replace all the occurrences of 'the_repository' with\n> > 'repo', where 'repo' is a pointer to 'struct repository' passed to the\n> > function 'cmd_fmt_merge_msg()' and thus remove the definition '#define\n> > USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that \"git\n> > fmt-merge-msg -h\" can be called outside a repository.\n>\n> This also moves the call to git_config()/repo_config() after\n> parse_options().\n>\n> It generally is a bad idea to read command line options first and\n> then read the configuration (it is a bug if such a flow causes\n> values from configuration to overwrite values from command line).\n> THe current set of options and configuration variables may not\n> overlap, in which case such a questionable arrangement happen to be\n> without bug right now, but it would prevent future developers from\n> adding new options and configuration variables and make them\n> interact with each other in the most natural way.\n>\n\nI understand it, but how do we tackle if NULL repository is passed.\n\n> In any case, the reason for this change of the order between config\n> and parse-options is not explained at all in the proposed log\n> message.\n>\n\n Apologies, I will mention it.\n"},{"id":"522989","messageId":"xmqqh5yuoc14.fsf@gitster.g","threadId":"63872","inReplyTo":"CAE7as+ZUcqRbnOC11DQ7=b+YB+9HTfjfqCvxzmz+mpSH6DxkGQ@mail.gmail.com","subject":"Re: [GSOC PATCH 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-29T22:41:27Z","receivedAt":"2025-07-29T22:41:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ayush Chandekar writes:\n\n>> It generally is a bad idea to read command line options first and\n>> then read the configuration (it is a bug if such a flow causes\n>> values from configuration to overwrite values from command line).\n>> THe current set of options and configuration variables may not\n>> overlap, in which case such a questionable arrangement happen to be\n>> without bug right now, but it would prevent future developers from\n>> adding new options and configuration variables and make them\n>> interact with each other in the most natural way.\n>\n> I understand it, but how do we tackle if NULL repository is passed.\n\nPerhaps you want to study the problem space and related past changes\nbefore going forward. The first place to look at is what happens\nwhen you call repo_config(), outside a repository or a working tree\nand repository is NULL.\n\nf29f1990 (config: teach repo_config to allow `repo` to be NULL,\n2025-03-08) and what it calls \"the following commits\" may be\nilluminating.\n\n"},{"id":"523002","messageId":"1f07119e-64b6-453f-ae83-64a5fb486188@gmail.com","threadId":"63872","inReplyTo":"CAE7as+Y_S=J8D4xrV75w2KJCKzpHamYt4Ug_iGD068i3Kdq5JA@mail.gmail.com","subject":"Re: [GSOC PATCH 1/2] environment: remove the global variable 'merge_log_config'","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-07-30T08:53:51Z","receivedAt":"2025-07-30T08:53:59Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ayush\n\nOn 29/07/2025 22:16, Ayush Chandekar wrote:\n> \n> On Wed, Jul 30, 2025 at 12:37 AM Phillip Wood wrote:\n>>\n>> Hi Ayush\n>>\n>> On 29/07/2025 17:19, Ayush Chandekar wrote:\n>>>\n>>> @@ -26,14 +26,7 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;\n>>> int fmt_merge_msg_config(const char *key, const char *value,\n>>> const struct config_context *ctx, void *cb)\n>>> {\n>>> - if (!strcmp(key, \"merge.log\") || !strcmp(key, \"merge.summary\")) {\n>>> - int is_bool;\n>>> - merge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);\n>>> - if (!is_bool && merge_log_config < 0)\n>>> - return error(\"%s: negative length %s\", key, value);\n>>> - if (is_bool && merge_log_config)\n>>> - merge_log_config = DEFAULT_MERGE_LOG_LEN;\n>>> - } else if (!strcmp(key, \"merge.branchdesc\")) {\n>>\n>> In the old code if both \"merge.log\" and \"merge.summary\" are set in the\n>> config file the last one wins\n>>\n>>> +void adjust_shortlog_len(struct repository *r, int *shortlog_len)\n>>> +{\n>>> + const char *keys[] = { \"merge.log\", \"merge.summary\", NULL};\n>>> +\n>>> + if (*shortlog_len >= 0)\n>>> + return;\n>>> +\n>>> + for (const char **key = keys; *key; ++key) {\n>>> + int is_bool, value;\n>>> + if (!repo_config_get_bool_or_int(r, *key, &is_bool, &value)) {\n>>> + if (!is_bool && value < 0) {\n>>> + error(\"%s: negative length %d\", *key, value);\n>>> + return;\n>>> + }\n>>> + *shortlog_len = (is_bool && value) ? DEFAULT_MERGE_LOG_LEN : value;\n>>> + return;\n>>\n>> In the new code \"merge.log\" is always used in preference to\n>> \"merge.summary\" even if \"merge.summary\" appears later in the config\n>> file. When you have two keys setting the same variable I think the only\n>> way to preserve the last one wins behavior is to keep using a callback\n>> that updates the value as the config files are parsed.\n>>\n> \n> Sorry for not mentioning this in the commit message.\n> \n> I had looked at the documentation which says:\n> \n> Documentation/git-fmt-merge-msg.adoc\n> merge.summary::\n> Synonym to `merge.log`; this is deprecated and will be removed in\n> the future.\n> \n> So I thought that I should give precedence to \"merge.log\" as\n> \"merge.summary\" is deprecated.\n\nIf it is deprecated we still need to support it until it is removed \n(maybe we should do that in Git 3.0?). We cannot change the behavior \njust because a setting is deprecated.\n\nThanks\n\nPhillip\n\n>> Thanks\n>>\n>> Phillip\n>>\n> \n> Thanks\n> Ayush\n> \n\n"},{"id":"523886","messageId":"CAE7as+atKMV30Bi961bZCDCq7zqyJMmNeq9nK9J6ywurLHU-bg@mail.gmail.com","threadId":"63872","inReplyTo":"cover.1753804956.git.ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH 0/2] builtin/fmt-merge-msg: remove dependency on global variables and 'the_repository'","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-08-10T15:33:12Z","receivedAt":"2025-08-10T15:33:23Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"Just an update, I'm still working on this patch series.\n\nThanks,\nAyush\n"},{"id":"523901","messageId":"cover.1754868681.git.ayu.chandekar@gmail.com","threadId":"63872","inReplyTo":"cover.1753804956.git.ayu.chandekar@gmail.com","subject":"[GSOC PATCH v2 0/2] builtin/fmt-merge-msg: remove dependency on global variables and 'the_repository'","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-08-10T23:45:44Z","receivedAt":"2025-08-10T23:46:09Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"The aim of this patch series is to remove the definition '#define USE_THE_REPOSITORY_VARIABLE'\nfrom \"builtin/fmt-merge-msg.c\" by removing global variable 'merge_log_config' and the global \n'the_repository'\n\nThis patch series contains two patches:\n\n1 - Remove the global varaible 'merge_log_config' and localize it in\n 'cmd_fmt_merge_msg()' and 'cmd_merge()'. Set its value by passing it in\n 'fmt_merge_msg_config()' by passing its pointer to the function via the\n callback parameter.\n\n2 - Remove the dependency of 'the_repository' in \"builtin/fmt-merge-msg.c\", allowing the removal\n of the definition '#define USE_THE_REPOSITORY_VARIABLE'. Also add a test to make sure that\n \"git fmt-merge-msg -h\" can be called with repository being NULL.\n\nThanks to Junio and Phillip for reviewing my patch series and Christian for mentoring me!\n\nAyush Chandekar (2):\n environment: remove the global variable 'merge_log_config'\n builtin/fmt-merge-msg: stop depending on 'the_repository'\n\n builtin/fmt-merge-msg.c | 6 +++---\n builtin/merge.c | 3 ++-\n environment.c | 1 -\n fmt-merge-msg.c | 10 ++++++----\n fmt-merge-msg.h | 1 -\n t/t1517-outside-repo.sh | 7 +++++++\n 6 files changed, 18 insertions(+), 10 deletions(-)\n\nRange-diff against v1:\n1: c82620a1f5 < -: ---------- environment: remove the global variable 'merge_log_config'\n-: ---------- > 1: 3aa014ed46 environment: remove the global variable 'merge_log_config'\n2: 04d6f682a6 ! 2: 8e55516cda builtin/fmt-merge-msg: stop depending on 'the_repository'\n @@ Commit message\n builtin/fmt-merge-msg: stop depending on 'the_repository'\n \n Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global\n - 'the_repository'. Replace all the occurrences of 'the_repository' with\n - 'repo', where 'repo' is a pointer to 'struct repository' passed to the\n - function 'cmd_fmt_merge_msg()' and thus remove the definition '#define\n - USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that \"git\n - fmt-merge-msg -h\" can be called outside a repository.\n + 'the_repository'. Remove the 'UNUSED' macro from the 'struct repository'\n + parameter and replace 'git_config()' with 'repo_config()' so that\n + configuration is read from the passed repository. Also, add a test to\n + make sure that \"git fmt-merge-msg -h\" can be called outside a\n + repository.\n \n Mentored-by: Christian Couder \n Mentored-by: Ghanshyam Thakkar \n @@ builtin/fmt-merge-msg.c: int cmd_fmt_merge_msg(int argc,\n \tint ret;\n \tstruct fmt_merge_msg_opts opts;\n \n --\tgit_config(fmt_merge_msg_config, NULL);\n +-\tgit_config(fmt_merge_msg_config, &merge_log_config);\n ++\trepo_config(repo, fmt_merge_msg_config, &merge_log_config);\n \targc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,\n \t\t\t 0);\n \tif (argc > 0)\n - \t\tusage_with_options(fmt_merge_msg_usage, options);\n -+\trepo_config(repo, fmt_merge_msg_config, NULL);\n - \n --\tadjust_shortlog_len(the_repository, &shortlog_len);\n -+\tadjust_shortlog_len(repo, &shortlog_len);\n - \n - \tif (inpath && strcmp(inpath, \"-\")) {\n - \t\tin = fopen(inpath, \"r\");\n \n ## t/t1517-outside-repo.sh ##\n @@ t/t1517-outside-repo.sh: test_expect_success 'prune does not crash with -h' '\n\n-- \n2.49.0\n\n"},{"id":"523902","messageId":"3aa014ed46d14e31ea0c2f6b7631e7e4cbbd3943.1754868681.git.ayu.chandekar@gmail.com","threadId":"63872","inReplyTo":"cover.1754868681.git.ayu.chandekar@gmail.com","subject":"[GSOC PATCH v2 1/2] environment: remove the global variable 'merge_log_config'","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-08-10T23:45:45Z","receivedAt":"2025-08-10T23:46:18Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"The global variable 'merge_log_config', set via the \"merge.log\" or\n\"merge.summary\" settings, is only used in 'cmd_fmt_merge_msg()' and\n'cmd_merge()' to adjust the 'shortlog_len' variable.\n\nRemove 'merge_log_config' globally and localize it in\n'cmd_fmt_merge_msg()' and 'cmd_merge()'. Set its value by passing it in\n'fmt_merge_msg_config()' by passing its pointer to the function via the\ncallback parameter.\n\nThis change is part of an ongoing effort to eliminate global variables,\nimprove modularity and help libify the codebase.\n\nMentored-by: Christian Couder \nMentored-by: Ghanshyam Thakkar \nSigned-off-by: Ayush Chandekar \n---\n builtin/fmt-merge-msg.c | 3 ++-\n builtin/merge.c | 3 ++-\n environment.c | 1 -\n fmt-merge-msg.c | 10 ++++++----\n fmt-merge-msg.h | 1 -\n 5 files changed, 10 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 3b6aac2cf7..4b24de32fb 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -19,6 +19,7 @@ int cmd_fmt_merge_msg(int argc,\n \tconst char *message = NULL;\n \tchar *into_name = NULL;\n \tint shortlog_len = -1;\n+\tint merge_log_config = -1;\n \tstruct option options[] = {\n \t\t{\n \t\t\t.type = OPTION_INTEGER,\n@@ -53,7 +54,7 @@ int cmd_fmt_merge_msg(int argc,\n \tint ret;\n \tstruct fmt_merge_msg_opts opts;\n \n-\tgit_config(fmt_merge_msg_config, NULL);\n+\tgit_config(fmt_merge_msg_config, &merge_log_config);\n \targc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,\n \t\t\t 0);\n \tif (argc > 0)\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 18b22c0a26..c2089b5e6f 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1374,6 +1374,7 @@ int cmd_merge(int argc,\n \tstruct commit_list *remoteheads = NULL, *p;\n \tvoid *branch_to_free;\n \tint orig_argc = argc;\n+\tint merge_log_config = -1;\n \n \tshow_usage_with_options_if_asked(argc, argv,\n \t\t\t\t\t builtin_merge_usage, builtin_merge_options);\n@@ -1392,7 +1393,7 @@ int cmd_merge(int argc,\n \t\tskip_prefix(branch, \"refs/heads/\", &branch);\n \n \tinit_diff_ui_defaults();\n-\tgit_config(git_merge_config, NULL);\n+\tgit_config(git_merge_config, &merge_log_config);\n \n \tif (!branch || is_null_oid(&head_oid))\n \t\thead_commit = NULL;\ndiff --git a/environment.c b/environment.c\nindex 7c2480b22e..6751aa5683 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -66,7 +66,6 @@ int grafts_keep_true_parents;\n int core_apply_sparse_checkout;\n int core_sparse_checkout_cone;\n int sparse_expect_files_outside_of_patterns;\n-int merge_log_config = -1;\n int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */\n unsigned long pack_size_limit_cfg;\n int max_allowed_tree_depth =\ndiff --git a/fmt-merge-msg.c b/fmt-merge-msg.c\nindex 40174efa3d..c9085edc40 100644\n--- a/fmt-merge-msg.c\n+++ b/fmt-merge-msg.c\n@@ -26,13 +26,15 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;\n int fmt_merge_msg_config(const char *key, const char *value,\n \t\t\t const struct config_context *ctx, void *cb)\n {\n+\tint *merge_log_config = cb;\n+\n \tif (!strcmp(key, \"merge.log\") || !strcmp(key, \"merge.summary\")) {\n \t\tint is_bool;\n-\t\tmerge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);\n-\t\tif (!is_bool && merge_log_config < 0)\n+\t\t*merge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);\n+\t\tif (!is_bool && *merge_log_config < 0)\n \t\t\treturn error(\"%s: negative length %s\", key, value);\n-\t\tif (is_bool && merge_log_config)\n-\t\t\tmerge_log_config = DEFAULT_MERGE_LOG_LEN;\n+\t\tif (is_bool && *merge_log_config)\n+\t\t\t*merge_log_config = DEFAULT_MERGE_LOG_LEN;\n \t} else if (!strcmp(key, \"merge.branchdesc\")) {\n \t\tuse_branch_desc = git_config_bool(key, value);\n \t} else if (!strcmp(key, \"merge.suppressdest\")) {\ndiff --git a/fmt-merge-msg.h b/fmt-merge-msg.h\nindex 73ca3e4465..c066d83761 100644\n--- a/fmt-merge-msg.h\n+++ b/fmt-merge-msg.h\n@@ -12,7 +12,6 @@ struct fmt_merge_msg_opts {\n \tconst char *into_name;\n };\n \n-extern int merge_log_config;\n int fmt_merge_msg_config(const char *key, const char *value,\n \t\t\t const struct config_context *ctx, void *cb);\n int fmt_merge_msg(struct strbuf *in, struct strbuf *out,\n-- \n2.49.0\n\n"},{"id":"523903","messageId":"8e55516cdae9e8dc0ea85e2cd7b72ddc93790997.1754868681.git.ayu.chandekar@gmail.com","threadId":"63872","inReplyTo":"cover.1754868681.git.ayu.chandekar@gmail.com","subject":"[GSOC PATCH v2 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-08-10T23:45:46Z","receivedAt":"2025-08-10T23:46:24Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global\n'the_repository'. Remove the 'UNUSED' macro from the 'struct repository'\nparameter and replace 'git_config()' with 'repo_config()' so that\nconfiguration is read from the passed repository. Also, add a test to\nmake sure that \"git fmt-merge-msg -h\" can be called outside a\nrepository.\n\nMentored-by: Christian Couder \nMentored-by: Ghanshyam Thakkar \nSigned-off-by: Ayush Chandekar \n---\n builtin/fmt-merge-msg.c | 5 ++---\n t/t1517-outside-repo.sh | 7 +++++++\n 2 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 4b24de32fb..cf4273a52c 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -1,4 +1,3 @@\n-#define USE_THE_REPOSITORY_VARIABLE\n #include \"builtin.h\"\n #include \"config.h\"\n #include \"fmt-merge-msg.h\"\n@@ -13,7 +12,7 @@ static const char * const fmt_merge_msg_usage[] = {\n int cmd_fmt_merge_msg(int argc,\n \t\t const char **argv,\n \t\t const char *prefix,\n-\t\t struct repository *repo UNUSED)\n+\t\t struct repository *repo)\n {\n \tchar *inpath = NULL;\n \tconst char *message = NULL;\n@@ -54,7 +53,7 @@ int cmd_fmt_merge_msg(int argc,\n \tint ret;\n \tstruct fmt_merge_msg_opts opts;\n \n-\tgit_config(fmt_merge_msg_config, &merge_log_config);\n+\trepo_config(repo, fmt_merge_msg_config, &merge_log_config);\n \targc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,\n \t\t\t 0);\n \tif (argc > 0)\ndiff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh\nindex 8f59b867f2..4b4e645860 100755\n--- a/t/t1517-outside-repo.sh\n+++ b/t/t1517-outside-repo.sh\n@@ -121,4 +121,11 @@ test_expect_success 'prune does not crash with -h' '\n \ttest_grep \"[Uu]sage: git prune \" usage\n '\n \n+test_expect_success 'fmt-merge-msg does not crash with -h' '\n+\ttest_expect_code 129 git fmt-merge-msg -h >usage &&\n+\ttest_grep \"[Uu]sage: git fmt-merge-msg \" usage &&\n+\ttest_expect_code 129 nongit git fmt-merge-msg -h >usage &&\n+\ttest_grep \"[Uu]sage: git fmt-merge-msg \" usage\n+'\n+\n test_done\n-- \n2.49.0\n\n"},{"id":"523948","messageId":"076c19ae-58fc-4823-9679-1d5fe6e46211@gmail.com","threadId":"63872","inReplyTo":"3aa014ed46d14e31ea0c2f6b7631e7e4cbbd3943.1754868681.git.ayu.chandekar@gmail.com","subject":"Re: [GSOC PATCH v2 1/2] environment: remove the global variable 'merge_log_config'","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-08-11T14:42:48Z","receivedAt":"2025-08-11T14:42:52Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ayush\n\nOn 11/08/2025 00:45, Ayush Chandekar wrote:\n> The global variable 'merge_log_config', set via the \"merge.log\" or\n> \"merge.summary\" settings, is only used in 'cmd_fmt_merge_msg()' and\n> 'cmd_merge()' to adjust the 'shortlog_len' variable.\n> \n> Remove 'merge_log_config' globally and localize it in\n> 'cmd_fmt_merge_msg()' and 'cmd_merge()'. Set its value by passing it in\n> 'fmt_merge_msg_config()' by passing its pointer to the function via the\n> callback parameter.\n\nThis looks like a good solution\n\nThanks\n\nPhillip\n\n> This change is part of an ongoing effort to eliminate global variables,\n> improve modularity and help libify the codebase.\n> \n> Mentored-by: Christian Couder \n> Mentored-by: Ghanshyam Thakkar \n> Signed-off-by: Ayush Chandekar \n> ---\n> builtin/fmt-merge-msg.c | 3 ++-\n> builtin/merge.c | 3 ++-\n> environment.c | 1 -\n> fmt-merge-msg.c | 10 ++++++----\n> fmt-merge-msg.h | 1 -\n> 5 files changed, 10 insertions(+), 8 deletions(-)\n> \n> diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\n> index 3b6aac2cf7..4b24de32fb 100644\n> --- a/builtin/fmt-merge-msg.c\n> +++ b/builtin/fmt-merge-msg.c\n> @@ -19,6 +19,7 @@ int cmd_fmt_merge_msg(int argc,\n> \tconst char *message = NULL;\n> \tchar *into_name = NULL;\n> \tint shortlog_len = -1;\n> +\tint merge_log_config = -1;\n> \tstruct option options[] = {\n> \t\t{\n> \t\t\t.type = OPTION_INTEGER,\n> @@ -53,7 +54,7 @@ int cmd_fmt_merge_msg(int argc,\n> \tint ret;\n> \tstruct fmt_merge_msg_opts opts;\n> \n> -\tgit_config(fmt_merge_msg_config, NULL);\n> +\tgit_config(fmt_merge_msg_config, &merge_log_config);\n> \targc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,\n> \t\t\t 0);\n> \tif (argc > 0)\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index 18b22c0a26..c2089b5e6f 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -1374,6 +1374,7 @@ int cmd_merge(int argc,\n> \tstruct commit_list *remoteheads = NULL, *p;\n> \tvoid *branch_to_free;\n> \tint orig_argc = argc;\n> +\tint merge_log_config = -1;\n> \n> \tshow_usage_with_options_if_asked(argc, argv,\n> \t\t\t\t\t builtin_merge_usage, builtin_merge_options);\n> @@ -1392,7 +1393,7 @@ int cmd_merge(int argc,\n> \t\tskip_prefix(branch, \"refs/heads/\", &branch);\n> \n> \tinit_diff_ui_defaults();\n> -\tgit_config(git_merge_config, NULL);\n> +\tgit_config(git_merge_config, &merge_log_config);\n> \n> \tif (!branch || is_null_oid(&head_oid))\n> \t\thead_commit = NULL;\n> diff --git a/environment.c b/environment.c\n> index 7c2480b22e..6751aa5683 100644\n> --- a/environment.c\n> +++ b/environment.c\n> @@ -66,7 +66,6 @@ int grafts_keep_true_parents;\n> int core_apply_sparse_checkout;\n> int core_sparse_checkout_cone;\n> int sparse_expect_files_outside_of_patterns;\n> -int merge_log_config = -1;\n> int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */\n> unsigned long pack_size_limit_cfg;\n> int max_allowed_tree_depth =\n> diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c\n> index 40174efa3d..c9085edc40 100644\n> --- a/fmt-merge-msg.c\n> +++ b/fmt-merge-msg.c\n> @@ -26,13 +26,15 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;\n> int fmt_merge_msg_config(const char *key, const char *value,\n> \t\t\t const struct config_context *ctx, void *cb)\n> {\n> +\tint *merge_log_config = cb;\n> +\n> \tif (!strcmp(key, \"merge.log\") || !strcmp(key, \"merge.summary\")) {\n> \t\tint is_bool;\n> -\t\tmerge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);\n> -\t\tif (!is_bool && merge_log_config < 0)\n> +\t\t*merge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);\n> +\t\tif (!is_bool && *merge_log_config < 0)\n> \t\t\treturn error(\"%s: negative length %s\", key, value);\n> -\t\tif (is_bool && merge_log_config)\n> -\t\t\tmerge_log_config = DEFAULT_MERGE_LOG_LEN;\n> +\t\tif (is_bool && *merge_log_config)\n> +\t\t\t*merge_log_config = DEFAULT_MERGE_LOG_LEN;\n> \t} else if (!strcmp(key, \"merge.branchdesc\")) {\n> \t\tuse_branch_desc = git_config_bool(key, value);\n> \t} else if (!strcmp(key, \"merge.suppressdest\")) {\n> diff --git a/fmt-merge-msg.h b/fmt-merge-msg.h\n> index 73ca3e4465..c066d83761 100644\n> --- a/fmt-merge-msg.h\n> +++ b/fmt-merge-msg.h\n> @@ -12,7 +12,6 @@ struct fmt_merge_msg_opts {\n> \tconst char *into_name;\n> };\n> \n> -extern int merge_log_config;\n> int fmt_merge_msg_config(const char *key, const char *value,\n> \t\t\t const struct config_context *ctx, void *cb);\n> int fmt_merge_msg(struct strbuf *in, struct strbuf *out,\n\n"},{"id":"523958","messageId":"xmqqikit3kgo.fsf@gitster.g","threadId":"63872","inReplyTo":"076c19ae-58fc-4823-9679-1d5fe6e46211@gmail.com","subject":"Re: [GSOC PATCH v2 1/2] environment: remove the global variable 'merge_log_config'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-08-11T16:13:27Z","receivedAt":"2025-08-11T16:13:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> Hi Ayush\n>\n> On 11/08/2025 00:45, Ayush Chandekar wrote:\n>> The global variable 'merge_log_config', set via the \"merge.log\" or\n>> \"merge.summary\" settings, is only used in 'cmd_fmt_merge_msg()' and\n>> 'cmd_merge()' to adjust the 'shortlog_len' variable.\n>> Remove 'merge_log_config' globally and localize it in\n>> 'cmd_fmt_merge_msg()' and 'cmd_merge()'. Set its value by passing it in\n>> 'fmt_merge_msg_config()' by passing its pointer to the function via the\n>> callback parameter.\n>\n> This looks like a good solution\n\nWhen fmt_merge_msg_config() needs to read more stuff, the callback\nparameter may have to be updated, but this will do for now.\n\nThanks.\n"},{"id":"523960","messageId":"CAE7as+YdYmmUvVxzY+CeA2vNU01av_QtvsmcUTy=FibqVRrudA@mail.gmail.com","threadId":"63872","inReplyTo":"xmqqikit3kgo.fsf@gitster.g","subject":"Re: [GSOC PATCH v2 1/2] environment: remove the global variable 'merge_log_config'","fromName":"Ayush Chandekar","fromEmail":"ayu.chandekar@gmail.com","sentAt":"2025-08-11T18:25:34Z","receivedAt":"2025-08-11T18:25:48Z","isPatch":true,"sender":{"key":"ayu.chandekar@gmail.com","avatar":"https://avatars.githubusercontent.com/u/137001939?v=4"},"body":"On Mon, Aug 11, 2025 at 9:43 PM Junio C Hamano wrote:\n>\n> Phillip Wood writes:\n>\n> > Hi Ayush\n> >\n> > On 11/08/2025 00:45, Ayush Chandekar wrote:\n> >> The global variable 'merge_log_config', set via the \"merge.log\" or\n> >> \"merge.summary\" settings, is only used in 'cmd_fmt_merge_msg()' and\n> >> 'cmd_merge()' to adjust the 'shortlog_len' variable.\n> >> Remove 'merge_log_config' globally and localize it in\n> >> 'cmd_fmt_merge_msg()' and 'cmd_merge()'. Set its value by passing it in\n> >> 'fmt_merge_msg_config()' by passing its pointer to the function via the\n> >> callback parameter.\n> >\n> > This looks like a good solution\n>\n> When fmt_merge_msg_config() needs to read more stuff, the callback\n> parameter may have to be updated, but this will do for now.\n>\n> Thanks.\n\nYes, then we can create a struct and pass the struct instead, maybe.\n\nThanks,\nAyush\n"}]}