threads / patch / 63872

patch, 2 partsbuiltin/fmt-merge-msg: remove dependency on global variables and 'the_repository'

Subject: [GSOC PATCH 0/2] builtin/fmt-merge-msg: remove dependency on global variables and 'the_repository'

## tl;dr

19 messages between Jul 29, 2025 and Aug 11, 2025. Diffs are folded; open one to read it.

replies: 18people: 3as markdown or json

Ayush Chandekar· Jul 29, 2025, 16:19 UTC · lore

The aim of this patch series is to remove the definition '#define USE_THE_REPOSITORY_VARIABLE' from "builtin/fmt-merge-msg.c" by removing global variable 'merge_log_config' and the global 'the_repository'

This patch series contains two patches:
1 - Remove the global varaible 'merge_log_config' and add a function 'adjust_shortlog_len()'
    in fmt-merge-msg.{c,h} to replicate the variable's usage.
2 - Remove the dependency of 'the_repository' in "builtin/fmt-merge-msg.c", allowing the removal
    of the definition '#define USE_THE_REPOSITORY_VARIABLE'. Also add a test to make sure that
    "git fmt-merge-msg -h" can be called with repository being NULL.
Ayush Chandekar (2):
  environment: remove the global variable 'merge_log_config'
  builtin/fmt-merge-msg: stop depending on 'the_repository'
 builtin/fmt-merge-msg.c |  9 ++++-----
 builtin/merge.c         |  4 ++--
 environment.c           |  2 --
 fmt-merge-msg.c         | 30 ++++++++++++++++++++++--------
 fmt-merge-msg.h         |  3 ++-
 t/t1517-outside-repo.sh |  7 +++++++
 6 files changed, 37 insertions(+), 18 deletions(-)
-- 
2.49.0
Ayush Chandekar· Jul 29, 2025, 16:19 UTC · re: Ayush Chandekar · lore

[GSOC PATCH 1/2] environment: remove the global variable 'merge_log_config'

The global variable 'merge_log_config', set via the "merge.log" or "merge.summary" settings, is only used in 'cmd_fmt_merge_msg()' and 'cmd_merge()' to adjust the 'shortlog_len' variable.

Remove 'merge_log_config' and introduce a function 'adjust_shortlog_len()' in fmt-merge-msg.c to handle the 'shortlog_len' variable.

This change is part of an ongoing effort to eliminate global variables, improve modularity and help libify the codebase.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/fmt-merge-msg.c |  4 ++--
 builtin/merge.c         |  4 ++--
 environment.c           |  2 --
 fmt-merge-msg.c         | 30 ++++++++++++++++++++++--------
 fmt-merge-msg.h         |  3 ++-
 5 files changed, 28 insertions(+), 15 deletions(-)
Show changes to 5 files +28 −15

builtin/fmt-merge-msg.c, builtin/merge.c, environment.c, fmt-merge-msg.c, fmt-merge-msg.h

diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 3b6aac2cf7..fed8163825 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -58,8 +58,8 @@ int cmd_fmt_merge_msg(int argc,
 			     0);
 	if (argc > 0)
 		usage_with_options(fmt_merge_msg_usage, options);
-	if (shortlog_len < 0)
-		shortlog_len = (merge_log_config > 0) ? merge_log_config : 0;
+
+	adjust_shortlog_len(the_repository, &shortlog_len);
 
 	if (inpath && strcmp(inpath, "-")) {
 		in = fopen(inpath, "r");
diff --git a/builtin/merge.c b/builtin/merge.c
index 18b22c0a26..e1cf2a6d63 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1403,8 +1403,8 @@ int cmd_merge(int argc,
 		parse_branch_merge_options(branch_mergeoptions);
 	argc = parse_options(argc, argv, prefix, builtin_merge_options,
 			builtin_merge_usage, 0);
-	if (shortlog_len < 0)
-		shortlog_len = (merge_log_config > 0) ? merge_log_config : 0;
+
+	adjust_shortlog_len(the_repository, &shortlog_len);
 
 	if (verbosity < 0 && show_progress == -1)
 		show_progress = 0;
diff --git a/environment.c b/environment.c
index 7c2480b22e..74c9838b31 100644
--- a/environment.c
+++ b/environment.c
@@ -20,7 +20,6 @@
 #include "repository.h"
 #include "config.h"
 #include "refs.h"
-#include "fmt-merge-msg.h"
 #include "commit.h"
 #include "strvec.h"
 #include "path.h"
@@ -66,7 +65,6 @@ int grafts_keep_true_parents;
 int core_apply_sparse_checkout;
 int core_sparse_checkout_cone;
 int sparse_expect_files_outside_of_patterns;
-int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
 unsigned long pack_size_limit_cfg;
 int max_allowed_tree_depth =
diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
index 40174efa3d..2ceaebb0ce 100644
--- a/fmt-merge-msg.c
+++ b/fmt-merge-msg.c
@@ -26,14 +26,7 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;
 int fmt_merge_msg_config(const char *key, const char *value,
 			 const struct config_context *ctx, void *cb)
 {
-	if (!strcmp(key, "merge.log") || !strcmp(key, "merge.summary")) {
-		int is_bool;
-		merge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);
-		if (!is_bool && merge_log_config < 0)
-			return error("%s: negative length %s", key, value);
-		if (is_bool && merge_log_config)
-			merge_log_config = DEFAULT_MERGE_LOG_LEN;
-	} else if (!strcmp(key, "merge.branchdesc")) {
+	if (!strcmp(key, "merge.branchdesc")) {
 		use_branch_desc = git_config_bool(key, value);
 	} else if (!strcmp(key, "merge.suppressdest")) {
 		if (!value)
@@ -645,6 +638,27 @@ static void find_merge_parents(struct merge_parents *result,
 	result->nr = j;
 }
 
+void adjust_shortlog_len(struct repository *r, int *shortlog_len)
+{
+	const char *keys[] = { "merge.log", "merge.summary", NULL};
+	
+	if (*shortlog_len >= 0)
+		return;
+
+	for (const char **key = keys; *key; ++key) {
+		int is_bool, value;
+		if (!repo_config_get_bool_or_int(r, *key, &is_bool, &value)) {
+			if (!is_bool && value < 0) {
+				error("%s: negative length %d", *key, value);
+				return;
+			}
+			*shortlog_len = (is_bool && value) ? DEFAULT_MERGE_LOG_LEN : value;
+			return;
+		}
+	}
+
+	*shortlog_len = 0;
+}
 
 int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 		  struct fmt_merge_msg_opts *opts)
diff --git a/fmt-merge-msg.h b/fmt-merge-msg.h
index 73ca3e4465..f54f00d26f 100644
--- a/fmt-merge-msg.h
+++ b/fmt-merge-msg.h
@@ -2,6 +2,7 @@
 #define FMT_MERGE_MSG_H
 
 #include "strbuf.h"
+#include "repository.h"
 
 #define DEFAULT_MERGE_LOG_LEN 20
 
@@ -12,9 +13,9 @@ struct fmt_merge_msg_opts {
 	const char *into_name;
 };
 
-extern int merge_log_config;
 int fmt_merge_msg_config(const char *key, const char *value,
 			 const struct config_context *ctx, void *cb);
+void adjust_shortlog_len(struct repository *r, int *shortlog_len);
 int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 		  struct fmt_merge_msg_opts *);
 
-- 
2.49.0
Junio C Hamano· Jul 29, 2025, 16:48 UTC · re: Ayush Chandekar · lore

Re: [GSOC PATCH 1/2] environment: remove the global variable 'merge_log_config'

Ayush Chandekar <ayu.chandekar@gmail.com> writes:
Show 10 quoted lines
> The global variable 'merge_log_config', set via the "merge.log" or
> "merge.summary" settings, is only used in 'cmd_fmt_merge_msg()' and
> 'cmd_merge()' to adjust the 'shortlog_len' variable.
>
> Remove 'merge_log_config' and introduce a function
> 'adjust_shortlog_len()' in fmt-merge-msg.c to handle the 'shortlog_len'
> variable.
>
> This change is part of an ongoing effort to eliminate global variables,
> improve modularity and help libify the codebase.
And the downsides of this change are...?

One obvious behaviour change I can see can happen when you have an invalid value set to merge.summary and run the command with command line override with the "--log" option. In the current code, the config callback barfs when it notices an invalid merge.summary setting, even though it won't be used because the valid value given via the "--log" option would override it. In the updated code, adjust_shortlog_len() would short-circuit and does not even bother reading from the configuration, so the user will not be notified of a broken configuration.

It is not immediately obvious if this particular behaviour change is a regression or an improvement, but it probably deserves to be noted somewhere to help future developers what our thinking was.

Show 66 quoted lines
> @@ -26,14 +26,7 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;
>  int fmt_merge_msg_config(const char *key, const char *value,
>  			 const struct config_context *ctx, void *cb)
>  {
> -	if (!strcmp(key, "merge.log") || !strcmp(key, "merge.summary")) {
> -		int is_bool;
> -		merge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);
> -		if (!is_bool && merge_log_config < 0)
> -			return error("%s: negative length %s", key, value);
> -		if (is_bool && merge_log_config)
> -			merge_log_config = DEFAULT_MERGE_LOG_LEN;
> -	} else if (!strcmp(key, "merge.branchdesc")) {
> +	if (!strcmp(key, "merge.branchdesc")) {
>  		use_branch_desc = git_config_bool(key, value);
>  	} else if (!strcmp(key, "merge.suppressdest")) {
>  		if (!value)
> @@ -645,6 +638,27 @@ static void find_merge_parents(struct merge_parents *result,
>  	result->nr = j;
>  }
>  
> +void adjust_shortlog_len(struct repository *r, int *shortlog_len)
> +{
> +	const char *keys[] = { "merge.log", "merge.summary", NULL};
> +	
> +	if (*shortlog_len >= 0)
> +		return;
> +
> +	for (const char **key = keys; *key; ++key) {
> +		int is_bool, value;
> +		if (!repo_config_get_bool_or_int(r, *key, &is_bool, &value)) {
> +			if (!is_bool && value < 0) {
> +				error("%s: negative length %d", *key, value);
> +				return;
> +			}
> +			*shortlog_len = (is_bool && value) ? DEFAULT_MERGE_LOG_LEN : value;
> +			return;
> +		}
> +	}
> +
> +	*shortlog_len = 0;
> +}
>  
>  int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
>  		  struct fmt_merge_msg_opts *opts)
> diff --git a/fmt-merge-msg.h b/fmt-merge-msg.h
> index 73ca3e4465..f54f00d26f 100644
> --- a/fmt-merge-msg.h
> +++ b/fmt-merge-msg.h
> @@ -2,6 +2,7 @@
>  #define FMT_MERGE_MSG_H
>  
>  #include "strbuf.h"
> +#include "repository.h"
>  
>  #define DEFAULT_MERGE_LOG_LEN 20
>  
> @@ -12,9 +13,9 @@ struct fmt_merge_msg_opts {
>  	const char *into_name;
>  };
>  
> -extern int merge_log_config;
>  int fmt_merge_msg_config(const char *key, const char *value,
>  			 const struct config_context *ctx, void *cb);
> +void adjust_shortlog_len(struct repository *r, int *shortlog_len);
>  int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
>  		  struct fmt_merge_msg_opts *);
Ayush Chandekar· Jul 29, 2025, 17:30 UTC · re: Junio C Hamano · lore

Re: [GSOC PATCH 1/2] environment: remove the global variable 'merge_log_config'

Hi Junio,
On Tue, Jul 29, 2025 at 10:18 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 29 quoted lines
>
> Ayush Chandekar <ayu.chandekar@gmail.com> writes:
>
> > The global variable 'merge_log_config', set via the "merge.log" or
> > "merge.summary" settings, is only used in 'cmd_fmt_merge_msg()' and
> > 'cmd_merge()' to adjust the 'shortlog_len' variable.
> >
> > Remove 'merge_log_config' and introduce a function
> > 'adjust_shortlog_len()' in fmt-merge-msg.c to handle the 'shortlog_len'
> > variable.
> >
> > This change is part of an ongoing effort to eliminate global variables,
> > improve modularity and help libify the codebase.
>
> And the downsides of this change are...?
>
> One obvious behaviour change I can see can happen when you have an
> invalid value set to merge.summary and run the command with command
> line override with the "--log" option.  In the current code, the
> config callback barfs when it notices an invalid merge.summary
> setting, even though it won't be used because the valid value given
> via the "--log" option would override it.  In the updated code,
> adjust_shortlog_len() would short-circuit and does not even bother
> reading from the configuration, so the user will not be notified of
> a broken configuration.
>
> It is not immediately obvious if this particular behaviour change is
> a regression or an improvement, but it probably deserves to be noted
> somewhere to help future developers what our thinking was.

Oh right, I did not mention this in the commit message. I am not sure if this behaviour is good or not.

Technically, if the user wants to use the "--log" option, they would not care about the config. Whereas, if the user wants to use the config, they would be notified in case of an invalid one.

I will mention this in the commit message, but do you think this behaviour is fine?

Thanks Ayush

Junio C Hamano· Jul 29, 2025, 17:53 UTC · re: Ayush Chandekar · lore

Re: [GSOC PATCH 1/2] environment: remove the global variable 'merge_log_config'

Ayush Chandekar <ayu.chandekar@gmail.com> writes:
> I will mention this in the commit message, but do you think this
> behaviour is fine?

I cannot answer that one immediately after saying that I do not know if this is a regression or an improvement.

If you still pushed me to answer it immediately, you'd get my default position, a conservative "any changes in behaviour caused by an internal code clean-up is bad and is a serious regression" ;-).

Phillip Wood· Jul 29, 2025, 19:07 UTC · re: Ayush Chandekar · lore

Re: [GSOC PATCH 1/2] environment: remove the global variable 'merge_log_config'

Hi Ayush
On 29/07/2025 17:19, Ayush Chandekar wrote:
Show 13 quoted lines
> 
> @@ -26,14 +26,7 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;
>   int fmt_merge_msg_config(const char *key, const char *value,
>   			 const struct config_context *ctx, void *cb)
>   {
> -	if (!strcmp(key, "merge.log") || !strcmp(key, "merge.summary")) {
> -		int is_bool;
> -		merge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);
> -		if (!is_bool && merge_log_config < 0)
> -			return error("%s: negative length %s", key, value);
> -		if (is_bool && merge_log_config)
> -			merge_log_config = DEFAULT_MERGE_LOG_LEN;
> -	} else if (!strcmp(key, "merge.branchdesc")) {

In the old code if both "merge.log" and "merge.summary" are set in the config file the last one wins

Show 16 quoted lines
> +void adjust_shortlog_len(struct repository *r, int *shortlog_len)
> +{
> +	const char *keys[] = { "merge.log", "merge.summary", NULL};
> +	
> +	if (*shortlog_len >= 0)
> +		return;
> +
> +	for (const char **key = keys; *key; ++key) {
> +		int is_bool, value;
> +		if (!repo_config_get_bool_or_int(r, *key, &is_bool, &value)) {
> +			if (!is_bool && value < 0) {
> +				error("%s: negative length %d", *key, value);
> +				return;
> +			}
> +			*shortlog_len = (is_bool && value) ? DEFAULT_MERGE_LOG_LEN : value;
> +			return;

In the new code "merge.log" is always used in preference to "merge.summary" even if "merge.summary" appears later in the config file. When you have two keys setting the same variable I think the only way to preserve the last one wins behavior is to keep using a callback that updates the value as the config files are parsed.

Thanks
Phillip
Ayush Chandekar· Jul 29, 2025, 21:16 UTC · re: Phillip Wood · lore

Re: [GSOC PATCH 1/2] environment: remove the global variable 'merge_log_config'

Hi Phillip,
On Wed, Jul 30, 2025 at 12:37 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
Show 44 quoted lines
>
> Hi Ayush
>
> On 29/07/2025 17:19, Ayush Chandekar wrote:
> >
> > @@ -26,14 +26,7 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;
> >   int fmt_merge_msg_config(const char *key, const char *value,
> >                        const struct config_context *ctx, void *cb)
> >   {
> > -     if (!strcmp(key, "merge.log") || !strcmp(key, "merge.summary")) {
> > -             int is_bool;
> > -             merge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);
> > -             if (!is_bool && merge_log_config < 0)
> > -                     return error("%s: negative length %s", key, value);
> > -             if (is_bool && merge_log_config)
> > -                     merge_log_config = DEFAULT_MERGE_LOG_LEN;
> > -     } else if (!strcmp(key, "merge.branchdesc")) {
>
> In the old code if both "merge.log" and "merge.summary" are set in the
> config file the last one wins
>
> > +void adjust_shortlog_len(struct repository *r, int *shortlog_len)
> > +{
> > +     const char *keys[] = { "merge.log", "merge.summary", NULL};
> > +
> > +     if (*shortlog_len >= 0)
> > +             return;
> > +
> > +     for (const char **key = keys; *key; ++key) {
> > +             int is_bool, value;
> > +             if (!repo_config_get_bool_or_int(r, *key, &is_bool, &value)) {
> > +                     if (!is_bool && value < 0) {
> > +                             error("%s: negative length %d", *key, value);
> > +                             return;
> > +                     }
> > +                     *shortlog_len = (is_bool && value) ? DEFAULT_MERGE_LOG_LEN : value;
> > +                     return;
>
> In the new code "merge.log" is always used in preference to
> "merge.summary" even if "merge.summary" appears later in the config
> file. When you have two keys setting the same variable I think the only
> way to preserve the last one wins behavior is to keep using a callback
> that updates the value as the config files are parsed.
>
Sorry for not mentioning this in the commit message.
I had looked at the documentation which says:

Documentation/git-fmt-merge-msg.adoc merge.summary:: Synonym to `merge.log`; this is deprecated and will be removed in the future.

So I thought that I should give precedence to "merge.log" as "merge.summary" is deprecated.

> Thanks
>
> Phillip
>

Thanks Ayush

Phillip Wood· Jul 30, 2025, 08:53 UTC · re: Ayush Chandekar · lore

Re: [GSOC PATCH 1/2] environment: remove the global variable 'merge_log_config'

Hi Ayush
On 29/07/2025 22:16, Ayush Chandekar wrote:
Show 58 quoted lines
>  
> On Wed, Jul 30, 2025 at 12:37 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> Hi Ayush
>>
>> On 29/07/2025 17:19, Ayush Chandekar wrote:
>>>
>>> @@ -26,14 +26,7 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;
>>>    int fmt_merge_msg_config(const char *key, const char *value,
>>>                         const struct config_context *ctx, void *cb)
>>>    {
>>> -     if (!strcmp(key, "merge.log") || !strcmp(key, "merge.summary")) {
>>> -             int is_bool;
>>> -             merge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);
>>> -             if (!is_bool && merge_log_config < 0)
>>> -                     return error("%s: negative length %s", key, value);
>>> -             if (is_bool && merge_log_config)
>>> -                     merge_log_config = DEFAULT_MERGE_LOG_LEN;
>>> -     } else if (!strcmp(key, "merge.branchdesc")) {
>>
>> In the old code if both "merge.log" and "merge.summary" are set in the
>> config file the last one wins
>>
>>> +void adjust_shortlog_len(struct repository *r, int *shortlog_len)
>>> +{
>>> +     const char *keys[] = { "merge.log", "merge.summary", NULL};
>>> +
>>> +     if (*shortlog_len >= 0)
>>> +             return;
>>> +
>>> +     for (const char **key = keys; *key; ++key) {
>>> +             int is_bool, value;
>>> +             if (!repo_config_get_bool_or_int(r, *key, &is_bool, &value)) {
>>> +                     if (!is_bool && value < 0) {
>>> +                             error("%s: negative length %d", *key, value);
>>> +                             return;
>>> +                     }
>>> +                     *shortlog_len = (is_bool && value) ? DEFAULT_MERGE_LOG_LEN : value;
>>> +                     return;
>>
>> In the new code "merge.log" is always used in preference to
>> "merge.summary" even if "merge.summary" appears later in the config
>> file. When you have two keys setting the same variable I think the only
>> way to preserve the last one wins behavior is to keep using a callback
>> that updates the value as the config files are parsed.
>>
> 
> Sorry for not mentioning this in the commit message.
> 
> I had looked at the documentation which says:
> 
> Documentation/git-fmt-merge-msg.adoc
> merge.summary::
> Synonym to `merge.log`; this is deprecated and will be removed in
> the future.
> 
> So I thought that I should give precedence to "merge.log" as
> "merge.summary" is deprecated.

If it is deprecated we still need to support it until it is removed (maybe we should do that in Git 3.0?). We cannot change the behavior just because a setting is deprecated.

Thanks
Phillip
Show 8 quoted lines
>> Thanks
>>
>> Phillip
>>
> 
> Thanks
> Ayush
> 
Ayush Chandekar· Jul 29, 2025, 16:19 UTC · re: Ayush Chandekar · lore

[GSOC PATCH 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'

Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global 'the_repository'. Replace all the occurrences of 'the_repository' with 'repo', where 'repo' is a pointer to 'struct repository' passed to the function 'cmd_fmt_merge_msg()' and thus remove the definition '#define USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that "git fmt-merge-msg -h" can be called outside a repository.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/fmt-merge-msg.c | 7 +++----
 t/t1517-outside-repo.sh | 7 +++++++
 2 files changed, 10 insertions(+), 4 deletions(-)
Show changes to 2 files +10 −4

builtin/fmt-merge-msg.c, t/t1517-outside-repo.sh

diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index fed8163825..848498b8e6 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #include "builtin.h"
 #include "config.h"
 #include "fmt-merge-msg.h"
@@ -13,7 +12,7 @@ static const char * const fmt_merge_msg_usage[] = {
 int cmd_fmt_merge_msg(int argc,
 		      const char **argv,
 		      const char *prefix,
-		      struct repository *repo UNUSED)
+		      struct repository *repo)
 {
 	char *inpath = NULL;
 	const char *message = NULL;
@@ -53,13 +52,13 @@ int cmd_fmt_merge_msg(int argc,
 	int ret;
 	struct fmt_merge_msg_opts opts;
 
-	git_config(fmt_merge_msg_config, NULL);
 	argc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,
 			     0);
 	if (argc > 0)
 		usage_with_options(fmt_merge_msg_usage, options);
+	repo_config(repo, fmt_merge_msg_config, NULL);
 
-	adjust_shortlog_len(the_repository, &shortlog_len);
+	adjust_shortlog_len(repo, &shortlog_len);
 
 	if (inpath && strcmp(inpath, "-")) {
 		in = fopen(inpath, "r");
diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
index 8f59b867f2..4b4e645860 100755
--- a/t/t1517-outside-repo.sh
+++ b/t/t1517-outside-repo.sh
@@ -121,4 +121,11 @@ test_expect_success 'prune does not crash with -h' '
 	test_grep "[Uu]sage: git prune " usage
 '
 
+test_expect_success 'fmt-merge-msg does not crash with -h' '
+	test_expect_code 129 git fmt-merge-msg -h >usage &&
+	test_grep "[Uu]sage: git fmt-merge-msg " usage &&
+	test_expect_code 129 nongit git fmt-merge-msg -h >usage &&
+	test_grep "[Uu]sage: git fmt-merge-msg " usage
+'
+
 test_done
-- 
2.49.0
Junio C Hamano· Jul 29, 2025, 16:41 UTC · re: Ayush Chandekar · lore

Re: [GSOC PATCH 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'

Ayush Chandekar <ayu.chandekar@gmail.com> writes:
Show 6 quoted lines
> Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global
> 'the_repository'. Replace all the occurrences of 'the_repository' with
> 'repo', where 'repo' is a pointer to 'struct repository' passed to the
> function 'cmd_fmt_merge_msg()' and thus remove the definition '#define
> USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that "git
> fmt-merge-msg -h" can be called outside a repository.

This also moves the call to git_config()/repo_config() after parse_options().

It generally is a bad idea to read command line options first and then read the configuration (it is a bug if such a flow causes values from configuration to overwrite values from command line). THe current set of options and configuration variables may not overlap, in which case such a questionable arrangement happen to be without bug right now, but it would prevent future developers from adding new options and configuration variables and make them interact with each other in the most natural way.

In any case, the reason for this change of the order between config and parse-options is not explained at all in the proposed log message.

Show 58 quoted lines
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> ---
>  builtin/fmt-merge-msg.c | 7 +++----
>  t/t1517-outside-repo.sh | 7 +++++++
>  2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
> index fed8163825..848498b8e6 100644
> --- a/builtin/fmt-merge-msg.c
> +++ b/builtin/fmt-merge-msg.c
> @@ -1,4 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
>  #include "builtin.h"
>  #include "config.h"
>  #include "fmt-merge-msg.h"
> @@ -13,7 +12,7 @@ static const char * const fmt_merge_msg_usage[] = {
>  int cmd_fmt_merge_msg(int argc,
>  		      const char **argv,
>  		      const char *prefix,
> -		      struct repository *repo UNUSED)
> +		      struct repository *repo)
>  {
>  	char *inpath = NULL;
>  	const char *message = NULL;
> @@ -53,13 +52,13 @@ int cmd_fmt_merge_msg(int argc,
>  	int ret;
>  	struct fmt_merge_msg_opts opts;
>  
> -	git_config(fmt_merge_msg_config, NULL);
>  	argc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,
>  			     0);
>  	if (argc > 0)
>  		usage_with_options(fmt_merge_msg_usage, options);
> +	repo_config(repo, fmt_merge_msg_config, NULL);
>  
> -	adjust_shortlog_len(the_repository, &shortlog_len);
> +	adjust_shortlog_len(repo, &shortlog_len);
>  
>  	if (inpath && strcmp(inpath, "-")) {
>  		in = fopen(inpath, "r");
> diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
> index 8f59b867f2..4b4e645860 100755
> --- a/t/t1517-outside-repo.sh
> +++ b/t/t1517-outside-repo.sh
> @@ -121,4 +121,11 @@ test_expect_success 'prune does not crash with -h' '
>  	test_grep "[Uu]sage: git prune " usage
>  '
>  
> +test_expect_success 'fmt-merge-msg does not crash with -h' '
> +	test_expect_code 129 git fmt-merge-msg -h >usage &&
> +	test_grep "[Uu]sage: git fmt-merge-msg " usage &&
> +	test_expect_code 129 nongit git fmt-merge-msg -h >usage &&
> +	test_grep "[Uu]sage: git fmt-merge-msg " usage
> +'
> +
>  test_done
Ayush Chandekar· Jul 29, 2025, 21:49 UTC · re: Junio C Hamano · lore

Re: [GSOC PATCH 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'

On Tue, Jul 29, 2025 at 10:11 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 22 quoted lines
>
> Ayush Chandekar <ayu.chandekar@gmail.com> writes:
>
> > Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global
> > 'the_repository'. Replace all the occurrences of 'the_repository' with
> > 'repo', where 'repo' is a pointer to 'struct repository' passed to the
> > function 'cmd_fmt_merge_msg()' and thus remove the definition '#define
> > USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that "git
> > fmt-merge-msg -h" can be called outside a repository.
>
> This also moves the call to git_config()/repo_config() after
> parse_options().
>
> It generally is a bad idea to read command line options first and
> then read the configuration (it is a bug if such a flow causes
> values from configuration to overwrite values from command line).
> THe current set of options and configuration variables may not
> overlap, in which case such a questionable arrangement happen to be
> without bug right now, but it would prevent future developers from
> adding new options and configuration variables and make them
> interact with each other in the most natural way.
>
I understand it, but how do we tackle if NULL repository is passed.
> In any case, the reason for this change of the order between config
> and parse-options is not explained at all in the proposed log
> message.
>
 Apologies, I will mention it.
Junio C Hamano· Jul 29, 2025, 22:41 UTC · re: Ayush Chandekar · lore

Re: [GSOC PATCH 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'

Ayush Chandekar <ayu.chandekar@gmail.com> writes:
Show 10 quoted lines
>> It generally is a bad idea to read command line options first and
>> then read the configuration (it is a bug if such a flow causes
>> values from configuration to overwrite values from command line).
>> THe current set of options and configuration variables may not
>> overlap, in which case such a questionable arrangement happen to be
>> without bug right now, but it would prevent future developers from
>> adding new options and configuration variables and make them
>> interact with each other in the most natural way.
>
> I understand it, but how do we tackle if NULL repository is passed.

Perhaps you want to study the problem space and related past changes before going forward. The first place to look at is what happens when you call repo_config(), outside a repository or a working tree and repository is NULL.

f29f1990 (config: teach repo_config to allow `repo` to be NULL, 2025-03-08) and what it calls "the following commits" may be illuminating.

Ayush Chandekar· Aug 10, 2025, 23:45 UTC · re: Ayush Chandekar · lore

[GSOC PATCH v2 0/2] builtin/fmt-merge-msg: remove dependency on global variables and 'the_repository'

The aim of this patch series is to remove the definition '#define USE_THE_REPOSITORY_VARIABLE' from "builtin/fmt-merge-msg.c" by removing global variable 'merge_log_config' and the global 'the_repository'

This patch series contains two patches:
1 - Remove the global varaible 'merge_log_config' and localize it in
    'cmd_fmt_merge_msg()' and 'cmd_merge()'. Set its value by passing it in
    'fmt_merge_msg_config()' by passing its pointer to the function via the
    callback parameter.
2 - Remove the dependency of 'the_repository' in "builtin/fmt-merge-msg.c", allowing the removal
    of the definition '#define USE_THE_REPOSITORY_VARIABLE'. Also add a test to make sure that
    "git fmt-merge-msg -h" can be called with repository being NULL.
Thanks to Junio and Phillip for reviewing my patch series and Christian for mentoring me!
Ayush Chandekar (2):
  environment: remove the global variable 'merge_log_config'
  builtin/fmt-merge-msg: stop depending on 'the_repository'
 builtin/fmt-merge-msg.c |  6 +++---
 builtin/merge.c         |  3 ++-
 environment.c           |  1 -
 fmt-merge-msg.c         | 10 ++++++----
 fmt-merge-msg.h         |  1 -
 t/t1517-outside-repo.sh |  7 +++++++
 6 files changed, 18 insertions(+), 10 deletions(-)
Range-diff against v1:
1:  c82620a1f5 < -:  ---------- environment: remove the global variable 'merge_log_config'
-:  ---------- > 1:  3aa014ed46 environment: remove the global variable 'merge_log_config'
2:  04d6f682a6 ! 2:  8e55516cda builtin/fmt-merge-msg: stop depending on 'the_repository'
    @@ Commit message
         builtin/fmt-merge-msg: stop depending on 'the_repository'
     
         Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global
    -    'the_repository'. Replace all the occurrences of 'the_repository' with
    -    'repo', where 'repo' is a pointer to 'struct repository' passed to the
    -    function 'cmd_fmt_merge_msg()' and thus remove the definition '#define
    -    USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that "git
    -    fmt-merge-msg -h" can be called outside a repository.
    +    'the_repository'. Remove the 'UNUSED' macro from the 'struct repository'
    +    parameter and replace 'git_config()' with 'repo_config()' so that
    +    configuration is read from the passed repository. Also, add a test to
    +    make sure that "git fmt-merge-msg -h" can be called outside a
    +    repository.
     
         Mentored-by: Christian Couder <christian.couder@gmail.com>
         Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
    @@ builtin/fmt-merge-msg.c: int cmd_fmt_merge_msg(int argc,
      	int ret;
      	struct fmt_merge_msg_opts opts;
      
    --	git_config(fmt_merge_msg_config, NULL);
    +-	git_config(fmt_merge_msg_config, &merge_log_config);
    ++	repo_config(repo, fmt_merge_msg_config, &merge_log_config);
      	argc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,
      			     0);
      	if (argc > 0)
    - 		usage_with_options(fmt_merge_msg_usage, options);
    -+	repo_config(repo, fmt_merge_msg_config, NULL);
    - 
    --	adjust_shortlog_len(the_repository, &shortlog_len);
    -+	adjust_shortlog_len(repo, &shortlog_len);
    - 
    - 	if (inpath && strcmp(inpath, "-")) {
    - 		in = fopen(inpath, "r");
     
      ## t/t1517-outside-repo.sh ##
     @@ t/t1517-outside-repo.sh: test_expect_success 'prune does not crash with -h' '
-- 
2.49.0
Ayush Chandekar· Aug 10, 2025, 23:45 UTC · re: Ayush Chandekar · lore

[GSOC PATCH v2 1/2] environment: remove the global variable 'merge_log_config'

The global variable 'merge_log_config', set via the "merge.log" or "merge.summary" settings, is only used in 'cmd_fmt_merge_msg()' and 'cmd_merge()' to adjust the 'shortlog_len' variable.

Remove 'merge_log_config' globally and localize it in 'cmd_fmt_merge_msg()' and 'cmd_merge()'. Set its value by passing it in 'fmt_merge_msg_config()' by passing its pointer to the function via the callback parameter.

This change is part of an ongoing effort to eliminate global variables, improve modularity and help libify the codebase.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/fmt-merge-msg.c |  3 ++-
 builtin/merge.c         |  3 ++-
 environment.c           |  1 -
 fmt-merge-msg.c         | 10 ++++++----
 fmt-merge-msg.h         |  1 -
 5 files changed, 10 insertions(+), 8 deletions(-)
Show changes to 5 files +10 −8

builtin/fmt-merge-msg.c, builtin/merge.c, environment.c, fmt-merge-msg.c, fmt-merge-msg.h

diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 3b6aac2cf7..4b24de32fb 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -19,6 +19,7 @@ int cmd_fmt_merge_msg(int argc,
 	const char *message = NULL;
 	char *into_name = NULL;
 	int shortlog_len = -1;
+	int merge_log_config = -1;
 	struct option options[] = {
 		{
 			.type = OPTION_INTEGER,
@@ -53,7 +54,7 @@ int cmd_fmt_merge_msg(int argc,
 	int ret;
 	struct fmt_merge_msg_opts opts;
 
-	git_config(fmt_merge_msg_config, NULL);
+	git_config(fmt_merge_msg_config, &merge_log_config);
 	argc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,
 			     0);
 	if (argc > 0)
diff --git a/builtin/merge.c b/builtin/merge.c
index 18b22c0a26..c2089b5e6f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1374,6 +1374,7 @@ int cmd_merge(int argc,
 	struct commit_list *remoteheads = NULL, *p;
 	void *branch_to_free;
 	int orig_argc = argc;
+	int merge_log_config = -1;
 
 	show_usage_with_options_if_asked(argc, argv,
 					 builtin_merge_usage, builtin_merge_options);
@@ -1392,7 +1393,7 @@ int cmd_merge(int argc,
 		skip_prefix(branch, "refs/heads/", &branch);
 
 	init_diff_ui_defaults();
-	git_config(git_merge_config, NULL);
+	git_config(git_merge_config, &merge_log_config);
 
 	if (!branch || is_null_oid(&head_oid))
 		head_commit = NULL;
diff --git a/environment.c b/environment.c
index 7c2480b22e..6751aa5683 100644
--- a/environment.c
+++ b/environment.c
@@ -66,7 +66,6 @@ int grafts_keep_true_parents;
 int core_apply_sparse_checkout;
 int core_sparse_checkout_cone;
 int sparse_expect_files_outside_of_patterns;
-int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
 unsigned long pack_size_limit_cfg;
 int max_allowed_tree_depth =
diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
index 40174efa3d..c9085edc40 100644
--- a/fmt-merge-msg.c
+++ b/fmt-merge-msg.c
@@ -26,13 +26,15 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;
 int fmt_merge_msg_config(const char *key, const char *value,
 			 const struct config_context *ctx, void *cb)
 {
+	int *merge_log_config = cb;
+
 	if (!strcmp(key, "merge.log") || !strcmp(key, "merge.summary")) {
 		int is_bool;
-		merge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);
-		if (!is_bool && merge_log_config < 0)
+		*merge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);
+		if (!is_bool && *merge_log_config < 0)
 			return error("%s: negative length %s", key, value);
-		if (is_bool && merge_log_config)
-			merge_log_config = DEFAULT_MERGE_LOG_LEN;
+		if (is_bool && *merge_log_config)
+			*merge_log_config = DEFAULT_MERGE_LOG_LEN;
 	} else if (!strcmp(key, "merge.branchdesc")) {
 		use_branch_desc = git_config_bool(key, value);
 	} else if (!strcmp(key, "merge.suppressdest")) {
diff --git a/fmt-merge-msg.h b/fmt-merge-msg.h
index 73ca3e4465..c066d83761 100644
--- a/fmt-merge-msg.h
+++ b/fmt-merge-msg.h
@@ -12,7 +12,6 @@ struct fmt_merge_msg_opts {
 	const char *into_name;
 };
 
-extern int merge_log_config;
 int fmt_merge_msg_config(const char *key, const char *value,
 			 const struct config_context *ctx, void *cb);
 int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
-- 
2.49.0
Phillip Wood· Aug 11, 2025, 14:42 UTC · re: Ayush Chandekar · lore

Re: [GSOC PATCH v2 1/2] environment: remove the global variable 'merge_log_config'

Hi Ayush
On 11/08/2025 00:45, Ayush Chandekar wrote:
Show 8 quoted lines
> The global variable 'merge_log_config', set via the "merge.log" or
> "merge.summary" settings, is only used in 'cmd_fmt_merge_msg()' and
> 'cmd_merge()' to adjust the 'shortlog_len' variable.
> 
> Remove 'merge_log_config' globally and localize it in
> 'cmd_fmt_merge_msg()' and 'cmd_merge()'. Set its value by passing it in
> 'fmt_merge_msg_config()' by passing its pointer to the function via the
> callback parameter.
This looks like a good solution
Thanks
Phillip
Show 104 quoted lines
> This change is part of an ongoing effort to eliminate global variables,
> improve modularity and help libify the codebase.
> 
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> ---
>   builtin/fmt-merge-msg.c |  3 ++-
>   builtin/merge.c         |  3 ++-
>   environment.c           |  1 -
>   fmt-merge-msg.c         | 10 ++++++----
>   fmt-merge-msg.h         |  1 -
>   5 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
> index 3b6aac2cf7..4b24de32fb 100644
> --- a/builtin/fmt-merge-msg.c
> +++ b/builtin/fmt-merge-msg.c
> @@ -19,6 +19,7 @@ int cmd_fmt_merge_msg(int argc,
>   	const char *message = NULL;
>   	char *into_name = NULL;
>   	int shortlog_len = -1;
> +	int merge_log_config = -1;
>   	struct option options[] = {
>   		{
>   			.type = OPTION_INTEGER,
> @@ -53,7 +54,7 @@ int cmd_fmt_merge_msg(int argc,
>   	int ret;
>   	struct fmt_merge_msg_opts opts;
>   
> -	git_config(fmt_merge_msg_config, NULL);
> +	git_config(fmt_merge_msg_config, &merge_log_config);
>   	argc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,
>   			     0);
>   	if (argc > 0)
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 18b22c0a26..c2089b5e6f 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1374,6 +1374,7 @@ int cmd_merge(int argc,
>   	struct commit_list *remoteheads = NULL, *p;
>   	void *branch_to_free;
>   	int orig_argc = argc;
> +	int merge_log_config = -1;
>   
>   	show_usage_with_options_if_asked(argc, argv,
>   					 builtin_merge_usage, builtin_merge_options);
> @@ -1392,7 +1393,7 @@ int cmd_merge(int argc,
>   		skip_prefix(branch, "refs/heads/", &branch);
>   
>   	init_diff_ui_defaults();
> -	git_config(git_merge_config, NULL);
> +	git_config(git_merge_config, &merge_log_config);
>   
>   	if (!branch || is_null_oid(&head_oid))
>   		head_commit = NULL;
> diff --git a/environment.c b/environment.c
> index 7c2480b22e..6751aa5683 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -66,7 +66,6 @@ int grafts_keep_true_parents;
>   int core_apply_sparse_checkout;
>   int core_sparse_checkout_cone;
>   int sparse_expect_files_outside_of_patterns;
> -int merge_log_config = -1;
>   int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
>   unsigned long pack_size_limit_cfg;
>   int max_allowed_tree_depth =
> diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c
> index 40174efa3d..c9085edc40 100644
> --- a/fmt-merge-msg.c
> +++ b/fmt-merge-msg.c
> @@ -26,13 +26,15 @@ static struct string_list suppress_dest_patterns = STRING_LIST_INIT_DUP;
>   int fmt_merge_msg_config(const char *key, const char *value,
>   			 const struct config_context *ctx, void *cb)
>   {
> +	int *merge_log_config = cb;
> +
>   	if (!strcmp(key, "merge.log") || !strcmp(key, "merge.summary")) {
>   		int is_bool;
> -		merge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);
> -		if (!is_bool && merge_log_config < 0)
> +		*merge_log_config = git_config_bool_or_int(key, value, ctx->kvi, &is_bool);
> +		if (!is_bool && *merge_log_config < 0)
>   			return error("%s: negative length %s", key, value);
> -		if (is_bool && merge_log_config)
> -			merge_log_config = DEFAULT_MERGE_LOG_LEN;
> +		if (is_bool && *merge_log_config)
> +			*merge_log_config = DEFAULT_MERGE_LOG_LEN;
>   	} else if (!strcmp(key, "merge.branchdesc")) {
>   		use_branch_desc = git_config_bool(key, value);
>   	} else if (!strcmp(key, "merge.suppressdest")) {
> diff --git a/fmt-merge-msg.h b/fmt-merge-msg.h
> index 73ca3e4465..c066d83761 100644
> --- a/fmt-merge-msg.h
> +++ b/fmt-merge-msg.h
> @@ -12,7 +12,6 @@ struct fmt_merge_msg_opts {
>   	const char *into_name;
>   };
>   
> -extern int merge_log_config;
>   int fmt_merge_msg_config(const char *key, const char *value,
>   			 const struct config_context *ctx, void *cb);
>   int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
Junio C Hamano· Aug 11, 2025, 16:13 UTC · re: Phillip Wood · lore

Re: [GSOC PATCH v2 1/2] environment: remove the global variable 'merge_log_config'

Phillip Wood <phillip.wood123@gmail.com> writes:
Show 12 quoted lines
> Hi Ayush
>
> On 11/08/2025 00:45, Ayush Chandekar wrote:
>> The global variable 'merge_log_config', set via the "merge.log" or
>> "merge.summary" settings, is only used in 'cmd_fmt_merge_msg()' and
>> 'cmd_merge()' to adjust the 'shortlog_len' variable.
>> Remove 'merge_log_config' globally and localize it in
>> 'cmd_fmt_merge_msg()' and 'cmd_merge()'. Set its value by passing it in
>> 'fmt_merge_msg_config()' by passing its pointer to the function via the
>> callback parameter.
>
> This looks like a good solution

When fmt_merge_msg_config() needs to read more stuff, the callback parameter may have to be updated, but this will do for now.

Thanks.
Ayush Chandekar· Aug 11, 2025, 18:25 UTC · re: Junio C Hamano · lore

Re: [GSOC PATCH v2 1/2] environment: remove the global variable 'merge_log_config'

On Mon, Aug 11, 2025 at 9:43 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 20 quoted lines
>
> Phillip Wood <phillip.wood123@gmail.com> writes:
>
> > Hi Ayush
> >
> > On 11/08/2025 00:45, Ayush Chandekar wrote:
> >> The global variable 'merge_log_config', set via the "merge.log" or
> >> "merge.summary" settings, is only used in 'cmd_fmt_merge_msg()' and
> >> 'cmd_merge()' to adjust the 'shortlog_len' variable.
> >> Remove 'merge_log_config' globally and localize it in
> >> 'cmd_fmt_merge_msg()' and 'cmd_merge()'. Set its value by passing it in
> >> 'fmt_merge_msg_config()' by passing its pointer to the function via the
> >> callback parameter.
> >
> > This looks like a good solution
>
> When fmt_merge_msg_config() needs to read more stuff, the callback
> parameter may have to be updated, but this will do for now.
>
> Thanks.
Yes, then we can create a struct and pass the struct instead, maybe.

Thanks, Ayush

Ayush Chandekar· Aug 10, 2025, 23:45 UTC · re: Ayush Chandekar · lore

[GSOC PATCH v2 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'

Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global 'the_repository'. Remove the 'UNUSED' macro from the 'struct repository' parameter and replace 'git_config()' with 'repo_config()' so that configuration is read from the passed repository. Also, add a test to make sure that "git fmt-merge-msg -h" can be called outside a repository.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
---
 builtin/fmt-merge-msg.c | 5 ++---
 t/t1517-outside-repo.sh | 7 +++++++
 2 files changed, 9 insertions(+), 3 deletions(-)
Show changes to 2 files +9 −3

builtin/fmt-merge-msg.c, t/t1517-outside-repo.sh

diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 4b24de32fb..cf4273a52c 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #include "builtin.h"
 #include "config.h"
 #include "fmt-merge-msg.h"
@@ -13,7 +12,7 @@ static const char * const fmt_merge_msg_usage[] = {
 int cmd_fmt_merge_msg(int argc,
 		      const char **argv,
 		      const char *prefix,
-		      struct repository *repo UNUSED)
+		      struct repository *repo)
 {
 	char *inpath = NULL;
 	const char *message = NULL;
@@ -54,7 +53,7 @@ int cmd_fmt_merge_msg(int argc,
 	int ret;
 	struct fmt_merge_msg_opts opts;
 
-	git_config(fmt_merge_msg_config, &merge_log_config);
+	repo_config(repo, fmt_merge_msg_config, &merge_log_config);
 	argc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,
 			     0);
 	if (argc > 0)
diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
index 8f59b867f2..4b4e645860 100755
--- a/t/t1517-outside-repo.sh
+++ b/t/t1517-outside-repo.sh
@@ -121,4 +121,11 @@ test_expect_success 'prune does not crash with -h' '
 	test_grep "[Uu]sage: git prune " usage
 '
 
+test_expect_success 'fmt-merge-msg does not crash with -h' '
+	test_expect_code 129 git fmt-merge-msg -h >usage &&
+	test_grep "[Uu]sage: git fmt-merge-msg " usage &&
+	test_expect_code 129 nongit git fmt-merge-msg -h >usage &&
+	test_grep "[Uu]sage: git fmt-merge-msg " usage
+'
+
 test_done
-- 
2.49.0

← back to recent threads