Re: [PATCH] push: don't fetch commit object when checking existence

From: Junio C Hamano <gitster@pobox.com>
Date: May 22, 2024, 19:16 UTC
Message-ID: <xmqq8r014pyn.fsf@gitster.g>
In-Reply-To: <20240522133621.1308393-1-tom@compton.nu>

Tom Hughes <tom@compton.nu> writes:

Show 9 quoted lines

> If we're checking to see whether to tell the user to do a fetch
> before pushing there's no need for us to actually fetch the object
> from the remote if the clone is partial.
>
> Because the promisor doesn't do negotiation actually trying to do
> the fetch of the new head can be very expensive as it will try and
> include history that we already have and it just results in rejecting
> the push with a different message, and in behavior that is different
> to a clone that is not partial.

Interesting. Is this something that is easily testable, perhaps by preparing a partial clone and try to push from there and checking the non-existence of the object after seeing that push failed?

Thanks.

Show 18 quoted lines

> Signed-off-by: Tom Hughes <tom@compton.nu>
> ---
>  remote.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/remote.c b/remote.c
> index 2b650b813b..20395bbbd0 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1773,7 +1773,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
>  		if (!reject_reason && !ref->deletion && !is_null_oid(&ref->old_oid)) {
>  			if (starts_with(ref->name, "refs/tags/"))
>  				reject_reason = REF_STATUS_REJECT_ALREADY_EXISTS;
> -			else if (!repo_has_object_file(the_repository, &ref->old_oid))
> +			else if (!repo_has_object_file_with_flags(the_repository, &ref->old_oid, OBJECT_INFO_SKIP_FETCH_OBJECT))
>  				reject_reason = REF_STATUS_REJECT_FETCH_FIRST;
>  			else if (!lookup_commit_reference_gently(the_repository, &ref->old_oid, 1) ||
>  				 !lookup_commit_reference_gently(the_repository, &ref->new_oid, 1))