Merge branch 'jc/cherry'

* jc/cherry:
  Documentation: --cherry-pick
  git-log --cherry-pick A...B
  Refactor patch-id filtering out of git-cherry and git-format-patch.
  Add %m to '--pretty=format:'

Documentation/git-rev-list.txt

@@ -22,6 +22,7 @@ SYNOPSIS
 	     [ \--topo-order ]
 	     [ \--parents ]
 	     [ \--left-right ]
+	     [ \--cherry-pick ]
 	     [ \--encoding[=<encoding>] ]
 	     [ \--(author|committer|grep)=<pattern> ]
 	     [ [\--objects | \--objects-edge] [ \--unpacked ] ]
@@ -224,6 +225,20 @@ limiting may be applied.
 	In addition to the '<commit>' listed on the command
 	line, read them from the standard input.
 
+--cherry-pick::
+
+	Omit any commit that introduces the same change as
+	another commit on the "other side" when the set of
+	commits are limited with symmetric difference.
++
+For example, if you have two branches, `A` and `B`, a usual way
+to list all commits on only one side of them is with
+`--left-right`, like the example above in the description of
+that option.  It however shows the commits that were cherry-picked
+from the other branch (for example, "3rd on b" may be cherry-picked
+from branch A).  With this option, such pairs of commits are
+excluded from the output.
+
 -g, --walk-reflogs::
 
 	Instead of walking the commit ancestry chain, walk

Documentation/pretty-formats.txt

@@ -117,6 +117,7 @@ The placeholders are:
 - '%Cgreen': switch color to green
 - '%Cblue': switch color to blue
 - '%Creset': reset color
+- '%m': left, right or boundary mark
 - '%n': newline
 
 

Makefile

@@ -283,7 +283,7 @@ LIB_H = \
 	diff.h object.h pack.h pkt-line.h quote.h refs.h list-objects.h sideband.h \
 	run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
 	tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \
-	utf8.h reflog-walk.h
+	utf8.h reflog-walk.h patch-ids.h
 
 DIFF_OBJS = \
 	diff.o diff-lib.o diffcore-break.o diffcore-order.o \
@@ -295,6 +295,7 @@ LIB_OBJS = \
 	date.o diff-delta.o entry.o exec_cmd.o ident.o \
 	interpolate.o \
 	lockfile.o \
+	patch-ids.o \
 	object.o pack-check.o patch-delta.o path.o pkt-line.o sideband.o \
 	reachable.o reflog-walk.o \
 	quote.o read-cache.o refs.o run-command.o dir.o object-refs.o \

builtin-log.c

@@ -12,6 +12,7 @@
 #include "builtin.h"
 #include "tag.h"
 #include "reflog-walk.h"
+#include "patch-ids.h"
 
 static int default_show_root = 1;
 
@@ -333,25 +334,12 @@ static int reopen_stdout(struct commit *commit, int nr, int keep_subject)
 
 }
 
-static int get_patch_id(struct commit *commit, struct diff_options *options,
-		unsigned char *sha1)
-{
-	if (commit->parents)
-		diff_tree_sha1(commit->parents->item->object.sha1,
-		               commit->object.sha1, "", options);
-	else
-		diff_root_tree_sha1(commit->object.sha1, "", options);
-	diffcore_std(options);
-	return diff_flush_patch_id(options, sha1);
-}
-
-static void get_patch_ids(struct rev_info *rev, struct diff_options *options, const char *prefix)
+static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids, const char *prefix)
 {
 	struct rev_info check_rev;
 	struct commit *commit;
 	struct object *o1, *o2;
 	unsigned flags1, flags2;
-	unsigned char sha1[20];
 
 	if (rev->pending.nr != 2)
 		die("Need exactly one range.");
@@ -364,10 +352,7 @@ static void get_patch_ids(struct rev_info *rev, struct diff_options *options, co
 	if ((flags1 & UNINTERESTING) == (flags2 & UNINTERESTING))
 		die("Not a range.");
 
-	diff_setup(options);
-	options->recursive = 1;
-	if (diff_setup_done(options) < 0)
-		die("diff_setup_done failed");
+	init_patch_ids(ids);
 
 	/* given a range a..b get all patch ids for b..a */
 	init_revisions(&check_rev, prefix);
@@ -382,8 +367,7 @@ static void get_patch_ids(struct rev_info *rev, struct diff_options *options, co
 		if (commit->parents && commit->parents->next)
 			continue;
 
-		if (!get_patch_id(commit, options, sha1))
-			created_object(sha1, xcalloc(1, sizeof(struct object)));
+		add_commit_patch_id(commit, ids);
 	}
 
 	/* reset for next revision walk */
@@ -421,7 +405,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	int ignore_if_in_upstream = 0;
 	int thread = 0;
 	const char *in_reply_to = NULL;
-	struct diff_options patch_id_opts;
+	struct patch_ids ids;
 	char *add_signoff = NULL;
 	char message_id[1024];
 	char ref_message_id[1024];
@@ -559,22 +543,19 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	}
 
 	if (ignore_if_in_upstream)
-		get_patch_ids(&rev, &patch_id_opts, prefix);
+		get_patch_ids(&rev, &ids, prefix);
 
 	if (!use_stdout)
 		realstdout = fdopen(dup(1), "w");
 
 	prepare_revision_walk(&rev);
 	while ((commit = get_revision(&rev)) != NULL) {
-		unsigned char sha1[20];
-
 		/* ignore merges */
 		if (commit->parents && commit->parents->next)
 			continue;
 
 		if (ignore_if_in_upstream &&
-				!get_patch_id(commit, &patch_id_opts, sha1) &&
-				lookup_object(sha1))
+				has_commit_patch_id(commit, &ids))
 			continue;
 
 		nr++;
@@ -629,6 +610,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 			fclose(stdout);
 	}
 	free(list);
+	if (ignore_if_in_upstream)
+		free_patch_ids(&ids);
 	return 0;
 }
 
@@ -651,7 +634,7 @@ static const char cherry_usage[] =
 int cmd_cherry(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
-	struct diff_options patch_id_opts;
+	struct patch_ids ids;
 	struct commit *commit;
 	struct commit_list *list = NULL;
 	const char *upstream;
@@ -697,7 +680,7 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 			return 0;
 	}
 
-	get_patch_ids(&revs, &patch_id_opts, prefix);
+	get_patch_ids(&revs, &ids, prefix);
 
 	if (limit && add_pending_commit(limit, &revs, UNINTERESTING))
 		die("Unknown commit %s", limit);
@@ -713,12 +696,10 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 	}
 
 	while (list) {
-		unsigned char sha1[20];
 		char sign = '+';
 
 		commit = list->item;
-		if (!get_patch_id(commit, &patch_id_opts, sha1) &&
-		    lookup_object(sha1))
+		if (has_commit_patch_id(commit, &ids))
 			sign = '-';
 
 		if (verbose) {
@@ -736,5 +717,6 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 		list = list->next;
 	}
 
+	free_patch_ids(&ids);
 	return 0;
 }

commit.c

@@ -4,6 +4,8 @@
 #include "pkt-line.h"
 #include "utf8.h"
 #include "interpolate.h"
+#include "diff.h"
+#include "revision.h"
 
 int save_commit_buffer = 1;
 
@@ -808,7 +810,8 @@ static long format_commit_message(const struct commit *commit,
 		{ "%Cgreen" },	/* green */
 		{ "%Cblue" },	/* blue */
 		{ "%Creset" },	/* reset color */
-		{ "%n" }	/* newline */
+		{ "%n" },	/* newline */
+		{ "%m" },	/* left/right/bottom */
 	};
 	enum interp_index {
 		IHASH = 0, IHASH_ABBREV,
@@ -824,14 +827,15 @@ static long format_commit_message(const struct commit *commit,
 		ISUBJECT,
 		IBODY,
 		IRED, IGREEN, IBLUE, IRESET_COLOR,
-		INEWLINE
+		INEWLINE,
+		ILEFT_RIGHT,
 	};
 	struct commit_list *p;
 	char parents[1024];
 	int i;
 	enum { HEADER, SUBJECT, BODY } state;
 
-	if (INEWLINE + 1 != ARRAY_SIZE(table))
+	if (ILEFT_RIGHT + 1 != ARRAY_SIZE(table))
 		die("invalid interp table!");
 
 	/* these are independent of the commit */
@@ -852,6 +856,12 @@ static long format_commit_message(const struct commit *commit,
 	interp_set_entry(table, ITREE_ABBREV,
 			find_unique_abbrev(commit->tree->object.sha1,
 				DEFAULT_ABBREV));
+	interp_set_entry(table, ILEFT_RIGHT,
+			 (commit->object.flags & BOUNDARY)
+			 ? "-"
+			 : (commit->object.flags & SYMMETRIC_LEFT)
+			 ? "<"
+			 : ">");
 
 	parents[1] = 0;
 	for (i = 0, p = commit->parents;

patch-ids.c

@@ -0,0 +1,192 @@
+#include "cache.h"
+#include "diff.h"
+#include "commit.h"
+#include "patch-ids.h"
+
+static int commit_patch_id(struct commit *commit, struct diff_options *options,
+		    unsigned char *sha1)
+{
+	if (commit->parents)
+		diff_tree_sha1(commit->parents->item->object.sha1,
+		               commit->object.sha1, "", options);
+	else
+		diff_root_tree_sha1(commit->object.sha1, "", options);
+	diffcore_std(options);
+	return diff_flush_patch_id(options, sha1);
+}
+
+static uint32_t take2(const unsigned char *id)
+{
+	return ((id[0] << 8) | id[1]);
+}
+
+/*
+ * Conventional binary search loop looks like this:
+ *
+ *      do {
+ *              int mi = (lo + hi) / 2;
+ *              int cmp = "entry pointed at by mi" minus "target";
+ *              if (!cmp)
+ *                      return (mi is the wanted one)
+ *              if (cmp > 0)
+ *                      hi = mi; "mi is larger than target"
+ *              else
+ *                      lo = mi+1; "mi is smaller than target"
+ *      } while (lo < hi);
+ *
+ * The invariants are:
+ *
+ * - When entering the loop, lo points at a slot that is never
+ *   above the target (it could be at the target), hi points at a
+ *   slot that is guaranteed to be above the target (it can never
+ *   be at the target).
+ *
+ * - We find a point 'mi' between lo and hi (mi could be the same
+ *   as lo, but never can be the same as hi), and check if it hits
+ *   the target.  There are three cases:
+ *
+ *    - if it is a hit, we are happy.
+ *
+ *    - if it is strictly higher than the target, we update hi with
+ *      it.
+ *
+ *    - if it is strictly lower than the target, we update lo to be
+ *      one slot after it, because we allow lo to be at the target.
+ *
+ * When choosing 'mi', we do not have to take the "middle" but
+ * anywhere in between lo and hi, as long as lo <= mi < hi is
+ * satisfied.  When we somehow know that the distance between the
+ * target and lo is much shorter than the target and hi, we could
+ * pick mi that is much closer to lo than the midway.
+ */
+static int patch_pos(struct patch_id **table, int nr, const unsigned char *id)
+{
+	int hi = nr;
+	int lo = 0;
+	int mi = 0;
+
+	if (!nr)
+		return -1;
+
+	if (nr != 1) {
+		unsigned lov, hiv, miv, ofs;
+
+		for (ofs = 0; ofs < 18; ofs += 2) {
+			lov = take2(table[0]->patch_id + ofs);
+			hiv = take2(table[nr-1]->patch_id + ofs);
+			miv = take2(id + ofs);
+			if (miv < lov)
+				return -1;
+			if (hiv < miv)
+				return -1 - nr;
+			if (lov != hiv) {
+				/*
+				 * At this point miv could be equal
+				 * to hiv (but id could still be higher);
+				 * the invariant of (mi < hi) should be
+				 * kept.
+				 */
+				mi = (nr-1) * (miv - lov) / (hiv - lov);
+				if (lo <= mi && mi < hi)
+					break;
+				die("oops");
+			}
+		}
+		if (18 <= ofs)
+			die("cannot happen -- lo and hi are identical");
+	}
+
+	do {
+		int cmp;
+		cmp = hashcmp(table[mi]->patch_id, id);
+		if (!cmp)
+			return mi;
+		if (cmp > 0)
+			hi = mi;
+		else
+			lo = mi + 1;
+		mi = (hi + lo) / 2;
+	} while (lo < hi);
+	return -lo-1;
+}
+
+#define BUCKET_SIZE 190 /* 190 * 21 = 3990, with slop close enough to 4K */
+struct patch_id_bucket {
+	struct patch_id_bucket *next;
+	int nr;
+	struct patch_id bucket[BUCKET_SIZE];
+};
+
+int init_patch_ids(struct patch_ids *ids)
+{
+	memset(ids, 0, sizeof(*ids));
+	diff_setup(&ids->diffopts);
+	ids->diffopts.recursive = 1;
+	if (diff_setup_done(&ids->diffopts) < 0)
+		return error("diff_setup_done failed");
+	return 0;
+}
+
+int free_patch_ids(struct patch_ids *ids)
+{
+	struct patch_id_bucket *next, *patches;
+
+	free(ids->table);
+	for (patches = ids->patches; patches; patches = next) {
+		next = patches->next;
+		free(patches);
+	}
+	return 0;
+}
+
+static struct patch_id *add_commit(struct commit *commit,
+				   struct patch_ids *ids,
+				   int no_add)
+{
+	struct patch_id_bucket *bucket;
+	struct patch_id *ent;
+	unsigned char sha1[20];
+	int pos;
+
+	if (commit_patch_id(commit, &ids->diffopts, sha1))
+		return NULL;
+	pos = patch_pos(ids->table, ids->nr, sha1);
+	if (0 <= pos)
+		return ids->table[pos];
+	if (no_add)
+		return NULL;
+
+	pos = -1 - pos;
+
+	bucket = ids->patches;
+	if (!bucket || (BUCKET_SIZE <= bucket->nr)) {
+		bucket = xcalloc(1, sizeof(*bucket));
+		bucket->next = ids->patches;
+		ids->patches = bucket;
+	}
+	ent = &bucket->bucket[bucket->nr++];
+	hashcpy(ent->patch_id, sha1);
+
+	if (ids->alloc <= ids->nr) {
+		ids->alloc = alloc_nr(ids->nr);
+		ids->table = xrealloc(ids->table, sizeof(ent) * ids->alloc);
+	}
+	if (pos < ids->nr)
+		memmove(ids->table + pos + 1, ids->table + pos,
+			sizeof(ent) * (ids->nr - pos));
+	ids->nr++;
+	ids->table[pos] = ent;
+	return ids->table[pos];
+}
+
+struct patch_id *has_commit_patch_id(struct commit *commit,
+				     struct patch_ids *ids)
+{
+	return add_commit(commit, ids, 1);
+}
+
+struct patch_id *add_commit_patch_id(struct commit *commit,
+				     struct patch_ids *ids)
+{
+	return add_commit(commit, ids, 0);
+}

patch-ids.h

@@ -0,0 +1,21 @@
+#ifndef PATCH_IDS_H
+#define PATCH_IDS_H
+
+struct patch_id {
+	unsigned char patch_id[20];
+	char seen;
+};
+
+struct patch_ids {
+	struct diff_options diffopts;
+	int nr, alloc;
+	struct patch_id **table;
+	struct patch_id_bucket *patches;
+};
+
+int init_patch_ids(struct patch_ids *);
+int free_patch_ids(struct patch_ids *);
+struct patch_id *add_commit_patch_id(struct commit *, struct patch_ids *);
+struct patch_id *has_commit_patch_id(struct commit *, struct patch_ids *);
+
+#endif /* PATCH_IDS_H */

revision.c

@@ -8,6 +8,7 @@
 #include "revision.h"
 #include "grep.h"
 #include "reflog-walk.h"
+#include "patch-ids.h"
 
 static char *path_name(struct name_path *path, const char *name)
 {
@@ -422,6 +423,86 @@ static void add_parents_to_list(struct rev_info *revs, struct commit *commit, st
 	}
 }
 
+static void cherry_pick_list(struct commit_list *list)
+{
+	struct commit_list *p;
+	int left_count = 0, right_count = 0;
+	int left_first;
+	struct patch_ids ids;
+
+	/* First count the commits on the left and on the right */
+	for (p = list; p; p = p->next) {
+		struct commit *commit = p->item;
+		unsigned flags = commit->object.flags;
+		if (flags & BOUNDARY)
+			;
+		else if (flags & SYMMETRIC_LEFT)
+			left_count++;
+		else
+			right_count++;
+	}
+
+	left_first = left_count < right_count;
+	init_patch_ids(&ids);
+
+	/* Compute patch-ids for one side */
+	for (p = list; p; p = p->next) {
+		struct commit *commit = p->item;
+		unsigned flags = commit->object.flags;
+
+		if (flags & BOUNDARY)
+			continue;
+		/*
+		 * If we have fewer left, left_first is set and we omit
+		 * commits on the right branch in this loop.  If we have
+		 * fewer right, we skip the left ones.
+		 */
+		if (left_first != !!(flags & SYMMETRIC_LEFT))
+			continue;
+		commit->util = add_commit_patch_id(commit, &ids);
+	}
+
+	/* Check the other side */
+	for (p = list; p; p = p->next) {
+		struct commit *commit = p->item;
+		struct patch_id *id;
+		unsigned flags = commit->object.flags;
+
+		if (flags & BOUNDARY)
+			continue;
+		/*
+		 * If we have fewer left, left_first is set and we omit
+		 * commits on the left branch in this loop.
+		 */
+		if (left_first == !!(flags & SYMMETRIC_LEFT))
+			continue;
+
+		/*
+		 * Have we seen the same patch id?
+		 */
+		id = has_commit_patch_id(commit, &ids);
+		if (!id)
+			continue;
+		id->seen = 1;
+		commit->object.flags |= SHOWN;
+	}
+
+	/* Now check the original side for seen ones */
+	for (p = list; p; p = p->next) {
+		struct commit *commit = p->item;
+		struct patch_id *ent;
+
+		ent = commit->util;
+		if (!ent)
+			continue;
+		if (ent->seen)
+			commit->object.flags |= SHOWN;
+		commit->util = NULL;
+	}
+
+	free_patch_ids(&ids);
+}
+
 static void limit_list(struct rev_info *revs)
 {
 	struct commit_list *list = revs->commits;
@@ -449,6 +530,9 @@ static void limit_list(struct rev_info *revs)
 			continue;
 		p = &commit_list_insert(commit, p)->next;
 	}
+	if (revs->cherry_pick)
+		cherry_pick_list(newlist);
+
 	revs->commits = newlist;
 }
 
@@ -914,6 +998,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				revs->left_right = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--cherry-pick")) {
+				revs->cherry_pick = 1;
+				continue;
+			}
 			if (!strcmp(arg, "--objects")) {
 				revs->tag_objects = 1;
 				revs->tree_objects = 1;

revision.h

@@ -47,6 +47,7 @@ struct rev_info {
 			left_right:1,
 			parents:1,
 			reverse:1,
+			cherry_pick:1,
 			first_parent_only:1;
 
 	/* Diff flags */