Git development
 help / color / mirror / Atom feed
* Re: Use a *real* built-in diff generator
From: Linus Torvalds @ 2006-03-25 15:56 UTC (permalink / raw)
  To: Morten Welinder; +Cc: Junio C Hamano, Git Mailing List, Davide Libenzi
In-Reply-To: <Pine.LNX.4.64.0603250734130.15714@g5.osdl.org>



On Sat, 25 Mar 2006, Linus Torvalds wrote:
> 
> I'll be taking a look at trying to fix it. 

Actually, it ended up being easier than I expected it to be.

This (on top of the previous patch) should fix it.

And yes, with this, I can pass the output of

	git diff v2.6.16..

to "git-apply" and it not only passes the "--stat" thing (which verifies 
that git-apply is happy with the diff) but it also results in exactly the 
same tree when applied on top of v2.6.16 (and the patch has two cases 
where the "no newline" test triggers).

The speed-up is quite noticeable, especially when doing things like

	git diff v2.6.16.. | git-apply --stat

which just _used_ to be painfully slow (25 seconds for me) and is now 
under five seconds. That's the difference between "twiddling your thumbs" 
and "ok, that wasn't too bad".

Now, to be honest, the real reason I wanted a built-in diff wasn't the 
speed advantage, but the fact that it's so much more flexible. The lack of 
fork/exec just allows us to do things that weren't practical before.

		Linus

----
diff --git a/xdiff/xutils.c b/xdiff/xutils.c
index 01e6765..b68afa2 100644
--- a/xdiff/xutils.c
+++ b/xdiff/xutils.c
@@ -31,14 +31,22 @@
 
 int xdl_emit_diffrec(char const *rec, long size, char const *pre, long psize,
 		     xdemitcb_t *ecb) {
-	mmbuffer_t mb[2];
+	mmbuffer_t mb[3];
+	int i;
 
 	mb[0].ptr = (char *) pre;
 	mb[0].size = psize;
 	mb[1].ptr = (char *) rec;
 	mb[1].size = size;
+	i = 2;
 
-	if (ecb->outf(ecb->priv, mb, 2) < 0) {
+	if (!size || rec[size-1] != '\n') {
+		mb[2].ptr = "\n\\ No newline at end of file\n";
+		mb[2].size = strlen(mb[2].ptr);
+		i = 3;
+	}
+
+	if (ecb->outf(ecb->priv, mb, i) < 0) {
 
 		return -1;
 	}

^ permalink raw reply related

* Re: Use a *real* built-in diff generator
From: Linus Torvalds @ 2006-03-25 16:09 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Junio C Hamano, Git Mailing List, Davide Libenzi
In-Reply-To: <81b0412b0603250456o7f5925e6kbbfc0054aec564a1@mail.gmail.com>



On Sat, 25 Mar 2006, Alex Riesen wrote:
> 
> Even more impressive on Cygwin (>50x!):
> 
> .../git-win$ time git --exec-path=$(pwd) diff initial.. > /dev/null
> real    0m1.485s
> user    0m0.567s
> sys     0m0.840s
> 
> ../git-win$ time git diff initial.. >/dev/null
> real    1m20.781s
> user    0m31.806s
> sys     0m20.717s

Yeah. That's the difference between "unusable" and "retty damn good".

Now, if we didn't even bother to write temporary files (and just did the 
object entirely in memory) I'd be even happier. I suspect it would help 
cygwin more too.

I've done a "strace" on "git-diff-tree" doing the 5-second diff of the 
kernel tree, and almost all of it looks like this:

	..
	open("/tmp/.diff_WgWi1X", O_RDWR|O_CREAT|O_EXCL, 0600) = 3
	write(3, "/*\n * Driver for Digigram pcxhr "..., 6121) = 6121
	close(3)                                = 0
	open("/tmp/.diff_hCzrFe", O_RDWR|O_CREAT|O_EXCL, 0600) = 3
	write(3, "/*\n * Driver for Digigram pcxhr "..., 6138) = 6138
	close(3)                                = 0
	rt_sigaction(SIGINT, {0x1000f650, [INT], SA_RESTART}, {0x1000f650, [INT], SA_RESTART}, 8) = 0
	open("/tmp/.diff_WgWi1X", O_RDONLY)     = 3
	fstat64(3, {st_mode=S_IFREG|0600, st_size=6121, ...}) = 0
	read(3, "/*\n * Driver for Digigram pcxhr "..., 6121) = 6121
	close(3)                                = 0
	open("/tmp/.diff_hCzrFe", O_RDONLY)     = 3
	fstat64(3, {st_mode=S_IFREG|0600, st_size=6138, ...}) = 0
	read(3, "/*\n * Driver for Digigram pcxhr "..., 6138) = 6138
	close(3)                                = 0
	unlink("/tmp/.diff_WgWi1X")             = 0
	unlink("/tmp/.diff_hCzrFe")             = 0
	..

which is just ridiculous. Those are _literally_ the only system calls we 
do any more after the conversion, if you ignore a few "brk()" calls here 
and there to allocate/free memory and obviously a number of "write(1,..." 
calls to actually write out the result!

(This is with a fully packed tree, so we just set up the object store with 
a single mmap at the beginning, which is why there are no reads to read 
the actual source contents).

Now, Linux is good at temp-files, but still: it adds nothing but overhead 
to first write out and then read back in over three _thousand_ filepairs 
(only to delete them immediately after reading), when the new code 
actually just wants to do the diff in memory anyway.

		Linus

^ permalink raw reply

* Re: Use a *real* built-in diff generator
From: Linus Torvalds @ 2006-03-25 17:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk6ajxbe5.fsf@assigned-by-dhcp.cox.net>



On Fri, 24 Mar 2006, Junio C Hamano wrote:
> 
> Another thing I noticed is that while libxdiff always shows full
> line counts "-n,m +l,k" GNU seems to omit them when it can (m,k
> <=1).  I am not sure if apply.c is set up to grok what libxdiff
> emits correctly.  Running t/t1200 shows some obvious examples.

Actually, the GNU diff output is the special case, and git-apply handles 
it as such. 

We could make libxdiff do the same @@-shortening, but it doesn't seem to 
be huge deal.

		Linus

^ permalink raw reply

* Re: Use a *real* built-in diff generator
From: Davide Libenzi @ 2006-03-25 18:14 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Morten Welinder, Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0603250742340.15714@g5.osdl.org>

On Sat, 25 Mar 2006, Linus Torvalds wrote:

> On Sat, 25 Mar 2006, Linus Torvalds wrote:
>>
>> I'll be taking a look at trying to fix it.
>
> Actually, it ended up being easier than I expected it to be.
>
> This (on top of the previous patch) should fix it.
>
> And yes, with this, I can pass the output of
>
> 	git diff v2.6.16..
>
> to "git-apply" and it not only passes the "--stat" thing (which verifies
> that git-apply is happy with the diff) but it also results in exactly the
> same tree when applied on top of v2.6.16 (and the patch has two cases
> where the "no newline" test triggers).
>
> The speed-up is quite noticeable, especially when doing things like
>
> 	git diff v2.6.16.. | git-apply --stat
>
> which just _used_ to be painfully slow (25 seconds for me) and is now
> under five seconds. That's the difference between "twiddling your thumbs"
> and "ok, that wasn't too bad".

Yeah, that works. It has never been an algorithm problem, but a diff 
output one. And following what GNU diff does looks fine to me. I'll fix 
libxdiff with that. I also have to teach libxdiff patch algo to recognize 
the tag and do the right thing during the patch operation.



> Now, to be honest, the real reason I wanted a built-in diff wasn't the
> speed advantage, but the fact that it's so much more flexible. The lack of
> fork/exec just allows us to do things that weren't practical before.

I don't know if git is patch-forkexec sensitive or not, but if it is you 
can take a look at libxdiff's xdl_patch(), or at libifying GNU patch.



- Davide

^ permalink raw reply

* Re: Use a *real* built-in diff generator
From: Linus Torvalds @ 2006-03-25 18:39 UTC (permalink / raw)
  To: Davide Libenzi; +Cc: Morten Welinder, Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0603251009500.11968@alien.or.mcafeemobile.com>



On Sat, 25 Mar 2006, Davide Libenzi wrote:
> 
> > Now, to be honest, the real reason I wanted a built-in diff wasn't the
> > speed advantage, but the fact that it's so much more flexible. The lack of
> > fork/exec just allows us to do things that weren't practical before.
> 
> I don't know if git is patch-forkexec sensitive or not, but if it is you can
> take a look at libxdiff's xdl_patch(), or at libifying GNU patch.

I don't need "patch", since I wrote my own anyway. It's just called 
"apply" instead of "patch".

Doing "apply" is not only much simpler than doing "diff", but I needed my 
own much earlier: it's much more timing-critical for me (applying 200 
patches in one go), and git needed something that could honor renames and 
copies, and the mode bits too.

Besides, I hate how GNU patch bends over backwards in applying crap that 
isn't a proper patch at all (whitespace-corruption, you name it: GNU patch 
will accept it). Also, I made "git-apply" be all-or-nothing: either it 
applies the _whole_ patch (across many different files) or it applies none 
of it. With GNU patch, if you get an error on the fifth file, the four 
first files have been modified already - aarrgghhh..

See "apply.c" for details if you care. It's stupid, but it works (and it 
_only_ handles unified diffs - with the git extensions, of course).

(I also absolutely hate the GNU coding standards, so I'd be very unlikely 
to libify any of the FSF projects. With libxdiff, I can actually read the 
code: it may be a bit dense at times, but at least the code is written to 
be readable, unlike most FSF projects).

			Linus

^ permalink raw reply

* Re: Use a *real* built-in diff generator
From: Linus Torvalds @ 2006-03-25 18:48 UTC (permalink / raw)
  To: Davide Libenzi; +Cc: Morten Welinder, Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0603251009500.11968@alien.or.mcafeemobile.com>



On Sat, 25 Mar 2006, Davide Libenzi wrote:
>
> I also have to teach libxdiff patch algo to recognize the tag and do the
> right thing during the patch operation.

Btw, git-apply does it, and it's actually quite simple: the code to handle 
the "\ No newline" case is literally just this:

                /*
                 * "plen" is how much of the line we should use for 
                 * the actual patch data. Normally we just remove the
                 * first character on the line, but if the line is
                 * followed by "\ No newline", then we also remove the
                 * last one (which is the newline, of course).
                 */
                plen = len-1;
                if (len < size && patch[len] == '\\')
                        plen--;

if we just remove the last '\n' on a line, if the _next_ line starts with 
a '\\' (so the git-apply code actually depends on knowing that the patch 
text is dense, and that it's also padded out so that you can look one byte 
past the end of the diff and it won't be a '\\').

I don't know how well that fits into xpatch (I never looked at the patch 
side, since I already had my own ;), but my point being that handling this 
special case _can_ be very simple if the data structures are just set up 
for it.

It's also important to realize that (a) you can't actually check the "No 
newline" string, because that depends on your locale and (b) it's not 
necessarily at the _end_ of the patch, because you can have a patch that 
looks like

	-	next-to-last-line
	-	last-line
	\ No newline at end of file
	+	new-end-of-file
	+	new-last-line
	\ No newline at end of file

ie the first "\ No newline" is in the middle, because it relates to the 
last removed line (while the second one obviously relates to the last 
added one).

The xdiff patch I sent out automatically does that when generating these 
things, of course.

			Linus

^ permalink raw reply

* Re: Use a *real* built-in diff generator
From: Junio C Hamano @ 2006-03-25 19:52 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git, Davide Libenzi
In-Reply-To: <Pine.LNX.4.64.0603250742340.15714@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> On Sat, 25 Mar 2006, Linus Torvalds wrote:
>> 
>> I'll be taking a look at trying to fix it. 
>
> Actually, it ended up being easier than I expected it to be.
>
> This (on top of the previous patch) should fix it.
>...
> +	if (!size || rec[size-1] != '\n') {
> +		mb[2].ptr = "\n\\ No newline at end of file\n";
>... 

Thanks.  Here is what I cooked last night, which I rebased
on top of your two patches, to:

 - Punt "binary" diff like GNU does;

 - Minimally support GIT_DIFF_OPTS to allow passing -u0;

 - Omit ---/+++ lines when we do not have any diff output;

 - Adjust the tests to "@@ -k,l +m,n @@" form.

 - Adjust the tests to lack of "diff -p".

The first two are hacks but I think they are good enough hacks
for real life.  The third one is a real fix.  I agree with you
that libxdiff form is more rational, and that is the
justification for the fourth point.

-- >8 --
diff --git a/diff.c b/diff.c
index f6a1f5d..cd2ce0f 100644
--- a/diff.c
+++ b/diff.c
@@ -212,13 +212,34 @@
 	return 0;
 }
 
+struct emit_callback {
+	const char **label_path;
+};
+
 static int fn_out(void *priv, mmbuffer_t *mb, int nbuf)
 {
 	int i;
+	struct emit_callback *ecbdata = priv;
 
+	if (ecbdata->label_path[0]) {
+		printf("--- %s\n", ecbdata->label_path[0]);
+		printf("+++ %s\n", ecbdata->label_path[1]);
+		ecbdata->label_path[0] = ecbdata->label_path[1] = NULL;
+	}
 	for (i = 0; i < nbuf; i++)
 		if (!fwrite(mb[i].ptr, mb[i].size, 1, stdout))
 			return -1;
+	return 0;
+}
+
+#define FIRST_FEW_BYTES 8000
+static int mmfile_is_binary(mmfile_t *mf)
+{
+	long sz = mf->size;
+	if (FIRST_FEW_BYTES < sz)
+		sz = FIRST_FEW_BYTES;
+	if (memchr(mf->ptr, 0, sz))
+		return 1;
 	return 0;
 }
 
@@ -306,22 +327,32 @@
 	if (label_path[1][0] != '/')
 		label_path[1] = quote_two("b/", name_b);
 
-	printf("--- %s\n", label_path[0]);
-	printf("+++ %s\n", label_path[1]);
-
 	if (fill_mmfile(&mf1, temp[0].name) < 0 ||
 	    fill_mmfile(&mf2, temp[1].name) < 0)
 		die("unable to read files to diff");
 
-	/* Crazy xdl interfaces.. */
-	{
+	if (mmfile_is_binary(&mf1) || mmfile_is_binary(&mf2))
+		printf("Binary files %s and %s differ\n",
+		       label_path[0], label_path[1]);
+	else {
+		/* Crazy xdl interfaces.. */
+		const char *diffopts = getenv("GIT_DIFF_OPTS");
 		xpparam_t xpp;
 		xdemitconf_t xecfg;
 		xdemitcb_t ecb;
+		struct emit_callback ecbdata;
 
+		ecbdata.label_path = label_path;
 		xpp.flags = XDF_NEED_MINIMAL;
 		xecfg.ctxlen = 3;
+		if (!diffopts)
+			;
+		else if (!strncmp(diffopts, "--unified=", 10))
+			xecfg.ctxlen = strtoul(diffopts + 10, NULL, 10);
+		else if (!strncmp(diffopts, "-u", 2))
+			xecfg.ctxlen = strtoul(diffopts + 2, NULL, 10);
 		ecb.outf = fn_out;
+		ecb.priv = &ecbdata;
 		xdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
 	}
 
diff --git a/t/t1200-tutorial.sh b/t/t1200-tutorial.sh
index 1002413..5ac373f 100755
--- a/t/t1200-tutorial.sh
+++ b/t/t1200-tutorial.sh
@@ -22,7 +22,7 @@
 index 557db03..263414f 100644
 --- a/hello
 +++ b/hello
-@@ -1 +1,2 @@
+@@ -1,1 +1,2 @@
  Hello World
 +It's a new day for git
 EOF
@@ -60,14 +60,14 @@
 index 0000000..f24c74a
 --- /dev/null
 +++ b/example
-@@ -0,0 +1 @@
+@@ -0,0 +1,1 @@
 +Silly example
 diff --git a/hello b/hello
 new file mode 100644
 index 0000000..557db03
 --- /dev/null
 +++ b/hello
-@@ -0,0 +1 @@
+@@ -0,0 +1,1 @@
 +Hello World
 EOF
 
diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
index 2e3c20d..08c1131 100755
--- a/t/t4001-diff-rename.sh
+++ b/t/t4001-diff-rename.sh
@@ -49,7 +49,7 @@
 rename to path1
 --- a/path0
 +++ b/path1
-@@ -8,7 +8,7 @@ Line 7
+@@ -8,7 +8,7 @@
  Line 8
  Line 9
  Line 10
diff --git a/t/t4003-diff-rename-1.sh b/t/t4003-diff-rename-1.sh
index 2751970..af744fd 100755
--- a/t/t4003-diff-rename-1.sh
+++ b/t/t4003-diff-rename-1.sh
@@ -36,7 +36,7 @@
 copy to COPYING.1
 --- a/COPYING
 +++ b/COPYING.1
-@@ -6 +6 @@
+@@ -6,1 +6,1 @@
 - HOWEVER, in order to allow a migration to GPLv3 if that seems like
 + However, in order to allow a migration to GPLv3 if that seems like
 diff --git a/COPYING b/COPYING.2
@@ -44,13 +44,13 @@
 rename to COPYING.2
 --- a/COPYING
 +++ b/COPYING.2
-@@ -2 +2 @@
+@@ -2,1 +2,1 @@
 - Note that the only valid version of the GPL as far as this project
 + Note that the only valid version of the G.P.L as far as this project
-@@ -6 +6 @@
+@@ -6,1 +6,1 @@
 - HOWEVER, in order to allow a migration to GPLv3 if that seems like
 + HOWEVER, in order to allow a migration to G.P.Lv3 if that seems like
-@@ -12 +12 @@
+@@ -12,1 +12,1 @@
 -	This file is licensed under the GPL v2, or a later version
 +	This file is licensed under the G.P.L v2, or a later version
 EOF
@@ -74,13 +74,13 @@
 diff --git a/COPYING b/COPYING
 --- a/COPYING
 +++ b/COPYING
-@@ -2 +2 @@
+@@ -2,1 +2,1 @@
 - Note that the only valid version of the GPL as far as this project
 + Note that the only valid version of the G.P.L as far as this project
-@@ -6 +6 @@
+@@ -6,1 +6,1 @@
 - HOWEVER, in order to allow a migration to GPLv3 if that seems like
 + HOWEVER, in order to allow a migration to G.P.Lv3 if that seems like
-@@ -12 +12 @@
+@@ -12,1 +12,1 @@
 -	This file is licensed under the GPL v2, or a later version
 +	This file is licensed under the G.P.L v2, or a later version
 diff --git a/COPYING b/COPYING.1
@@ -88,7 +88,7 @@
 copy to COPYING.1
 --- a/COPYING
 +++ b/COPYING.1
-@@ -6 +6 @@
+@@ -6,1 +6,1 @@
 - HOWEVER, in order to allow a migration to GPLv3 if that seems like
 + However, in order to allow a migration to GPLv3 if that seems like
 EOF
@@ -116,7 +116,7 @@
 copy to COPYING.1
 --- a/COPYING
 +++ b/COPYING.1
-@@ -6 +6 @@
+@@ -6,1 +6,1 @@
 - HOWEVER, in order to allow a migration to GPLv3 if that seems like
 + However, in order to allow a migration to GPLv3 if that seems like
 EOF
diff --git a/t/t4004-diff-rename-symlink.sh b/t/t4004-diff-rename-symlink.sh
index a23aaa0..04f0147 100755
--- a/t/t4004-diff-rename-symlink.sh
+++ b/t/t4004-diff-rename-symlink.sh
@@ -40,7 +40,7 @@
 new file mode 120000
 --- /dev/null
 +++ b/bozbar
-@@ -0,0 +1 @@
+@@ -0,0 +1,1 @@
 +xzzzy
 \ No newline at end of file
 diff --git a/frotz b/nitfol
@@ -55,7 +55,7 @@
 deleted file mode 100644
 --- a/yomin
 +++ /dev/null
-@@ -1 +0,0 @@
+@@ -1,1 +0,0 @@
 -xyzzy
 \ No newline at end of file
 EOF
diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh
index 379a831..36ac16d 100755
--- a/t/t4011-diff-symlink.sh
+++ b/t/t4011-diff-symlink.sh
@@ -15,7 +15,7 @@
 index 0000000..7c465af
 --- /dev/null
 +++ b/frotz
-@@ -0,0 +1 @@
+@@ -0,0 +1,1 @@
 +xyzzy
 \ No newline at end of file
 EOF
@@ -41,7 +41,7 @@
 index 7c465af..0000000
 --- a/frotz
 +++ /dev/null
-@@ -1 +0,0 @@
+@@ -1,1 +0,0 @@
 -xyzzy
 \ No newline at end of file
 EOF
@@ -68,7 +68,7 @@
 index 7c465af..df1db54 120000
 --- a/frotz
 +++ b/frotz
-@@ -1 +1 @@
+@@ -1,1 +1,1 @@
 -xyzzy
 \ No newline at end of file
 +yxyyz

^ permalink raw reply related

* Re: [PATCH] cogito: Avoid slowness when timewarping large trees.
From: Junio C Hamano @ 2006-03-25 19:55 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20060325093641.GA26284@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Fri, Mar 24, 2006 at 11:43:52AM -0500, Shawn Pearce wrote:
>
>> Now that I think about it isn't this sort of where you were before
>> in cg-seek?
>
> Yes, that's basically it. Short of Junio explaining how the manual file
> removal can be avoided,...

Please don't let me slow you down -- that "git-read-tree --reset"
one was just confused.  I was not thinking clearly about the
modification you already had in the working tree which you
wanted to temporarily reset while switching.

^ permalink raw reply

* Re: Use a *real* built-in diff generator
From: Junio C Hamano @ 2006-03-25 20:17 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: git, Davide Libenzi
In-Reply-To: <Pine.LNX.4.64.0603250742340.15714@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> On Sat, 25 Mar 2006, Linus Torvalds wrote:
>> 
>> I'll be taking a look at trying to fix it. 
>
> Actually, it ended up being easier than I expected it to be.
>
> This (on top of the previous patch) should fix it.

This is a replacement for my previous one, which reduces the
changes to the testsuite.

I'll find time to omit prepare_temp_file() calls from diffcore
emit routines over the weekend.  Another useful project would be
to redo the combine-diff.c using the real built-in diff.

-- >8 --
[PATCH] built-in diff: minimum tweaks

This fixes up a couple of minor issues with the real built-in
diff to be more usable:

 - Omit ---/+++ header unless we emit diff output;

 - Detect and punt binary diff like GNU does;

 - Honor GIT_DIFF_OPTS minimally (only -u<number> and
   --unified=<number> are currently supported);

 - Omit line count of 1 from "@@ -l,k +m,n @@" hunk header
   (i.e. when k == 1 or n == 1)

 - Adjust testsuite for the lack of -p support.

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

 diff.c                 |   41 ++++++++++++++++++++++++++++++++++++-----
 t/t4001-diff-rename.sh |    2 +-
 xdiff/xutils.c         |   16 ++++++++++------
 3 files changed, 47 insertions(+), 12 deletions(-)

6ff2f393f16e56a5d3d611b4d3e8f039994ce0a8
diff --git a/diff.c b/diff.c
index f6a1f5d..cd2ce0f 100644
--- a/diff.c
+++ b/diff.c
@@ -212,16 +212,37 @@ static int fill_mmfile(mmfile_t *mf, con
 	return 0;
 }
 
+struct emit_callback {
+	const char **label_path;
+};
+
 static int fn_out(void *priv, mmbuffer_t *mb, int nbuf)
 {
 	int i;
+	struct emit_callback *ecbdata = priv;
 
+	if (ecbdata->label_path[0]) {
+		printf("--- %s\n", ecbdata->label_path[0]);
+		printf("+++ %s\n", ecbdata->label_path[1]);
+		ecbdata->label_path[0] = ecbdata->label_path[1] = NULL;
+	}
 	for (i = 0; i < nbuf; i++)
 		if (!fwrite(mb[i].ptr, mb[i].size, 1, stdout))
 			return -1;
 	return 0;
 }
 
+#define FIRST_FEW_BYTES 8000
+static int mmfile_is_binary(mmfile_t *mf)
+{
+	long sz = mf->size;
+	if (FIRST_FEW_BYTES < sz)
+		sz = FIRST_FEW_BYTES;
+	if (memchr(mf->ptr, 0, sz))
+		return 1;
+	return 0;
+}
+
 static const char *builtin_diff(const char *name_a,
 			 const char *name_b,
 			 struct diff_tempfile *temp,
@@ -306,22 +327,32 @@ static const char *builtin_diff(const ch
 	if (label_path[1][0] != '/')
 		label_path[1] = quote_two("b/", name_b);
 
-	printf("--- %s\n", label_path[0]);
-	printf("+++ %s\n", label_path[1]);
-
 	if (fill_mmfile(&mf1, temp[0].name) < 0 ||
 	    fill_mmfile(&mf2, temp[1].name) < 0)
 		die("unable to read files to diff");
 
-	/* Crazy xdl interfaces.. */
-	{
+	if (mmfile_is_binary(&mf1) || mmfile_is_binary(&mf2))
+		printf("Binary files %s and %s differ\n",
+		       label_path[0], label_path[1]);
+	else {
+		/* Crazy xdl interfaces.. */
+		const char *diffopts = getenv("GIT_DIFF_OPTS");
 		xpparam_t xpp;
 		xdemitconf_t xecfg;
 		xdemitcb_t ecb;
+		struct emit_callback ecbdata;
 
+		ecbdata.label_path = label_path;
 		xpp.flags = XDF_NEED_MINIMAL;
 		xecfg.ctxlen = 3;
+		if (!diffopts)
+			;
+		else if (!strncmp(diffopts, "--unified=", 10))
+			xecfg.ctxlen = strtoul(diffopts + 10, NULL, 10);
+		else if (!strncmp(diffopts, "-u", 2))
+			xecfg.ctxlen = strtoul(diffopts + 2, NULL, 10);
 		ecb.outf = fn_out;
+		ecb.priv = &ecbdata;
 		xdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
 	}
 
diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
index 2e3c20d..08c1131 100755
--- a/t/t4001-diff-rename.sh
+++ b/t/t4001-diff-rename.sh
@@ -49,7 +49,7 @@ rename from path0
 rename to path1
 --- a/path0
 +++ b/path1
-@@ -8,7 +8,7 @@ Line 7
+@@ -8,7 +8,7 @@
  Line 8
  Line 9
  Line 10
diff --git a/xdiff/xutils.c b/xdiff/xutils.c
index b68afa2..8221806 100644
--- a/xdiff/xutils.c
+++ b/xdiff/xutils.c
@@ -245,20 +245,24 @@ int xdl_emit_hunk_hdr(long s1, long c1, 
 
 	nb += xdl_num_out(buf + nb, c1 ? s1: 0);
 
-	memcpy(buf + nb, ",", 1);
-	nb += 1;
+	if (c1 != 1) {
+		memcpy(buf + nb, ",", 1);
+		nb += 1;
 
-	nb += xdl_num_out(buf + nb, c1);
+		nb += xdl_num_out(buf + nb, c1);
+	}
 
 	memcpy(buf + nb, " +", 2);
 	nb += 2;
 
 	nb += xdl_num_out(buf + nb, c2 ? s2: 0);
 
-	memcpy(buf + nb, ",", 1);
-	nb += 1;
+	if (c2 != 1) {
+		memcpy(buf + nb, ",", 1);
+		nb += 1;
 
-	nb += xdl_num_out(buf + nb, c2);
+		nb += xdl_num_out(buf + nb, c2);
+	}
 
 	memcpy(buf + nb, " @@\n", 4);
 	nb += 4;
-- 
1.2.4.g88d9

^ permalink raw reply related

* Re: [PATCH 4/4] send-email: add support for mutt aliases files
From: Junio C Hamano @ 2006-03-25 20:31 UTC (permalink / raw)
  To: Eric Wong; +Cc: git
In-Reply-To: <11432834111445-git-send-email-normalperson@yhbt.net>

Eric Wong <normalperson@yhbt.net> writes:

> More email clients/address book formats can easily be supported
> in the future.

> +if (my $mutt_aliases = `git-repo-config sendemail.muttaliases`) {
> +    chomp $mutt_aliases;
> +    open my $ma, '<', $mutt_aliases or die "opening $mutt_aliases: $!\n";
> +    while (<$ma>) { if (/^alias\s+(\S+)\s+(.*)/) { $aliases{$1} = $2 } }
> +    close $ma;
> +}
> +# aliases for more mail clients can be supported here:
> +

I'd rather avoid proliferation of sendemail.{foo,bar,mutt,pine,...}aliases
variables.  Can we autodetect the alias file format and parse
the given file accordingly?

^ permalink raw reply

* Re: [PATCH 3/4] send-email: lazy-load Email::Valid and make it optional
From: Junio C Hamano @ 2006-03-25 20:33 UTC (permalink / raw)
  To: Eric Wong; +Cc: git
In-Reply-To: <11432834111972-git-send-email-normalperson@yhbt.net>

Eric Wong <normalperson@yhbt.net> writes:

> +my $have_email_valid = eval { require Email::Valid or undef };

Merlyn already commented on this and I'd appreciate this part to
be redone.

^ permalink raw reply

* Re: Use a *real* built-in diff generator
From: Linus Torvalds @ 2006-03-25 20:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Davide Libenzi
In-Reply-To: <7vslp6uvn1.fsf@assigned-by-dhcp.cox.net>



On Sat, 25 Mar 2006, Junio C Hamano wrote:
> 
> This is a replacement for my previous one, which reduces the
> changes to the testsuite.

Looks good. With this, I think it's certainly ready for the "next" branch, 
and with some testing should be quite mergable into the main branch too.

I really didn't have to mess up the libxdiff files very much, so I don't 
think I introduced any new bugs, and libxdiff itself seems to be pretty 
stable (and that's partly from running the tests, but even more from just 
looking at the code and not having any 'Ewww! Gross!'-moments).

Which is not to say that there couldn't be bugs, but I think this is very 
much worth merging quickly..

> I'll find time to omit prepare_temp_file() calls from diffcore
> emit routines over the weekend.  Another useful project would be
> to redo the combine-diff.c using the real built-in diff.

I think the pickaxe improvements would be an even bigger thing..

		Linus

^ permalink raw reply

* Re: Use a *real* built-in diff generator
From: Linus Torvalds @ 2006-03-25 21:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Davide Libenzi
In-Reply-To: <7vslp6uvn1.fsf@assigned-by-dhcp.cox.net>



On Sat, 25 Mar 2006, Junio C Hamano wrote:
> 
> This is a replacement for my previous one, which reduces the
> changes to the testsuite.

Btw, your comment about the GNU diff @@-line simplification made me look 
at that again.

git-apply used to believe that a simplified line (ie with a single value) 
meant "x,x". While it seems that it really means "x,1".

Now, at worst, this would probably mean that some patches would get 
rejected (rather than mis-applied), so I think we've just never hit it. 
Also, with the default 3-line context I think that it simply _cannot_ be 
hit (since the only way that the result or source would have only one line 
in the resulting is it was the first line).

But if the "x" shorthand really means "x,1" (and I think you're right - 
using "-U 1" I can get that kind of shorthand) then apply.c should be 
fixed as follows (actual patch: removal of two characters).

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---
diff --git a/apply.c b/apply.c
index 2da225a..e80cd15 100644
--- a/apply.c
+++ b/apply.c
@@ -693,7 +693,7 @@ static int parse_range(const char *line,
 	line += digits;
 	len -= digits;
 
-	*p2 = *p1;
+	*p2 = 1;
 	if (*line == ',') {
 		digits = parse_num(line+1, p2);
 		if (!digits)

^ permalink raw reply related

* starting completly new repository
From: Grzegorz Kulewski @ 2006-03-25 21:06 UTC (permalink / raw)
  To: git

Hi,

First sorry if it is anwsered somewhere in the docs, but I am just 
(slowly) learning how to use git and reading the documentation. Any 
pointers to some article or tutorial that anwsers my questions will be 
appreciated.

What I am trying to do is to construct several repositories for the 
following work order:

1. There is some remote repo that is the main repository (call it main). 
It lives in some server and geneerally only one or two persons can push 
into it. All others can only pull from it.

2. Everybody has his own repo derived from main and she/he can make 
patches and send it to everybody else. One person is responsible for 
taking the patches and pushing them into main.

I need this set up fast and want to know how to do it. What I tried is 
basically:

(on the server)
mkdir main
git-init-db
touch .git/git-daemon-export-ok

(on my computer)
git-clone git://host/main main

But it looks like I must first do some commit on the server? But I can not 
make empty commit just to have things started? Or maybe there is some 
other way...

Also I wonder if I can do push over git protocol or I must use real ssh 
account on the server? This is not clear from the docs... At least not for 
me. How should I set up my repo (on my computer) to be able to push 
commits into main repo?

Also what should I set up additionally? How can I easily set author name 
and email for each repo? What is the difference between author and 
commiter and how should I set this up here?

Is there any documentation about git config file? Can I set author name, 
email and preffered editor in it or must I use environment?

Is there some irc channel for asking dumb questions as above and having 
them anwsered fast or should I use this mailing list?


Thanks in advance,

Grzegorz Kulewski

^ permalink raw reply

* More "git-apply" safety fixes
From: Linus Torvalds @ 2006-03-25 21:28 UTC (permalink / raw)
  To: Junio C Hamano, Git Mailing List


This was triggered by me testing the "@@" numbering shorthand by GNU 
patch, which not only showed that git-apply thought it meant the number 
was duplicated (when it means that the second number is 1), but my tests 
showed than when git-apply mis-understood the number, it would then not 
raise an alarm about it if the patch ended early.

Now, this doesn't actually _matter_, since with a three-line context, the 
only case that "x,1" will be shorthanded to "x" is when x itself is 1 (in 
which case git-apply got it right), but the fact that git-apply would also 
silently accept truncated patches was a missed opportunity for additional 
sanity-checking.

So make git-apply refuse to look at a patch fragment that ends early.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
----

If you already applied the first hunk, just ignore that part, and apply 
the second one that adds the test for oldlines/newlines being zero after 
the patch.

Neither of these two changes should matter for a well-formed patch, but 
it's definitely the right thing to do.

diff --git a/apply.c b/apply.c
index 2da225a..c50b3a6 100644
--- a/apply.c
+++ b/apply.c
@@ -693,7 +693,7 @@ static int parse_range(const char *line,
 	line += digits;
 	len -= digits;
 
-	*p2 = *p1;
+	*p2 = 1;
 	if (*line == ',') {
 		digits = parse_num(line+1, p2);
 		if (!digits)
@@ -901,6 +901,8 @@ static int parse_fragment(char *line, un
 			break;
 		}
 	}
+	if (oldlines || newlines)
+		return -1;
 	/* If a fragment ends with an incomplete line, we failed to include
 	 * it in the above loop because we hit oldlines == newlines == 0
 	 * before seeing it.

^ permalink raw reply related

* Re: starting completly new repository
From: sean @ 2006-03-25 21:45 UTC (permalink / raw)
  To: Grzegorz Kulewski; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0603252148550.14361@alpha.polcom.net>

On Sat, 25 Mar 2006 22:06:42 +0100 (CET)
Grzegorz Kulewski <kangur@polcom.net> wrote:

> (on the server)
> mkdir main
> git-init-db
> touch .git/git-daemon-export-ok
> 
> (on my computer)
> git-clone git://host/main main
> 
> But it looks like I must first do some commit on the server? But I can not 
> make empty commit just to have things started? Or maybe there is some 
> other way...

Did you actually start the git-daemon on the server?
 
> Also I wonder if I can do push over git protocol or I must use real ssh 
> account on the server? This is not clear from the docs... At least not for 
> me. How should I set up my repo (on my computer) to be able to push 
> commits into main repo?

You can't commit over the git protocol, you'll need to use ssh.  
No need to do anything to your repo in order to push.

> Also what should I set up additionally? How can I easily set author name 
> and email for each repo? 

See below.

> What is the difference between author and commiter and how should I 
> set this up here?

Just what you might imagine, the person who created the patch and
the person who entered it into the repository respectively.
 
> Is there any documentation about git config file? Can I set author name, 
> email and preffered editor in it or must I use environment?

$ git repo-config user.email "user@email.com"
$ git repo-config user.name  "full name"

use the EDITOR environment variable to choose your desired editor.

> Is there some irc channel for asking dumb questions as above and having 
> them anwsered fast or should I use this mailing list?
 
#git on irc.freenode.net

Sean

^ permalink raw reply

* Re: starting completly new repository
From: Petr Baudis @ 2006-03-25 21:49 UTC (permalink / raw)
  To: Grzegorz Kulewski; +Cc: git
In-Reply-To: <Pine.LNX.4.63.0603252148550.14361@alpha.polcom.net>

Hi,

Dear diary, on Sat, Mar 25, 2006 at 10:06:42PM CET, I got a letter
where Grzegorz Kulewski <kangur@polcom.net> said that...
> First sorry if it is anwsered somewhere in the docs, but I am just 
> (slowly) learning how to use git and reading the documentation. Any 
> pointers to some article or tutorial that anwsers my questions will be 
> appreciated.

well, there's Documentation/tutorial.txt, but it doesn't cover your
questions, it looks.

> But it looks like I must first do some commit on the server? But I can not 
> make empty commit just to have things started? Or maybe there is some 
> other way...

You can of course make an empty commit, but it seems strange that you
would want that - won't the people start working on some initial version
of the project?

> Also I wonder if I can do push over git protocol or I must use real ssh 
> account on the server? This is not clear from the docs... At least not for 
> me. How should I set up my repo (on my computer) to be able to push 
> commits into main repo?

You must use real ssh account on the server. You can limit the account
to be able to do only git pushes/pulls by setting its shell to
git-shell.

> Also what should I set up additionally? How can I easily set author name 
> and email for each repo? What is the difference between author and 
> commiter and how should I set this up here?

 From Cogito's cg-commit documentation:

Each commit has two user identification fields - commit author and
committer.  By default, it is recorded that you authored the commit, but
it is considered a good practice to change this to the actual author of
the change if you are merely applying someone else's patch. It is always
recorded that you were the patch committer.

> Is there any documentation about git config file? Can I set author name, 
> email and preffered editor in it or must I use environment?

See git-commit-tree(1), the COMMIT INFORMATION section.

One big problem with Git documentation (other than e.g. falling
out-of-date occassionaly) is that information is scattered between
porcelain and plumbing or even between several commands (e.g.
git-whatchanged(1) which doesn't even directly mention where to gather
the full list of available options).

Thankfully, the plumbing documentation is mostly coherent. ;-)

> Is there some irc channel for asking dumb questions as above and having 
> them anwsered fast or should I use this mailing list?

#git on Freenode (surprisingly ;)

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Right now I am having amnesia and deja-vu at the same time.  I think
I have forgotten this before.

^ permalink raw reply

* Why would merge fail on a one-line addition?
From: Marc Singer @ 2006-03-25 22:26 UTC (permalink / raw)
  To: Git Mailing List

One of the unmerged files leaves this trail.

  elf@florence ~...git/linux-2.6 > git-ls-files --unmerged
  100644 6262d449120cdcde5db1b079806dcc0d9b5e6b7c 1       arch/arm/mach-lh7a40x/irq-lpd7a40x.c
  100644 dcb4e17b941990eabe8992680c9aa9b67afb6fd4 3       arch/arm/mach-lh7a40x/irq-lpd7a40x.c

  elf@florence ~...git/linux-2.6 > git-cat-file blob 6262d449120cdcde5db1b079806dcc0d9b5e6b7c > a
  elf@florence ~...git/linux-2.6 > git-cat-file blob dcb4e17b941990eabe8992680c9aa9b67afb6fd4 > b
  elf@florence ~...git/linux-2.6 > diff a b                                       21a22
  > #include "common.h"

Why would git have a problem with this?

^ permalink raw reply

* [PATCH]: git-svnimport: if a limit is specified, respect it
From: Anand Kumria @ 2006-03-25 22:43 UTC (permalink / raw)
  To: git; +Cc: Anand Kumria


Hi,

git-svnimport will import the same revision over and over again if a
limit (-l <rev>) has been specified. Instead if that revision has already
been processed, exit with an up-to-date message.

Please apply.

Thanks,
Anand

Signed-off-by: Anand Kumria <wildfire@progsoc.org>


---

 git-svnimport.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

66e05a5854eb269e3e61fcaa9f53a4b2a45b17d8
diff --git a/git-svnimport.perl b/git-svnimport.perl
index 639aa41..114784f 100755
--- a/git-svnimport.perl
+++ b/git-svnimport.perl
@@ -851,7 +851,7 @@ sub commit_all {
 
 $opt_l = $svn->{'maxrev'} if not defined $opt_l or $opt_l > $svn->{'maxrev'};
 
-if ($svn->{'maxrev'} < $current_rev) {
+if ($opt_l < $current_rev) {
     print "Up to date: no new revisions to fetch!\n" if $opt_v;
     unlink("$git_dir/SVN2GIT_HEAD");
     exit;
-- 
1.2.4

^ permalink raw reply related

* [PATCH] Do not print header in diff-tree --root unless asked to
From: Petr Baudis @ 2006-03-25 23:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Currently cg-log -f is broken (shows sha1 in files list for the initial
commit) since git-diff-tree would always return the sha1 of the commit
when --root was passed. I assume it should do this only when -v was also
passed; I'm certain that I don't want it when processing the output.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 diff-tree.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/diff-tree.c b/diff-tree.c
index f55a35a..8d82b5b 100644
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -107,7 +107,8 @@ static int diff_tree_commit(struct commi
 
 	/* Root commit? */
 	if (show_root_diff && !commit->parents) {
-		header = generate_header(sha1, NULL, commit);
+		if (verbose_header)
+			header = generate_header(sha1, NULL, commit);
 		diff_root_tree(sha1, "");
 	}
 

^ permalink raw reply related

* Re: [PATCH 4/4] send-email: add support for mutt aliases files
From: Ryan Anderson @ 2006-03-25 23:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Eric Wong, git
In-Reply-To: <7vfyl6uuzt.fsf@assigned-by-dhcp.cox.net>

On Sat, Mar 25, 2006 at 12:31:18PM -0800, Junio C Hamano wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > More email clients/address book formats can easily be supported
> > in the future.
> 
> > +if (my $mutt_aliases = `git-repo-config sendemail.muttaliases`) {
> > +    chomp $mutt_aliases;
> > +    open my $ma, '<', $mutt_aliases or die "opening $mutt_aliases: $!\n";
> > +    while (<$ma>) { if (/^alias\s+(\S+)\s+(.*)/) { $aliases{$1} = $2 } }
> > +    close $ma;
> > +}
> > +# aliases for more mail clients can be supported here:
> > +
> 
> I'd rather avoid proliferation of sendemail.{foo,bar,mutt,pine,...}aliases
> variables.  Can we autodetect the alias file format and parse
> the given file accordingly?

Don't bother - instead of lots of variables, just have two:
	sendemail.aliasesfile
	sendemail.aliasfiletype

-- 

Ryan Anderson
  sometimes Pug Majere

^ permalink raw reply

* Re: [PATCH 1/4] send-email: Change from Mail::Sendmail to Net::SMTP
From: Ryan Anderson @ 2006-03-25 23:58 UTC (permalink / raw)
  To: Eric Wong; +Cc: Junio C Hamano, git, Greg KH
In-Reply-To: <11432834102700-git-send-email-normalperson@yhbt.net>

On Sat, Mar 25, 2006 at 02:43:30AM -0800, Eric Wong wrote:
> Net::SMTP is in the base Perl distribution, so users are more
> likely to have it.  Net::SMTP also allows reusing the SMTP
> connection, so sending multiple emails is faster.

Overall, I like this set of cleanups, just one thing struck me as,
"why?"

>  	if ($quiet) {
> -		printf "Sent %s\n", $subject;
> +		print "Sent $subject\n";

This seems to be a pointless change, and actually, might be long-term
counterproductive.

Assumption: Eventually, we're going to want to internationalize git.

If that is true, we'll eventually do something like this to lines like
that:
	printf( gettext("Send %s\n"), $subject);

The alternative:
	print gettext("Send $subject\n");
does not work.

(The line that xgettext will see is 'Send $subject\n', but when the
program actually runs, gettext will see the interpolated version, which
fails.)

Internationalization may still be a ways off, but I think we're reaching
the point where it might be something we care to think about.

-- 

Ryan Anderson
  sometimes Pug Majere

^ permalink raw reply

* Re: Why would merge fail on a one-line addition?
From: Linus Torvalds @ 2006-03-26  0:31 UTC (permalink / raw)
  To: Marc Singer; +Cc: Git Mailing List
In-Reply-To: <20060325222601.GA1500@buici.com>



On Sat, 25 Mar 2006, Marc Singer wrote:

> One of the unmerged files leaves this trail.
> 
>   elf@florence ~...git/linux-2.6 > git-ls-files --unmerged
>   100644 6262d449120cdcde5db1b079806dcc0d9b5e6b7c 1       arch/arm/mach-lh7a40x/irq-lpd7a40x.c
>   100644 dcb4e17b941990eabe8992680c9aa9b67afb6fd4 3       arch/arm/mach-lh7a40x/irq-lpd7a40x.c
> 
>   elf@florence ~...git/linux-2.6 > git-cat-file blob 6262d449120cdcde5db1b079806dcc0d9b5e6b7c > a
>   elf@florence ~...git/linux-2.6 > git-cat-file blob dcb4e17b941990eabe8992680c9aa9b67afb6fd4 > b
>   elf@florence ~...git/linux-2.6 > diff a b                                       21a22
>   > #include "common.h"
> 
> Why would git have a problem with this?

That whole file was apparently removed in the branch you are merging into 
(no stage 2). So what should the merge do? Throw away the one-liner 
addition (likely the correct thing) or maybe it should go somewhere else 
(ie maybe it wasn't removed outright, but the contents moved into another 
file, which would now need the one-line addition).

		Linus

^ permalink raw reply

* [PATCH] send-email: lazy-load Email::Valid and make it optional
From: Eric Wong @ 2006-03-26  0:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Eric Wong
In-Reply-To: <7v8xqyuuvo.fsf@assigned-by-dhcp.cox.net>

It's not installed on enough machines, and is overkill most of
the time.  We'll fallback to a very basic regexp just in case,
but nothing like the monster regexp Email::Valid has to offer :)

Small cleanup from Merlyn.

Signed-off-by: Eric Wong <normalperson@yhbt.net>

---

 git-send-email.perl |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

3f09a822e3871eeae521da80c748602862fc52ce
diff --git a/git-send-email.perl b/git-send-email.perl
index 5e08817..7cbf11d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -22,12 +22,12 @@ use Term::ReadLine;
 use Getopt::Long;
 use Data::Dumper;
 use Net::SMTP;
-use Email::Valid;
 
 # most mail servers generate the Date: header, but not all...
 $ENV{LC_ALL} = 'C';
 use POSIX qw/strftime/;
 
+my $have_email_valid = eval { require Email::Valid; 1 };
 my $smtp;
 
 sub unique_email_list(@);
@@ -250,6 +250,16 @@ EOT
 # Variables we set as part of the loop over files
 our ($message_id, $cc, %mail, $subject, $reply_to, $message);
 
+sub extract_valid_address {
+	my $address = shift;
+	if ($have_email_valid) {
+		return Email::Valid->address($address);
+	} else {
+		# less robust/correct than the monster regexp in Email::Valid,
+		# but still does a 99% job, and one less dependency
+		return ($address =~ /([^\"<>\s]+@[^<>\s]+)/);
+	}
+}
 
 # Usually don't need to change anything below here.
 
@@ -259,7 +269,7 @@ our ($message_id, $cc, %mail, $subject, 
 # 1 second since the last time we were called.
 
 # We'll setup a template for the message id, using the "from" address:
-my $message_id_from = Email::Valid->address($from);
+my $message_id_from = extract_valid_address($from);
 my $message_id_template = "<%s-git-send-email-$message_id_from>";
 
 sub make_message_id
@@ -412,7 +422,7 @@ sub unique_email_list(@) {
 	my @emails;
 
 	foreach my $entry (@_) {
-		my $clean = Email::Valid->address($entry);
+		my $clean = extract_valid_address($entry);
 		next if $seen{$clean}++;
 		push @emails, $entry;
 	}
-- 
1.2.4.gb622a

^ permalink raw reply related

* Re: [PATCH] Do not print header in diff-tree --root unless asked to
From: Junio C Hamano @ 2006-03-26  0:48 UTC (permalink / raw)
  To: Petr Baudis; +Cc: git
In-Reply-To: <20060325232807.9146.12846.stgit@machine.or.cz>

Petr Baudis <pasky@suse.cz> writes:

> ... git-diff-tree would always return the sha1 of the commit
> when --root was passed.

I am not sure why this change is needed.

The output from "git-diff-tree --root e83c51" (the very initial
"git") and "git-diff-tree 8bc9a0" (the second commit) without
any other parameters (specifically, there is no '-v') look
comparable right now, but I suspect this change would break it.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox