git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Remove the line length limit for graft files
@ 2013-12-27 20:49 Johannes Schindelin
  2013-12-27 21:04 ` Jonathan Nieder
  2013-12-27 21:48 ` Junio C Hamano
  0 siblings, 2 replies; 9+ messages in thread
From: Johannes Schindelin @ 2013-12-27 20:49 UTC (permalink / raw)
  To: git; +Cc: msysgit, gitster


Support for grafts predates Git's strbuf, and hence it is understandable
that there was a hard-coded line length limit of 1023 characters (which
was chosen a bit awkwardly, given that it is *exactly* one byte short of
aligning with the 41 bytes occupied by a commit name and the following
space or new-line character).

While regular commit histories hardly win comprehensibility in general
if they merge more than twenty-two branches in one go, it is not Git's
business to limit grafts in such a way.

In this particular developer's case, the use case that requires
substantially longer graft lines to be supported is the visualization of
the commits' order implied by their changes: commits are considered to
have an implicit relationship iff exchanging them in an interactive
rebase would result in merge conflicts.

Thusly implied branches tend to be very shallow in general, and the
resulting thicket of implied branches is usually very wide; It is
actually quite common that *most* of the commits in a topic branch have
not even one implied parent, so that a final merge commit has about as
many implied parents as there are commits in said branch.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin/blame.c |  8 ++++----
 commit.c        | 10 +++++-----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 1407ae7..9047b6e 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1804,17 +1804,17 @@ static int prepare_lines(struct scoreboard *sb)
 static int read_ancestry(const char *graft_file)
 {
 	FILE *fp = fopen(graft_file, "r");
-	char buf[1024];
+	struct strbuf buf = STRBUF_INIT;
 	if (!fp)
 		return -1;
-	while (fgets(buf, sizeof(buf), fp)) {
+	while (!strbuf_getwholeline(&buf, fp, '\n')) {
 		/* The format is just "Commit Parent1 Parent2 ...\n" */
-		int len = strlen(buf);
-		struct commit_graft *graft = read_graft_line(buf, len);
+		struct commit_graft *graft = read_graft_line(buf.buf, buf.len);
 		if (graft)
 			register_commit_graft(graft, 0);
 	}
 	fclose(fp);
+	strbuf_release(&buf);
 	return 0;
 }
 
diff --git a/commit.c b/commit.c
index de16a3c..57ebea2 100644
--- a/commit.c
+++ b/commit.c
@@ -196,19 +196,19 @@ bad_graft_data:
 static int read_graft_file(const char *graft_file)
 {
 	FILE *fp = fopen(graft_file, "r");
-	char buf[1024];
+	struct strbuf buf = STRBUF_INIT;
 	if (!fp)
 		return -1;
-	while (fgets(buf, sizeof(buf), fp)) {
+	while (!strbuf_getwholeline(&buf, fp, '\n')) {
 		/* The format is just "Commit Parent1 Parent2 ...\n" */
-		int len = strlen(buf);
-		struct commit_graft *graft = read_graft_line(buf, len);
+		struct commit_graft *graft = read_graft_line(buf.buf, buf.len);
 		if (!graft)
 			continue;
 		if (register_commit_graft(graft, 1))
-			error("duplicate graft data: %s", buf);
+			error("duplicate graft data: %s", buf.buf);
 	}
 	fclose(fp);
+	strbuf_release(&buf);
 	return 0;
 }
 
-- 
1.8.4.msysgit.0.1109.g3c58b16

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Remove the line length limit for graft files
  2013-12-27 20:49 [PATCH] Remove the line length limit for graft files Johannes Schindelin
@ 2013-12-27 21:04 ` Jonathan Nieder
  2013-12-27 21:14   ` Johannes Schindelin
  2013-12-27 21:48 ` Junio C Hamano
  1 sibling, 1 reply; 9+ messages in thread
From: Jonathan Nieder @ 2013-12-27 21:04 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, msysgit, gitster

Hi,

Johannes Schindelin wrote:

> While regular commit histories hardly win comprehensibility in general
> if they merge more than twenty-two branches in one go, it is not Git's
> business to limit grafts in such a way.

Fun. :)  Makes sense.

[...]
> ---
>  builtin/blame.c |  8 ++++----
>  commit.c        | 10 +++++-----
>  2 files changed, 9 insertions(+), 9 deletions(-)

Is this easy to reproduce so some interested but lazy person could
write a test?

[...]
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1804,17 +1804,17 @@ static int prepare_lines(struct scoreboard *sb)
>  static int read_ancestry(const char *graft_file)
>  {
>  	FILE *fp = fopen(graft_file, "r");
> -	char buf[1024];
> +	struct strbuf buf = STRBUF_INIT;
>  	if (!fp)
>  		return -1;
> -	while (fgets(buf, sizeof(buf), fp)) {
> +	while (!strbuf_getwholeline(&buf, fp, '\n')) {

If there is no newline at EOF, this will skip the last line, while the
old behavior was to pay attention to it.  I haven't thought through
whether that's a good or bad change.  Maybe it should just be
documented?

[...]
> --- a/commit.c
> +++ b/commit.c
> @@ -196,19 +196,19 @@ static int read_graft_file(const char *graft_file)
[...]
> -	while (fgets(buf, sizeof(buf), fp)) {
> +	while (!strbuf_getwholeline(&buf, fp, '\n')) {

Likewise.

The rest of the patch looks good.

Merry christmas,
Jonathan

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Remove the line length limit for graft files
  2013-12-27 21:04 ` Jonathan Nieder
@ 2013-12-27 21:14   ` Johannes Schindelin
  2013-12-27 21:59     ` Jonathan Nieder
  2013-12-27 22:32     ` Jonathan Nieder
  0 siblings, 2 replies; 9+ messages in thread
From: Johannes Schindelin @ 2013-12-27 21:14 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, msysgit, gitster

Hi,

On Fri, 27 Dec 2013, Jonathan Nieder wrote:

> Johannes Schindelin wrote:
> 
> [...]
> > ---
> >  builtin/blame.c |  8 ++++----
> >  commit.c        | 10 +++++-----
> >  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> Is this easy to reproduce so some interested but lazy person could
> write a test?

Yep. Make 25 orphan commits, add a graft line to make the first a merge of
the rest.

> > --- a/builtin/blame.c
> > +++ b/builtin/blame.c
> > @@ -1804,17 +1804,17 @@ static int prepare_lines(struct scoreboard *sb)
> >  static int read_ancestry(const char *graft_file)
> >  {
> >  	FILE *fp = fopen(graft_file, "r");
> > -	char buf[1024];
> > +	struct strbuf buf = STRBUF_INIT;
> >  	if (!fp)
> >  		return -1;
> > -	while (fgets(buf, sizeof(buf), fp)) {
> > +	while (!strbuf_getwholeline(&buf, fp, '\n')) {
> 
> If there is no newline at EOF, this will skip the last line, while the
> old behavior was to pay attention to it.  I haven't thought through
> whether that's a good or bad change.  Maybe it should just be
> documented?

The way I read

	int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)
	{
		int ch;

		if (feof(fp))
			return EOF;

		strbuf_reset(sb);
		while ((ch = fgetc(fp)) != EOF) {
			strbuf_grow(sb, 1);
			sb->buf[sb->len++] = ch;
			if (ch == term)
				break;
		}
		if (ch == EOF && sb->len == 0)
			return EOF;

		sb->buf[sb->len] = '\0';
		return 0;
	}

it returns EOF only if ch == EOF *and* sb->len == 0, i.e. if no characters
have been read before hitting EOF.

In other words, strbuf_getwholeline() -- despite requiring an explicit
terminating character argument -- does not require the last line to end
with that terminating character.

A quick test (in my case, because I am lazy, modifying test-mergesort.c to
output the lines that were read by strbuf_getwholeline()) also confirms my
suspicion.

Or maybe I missed something?

Ciao,
Dscho

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Remove the line length limit for graft files
  2013-12-27 20:49 [PATCH] Remove the line length limit for graft files Johannes Schindelin
  2013-12-27 21:04 ` Jonathan Nieder
@ 2013-12-27 21:48 ` Junio C Hamano
  1 sibling, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2013-12-27 21:48 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, msysgit

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Support for grafts predates Git's strbuf, and hence it is understandable
> that there was a hard-coded line length limit of 1023 characters (which
> was chosen a bit awkwardly, given that it is *exactly* one byte short of
> aligning with the 41 bytes occupied by a commit name and the following
> space or new-line character).
>
> While regular commit histories hardly win comprehensibility in general
> if they merge more than twenty-two branches in one go, it is not Git's
> business to limit grafts in such a way.
>
> In this particular developer's case, the use case that requires
> substantially longer graft lines to be supported is the visualization of
> the commits' order implied by their changes: commits are considered to
> have an implicit relationship iff exchanging them in an interactive
> rebase would result in merge conflicts.
>
> Thusly implied branches tend to be very shallow in general, and the
> resulting thicket of implied branches is usually very wide; It is
> actually quite common that *most* of the commits in a topic branch have
> not even one implied parent, so that a final merge commit has about as
> many implied parents as there are commits in said branch.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  builtin/blame.c |  8 ++++----
>  commit.c        | 10 +++++-----
>  2 files changed, 9 insertions(+), 9 deletions(-)

Makes sense.  It is in line with the spirit of ef98c5cafb3e
(commit-tree: lift completely arbitrary limit of 16 parents,
2008-06-27), too ;-)

Thanks, will queue.


> diff --git a/builtin/blame.c b/builtin/blame.c
> index 1407ae7..9047b6e 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1804,17 +1804,17 @@ static int prepare_lines(struct scoreboard *sb)
>  static int read_ancestry(const char *graft_file)
>  {
>  	FILE *fp = fopen(graft_file, "r");
> -	char buf[1024];
> +	struct strbuf buf = STRBUF_INIT;
>  	if (!fp)
>  		return -1;
> -	while (fgets(buf, sizeof(buf), fp)) {
> +	while (!strbuf_getwholeline(&buf, fp, '\n')) {
>  		/* The format is just "Commit Parent1 Parent2 ...\n" */
> -		int len = strlen(buf);
> -		struct commit_graft *graft = read_graft_line(buf, len);
> +		struct commit_graft *graft = read_graft_line(buf.buf, buf.len);
>  		if (graft)
>  			register_commit_graft(graft, 0);
>  	}
>  	fclose(fp);
> +	strbuf_release(&buf);
>  	return 0;
>  }
>  
> diff --git a/commit.c b/commit.c
> index de16a3c..57ebea2 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -196,19 +196,19 @@ bad_graft_data:
>  static int read_graft_file(const char *graft_file)
>  {
>  	FILE *fp = fopen(graft_file, "r");
> -	char buf[1024];
> +	struct strbuf buf = STRBUF_INIT;
>  	if (!fp)
>  		return -1;
> -	while (fgets(buf, sizeof(buf), fp)) {
> +	while (!strbuf_getwholeline(&buf, fp, '\n')) {
>  		/* The format is just "Commit Parent1 Parent2 ...\n" */
> -		int len = strlen(buf);
> -		struct commit_graft *graft = read_graft_line(buf, len);
> +		struct commit_graft *graft = read_graft_line(buf.buf, buf.len);
>  		if (!graft)
>  			continue;
>  		if (register_commit_graft(graft, 1))
> -			error("duplicate graft data: %s", buf);
> +			error("duplicate graft data: %s", buf.buf);
>  	}
>  	fclose(fp);
> +	strbuf_release(&buf);
>  	return 0;
>  }
>  
> -- 
> 1.8.4.msysgit.0.1109.g3c58b16
>
> -- 

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Remove the line length limit for graft files
  2013-12-27 21:14   ` Johannes Schindelin
@ 2013-12-27 21:59     ` Jonathan Nieder
  2013-12-27 22:51       ` Johannes Schindelin
  2013-12-27 22:32     ` Jonathan Nieder
  1 sibling, 1 reply; 9+ messages in thread
From: Jonathan Nieder @ 2013-12-27 21:59 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, msysgit, gitster

Johannes Schindelin wrote:

> it returns EOF only if ch == EOF *and* sb->len == 0, i.e. if no characters
> have been read before hitting EOF.

Yep.  api-strbuf.txt even says so.  Sorry for the nonsense.

For what it's worth,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Remove the line length limit for graft files
  2013-12-27 21:14   ` Johannes Schindelin
  2013-12-27 21:59     ` Jonathan Nieder
@ 2013-12-27 22:32     ` Jonathan Nieder
  2013-12-27 22:53       ` Johannes Schindelin
  2013-12-28  0:50       ` Junio C Hamano
  1 sibling, 2 replies; 9+ messages in thread
From: Jonathan Nieder @ 2013-12-27 22:32 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, msysgit, gitster

Johannes Schindelin wrote:
> On Fri, 27 Dec 2013, Jonathan Nieder wrote:

>> Is this easy to reproduce so some interested but lazy person could
>> write a test?
>
> Yep. Make 25 orphan commits, add a graft line to make the first a merge of
> the rest.

Thanks.  Here's a pair of tests doing that.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/annotate-tests.sh          | 21 +++++++++++++++++++++
 t/t6101-rev-parse-parents.sh | 16 +++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index c9d105d..304c7b7 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -116,6 +116,27 @@ test_expect_success 'blame evil merge' '
 	check_count A 2 B 1 B1 2 B2 1 "A U Thor" 1
 '
 
+test_expect_success 'blame huge graft' '
+	test_when_finished "git checkout branch2" &&
+	test_when_finished "rm -f .git/info/grafts" &&
+	graft= &&
+	for i in 0 1 2
+	do
+		for j in 0 1 2 3 4 5 6 7 8 9
+		do
+			git checkout --orphan "$i$j" &&
+			printf "%s\n" "$i" "$j" >file &&
+			test_tick &&
+			GIT_AUTHOR_NAME=$i$j GIT_AUTHOR_EMAIL=$i$j@test.git \
+			git commit -a -m "$i$j" &&
+			commit=$(git rev-parse --verify HEAD) &&
+			graft="$graft$commit "
+		done
+	done &&
+	printf "%s " $graft >.git/info/grafts &&
+	check_count -h 00 01 1 10 1
+'
+
 test_expect_success 'setup incomplete line' '
 	echo "incomplete" | tr -d "\\012" >>file &&
 	GIT_AUTHOR_NAME="C" GIT_AUTHOR_EMAIL="C@test.git" \
diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
index 7ea14ce..10b1452 100755
--- a/t/t6101-rev-parse-parents.sh
+++ b/t/t6101-rev-parse-parents.sh
@@ -20,7 +20,17 @@ test_expect_success 'setup' '
 	test_commit start2 &&
 	git checkout master &&
 	git merge -m next start2 &&
-	test_commit final
+	test_commit final &&
+
+	test_seq 40 |
+	while read i
+	do
+		git checkout --orphan "b$i" &&
+		test_tick &&
+		git commit --allow-empty -m "$i" &&
+		commit=$(git rev-parse --verify HEAD) &&
+		printf "$commit " >>.git/info/grafts
+	done
 '
 
 test_expect_success 'start is valid' '
@@ -79,6 +89,10 @@ test_expect_success 'final^1^! = final^1 ^final^1^1 ^final^1^2' '
 	test_cmp expect actual
 '
 
+test_expect_success 'large graft octopus' '
+	test_cmp_rev_output b31 "git rev-parse --verify b1^30"
+'
+
 test_expect_success 'repack for next test' '
 	git repack -a -d
 '
-- 
1.8.5.1

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Remove the line length limit for graft files
  2013-12-27 21:59     ` Jonathan Nieder
@ 2013-12-27 22:51       ` Johannes Schindelin
  0 siblings, 0 replies; 9+ messages in thread
From: Johannes Schindelin @ 2013-12-27 22:51 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, msysgit, gitster

Hi Jonathan,

On Fri, 27 Dec 2013, Jonathan Nieder wrote:

> Johannes Schindelin wrote:
> 
> > it returns EOF only if ch == EOF *and* sb->len == 0, i.e. if no characters
> > have been read before hitting EOF.
> 
> Yep.  api-strbuf.txt even says so.

I never bothered to look ;-)

> Sorry for the nonsense.

Nope, no nonsense at all. I actually had a look only after your review,
and it definitely makes sense to double-check.

> For what it's worth,
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

It is worth a lot. Believe me, I know just how thankless a task reviewing
is, and instead of getting praise for it, you even get abused by
contributors who would rather self-review their code for fear of having a
twist in their knockers pointed out publicly.

Your review makes the code better, even if it does not result in code
changes all the time: it increases the confidence that the code is good.

Thank you, Jonathan!
Dscho

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Remove the line length limit for graft files
  2013-12-27 22:32     ` Jonathan Nieder
@ 2013-12-27 22:53       ` Johannes Schindelin
  2013-12-28  0:50       ` Junio C Hamano
  1 sibling, 0 replies; 9+ messages in thread
From: Johannes Schindelin @ 2013-12-27 22:53 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, msysgit, gitster

Hi Jonathan,

On Fri, 27 Dec 2013, Jonathan Nieder wrote:

> Johannes Schindelin wrote:
> > On Fri, 27 Dec 2013, Jonathan Nieder wrote:
> 
> >> Is this easy to reproduce so some interested but lazy person could
> >> write a test?
> >
> > Yep. Make 25 orphan commits, add a graft line to make the first a merge of
> > the rest.
> 
> Thanks.  Here's a pair of tests doing that.

Thank you very much!

> +	for i in 0 1 2
> +	do
> +		for j in 0 1 2 3 4 5 6 7 8 9
> +		do

for the record, I usually prefer

	i=0
	while test $i -t 30
	do
		...
		i=$(($i+1))
	done

but your code is just as good!

Ciao,
Dscho

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Remove the line length limit for graft files
  2013-12-27 22:32     ` Jonathan Nieder
  2013-12-27 22:53       ` Johannes Schindelin
@ 2013-12-28  0:50       ` Junio C Hamano
  1 sibling, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2013-12-28  0:50 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Johannes Schindelin, git, msysgit

Jonathan Nieder <jrnieder@gmail.com> writes:

> Johannes Schindelin wrote:
>> On Fri, 27 Dec 2013, Jonathan Nieder wrote:
>
>>> Is this easy to reproduce so some interested but lazy person could
>>> write a test?
>>
>> Yep. Make 25 orphan commits, add a graft line to make the first a merge of
>> the rest.
>
> Thanks.  Here's a pair of tests doing that.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
>  t/annotate-tests.sh          | 21 +++++++++++++++++++++
>  t/t6101-rev-parse-parents.sh | 16 +++++++++++++++-
>  2 files changed, 36 insertions(+), 1 deletion(-)

Makes sense.

Thanks, both.  Small lint-picking like this change to perfect the
system, as opposed to earth-shattering new shinies, tend to often
get neglected but are very much appreciated.

> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
> index c9d105d..304c7b7 100644
> --- a/t/annotate-tests.sh
> +++ b/t/annotate-tests.sh
> @@ -116,6 +116,27 @@ test_expect_success 'blame evil merge' '
>  	check_count A 2 B 1 B1 2 B2 1 "A U Thor" 1
>  '
>  
> +test_expect_success 'blame huge graft' '
> +	test_when_finished "git checkout branch2" &&
> +	test_when_finished "rm -f .git/info/grafts" &&
> +	graft= &&
> +	for i in 0 1 2
> +	do
> +		for j in 0 1 2 3 4 5 6 7 8 9
> +		do
> +			git checkout --orphan "$i$j" &&
> +			printf "%s\n" "$i" "$j" >file &&
> +			test_tick &&
> +			GIT_AUTHOR_NAME=$i$j GIT_AUTHOR_EMAIL=$i$j@test.git \
> +			git commit -a -m "$i$j" &&
> +			commit=$(git rev-parse --verify HEAD) &&
> +			graft="$graft$commit "
> +		done
> +	done &&
> +	printf "%s " $graft >.git/info/grafts &&
> +	check_count -h 00 01 1 10 1
> +'
> +
>  test_expect_success 'setup incomplete line' '
>  	echo "incomplete" | tr -d "\\012" >>file &&
>  	GIT_AUTHOR_NAME="C" GIT_AUTHOR_EMAIL="C@test.git" \
> diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh
> index 7ea14ce..10b1452 100755
> --- a/t/t6101-rev-parse-parents.sh
> +++ b/t/t6101-rev-parse-parents.sh
> @@ -20,7 +20,17 @@ test_expect_success 'setup' '
>  	test_commit start2 &&
>  	git checkout master &&
>  	git merge -m next start2 &&
> -	test_commit final
> +	test_commit final &&
> +
> +	test_seq 40 |
> +	while read i
> +	do
> +		git checkout --orphan "b$i" &&
> +		test_tick &&
> +		git commit --allow-empty -m "$i" &&
> +		commit=$(git rev-parse --verify HEAD) &&
> +		printf "$commit " >>.git/info/grafts
> +	done
>  '
>  
>  test_expect_success 'start is valid' '
> @@ -79,6 +89,10 @@ test_expect_success 'final^1^! = final^1 ^final^1^1 ^final^1^2' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'large graft octopus' '
> +	test_cmp_rev_output b31 "git rev-parse --verify b1^30"
> +'
> +
>  test_expect_success 'repack for next test' '
>  	git repack -a -d
>  '
> -- 
> 1.8.5.1
>
> -- 

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-12-28  0:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-27 20:49 [PATCH] Remove the line length limit for graft files Johannes Schindelin
2013-12-27 21:04 ` Jonathan Nieder
2013-12-27 21:14   ` Johannes Schindelin
2013-12-27 21:59     ` Jonathan Nieder
2013-12-27 22:51       ` Johannes Schindelin
2013-12-27 22:32     ` Jonathan Nieder
2013-12-27 22:53       ` Johannes Schindelin
2013-12-28  0:50       ` Junio C Hamano
2013-12-27 21:48 ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).