Re: [GSoC PATCH v1] builtin: stop using the_repository
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 13, 2026, 16:56 UTC
- Message-ID
- <xmqq7btljvt2.fsf@gitster.g>
- In-Reply-To
- <aWZkEYHhcIhdAjkh@Adekunles-MacBook-Air.local>
Abraham Samuel Adekunle <abrahamadekunle50@gmail.com> 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.
Show 35 quoted lines
> Signed-off-by: Abraham Samuel Adekunle <abrahamadekunle50@gmail.com>
> ---
> 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 : "<unset>");
> }
>
> -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.
Show 36 quoted lines
> @@ -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