Re: [PATCH 2/2] apply: do not use the_repository

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> 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.