[PATCH] [GSoC][MICRO] Forbid "log --graph --no-walk"

2015-03-06 Thread Dongcan Jiang 
Because --graph is about connected history while --no-walk is about discrete 
points.

revision.c: Judge whether --graph and --no-walk come together when running 
git-log.
buildin/log.c: Set git-log cmd flag.
Documentation/rev-list-options.txt: Add specification on the forbidden usage.

Signed-off-by: Dongcan Jiang 
---
 Documentation/rev-list-options.txt | 2 ++
 builtin/log.c  | 1 +
 revision.c | 4 
 revision.h | 3 +++
 4 files changed, 10 insertions(+)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 4ed8587..eea2c0a 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -679,6 +679,7 @@ endif::git-rev-list[]
given on the command line. Otherwise (if `sorted` or no argument
was given), the commits are shown in reverse chronological order
by commit time.
+   Cannot be combined with `--graph` when running git-log.
 
 --do-walk::
Overrides a previous `--no-walk`.
@@ -781,6 +782,7 @@ you would get an output like this:
on the left hand side of the output.  This may cause extra lines
to be printed in between commits, in order for the graph history
to be drawn properly.
+   Cannot be combined with `--no-walk` when running git-log.
 +
 This enables parent rewriting, see 'History Simplification' below.
 +
diff --git a/builtin/log.c b/builtin/log.c
index dd8f3fc..7bf5adb 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -627,6 +627,7 @@ int cmd_log(int argc, const char **argv, const char *prefix)
git_config(git_log_config, NULL);
 
init_revisions(&rev, prefix);
+   rev.cmd_is_log = 1;
rev.always_show_header = 1;
memset(&opt, 0, sizeof(opt));
opt.def = "HEAD";
diff --git a/revision.c b/revision.c
index 66520c6..5f62c89 100644
--- a/revision.c
+++ b/revision.c
@@ -1399,6 +1399,8 @@ void init_revisions(struct rev_info *revs, const char 
*prefix)
 
revs->commit_format = CMIT_FMT_DEFAULT;
 
+   revs->cmd_is_log = 0;
+
init_grep_defaults();
grep_init(&revs->grep_filter, prefix);
revs->grep_filter.status_only = 1;
@@ -2339,6 +2341,8 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s
 
if (revs->reflog_info && revs->graph)
die("cannot combine --walk-reflogs with --graph");
+   if (revs->no_walk && revs->graph && revs->cmd_is_log)
+   die("cannot combine --no-walk with --graph when running 
git-log");
if (!revs->reflog_info && revs->grep_filter.use_reflog_filter)
die("cannot use --grep-reflog without --walk-reflogs");
 
diff --git a/revision.h b/revision.h
index 0ea8b4e..255982a 100644
--- a/revision.h
+++ b/revision.h
@@ -146,6 +146,9 @@ struct rev_info {
track_first_time:1,
linear:1;
 
+   /* cmd type */
+   unsigned int  cmd_is_log:1;
+
enum date_mode date_mode;
 
unsigned intabbrev;
-- 
2.3.1.251.g83036f8

--
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][MICRO] Forbid "log --graph --no-walk"

2015-03-06 Thread Dongcan Jiang 
Hi, Eric and René

Thanks for your suggestions. Good ideas!

> Genuine question: Despite the GSoC micro-project mentioning only
> 'log', is it ever meaningful for these two options to be specified
> together? I suspect not, but it would be nice to hear from someone
> more familiar with the issue. If not specific to 'log', then the > patch
> can be simplified a good deal.

> Why only for git log?  Doesn't the justification given in the
> commit message above apply in general?

At first, I also tried to only judge the value of "revs->no_walk &&
revs->graph", but unfortunately, it failed to pass all cases in
t4052-stat-output.sh.
e.g. command "git show --stat --graph" failed to get the correct result.

Finally, this is because that "revs->no_walk" gets set when it comes
to "git show --stat". That's why I add the parameter
"revs->cmd_is_log" as judgement. Of course, it causes the limitation
you've mentioned. I will consider better solution in the next patch
edition as soon as possible.

Best Regards

Dongcan

2015-03-06 17:56 GMT+08:00 Eric Sunshine :
> On Fri, Mar 6, 2015 at 3:55 AM, Dongcan Jiang  wrote:
>> Forbid "log --graph --no-walk
>
> Style: drop capitalization in the Subject: line. Also prefix with the
> command or module being modified, followed by a colon. So:
>
> log: forbid combining --graph and --no-walk
>
> or:
>
> revision: forbid combining --graph and --no-walk
>
>> Because --graph is about connected history while --no-walk is about discrete 
>> points.
>
> Okay. You might also want to cite the wider discussion[1].
>
> [1]: http://article.gmane.org/gmane.comp.version-control.git/216083
>
>> revision.c: Judge whether --graph and --no-walk come together when running 
>> git-log.
>> buildin/log.c: Set git-log cmd flag.
>> Documentation/rev-list-options.txt: Add specification on the forbidden usage.
>
> No need to repeat in prose what the patch itself states more clearly
> and concisely.
>
> Also, such a change should be accompanied by new test(s).
>
>> Signed-off-by: Dongcan Jiang 
>> ---
>> diff --git a/Documentation/rev-list-options.txt 
>> b/Documentation/rev-list-options.txt
>> index 4ed8587..eea2c0a 100644
>> --- a/Documentation/rev-list-options.txt
>> +++ b/Documentation/rev-list-options.txt
>> @@ -679,6 +679,7 @@ endif::git-rev-list[]
>> given on the command line. Otherwise (if `sorted` or no argument
>> was given), the commits are shown in reverse chronological order
>> by commit time.
>> +   Cannot be combined with `--graph` when running git-log.
>>
>>  --do-walk::
>> Overrides a previous `--no-walk`.
>> @@ -781,6 +782,7 @@ you would get an output like this:
>> on the left hand side of the output.  This may cause extra lines
>> to be printed in between commits, in order for the graph history
>> to be drawn properly.
>> +   Cannot be combined with `--no-walk` when running git-log.
>
> Nice to see documentation updates. More below.
>
>>  This enables parent rewriting, see 'History Simplification' below.
>>  +
>> diff --git a/builtin/log.c b/builtin/log.c
>> index dd8f3fc..7bf5adb 100644
>> --- a/builtin/log.c
>> +++ b/builtin/log.c
>> @@ -627,6 +627,7 @@ int cmd_log(int argc, const char **argv, const char 
>> *prefix)
>> git_config(git_log_config, NULL);
>>
>> init_revisions(&rev, prefix);
>> +   rev.cmd_is_log = 1;
>> rev.always_show_header = 1;
>> memset(&opt, 0, sizeof(opt));
>> opt.def = "HEAD";
>> diff --git a/revision.c b/revision.c
>> index 66520c6..5f62c89 100644
>> --- a/revision.c
>> +++ b/revision.c
>> @@ -1399,6 +1399,8 @@ void init_revisions(struct rev_info *revs, const char 
>> *prefix)
>>
>> revs->commit_format = CMIT_FMT_DEFAULT;
>>
>> +   revs->cmd_is_log = 0;
>> +
>> init_grep_defaults();
>> grep_init(&revs->grep_filter, prefix);
>> revs->grep_filter.status_only = 1;
>> @@ -2339,6 +2341,8 @@ int setup_revisions(int argc, const char **argv, 
>> struct rev_info *revs, struct s
>>
>> if (revs->reflog_info && revs->graph)
>> die("cannot combine --walk-reflogs with --graph");
>> +   if (revs->no_walk && revs->graph && revs->cmd_is_log)
>
> Placing 'revs->cmd_is_log' first would make it clear at a glance that
>

[PATCH v2/GSoC/MICRO] revision: forbid combining --graph and --no-walk

2015-03-06 Thread Dongcan Jiang 
Because --graph is about connected history while
--no-walk is about discrete points. [1]

It's a pity that git-show has to allow such combination
in order to make t4052-stat-output.sh compatible. [2]

2 testcases have been added to test this patch.

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

Signed-off-by: Dongcan Jiang 

Thanks-to: Eric Sunshine, René Scharfe, Junio C Hamano
---
 Documentation/rev-list-options.txt | 2 ++
 builtin/log.c  | 1 +
 revision.c | 4 
 revision.h | 3 +++
 t/t4202-log.sh | 6 ++
 t/t6014-rev-list-all.sh| 6 ++
 6 files changed, 22 insertions(+)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 4ed8587..136abdf 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -679,6 +679,7 @@ endif::git-rev-list[]
given on the command line. Otherwise (if `sorted` or no argument
was given), the commits are shown in reverse chronological order
by commit time.
+   Cannot be combined with `--graph`.

 --do-walk::
Overrides a previous `--no-walk`.
@@ -781,6 +782,7 @@ you would get an output like this:
on the left hand side of the output.  This may cause extra lines
to be printed in between commits, in order for the graph history
to be drawn properly.
+   Cannot be combined with `--no-walk`.
 +
 This enables parent rewriting, see 'History Simplification' below.
 +
diff --git a/builtin/log.c b/builtin/log.c
index dd8f3fc..5b5d028 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -524,6 +524,7 @@ int cmd_show(int argc, const char **argv, const char 
*prefix)

memset(&match_all, 0, sizeof(match_all));
init_revisions(&rev, prefix);
+   rev.cmd_is_show = 1;
rev.diff = 1;
rev.always_show_header = 1;
rev.no_walk = REVISION_WALK_NO_WALK_SORTED;
diff --git a/revision.c b/revision.c
index 66520c6..5d6fbef 100644
--- a/revision.c
+++ b/revision.c
@@ -1399,6 +1399,8 @@ void init_revisions(struct rev_info *revs, const char 
*prefix)

revs->commit_format = CMIT_FMT_DEFAULT;

+   revs->cmd_is_show = 0;
+
init_grep_defaults();
grep_init(&revs->grep_filter, prefix);
revs->grep_filter.status_only = 1;
@@ -2339,6 +2341,8 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s

if (revs->reflog_info && revs->graph)
die("cannot combine --walk-reflogs with --graph");
+   if (!revs->cmd_is_show && revs->no_walk && revs->graph)
+   die("cannot combine --no-walk with --graph");
if (!revs->reflog_info && revs->grep_filter.use_reflog_filter)
die("cannot use --grep-reflog without --walk-reflogs");

diff --git a/revision.h b/revision.h
index 0ea8b4e..378c3bf 100644
--- a/revision.h
+++ b/revision.h
@@ -146,6 +146,9 @@ struct rev_info {
track_first_time:1,
linear:1;

+   /* cmd type */
+   unsigned int  cmd_is_show:1;
+
enum date_mode date_mode;

unsigned intabbrev;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 5f2b290..fed162e 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -887,4 +887,10 @@ test_expect_success GPG 'log --graph --show-signature for 
merged tag' '
grep "^| | gpg: Good signature" actual
 '

+test_expect_success 'log --graph --no-walk is forbidden' '
+   echo "fatal: cannot combine --no-walk with --graph" >expect-error &&
+   test_must_fail git log --graph --no-walk 2>error &&
+   test_cmp expect-error error
+'
+
 test_done
diff --git a/t/t6014-rev-list-all.sh b/t/t6014-rev-list-all.sh
index 991ab4a..574e8d9 100755
--- a/t/t6014-rev-list-all.sh
+++ b/t/t6014-rev-list-all.sh
@@ -35,4 +35,10 @@ test_expect_success 'repack does not lose detached HEAD' '

 '

+test_expect_success 'rev-list --graph --no-walk is forbidden' '
+   echo "fatal: cannot combine --no-walk with --graph" >expect-error &&
+   test_must_fail git rev-list --graph --no-walk 2>error &&
+   test_cmp expect-error error
+'
+
 test_done
--
2.3.1.253.g3de5837

--
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

[GSoC/RFC] Ideas on git fetch --deepen

2015-03-06 Thread Dongcan Jiang 
Hi all,

My name is Dongcan Jiang. I am studying for my Master Degree at Peking
University
majoring in Computer Science. I have been using Git to manage my projects
for about half a year. It's really exciting that Git has been helping me make
revision control much more convenient. Therefore, I am very interested in
doing some works for Git in GSoC 2015.

I have submitted my v2 patch on a microproject recently, and I am waiting
for comments on v2 now.

In the meantime, I have been scanning ideas on the git gsoc page and
their related mails. I find that I am interested in most of them, especially
"git fetch --deepen" idea.

Here is my understanding about this idea.

Although "deepen" and "depth" have different behavior, "deepen" can be
achieved by "depth" with some extra calculation. If we know the
distance between "my history bottom" and "your tips", we can set the sum
of "deepen" step and this distance as "depth" step. Then we can reuse
the logics of processing "depth" to complete it.

Take graph in [1] as an example.

>  (upstream)
>   ---o---o---o---A---B
>
>  (you)
>  A---B

the distance of "my history bottom"(A) and "your tips"(B) is 1, then
"git fetch --deepen=3" can be achieved by "git fetch --depth=4"

However, I am a little worried about whether this idea is enough for a
GSoC project. I hope you can give me some comments and suggestions.

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

Thanks,
Dongcan

-- 
江东灿（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: [GSoC/RFC] Ideas on git fetch --deepen

2015-03-08 Thread Dongcan Jiang 
Hi, all

After digging into how "git fetch" works, I have found that my previous
understanding was too rash. I'm sorry for that.

I find that the current workflow of "git fetch --depth" is as follows:

  1. 'fetch module' calls 'git-upload-pack service' via 'transport module'
 to fetch ref with `depth`.
  2. Such call is finished by pipe I/O.
  3. git-upload-pack receives arguments such as 'depth' to fetch commits,
 and send back to the caller.

Therefore, if we want to implement "--deepen" for "git fetch", we have to
modify the arguments protocol of git-upload-pack service by adding the
'shallow commit' hash.
Then, we can fetch `depth` commits before the 'shallow commit' in
git-upload-pack service.
Apparently, we have to output error message when the 'shallow commit' hash
is not in the repository.

However, I still have a question for [1]. I am not quite following this:

> If you want the new history leading to the updated tip,
> you can just say:
>
>git fetch
>
> without any --depth nor --deepen option to end up with:
>
>  (you)
>  A---B---C---D---E---F---...---W---X---Y---Z

As far as I know, git fetch --depth would fetch new history to the
local remotes' refs.
Does it mean that we have to change the behavior of "git fetch --depth"?

I hope you can give me some comments or suggestions, letting me know
whether I am in the right way.

Thanks.

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

2015-03-07 14:32 GMT+08:00 Dongcan Jiang :
> Hi all,
>
> My name is Dongcan Jiang. I am studying for my Master Degree at Peking
> University
> majoring in Computer Science. I have been using Git to manage my projects
> for about half a year. It's really exciting that Git has been helping me make
> revision control much more convenient. Therefore, I am very interested in
> doing some works for Git in GSoC 2015.
>
> I have submitted my v2 patch on a microproject recently, and I am waiting
> for comments on v2 now.
>
> In the meantime, I have been scanning ideas on the git gsoc page and
> their related mails. I find that I am interested in most of them, especially
> "git fetch --deepen" idea.
>
> Here is my understanding about this idea.
>
> Although "deepen" and "depth" have different behavior, "deepen" can be
> achieved by "depth" with some extra calculation. If we know the
> distance between "my history bottom" and "your tips", we can set the sum
> of "deepen" step and this distance as "depth" step. Then we can reuse
> the logics of processing "depth" to complete it.
>
> Take graph in [1] as an example.
>
>>  (upstream)
>>   ---o---o---o---A---B
>>
>>  (you)
>>  A---B
>
> the distance of "my history bottom"(A) and "your tips"(B) is 1, then
> "git fetch --deepen=3" can be achieved by "git fetch --depth=4"
>
> However, I am a little worried about whether this idea is enough for a
> GSoC project. I hope you can give me some comments and suggestions.
>
> [1] http://article.gmane.org/gmane.comp.version-control.git/212950
>
> Thanks,
> Dongcan
>
> --
> 江东灿（Dongcan Jiang）
> Team of Search Engine & Web Mining
> School of Electronic Engineering & Computer Science
> Peking University, Beijing, 100871, P.R.China



-- 
江东灿（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: [GSoC/RFC] Ideas on git fetch --deepen

2015-03-08 Thread Dongcan Jiang 
It is really helpful. Thanks a lot!

2015-03-08 17:34 GMT+08:00 Duy Nguyen :
> On Sun, Mar 8, 2015 at 3:57 PM, Dongcan Jiang  wrote:
>> Hi, all
>>
>> After digging into how "git fetch" works, I have found that my previous
>> understanding was too rash. I'm sorry for that.
>>
>> I find that the current workflow of "git fetch --depth" is as follows:
>>
>>   1. 'fetch module' calls 'git-upload-pack service' via 'transport module'
>>  to fetch ref with `depth`.
>>   2. Such call is finished by pipe I/O.
>>   3. git-upload-pack receives arguments such as 'depth' to fetch commits,
>>  and send back to the caller.
>>
>> Therefore, if we want to implement "--deepen" for "git fetch", we have to
>> modify the arguments protocol of git-upload-pack service by adding the
>> 'shallow commit' hash.
>> Then, we can fetch `depth` commits before the 'shallow commit' in
>> git-upload-pack service.
>> Apparently, we have to output error message when the 'shallow commit' hash
>> is not in the repository.
>
> Close. You can't figure this shallow commit hash from client side
> (client repo is shortened). So you can't send it. What you send is
> exactly what the user gives you (e.g. --deepen=5, then you send
> "deepen 5" or similar). The server side (git-upload-pack) knows about
> the shallow boundary of the client and can walk its (i.e. server-side)
> commit graph to find out the new, deepened boundary. Then the server
> sends necessary objects plus instructions to update shallow boundary
> to the client.
>
> Also, I think  this work would include support for smart-http
> protocol. It goes a slightly different route, "git fetch" ends up at
> transport.c, but then transport-helper.c and calls git-remote-http(s)
> which is implemented by remote-curl.c. This one handles http stuff
> then passes control to git-fetch-pack.c. It's fetch-pack that talks to
> upload-pack.c
>
> Have a look at Documentation/technical/pack-protocol.txt and
> protocol-capabilities.txt (and http-protocol.txt for smart-http). The
> function that "draws" shallow boundary based on --depth is
> get_shallow_commits() in shallow.c. I suspect you need to improve it a
> bit to use with --deepen. I guess you can look at these [1]. Those
> changes touch shallow repo in a bit different way, but the main code
> path is more or less the same (smart-http not touched).
>
> [1] 
> https://github.com/pclouds/git/commits/7edde8b83a20abb3cd404e2c5f07e3c29a2891f7
> --
> 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

[PATCH v3/GSoC/MICRO] revision: forbid combining --graph and --no-walk

2015-03-08 Thread Dongcan Jiang 
Because "--graph" is about connected history while --no-walk
is about discrete points, it does not make sense to allow
giving these two options at the same time. [1]

This change allows git-show to have such options' combination
as a special case, because git-show itself has underlying
--no-walk option, while "git show --graph" is a legal and
useful operation which is tested in t4052. [2,3]

2 testcases have been added to test this patch.

[1]: http://article.gmane.org/gmane.comp.version-control.git/216083
[2]: http://article.gmane.org/gmane.comp.version-control.git/264950
[3]: http://article.gmane.org/gmane.comp.version-control.git/265107

Helped-By: Eric Sunshine 
Helped-By: René Scharfe 
Helped-By: Junio C Hamano 
Signed-off-by: Dongcan Jiang 
---
 Documentation/rev-list-options.txt | 2 ++
 builtin/log.c  | 1 +
 revision.c | 4 
 revision.h | 3 +++
 t/t4202-log.sh | 4 
 t/t6014-rev-list-all.sh| 4 
 6 files changed, 18 insertions(+)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 4ed8587..136abdf 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -679,6 +679,7 @@ endif::git-rev-list[]
given on the command line. Otherwise (if `sorted` or no argument
was given), the commits are shown in reverse chronological order
by commit time.
+   Cannot be combined with `--graph`.
 
 --do-walk::
Overrides a previous `--no-walk`.
@@ -781,6 +782,7 @@ you would get an output like this:
on the left hand side of the output.  This may cause extra lines
to be printed in between commits, in order for the graph history
to be drawn properly.
+   Cannot be combined with `--no-walk`.
 +
 This enables parent rewriting, see 'History Simplification' below.
 +
diff --git a/builtin/log.c b/builtin/log.c
index dd8f3fc..5b5d028 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -524,6 +524,7 @@ int cmd_show(int argc, const char **argv, const char 
*prefix)
 
memset(&match_all, 0, sizeof(match_all));
init_revisions(&rev, prefix);
+   rev.cmd_is_show = 1;
rev.diff = 1;
rev.always_show_header = 1;
rev.no_walk = REVISION_WALK_NO_WALK_SORTED;
diff --git a/revision.c b/revision.c
index 66520c6..5d6fbef 100644
--- a/revision.c
+++ b/revision.c
@@ -1399,6 +1399,8 @@ void init_revisions(struct rev_info *revs, const char 
*prefix)
 
revs->commit_format = CMIT_FMT_DEFAULT;
 
+   revs->cmd_is_show = 0;
+
init_grep_defaults();
grep_init(&revs->grep_filter, prefix);
revs->grep_filter.status_only = 1;
@@ -2339,6 +2341,8 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s
 
if (revs->reflog_info && revs->graph)
die("cannot combine --walk-reflogs with --graph");
+   if (!revs->cmd_is_show && revs->no_walk && revs->graph)
+   die("cannot combine --no-walk with --graph");
if (!revs->reflog_info && revs->grep_filter.use_reflog_filter)
die("cannot use --grep-reflog without --walk-reflogs");
 
diff --git a/revision.h b/revision.h
index 0ea8b4e..378c3bf 100644
--- a/revision.h
+++ b/revision.h
@@ -146,6 +146,9 @@ struct rev_info {
track_first_time:1,
linear:1;
 
+   /* cmd type */
+   unsigned int  cmd_is_show:1;
+
enum date_mode date_mode;
 
unsigned intabbrev;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 5f2b290..f111705 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -887,4 +887,8 @@ test_expect_success GPG 'log --graph --show-signature for 
merged tag' '
grep "^| | gpg: Good signature" actual
 '
 
+test_expect_success 'log --graph --no-walk is forbidden' '
+   test_must_fail git log --graph --no-walk
+'
+
 test_done
diff --git a/t/t6014-rev-list-all.sh b/t/t6014-rev-list-all.sh
index 991ab4a..c9bedd2 100755
--- a/t/t6014-rev-list-all.sh
+++ b/t/t6014-rev-list-all.sh
@@ -35,4 +35,8 @@ test_expect_success 'repack does not lose detached HEAD' '
 
 '
 
+test_expect_success 'rev-list --graph --no-walk is forbidden' '
+   test_must_fail git rev-list --graph --no-walk HEAD
+'
+
 test_done
-- 
2.3.1.252.ge67f612

--
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 v4/GSoC/MICRO] revision: forbid combining --graph and --no-walk

2015-03-10 Thread Dongcan Jiang 
Because "--graph" is about connected history while --no-walk
is about discrete points, it does not make sense to allow
giving these two options at the same time. [1]

This change makes a few calls to "show --graph" fail in
t4052, but asking to show one commit with graph is a
nonsensical thing to do. Thus, tests on "show --graph" in
t4052 have been removed. [2,3] Same tests on "show" without
--graph option have already been tested in 4052.

3 testcases have been added to test this patch.

[1]: http://article.gmane.org/gmane.comp.version-control.git/216083
[2]: http://article.gmane.org/gmane.comp.version-control.git/264950
[3]: http://article.gmane.org/gmane.comp.version-control.git/265107

Helped-By: Eric Sunshine 
Helped-By: René Scharfe 
Helped-By: Junio C Hamano 
Signed-off-by: Dongcan Jiang 
---
 Documentation/rev-list-options.txt |  2 ++
 revision.c |  2 ++
 t/t4052-stat-output.sh | 14 +++---
 t/t4202-log.sh |  4 
 t/t6014-rev-list-all.sh|  4 
 t/t7007-show.sh|  4 
 6 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 4ed8587..136abdf 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -679,6 +679,7 @@ endif::git-rev-list[]
given on the command line. Otherwise (if `sorted` or no argument
was given), the commits are shown in reverse chronological order
by commit time.
+   Cannot be combined with `--graph`.
 
 --do-walk::
Overrides a previous `--no-walk`.
@@ -781,6 +782,7 @@ you would get an output like this:
on the left hand side of the output.  This may cause extra lines
to be printed in between commits, in order for the graph history
to be drawn properly.
+   Cannot be combined with `--no-walk`.
 +
 This enables parent rewriting, see 'History Simplification' below.
 +
diff --git a/revision.c b/revision.c
index 66520c6..6cd91dd 100644
--- a/revision.c
+++ b/revision.c
@@ -2339,6 +2339,8 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s
 
if (revs->reflog_info && revs->graph)
die("cannot combine --walk-reflogs with --graph");
+   if (revs->no_walk && revs->graph)
+   die("cannot combine --no-walk with --graph");
if (!revs->reflog_info && revs->grep_filter.use_reflog_filter)
die("cannot use --grep-reflog without --walk-reflogs");
 
diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index b68afef..a989e8f 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -99,7 +99,7 @@ do
test_cmp "$expect" actual
'
 
-   test "$cmd" != diff || continue
+   test "$cmd" != diff -a "$cmd" != show || continue
 
test_expect_success "$cmd --graph $verb COLUMNS (big change)" '
COLUMNS=200 git $cmd $args --graph >output
@@ -127,7 +127,7 @@ do
test_cmp "$expect" actual
'
 
-   test "$cmd" != diff || continue
+   test "$cmd" != diff -a "$cmd" != show || continue
 
test_expect_success "$cmd --graph $verb not enough COLUMNS (big 
change)" '
COLUMNS=40 git $cmd $args --graph >output
@@ -155,7 +155,7 @@ do
test_cmp "$expect" actual
'
 
-   test "$cmd" != diff || continue
+   test "$cmd" != diff -a "$cmd" != show || continue
 
test_expect_success "$cmd --graph $verb statGraphWidth config" '
git -c diff.statGraphWidth=26 $cmd $args --graph >output
@@ -196,7 +196,7 @@ do
test_cmp expect actual
'
 
-   test "$cmd" != diff || continue
+   test "$cmd" != diff -a "$cmd" != show || continue
 
test_expect_success "$cmd --stat-width=width --graph with big change" '
git $cmd $args --stat-width=40 --graph >output
@@ -236,7 +236,7 @@ do
test_cmp expect actual
'
 
-   test "$cmd" != diff || continue
+   test "$cmd" != diff -a "$cmd" != show || continue
 
test_expect_success "$cmd --stat=width --graph with big change is 
balanced" '
git $cmd $args --stat-width=60 --graph >output &&
@@ -270,7 +270,7 @@ do
test_cmp "$expect" actual
'
 
-   test "$cmd" != diff || continue
+   test "$cmd" != diff -a "$cmd" != show || continue
 
test_expect_success "$

[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 
Signed-off-by: Dongcan Jiang 
---
 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 100

Re: [PATCH v3] log: forbid log --graph --no-walk

2015-03-14 Thread Dongcan Jiang 
it seems that your patch could not pass t4052-stat-output.sh.

I think it would be better if you could improve the specification for
this change in Document/rev-list-options.txt

2015-03-15 8:43 GMT+08:00 Manos Pitsidianakis :
> In git-log, --graph shows a graphical representation of a continuous
> commit history, and --no-walk shows discrete specified commits without
> continuity. Using both doesn't make sense, so we forbid the combined use
> of these flags.
>
> Signed-off-by: Manos Pitsidianakis 
> ---
> This is a microproject intended to complement my GSoC application.
>  builtin/log.c  | 2 ++
>  t/t4202-log.sh | 4 
>  2 files changed, 6 insertions(+)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index dd8f3fc..5aaf964 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -155,6 +155,8 @@ static void cmd_log_init_finish(int argc, const char 
> **argv, const char *prefix,
> memset(&w, 0, sizeof(w));
> userformat_find_requirements(NULL, &w);
>
> +   if (rev->graph && rev->no_walk)
> +   die("--graph and --no-walk are incompatible");
> if (!rev->show_notes_given && (!rev->pretty_given || w.notes))
> rev->show_notes = 1;
> if (rev->show_notes)
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index 5f2b290..5d72713 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -887,4 +887,8 @@ test_expect_success GPG 'log --graph --show-signature for 
> merged tag' '
> grep "^| | gpg: Good signature" actual
>  '
>
> +test_expect_success 'forbid log --graph --no-walk' '
> +   test_must_fail git log --graph --no-walk
> +'
> +
>  test_done
> --
> 2.1.4
>
>
>
> --
> 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



-- 
江东灿（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 v3] log: forbid log --graph --no-walk

2015-03-14 Thread Dongcan Jiang 
Because "revs->no_walk" gets set when it comes to "git show". You can
find more information on [1].

[1] http://article.gmane.org/gmane.comp.version-control.git/264921

2015-03-15 9:24 GMT+08:00 Manos Pitsidianakis :
> On 03/15/2015 03:08 AM, Dongcan Jiang wrote:
>> it seems that your patch could not pass t4052-stat-output.sh.
>>
>> I think it would be better if you could improve the specification for
>> this change in Document/rev-list-options.txt
>
> Can't grok why this happens. What exactly is happening in
> t4052-stat-output.sh? Is it testing every possible combination of git
> commands and arguments?



-- 
江东灿（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 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 :
> On Fri, Mar 13, 2015 at 9:04 AM, Dongcan Jiang  
> 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 
>> ---
>> 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-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 :
> On Fri, Mar 13, 2015 at 8:04 PM, Dongcan Jiang  
> 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 :
> Dongcan Jiang  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
>

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 :
> On Mon, Mar 16, 2015 at 11:10 PM, Dongcan Jiang  
> 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  
> 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

[PATCH v5/GSoC/MICRO] revision: forbid combining --graph and --no-walk

2015-03-17 Thread Dongcan Jiang 
Because "--graph" is about connected history while --no-walk
is about discrete points, it does not make sense to allow
giving these two options at the same time. [1]

This change makes a few calls to "show --graph" fail in
t4052, but asking to show one commit with graph is a
nonsensical thing to do. Thus, tests on "show --graph" in
t4052 have been removed. [2,3] Same tests on "show" without
--graph option have already been tested in 4052.

3 testcases have been added to test this patch.

[1]: http://article.gmane.org/gmane.comp.version-control.git/216083
[2]: http://article.gmane.org/gmane.comp.version-control.git/264950
[3]: http://article.gmane.org/gmane.comp.version-control.git/265107

Helped-By: Eric Sunshine 
Helped-By: René Scharfe 
Helped-By: Junio C Hamano 
Signed-off-by: Dongcan Jiang 
---
 Documentation/rev-list-options.txt |  2 ++
 revision.c |  2 ++
 t/t4052-stat-output.sh | 14 +++---
 t/t4202-log.sh |  4 
 t/t6014-rev-list-all.sh|  4 
 t/t7007-show.sh|  4 
 6 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 4ed8587..136abdf 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -679,6 +679,7 @@ endif::git-rev-list[]
given on the command line. Otherwise (if `sorted` or no argument
was given), the commits are shown in reverse chronological order
by commit time.
+   Cannot be combined with `--graph`.
 
 --do-walk::
Overrides a previous `--no-walk`.
@@ -781,6 +782,7 @@ you would get an output like this:
on the left hand side of the output.  This may cause extra lines
to be printed in between commits, in order for the graph history
to be drawn properly.
+   Cannot be combined with `--no-walk`.
 +
 This enables parent rewriting, see 'History Simplification' below.
 +
diff --git a/revision.c b/revision.c
index 66520c6..6cd91dd 100644
--- a/revision.c
+++ b/revision.c
@@ -2339,6 +2339,8 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s
 
if (revs->reflog_info && revs->graph)
die("cannot combine --walk-reflogs with --graph");
+   if (revs->no_walk && revs->graph)
+   die("cannot combine --no-walk with --graph");
if (!revs->reflog_info && revs->grep_filter.use_reflog_filter)
die("cannot use --grep-reflog without --walk-reflogs");
 
diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh
index b68afef..54f10cf 100755
--- a/t/t4052-stat-output.sh
+++ b/t/t4052-stat-output.sh
@@ -99,7 +99,7 @@ do
test_cmp "$expect" actual
'
 
-   test "$cmd" != diff || continue
+   test "$cmd" != diff && test "$cmd" != show || continue
 
test_expect_success "$cmd --graph $verb COLUMNS (big change)" '
COLUMNS=200 git $cmd $args --graph >output
@@ -127,7 +127,7 @@ do
test_cmp "$expect" actual
'
 
-   test "$cmd" != diff || continue
+   test "$cmd" != diff && test "$cmd" != show || continue
 
test_expect_success "$cmd --graph $verb not enough COLUMNS (big 
change)" '
COLUMNS=40 git $cmd $args --graph >output
@@ -155,7 +155,7 @@ do
test_cmp "$expect" actual
'
 
-   test "$cmd" != diff || continue
+   test "$cmd" != diff && test "$cmd" != show || continue
 
test_expect_success "$cmd --graph $verb statGraphWidth config" '
git -c diff.statGraphWidth=26 $cmd $args --graph >output
@@ -196,7 +196,7 @@ do
test_cmp expect actual
'
 
-   test "$cmd" != diff || continue
+   test "$cmd" != diff && test "$cmd" != show || continue
 
test_expect_success "$cmd --stat-width=width --graph with big change" '
git $cmd $args --stat-width=40 --graph >output
@@ -236,7 +236,7 @@ do
test_cmp expect actual
'
 
-   test "$cmd" != diff || continue
+   test "$cmd" != diff && test "$cmd" != show || continue
 
test_expect_success "$cmd --stat=width --graph with big change is 
balanced" '
git $cmd $args --stat-width=60 --graph >output &&
@@ -270,7 +270,7 @@ do
test_cmp "$expect" actual
'
 
-   test "$cmd" != diff || continue
+   test "$cmd" != diff &&

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

2015-03-22 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]

[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 
Helped-By: Eric Sunshine 
Helped-By: Junio C Hamano 
Signed-off-by: Dongcan Jiang 
---

-Background-

At present the command "git-fetch" has one option "depth" which deepens or 
shortens the history of a shallow repository created by git clone with 
--depth= option (see git-clone[1]) to the specified number of commits 
from the tip of each remote branch history [2].

The drawback is that the starting point for deepening history is limited to the 
tip of each remote branch history. However, it's not able to satisfy the needs 
of users in some cases:
1) If the user wants to know the process of how a function was created and 
developed. What he expects is that he goes to the point of latest modification 
about the function and deepens the history further back from this current 
cut-off point by N commit directly, rather than having to guess what the depth 
is from tip of remote branch.
2) What's more, if the user only cares about the revisions of this function, 
the other commits after the latest modification are redundant for him, but he 
has to fetch them concomitantly, which wastes unnecessary time and space.


For the convenience of users, a new option should be added as "deepen" to allow 
users to just get history commits deepened by N commits from current cut-off 
point, irrespective of location where the tips are [3].


-Goals-

1) Supports for command "git fetch --deepen";
2) Conflict Detection for this option to guarantee robustness;
3) Sufficient tests;
4) Guarantee for compatibility;
5) Sufficient documents;

More Details are shown in next section.


-Details-

--1. Current Workflow of git-fetch--

 +-+-+-+-+-+-+-+-+-+  +-+-+-+-+-+-+-+-+-+-+-+
 | git-upload-pack |  | git-unpack-objects  |
 +-+-+-+-+-+-+-+-+-+  +-+-+-+-+-+-+-+-+-+-+-+
|   ^  |   ^
v   |  v   |
 fetch   +-+-+-+-+-+-+-+-+-+  +-+-+-+-+-+-+-+  +-+-+-+-+-+-+
---> | get remote refs | ---> | fetch refs  | ---> | get_pack  | -+
 +-+-+-+-+-+-+-+-+-+  +-+-+-+-+-+-+-+  +-+-+-+-+-+-+  |
   | ^|
   v |v
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+  +-+-+-+-+-+-+-+
| git-upload-pack   |  |   update|
+  +-+-+-+-+-+-+-+-+-+-+-+-+  +-+-+-+-+-+-+-+-+-+-+ +  | local refs  |
|  | generate wanted objs  | ---> | create_pack_file  | |  +-+-+-+-+-+-+-+
+  |update shallows|  +-+-+-+-+-+-+-+-+-+-+ +
|  +-+-+-+-+-+-+-+-+-+-+-+-+| ^ |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| |
v |
  +-+-+-+-+-+-+-+-+-+-+
  | git-pack-objects  |
  +-+-+-+-+-+-+-+-+-+-+
  Figure 1. Workflow of git-fetch (Please view it in fixed-width fonts)

Figure 1 illustrates the workflow of git-fetch. When it comes to git-fetch, a 
series of operations are performed in cooperation with the server side via 
`transport` module.

At first, it gets remote refs from git-upload-pack, and then it fetches refs, 
i.e. generating wanted objs, updating shallows and creating pack file. Finally, 
it unpacks objects from the pack file and updates the local refs.

Actually, there are several prot