[Outreachy PATCH v4 0/8] stop using the_repository global variable.
- From
Usman Akinyemi <usmanakinyemi202@gmail.com>
- Date
- Mar 7, 2025, 23:34 UTC
- Message-ID
- <20250307233543.1721552-1-usmanakinyemi202@gmail.com>
- In-Reply-To
- <20250306143629.1267358-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.
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 <chriscool@tuxfamily.org>
Signed-off-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
@@ 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,-- 2.48.1