Re: [PATCH v2] builtin/mktree: remove USE_THE_REPOSITORY_VARIABLE
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 12, 2026, 17:11 UTC
- Message-ID
- <xmqqsea5ezwl.fsf@gitster.g>
- In-Reply-To
- <20260312164203.964033-1-cat@malon.dev>
Tian Yuchen <cat@malon.dev> writes:
Show 12 quoted lines
> The 'cmd_mktree()' function already receives a 'struct repository *repo' > pointer, but it was previously marked as UNUSED. > > Pass the 'repo' pointer down to 'mktree_line()' and 'write_tree()'. > Consequently, remove the 'USE_THE_REPOSITORY_VARIABLE' macro, replace > usages of 'the_repository', and swap 'parse_oid_hex()' with its context-aware > version 'parse_oid_hex_algop()'. > > This refactoring is safe because 'cmd_mktree()' is registered with the > 'RUN_SETUP' flag in 'git.c', which guarantees that the command is > executed within a initialized repository, ensuring that the passed 'repo' > pointer is never 'NULL'.
RUN_SETUP also guarantees that the repo points at the_repository.
The patch is not wrong per-se, but at the same time, it is not a very interesting change exactly for this reason.
Where did you read that dropping USE_THE_REPOSITORY_VARIABLE is a good idea?
As somebody (Phillip?) said earlier, we probably should update document and clearly say that removing USE_THE_REPOSITORY_VARIABLE is not a high-value target when done in the builtin/ directory, even though it is very desirable thing to do for more library-ish part of the codebase.
Show 93 quoted lines
> Signed-off-by: Tian Yuchen <cat@malon.dev>
> ---
> builtin/mktree.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/mktree.c b/builtin/mktree.c
> index 12772303f5..4084e32476 100644
> --- a/builtin/mktree.c
> +++ b/builtin/mktree.c
> @@ -3,7 +3,6 @@
> *
> * Copyright (c) Junio C Hamano, 2006, 2009
> */
> -#define USE_THE_REPOSITORY_VARIABLE
> #include "builtin.h"
> #include "gettext.h"
> #include "hex.h"
> @@ -46,7 +45,7 @@ static int ent_compare(const void *a_, const void *b_)
> b->name, b->len, b->mode);
> }
>
> -static void write_tree(struct object_id *oid)
> +static void write_tree(struct repository *repo, struct object_id *oid)
> {
> struct strbuf buf;
> size_t size;
> @@ -60,10 +59,10 @@ static void write_tree(struct object_id *oid)
> for (i = 0; i < used; i++) {
> struct treeent *ent = entries[i];
> strbuf_addf(&buf, "%o %s%c", ent->mode, ent->name, '\0');
> - strbuf_add(&buf, ent->oid.hash, the_hash_algo->rawsz);
> + strbuf_add(&buf, ent->oid.hash, repo->hash_algo->rawsz);
> }
>
> - odb_write_object(the_repository->objects, buf.buf, buf.len, OBJ_TREE, oid);
> + odb_write_object(repo->objects, buf.buf, buf.len, OBJ_TREE, oid);
> strbuf_release(&buf);
> }
>
> @@ -72,7 +71,7 @@ static const char *const mktree_usage[] = {
> NULL
> };
>
> -static void mktree_line(char *buf, int nul_term_line, int allow_missing)
> +static void mktree_line(struct repository *repo, char *buf, int nul_term_line, int allow_missing)
> {
> char *ptr, *ntr;
> const char *p;
> @@ -93,7 +92,7 @@ static void mktree_line(char *buf, int nul_term_line, int allow_missing)
> die("input format error: %s", buf);
> ptr = ntr + 1; /* type */
> ntr = strchr(ptr, ' ');
> - if (!ntr || parse_oid_hex(ntr + 1, &oid, &p) ||
> + if (!ntr || parse_oid_hex_algop(ntr + 1, &oid, &p, repo->hash_algo) ||
> *p != '\t')
> die("input format error: %s", buf);
>
> @@ -124,7 +123,7 @@ static void mktree_line(char *buf, int nul_term_line, int allow_missing)
>
> /* Check the type of object identified by oid without fetching objects */
> oi.typep = &obj_type;
> - if (odb_read_object_info_extended(the_repository->objects, &oid, &oi,
> + if (odb_read_object_info_extended(repo->objects, &oid, &oi,
> OBJECT_INFO_LOOKUP_REPLACE |
> OBJECT_INFO_QUICK |
> OBJECT_INFO_SKIP_FETCH_OBJECT) < 0)
> @@ -155,7 +154,7 @@ static void mktree_line(char *buf, int nul_term_line, int allow_missing)
> int cmd_mktree(int ac,
> const char **av,
> const char *prefix,
> - struct repository *repo UNUSED)
> + struct repository *repo)
> {
> struct strbuf sb = STRBUF_INIT;
> struct object_id oid;
> @@ -187,7 +186,7 @@ int cmd_mktree(int ac,
> break;
> die("input format error: (blank line only valid in batch mode)");
> }
> - mktree_line(sb.buf, nul_term_line, allow_missing);
> + mktree_line(repo, sb.buf, nul_term_line, allow_missing);
> }
> if (is_batch_mode && got_eof && used < 1) {
> /*
> @@ -197,7 +196,7 @@ int cmd_mktree(int ac,
> */
> ; /* skip creating an empty tree */
> } else {
> - write_tree(&oid);
> + write_tree(repo, &oid);
> puts(oid_to_hex(&oid));
> fflush(stdout);
> }