{"thread":{"id":"63152","subject":"[PATCH 1/9] config: teach repo_config to allow `repo` to be NULL","startedAt":"2025-03-18T11:59:18Z","lastAt":"2025-03-21T07:00:48Z","messageCount":17,"participants":["Usman Akinyemi","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":9},"messages":[{"id":"514516","messageId":"20250318115912.2978992-1-usmanakinyemi202@gmail.com","threadId":"63152","inReplyTo":null,"subject":"[PATCH 0/9] remove unnecessary if statement","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2025-03-18T11:58:52Z","receivedAt":"2025-03-18T11:59:18Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"In an earlier patch[1] which has been merged to the master,\nWe checked `repo` is not NULL before making call to `repo_config()`.\nLater, in another patch series[2] which has been merged to next,\n`repo_config()` was taught to allow `repo` to be NULL.\n\nSo there is not need for checking if the `repo` is NULL before calling\nrepo_config() in the earlier patch.\n\nNote, I have already sent the first 8 patches in [2] but, the 9th\npatch depends on the first patch of that series. \n\n[1] https://public-inbox.org/git/20250210181103.3609495-1-usmanakinyemi202@gmail.com/\n[2] https://public-inbox.org/git/20250307233543.1721552-1-usmanakinyemi202@gmail.com/\n\nUsman Akinyemi (9):\n  config: teach repo_config to allow `repo` to be NULL\n  builtin/verify-tag: stop using `the_repository`\n  builtin/verify-commit: stop using `the_repository`\n  builtin/send-pack: stop using `the_repository`\n  builtin/pack-refs: stop using `the_repository`\n  builtin/ls-files: stop using `the_repository`\n  builtin/for-each-ref: stop using `the_repository`\n  builtin/checkout-index: stop using `the_repository`\n  builtin/update-server-info: remove unnecessary if statement\n\n builtin/checkout-index.c        | 43 ++++++++++++++++-----------------\n builtin/for-each-ref.c          |  5 ++--\n builtin/ls-files.c              | 32 ++++++++++++------------\n builtin/pack-refs.c             |  8 +++---\n builtin/send-pack.c             |  7 +++---\n builtin/update-server-info.c    |  4 +--\n builtin/verify-commit.c         | 13 +++++-----\n builtin/verify-tag.c            |  7 +++---\n config.c                        |  4 +++\n config.h                        |  9 +++++++\n t/t0610-reftable-basics.sh      |  7 ++++++\n t/t2006-checkout-index-basic.sh |  7 ++++++\n t/t3004-ls-files-basic.sh       |  7 ++++++\n t/t5400-send-pack.sh            |  7 ++++++\n t/t6300-for-each-ref.sh         |  7 ++++++\n t/t7030-verify-tag.sh           |  7 ++++++\n t/t7510-signed-commit.sh        |  7 ++++++\n 17 files changed, 118 insertions(+), 63 deletions(-)\n\n-- \n2.48.1\n\n"},{"id":"514515","messageId":"20250318115912.2978992-2-usmanakinyemi202@gmail.com","threadId":"63152","inReplyTo":"20250318115912.2978992-1-usmanakinyemi202@gmail.com","subject":"[PATCH 1/9] config: teach repo_config to allow `repo` to be NULL","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2025-03-18T11:58:53Z","receivedAt":"2025-03-18T11:59:22Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"The `repo` value can be NULL if a builtin command is run outside\nany repository. The current implementation of `repo_config()` will\nfail if `repo` is NULL.\n\nIf the `repo` is NULL the `repo_config()` can ignore the repository\nconfiguration but it should read the other configuration sources like\nthe system-side configuration instead of failing.\n\nTeach the `repo_config()` to allow `repo` to be NULL by calling the\n`read_very_early_config()` which read config but only enumerate system\nand global settings.\n\nThis will be useful in the following commits.\n\nSuggested-by: Junio C Hamano <gitster@pobox.com>\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n config.c | 4 ++++\n config.h | 9 +++++++++\n 2 files changed, 13 insertions(+)\n\ndiff --git a/config.c b/config.c\nindex 658569af08..e127afaa8f 100644\n--- a/config.c\n+++ b/config.c\n@@ -2521,6 +2521,10 @@ void repo_config_clear(struct repository *repo)\n \n void repo_config(struct repository *repo, config_fn_t fn, void *data)\n {\n+\tif (!repo) {\n+\t\tread_very_early_config(fn, data);\n+\t\treturn;\n+\t}\n \tgit_config_check_init(repo);\n \tconfigset_iter(repo->config, fn, data);\n }\ndiff --git a/config.h b/config.h\nindex 5c730c4f89..29a0277483 100644\n--- a/config.h\n+++ b/config.h\n@@ -219,6 +219,15 @@ void read_very_early_config(config_fn_t cb, void *data);\n  * repo-specific one; by overwriting, the higher-priority repo-specific\n  * value is left at the end).\n  *\n+ * In cases where the repository variable is NULL, repo_config() will\n+ * skip the per-repository config but retain system and global configs\n+ * by calling read_very_early_config() which also ignores one-time\n+ * overrides like \"git -c var=val\". This is to support handling \"git foo -h\"\n+ * (which lets git.c:run_builtin() to pass NULL and have the cmd_foo()\n+ * call repo_config() before calling parse_options() to notice \"-h\", give\n+ * help and exit) for a command that ordinarily require a repository\n+ * so this limitation may be OK (but if needed you are welcome to fix it).\n+ *\n  * Unlike git_config_from_file(), this function respects includes.\n  */\n void repo_config(struct repository *r, config_fn_t fn, void *);\n-- \n2.48.1\n\n"},{"id":"514517","messageId":"20250318115912.2978992-3-usmanakinyemi202@gmail.com","threadId":"63152","inReplyTo":"20250318115912.2978992-1-usmanakinyemi202@gmail.com","subject":"[PATCH 2/9] builtin/verify-tag: stop using `the_repository`","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2025-03-18T11:58:54Z","receivedAt":"2025-03-18T11:59:25Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Remove the_repository global variable in favor of the repository\nargument that gets passed in \"builtin/verify-tag.c\".\n\nWhen `-h` is passed to the command outside a Git repository, the\n`run_builtin()` will call the `cmd_verify_tag()` function with `repo` set\nto NULL and then early in the function, `parse_options()` call will give\nthe options help and exit.\n\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n builtin/verify-tag.c  | 7 +++----\n t/t7030-verify-tag.sh | 7 +++++++\n 2 files changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\nindex f6b97048a5..ed1c40338f 100644\n--- a/builtin/verify-tag.c\n+++ b/builtin/verify-tag.c\n@@ -5,7 +5,6 @@\n  *\n  * Based on git-verify-tag.sh\n  */\n-#define USE_THE_REPOSITORY_VARIABLE\n #include \"builtin.h\"\n #include \"config.h\"\n #include \"gettext.h\"\n@@ -23,7 +22,7 @@ static const char * const verify_tag_usage[] = {\n int cmd_verify_tag(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 \tint i = 1, verbose = 0, had_error = 0;\n \tunsigned flags = 0;\n@@ -35,7 +34,7 @@ int cmd_verify_tag(int argc,\n \t\tOPT_END()\n \t};\n \n-\tgit_config(git_default_config, NULL);\n+\trepo_config(repo, git_default_config, NULL);\n \n \targc = parse_options(argc, argv, prefix, verify_tag_options,\n \t\t\t     verify_tag_usage, PARSE_OPT_KEEP_ARGV0);\n@@ -56,7 +55,7 @@ int cmd_verify_tag(int argc,\n \t\tstruct object_id oid;\n \t\tconst char *name = argv[i++];\n \n-\t\tif (repo_get_oid(the_repository, name, &oid)) {\n+\t\tif (repo_get_oid(repo, name, &oid)) {\n \t\t\thad_error = !!error(\"tag '%s' not found.\", name);\n \t\t\tcontinue;\n \t\t}\ndiff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh\nindex 6f526c37c2..2c147072c1 100755\n--- a/t/t7030-verify-tag.sh\n+++ b/t/t7030-verify-tag.sh\n@@ -7,6 +7,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n . ./test-lib.sh\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n \n+test_expect_success GPG 'verify-tag does not crash with -h' '\n+\ttest_expect_code 129 git verify-tag -h >usage &&\n+\ttest_grep \"[Uu]sage: git verify-tag \" usage &&\n+\ttest_expect_code 129 nongit git verify-tag -h >usage &&\n+\ttest_grep \"[Uu]sage: git verify-tag \" usage\n+'\n+\n test_expect_success GPG 'create signed tags' '\n \techo 1 >file && git add file &&\n \ttest_tick && git commit -m initial &&\n-- \n2.48.1\n\n"},{"id":"514518","messageId":"20250318115912.2978992-4-usmanakinyemi202@gmail.com","threadId":"63152","inReplyTo":"20250318115912.2978992-1-usmanakinyemi202@gmail.com","subject":"[PATCH 3/9] builtin/verify-commit: stop using `the_repository`","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2025-03-18T11:58:55Z","receivedAt":"2025-03-18T11:59:28Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Remove the_repository global variable in favor of the repository\nargument that gets passed in \"builtin/verify-commit.c\".\n\nWhen `-h` is passed to the command outside a Git repository, the\n`run_builtin()` will call the `cmd_verify_commit()` function with `repo`\nset to NULL and then early in the function, `parse_options()` call will\ngive the options help and exit.\n\nPass the repository available in the calling context to `verify_commit()`\nto remove it's dependency on the global `the_repository` variable.\n\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n builtin/verify-commit.c  | 13 ++++++-------\n t/t7510-signed-commit.sh |  7 +++++++\n 2 files changed, 13 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/verify-commit.c b/builtin/verify-commit.c\nindex 779b7988ca..5f749a30da 100644\n--- a/builtin/verify-commit.c\n+++ b/builtin/verify-commit.c\n@@ -5,7 +5,6 @@\n  *\n  * Based on git-verify-tag\n  */\n-#define USE_THE_REPOSITORY_VARIABLE\n #include \"builtin.h\"\n #include \"config.h\"\n #include \"gettext.h\"\n@@ -33,15 +32,15 @@ static int run_gpg_verify(struct commit *commit, unsigned flags)\n \treturn ret;\n }\n \n-static int verify_commit(const char *name, unsigned flags)\n+static int verify_commit(struct repository *repo, const char *name, unsigned flags)\n {\n \tstruct object_id oid;\n \tstruct object *obj;\n \n-\tif (repo_get_oid(the_repository, name, &oid))\n+\tif (repo_get_oid(repo, name, &oid))\n \t\treturn error(\"commit '%s' not found.\", name);\n \n-\tobj = parse_object(the_repository, &oid);\n+\tobj = parse_object(repo, &oid);\n \tif (!obj)\n \t\treturn error(\"%s: unable to read file.\", name);\n \tif (obj->type != OBJ_COMMIT)\n@@ -54,7 +53,7 @@ static int verify_commit(const char *name, unsigned flags)\n int cmd_verify_commit(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 \tint i = 1, verbose = 0, had_error = 0;\n \tunsigned flags = 0;\n@@ -64,7 +63,7 @@ int cmd_verify_commit(int argc,\n \t\tOPT_END()\n \t};\n \n-\tgit_config(git_default_config, NULL);\n+\trepo_config(repo, git_default_config, NULL);\n \n \targc = parse_options(argc, argv, prefix, verify_commit_options,\n \t\t\t     verify_commit_usage, PARSE_OPT_KEEP_ARGV0);\n@@ -78,7 +77,7 @@ int cmd_verify_commit(int argc,\n \t * was received in the process of writing the gpg input: */\n \tsignal(SIGPIPE, SIG_IGN);\n \twhile (i < argc)\n-\t\tif (verify_commit(argv[i++], flags))\n+\t\tif (verify_commit(repo, argv[i++], flags))\n \t\t\thad_error = 1;\n \treturn had_error;\n }\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 0d2dd29fe6..39677e859a 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -8,6 +8,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n GNUPGHOME_NOT_USED=$GNUPGHOME\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n \n+test_expect_success GPG 'verify-commit does not crash with -h' '\n+\ttest_expect_code 129 git verify-commit -h >usage &&\n+\ttest_grep \"[Uu]sage: git verify-commit \" usage &&\n+\ttest_expect_code 129 nongit git verify-commit -h >usage &&\n+\ttest_grep \"[Uu]sage: git verify-commit \" usage\n+'\n+\n test_expect_success GPG 'create signed commits' '\n \ttest_oid_cache <<-\\EOF &&\n \theader sha1:gpgsig\n-- \n2.48.1\n\n"},{"id":"514519","messageId":"20250318115912.2978992-5-usmanakinyemi202@gmail.com","threadId":"63152","inReplyTo":"20250318115912.2978992-1-usmanakinyemi202@gmail.com","subject":"[PATCH 4/9] builtin/send-pack: stop using `the_repository`","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2025-03-18T11:58:56Z","receivedAt":"2025-03-18T11:59:31Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Remove the_repository global variable in favor of the repository\nargument that gets passed in \"builtin/send-pack.c\".\n\nWhen `-h` is passed to the command outside a Git repository, the\n`run_builtin()` will call the `cmd_send_pack()` function with `repo` set\nto NULL and then early in the function, `parse_options()` call will give\nthe options help and exit.\n\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n builtin/send-pack.c  | 7 +++----\n t/t5400-send-pack.sh | 7 +++++++\n 2 files changed, 10 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex 8d461008e2..c6e0e9d051 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -1,4 +1,3 @@\n-#define USE_THE_REPOSITORY_VARIABLE\n #include \"builtin.h\"\n #include \"config.h\"\n #include \"hex.h\"\n@@ -151,7 +150,7 @@ static int send_pack_config(const char *k, const char *v,\n int cmd_send_pack(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 \tstruct refspec rs = REFSPEC_INIT_PUSH;\n \tconst char *remote_name = NULL;\n@@ -212,7 +211,7 @@ int cmd_send_pack(int argc,\n \t\tOPT_END()\n \t};\n \n-\tgit_config(send_pack_config, NULL);\n+\trepo_config(repo, send_pack_config, NULL);\n \targc = parse_options(argc, argv, prefix, options, send_pack_usage, 0);\n \tif (argc > 0) {\n \t\tdest = argv[0];\n@@ -317,7 +316,7 @@ int cmd_send_pack(int argc,\n \tset_ref_status_for_push(remote_refs, args.send_mirror,\n \t\targs.force_update);\n \n-\tret = send_pack(the_repository, &args, fd, conn, remote_refs, &extra_have);\n+\tret = send_pack(repo, &args, fd, conn, remote_refs, &extra_have);\n \n \tif (helper_status)\n \t\tprint_helper_status(remote_refs);\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex 3f81f16e13..8f018d2f23 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -55,6 +55,13 @@ test_expect_success setup '\n \techo Rebase &&\n \tgit log'\n \n+test_expect_success 'send-pack does not crash with -h' '\n+\ttest_expect_code 129 git send-pack -h >usage &&\n+\ttest_grep \"[Uu]sage: git send-pack \" usage &&\n+\ttest_expect_code 129 nongit git send-pack -h >usage &&\n+\ttest_grep \"[Uu]sage: git send-pack \" usage\n+'\n+\n test_expect_success 'pack the source repository' '\n \tgit repack -a -d &&\n \tgit prune\n-- \n2.48.1\n\n"},{"id":"514520","messageId":"20250318115912.2978992-6-usmanakinyemi202@gmail.com","threadId":"63152","inReplyTo":"20250318115912.2978992-1-usmanakinyemi202@gmail.com","subject":"[PATCH 5/9] builtin/pack-refs: stop using `the_repository`","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2025-03-18T11:58:57Z","receivedAt":"2025-03-18T11:59:35Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Remove the_repository global variable in favor of the repository\nargument that gets passed in \"builtin/pack-refs.c\".\n\nWhen `-h` is passed to the command outside a Git repository, the\n`run_builtin()` will call the `cmd_pack_refs()` function with `repo` set\nto NULL and then early in the function, `parse_options()` call will give\nthe options help and exit.\n\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n builtin/pack-refs.c        | 8 +++-----\n t/t0610-reftable-basics.sh | 7 +++++++\n 2 files changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/pack-refs.c b/builtin/pack-refs.c\nindex 4fdd68880e..e47bae1c80 100644\n--- a/builtin/pack-refs.c\n+++ b/builtin/pack-refs.c\n@@ -1,5 +1,3 @@\n-#define USE_THE_REPOSITORY_VARIABLE\n-\n #include \"builtin.h\"\n #include \"config.h\"\n #include \"gettext.h\"\n@@ -15,7 +13,7 @@ static char const * const pack_refs_usage[] = {\n int cmd_pack_refs(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 \tstruct ref_exclusions excludes = REF_EXCLUSIONS_INIT;\n \tstruct string_list included_refs = STRING_LIST_INIT_NODUP;\n@@ -39,7 +37,7 @@ int cmd_pack_refs(int argc,\n \t\t\tN_(\"references to exclude\")),\n \t\tOPT_END(),\n \t};\n-\tgit_config(git_default_config, NULL);\n+\trepo_config(repo, git_default_config, NULL);\n \tif (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))\n \t\tusage_with_options(pack_refs_usage, opts);\n \n@@ -52,7 +50,7 @@ int cmd_pack_refs(int argc,\n \tif (!pack_refs_opts.includes->nr)\n \t\tstring_list_append(pack_refs_opts.includes, \"refs/tags/*\");\n \n-\tret = refs_pack_refs(get_main_ref_store(the_repository), &pack_refs_opts);\n+\tret = refs_pack_refs(get_main_ref_store(repo), &pack_refs_opts);\n \n \tclear_ref_exclusions(&excludes);\n \tstring_list_clear(&included_refs, 0);\ndiff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh\nindex 4618ffc108..002a75dee8 100755\n--- a/t/t0610-reftable-basics.sh\n+++ b/t/t0610-reftable-basics.sh\n@@ -14,6 +14,13 @@ export GIT_TEST_DEFAULT_REF_FORMAT\n \n INVALID_OID=$(test_oid 001)\n \n+test_expect_success 'pack-refs does not crash with -h' '\n+\ttest_expect_code 129 git pack-refs -h >usage &&\n+\ttest_grep \"[Uu]sage: git pack-refs \" usage &&\n+\ttest_expect_code 129 nongit git pack-refs -h >usage &&\n+\ttest_grep \"[Uu]sage: git pack-refs \" usage\n+'\n+\n test_expect_success 'init: creates basic reftable structures' '\n \ttest_when_finished \"rm -rf repo\" &&\n \tgit init repo &&\n-- \n2.48.1\n\n"},{"id":"514521","messageId":"20250318115912.2978992-7-usmanakinyemi202@gmail.com","threadId":"63152","inReplyTo":"20250318115912.2978992-1-usmanakinyemi202@gmail.com","subject":"[PATCH 6/9] builtin/ls-files: stop using `the_repository`","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2025-03-18T11:58:58Z","receivedAt":"2025-03-18T11:59:38Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Remove the_repository global variable in favor of the repository\nargument that gets passed in \"builtin/ls-files.c\".\n\nWhen `-h` is passed to the command outside a Git repository, the\n`run_builtin()` will call the `cmd_ls_files()` function with `repo` set\nto NULL and then early in the function, `show_usage_with_options_if_asked()`\ncall will give the options help and exit.\n\nPass the repository available in the calling context to both\n`expand_objectsize()` and `show_ru_info()` to remove their\ndependency on the global `the_repository` variable.\n\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n builtin/ls-files.c        | 32 ++++++++++++++++----------------\n t/t3004-ls-files-basic.sh |  7 +++++++\n 2 files changed, 23 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex a4431429b7..70a377e9c0 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -6,7 +6,6 @@\n  * Copyright (C) Linus Torvalds, 2005\n  */\n \n-#define USE_THE_REPOSITORY_VARIABLE\n #define DISABLE_SIGN_COMPARE_WARNINGS\n \n #include \"builtin.h\"\n@@ -245,12 +244,13 @@ static void show_submodule(struct repository *superproject,\n \trepo_clear(&subrepo);\n }\n \n-static void expand_objectsize(struct strbuf *line, const struct object_id *oid,\n+static void expand_objectsize(struct repository *repo, struct strbuf *line,\n+\t\t\t      const struct object_id *oid,\n \t\t\t      const enum object_type type, unsigned int padded)\n {\n \tif (type == OBJ_BLOB) {\n \t\tunsigned long size;\n-\t\tif (oid_object_info(the_repository, oid, &size) < 0)\n+\t\tif (oid_object_info(repo, oid, &size) < 0)\n \t\t\tdie(_(\"could not get object info about '%s'\"),\n \t\t\t    oid_to_hex(oid));\n \t\tif (padded)\n@@ -283,10 +283,10 @@ static void show_ce_fmt(struct repository *repo, const struct cache_entry *ce,\n \t\telse if (skip_prefix(format, \"(objecttype)\", &format))\n \t\t\tstrbuf_addstr(&sb, type_name(object_type(ce->ce_mode)));\n \t\telse if (skip_prefix(format, \"(objectsize:padded)\", &format))\n-\t\t\texpand_objectsize(&sb, &ce->oid,\n+\t\t\texpand_objectsize(repo, &sb, &ce->oid,\n \t\t\t\t\t  object_type(ce->ce_mode), 1);\n \t\telse if (skip_prefix(format, \"(objectsize)\", &format))\n-\t\t\texpand_objectsize(&sb, &ce->oid,\n+\t\t\texpand_objectsize(repo, &sb, &ce->oid,\n \t\t\t\t\t  object_type(ce->ce_mode), 0);\n \t\telse if (skip_prefix(format, \"(stage)\", &format))\n \t\t\tstrbuf_addf(&sb, \"%d\", ce_stage(ce));\n@@ -348,7 +348,7 @@ static void show_ce(struct repository *repo, struct dir_struct *dir,\n \t}\n }\n \n-static void show_ru_info(struct index_state *istate)\n+static void show_ru_info(struct repository *repo, struct index_state *istate)\n {\n \tstruct string_list_item *item;\n \n@@ -370,7 +370,7 @@ static void show_ru_info(struct index_state *istate)\n \t\t\tif (!ui->mode[i])\n \t\t\t\tcontinue;\n \t\t\tprintf(\"%s%06o %s %d\\t\", tag_resolve_undo, ui->mode[i],\n-\t\t\t       repo_find_unique_abbrev(the_repository, &ui->oid[i], abbrev),\n+\t\t\t       repo_find_unique_abbrev(repo, &ui->oid[i], abbrev),\n \t\t\t       i + 1);\n \t\t\twrite_name(path);\n \t\t}\n@@ -567,7 +567,7 @@ static int option_parse_exclude_standard(const struct option *opt,\n int cmd_ls_files(int argc,\n \t\t const char **argv,\n \t\t const char *cmd_prefix,\n-\t\t struct repository *repo UNUSED)\n+\t\t struct repository *repo)\n {\n \tint require_work_tree = 0, show_tag = 0, i;\n \tchar *max_prefix;\n@@ -647,15 +647,15 @@ int cmd_ls_files(int argc,\n \tshow_usage_with_options_if_asked(argc, argv,\n \t\t\t\t\t ls_files_usage, builtin_ls_files_options);\n \n-\tprepare_repo_settings(the_repository);\n-\tthe_repository->settings.command_requires_full_index = 0;\n+\tprepare_repo_settings(repo);\n+\trepo->settings.command_requires_full_index = 0;\n \n \tprefix = cmd_prefix;\n \tif (prefix)\n \t\tprefix_len = strlen(prefix);\n-\tgit_config(git_default_config, NULL);\n+\trepo_config(repo, git_default_config, NULL);\n \n-\tif (repo_read_index(the_repository) < 0)\n+\tif (repo_read_index(repo) < 0)\n \t\tdie(\"index file corrupt\");\n \n \targc = parse_options(argc, argv, prefix, builtin_ls_files_options,\n@@ -724,7 +724,7 @@ int cmd_ls_files(int argc,\n \t\tmax_prefix = common_prefix(&pathspec);\n \tmax_prefix_len = get_common_prefix_len(max_prefix);\n \n-\tprune_index(the_repository->index, max_prefix, max_prefix_len);\n+\tprune_index(repo->index, max_prefix, max_prefix_len);\n \n \t/* Treat unmatching pathspec elements as errors */\n \tif (pathspec.nr && error_unmatch)\n@@ -748,13 +748,13 @@ int cmd_ls_files(int argc,\n \t\t */\n \t\tif (show_stage || show_unmerged)\n \t\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"ls-files --with-tree\", \"-s/-u\");\n-\t\toverlay_tree_on_index(the_repository->index, with_tree, max_prefix);\n+\t\toverlay_tree_on_index(repo->index, with_tree, max_prefix);\n \t}\n \n-\tshow_files(the_repository, &dir);\n+\tshow_files(repo, &dir);\n \n \tif (show_resolve_undo)\n-\t\tshow_ru_info(the_repository->index);\n+\t\tshow_ru_info(repo, repo->index);\n \n \tif (ps_matched && report_path_error(ps_matched, &pathspec)) {\n \t\tfprintf(stderr, \"Did you forget to 'git add'?\\n\");\ndiff --git a/t/t3004-ls-files-basic.sh b/t/t3004-ls-files-basic.sh\nindex a1078f8701..4034a5a59f 100755\n--- a/t/t3004-ls-files-basic.sh\n+++ b/t/t3004-ls-files-basic.sh\n@@ -34,6 +34,13 @@ test_expect_success 'ls-files -h in corrupt repository' '\n \ttest_grep \"[Uu]sage: git ls-files \" broken/usage\n '\n \n+test_expect_success 'ls-files does not crash with -h' '\n+\ttest_expect_code 129 git ls-files -h >usage &&\n+\ttest_grep \"[Uu]sage: git ls-files \" usage &&\n+\ttest_expect_code 129 nongit git ls-files -h >usage &&\n+\ttest_grep \"[Uu]sage: git ls-files \" usage\n+'\n+\n test_expect_success SYMLINKS 'ls-files with absolute paths to symlinks' '\n \tmkdir subs &&\n \tln -s nosuch link &&\n-- \n2.48.1\n\n"},{"id":"514523","messageId":"20250318115912.2978992-8-usmanakinyemi202@gmail.com","threadId":"63152","inReplyTo":"20250318115912.2978992-1-usmanakinyemi202@gmail.com","subject":"[PATCH 7/9] builtin/for-each-ref: stop using `the_repository`","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2025-03-18T11:58:59Z","receivedAt":"2025-03-18T11:59:41Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Remove the_repository global variable in favor of the repository\nargument that gets passed in \"builtin/for-each-ref.c\".\n\nWhen `-h` is passed to the command outside a Git repository, the\n`run_builtin()` will call the `cmd_for_each_ref()` function with `repo`\nset to NULL and then early in the function, `parse_options()` call will\ngive the options help and exit.\n\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n builtin/for-each-ref.c  | 5 ++---\n t/t6300-for-each-ref.sh | 7 +++++++\n 2 files changed, 9 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 8085ebd8fe..3d2207ec77 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -1,4 +1,3 @@\n-#define USE_THE_REPOSITORY_VARIABLE\n #include \"builtin.h\"\n #include \"commit.h\"\n #include \"config.h\"\n@@ -20,7 +19,7 @@ static char const * const for_each_ref_usage[] = {\n int cmd_for_each_ref(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 \tstruct ref_sorting *sorting;\n \tstruct string_list sorting_options = STRING_LIST_INIT_DUP;\n@@ -63,7 +62,7 @@ int cmd_for_each_ref(int argc,\n \n \tformat.format = \"%(objectname) %(objecttype)\\t%(refname)\";\n \n-\tgit_config(git_default_config, NULL);\n+\trepo_config(repo, git_default_config, NULL);\n \n \t/* Set default (refname) sorting */\n \tstring_list_append(&sorting_options, \"refname\");\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex a5c7794385..9b4f4306c4 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -292,6 +292,13 @@ test_expect_success 'Check invalid atoms names are errors' '\n \ttest_must_fail git for-each-ref --format=\"%(INVALID)\" refs/heads\n '\n \n+test_expect_success 'for-each-ref does not crash with -h' '\n+\ttest_expect_code 129 git for-each-ref -h >usage &&\n+\ttest_grep \"[Uu]sage: git for-each-ref \" usage &&\n+\ttest_expect_code 129 nongit git for-each-ref -h >usage &&\n+\ttest_grep \"[Uu]sage: git for-each-ref \" usage\n+'\n+\n test_expect_success 'Check format specifiers are ignored in naming date atoms' '\n \tgit for-each-ref --format=\"%(authordate)\" refs/heads &&\n \tgit for-each-ref --format=\"%(authordate:default) %(authordate)\" refs/heads &&\n-- \n2.48.1\n\n"},{"id":"514522","messageId":"20250318115912.2978992-9-usmanakinyemi202@gmail.com","threadId":"63152","inReplyTo":"20250318115912.2978992-1-usmanakinyemi202@gmail.com","subject":"[PATCH 8/9] builtin/checkout-index: stop using `the_repository`","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2025-03-18T11:59:00Z","receivedAt":"2025-03-18T11:59:45Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Remove the_repository global variable in favor of the repository\nargument that gets passed in \"builtin/checkout-index.c\".\n\nWhen `-h` is passed to the command outside a Git repository, the\n`run_builtin()` will call the `cmd_checkout_index()` function with `repo`\nset to NULL and then early in the function, `show_usage_with_options_if_asked()`\ncall will give the options help and exit.\n\nPass an instance of \"struct index_state\" available in the calling\ncontext to both `checkout_all()` and `checkout_file()` to remove their\ndependency on the global `the_repository` variable.\n\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n builtin/checkout-index.c        | 43 ++++++++++++++++-----------------\n t/t2006-checkout-index-basic.sh |  7 ++++++\n 2 files changed, 28 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/checkout-index.c b/builtin/checkout-index.c\nindex e30086c7d4..7f74bc702f 100644\n--- a/builtin/checkout-index.c\n+++ b/builtin/checkout-index.c\n@@ -5,7 +5,6 @@\n  *\n  */\n \n-#define USE_THE_REPOSITORY_VARIABLE\n #define DISABLE_SIGN_COMPARE_WARNINGS\n \n #include \"builtin.h\"\n@@ -68,10 +67,10 @@ static void write_tempfile_record(const char *name, const char *prefix)\n \t}\n }\n \n-static int checkout_file(const char *name, const char *prefix)\n+static int checkout_file(struct index_state *index, const char *name, const char *prefix)\n {\n \tint namelen = strlen(name);\n-\tint pos = index_name_pos(the_repository->index, name, namelen);\n+\tint pos = index_name_pos(index, name, namelen);\n \tint has_same_name = 0;\n \tint is_file = 0;\n \tint is_skipped = 1;\n@@ -81,8 +80,8 @@ static int checkout_file(const char *name, const char *prefix)\n \tif (pos < 0)\n \t\tpos = -pos - 1;\n \n-\twhile (pos <the_repository->index->cache_nr) {\n-\t\tstruct cache_entry *ce =the_repository->index->cache[pos];\n+\twhile (pos < index->cache_nr) {\n+\t\tstruct cache_entry *ce = index->cache[pos];\n \t\tif (ce_namelen(ce) != namelen ||\n \t\t    memcmp(ce->name, name, namelen))\n \t\t\tbreak;\n@@ -137,13 +136,13 @@ static int checkout_file(const char *name, const char *prefix)\n \treturn -1;\n }\n \n-static int checkout_all(const char *prefix, int prefix_length)\n+static int checkout_all(struct index_state *index, const char *prefix, int prefix_length)\n {\n \tint i, errs = 0;\n \tstruct cache_entry *last_ce = NULL;\n \n-\tfor (i = 0; i < the_repository->index->cache_nr ; i++) {\n-\t\tstruct cache_entry *ce = the_repository->index->cache[i];\n+\tfor (i = 0; i < index->cache_nr ; i++) {\n+\t\tstruct cache_entry *ce = index->cache[i];\n \n \t\tif (S_ISSPARSEDIR(ce->ce_mode)) {\n \t\t\tif (!ce_skip_worktree(ce))\n@@ -156,8 +155,8 @@ static int checkout_all(const char *prefix, int prefix_length)\n \t\t\t * first entry inside the expanded sparse directory).\n \t\t\t */\n \t\t\tif (ignore_skip_worktree) {\n-\t\t\t\tensure_full_index(the_repository->index);\n-\t\t\t\tce = the_repository->index->cache[i];\n+\t\t\t\tensure_full_index(index);\n+\t\t\t\tce = index->cache[i];\n \t\t\t}\n \t\t}\n \n@@ -213,7 +212,7 @@ static int option_parse_stage(const struct option *opt,\n int cmd_checkout_index(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 \tint i;\n \tstruct lock_file lock_file = LOCK_INIT;\n@@ -253,19 +252,19 @@ int cmd_checkout_index(int argc,\n \tshow_usage_with_options_if_asked(argc, argv,\n \t\t\t\t\t builtin_checkout_index_usage,\n \t\t\t\t\t builtin_checkout_index_options);\n-\tgit_config(git_default_config, NULL);\n+\trepo_config(repo, git_default_config, NULL);\n \tprefix_length = prefix ? strlen(prefix) : 0;\n \n-\tprepare_repo_settings(the_repository);\n-\tthe_repository->settings.command_requires_full_index = 0;\n+\tprepare_repo_settings(repo);\n+\trepo->settings.command_requires_full_index = 0;\n \n-\tif (repo_read_index(the_repository) < 0) {\n+\tif (repo_read_index(repo) < 0) {\n \t\tdie(\"invalid cache\");\n \t}\n \n \targc = parse_options(argc, argv, prefix, builtin_checkout_index_options,\n \t\t\tbuiltin_checkout_index_usage, 0);\n-\tstate.istate = the_repository->index;\n+\tstate.istate = repo->index;\n \tstate.force = force;\n \tstate.quiet = quiet;\n \tstate.not_new = not_new;\n@@ -285,8 +284,8 @@ int cmd_checkout_index(int argc,\n \t */\n \tif (index_opt && !state.base_dir_len && !to_tempfile) {\n \t\tstate.refresh_cache = 1;\n-\t\tstate.istate = the_repository->index;\n-\t\trepo_hold_locked_index(the_repository, &lock_file,\n+\t\tstate.istate = repo->index;\n+\t\trepo_hold_locked_index(repo, &lock_file,\n \t\t\t\t       LOCK_DIE_ON_ERROR);\n \t}\n \n@@ -304,7 +303,7 @@ int cmd_checkout_index(int argc,\n \t\tif (read_from_stdin)\n \t\t\tdie(\"git checkout-index: don't mix '--stdin' and explicit filenames\");\n \t\tp = prefix_path(prefix, prefix_length, arg);\n-\t\terr |= checkout_file(p, prefix);\n+\t\terr |= checkout_file(repo->index, p, prefix);\n \t\tfree(p);\n \t}\n \n@@ -326,7 +325,7 @@ int cmd_checkout_index(int argc,\n \t\t\t\tstrbuf_swap(&buf, &unquoted);\n \t\t\t}\n \t\t\tp = prefix_path(prefix, prefix_length, buf.buf);\n-\t\t\terr |= checkout_file(p, prefix);\n+\t\t\terr |= checkout_file(repo->index, p, prefix);\n \t\t\tfree(p);\n \t\t}\n \t\tstrbuf_release(&unquoted);\n@@ -334,7 +333,7 @@ int cmd_checkout_index(int argc,\n \t}\n \n \tif (all)\n-\t\terr |= checkout_all(prefix, prefix_length);\n+\t\terr |= checkout_all(repo->index, prefix, prefix_length);\n \n \tif (pc_workers > 1)\n \t\terr |= run_parallel_checkout(&state, pc_workers, pc_threshold,\n@@ -344,7 +343,7 @@ int cmd_checkout_index(int argc,\n \t\treturn 1;\n \n \tif (is_lock_file_locked(&lock_file) &&\n-\t    write_locked_index(the_repository->index, &lock_file, COMMIT_LOCK))\n+\t    write_locked_index(repo->index, &lock_file, COMMIT_LOCK))\n \t\tdie(\"Unable to write new index file\");\n \treturn 0;\n }\ndiff --git a/t/t2006-checkout-index-basic.sh b/t/t2006-checkout-index-basic.sh\nindex bac231b167..fedd2cc097 100755\n--- a/t/t2006-checkout-index-basic.sh\n+++ b/t/t2006-checkout-index-basic.sh\n@@ -21,6 +21,13 @@ test_expect_success 'checkout-index -h in broken repository' '\n \ttest_grep \"[Uu]sage\" broken/usage\n '\n \n+test_expect_success 'checkout-index does not crash with -h' '\n+\ttest_expect_code 129 git checkout-index -h >usage &&\n+\ttest_grep \"[Uu]sage: git checkout-index \" usage &&\n+\ttest_expect_code 129 nongit git checkout-index -h >usage &&\n+\ttest_grep \"[Uu]sage: git checkout-index \" usage\n+'\n+\n test_expect_success 'checkout-index reports errors (cmdline)' '\n \ttest_must_fail git checkout-index -- does-not-exist 2>stderr &&\n \ttest_grep not.in.the.cache stderr\n-- \n2.48.1\n\n"},{"id":"514524","messageId":"20250318115912.2978992-10-usmanakinyemi202@gmail.com","threadId":"63152","inReplyTo":"20250318115912.2978992-1-usmanakinyemi202@gmail.com","subject":"[PATCH 9/9] builtin/update-server-info: remove unnecessary if statement","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2025-03-18T11:59:01Z","receivedAt":"2025-03-18T11:59:48Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"Since we already teach the `repo_config()` to allow `repo`\nto be NULL, no need to check if `repo` is NULL before calling\n`repo_config()`.\n\nMentored-by: Christian Couder <chriscool@tuxfamily.org>\nSigned-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>\n---\n builtin/update-server-info.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/update-server-info.c b/builtin/update-server-info.c\nindex d7467290a8..ba702d30ef 100644\n--- a/builtin/update-server-info.c\n+++ b/builtin/update-server-info.c\n@@ -20,8 +20,8 @@ int cmd_update_server_info(int argc,\n \t\tOPT_END()\n \t};\n \n-\tif (repo)\n-\t\trepo_config(repo, git_default_config, NULL);\n+\trepo_config(repo, git_default_config, NULL);\n+\n \targc = parse_options(argc, argv, prefix, options,\n \t\t\t     update_server_info_usage, 0);\n \tif (argc > 0)\n-- \n2.48.1\n\n"},{"id":"514549","messageId":"xmqqmsdi144i.fsf@gitster.g","threadId":"63152","inReplyTo":"20250318115912.2978992-1-usmanakinyemi202@gmail.com","subject":"Re: [PATCH 0/9] remove unnecessary if statement","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-18T20:21:49Z","receivedAt":"2025-03-18T20:21:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Usman Akinyemi <usmanakinyemi202@gmail.com> writes:\n\n> In an earlier patch[1] which has been merged to the master,\n> We checked `repo` is not NULL before making call to `repo_config()`.\n> Later, in another patch series[2] which has been merged to next,\n> `repo_config()` was taught to allow `repo` to be NULL.\n>\n> So there is not need for checking if the `repo` is NULL before calling\n> repo_config() in the earlier patch.\n\nOK, that sounds good.\n\nAre we confident that our half-hearted choice of \"there is no repo,\nso just do a very-early-config thing\" is appropriate for any code\npaths?\n\nAt least we should be perfectly happy with that choice applied to\nall of these code paths touched by this series.\n\n> Note, I have already sent the first 8 patches in [2] but, the 9th\n> patch depends on the first patch of that series. \n\nSo, is this [v2 0/9] of ua/some-builtins-wo-the-repository?\n\nI think that topic has long been merged to 'next', and it is way too\nlate to do a wholesale replacement like this.\n\n> [1] https://public-inbox.org/git/20250210181103.3609495-1-usmanakinyemi202@gmail.com/\n> [2] https://public-inbox.org/git/20250307233543.1721552-1-usmanakinyemi202@gmail.com/\n>\n> Usman Akinyemi (9):\n>   config: teach repo_config to allow `repo` to be NULL\n>   builtin/verify-tag: stop using `the_repository`\n>   builtin/verify-commit: stop using `the_repository`\n>   builtin/send-pack: stop using `the_repository`\n>   builtin/pack-refs: stop using `the_repository`\n>   builtin/ls-files: stop using `the_repository`\n>   builtin/for-each-ref: stop using `the_repository`\n>   builtin/checkout-index: stop using `the_repository`\n>   builtin/update-server-info: remove unnecessary if statement\n>\n>  builtin/checkout-index.c        | 43 ++++++++++++++++-----------------\n>  builtin/for-each-ref.c          |  5 ++--\n>  builtin/ls-files.c              | 32 ++++++++++++------------\n>  builtin/pack-refs.c             |  8 +++---\n>  builtin/send-pack.c             |  7 +++---\n>  builtin/update-server-info.c    |  4 +--\n>  builtin/verify-commit.c         | 13 +++++-----\n>  builtin/verify-tag.c            |  7 +++---\n>  config.c                        |  4 +++\n>  config.h                        |  9 +++++++\n>  t/t0610-reftable-basics.sh      |  7 ++++++\n>  t/t2006-checkout-index-basic.sh |  7 ++++++\n>  t/t3004-ls-files-basic.sh       |  7 ++++++\n>  t/t5400-send-pack.sh            |  7 ++++++\n>  t/t6300-for-each-ref.sh         |  7 ++++++\n>  t/t7030-verify-tag.sh           |  7 ++++++\n>  t/t7510-signed-commit.sh        |  7 ++++++\n>  17 files changed, 118 insertions(+), 63 deletions(-)\n"},{"id":"514603","messageId":"CAPSxiM9pW4V5_TxttNrkEKhgv-qaqgRGDgr+jkSCA0vNj71NsA@mail.gmail.com","threadId":"63152","inReplyTo":"xmqqmsdi144i.fsf@gitster.g","subject":"Re: [PATCH 0/9] remove unnecessary if statement","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2025-03-19T03:53:19Z","receivedAt":"2025-03-19T03:53:31Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Wed, Mar 19, 2025 at 1:51 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Usman Akinyemi <usmanakinyemi202@gmail.com> writes:\n>\n> > In an earlier patch[1] which has been merged to the master,\n> > We checked `repo` is not NULL before making call to `repo_config()`.\n> > Later, in another patch series[2] which has been merged to next,\n> > `repo_config()` was taught to allow `repo` to be NULL.\n> >\n> > So there is not need for checking if the `repo` is NULL before calling\n> > repo_config() in the earlier patch.\n>\n> OK, that sounds good.\n>\n> Are we confident that our half-hearted choice of \"there is no repo,\n> so just do a very-early-config thing\" is appropriate for any code\n> paths?\n>\n> At least we should be perfectly happy with that choice applied to\n> all of these code paths touched by this series.\nThis is fine for the commands which use only RUN_SETUP. I also picked and\nchecked those code paths touched by this series to ensure they do what we intend\nwhat we intend them to do. I mean, if there is any better option, that\nwould be cool.\n>\n> > Note, I have already sent the first 8 patches in [2] but, the 9th\n> > patch depends on the first patch of that series.\n>\n> So, is this [v2 0/9] of ua/some-builtins-wo-the-repository?\n>\n> I think that topic has long been merged to 'next', and it is way too\n> late to do a wholesale replacement like this.\nIt is not a replacement for that patch series but, the last patch here\n9/9 depends\non the first patch in that series. That is why I sent everything together.\n\nThank you.\n>\n> > [1] https://public-inbox.org/git/20250210181103.3609495-1-usmanakinyemi202@gmail.com/\n> > [2] https://public-inbox.org/git/20250307233543.1721552-1-usmanakinyemi202@gmail.com/\n> >\n> > Usman Akinyemi (9):\n> >   config: teach repo_config to allow `repo` to be NULL\n> >   builtin/verify-tag: stop using `the_repository`\n> >   builtin/verify-commit: stop using `the_repository`\n> >   builtin/send-pack: stop using `the_repository`\n> >   builtin/pack-refs: stop using `the_repository`\n> >   builtin/ls-files: stop using `the_repository`\n> >   builtin/for-each-ref: stop using `the_repository`\n> >   builtin/checkout-index: stop using `the_repository`\n> >   builtin/update-server-info: remove unnecessary if statement\n> >\n> >  builtin/checkout-index.c        | 43 ++++++++++++++++-----------------\n> >  builtin/for-each-ref.c          |  5 ++--\n> >  builtin/ls-files.c              | 32 ++++++++++++------------\n> >  builtin/pack-refs.c             |  8 +++---\n> >  builtin/send-pack.c             |  7 +++---\n> >  builtin/update-server-info.c    |  4 +--\n> >  builtin/verify-commit.c         | 13 +++++-----\n> >  builtin/verify-tag.c            |  7 +++---\n> >  config.c                        |  4 +++\n> >  config.h                        |  9 +++++++\n> >  t/t0610-reftable-basics.sh      |  7 ++++++\n> >  t/t2006-checkout-index-basic.sh |  7 ++++++\n> >  t/t3004-ls-files-basic.sh       |  7 ++++++\n> >  t/t5400-send-pack.sh            |  7 ++++++\n> >  t/t6300-for-each-ref.sh         |  7 ++++++\n> >  t/t7030-verify-tag.sh           |  7 ++++++\n> >  t/t7510-signed-commit.sh        |  7 ++++++\n> >  17 files changed, 118 insertions(+), 63 deletions(-)\n"},{"id":"514718","messageId":"Z9vB-Mm9YpIcv6UK@pks.im","threadId":"63152","inReplyTo":"20250318115912.2978992-2-usmanakinyemi202@gmail.com","subject":"Re: [PATCH 1/9] config: teach repo_config to allow `repo` to be NULL","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-20T07:21:28Z","receivedAt":"2025-03-20T07:21:38Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Mar 18, 2025 at 05:28:53PM +0530, Usman Akinyemi wrote:\n> diff --git a/config.c b/config.c\n> index 658569af08..e127afaa8f 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -2521,6 +2521,10 @@ void repo_config_clear(struct repository *repo)\n>  \n>  void repo_config(struct repository *repo, config_fn_t fn, void *data)\n>  {\n> +\tif (!repo) {\n> +\t\tread_very_early_config(fn, data);\n> +\t\treturn;\n> +\t}\n\nI remember discussion that `read_very_early_config()` may not be a good\nfit here. Most importantly, it ignores any configuration passed on the\ncommand line, which I would think is very surprising behaviour. So\nshould we adapt this to instead manually call `config_with_options()`\nwith the expected bits set?\n\nPatrick\n"},{"id":"514719","messageId":"Z9vCCfZJBsyp8Y3-@pks.im","threadId":"63152","inReplyTo":"20250318115912.2978992-10-usmanakinyemi202@gmail.com","subject":"Re: [PATCH 9/9] builtin/update-server-info: remove unnecessary if statement","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-20T07:21:45Z","receivedAt":"2025-03-20T07:21:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Mar 18, 2025 at 05:29:01PM +0530, Usman Akinyemi wrote:\n> Since we already teach the `repo_config()` to allow `repo`\n> to be NULL, no need to check if `repo` is NULL before calling\n> `repo_config()`.\n\nI think it would be preferable to reorder this patch so that it comes\nimmediately after the one where you adapt `repo_config()`.\n\nWe also have a couple of additional sites where we call the function\nconditionally:\n\n  - builtin/add.c\n  - builtin/difftool.c\n  - builtin/update-server-info.c\n\nIt would probably make sense to also adapt those.\n\nPatrick\n"},{"id":"514720","messageId":"Z9vCDFRUG7IzU_AG@pks.im","threadId":"63152","inReplyTo":"20250318115912.2978992-3-usmanakinyemi202@gmail.com","subject":"Re: [PATCH 2/9] builtin/verify-tag: stop using `the_repository`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-20T07:21:48Z","receivedAt":"2025-03-20T07:21:53Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Mar 18, 2025 at 05:28:54PM +0530, Usman Akinyemi wrote:\n> diff --git a/t/t7030-verify-tag.sh b/t/t7030-verify-tag.sh\n> index 6f526c37c2..2c147072c1 100755\n> --- a/t/t7030-verify-tag.sh\n> +++ b/t/t7030-verify-tag.sh\n> @@ -7,6 +7,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n>  . ./test-lib.sh\n>  . \"$TEST_DIRECTORY/lib-gpg.sh\"\n>  \n> +test_expect_success GPG 'verify-tag does not crash with -h' '\n> +\ttest_expect_code 129 git verify-tag -h >usage &&\n> +\ttest_grep \"[Uu]sage: git verify-tag \" usage &&\n> +\ttest_expect_code 129 nongit git verify-tag -h >usage &&\n> +\ttest_grep \"[Uu]sage: git verify-tag \" usage\n> +'\n> +\n>  test_expect_success GPG 'create signed tags' '\n>  \techo 1 >file && git add file &&\n>  \ttest_tick && git commit -m initial &&\n\nWe have \"t1517-outside-repo.sh\". Maybe it would preferable to add tests\nlike these to that test suite instead?\n\nPatrick\n"},{"id":"514822","messageId":"CAPSxiM9RDiBqqGsQmAFD5RvCh6ABXegUvpyzOC81Te3-NKX7Yg@mail.gmail.com","threadId":"63152","inReplyTo":"Z9vB-Mm9YpIcv6UK@pks.im","subject":"Re: [PATCH 1/9] config: teach repo_config to allow `repo` to be NULL","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2025-03-21T06:56:54Z","receivedAt":"2025-03-21T06:57:06Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Thu, Mar 20, 2025 at 12:51 PM Patrick Steinhardt <ps@pks.im> wrote:\n>\n> On Tue, Mar 18, 2025 at 05:28:53PM +0530, Usman Akinyemi wrote:\n> > diff --git a/config.c b/config.c\n> > index 658569af08..e127afaa8f 100644\n> > --- a/config.c\n> > +++ b/config.c\n> > @@ -2521,6 +2521,10 @@ void repo_config_clear(struct repository *repo)\n> >\n> >  void repo_config(struct repository *repo, config_fn_t fn, void *data)\n> >  {\n> > +     if (!repo) {\n> > +             read_very_early_config(fn, data);\n> > +             return;\n> > +     }\n>\n> I remember discussion that `read_very_early_config()` may not be a good\n> fit here. Most importantly, it ignores any configuration passed on the\n> command line, which I would think is very surprising behaviour. So\n> should we adapt this to instead manually call `config_with_options()`\n> with the expected bits set?\nHi Patrick,\n\nSorry for the confusion.\n\nThere was a discussion about this here.\nhttps://public-inbox.org/git/xmqqbjum2ayc.fsf@gitster.g/\nhttps://public-inbox.org/git/xmqqcyeuhwqb.fsf@gitster.g/\n\nMainliy ignoring the configuration passed from cmdline for this purpose.\n\nI was sending this patch again mainly for the 9/9 patch.\n\nThanks.\n>\n> Patrick\n"},{"id":"514823","messageId":"CAPSxiM8i=BRsn=7E5e28T1HndebefzrE8nuwZ0CdN+P+YymV4w@mail.gmail.com","threadId":"63152","inReplyTo":"Z9vCCfZJBsyp8Y3-@pks.im","subject":"Re: [PATCH 9/9] builtin/update-server-info: remove unnecessary if statement","fromName":"Usman Akinyemi","fromEmail":"usmanakinyemi202@gmail.com","sentAt":"2025-03-21T07:00:35Z","receivedAt":"2025-03-21T07:00:48Z","isPatch":true,"sender":{"key":"usmanakinyemi202@gmail.com","avatar":"https://avatars.githubusercontent.com/u/86585626?v=4"},"body":"On Thu, Mar 20, 2025 at 12:51 PM Patrick Steinhardt <ps@pks.im> wrote:\n>\n> On Tue, Mar 18, 2025 at 05:29:01PM +0530, Usman Akinyemi wrote:\n> > Since we already teach the `repo_config()` to allow `repo`\n> > to be NULL, no need to check if `repo` is NULL before calling\n> > `repo_config()`.\n>\n> I think it would be preferable to reorder this patch so that it comes\n> immediately after the one where you adapt `repo_config()`.\n>\n> We also have a couple of additional sites where we call the function\n> conditionally:\n>\n>   - builtin/add.c\n>   - builtin/difftool.c\n>   - builtin/update-server-info.c\n>\n> It would probably make sense to also adapt those.\n\nYeah, it has caused a little confusion. Maybe, let's just ignore this\npatch series.\n\nThe other series which introduces the first 8 patches of this has been\nmarked to be merged to master. I will resend the 9th patch(\nbuiltin/update-server-info.c) then other functions like difftool.\n\nThank you.\n>\n> Patrick\n"}]}