Re: [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want

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

Fredrik Medley <fredrik.medley@gmail.com> writes:

> Subject: Re: [PATCH 2/3] upload-pack: Prepare to extend allow-tip-sha1-in-want

s/: Prepare/: prepare/;

> Rename the allow_tip_sha1_in_want variable to
> allow_request_with_bare_object_name to allow for future extensions, e.g.
> allowing non-tip sha1.
> ---

Sign-off?

>  fetch-pack.c  |  9 ++++++---
>  upload-pack.c | 18 +++++++++++-------
>  2 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 48526aa..77174f9 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -43,7 +43,10 @@ static int marked;
>  #define MAX_IN_VAIN 256
>  
>  static struct prio_queue rev_list = { compare_commits_by_commit_date };
> -static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;
> +static int non_common_revs, multi_ack, use_sideband;
> +/* Allow specifying sha1 if it is a ref tip. */
> +#define ALLOW_TIP	01
> +static int allow_request_with_bare_object_name;

This side is OK, as "request" is by the end user giving the object
name from its command line.

> diff --git a/upload-pack.c b/upload-pack.c
> index aa84576..708a502 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -35,7 +35,9 @@ static int multi_ack;
>  static int no_done;
>  static int use_thin_pack, use_ofs_delta, use_include_tag;
>  static int no_progress, daemon_mode;
> -static int allow_tip_sha1_in_want;
> +/* Allow specifying sha1 if it is a ref tip. */
> +#define ALLOW_TIP	01
> +static int allow_request_with_bare_object_name;

This side is not quite good, as the request coming over wire is
always 40-hex bare object name.  We are allowing requests against
what we did not advertise (either the tip of hidden refs, or
somewhere deep in the history from some tip that may or may not have
been advertised).

allow-unadvertised-object-request or something, perhaps?

>  static int shallow_nr;
>  static struct object_array have_obj;
>  static struct object_array want_obj;
> @@ -442,8 +444,8 @@ static int get_common_commits(void)
>  
>  static int is_our_ref(struct object *o)
>  {
> -	return o->flags &
> -		((allow_tip_sha1_in_want ? HIDDEN_REF : 0) | OUR_REF);
> +	int allow_hidden_ref = (allow_request_with_bare_object_name & ALLOW_TIP);
> +	return o->flags & ((allow_hidden_ref ? HIDDEN_REF : 0) | OUR_REF);
>  }
>  
>  static void check_non_tip(void)
> @@ -727,7 +729,8 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
>  		packet_write(1, "%s %s%c%s%s%s%s agent=%s\n",
>  			     sha1_to_hex(sha1), refname_nons,
>  			     0, capabilities,
> -			     allow_tip_sha1_in_want ? " allow-tip-sha1-in-want" : "",
> +			     (allow_request_with_bare_object_name & ALLOW_TIP) ?
> +				     " allow-tip-sha1-in-want" : "",
>  			     stateless_rpc ? " no-done" : "",
>  			     symref_info.buf,
>  			     git_user_agent_sanitized());
> @@ -787,9 +790,10 @@ static void upload_pack(void)
>  
>  static int upload_pack_config(const char *var, const char *value, void *unused)
>  {
> -	if (!strcmp("uploadpack.allowtipsha1inwant", var))
> -		allow_tip_sha1_in_want = git_config_bool(var, value);
> -	else if (!strcmp("uploadpack.keepalive", var)) {
> +	if (!strcmp("uploadpack.allowtipsha1inwant", var)) {
> +		if (git_config_bool(var, value))
> +			allow_request_with_bare_object_name |= ALLOW_TIP;
> +	} else if (!strcmp("uploadpack.keepalive", var)) {
>  		keepalive = git_config_int(var, value);
>  		if (!keepalive)
>  			keepalive = -1;