Re: [PATCH 2/4] push: teach --base for ssh:// and file://
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 8, 2020, 19:30 UTC
- Message-ID
- <xmqq4klzejdv.fsf@gitster.c.googlers.com>
- In-Reply-To
- <148e39960a2185d2355cdfe34f8856e708fb1b80.1604362701.git.jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
Show 26 quoted lines
> 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.
Show 16 quoted lines
> @@ -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.
Show 6 quoted lines
> @@ -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.