Re: [PATCH 3/5] pack-write: pass hash_algo to `write_idx_file()`

> Karthik Nayak via B4 Relay
> <devnull+karthik.188.gmail.com@kernel.org> writes:
>
>> From: Karthik Nayak <karthik.188@gmail.com>
>>
>> The `write_idx_file()` function uses the global `the_hash_algo` variable
>> to access the repository's hash function. To avoid global variable
>> usage, pass the hash function from the layers above.
>>
>> Altough the layers above could have access to the hash function
>> internally, simply pass in `the_hash_algo`. This avoids any
>> compatibility issues and bubbles up global variable usage to upper
>> layers which can be eventually resolved.
>> ...
>> -void stage_tmp_packfiles(struct strbuf *name_buffer,
>> +void stage_tmp_packfiles(const struct git_hash_algo *hash_algo,
>> +			 struct strbuf *name_buffer,
>>  			 const char *pack_tmp_name,
>>  			 struct pack_idx_entry **written_list,
>>  			 uint32_t nr_written,
>> @@ -561,8 +563,8 @@ void stage_tmp_packfiles(struct strbuf *name_buffer,
>>  	if (adjust_shared_perm(pack_tmp_name))
>>  		die_errno("unable to make temporary pack file readable");
>>
>> -	*idx_tmp_name = (char *)write_idx_file(NULL, written_list, nr_written,
>> -					       pack_idx_opts, hash);
>> +	*idx_tmp_name = (char *)write_idx_file(hash_algo, NULL, written_list,
>> +					       nr_written, pack_idx_opts, hash);
>
> The proposed log message should mention the reason why this
> stage_tmp_packfiles() function needs to be singled out among many
> other direct callers of write_idx_file() function.
>

> In other words, ...
>
>> @@ -798,8 +798,8 @@ static const char *create_index(void)
>>  	if (c != last)
>>  		die("internal consistency error creating the index");
>>
>> -	tmpfile = write_idx_file(NULL, idx, object_count, &pack_idx_opts,
>> -				 pack_data->hash);
>> +	tmpfile = write_idx_file(the_hash_algo, NULL, idx, object_count,
>> +				 &pack_idx_opts, pack_data->hash);
>>  	free(idx);
>>  	return tmpfile;
>>  }
>
> ... this hunk could have made create_index() to take a git_hash_algo
> object and pass it down to write_idx_file(), while changing all the
> callers of create_index() pass the_hash_algo, but we did not do so.
>
> But stage_tmp_packfiles() got that treatment.  Please tell your
> readers in the proposed log message what makes it special.
>
> Thanks.
>