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

2015-03-17 Thread Duy Nguyen
On Mon, Mar 16, 2015 at 11:10 PM, Dongcan Jiang dongcan.ji...@gmail.com wrote:
 Hi Duy,

 Sorry for my late response.

 we need to make sure that upload-pack barf if some client sends both 
 deepen and
 depth.

 Actually, in my current design, when client just wants depth, it
 sends depth N;
 when it want deepen, it sends depth N as well as depth_deepen.
 For the latter
 case, it tells upload-pack to collect objects for deepen N.

 Thus, I'm not so sure whether upload-pack needs to check their exclusion.

C Git is not the only client that can talk to upload-pack. A buggy
client may send both. The least we need is make sure it would not
crash or break the server security in any way. But if we have to
consider that, we may as well reject with a clear message, which would
help the client writer. And speaking of clients..

On Mon, Mar 16, 2015 at 11:08 PM, Dongcan Jiang dongcan.ji...@gmail.com wrote:
 - 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).

You missed Junio's keyword existing. Your new client will work well
with your new server. But it breaks git clone --depth (or git fetch
--depth) for all existing git installations.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-17 Thread Dongcan Jiang
 C Git is not the only client that can talk to upload-pack. A buggy
 client may send both. The least we need is make sure it would not
 crash or break the server security in any way. But if we have to
 consider that, we may as well reject with a clear message, which would
 help the client writer. And speaking of clients..

Actually, I mean that the upload-pack in this patch will not crash,
whatever it receives.
e.g. depth N, depth_deepen, depth N+depth_deepen

Because depth_deepen is just a boolean signal to tell the upload-pack
that depth N means deepen N, it takes effect only when depth N
is given. Otherwise, depth_deepen will be ignored.


 You missed Junio's keyword existing. Your new client will work well
 with your new server. But it breaks git clone --depth (or git fetch
 --depth) for all existing git installations.

Oh, thank you for pointing out this. And sorry Junio. I misunderstood
what you said. I haven't realized the problem of existing git server.
You've reminded me of the importance of compatibility. Thank you!

2015-03-17 18:44 GMT+08:00 Duy Nguyen pclo...@gmail.com:
 On Mon, Mar 16, 2015 at 11:10 PM, Dongcan Jiang dongcan.ji...@gmail.com 
 wrote:
 Hi Duy,

 Sorry for my late response.

 we need to make sure that upload-pack barf if some client sends both 
 deepen and
 depth.

 Actually, in my current design, when client just wants depth, it
 sends depth N;
 when it want deepen, it sends depth N as well as depth_deepen.
 For the latter
 case, it tells upload-pack to collect objects for deepen N.

 Thus, I'm not so sure whether upload-pack needs to check their exclusion.

 C Git is not the only client that can talk to upload-pack. A buggy
 client may send both. The least we need is make sure it would not
 crash or break the server security in any way. But if we have to
 consider that, we may as well reject with a clear message, which would
 help the client writer. And speaking of clients..

 On Mon, Mar 16, 2015 at 11:08 PM, Dongcan Jiang dongcan.ji...@gmail.com 
 wrote:
 - 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).

 You missed Junio's keyword existing. Your new client will work well
 with your new server. But it breaks git clone --depth (or git fetch
 --depth) for all existing git installations.
 --
 Duy



-- 
江东灿(Dongcan Jiang)
Team of Search Engine  Web Mining
School of Electronic Engineering  Computer Science
Peking University, Beijing, 100871, P.R.China
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-16 Thread Dongcan Jiang
Hi Duy,

Sorry for my late response.

 we need to make sure that upload-pack barf if some client sends both deepen 
 and
 depth.

Actually, in my current design, when client just wants depth, it
sends depth N;
when it want deepen, it sends depth N as well as depth_deepen.
For the latter
case, it tells upload-pack to collect objects for deepen N.

Thus, I'm not so sure whether upload-pack needs to check their exclusion.

 I was about to suggest you use deepen for both --depth and
 --deepen: --depth sends deepen NUM while --deepen sends deepen
 +NUM. The protocol as described in pack-protocol.txt may allow this
 extension.

This suggestion looks neat! However, I'm afraid it may involves quite
a bit changes
as you've mentioned, which would be better to take in next stage.

Thanks,
Dongcan

2015-03-14 18:56 GMT+08:00 Duy Nguyen pclo...@gmail.com:
 On Fri, Mar 13, 2015 at 8:04 PM, Dongcan Jiang dongcan.ji...@gmail.com 
 wrote:
 @@ -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;

 Junio already questioned about your replacing starts_with(deepen ..)
 with starts_with(depth ..), this is part of that question. If you
 run make test you should find some breakages, or we need to improve
 our test suite.

 I think you just need to keep the old if (args-depth  0) send
 depth unchanged and add two new statements that check
 args-depth_deepen and sends depth . BTW, I think deepen-more or
 deepen-relative would be better than depth (*), which is a noun.
 But that's a minor point at this stage.

 And because --depth and --deepen are mutually exclusive, you need to
 make sure that the user must not specify that. The user includes the
 client from the server perspective, we need to make sure that
 upload-pack barf if some client sends both deepen and depth.

 (*) I was about to suggest you use deepen for both --depth and
 --deepen: --depth sends deepen NUM while --deepen sends deepen
 +NUM. The protocol as described in pack-protocol.txt may allow this
 extension. Unfortunately the current implementation is too relaxed. We
 use strtol() to parse NUM and it would accept the leading +
 instead of barfing out. So old clients would mistakes --deepen as
 --depth, not good. And it even accepts base 8 and 16!! We should fix
 this.
 --
 Duy



-- 
江东灿(Dongcan Jiang)
Team of Search Engine  Web Mining
School of Electronic Engineering  Computer Science
Peking University, Beijing, 100871, P.R.China
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-16 Thread Dongcan Jiang
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 gits...@pobox.com:
 Dongcan Jiang dongcan.ji...@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)
 -  

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

2015-03-16 Thread Dongcan Jiang
Hi Eric,

Sorry for my late response. Thank you for your suggestions! I will try
to use them in my next patch version.

Best Regards,
Dongcan

2015-03-14 3:42 GMT+08:00 Eric Sunshine sunsh...@sunshineco.com:
 On Fri, Mar 13, 2015 at 9:04 AM, Dongcan Jiang dongcan.ji...@gmail.com 
 wrote:
 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]
 [...]
 Of course, as a patch for discussion, it remains a long way to go
 before being complete.
 [...]
 Signed-off-by: Dongcan Jiang dongcan.ji...@gmail.com
 ---
 diff --git a/fetch-pack.c b/fetch-pack.c
 index 655ee64..6f4adca 100644
 --- a/fetch-pack.c
 +++ b/fetch-pack.c
 @@ -295,6 +295,7 @@ static int find_common(struct fetch_pack_args *args,
 if (no_done)strbuf_addstr(c,  
 no-done);
 if (use_sideband == 2)  strbuf_addstr(c,  
 side-band-64k);
 if (use_sideband == 1)  strbuf_addstr(c,  
 side-band);
 +   if (args-depth_deepen)  strbuf_addstr(c,  
 depth_deepen);

 For consistency, should this be depth-deepen rather than depth_deepen?

 if (args-use_thin_pack) strbuf_addstr(c,  
 thin-pack);
 if (args-no_progress)   strbuf_addstr(c,  
 no-progress);
 if (args-include_tag)   strbuf_addstr(c,  
 include-tag);
 diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
 index d78f320..6738006 100755
 --- a/t/t5510-fetch.sh
 +++ b/t/t5510-fetch.sh
 @@ -708,4 +708,15 @@ test_expect_success 'fetching a one-level ref works' '
 )
  '

 +test_expect_success 'fetching deepen' '
 +   git clone . deepen --depth=1 
 +   cd deepen 

 When this tests ends, the working directory will still be 'deepen',
 which will likely break tests added after this one. If you wrap the
 'cd' and following statements in a subshell via '(' and ')', then the
 'cd' will affect the subshell but leave the parent shell's working
 directory alone, and thus not negatively impact subsequent tests.

 +   git fetch .. foo --depth=1
 +   git show foo
 +   test_must_fail git show foo~
 +   git fetch .. foo --depth=1 --deepen
 +   git show foo~
 +   test_must_fail git show foo~2

 Broken -chain throughout.

 +'
 +
  test_done



-- 
江东灿(Dongcan Jiang)
Team of Search Engine  Web Mining
School of Electronic Engineering  Computer Science
Peking University, Beijing, 100871, P.R.China
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-14 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Sat, Mar 14, 2015 at 12:35 PM, Junio C Hamano gits...@pobox.com wrote:
 Dongcan Jiang dongcan.ji...@gmail.com writes:
 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.

 That would make it harder to implement fetch --deepen than the
 version that moves refs if they are updated.

The comment you are responding to was in the context of What does
the illustrated 'your' history mean?  Is it a history of a single
ref (if so in which repository)? to clarify that the example was
not fetching _new_ history (and with the RFC/POC design that was
posted, my understanding is that deepen was meant to only deepen).

The paragraph you are reacting to is not an endorsement for that
only deepen, never advance design. It merely is a clarification of
the explanation that was in the paragraph that follows.

 And I think what Dongcan
 implemented moves refs. From the user point of view, I think it's ok
 with either version, so the one that's easier to implement wins.

 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.

 [1] http://article.gmane.org/gmane.comp.version-control.git/212950
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-14 Thread Duy Nguyen
On Sat, Mar 14, 2015 at 12:35 PM, Junio C Hamano gits...@pobox.com wrote:
 Dongcan Jiang dongcan.ji...@gmail.com writes:
 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.

That would make it harder to implement fetch --deepen than the
version that moves refs if they are updated. And I think what Dongcan
implemented moves refs. From the user point of view, I think it's ok
with either version, so the one that's easier to implement wins.

 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.

 [1] http://article.gmane.org/gmane.comp.version-control.git/212950
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-14 Thread Duy Nguyen
On Fri, Mar 13, 2015 at 8:04 PM, Dongcan Jiang dongcan.ji...@gmail.com wrote:
 @@ -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;

Junio already questioned about your replacing starts_with(deepen ..)
with starts_with(depth ..), this is part of that question. If you
run make test you should find some breakages, or we need to improve
our test suite.

I think you just need to keep the old if (args-depth  0) send
depth unchanged and add two new statements that check
args-depth_deepen and sends depth . BTW, I think deepen-more or
deepen-relative would be better than depth (*), which is a noun.
But that's a minor point at this stage.

And because --depth and --deepen are mutually exclusive, you need to
make sure that the user must not specify that. The user includes the
client from the server perspective, we need to make sure that
upload-pack barf if some client sends both deepen and depth.

(*) I was about to suggest you use deepen for both --depth and
--deepen: --depth sends deepen NUM while --deepen sends deepen
+NUM. The protocol as described in pack-protocol.txt may allow this
extension. Unfortunately the current implementation is too relaxed. We
use strtol() to parse NUM and it would accept the leading +
instead of barfing out. So old clients would mistakes --deepen as
--depth, not good. And it even accepts base 8 and 16!! We should fix
this.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-13 Thread Eric Sunshine
On Fri, Mar 13, 2015 at 9:04 AM, Dongcan Jiang dongcan.ji...@gmail.com wrote:
 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]
 [...]
 Of course, as a patch for discussion, it remains a long way to go
 before being complete.
 [...]
 Signed-off-by: Dongcan Jiang dongcan.ji...@gmail.com
 ---
 diff --git a/fetch-pack.c b/fetch-pack.c
 index 655ee64..6f4adca 100644
 --- a/fetch-pack.c
 +++ b/fetch-pack.c
 @@ -295,6 +295,7 @@ static int find_common(struct fetch_pack_args *args,
 if (no_done)strbuf_addstr(c,  no-done);
 if (use_sideband == 2)  strbuf_addstr(c,  
 side-band-64k);
 if (use_sideband == 1)  strbuf_addstr(c,  
 side-band);
 +   if (args-depth_deepen)  strbuf_addstr(c,  
 depth_deepen);

For consistency, should this be depth-deepen rather than depth_deepen?

 if (args-use_thin_pack) strbuf_addstr(c,  
 thin-pack);
 if (args-no_progress)   strbuf_addstr(c,  
 no-progress);
 if (args-include_tag)   strbuf_addstr(c,  
 include-tag);
 diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
 index d78f320..6738006 100755
 --- a/t/t5510-fetch.sh
 +++ b/t/t5510-fetch.sh
 @@ -708,4 +708,15 @@ test_expect_success 'fetching a one-level ref works' '
 )
  '

 +test_expect_success 'fetching deepen' '
 +   git clone . deepen --depth=1 
 +   cd deepen 

When this tests ends, the working directory will still be 'deepen',
which will likely break tests added after this one. If you wrap the
'cd' and following statements in a subshell via '(' and ')', then the
'cd' will affect the subshell but leave the parent shell's working
directory alone, and thus not negatively impact subsequent tests.

 +   git fetch .. foo --depth=1
 +   git show foo
 +   test_must_fail git show foo~
 +   git fetch .. foo --depth=1 --deepen
 +   git show foo~
 +   test_must_fail git show foo~2

Broken -chain throughout.

 +'
 +
  test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-13 Thread Junio C Hamano
Dongcan Jiang dongcan.ji...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-03-13 Thread Dongcan Jiang
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?

I hope you could give me some comments and suggestions on this
patch. I would like to know whether I'm off the right way.

[1] http://article.gmane.org/gmane.comp.version-control.git/212950
[2] http://article.gmane.org/gmane.comp.version-control.git/265050

Helped-by: Duy Nguyen pclo...@gmail.com
Signed-off-by: Dongcan Jiang dongcan.ji...@gmail.com
---
 builtin/fetch.c  |  5 -
 fetch-pack.c |  3 ++-
 fetch-pack.h |  1 +
 t/t5510-fetch.sh | 11 +++
 transport.c  |  4 
 transport.h  |  4 
 upload-pack.c| 15 +++
 7 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index f951265..6861207 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -33,7 +33,7 @@ static int fetch_prune_config = -1; /* unspecified */
 static int prune = -1; /* unspecified */
 #define PRUNE_BY_DEFAULT 0 /* do we prune by default? */

-static int all, append, dry_run, force, keep, multiple, update_head_ok, 
verbosity;
+static int all, append, dry_run, force, keep, multiple, update_head_ok, 
verbosity, deepen;
 static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static int tags = TAGS_DEFAULT, unshallow, update_shallow;
 static const char *depth;
@@ -111,6 +111,7 @@ static struct option builtin_fetch_options[] = {
OPT_BOOL(0, progress, progress, N_(force progress reporting)),
OPT_STRING(0, depth, depth, N_(depth),
   N_(deepen history of shallow clone)),
+   OPT_BOOL(0, deepen, deepen, N_(deepen)),
{ OPTION_SET_INT, 0, unshallow, unshallow, NULL,
   N_(convert to a complete repository),
   PARSE_OPT_NONEG | PARSE_OPT_NOARG, NULL, 1 },
@@ -853,6 +854,8 @@ static struct transport *prepare_transport(struct remote 
*remote)
set_option(transport, TRANS_OPT_KEEP, yes);
if (depth)
set_option(transport, TRANS_OPT_DEPTH, depth);
+   if (deepen)
+   set_option(transport, TRANS_OPT_DEEPEN, yes);
if (update_shallow)
set_option(transport, TRANS_OPT_UPDATE_SHALLOW, yes);
return transport;
diff --git a/fetch-pack.c b/fetch-pack.c
index 655ee64..6f4adca 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -295,6 +295,7 @@ static int find_common(struct fetch_pack_args *args,
if (no_done)strbuf_addstr(c,  no-done);
if (use_sideband == 2)  strbuf_addstr(c,  
side-band-64k);
if (use_sideband == 1)  strbuf_addstr(c,  side-band);
+   if (args-depth_deepen)  strbuf_addstr(c,  
depth_deepen);
if (args-use_thin_pack) strbuf_addstr(c,  
thin-pack);
if (args-no_progress)   strbuf_addstr(c,  
no-progress);
if (args-include_tag)   strbuf_addstr(c,  
include-tag);
@@ -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;

diff --git a/fetch-pack.h b/fetch-pack.h
index bb7fd76..200ac78 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -10,6 +10,7 @@ struct fetch_pack_args {
const char *uploadpack;
int unpacklimit;
int depth;
+   unsigned depth_deepen:1;
unsigned quiet:1;
unsigned keep_pack:1;
unsigned lock_pack:1;
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index d78f320..6738006 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -708,4 +708,15 @@