git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 00/12][Outreachy] stop using the_repository global variable.

From
Usman Akinyemi <usmanakinyemi202@gmail.com>
Date
Feb 19, 2025, 20:32 UTC
Message-ID
<20250219203349.787173-1-usmanakinyemi202@gmail.com>
In-Reply-To
<20250214230210.1460111-1-usmanakinyemi202@gmail.com>

Remove `the_repository` global variable in favor of the repository argument that gets passed in builtin commands.

These sets of commands are commands that use only RUN_SETUP macro in "git.c". Basically, When `-h` is passed to any of this command outside a Git repository, the `run_builtin()` will call the `cmd_x()` function (where `x` is any of the command from the sets of builtin commands that `the_repository` is removed from) with `repo` set to NULL and then early in the function, `parse_options()` or show_usage_with_options_if_asked() call will give the options help and exit, without having to consult much of the configuration file.

As there exist some builtin commands where the `repository` variable is accessed before options is given exit and fail, we will move the functions which accessed the `repository` below the `usage_with_options()` and it variants which will exit before getting to the function accessing the `repository` variable. We will do this in a preparatory patches.

Some, functions also uses `the_repository` global internally, so, let's let's refactor them and pass `struct repo` as one of the argument.

I picked some of this files based on the above explanation, how easy they are to resolve and how easy easy to review.

[1]: https://public-inbox.org/git/20250210181103.3609495-1-usmanakinyemi202@gmail.com/
Changes since v1
================
- Add some new preparatory patches to move the `git_config()` below the
`usage_with_options()`.
- Fix some errors in the commit messages.
Usman Akinyemi (12):
  builtin/verify-tag: refactor `cmd_verify_tag()`
  builtin/verify-tag: stop using `the_repository`
  builtin/verify-commit: refactor `cmd_verify_commit()`
  builtin/verify-commit: stop using `the_repository`
  builtin/send-pack: refactor `cmd_send_pack()`
  builtin/send-pack: stop using `the_repository`
  builtin/pack-refs: refactor `cmd_pack_refs()`
  builtin/pack-refs: stop using `the_repository`
  builtin/ls-files: stop using `the_repository`
  builtin/for-each-ref: refactor `cmd_for_each_ref()`
  builtin/for-each-ref: stop using `the_repository`
  builtin/checkout-index: stop using `the_repository`
 builtin/checkout-index.c | 43 ++++++++++++++++++++--------------------
 builtin/for-each-ref.c   |  6 ++----
 builtin/ls-files.c       | 32 +++++++++++++++---------------
 builtin/pack-refs.c      |  9 ++++-----
 builtin/send-pack.c      |  8 ++++----
 builtin/verify-commit.c  | 15 +++++++-------
 builtin/verify-tag.c     |  9 ++++-----
 7 files changed, 58 insertions(+), 64 deletions(-)
Range-diff versus v1:
 -:  ---------- >  1:  ceb03199e1 builtin/verify-tag: refactor `cmd_verify_tag()`
 1:  3e8d11ccfe !  2:  ecfb834600 builtin/verify-tag: stop using `the_repository`
    @@ Commit message
         `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, without having to consult much of the
    -    configuration file. So it is safe to omit reading the config when `repo`
    -    argument the caller gave us is NULL.
    +    configuration file.
     
         Mentored-by: Christian Couder <chriscool@tuxfamily.org>
         Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
    @@ builtin/verify-tag.c: static const char * const verify_tag_usage[] = {
      	int i = 1, verbose = 0, had_error = 0;
      	unsigned flags = 0;
     @@ builtin/verify-tag.c: int cmd_verify_tag(int argc,
    - 		OPT_END()
    - 	};
    + 		flags |= GPG_VERIFY_OMIT_STATUS;
    + 	}
      
     -	git_config(git_default_config, NULL);
    -+	if (repo)
    -+		repo_config(repo, 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);
    -@@ builtin/verify-tag.c: int cmd_verify_tag(int argc,
    + 	while (i < argc) {
      		struct object_id oid;
      		const char *name = argv[i++];
      
 -:  ---------- >  3:  2f770824b1 builtin/verify-commit: refactor `cmd_verify_commit()`
 2:  88af56e220 !  4:  546588a6ae builtin/verify-commit.c: stop using `the_repository`
    @@ Metadata
     Author: Usman Akinyemi <usmanakinyemi202@gmail.com>
     
      ## Commit message ##
    -    builtin/verify-commit.c: stop using `the_repository`
    +    builtin/verify-commit: stop using `the_repository`
     
         Remove the_repository global variable in favor of the repository
    -    argument that gets passed in "builtin/verify-tag.c".
    +    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, without having to consult much of the
    -    configuration file. So it is safe to omit reading the config when `repo`
    -    argument the caller gave us is NULL.
    +    configuration file.
     
    -    Let's pass `repository` argument to `verify_commit()` function to remove
    -    it's dependency on the global `the_repository` variable.
    +    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: static int verify_commit(const char *name, unsigned fla
      	int i = 1, verbose = 0, had_error = 0;
      	unsigned flags = 0;
     @@ builtin/verify-commit.c: int cmd_verify_commit(int argc,
    - 		OPT_END()
    - 	};
    + 	if (verbose)
    + 		flags |= GPG_VERIFY_VERBOSE;
      
     -	git_config(git_default_config, NULL);
    -+	if (repo)
    -+		repo_config(repo, 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);
    -@@ builtin/verify-commit.c: int cmd_verify_commit(int argc,
    + 	/* sometimes the program was terminated because this signal
      	 * was received in the process of writing the gpg input: */
      	signal(SIGPIPE, SIG_IGN);
      	while (i < argc)
 -:  ---------- >  5:  99f10469bd builtin/send-pack: refactor `cmd_send_pack()`
 3:  39409ea113 !  6:  cb3886bc46 builtin/send-pack.c: stop using `the_repository`
    @@ Metadata
     Author: Usman Akinyemi <usmanakinyemi202@gmail.com>
     
      ## Commit message ##
    -    builtin/send-pack.c: stop using `the_repository`
    +    builtin/send-pack: stop using `the_repository`
     
         Remove the_repository global variable in favor of the repository
         argument that gets passed in "builtin/send-pack.c".
    @@ Commit message
         `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, without having to consult much of the
    -    configuration file. So it is safe to omit reading the config when `repo`
    -    argument the caller gave us is NULL.
    +    configuration file.
     
         Mentored-by: Christian Couder <chriscool@tuxfamily.org>
         Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
    @@ builtin/send-pack.c: static int send_pack_config(const char *k, const char *v,
      	struct refspec rs = REFSPEC_INIT_PUSH;
      	const char *remote_name = NULL;
     @@ builtin/send-pack.c: int cmd_send_pack(int argc,
    - 		OPT_END()
    - 	};
    + 	if (!dest)
    + 		usage_with_options(send_pack_usage, options);
      
     -	git_config(send_pack_config, NULL);
    -+	if (repo)
    -+		repo_config(repo, send_pack_config, NULL);
    - 	argc = parse_options(argc, argv, prefix, options, send_pack_usage, 0);
    - 	if (argc > 0) {
    - 		dest = argv[0];
    ++	repo_config(repo, send_pack_config, NULL);
    + 
    + 	args.verbose = verbose;
    + 	args.dry_run = dry_run;
     @@ builtin/send-pack.c: int cmd_send_pack(int argc,
      	set_ref_status_for_push(remote_refs, args.send_mirror,
      		args.force_update);
 -:  ---------- >  7:  d104522e30 builtin/pack-refs: refactor `cmd_pack_refs()`
 4:  6fd5f4727c !  8:  7f02e48663 builtin/pack-refs: stop using `the_repository`
    @@ Commit message
         `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, without having to consult much of the
    -    configuration file. So it is safe to omit reading the config when `repo`
    -    argument the caller gave us is NULL.
    +    configuration file.
     
         Mentored-by: Christian Couder <chriscool@tuxfamily.org>
         Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
    @@ builtin/pack-refs.c: static char const * const pack_refs_usage[] = {
      	struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
      	struct string_list included_refs = STRING_LIST_INIT_NODUP;
     @@ builtin/pack-refs.c: int cmd_pack_refs(int argc,
    - 			N_("references to exclude")),
    - 		OPT_END(),
    - 	};
    --	git_config(git_default_config, NULL);
    -+	if (repo)
    -+		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);
      
    +-	git_config(git_default_config, NULL);
    ++	repo_config(repo, git_default_config, NULL);
    + 	for_each_string_list_item(item, &option_excluded_refs)
    + 		add_ref_exclusion(pack_refs_opts.exclusions, item->string);
    + 
     @@ builtin/pack-refs.c: int cmd_pack_refs(int argc,
      	if (!pack_refs_opts.includes->nr)
      		string_list_append(pack_refs_opts.includes, "refs/tags/*");
 5:  c58f27988b !  9:  b706fab321 builtin/ls-files: stop using `the_repository`
    @@ Commit message
         call will give the options help and exit, without having to consult much
         of the configuration file.
     
    -    Let's pass `repository` argument to `expand_objectsize()`,
    -    `show_ru_info()` functions to remove their dependency on the global
    -    `the_repository` variable.
    +    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>
 -:  ---------- > 10:  d2fc527d51 builtin/for-each-ref: refactor `cmd_for_each_ref()`
 6:  4bbca37330 ! 11:  c3a3a6cff7 builtin/for-each-ref: stop using `the_repository`
    @@ Commit message
         `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, without having to consult much of the
    -    configuration file. So it is safe to omit reading the config when `repo`
    -    argument the caller gave us is NULL.
    +    configuration file.
     
         Mentored-by: Christian Couder <chriscool@tuxfamily.org>
         Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
    @@ builtin/for-each-ref.c: static char const * const for_each_ref_usage[] = {
      	struct ref_sorting *sorting;
      	struct string_list sorting_options = STRING_LIST_INIT_DUP;
     @@ builtin/for-each-ref.c: int cmd_for_each_ref(int argc,
    - 
    - 	format.format = "%(objectname) %(objecttype)\t%(refname)";
    + 	if (verify_ref_format(&format))
    + 		usage_with_options(for_each_ref_usage, opts);
      
     -	git_config(git_default_config, NULL);
    -+	if (repo)
    -+		repo_config(repo, git_default_config, NULL);
    - 
    - 	/* Set default (refname) sorting */
    - 	string_list_append(&sorting_options, "refname");
    ++	repo_config(repo, git_default_config, NULL);
    + 	sorting = ref_sorting_options(&sorting_options);
    + 	ref_sorting_set_sort_flags_all(sorting, REF_SORTING_ICASE, icase);
    + 	filter.ignore_case = icase;
 7:  369008d554 ! 12:  d3a18f857e builtin/checkout-index.c: stop using `the_repository`
    @@ Metadata
     Author: Usman Akinyemi <usmanakinyemi202@gmail.com>
     
      ## Commit message ##
    -    builtin/checkout-index.c: stop using `the_repository`
    +    builtin/checkout-index: stop using `the_repository`
     
         Remove the_repository global variable in favor of the repository
         argument that gets passed in "builtin/checkout-index.c".
    @@ Commit message
         call will give the options help and exit, without having to consult much
         of the configuration file.
     
    -    Let's pass `repository` argument to `checkout_all()` and `checkout_file()`
    -    functions to remove their dependency on the global `the_repository` variable.
    +    Pass the repository 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>
-- 
2.48.1
Previous: Usman AkinyemiNext: Usman Akinyemi
Message 20 of 63 in “[Outreachy] stop using the_repository global variable.”
  1. 0/7 [Outreachy] stop using the_repository global variable.Usman Akinyemi, Feb 14, 2025
  2. 1/7 builtin/verify-tag: stop using `the_repository`Usman Akinyemi, Feb 14, 2025
  3. Patrick SteinhardtFeb 17, 2025
  4. Usman AkinyemiFeb 17, 2025
  5. Patrick SteinhardtFeb 17, 2025
  6. Usman AkinyemiFeb 17, 2025
  7. Patrick SteinhardtFeb 17, 2025
  8. 2/7 builtin/verify-commit.c: stop using `the_repository`Usman Akinyemi, Feb 14, 2025
  9. shejialuoFeb 16, 2025
  10. Usman AkinyemiFeb 17, 2025
  11. 3/7 builtin/send-pack.c: stop using `the_repository`Usman Akinyemi, Feb 14, 2025
  12. 4/7 builtin/pack-refs: stop using `the_repository`Usman Akinyemi, Feb 14, 2025
  13. 5/7 builtin/ls-files: stop using `the_repository`Usman Akinyemi, Feb 14, 2025
  14. Patrick SteinhardtFeb 17, 2025
  15. Usman AkinyemiFeb 17, 2025
  16. 6/7 builtin/for-each-ref: stop using `the_repository`Usman Akinyemi, Feb 14, 2025
  17. 7/7 builtin/checkout-index.c: stop using `the_repository`Usman Akinyemi, Feb 14, 2025
  18. shejialuoFeb 16, 2025
  19. Usman AkinyemiFeb 17, 2025
  20. 00/12 [Outreachy] stop using the_repository global variable.Usman Akinyemi, Feb 19, 2025
  21. 01/12 builtin/verify-tag: refactor `cmd_verify_tag()`Usman Akinyemi, Feb 19, 2025
  22. Junio C HamanoFeb 20, 2025
  23. 02/12 builtin/verify-tag: stop using `the_repository`Usman Akinyemi, Feb 19, 2025
  24. Junio C HamanoFeb 20, 2025
  25. Usman AkinyemiFeb 27, 2025
  26. Junio C HamanoFeb 27, 2025
  27. 03/12 builtin/verify-commit: refactor `cmd_verify_commit()`Usman Akinyemi, Feb 19, 2025
  28. 04/12 builtin/verify-commit: stop using `the_repository`Usman Akinyemi, Feb 19, 2025
  29. 05/12 builtin/send-pack: refactor `cmd_send_pack()`Usman Akinyemi, Feb 19, 2025
  30. 06/12 builtin/send-pack: stop using `the_repository`Usman Akinyemi, Feb 19, 2025
  31. 07/12 builtin/pack-refs: refactor `cmd_pack_refs()`Usman Akinyemi, Feb 19, 2025
  32. 08/12 builtin/pack-refs: stop using `the_repository`Usman Akinyemi, Feb 19, 2025
  33. 09/12 builtin/ls-files: stop using `the_repository`Usman Akinyemi, Feb 19, 2025
  34. 10/12 builtin/for-each-ref: refactor `cmd_for_each_ref()`Usman Akinyemi, Feb 19, 2025
  35. 11/12 builtin/for-each-ref: stop using `the_repository`Usman Akinyemi, Feb 19, 2025
  36. 12/12 builtin/checkout-index: stop using `the_repository`Usman Akinyemi, Feb 19, 2025
  37. 0/8 stop using the_repository global variable.Usman Akinyemi, Mar 6, 2025
  38. 1/8 config: teach repo_config to allow `repo` to be NULLUsman Akinyemi, Mar 6, 2025
  39. Junio C HamanoMar 6, 2025
  40. Usman AkinyemiMar 7, 2025
  41. Phillip WoodMar 7, 2025
  42. 2/8 builtin/verify-tag: stop using `the_repository`Usman Akinyemi, Mar 6, 2025
  43. Junio C HamanoMar 6, 2025
  44. 3/8 builtin/verify-commit: stop using `the_repository`Usman Akinyemi, Mar 6, 2025
  45. 4/8 builtin/send-pack: stop using `the_repository`Usman Akinyemi, Mar 6, 2025
  46. 5/8 builtin/pack-refs: stop using `the_repository`Usman Akinyemi, Mar 6, 2025
  47. 6/8 builtin/ls-files: stop using `the_repository`Usman Akinyemi, Mar 6, 2025
  48. Junio C HamanoMar 6, 2025
  49. 7/8 builtin/for-each-ref: stop using `the_repository`Usman Akinyemi, Mar 6, 2025
  50. 8/8 builtin/checkout-index: stop using `the_repository`Usman Akinyemi, Mar 6, 2025
  51. Junio C HamanoMar 6, 2025
  52. Usman AkinyemiMar 7, 2025
  53. Junio C HamanoMar 7, 2025
  54. Usman AkinyemiMar 7, 2025
  55. 0/8 stop using the_repository global variable.Usman Akinyemi, Mar 7, 2025
  56. 1/8 config: teach repo_config to allow `repo` to be NULLUsman Akinyemi, Mar 7, 2025
  57. 2/8 builtin/verify-tag: stop using `the_repository`Usman Akinyemi, Mar 7, 2025
  58. 3/8 builtin/verify-commit: stop using `the_repository`Usman Akinyemi, Mar 7, 2025
  59. 4/8 builtin/send-pack: stop using `the_repository`Usman Akinyemi, Mar 7, 2025
  60. 5/8 builtin/pack-refs: stop using `the_repository`Usman Akinyemi, Mar 7, 2025
  61. 6/8 builtin/ls-files: stop using `the_repository`Usman Akinyemi, Mar 7, 2025
  62. 7/8 builtin/for-each-ref: stop using `the_repository`Usman Akinyemi, Mar 7, 2025
  63. 8/8 builtin/checkout-index: stop using `the_repository`Usman Akinyemi, Mar 7, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.