Re: [WIP 06/15] transport: use get_refs_via_connect to get refs

[Brandon Williams]

On 12/06, Junio C Hamano wrote:
> Brandon Williams  writes:
> 
> > Remove code duplication and use the existing 'get_refs_via_connect()'
> > function to retrieve a remote's heads in 'fetch_refs_via_pack()' and
> > 'git_transport_push()'.
> >
> > Signed-off-by: Brandon Williams 
> > ---
> >  transport.c | 18 --
> >  1 file changed, 4 insertions(+), 14 deletions(-)
> >
> > diff --git a/transport.c b/transport.c
> > index d75ff0514..7c969f285 100644
> > --- a/transport.c
> > +++ b/transport.c
> > @@ -230,12 +230,8 @@ static int fetch_refs_via_pack(struct transport 
> > *transport,
> > args.cloning = transport->cloning;
> > args.update_shallow = data->options.update_shallow;
> >  
> > -   if (!data->got_remote_heads) {
> > -   connect_setup(transport, 0);
> > -   get_remote_heads(data->fd[0], NULL, 0, _tmp, 0,
> > -NULL, >shallow);
> > -   data->got_remote_heads = 1;
> > -   }
> > +   if (!data->got_remote_heads)
> > +   refs_tmp = get_refs_via_connect(transport, 0);
> 
> The updated version is equivalent to the original as long as
> transport->data->extra_have is empty at this point.  Were we
> deliberately sending NULL, instead of >extra_have, in the
> original, or is it a mere oversight?
> 
> The same comment applies to the other hunk of this patch.

extra_have is only ever used by the push logic, so they're shouldn't be
any harm is passing it through on the fetch side, especially since
upload-pack doesn't send .have lines.

The push side is what uses the .have lines.  From a quick look through
the code it seems like get_refs_via_connect is always called before
git_transport_push, so the extra check to make sure ref's have been
retrieved doesn't get executed.  But if it ever did get executed, we
would silently ignore a server's .have lines.

-- 
Brandon Williams

Re: [WIP 06/15] transport: use get_refs_via_connect to get refs

[Junio C Hamano]

Brandon Williams  writes:

> Remove code duplication and use the existing 'get_refs_via_connect()'
> function to retrieve a remote's heads in 'fetch_refs_via_pack()' and
> 'git_transport_push()'.
>
> Signed-off-by: Brandon Williams 
> ---
>  transport.c | 18 --
>  1 file changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/transport.c b/transport.c
> index d75ff0514..7c969f285 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -230,12 +230,8 @@ static int fetch_refs_via_pack(struct transport 
> *transport,
>   args.cloning = transport->cloning;
>   args.update_shallow = data->options.update_shallow;
>  
> - if (!data->got_remote_heads) {
> - connect_setup(transport, 0);
> - get_remote_heads(data->fd[0], NULL, 0, _tmp, 0,
> -  NULL, >shallow);
> - data->got_remote_heads = 1;
> - }
> + if (!data->got_remote_heads)
> + refs_tmp = get_refs_via_connect(transport, 0);

The updated version is equivalent to the original as long as
transport->data->extra_have is empty at this point.  Were we
deliberately sending NULL, instead of >extra_have, in the
original, or is it a mere oversight?

The same comment applies to the other hunk of this patch.

[WIP 06/15] transport: use get_refs_via_connect to get refs

[Brandon Williams]

Remove code duplication and use the existing 'get_refs_via_connect()'
function to retrieve a remote's heads in 'fetch_refs_via_pack()' and
'git_transport_push()'.

Signed-off-by: Brandon Williams 
---
 transport.c | 18 --
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/transport.c b/transport.c
index d75ff0514..7c969f285 100644
--- a/transport.c
+++ b/transport.c
@@ -230,12 +230,8 @@ static int fetch_refs_via_pack(struct transport *transport,
args.cloning = transport->cloning;
args.update_shallow = data->options.update_shallow;
 
-   if (!data->got_remote_heads) {
-   connect_setup(transport, 0);
-   get_remote_heads(data->fd[0], NULL, 0, _tmp, 0,
-NULL, >shallow);
-   data->got_remote_heads = 1;
-   }
+   if (!data->got_remote_heads)
+   refs_tmp = get_refs_via_connect(transport, 0);
 
refs = fetch_pack(, data->fd, data->conn,
  refs_tmp ? refs_tmp : transport->remote_refs,
@@ -542,14 +538,8 @@ static int git_transport_push(struct transport *transport, 
struct ref *remote_re
struct send_pack_args args;
int ret;
 
-   if (!data->got_remote_heads) {
-   struct ref *tmp_refs;
-   connect_setup(transport, 1);
-
-   get_remote_heads(data->fd[0], NULL, 0, _refs, REF_NORMAL,
-NULL, >shallow);
-   data->got_remote_heads = 1;
-   }
+   if (!data->got_remote_heads)
+   get_refs_via_connect(transport, 1);
 
memset(, 0, sizeof(args));
args.send_mirror = !!(flags & TRANSPORT_PUSH_MIRROR);
-- 
2.15.1.424.g9478a66081-goog