[PATCH 0/9] remove unnecessary if statement

[PATCH 0/9] remove unnecessary if statement

[Usman Akinyemi]

In an earlier patch[1] which has been merged to the master,
We checked `repo` is not NULL before making call to `repo_config()`.
Later, in another patch series[2] which has been merged to next,
`repo_config()` was taught to allow `repo` to be NULL.

So there is not need for checking if the `repo` is NULL before calling
repo_config() in the earlier patch.

Note, I have already sent the first 8 patches in [2] but, the 9th
patch depends on the first patch of that series. 

[1] https://public-inbox.org/git/20250210181103.3609495-1-usmanakinyemi202@gmail.com/
[2] https://public-inbox.org/git/20250307233543.1721552-1-usmanakinyemi202@gmail.com/

Usman Akinyemi (9):
  config: teach repo_config to allow `repo` to be NULL
  builtin/verify-tag: stop using `the_repository`
  builtin/verify-commit: stop using `the_repository`
  builtin/send-pack: stop using `the_repository`
  builtin/pack-refs: stop using `the_repository`
  builtin/ls-files: stop using `the_repository`
  builtin/for-each-ref: stop using `the_repository`
  builtin/checkout-index: stop using `the_repository`
  builtin/update-server-info: remove unnecessary if statement

 builtin/checkout-index.c        | 43 ++++++++++++++++-----------------
 builtin/for-each-ref.c          |  5 ++--
 builtin/ls-files.c              | 32 ++++++++++++------------
 builtin/pack-refs.c             |  8 +++---
 builtin/send-pack.c             |  7 +++---
 builtin/update-server-info.c    |  4 +--
 builtin/verify-commit.c         | 13 +++++-----
 builtin/verify-tag.c            |  7 +++---
 config.c                        |  4 +++
 config.h                        |  9 +++++++
 t/t0610-reftable-basics.sh      |  7 ++++++
 t/t2006-checkout-index-basic.sh |  7 ++++++
 t/t3004-ls-files-basic.sh       |  7 ++++++
 t/t5400-send-pack.sh            |  7 ++++++
 t/t6300-for-each-ref.sh         |  7 ++++++
 t/t7030-verify-tag.sh           |  7 ++++++
 t/t7510-signed-commit.sh        |  7 ++++++
 17 files changed, 118 insertions(+), 63 deletions(-)

-- 
2.48.1

[PATCH 1/9] config: teach repo_config to allow `repo` to be NULL

[Usman Akinyemi]

The `repo` value can be NULL if a builtin command is run outside
any repository. The current implementation of `repo_config()` will
fail if `repo` is NULL.

If the `repo` is NULL the `repo_config()` can ignore the repository
configuration but it should read the other configuration sources like
the system-side configuration instead of failing.

Teach the `repo_config()` to allow `repo` to be NULL by calling the
`read_very_early_config()` which read config but only enumerate system
and global settings.

This will be useful in the following commits.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
---
 config.c | 4 ++++
 config.h | 9 +++++++++
 2 files changed, 13 insertions(+)

diff --git a/config.c b/config.c
index 658569af08..e127afaa8f 100644
--- a/config.c
+++ b/config.c
@@ -2521,6 +2521,10 @@ void repo_config_clear(struct repository *repo)
 
 void repo_config(struct repository *repo, config_fn_t fn, void *data)
 {
+	if (!repo) {
+		read_very_early_config(fn, data);
+		return;
+	}
 	git_config_check_init(repo);
 	configset_iter(repo->config, fn, data);
 }
diff --git a/config.h b/config.h
index 5c730c4f89..29a0277483 100644
--- a/config.h
+++ b/config.h
@@ -219,6 +219,15 @@ void read_very_early_config(config_fn_t cb, void *data);
  * repo-specific one; by overwriting, the higher-priority repo-specific
  * value is left at the end).
  *
+ * In cases where the repository variable is NULL, repo_config() will
+ * skip the per-repository config but retain system and global configs
+ * by calling read_very_early_config() which also ignores one-time
+ * overrides like "git -c var=val". This is to support handling "git foo -h"
+ * (which lets git.c:run_builtin() to pass NULL and have the cmd_foo()
+ * call repo_config() before calling parse_options() to notice "-h", give
+ * help and exit) for a command that ordinarily require a repository
+ * so this limitation may be OK (but if needed you are welcome to fix it).
+ *
  * Unlike git_config_from_file(), this function respects includes.
  */
 void repo_config(struct repository *r, config_fn_t fn, void *);
-- 
2.48.1

Re: [PATCH 1/9] config: teach repo_config to allow `repo` to be NULL

[Patrick Steinhardt]

On Tue, Mar 18, 2025 at 05:28:53PM +0530, Usman Akinyemi wrote:
> diff --git a/config.c b/config.c
> index 658569af08..e127afaa8f 100644
> --- a/config.c
> +++ b/config.c
> @@ -2521,6 +2521,10 @@ void repo_config_clear(struct repository *repo)
>  
>  void repo_config(struct repository *repo, config_fn_t fn, void *data)
>  {
> +	if (!repo) {
> +		read_very_early_config(fn, data);
> +		return;
> +	}

I remember discussion that `read_very_early_config()` may not be a good
fit here. Most importantly, it ignores any configuration passed on the
command line, which I would think is very surprising behaviour. So
should we adapt this to instead manually call `config_with_options()`
with the expected bits set?

Patrick

Re: [PATCH 1/9] config: teach repo_config to allow `repo` to be NULL

[Usman Akinyemi]

On Thu, Mar 20, 2025 at 12:51 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Tue, Mar 18, 2025 at 05:28:53PM +0530, Usman Akinyemi wrote:
> > diff --git a/config.c b/config.c
> > index 658569af08..e127afaa8f 100644
> > --- a/config.c
> > +++ b/config.c
> > @@ -2521,6 +2521,10 @@ void repo_config_clear(struct repository *repo)
> >
> >  void repo_config(struct repository *repo, config_fn_t fn, void *data)
> >  {
> > +     if (!repo) {
> > +             read_very_early_config(fn, data);
> > +             return;
> > +     }
>
> I remember discussion that `read_very_early_config()` may not be a good
> fit here. Most importantly, it ignores any configuration passed on the
> command line, which I would think is very surprising behaviour. So
> should we adapt this to instead manually call `config_with_options()`
> with the expected bits set?
Hi Patrick,

Sorry for the confusion.

There was a discussion about this here.
https://public-inbox.org/git/xmqqbjum2ayc.fsf@gitster.g/
https://public-inbox.org/git/xmqqcyeuhwqb.fsf@gitster.g/

Mainliy ignoring the configuration passed from cmdline for this purpose.

I was sending this patch again mainly for the 9/9 patch.

Thanks.
>
> Patrick

[PATCH 2/9] builtin/verify-tag: stop using `the_repository`

[Usman Akinyemi]

Remove the_repository global variable in favor of the repository
argument that gets passed in "builtin/verify-tag.c".

When `-h` is passed to the command outside a Git repository, the
`run_builtin()` will call the `cmd_verify_tag()` function with `repo` set
to NULL and then early in the function, `parse_options()` call will give
the options help and exit.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
---
 builtin/verify-tag.c  | 7 +++----
 t/t7030-verify-tag.sh | 7 +++++++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index f6b97048a5..ed1c40338f 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -5,7 +5,6 @@
  *
  * Based on git-verify-tag.sh
  */
-#define USE_THE_REPOSITORY_VARIABLE
 #include "builtin.h"
 #include "config.h"
 #include "gettext.h"
@@ -23,7 +22,7 @@ static const char * const verify_tag_usage[] = {
 int cmd_verify_tag(int argc,
 		   const char **argv,
 		   const char *prefix,
-		   struct repository *repo UNUSED)
+		   struct repository *repo)
 {
 	int i = 1, verbose = 0, had_error = 0;
 	unsigned flags = 0;
@@ -35,7 +34,7 @@ int cmd_verify_tag(int argc,
 		OPT_END()
 	};
 
-	git_config(git_default_config, NULL);
+	repo_config(repo, git_default_config, NULL);
 
 	argc = parse_options(argc, argv, prefix, verify_tag_options,
 			     verify_tag_usage, PARSE_OPT_KEEP_ARGV0);
@@ -56,7 +55,7 @@ int cmd_verify_tag(int argc,
 		struct object_id oid;
 		const char *name = argv[i++];
 
-		if (repo_get_oid(the_repository, name, &oid)) {
+		if (repo_get_oid(repo, name, &oid)) {
 			had_error = !!error("tag '%s' not found.", name);
 			continue;
 		}
diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
index 6f526c37c2..2c147072c1 100755
--- a/t/t7030-verify-tag.sh
+++ b/t/t7030-verify-tag.sh
@@ -7,6 +7,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY/lib-gpg.sh"
 
+test_expect_success GPG 'verify-tag does not crash with -h' '
+	test_expect_code 129 git verify-tag -h >usage &&
+	test_grep "[Uu]sage: git verify-tag " usage &&
+	test_expect_code 129 nongit git verify-tag -h >usage &&
+	test_grep "[Uu]sage: git verify-tag " usage
+'
+
 test_expect_success GPG 'create signed tags' '
 	echo 1 >file && git add file &&
 	test_tick && git commit -m initial &&
-- 
2.48.1

Re: [PATCH 2/9] builtin/verify-tag: stop using `the_repository`

[Patrick Steinhardt]

On Tue, Mar 18, 2025 at 05:28:54PM +0530, Usman Akinyemi wrote:
> diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh
> index 6f526c37c2..2c147072c1 100755
> --- a/t/t7030-verify-tag.sh
> +++ b/t/t7030-verify-tag.sh
> @@ -7,6 +7,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>  . ./test-lib.sh
>  . "$TEST_DIRECTORY/lib-gpg.sh"
>  
> +test_expect_success GPG 'verify-tag does not crash with -h' '
> +	test_expect_code 129 git verify-tag -h >usage &&
> +	test_grep "[Uu]sage: git verify-tag " usage &&
> +	test_expect_code 129 nongit git verify-tag -h >usage &&
> +	test_grep "[Uu]sage: git verify-tag " usage
> +'
> +
>  test_expect_success GPG 'create signed tags' '
>  	echo 1 >file && git add file &&
>  	test_tick && git commit -m initial &&

We have "t1517-outside-repo.sh". Maybe it would preferable to add tests
like these to that test suite instead?

Patrick

[PATCH 3/9] builtin/verify-commit: stop using `the_repository`

[Usman Akinyemi]

Remove the_repository global variable in favor of the repository
argument that gets passed in "builtin/verify-commit.c".

When `-h` is passed to the command outside a Git repository, the
`run_builtin()` will call the `cmd_verify_commit()` function with `repo`
set to NULL and then early in the function, `parse_options()` call will
give the options help and exit.

Pass the repository available in the calling context to `verify_commit()`
to remove it's dependency on the global `the_repository` variable.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
---
 builtin/verify-commit.c  | 13 ++++++-------
 t/t7510-signed-commit.sh |  7 +++++++
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/builtin/verify-commit.c b/builtin/verify-commit.c
index 779b7988ca..5f749a30da 100644
--- a/builtin/verify-commit.c
+++ b/builtin/verify-commit.c
@@ -5,7 +5,6 @@
  *
  * Based on git-verify-tag
  */
-#define USE_THE_REPOSITORY_VARIABLE
 #include "builtin.h"
 #include "config.h"
 #include "gettext.h"
@@ -33,15 +32,15 @@ static int run_gpg_verify(struct commit *commit, unsigned flags)
 	return ret;
 }
 
-static int verify_commit(const char *name, unsigned flags)
+static int verify_commit(struct repository *repo, const char *name, unsigned flags)
 {
 	struct object_id oid;
 	struct object *obj;
 
-	if (repo_get_oid(the_repository, name, &oid))
+	if (repo_get_oid(repo, name, &oid))
 		return error("commit '%s' not found.", name);
 
-	obj = parse_object(the_repository, &oid);
+	obj = parse_object(repo, &oid);
 	if (!obj)
 		return error("%s: unable to read file.", name);
 	if (obj->type != OBJ_COMMIT)
@@ -54,7 +53,7 @@ static int verify_commit(const char *name, unsigned flags)
 int cmd_verify_commit(int argc,
 		      const char **argv,
 		      const char *prefix,
-		      struct repository *repo UNUSED)
+		      struct repository *repo)
 {
 	int i = 1, verbose = 0, had_error = 0;
 	unsigned flags = 0;
@@ -64,7 +63,7 @@ int cmd_verify_commit(int argc,
 		OPT_END()
 	};
 
-	git_config(git_default_config, NULL);
+	repo_config(repo, git_default_config, NULL);
 
 	argc = parse_options(argc, argv, prefix, verify_commit_options,
 			     verify_commit_usage, PARSE_OPT_KEEP_ARGV0);
@@ -78,7 +77,7 @@ int cmd_verify_commit(int argc,
 	 * was received in the process of writing the gpg input: */
 	signal(SIGPIPE, SIG_IGN);
 	while (i < argc)
-		if (verify_commit(argv[i++], flags))
+		if (verify_commit(repo, argv[i++], flags))
 			had_error = 1;
 	return had_error;
 }
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 0d2dd29fe6..39677e859a 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -8,6 +8,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 GNUPGHOME_NOT_USED=$GNUPGHOME
 . "$TEST_DIRECTORY/lib-gpg.sh"
 
+test_expect_success GPG 'verify-commit does not crash with -h' '
+	test_expect_code 129 git verify-commit -h >usage &&
+	test_grep "[Uu]sage: git verify-commit " usage &&
+	test_expect_code 129 nongit git verify-commit -h >usage &&
+	test_grep "[Uu]sage: git verify-commit " usage
+'
+
 test_expect_success GPG 'create signed commits' '
 	test_oid_cache <<-\EOF &&
 	header sha1:gpgsig
-- 
2.48.1

[PATCH 4/9] builtin/send-pack: stop using `the_repository`

[Usman Akinyemi]

Remove the_repository global variable in favor of the repository
argument that gets passed in "builtin/send-pack.c".

When `-h` is passed to the command outside a Git repository, the
`run_builtin()` will call the `cmd_send_pack()` function with `repo` set
to NULL and then early in the function, `parse_options()` call will give
the options help and exit.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
---
 builtin/send-pack.c  | 7 +++----
 t/t5400-send-pack.sh | 7 +++++++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 8d461008e2..c6e0e9d051 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #include "builtin.h"
 #include "config.h"
 #include "hex.h"
@@ -151,7 +150,7 @@ static int send_pack_config(const char *k, const char *v,
 int cmd_send_pack(int argc,
 		  const char **argv,
 		  const char *prefix,
-		  struct repository *repo UNUSED)
+		  struct repository *repo)
 {
 	struct refspec rs = REFSPEC_INIT_PUSH;
 	const char *remote_name = NULL;
@@ -212,7 +211,7 @@ int cmd_send_pack(int argc,
 		OPT_END()
 	};
 
-	git_config(send_pack_config, NULL);
+	repo_config(repo, send_pack_config, NULL);
 	argc = parse_options(argc, argv, prefix, options, send_pack_usage, 0);
 	if (argc > 0) {
 		dest = argv[0];
@@ -317,7 +316,7 @@ int cmd_send_pack(int argc,
 	set_ref_status_for_push(remote_refs, args.send_mirror,
 		args.force_update);
 
-	ret = send_pack(the_repository, &args, fd, conn, remote_refs, &extra_have);
+	ret = send_pack(repo, &args, fd, conn, remote_refs, &extra_have);
 
 	if (helper_status)
 		print_helper_status(remote_refs);
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 3f81f16e13..8f018d2f23 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -55,6 +55,13 @@ test_expect_success setup '
 	echo Rebase &&
 	git log'
 
+test_expect_success 'send-pack does not crash with -h' '
+	test_expect_code 129 git send-pack -h >usage &&
+	test_grep "[Uu]sage: git send-pack " usage &&
+	test_expect_code 129 nongit git send-pack -h >usage &&
+	test_grep "[Uu]sage: git send-pack " usage
+'
+
 test_expect_success 'pack the source repository' '
 	git repack -a -d &&
 	git prune
-- 
2.48.1

[PATCH 5/9] builtin/pack-refs: stop using `the_repository`

[Usman Akinyemi]

Remove the_repository global variable in favor of the repository
argument that gets passed in "builtin/pack-refs.c".

When `-h` is passed to the command outside a Git repository, the
`run_builtin()` will call the `cmd_pack_refs()` function with `repo` set
to NULL and then early in the function, `parse_options()` call will give
the options help and exit.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
---
 builtin/pack-refs.c        | 8 +++-----
 t/t0610-reftable-basics.sh | 7 +++++++
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index 4fdd68880e..e47bae1c80 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -1,5 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
-
 #include "builtin.h"
 #include "config.h"
 #include "gettext.h"
@@ -15,7 +13,7 @@ static char const * const pack_refs_usage[] = {
 int cmd_pack_refs(int argc,
 		  const char **argv,
 		  const char *prefix,
-		  struct repository *repo UNUSED)
+		  struct repository *repo)
 {
 	struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
 	struct string_list included_refs = STRING_LIST_INIT_NODUP;
@@ -39,7 +37,7 @@ int cmd_pack_refs(int argc,
 			N_("references to exclude")),
 		OPT_END(),
 	};
-	git_config(git_default_config, NULL);
+	repo_config(repo, git_default_config, NULL);
 	if (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))
 		usage_with_options(pack_refs_usage, opts);
 
@@ -52,7 +50,7 @@ int cmd_pack_refs(int argc,
 	if (!pack_refs_opts.includes->nr)
 		string_list_append(pack_refs_opts.includes, "refs/tags/*");
 
-	ret = refs_pack_refs(get_main_ref_store(the_repository), &pack_refs_opts);
+	ret = refs_pack_refs(get_main_ref_store(repo), &pack_refs_opts);
 
 	clear_ref_exclusions(&excludes);
 	string_list_clear(&included_refs, 0);
diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index 4618ffc108..002a75dee8 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -14,6 +14,13 @@ export GIT_TEST_DEFAULT_REF_FORMAT
 
 INVALID_OID=$(test_oid 001)
 
+test_expect_success 'pack-refs does not crash with -h' '
+	test_expect_code 129 git pack-refs -h >usage &&
+	test_grep "[Uu]sage: git pack-refs " usage &&
+	test_expect_code 129 nongit git pack-refs -h >usage &&
+	test_grep "[Uu]sage: git pack-refs " usage
+'
+
 test_expect_success 'init: creates basic reftable structures' '
 	test_when_finished "rm -rf repo" &&
 	git init repo &&
-- 
2.48.1

[PATCH 6/9] builtin/ls-files: stop using `the_repository`

[Usman Akinyemi]

Remove the_repository global variable in favor of the repository
argument that gets passed in "builtin/ls-files.c".

When `-h` is passed to the command outside a Git repository, the
`run_builtin()` will call the `cmd_ls_files()` function with `repo` set
to NULL and then early in the function, `show_usage_with_options_if_asked()`
call will give the options help and exit.

Pass the repository available in the calling context to both
`expand_objectsize()` and `show_ru_info()` to remove their
dependency on the global `the_repository` variable.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
---
 builtin/ls-files.c        | 32 ++++++++++++++++----------------
 t/t3004-ls-files-basic.sh |  7 +++++++
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index a4431429b7..70a377e9c0 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -6,7 +6,6 @@
  * Copyright (C) Linus Torvalds, 2005
  */
 
-#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "builtin.h"
@@ -245,12 +244,13 @@ static void show_submodule(struct repository *superproject,
 	repo_clear(&subrepo);
 }
 
-static void expand_objectsize(struct strbuf *line, const struct object_id *oid,
+static void expand_objectsize(struct repository *repo, struct strbuf *line,
+			      const struct object_id *oid,
 			      const enum object_type type, unsigned int padded)
 {
 	if (type == OBJ_BLOB) {
 		unsigned long size;
-		if (oid_object_info(the_repository, oid, &size) < 0)
+		if (oid_object_info(repo, oid, &size) < 0)
 			die(_("could not get object info about '%s'"),
 			    oid_to_hex(oid));
 		if (padded)
@@ -283,10 +283,10 @@ static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,
 		else if (skip_prefix(format, "(objecttype)", &format))
 			strbuf_addstr(&sb, type_name(object_type(ce->ce_mode)));
 		else if (skip_prefix(format, "(objectsize:padded)", &format))
-			expand_objectsize(&sb, &ce->oid,
+			expand_objectsize(repo, &sb, &ce->oid,
 					  object_type(ce->ce_mode), 1);
 		else if (skip_prefix(format, "(objectsize)", &format))
-			expand_objectsize(&sb, &ce->oid,
+			expand_objectsize(repo, &sb, &ce->oid,
 					  object_type(ce->ce_mode), 0);
 		else if (skip_prefix(format, "(stage)", &format))
 			strbuf_addf(&sb, "%d", ce_stage(ce));
@@ -348,7 +348,7 @@ static void show_ce(struct repository *repo, struct dir_struct *dir,
 	}
 }
 
-static void show_ru_info(struct index_state *istate)
+static void show_ru_info(struct repository *repo, struct index_state *istate)
 {
 	struct string_list_item *item;
 
@@ -370,7 +370,7 @@ static void show_ru_info(struct index_state *istate)
 			if (!ui->mode[i])
 				continue;
 			printf("%s%06o %s %d\t", tag_resolve_undo, ui->mode[i],
-			       repo_find_unique_abbrev(the_repository, &ui->oid[i], abbrev),
+			       repo_find_unique_abbrev(repo, &ui->oid[i], abbrev),
 			       i + 1);
 			write_name(path);
 		}
@@ -567,7 +567,7 @@ static int option_parse_exclude_standard(const struct option *opt,
 int cmd_ls_files(int argc,
 		 const char **argv,
 		 const char *cmd_prefix,
-		 struct repository *repo UNUSED)
+		 struct repository *repo)
 {
 	int require_work_tree = 0, show_tag = 0, i;
 	char *max_prefix;
@@ -647,15 +647,15 @@ int cmd_ls_files(int argc,
 	show_usage_with_options_if_asked(argc, argv,
 					 ls_files_usage, builtin_ls_files_options);
 
-	prepare_repo_settings(the_repository);
-	the_repository->settings.command_requires_full_index = 0;
+	prepare_repo_settings(repo);
+	repo->settings.command_requires_full_index = 0;
 
 	prefix = cmd_prefix;
 	if (prefix)
 		prefix_len = strlen(prefix);
-	git_config(git_default_config, NULL);
+	repo_config(repo, git_default_config, NULL);
 
-	if (repo_read_index(the_repository) < 0)
+	if (repo_read_index(repo) < 0)
 		die("index file corrupt");
 
 	argc = parse_options(argc, argv, prefix, builtin_ls_files_options,
@@ -724,7 +724,7 @@ int cmd_ls_files(int argc,
 		max_prefix = common_prefix(&pathspec);
 	max_prefix_len = get_common_prefix_len(max_prefix);
 
-	prune_index(the_repository->index, max_prefix, max_prefix_len);
+	prune_index(repo->index, max_prefix, max_prefix_len);
 
 	/* Treat unmatching pathspec elements as errors */
 	if (pathspec.nr && error_unmatch)
@@ -748,13 +748,13 @@ int cmd_ls_files(int argc,
 		 */
 		if (show_stage || show_unmerged)
 			die(_("options '%s' and '%s' cannot be used together"), "ls-files --with-tree", "-s/-u");
-		overlay_tree_on_index(the_repository->index, with_tree, max_prefix);
+		overlay_tree_on_index(repo->index, with_tree, max_prefix);
 	}
 
-	show_files(the_repository, &dir);
+	show_files(repo, &dir);
 
 	if (show_resolve_undo)
-		show_ru_info(the_repository->index);
+		show_ru_info(repo, repo->index);
 
 	if (ps_matched && report_path_error(ps_matched, &pathspec)) {
 		fprintf(stderr, "Did you forget to 'git add'?\n");
diff --git a/t/t3004-ls-files-basic.sh b/t/t3004-ls-files-basic.sh
index a1078f8701..4034a5a59f 100755
--- a/t/t3004-ls-files-basic.sh
+++ b/t/t3004-ls-files-basic.sh
@@ -34,6 +34,13 @@ test_expect_success 'ls-files -h in corrupt repository' '
 	test_grep "[Uu]sage: git ls-files " broken/usage
 '
 
+test_expect_success 'ls-files does not crash with -h' '
+	test_expect_code 129 git ls-files -h >usage &&
+	test_grep "[Uu]sage: git ls-files " usage &&
+	test_expect_code 129 nongit git ls-files -h >usage &&
+	test_grep "[Uu]sage: git ls-files " usage
+'
+
 test_expect_success SYMLINKS 'ls-files with absolute paths to symlinks' '
 	mkdir subs &&
 	ln -s nosuch link &&
-- 
2.48.1

[PATCH 7/9] builtin/for-each-ref: stop using `the_repository`

[Usman Akinyemi]

Remove the_repository global variable in favor of the repository
argument that gets passed in "builtin/for-each-ref.c".

When `-h` is passed to the command outside a Git repository, the
`run_builtin()` will call the `cmd_for_each_ref()` function with `repo`
set to NULL and then early in the function, `parse_options()` call will
give the options help and exit.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
---
 builtin/for-each-ref.c  | 5 ++---
 t/t6300-for-each-ref.sh | 7 +++++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 8085ebd8fe..3d2207ec77 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #include "builtin.h"
 #include "commit.h"
 #include "config.h"
@@ -20,7 +19,7 @@ static char const * const for_each_ref_usage[] = {
 int cmd_for_each_ref(int argc,
 		     const char **argv,
 		     const char *prefix,
-		     struct repository *repo UNUSED)
+		     struct repository *repo)
 {
 	struct ref_sorting *sorting;
 	struct string_list sorting_options = STRING_LIST_INIT_DUP;
@@ -63,7 +62,7 @@ int cmd_for_each_ref(int argc,
 
 	format.format = "%(objectname) %(objecttype)\t%(refname)";
 
-	git_config(git_default_config, NULL);
+	repo_config(repo, git_default_config, NULL);
 
 	/* Set default (refname) sorting */
 	string_list_append(&sorting_options, "refname");
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index a5c7794385..9b4f4306c4 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -292,6 +292,13 @@ test_expect_success 'Check invalid atoms names are errors' '
 	test_must_fail git for-each-ref --format="%(INVALID)" refs/heads
 '
 
+test_expect_success 'for-each-ref does not crash with -h' '
+	test_expect_code 129 git for-each-ref -h >usage &&
+	test_grep "[Uu]sage: git for-each-ref " usage &&
+	test_expect_code 129 nongit git for-each-ref -h >usage &&
+	test_grep "[Uu]sage: git for-each-ref " usage
+'
+
 test_expect_success 'Check format specifiers are ignored in naming date atoms' '
 	git for-each-ref --format="%(authordate)" refs/heads &&
 	git for-each-ref --format="%(authordate:default) %(authordate)" refs/heads &&
-- 
2.48.1

[PATCH 8/9] builtin/checkout-index: stop using `the_repository`

[Usman Akinyemi]

Remove the_repository global variable in favor of the repository
argument that gets passed in "builtin/checkout-index.c".

When `-h` is passed to the command outside a Git repository, the
`run_builtin()` will call the `cmd_checkout_index()` function with `repo`
set to NULL and then early in the function, `show_usage_with_options_if_asked()`
call will give the options help and exit.

Pass an instance of "struct index_state" available in the calling
context to both `checkout_all()` and `checkout_file()` to remove their
dependency on the global `the_repository` variable.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
---
 builtin/checkout-index.c        | 43 ++++++++++++++++-----------------
 t/t2006-checkout-index-basic.sh |  7 ++++++
 2 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index e30086c7d4..7f74bc702f 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -5,7 +5,6 @@
  *
  */
 
-#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "builtin.h"
@@ -68,10 +67,10 @@ static void write_tempfile_record(const char *name, const char *prefix)
 	}
 }
 
-static int checkout_file(const char *name, const char *prefix)
+static int checkout_file(struct index_state *index, const char *name, const char *prefix)
 {
 	int namelen = strlen(name);
-	int pos = index_name_pos(the_repository->index, name, namelen);
+	int pos = index_name_pos(index, name, namelen);
 	int has_same_name = 0;
 	int is_file = 0;
 	int is_skipped = 1;
@@ -81,8 +80,8 @@ static int checkout_file(const char *name, const char *prefix)
 	if (pos < 0)
 		pos = -pos - 1;
 
-	while (pos <the_repository->index->cache_nr) {
-		struct cache_entry *ce =the_repository->index->cache[pos];
+	while (pos < index->cache_nr) {
+		struct cache_entry *ce = index->cache[pos];
 		if (ce_namelen(ce) != namelen ||
 		    memcmp(ce->name, name, namelen))
 			break;
@@ -137,13 +136,13 @@ static int checkout_file(const char *name, const char *prefix)
 	return -1;
 }
 
-static int checkout_all(const char *prefix, int prefix_length)
+static int checkout_all(struct index_state *index, const char *prefix, int prefix_length)
 {
 	int i, errs = 0;
 	struct cache_entry *last_ce = NULL;
 
-	for (i = 0; i < the_repository->index->cache_nr ; i++) {
-		struct cache_entry *ce = the_repository->index->cache[i];
+	for (i = 0; i < index->cache_nr ; i++) {
+		struct cache_entry *ce = index->cache[i];
 
 		if (S_ISSPARSEDIR(ce->ce_mode)) {
 			if (!ce_skip_worktree(ce))
@@ -156,8 +155,8 @@ static int checkout_all(const char *prefix, int prefix_length)
 			 * first entry inside the expanded sparse directory).
 			 */
 			if (ignore_skip_worktree) {
-				ensure_full_index(the_repository->index);
-				ce = the_repository->index->cache[i];
+				ensure_full_index(index);
+				ce = index->cache[i];
 			}
 		}
 
@@ -213,7 +212,7 @@ static int option_parse_stage(const struct option *opt,
 int cmd_checkout_index(int argc,
 		       const char **argv,
 		       const char *prefix,
-		       struct repository *repo UNUSED)
+		       struct repository *repo)
 {
 	int i;
 	struct lock_file lock_file = LOCK_INIT;
@@ -253,19 +252,19 @@ int cmd_checkout_index(int argc,
 	show_usage_with_options_if_asked(argc, argv,
 					 builtin_checkout_index_usage,
 					 builtin_checkout_index_options);
-	git_config(git_default_config, NULL);
+	repo_config(repo, git_default_config, NULL);
 	prefix_length = prefix ? strlen(prefix) : 0;
 
-	prepare_repo_settings(the_repository);
-	the_repository->settings.command_requires_full_index = 0;
+	prepare_repo_settings(repo);
+	repo->settings.command_requires_full_index = 0;
 
-	if (repo_read_index(the_repository) < 0) {
+	if (repo_read_index(repo) < 0) {
 		die("invalid cache");
 	}
 
 	argc = parse_options(argc, argv, prefix, builtin_checkout_index_options,
 			builtin_checkout_index_usage, 0);
-	state.istate = the_repository->index;
+	state.istate = repo->index;
 	state.force = force;
 	state.quiet = quiet;
 	state.not_new = not_new;
@@ -285,8 +284,8 @@ int cmd_checkout_index(int argc,
 	 */
 	if (index_opt && !state.base_dir_len && !to_tempfile) {
 		state.refresh_cache = 1;
-		state.istate = the_repository->index;
-		repo_hold_locked_index(the_repository, &lock_file,
+		state.istate = repo->index;
+		repo_hold_locked_index(repo, &lock_file,
 				       LOCK_DIE_ON_ERROR);
 	}
 
@@ -304,7 +303,7 @@ int cmd_checkout_index(int argc,
 		if (read_from_stdin)
 			die("git checkout-index: don't mix '--stdin' and explicit filenames");
 		p = prefix_path(prefix, prefix_length, arg);
-		err |= checkout_file(p, prefix);
+		err |= checkout_file(repo->index, p, prefix);
 		free(p);
 	}
 
@@ -326,7 +325,7 @@ int cmd_checkout_index(int argc,
 				strbuf_swap(&buf, &unquoted);
 			}
 			p = prefix_path(prefix, prefix_length, buf.buf);
-			err |= checkout_file(p, prefix);
+			err |= checkout_file(repo->index, p, prefix);
 			free(p);
 		}
 		strbuf_release(&unquoted);
@@ -334,7 +333,7 @@ int cmd_checkout_index(int argc,
 	}
 
 	if (all)
-		err |= checkout_all(prefix, prefix_length);
+		err |= checkout_all(repo->index, prefix, prefix_length);
 
 	if (pc_workers > 1)
 		err |= run_parallel_checkout(&state, pc_workers, pc_threshold,
@@ -344,7 +343,7 @@ int cmd_checkout_index(int argc,
 		return 1;
 
 	if (is_lock_file_locked(&lock_file) &&
-	    write_locked_index(the_repository->index, &lock_file, COMMIT_LOCK))
+	    write_locked_index(repo->index, &lock_file, COMMIT_LOCK))
 		die("Unable to write new index file");
 	return 0;
 }
diff --git a/t/t2006-checkout-index-basic.sh b/t/t2006-checkout-index-basic.sh
index bac231b167..fedd2cc097 100755
--- a/t/t2006-checkout-index-basic.sh
+++ b/t/t2006-checkout-index-basic.sh
@@ -21,6 +21,13 @@ test_expect_success 'checkout-index -h in broken repository' '
 	test_grep "[Uu]sage" broken/usage
 '
 
+test_expect_success 'checkout-index does not crash with -h' '
+	test_expect_code 129 git checkout-index -h >usage &&
+	test_grep "[Uu]sage: git checkout-index " usage &&
+	test_expect_code 129 nongit git checkout-index -h >usage &&
+	test_grep "[Uu]sage: git checkout-index " usage
+'
+
 test_expect_success 'checkout-index reports errors (cmdline)' '
 	test_must_fail git checkout-index -- does-not-exist 2>stderr &&
 	test_grep not.in.the.cache stderr
-- 
2.48.1

[PATCH 9/9] builtin/update-server-info: remove unnecessary if statement

[Usman Akinyemi]

Since we already teach the `repo_config()` to allow `repo`
to be NULL, no need to check if `repo` is NULL before calling
`repo_config()`.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
---
 builtin/update-server-info.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/update-server-info.c b/builtin/update-server-info.c
index d7467290a8..ba702d30ef 100644
--- a/builtin/update-server-info.c
+++ b/builtin/update-server-info.c
@@ -20,8 +20,8 @@ int cmd_update_server_info(int argc,
 		OPT_END()
 	};
 
-	if (repo)
-		repo_config(repo, git_default_config, NULL);
+	repo_config(repo, git_default_config, NULL);
+
 	argc = parse_options(argc, argv, prefix, options,
 			     update_server_info_usage, 0);
 	if (argc > 0)
-- 
2.48.1

Re: [PATCH 9/9] builtin/update-server-info: remove unnecessary if statement

[Patrick Steinhardt]

On Tue, Mar 18, 2025 at 05:29:01PM +0530, Usman Akinyemi wrote:
> Since we already teach the `repo_config()` to allow `repo`
> to be NULL, no need to check if `repo` is NULL before calling
> `repo_config()`.

I think it would be preferable to reorder this patch so that it comes
immediately after the one where you adapt `repo_config()`.

We also have a couple of additional sites where we call the function
conditionally:

  - builtin/add.c
  - builtin/difftool.c
  - builtin/update-server-info.c

It would probably make sense to also adapt those.

Patrick

Re: [PATCH 9/9] builtin/update-server-info: remove unnecessary if statement

[Usman Akinyemi]

On Thu, Mar 20, 2025 at 12:51 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Tue, Mar 18, 2025 at 05:29:01PM +0530, Usman Akinyemi wrote:
> > Since we already teach the `repo_config()` to allow `repo`
> > to be NULL, no need to check if `repo` is NULL before calling
> > `repo_config()`.
>
> I think it would be preferable to reorder this patch so that it comes
> immediately after the one where you adapt `repo_config()`.
>
> We also have a couple of additional sites where we call the function
> conditionally:
>
>   - builtin/add.c
>   - builtin/difftool.c
>   - builtin/update-server-info.c
>
> It would probably make sense to also adapt those.

Yeah, it has caused a little confusion. Maybe, let's just ignore this
patch series.

The other series which introduces the first 8 patches of this has been
marked to be merged to master. I will resend the 9th patch(
builtin/update-server-info.c) then other functions like difftool.

Thank you.
>
> Patrick

Re: [PATCH 0/9] remove unnecessary if statement

[Junio C Hamano]

Usman Akinyemi <usmanakinyemi202@gmail.com> writes:

> In an earlier patch[1] which has been merged to the master,
> We checked `repo` is not NULL before making call to `repo_config()`.
> Later, in another patch series[2] which has been merged to next,
> `repo_config()` was taught to allow `repo` to be NULL.
>
> So there is not need for checking if the `repo` is NULL before calling
> repo_config() in the earlier patch.

OK, that sounds good.

Are we confident that our half-hearted choice of "there is no repo,
so just do a very-early-config thing" is appropriate for any code
paths?

At least we should be perfectly happy with that choice applied to
all of these code paths touched by this series.

> Note, I have already sent the first 8 patches in [2] but, the 9th
> patch depends on the first patch of that series. 

So, is this [v2 0/9] of ua/some-builtins-wo-the-repository?

I think that topic has long been merged to 'next', and it is way too
late to do a wholesale replacement like this.

> [1] https://public-inbox.org/git/20250210181103.3609495-1-usmanakinyemi202@gmail.com/
> [2] https://public-inbox.org/git/20250307233543.1721552-1-usmanakinyemi202@gmail.com/
>
> Usman Akinyemi (9):
>   config: teach repo_config to allow `repo` to be NULL
>   builtin/verify-tag: stop using `the_repository`
>   builtin/verify-commit: stop using `the_repository`
>   builtin/send-pack: stop using `the_repository`
>   builtin/pack-refs: stop using `the_repository`
>   builtin/ls-files: stop using `the_repository`
>   builtin/for-each-ref: stop using `the_repository`
>   builtin/checkout-index: stop using `the_repository`
>   builtin/update-server-info: remove unnecessary if statement
>
>  builtin/checkout-index.c        | 43 ++++++++++++++++-----------------
>  builtin/for-each-ref.c          |  5 ++--
>  builtin/ls-files.c              | 32 ++++++++++++------------
>  builtin/pack-refs.c             |  8 +++---
>  builtin/send-pack.c             |  7 +++---
>  builtin/update-server-info.c    |  4 +--
>  builtin/verify-commit.c         | 13 +++++-----
>  builtin/verify-tag.c            |  7 +++---
>  config.c                        |  4 +++
>  config.h                        |  9 +++++++
>  t/t0610-reftable-basics.sh      |  7 ++++++
>  t/t2006-checkout-index-basic.sh |  7 ++++++
>  t/t3004-ls-files-basic.sh       |  7 ++++++
>  t/t5400-send-pack.sh            |  7 ++++++
>  t/t6300-for-each-ref.sh         |  7 ++++++
>  t/t7030-verify-tag.sh           |  7 ++++++
>  t/t7510-signed-commit.sh        |  7 ++++++
>  17 files changed, 118 insertions(+), 63 deletions(-)

Re: [PATCH 0/9] remove unnecessary if statement

[Usman Akinyemi]

On Wed, Mar 19, 2025 at 1:51 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Usman Akinyemi <usmanakinyemi202@gmail.com> writes:
>
> > In an earlier patch[1] which has been merged to the master,
> > We checked `repo` is not NULL before making call to `repo_config()`.
> > Later, in another patch series[2] which has been merged to next,
> > `repo_config()` was taught to allow `repo` to be NULL.
> >
> > So there is not need for checking if the `repo` is NULL before calling
> > repo_config() in the earlier patch.
>
> OK, that sounds good.
>
> Are we confident that our half-hearted choice of "there is no repo,
> so just do a very-early-config thing" is appropriate for any code
> paths?
>
> At least we should be perfectly happy with that choice applied to
> all of these code paths touched by this series.
This is fine for the commands which use only RUN_SETUP. I also picked and
checked those code paths touched by this series to ensure they do what we intend
what we intend them to do. I mean, if there is any better option, that
would be cool.
>
> > Note, I have already sent the first 8 patches in [2] but, the 9th
> > patch depends on the first patch of that series.
>
> So, is this [v2 0/9] of ua/some-builtins-wo-the-repository?
>
> I think that topic has long been merged to 'next', and it is way too
> late to do a wholesale replacement like this.
It is not a replacement for that patch series but, the last patch here
9/9 depends
on the first patch in that series. That is why I sent everything together.

Thank you.
>
> > [1] https://public-inbox.org/git/20250210181103.3609495-1-usmanakinyemi202@gmail.com/
> > [2] https://public-inbox.org/git/20250307233543.1721552-1-usmanakinyemi202@gmail.com/
> >
> > Usman Akinyemi (9):
> >   config: teach repo_config to allow `repo` to be NULL
> >   builtin/verify-tag: stop using `the_repository`
> >   builtin/verify-commit: stop using `the_repository`
> >   builtin/send-pack: stop using `the_repository`
> >   builtin/pack-refs: stop using `the_repository`
> >   builtin/ls-files: stop using `the_repository`
> >   builtin/for-each-ref: stop using `the_repository`
> >   builtin/checkout-index: stop using `the_repository`
> >   builtin/update-server-info: remove unnecessary if statement
> >
> >  builtin/checkout-index.c        | 43 ++++++++++++++++-----------------
> >  builtin/for-each-ref.c          |  5 ++--
> >  builtin/ls-files.c              | 32 ++++++++++++------------
> >  builtin/pack-refs.c             |  8 +++---
> >  builtin/send-pack.c             |  7 +++---
> >  builtin/update-server-info.c    |  4 +--
> >  builtin/verify-commit.c         | 13 +++++-----
> >  builtin/verify-tag.c            |  7 +++---
> >  config.c                        |  4 +++
> >  config.h                        |  9 +++++++
> >  t/t0610-reftable-basics.sh      |  7 ++++++
> >  t/t2006-checkout-index-basic.sh |  7 ++++++
> >  t/t3004-ls-files-basic.sh       |  7 ++++++
> >  t/t5400-send-pack.sh            |  7 ++++++
> >  t/t6300-for-each-ref.sh         |  7 ++++++
> >  t/t7030-verify-tag.sh           |  7 ++++++
> >  t/t7510-signed-commit.sh        |  7 ++++++
> >  17 files changed, 118 insertions(+), 63 deletions(-)