Re: [GIT PATCH] Selective diff-tree

Re: [GIT PATCH] Selective diff-tree

[Linus Torvalds]

On Wed, 13 Apr 2005, David Woodhouse wrote:
>
> The patch below makes diff-tree take extra arguments, specifying the
> files or directories which should be considered 'interesting'. Changes
> in uninteresting directories are not reported; we don't even bother to
> recurse down into those trees.

David, I took your patch, updated it for some changes in diff-tree, and
then totally rewrote (and simplified) your testing for "interesting".
There was no reason to consider "/" special, as it falls out of the other
cases quite naturally.

It passes all the tests I threw at it, but they weren't exhaustive (but I 
do think I tested the border cases). Mind double-checking that it works 
for your cases too?

		Linus

Re: [GIT PATCH] Selective diff-tree

[David Woodhouse]

On Sat, 2005-04-23 at 22:09 -0700, Linus Torvalds wrote:
> It passes all the tests I threw at it, but they weren't exhaustive (but I 
> do think I tested the border cases). Mind double-checking that it works 
> for your cases too?

It seems to work at least as well as my version did -- which is to say
it's fine for the handful of test cases I tried, which is probably about
the same as what you did.

On the other hand, my gitfilelog.sh is fairly fundamentally flawed and
probably needs rewriting in C based on rev-tree.

-- 
dwmw2

Re: [GIT PATCH] Selective diff-tree

[Linus Torvalds]

On Sun, 24 Apr 2005, David Woodhouse wrote:
> 
> On the other hand, my gitfilelog.sh is fairly fundamentally flawed and
> probably needs rewriting in C based on rev-tree.

Yeah, I think I'll make the "tree-diff" functions be available to others,
since they are actually _very_ cheap, and it's a bit sad to have to
execute a whole new process just to get a tree-diff. Then writing some
rev-list thing that just does tree-diff between different versions should
be trivial and quite efficient for finding things like "when did this set
of files/subdirectory last change".

Which definitely is very useful information.

		Linus

Re: [GIT PATCH] Selective diff-tree

[Junio C Hamano]

Is either of you planning to do the same for diff-cache?

Re: [GIT PATCH] Selective diff-tree

[Linus Torvalds]

On Sun, 24 Apr 2005, Junio C Hamano wrote:
>
> Is either of you planning to do the same for diff-cache?

I wasn't. I don't think it's nearly as relevant for diff-cache, since 
diff-cache is never run thousands of times.

IOW, with diff-cache you can just do

	diff-cache <tree> | grep "interesting"

and you're done.

Diff-tree is different, simply because diff-tree is the way you find out
which files changed between two revisions, so if you search for "when did
this file change" you may well have to do _thousands_ of different
diff-tree's. That's why pruning the diff-tree output is important: it
makes diff-tree _much_ cheaper.

diff-cache usually doesn't make sense to do against more than one or two
threes (the parent, and after a merge but before you commit maybe the
parent_s_).

		Linus

[PATCH 0/2] diff-tree/diff-cache helper

[Junio C Hamano]

I use a set of small scripts [*1*] directly on top of the core
git, which needed to make patches out of diff-tree and
diff-cache output.  Its output is compatible with what show-diff
produces.

Since this helper program would be generally useful for other
wrapper layer programs like Cogito, I am submitting it in two
parts.  The first part is to rip out the external diff program
interface part out of show-diff.c and moving it into a common
library.  The second part introduces the diff-tree-helper
program.

[Footnotes]

*1* This is found at http://members.cox.net/junkio/

[PATCH 1/2] Split external diff command interface to a separate file.

[Junio C Hamano]

With this patch, the non-core'ish part of show-diff command that
invokes an external "diff" comand to obtain patches is split
into a separate file.  The next patch will introduce a new
command, diff-tree-helper, which uses this common diff interface
to format diff-tree and diff-cache output into a patch form.

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

Makefile    |    4 ++
diff.c      |  106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
diff.h      |   17 +++++++++
show-diff.c |  101 +--------------------------------------------------------
4 files changed, 130 insertions(+), 98 deletions(-)

--- k/Makefile
+++ l/Makefile
@@ -27,6 +27,9 @@ LIB_OBJS=read-cache.o sha1_file.o usage.
 LIB_FILE=libgit.a
 LIB_H=cache.h object.h
 
+LIB_H += diff.h
+LIB_OBJS += diff.o
+
 LIBS = $(LIB_FILE)
 LIBS += -lz
 
@@ -66,6 +69,7 @@ checkout-cache.o: $(LIB_H)
 commit.o: $(LIB_H)
 commit-tree.o: $(LIB_H)
 convert-cache.o: $(LIB_H)
+diff.o: $(LIB_H)
 diff-cache.o: $(LIB_H)
 diff-tree.o: $(LIB_H)
 fsck-cache.o: $(LIB_H)
--- k/diff.c
+++ l/diff.c
@@ -0,0 +1,106 @@
+#include "cache.h"
+#include "diff.h"
+
+static char *diff_cmd = "diff -L 'k/%s' -L 'l/%s' ";
+static char *diff_opts = "-p -u";
+static char *diff_arg_forward  = " - '%s'";
+static char *diff_arg_reverse  = " '%s' -";
+
+void prepare_diff_cmd(void)
+{
+	/*
+	 * Default values above are meant to match the
+	 * Linux kernel development style.  Examples of
+	 * alternative styles you can specify via environment
+	 * variables are:
+	 *
+	 * GIT_DIFF_CMD="diff -L '%s' -L '%s'"
+	 * GIT_DIFF_OPTS="-c";
+	 */
+	diff_cmd = getenv("GIT_DIFF_CMD") ? : diff_cmd;
+	diff_opts = getenv("GIT_DIFF_OPTS") ? : diff_opts;
+}
+
+/* Help to copy the thing properly quoted for the shell safety.
+ * any single quote is replaced with '\'', and the caller is
+ * expected to enclose the result within a single quote pair.
+ *
+ * E.g.
+ *  original     sq_expand     result
+ *  name     ==> name      ==> 'name'
+ *  a b      ==> a b       ==> 'a b'
+ *  a'b      ==> a'\''b    ==> 'a'\''b'
+ */
+static char *sq_expand(const char *src)
+{
+	static char *buf = NULL;
+	int cnt, c;
+	const char *cp;
+	char *bp;
+
+	/* count bytes needed to store the quoted string. */ 
+	for (cnt = 1, cp = src; *cp; cnt++, cp++)
+		if (*cp == '\'')
+			cnt += 3;
+
+	if (! (buf = malloc(cnt)))
+	    return buf;
+	bp = buf;
+	while ((c = *src++)) {
+		if (c != '\'')
+			*bp++ = c;
+		else {
+			bp = strcpy(bp, "'\\''");
+			bp += 4;
+		}
+	}
+	*bp = 0;
+	return buf;
+}
+
+void show_differences(const char *name, /* filename on the filesystem */
+		      const char *label, /* diff label to use */
+		      void *old_contents, /* contents in core */
+		      unsigned long long old_size, /* size in core */
+		      int reverse /* 0: diff core file
+				     1: diff file core */)
+{
+	FILE *f;
+	char *name_sq = sq_expand(name);
+	const char *label_sq = (name != label) ? sq_expand(label) : name_sq;
+	char *diff_arg = reverse ? diff_arg_reverse : diff_arg_forward;
+	int cmd_size = strlen(name_sq) + strlen(label_sq) * 2 +
+		strlen(diff_cmd) + strlen(diff_opts) + strlen(diff_arg);
+	char *cmd = malloc(cmd_size);
+	int next_at;
+
+	fflush(stdout);
+	next_at = snprintf(cmd, cmd_size, diff_cmd, label_sq, label_sq);
+	next_at += snprintf(cmd+next_at, cmd_size-next_at, "%s", diff_opts);
+	next_at += snprintf(cmd+next_at, cmd_size-next_at, diff_arg, name_sq);
+	f = popen(cmd, "w");
+	if (old_size)
+		fwrite(old_contents, old_size, 1, f);
+	pclose(f);
+	if (label_sq != name_sq)
+		free((void*)label_sq); /* constness */
+	free(name_sq);
+	free(cmd);
+}
+
+void show_diff_empty(const unsigned char *sha1,
+		     const char *name,
+		     int reverse)
+{
+	char *old;
+	unsigned long int size;
+	unsigned char type[20];
+
+	old = read_sha1_file(sha1, type, &size);
+	if (! old) {
+		error("unable to read blob object for %s (%s)", name,
+		      sha1_to_hex(sha1));
+		return;
+	}
+	show_differences("/dev/null", name, old, size, reverse);
+}
--- k/diff.h
+++ l/diff.h
@@ -0,0 +1,17 @@
+#ifndef DIFF_H
+#define DIFF_H
+
+extern void prepare_diff_cmd(void);
+
+extern void show_differences(const char *name, /* filename on the filesystem */
+			     const char *label, /* diff label to use */
+			     void *old_contents, /* contents in core */
+			     unsigned long long old_size, /* size in core */
+			     int reverse /* 0: diff core file
+					    1: diff file core */);
+
+extern void show_diff_empty(const unsigned char *sha1,
+			    const char *name,
+			    int reverse);
+
+#endif /* DIFF_H */
--- k/show-diff.c
+++ l/show-diff.c
@@ -4,103 +4,7 @@
  * Copyright (C) Linus Torvalds, 2005
  */
 #include "cache.h"
-
-static char *diff_cmd = "diff -L 'a/%s' -L 'b/%s' ";
-static char *diff_opts = "-p -u";
-static char *diff_arg_forward  = " - '%s'";
-static char *diff_arg_reverse  = " '%s' -";
-
-static void prepare_diff_cmd(void)
-{
-	/*
-	 * Default values above are meant to match the
-	 * Linux kernel development style.  Examples of
-	 * alternative styles you can specify via environment
-	 * variables are:
-	 *
-	 * GIT_DIFF_CMD="diff -L '%s' -L '%s'"
-	 * GIT_DIFF_OPTS="-c";
-	 */
-	diff_cmd = getenv("GIT_DIFF_CMD") ? : diff_cmd;
-	diff_opts = getenv("GIT_DIFF_OPTS") ? : diff_opts;
-}
-
-/* Help to copy the thing properly quoted for the shell safety.
- * any single quote is replaced with '\'', and the caller is
- * expected to enclose the result within a single quote pair.
- *
- * E.g.
- *  original     sq_expand     result
- *  name     ==> name      ==> 'name'
- *  a b      ==> a b       ==> 'a b'
- *  a'b      ==> a'\''b    ==> 'a'\''b'
- */
-static char *sq_expand(char *src)
-{
-	static char *buf = NULL;
-	int cnt, c;
-	char *cp;
-
-	/* count bytes needed to store the quoted string. */ 
-	for (cnt = 1, cp = src; *cp; cnt++, cp++)
-		if (*cp == '\'')
-			cnt += 3;
-
-	if (! (buf = malloc(cnt)))
-	    return buf;
-	cp = buf;
-	while ((c = *src++)) {
-		if (c != '\'')
-			*cp++ = c;
-		else {
-			cp = strcpy(cp, "'\\''");
-			cp += 4;
-		}
-	}
-	*cp = 0;
-	return buf;
-}
-
-static void show_differences(char *name, char *label, void *old_contents,
-			     unsigned long long old_size, int reverse)
-{
-	FILE *f;
-	char *name_sq = sq_expand(name);
-	char *label_sq = (name != label) ? sq_expand(label) : name_sq;
-	char *diff_arg = reverse ? diff_arg_reverse : diff_arg_forward;
-	int cmd_size = strlen(name_sq) + strlen(label_sq) * 2 +
-		strlen(diff_cmd) + strlen(diff_opts) + strlen(diff_arg);
-	char *cmd = malloc(cmd_size);
-	int next_at;
-
-	fflush(stdout);
-	next_at = snprintf(cmd, cmd_size, diff_cmd, label_sq, label_sq);
-	next_at += snprintf(cmd+next_at, cmd_size-next_at, "%s", diff_opts);
-	next_at += snprintf(cmd+next_at, cmd_size-next_at, diff_arg, name_sq);
-	f = popen(cmd, "w");
-	if (old_size)
-		fwrite(old_contents, old_size, 1, f);
-	pclose(f);
-	if (label_sq != name_sq)
-		free(label_sq);
-	free(name_sq);
-	free(cmd);
-}
-
-static void show_diff_empty(struct cache_entry *ce, int reverse)
-{
-	char *old;
-	unsigned long int size;
-	unsigned char type[20];
-
-	old = read_sha1_file(ce->sha1, type, &size);
-	if (! old) {
-		error("unable to read blob object for %s (%s)", ce->name,
-		      sha1_to_hex(ce->sha1));
-		return;
-	}
-	show_differences("/dev/null", ce->name, old, size, reverse);
-}
+#include "diff.h"
 
 static const char *show_diff_usage = "show-diff [-q] [-s] [-z] [paths...]";
 
@@ -183,7 +87,8 @@ int main(int argc, char **argv)
 			else {
 				printf("%s: %s\n", ce->name, strerror(errno));
 				if (errno == ENOENT)
-					show_diff_empty(ce, reverse);
+					show_diff_empty(ce->sha1, ce->name,
+							reverse);
 			}
 			continue;
 		}

[PATCH 2/2] Introduce diff-tree-helper.

[Junio C Hamano]

This patch introduces a new program, diff-tree-helper.  It reads
output from diff-cache and diff-tree, and produces a patch file.
The diff format customization can be done the same way the
show-diff uses; the same external diff interface introduced by
the previous patch to drive diff from show-diff is used so this
is not surprising.

It is used like the following examples:

   $ diff-cache --cached -z <tree> | diff-tree-helper -z -R paths...
   $ diff-tree -r -z <tree1> <tree2> | diff-tree-helper -z paths...

 - As usual, the use of the -z flag is recommended in the script
   to pass NUL-terminated filenames through the pipe between
   commands.

 - The -R flag is used to generate reverse diff.  It does not
   matter for diff-tree case, but it is sometimes useful to get
   a patch in the desired direction out of diff-cache.

 - The paths parameters are used to restrict the paths that
   appears in the output.  Again this is useful to use with
   diff-cache, which, unlike diff-tree, does not take such paths
   restriction parameters.

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

Makefile           |    6 -
diff-tree-helper.c |  302 +++++++++++++++++++++++++++++++++++++++++++++++++++++
strbuf.c           |   43 +++++++
strbuf.h           |   13 ++
4 files changed, 363 insertions(+), 1 deletion(-)

--- k/Makefile
+++ l/Makefile
@@ -16,7 +16,8 @@ AR=ar
 PROG=   update-cache show-diff init-db write-tree read-tree commit-tree \
 	cat-file fsck-cache checkout-cache diff-tree rev-tree show-files \
 	check-files ls-tree merge-base merge-cache unpack-file git-export \
-	diff-cache convert-cache http-pull rpush rpull rev-list
+	diff-cache convert-cache http-pull rpush rpull rev-list \
+	diff-tree-helper
 
 all: $(PROG)
 
@@ -27,6 +28,9 @@ LIB_OBJS=read-cache.o sha1_file.o usage.
 LIB_FILE=libgit.a
 LIB_H=cache.h object.h
 
+LIB_H += strbuf.h
+LIB_OBJS += strbuf.o
+
 LIB_H += diff.h
 LIB_OBJS += diff.o
 
--- k/diff-tree-helper.c
+++ l/diff-tree-helper.c
@@ -0,0 +1,302 @@
+#include "cache.h"
+#include "strbuf.h"
+#include "diff.h"
+
+static int matches_pathspec(const char *name, char **spec, int cnt)
+{
+	int i;
+	int namelen = strlen(name);
+	for (i = 0; i < cnt; i++) {
+		int speclen = strlen(spec[i]);
+		if (! strncmp(spec[i], name, speclen) &&
+		    speclen <= namelen &&
+		    (name[speclen] == 0 ||
+		     name[speclen] == '/'))
+			return 1;
+	}
+	return 0;
+}
+
+static int parse_oneside_change(const char *cp, unsigned char *sha1,
+				char *path) {
+	int ch;
+	while ((ch = *cp) && '0' <= ch && ch <= '7')
+		cp++; /* skip mode bits */
+	if (strncmp(cp, "\tblob\t", 6))
+		return -1;
+	cp += 6;
+	if (get_sha1_hex(cp, sha1))
+		return -1;
+	cp += 40;
+	if (*cp++ != '\t')
+		return -1;
+	strcpy(path, cp);
+	return 0;
+}
+
+#define STATUS_CACHED    0 /* cached and sha1 valid */
+#define STATUS_ABSENT    1 /* diff-tree says old removed or new added */
+#define STATUS_UNCACHED  2 /* diff-cache output: read from working tree */
+
+static int parse_diff_tree_output(const char *buf,
+				  unsigned char *old_sha1,
+				  int *old_status,
+				  unsigned char *new_sha1,
+				  int *new_status,
+				  char *path) {
+	const char *cp = buf;
+	int ch;
+	static unsigned char null_sha[20] = { 0, };
+
+	switch (*cp++) {
+	case '+':
+		*old_status = STATUS_ABSENT;
+		*new_status = (memcmp(new_sha1, null_sha, sizeof(null_sha)) ?
+			       STATUS_CACHED : STATUS_UNCACHED);
+		return parse_oneside_change(cp, new_sha1, path);
+	case '-':
+		*new_status = STATUS_ABSENT;
+		*old_status = (memcmp(old_sha1, null_sha, sizeof(null_sha)) ?
+			       STATUS_CACHED : STATUS_UNCACHED);
+		return parse_oneside_change(cp, old_sha1, path);
+	case '*':
+		break;
+	default:
+		return -1;
+	}
+	
+	/* This is for '*' entries */
+	while ((ch = *cp) && ('0' <= ch && ch <= '7'))
+		cp++; /* skip mode bits */
+	if (strncmp(cp, "->", 2))
+		return -1;
+	cp += 2;
+	while ((ch = *cp) && ('0' <= ch && ch <= '7'))
+		cp++; /* skip mode bits */
+	if (strncmp(cp, "\tblob\t", 6))
+		return -1;
+	cp += 6;
+	if (get_sha1_hex(cp, old_sha1))
+		return -1;
+	cp += 40;
+	if (strncmp(cp, "->", 2))
+		return -1;
+	cp += 2;
+	if (get_sha1_hex(cp, new_sha1))
+		return -1;
+	cp += 40;
+	if (*cp++ != '\t')
+		return -1;
+	strcpy(path, cp);
+	*old_status = (memcmp(old_sha1, null_sha, sizeof(null_sha)) ?
+		       STATUS_CACHED : STATUS_UNCACHED);
+	*new_status = (memcmp(new_sha1, null_sha, sizeof(null_sha)) ?
+		       STATUS_CACHED : STATUS_UNCACHED);
+	return 0;
+}
+
+static int sha1err(const char *path, const unsigned char *sha1)
+{
+	return error("diff-tree-helper: unable to read sha1 file of %s (%s)",
+		     path, sha1_to_hex(sha1));
+}
+
+static int fserr(const char *path)
+{
+	return error("diff-tree-helper: unable to read file %s", path);
+}
+
+static char *map_whole_file(const char *path, unsigned long *size) {
+	int fd;
+	struct stat st;
+	void *buf;
+
+	if ((fd = open(path, O_RDONLY)) < 0) {
+		error("diff-tree-helper: unable to read file %s", path);
+		return 0;
+	}
+	if (fstat(fd, &st) < 0) {
+		close(fd);
+		error("diff-tree-helper: unable to stat file %s", path);
+		return 0;
+	}
+	*size = st.st_size;
+	buf = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+	close(fd);
+	return buf;
+}
+
+static int show_diff(const unsigned char *old_sha1, int old_status,
+		     const unsigned char *new_sha1, int new_status,
+		     const char *path, int reverse_diff)
+{
+	char other[PATH_MAX];
+	unsigned long size;
+	char type[20];
+	int fd;
+	int reverse;
+	void *blob = 0;
+	const char *fs = 0;
+	int need_unmap = 0;
+	int need_unlink = 0;
+
+
+	switch (old_status) {
+	case STATUS_CACHED:
+		blob = read_sha1_file(old_sha1, type, &size);
+		if (! blob)
+			return sha1err(path, old_sha1);
+			
+		switch (new_status) {
+		case STATUS_CACHED:
+			strcpy(other, ".diff_tree_helper_XXXXXX");
+			fd = mkstemp(other);
+			if (fd < 0)
+				die("unable to create temp-file");
+			if (write(fd, blob, size) != size)
+				die("unable to write temp-file");
+			close(fd);
+			free(blob);
+
+			blob = read_sha1_file(new_sha1, type, &size);
+			if (! blob)
+				return sha1err(path, new_sha1);
+
+			need_unlink = 1;
+			/* new = blob, old = fs */
+			reverse = !reverse_diff;
+			fs = other;
+			break;
+
+		case STATUS_ABSENT:
+		case STATUS_UNCACHED:
+			fs = ((new_status == STATUS_ABSENT) ?
+			      "/dev/null" : path);
+			reverse = reverse_diff;
+			break;
+
+		default:
+ 			reverse = reverse_diff;
+		}
+		break;
+
+	case STATUS_ABSENT:
+		switch (new_status) {
+		case STATUS_CACHED:
+			blob = read_sha1_file(new_sha1, type, &size);
+			if (! blob)
+				return sha1err(path, new_sha1);
+			/* old = fs, new = blob */
+			fs = "/dev/null";
+			reverse = !reverse_diff;
+			break;
+
+		case STATUS_ABSENT:
+			return error("diff-tree-helper: absent from both old and new?");
+		case STATUS_UNCACHED:
+			fs = path;
+			blob = strdup("");
+			size = 0;
+			/* old = blob, new = fs */
+			reverse = reverse_diff;
+			break;
+		default:
+			reverse = reverse_diff;
+		}
+		break;
+
+	case STATUS_UNCACHED:
+		fs = path; /* old = fs, new = blob */
+		reverse = !reverse_diff;
+
+		switch (new_status) {
+		case STATUS_CACHED:
+			blob = read_sha1_file(new_sha1, type, &size);
+			if (! blob)
+				return sha1err(path, new_sha1);
+			break;
+
+		case STATUS_ABSENT:
+			blob = strdup("");
+			size = 0;
+			break;
+
+		case STATUS_UNCACHED:
+			/* old = fs */
+			blob = map_whole_file(path, &size);
+			if (! blob)
+				return fserr(path);
+			need_unmap = 1;
+			break;
+		default:
+			reverse = reverse_diff;
+		}
+		break;
+
+	default:
+		reverse = reverse_diff;
+	}
+	
+	if (fs)
+		show_differences(fs,
+				 path, /* label */
+				 blob,
+				 size,
+				 reverse /* 0: diff blob fs
+					    1: diff fs blob */);
+
+	if (need_unlink)
+		unlink(other);
+	if (need_unmap && blob)
+		munmap(blob, size);
+	else
+		free(blob);
+	return 0;
+}
+
+static const char *diff_tree_helper_usage =
+"diff-tree-helper [-R] [-z] paths...";
+
+int main(int ac, char **av) {
+	struct strbuf sb;
+	int reverse_diff = 0;
+	int line_termination = '\n';
+
+	strbuf_init(&sb);
+
+	while (1 < ac && av[1][0] == '-') {
+		if (av[1][1] == 'R')
+			reverse_diff = 1;
+		else if (av[1][1] == 'z')
+			line_termination = 0;
+		else
+			usage(diff_tree_helper_usage);
+		ac--; av++;
+	}
+	/* the remaining parameters are paths patterns */
+
+	prepare_diff_cmd();
+
+	while (1) {
+		int old_status, new_status;
+		unsigned char old_sha1[20], new_sha1[20];
+		char path[PATH_MAX];
+		read_line(&sb, stdin, line_termination);
+		if (sb.eof)
+			break;
+		if (parse_diff_tree_output(sb.buf,
+					   old_sha1, &old_status,
+					   new_sha1, &new_status,
+					   path)) {
+			fprintf(stderr, "cannot parse %s\n", sb.buf);
+			continue;
+		}
+		if (1 < ac && ! matches_pathspec(path, av+1, ac-1))
+			continue;
+
+		show_diff(old_sha1, old_status,
+			  new_sha1, new_status,
+			  path, reverse_diff);
+	}
+	return 0;
+}
--- k/strbuf.c
+++ l/strbuf.c
@@ -0,0 +1,43 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include "strbuf.h"
+
+void strbuf_init(struct strbuf *sb) {
+	sb->buf = 0;
+	sb->eof = sb->alloc = sb->len = 0;
+}
+
+static void strbuf_begin(struct strbuf *sb) {
+	free(sb->buf);
+	strbuf_init(sb);
+}
+
+static void inline strbuf_add(struct strbuf *sb, int ch) {
+	if (sb->alloc <= sb->len) {
+		sb->alloc = sb->alloc * 3 / 2 + 16;
+		sb->buf = realloc(sb->buf, sb->alloc);
+	}
+	sb->buf[sb->len++] = ch;
+}
+
+static void strbuf_end(struct strbuf *sb) {
+	strbuf_add(sb, 0);
+}
+
+void read_line(struct strbuf *sb, FILE *fp, int term) {
+	int ch;
+	strbuf_begin(sb);
+	if (feof(fp)) {
+		sb->eof = 1;
+		return;
+	}
+	while ((ch = fgetc(fp)) != EOF) {
+		if (ch == term)
+			break;
+		strbuf_add(sb, ch);
+	}
+	if (sb->len == 0)
+		sb->eof = 1;
+	strbuf_end(sb);
+}
+
--- k/strbuf.h
+++ l/strbuf.h
@@ -0,0 +1,13 @@
+#ifndef STRBUF_H
+#define STRBUF_H
+struct strbuf {
+	int alloc;
+	int len;
+	int eof;
+	unsigned char *buf;
+};
+
+extern void strbuf_init(struct strbuf *);
+extern void read_line(struct strbuf *, FILE *, int);
+
+#endif /* STRBUF_H */

Re: [PATCH 0/2] diff-tree/diff-cache helper

[Linus Torvalds]

On Sun, 24 Apr 2005, Junio C Hamano wrote:
>
> I use a set of small scripts [*1*] directly on top of the core
> git, which needed to make patches out of diff-tree and
> diff-cache output.  Its output is compatible with what show-diff
> produces.

Good, applied.

This also makes me think that we should just make "show-diff" show the
same format, at which point show-diff actually matches all the other
tools, and it is likely to make show-diff more useful to boot.

The thing I personally use "show-diff" for these days is actually just to
check whether I have anything dirty in my tree, and then it would actually
be preferable to just get the filenaname printout (in the same old
"diff-cache" format) rather than the full diff.

Maybe rename the "show-diff" command to be "cache-diff", and if somebody
wants the old "show-diff" thing, just have a script that does

	#!/bin/sh
	cache-diff | diff-tree-helper

and nothing more.

Talking about renaming, at some point we really should prepend "git-" to 
all the git commands. I didn't want to do the extra typing when I started 
out and was unsure about the name, but hey, by now we really should.

Junio, what do you think?

		Linus

Re: [PATCH 0/2] diff-tree/diff-cache helper

[Nicolas Pitre]

On Mon, 25 Apr 2005, Linus Torvalds wrote:

> This also makes me think that we should just make "show-diff" show the
> same format, at which point show-diff actually matches all the other
> tools, and it is likely to make show-diff more useful to boot.
> 
> The thing I personally use "show-diff" for these days is actually just to
> check whether I have anything dirty in my tree, and then it would actually
> be preferable to just get the filenaname printout (in the same old
> "diff-cache" format) rather than the full diff.

That makes a lot of sense.  And I think that path filtering in diff-tree 
should be factored out and supported into all of the diff-* commands as 
well (not necessarily in diff-tree-helper).


Nicolas

Re: [PATCH 0/2] diff-tree/diff-cache helper

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> Good, applied.

LT> This also makes me think that we should just make "show-diff" show the
LT> same format, at which point show-diff actually matches all the other
LT> tools, and it is likely to make show-diff more useful to boot.

Matching is good, ...

LT> Maybe rename the "show-diff" command to be "cache-diff", and if somebody
LT> wants the old "show-diff" thing, just have a script that does

LT> 	#!/bin/sh
LT> 	cache-diff | diff-tree-helper

LT> and nothing more.

Well, great minds do not think alike ;-) I was actually going in
quite the opposite way for the same goal of "matching".  My plan
was to rewrite the external diff interface once more to make it
more generic.  Then to add -p (patch) flag to diff-tree and
diff-cache, so that we do not need diff-tree-helper anymore.  My
ultimate motive for all of this is to make the core GIT useful
enough to render Cogito or any other wrapper layer more or less
irrelevant ;-).  Well type ^W a couple of times and rephrase
that to make things easier for the wrapper layer.

Jokes aside, I have updated the external diff interface and will
be sending you a patch in a separate message.  The existing
external diff interface had a horrible interface to the callers,
and it had a hardcoded knowledge of how to call diff and what
parameters to pass in which order, so the customization the end
user or the scripts could make was quite limited.  The updated
interface allows pretty much arbitrary formatting, so "git diff"
can put SHA1 instead of short-and-sweet 'a' or 'b' as directory
prefix in the patch output, for example.

When an environment variable GIT_EXTERNAL_DIFF exists, it names
a script that takes 7 parameters:

    name file1 sha1-1 mode1 file2 sha1-2 mode2

This is essentially the same idea you used for merge-cache.
Then the named command can use these information to generate and
format the diff any way it wants.  See how it interfaces using
the examples like this:

    GIT_EXTERNAL_DIFF=echo show-diff
    diff-cache $(cat .git/HEAD) | \
    GIT_EXTERNAL_DIFF=./jit-external-diff-script diff-tree-helper

The patch comes with a sample script, jit-external-diff-script,
but it is not much better than the built-in one; I do not expect
it to be used for anything other than starting point for wrapper
writers.

Without the environment variable, it uses a built-in
implementation that behaves exactly like the traditional
show-diff, and it can be customized via GIT_DIFF_CMD and
GIT_DIFF_OPTS as before.

[PATCH] Diff-tree-helper take two.

[Junio C Hamano]

This patch reworks the diff-tree-helper and show-diff to further
make external diff command interface simpler.  These commands
now honor GIT_EXTERNAL_DIFF environment variable which can point
at an arbitrary program that takes 7 parameters:

  name file1 file1-sha1 file1-mode file2 file2-sha1 file2-mode

The parameters for an external diff command are as follows:

  name        this invocation of the command is to emit diff
	      for the named cache/tree entry.

  file1       pathname that holds the contents of the first
	      file.  This can be a file inside the working
	      tree, or a temporary file created from the blob
	      object, or /dev/null.  The command should not
	      attempt to unlink it -- the temporary is
	      unlinked by the caller.

  file1-sha1  sha1 hash if file1 is a blob object, or "."
	      otherwise.

  file1-mode  mode bits for file1, or "." for a deleted file.

If GIT_EXTERNAL_DIFF environment variable is not set, the
default is to invoke diff with the set of parameters old
show-diff used to use.  This built-in implementation honors the
GIT_DIFF_CMD and GIT_DIFF_OPTS environment variables as before.

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

diff-tree-helper.c       |  242 +++++++-------------------------------------
diff.c                   |  254 ++++++++++++++++++++++++++++++++++++++---------
diff.h                   |   34 ++++--
jit-external-diff-script |   14 ++
show-diff.c              |   17 ---
5 files changed, 288 insertions(+), 273 deletions(-)

--- k/diff-tree-helper.c
+++ l/diff-tree-helper.c
@@ -1,3 +1,6 @@
+/*
+ * Copyright (C) 2005 Junio C Hamano
+ */
 #include "cache.h"
 #include "strbuf.h"
 #include "diff.h"
@@ -17,15 +20,22 @@ static int matches_pathspec(const char *
 	return 0;
 }
 
-static int parse_oneside_change(const char *cp, unsigned char *sha1,
-				char *path) {
+static int parse_oneside_change(const char *cp, struct diff_spec *one,
+				char *path)
+{
 	int ch;
-	while ((ch = *cp) && '0' <= ch && ch <= '7')
-		cp++; /* skip mode bits */
+
+	one->file_valid = one->sha1_valid = 1;
+	one->mode = 0;
+	while ((ch = *cp) && '0' <= ch && ch <= '7') {
+		one->mode = (one->mode << 3) | (ch - '0');
+		cp++;
+	}
+
 	if (strncmp(cp, "\tblob\t", 6))
 		return -1;
 	cp += 6;
-	if (get_sha1_hex(cp, sha1))
+	if (get_sha1_hex(cp, one->u.sha1))
 		return -1;
 	cp += 40;
 	if (*cp++ != '\t')
@@ -34,31 +44,20 @@ static int parse_oneside_change(const ch
 	return 0;
 }
 
-#define STATUS_CACHED    0 /* cached and sha1 valid */
-#define STATUS_ABSENT    1 /* diff-tree says old removed or new added */
-#define STATUS_UNCACHED  2 /* diff-cache output: read from working tree */
-
 static int parse_diff_tree_output(const char *buf,
-				  unsigned char *old_sha1,
-				  int *old_status,
-				  unsigned char *new_sha1,
-				  int *new_status,
+				  struct diff_spec *old,
+				  struct diff_spec *new,
 				  char *path) {
 	const char *cp = buf;
 	int ch;
-	static unsigned char null_sha[20] = { 0, };
 
 	switch (*cp++) {
 	case '+':
-		*old_status = STATUS_ABSENT;
-		*new_status = (memcmp(new_sha1, null_sha, sizeof(null_sha)) ?
-			       STATUS_CACHED : STATUS_UNCACHED);
-		return parse_oneside_change(cp, new_sha1, path);
+		old->file_valid = 0;
+		return parse_oneside_change(cp, new, path);
 	case '-':
-		*new_status = STATUS_ABSENT;
-		*old_status = (memcmp(old_sha1, null_sha, sizeof(null_sha)) ?
-			       STATUS_CACHED : STATUS_UNCACHED);
-		return parse_oneside_change(cp, old_sha1, path);
+		new->file_valid = 0;
+		return parse_oneside_change(cp, old, path);
 	case '*':
 		break;
 	default:
@@ -66,191 +65,36 @@ static int parse_diff_tree_output(const 
 	}
 	
 	/* This is for '*' entries */
-	while ((ch = *cp) && ('0' <= ch && ch <= '7'))
-		cp++; /* skip mode bits */
+	old->file_valid = old->sha1_valid = 1;
+	new->file_valid = new->sha1_valid = 1;
+
+	old->mode = new->mode = 0;
+	while ((ch = *cp) && ('0' <= ch && ch <= '7')) {
+		old->mode = (old->mode << 3) | (ch - '0');
+		cp++;
+	}
 	if (strncmp(cp, "->", 2))
 		return -1;
 	cp += 2;
-	while ((ch = *cp) && ('0' <= ch && ch <= '7'))
-		cp++; /* skip mode bits */
+	while ((ch = *cp) && ('0' <= ch && ch <= '7')) {
+		new->mode = (new->mode << 3) | (ch - '0');
+		cp++;
+	}
 	if (strncmp(cp, "\tblob\t", 6))
 		return -1;
 	cp += 6;
-	if (get_sha1_hex(cp, old_sha1))
+	if (get_sha1_hex(cp, old->u.sha1))
 		return -1;
 	cp += 40;
 	if (strncmp(cp, "->", 2))
 		return -1;
 	cp += 2;
-	if (get_sha1_hex(cp, new_sha1))
+	if (get_sha1_hex(cp, new->u.sha1))
 		return -1;
 	cp += 40;
 	if (*cp++ != '\t')
 		return -1;
 	strcpy(path, cp);
-	*old_status = (memcmp(old_sha1, null_sha, sizeof(null_sha)) ?
-		       STATUS_CACHED : STATUS_UNCACHED);
-	*new_status = (memcmp(new_sha1, null_sha, sizeof(null_sha)) ?
-		       STATUS_CACHED : STATUS_UNCACHED);
-	return 0;
-}
-
-static int sha1err(const char *path, const unsigned char *sha1)
-{
-	return error("diff-tree-helper: unable to read sha1 file of %s (%s)",
-		     path, sha1_to_hex(sha1));
-}
-
-static int fserr(const char *path)
-{
-	return error("diff-tree-helper: unable to read file %s", path);
-}
-
-static char *map_whole_file(const char *path, unsigned long *size) {
-	int fd;
-	struct stat st;
-	void *buf;
-
-	if ((fd = open(path, O_RDONLY)) < 0) {
-		error("diff-tree-helper: unable to read file %s", path);
-		return 0;
-	}
-	if (fstat(fd, &st) < 0) {
-		close(fd);
-		error("diff-tree-helper: unable to stat file %s", path);
-		return 0;
-	}
-	*size = st.st_size;
-	buf = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
-	close(fd);
-	return buf;
-}
-
-static int show_diff(const unsigned char *old_sha1, int old_status,
-		     const unsigned char *new_sha1, int new_status,
-		     const char *path, int reverse_diff)
-{
-	char other[PATH_MAX];
-	unsigned long size;
-	char type[20];
-	int fd;
-	int reverse;
-	void *blob = 0;
-	const char *fs = 0;
-	int need_unmap = 0;
-	int need_unlink = 0;
-
-
-	switch (old_status) {
-	case STATUS_CACHED:
-		blob = read_sha1_file(old_sha1, type, &size);
-		if (! blob)
-			return sha1err(path, old_sha1);
-			
-		switch (new_status) {
-		case STATUS_CACHED:
-			strcpy(other, ".diff_tree_helper_XXXXXX");
-			fd = mkstemp(other);
-			if (fd < 0)
-				die("unable to create temp-file");
-			if (write(fd, blob, size) != size)
-				die("unable to write temp-file");
-			close(fd);
-			free(blob);
-
-			blob = read_sha1_file(new_sha1, type, &size);
-			if (! blob)
-				return sha1err(path, new_sha1);
-
-			need_unlink = 1;
-			/* new = blob, old = fs */
-			reverse = !reverse_diff;
-			fs = other;
-			break;
-
-		case STATUS_ABSENT:
-		case STATUS_UNCACHED:
-			fs = ((new_status == STATUS_ABSENT) ?
-			      "/dev/null" : path);
-			reverse = reverse_diff;
-			break;
-
-		default:
- 			reverse = reverse_diff;
-		}
-		break;
-
-	case STATUS_ABSENT:
-		switch (new_status) {
-		case STATUS_CACHED:
-			blob = read_sha1_file(new_sha1, type, &size);
-			if (! blob)
-				return sha1err(path, new_sha1);
-			/* old = fs, new = blob */
-			fs = "/dev/null";
-			reverse = !reverse_diff;
-			break;
-
-		case STATUS_ABSENT:
-			return error("diff-tree-helper: absent from both old and new?");
-		case STATUS_UNCACHED:
-			fs = path;
-			blob = strdup("");
-			size = 0;
-			/* old = blob, new = fs */
-			reverse = reverse_diff;
-			break;
-		default:
-			reverse = reverse_diff;
-		}
-		break;
-
-	case STATUS_UNCACHED:
-		fs = path; /* old = fs, new = blob */
-		reverse = !reverse_diff;
-
-		switch (new_status) {
-		case STATUS_CACHED:
-			blob = read_sha1_file(new_sha1, type, &size);
-			if (! blob)
-				return sha1err(path, new_sha1);
-			break;
-
-		case STATUS_ABSENT:
-			blob = strdup("");
-			size = 0;
-			break;
-
-		case STATUS_UNCACHED:
-			/* old = fs */
-			blob = map_whole_file(path, &size);
-			if (! blob)
-				return fserr(path);
-			need_unmap = 1;
-			break;
-		default:
-			reverse = reverse_diff;
-		}
-		break;
-
-	default:
-		reverse = reverse_diff;
-	}
-	
-	if (fs)
-		show_differences(fs,
-				 path, /* label */
-				 blob,
-				 size,
-				 reverse /* 0: diff blob fs
-					    1: diff fs blob */);
-
-	if (need_unlink)
-		unlink(other);
-	if (need_unmap && blob)
-		munmap(blob, size);
-	else
-		free(blob);
 	return 0;
 }
 
@@ -275,28 +119,20 @@ int main(int ac, char **av) {
 	}
 	/* the remaining parameters are paths patterns */
 
-	prepare_diff_cmd();
-
 	while (1) {
-		int old_status, new_status;
-		unsigned char old_sha1[20], new_sha1[20];
+		struct diff_spec old, new;
 		char path[PATH_MAX];
 		read_line(&sb, stdin, line_termination);
 		if (sb.eof)
 			break;
-		if (parse_diff_tree_output(sb.buf,
-					   old_sha1, &old_status,
-					   new_sha1, &new_status,
-					   path)) {
+		if (parse_diff_tree_output(sb.buf, &old, &new, path)) { 
 			fprintf(stderr, "cannot parse %s\n", sb.buf);
 			continue;
 		}
-		if (1 < ac && ! matches_pathspec(path, av+1, ac-1))
+		if (1 < ac && !matches_pathspec(path, av+1, ac-1))
 			continue;
 
-		show_diff(old_sha1, old_status,
-			  new_sha1, new_status,
-			  path, reverse_diff);
+		run_external_diff(path, &old, &new);
 	}
 	return 0;
 }
--- k/diff.c
+++ l/diff.c
@@ -1,13 +1,22 @@
+/*
+ * Copyright (C) 2005 Junio C Hamano
+ */
+#include <sys/types.h>
+#include <sys/wait.h>
 #include "cache.h"
 #include "diff.h"
 
-static char *diff_cmd = "diff -L 'k/%s' -L 'l/%s' ";
-static char *diff_opts = "-p -u";
-static char *diff_arg_forward  = " - '%s'";
-static char *diff_arg_reverse  = " '%s' -";
+static char *diff_cmd = "diff -L'k/%s' -L'l/%s'";
+static char *diff_opts = "-pu";
 
-void prepare_diff_cmd(void)
+static const char *external_diff(void)
 {
+	static char *external_diff_cmd = NULL;
+	static int done_preparing = 0;
+
+	if (done_preparing)
+		return external_diff_cmd;
+
 	/*
 	 * Default values above are meant to match the
 	 * Linux kernel development style.  Examples of
@@ -17,8 +26,15 @@ void prepare_diff_cmd(void)
 	 * GIT_DIFF_CMD="diff -L '%s' -L '%s'"
 	 * GIT_DIFF_OPTS="-c";
 	 */
+	if (getenv("GIT_EXTERNAL_DIFF"))
+		external_diff_cmd = getenv("GIT_EXTERNAL_DIFF");
+
+	/* In case external diff fails... */
 	diff_cmd = getenv("GIT_DIFF_CMD") ? : diff_cmd;
 	diff_opts = getenv("GIT_DIFF_OPTS") ? : diff_opts;
+
+	done_preparing = 1;
+	return external_diff_cmd;
 }
 
 /* Help to copy the thing properly quoted for the shell safety.
@@ -58,49 +74,195 @@ static char *sq_expand(const char *src)
 	return buf;
 }
 
-void show_differences(const char *name, /* filename on the filesystem */
-		      const char *label, /* diff label to use */
-		      void *old_contents, /* contents in core */
-		      unsigned long long old_size, /* size in core */
-		      int reverse /* 0: diff core file
-				     1: diff file core */)
-{
-	FILE *f;
-	char *name_sq = sq_expand(name);
-	const char *label_sq = (name != label) ? sq_expand(label) : name_sq;
-	char *diff_arg = reverse ? diff_arg_reverse : diff_arg_forward;
-	int cmd_size = strlen(name_sq) + strlen(label_sq) * 2 +
-		strlen(diff_cmd) + strlen(diff_opts) + strlen(diff_arg);
+static struct diff_tempfile {
+	const char *name;
+	char hex[41];
+	char mode[10];
+	char tmp_path[50];
+} diff_temp[2];
+
+static void builtin_diff(const char *name,
+			 struct diff_tempfile *temp)
+{
+	static char *diff_arg  = "'%s' '%s'";
+	const char *name_1_sq = sq_expand(temp[0].name);
+	const char *name_2_sq = sq_expand(temp[1].name);
+	const char *name_sq = sq_expand(name);
+
+	/* diff_cmd and diff_arg have 4 %s in total which makes
+	 * the sum of these strings 8 bytes larger than required.
+	 * we use 2 spaces around diff-opts, and we need to count
+	 * terminating NUL, so we subtract 5 here.
+	 */
+	int cmd_size = (strlen(diff_cmd) + 
+			strlen(name_sq) * 2 +
+			strlen(diff_opts) +
+			strlen(diff_arg) +
+			strlen(name_1_sq) + strlen(name_2_sq)
+			- 5);
 	char *cmd = malloc(cmd_size);
-	int next_at;
+	int next_at = 0;
+
+	next_at += snprintf(cmd+next_at, cmd_size-next_at,
+			    diff_cmd, name_sq, name_sq);
+	next_at += snprintf(cmd+next_at, cmd_size-next_at,
+			    " %s ", diff_opts);
+	next_at += snprintf(cmd+next_at, cmd_size-next_at,
+			    diff_arg, name_1_sq, name_2_sq);
+	execlp("/bin/sh","sh", "-c", cmd, NULL);
+}
+
+static void prepare_temp_file(const char *name,
+			      struct diff_tempfile *temp,
+			      struct diff_spec *one)
+{
+	static unsigned char null_sha1[20] = { 0, };
 
-	fflush(stdout);
-	next_at = snprintf(cmd, cmd_size, diff_cmd, label_sq, label_sq);
-	next_at += snprintf(cmd+next_at, cmd_size-next_at, "%s", diff_opts);
-	next_at += snprintf(cmd+next_at, cmd_size-next_at, diff_arg, name_sq);
-	f = popen(cmd, "w");
-	if (old_size)
-		fwrite(old_contents, old_size, 1, f);
-	pclose(f);
-	if (label_sq != name_sq)
-		free((void*)label_sq); /* constness */
-	free(name_sq);
-	free(cmd);
-}
-
-void show_diff_empty(const unsigned char *sha1,
-		     const char *name,
-		     int reverse)
-{
-	char *old;
-	unsigned long int size;
-	unsigned char type[20];
-
-	old = read_sha1_file(sha1, type, &size);
-	if (! old) {
-		error("unable to read blob object for %s (%s)", name,
-		      sha1_to_hex(sha1));
+	if (!one->file_valid) {
+	not_a_valid_file:
+		temp->name = "/dev/null";
+		strcpy(temp->hex, ".");
+		strcpy(temp->mode, ".");
 		return;
 	}
-	show_differences("/dev/null", name, old, size, reverse);
+
+	if (one->sha1_valid &&
+	    !memcmp(one->u.sha1, null_sha1, sizeof(null_sha1))) {
+		one->sha1_valid = 0;
+		one->u.name = name;
+	}
+
+	if (!one->sha1_valid) {
+		struct stat st;
+		temp->name = one->u.name;
+		if (stat(temp->name, &st) < 0) {
+			if (errno == ENOENT)
+				goto not_a_valid_file;
+			die("stat(%s): %s", temp->name, strerror(errno));
+		}
+		strcpy(temp->hex, ".");
+		sprintf(temp->mode, "%06o",
+			S_IFREG |ce_permissions(st.st_mode));
+	}
+	else {
+		int fd;
+		void *blob;
+		char type[20];
+		unsigned long size;
+
+		blob = read_sha1_file(one->u.sha1, type, &size);
+		if (!blob || strcmp(type, "blob"))
+			die("unable to read blob object for %s (%s)",
+			    name, sha1_to_hex(one->u.sha1));
+
+		strcpy(temp->tmp_path, ".diff_XXXXXX");
+		fd = mkstemp(temp->tmp_path);
+		if (fd < 0)
+			die("unable to create temp-file");
+		if (write(fd, blob, size) != size)
+			die("unable to write temp-file");
+		close(fd);
+		free(blob);
+		temp->name = temp->tmp_path;
+		strcpy(temp->hex, sha1_to_hex(one->u.sha1));
+		temp->hex[40] = 0;
+		sprintf(temp->mode, "%06o", one->mode);
+	}
+}
+
+static void remove_tempfile(void)
+{
+	int i;
+
+	for (i = 0; i < 2; i++)
+		if (diff_temp[i].name == diff_temp[i].tmp_path) {
+			unlink(diff_temp[i].name);
+			diff_temp[i].name = NULL;
+		}
+}
+
+/* An external diff command takes:
+ *
+ * diff-cmd name infile1 infile1-sha1 infile1-mode \
+ *               infile2 infile2-sha1 infile2-mode.
+ *
+ */
+void run_external_diff(const char *name,
+		       struct diff_spec *one,
+		       struct diff_spec *two)
+{
+	struct diff_tempfile *temp = diff_temp;
+	int pid, status;
+	static int atexit_asked = 0;
+
+	prepare_temp_file(name, &temp[0], one);
+	prepare_temp_file(name, &temp[1], two);
+	if (! atexit_asked &&
+	    (temp[0].name == temp[0].tmp_path ||
+	     temp[1].name == temp[1].tmp_path)) {
+		atexit_asked = 1;
+		atexit(remove_tempfile);
+	}
+
+	fflush(NULL);
+	pid = fork();
+	if (pid < 0)
+		die("unable to fork");
+	if (!pid) {
+		const char *pgm = external_diff();
+		if (pgm)
+			execlp(pgm, pgm,
+			       name,
+			       temp[0].name, temp[0].hex, temp[0].mode,
+			       temp[1].name, temp[1].hex, temp[1].mode,
+			       NULL);
+		/*
+		 * otherwise we use the built-in one.
+		 */
+		builtin_diff(name, temp);
+		exit(0);
+	}
+	if (waitpid(pid, &status, 0) < 0 || !WIFEXITED(status))
+		die("diff program failed");
+
+	remove_tempfile();
+}
+
+void show_diff_empty(const struct cache_entry *ce, int reverse)
+{
+	struct diff_spec spec[2], *one, *two;
+
+	memcpy(spec[0].u.sha1, ce->sha1, 20);
+	spec[0].mode = ntohl(ce->ce_mode);
+	spec[0].sha1_valid = spec[0].file_valid = 1;
+	spec[1].file_valid = 0;
+
+	if (reverse) {
+		one = spec + 1; two = spec;
+	} else {
+		one = spec; two = one + 1;
+	}
+
+	run_external_diff(ce->name, one, two);
+}
+
+void show_differences(const struct cache_entry *ce, int reverse) 
+{
+	struct diff_spec spec[2], *one, *two;
+
+	memcpy(spec[0].u.sha1, ce->sha1, 20);
+	spec[0].mode = ntohl(ce->ce_mode);
+	spec[0].sha1_valid = spec[0].file_valid = 1;
+
+	spec[1].u.name = ce->name; /* the name we stated */
+	spec[1].sha1_valid = 0;
+	spec[1].file_valid = 1;
+
+	if (reverse) {
+		one = spec + 1; two = spec;
+	} else {
+		one = spec; two = one + 1;
+	}
+
+	run_external_diff(ce->name, one, two);
 }
--- k/diff.h
+++ l/diff.h
@@ -1,17 +1,31 @@
+/*
+ * Copyright (C) 2005 Junio C Hamano
+ */
 #ifndef DIFF_H
 #define DIFF_H
 
-extern void prepare_diff_cmd(void);
+/* These two are for backward compatibility with show-diff;
+ * new users should not use them.
+ */
+extern void show_differences(const struct cache_entry *ce, int reverse);
+extern void show_diff_empty(const struct cache_entry *ce, int reverse);
 
-extern void show_differences(const char *name, /* filename on the filesystem */
-			     const char *label, /* diff label to use */
-			     void *old_contents, /* contents in core */
-			     unsigned long long old_size, /* size in core */
-			     int reverse /* 0: diff core file
-					    1: diff file core */);
+struct diff_spec {
+	union {
+		const char *name;       /* path on the filesystem */
+		unsigned char sha1[20]; /* blob object ID */
+	} u;
+	unsigned short mode;	 /* file mode */
+	unsigned sha1_valid : 1; /* if true, use u.sha1 and trust mode.
+				  * (however with a NULL SHA1, read them
+				  * from the file!).
+				  * if false, use u.name and read mode from
+				  * the filesystem.
+				  */
+	unsigned file_valid : 1; /* if false the file does not even exist */
+};
 
-extern void show_diff_empty(const unsigned char *sha1,
-			    const char *name,
-			    int reverse);
+extern void run_external_diff(const char *name,
+			      struct diff_spec *, struct diff_spec *);
 
 #endif /* DIFF_H */
--- k/jit-external-diff-script
+++ l/jit-external-diff-script
@@ -0,0 +1,14 @@
+#!/bin/sh
+#
+# Copyright (C) 2005 Junio C Hamano
+#
+
+name="$1" name1="$2" sha11="$3" mode1="$4"
+          name2="$5" sha12="$6" mode2="$7"
+
+case "$sha11" in .) sha11=file-not-in-blob-but-in-the-working-tree ;; esac
+case "$sha12" in .) sha12=file-not-in-blob-but-in-the-working-tree ;; esac
+case "$mode1" in .) mode1=;; *) mode1=" ($mode1)" ;;esac
+case "$mode2" in .) mode2=;; *) mode2=" ($mode2)" ;;esac
+
+diff -pu -L "$sha11/$name$mode1" -L "$sha12/$name$mode2" "$name1" "$name2"
--- k/show-diff.c
+++ l/show-diff.c
@@ -53,14 +53,11 @@ int main(int argc, char **argv)
 		perror("read_cache");
 		exit(1);
 	}
-	prepare_diff_cmd();
+
 	for (i = 0; i < entries; i++) {
 		struct stat st;
 		struct cache_entry *ce = active_cache[i];
 		int changed;
-		unsigned long size;
-		char type[20];
-		void *old;
 
 		if (1 < argc &&
 		    ! matches_pathspec(ce, argv+1, argc-1))
@@ -87,8 +84,7 @@ int main(int argc, char **argv)
 			else {
 				printf("%s: %s\n", ce->name, strerror(errno));
 				if (errno == ENOENT)
-					show_diff_empty(ce->sha1, ce->name,
-							reverse);
+					show_diff_empty(ce, reverse);
 			}
 			continue;
 		}
@@ -104,14 +100,7 @@ int main(int argc, char **argv)
 		if (silent)
 			continue;
 
-		old = read_sha1_file(ce->sha1, type, &size);
-		if (! old)
-			error("unable to read blob object for %s (%s)",
-			      ce->name, sha1_to_hex(ce->sha1));
-		else
-			show_differences(ce->name, ce->name, old, size,
-					 reverse);
-		free(old);
+		show_differences(ce, reverse);
 	}
 	return 0;
 }

[PATCH] Add -r flag to show-diff for diff-cache/diff-tree like output.

[Junio C Hamano]

This adds a new option -r (rational) to show-diff command, to
produce diff-cache/diff-tree compatible output.  When -r is
specified, show-diff simply skips unmerged entries.  This makes
the following thee behave quite similarly: 

    show-diff  -r -z
    diff-cache -r -z [--cached] <tree-or-commit>
    diff-tree  -r -z <tree-or-commit> <tree-or-commit>

When -z (machine readable) is used, it implies rational.  In
addition to terminating each output record with NUL instead of
'\n', machine readable format also may output lines of the form:

    U filename <NUL>

to show that there are unmerged entries, once per path (this is
to help scripts so that they do not have to run "sed | uniq" on
output from "show-files --unmerged", just to get the list of
unmerged files).

The earlier machine readable output format had another form:

    X filename <NUL>

to show deleted files.  This has been removed; these paths now
give the diff-cache compatible "-<mode> <sha1> <path>" output.

I resisted the temptation to change those tabs between columns
into spaces.  Linus, what do you think?  I do not think Cogito
cares and changing them into spaces would certainly make them
more readable by humans.

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

show-diff.c |   44 +++++++++++++++++++++++++++++++++-----------
1 files changed, 33 insertions(+), 11 deletions(-)

--- k/show-diff.c
+++ l/show-diff.c
@@ -6,7 +6,8 @@
 #include "cache.h"
 #include "diff.h"
 
-static const char *show_diff_usage = "show-diff [-q] [-s] [-z] [paths...]";
+static const char *show_diff_usage =
+"show-diff [-q] [-s] [-r] [-z] [paths...]";
 
 static int matches_pathspec(struct cache_entry *ce, char **spec, int cnt)
 {
@@ -23,10 +24,17 @@ static int matches_pathspec(struct cache
 	return 0;
 }
 
+static void show_file(int pfx, struct cache_entry *ce, int NUL_terminate)
+{
+	printf("%c%o\t%s\t%s\t%s%c", pfx, ntohl(ce->ce_mode), "blob",
+	       sha1_to_hex(ce->sha1), ce->name, NUL_terminate ? 0 : '\n');
+}
+
 int main(int argc, char **argv)
 {
 	int silent = 0;
 	int silent_on_nonexisting_files = 0;
+	int rational = 0;
 	int machine_readable = 0;
 	int reverse = 0;
 	int entries = read_cache();
@@ -39,8 +47,12 @@ int main(int argc, char **argv)
 			silent_on_nonexisting_files = silent = 1;
 		else if (!strcmp(argv[1], "-q"))
 			silent_on_nonexisting_files = 1;
+		else if (!strcmp(argv[1], "-r"))
+			/* diff-cache and diff-tree compatible */
+			rational = 1;
 		else if (!strcmp(argv[1], "-z"))
-			machine_readable = 1;
+			/* machine readable implies rational */
+			rational = machine_readable = 1;
 		else
 			usage(show_diff_usage);
 		argv++; argc--;
@@ -64,11 +76,16 @@ int main(int argc, char **argv)
 			continue;
 
 		if (ce_stage(ce)) {
+			/* machine-readble == rational in most cases,
+			 * but rational does not care about unmerged.
+			 * In some cases we want the list of unmerged
+			 * files and running sort -u on show-files -z
+			 * --unmerged for that is a pain.
+			 */
 			if (machine_readable)
 				printf("U %s%c", ce->name, 0);
-			else
-				printf("%s: Unmerged\n",
-				       ce->name);
+			else if (!rational)
+				printf("%s: unmerged\n", ce->name);
 			while (i < entries &&
 			       !strcmp(ce->name, active_cache[i]->name))
 				i++;
@@ -79,8 +96,10 @@ int main(int argc, char **argv)
 		if (stat(ce->name, &st) < 0) {
 			if (errno == ENOENT && silent_on_nonexisting_files)
 				continue;
-			if (machine_readable)
-				printf("X %s%c", ce->name, 0);
+			if (rational) {
+				/* deleted */
+				show_file('-', ce, machine_readable);
+			}
 			else {
 				printf("%s: %s\n", ce->name, strerror(errno));
 				if (errno == ENOENT)
@@ -91,10 +110,13 @@ int main(int argc, char **argv)
 		changed = cache_match_stat(ce, &st);
 		if (!changed)
 			continue;
-		if (!machine_readable)
-			printf("%s: %s\n", ce->name, sha1_to_hex(ce->sha1));
-		else {
-			printf("%s %s%c", sha1_to_hex(ce->sha1), ce->name, 0);
+		if (rational) {
+			static char *no_sha1_hex = 
+				"0000000000000000000000000000000000000000";
+			printf("*%o->%o\t%s\t%s->%s\t%s%c",
+			       ntohl(ce->ce_mode), st.st_mode,
+			       "blob", sha1_to_hex(ce->sha1), no_sha1_hex,
+			       ce->name, (machine_readable ? 0 : '\n'));
 			continue;
 		}
 		if (silent)

Re: [PATCH] Add -r flag to show-diff for diff-cache/diff-tree like output.

[Linus Torvalds]

On Tue, 26 Apr 2005, Junio C Hamano wrote:
>
> This adds a new option -r (rational) to show-diff command, to
> produce diff-cache/diff-tree compatible output.

Why not just make this the default? Who really cares about show-diff? I 
see that "cg-merge" uses it, but does so with the "s" flag, just to see 
whether there are any changes at all.

So why not just make "rational" the standard format, and then make 
"diff-tree-helper" warn about unmerged ("U") files?

As far as I can tell, that is really what _everybody_ wants.

And calling "-r" "rational", when it means "recursive" for diff-tree and 
diff-cache doesn't sound rational to me. It _is_ rational to just silently 
accept it as "recursive", the same way diff-cache does.

		Linus

Re: [PATCH] Add -r flag to show-diff for diff-cache/diff-tree like output.

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> Why not just make this the default? Who really cares about
LT> show-diff?

Me, to care enough about it to send patches in ;-).

LT> So why not just make "rational" the standard format, and then make 
LT> "diff-tree-helper" warn about unmerged ("U") files?

LT> As far as I can tell, that is really what _everybody_ wants.

I do not think of a good reason to forbid people from getting a
patch out of core GIT without going through the wrapper layer,
and the code is already there.  That said, I agree that scripts
usually do not use patch generating form.  I checked Cogito
before doing the patch to make sure it is not affected by this,
and my JIT core tools do not use it either.

I think making the diff-tree/cache compatible output default
would be fine.  The current diff-producing behaviour can be made
into an option (yes, I want to keep it).

LT> And calling "-r" "rational", when it means "recursive" for
LT> diff-tree and diff-cache doesn't sound rational to me.

Well, the truth is that -r does not stand for anything.  It just
is there to match useless -r to diff-cache (the change you made
to it to accept the same command line parameters -r (and
optionally -z) people would always give diff-tree.  You can call
it recursive if you want.

How about me doing the following and resubmitting?

 - Make this diff-tree/cache compatible output the default.

 - Take but ignore -r flag like diff-cache.

 - Add U warning to diff-tree-helper.

 - Add -p flag (patch) and have it cause the patch generating
   behaviour. 

Later I'll add -p flag to diff-cache and diff-tree, so the usage
of these three commands match.  That is:

    show-diff  -r (useless) -z           | diff-tree-helper -z
    diff-cache -r (useless) -z tree      | diff-tree-helper -z
    diff-tree  -r           -z tree tree | diff-tree-helper -z

    show-diff  -p [-r (useless)]
    diff-cache -p [-r (useless)]
    diff-tree  -p [-r]

With the GIT_EXTERNAL_DIFF envioronment variable, I suspect that
wrapper scripts do not have to use the diff-tree-helper but
directly use the -p form; but that will come later.  Thoughts?

Re: [PATCH] Add -r flag to show-diff for diff-cache/diff-tree like output.

[Linus Torvalds]

On Tue, 26 Apr 2005, Junio C Hamano wrote:
>
> >>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:
> 
> LT> Why not just make this the default? Who really cares about
> LT> show-diff?
> 
> Me, to care enough about it to send patches in ;-).

Well, I actually care abotu "show-diff" too, in the sense that I actually 
use it all the time.

The thing is, I just care about the format. To me, a "diff-tree" like
format is actually _better_ than the full diff, 99% of the time, since
what I really want to know "what state is my repository in", I don't
usually care about the details within the files.

Ie I want to know exactly the same thing that most scripts would want to 
know: which files are dirty and need to be checked in, which ones are 
still unmerged etc.

> Later I'll add -p flag to diff-cache and diff-tree, so the usage
> of these three commands match.

I don't know why you're so dead set on creating the diff directly, when 
you _don't_ actually create the diff directly anyway (ie you call out to a 
helper process - "diff", regardless).

The thing is, "-p" is strictly weaker than doing the UNIX pipe way, since 
the latter trivially does the same time (add a simple script if you don't 
want to type it), but can also do things like "grep the filenames going 
past" or similar.

But hey, as long as the source code is clean, I don't care _that_ much. 

		Linus

Re: [PATCH] Add -r flag to show-diff for diff-cache/diff-tree like output.

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

>> Later I'll add -p flag to diff-cache and diff-tree, so the usage
>> of these three commands match.

LT> The thing is, "-p" is strictly weaker than doing the UNIX
LT> pipe way, since the latter trivially does the same time (add
LT> a simple script if you don't want to type it), but can also
LT> do things like "grep the filenames going past" or similar.

I do not disagree with that.  Having only "-p" and not having
diff-cache/tree output _is_ weaker, and I am _not_ advocating
for removing the diff-cache/tree like output format from these
three commands.

What I _am_ advocating for is to obsolete the diff-tree-helper
program.  What it does can be done, with the diff.[ch] change
you merged this morning, without going through a pipe to the
diff-tree-helper process but directly from these three commands,
once diff-cache/tree acquires the "-p" flag.

By the way, how about renaming show-diff to diff-file?

    diff-tree  : compares two trees.
    diff-cache : compares a tree and the cache, or a tree and files.
    diff-file  : compares the cache and files.

Re: [PATCH] Add -r flag to show-diff for diff-cache/diff-tree like output.

[Linus Torvalds]

On Tue, 26 Apr 2005, Junio C Hamano wrote:
> 
> By the way, how about renaming show-diff to diff-file?
> 
>     diff-tree  : compares two trees.
>     diff-cache : compares a tree and the cache, or a tree and files.
>     diff-file  : compares the cache and files.

Yes. Except I think that the "big renaming" is coming up, and we should 
just rename them to have a "git-" prefix too.

		Linus

[PATCH] diff-cache/tree compatible output for show-diff (take 2).

[Junio C Hamano]

This patch changes the output format of the show-diff command to
match that of the diff-cache/tree commands.  One type of record
it can produce that diff-cache/tree do not is of this form:

    U path <record-terminator>

This is emitted once per unmerged path, no matter how many
unmerged stages there are.  The diff-tree-helper program is also
taught about this and warns about such input records.

The -z flag has the same meaning as diff-cache/tree commands;
the output records are terminated with a NUL instead of a '\n'.
Just like diff-cache takes a meaningless -r flag, it also
ignores a -r.

The previous default behaviour of getting patch output can be
obtained by specifying a -p flag.

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

diff-tree-helper.c |   27 ++++++++++++++++---------
show-diff.c        |   57 +++++++++++++++++++++++++++++++++++++----------------
2 files changed, 58 insertions(+), 26 deletions(-)

--- k/diff-tree-helper.c
+++ l/diff-tree-helper.c
@@ -44,6 +44,9 @@ static int parse_oneside_change(const ch
 	return 0;
 }
 
+#define PLEASE_WARN -1
+#define WARNED_OURSELVES -2
+ 
 static int parse_diff_tree_output(const char *buf,
 				  struct diff_spec *old,
 				  struct diff_spec *new,
@@ -52,6 +55,9 @@ static int parse_diff_tree_output(const 
 	int ch;
 
 	switch (*cp++) {
+	case 'U':
+		fprintf(stderr, "warning: unmerged path %s\n", cp+1);
+		return WARNED_OURSELVES;
 	case '+':
 		old->file_valid = 0;
 		return parse_oneside_change(cp, new, path);
@@ -61,7 +67,7 @@ static int parse_diff_tree_output(const 
 	case '*':
 		break;
 	default:
-		return -1;
+		return PLEASE_WARN;
 	}
 	
 	/* This is for '*' entries */
@@ -74,26 +80,26 @@ static int parse_diff_tree_output(const 
 		cp++;
 	}
 	if (strncmp(cp, "->", 2))
-		return -1;
+		return PLEASE_WARN;
 	cp += 2;
 	while ((ch = *cp) && ('0' <= ch && ch <= '7')) {
 		new->mode = (new->mode << 3) | (ch - '0');
 		cp++;
 	}
 	if (strncmp(cp, "\tblob\t", 6))
-		return -1;
+		return PLEASE_WARN;
 	cp += 6;
 	if (get_sha1_hex(cp, old->u.sha1))
-		return -1;
+		return PLEASE_WARN;
 	cp += 40;
 	if (strncmp(cp, "->", 2))
-		return -1;
+		return PLEASE_WARN;
 	cp += 2;
 	if (get_sha1_hex(cp, new->u.sha1))
-		return -1;
+		return PLEASE_WARN;
 	cp += 40;
 	if (*cp++ != '\t')
-		return -1;
+		return PLEASE_WARN;
 	strcpy(path, cp);
 	return 0;
 }
@@ -120,13 +126,16 @@ int main(int ac, char **av) {
 	/* the remaining parameters are paths patterns */
 
 	while (1) {
+		int status;
 		struct diff_spec old, new;
 		char path[PATH_MAX];
 		read_line(&sb, stdin, line_termination);
 		if (sb.eof)
 			break;
-		if (parse_diff_tree_output(sb.buf, &old, &new, path)) { 
-			fprintf(stderr, "cannot parse %s\n", sb.buf);
+		status = parse_diff_tree_output(sb.buf, &old, &new, path);
+		if (status) {
+			if (status == PLEASE_WARN)
+				fprintf(stderr, "cannot parse %s\n", sb.buf);
 			continue;
 		}
 		if (1 < ac && !matches_pathspec(path, av+1, ac-1))
--- k/show-diff.c
+++ l/show-diff.c
@@ -6,7 +6,8 @@
 #include "cache.h"
 #include "diff.h"
 
-static const char *show_diff_usage = "show-diff [-q] [-s] [-z] [paths...]";
+static const char *show_diff_usage =
+"show-diff [-q] [-s] [-r] [-z] [-p] [paths...]";
 
 static int matches_pathspec(struct cache_entry *ce, char **spec, int cnt)
 {
@@ -23,24 +24,40 @@ static int matches_pathspec(struct cache
 	return 0;
 }
 
+static void show_file(int pfx, struct cache_entry *ce, int line_termination)
+{
+	printf("%c%o\t%s\t%s\t%s%c", pfx, ntohl(ce->ce_mode), "blob",
+	       sha1_to_hex(ce->sha1), ce->name, line_termination);
+}
+
 int main(int argc, char **argv)
 {
 	int silent = 0;
 	int silent_on_nonexisting_files = 0;
-	int machine_readable = 0;
+	int patch = 0;
+	int line_termination = '\n';
 	int reverse = 0;
 	int entries = read_cache();
 	int i;
 
 	while (1 < argc && argv[1][0] == '-') {
 		if  (!strcmp(argv[1], "-R"))
-			reverse = 1;
+			patch = reverse = 1; /* works only for patch */
 		else if (!strcmp(argv[1], "-s"))
-			silent_on_nonexisting_files = silent = 1;
+			patch = silent_on_nonexisting_files = silent = 1;
 		else if (!strcmp(argv[1], "-q"))
-			silent_on_nonexisting_files = 1;
+			patch = silent_on_nonexisting_files = 1;
+		else if (!strcmp(argv[1], "-p")) {
+			patch = 1;
+			line_termination = '\n';
+		}
+		else if (!strcmp(argv[1], "-r"))
+			; /* diff-cache and diff-tree compatible
+			   * is the default now.
+			   */
 		else if (!strcmp(argv[1], "-z"))
-			machine_readable = 1;
+			/* makes sense only non-patch */
+			patch = line_termination = 0;
 		else
 			usage(show_diff_usage);
 		argv++; argc--;
@@ -64,11 +81,10 @@ int main(int argc, char **argv)
 			continue;
 
 		if (ce_stage(ce)) {
-			if (machine_readable)
-				printf("U %s%c", ce->name, 0);
+			if (patch)
+				printf("%s: unmerged\n", ce->name);
 			else
-				printf("%s: Unmerged\n",
-				       ce->name);
+				printf("U %s%c", ce->name, line_termination);
 			while (i < entries &&
 			       !strcmp(ce->name, active_cache[i]->name))
 				i++;
@@ -77,26 +93,33 @@ int main(int argc, char **argv)
 		}
  
 		if (stat(ce->name, &st) < 0) {
+			/* deleted */
 			if (errno == ENOENT && silent_on_nonexisting_files)
 				continue;
-			if (machine_readable)
-				printf("X %s%c", ce->name, 0);
-			else {
+			if (patch) {
 				printf("%s: %s\n", ce->name, strerror(errno));
 				if (errno == ENOENT)
 					show_diff_empty(ce, reverse);
 			}
+			else
+				show_file('-', ce, line_termination);
 			continue;
 		}
 		changed = cache_match_stat(ce, &st);
 		if (!changed)
 			continue;
-		if (!machine_readable)
-			printf("%s: %s\n", ce->name, sha1_to_hex(ce->sha1));
-		else {
-			printf("%s %s%c", sha1_to_hex(ce->sha1), ce->name, 0);
+		if (!patch) {
+			static char *no_sha1_hex = 
+				"0000000000000000000000000000000000000000";
+			printf("*%o->%o\t%s\t%s->%s\t%s%c",
+			       ntohl(ce->ce_mode), st.st_mode,
+			       "blob", sha1_to_hex(ce->sha1), no_sha1_hex,
+			       ce->name, line_termination);
 			continue;
 		}
+		else
+			printf("%s %s%c", sha1_to_hex(ce->sha1), ce->name,
+			       line_termination);
 		if (silent)
 			continue;
 

Re: [PATCH] diff-cache/tree compatible output for show-diff (take 2).

[Linus Torvalds]

On Tue, 26 Apr 2005, Junio C Hamano wrote:
>
> This patch changes the output format of the show-diff command to
> match that of the diff-cache/tree commands.  One type of record
> it can produce that diff-cache/tree do not is of this form:

Dang, I already did this in my tree. I pushed mine out, and I don't want 
to see the "-p" flag until the others also do it (ie diff-tree and 
diff-cache ;).

Btw, diff-cache definitely _can_ output this form, so we probably should 
make diff-cache do so too, to match, no?

		Linus

Re: [PATCH] diff-cache/tree compatible output for show-diff (take 2).

[Linus Torvalds]

On Tue, 26 Apr 2005, Linus Torvalds wrote:
>
> [ "U" for "unmerged" ]
> 
> Btw, diff-cache definitely _can_ output this form, so we probably should 
> make diff-cache do so too, to match, no?

Actually, we should decide on what diff-tree-helper does before that. 
Right now it always either calls out to the external diff program, or it 
says "cannot parse".

It's possible that an external "diff" program might actually want to know
about unmerged files (maybe people don't actually do "diff", but something
else altogether), so one approach might be to just make that an option.

The other approach is to just have something like "<pathname> is unmerged"
as output from diff-tree-helper. That isn't quite as flexible, but it sure 
is simpler..

		Linus

Re: [PATCH] diff-cache/tree compatible output for show-diff (take 2).

[Linus Torvalds]

On Tue, 26 Apr 2005, Linus Torvalds wrote:
> 
> I'll need to fix that up, because right now it does the wrong thing (which
> it has always done): since we _remove_ all the merge entries, we'll first
> warn about them, but then we'll show the original file going away if there
> was one. Which is bogus.

Fixed. I think. My solution is clever, but untested.

What I do is that instead of _removing_ the unmerged entries (which we 
can't do, or we'd later think that the file has gone away if we see that 
same name in the tree we're comparing against), I make all unmerged 
entries be in "stage 3".

Then, when we read in the tree to compare against into stage 1, we have a 
few cases:

 - stage 0 only: new file
 - stage 0 and 1: modified file
 - stage 1 only: deleted file
 - stage 1 and 3: unmerged
 - stage 3 only: unmerged

See any problems with this? (a mix of 0 and 3 cannot happen - a file is
either unmerged or it is ok, since inserting a stage 0 entry always
removes all unmerged entires).

		Linus