From: Samuel Abraham Date: Tue, 13 Jan 2026 18:07:06 GMT Subject: Re: [GSoC PATCH v1] builtin: stop using the_repository Message-ID: In-Reply-To: On Tue, Jan 13, 2026 at 5:56 PM Junio C Hamano wrote: > > Abraham Samuel Adekunle writes: > > > The builtins use the_repository global variable which might > > not work well when running many repos in the same process at once. > > This is true, but ... > > > Stop using the_repository in these builtins to align with the goal of > > libification of Git. > > ... in general, each file under builtin/ is about a single command > that _uses_ libified part of Git. So it is perfectly fine for the > libification goal to include "libified functions should not assume > that it works on the_repository, but they should accept a repo > parameter to tell them which repository to work with". But it is > not necessary, and I would say it is harmful, to subject builtin/*.c > to the same criteria. The builtin command implementations can call > libified function by passing the_repository to libified API function > that expects a repo parameter. Oh thank you Junio for clarifying > > > Signed-off-by: Abraham Samuel Adekunle > > --- > > builtin/bugreport.c | 13 ++++++------- > > builtin/bundle.c | 13 ++++++------- > > builtin/check-attr.c | 26 +++++++++++++------------- > > builtin/check-ignore.c | 27 +++++++++++++++------------ > > 4 files changed, 40 insertions(+), 39 deletions(-) > > > > diff --git a/builtin/bugreport.c b/builtin/bugreport.c > > index f78c3f2aed..77eb8bd9c1 100644 > > --- a/builtin/bugreport.c > > +++ b/builtin/bugreport.c > > @@ -1,4 +1,3 @@ > > -#define USE_THE_REPOSITORY_VARIABLE > > #include "builtin.h" > > #include "abspath.h" > > #include "editor.h" > > @@ -37,7 +36,7 @@ static void get_system_info(struct strbuf *sys_info) > > shell ? shell : ""); > > } > > > > -static void get_populated_hooks(struct strbuf *hook_info, int nongit) > > +static void get_populated_hooks(struct repository *repo, struct strbuf *hook_info, int nongit) > > { > > const char **p; > > > > @@ -50,7 +49,7 @@ static void get_populated_hooks(struct strbuf *hook_info, int nongit) > > for (p = hook_name_list; *p; p++) { > > const char *hook = *p; > > > > - if (hook_exists(the_repository, hook)) > > + if (hook_exists(repo, hook)) > > strbuf_addf(hook_info, "%s\n", hook); > > } > > } > > It is not strictly necessary to churn a file-scope static function > like this one into taking an arbitrary repo parameter, as the only > caller of the function, presumably cmd_foo() in the builtin/foo.c > file, would pass the_repository anyway, whether it explicitly names > the_repository or passes the repo parameter that it got from its > caller, git.c:run_builtin(). We _can_ consider a change like the > above as a preparation to potentially move these functions to the > libified part of Git, so even though I said it is not necessary, it > is also OK to perform such a change. Okay thank you > > > @@ -93,7 +92,7 @@ static void get_header(struct strbuf *buf, const char *title) > > int cmd_bugreport(int argc, > > const char **argv, > > const char *prefix, > > - struct repository *repo UNUSED) > > + struct repository *repo) > > { > > struct strbuf buffer = STRBUF_INIT; > > struct strbuf report_path = STRBUF_INIT; > > @@ -141,7 +140,7 @@ int cmd_bugreport(int argc, > > } > > strbuf_addstr(&report_path, ".txt"); > > > > - switch (safe_create_leading_directories(the_repository, report_path.buf)) { > > + switch (safe_create_leading_directories(repo, report_path.buf)) { > > case SCLD_OK: > > case SCLD_EXISTS: > > break; > > @@ -158,7 +157,7 @@ int cmd_bugreport(int argc, > > strbuf_addftime(&zip_path, option_suffix, localtime_r(&now, &tm), 0, 0); > > strbuf_addstr(&zip_path, ".zip"); > > > > - if (create_diagnostics_archive(the_repository, &zip_path, diagnose)) > > + if (create_diagnostics_archive(repo, &zip_path, diagnose)) > > die_errno(_("unable to create diagnostics archive %s"), zip_path.buf); > > > > strbuf_release(&zip_path); > > @@ -171,7 +170,7 @@ int cmd_bugreport(int argc, > > get_system_info(&buffer); > > > > get_header(&buffer, _("Enabled Hooks")); > > - get_populated_hooks(&buffer, !startup_info->have_repository); > > + get_populated_hooks(repo, &buffer, !startup_info->have_repository); > > > > /* fopen doesn't offer us an O_EXCL alternative, except with glibc. */ > > report = xopen(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666); > > All of the above look fine. > > > diff --git a/builtin/bundle.c b/builtin/bundle.c > > index 1e170e9278..ef21ccfd89 100644 > > --- a/builtin/bundle.c > > +++ b/builtin/bundle.c > > Is this patch meant as a microproject in preparation for applying > for GSoC? If so, we ask to limit one quality focused one per > applicant. > > https://git.github.io/General-Microproject-Information/#only-one-quality-focused-microproject-per-applicant No, this patch is not meant as a microproject. I previously read in the General-Microproject-Information section "After it's done, work on different things" that we can keep contributing after a micoproject if we are itching to do more. So I just want to keep contributing. Do I drop the "GSoC" tag and send a v2? I will not include the tag in subsequent patches. Thanks Abraham.