Patrick Steinhardt <ps@xxxxxx> writes:
> On Tue, May 28, 2024 at 06:32:01AM +0000, Philip Peterson via GitGitGadget wrote:
>> diff --git a/apply.c b/apply.c
>> index 901b67e6255..364c05fbd06 100644
>> --- a/apply.c
>> +++ b/apply.c
>> @@ -3218,13 +3218,13 @@ static int apply_binary(struct apply_state *state,
>> return 0; /* deletion patch */
>> }
>>
>> - if (has_object(the_repository, &oid, 0)) {
>> + if (has_object(state->repo, &oid, 0)) {
>> /* We already have the postimage */
>> enum object_type type;
>> unsigned long size;
>> char *result;
>>
>> - result = repo_read_object_file(the_repository, &oid, &type,
>> + result = repo_read_object_file(state->repo, &oid, &type,
>> &size);
>> if (!result)
>> return error(_("the necessary postimage %s for "
>
> We call `get_oid_hex()` in this function, which ultimately ends up
> accessing `the_repository` via `the_hash_algo`. We should likely convert
> those to `repo_get_oid()`.
>
> There are also other accesses to `the_hash_algo` in this function, which
> should be converted to use `state->repo->hash_algo`, as well.
We as a more experienced developers should come up with a way to
help developers who are less familiar with the API set, so that they
can chip in this effort to wean ourselves off of globals.
Would a bug like the ones you pointed out be easily caught by say
running "GIT_TEST_DEFAULT_HASH=sha256 make test"?
By the way, for commands like "git apply" that can and does work
outside a repository, a change to use state->repo instead of
the_repository is only half a story. Dealing with cases where there
is no repository is the other half. I _think_ we have drawn a
reasonable line to punt and refuse binary patches and three-way
fallback outside a repository, but there may be use cases that
benefit from being able to do these things in a tarball extract.
