Re: [PATCH v4 2/2] remote: find tracking branches for URL push destinations

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

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +static bool remote_has_push_url(struct remote *remote, const char *url)
> +{
> +	const struct strvec *push_urls = push_url_of_remote(remote);
> +
> +	for (size_t i = 0; i < push_urls->nr; i++) {
> +		if (!strcmp(push_urls->v[i], url))
> +			return true;
> +	}
> +	return false;
> +}

A new helper is very much welcome.

>  void ref_push_report_free(struct ref_push_report *report)
>  {
>  	while (report) {
> @@ -1887,13 +1898,45 @@ const char *branch_get_upstream(struct branch *branch, struct strbuf *err)
>  	return branch->merge[0]->dst;
>  }
>  
> -static char *tracking_for_push_dest(struct repository *repo UNUSED,
> +struct remote *repo_remote_for_push_tracking(struct repository *repo,
> +					     struct remote *remote)
> +{
> +	const struct strvec *push_urls;
> +	struct remote *first_match = NULL;
> +	struct remote_state *remote_state = repo->remote_state;
> +	const char *check_url;
> +
> +	if (remote->origin != REMOTE_UNCONFIGURED)
> +		return remote;
> +
> +	push_urls = push_url_of_remote(remote);
> +	if (push_urls->nr != 1)
> +		return remote;
> +	check_url = push_urls->v[0];
> +
> +	for (int i = 0; i < remote_state->remotes_nr; i++) {
> +		struct remote *candidate = remote_state->remotes[i];
> +
> +		if (!candidate || candidate == remote ||
> +		    !remote_is_configured(candidate, 0) ||
> +		    !remote_has_push_url(candidate, check_url))

This part used to use remote_has_url(candidate, remote->url.v[0]),
which only looked at the .url and ignored .pushurl.  Now it uses
remote_has_push_url() so we grab the effective push URL for the
remote we are dealing with and match it against the effective push
URL of the candidates.  Looks correct.

> diff --git a/transport.c b/transport.c
> index fc144f0aed..30a4ab2cd5 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -1553,8 +1553,11 @@ int transport_push(struct repository *r,
>  	if (!(flags & (TRANSPORT_PUSH_DRY_RUN |
>  		       TRANSPORT_RECURSE_SUBMODULES_ONLY))) {
>  		struct ref *ref;
> +		struct remote *tracking_remote = repo_remote_for_push_tracking(
> +			r, transport->remote);

Personally, I would have line-wrapped the above more like this:

		struct remote *tracking_remote =
			repo_remote_for_push_tracking(r, transport->remote);

This is just for a future reference; it is certainly not critical
enough to warrant a new iteration just for this.

Thanks.