Re: [PATCH v4 19/35] push: pass ref patterns when pushing

[Brandon Williams]

On 03/02, Junio C Hamano wrote:
> Brandon Williams  writes:
> 
> > Construct a list of ref patterns to be passed to 'get_refs_list()' from
> > the refspec to be used during the push.  This list of ref patterns will
> > be used to allow the server to filter the ref advertisement when
> > communicating using protocol v2.
> >
> > Signed-off-by: Brandon Williams 
> > ---
> >  transport.c | 26 +-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> When you are pushing 'master', we no longer hear what the other end
> has at 'next', with this change, no?
> 
> In a project whose 'master' is extended primarily by merging topics
> that have been cooking in 'next', old way of pushing would only have
> transferred the merge commits and resulting trees but not bulk of
> the blob data because they are all available on 'next', would it
> make the object transfer far less efficient, I wonder?
> 
> I guess it is OK only because the push side of the current protocol
> does not do common ancestor discovery exchange ;-)

Yep, though we've been throwing around ideas of adding that in push v2
after we figure out a good way to improve negotiation with fetch.

> 
> >
> > diff --git a/transport.c b/transport.c
> > index dfc603b36..bf7ba6879 100644
> > --- a/transport.c
> > +++ b/transport.c
> > @@ -1026,11 +1026,35 @@ int transport_push(struct transport *transport,
> > int porcelain = flags & TRANSPORT_PUSH_PORCELAIN;
> > int pretend = flags & TRANSPORT_PUSH_DRY_RUN;
> > int push_ret, ret, err;
> > +   struct refspec *tmp_rs;
> > +   struct argv_array ref_patterns = ARGV_ARRAY_INIT;
> > +   int i;
> >  
> > if (check_push_refs(local_refs, refspec_nr, refspec) < 0)
> > return -1;
> >  
> > -   remote_refs = transport->vtable->get_refs_list(transport, 1, 
> > NULL);
> > +   tmp_rs = parse_push_refspec(refspec_nr, refspec);
> > +   for (i = 0; i < refspec_nr; i++) {
> > +   const char *pattern = NULL;
> > +
> > +   if (tmp_rs[i].dst)
> > +   pattern = tmp_rs[i].dst;
> > +   else if (tmp_rs[i].src && !tmp_rs[i].exact_sha1)
> > +   pattern = tmp_rs[i].src;
> > +
> > +   if (pattern) {
> > +   if (tmp_rs[i].pattern)
> > +   argv_array_push(_patterns, pattern);
> > +   else
> > +   expand_ref_pattern(_patterns, 
> > pattern);
> > +   }
> > +   }
> > +
> > +   remote_refs = transport->vtable->get_refs_list(transport, 1,
> > +  _patterns);
> > +
> > +   argv_array_clear(_patterns);
> > +   free_refspec(refspec_nr, tmp_rs);
> >  
> > if (flags & TRANSPORT_PUSH_ALL)
> > match_flags |= MATCH_REFS_ALL;

-- 
Brandon Williams

Re: [PATCH v4 19/35] push: pass ref patterns when pushing

[Junio C Hamano]

Brandon Williams  writes:

> Construct a list of ref patterns to be passed to 'get_refs_list()' from
> the refspec to be used during the push.  This list of ref patterns will
> be used to allow the server to filter the ref advertisement when
> communicating using protocol v2.
>
> Signed-off-by: Brandon Williams 
> ---
>  transport.c | 26 +-
>  1 file changed, 25 insertions(+), 1 deletion(-)

When you are pushing 'master', we no longer hear what the other end
has at 'next', with this change, no?

In a project whose 'master' is extended primarily by merging topics
that have been cooking in 'next', old way of pushing would only have
transferred the merge commits and resulting trees but not bulk of
the blob data because they are all available on 'next', would it
make the object transfer far less efficient, I wonder?

I guess it is OK only because the push side of the current protocol
does not do common ancestor discovery exchange ;-)

>
> diff --git a/transport.c b/transport.c
> index dfc603b36..bf7ba6879 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -1026,11 +1026,35 @@ int transport_push(struct transport *transport,
>   int porcelain = flags & TRANSPORT_PUSH_PORCELAIN;
>   int pretend = flags & TRANSPORT_PUSH_DRY_RUN;
>   int push_ret, ret, err;
> + struct refspec *tmp_rs;
> + struct argv_array ref_patterns = ARGV_ARRAY_INIT;
> + int i;
>  
>   if (check_push_refs(local_refs, refspec_nr, refspec) < 0)
>   return -1;
>  
> - remote_refs = transport->vtable->get_refs_list(transport, 1, 
> NULL);
> + tmp_rs = parse_push_refspec(refspec_nr, refspec);
> + for (i = 0; i < refspec_nr; i++) {
> + const char *pattern = NULL;
> +
> + if (tmp_rs[i].dst)
> + pattern = tmp_rs[i].dst;
> + else if (tmp_rs[i].src && !tmp_rs[i].exact_sha1)
> + pattern = tmp_rs[i].src;
> +
> + if (pattern) {
> + if (tmp_rs[i].pattern)
> + argv_array_push(_patterns, pattern);
> + else
> + expand_ref_pattern(_patterns, 
> pattern);
> + }
> + }
> +
> + remote_refs = transport->vtable->get_refs_list(transport, 1,
> +_patterns);
> +
> + argv_array_clear(_patterns);
> + free_refspec(refspec_nr, tmp_rs);
>  
>   if (flags & TRANSPORT_PUSH_ALL)
>   match_flags |= MATCH_REFS_ALL;