[Outreachy,v4,0/8] stop using the_repository global variable.

Message

Usman Akinyemi March 7, 2025, 11:34 p.m. UTC

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.

Some functions also uses `the_repository` global internally, so, let's
let's refactor them and pass `struct repo` as one of the argument. Some
functions only need an instance of "struct index_state", so, let's pass
it to such functions.

As 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 make it possible for us to savely replace
`git_config()` with `repo_config()`.

Changes since v3
================
- Add a comment to describe why we teach `repo_config()` to take NULL
repo value and what we intend to achieve.
- Pass "struct index_state" instead of repo in functions inside
"builtin/checkout-index.c"
- Fix some typo.

Usman Akinyemi (8):
  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/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/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 ++++++
 16 files changed, 116 insertions(+), 61 deletions(-)

Range-diff versus v3:

1:  a5c69f3753 ! 1:  f53677cbd6 config: teach repo_config to allow `repo` to be NULL
    @@ config.h: void read_very_early_config(config_fn_t cb, void *data);
       * value is left at the end).
       *
     + * In cases where the repository variable is NULL, repo_config() will
    -+ * call read_early_config().
    ++ * 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.
       */
2:  dfa0da4061 = 2:  6560218b7a builtin/verify-tag: stop using `the_repository`
3:  ade2d026cb = 3:  22681bad00 builtin/verify-commit: stop using `the_repository`
4:  e3b58bc6cf = 4:  a2a97b10a6 builtin/send-pack: stop using `the_repository`
5:  b11e99627c = 5:  b88e45e795 builtin/pack-refs: stop using `the_repository`
6:  51c80f9273 = 6:  d976fab012 builtin/ls-files: stop using `the_repository`
7:  63bb89291f = 7:  6a44666310 builtin/for-each-ref: stop using `the_repository`
8:  8dfe6b40c8 ! 8:  677e088e55 builtin/checkout-index: stop using `the_repository`
    @@ Commit message
         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 `checkout_all()`
    -    and `checkout_file()` to remove their dependency on the global
    -    `the_repository` variable.
    +    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 <[e-mail]>
         Signed-off-by: Usman Akinyemi <[e-mail]>
    @@ builtin/checkout-index.c: static void write_tempfile_record(const char *name, co
      }
      
     -static int checkout_file(const char *name, const char *prefix)
    -+static int checkout_file(struct repository *repo, 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(repo->index, name, namelen);
    ++	int pos = index_name_pos(index, name, namelen);
      	int has_same_name = 0;
      	int is_file = 0;
      	int is_skipped = 1;
    @@ builtin/checkout-index.c: static int checkout_file(const char *name, const char
      
     -	while (pos <the_repository->index->cache_nr) {
     -		struct cache_entry *ce =the_repository->index->cache[pos];
    -+	while (pos < repo->index->cache_nr) {
    -+		struct cache_entry *ce =repo->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;
    @@ builtin/checkout-index.c: static int checkout_file(const char *name, const char
      }
      
     -static int checkout_all(const char *prefix, int prefix_length)
    -+static int checkout_all(struct repository *repo, 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 < repo->index->cache_nr ; i++) {
    -+		struct cache_entry *ce = repo->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))
    @@ builtin/checkout-index.c: static int checkout_all(const char *prefix, int prefix
      			if (ignore_skip_worktree) {
     -				ensure_full_index(the_repository->index);
     -				ce = the_repository->index->cache[i];
    -+				ensure_full_index(repo->index);
    -+				ce = repo->index->cache[i];
    ++				ensure_full_index(index);
    ++				ce = index->cache[i];
      			}
      		}
      
    @@ builtin/checkout-index.c: int cmd_checkout_index(int argc,
      			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, p, prefix);
    ++		err |= checkout_file(repo->index, p, prefix);
      		free(p);
      	}
      
    @@ builtin/checkout-index.c: int cmd_checkout_index(int argc,
      			}
      			p = prefix_path(prefix, prefix_length, buf.buf);
     -			err |= checkout_file(p, prefix);
    -+			err |= checkout_file(repo, p, prefix);
    ++			err |= checkout_file(repo->index, p, prefix);
      			free(p);
      		}
      		strbuf_release(&unquoted);
    @@ builtin/checkout-index.c: int cmd_checkout_index(int argc,
      
      	if (all)
     -		err |= checkout_all(prefix, prefix_length);
    -+		err |= checkout_all(repo, prefix, prefix_length);
    ++		err |= checkout_all(repo->index, prefix, prefix_length);
      
      	if (pc_workers > 1)
      		err |= run_parallel_checkout(&state, pc_workers, pc_threshold,