[PATCH 5/3] read-tree: loosen too strict index requirements

[PATCH 5/3] read-tree: loosen too strict index requirements

[Junio C Hamano]

This patch teaches read-tree 3-way merge that, when only "the
other tree" changed a path, and if the index file already has
the same change, we are not in a situation that would clobber
the index and the work tree, and lets the merge succeed; this is
case #14ALT in t1000 test.  It does not change the result of the
merge, but prevents it from failing when it does not have to.

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

*** This is another leftover bits from the earlier 3-piece series.
*** This is not necessary for the --emu23 two-way improvements,
*** but rather help a case that would not be so uncommon in the
*** 3-way merge case.

 read-tree.c                 |    6 ++++++
 t/t1000-read-tree-m-3way.sh |    9 +++++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/read-tree.c b/read-tree.c
--- a/read-tree.c
+++ b/read-tree.c
@@ -280,6 +280,12 @@ static int threeway_merge(struct cache_e
 		}
 		/* otherwise we will apply the original rule */
 	}
+	/* #14ALT */
+	if (a && b && c && same(a, b) && !same(a, c)) {
+		if (old && same(old, c))
+			return merged_entry_allow_dirty(c, old, dst);
+		/* otherwise the regular rule applies */
+	}
 	/*
 	 * If we have an entry in the index cache ("old"), then we want
 	 * to make sure that it matches any entries in stage 2 ("first
diff --git a/t/t1000-read-tree-m-3way.sh b/t/t1000-read-tree-m-3way.sh
--- a/t/t1000-read-tree-m-3way.sh
+++ b/t/t1000-read-tree-m-3way.sh
@@ -464,6 +464,15 @@ test_expect_success \
      git-read-tree -m $tree_O $tree_A $tree_B &&
      check_result"
 
+test_expect_success \
+    '14 - may match B in O && A && B && O==A && O!=B case' \
+    "rm -f .git/index NM &&
+     cp .orig-B/NM NM &&
+     git-update-cache --add NM &&
+     echo extra >>NM &&
+     git-read-tree -m $tree_O $tree_A $tree_B &&
+     check_result"
+
 test_expect_failure \
     '14 (fail) - must match and be up-to-date in O && A && B && O==A && O!=B case' \
     "rm -f .git/index NM &&
------------

[PATCH] Fix rename/copy when dealing with temporarily broken pairs.

[Junio C Hamano]

When rename/copy uses a file that was broken by diffcore-break
as the source, and the broken filepair gets merged back later,
the output was mislabeled as a rename.  In this case, the source
file ends up staying in the output, so we should label it as a
copy instead.

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

 diff.c            |    1 +
 diffcore-rename.c |   10 ++++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -661,6 +661,7 @@ struct diff_filepair *diff_queue(struct 
 	dp->one = one;
 	dp->two = two;
 	dp->score = 0;
+	dp->status = 0;
 	dp->source_stays = 0;
 	dp->broken_pair = 0;
 	diff_q(queue, dp);
diff --git a/diffcore-rename.c b/diffcore-rename.c
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -249,8 +249,14 @@ void diffcore_rename(int detect_rename, 
 				continue; /* unmerged */
 			else
 				locate_rename_dst(p->two, 1);
-		else if (!DIFF_FILE_VALID(p->two))
-			register_rename_src(p->one, 0);
+		else if (!DIFF_FILE_VALID(p->two)) {
+			/* If the source is a broken "delete", and
+			 * they did not really want to get broken,
+			 * that means the source actually stays.
+			 */
+			int stays = (p->broken_pair && !p->score);
+			register_rename_src(p->one, stays);
+		}
 		else if (detect_rename == DIFF_DETECT_COPY)
 			register_rename_src(p->one, 1);
 	}
------------

[PATCH] Add --diff-filter= output restriction to diff-* family.

[Junio C Hamano]

This is a halfway between debugging aid and a helper to write an
ultra-smart merge scripts.  The new option takes a string that
consists of a list of "status" letters, and limits the diff
output to only those classes of changes, with two exceptions:

 - A broken pair (aka "complete rewrite"), does not match D
   (deleted) or N (created).  Use B to look for them.

 - The letter "A" in the diff-filter string does not match
   anything itself, but causes the entire diff that contains
   selected patches to be output (this behaviour is similar to
   that of --pickaxe-all for the -S option).

For example,

    $ git-rev-list HEAD |
      git-diff-tree --stdin -s -v -B -C --diff-filter=BCR

shows a list of commits that have complete rewrite, copy, or
rename.

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

 diff.h        |    8 +++++--
 diff-cache.c  |    9 ++++++-
 diff-files.c  |    7 ++++--
 diff-helper.c |   15 +++++++-----
 diff-stages.c |    8 +++++--
 diff-tree.c   |   12 +++++++---
 diff.c        |   70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 7 files changed, 108 insertions(+), 21 deletions(-)

diff --git a/diff.h b/diff.h
--- a/diff.h
+++ b/diff.h
@@ -47,7 +47,11 @@ extern void diffcore_std(const char **pa
 			 int detect_rename, int rename_score,
 			 const char *pickaxe, int pickaxe_opts,
 			 int break_opt,
-			 const char *orderfile);
+			 const char *orderfile, const char *filter);
+
+extern void diffcore_std_no_resolve(const char **paths,
+				    const char *pickaxe, int pickaxe_opts,
+				    const char *orderfile, const char *filter);
 
 extern int diff_queue_is_empty(void);
 
@@ -56,6 +60,6 @@ extern int diff_queue_is_empty(void);
 #define DIFF_FORMAT_PATCH	2
 #define DIFF_FORMAT_NO_OUTPUT	3
 
-extern void diff_flush(int output_style, int resolve_rename_copy);
+extern void diff_flush(int output_style);
 
 #endif /* DIFF_H */
diff --git a/diff-cache.c b/diff-cache.c
--- a/diff-cache.c
+++ b/diff-cache.c
@@ -11,6 +11,7 @@ static const char *pickaxe = NULL;
 static int pickaxe_opts = 0;
 static int diff_break_opt = -1;
 static const char *orderfile = NULL;
+static const char *diff_filter = NULL;
 
 /* A file entry went away or appeared */
 static void show_file(const char *prefix, struct cache_entry *ce, unsigned char *sha1, unsigned int mode)
@@ -224,6 +225,10 @@ int main(int argc, const char **argv)
 			pickaxe = arg + 2;
 			continue;
 		}
+		if (!strncmp(arg, "--diff-filter=", 14)) {
+			diff_filter = arg + 14;
+			continue;
+		}
 		if (!strncmp(arg, "-O", 2)) {
 			orderfile = arg + 2;
 			continue;
@@ -263,7 +268,7 @@ int main(int argc, const char **argv)
 		     detect_rename, diff_score_opt,
 		     pickaxe, pickaxe_opts,
 		     diff_break_opt,
-		     orderfile);
-	diff_flush(diff_output_format, 1);
+		     orderfile, diff_filter);
+	diff_flush(diff_output_format);
 	return ret;
 }
diff --git a/diff-files.c b/diff-files.c
--- a/diff-files.c
+++ b/diff-files.c
@@ -17,6 +17,7 @@ static const char *pickaxe = NULL;
 static int pickaxe_opts = 0;
 static int diff_break_opt = -1;
 static const char *orderfile = NULL;
+static const char *diff_filter = NULL;
 static int silent = 0;
 
 static void show_unmerge(const char *path)
@@ -59,6 +60,8 @@ int main(int argc, const char **argv)
 			pickaxe = argv[1] + 2;
 		else if (!strncmp(argv[1], "-O", 2))
 			orderfile = argv[1] + 2;
+		else if (!strncmp(argv[1], "--diff-filter=", 14))
+			diff_filter = argv[1] + 14;
 		else if (!strcmp(argv[1], "--pickaxe-all"))
 			pickaxe_opts = DIFF_PICKAXE_ALL;
 		else if (!strncmp(argv[1], "-B", 2)) {
@@ -131,7 +134,7 @@ int main(int argc, const char **argv)
 		     detect_rename, diff_score_opt,
 		     pickaxe, pickaxe_opts,
 		     diff_break_opt,
-		     orderfile);
-	diff_flush(diff_output_format, 1);
+		     orderfile, diff_filter);
+	diff_flush(diff_output_format);
 	return 0;
 }
diff --git a/diff-helper.c b/diff-helper.c
--- a/diff-helper.c
+++ b/diff-helper.c
@@ -8,17 +8,16 @@
 static const char *pickaxe = NULL;
 static int pickaxe_opts = 0;
 static const char *orderfile = NULL;
+static const char *diff_filter = NULL;
 static int line_termination = '\n';
 static int inter_name_termination = '\t';
 
 static void flush_them(int ac, const char **av)
 {
-	diffcore_std(av + 1,
-		     0, 0, /* no renames */
-		     pickaxe, pickaxe_opts,
-		     -1, /* no breaks */
-		     orderfile);
-	diff_flush(DIFF_FORMAT_PATCH, 0);
+	diffcore_std_no_resolve(av + 1,
+				pickaxe, pickaxe_opts,
+				orderfile, diff_filter);
+	diff_flush(DIFF_FORMAT_PATCH);
 }
 
 static const char *diff_helper_usage =
@@ -38,6 +37,10 @@ int main(int ac, const char **av) {
 		}
 		else if (!strcmp(av[1], "--pickaxe-all"))
 			pickaxe_opts = DIFF_PICKAXE_ALL;
+		else if (!strncmp(av[1], "--diff-filter=", 14))
+			diff_filter = av[1] + 14;
+		else if (!strncmp(av[1], "-O", 2))
+			orderfile = av[1] + 2;
 		else
 			usage(diff_helper_usage);
 		ac--; av++;
diff --git a/diff-stages.c b/diff-stages.c
--- a/diff-stages.c
+++ b/diff-stages.c
@@ -13,6 +13,7 @@ static const char *pickaxe = NULL;
 static int pickaxe_opts = 0;
 static int diff_break_opt = -1;
 static const char *orderfile = NULL;
+static const char *diff_filter = NULL;
 
 static char *diff_stages_usage =
 "git-diff-stages [-p] [-r] [-z] [-M] [-C] [-R] [-S<string>] [-O<orderfile>] <stage1> <stage2> [<path>...]";
@@ -88,6 +89,8 @@ int main(int ac, const char **av)
 			pickaxe = arg + 2;
 		else if (!strncmp(arg, "-O", 2))
 			orderfile = arg + 2;
+		else if (!strncmp(arg, "--diff-filter=", 14))
+			diff_filter = arg + 14;
 		else if (!strcmp(arg, "--pickaxe-all"))
 			pickaxe_opts = DIFF_PICKAXE_ALL;
 		else
@@ -111,7 +114,8 @@ int main(int ac, const char **av)
 		     detect_rename, diff_score_opt,
 		     pickaxe, pickaxe_opts,
 		     diff_break_opt,
-		     orderfile);
-	diff_flush(diff_output_format, 1);
+		     orderfile,
+		     diff_filter);
+	diff_flush(diff_output_format);
 	return 0;
 }
diff --git a/diff-tree.c b/diff-tree.c
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -18,6 +18,7 @@ static const char *pickaxe = NULL;
 static int pickaxe_opts = 0;
 static int diff_break_opt = -1;
 static const char *orderfile = NULL;
+static const char *diff_filter = NULL;
 static const char *header = NULL;
 static const char *header_prefix = "";
 static enum cmit_fmt commit_format = CMIT_FMT_RAW;
@@ -272,9 +273,10 @@ static int call_diff_flush(void)
 		     detect_rename, diff_score_opt,
 		     pickaxe, pickaxe_opts,
 		     diff_break_opt,
-		     orderfile);
+		     orderfile,
+		     diff_filter);
 	if (diff_queue_is_empty()) {
-		diff_flush(DIFF_FORMAT_NO_OUTPUT, 0);
+		diff_flush(DIFF_FORMAT_NO_OUTPUT);
 		return 0;
 	}
 	if (header) {
@@ -285,7 +287,7 @@ static int call_diff_flush(void)
 		printf(fmt, header, 0);
 		header = NULL;
 	}
-	diff_flush(diff_output_format, 1);
+	diff_flush(diff_output_format);
 	return 1;
 }
 
@@ -455,6 +457,10 @@ int main(int argc, const char **argv)
 			orderfile = arg + 2;
 			continue;
 		}
+		if (!strncmp(arg, "--diff-filter=", 14)) {
+			diff_filter = arg + 14;
+			continue;
+		}
 		if (!strcmp(arg, "--pickaxe-all")) {
 			pickaxe_opts = DIFF_PICKAXE_ALL;
 			continue;
diff --git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -920,7 +920,7 @@ static void diff_resolve_rename_copy(voi
 	diff_debug_queue("resolve-rename-copy done", q);
 }
 
-void diff_flush(int diff_output_style, int resolve_rename_copy)
+void diff_flush(int diff_output_style)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
 	int i;
@@ -929,8 +929,6 @@ void diff_flush(int diff_output_style, i
 
 	if (diff_output_style == DIFF_FORMAT_MACHINE)
 		line_termination = inter_name_termination = 0;
-	if (resolve_rename_copy)
-		diff_resolve_rename_copy();
 
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
@@ -957,11 +955,58 @@ void diff_flush(int diff_output_style, i
 	q->nr = q->alloc = 0;
 }
 
+static void diffcore_apply_filter(const char *filter)
+{
+	int i;
+	struct diff_queue_struct *q = &diff_queued_diff;
+	struct diff_queue_struct outq;
+	outq.queue = NULL;
+	outq.nr = outq.alloc = 0;
+
+	if (!filter)
+		return;
+
+	if (strchr(filter, 'A')) {
+		/* All-or-none */
+		int found;
+		for (i = found = 0; !found && i < q->nr; i++) {
+			struct diff_filepair *p = q->queue[i];
+			if ((p->broken_pair && strchr(filter, 'B')) ||
+			    (!p->broken_pair && strchr(filter, p->status)))
+				found++;
+		}
+		if (found)
+			return;
+
+		/* otherwise we will clear the whole queue
+		 * by copying the empty outq at the end of this
+		 * function, but first clear the current entries
+		 * in the queue.
+		 */
+		for (i = 0; i < q->nr; i++)
+			diff_free_filepair(q->queue[i]);
+	}
+	else {
+		/* Only the matching ones */
+		for (i = 0; i < q->nr; i++) {
+			struct diff_filepair *p = q->queue[i];
+			if ((p->broken_pair && strchr(filter, 'B')) ||
+			    (!p->broken_pair && strchr(filter, p->status)))
+				diff_q(&outq, p);
+			else
+				diff_free_filepair(p);
+		}
+	}
+	free(q->queue);
+	*q = outq;
+}
+
 void diffcore_std(const char **paths,
 		  int detect_rename, int rename_score,
 		  const char *pickaxe, int pickaxe_opts,
 		  int break_opt,
-		  const char *orderfile)
+		  const char *orderfile,
+		  const char *filter)
 {
 	if (paths && paths[0])
 		diffcore_pathspec(paths);
@@ -975,6 +1020,23 @@ void diffcore_std(const char **paths,
 		diffcore_pickaxe(pickaxe, pickaxe_opts);
 	if (orderfile)
 		diffcore_order(orderfile);
+	diff_resolve_rename_copy();
+	diffcore_apply_filter(filter);
+}
+
+
+void diffcore_std_no_resolve(const char **paths,
+			     const char *pickaxe, int pickaxe_opts,
+			     const char *orderfile,
+			     const char *filter)
+{
+	if (paths && paths[0])
+		diffcore_pathspec(paths);
+	if (pickaxe)
+		diffcore_pickaxe(pickaxe, pickaxe_opts);
+	if (orderfile)
+		diffcore_order(orderfile);
+	diffcore_apply_filter(filter);
 }
 
 void diff_addremove(int addremove, unsigned mode,
------------

Re: [PATCH] Fix rename/copy when dealing with temporarily broken pairs.

[Jon Seymour]

G'day Junio,

On 6/12/05, Junio C Hamano <junkio@cox.net> wrote:
> When rename/copy uses a file that was broken by diffcore-break
> as the source, and the broken filepair gets merged back later,
> the output was mislabeled as a rename.  In this case, the source
> file ends up staying in the output, so we should label it as a
> copy instead.

I've got a patch series that renames a file, but the patches generated by your
git-format-patch-script fail to rename the file to the new name.

I wondered if this patch might fix it, but it doesn't seem to.

Here's a hunk from the patch:

diff --git a/epoch.c b/traversal.c
similarity index 100%
rename from epoch.c
rename to traversal.c
--- a/epoch.c
+++ b/traversal.c
@@ -14,7 +14,7 @@

 #include "cache.h"
 #include "commit.h"
-#include "epoch.h"
+#include "traversal.h"

 struct fraction {
        BIGNUM numerator;

What tool am I meant to be using to apply the patch? 

I am currently using "patch -p1"

jon.

Re: [PATCH] Fix rename/copy when dealing with temporarily broken pairs.

[Petr Baudis]

Dear diary, on Mon, Jun 13, 2005 at 06:24:16PM CEST, I got a letter
where Jon Seymour <jon.seymour@gmail.com> told me that...
> diff --git a/epoch.c b/traversal.c
> similarity index 100%
> rename from epoch.c
> rename to traversal.c
> What tool am I meant to be using to apply the patch? 
> 
> I am currently using "patch -p1"

Try git-apply.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
<Espy> be careful, some twit might quote you out of context..

Re: [PATCH] Fix rename/copy when dealing with temporarily broken pairs.

[Junio C Hamano]

>>>>> "JS" == Jon Seymour <jon.seymour@gmail.com> writes:

JS> I've got a patch series that renames a file, but the patches
JS> generated by your git-format-patch-script fail to rename the
JS> file to the new name.

JS> I wondered if this patch might fix it, but it doesn't seem to.

I doubt the problem you have is related to the problem this
patch addresses.  This is a fix for a case that the machinery
sometimes says rename when it should say copy.

Regular "patch" cannot grok either copy or rename.  If your
recipient uses "patch", then you must not use -C nor -M when you
are preparing a diff for his consumption.

JS> What tool am I meant to be using to apply the patch? 
JS> I am currently using "patch -p1"

As Petr already responded, the git-apply program from the King
Penguin himself should work [*1*].

One thing I just want to make sure is this.  You are not
reporting that the patch you sent that said "rename" should have
been "copy", are you?  

I understand that your change creates traversal.c out of
epoch.c.  Say the output is from between commitA and commitB.

What I would like to know is if commitB still contains epoch.c,
or it does not have epoch.c anymore [*2*].  If it is the former
then I think you spotted a bug in the patch your message is a
response to.  I hope that is not the case.

P.S.  I will be offline shortly and will not be able to offer
further help until perhaps late next week.


[Footnote]

*1* Find the thread in the mailing list archive that contains
this message, which I think would be of help too.

    Date: Sun, 5 Jun 2005 16:17:56 -0700 (PDT)
    From: Linus Torvalds <torvalds@osdl.org>
    To: Junio C Hamano <junkio@cox.net>
    cc: Daniel Barkalow <barkalow@iabervon.org>, git@vger.kernel.org
    Subject: Re: [PATCH-CAREFUL/RENAME] rename git-rpush and git-rpull to git-ssh-push and git-ssh-pull
    Message-ID: <Pine.LNX.4.58.0506051613370.1876@ppc970.osdl.org>
    References: <7vk6l9flzr.fsf@assigned-by-dhcp.cox.net>
     <Pine.LNX.4.21.0506050132590.30848-100000@iabervon.org>
     <7vfyvxb89m.fsf_-_@assigned-by-dhcp.cox.net> <Pine.LNX.4.58.0506051427280.1876@ppc970.osdl.org>
     <7vmzq4zata.fsf@assigned-by-dhcp.cox.net>

    On Sun, 5 Jun 2005, Junio C Hamano wrote:
    > 
    > Here is a rebase.  To apply with git-apply, you need the "Yes,
    > sir" patch to grok "rename from/to".

    Seems to have applied correctly. Very nice.

                    Linus

Re: [PATCH] Fix rename/copy when dealing with temporarily broken pairs.

[Jon Seymour]

On 6/14/05, Junio C Hamano <junkio@cox.net> wrote:
> JS> I wondered if this patch might fix it, but it doesn't seem to.
> Regular "patch" cannot grok either copy or rename.  If your
> recipient uses "patch", then you must not use -C nor -M when you
> are preparing a diff for his consumption.

Ok, there was no problem other than me not understanding what
git-apply did - thank you Pasky for bringing light to the dark regions
of my sleep-deprived brain!

jon.