Re: [PATCH/GSoC/RFC] fetch: git fetch --deepen

[Dongcan Jiang <dongcan.jiang@gmail.com>]

Hi Junio,

Sorry for my late response.

> As this operation is not about moving _any_ refs, whether local
> branches or remote-tracking branches, any ref that used to point at
> commit B before you executed "fetch --deepen" would point at the
> same commit after the command finishes.

Thanks for clarifying the definition of "you". In this patch, it
actually would update the remote-tracking branches to the newest history.
I will keep working on it to figure out the reason.

It looks that it would be more efficient if the new history is not fetched,
as it transports less objects. Is this your main consideration on not
updating any refs?

>> -             if (starts_with(line, "deepen ")) {
>> +             if (starts_with(line, "depth ")) {
>>                       char *end;
>> -                     depth = strtol(line + 7, &end, 0);
>> -                     if (end == line + 7 || depth <= 0)
>> -                             die("Invalid deepen: %s", line);
>> +                     depth = strtol(line + 6, &end, 0);
>> +                     if (end == line + 6 || depth <= 0)
>> +                             die("Invalid depth: %s", line);
>>                       continue;
>>               }
>>               if (!starts_with(line, "want ") ||
>
> I do not quite understand this hunk.  What happens when this program
> is talking to an existing fetch-pack that requested "deepen"?

In this hunk, I actually just changed the one command name in upload-pack
service from "deepen" to "depth". I made this change because the name
"deepen" here is quite misleading, as it just means "depth". Of course,
I've changed the caller's sent pack from "deepen" to "depth" too (See below).

> @@ -317,7 +318,7 @@ static int find_common(struct fetch_pack_args *args,
>         if (is_repository_shallow())
>                 write_shallow_commits(&req_buf, 1, NULL);
>         if (args->depth > 0)
> -               packet_buf_write(&req_buf, "deepen %d", args->depth);
> +               packet_buf_write(&req_buf, "depth %d", args->depth);
>         packet_buf_flush(&req_buf);
>         state_len = req_buf.len;

If the user wants "deepen", he should send a "depth_deepen" signal as well as
"depth N". When the upload-pack service in this patch receive "depth_deepen"
and "depth N", it collects N more commits ahead of the shallow commit and send
back to the caller.

Thanks,
Dongcan

2015-03-14 13:35 GMT+08:00 Junio C Hamano <gitster@pobox.com>:
> Dongcan Jiang <dongcan.jiang@gmail.com> writes:
>
>> This patch is just for discusstion. An option --deepen is added to
>> 'git fetch'. When it comes to '--deepen', git should fetch N more
>> commits ahead the local shallow commit, where N is indicated by
>> '--depth=N'. [1]
>>
>> e.g.
>>
>>>  (upstream)
>>>   ---o---o---o---A---B
>>>
>>>  (you)
>>>                  A---B
>>
>> After excuting "git fetch --depth=1 --deepen", (you) get one more
>> tip and it becomes
>>
>>>  (you)
>>>              o---A---B
>>
>> '--deepen' is designed to be a boolean option in this patch, which
>> is a little different from [1]. It's designed in this way, because
>> it can reuse '--depth' in the program, and just costs one more bit
>> in some data structure, such as fetch_pack_args,
>> git_transport_options.
>>
>> Of course, as a patch for discussion, it remains a long way to go
>> before being complete.
>>
>>       1) Documents should be completed.
>>       2) More test cases, expecially corner cases, should be added.
>>       3) No need to get remote refs when it comes to '--deepen' option.
>>       4) Validity on options combination should be checked.
>>       5) smart-http protocol remains to be supported. [2]
>>
>> Besides, I still have one question:
>> What does (you) exactly mean in [1]? The local branch or the local
>> remote ref?
>
> As this operation is not about moving _any_ refs, whether local
> branches or remote-tracking branches, any ref that used to point at
> commit B before you executed "fetch --deepen" would point at the
> same commit after the command finishes.
>
> The "you" does not explicitly depict any ref, because the picture is
> meant to illustrate _everything_ the repository at the receiving end
> of "fetch" has.  It used to have two commits, A and B (and the tree
> and blob objects necessary to complete these two commits).  After
> deepening the history by one commit, it then will have commit A^ and
> its trees and blobs.
>
>> diff --git a/upload-pack.c b/upload-pack.c
>> index b531a32..8004f2f 100644
>> --- a/upload-pack.c
>> +++ b/upload-pack.c
>> @@ -31,6 +31,7 @@ static const char upload_pack_usage[] = "git upload-pack [--strict] [--timeout=<
>>
>>  static unsigned long oldest_have;
>>
>> +static int depth_deepen;
>>  static int multi_ack;
>>  static int no_done;
>>  static int use_thin_pack, use_ofs_delta, use_include_tag;
>> @@ -563,11 +564,11 @@ static void receive_needs(void)
>>                       }
>>                       continue;
>>               }
>> -             if (starts_with(line, "deepen ")) {
>> +             if (starts_with(line, "depth ")) {
>>                       char *end;
>> -                     depth = strtol(line + 7, &end, 0);
>> -                     if (end == line + 7 || depth <= 0)
>> -                             die("Invalid deepen: %s", line);
>> +                     depth = strtol(line + 6, &end, 0);
>> +                     if (end == line + 6 || depth <= 0)
>> +                             die("Invalid depth: %s", line);
>>                       continue;
>>               }
>>               if (!starts_with(line, "want ") ||
>
> I do not quite understand this hunk.  What happens when this program
> is talking to an existing fetch-pack that requested "deepen"?
>
>> @@ -577,6 +578,8 @@ static void receive_needs(void)
>>
>>               features = line + 45;
>>
>> +             if (parse_feature_request(features, "depth_deepen"))
>> +                     depth_deepen = 1;
>>               if (parse_feature_request(features, "multi_ack_detailed"))
>>                       multi_ack = 2;
>>               else if (parse_feature_request(features, "multi_ack"))
>> @@ -631,6 +634,10 @@ static void receive_needs(void)
>>                               struct object *object = shallows.objects[i].item;
>>                               object->flags |= NOT_SHALLOW;
>>                       }
>> +             else if (depth_deepen)
>> +                     backup = result =
>> +                             get_shallow_commits(&shallows, depth + 1,
>> +                                                 SHALLOW, NOT_SHALLOW);
>>               else
>>                       backup = result =
>>                               get_shallow_commits(&want_obj, depth,
>> --
>> 2.3.1.253.gb3fd89e



-- 
江东灿（Dongcan Jiang）
Team of Search Engine & Web Mining
School of Electronic Engineering & Computer Science
Peking University, Beijing, 100871, P.R.China