Re: [PATCH 2/4] push: teach --base for ssh:// and file://

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

Jonathan Tan <jonathantanmy@google.com> writes:

>  static int do_push(int flags,
>  		   const struct string_list *push_options,
> -		   struct remote *remote)
> +		   struct remote *remote,
> +		   const char *push_base)
>  {
>  	int i, errs;
>  	const char **url;
> @@ -405,6 +406,8 @@ static int do_push(int flags,
>  				transport_get(remote, url[i]);
>  			if (flags & TRANSPORT_PUSH_OPTIONS)
>  				transport->push_options = push_options;
> +			if (push_base)
> +				transport_set_option(transport, TRANS_OPT_PUSH_BASE, push_base);
>  			if (push_with_options(transport, push_refspec, flags))
>  				errs++;
>  		}
> @@ -413,6 +416,8 @@ static int do_push(int flags,
>  			transport_get(remote, NULL);
>  		if (flags & TRANSPORT_PUSH_OPTIONS)
>  			transport->push_options = push_options;
> +		if (push_base)
> +			transport_set_option(transport, TRANS_OPT_PUSH_BASE, push_base);
>  		if (push_with_options(transport, push_refspec, flags))
>  			errs++;
>  	}

These just send push_base as-is.

> @@ -526,6 +531,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
>  	struct string_list *push_options;
>  	const struct string_list_item *item;
>  	struct remote *remote;
> +	const char *push_base = NULL;
>  
>  	struct option options[] = {
>  		OPT__VERBOSITY(&verbosity),
> @@ -562,6 +568,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
>  				TRANSPORT_FAMILY_IPV4),
>  		OPT_SET_INT('6', "ipv6", &family, N_("use IPv6 addresses only"),
>  				TRANSPORT_FAMILY_IPV6),
> +		OPT_STRING(0, "base", &push_base, N_("revision"),
> +			   N_("ancestor of commits to be pushed that is believed to be known by the server")),
>  		OPT_END()
>  	};

And this takes push_base as a string that is not even validated for
any constraints.

> @@ -629,7 +637,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
>  		if (strchr(item->string, '\n'))
>  			die(_("push options must not have new line characters"));
>  
> -	rc = do_push(flags, push_options, remote);
> +	rc = do_push(flags, push_options, remote, push_base);


And passes that arbitrary cruft given by the user down to the
transport.

As the spirit of the "base" parameter is to tell the other side that
it is what the receiving end believes to be common, shouldn't we
make sure we do have it on our side after getting it from the user
with OPT_STRING() before passing it down to the transport layer and
have the transport layer convert it to an object name?  This patch
assumes that running get_oid_hex() at the transport layer and
assuming that the transport would keep working on the_repository
(hence when we say "We expect that the receiving end has 'master'",
the transport somehow knows that is 'master' in our repository, not
in a submodule repository, for example), but by converting it to
full object name early, we do not have to assume transport to stay
that way.