Subject: Re: [PATCH 1/2] add-patch: do not use the_repository

From: Junio C Hamano <[email-masked]>
Date: Tue, 28 May 2024 14:41:47 -0700

"Philip Peterson via GitGitGadget" <[email-masked]> writes:

> -static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
> +static int parse_diff(struct repository *r, struct add_p_state *s, const struct pathspec *ps)

Given that add_p_state has add_i_state in it, which in turn as a
pointer to the repository, which is initialized like so:

        int run_add_p(struct repository *r, enum add_p_mode mode,
                      const char *revision, const struct pathspec *ps)
        {
                struct add_p_state s = {
                        { r }, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
                };
                size_t i, binary_count = 0;

                init_add_i_state(&s.s, r);

this patch looks wrong.  Adding a separate repository pointer to the
function means you are saying that this function should be able to
operate one repository in 'r' that may be DIFFERENT from the
repository add_p_state was initialized for.  I do not think you are
achieving that with this patch (and I do not think such a feature
makes much sense, either).

Instead just leave everything the same as before, and rewrite things
that depend on the_repository (either by explicitly referring to it,
or by implicitly using it, like empty_tree_oid_hex() which hardcodes
the use of the_hash_algo which is the_repository->hash_algo) to refer
to the repository the add_p_state was initialized for.