Re: [PATCHv4 3/4] Support ref namespaces for remote repositories via upload-pack and receive-pack

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jamey Sharp <jamey@xxxxxxxxxxx> writes:

> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index e1a687a..9bb268a 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -109,6 +109,7 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
>  
>  static int show_ref(const char *path, const unsigned char *sha1, int flag, void *cb_data)
>  {
> +	path = path ? strip_namespace(path) : "capabilities^{}";
>  	if (sent_capabilities)
>  		packet_write(1, "%s %s\n", sha1_to_hex(sha1), path);
>  	else

This feels really ugly.

Logically the stripping of "path" should happen before the caller calls
this function, as the purpose of this function is "given a token and
object name, produce one line of 'I have this at here' protocol message,
which is defined to have the capability list tucked after the first of
such messages in an exchange". It now is "the token has to be a path in a
namespace; the only exception is when the token is NULL, in which case we
always send 'capabilities^{}'".

It also is a very selfish solution for an immediate issue(*) that does not
give much considertation for people who may want to add new things in the
future, as the _only_ possible special case is to send in NULL.

The immediate issue you wanted to solve, I think, is that it is not
convenient to strip in the caller as this is a callback. Still, I think it
should be easy to do something like...

	static int show_ref_message(const char *path,
        				 const unsigned char *sha1)
	{
		... original show_ref() implementation comes here ...
	}

        static int show_ref_cb(const char *path,
			        const unsigned char *sha1,
                                int flag, void *cb_data)
	{
		return show_ref_message(strip_namespace(path), sha1);
        }
        
and give the latter as the callback to for_each_ref_in_namespace().

And the call to run "capabilities^{}" when there is no ref can call
show_ref_message() directly.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]