git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 00/29] Add blame to libgit
@ 2017-05-24  5:15 Jeff Smith
  2017-05-24  5:15 ` [PATCH 01/29] blame: remove unneeded dependency on blob.h Jeff Smith
                   ` (30 more replies)
  0 siblings, 31 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Rather than duplicate large portions of builtin/blame.c in cgit, it
would be better to shift its core functionality into libgit.a.  The
functionality left in builtin/blame.c mostly relates to terminal
presentation.

Since RFC v2 patchset:
  Rebased (merged in timestamp_t changes)
  Reorganized to split out and group 'mechanical' changes

Jeff Smith (29):
  blame: remove unneeded dependency on blob.h
  blame: move textconv_object with related functions
  blame: remove unused parameters
  blame: rename origin structure to blame_origin
  blame: rename scoreboard structure to blame_scoreboard
  blame: rename origin-related functions
  blame: rename coalesce function
  blame: rename ent_score function
  blame: rename nth_line function
  blame: move stat counters to scoreboard
  blame: move copy/move thresholds to scoreboard
  blame: move contents_from to scoreboard
  blame: move reverse flag to scoreboard
  blame: move show_root flag to scoreboard
  blame: move xdl_opts flags to scoreboard
  blame: move no_whole_file_rename flag to scoreboard
  blame: make sanity_check use a callback in scoreboard
  blame: move progess updates to a scoreboard callback
  blame: wrap blame_sort and compare_blame_final
  blame: rework methods that determine 'final' commit
  blame: create scoreboard init function
  blame: create scoreboard setup function
  blame: create entry prepend function
  blame: move core structures to header
  blame: move origin-related methods to libgit
  blame: move fake-commit-related methods to libgit
  blame: move scoreboard-related methods to libgit
  blame: move scoreboard setup to libgit
  blame: move entry prepend to libgit

 Makefile           |    1 +
 blame.c            | 1863 +++++++++++++++++++++++++++++++++++++++++++++
 blame.h            |  175 +++++
 builtin.h          |    2 -
 builtin/blame.c    | 2130 ++--------------------------------------------------
 builtin/cat-file.c |    1 +
 diff.c             |   23 +
 diff.h             |    7 +
 8 files changed, 2143 insertions(+), 2059 deletions(-)
 create mode 100644 blame.c
 create mode 100644 blame.h

-- 
2.9.3


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

* [PATCH 01/29] blame: remove unneeded dependency on blob.h
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 02/29] blame: move textconv_object with related functions Jeff Smith
                   ` (29 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

With commit 21666f1 ("convert object type handling from a string to a
number", 2007-02-26), there was no longer a need for blame.c to include
blob.h but it was not removed.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index f00eda1..d39f6af 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -8,7 +8,6 @@
 #include "cache.h"
 #include "refs.h"
 #include "builtin.h"
-#include "blob.h"
 #include "commit.h"
 #include "tag.h"
 #include "tree-walk.h"
-- 
2.9.3


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

* [PATCH 02/29] blame: move textconv_object with related functions
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
  2017-05-24  5:15 ` [PATCH 01/29] blame: remove unneeded dependency on blob.h Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 03/29] blame: remove unused parameters Jeff Smith
                   ` (28 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

textconv_object is used in places other than blame.c and should be moved
to a more appropriate location.  Other textconv related functions are
located in diff.c so that seems as good a place as any.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin.h          |  2 --
 builtin/blame.c    | 28 ----------------------------
 builtin/cat-file.c |  1 +
 diff.c             | 23 +++++++++++++++++++++++
 diff.h             |  7 +++++++
 5 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/builtin.h b/builtin.h
index 9e4a898..498ac80 100644
--- a/builtin.h
+++ b/builtin.h
@@ -25,8 +25,6 @@ struct fmt_merge_msg_opts {
 extern int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 			 struct fmt_merge_msg_opts *);
 
-extern int textconv_object(const char *path, unsigned mode, const struct object_id *oid, int oid_valid, char **buf, unsigned long *buf_size);
-
 extern int is_builtin(const char *s);
 
 extern int cmd_add(int argc, const char **argv, const char *prefix);
diff --git a/builtin/blame.c b/builtin/blame.c
index d39f6af..fbd757e 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -147,34 +147,6 @@ static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b,
 }
 
 /*
- * Prepare diff_filespec and convert it using diff textconv API
- * if the textconv driver exists.
- * Return 1 if the conversion succeeds, 0 otherwise.
- */
-int textconv_object(const char *path,
-		    unsigned mode,
-		    const struct object_id *oid,
-		    int oid_valid,
-		    char **buf,
-		    unsigned long *buf_size)
-{
-	struct diff_filespec *df;
-	struct userdiff_driver *textconv;
-
-	df = alloc_filespec(path);
-	fill_filespec(df, oid->hash, oid_valid, mode);
-	textconv = get_textconv(df);
-	if (!textconv) {
-		free_filespec(df);
-		return 0;
-	}
-
-	*buf_size = fill_textconv(textconv, df, buf);
-	free_filespec(df);
-	return 1;
-}
-
-/*
  * Given an origin, prepare mmfile_t structure to be used by the
  * diff machinery
  */
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 1890d7a..79a2c82 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -5,6 +5,7 @@
  */
 #include "cache.h"
 #include "builtin.h"
+#include "diff.h"
 #include "parse-options.h"
 #include "userdiff.h"
 #include "streaming.h"
diff --git a/diff.c b/diff.c
index 74283d9..040fb25 100644
--- a/diff.c
+++ b/diff.c
@@ -5270,6 +5270,29 @@ size_t fill_textconv(struct userdiff_driver *driver,
 	return size;
 }
 
+int textconv_object(const char *path,
+		    unsigned mode,
+		    const struct object_id *oid,
+		    int oid_valid,
+		    char **buf,
+		    unsigned long *buf_size)
+{
+	struct diff_filespec *df;
+	struct userdiff_driver *textconv;
+
+	df = alloc_filespec(path);
+	fill_filespec(df, oid->hash, oid_valid, mode);
+	textconv = get_textconv(df);
+	if (!textconv) {
+		free_filespec(df);
+		return 0;
+	}
+
+	*buf_size = fill_textconv(textconv, df, buf);
+	free_filespec(df);
+	return 1;
+}
+
 void setup_diff_pager(struct diff_options *opt)
 {
 	/*
diff --git a/diff.h b/diff.h
index 5be1ee7..52ebd54 100644
--- a/diff.h
+++ b/diff.h
@@ -385,6 +385,13 @@ extern size_t fill_textconv(struct userdiff_driver *driver,
  */
 extern struct userdiff_driver *get_textconv(struct diff_filespec *one);
 
+/*
+ * Prepare diff_filespec and convert it using diff textconv API
+ * if the textconv driver exists.
+ * Return 1 if the conversion succeeds, 0 otherwise.
+ */
+extern int textconv_object(const char *path, unsigned mode, const struct object_id *oid, int oid_valid, char **buf, unsigned long *buf_size);
+
 extern int parse_rename_score(const char **cp_p);
 
 extern long parse_algorithm_value(const char *value);
-- 
2.9.3


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

* [PATCH 03/29] blame: remove unused parameters
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
  2017-05-24  5:15 ` [PATCH 01/29] blame: remove unneeded dependency on blob.h Jeff Smith
  2017-05-24  5:15 ` [PATCH 02/29] blame: move textconv_object with related functions Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 04/29] blame: rename origin structure to blame_origin Jeff Smith
                   ` (27 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Clean up blame code before moving it into libgit

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index fbd757e..3529f01 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -449,9 +449,7 @@ static struct origin *make_origin(struct commit *commit, const char *path)
  * Locate an existing origin or create a new one.
  * This moves the origin to front position in the commit util list.
  */
-static struct origin *get_origin(struct scoreboard *sb,
-				 struct commit *commit,
-				 const char *path)
+static struct origin *get_origin(struct commit *commit, const char *path)
 {
 	struct origin *o, *l;
 
@@ -499,8 +497,7 @@ static int fill_blob_sha1_and_mode(struct origin *origin)
  * We have an origin -- check if the same path exists in the
  * parent and return an origin structure to represent it.
  */
-static struct origin *find_origin(struct scoreboard *sb,
-				  struct commit *parent,
+static struct origin *find_origin(struct commit *parent,
 				  struct origin *origin)
 {
 	struct origin *porigin;
@@ -543,7 +540,7 @@ static struct origin *find_origin(struct scoreboard *sb,
 
 	if (!diff_queued_diff.nr) {
 		/* The path is the same as parent */
-		porigin = get_origin(sb, parent, origin->path);
+		porigin = get_origin(parent, origin->path);
 		oidcpy(&porigin->blob_oid, &origin->blob_oid);
 		porigin->mode = origin->mode;
 	} else {
@@ -569,7 +566,7 @@ static struct origin *find_origin(struct scoreboard *sb,
 			die("internal error in blame::find_origin (%c)",
 			    p->status);
 		case 'M':
-			porigin = get_origin(sb, parent, origin->path);
+			porigin = get_origin(parent, origin->path);
 			oidcpy(&porigin->blob_oid, &p->one->oid);
 			porigin->mode = p->one->mode;
 			break;
@@ -588,8 +585,7 @@ static struct origin *find_origin(struct scoreboard *sb,
  * We have an origin -- find the path that corresponds to it in its
  * parent and return an origin structure to represent it.
  */
-static struct origin *find_rename(struct scoreboard *sb,
-				  struct commit *parent,
+static struct origin *find_rename(struct commit *parent,
 				  struct origin *origin)
 {
 	struct origin *porigin = NULL;
@@ -615,7 +611,7 @@ static struct origin *find_rename(struct scoreboard *sb,
 		struct diff_filepair *p = diff_queued_diff.queue[i];
 		if ((p->status == 'R' || p->status == 'C') &&
 		    !strcmp(p->two->path, origin->path)) {
-			porigin = get_origin(sb, parent, p->one->path);
+			porigin = get_origin(parent, p->one->path);
 			oidcpy(&porigin->blob_oid, &p->one->oid);
 			porigin->mode = p->one->mode;
 			break;
@@ -1270,7 +1266,7 @@ static void find_copy_in_parent(struct scoreboard *sb,
 				/* find_move already dealt with this path */
 				continue;
 
-			norigin = get_origin(sb, parent, p->one->path);
+			norigin = get_origin(parent, p->one->path);
 			oidcpy(&norigin->blob_oid, &p->one->oid);
 			norigin->mode = p->one->mode;
 			fill_origin_blob(&sb->revs->diffopt, norigin, &file_p);
@@ -1404,8 +1400,7 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
 	 * common cases, then we look for renames in the second pass.
 	 */
 	for (pass = 0; pass < 2 - no_whole_file_rename; pass++) {
-		struct origin *(*find)(struct scoreboard *,
-				       struct commit *, struct origin *);
+		struct origin *(*find)(struct commit *, struct origin *);
 		find = pass ? find_rename : find_origin;
 
 		for (i = 0, sg = first_scapegoat(revs, commit);
@@ -1418,7 +1413,7 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
 				continue;
 			if (parse_commit(p))
 				continue;
-			porigin = find(sb, p, origin);
+			porigin = find(p, origin);
 			if (!porigin)
 				continue;
 			if (!oidcmp(&porigin->blob_oid, &origin->blob_oid)) {
@@ -2806,7 +2801,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		sb.final_buf_size = o->file.size;
 	}
 	else {
-		o = get_origin(&sb, sb.final, path);
+		o = get_origin(sb.final, path);
 		if (fill_blob_sha1_and_mode(o))
 			die(_("no such path %s in %s"), path, final_commit_name);
 
-- 
2.9.3


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

* [PATCH 04/29] blame: rename origin structure to blame_origin
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (2 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 03/29] blame: remove unused parameters Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 05/29] blame: rename scoreboard structure to blame_scoreboard Jeff Smith
                   ` (26 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

The origin structure is core to the blame interface.  Since origin will
become more exposed, rename it to blame_origin to clarify what it is a
part of.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 114 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 57 insertions(+), 57 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 3529f01..3e8aaa4 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -87,10 +87,10 @@ static unsigned blame_copy_score;
 /*
  * One blob in a commit that is being suspected
  */
-struct origin {
+struct blame_origin {
 	int refcnt;
 	/* Record preceding blame record for this blob */
-	struct origin *previous;
+	struct blame_origin *previous;
 	/* origins are put in a list linked via `next' hanging off the
 	 * corresponding commit's util field in order to make finding
 	 * them fast.  The presence in this chain does not count
@@ -108,7 +108,7 @@ struct origin {
 	 * us get tripped up by this case, it certainly does not seem
 	 * worth optimizing for.
 	 */
-	struct origin *next;
+	struct blame_origin *next;
 	struct commit *commit;
 	/* `suspects' contains blame entries that may be attributed to
 	 * this origin's commit or to parent commits.  When a commit
@@ -151,7 +151,7 @@ static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b,
  * diff machinery
  */
 static void fill_origin_blob(struct diff_options *opt,
-			     struct origin *o, mmfile_t *file)
+			     struct blame_origin *o, mmfile_t *file)
 {
 	if (!o->file.ptr) {
 		enum object_type type;
@@ -180,17 +180,17 @@ static void fill_origin_blob(struct diff_options *opt,
  * Origin is refcounted and usually we keep the blob contents to be
  * reused.
  */
-static inline struct origin *origin_incref(struct origin *o)
+static inline struct blame_origin *origin_incref(struct blame_origin *o)
 {
 	if (o)
 		o->refcnt++;
 	return o;
 }
 
-static void origin_decref(struct origin *o)
+static void origin_decref(struct blame_origin *o)
 {
 	if (o && --o->refcnt <= 0) {
-		struct origin *p, *l = NULL;
+		struct blame_origin *p, *l = NULL;
 		if (o->previous)
 			origin_decref(o->previous);
 		free(o->file.ptr);
@@ -209,7 +209,7 @@ static void origin_decref(struct origin *o)
 	}
 }
 
-static void drop_origin_blob(struct origin *o)
+static void drop_origin_blob(struct blame_origin *o)
 {
 	if (o->file.ptr) {
 		free(o->file.ptr);
@@ -238,7 +238,7 @@ struct blame_entry {
 	int num_lines;
 
 	/* the commit that introduced this group into the final image */
-	struct origin *suspect;
+	struct blame_origin *suspect;
 
 	/* the line number of the first line of this group in the
 	 * suspect's file; internally all line numbers are 0 based.
@@ -410,13 +410,13 @@ static void coalesce(struct scoreboard *sb)
  * the commit priority queue of the score board.
  */
 
-static void queue_blames(struct scoreboard *sb, struct origin *porigin,
+static void queue_blames(struct scoreboard *sb, struct blame_origin *porigin,
 			 struct blame_entry *sorted)
 {
 	if (porigin->suspects)
 		porigin->suspects = blame_merge(porigin->suspects, sorted);
 	else {
-		struct origin *o;
+		struct blame_origin *o;
 		for (o = porigin->commit->util; o; o = o->next) {
 			if (o->suspects) {
 				porigin->suspects = sorted;
@@ -434,9 +434,9 @@ static void queue_blames(struct scoreboard *sb, struct origin *porigin,
  * get_origin() to obtain shared, refcounted copy instead of calling
  * this function directly.
  */
-static struct origin *make_origin(struct commit *commit, const char *path)
+static struct blame_origin *make_origin(struct commit *commit, const char *path)
 {
-	struct origin *o;
+	struct blame_origin *o;
 	FLEX_ALLOC_STR(o, path, path);
 	o->commit = commit;
 	o->refcnt = 1;
@@ -449,9 +449,9 @@ static struct origin *make_origin(struct commit *commit, const char *path)
  * Locate an existing origin or create a new one.
  * This moves the origin to front position in the commit util list.
  */
-static struct origin *get_origin(struct commit *commit, const char *path)
+static struct blame_origin *get_origin(struct commit *commit, const char *path)
 {
-	struct origin *o, *l;
+	struct blame_origin *o, *l;
 
 	for (o = commit->util, l = NULL; o; l = o, o = o->next) {
 		if (!strcmp(o->path, path)) {
@@ -476,7 +476,7 @@ static struct origin *get_origin(struct commit *commit, const char *path)
  *
  * This also fills origin->mode for corresponding tree path.
  */
-static int fill_blob_sha1_and_mode(struct origin *origin)
+static int fill_blob_sha1_and_mode(struct blame_origin *origin)
 {
 	if (!is_null_oid(&origin->blob_oid))
 		return 0;
@@ -497,10 +497,10 @@ static int fill_blob_sha1_and_mode(struct origin *origin)
  * We have an origin -- check if the same path exists in the
  * parent and return an origin structure to represent it.
  */
-static struct origin *find_origin(struct commit *parent,
-				  struct origin *origin)
+static struct blame_origin *find_origin(struct commit *parent,
+				  struct blame_origin *origin)
 {
-	struct origin *porigin;
+	struct blame_origin *porigin;
 	struct diff_options diff_opts;
 	const char *paths[2];
 
@@ -585,10 +585,10 @@ static struct origin *find_origin(struct commit *parent,
  * We have an origin -- find the path that corresponds to it in its
  * parent and return an origin structure to represent it.
  */
-static struct origin *find_rename(struct commit *parent,
-				  struct origin *origin)
+static struct blame_origin *find_rename(struct commit *parent,
+				  struct blame_origin *origin)
 {
-	struct origin *porigin = NULL;
+	struct blame_origin *porigin = NULL;
 	struct diff_options diff_opts;
 	int i;
 
@@ -680,7 +680,7 @@ static const char *nth_line_cb(void *data, long lno)
 static void split_overlap(struct blame_entry *split,
 			  struct blame_entry *e,
 			  int tlno, int plno, int same,
-			  struct origin *parent)
+			  struct blame_origin *parent)
 {
 	int chunk_end_lno;
 	memset(split, 0, sizeof(struct blame_entry [3]));
@@ -804,7 +804,7 @@ static struct blame_entry *reverse_blame(struct blame_entry *head,
  */
 static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq,
 			int tlno, int offset, int same,
-			struct origin *parent)
+			struct blame_origin *parent)
 {
 	struct blame_entry *e = **srcq;
 	struct blame_entry *samep = NULL, *diffp = NULL;
@@ -895,7 +895,7 @@ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq,
 }
 
 struct blame_chunk_cb_data {
-	struct origin *parent;
+	struct blame_origin *parent;
 	long offset;
 	struct blame_entry **dstq;
 	struct blame_entry **srcq;
@@ -920,8 +920,8 @@ static int blame_chunk_cb(long start_a, long count_a,
  * which lines came from parent and pass blame for them.
  */
 static void pass_blame_to_parent(struct scoreboard *sb,
-				 struct origin *target,
-				 struct origin *parent)
+				 struct blame_origin *target,
+				 struct blame_origin *parent)
 {
 	mmfile_t file_p, file_o;
 	struct blame_chunk_cb_data d;
@@ -1023,7 +1023,7 @@ static void copy_split_if_better(struct scoreboard *sb,
 static void handle_split(struct scoreboard *sb,
 			 struct blame_entry *ent,
 			 int tlno, int plno, int same,
-			 struct origin *parent,
+			 struct blame_origin *parent,
 			 struct blame_entry *split)
 {
 	if (ent->num_lines <= tlno)
@@ -1041,7 +1041,7 @@ static void handle_split(struct scoreboard *sb,
 struct handle_split_cb_data {
 	struct scoreboard *sb;
 	struct blame_entry *ent;
-	struct origin *parent;
+	struct blame_origin *parent;
 	struct blame_entry *split;
 	long plno;
 	long tlno;
@@ -1065,7 +1065,7 @@ static int handle_split_cb(long start_a, long count_a,
  */
 static void find_copy_in_blob(struct scoreboard *sb,
 			      struct blame_entry *ent,
-			      struct origin *parent,
+			      struct blame_origin *parent,
 			      struct blame_entry *split,
 			      mmfile_t *file_p)
 {
@@ -1129,8 +1129,8 @@ static struct blame_entry **filter_small(struct scoreboard *sb,
 static void find_move_in_parent(struct scoreboard *sb,
 				struct blame_entry ***blamed,
 				struct blame_entry **toosmall,
-				struct origin *target,
-				struct origin *parent)
+				struct blame_origin *target,
+				struct blame_origin *parent)
 {
 	struct blame_entry *e, split[3];
 	struct blame_entry *unblamed = target->suspects;
@@ -1205,9 +1205,9 @@ static struct blame_list *setup_blame_list(struct blame_entry *unblamed,
 static void find_copy_in_parent(struct scoreboard *sb,
 				struct blame_entry ***blamed,
 				struct blame_entry **toosmall,
-				struct origin *target,
+				struct blame_origin *target,
 				struct commit *parent,
-				struct origin *porigin,
+				struct blame_origin *porigin,
 				int opt)
 {
 	struct diff_options diff_opts;
@@ -1254,7 +1254,7 @@ static void find_copy_in_parent(struct scoreboard *sb,
 
 		for (i = 0; i < diff_queued_diff.nr; i++) {
 			struct diff_filepair *p = diff_queued_diff.queue[i];
-			struct origin *norigin;
+			struct blame_origin *norigin;
 			mmfile_t file_p;
 			struct blame_entry this[3];
 
@@ -1309,7 +1309,7 @@ static void find_copy_in_parent(struct scoreboard *sb,
  * origin is suspected for can be blamed on the parent.
  */
 static void pass_whole_blame(struct scoreboard *sb,
-			     struct origin *origin, struct origin *porigin)
+			     struct blame_origin *origin, struct blame_origin *porigin)
 {
 	struct blame_entry *e, *suspects;
 
@@ -1361,7 +1361,7 @@ static void distribute_blame(struct scoreboard *sb, struct blame_entry *blamed)
 	blamed = blame_sort(blamed, compare_blame_suspect);
 	while (blamed)
 	{
-		struct origin *porigin = blamed->suspect;
+		struct blame_origin *porigin = blamed->suspect;
 		struct blame_entry *suspects = NULL;
 		do {
 			struct blame_entry *next = blamed->next;
@@ -1376,14 +1376,14 @@ static void distribute_blame(struct scoreboard *sb, struct blame_entry *blamed)
 
 #define MAXSG 16
 
-static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
+static void pass_blame(struct scoreboard *sb, struct blame_origin *origin, int opt)
 {
 	struct rev_info *revs = sb->revs;
 	int i, pass, num_sg;
 	struct commit *commit = origin->commit;
 	struct commit_list *sg;
-	struct origin *sg_buf[MAXSG];
-	struct origin *porigin, **sg_origin = sg_buf;
+	struct blame_origin *sg_buf[MAXSG];
+	struct blame_origin *porigin, **sg_origin = sg_buf;
 	struct blame_entry *toosmall = NULL;
 	struct blame_entry *blames, **blametail = &blames;
 
@@ -1400,7 +1400,7 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
 	 * common cases, then we look for renames in the second pass.
 	 */
 	for (pass = 0; pass < 2 - no_whole_file_rename; pass++) {
-		struct origin *(*find)(struct commit *, struct origin *);
+		struct blame_origin *(*find)(struct commit *, struct blame_origin *);
 		find = pass ? find_rename : find_origin;
 
 		for (i = 0, sg = first_scapegoat(revs, commit);
@@ -1438,7 +1438,7 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
 	for (i = 0, sg = first_scapegoat(revs, commit);
 	     i < num_sg && sg;
 	     sg = sg->next, i++) {
-		struct origin *porigin = sg_origin[i];
+		struct blame_origin *porigin = sg_origin[i];
 		if (!porigin)
 			continue;
 		if (!origin->previous) {
@@ -1459,7 +1459,7 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
 			for (i = 0, sg = first_scapegoat(revs, commit);
 			     i < num_sg && sg;
 			     sg = sg->next, i++) {
-				struct origin *porigin = sg_origin[i];
+				struct blame_origin *porigin = sg_origin[i];
 				if (!porigin)
 					continue;
 				find_move_in_parent(sb, &blametail, &toosmall, origin, porigin);
@@ -1486,7 +1486,7 @@ static void pass_blame(struct scoreboard *sb, struct origin *origin, int opt)
 		for (i = 0, sg = first_scapegoat(revs, commit);
 		     i < num_sg && sg;
 		     sg = sg->next, i++) {
-			struct origin *porigin = sg_origin[i];
+			struct blame_origin *porigin = sg_origin[i];
 			find_copy_in_parent(sb, &blametail, &toosmall,
 					    origin, sg->item, porigin, opt);
 			if (!origin->suspects)
@@ -1665,10 +1665,10 @@ static void get_commit_info(struct commit *commit,
  * To allow LF and other nonportable characters in pathnames,
  * they are c-style quoted as needed.
  */
-static void write_filename_info(struct origin *suspect)
+static void write_filename_info(struct blame_origin *suspect)
 {
 	if (suspect->previous) {
-		struct origin *prev = suspect->previous;
+		struct blame_origin *prev = suspect->previous;
 		printf("previous %s ", oid_to_hex(&prev->commit->object.oid));
 		write_name_quoted(prev->path, stdout, '\n');
 	}
@@ -1682,7 +1682,7 @@ static void write_filename_info(struct origin *suspect)
  * the first time each commit appears in the output (unless the
  * user has specifically asked for us to repeat).
  */
-static int emit_one_suspect_detail(struct origin *suspect, int repeat)
+static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)
 {
 	struct commit_info ci;
 
@@ -1716,7 +1716,7 @@ static void found_guilty_entry(struct blame_entry *ent,
 			   struct progress_info *pi)
 {
 	if (incremental) {
-		struct origin *suspect = ent->suspect;
+		struct blame_origin *suspect = ent->suspect;
 
 		printf("%s %d %d %d\n",
 		       oid_to_hex(&suspect->commit->object.oid),
@@ -1745,7 +1745,7 @@ static void assign_blame(struct scoreboard *sb, int opt)
 
 	while (commit) {
 		struct blame_entry *ent;
-		struct origin *suspect = commit->util;
+		struct blame_origin *suspect = commit->util;
 
 		/* find one suspect to break down */
 		while (suspect && !suspect->suspects)
@@ -1842,7 +1842,7 @@ static const char *format_time(timestamp_t time, const char *tz_str,
 #define OUTPUT_SHOW_EMAIL	0400
 #define OUTPUT_LINE_PORCELAIN 01000
 
-static void emit_porcelain_details(struct origin *suspect, int repeat)
+static void emit_porcelain_details(struct blame_origin *suspect, int repeat)
 {
 	if (emit_one_suspect_detail(suspect, repeat) ||
 	    (suspect->commit->object.flags & MORE_THAN_ONE_PATH))
@@ -1855,7 +1855,7 @@ static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent,
 	int repeat = opt & OUTPUT_LINE_PORCELAIN;
 	int cnt;
 	const char *cp;
-	struct origin *suspect = ent->suspect;
+	struct blame_origin *suspect = ent->suspect;
 	char hex[GIT_MAX_HEXSZ + 1];
 
 	oid_to_hex_r(hex, &suspect->commit->object.oid);
@@ -1892,7 +1892,7 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
 {
 	int cnt;
 	const char *cp;
-	struct origin *suspect = ent->suspect;
+	struct blame_origin *suspect = ent->suspect;
 	struct commit_info ci;
 	char hex[GIT_MAX_HEXSZ + 1];
 	int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
@@ -1974,7 +1974,7 @@ static void output(struct scoreboard *sb, int option)
 	if (option & OUTPUT_PORCELAIN) {
 		for (ent = sb->ent; ent; ent = ent->next) {
 			int count = 0;
-			struct origin *suspect;
+			struct blame_origin *suspect;
 			struct commit *commit = ent->suspect->commit;
 			if (commit->object.flags & MORE_THAN_ONE_PATH)
 				continue;
@@ -2052,7 +2052,7 @@ static int read_ancestry(const char *graft_file)
 	return 0;
 }
 
-static int update_auto_abbrev(int auto_abbrev, struct origin *suspect)
+static int update_auto_abbrev(int auto_abbrev, struct blame_origin *suspect)
 {
 	const char *uniq = find_unique_abbrev(suspect->commit->object.oid.hash,
 					      auto_abbrev);
@@ -2076,7 +2076,7 @@ static void find_alignment(struct scoreboard *sb, int *option)
 	int auto_abbrev = DEFAULT_ABBREV;
 
 	for (e = sb->ent; e; e = e->next) {
-		struct origin *suspect = e->suspect;
+		struct blame_origin *suspect = e->suspect;
 		int num;
 
 		if (compute_auto_abbrev)
@@ -2268,7 +2268,7 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
 					       const char *contents_from)
 {
 	struct commit *commit;
-	struct origin *origin;
+	struct blame_origin *origin;
 	struct commit_list **parent_tail, *parent;
 	struct object_id head_oid;
 	struct strbuf buf = STRBUF_INIT;
@@ -2523,7 +2523,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	struct rev_info revs;
 	const char *path;
 	struct scoreboard sb;
-	struct origin *o;
+	struct blame_origin *o;
 	struct blame_entry *ent = NULL;
 	long dashdash_pos, lno;
 	char *final_commit_name = NULL;
-- 
2.9.3


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

* [PATCH 05/29] blame: rename scoreboard structure to blame_scoreboard
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (3 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 04/29] blame: rename origin structure to blame_origin Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 06/29] blame: rename origin-related functions Jeff Smith
                   ` (25 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

The scoreboard structure is core to the blame interface. Since
scoreboard will become more exposed, rename it to blame_scoreboard to
clarify what it is a part of.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 58 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 3e8aaa4..717868e 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -353,7 +353,7 @@ static int compare_commits_by_reverse_commit_date(const void *a,
 /*
  * The current state of the blame assignment.
  */
-struct scoreboard {
+struct blame_scoreboard {
 	/* the final commit (i.e. where we started digging from) */
 	struct commit *final;
 	/* Priority queue for commits with unassigned blame records */
@@ -377,14 +377,14 @@ struct scoreboard {
 	int *lineno;
 };
 
-static void sanity_check_refcnt(struct scoreboard *);
+static void sanity_check_refcnt(struct blame_scoreboard *);
 
 /*
  * If two blame entries that are next to each other came from
  * contiguous lines in the same origin (i.e. <commit, path> pair),
  * merge them together.
  */
-static void coalesce(struct scoreboard *sb)
+static void coalesce(struct blame_scoreboard *sb)
 {
 	struct blame_entry *ent, *next;
 
@@ -410,7 +410,7 @@ static void coalesce(struct scoreboard *sb)
  * the commit priority queue of the score board.
  */
 
-static void queue_blames(struct scoreboard *sb, struct blame_origin *porigin,
+static void queue_blames(struct blame_scoreboard *sb, struct blame_origin *porigin,
 			 struct blame_entry *sorted)
 {
 	if (porigin->suspects)
@@ -653,14 +653,14 @@ static void dup_entry(struct blame_entry ***queue,
 	*queue = &dst->next;
 }
 
-static const char *nth_line(struct scoreboard *sb, long lno)
+static const char *nth_line(struct blame_scoreboard *sb, long lno)
 {
 	return sb->final_buf + sb->lineno[lno];
 }
 
 static const char *nth_line_cb(void *data, long lno)
 {
-	return nth_line((struct scoreboard *)data, lno);
+	return nth_line((struct blame_scoreboard *)data, lno);
 }
 
 /*
@@ -919,7 +919,7 @@ static int blame_chunk_cb(long start_a, long count_a,
  * for the lines it is suspected to its parent.  Run diff to find
  * which lines came from parent and pass blame for them.
  */
-static void pass_blame_to_parent(struct scoreboard *sb,
+static void pass_blame_to_parent(struct blame_scoreboard *sb,
 				 struct blame_origin *target,
 				 struct blame_origin *parent)
 {
@@ -959,7 +959,7 @@ static void pass_blame_to_parent(struct scoreboard *sb,
  *
  * Compute how trivial the lines in the blame_entry are.
  */
-static unsigned ent_score(struct scoreboard *sb, struct blame_entry *e)
+static unsigned ent_score(struct blame_scoreboard *sb, struct blame_entry *e)
 {
 	unsigned score;
 	const char *cp, *ep;
@@ -986,7 +986,7 @@ static unsigned ent_score(struct scoreboard *sb, struct blame_entry *e)
  * so far, by comparing this and best_so_far and copying this into
  * bst_so_far as needed.
  */
-static void copy_split_if_better(struct scoreboard *sb,
+static void copy_split_if_better(struct blame_scoreboard *sb,
 				 struct blame_entry *best_so_far,
 				 struct blame_entry *this)
 {
@@ -1020,7 +1020,7 @@ static void copy_split_if_better(struct scoreboard *sb,
  *
  * All line numbers are 0-based.
  */
-static void handle_split(struct scoreboard *sb,
+static void handle_split(struct blame_scoreboard *sb,
 			 struct blame_entry *ent,
 			 int tlno, int plno, int same,
 			 struct blame_origin *parent,
@@ -1039,7 +1039,7 @@ static void handle_split(struct scoreboard *sb,
 }
 
 struct handle_split_cb_data {
-	struct scoreboard *sb;
+	struct blame_scoreboard *sb;
 	struct blame_entry *ent;
 	struct blame_origin *parent;
 	struct blame_entry *split;
@@ -1063,7 +1063,7 @@ static int handle_split_cb(long start_a, long count_a,
  * we can pass blames to it.  file_p has the blob contents for
  * the parent.
  */
-static void find_copy_in_blob(struct scoreboard *sb,
+static void find_copy_in_blob(struct blame_scoreboard *sb,
 			      struct blame_entry *ent,
 			      struct blame_origin *parent,
 			      struct blame_entry *split,
@@ -1099,7 +1099,7 @@ static void find_copy_in_blob(struct scoreboard *sb,
  * Returns a pointer to the link pointing to the old head of the small list.
  */
 
-static struct blame_entry **filter_small(struct scoreboard *sb,
+static struct blame_entry **filter_small(struct blame_scoreboard *sb,
 					 struct blame_entry **small,
 					 struct blame_entry **source,
 					 unsigned score_min)
@@ -1126,7 +1126,7 @@ static struct blame_entry **filter_small(struct scoreboard *sb,
  * See if lines currently target is suspected for can be attributed to
  * parent.
  */
-static void find_move_in_parent(struct scoreboard *sb,
+static void find_move_in_parent(struct blame_scoreboard *sb,
 				struct blame_entry ***blamed,
 				struct blame_entry **toosmall,
 				struct blame_origin *target,
@@ -1202,7 +1202,7 @@ static struct blame_list *setup_blame_list(struct blame_entry *unblamed,
  * across file boundary from the parent commit.  porigin is the path
  * in the parent we already tried.
  */
-static void find_copy_in_parent(struct scoreboard *sb,
+static void find_copy_in_parent(struct blame_scoreboard *sb,
 				struct blame_entry ***blamed,
 				struct blame_entry **toosmall,
 				struct blame_origin *target,
@@ -1308,7 +1308,7 @@ static void find_copy_in_parent(struct scoreboard *sb,
  * The blobs of origin and porigin exactly match, so everything
  * origin is suspected for can be blamed on the parent.
  */
-static void pass_whole_blame(struct scoreboard *sb,
+static void pass_whole_blame(struct blame_scoreboard *sb,
 			     struct blame_origin *origin, struct blame_origin *porigin)
 {
 	struct blame_entry *e, *suspects;
@@ -1356,7 +1356,7 @@ static int num_scapegoats(struct rev_info *revs, struct commit *commit)
 /* Distribute collected unsorted blames to the respected sorted lists
  * in the various origins.
  */
-static void distribute_blame(struct scoreboard *sb, struct blame_entry *blamed)
+static void distribute_blame(struct blame_scoreboard *sb, struct blame_entry *blamed)
 {
 	blamed = blame_sort(blamed, compare_blame_suspect);
 	while (blamed)
@@ -1376,7 +1376,7 @@ static void distribute_blame(struct scoreboard *sb, struct blame_entry *blamed)
 
 #define MAXSG 16
 
-static void pass_blame(struct scoreboard *sb, struct blame_origin *origin, int opt)
+static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin, int opt)
 {
 	struct rev_info *revs = sb->revs;
 	int i, pass, num_sg;
@@ -1733,7 +1733,7 @@ static void found_guilty_entry(struct blame_entry *ent,
  * The main loop -- while we have blobs with lines whose true origin
  * is still unknown, pick one blob, and allow its lines to pass blames
  * to its parents. */
-static void assign_blame(struct scoreboard *sb, int opt)
+static void assign_blame(struct blame_scoreboard *sb, int opt)
 {
 	struct rev_info *revs = sb->revs;
 	struct commit *commit = prio_queue_get(&sb->commits);
@@ -1849,7 +1849,7 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)
 		write_filename_info(suspect);
 }
 
-static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent,
+static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 			   int opt)
 {
 	int repeat = opt & OUTPUT_LINE_PORCELAIN;
@@ -1888,7 +1888,7 @@ static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent,
 		putchar('\n');
 }
 
-static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
+static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int opt)
 {
 	int cnt;
 	const char *cp;
@@ -1967,7 +1967,7 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
 	commit_info_destroy(&ci);
 }
 
-static void output(struct scoreboard *sb, int option)
+static void output(struct blame_scoreboard *sb, int option)
 {
 	struct blame_entry *ent;
 
@@ -2006,7 +2006,7 @@ static const char *get_next_line(const char *start, const char *end)
  * To allow quick access to the contents of nth line in the
  * final image, prepare an index in the scoreboard.
  */
-static int prepare_lines(struct scoreboard *sb)
+static int prepare_lines(struct blame_scoreboard *sb)
 {
 	const char *buf = sb->final_buf;
 	unsigned long len = sb->final_buf_size;
@@ -2066,7 +2066,7 @@ static int update_auto_abbrev(int auto_abbrev, struct blame_origin *suspect)
  * How many columns do we need to show line numbers, authors,
  * and filenames?
  */
-static void find_alignment(struct scoreboard *sb, int *option)
+static void find_alignment(struct blame_scoreboard *sb, int *option)
 {
 	int longest_src_lines = 0;
 	int longest_dst_lines = 0;
@@ -2120,7 +2120,7 @@ static void find_alignment(struct scoreboard *sb, int *option)
  * For debugging -- origin is refcounted, and this asserts that
  * we do not underflow.
  */
-static void sanity_check_refcnt(struct scoreboard *sb)
+static void sanity_check_refcnt(struct blame_scoreboard *sb)
 {
 	int baa = 0;
 	struct blame_entry *ent;
@@ -2411,14 +2411,14 @@ static struct commit *find_single_final(struct rev_info *revs,
 	return found;
 }
 
-static char *prepare_final(struct scoreboard *sb)
+static char *prepare_final(struct blame_scoreboard *sb)
 {
 	const char *name;
 	sb->final = find_single_final(sb->revs, &name);
 	return xstrdup_or_null(name);
 }
 
-static const char *dwim_reverse_initial(struct scoreboard *sb)
+static const char *dwim_reverse_initial(struct blame_scoreboard *sb)
 {
 	/*
 	 * DWIM "git blame --reverse ONE -- PATH" as
@@ -2453,7 +2453,7 @@ static const char *dwim_reverse_initial(struct scoreboard *sb)
 	return sb->revs->pending.objects[0].name;
 }
 
-static char *prepare_initial(struct scoreboard *sb)
+static char *prepare_initial(struct blame_scoreboard *sb)
 {
 	int i;
 	const char *final_commit_name = NULL;
@@ -2522,7 +2522,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
 	const char *path;
-	struct scoreboard sb;
+	struct blame_scoreboard sb;
 	struct blame_origin *o;
 	struct blame_entry *ent = NULL;
 	long dashdash_pos, lno;
-- 
2.9.3


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

* [PATCH 06/29] blame: rename origin-related functions
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (4 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 05/29] blame: rename scoreboard structure to blame_scoreboard Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 07/29] blame: rename coalesce function Jeff Smith
                   ` (24 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Functions related to blame_origin that will be publicly exposed should
have names that better reflect what they are a part of.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 58 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 717868e..7854770 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -180,19 +180,19 @@ static void fill_origin_blob(struct diff_options *opt,
  * Origin is refcounted and usually we keep the blob contents to be
  * reused.
  */
-static inline struct blame_origin *origin_incref(struct blame_origin *o)
+static inline struct blame_origin *blame_origin_incref(struct blame_origin *o)
 {
 	if (o)
 		o->refcnt++;
 	return o;
 }
 
-static void origin_decref(struct blame_origin *o)
+static void blame_origin_decref(struct blame_origin *o)
 {
 	if (o && --o->refcnt <= 0) {
 		struct blame_origin *p, *l = NULL;
 		if (o->previous)
-			origin_decref(o->previous);
+			blame_origin_decref(o->previous);
 		free(o->file.ptr);
 		/* Should be present exactly once in commit chain */
 		for (p = o->commit->util; p; l = p, p = p->next) {
@@ -205,7 +205,7 @@ static void origin_decref(struct blame_origin *o)
 				return;
 			}
 		}
-		die("internal error in blame::origin_decref");
+		die("internal error in blame_origin_decref");
 	}
 }
 
@@ -393,7 +393,7 @@ static void coalesce(struct blame_scoreboard *sb)
 		    ent->s_lno + ent->num_lines == next->s_lno) {
 			ent->num_lines += next->num_lines;
 			ent->next = next->next;
-			origin_decref(next->suspect);
+			blame_origin_decref(next->suspect);
 			free(next);
 			ent->score = 0;
 			next = ent; /* again */
@@ -461,7 +461,7 @@ static struct blame_origin *get_origin(struct commit *commit, const char *path)
 				o->next = commit->util;
 				commit->util = o;
 			}
-			return origin_incref(o);
+			return blame_origin_incref(o);
 		}
 	}
 	return make_origin(commit, path);
@@ -511,7 +511,7 @@ static struct blame_origin *find_origin(struct commit *parent,
 			 * The same path between origin and its parent
 			 * without renaming -- the most common case.
 			 */
-			return origin_incref (porigin);
+			return blame_origin_incref (porigin);
 		}
 
 	/* See if the origin->path is different between parent
@@ -630,7 +630,7 @@ static void add_blame_entry(struct blame_entry ***queue,
 {
 	struct blame_entry *e = xmalloc(sizeof(*e));
 	memcpy(e, src, sizeof(*e));
-	origin_incref(e->suspect);
+	blame_origin_incref(e->suspect);
 
 	e->next = **queue;
 	**queue = e;
@@ -645,8 +645,8 @@ static void add_blame_entry(struct blame_entry ***queue,
 static void dup_entry(struct blame_entry ***queue,
 		      struct blame_entry *dst, struct blame_entry *src)
 {
-	origin_incref(src->suspect);
-	origin_decref(dst->suspect);
+	blame_origin_incref(src->suspect);
+	blame_origin_decref(dst->suspect);
 	memcpy(dst, src, sizeof(*src));
 	dst->next = **queue;
 	**queue = dst;
@@ -687,7 +687,7 @@ static void split_overlap(struct blame_entry *split,
 
 	if (e->s_lno < tlno) {
 		/* there is a pre-chunk part not blamed on parent */
-		split[0].suspect = origin_incref(e->suspect);
+		split[0].suspect = blame_origin_incref(e->suspect);
 		split[0].lno = e->lno;
 		split[0].s_lno = e->s_lno;
 		split[0].num_lines = tlno - e->s_lno;
@@ -701,7 +701,7 @@ static void split_overlap(struct blame_entry *split,
 
 	if (same < e->s_lno + e->num_lines) {
 		/* there is a post-chunk part not blamed on parent */
-		split[2].suspect = origin_incref(e->suspect);
+		split[2].suspect = blame_origin_incref(e->suspect);
 		split[2].lno = e->lno + (same - e->s_lno);
 		split[2].s_lno = e->s_lno + (same - e->s_lno);
 		split[2].num_lines = e->s_lno + e->num_lines - same;
@@ -717,7 +717,7 @@ static void split_overlap(struct blame_entry *split,
 	 */
 	if (split[1].num_lines < 1)
 		return;
-	split[1].suspect = origin_incref(parent);
+	split[1].suspect = blame_origin_incref(parent);
 }
 
 /*
@@ -767,7 +767,7 @@ static void decref_split(struct blame_entry *split)
 	int i;
 
 	for (i = 0; i < 3; i++)
-		origin_decref(split[i].suspect);
+		blame_origin_decref(split[i].suspect);
 }
 
 /*
@@ -830,10 +830,10 @@ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq,
 			n->next = diffp;
 			diffp = n;
 		} else
-			origin_decref(e->suspect);
+			blame_origin_decref(e->suspect);
 		/* Pass blame for everything before the differing
 		 * chunk to the parent */
-		e->suspect = origin_incref(parent);
+		e->suspect = blame_origin_incref(parent);
 		e->s_lno += offset;
 		e->next = samep;
 		samep = e;
@@ -874,7 +874,7 @@ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq,
 			 */
 			int len = same - e->s_lno;
 			struct blame_entry *n = xcalloc(1, sizeof (struct blame_entry));
-			n->suspect = origin_incref(e->suspect);
+			n->suspect = blame_origin_incref(e->suspect);
 			n->lno = e->lno + len;
 			n->s_lno = e->s_lno + len;
 			n->num_lines = e->num_lines - len;
@@ -1000,7 +1000,7 @@ static void copy_split_if_better(struct blame_scoreboard *sb,
 	}
 
 	for (i = 0; i < 3; i++)
-		origin_incref(this[i].suspect);
+		blame_origin_incref(this[i].suspect);
 	decref_split(best_so_far);
 	memcpy(best_so_far, this, sizeof(struct blame_entry [3]));
 }
@@ -1280,7 +1280,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 						     this);
 				decref_split(this);
 			}
-			origin_decref(norigin);
+			blame_origin_decref(norigin);
 		}
 
 		for (j = 0; j < num_ents; j++) {
@@ -1321,8 +1321,8 @@ static void pass_whole_blame(struct blame_scoreboard *sb,
 	suspects = origin->suspects;
 	origin->suspects = NULL;
 	for (e = suspects; e; e = e->next) {
-		origin_incref(porigin);
-		origin_decref(e->suspect);
+		blame_origin_incref(porigin);
+		blame_origin_decref(e->suspect);
 		e->suspect = porigin;
 	}
 	queue_blames(sb, porigin, suspects);
@@ -1418,7 +1418,7 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin,
 				continue;
 			if (!oidcmp(&porigin->blob_oid, &origin->blob_oid)) {
 				pass_whole_blame(sb, origin, porigin);
-				origin_decref(porigin);
+				blame_origin_decref(porigin);
 				goto finish;
 			}
 			for (j = same = 0; j < i; j++)
@@ -1430,7 +1430,7 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin,
 			if (!same)
 				sg_origin[i] = porigin;
 			else
-				origin_decref(porigin);
+				blame_origin_decref(porigin);
 		}
 	}
 
@@ -1442,7 +1442,7 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin,
 		if (!porigin)
 			continue;
 		if (!origin->previous) {
-			origin_incref(porigin);
+			blame_origin_incref(porigin);
 			origin->previous = porigin;
 		}
 		pass_blame_to_parent(sb, origin, porigin);
@@ -1513,7 +1513,7 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin,
 	for (i = 0; i < num_sg; i++) {
 		if (sg_origin[i]) {
 			drop_origin_blob(sg_origin[i]);
-			origin_decref(sg_origin[i]);
+			blame_origin_decref(sg_origin[i]);
 		}
 	}
 	drop_origin_blob(origin);
@@ -1762,7 +1762,7 @@ static void assign_blame(struct blame_scoreboard *sb, int opt)
 		 * We will use this suspect later in the loop,
 		 * so hold onto it in the meantime.
 		 */
-		origin_incref(suspect);
+		blame_origin_incref(suspect);
 		parse_commit(commit);
 		if (reverse ||
 		    (!(commit->object.flags & UNINTERESTING) &&
@@ -1794,7 +1794,7 @@ static void assign_blame(struct blame_scoreboard *sb, int opt)
 				break;
 			}
 		}
-		origin_decref(suspect);
+		blame_origin_decref(suspect);
 
 		if (DEBUG) /* sanity */
 			sanity_check_refcnt(sb);
@@ -2857,13 +2857,13 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		ent->suspect = o;
 		ent->s_lno = bottom;
 		ent->next = next;
-		origin_incref(o);
+		blame_origin_incref(o);
 	}
 
 	o->suspects = ent;
 	prio_queue_put(&sb.commits, o->commit);
 
-	origin_decref(o);
+	blame_origin_decref(o);
 
 	range_set_release(&ranges);
 	string_list_clear(&range_list, 0);
-- 
2.9.3


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

* [PATCH 07/29] blame: rename coalesce function
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (5 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 06/29] blame: rename origin-related functions Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 08/29] blame: rename ent_score function Jeff Smith
                   ` (23 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Functions that will be publicly exposed should have names that better
reflect what they are a part of.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 7854770..7c493d2 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -384,7 +384,7 @@ static void sanity_check_refcnt(struct blame_scoreboard *);
  * contiguous lines in the same origin (i.e. <commit, path> pair),
  * merge them together.
  */
-static void coalesce(struct blame_scoreboard *sb)
+static void blame_coalesce(struct blame_scoreboard *sb)
 {
 	struct blame_entry *ent, *next;
 
@@ -2885,7 +2885,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 
 	sb.ent = blame_sort(sb.ent, compare_blame_final);
 
-	coalesce(&sb);
+	blame_coalesce(&sb);
 
 	if (!(output_option & OUTPUT_PORCELAIN))
 		find_alignment(&sb, &output_option);
-- 
2.9.3


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

* [PATCH 08/29] blame: rename ent_score function
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (6 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 07/29] blame: rename coalesce function Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 09/29] blame: rename nth_line function Jeff Smith
                   ` (22 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Functions that will be publicly exposed should have names that better
reflect what they are a part of.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 7c493d2..129ef28 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -959,7 +959,7 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb,
  *
  * Compute how trivial the lines in the blame_entry are.
  */
-static unsigned ent_score(struct blame_scoreboard *sb, struct blame_entry *e)
+static unsigned blame_entry_score(struct blame_scoreboard *sb, struct blame_entry *e)
 {
 	unsigned score;
 	const char *cp, *ep;
@@ -995,7 +995,7 @@ static void copy_split_if_better(struct blame_scoreboard *sb,
 	if (!this[1].suspect)
 		return;
 	if (best_so_far[1].suspect) {
-		if (ent_score(sb, &this[1]) < ent_score(sb, &best_so_far[1]))
+		if (blame_entry_score(sb, &this[1]) < blame_entry_score(sb, &best_so_far[1]))
 			return;
 	}
 
@@ -1107,7 +1107,7 @@ static struct blame_entry **filter_small(struct blame_scoreboard *sb,
 	struct blame_entry *p = *source;
 	struct blame_entry *oldsmall = *small;
 	while (p) {
-		if (ent_score(sb, p) <= score_min) {
+		if (blame_entry_score(sb, p) <= score_min) {
 			*small = p;
 			small = &p->next;
 			p = *small;
@@ -1156,7 +1156,7 @@ static void find_move_in_parent(struct blame_scoreboard *sb,
 			next = e->next;
 			find_copy_in_blob(sb, e, parent, split, &file_p);
 			if (split[1].suspect &&
-			    blame_move_score < ent_score(sb, &split[1])) {
+			    blame_move_score < blame_entry_score(sb, &split[1])) {
 				split_blame(blamed, &unblamedtail, split, e);
 			} else {
 				e->next = leftover;
@@ -1286,7 +1286,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 		for (j = 0; j < num_ents; j++) {
 			struct blame_entry *split = blame_list[j].split;
 			if (split[1].suspect &&
-			    blame_copy_score < ent_score(sb, &split[1])) {
+			    blame_copy_score < blame_entry_score(sb, &split[1])) {
 				split_blame(blamed, &unblamedtail, split,
 					    blame_list[j].ent);
 			} else {
@@ -2104,8 +2104,8 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)
 		num = e->lno + e->num_lines;
 		if (longest_dst_lines < num)
 			longest_dst_lines = num;
-		if (largest_score < ent_score(sb, e))
-			largest_score = ent_score(sb, e);
+		if (largest_score < blame_entry_score(sb, e))
+			largest_score = blame_entry_score(sb, e);
 	}
 	max_orig_digits = decimal_width(longest_src_lines);
 	max_digits = decimal_width(longest_dst_lines);
-- 
2.9.3


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

* [PATCH 09/29] blame: rename nth_line function
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (7 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 08/29] blame: rename ent_score function Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 10/29] blame: move stat counters to scoreboard Jeff Smith
                   ` (21 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Functions that will be publicly exposed should have names that better
reflect what they are a part of.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 129ef28..5082543 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -653,14 +653,14 @@ static void dup_entry(struct blame_entry ***queue,
 	*queue = &dst->next;
 }
 
-static const char *nth_line(struct blame_scoreboard *sb, long lno)
+static const char *blame_nth_line(struct blame_scoreboard *sb, long lno)
 {
 	return sb->final_buf + sb->lineno[lno];
 }
 
 static const char *nth_line_cb(void *data, long lno)
 {
-	return nth_line((struct blame_scoreboard *)data, lno);
+	return blame_nth_line((struct blame_scoreboard *)data, lno);
 }
 
 /*
@@ -968,8 +968,8 @@ static unsigned blame_entry_score(struct blame_scoreboard *sb, struct blame_entr
 		return e->score;
 
 	score = 1;
-	cp = nth_line(sb, e->lno);
-	ep = nth_line(sb, e->lno + e->num_lines);
+	cp = blame_nth_line(sb, e->lno);
+	ep = blame_nth_line(sb, e->lno + e->num_lines);
 	while (cp < ep) {
 		unsigned ch = *((unsigned char *)cp);
 		if (isalnum(ch))
@@ -1078,9 +1078,9 @@ static void find_copy_in_blob(struct blame_scoreboard *sb,
 	/*
 	 * Prepare mmfile that contains only the lines in ent.
 	 */
-	cp = nth_line(sb, ent->lno);
+	cp = blame_nth_line(sb, ent->lno);
 	file_o.ptr = (char *) cp;
-	file_o.size = nth_line(sb, ent->lno + ent->num_lines) - cp;
+	file_o.size = blame_nth_line(sb, ent->lno + ent->num_lines) - cp;
 
 	/*
 	 * file_o is a part of final image we are annotating.
@@ -1866,7 +1866,7 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,
 	       ent->num_lines);
 	emit_porcelain_details(suspect, repeat);
 
-	cp = nth_line(sb, ent->lno);
+	cp = blame_nth_line(sb, ent->lno);
 	for (cnt = 0; cnt < ent->num_lines; cnt++) {
 		char ch;
 		if (cnt) {
@@ -1900,7 +1900,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
 	get_commit_info(suspect->commit, &ci, 1);
 	oid_to_hex_r(hex, &suspect->commit->object.oid);
 
-	cp = nth_line(sb, ent->lno);
+	cp = blame_nth_line(sb, ent->lno);
 	for (cnt = 0; cnt < ent->num_lines; cnt++) {
 		char ch;
 		int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
-- 
2.9.3


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

* [PATCH 10/29] blame: move stat counters to scoreboard
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (8 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 09/29] blame: rename nth_line function Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 11/29] blame: move copy/move thresholds " Jeff Smith
                   ` (20 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Statistic counters are used in parts of blame that are being moved to
libgit, and should be accessible via the scoreboard structure.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 5082543..f0c9ab8 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -61,11 +61,6 @@ static struct string_list mailmap = STRING_LIST_INIT_NODUP;
 #define DEBUG 0
 #endif
 
-/* stats */
-static int num_read_blob;
-static int num_get_patch;
-static int num_commits;
-
 #define PICKAXE_BLAME_MOVE		01
 #define PICKAXE_BLAME_COPY		02
 #define PICKAXE_BLAME_COPY_HARDER	04
@@ -151,13 +146,13 @@ static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b,
  * diff machinery
  */
 static void fill_origin_blob(struct diff_options *opt,
-			     struct blame_origin *o, mmfile_t *file)
+			     struct blame_origin *o, mmfile_t *file, int *num_read_blob)
 {
 	if (!o->file.ptr) {
 		enum object_type type;
 		unsigned long file_size;
 
-		num_read_blob++;
+		(*num_read_blob)++;
 		if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
 		    textconv_object(o->path, o->mode, &o->blob_oid, 1, &file->ptr, &file_size))
 			;
@@ -375,6 +370,11 @@ struct blame_scoreboard {
 	/* look-up a line in the final buffer */
 	int num_lines;
 	int *lineno;
+
+	/* stats */
+	int num_read_blob;
+	int num_get_patch;
+	int num_commits;
 };
 
 static void sanity_check_refcnt(struct blame_scoreboard *);
@@ -934,9 +934,9 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb,
 	d.offset = 0;
 	d.dstq = &newdest; d.srcq = &target->suspects;
 
-	fill_origin_blob(&sb->revs->diffopt, parent, &file_p);
-	fill_origin_blob(&sb->revs->diffopt, target, &file_o);
-	num_get_patch++;
+	fill_origin_blob(&sb->revs->diffopt, parent, &file_p, &sb->num_read_blob);
+	fill_origin_blob(&sb->revs->diffopt, target, &file_o, &sb->num_read_blob);
+	sb->num_get_patch++;
 
 	if (diff_hunks(&file_p, &file_o, blame_chunk_cb, &d))
 		die("unable to generate diff (%s -> %s)",
@@ -1140,7 +1140,7 @@ static void find_move_in_parent(struct blame_scoreboard *sb,
 	if (!unblamed)
 		return; /* nothing remains for this target */
 
-	fill_origin_blob(&sb->revs->diffopt, parent, &file_p);
+	fill_origin_blob(&sb->revs->diffopt, parent, &file_p, &sb->num_read_blob);
 	if (!file_p.ptr)
 		return;
 
@@ -1269,7 +1269,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 			norigin = get_origin(parent, p->one->path);
 			oidcpy(&norigin->blob_oid, &p->one->oid);
 			norigin->mode = p->one->mode;
-			fill_origin_blob(&sb->revs->diffopt, norigin, &file_p);
+			fill_origin_blob(&sb->revs->diffopt, norigin, &file_p, &sb->num_read_blob);
 			if (!file_p.ptr)
 				continue;
 
@@ -1434,7 +1434,7 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin,
 		}
 	}
 
-	num_commits++;
+	sb->num_commits++;
 	for (i = 0, sg = first_scapegoat(revs, commit);
 	     i < num_sg && sg;
 	     sg = sg->next, i++) {
@@ -2818,7 +2818,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 			    oid_to_hex(&o->blob_oid),
 			    path);
 	}
-	num_read_blob++;
+	sb.num_read_blob++;
 	lno = prepare_lines(&sb);
 
 	if (lno && !range_list.nr)
@@ -2899,9 +2899,9 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	}
 
 	if (show_stats) {
-		printf("num read blob: %d\n", num_read_blob);
-		printf("num get patch: %d\n", num_get_patch);
-		printf("num commits: %d\n", num_commits);
+		printf("num read blob: %d\n", sb.num_read_blob);
+		printf("num get patch: %d\n", sb.num_get_patch);
+		printf("num commits: %d\n", sb.num_commits);
 	}
 	return 0;
 }
-- 
2.9.3


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

* [PATCH 11/29] blame: move copy/move thresholds to scoreboard
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (9 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 10/29] blame: move stat counters to scoreboard Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 12/29] blame: move contents_from " Jeff Smith
                   ` (19 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Copy and move score thresholds are used in parts of blame that are being
moved to libgit, and should be accessible via the scoreboard structure.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index f0c9ab8..643f847 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -66,10 +66,6 @@ static struct string_list mailmap = STRING_LIST_INIT_NODUP;
 #define PICKAXE_BLAME_COPY_HARDER	04
 #define PICKAXE_BLAME_COPY_HARDEST	010
 
-/*
- * blame for a blame_entry with score lower than these thresholds
- * is not passed to the parent using move/copy logic.
- */
 static unsigned blame_move_score;
 static unsigned blame_copy_score;
 #define BLAME_DEFAULT_MOVE_SCORE	20
@@ -375,6 +371,13 @@ struct blame_scoreboard {
 	int num_read_blob;
 	int num_get_patch;
 	int num_commits;
+
+	/*
+	 * blame for a blame_entry with score lower than these thresholds
+	 * is not passed to the parent using move/copy logic.
+	 */
+	unsigned move_score;
+	unsigned copy_score;
 };
 
 static void sanity_check_refcnt(struct blame_scoreboard *);
@@ -1156,7 +1159,7 @@ static void find_move_in_parent(struct blame_scoreboard *sb,
 			next = e->next;
 			find_copy_in_blob(sb, e, parent, split, &file_p);
 			if (split[1].suspect &&
-			    blame_move_score < blame_entry_score(sb, &split[1])) {
+			    sb->move_score < blame_entry_score(sb, &split[1])) {
 				split_blame(blamed, &unblamedtail, split, e);
 			} else {
 				e->next = leftover;
@@ -1165,7 +1168,7 @@ static void find_move_in_parent(struct blame_scoreboard *sb,
 			decref_split(split);
 		}
 		*unblamedtail = NULL;
-		toosmall = filter_small(sb, toosmall, &unblamed, blame_move_score);
+		toosmall = filter_small(sb, toosmall, &unblamed, sb->move_score);
 	} while (unblamed);
 	target->suspects = reverse_blame(leftover, NULL);
 }
@@ -1286,7 +1289,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 		for (j = 0; j < num_ents; j++) {
 			struct blame_entry *split = blame_list[j].split;
 			if (split[1].suspect &&
-			    blame_copy_score < blame_entry_score(sb, &split[1])) {
+			    sb->copy_score < blame_entry_score(sb, &split[1])) {
 				split_blame(blamed, &unblamedtail, split,
 					    blame_list[j].ent);
 			} else {
@@ -1297,7 +1300,7 @@ static void find_copy_in_parent(struct blame_scoreboard *sb,
 		}
 		free(blame_list);
 		*unblamedtail = NULL;
-		toosmall = filter_small(sb, toosmall, &unblamed, blame_copy_score);
+		toosmall = filter_small(sb, toosmall, &unblamed, sb->copy_score);
 	} while (unblamed);
 	target->suspects = reverse_blame(leftover, NULL);
 	diff_flush(&diff_opts);
@@ -1454,7 +1457,7 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin,
 	 * Optionally find moves in parents' files.
 	 */
 	if (opt & PICKAXE_BLAME_MOVE) {
-		filter_small(sb, &toosmall, &origin->suspects, blame_move_score);
+		filter_small(sb, &toosmall, &origin->suspects, sb->move_score);
 		if (origin->suspects) {
 			for (i = 0, sg = first_scapegoat(revs, commit);
 			     i < num_sg && sg;
@@ -1473,12 +1476,12 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin,
 	 * Optionally find copies from parents' files.
 	 */
 	if (opt & PICKAXE_BLAME_COPY) {
-		if (blame_copy_score > blame_move_score)
-			filter_small(sb, &toosmall, &origin->suspects, blame_copy_score);
-		else if (blame_copy_score < blame_move_score) {
+		if (sb->copy_score > sb->move_score)
+			filter_small(sb, &toosmall, &origin->suspects, sb->copy_score);
+		else if (sb->copy_score < sb->move_score) {
 			origin->suspects = blame_merge(origin->suspects, toosmall);
 			toosmall = NULL;
-			filter_small(sb, &toosmall, &origin->suspects, blame_copy_score);
+			filter_small(sb, &toosmall, &origin->suspects, sb->copy_score);
 		}
 		if (!origin->suspects)
 			goto finish;
@@ -2675,11 +2678,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		opt |= (PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE |
 			PICKAXE_BLAME_COPY_HARDER);
 
-	if (!blame_move_score)
-		blame_move_score = BLAME_DEFAULT_MOVE_SCORE;
-	if (!blame_copy_score)
-		blame_copy_score = BLAME_DEFAULT_COPY_SCORE;
-
 	/*
 	 * We have collected options unknown to us in argv[1..unk]
 	 * which are to be passed to revision machinery if we are
@@ -2733,6 +2731,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	revs.disable_stdin = 1;
 	setup_revisions(argc, argv, &revs, NULL);
 	memset(&sb, 0, sizeof(sb));
+	sb.move_score = BLAME_DEFAULT_MOVE_SCORE;
+	sb.copy_score = BLAME_DEFAULT_COPY_SCORE;
 
 	sb.revs = &revs;
 	if (!reverse) {
@@ -2871,6 +2871,11 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	sb.ent = NULL;
 	sb.path = path;
 
+	if (blame_move_score)
+		sb.move_score = blame_move_score;
+	if (blame_copy_score)
+		sb.copy_score = blame_copy_score;
+
 	read_mailmap(&mailmap, NULL);
 
 	assign_blame(&sb, opt);
-- 
2.9.3


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

* [PATCH 12/29] blame: move contents_from to scoreboard
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (10 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 11/29] blame: move copy/move thresholds " Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 13/29] blame: move reverse flag " Jeff Smith
                   ` (18 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

The argument from --contents is used in parts of blame that are being
moved to libgit, and should be accessible via the scoreboard structure.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/builtin/blame.c b/builtin/blame.c
index 643f847..0955fc1 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -378,6 +378,9 @@ struct blame_scoreboard {
 	 */
 	unsigned move_score;
 	unsigned copy_score;
+
+	/* use this file's contents as the final image */
+	const char *contents_from;
 };
 
 static void sanity_check_refcnt(struct blame_scoreboard *);
@@ -2735,6 +2738,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	sb.copy_score = BLAME_DEFAULT_COPY_SCORE;
 
 	sb.revs = &revs;
+	sb.contents_from = contents_from;
 	if (!reverse) {
 		final_commit_name = prepare_final(&sb);
 		sb.commits.compare = compare_commits_by_commit_date;
-- 
2.9.3


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

* [PATCH 13/29] blame: move reverse flag to scoreboard
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (11 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 12/29] blame: move contents_from " Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 14/29] blame: move show_root " Jeff Smith
                   ` (17 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

The reverse flag is used in parts of blame that are being moved to
libgit, and should be accessible via the scoreboard structure.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 0955fc1..161d15c 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -381,6 +381,9 @@ struct blame_scoreboard {
 
 	/* use this file's contents as the final image */
 	const char *contents_from;
+
+	/* flags */
+	int reverse;
 };
 
 static void sanity_check_refcnt(struct blame_scoreboard *);
@@ -1339,7 +1342,8 @@ static void pass_whole_blame(struct blame_scoreboard *sb,
  * "parent" (and "porigin"), but what we mean is to find scapegoat to
  * exonerate ourselves.
  */
-static struct commit_list *first_scapegoat(struct rev_info *revs, struct commit *commit)
+static struct commit_list *first_scapegoat(struct rev_info *revs, struct commit *commit,
+					   int reverse)
 {
 	if (!reverse) {
 		if (revs->first_parent_only &&
@@ -1353,9 +1357,9 @@ static struct commit_list *first_scapegoat(struct rev_info *revs, struct commit
 	return lookup_decoration(&revs->children, &commit->object);
 }
 
-static int num_scapegoats(struct rev_info *revs, struct commit *commit)
+static int num_scapegoats(struct rev_info *revs, struct commit *commit, int reverse)
 {
-	struct commit_list *l = first_scapegoat(revs, commit);
+	struct commit_list *l = first_scapegoat(revs, commit, reverse);
 	return commit_list_count(l);
 }
 
@@ -1393,7 +1397,7 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin,
 	struct blame_entry *toosmall = NULL;
 	struct blame_entry *blames, **blametail = &blames;
 
-	num_sg = num_scapegoats(revs, commit);
+	num_sg = num_scapegoats(revs, commit, sb->reverse);
 	if (!num_sg)
 		goto finish;
 	else if (num_sg < ARRAY_SIZE(sg_buf))
@@ -1409,7 +1413,7 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin,
 		struct blame_origin *(*find)(struct commit *, struct blame_origin *);
 		find = pass ? find_rename : find_origin;
 
-		for (i = 0, sg = first_scapegoat(revs, commit);
+		for (i = 0, sg = first_scapegoat(revs, commit, sb->reverse);
 		     i < num_sg && sg;
 		     sg = sg->next, i++) {
 			struct commit *p = sg->item;
@@ -1441,7 +1445,7 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin,
 	}
 
 	sb->num_commits++;
-	for (i = 0, sg = first_scapegoat(revs, commit);
+	for (i = 0, sg = first_scapegoat(revs, commit, sb->reverse);
 	     i < num_sg && sg;
 	     sg = sg->next, i++) {
 		struct blame_origin *porigin = sg_origin[i];
@@ -1462,7 +1466,7 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin,
 	if (opt & PICKAXE_BLAME_MOVE) {
 		filter_small(sb, &toosmall, &origin->suspects, sb->move_score);
 		if (origin->suspects) {
-			for (i = 0, sg = first_scapegoat(revs, commit);
+			for (i = 0, sg = first_scapegoat(revs, commit, sb->reverse);
 			     i < num_sg && sg;
 			     sg = sg->next, i++) {
 				struct blame_origin *porigin = sg_origin[i];
@@ -1489,7 +1493,7 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin,
 		if (!origin->suspects)
 			goto finish;
 
-		for (i = 0, sg = first_scapegoat(revs, commit);
+		for (i = 0, sg = first_scapegoat(revs, commit, sb->reverse);
 		     i < num_sg && sg;
 		     sg = sg->next, i++) {
 			struct blame_origin *porigin = sg_origin[i];
@@ -1770,7 +1774,7 @@ static void assign_blame(struct blame_scoreboard *sb, int opt)
 		 */
 		blame_origin_incref(suspect);
 		parse_commit(commit);
-		if (reverse ||
+		if (sb->reverse ||
 		    (!(commit->object.flags & UNINTERESTING) &&
 		     !(revs->max_age != -1 && commit->date < revs->max_age)))
 			pass_blame(sb, suspect, opt);
@@ -2739,6 +2743,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 
 	sb.revs = &revs;
 	sb.contents_from = contents_from;
+	sb.reverse = reverse;
 	if (!reverse) {
 		final_commit_name = prepare_final(&sb);
 		sb.commits.compare = compare_commits_by_commit_date;
-- 
2.9.3


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

* [PATCH 14/29] blame: move show_root flag to scoreboard
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (12 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 13/29] blame: move reverse flag " Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 15/29] blame: move xdl_opts flags " Jeff Smith
                   ` (16 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

The show_root flag is used in parts of blame that are being moved to
libgit, and should be accessible via the scoreboard structure.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 161d15c..fdd41b4 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -384,6 +384,7 @@ struct blame_scoreboard {
 
 	/* flags */
 	int reverse;
+	int show_root;
 };
 
 static void sanity_check_refcnt(struct blame_scoreboard *);
@@ -1784,7 +1785,7 @@ static void assign_blame(struct blame_scoreboard *sb, int opt)
 				mark_parents_uninteresting(commit);
 		}
 		/* treat root commit as boundary */
-		if (!commit->parents && !show_root)
+		if (!commit->parents && !sb->show_root)
 			commit->object.flags |= UNINTERESTING;
 
 		/* Take responsibility for the remaining entries */
@@ -2885,6 +2886,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	if (blame_copy_score)
 		sb.copy_score = blame_copy_score;
 
+	sb.show_root = show_root;
+
 	read_mailmap(&mailmap, NULL);
 
 	assign_blame(&sb, opt);
-- 
2.9.3


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

* [PATCH 15/29] blame: move xdl_opts flags to scoreboard
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (13 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 14/29] blame: move show_root " Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 16/29] blame: move no_whole_file_rename flag " Jeff Smith
                   ` (15 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

The xdl_opts flags are used in parts of blame that are being moved to
libgit, and should be accessible via the scoreboard structure.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index fdd41b4..8e676fb 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -125,7 +125,7 @@ struct progress_info {
 };
 
 static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b,
-		      xdl_emit_hunk_consume_func_t hunk_func, void *cb_data)
+		      xdl_emit_hunk_consume_func_t hunk_func, void *cb_data, int xdl_opts)
 {
 	xpparam_t xpp = {0};
 	xdemitconf_t xecfg = {0};
@@ -385,6 +385,7 @@ struct blame_scoreboard {
 	/* flags */
 	int reverse;
 	int show_root;
+	int xdl_opts;
 };
 
 static void sanity_check_refcnt(struct blame_scoreboard *);
@@ -948,7 +949,7 @@ static void pass_blame_to_parent(struct blame_scoreboard *sb,
 	fill_origin_blob(&sb->revs->diffopt, target, &file_o, &sb->num_read_blob);
 	sb->num_get_patch++;
 
-	if (diff_hunks(&file_p, &file_o, blame_chunk_cb, &d))
+	if (diff_hunks(&file_p, &file_o, blame_chunk_cb, &d, sb->xdl_opts))
 		die("unable to generate diff (%s -> %s)",
 		    oid_to_hex(&parent->commit->object.oid),
 		    oid_to_hex(&target->commit->object.oid));
@@ -1097,7 +1098,7 @@ static void find_copy_in_blob(struct blame_scoreboard *sb,
 	 * file_p partially may match that image.
 	 */
 	memset(split, 0, sizeof(struct blame_entry [3]));
-	if (diff_hunks(file_p, &file_o, handle_split_cb, &d))
+	if (diff_hunks(file_p, &file_o, handle_split_cb, &d, sb->xdl_opts))
 		die("unable to generate diff (%s)",
 		    oid_to_hex(&parent->commit->object.oid));
 	/* remainder, if any, all match the preimage */
@@ -2887,6 +2888,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 		sb.copy_score = blame_copy_score;
 
 	sb.show_root = show_root;
+	sb.xdl_opts = xdl_opts;
 
 	read_mailmap(&mailmap, NULL);
 
-- 
2.9.3


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

* [PATCH 16/29] blame: move no_whole_file_rename flag to scoreboard
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (14 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 15/29] blame: move xdl_opts flags " Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 17/29] blame: make sanity_check use a callback in scoreboard Jeff Smith
                   ` (14 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

The no_whole_file_rename flag is used in parts of blame that are being
moved to libgit, and should be accessible via the scoreboard structure.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 8e676fb..90c643c 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -386,6 +386,7 @@ struct blame_scoreboard {
 	int reverse;
 	int show_root;
 	int xdl_opts;
+	int no_whole_file_rename;
 };
 
 static void sanity_check_refcnt(struct blame_scoreboard *);
@@ -1411,7 +1412,7 @@ static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin,
 	 * The first pass looks for unrenamed path to optimize for
 	 * common cases, then we look for renames in the second pass.
 	 */
-	for (pass = 0; pass < 2 - no_whole_file_rename; pass++) {
+	for (pass = 0; pass < 2 - sb->no_whole_file_rename; pass++) {
 		struct blame_origin *(*find)(struct commit *, struct blame_origin *);
 		find = pass ? find_rename : find_origin;
 
@@ -2889,6 +2890,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 
 	sb.show_root = show_root;
 	sb.xdl_opts = xdl_opts;
+	sb.no_whole_file_rename = no_whole_file_rename;
 
 	read_mailmap(&mailmap, NULL);
 
-- 
2.9.3


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

* [PATCH 17/29] blame: make sanity_check use a callback in scoreboard
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (15 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 16/29] blame: move no_whole_file_rename flag " Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 18/29] blame: move progess updates to a scoreboard callback Jeff Smith
                   ` (13 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Allow the interface user to decide how to handle a failed sanity check,
whether that be to output with the current state or to do nothing.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 90c643c..1b53325 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -387,6 +387,10 @@ struct blame_scoreboard {
 	int show_root;
 	int xdl_opts;
 	int no_whole_file_rename;
+	int debug;
+
+	/* callbacks */
+	void(*on_sanity_fail)(struct blame_scoreboard *, int);
 };
 
 static void sanity_check_refcnt(struct blame_scoreboard *);
@@ -412,7 +416,7 @@ static void blame_coalesce(struct blame_scoreboard *sb)
 		}
 	}
 
-	if (DEBUG) /* sanity */
+	if (sb->debug) /* sanity */
 		sanity_check_refcnt(sb);
 }
 
@@ -1809,7 +1813,7 @@ static void assign_blame(struct blame_scoreboard *sb, int opt)
 		}
 		blame_origin_decref(suspect);
 
-		if (DEBUG) /* sanity */
+		if (sb->debug) /* sanity */
 			sanity_check_refcnt(sb);
 	}
 
@@ -2148,12 +2152,16 @@ static void sanity_check_refcnt(struct blame_scoreboard *sb)
 			baa = 1;
 		}
 	}
-	if (baa) {
-		int opt = 0160;
-		find_alignment(sb, &opt);
-		output(sb, opt);
-		die("Baa %d!", baa);
-	}
+	if (baa)
+		sb->on_sanity_fail(sb, baa);
+}
+
+static void sanity_check_on_fail(struct blame_scoreboard *sb, int baa)
+{
+	int opt = OUTPUT_SHOW_SCORE | OUTPUT_SHOW_NUMBER | OUTPUT_SHOW_NAME;
+	find_alignment(sb, &opt);
+	output(sb, opt);
+	die("Baa %d!", baa);
 }
 
 static unsigned parse_score(const char *arg)
@@ -2888,6 +2896,9 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	if (blame_copy_score)
 		sb.copy_score = blame_copy_score;
 
+	sb.debug = DEBUG;
+	sb.on_sanity_fail = &sanity_check_on_fail;
+
 	sb.show_root = show_root;
 	sb.xdl_opts = xdl_opts;
 	sb.no_whole_file_rename = no_whole_file_rename;
-- 
2.9.3


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

* [PATCH 18/29] blame: move progess updates to a scoreboard callback
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (16 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 17/29] blame: make sanity_check use a callback in scoreboard Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-25  4:16   ` Junio C Hamano
  2017-05-24  5:15 ` [PATCH 19/29] blame: wrap blame_sort and compare_blame_final Jeff Smith
                   ` (12 subsequent siblings)
  30 siblings, 1 reply; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Allow the interface user to decide how to handle a progress update.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 1b53325..d05907b 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -391,6 +391,9 @@ struct blame_scoreboard {
 
 	/* callbacks */
 	void(*on_sanity_fail)(struct blame_scoreboard *, int);
+	void(*found_guilty_entry)(struct blame_entry *, void *);
+
+	void *found_guilty_entry_data;
 };
 
 static void sanity_check_refcnt(struct blame_scoreboard *);
@@ -1729,9 +1732,10 @@ static int emit_one_suspect_detail(struct blame_origin *suspect, int repeat)
  * The blame_entry is found to be guilty for the range.
  * Show it in incremental output.
  */
-static void found_guilty_entry(struct blame_entry *ent,
-			   struct progress_info *pi)
+static void found_guilty_entry(struct blame_entry *ent, void *data)
 {
+	struct progress_info *pi = (struct progress_info *)data;
+
 	if (incremental) {
 		struct blame_origin *suspect = ent->suspect;
 
@@ -1754,11 +1758,6 @@ static void assign_blame(struct blame_scoreboard *sb, int opt)
 {
 	struct rev_info *revs = sb->revs;
 	struct commit *commit = prio_queue_get(&sb->commits);
-	struct progress_info pi = { NULL, 0 };
-
-	if (show_progress)
-		pi.progress = start_progress_delay(_("Blaming lines"),
-						   sb->num_lines, 50, 1);
 
 	while (commit) {
 		struct blame_entry *ent;
@@ -1800,7 +1799,8 @@ static void assign_blame(struct blame_scoreboard *sb, int opt)
 			suspect->guilty = 1;
 			for (;;) {
 				struct blame_entry *next = ent->next;
-				found_guilty_entry(ent, &pi);
+				if (sb->found_guilty_entry)
+					sb->found_guilty_entry(ent, sb->found_guilty_entry_data);
 				if (next) {
 					ent = next;
 					continue;
@@ -1816,8 +1816,6 @@ static void assign_blame(struct blame_scoreboard *sb, int opt)
 		if (sb->debug) /* sanity */
 			sanity_check_refcnt(sb);
 	}
-
-	stop_progress(&pi.progress);
 }
 
 static const char *format_time(timestamp_t time, const char *tz_str,
@@ -2550,6 +2548,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	char *final_commit_name = NULL;
 	enum object_type type;
 	struct commit *final_commit = NULL;
+	struct progress_info pi = { NULL, 0 };
 
 	struct string_list range_list = STRING_LIST_INIT_NODUP;
 	int output_option = 0, opt = 0;
@@ -2905,8 +2904,16 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 
 	read_mailmap(&mailmap, NULL);
 
+	sb.found_guilty_entry = &found_guilty_entry;
+	sb.found_guilty_entry_data = &pi;
+	if (show_progress)
+		pi.progress = start_progress_delay(_("Blaming lines"),
+						   sb.num_lines, 50, 1);
+
 	assign_blame(&sb, opt);
 
+	stop_progress(&pi.progress);
+
 	if (!incremental)
 		setup_pager();
 
-- 
2.9.3


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

* [PATCH 19/29] blame: wrap blame_sort and compare_blame_final
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (17 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 18/29] blame: move progess updates to a scoreboard callback Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 20/29] blame: rework methods that determine 'final' commit Jeff Smith
                   ` (11 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

The new method's interface is marginally cleaner than blame_sort, and
will avoid the need to expose the compare_blame_final method.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index d05907b..61fd5b4 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -328,12 +328,6 @@ static int compare_blame_suspect(const void *p1, const void *p2)
 	return s1->s_lno > s2->s_lno ? 1 : -1;
 }
 
-static struct blame_entry *blame_sort(struct blame_entry *head,
-				      int (*compare_fn)(const void *, const void *))
-{
-	return llist_mergesort (head, get_next_blame, set_next_blame, compare_fn);
-}
-
 static int compare_commits_by_reverse_commit_date(const void *a,
 						  const void *b,
 						  void *c)
@@ -396,6 +390,12 @@ struct blame_scoreboard {
 	void *found_guilty_entry_data;
 };
 
+static void blame_sort_final(struct blame_scoreboard *sb)
+{
+	sb->ent = llist_mergesort(sb->ent, get_next_blame, set_next_blame,
+				  compare_blame_final);
+}
+
 static void sanity_check_refcnt(struct blame_scoreboard *);
 
 /*
@@ -1378,7 +1378,8 @@ static int num_scapegoats(struct rev_info *revs, struct commit *commit, int reve
  */
 static void distribute_blame(struct blame_scoreboard *sb, struct blame_entry *blamed)
 {
-	blamed = blame_sort(blamed, compare_blame_suspect);
+	blamed = llist_mergesort(blamed, get_next_blame, set_next_blame,
+				 compare_blame_suspect);
 	while (blamed)
 	{
 		struct blame_origin *porigin = blamed->suspect;
@@ -2922,7 +2923,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	if (incremental)
 		return 0;
 
-	sb.ent = blame_sort(sb.ent, compare_blame_final);
+	blame_sort_final(&sb);
 
 	blame_coalesce(&sb);
 
-- 
2.9.3


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

* [PATCH 20/29] blame: rework methods that determine 'final' commit
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (18 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 19/29] blame: wrap blame_sort and compare_blame_final Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-25  4:59   ` Junio C Hamano
  2017-05-24  5:15 ` [PATCH 21/29] blame: create scoreboard init function Jeff Smith
                   ` (10 subsequent siblings)
  30 siblings, 1 reply; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Either prepare_initial or prepare_final is used to determine which
commit is marked as 'final'. Call the underlying methods directly to
make this more clear.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 49 +++++++++++++++++++++++--------------------------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 61fd5b4..e343520 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2431,14 +2431,8 @@ static struct commit *find_single_final(struct rev_info *revs,
 	return found;
 }
 
-static char *prepare_final(struct blame_scoreboard *sb)
-{
-	const char *name;
-	sb->final = find_single_final(sb->revs, &name);
-	return xstrdup_or_null(name);
-}
-
-static const char *dwim_reverse_initial(struct blame_scoreboard *sb)
+static struct commit *dwim_reverse_initial(struct rev_info *revs,
+					   const char **name_p)
 {
 	/*
 	 * DWIM "git blame --reverse ONE -- PATH" as
@@ -2449,11 +2443,11 @@ static const char *dwim_reverse_initial(struct blame_scoreboard *sb)
 	struct commit *head_commit;
 	unsigned char head_sha1[20];
 
-	if (sb->revs->pending.nr != 1)
+	if (revs->pending.nr != 1)
 		return NULL;
 
 	/* Is that sole rev a committish? */
-	obj = sb->revs->pending.objects[0].item;
+	obj = revs->pending.objects[0].item;
 	obj = deref_tag(obj, NULL, 0);
 	if (obj->type != OBJ_COMMIT)
 		return NULL;
@@ -2467,17 +2461,19 @@ static const char *dwim_reverse_initial(struct blame_scoreboard *sb)
 
 	/* Turn "ONE" into "ONE..HEAD" then */
 	obj->flags |= UNINTERESTING;
-	add_pending_object(sb->revs, &head_commit->object, "HEAD");
+	add_pending_object(revs, &head_commit->object, "HEAD");
 
-	sb->final = (struct commit *)obj;
-	return sb->revs->pending.objects[0].name;
+	if (name_p)
+		*name_p = revs->pending.objects[0].name;
+	return (struct commit *)obj;
 }
 
-static char *prepare_initial(struct blame_scoreboard *sb)
+static struct commit *find_single_initial(struct rev_info *revs,
+					  const char **name_p)
 {
 	int i;
 	const char *final_commit_name = NULL;
-	struct rev_info *revs = sb->revs;
+	struct commit *found = NULL;
 
 	/*
 	 * There must be one and only one negative commit, and it must be
@@ -2490,19 +2486,22 @@ static char *prepare_initial(struct blame_scoreboard *sb)
 		obj = deref_tag(obj, NULL, 0);
 		if (obj->type != OBJ_COMMIT)
 			die("Non commit %s?", revs->pending.objects[i].name);
-		if (sb->final)
+		if (found)
 			die("More than one commit to dig up from, %s and %s?",
 			    revs->pending.objects[i].name,
 			    final_commit_name);
-		sb->final = (struct commit *) obj;
+		found = (struct commit *) obj;
 		final_commit_name = revs->pending.objects[i].name;
 	}
 
 	if (!final_commit_name)
-		final_commit_name = dwim_reverse_initial(sb);
+		found = dwim_reverse_initial(revs, &final_commit_name);
 	if (!final_commit_name)
 		die("No commit to dig up from?");
-	return xstrdup(final_commit_name);
+
+	if (name_p)
+		*name_p = final_commit_name;
+	return found;
 }
 
 static int blame_copy_callback(const struct option *option, const char *arg, int unset)
@@ -2546,7 +2545,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	struct blame_origin *o;
 	struct blame_entry *ent = NULL;
 	long dashdash_pos, lno;
-	char *final_commit_name = NULL;
+	const char *final_commit_name = NULL;
 	enum object_type type;
 	struct commit *final_commit = NULL;
 	struct progress_info pi = { NULL, 0 };
@@ -2755,14 +2754,15 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	sb.revs = &revs;
 	sb.contents_from = contents_from;
 	sb.reverse = reverse;
+
 	if (!reverse) {
-		final_commit_name = prepare_final(&sb);
+		sb.final = find_single_final(&revs, &final_commit_name);
 		sb.commits.compare = compare_commits_by_commit_date;
 	}
 	else if (contents_from)
 		die(_("--contents and --reverse do not blend well."));
 	else {
-		final_commit_name = prepare_initial(&sb);
+		sb.final = find_single_initial(&revs, &final_commit_name);
 		sb.commits.compare = compare_commits_by_reverse_commit_date;
 		if (revs.first_parent_only)
 			revs.children.name = NULL;
@@ -2917,10 +2917,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 
 	if (!incremental)
 		setup_pager();
-
-	free(final_commit_name);
-
-	if (incremental)
+	else
 		return 0;
 
 	blame_sort_final(&sb);
-- 
2.9.3


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

* [PATCH 21/29] blame: create scoreboard init function
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (19 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 20/29] blame: rework methods that determine 'final' commit Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 22/29] blame: create scoreboard setup function Jeff Smith
                   ` (9 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Create function that initializes blame_scoreboard to default values.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index e343520..f839571 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2537,6 +2537,13 @@ static int blame_move_callback(const struct option *option, const char *arg, int
 	return 0;
 }
 
+void init_scoreboard(struct blame_scoreboard *sb)
+{
+	memset(sb, 0, sizeof(struct blame_scoreboard));
+	sb->move_score = BLAME_DEFAULT_MOVE_SCORE;
+	sb->copy_score = BLAME_DEFAULT_COPY_SCORE;
+}
+
 int cmd_blame(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
@@ -2747,10 +2754,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 
 	revs.disable_stdin = 1;
 	setup_revisions(argc, argv, &revs, NULL);
-	memset(&sb, 0, sizeof(sb));
-	sb.move_score = BLAME_DEFAULT_MOVE_SCORE;
-	sb.copy_score = BLAME_DEFAULT_COPY_SCORE;
 
+	init_scoreboard(&sb);
 	sb.revs = &revs;
 	sb.contents_from = contents_from;
 	sb.reverse = reverse;
-- 
2.9.3


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

* [PATCH 22/29] blame: create scoreboard setup function
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (20 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 21/29] blame: create scoreboard init function Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-25  5:15   ` Junio C Hamano
  2017-05-24  5:15 ` [PATCH 23/29] blame: create entry prepend function Jeff Smith
                   ` (8 subsequent siblings)
  30 siblings, 1 reply; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Create function that completes setting up blame_scoreboard structure.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 190 ++++++++++++++++++++++++++++++--------------------------
 1 file changed, 101 insertions(+), 89 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index f839571..fd41551 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2544,6 +2544,105 @@ void init_scoreboard(struct blame_scoreboard *sb)
 	sb->copy_score = BLAME_DEFAULT_COPY_SCORE;
 }
 
+void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blame_origin **orig)
+{
+	const char *final_commit_name = NULL;
+	struct blame_origin *o;
+	struct commit *final_commit = NULL;
+	enum object_type type;
+
+	if (sb->reverse && sb->contents_from)
+		die(_("--contents and --reverse do not blend well."));
+
+	if (!sb->reverse) {
+		sb->final = find_single_final(sb->revs, &final_commit_name);
+		sb->commits.compare = compare_commits_by_commit_date;
+	} else {
+		sb->final = find_single_initial(sb->revs, &final_commit_name);
+		sb->commits.compare = compare_commits_by_reverse_commit_date;
+	}
+
+	if (sb->final && sb->contents_from)
+		die(_("cannot use --contents with final commit object name"));
+
+	if (sb->reverse && sb->revs->first_parent_only)
+		sb->revs->children.name = NULL;
+
+	if (!sb->final) {
+		/*
+		 * "--not A B -- path" without anything positive;
+		 * do not default to HEAD, but use the working tree
+		 * or "--contents".
+		 */
+		setup_work_tree();
+		sb->final = fake_working_tree_commit(&sb->revs->diffopt,
+						     path, sb->contents_from);
+		add_pending_object(sb->revs, &(sb->final->object), ":");
+	}
+
+	if (sb->reverse && sb->revs->first_parent_only) {
+		final_commit = find_single_final(sb->revs, NULL);
+		if (!final_commit)
+			die(_("--reverse and --first-parent together require specified latest commit"));
+	}
+
+	/*
+	 * If we have bottom, this will mark the ancestors of the
+	 * bottom commits we would reach while traversing as
+	 * uninteresting.
+	 */
+	if (prepare_revision_walk(sb->revs))
+		die(_("revision walk setup failed"));
+
+	if (sb->reverse && sb->revs->first_parent_only) {
+		struct commit *c = final_commit;
+
+		sb->revs->children.name = "children";
+		while (c->parents &&
+		       oidcmp(&c->object.oid, &sb->final->object.oid)) {
+			struct commit_list *l = xcalloc(1, sizeof(*l));
+
+			l->item = c;
+			if (add_decoration(&sb->revs->children,
+					   &c->parents->item->object, l))
+				die("BUG: not unique item in first-parent chain");
+			c = c->parents->item;
+		}
+
+		if (oidcmp(&c->object.oid, &sb->final->object.oid))
+			die(_("--reverse --first-parent together require range along first-parent chain"));
+	}
+
+	if (is_null_oid(&sb->final->object.oid)) {
+		o = sb->final->util;
+		sb->final_buf = xmemdupz(o->file.ptr, o->file.size);
+		sb->final_buf_size = o->file.size;
+	}
+	else {
+		o = get_origin(sb->final, path);
+		if (fill_blob_sha1_and_mode(o))
+			die(_("no such path %s in %s"), path, final_commit_name);
+
+		if (DIFF_OPT_TST(&sb->revs->diffopt, ALLOW_TEXTCONV) &&
+		    textconv_object(path, o->mode, &o->blob_oid, 1, (char **) &sb->final_buf,
+				    &sb->final_buf_size))
+			;
+		else
+			sb->final_buf = read_sha1_file(o->blob_oid.hash, &type,
+						       &sb->final_buf_size);
+
+		if (!sb->final_buf)
+			die(_("cannot read blob %s for path %s"),
+			    oid_to_hex(&o->blob_oid),
+			    path);
+	}
+	sb->num_read_blob++;
+	prepare_lines(sb);
+
+	if (orig)
+		*orig = o;
+}
+
 int cmd_blame(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
@@ -2552,9 +2651,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	struct blame_origin *o;
 	struct blame_entry *ent = NULL;
 	long dashdash_pos, lno;
-	const char *final_commit_name = NULL;
-	enum object_type type;
-	struct commit *final_commit = NULL;
 	struct progress_info pi = { NULL, 0 };
 
 	struct string_list range_list = STRING_LIST_INIT_NODUP;
@@ -2759,92 +2855,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	sb.revs = &revs;
 	sb.contents_from = contents_from;
 	sb.reverse = reverse;
-
-	if (!reverse) {
-		sb.final = find_single_final(&revs, &final_commit_name);
-		sb.commits.compare = compare_commits_by_commit_date;
-	}
-	else if (contents_from)
-		die(_("--contents and --reverse do not blend well."));
-	else {
-		sb.final = find_single_initial(&revs, &final_commit_name);
-		sb.commits.compare = compare_commits_by_reverse_commit_date;
-		if (revs.first_parent_only)
-			revs.children.name = NULL;
-	}
-
-	if (!sb.final) {
-		/*
-		 * "--not A B -- path" without anything positive;
-		 * do not default to HEAD, but use the working tree
-		 * or "--contents".
-		 */
-		setup_work_tree();
-		sb.final = fake_working_tree_commit(&sb.revs->diffopt,
-						    path, contents_from);
-		add_pending_object(&revs, &(sb.final->object), ":");
-	}
-	else if (contents_from)
-		die(_("cannot use --contents with final commit object name"));
-
-	if (reverse && revs.first_parent_only) {
-		final_commit = find_single_final(sb.revs, NULL);
-		if (!final_commit)
-			die(_("--reverse and --first-parent together require specified latest commit"));
-	}
-
-	/*
-	 * If we have bottom, this will mark the ancestors of the
-	 * bottom commits we would reach while traversing as
-	 * uninteresting.
-	 */
-	if (prepare_revision_walk(&revs))
-		die(_("revision walk setup failed"));
-
-	if (reverse && revs.first_parent_only) {
-		struct commit *c = final_commit;
-
-		sb.revs->children.name = "children";
-		while (c->parents &&
-		       oidcmp(&c->object.oid, &sb.final->object.oid)) {
-			struct commit_list *l = xcalloc(1, sizeof(*l));
-
-			l->item = c;
-			if (add_decoration(&sb.revs->children,
-					   &c->parents->item->object, l))
-				die("BUG: not unique item in first-parent chain");
-			c = c->parents->item;
-		}
-
-		if (oidcmp(&c->object.oid, &sb.final->object.oid))
-			die(_("--reverse --first-parent together require range along first-parent chain"));
-	}
-
-	if (is_null_oid(&sb.final->object.oid)) {
-		o = sb.final->util;
-		sb.final_buf = xmemdupz(o->file.ptr, o->file.size);
-		sb.final_buf_size = o->file.size;
-	}
-	else {
-		o = get_origin(sb.final, path);
-		if (fill_blob_sha1_and_mode(o))
-			die(_("no such path %s in %s"), path, final_commit_name);
-
-		if (DIFF_OPT_TST(&sb.revs->diffopt, ALLOW_TEXTCONV) &&
-		    textconv_object(path, o->mode, &o->blob_oid, 1, (char **) &sb.final_buf,
-				    &sb.final_buf_size))
-			;
-		else
-			sb.final_buf = read_sha1_file(o->blob_oid.hash, &type,
-						      &sb.final_buf_size);
-
-		if (!sb.final_buf)
-			die(_("cannot read blob %s for path %s"),
-			    oid_to_hex(&o->blob_oid),
-			    path);
-	}
-	sb.num_read_blob++;
-	lno = prepare_lines(&sb);
+	setup_scoreboard(&sb, path, &o);
+	lno = sb.num_lines;
 
 	if (lno && !range_list.nr)
 		string_list_append(&range_list, "1");
-- 
2.9.3


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

* [PATCH 23/29] blame: create entry prepend function
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (21 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 22/29] blame: create scoreboard setup function Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-25  5:21   ` Junio C Hamano
  2017-05-24  5:15 ` [PATCH 24/29] blame: move core structures to header Jeff Smith
                   ` (7 subsequent siblings)
  30 siblings, 1 reply; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Create function that populates a blame_entry and prepends it to a list.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 builtin/blame.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index fd41551..29771b7 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2643,6 +2643,20 @@ void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blam
 		*orig = o;
 }
 
+struct blame_entry *blame_entry_prepend(struct blame_entry *head,
+					long start, long end,
+					struct blame_origin *o)
+{
+	struct blame_entry *new_head = xcalloc(1, sizeof(struct blame_entry));
+	new_head->lno = start;
+	new_head->num_lines = end - start;
+	new_head->suspect = o;
+	new_head->s_lno = start;
+	new_head->next = head;
+	blame_origin_incref(o);
+	return new_head;
+}
+
 int cmd_blame(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
@@ -2885,16 +2899,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 
 	for (range_i = ranges.nr; range_i > 0; --range_i) {
 		const struct range *r = &ranges.ranges[range_i - 1];
-		long bottom = r->start;
-		long top = r->end;
-		struct blame_entry *next = ent;
-		ent = xcalloc(1, sizeof(*ent));
-		ent->lno = bottom;
-		ent->num_lines = top - bottom;
-		ent->suspect = o;
-		ent->s_lno = bottom;
-		ent->next = next;
-		blame_origin_incref(o);
+		ent = blame_entry_prepend(ent, r->start, r->end, o);
 	}
 
 	o->suspects = ent;
-- 
2.9.3


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

* [PATCH 24/29] blame: move core structures to header
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (22 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 23/29] blame: create entry prepend function Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-25  5:25   ` Junio C Hamano
  2017-05-24  5:15 ` [PATCH 25/29] blame: move origin-related methods to libgit Jeff Smith
                   ` (6 subsequent siblings)
  30 siblings, 1 reply; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

The origin, entry, and scoreboard structures are core to the blame
interface and need to be exposed for blame functionality.

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 blame.h         | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 builtin/blame.c | 134 +---------------------------------------------------
 2 files changed, 144 insertions(+), 133 deletions(-)
 create mode 100644 blame.h

diff --git a/blame.h b/blame.h
new file mode 100644
index 0000000..c064d92
--- /dev/null
+++ b/blame.h
@@ -0,0 +1,143 @@
+#ifndef BLAME_H
+#define BLAME_H
+
+#include "cache.h"
+#include "commit.h"
+#include "xdiff-interface.h"
+#include "revision.h"
+#include "prio-queue.h"
+
+/*
+ * One blob in a commit that is being suspected
+ */
+struct blame_origin {
+	int refcnt;
+	/* Record preceding blame record for this blob */
+	struct blame_origin *previous;
+	/* origins are put in a list linked via `next' hanging off the
+	 * corresponding commit's util field in order to make finding
+	 * them fast.  The presence in this chain does not count
+	 * towards the origin's reference count.  It is tempting to
+	 * let it count as long as the commit is pending examination,
+	 * but even under circumstances where the commit will be
+	 * present multiple times in the priority queue of unexamined
+	 * commits, processing the first instance will not leave any
+	 * work requiring the origin data for the second instance.  An
+	 * interspersed commit changing that would have to be
+	 * preexisting with a different ancestry and with the same
+	 * commit date in order to wedge itself between two instances
+	 * of the same commit in the priority queue _and_ produce
+	 * blame entries relevant for it.  While we don't want to let
+	 * us get tripped up by this case, it certainly does not seem
+	 * worth optimizing for.
+	 */
+	struct blame_origin *next;
+	struct commit *commit;
+	/* `suspects' contains blame entries that may be attributed to
+	 * this origin's commit or to parent commits.  When a commit
+	 * is being processed, all suspects will be moved, either by
+	 * assigning them to an origin in a different commit, or by
+	 * shipping them to the scoreboard's ent list because they
+	 * cannot be attributed to a different commit.
+	 */
+	struct blame_entry *suspects;
+	mmfile_t file;
+	struct object_id blob_oid;
+	unsigned mode;
+	/* guilty gets set when shipping any suspects to the final
+	 * blame list instead of other commits
+	 */
+	char guilty;
+	char path[FLEX_ARRAY];
+};
+
+/*
+ * Each group of lines is described by a blame_entry; it can be split
+ * as we pass blame to the parents.  They are arranged in linked lists
+ * kept as `suspects' of some unprocessed origin, or entered (when the
+ * blame origin has been finalized) into the scoreboard structure.
+ * While the scoreboard structure is only sorted at the end of
+ * processing (according to final image line number), the lists
+ * attached to an origin are sorted by the target line number.
+ */
+struct blame_entry {
+	struct blame_entry *next;
+
+	/* the first line of this group in the final image;
+	 * internally all line numbers are 0 based.
+	 */
+	int lno;
+
+	/* how many lines this group has */
+	int num_lines;
+
+	/* the commit that introduced this group into the final image */
+	struct blame_origin *suspect;
+
+	/* the line number of the first line of this group in the
+	 * suspect's file; internally all line numbers are 0 based.
+	 */
+	int s_lno;
+
+	/* how significant this entry is -- cached to avoid
+	 * scanning the lines over and over.
+	 */
+	unsigned score;
+};
+
+/*
+ * The current state of the blame assignment.
+ */
+struct blame_scoreboard {
+	/* the final commit (i.e. where we started digging from) */
+	struct commit *final;
+	/* Priority queue for commits with unassigned blame records */
+	struct prio_queue commits;
+	struct rev_info *revs;
+	const char *path;
+
+	/*
+	 * The contents in the final image.
+	 * Used by many functions to obtain contents of the nth line,
+	 * indexed with scoreboard.lineno[blame_entry.lno].
+	 */
+	const char *final_buf;
+	unsigned long final_buf_size;
+
+	/* linked list of blames */
+	struct blame_entry *ent;
+
+	/* look-up a line in the final buffer */
+	int num_lines;
+	int *lineno;
+
+	/* stats */
+	int num_read_blob;
+	int num_get_patch;
+	int num_commits;
+
+	/*
+	 * blame for a blame_entry with score lower than these thresholds
+	 * is not passed to the parent using move/copy logic.
+	 */
+	unsigned move_score;
+	unsigned copy_score;
+
+	/* use this file's contents as the final image */
+	const char *contents_from;
+
+	/* flags */
+	int reverse;
+	int show_root;
+	int xdl_opts;
+	int no_whole_file_rename;
+	int debug;
+
+	/* callbacks */
+	void(*on_sanity_fail)(struct blame_scoreboard *, int);
+	void(*found_guilty_entry)(struct blame_entry *, void *);
+
+	void *found_guilty_entry_data;
+};
+
+#endif /* BLAME_H */
diff --git a/builtin/blame.c b/builtin/blame.c
index 29771b7..07b1a76 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -28,6 +28,7 @@
 #include "line-log.h"
 #include "dir.h"
 #include "progress.h"
+#include "blame.h"
 
 static char blame_usage[] = N_("git blame [<options>] [<rev-opts>] [<rev>] [--] <file>");
 
@@ -75,50 +76,6 @@ static unsigned blame_copy_score;
 #define METAINFO_SHOWN		(1u<<12)
 #define MORE_THAN_ONE_PATH	(1u<<13)
 
-/*
- * One blob in a commit that is being suspected
- */
-struct blame_origin {
-	int refcnt;
-	/* Record preceding blame record for this blob */
-	struct blame_origin *previous;
-	/* origins are put in a list linked via `next' hanging off the
-	 * corresponding commit's util field in order to make finding
-	 * them fast.  The presence in this chain does not count
-	 * towards the origin's reference count.  It is tempting to
-	 * let it count as long as the commit is pending examination,
-	 * but even under circumstances where the commit will be
-	 * present multiple times in the priority queue of unexamined
-	 * commits, processing the first instance will not leave any
-	 * work requiring the origin data for the second instance.  An
-	 * interspersed commit changing that would have to be
-	 * preexisting with a different ancestry and with the same
-	 * commit date in order to wedge itself between two instances
-	 * of the same commit in the priority queue _and_ produce
-	 * blame entries relevant for it.  While we don't want to let
-	 * us get tripped up by this case, it certainly does not seem
-	 * worth optimizing for.
-	 */
-	struct blame_origin *next;
-	struct commit *commit;
-	/* `suspects' contains blame entries that may be attributed to
-	 * this origin's commit or to parent commits.  When a commit
-	 * is being processed, all suspects will be moved, either by
-	 * assigning them to an origin in a different commit, or by
-	 * shipping them to the scoreboard's ent list because they
-	 * cannot be attributed to a different commit.
-	 */
-	struct blame_entry *suspects;
-	mmfile_t file;
-	struct object_id blob_oid;
-	unsigned mode;
-	/* guilty gets set when shipping any suspects to the final
-	 * blame list instead of other commits
-	 */
-	char guilty;
-	char path[FLEX_ARRAY];
-};
-
 struct progress_info {
 	struct progress *progress;
 	int blamed_lines;
@@ -209,40 +166,6 @@ static void drop_origin_blob(struct blame_origin *o)
 }
 
 /*
- * Each group of lines is described by a blame_entry; it can be split
- * as we pass blame to the parents.  They are arranged in linked lists
- * kept as `suspects' of some unprocessed origin, or entered (when the
- * blame origin has been finalized) into the scoreboard structure.
- * While the scoreboard structure is only sorted at the end of
- * processing (according to final image line number), the lists
- * attached to an origin are sorted by the target line number.
- */
-struct blame_entry {
-	struct blame_entry *next;
-
-	/* the first line of this group in the final image;
-	 * internally all line numbers are 0 based.
-	 */
-	int lno;
-
-	/* how many lines this group has */
-	int num_lines;
-
-	/* the commit that introduced this group into the final image */
-	struct blame_origin *suspect;
-
-	/* the line number of the first line of this group in the
-	 * suspect's file; internally all line numbers are 0 based.
-	 */
-	int s_lno;
-
-	/* how significant this entry is -- cached to avoid
-	 * scanning the lines over and over.
-	 */
-	unsigned score;
-};
-
-/*
  * Any merge of blames happens on lists of blames that arrived via
  * different parents in a single suspect.  In this case, we want to
  * sort according to the suspect line numbers as opposed to the final
@@ -335,61 +258,6 @@ static int compare_commits_by_reverse_commit_date(const void *a,
 	return -compare_commits_by_commit_date(a, b, c);
 }
 
-/*
- * The current state of the blame assignment.
- */
-struct blame_scoreboard {
-	/* the final commit (i.e. where we started digging from) */
-	struct commit *final;
-	/* Priority queue for commits with unassigned blame records */
-	struct prio_queue commits;
-	struct rev_info *revs;
-	const char *path;
-
-	/*
-	 * The contents in the final image.
-	 * Used by many functions to obtain contents of the nth line,
-	 * indexed with scoreboard.lineno[blame_entry.lno].
-	 */
-	const char *final_buf;
-	unsigned long final_buf_size;
-
-	/* linked list of blames */
-	struct blame_entry *ent;
-
-	/* look-up a line in the final buffer */
-	int num_lines;
-	int *lineno;
-
-	/* stats */
-	int num_read_blob;
-	int num_get_patch;
-	int num_commits;
-
-	/*
-	 * blame for a blame_entry with score lower than these thresholds
-	 * is not passed to the parent using move/copy logic.
-	 */
-	unsigned move_score;
-	unsigned copy_score;
-
-	/* use this file's contents as the final image */
-	const char *contents_from;
-
-	/* flags */
-	int reverse;
-	int show_root;
-	int xdl_opts;
-	int no_whole_file_rename;
-	int debug;
-
-	/* callbacks */
-	void(*on_sanity_fail)(struct blame_scoreboard *, int);
-	void(*found_guilty_entry)(struct blame_entry *, void *);
-
-	void *found_guilty_entry_data;
-};
-
 static void blame_sort_final(struct blame_scoreboard *sb)
 {
 	sb->ent = llist_mergesort(sb->ent, get_next_blame, set_next_blame,
-- 
2.9.3


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

* [PATCH 25/29] blame: move origin-related methods to libgit
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (23 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 24/29] blame: move core structures to header Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 26/29] blame: move fake-commit-related " Jeff Smith
                   ` (5 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 Makefile        |  1 +
 blame.c         | 62 +++++++++++++++++++++++++++++++++++++++++++++++++
 blame.h         | 15 ++++++++++++
 builtin/blame.c | 72 ---------------------------------------------------------
 4 files changed, 78 insertions(+), 72 deletions(-)
 create mode 100644 blame.c

diff --git a/Makefile b/Makefile
index e35542e..2d795ed 100644
--- a/Makefile
+++ b/Makefile
@@ -718,6 +718,7 @@ LIB_OBJS += argv-array.o
 LIB_OBJS += attr.o
 LIB_OBJS += base85.o
 LIB_OBJS += bisect.o
+LIB_OBJS += blame.o
 LIB_OBJS += blob.o
 LIB_OBJS += branch.o
 LIB_OBJS += bulk-checkin.o
diff --git a/blame.c b/blame.c
new file mode 100644
index 0000000..4855d6d
--- /dev/null
+++ b/blame.c
@@ -0,0 +1,62 @@
+#include "blame.h"
+
+void blame_origin_decref(struct blame_origin *o)
+{
+	if (o && --o->refcnt <= 0) {
+		struct blame_origin *p, *l = NULL;
+		if (o->previous)
+			blame_origin_decref(o->previous);
+		free(o->file.ptr);
+		/* Should be present exactly once in commit chain */
+		for (p = o->commit->util; p; l = p, p = p->next) {
+			if (p == o) {
+				if (l)
+					l->next = p->next;
+				else
+					o->commit->util = p->next;
+				free(o);
+				return;
+			}
+		}
+		die("internal error in blame_origin_decref");
+	}
+}
+
+/*
+ * Given a commit and a path in it, create a new origin structure.
+ * The callers that add blame to the scoreboard should use
+ * get_origin() to obtain shared, refcounted copy instead of calling
+ * this function directly.
+ */
+struct blame_origin *make_origin(struct commit *commit, const char *path)
+{
+	struct blame_origin *o;
+	FLEX_ALLOC_STR(o, path, path);
+	o->commit = commit;
+	o->refcnt = 1;
+	o->next = commit->util;
+	commit->util = o;
+	return o;
+}
+
+/*
+ * Locate an existing origin or create a new one.
+ * This moves the origin to front position in the commit util list.
+ */
+struct blame_origin *get_origin(struct commit *commit, const char *path)
+{
+	struct blame_origin *o, *l;
+
+	for (o = commit->util, l = NULL; o; l = o, o = o->next) {
+		if (!strcmp(o->path, path)) {
+			/* bump to front */
+			if (l) {
+				l->next = o->next;
+				o->next = commit->util;
+				commit->util = o;
+			}
+			return blame_origin_incref(o);
+		}
+	}
+	return make_origin(commit, path);
+}
diff --git a/blame.h b/blame.h
index c064d92..49b685e 100644
--- a/blame.h
+++ b/blame.h
@@ -140,4 +140,19 @@ struct blame_scoreboard {
 	void *found_guilty_entry_data;
 };
 
+/*
+ * Origin is refcounted and usually we keep the blob contents to be
+ * reused.
+ */
+static inline struct blame_origin *blame_origin_incref(struct blame_origin *o)
+{
+	if (o)
+		o->refcnt++;
+	return o;
+}
+extern void blame_origin_decref(struct blame_origin *o);
+
+extern struct blame_origin *make_origin(struct commit *commit, const char *path);
+extern struct blame_origin *get_origin(struct commit *commit, const char *path);
+
 #endif /* BLAME_H */
diff --git a/builtin/blame.c b/builtin/blame.c
index 07b1a76..2d6d834 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -124,39 +124,6 @@ static void fill_origin_blob(struct diff_options *opt,
 		*file = o->file;
 }
 
-/*
- * Origin is refcounted and usually we keep the blob contents to be
- * reused.
- */
-static inline struct blame_origin *blame_origin_incref(struct blame_origin *o)
-{
-	if (o)
-		o->refcnt++;
-	return o;
-}
-
-static void blame_origin_decref(struct blame_origin *o)
-{
-	if (o && --o->refcnt <= 0) {
-		struct blame_origin *p, *l = NULL;
-		if (o->previous)
-			blame_origin_decref(o->previous);
-		free(o->file.ptr);
-		/* Should be present exactly once in commit chain */
-		for (p = o->commit->util; p; l = p, p = p->next) {
-			if (p == o) {
-				if (l)
-					l->next = p->next;
-				else
-					o->commit->util = p->next;
-				free(o);
-				return;
-			}
-		}
-		die("internal error in blame_origin_decref");
-	}
-}
-
 static void drop_origin_blob(struct blame_origin *o)
 {
 	if (o->file.ptr) {
@@ -316,45 +283,6 @@ static void queue_blames(struct blame_scoreboard *sb, struct blame_origin *porig
 }
 
 /*
- * Given a commit and a path in it, create a new origin structure.
- * The callers that add blame to the scoreboard should use
- * get_origin() to obtain shared, refcounted copy instead of calling
- * this function directly.
- */
-static struct blame_origin *make_origin(struct commit *commit, const char *path)
-{
-	struct blame_origin *o;
-	FLEX_ALLOC_STR(o, path, path);
-	o->commit = commit;
-	o->refcnt = 1;
-	o->next = commit->util;
-	commit->util = o;
-	return o;
-}
-
-/*
- * Locate an existing origin or create a new one.
- * This moves the origin to front position in the commit util list.
- */
-static struct blame_origin *get_origin(struct commit *commit, const char *path)
-{
-	struct blame_origin *o, *l;
-
-	for (o = commit->util, l = NULL; o; l = o, o = o->next) {
-		if (!strcmp(o->path, path)) {
-			/* bump to front */
-			if (l) {
-				l->next = o->next;
-				o->next = commit->util;
-				commit->util = o;
-			}
-			return blame_origin_incref(o);
-		}
-	}
-	return make_origin(commit, path);
-}
-
-/*
  * Fill the blob_sha1 field of an origin if it hasn't, so that later
  * call to fill_origin_blob() can use it to locate the data.  blob_sha1
  * for an origin is also used to pass the blame for the entire file to
-- 
2.9.3


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

* [PATCH 26/29] blame: move fake-commit-related methods to libgit
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (24 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 25/29] blame: move origin-related methods to libgit Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 27/29] blame: move scoreboard-related " Jeff Smith
                   ` (4 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 blame.c         | 203 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 blame.h         |   4 +-
 builtin/blame.c | 197 ------------------------------------------------------
 3 files changed, 205 insertions(+), 199 deletions(-)

diff --git a/blame.c b/blame.c
index 4855d6d..8c025bc 100644
--- a/blame.c
+++ b/blame.c
@@ -1,3 +1,6 @@
+#include "cache.h"
+#include "refs.h"
+#include "cache-tree.h"
 #include "blame.h"
 
 void blame_origin_decref(struct blame_origin *o)
@@ -28,7 +31,7 @@ void blame_origin_decref(struct blame_origin *o)
  * get_origin() to obtain shared, refcounted copy instead of calling
  * this function directly.
  */
-struct blame_origin *make_origin(struct commit *commit, const char *path)
+static struct blame_origin *make_origin(struct commit *commit, const char *path)
 {
 	struct blame_origin *o;
 	FLEX_ALLOC_STR(o, path, path);
@@ -60,3 +63,201 @@ struct blame_origin *get_origin(struct commit *commit, const char *path)
 	}
 	return make_origin(commit, path);
 }
+
+
+
+static void verify_working_tree_path(struct commit *work_tree, const char *path)
+{
+	struct commit_list *parents;
+	int pos;
+
+	for (parents = work_tree->parents; parents; parents = parents->next) {
+		const struct object_id *commit_oid = &parents->item->object.oid;
+		struct object_id blob_oid;
+		unsigned mode;
+
+		if (!get_tree_entry(commit_oid->hash, path, blob_oid.hash, &mode) &&
+		    sha1_object_info(blob_oid.hash, NULL) == OBJ_BLOB)
+			return;
+	}
+
+	pos = cache_name_pos(path, strlen(path));
+	if (pos >= 0)
+		; /* path is in the index */
+	else if (-1 - pos < active_nr &&
+		 !strcmp(active_cache[-1 - pos]->name, path))
+		; /* path is in the index, unmerged */
+	else
+		die("no such path '%s' in HEAD", path);
+}
+
+static struct commit_list **append_parent(struct commit_list **tail, const struct object_id *oid)
+{
+	struct commit *parent;
+
+	parent = lookup_commit_reference(oid->hash);
+	if (!parent)
+		die("no such commit %s", oid_to_hex(oid));
+	return &commit_list_insert(parent, tail)->next;
+}
+
+static void append_merge_parents(struct commit_list **tail)
+{
+	int merge_head;
+	struct strbuf line = STRBUF_INIT;
+
+	merge_head = open(git_path_merge_head(), O_RDONLY);
+	if (merge_head < 0) {
+		if (errno == ENOENT)
+			return;
+		die("cannot open '%s' for reading", git_path_merge_head());
+	}
+
+	while (!strbuf_getwholeline_fd(&line, merge_head, '\n')) {
+		struct object_id oid;
+		if (line.len < GIT_SHA1_HEXSZ || get_oid_hex(line.buf, &oid))
+			die("unknown line in '%s': %s", git_path_merge_head(), line.buf);
+		tail = append_parent(tail, &oid);
+	}
+	close(merge_head);
+	strbuf_release(&line);
+}
+
+/*
+ * This isn't as simple as passing sb->buf and sb->len, because we
+ * want to transfer ownership of the buffer to the commit (so we
+ * must use detach).
+ */
+static void set_commit_buffer_from_strbuf(struct commit *c, struct strbuf *sb)
+{
+	size_t len;
+	void *buf = strbuf_detach(sb, &len);
+	set_commit_buffer(c, buf, len);
+}
+
+/*
+ * Prepare a dummy commit that represents the work tree (or staged) item.
+ * Note that annotating work tree item never works in the reverse.
+ */
+struct commit *fake_working_tree_commit(struct diff_options *opt,
+					const char *path,
+					const char *contents_from)
+{
+	struct commit *commit;
+	struct blame_origin *origin;
+	struct commit_list **parent_tail, *parent;
+	struct object_id head_oid;
+	struct strbuf buf = STRBUF_INIT;
+	const char *ident;
+	time_t now;
+	int size, len;
+	struct cache_entry *ce;
+	unsigned mode;
+	struct strbuf msg = STRBUF_INIT;
+
+	read_cache();
+	time(&now);
+	commit = alloc_commit_node();
+	commit->object.parsed = 1;
+	commit->date = now;
+	parent_tail = &commit->parents;
+
+	if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_oid.hash, NULL))
+		die("no such ref: HEAD");
+
+	parent_tail = append_parent(parent_tail, &head_oid);
+	append_merge_parents(parent_tail);
+	verify_working_tree_path(commit, path);
+
+	origin = make_origin(commit, path);
+
+	ident = fmt_ident("Not Committed Yet", "not.committed.yet", NULL, 0);
+	strbuf_addstr(&msg, "tree 0000000000000000000000000000000000000000\n");
+	for (parent = commit->parents; parent; parent = parent->next)
+		strbuf_addf(&msg, "parent %s\n",
+			    oid_to_hex(&parent->item->object.oid));
+	strbuf_addf(&msg,
+		    "author %s\n"
+		    "committer %s\n\n"
+		    "Version of %s from %s\n",
+		    ident, ident, path,
+		    (!contents_from ? path :
+		     (!strcmp(contents_from, "-") ? "standard input" : contents_from)));
+	set_commit_buffer_from_strbuf(commit, &msg);
+
+	if (!contents_from || strcmp("-", contents_from)) {
+		struct stat st;
+		const char *read_from;
+		char *buf_ptr;
+		unsigned long buf_len;
+
+		if (contents_from) {
+			if (stat(contents_from, &st) < 0)
+				die_errno("Cannot stat '%s'", contents_from);
+			read_from = contents_from;
+		}
+		else {
+			if (lstat(path, &st) < 0)
+				die_errno("Cannot lstat '%s'", path);
+			read_from = path;
+		}
+		mode = canon_mode(st.st_mode);
+
+		switch (st.st_mode & S_IFMT) {
+		case S_IFREG:
+			if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
+			    textconv_object(read_from, mode, &null_oid, 0, &buf_ptr, &buf_len))
+				strbuf_attach(&buf, buf_ptr, buf_len, buf_len + 1);
+			else if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)
+				die_errno("cannot open or read '%s'", read_from);
+			break;
+		case S_IFLNK:
+			if (strbuf_readlink(&buf, read_from, st.st_size) < 0)
+				die_errno("cannot readlink '%s'", read_from);
+			break;
+		default:
+			die("unsupported file type %s", read_from);
+		}
+	}
+	else {
+		/* Reading from stdin */
+		mode = 0;
+		if (strbuf_read(&buf, 0, 0) < 0)
+			die_errno("failed to read from stdin");
+	}
+	convert_to_git(path, buf.buf, buf.len, &buf, 0);
+	origin->file.ptr = buf.buf;
+	origin->file.size = buf.len;
+	pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_oid.hash);
+
+	/*
+	 * Read the current index, replace the path entry with
+	 * origin->blob_sha1 without mucking with its mode or type
+	 * bits; we are not going to write this index out -- we just
+	 * want to run "diff-index --cached".
+	 */
+	discard_cache();
+	read_cache();
+
+	len = strlen(path);
+	if (!mode) {
+		int pos = cache_name_pos(path, len);
+		if (0 <= pos)
+			mode = active_cache[pos]->ce_mode;
+		else
+			/* Let's not bother reading from HEAD tree */
+			mode = S_IFREG | 0644;
+	}
+	size = cache_entry_size(len);
+	ce = xcalloc(1, size);
+	oidcpy(&ce->oid, &origin->blob_oid);
+	memcpy(ce->name, path, len);
+	ce->ce_flags = create_ce_flags(0);
+	ce->ce_namelen = len;
+	ce->ce_mode = create_ce_mode(mode);
+	add_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
+
+	cache_tree_invalidate_path(&the_index, path);
+
+	return commit;
+}
diff --git a/blame.h b/blame.h
index 49b685e..0d10e17 100644
--- a/blame.h
+++ b/blame.h
@@ -6,6 +6,7 @@
 #include "xdiff-interface.h"
 #include "revision.h"
 #include "prio-queue.h"
+#include "diff.h"
 
 /*
  * One blob in a commit that is being suspected
@@ -152,7 +153,8 @@ static inline struct blame_origin *blame_origin_incref(struct blame_origin *o)
 }
 extern void blame_origin_decref(struct blame_origin *o);
 
-extern struct blame_origin *make_origin(struct commit *commit, const char *path);
 extern struct blame_origin *get_origin(struct commit *commit, const char *path);
 
+extern struct commit *fake_working_tree_commit(struct diff_options *opt, const char *path, const char *contents_from);
+
 #endif /* BLAME_H */
diff --git a/builtin/blame.c b/builtin/blame.c
index 2d6d834..3679214 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -16,7 +16,6 @@
 #include "revision.h"
 #include "quote.h"
 #include "xdiff-interface.h"
-#include "cache-tree.h"
 #include "string-list.h"
 #include "mailmap.h"
 #include "mergesort.h"
@@ -2006,202 +2005,6 @@ static int git_blame_config(const char *var, const char *value, void *cb)
 	return git_default_config(var, value, cb);
 }
 
-static void verify_working_tree_path(struct commit *work_tree, const char *path)
-{
-	struct commit_list *parents;
-	int pos;
-
-	for (parents = work_tree->parents; parents; parents = parents->next) {
-		const struct object_id *commit_oid = &parents->item->object.oid;
-		struct object_id blob_oid;
-		unsigned mode;
-
-		if (!get_tree_entry(commit_oid->hash, path, blob_oid.hash, &mode) &&
-		    sha1_object_info(blob_oid.hash, NULL) == OBJ_BLOB)
-			return;
-	}
-
-	pos = cache_name_pos(path, strlen(path));
-	if (pos >= 0)
-		; /* path is in the index */
-	else if (-1 - pos < active_nr &&
-		 !strcmp(active_cache[-1 - pos]->name, path))
-		; /* path is in the index, unmerged */
-	else
-		die("no such path '%s' in HEAD", path);
-}
-
-static struct commit_list **append_parent(struct commit_list **tail, const struct object_id *oid)
-{
-	struct commit *parent;
-
-	parent = lookup_commit_reference(oid->hash);
-	if (!parent)
-		die("no such commit %s", oid_to_hex(oid));
-	return &commit_list_insert(parent, tail)->next;
-}
-
-static void append_merge_parents(struct commit_list **tail)
-{
-	int merge_head;
-	struct strbuf line = STRBUF_INIT;
-
-	merge_head = open(git_path_merge_head(), O_RDONLY);
-	if (merge_head < 0) {
-		if (errno == ENOENT)
-			return;
-		die("cannot open '%s' for reading", git_path_merge_head());
-	}
-
-	while (!strbuf_getwholeline_fd(&line, merge_head, '\n')) {
-		struct object_id oid;
-		if (line.len < GIT_SHA1_HEXSZ || get_oid_hex(line.buf, &oid))
-			die("unknown line in '%s': %s", git_path_merge_head(), line.buf);
-		tail = append_parent(tail, &oid);
-	}
-	close(merge_head);
-	strbuf_release(&line);
-}
-
-/*
- * This isn't as simple as passing sb->buf and sb->len, because we
- * want to transfer ownership of the buffer to the commit (so we
- * must use detach).
- */
-static void set_commit_buffer_from_strbuf(struct commit *c, struct strbuf *sb)
-{
-	size_t len;
-	void *buf = strbuf_detach(sb, &len);
-	set_commit_buffer(c, buf, len);
-}
-
-/*
- * Prepare a dummy commit that represents the work tree (or staged) item.
- * Note that annotating work tree item never works in the reverse.
- */
-static struct commit *fake_working_tree_commit(struct diff_options *opt,
-					       const char *path,
-					       const char *contents_from)
-{
-	struct commit *commit;
-	struct blame_origin *origin;
-	struct commit_list **parent_tail, *parent;
-	struct object_id head_oid;
-	struct strbuf buf = STRBUF_INIT;
-	const char *ident;
-	time_t now;
-	int size, len;
-	struct cache_entry *ce;
-	unsigned mode;
-	struct strbuf msg = STRBUF_INIT;
-
-	read_cache();
-	time(&now);
-	commit = alloc_commit_node();
-	commit->object.parsed = 1;
-	commit->date = now;
-	parent_tail = &commit->parents;
-
-	if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_oid.hash, NULL))
-		die("no such ref: HEAD");
-
-	parent_tail = append_parent(parent_tail, &head_oid);
-	append_merge_parents(parent_tail);
-	verify_working_tree_path(commit, path);
-
-	origin = make_origin(commit, path);
-
-	ident = fmt_ident("Not Committed Yet", "not.committed.yet", NULL, 0);
-	strbuf_addstr(&msg, "tree 0000000000000000000000000000000000000000\n");
-	for (parent = commit->parents; parent; parent = parent->next)
-		strbuf_addf(&msg, "parent %s\n",
-			    oid_to_hex(&parent->item->object.oid));
-	strbuf_addf(&msg,
-		    "author %s\n"
-		    "committer %s\n\n"
-		    "Version of %s from %s\n",
-		    ident, ident, path,
-		    (!contents_from ? path :
-		     (!strcmp(contents_from, "-") ? "standard input" : contents_from)));
-	set_commit_buffer_from_strbuf(commit, &msg);
-
-	if (!contents_from || strcmp("-", contents_from)) {
-		struct stat st;
-		const char *read_from;
-		char *buf_ptr;
-		unsigned long buf_len;
-
-		if (contents_from) {
-			if (stat(contents_from, &st) < 0)
-				die_errno("Cannot stat '%s'", contents_from);
-			read_from = contents_from;
-		}
-		else {
-			if (lstat(path, &st) < 0)
-				die_errno("Cannot lstat '%s'", path);
-			read_from = path;
-		}
-		mode = canon_mode(st.st_mode);
-
-		switch (st.st_mode & S_IFMT) {
-		case S_IFREG:
-			if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
-			    textconv_object(read_from, mode, &null_oid, 0, &buf_ptr, &buf_len))
-				strbuf_attach(&buf, buf_ptr, buf_len, buf_len + 1);
-			else if (strbuf_read_file(&buf, read_from, st.st_size) != st.st_size)
-				die_errno("cannot open or read '%s'", read_from);
-			break;
-		case S_IFLNK:
-			if (strbuf_readlink(&buf, read_from, st.st_size) < 0)
-				die_errno("cannot readlink '%s'", read_from);
-			break;
-		default:
-			die("unsupported file type %s", read_from);
-		}
-	}
-	else {
-		/* Reading from stdin */
-		mode = 0;
-		if (strbuf_read(&buf, 0, 0) < 0)
-			die_errno("failed to read from stdin");
-	}
-	convert_to_git(path, buf.buf, buf.len, &buf, 0);
-	origin->file.ptr = buf.buf;
-	origin->file.size = buf.len;
-	pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_oid.hash);
-
-	/*
-	 * Read the current index, replace the path entry with
-	 * origin->blob_sha1 without mucking with its mode or type
-	 * bits; we are not going to write this index out -- we just
-	 * want to run "diff-index --cached".
-	 */
-	discard_cache();
-	read_cache();
-
-	len = strlen(path);
-	if (!mode) {
-		int pos = cache_name_pos(path, len);
-		if (0 <= pos)
-			mode = active_cache[pos]->ce_mode;
-		else
-			/* Let's not bother reading from HEAD tree */
-			mode = S_IFREG | 0644;
-	}
-	size = cache_entry_size(len);
-	ce = xcalloc(1, size);
-	oidcpy(&ce->oid, &origin->blob_oid);
-	memcpy(ce->name, path, len);
-	ce->ce_flags = create_ce_flags(0);
-	ce->ce_namelen = len;
-	ce->ce_mode = create_ce_mode(mode);
-	add_cache_entry(ce, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
-
-	cache_tree_invalidate_path(&the_index, path);
-
-	return commit;
-}
-
 static struct commit *find_single_final(struct rev_info *revs,
 					const char **name_p)
 {
-- 
2.9.3


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

* [PATCH 27/29] blame: move scoreboard-related methods to libgit
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (25 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 26/29] blame: move fake-commit-related " Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  5:15 ` [PATCH 28/29] blame: move scoreboard setup " Jeff Smith
                   ` (3 subsequent siblings)
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 blame.c         | 1313 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 blame.h         |   11 +
 builtin/blame.c | 1318 -------------------------------------------------------
 3 files changed, 1324 insertions(+), 1318 deletions(-)

diff --git a/blame.c b/blame.c
index 8c025bc..798e61b 100644
--- a/blame.c
+++ b/blame.c
@@ -1,6 +1,9 @@
 #include "cache.h"
 #include "refs.h"
 #include "cache-tree.h"
+#include "mergesort.h"
+#include "diff.h"
+#include "diffcore.h"
 #include "blame.h"
 
 void blame_origin_decref(struct blame_origin *o)
@@ -261,3 +264,1313 @@ struct commit *fake_working_tree_commit(struct diff_options *opt,
 
 	return commit;
 }
+
+
+
+static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b,
+		      xdl_emit_hunk_consume_func_t hunk_func, void *cb_data, int xdl_opts)
+{
+	xpparam_t xpp = {0};
+	xdemitconf_t xecfg = {0};
+	xdemitcb_t ecb = {NULL};
+
+	xpp.flags = xdl_opts;
+	xecfg.hunk_func = hunk_func;
+	ecb.priv = cb_data;
+	return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb);
+}
+
+/*
+ * Given an origin, prepare mmfile_t structure to be used by the
+ * diff machinery
+ */
+static void fill_origin_blob(struct diff_options *opt,
+			     struct blame_origin *o, mmfile_t *file, int *num_read_blob)
+{
+	if (!o->file.ptr) {
+		enum object_type type;
+		unsigned long file_size;
+
+		(*num_read_blob)++;
+		if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
+		    textconv_object(o->path, o->mode, &o->blob_oid, 1, &file->ptr, &file_size))
+			;
+		else
+			file->ptr = read_sha1_file(o->blob_oid.hash, &type,
+						   &file_size);
+		file->size = file_size;
+
+		if (!file->ptr)
+			die("Cannot read blob %s for path %s",
+			    oid_to_hex(&o->blob_oid),
+			    o->path);
+		o->file = *file;
+	}
+	else
+		*file = o->file;
+}
+
+static void drop_origin_blob(struct blame_origin *o)
+{
+	if (o->file.ptr) {
+		free(o->file.ptr);
+		o->file.ptr = NULL;
+	}
+}
+
+/*
+ * Any merge of blames happens on lists of blames that arrived via
+ * different parents in a single suspect.  In this case, we want to
+ * sort according to the suspect line numbers as opposed to the final
+ * image line numbers.  The function body is somewhat longish because
+ * it avoids unnecessary writes.
+ */
+
+static struct blame_entry *blame_merge(struct blame_entry *list1,
+				       struct blame_entry *list2)
+{
+	struct blame_entry *p1 = list1, *p2 = list2,
+		**tail = &list1;
+
+	if (!p1)
+		return p2;
+	if (!p2)
+		return p1;
+
+	if (p1->s_lno <= p2->s_lno) {
+		do {
+			tail = &p1->next;
+			if ((p1 = *tail) == NULL) {
+				*tail = p2;
+				return list1;
+			}
+		} while (p1->s_lno <= p2->s_lno);
+	}
+	for (;;) {
+		*tail = p2;
+		do {
+			tail = &p2->next;
+			if ((p2 = *tail) == NULL)  {
+				*tail = p1;
+				return list1;
+			}
+		} while (p1->s_lno > p2->s_lno);
+		*tail = p1;
+		do {
+			tail = &p1->next;
+			if ((p1 = *tail) == NULL) {
+				*tail = p2;
+				return list1;
+			}
+		} while (p1->s_lno <= p2->s_lno);
+	}
+}
+
+static void *get_next_blame(const void *p)
+{
+	return ((struct blame_entry *)p)->next;
+}
+
+static void set_next_blame(void *p1, void *p2)
+{
+	((struct blame_entry *)p1)->next = p2;
+}
+
+/*
+ * Final image line numbers are all different, so we don't need a
+ * three-way comparison here.
+ */
+
+static int compare_blame_final(const void *p1, const void *p2)
+{
+	return ((struct blame_entry *)p1)->lno > ((struct blame_entry *)p2)->lno
+		? 1 : -1;
+}
+
+static int compare_blame_suspect(const void *p1, const void *p2)
+{
+	const struct blame_entry *s1 = p1, *s2 = p2;
+	/*
+	 * to allow for collating suspects, we sort according to the
+	 * respective pointer value as the primary sorting criterion.
+	 * The actual relation is pretty unimportant as long as it
+	 * establishes a total order.  Comparing as integers gives us
+	 * that.
+	 */
+	if (s1->suspect != s2->suspect)
+		return (intptr_t)s1->suspect > (intptr_t)s2->suspect ? 1 : -1;
+	if (s1->s_lno == s2->s_lno)
+		return 0;
+	return s1->s_lno > s2->s_lno ? 1 : -1;
+}
+
+void blame_sort_final(struct blame_scoreboard *sb)
+{
+	sb->ent = llist_mergesort(sb->ent, get_next_blame, set_next_blame,
+				  compare_blame_final);
+}
+
+/*
+ * For debugging -- origin is refcounted, and this asserts that
+ * we do not underflow.
+ */
+static void sanity_check_refcnt(struct blame_scoreboard *sb)
+{
+	int baa = 0;
+	struct blame_entry *ent;
+
+	for (ent = sb->ent; ent; ent = ent->next) {
+		/* Nobody should have zero or negative refcnt */
+		if (ent->suspect->refcnt <= 0) {
+			fprintf(stderr, "%s in %s has negative refcnt %d\n",
+				ent->suspect->path,
+				oid_to_hex(&ent->suspect->commit->object.oid),
+				ent->suspect->refcnt);
+			baa = 1;
+		}
+	}
+	if (baa)
+		sb->on_sanity_fail(sb, baa);
+}
+
+/*
+ * If two blame entries that are next to each other came from
+ * contiguous lines in the same origin (i.e. <commit, path> pair),
+ * merge them together.
+ */
+void blame_coalesce(struct blame_scoreboard *sb)
+{
+	struct blame_entry *ent, *next;
+
+	for (ent = sb->ent; ent && (next = ent->next); ent = next) {
+		if (ent->suspect == next->suspect &&
+		    ent->s_lno + ent->num_lines == next->s_lno) {
+			ent->num_lines += next->num_lines;
+			ent->next = next->next;
+			blame_origin_decref(next->suspect);
+			free(next);
+			ent->score = 0;
+			next = ent; /* again */
+		}
+	}
+
+	if (sb->debug) /* sanity */
+		sanity_check_refcnt(sb);
+}
+
+/*
+ * Merge the given sorted list of blames into a preexisting origin.
+ * If there were no previous blames to that commit, it is entered into
+ * the commit priority queue of the score board.
+ */
+
+static void queue_blames(struct blame_scoreboard *sb, struct blame_origin *porigin,
+			 struct blame_entry *sorted)
+{
+	if (porigin->suspects)
+		porigin->suspects = blame_merge(porigin->suspects, sorted);
+	else {
+		struct blame_origin *o;
+		for (o = porigin->commit->util; o; o = o->next) {
+			if (o->suspects) {
+				porigin->suspects = sorted;
+				return;
+			}
+		}
+		porigin->suspects = sorted;
+		prio_queue_put(&sb->commits, porigin->commit);
+	}
+}
+
+/*
+ * We have an origin -- check if the same path exists in the
+ * parent and return an origin structure to represent it.
+ */
+static struct blame_origin *find_origin(struct commit *parent,
+				  struct blame_origin *origin)
+{
+	struct blame_origin *porigin;
+	struct diff_options diff_opts;
+	const char *paths[2];
+
+	/* First check any existing origins */
+	for (porigin = parent->util; porigin; porigin = porigin->next)
+		if (!strcmp(porigin->path, origin->path)) {
+			/*
+			 * The same path between origin and its parent
+			 * without renaming -- the most common case.
+			 */
+			return blame_origin_incref (porigin);
+		}
+
+	/* See if the origin->path is different between parent
+	 * and origin first.  Most of the time they are the
+	 * same and diff-tree is fairly efficient about this.
+	 */
+	diff_setup(&diff_opts);
+	DIFF_OPT_SET(&diff_opts, RECURSIVE);
+	diff_opts.detect_rename = 0;
+	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+	paths[0] = origin->path;
+	paths[1] = NULL;
+
+	parse_pathspec(&diff_opts.pathspec,
+		       PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL,
+		       PATHSPEC_LITERAL_PATH, "", paths);
+	diff_setup_done(&diff_opts);
+
+	if (is_null_oid(&origin->commit->object.oid))
+		do_diff_cache(parent->tree->object.oid.hash, &diff_opts);
+	else
+		diff_tree_sha1(parent->tree->object.oid.hash,
+			       origin->commit->tree->object.oid.hash,
+			       "", &diff_opts);
+	diffcore_std(&diff_opts);
+
+	if (!diff_queued_diff.nr) {
+		/* The path is the same as parent */
+		porigin = get_origin(parent, origin->path);
+		oidcpy(&porigin->blob_oid, &origin->blob_oid);
+		porigin->mode = origin->mode;
+	} else {
+		/*
+		 * Since origin->path is a pathspec, if the parent
+		 * commit had it as a directory, we will see a whole
+		 * bunch of deletion of files in the directory that we
+		 * do not care about.
+		 */
+		int i;
+		struct diff_filepair *p = NULL;
+		for (i = 0; i < diff_queued_diff.nr; i++) {
+			const char *name;
+			p = diff_queued_diff.queue[i];
+			name = p->one->path ? p->one->path : p->two->path;
+			if (!strcmp(name, origin->path))
+				break;
+		}
+		if (!p)
+			die("internal error in blame::find_origin");
+		switch (p->status) {
+		default:
+			die("internal error in blame::find_origin (%c)",
+			    p->status);
+		case 'M':
+			porigin = get_origin(parent, origin->path);
+			oidcpy(&porigin->blob_oid, &p->one->oid);
+			porigin->mode = p->one->mode;
+			break;
+		case 'A':
+		case 'T':
+			/* Did not exist in parent, or type changed */
+			break;
+		}
+	}
+	diff_flush(&diff_opts);
+	clear_pathspec(&diff_opts.pathspec);
+	return porigin;
+}
+
+/*
+ * We have an origin -- find the path that corresponds to it in its
+ * parent and return an origin structure to represent it.
+ */
+static struct blame_origin *find_rename(struct commit *parent,
+				  struct blame_origin *origin)
+{
+	struct blame_origin *porigin = NULL;
+	struct diff_options diff_opts;
+	int i;
+
+	diff_setup(&diff_opts);
+	DIFF_OPT_SET(&diff_opts, RECURSIVE);
+	diff_opts.detect_rename = DIFF_DETECT_RENAME;
+	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+	diff_opts.single_follow = origin->path;
+	diff_setup_done(&diff_opts);
+
+	if (is_null_oid(&origin->commit->object.oid))
+		do_diff_cache(parent->tree->object.oid.hash, &diff_opts);
+	else
+		diff_tree_sha1(parent->tree->object.oid.hash,
+			       origin->commit->tree->object.oid.hash,
+			       "", &diff_opts);
+	diffcore_std(&diff_opts);
+
+	for (i = 0; i < diff_queued_diff.nr; i++) {
+		struct diff_filepair *p = diff_queued_diff.queue[i];
+		if ((p->status == 'R' || p->status == 'C') &&
+		    !strcmp(p->two->path, origin->path)) {
+			porigin = get_origin(parent, p->one->path);
+			oidcpy(&porigin->blob_oid, &p->one->oid);
+			porigin->mode = p->one->mode;
+			break;
+		}
+	}
+	diff_flush(&diff_opts);
+	clear_pathspec(&diff_opts.pathspec);
+	return porigin;
+}
+
+/*
+ * Append a new blame entry to a given output queue.
+ */
+static void add_blame_entry(struct blame_entry ***queue,
+			    const struct blame_entry *src)
+{
+	struct blame_entry *e = xmalloc(sizeof(*e));
+	memcpy(e, src, sizeof(*e));
+	blame_origin_incref(e->suspect);
+
+	e->next = **queue;
+	**queue = e;
+	*queue = &e->next;
+}
+
+/*
+ * src typically is on-stack; we want to copy the information in it to
+ * a malloced blame_entry that gets added to the given queue.  The
+ * origin of dst loses a refcnt.
+ */
+static void dup_entry(struct blame_entry ***queue,
+		      struct blame_entry *dst, struct blame_entry *src)
+{
+	blame_origin_incref(src->suspect);
+	blame_origin_decref(dst->suspect);
+	memcpy(dst, src, sizeof(*src));
+	dst->next = **queue;
+	**queue = dst;
+	*queue = &dst->next;
+}
+
+const char *blame_nth_line(struct blame_scoreboard *sb, long lno)
+{
+	return sb->final_buf + sb->lineno[lno];
+}
+
+/*
+ * It is known that lines between tlno to same came from parent, and e
+ * has an overlap with that range.  it also is known that parent's
+ * line plno corresponds to e's line tlno.
+ *
+ *                <---- e ----->
+ *                   <------>
+ *                   <------------>
+ *             <------------>
+ *             <------------------>
+ *
+ * Split e into potentially three parts; before this chunk, the chunk
+ * to be blamed for the parent, and after that portion.
+ */
+static void split_overlap(struct blame_entry *split,
+			  struct blame_entry *e,
+			  int tlno, int plno, int same,
+			  struct blame_origin *parent)
+{
+	int chunk_end_lno;
+	memset(split, 0, sizeof(struct blame_entry [3]));
+
+	if (e->s_lno < tlno) {
+		/* there is a pre-chunk part not blamed on parent */
+		split[0].suspect = blame_origin_incref(e->suspect);
+		split[0].lno = e->lno;
+		split[0].s_lno = e->s_lno;
+		split[0].num_lines = tlno - e->s_lno;
+		split[1].lno = e->lno + tlno - e->s_lno;
+		split[1].s_lno = plno;
+	}
+	else {
+		split[1].lno = e->lno;
+		split[1].s_lno = plno + (e->s_lno - tlno);
+	}
+
+	if (same < e->s_lno + e->num_lines) {
+		/* there is a post-chunk part not blamed on parent */
+		split[2].suspect = blame_origin_incref(e->suspect);
+		split[2].lno = e->lno + (same - e->s_lno);
+		split[2].s_lno = e->s_lno + (same - e->s_lno);
+		split[2].num_lines = e->s_lno + e->num_lines - same;
+		chunk_end_lno = split[2].lno;
+	}
+	else
+		chunk_end_lno = e->lno + e->num_lines;
+	split[1].num_lines = chunk_end_lno - split[1].lno;
+
+	/*
+	 * if it turns out there is nothing to blame the parent for,
+	 * forget about the splitting.  !split[1].suspect signals this.
+	 */
+	if (split[1].num_lines < 1)
+		return;
+	split[1].suspect = blame_origin_incref(parent);
+}
+
+/*
+ * split_overlap() divided an existing blame e into up to three parts
+ * in split.  Any assigned blame is moved to queue to
+ * reflect the split.
+ */
+static void split_blame(struct blame_entry ***blamed,
+			struct blame_entry ***unblamed,
+			struct blame_entry *split,
+			struct blame_entry *e)
+{
+	if (split[0].suspect && split[2].suspect) {
+		/* The first part (reuse storage for the existing entry e) */
+		dup_entry(unblamed, e, &split[0]);
+
+		/* The last part -- me */
+		add_blame_entry(unblamed, &split[2]);
+
+		/* ... and the middle part -- parent */
+		add_blame_entry(blamed, &split[1]);
+	}
+	else if (!split[0].suspect && !split[2].suspect)
+		/*
+		 * The parent covers the entire area; reuse storage for
+		 * e and replace it with the parent.
+		 */
+		dup_entry(blamed, e, &split[1]);
+	else if (split[0].suspect) {
+		/* me and then parent */
+		dup_entry(unblamed, e, &split[0]);
+		add_blame_entry(blamed, &split[1]);
+	}
+	else {
+		/* parent and then me */
+		dup_entry(blamed, e, &split[1]);
+		add_blame_entry(unblamed, &split[2]);
+	}
+}
+
+/*
+ * After splitting the blame, the origins used by the
+ * on-stack blame_entry should lose one refcnt each.
+ */
+static void decref_split(struct blame_entry *split)
+{
+	int i;
+
+	for (i = 0; i < 3; i++)
+		blame_origin_decref(split[i].suspect);
+}
+
+/*
+ * reverse_blame reverses the list given in head, appending tail.
+ * That allows us to build lists in reverse order, then reverse them
+ * afterwards.  This can be faster than building the list in proper
+ * order right away.  The reason is that building in proper order
+ * requires writing a link in the _previous_ element, while building
+ * in reverse order just requires placing the list head into the
+ * _current_ element.
+ */
+
+static struct blame_entry *reverse_blame(struct blame_entry *head,
+					 struct blame_entry *tail)
+{
+	while (head) {
+		struct blame_entry *next = head->next;
+		head->next = tail;
+		tail = head;
+		head = next;
+	}
+	return tail;
+}
+
+/*
+ * Process one hunk from the patch between the current suspect for
+ * blame_entry e and its parent.  This first blames any unfinished
+ * entries before the chunk (which is where target and parent start
+ * differing) on the parent, and then splits blame entries at the
+ * start and at the end of the difference region.  Since use of -M and
+ * -C options may lead to overlapping/duplicate source line number
+ * ranges, all we can rely on from sorting/merging is the order of the
+ * first suspect line number.
+ */
+static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq,
+			int tlno, int offset, int same,
+			struct blame_origin *parent)
+{
+	struct blame_entry *e = **srcq;
+	struct blame_entry *samep = NULL, *diffp = NULL;
+
+	while (e && e->s_lno < tlno) {
+		struct blame_entry *next = e->next;
+		/*
+		 * current record starts before differing portion.  If
+		 * it reaches into it, we need to split it up and
+		 * examine the second part separately.
+		 */
+		if (e->s_lno + e->num_lines > tlno) {
+			/* Move second half to a new record */
+			int len = tlno - e->s_lno;
+			struct blame_entry *n = xcalloc(1, sizeof (struct blame_entry));
+			n->suspect = e->suspect;
+			n->lno = e->lno + len;
+			n->s_lno = e->s_lno + len;
+			n->num_lines = e->num_lines - len;
+			e->num_lines = len;
+			e->score = 0;
+			/* Push new record to diffp */
+			n->next = diffp;
+			diffp = n;
+		} else
+			blame_origin_decref(e->suspect);
+		/* Pass blame for everything before the differing
+		 * chunk to the parent */
+		e->suspect = blame_origin_incref(parent);
+		e->s_lno += offset;
+		e->next = samep;
+		samep = e;
+		e = next;
+	}
+	/*
+	 * As we don't know how much of a common stretch after this
+	 * diff will occur, the currently blamed parts are all that we
+	 * can assign to the parent for now.
+	 */
+
+	if (samep) {
+		**dstq = reverse_blame(samep, **dstq);
+		*dstq = &samep->next;
+	}
+	/*
+	 * Prepend the split off portions: everything after e starts
+	 * after the blameable portion.
+	 */
+	e = reverse_blame(diffp, e);
+
+	/*
+	 * Now retain records on the target while parts are different
+	 * from the parent.
+	 */
+	samep = NULL;
+	diffp = NULL;
+	while (e && e->s_lno < same) {
+		struct blame_entry *next = e->next;
+
+		/*
+		 * If current record extends into sameness, need to split.
+		 */
+		if (e->s_lno + e->num_lines > same) {
+			/*
+			 * Move second half to a new record to be
+			 * processed by later chunks
+			 */
+			int len = same - e->s_lno;
+			struct blame_entry *n = xcalloc(1, sizeof (struct blame_entry));
+			n->suspect = blame_origin_incref(e->suspect);
+			n->lno = e->lno + len;
+			n->s_lno = e->s_lno + len;
+			n->num_lines = e->num_lines - len;
+			e->num_lines = len;
+			e->score = 0;
+			/* Push new record to samep */
+			n->next = samep;
+			samep = n;
+		}
+		e->next = diffp;
+		diffp = e;
+		e = next;
+	}
+	**srcq = reverse_blame(diffp, reverse_blame(samep, e));
+	/* Move across elements that are in the unblamable portion */
+	if (diffp)
+		*srcq = &diffp->next;
+}
+
+struct blame_chunk_cb_data {
+	struct blame_origin *parent;
+	long offset;
+	struct blame_entry **dstq;
+	struct blame_entry **srcq;
+};
+
+/* diff chunks are from parent to target */
+static int blame_chunk_cb(long start_a, long count_a,
+			  long start_b, long count_b, void *data)
+{
+	struct blame_chunk_cb_data *d = data;
+	if (start_a - start_b != d->offset)
+		die("internal error in blame::blame_chunk_cb");
+	blame_chunk(&d->dstq, &d->srcq, start_b, start_a - start_b,
+		    start_b + count_b, d->parent);
+	d->offset = start_a + count_a - (start_b + count_b);
+	return 0;
+}
+
+/*
+ * We are looking at the origin 'target' and aiming to pass blame
+ * for the lines it is suspected to its parent.  Run diff to find
+ * which lines came from parent and pass blame for them.
+ */
+static void pass_blame_to_parent(struct blame_scoreboard *sb,
+				 struct blame_origin *target,
+				 struct blame_origin *parent)
+{
+	mmfile_t file_p, file_o;
+	struct blame_chunk_cb_data d;
+	struct blame_entry *newdest = NULL;
+
+	if (!target->suspects)
+		return; /* nothing remains for this target */
+
+	d.parent = parent;
+	d.offset = 0;
+	d.dstq = &newdest; d.srcq = &target->suspects;
+
+	fill_origin_blob(&sb->revs->diffopt, parent, &file_p, &sb->num_read_blob);
+	fill_origin_blob(&sb->revs->diffopt, target, &file_o, &sb->num_read_blob);
+	sb->num_get_patch++;
+
+	if (diff_hunks(&file_p, &file_o, blame_chunk_cb, &d, sb->xdl_opts))
+		die("unable to generate diff (%s -> %s)",
+		    oid_to_hex(&parent->commit->object.oid),
+		    oid_to_hex(&target->commit->object.oid));
+	/* The rest are the same as the parent */
+	blame_chunk(&d.dstq, &d.srcq, INT_MAX, d.offset, INT_MAX, parent);
+	*d.dstq = NULL;
+	queue_blames(sb, parent, newdest);
+
+	return;
+}
+
+/*
+ * The lines in blame_entry after splitting blames many times can become
+ * very small and trivial, and at some point it becomes pointless to
+ * blame the parents.  E.g. "\t\t}\n\t}\n\n" appears everywhere in any
+ * ordinary C program, and it is not worth to say it was copied from
+ * totally unrelated file in the parent.
+ *
+ * Compute how trivial the lines in the blame_entry are.
+ */
+unsigned blame_entry_score(struct blame_scoreboard *sb, struct blame_entry *e)
+{
+	unsigned score;
+	const char *cp, *ep;
+
+	if (e->score)
+		return e->score;
+
+	score = 1;
+	cp = blame_nth_line(sb, e->lno);
+	ep = blame_nth_line(sb, e->lno + e->num_lines);
+	while (cp < ep) {
+		unsigned ch = *((unsigned char *)cp);
+		if (isalnum(ch))
+			score++;
+		cp++;
+	}
+	e->score = score;
+	return score;
+}
+
+/*
+ * best_so_far[] and this[] are both a split of an existing blame_entry
+ * that passes blame to the parent.  Maintain best_so_far the best split
+ * so far, by comparing this and best_so_far and copying this into
+ * bst_so_far as needed.
+ */
+static void copy_split_if_better(struct blame_scoreboard *sb,
+				 struct blame_entry *best_so_far,
+				 struct blame_entry *this)
+{
+	int i;
+
+	if (!this[1].suspect)
+		return;
+	if (best_so_far[1].suspect) {
+		if (blame_entry_score(sb, &this[1]) < blame_entry_score(sb, &best_so_far[1]))
+			return;
+	}
+
+	for (i = 0; i < 3; i++)
+		blame_origin_incref(this[i].suspect);
+	decref_split(best_so_far);
+	memcpy(best_so_far, this, sizeof(struct blame_entry [3]));
+}
+
+/*
+ * We are looking at a part of the final image represented by
+ * ent (tlno and same are offset by ent->s_lno).
+ * tlno is where we are looking at in the final image.
+ * up to (but not including) same match preimage.
+ * plno is where we are looking at in the preimage.
+ *
+ * <-------------- final image ---------------------->
+ *       <------ent------>
+ *         ^tlno ^same
+ *    <---------preimage----->
+ *         ^plno
+ *
+ * All line numbers are 0-based.
+ */
+static void handle_split(struct blame_scoreboard *sb,
+			 struct blame_entry *ent,
+			 int tlno, int plno, int same,
+			 struct blame_origin *parent,
+			 struct blame_entry *split)
+{
+	if (ent->num_lines <= tlno)
+		return;
+	if (tlno < same) {
+		struct blame_entry this[3];
+		tlno += ent->s_lno;
+		same += ent->s_lno;
+		split_overlap(this, ent, tlno, plno, same, parent);
+		copy_split_if_better(sb, split, this);
+		decref_split(this);
+	}
+}
+
+struct handle_split_cb_data {
+	struct blame_scoreboard *sb;
+	struct blame_entry *ent;
+	struct blame_origin *parent;
+	struct blame_entry *split;
+	long plno;
+	long tlno;
+};
+
+static int handle_split_cb(long start_a, long count_a,
+			   long start_b, long count_b, void *data)
+{
+	struct handle_split_cb_data *d = data;
+	handle_split(d->sb, d->ent, d->tlno, d->plno, start_b, d->parent,
+		     d->split);
+	d->plno = start_a + count_a;
+	d->tlno = start_b + count_b;
+	return 0;
+}
+
+/*
+ * Find the lines from parent that are the same as ent so that
+ * we can pass blames to it.  file_p has the blob contents for
+ * the parent.
+ */
+static void find_copy_in_blob(struct blame_scoreboard *sb,
+			      struct blame_entry *ent,
+			      struct blame_origin *parent,
+			      struct blame_entry *split,
+			      mmfile_t *file_p)
+{
+	const char *cp;
+	mmfile_t file_o;
+	struct handle_split_cb_data d;
+
+	memset(&d, 0, sizeof(d));
+	d.sb = sb; d.ent = ent; d.parent = parent; d.split = split;
+	/*
+	 * Prepare mmfile that contains only the lines in ent.
+	 */
+	cp = blame_nth_line(sb, ent->lno);
+	file_o.ptr = (char *) cp;
+	file_o.size = blame_nth_line(sb, ent->lno + ent->num_lines) - cp;
+
+	/*
+	 * file_o is a part of final image we are annotating.
+	 * file_p partially may match that image.
+	 */
+	memset(split, 0, sizeof(struct blame_entry [3]));
+	if (diff_hunks(file_p, &file_o, handle_split_cb, &d, sb->xdl_opts))
+		die("unable to generate diff (%s)",
+		    oid_to_hex(&parent->commit->object.oid));
+	/* remainder, if any, all match the preimage */
+	handle_split(sb, ent, d.tlno, d.plno, ent->num_lines, parent, split);
+}
+
+/* Move all blame entries from list *source that have a score smaller
+ * than score_min to the front of list *small.
+ * Returns a pointer to the link pointing to the old head of the small list.
+ */
+
+static struct blame_entry **filter_small(struct blame_scoreboard *sb,
+					 struct blame_entry **small,
+					 struct blame_entry **source,
+					 unsigned score_min)
+{
+	struct blame_entry *p = *source;
+	struct blame_entry *oldsmall = *small;
+	while (p) {
+		if (blame_entry_score(sb, p) <= score_min) {
+			*small = p;
+			small = &p->next;
+			p = *small;
+		} else {
+			*source = p;
+			source = &p->next;
+			p = *source;
+		}
+	}
+	*small = oldsmall;
+	*source = NULL;
+	return small;
+}
+
+/*
+ * See if lines currently target is suspected for can be attributed to
+ * parent.
+ */
+static void find_move_in_parent(struct blame_scoreboard *sb,
+				struct blame_entry ***blamed,
+				struct blame_entry **toosmall,
+				struct blame_origin *target,
+				struct blame_origin *parent)
+{
+	struct blame_entry *e, split[3];
+	struct blame_entry *unblamed = target->suspects;
+	struct blame_entry *leftover = NULL;
+	mmfile_t file_p;
+
+	if (!unblamed)
+		return; /* nothing remains for this target */
+
+	fill_origin_blob(&sb->revs->diffopt, parent, &file_p, &sb->num_read_blob);
+	if (!file_p.ptr)
+		return;
+
+	/* At each iteration, unblamed has a NULL-terminated list of
+	 * entries that have not yet been tested for blame.  leftover
+	 * contains the reversed list of entries that have been tested
+	 * without being assignable to the parent.
+	 */
+	do {
+		struct blame_entry **unblamedtail = &unblamed;
+		struct blame_entry *next;
+		for (e = unblamed; e; e = next) {
+			next = e->next;
+			find_copy_in_blob(sb, e, parent, split, &file_p);
+			if (split[1].suspect &&
+			    sb->move_score < blame_entry_score(sb, &split[1])) {
+				split_blame(blamed, &unblamedtail, split, e);
+			} else {
+				e->next = leftover;
+				leftover = e;
+			}
+			decref_split(split);
+		}
+		*unblamedtail = NULL;
+		toosmall = filter_small(sb, toosmall, &unblamed, sb->move_score);
+	} while (unblamed);
+	target->suspects = reverse_blame(leftover, NULL);
+}
+
+struct blame_list {
+	struct blame_entry *ent;
+	struct blame_entry split[3];
+};
+
+/*
+ * Count the number of entries the target is suspected for,
+ * and prepare a list of entry and the best split.
+ */
+static struct blame_list *setup_blame_list(struct blame_entry *unblamed,
+					   int *num_ents_p)
+{
+	struct blame_entry *e;
+	int num_ents, i;
+	struct blame_list *blame_list = NULL;
+
+	for (e = unblamed, num_ents = 0; e; e = e->next)
+		num_ents++;
+	if (num_ents) {
+		blame_list = xcalloc(num_ents, sizeof(struct blame_list));
+		for (e = unblamed, i = 0; e; e = e->next)
+			blame_list[i++].ent = e;
+	}
+	*num_ents_p = num_ents;
+	return blame_list;
+}
+
+/*
+ * For lines target is suspected for, see if we can find code movement
+ * across file boundary from the parent commit.  porigin is the path
+ * in the parent we already tried.
+ */
+static void find_copy_in_parent(struct blame_scoreboard *sb,
+				struct blame_entry ***blamed,
+				struct blame_entry **toosmall,
+				struct blame_origin *target,
+				struct commit *parent,
+				struct blame_origin *porigin,
+				int opt)
+{
+	struct diff_options diff_opts;
+	int i, j;
+	struct blame_list *blame_list;
+	int num_ents;
+	struct blame_entry *unblamed = target->suspects;
+	struct blame_entry *leftover = NULL;
+
+	if (!unblamed)
+		return; /* nothing remains for this target */
+
+	diff_setup(&diff_opts);
+	DIFF_OPT_SET(&diff_opts, RECURSIVE);
+	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
+
+	diff_setup_done(&diff_opts);
+
+	/* Try "find copies harder" on new path if requested;
+	 * we do not want to use diffcore_rename() actually to
+	 * match things up; find_copies_harder is set only to
+	 * force diff_tree_sha1() to feed all filepairs to diff_queue,
+	 * and this code needs to be after diff_setup_done(), which
+	 * usually makes find-copies-harder imply copy detection.
+	 */
+	if ((opt & PICKAXE_BLAME_COPY_HARDEST)
+	    || ((opt & PICKAXE_BLAME_COPY_HARDER)
+		&& (!porigin || strcmp(target->path, porigin->path))))
+		DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
+
+	if (is_null_oid(&target->commit->object.oid))
+		do_diff_cache(parent->tree->object.oid.hash, &diff_opts);
+	else
+		diff_tree_sha1(parent->tree->object.oid.hash,
+			       target->commit->tree->object.oid.hash,
+			       "", &diff_opts);
+
+	if (!DIFF_OPT_TST(&diff_opts, FIND_COPIES_HARDER))
+		diffcore_std(&diff_opts);
+
+	do {
+		struct blame_entry **unblamedtail = &unblamed;
+		blame_list = setup_blame_list(unblamed, &num_ents);
+
+		for (i = 0; i < diff_queued_diff.nr; i++) {
+			struct diff_filepair *p = diff_queued_diff.queue[i];
+			struct blame_origin *norigin;
+			mmfile_t file_p;
+			struct blame_entry this[3];
+
+			if (!DIFF_FILE_VALID(p->one))
+				continue; /* does not exist in parent */
+			if (S_ISGITLINK(p->one->mode))
+				continue; /* ignore git links */
+			if (porigin && !strcmp(p->one->path, porigin->path))
+				/* find_move already dealt with this path */
+				continue;
+
+			norigin = get_origin(parent, p->one->path);
+			oidcpy(&norigin->blob_oid, &p->one->oid);
+			norigin->mode = p->one->mode;
+			fill_origin_blob(&sb->revs->diffopt, norigin, &file_p, &sb->num_read_blob);
+			if (!file_p.ptr)
+				continue;
+
+			for (j = 0; j < num_ents; j++) {
+				find_copy_in_blob(sb, blame_list[j].ent,
+						  norigin, this, &file_p);
+				copy_split_if_better(sb, blame_list[j].split,
+						     this);
+				decref_split(this);
+			}
+			blame_origin_decref(norigin);
+		}
+
+		for (j = 0; j < num_ents; j++) {
+			struct blame_entry *split = blame_list[j].split;
+			if (split[1].suspect &&
+			    sb->copy_score < blame_entry_score(sb, &split[1])) {
+				split_blame(blamed, &unblamedtail, split,
+					    blame_list[j].ent);
+			} else {
+				blame_list[j].ent->next = leftover;
+				leftover = blame_list[j].ent;
+			}
+			decref_split(split);
+		}
+		free(blame_list);
+		*unblamedtail = NULL;
+		toosmall = filter_small(sb, toosmall, &unblamed, sb->copy_score);
+	} while (unblamed);
+	target->suspects = reverse_blame(leftover, NULL);
+	diff_flush(&diff_opts);
+	clear_pathspec(&diff_opts.pathspec);
+}
+
+/*
+ * The blobs of origin and porigin exactly match, so everything
+ * origin is suspected for can be blamed on the parent.
+ */
+static void pass_whole_blame(struct blame_scoreboard *sb,
+			     struct blame_origin *origin, struct blame_origin *porigin)
+{
+	struct blame_entry *e, *suspects;
+
+	if (!porigin->file.ptr && origin->file.ptr) {
+		/* Steal its file */
+		porigin->file = origin->file;
+		origin->file.ptr = NULL;
+	}
+	suspects = origin->suspects;
+	origin->suspects = NULL;
+	for (e = suspects; e; e = e->next) {
+		blame_origin_incref(porigin);
+		blame_origin_decref(e->suspect);
+		e->suspect = porigin;
+	}
+	queue_blames(sb, porigin, suspects);
+}
+
+/*
+ * We pass blame from the current commit to its parents.  We keep saying
+ * "parent" (and "porigin"), but what we mean is to find scapegoat to
+ * exonerate ourselves.
+ */
+static struct commit_list *first_scapegoat(struct rev_info *revs, struct commit *commit,
+					int reverse)
+{
+	if (!reverse) {
+		if (revs->first_parent_only &&
+		    commit->parents &&
+		    commit->parents->next) {
+			free_commit_list(commit->parents->next);
+			commit->parents->next = NULL;
+		}
+		return commit->parents;
+	}
+	return lookup_decoration(&revs->children, &commit->object);
+}
+
+static int num_scapegoats(struct rev_info *revs, struct commit *commit, int reverse)
+{
+	struct commit_list *l = first_scapegoat(revs, commit, reverse);
+	return commit_list_count(l);
+}
+
+/* Distribute collected unsorted blames to the respected sorted lists
+ * in the various origins.
+ */
+static void distribute_blame(struct blame_scoreboard *sb, struct blame_entry *blamed)
+{
+	blamed = llist_mergesort(blamed, get_next_blame, set_next_blame,
+				 compare_blame_suspect);
+	while (blamed)
+	{
+		struct blame_origin *porigin = blamed->suspect;
+		struct blame_entry *suspects = NULL;
+		do {
+			struct blame_entry *next = blamed->next;
+			blamed->next = suspects;
+			suspects = blamed;
+			blamed = next;
+		} while (blamed && blamed->suspect == porigin);
+		suspects = reverse_blame(suspects, NULL);
+		queue_blames(sb, porigin, suspects);
+	}
+}
+
+#define MAXSG 16
+
+static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin, int opt)
+{
+	struct rev_info *revs = sb->revs;
+	int i, pass, num_sg;
+	struct commit *commit = origin->commit;
+	struct commit_list *sg;
+	struct blame_origin *sg_buf[MAXSG];
+	struct blame_origin *porigin, **sg_origin = sg_buf;
+	struct blame_entry *toosmall = NULL;
+	struct blame_entry *blames, **blametail = &blames;
+
+	num_sg = num_scapegoats(revs, commit, sb->reverse);
+	if (!num_sg)
+		goto finish;
+	else if (num_sg < ARRAY_SIZE(sg_buf))
+		memset(sg_buf, 0, sizeof(sg_buf));
+	else
+		sg_origin = xcalloc(num_sg, sizeof(*sg_origin));
+
+	/*
+	 * The first pass looks for unrenamed path to optimize for
+	 * common cases, then we look for renames in the second pass.
+	 */
+	for (pass = 0; pass < 2 - sb->no_whole_file_rename; pass++) {
+		struct blame_origin *(*find)(struct commit *, struct blame_origin *);
+		find = pass ? find_rename : find_origin;
+
+		for (i = 0, sg = first_scapegoat(revs, commit, sb->reverse);
+		     i < num_sg && sg;
+		     sg = sg->next, i++) {
+			struct commit *p = sg->item;
+			int j, same;
+
+			if (sg_origin[i])
+				continue;
+			if (parse_commit(p))
+				continue;
+			porigin = find(p, origin);
+			if (!porigin)
+				continue;
+			if (!oidcmp(&porigin->blob_oid, &origin->blob_oid)) {
+				pass_whole_blame(sb, origin, porigin);
+				blame_origin_decref(porigin);
+				goto finish;
+			}
+			for (j = same = 0; j < i; j++)
+				if (sg_origin[j] &&
+				    !oidcmp(&sg_origin[j]->blob_oid, &porigin->blob_oid)) {
+					same = 1;
+					break;
+				}
+			if (!same)
+				sg_origin[i] = porigin;
+			else
+				blame_origin_decref(porigin);
+		}
+	}
+
+	sb->num_commits++;
+	for (i = 0, sg = first_scapegoat(revs, commit, sb->reverse);
+	     i < num_sg && sg;
+	     sg = sg->next, i++) {
+		struct blame_origin *porigin = sg_origin[i];
+		if (!porigin)
+			continue;
+		if (!origin->previous) {
+			blame_origin_incref(porigin);
+			origin->previous = porigin;
+		}
+		pass_blame_to_parent(sb, origin, porigin);
+		if (!origin->suspects)
+			goto finish;
+	}
+
+	/*
+	 * Optionally find moves in parents' files.
+	 */
+	if (opt & PICKAXE_BLAME_MOVE) {
+		filter_small(sb, &toosmall, &origin->suspects, sb->move_score);
+		if (origin->suspects) {
+			for (i = 0, sg = first_scapegoat(revs, commit, sb->reverse);
+			     i < num_sg && sg;
+			     sg = sg->next, i++) {
+				struct blame_origin *porigin = sg_origin[i];
+				if (!porigin)
+					continue;
+				find_move_in_parent(sb, &blametail, &toosmall, origin, porigin);
+				if (!origin->suspects)
+					break;
+			}
+		}
+	}
+
+	/*
+	 * Optionally find copies from parents' files.
+	 */
+	if (opt & PICKAXE_BLAME_COPY) {
+		if (sb->copy_score > sb->move_score)
+			filter_small(sb, &toosmall, &origin->suspects, sb->copy_score);
+		else if (sb->copy_score < sb->move_score) {
+			origin->suspects = blame_merge(origin->suspects, toosmall);
+			toosmall = NULL;
+			filter_small(sb, &toosmall, &origin->suspects, sb->copy_score);
+		}
+		if (!origin->suspects)
+			goto finish;
+
+		for (i = 0, sg = first_scapegoat(revs, commit, sb->reverse);
+		     i < num_sg && sg;
+		     sg = sg->next, i++) {
+			struct blame_origin *porigin = sg_origin[i];
+			find_copy_in_parent(sb, &blametail, &toosmall,
+					    origin, sg->item, porigin, opt);
+			if (!origin->suspects)
+				goto finish;
+		}
+	}
+
+finish:
+	*blametail = NULL;
+	distribute_blame(sb, blames);
+	/*
+	 * prepend toosmall to origin->suspects
+	 *
+	 * There is no point in sorting: this ends up on a big
+	 * unsorted list in the caller anyway.
+	 */
+	if (toosmall) {
+		struct blame_entry **tail = &toosmall;
+		while (*tail)
+			tail = &(*tail)->next;
+		*tail = origin->suspects;
+		origin->suspects = toosmall;
+	}
+	for (i = 0; i < num_sg; i++) {
+		if (sg_origin[i]) {
+			drop_origin_blob(sg_origin[i]);
+			blame_origin_decref(sg_origin[i]);
+		}
+	}
+	drop_origin_blob(origin);
+	if (sg_buf != sg_origin)
+		free(sg_origin);
+}
+
+/*
+ * The main loop -- while we have blobs with lines whose true origin
+ * is still unknown, pick one blob, and allow its lines to pass blames
+ * to its parents. */
+void assign_blame(struct blame_scoreboard *sb, int opt)
+{
+	struct rev_info *revs = sb->revs;
+	struct commit *commit = prio_queue_get(&sb->commits);
+
+	while (commit) {
+		struct blame_entry *ent;
+		struct blame_origin *suspect = commit->util;
+
+		/* find one suspect to break down */
+		while (suspect && !suspect->suspects)
+			suspect = suspect->next;
+
+		if (!suspect) {
+			commit = prio_queue_get(&sb->commits);
+			continue;
+		}
+
+		assert(commit == suspect->commit);
+
+		/*
+		 * We will use this suspect later in the loop,
+		 * so hold onto it in the meantime.
+		 */
+		blame_origin_incref(suspect);
+		parse_commit(commit);
+		if (sb->reverse ||
+		    (!(commit->object.flags & UNINTERESTING) &&
+		     !(revs->max_age != -1 && commit->date < revs->max_age)))
+			pass_blame(sb, suspect, opt);
+		else {
+			commit->object.flags |= UNINTERESTING;
+			if (commit->object.parsed)
+				mark_parents_uninteresting(commit);
+		}
+		/* treat root commit as boundary */
+		if (!commit->parents && !sb->show_root)
+			commit->object.flags |= UNINTERESTING;
+
+		/* Take responsibility for the remaining entries */
+		ent = suspect->suspects;
+		if (ent) {
+			suspect->guilty = 1;
+			for (;;) {
+				struct blame_entry *next = ent->next;
+				if (sb->found_guilty_entry)
+					sb->found_guilty_entry(ent, sb->found_guilty_entry_data);
+				if (next) {
+					ent = next;
+					continue;
+				}
+				ent->next = sb->ent;
+				sb->ent = suspect->suspects;
+				suspect->suspects = NULL;
+				break;
+			}
+		}
+		blame_origin_decref(suspect);
+
+		if (sb->debug) /* sanity */
+			sanity_check_refcnt(sb);
+	}
+}
diff --git a/blame.h b/blame.h
index 0d10e17..df824b2 100644
--- a/blame.h
+++ b/blame.h
@@ -8,6 +8,11 @@
 #include "prio-queue.h"
 #include "diff.h"
 
+#define PICKAXE_BLAME_MOVE		01
+#define PICKAXE_BLAME_COPY		02
+#define PICKAXE_BLAME_COPY_HARDER	04
+#define PICKAXE_BLAME_COPY_HARDEST	010
+
 /*
  * One blob in a commit that is being suspected
  */
@@ -157,4 +162,10 @@ extern struct blame_origin *get_origin(struct commit *commit, const char *path);
 
 extern struct commit *fake_working_tree_commit(struct diff_options *opt, const char *path, const char *contents_from);
 
+extern void blame_coalesce(struct blame_scoreboard *sb);
+extern void blame_sort_final(struct blame_scoreboard *sb);
+extern unsigned blame_entry_score(struct blame_scoreboard *sb, struct blame_entry *e);
+extern void assign_blame(struct blame_scoreboard *sb, int opt);
+extern const char *blame_nth_line(struct blame_scoreboard *sb, long lno);
+
 #endif /* BLAME_H */
diff --git a/builtin/blame.c b/builtin/blame.c
index 3679214..461506c 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -12,13 +12,10 @@
 #include "tag.h"
 #include "tree-walk.h"
 #include "diff.h"
-#include "diffcore.h"
 #include "revision.h"
 #include "quote.h"
-#include "xdiff-interface.h"
 #include "string-list.h"
 #include "mailmap.h"
-#include "mergesort.h"
 #include "parse-options.h"
 #include "prio-queue.h"
 #include "utf8.h"
@@ -61,11 +58,6 @@ static struct string_list mailmap = STRING_LIST_INIT_NODUP;
 #define DEBUG 0
 #endif
 
-#define PICKAXE_BLAME_MOVE		01
-#define PICKAXE_BLAME_COPY		02
-#define PICKAXE_BLAME_COPY_HARDER	04
-#define PICKAXE_BLAME_COPY_HARDEST	010
-
 static unsigned blame_move_score;
 static unsigned blame_copy_score;
 #define BLAME_DEFAULT_MOVE_SCORE	20
@@ -80,143 +72,6 @@ struct progress_info {
 	int blamed_lines;
 };
 
-static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b,
-		      xdl_emit_hunk_consume_func_t hunk_func, void *cb_data, int xdl_opts)
-{
-	xpparam_t xpp = {0};
-	xdemitconf_t xecfg = {0};
-	xdemitcb_t ecb = {NULL};
-
-	xpp.flags = xdl_opts;
-	xecfg.hunk_func = hunk_func;
-	ecb.priv = cb_data;
-	return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb);
-}
-
-/*
- * Given an origin, prepare mmfile_t structure to be used by the
- * diff machinery
- */
-static void fill_origin_blob(struct diff_options *opt,
-			     struct blame_origin *o, mmfile_t *file, int *num_read_blob)
-{
-	if (!o->file.ptr) {
-		enum object_type type;
-		unsigned long file_size;
-
-		(*num_read_blob)++;
-		if (DIFF_OPT_TST(opt, ALLOW_TEXTCONV) &&
-		    textconv_object(o->path, o->mode, &o->blob_oid, 1, &file->ptr, &file_size))
-			;
-		else
-			file->ptr = read_sha1_file(o->blob_oid.hash, &type,
-						   &file_size);
-		file->size = file_size;
-
-		if (!file->ptr)
-			die("Cannot read blob %s for path %s",
-			    oid_to_hex(&o->blob_oid),
-			    o->path);
-		o->file = *file;
-	}
-	else
-		*file = o->file;
-}
-
-static void drop_origin_blob(struct blame_origin *o)
-{
-	if (o->file.ptr) {
-		free(o->file.ptr);
-		o->file.ptr = NULL;
-	}
-}
-
-/*
- * Any merge of blames happens on lists of blames that arrived via
- * different parents in a single suspect.  In this case, we want to
- * sort according to the suspect line numbers as opposed to the final
- * image line numbers.  The function body is somewhat longish because
- * it avoids unnecessary writes.
- */
-
-static struct blame_entry *blame_merge(struct blame_entry *list1,
-				       struct blame_entry *list2)
-{
-	struct blame_entry *p1 = list1, *p2 = list2,
-		**tail = &list1;
-
-	if (!p1)
-		return p2;
-	if (!p2)
-		return p1;
-
-	if (p1->s_lno <= p2->s_lno) {
-		do {
-			tail = &p1->next;
-			if ((p1 = *tail) == NULL) {
-				*tail = p2;
-				return list1;
-			}
-		} while (p1->s_lno <= p2->s_lno);
-	}
-	for (;;) {
-		*tail = p2;
-		do {
-			tail = &p2->next;
-			if ((p2 = *tail) == NULL)  {
-				*tail = p1;
-				return list1;
-			}
-		} while (p1->s_lno > p2->s_lno);
-		*tail = p1;
-		do {
-			tail = &p1->next;
-			if ((p1 = *tail) == NULL) {
-				*tail = p2;
-				return list1;
-			}
-		} while (p1->s_lno <= p2->s_lno);
-	}
-}
-
-static void *get_next_blame(const void *p)
-{
-	return ((struct blame_entry *)p)->next;
-}
-
-static void set_next_blame(void *p1, void *p2)
-{
-	((struct blame_entry *)p1)->next = p2;
-}
-
-/*
- * Final image line numbers are all different, so we don't need a
- * three-way comparison here.
- */
-
-static int compare_blame_final(const void *p1, const void *p2)
-{
-	return ((struct blame_entry *)p1)->lno > ((struct blame_entry *)p2)->lno
-		? 1 : -1;
-}
-
-static int compare_blame_suspect(const void *p1, const void *p2)
-{
-	const struct blame_entry *s1 = p1, *s2 = p2;
-	/*
-	 * to allow for collating suspects, we sort according to the
-	 * respective pointer value as the primary sorting criterion.
-	 * The actual relation is pretty unimportant as long as it
-	 * establishes a total order.  Comparing as integers gives us
-	 * that.
-	 */
-	if (s1->suspect != s2->suspect)
-		return (intptr_t)s1->suspect > (intptr_t)s2->suspect ? 1 : -1;
-	if (s1->s_lno == s2->s_lno)
-		return 0;
-	return s1->s_lno > s2->s_lno ? 1 : -1;
-}
-
 static int compare_commits_by_reverse_commit_date(const void *a,
 						  const void *b,
 						  void *c)
@@ -224,63 +79,6 @@ static int compare_commits_by_reverse_commit_date(const void *a,
 	return -compare_commits_by_commit_date(a, b, c);
 }
 
-static void blame_sort_final(struct blame_scoreboard *sb)
-{
-	sb->ent = llist_mergesort(sb->ent, get_next_blame, set_next_blame,
-				  compare_blame_final);
-}
-
-static void sanity_check_refcnt(struct blame_scoreboard *);
-
-/*
- * If two blame entries that are next to each other came from
- * contiguous lines in the same origin (i.e. <commit, path> pair),
- * merge them together.
- */
-static void blame_coalesce(struct blame_scoreboard *sb)
-{
-	struct blame_entry *ent, *next;
-
-	for (ent = sb->ent; ent && (next = ent->next); ent = next) {
-		if (ent->suspect == next->suspect &&
-		    ent->s_lno + ent->num_lines == next->s_lno) {
-			ent->num_lines += next->num_lines;
-			ent->next = next->next;
-			blame_origin_decref(next->suspect);
-			free(next);
-			ent->score = 0;
-			next = ent; /* again */
-		}
-	}
-
-	if (sb->debug) /* sanity */
-		sanity_check_refcnt(sb);
-}
-
-/*
- * Merge the given sorted list of blames into a preexisting origin.
- * If there were no previous blames to that commit, it is entered into
- * the commit priority queue of the score board.
- */
-
-static void queue_blames(struct blame_scoreboard *sb, struct blame_origin *porigin,
-			 struct blame_entry *sorted)
-{
-	if (porigin->suspects)
-		porigin->suspects = blame_merge(porigin->suspects, sorted);
-	else {
-		struct blame_origin *o;
-		for (o = porigin->commit->util; o; o = o->next) {
-			if (o->suspects) {
-				porigin->suspects = sorted;
-				return;
-			}
-		}
-		porigin->suspects = sorted;
-		prio_queue_put(&sb->commits, porigin->commit);
-	}
-}
-
 /*
  * Fill the blob_sha1 field of an origin if it hasn't, so that later
  * call to fill_origin_blob() can use it to locate the data.  blob_sha1
@@ -307,1037 +105,12 @@ static int fill_blob_sha1_and_mode(struct blame_origin *origin)
 	return -1;
 }
 
-/*
- * We have an origin -- check if the same path exists in the
- * parent and return an origin structure to represent it.
- */
-static struct blame_origin *find_origin(struct commit *parent,
-				  struct blame_origin *origin)
-{
-	struct blame_origin *porigin;
-	struct diff_options diff_opts;
-	const char *paths[2];
-
-	/* First check any existing origins */
-	for (porigin = parent->util; porigin; porigin = porigin->next)
-		if (!strcmp(porigin->path, origin->path)) {
-			/*
-			 * The same path between origin and its parent
-			 * without renaming -- the most common case.
-			 */
-			return blame_origin_incref (porigin);
-		}
-
-	/* See if the origin->path is different between parent
-	 * and origin first.  Most of the time they are the
-	 * same and diff-tree is fairly efficient about this.
-	 */
-	diff_setup(&diff_opts);
-	DIFF_OPT_SET(&diff_opts, RECURSIVE);
-	diff_opts.detect_rename = 0;
-	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
-	paths[0] = origin->path;
-	paths[1] = NULL;
-
-	parse_pathspec(&diff_opts.pathspec,
-		       PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL,
-		       PATHSPEC_LITERAL_PATH, "", paths);
-	diff_setup_done(&diff_opts);
-
-	if (is_null_oid(&origin->commit->object.oid))
-		do_diff_cache(parent->tree->object.oid.hash, &diff_opts);
-	else
-		diff_tree_sha1(parent->tree->object.oid.hash,
-			       origin->commit->tree->object.oid.hash,
-			       "", &diff_opts);
-	diffcore_std(&diff_opts);
-
-	if (!diff_queued_diff.nr) {
-		/* The path is the same as parent */
-		porigin = get_origin(parent, origin->path);
-		oidcpy(&porigin->blob_oid, &origin->blob_oid);
-		porigin->mode = origin->mode;
-	} else {
-		/*
-		 * Since origin->path is a pathspec, if the parent
-		 * commit had it as a directory, we will see a whole
-		 * bunch of deletion of files in the directory that we
-		 * do not care about.
-		 */
-		int i;
-		struct diff_filepair *p = NULL;
-		for (i = 0; i < diff_queued_diff.nr; i++) {
-			const char *name;
-			p = diff_queued_diff.queue[i];
-			name = p->one->path ? p->one->path : p->two->path;
-			if (!strcmp(name, origin->path))
-				break;
-		}
-		if (!p)
-			die("internal error in blame::find_origin");
-		switch (p->status) {
-		default:
-			die("internal error in blame::find_origin (%c)",
-			    p->status);
-		case 'M':
-			porigin = get_origin(parent, origin->path);
-			oidcpy(&porigin->blob_oid, &p->one->oid);
-			porigin->mode = p->one->mode;
-			break;
-		case 'A':
-		case 'T':
-			/* Did not exist in parent, or type changed */
-			break;
-		}
-	}
-	diff_flush(&diff_opts);
-	clear_pathspec(&diff_opts.pathspec);
-	return porigin;
-}
-
-/*
- * We have an origin -- find the path that corresponds to it in its
- * parent and return an origin structure to represent it.
- */
-static struct blame_origin *find_rename(struct commit *parent,
-				  struct blame_origin *origin)
-{
-	struct blame_origin *porigin = NULL;
-	struct diff_options diff_opts;
-	int i;
-
-	diff_setup(&diff_opts);
-	DIFF_OPT_SET(&diff_opts, RECURSIVE);
-	diff_opts.detect_rename = DIFF_DETECT_RENAME;
-	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
-	diff_opts.single_follow = origin->path;
-	diff_setup_done(&diff_opts);
-
-	if (is_null_oid(&origin->commit->object.oid))
-		do_diff_cache(parent->tree->object.oid.hash, &diff_opts);
-	else
-		diff_tree_sha1(parent->tree->object.oid.hash,
-			       origin->commit->tree->object.oid.hash,
-			       "", &diff_opts);
-	diffcore_std(&diff_opts);
-
-	for (i = 0; i < diff_queued_diff.nr; i++) {
-		struct diff_filepair *p = diff_queued_diff.queue[i];
-		if ((p->status == 'R' || p->status == 'C') &&
-		    !strcmp(p->two->path, origin->path)) {
-			porigin = get_origin(parent, p->one->path);
-			oidcpy(&porigin->blob_oid, &p->one->oid);
-			porigin->mode = p->one->mode;
-			break;
-		}
-	}
-	diff_flush(&diff_opts);
-	clear_pathspec(&diff_opts.pathspec);
-	return porigin;
-}
-
-/*
- * Append a new blame entry to a given output queue.
- */
-static void add_blame_entry(struct blame_entry ***queue,
-			    const struct blame_entry *src)
-{
-	struct blame_entry *e = xmalloc(sizeof(*e));
-	memcpy(e, src, sizeof(*e));
-	blame_origin_incref(e->suspect);
-
-	e->next = **queue;
-	**queue = e;
-	*queue = &e->next;
-}
-
-/*
- * src typically is on-stack; we want to copy the information in it to
- * a malloced blame_entry that gets added to the given queue.  The
- * origin of dst loses a refcnt.
- */
-static void dup_entry(struct blame_entry ***queue,
-		      struct blame_entry *dst, struct blame_entry *src)
-{
-	blame_origin_incref(src->suspect);
-	blame_origin_decref(dst->suspect);
-	memcpy(dst, src, sizeof(*src));
-	dst->next = **queue;
-	**queue = dst;
-	*queue = &dst->next;
-}
-
-static const char *blame_nth_line(struct blame_scoreboard *sb, long lno)
-{
-	return sb->final_buf + sb->lineno[lno];
-}
-
 static const char *nth_line_cb(void *data, long lno)
 {
 	return blame_nth_line((struct blame_scoreboard *)data, lno);
 }
 
 /*
- * It is known that lines between tlno to same came from parent, and e
- * has an overlap with that range.  it also is known that parent's
- * line plno corresponds to e's line tlno.
- *
- *                <---- e ----->
- *                   <------>
- *                   <------------>
- *             <------------>
- *             <------------------>
- *
- * Split e into potentially three parts; before this chunk, the chunk
- * to be blamed for the parent, and after that portion.
- */
-static void split_overlap(struct blame_entry *split,
-			  struct blame_entry *e,
-			  int tlno, int plno, int same,
-			  struct blame_origin *parent)
-{
-	int chunk_end_lno;
-	memset(split, 0, sizeof(struct blame_entry [3]));
-
-	if (e->s_lno < tlno) {
-		/* there is a pre-chunk part not blamed on parent */
-		split[0].suspect = blame_origin_incref(e->suspect);
-		split[0].lno = e->lno;
-		split[0].s_lno = e->s_lno;
-		split[0].num_lines = tlno - e->s_lno;
-		split[1].lno = e->lno + tlno - e->s_lno;
-		split[1].s_lno = plno;
-	}
-	else {
-		split[1].lno = e->lno;
-		split[1].s_lno = plno + (e->s_lno - tlno);
-	}
-
-	if (same < e->s_lno + e->num_lines) {
-		/* there is a post-chunk part not blamed on parent */
-		split[2].suspect = blame_origin_incref(e->suspect);
-		split[2].lno = e->lno + (same - e->s_lno);
-		split[2].s_lno = e->s_lno + (same - e->s_lno);
-		split[2].num_lines = e->s_lno + e->num_lines - same;
-		chunk_end_lno = split[2].lno;
-	}
-	else
-		chunk_end_lno = e->lno + e->num_lines;
-	split[1].num_lines = chunk_end_lno - split[1].lno;
-
-	/*
-	 * if it turns out there is nothing to blame the parent for,
-	 * forget about the splitting.  !split[1].suspect signals this.
-	 */
-	if (split[1].num_lines < 1)
-		return;
-	split[1].suspect = blame_origin_incref(parent);
-}
-
-/*
- * split_overlap() divided an existing blame e into up to three parts
- * in split.  Any assigned blame is moved to queue to
- * reflect the split.
- */
-static void split_blame(struct blame_entry ***blamed,
-			struct blame_entry ***unblamed,
-			struct blame_entry *split,
-			struct blame_entry *e)
-{
-	if (split[0].suspect && split[2].suspect) {
-		/* The first part (reuse storage for the existing entry e) */
-		dup_entry(unblamed, e, &split[0]);
-
-		/* The last part -- me */
-		add_blame_entry(unblamed, &split[2]);
-
-		/* ... and the middle part -- parent */
-		add_blame_entry(blamed, &split[1]);
-	}
-	else if (!split[0].suspect && !split[2].suspect)
-		/*
-		 * The parent covers the entire area; reuse storage for
-		 * e and replace it with the parent.
-		 */
-		dup_entry(blamed, e, &split[1]);
-	else if (split[0].suspect) {
-		/* me and then parent */
-		dup_entry(unblamed, e, &split[0]);
-		add_blame_entry(blamed, &split[1]);
-	}
-	else {
-		/* parent and then me */
-		dup_entry(blamed, e, &split[1]);
-		add_blame_entry(unblamed, &split[2]);
-	}
-}
-
-/*
- * After splitting the blame, the origins used by the
- * on-stack blame_entry should lose one refcnt each.
- */
-static void decref_split(struct blame_entry *split)
-{
-	int i;
-
-	for (i = 0; i < 3; i++)
-		blame_origin_decref(split[i].suspect);
-}
-
-/*
- * reverse_blame reverses the list given in head, appending tail.
- * That allows us to build lists in reverse order, then reverse them
- * afterwards.  This can be faster than building the list in proper
- * order right away.  The reason is that building in proper order
- * requires writing a link in the _previous_ element, while building
- * in reverse order just requires placing the list head into the
- * _current_ element.
- */
-
-static struct blame_entry *reverse_blame(struct blame_entry *head,
-					 struct blame_entry *tail)
-{
-	while (head) {
-		struct blame_entry *next = head->next;
-		head->next = tail;
-		tail = head;
-		head = next;
-	}
-	return tail;
-}
-
-/*
- * Process one hunk from the patch between the current suspect for
- * blame_entry e and its parent.  This first blames any unfinished
- * entries before the chunk (which is where target and parent start
- * differing) on the parent, and then splits blame entries at the
- * start and at the end of the difference region.  Since use of -M and
- * -C options may lead to overlapping/duplicate source line number
- * ranges, all we can rely on from sorting/merging is the order of the
- * first suspect line number.
- */
-static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq,
-			int tlno, int offset, int same,
-			struct blame_origin *parent)
-{
-	struct blame_entry *e = **srcq;
-	struct blame_entry *samep = NULL, *diffp = NULL;
-
-	while (e && e->s_lno < tlno) {
-		struct blame_entry *next = e->next;
-		/*
-		 * current record starts before differing portion.  If
-		 * it reaches into it, we need to split it up and
-		 * examine the second part separately.
-		 */
-		if (e->s_lno + e->num_lines > tlno) {
-			/* Move second half to a new record */
-			int len = tlno - e->s_lno;
-			struct blame_entry *n = xcalloc(1, sizeof (struct blame_entry));
-			n->suspect = e->suspect;
-			n->lno = e->lno + len;
-			n->s_lno = e->s_lno + len;
-			n->num_lines = e->num_lines - len;
-			e->num_lines = len;
-			e->score = 0;
-			/* Push new record to diffp */
-			n->next = diffp;
-			diffp = n;
-		} else
-			blame_origin_decref(e->suspect);
-		/* Pass blame for everything before the differing
-		 * chunk to the parent */
-		e->suspect = blame_origin_incref(parent);
-		e->s_lno += offset;
-		e->next = samep;
-		samep = e;
-		e = next;
-	}
-	/*
-	 * As we don't know how much of a common stretch after this
-	 * diff will occur, the currently blamed parts are all that we
-	 * can assign to the parent for now.
-	 */
-
-	if (samep) {
-		**dstq = reverse_blame(samep, **dstq);
-		*dstq = &samep->next;
-	}
-	/*
-	 * Prepend the split off portions: everything after e starts
-	 * after the blameable portion.
-	 */
-	e = reverse_blame(diffp, e);
-
-	/*
-	 * Now retain records on the target while parts are different
-	 * from the parent.
-	 */
-	samep = NULL;
-	diffp = NULL;
-	while (e && e->s_lno < same) {
-		struct blame_entry *next = e->next;
-
-		/*
-		 * If current record extends into sameness, need to split.
-		 */
-		if (e->s_lno + e->num_lines > same) {
-			/*
-			 * Move second half to a new record to be
-			 * processed by later chunks
-			 */
-			int len = same - e->s_lno;
-			struct blame_entry *n = xcalloc(1, sizeof (struct blame_entry));
-			n->suspect = blame_origin_incref(e->suspect);
-			n->lno = e->lno + len;
-			n->s_lno = e->s_lno + len;
-			n->num_lines = e->num_lines - len;
-			e->num_lines = len;
-			e->score = 0;
-			/* Push new record to samep */
-			n->next = samep;
-			samep = n;
-		}
-		e->next = diffp;
-		diffp = e;
-		e = next;
-	}
-	**srcq = reverse_blame(diffp, reverse_blame(samep, e));
-	/* Move across elements that are in the unblamable portion */
-	if (diffp)
-		*srcq = &diffp->next;
-}
-
-struct blame_chunk_cb_data {
-	struct blame_origin *parent;
-	long offset;
-	struct blame_entry **dstq;
-	struct blame_entry **srcq;
-};
-
-/* diff chunks are from parent to target */
-static int blame_chunk_cb(long start_a, long count_a,
-			  long start_b, long count_b, void *data)
-{
-	struct blame_chunk_cb_data *d = data;
-	if (start_a - start_b != d->offset)
-		die("internal error in blame::blame_chunk_cb");
-	blame_chunk(&d->dstq, &d->srcq, start_b, start_a - start_b,
-		    start_b + count_b, d->parent);
-	d->offset = start_a + count_a - (start_b + count_b);
-	return 0;
-}
-
-/*
- * We are looking at the origin 'target' and aiming to pass blame
- * for the lines it is suspected to its parent.  Run diff to find
- * which lines came from parent and pass blame for them.
- */
-static void pass_blame_to_parent(struct blame_scoreboard *sb,
-				 struct blame_origin *target,
-				 struct blame_origin *parent)
-{
-	mmfile_t file_p, file_o;
-	struct blame_chunk_cb_data d;
-	struct blame_entry *newdest = NULL;
-
-	if (!target->suspects)
-		return; /* nothing remains for this target */
-
-	d.parent = parent;
-	d.offset = 0;
-	d.dstq = &newdest; d.srcq = &target->suspects;
-
-	fill_origin_blob(&sb->revs->diffopt, parent, &file_p, &sb->num_read_blob);
-	fill_origin_blob(&sb->revs->diffopt, target, &file_o, &sb->num_read_blob);
-	sb->num_get_patch++;
-
-	if (diff_hunks(&file_p, &file_o, blame_chunk_cb, &d, sb->xdl_opts))
-		die("unable to generate diff (%s -> %s)",
-		    oid_to_hex(&parent->commit->object.oid),
-		    oid_to_hex(&target->commit->object.oid));
-	/* The rest are the same as the parent */
-	blame_chunk(&d.dstq, &d.srcq, INT_MAX, d.offset, INT_MAX, parent);
-	*d.dstq = NULL;
-	queue_blames(sb, parent, newdest);
-
-	return;
-}
-
-/*
- * The lines in blame_entry after splitting blames many times can become
- * very small and trivial, and at some point it becomes pointless to
- * blame the parents.  E.g. "\t\t}\n\t}\n\n" appears everywhere in any
- * ordinary C program, and it is not worth to say it was copied from
- * totally unrelated file in the parent.
- *
- * Compute how trivial the lines in the blame_entry are.
- */
-static unsigned blame_entry_score(struct blame_scoreboard *sb, struct blame_entry *e)
-{
-	unsigned score;
-	const char *cp, *ep;
-
-	if (e->score)
-		return e->score;
-
-	score = 1;
-	cp = blame_nth_line(sb, e->lno);
-	ep = blame_nth_line(sb, e->lno + e->num_lines);
-	while (cp < ep) {
-		unsigned ch = *((unsigned char *)cp);
-		if (isalnum(ch))
-			score++;
-		cp++;
-	}
-	e->score = score;
-	return score;
-}
-
-/*
- * best_so_far[] and this[] are both a split of an existing blame_entry
- * that passes blame to the parent.  Maintain best_so_far the best split
- * so far, by comparing this and best_so_far and copying this into
- * bst_so_far as needed.
- */
-static void copy_split_if_better(struct blame_scoreboard *sb,
-				 struct blame_entry *best_so_far,
-				 struct blame_entry *this)
-{
-	int i;
-
-	if (!this[1].suspect)
-		return;
-	if (best_so_far[1].suspect) {
-		if (blame_entry_score(sb, &this[1]) < blame_entry_score(sb, &best_so_far[1]))
-			return;
-	}
-
-	for (i = 0; i < 3; i++)
-		blame_origin_incref(this[i].suspect);
-	decref_split(best_so_far);
-	memcpy(best_so_far, this, sizeof(struct blame_entry [3]));
-}
-
-/*
- * We are looking at a part of the final image represented by
- * ent (tlno and same are offset by ent->s_lno).
- * tlno is where we are looking at in the final image.
- * up to (but not including) same match preimage.
- * plno is where we are looking at in the preimage.
- *
- * <-------------- final image ---------------------->
- *       <------ent------>
- *         ^tlno ^same
- *    <---------preimage----->
- *         ^plno
- *
- * All line numbers are 0-based.
- */
-static void handle_split(struct blame_scoreboard *sb,
-			 struct blame_entry *ent,
-			 int tlno, int plno, int same,
-			 struct blame_origin *parent,
-			 struct blame_entry *split)
-{
-	if (ent->num_lines <= tlno)
-		return;
-	if (tlno < same) {
-		struct blame_entry this[3];
-		tlno += ent->s_lno;
-		same += ent->s_lno;
-		split_overlap(this, ent, tlno, plno, same, parent);
-		copy_split_if_better(sb, split, this);
-		decref_split(this);
-	}
-}
-
-struct handle_split_cb_data {
-	struct blame_scoreboard *sb;
-	struct blame_entry *ent;
-	struct blame_origin *parent;
-	struct blame_entry *split;
-	long plno;
-	long tlno;
-};
-
-static int handle_split_cb(long start_a, long count_a,
-			   long start_b, long count_b, void *data)
-{
-	struct handle_split_cb_data *d = data;
-	handle_split(d->sb, d->ent, d->tlno, d->plno, start_b, d->parent,
-		     d->split);
-	d->plno = start_a + count_a;
-	d->tlno = start_b + count_b;
-	return 0;
-}
-
-/*
- * Find the lines from parent that are the same as ent so that
- * we can pass blames to it.  file_p has the blob contents for
- * the parent.
- */
-static void find_copy_in_blob(struct blame_scoreboard *sb,
-			      struct blame_entry *ent,
-			      struct blame_origin *parent,
-			      struct blame_entry *split,
-			      mmfile_t *file_p)
-{
-	const char *cp;
-	mmfile_t file_o;
-	struct handle_split_cb_data d;
-
-	memset(&d, 0, sizeof(d));
-	d.sb = sb; d.ent = ent; d.parent = parent; d.split = split;
-	/*
-	 * Prepare mmfile that contains only the lines in ent.
-	 */
-	cp = blame_nth_line(sb, ent->lno);
-	file_o.ptr = (char *) cp;
-	file_o.size = blame_nth_line(sb, ent->lno + ent->num_lines) - cp;
-
-	/*
-	 * file_o is a part of final image we are annotating.
-	 * file_p partially may match that image.
-	 */
-	memset(split, 0, sizeof(struct blame_entry [3]));
-	if (diff_hunks(file_p, &file_o, handle_split_cb, &d, sb->xdl_opts))
-		die("unable to generate diff (%s)",
-		    oid_to_hex(&parent->commit->object.oid));
-	/* remainder, if any, all match the preimage */
-	handle_split(sb, ent, d.tlno, d.plno, ent->num_lines, parent, split);
-}
-
-/* Move all blame entries from list *source that have a score smaller
- * than score_min to the front of list *small.
- * Returns a pointer to the link pointing to the old head of the small list.
- */
-
-static struct blame_entry **filter_small(struct blame_scoreboard *sb,
-					 struct blame_entry **small,
-					 struct blame_entry **source,
-					 unsigned score_min)
-{
-	struct blame_entry *p = *source;
-	struct blame_entry *oldsmall = *small;
-	while (p) {
-		if (blame_entry_score(sb, p) <= score_min) {
-			*small = p;
-			small = &p->next;
-			p = *small;
-		} else {
-			*source = p;
-			source = &p->next;
-			p = *source;
-		}
-	}
-	*small = oldsmall;
-	*source = NULL;
-	return small;
-}
-
-/*
- * See if lines currently target is suspected for can be attributed to
- * parent.
- */
-static void find_move_in_parent(struct blame_scoreboard *sb,
-				struct blame_entry ***blamed,
-				struct blame_entry **toosmall,
-				struct blame_origin *target,
-				struct blame_origin *parent)
-{
-	struct blame_entry *e, split[3];
-	struct blame_entry *unblamed = target->suspects;
-	struct blame_entry *leftover = NULL;
-	mmfile_t file_p;
-
-	if (!unblamed)
-		return; /* nothing remains for this target */
-
-	fill_origin_blob(&sb->revs->diffopt, parent, &file_p, &sb->num_read_blob);
-	if (!file_p.ptr)
-		return;
-
-	/* At each iteration, unblamed has a NULL-terminated list of
-	 * entries that have not yet been tested for blame.  leftover
-	 * contains the reversed list of entries that have been tested
-	 * without being assignable to the parent.
-	 */
-	do {
-		struct blame_entry **unblamedtail = &unblamed;
-		struct blame_entry *next;
-		for (e = unblamed; e; e = next) {
-			next = e->next;
-			find_copy_in_blob(sb, e, parent, split, &file_p);
-			if (split[1].suspect &&
-			    sb->move_score < blame_entry_score(sb, &split[1])) {
-				split_blame(blamed, &unblamedtail, split, e);
-			} else {
-				e->next = leftover;
-				leftover = e;
-			}
-			decref_split(split);
-		}
-		*unblamedtail = NULL;
-		toosmall = filter_small(sb, toosmall, &unblamed, sb->move_score);
-	} while (unblamed);
-	target->suspects = reverse_blame(leftover, NULL);
-}
-
-struct blame_list {
-	struct blame_entry *ent;
-	struct blame_entry split[3];
-};
-
-/*
- * Count the number of entries the target is suspected for,
- * and prepare a list of entry and the best split.
- */
-static struct blame_list *setup_blame_list(struct blame_entry *unblamed,
-					   int *num_ents_p)
-{
-	struct blame_entry *e;
-	int num_ents, i;
-	struct blame_list *blame_list = NULL;
-
-	for (e = unblamed, num_ents = 0; e; e = e->next)
-		num_ents++;
-	if (num_ents) {
-		blame_list = xcalloc(num_ents, sizeof(struct blame_list));
-		for (e = unblamed, i = 0; e; e = e->next)
-			blame_list[i++].ent = e;
-	}
-	*num_ents_p = num_ents;
-	return blame_list;
-}
-
-/*
- * For lines target is suspected for, see if we can find code movement
- * across file boundary from the parent commit.  porigin is the path
- * in the parent we already tried.
- */
-static void find_copy_in_parent(struct blame_scoreboard *sb,
-				struct blame_entry ***blamed,
-				struct blame_entry **toosmall,
-				struct blame_origin *target,
-				struct commit *parent,
-				struct blame_origin *porigin,
-				int opt)
-{
-	struct diff_options diff_opts;
-	int i, j;
-	struct blame_list *blame_list;
-	int num_ents;
-	struct blame_entry *unblamed = target->suspects;
-	struct blame_entry *leftover = NULL;
-
-	if (!unblamed)
-		return; /* nothing remains for this target */
-
-	diff_setup(&diff_opts);
-	DIFF_OPT_SET(&diff_opts, RECURSIVE);
-	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
-
-	diff_setup_done(&diff_opts);
-
-	/* Try "find copies harder" on new path if requested;
-	 * we do not want to use diffcore_rename() actually to
-	 * match things up; find_copies_harder is set only to
-	 * force diff_tree_sha1() to feed all filepairs to diff_queue,
-	 * and this code needs to be after diff_setup_done(), which
-	 * usually makes find-copies-harder imply copy detection.
-	 */
-	if ((opt & PICKAXE_BLAME_COPY_HARDEST)
-	    || ((opt & PICKAXE_BLAME_COPY_HARDER)
-		&& (!porigin || strcmp(target->path, porigin->path))))
-		DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
-
-	if (is_null_oid(&target->commit->object.oid))
-		do_diff_cache(parent->tree->object.oid.hash, &diff_opts);
-	else
-		diff_tree_sha1(parent->tree->object.oid.hash,
-			       target->commit->tree->object.oid.hash,
-			       "", &diff_opts);
-
-	if (!DIFF_OPT_TST(&diff_opts, FIND_COPIES_HARDER))
-		diffcore_std(&diff_opts);
-
-	do {
-		struct blame_entry **unblamedtail = &unblamed;
-		blame_list = setup_blame_list(unblamed, &num_ents);
-
-		for (i = 0; i < diff_queued_diff.nr; i++) {
-			struct diff_filepair *p = diff_queued_diff.queue[i];
-			struct blame_origin *norigin;
-			mmfile_t file_p;
-			struct blame_entry this[3];
-
-			if (!DIFF_FILE_VALID(p->one))
-				continue; /* does not exist in parent */
-			if (S_ISGITLINK(p->one->mode))
-				continue; /* ignore git links */
-			if (porigin && !strcmp(p->one->path, porigin->path))
-				/* find_move already dealt with this path */
-				continue;
-
-			norigin = get_origin(parent, p->one->path);
-			oidcpy(&norigin->blob_oid, &p->one->oid);
-			norigin->mode = p->one->mode;
-			fill_origin_blob(&sb->revs->diffopt, norigin, &file_p, &sb->num_read_blob);
-			if (!file_p.ptr)
-				continue;
-
-			for (j = 0; j < num_ents; j++) {
-				find_copy_in_blob(sb, blame_list[j].ent,
-						  norigin, this, &file_p);
-				copy_split_if_better(sb, blame_list[j].split,
-						     this);
-				decref_split(this);
-			}
-			blame_origin_decref(norigin);
-		}
-
-		for (j = 0; j < num_ents; j++) {
-			struct blame_entry *split = blame_list[j].split;
-			if (split[1].suspect &&
-			    sb->copy_score < blame_entry_score(sb, &split[1])) {
-				split_blame(blamed, &unblamedtail, split,
-					    blame_list[j].ent);
-			} else {
-				blame_list[j].ent->next = leftover;
-				leftover = blame_list[j].ent;
-			}
-			decref_split(split);
-		}
-		free(blame_list);
-		*unblamedtail = NULL;
-		toosmall = filter_small(sb, toosmall, &unblamed, sb->copy_score);
-	} while (unblamed);
-	target->suspects = reverse_blame(leftover, NULL);
-	diff_flush(&diff_opts);
-	clear_pathspec(&diff_opts.pathspec);
-}
-
-/*
- * The blobs of origin and porigin exactly match, so everything
- * origin is suspected for can be blamed on the parent.
- */
-static void pass_whole_blame(struct blame_scoreboard *sb,
-			     struct blame_origin *origin, struct blame_origin *porigin)
-{
-	struct blame_entry *e, *suspects;
-
-	if (!porigin->file.ptr && origin->file.ptr) {
-		/* Steal its file */
-		porigin->file = origin->file;
-		origin->file.ptr = NULL;
-	}
-	suspects = origin->suspects;
-	origin->suspects = NULL;
-	for (e = suspects; e; e = e->next) {
-		blame_origin_incref(porigin);
-		blame_origin_decref(e->suspect);
-		e->suspect = porigin;
-	}
-	queue_blames(sb, porigin, suspects);
-}
-
-/*
- * We pass blame from the current commit to its parents.  We keep saying
- * "parent" (and "porigin"), but what we mean is to find scapegoat to
- * exonerate ourselves.
- */
-static struct commit_list *first_scapegoat(struct rev_info *revs, struct commit *commit,
-					   int reverse)
-{
-	if (!reverse) {
-		if (revs->first_parent_only &&
-		    commit->parents &&
-		    commit->parents->next) {
-			free_commit_list(commit->parents->next);
-			commit->parents->next = NULL;
-		}
-		return commit->parents;
-	}
-	return lookup_decoration(&revs->children, &commit->object);
-}
-
-static int num_scapegoats(struct rev_info *revs, struct commit *commit, int reverse)
-{
-	struct commit_list *l = first_scapegoat(revs, commit, reverse);
-	return commit_list_count(l);
-}
-
-/* Distribute collected unsorted blames to the respected sorted lists
- * in the various origins.
- */
-static void distribute_blame(struct blame_scoreboard *sb, struct blame_entry *blamed)
-{
-	blamed = llist_mergesort(blamed, get_next_blame, set_next_blame,
-				 compare_blame_suspect);
-	while (blamed)
-	{
-		struct blame_origin *porigin = blamed->suspect;
-		struct blame_entry *suspects = NULL;
-		do {
-			struct blame_entry *next = blamed->next;
-			blamed->next = suspects;
-			suspects = blamed;
-			blamed = next;
-		} while (blamed && blamed->suspect == porigin);
-		suspects = reverse_blame(suspects, NULL);
-		queue_blames(sb, porigin, suspects);
-	}
-}
-
-#define MAXSG 16
-
-static void pass_blame(struct blame_scoreboard *sb, struct blame_origin *origin, int opt)
-{
-	struct rev_info *revs = sb->revs;
-	int i, pass, num_sg;
-	struct commit *commit = origin->commit;
-	struct commit_list *sg;
-	struct blame_origin *sg_buf[MAXSG];
-	struct blame_origin *porigin, **sg_origin = sg_buf;
-	struct blame_entry *toosmall = NULL;
-	struct blame_entry *blames, **blametail = &blames;
-
-	num_sg = num_scapegoats(revs, commit, sb->reverse);
-	if (!num_sg)
-		goto finish;
-	else if (num_sg < ARRAY_SIZE(sg_buf))
-		memset(sg_buf, 0, sizeof(sg_buf));
-	else
-		sg_origin = xcalloc(num_sg, sizeof(*sg_origin));
-
-	/*
-	 * The first pass looks for unrenamed path to optimize for
-	 * common cases, then we look for renames in the second pass.
-	 */
-	for (pass = 0; pass < 2 - sb->no_whole_file_rename; pass++) {
-		struct blame_origin *(*find)(struct commit *, struct blame_origin *);
-		find = pass ? find_rename : find_origin;
-
-		for (i = 0, sg = first_scapegoat(revs, commit, sb->reverse);
-		     i < num_sg && sg;
-		     sg = sg->next, i++) {
-			struct commit *p = sg->item;
-			int j, same;
-
-			if (sg_origin[i])
-				continue;
-			if (parse_commit(p))
-				continue;
-			porigin = find(p, origin);
-			if (!porigin)
-				continue;
-			if (!oidcmp(&porigin->blob_oid, &origin->blob_oid)) {
-				pass_whole_blame(sb, origin, porigin);
-				blame_origin_decref(porigin);
-				goto finish;
-			}
-			for (j = same = 0; j < i; j++)
-				if (sg_origin[j] &&
-				    !oidcmp(&sg_origin[j]->blob_oid, &porigin->blob_oid)) {
-					same = 1;
-					break;
-				}
-			if (!same)
-				sg_origin[i] = porigin;
-			else
-				blame_origin_decref(porigin);
-		}
-	}
-
-	sb->num_commits++;
-	for (i = 0, sg = first_scapegoat(revs, commit, sb->reverse);
-	     i < num_sg && sg;
-	     sg = sg->next, i++) {
-		struct blame_origin *porigin = sg_origin[i];
-		if (!porigin)
-			continue;
-		if (!origin->previous) {
-			blame_origin_incref(porigin);
-			origin->previous = porigin;
-		}
-		pass_blame_to_parent(sb, origin, porigin);
-		if (!origin->suspects)
-			goto finish;
-	}
-
-	/*
-	 * Optionally find moves in parents' files.
-	 */
-	if (opt & PICKAXE_BLAME_MOVE) {
-		filter_small(sb, &toosmall, &origin->suspects, sb->move_score);
-		if (origin->suspects) {
-			for (i = 0, sg = first_scapegoat(revs, commit, sb->reverse);
-			     i < num_sg && sg;
-			     sg = sg->next, i++) {
-				struct blame_origin *porigin = sg_origin[i];
-				if (!porigin)
-					continue;
-				find_move_in_parent(sb, &blametail, &toosmall, origin, porigin);
-				if (!origin->suspects)
-					break;
-			}
-		}
-	}
-
-	/*
-	 * Optionally find copies from parents' files.
-	 */
-	if (opt & PICKAXE_BLAME_COPY) {
-		if (sb->copy_score > sb->move_score)
-			filter_small(sb, &toosmall, &origin->suspects, sb->copy_score);
-		else if (sb->copy_score < sb->move_score) {
-			origin->suspects = blame_merge(origin->suspects, toosmall);
-			toosmall = NULL;
-			filter_small(sb, &toosmall, &origin->suspects, sb->copy_score);
-		}
-		if (!origin->suspects)
-			goto finish;
-
-		for (i = 0, sg = first_scapegoat(revs, commit, sb->reverse);
-		     i < num_sg && sg;
-		     sg = sg->next, i++) {
-			struct blame_origin *porigin = sg_origin[i];
-			find_copy_in_parent(sb, &blametail, &toosmall,
-					    origin, sg->item, porigin, opt);
-			if (!origin->suspects)
-				goto finish;
-		}
-	}
-
-finish:
-	*blametail = NULL;
-	distribute_blame(sb, blames);
-	/*
-	 * prepend toosmall to origin->suspects
-	 *
-	 * There is no point in sorting: this ends up on a big
-	 * unsorted list in the caller anyway.
-	 */
-	if (toosmall) {
-		struct blame_entry **tail = &toosmall;
-		while (*tail)
-			tail = &(*tail)->next;
-		*tail = origin->suspects;
-		origin->suspects = toosmall;
-	}
-	for (i = 0; i < num_sg; i++) {
-		if (sg_origin[i]) {
-			drop_origin_blob(sg_origin[i]);
-			blame_origin_decref(sg_origin[i]);
-		}
-	}
-	drop_origin_blob(origin);
-	if (sg_buf != sg_origin)
-		free(sg_origin);
-}
-
-/*
  * Information on commits, used for output.
  */
 struct commit_info {
@@ -1546,74 +319,6 @@ static void found_guilty_entry(struct blame_entry *ent, void *data)
 	display_progress(pi->progress, pi->blamed_lines);
 }
 
-/*
- * The main loop -- while we have blobs with lines whose true origin
- * is still unknown, pick one blob, and allow its lines to pass blames
- * to its parents. */
-static void assign_blame(struct blame_scoreboard *sb, int opt)
-{
-	struct rev_info *revs = sb->revs;
-	struct commit *commit = prio_queue_get(&sb->commits);
-
-	while (commit) {
-		struct blame_entry *ent;
-		struct blame_origin *suspect = commit->util;
-
-		/* find one suspect to break down */
-		while (suspect && !suspect->suspects)
-			suspect = suspect->next;
-
-		if (!suspect) {
-			commit = prio_queue_get(&sb->commits);
-			continue;
-		}
-
-		assert(commit == suspect->commit);
-
-		/*
-		 * We will use this suspect later in the loop,
-		 * so hold onto it in the meantime.
-		 */
-		blame_origin_incref(suspect);
-		parse_commit(commit);
-		if (sb->reverse ||
-		    (!(commit->object.flags & UNINTERESTING) &&
-		     !(revs->max_age != -1 && commit->date < revs->max_age)))
-			pass_blame(sb, suspect, opt);
-		else {
-			commit->object.flags |= UNINTERESTING;
-			if (commit->object.parsed)
-				mark_parents_uninteresting(commit);
-		}
-		/* treat root commit as boundary */
-		if (!commit->parents && !sb->show_root)
-			commit->object.flags |= UNINTERESTING;
-
-		/* Take responsibility for the remaining entries */
-		ent = suspect->suspects;
-		if (ent) {
-			suspect->guilty = 1;
-			for (;;) {
-				struct blame_entry *next = ent->next;
-				if (sb->found_guilty_entry)
-					sb->found_guilty_entry(ent, sb->found_guilty_entry_data);
-				if (next) {
-					ent = next;
-					continue;
-				}
-				ent->next = sb->ent;
-				sb->ent = suspect->suspects;
-				suspect->suspects = NULL;
-				break;
-			}
-		}
-		blame_origin_decref(suspect);
-
-		if (sb->debug) /* sanity */
-			sanity_check_refcnt(sb);
-	}
-}
-
 static const char *format_time(timestamp_t time, const char *tz_str,
 			       int show_raw_time)
 {
@@ -1927,29 +632,6 @@ static void find_alignment(struct blame_scoreboard *sb, int *option)
 		abbrev = auto_abbrev + 1;
 }
 
-/*
- * For debugging -- origin is refcounted, and this asserts that
- * we do not underflow.
- */
-static void sanity_check_refcnt(struct blame_scoreboard *sb)
-{
-	int baa = 0;
-	struct blame_entry *ent;
-
-	for (ent = sb->ent; ent; ent = ent->next) {
-		/* Nobody should have zero or negative refcnt */
-		if (ent->suspect->refcnt <= 0) {
-			fprintf(stderr, "%s in %s has negative refcnt %d\n",
-				ent->suspect->path,
-				oid_to_hex(&ent->suspect->commit->object.oid),
-				ent->suspect->refcnt);
-			baa = 1;
-		}
-	}
-	if (baa)
-		sb->on_sanity_fail(sb, baa);
-}
-
 static void sanity_check_on_fail(struct blame_scoreboard *sb, int baa)
 {
 	int opt = OUTPUT_SHOW_SCORE | OUTPUT_SHOW_NUMBER | OUTPUT_SHOW_NAME;
-- 
2.9.3


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

* [PATCH 28/29] blame: move scoreboard setup to libgit
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (26 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 27/29] blame: move scoreboard-related " Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-25  5:53   ` Junio C Hamano
  2017-05-24  5:15 ` [PATCH 29/29] blame: move entry prepend " Jeff Smith
                   ` (2 subsequent siblings)
  30 siblings, 1 reply; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 blame.c         | 279 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 blame.h         |  10 +-
 builtin/blame.c | 276 -------------------------------------------------------
 3 files changed, 281 insertions(+), 284 deletions(-)

diff --git a/blame.c b/blame.c
index 798e61b..f6c9cb7 100644
--- a/blame.c
+++ b/blame.c
@@ -4,6 +4,7 @@
 #include "mergesort.h"
 #include "diff.h"
 #include "diffcore.h"
+#include "tag.h"
 #include "blame.h"
 
 void blame_origin_decref(struct blame_origin *o)
@@ -49,7 +50,7 @@ static struct blame_origin *make_origin(struct commit *commit, const char *path)
  * Locate an existing origin or create a new one.
  * This moves the origin to front position in the commit util list.
  */
-struct blame_origin *get_origin(struct commit *commit, const char *path)
+static struct blame_origin *get_origin(struct commit *commit, const char *path)
 {
 	struct blame_origin *o, *l;
 
@@ -142,9 +143,9 @@ static void set_commit_buffer_from_strbuf(struct commit *c, struct strbuf *sb)
  * Prepare a dummy commit that represents the work tree (or staged) item.
  * Note that annotating work tree item never works in the reverse.
  */
-struct commit *fake_working_tree_commit(struct diff_options *opt,
-					const char *path,
-					const char *contents_from)
+static struct commit *fake_working_tree_commit(struct diff_options *opt,
+					       const char *path,
+					       const char *contents_from)
 {
 	struct commit *commit;
 	struct blame_origin *origin;
@@ -410,6 +411,13 @@ void blame_sort_final(struct blame_scoreboard *sb)
 				  compare_blame_final);
 }
 
+static int compare_commits_by_reverse_commit_date(const void *a,
+						  const void *b,
+						  void *c)
+{
+	return -compare_commits_by_commit_date(a, b, c);
+}
+
 /*
  * For debugging -- origin is refcounted, and this asserts that
  * we do not underflow.
@@ -483,6 +491,32 @@ static void queue_blames(struct blame_scoreboard *sb, struct blame_origin *porig
 }
 
 /*
+ * Fill the blob_sha1 field of an origin if it hasn't, so that later
+ * call to fill_origin_blob() can use it to locate the data.  blob_sha1
+ * for an origin is also used to pass the blame for the entire file to
+ * the parent to detect the case where a child's blob is identical to
+ * that of its parent's.
+ *
+ * This also fills origin->mode for corresponding tree path.
+ */
+static int fill_blob_sha1_and_mode(struct blame_origin *origin)
+{
+	if (!is_null_oid(&origin->blob_oid))
+		return 0;
+	if (get_tree_entry(origin->commit->object.oid.hash,
+			   origin->path,
+			   origin->blob_oid.hash, &origin->mode))
+		goto error_out;
+	if (sha1_object_info(origin->blob_oid.hash, NULL) != OBJ_BLOB)
+		goto error_out;
+	return 0;
+ error_out:
+	oidclr(&origin->blob_oid);
+	origin->mode = S_IFINVALID;
+	return -1;
+}
+
+/*
  * We have an origin -- check if the same path exists in the
  * parent and return an origin structure to represent it.
  */
@@ -1574,3 +1608,240 @@ void assign_blame(struct blame_scoreboard *sb, int opt)
 			sanity_check_refcnt(sb);
 	}
 }
+
+static const char *get_next_line(const char *start, const char *end)
+{
+	const char *nl = memchr(start, '\n', end - start);
+	return nl ? nl + 1 : end;
+}
+
+/*
+ * To allow quick access to the contents of nth line in the
+ * final image, prepare an index in the scoreboard.
+ */
+static int prepare_lines(struct blame_scoreboard *sb)
+{
+	const char *buf = sb->final_buf;
+	unsigned long len = sb->final_buf_size;
+	const char *end = buf + len;
+	const char *p;
+	int *lineno;
+	int num = 0;
+
+	for (p = buf; p < end; p = get_next_line(p, end))
+		num++;
+
+	ALLOC_ARRAY(sb->lineno, num + 1);
+	lineno = sb->lineno;
+
+	for (p = buf; p < end; p = get_next_line(p, end))
+		*lineno++ = p - buf;
+
+	*lineno = len;
+
+	sb->num_lines = num;
+	return sb->num_lines;
+}
+
+static struct commit *find_single_final(struct rev_info *revs,
+					const char **name_p)
+{
+	int i;
+	struct commit *found = NULL;
+	const char *name = NULL;
+
+	for (i = 0; i < revs->pending.nr; i++) {
+		struct object *obj = revs->pending.objects[i].item;
+		if (obj->flags & UNINTERESTING)
+			continue;
+		obj = deref_tag(obj, NULL, 0);
+		if (obj->type != OBJ_COMMIT)
+			die("Non commit %s?", revs->pending.objects[i].name);
+		if (found)
+			die("More than one commit to dig from %s and %s?",
+			    revs->pending.objects[i].name, name);
+		found = (struct commit *)obj;
+		name = revs->pending.objects[i].name;
+	}
+	if (name_p)
+		*name_p = name;
+	return found;
+}
+
+static struct commit *dwim_reverse_initial(struct rev_info *revs,
+					   const char **name_p)
+{
+	/*
+	 * DWIM "git blame --reverse ONE -- PATH" as
+	 * "git blame --reverse ONE..HEAD -- PATH" but only do so
+	 * when it makes sense.
+	 */
+	struct object *obj;
+	struct commit *head_commit;
+	unsigned char head_sha1[20];
+
+	if (revs->pending.nr != 1)
+		return NULL;
+
+	/* Is that sole rev a committish? */
+	obj = revs->pending.objects[0].item;
+	obj = deref_tag(obj, NULL, 0);
+	if (obj->type != OBJ_COMMIT)
+		return NULL;
+
+	/* Do we have HEAD? */
+	if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_sha1, NULL))
+		return NULL;
+	head_commit = lookup_commit_reference_gently(head_sha1, 1);
+	if (!head_commit)
+		return NULL;
+
+	/* Turn "ONE" into "ONE..HEAD" then */
+	obj->flags |= UNINTERESTING;
+	add_pending_object(revs, &head_commit->object, "HEAD");
+
+	if (name_p)
+		*name_p = revs->pending.objects[0].name;
+	return (struct commit *)obj;
+}
+
+static struct commit *find_single_initial(struct rev_info *revs,
+					  const char **name_p)
+{
+	int i;
+	struct commit *found = NULL;
+	const char *name = NULL;
+
+	/*
+	 * There must be one and only one negative commit, and it must be
+	 * the boundary.
+	 */
+	for (i = 0; i < revs->pending.nr; i++) {
+		struct object *obj = revs->pending.objects[i].item;
+		if (!(obj->flags & UNINTERESTING))
+			continue;
+		obj = deref_tag(obj, NULL, 0);
+		if (obj->type != OBJ_COMMIT)
+			die("Non commit %s?", revs->pending.objects[i].name);
+		if (found)
+			die("More than one commit to dig up from, %s and %s?",
+			    revs->pending.objects[i].name, name);
+		found = (struct commit *) obj;
+		name = revs->pending.objects[i].name;
+	}
+
+	if (!name)
+		found = dwim_reverse_initial(revs, &name);
+	if (!name)
+		die("No commit to dig up from?");
+
+	if (name_p)
+		*name_p = name;
+	return found;
+}
+
+void init_scoreboard(struct blame_scoreboard *sb)
+{
+	memset(sb, 0, sizeof(struct blame_scoreboard));
+	sb->move_score = BLAME_DEFAULT_MOVE_SCORE;
+	sb->copy_score = BLAME_DEFAULT_COPY_SCORE;
+}
+
+void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blame_origin **orig)
+{
+	const char *final_commit_name = NULL;
+	struct blame_origin *o;
+	struct commit *final_commit = NULL;
+	enum object_type type;
+
+	if (sb->reverse && sb->contents_from)
+		die(_("--contents and --reverse do not blend well."));
+
+	if (!sb->reverse) {
+		sb->final = find_single_final(sb->revs, &final_commit_name);
+		sb->commits.compare = compare_commits_by_commit_date;
+	} else {
+		sb->final = find_single_initial(sb->revs, &final_commit_name);
+		sb->commits.compare = compare_commits_by_reverse_commit_date;
+	}
+
+	if (sb->final && sb->contents_from)
+		die(_("cannot use --contents with final commit object name"));
+
+	if (sb->reverse && sb->revs->first_parent_only)
+		sb->revs->children.name = NULL;
+
+	if (!sb->final) {
+		/*
+		 * "--not A B -- path" without anything positive;
+		 * do not default to HEAD, but use the working tree
+		 * or "--contents".
+		 */
+		setup_work_tree();
+		sb->final = fake_working_tree_commit(&sb->revs->diffopt,
+						     path, sb->contents_from);
+		add_pending_object(sb->revs, &(sb->final->object), ":");
+	}
+
+	if (sb->reverse && sb->revs->first_parent_only) {
+		final_commit = find_single_final(sb->revs, NULL);
+		if (!final_commit)
+			die(_("--reverse and --first-parent together require specified latest commit"));
+	}
+
+	/*
+	 * If we have bottom, this will mark the ancestors of the
+	 * bottom commits we would reach while traversing as
+	 * uninteresting.
+	 */
+	if (prepare_revision_walk(sb->revs))
+		die(_("revision walk setup failed"));
+
+	if (sb->reverse && sb->revs->first_parent_only) {
+		struct commit *c = final_commit;
+
+		sb->revs->children.name = "children";
+		while (c->parents &&
+		       oidcmp(&c->object.oid, &sb->final->object.oid)) {
+			struct commit_list *l = xcalloc(1, sizeof(*l));
+
+			l->item = c;
+			if (add_decoration(&sb->revs->children,
+					   &c->parents->item->object, l))
+				die("BUG: not unique item in first-parent chain");
+			c = c->parents->item;
+		}
+
+		if (oidcmp(&c->object.oid, &sb->final->object.oid))
+			die(_("--reverse --first-parent together require range along first-parent chain"));
+	}
+
+	if (is_null_oid(&sb->final->object.oid)) {
+		o = sb->final->util;
+		sb->final_buf = xmemdupz(o->file.ptr, o->file.size);
+		sb->final_buf_size = o->file.size;
+	}
+	else {
+		o = get_origin(sb->final, path);
+		if (fill_blob_sha1_and_mode(o))
+			die(_("no such path %s in %s"), path, final_commit_name);
+
+		if (DIFF_OPT_TST(&sb->revs->diffopt, ALLOW_TEXTCONV) &&
+		    textconv_object(path, o->mode, &o->blob_oid, 1, (char **) &sb->final_buf,
+				    &sb->final_buf_size))
+			;
+		else
+			sb->final_buf = read_sha1_file(o->blob_oid.hash, &type,
+						       &sb->final_buf_size);
+
+		if (!sb->final_buf)
+			die(_("cannot read blob %s for path %s"),
+			    oid_to_hex(&o->blob_oid),
+			    path);
+	}
+	sb->num_read_blob++;
+	prepare_lines(sb);
+
+	if (orig)
+		*orig = o;
+}
diff --git a/blame.h b/blame.h
index df824b2..76fd8ef 100644
--- a/blame.h
+++ b/blame.h
@@ -13,6 +13,9 @@
 #define PICKAXE_BLAME_COPY_HARDER	04
 #define PICKAXE_BLAME_COPY_HARDEST	010
 
+#define BLAME_DEFAULT_MOVE_SCORE	20
+#define BLAME_DEFAULT_COPY_SCORE	40
+
 /*
  * One blob in a commit that is being suspected
  */
@@ -158,14 +161,13 @@ static inline struct blame_origin *blame_origin_incref(struct blame_origin *o)
 }
 extern void blame_origin_decref(struct blame_origin *o);
 
-extern struct blame_origin *get_origin(struct commit *commit, const char *path);
-
-extern struct commit *fake_working_tree_commit(struct diff_options *opt, const char *path, const char *contents_from);
-
 extern void blame_coalesce(struct blame_scoreboard *sb);
 extern void blame_sort_final(struct blame_scoreboard *sb);
 extern unsigned blame_entry_score(struct blame_scoreboard *sb, struct blame_entry *e);
 extern void assign_blame(struct blame_scoreboard *sb, int opt);
 extern const char *blame_nth_line(struct blame_scoreboard *sb, long lno);
 
+extern void init_scoreboard(struct blame_scoreboard *sb);
+extern void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blame_origin **orig);
+
 #endif /* BLAME_H */
diff --git a/builtin/blame.c b/builtin/blame.c
index 461506c..7d9e322 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -6,11 +6,8 @@
  */
 
 #include "cache.h"
-#include "refs.h"
 #include "builtin.h"
 #include "commit.h"
-#include "tag.h"
-#include "tree-walk.h"
 #include "diff.h"
 #include "revision.h"
 #include "quote.h"
@@ -60,8 +57,6 @@ static struct string_list mailmap = STRING_LIST_INIT_NODUP;
 
 static unsigned blame_move_score;
 static unsigned blame_copy_score;
-#define BLAME_DEFAULT_MOVE_SCORE	20
-#define BLAME_DEFAULT_COPY_SCORE	40
 
 /* Remember to update object flag allocation in object.h */
 #define METAINFO_SHOWN		(1u<<12)
@@ -72,39 +67,6 @@ struct progress_info {
 	int blamed_lines;
 };
 
-static int compare_commits_by_reverse_commit_date(const void *a,
-						  const void *b,
-						  void *c)
-{
-	return -compare_commits_by_commit_date(a, b, c);
-}
-
-/*
- * Fill the blob_sha1 field of an origin if it hasn't, so that later
- * call to fill_origin_blob() can use it to locate the data.  blob_sha1
- * for an origin is also used to pass the blame for the entire file to
- * the parent to detect the case where a child's blob is identical to
- * that of its parent's.
- *
- * This also fills origin->mode for corresponding tree path.
- */
-static int fill_blob_sha1_and_mode(struct blame_origin *origin)
-{
-	if (!is_null_oid(&origin->blob_oid))
-		return 0;
-	if (get_tree_entry(origin->commit->object.oid.hash,
-			   origin->path,
-			   origin->blob_oid.hash, &origin->mode))
-		goto error_out;
-	if (sha1_object_info(origin->blob_oid.hash, NULL) != OBJ_BLOB)
-		goto error_out;
-	return 0;
- error_out:
-	oidclr(&origin->blob_oid);
-	origin->mode = S_IFINVALID;
-	return -1;
-}
-
 static const char *nth_line_cb(void *data, long lno)
 {
 	return blame_nth_line((struct blame_scoreboard *)data, lno);
@@ -512,40 +474,6 @@ static void output(struct blame_scoreboard *sb, int option)
 	}
 }
 
-static const char *get_next_line(const char *start, const char *end)
-{
-	const char *nl = memchr(start, '\n', end - start);
-	return nl ? nl + 1 : end;
-}
-
-/*
- * To allow quick access to the contents of nth line in the
- * final image, prepare an index in the scoreboard.
- */
-static int prepare_lines(struct blame_scoreboard *sb)
-{
-	const char *buf = sb->final_buf;
-	unsigned long len = sb->final_buf_size;
-	const char *end = buf + len;
-	const char *p;
-	int *lineno;
-	int num = 0;
-
-	for (p = buf; p < end; p = get_next_line(p, end))
-		num++;
-
-	ALLOC_ARRAY(sb->lineno, num + 1);
-	lineno = sb->lineno;
-
-	for (p = buf; p < end; p = get_next_line(p, end))
-		*lineno++ = p - buf;
-
-	*lineno = len;
-
-	sb->num_lines = num;
-	return sb->num_lines;
-}
-
 /*
  * Add phony grafts for use with -S; this is primarily to
  * support git's cvsserver that wants to give a linear history
@@ -687,104 +615,6 @@ static int git_blame_config(const char *var, const char *value, void *cb)
 	return git_default_config(var, value, cb);
 }
 
-static struct commit *find_single_final(struct rev_info *revs,
-					const char **name_p)
-{
-	int i;
-	struct commit *found = NULL;
-	const char *name = NULL;
-
-	for (i = 0; i < revs->pending.nr; i++) {
-		struct object *obj = revs->pending.objects[i].item;
-		if (obj->flags & UNINTERESTING)
-			continue;
-		obj = deref_tag(obj, NULL, 0);
-		if (obj->type != OBJ_COMMIT)
-			die("Non commit %s?", revs->pending.objects[i].name);
-		if (found)
-			die("More than one commit to dig from %s and %s?",
-			    revs->pending.objects[i].name, name);
-		found = (struct commit *)obj;
-		name = revs->pending.objects[i].name;
-	}
-	if (name_p)
-		*name_p = name;
-	return found;
-}
-
-static struct commit *dwim_reverse_initial(struct rev_info *revs,
-					   const char **name_p)
-{
-	/*
-	 * DWIM "git blame --reverse ONE -- PATH" as
-	 * "git blame --reverse ONE..HEAD -- PATH" but only do so
-	 * when it makes sense.
-	 */
-	struct object *obj;
-	struct commit *head_commit;
-	unsigned char head_sha1[20];
-
-	if (revs->pending.nr != 1)
-		return NULL;
-
-	/* Is that sole rev a committish? */
-	obj = revs->pending.objects[0].item;
-	obj = deref_tag(obj, NULL, 0);
-	if (obj->type != OBJ_COMMIT)
-		return NULL;
-
-	/* Do we have HEAD? */
-	if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, head_sha1, NULL))
-		return NULL;
-	head_commit = lookup_commit_reference_gently(head_sha1, 1);
-	if (!head_commit)
-		return NULL;
-
-	/* Turn "ONE" into "ONE..HEAD" then */
-	obj->flags |= UNINTERESTING;
-	add_pending_object(revs, &head_commit->object, "HEAD");
-
-	if (name_p)
-		*name_p = revs->pending.objects[0].name;
-	return (struct commit *)obj;
-}
-
-static struct commit *find_single_initial(struct rev_info *revs,
-					  const char **name_p)
-{
-	int i;
-	const char *final_commit_name = NULL;
-	struct commit *found = NULL;
-
-	/*
-	 * There must be one and only one negative commit, and it must be
-	 * the boundary.
-	 */
-	for (i = 0; i < revs->pending.nr; i++) {
-		struct object *obj = revs->pending.objects[i].item;
-		if (!(obj->flags & UNINTERESTING))
-			continue;
-		obj = deref_tag(obj, NULL, 0);
-		if (obj->type != OBJ_COMMIT)
-			die("Non commit %s?", revs->pending.objects[i].name);
-		if (found)
-			die("More than one commit to dig up from, %s and %s?",
-			    revs->pending.objects[i].name,
-			    final_commit_name);
-		found = (struct commit *) obj;
-		final_commit_name = revs->pending.objects[i].name;
-	}
-
-	if (!final_commit_name)
-		found = dwim_reverse_initial(revs, &final_commit_name);
-	if (!final_commit_name)
-		die("No commit to dig up from?");
-
-	if (name_p)
-		*name_p = final_commit_name;
-	return found;
-}
-
 static int blame_copy_callback(const struct option *option, const char *arg, int unset)
 {
 	int *opt = option->value;
@@ -818,112 +648,6 @@ static int blame_move_callback(const struct option *option, const char *arg, int
 	return 0;
 }
 
-void init_scoreboard(struct blame_scoreboard *sb)
-{
-	memset(sb, 0, sizeof(struct blame_scoreboard));
-	sb->move_score = BLAME_DEFAULT_MOVE_SCORE;
-	sb->copy_score = BLAME_DEFAULT_COPY_SCORE;
-}
-
-void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blame_origin **orig)
-{
-	const char *final_commit_name = NULL;
-	struct blame_origin *o;
-	struct commit *final_commit = NULL;
-	enum object_type type;
-
-	if (sb->reverse && sb->contents_from)
-		die(_("--contents and --reverse do not blend well."));
-
-	if (!sb->reverse) {
-		sb->final = find_single_final(sb->revs, &final_commit_name);
-		sb->commits.compare = compare_commits_by_commit_date;
-	} else {
-		sb->final = find_single_initial(sb->revs, &final_commit_name);
-		sb->commits.compare = compare_commits_by_reverse_commit_date;
-	}
-
-	if (sb->final && sb->contents_from)
-		die(_("cannot use --contents with final commit object name"));
-
-	if (sb->reverse && sb->revs->first_parent_only)
-		sb->revs->children.name = NULL;
-
-	if (!sb->final) {
-		/*
-		 * "--not A B -- path" without anything positive;
-		 * do not default to HEAD, but use the working tree
-		 * or "--contents".
-		 */
-		setup_work_tree();
-		sb->final = fake_working_tree_commit(&sb->revs->diffopt,
-						     path, sb->contents_from);
-		add_pending_object(sb->revs, &(sb->final->object), ":");
-	}
-
-	if (sb->reverse && sb->revs->first_parent_only) {
-		final_commit = find_single_final(sb->revs, NULL);
-		if (!final_commit)
-			die(_("--reverse and --first-parent together require specified latest commit"));
-	}
-
-	/*
-	 * If we have bottom, this will mark the ancestors of the
-	 * bottom commits we would reach while traversing as
-	 * uninteresting.
-	 */
-	if (prepare_revision_walk(sb->revs))
-		die(_("revision walk setup failed"));
-
-	if (sb->reverse && sb->revs->first_parent_only) {
-		struct commit *c = final_commit;
-
-		sb->revs->children.name = "children";
-		while (c->parents &&
-		       oidcmp(&c->object.oid, &sb->final->object.oid)) {
-			struct commit_list *l = xcalloc(1, sizeof(*l));
-
-			l->item = c;
-			if (add_decoration(&sb->revs->children,
-					   &c->parents->item->object, l))
-				die("BUG: not unique item in first-parent chain");
-			c = c->parents->item;
-		}
-
-		if (oidcmp(&c->object.oid, &sb->final->object.oid))
-			die(_("--reverse --first-parent together require range along first-parent chain"));
-	}
-
-	if (is_null_oid(&sb->final->object.oid)) {
-		o = sb->final->util;
-		sb->final_buf = xmemdupz(o->file.ptr, o->file.size);
-		sb->final_buf_size = o->file.size;
-	}
-	else {
-		o = get_origin(sb->final, path);
-		if (fill_blob_sha1_and_mode(o))
-			die(_("no such path %s in %s"), path, final_commit_name);
-
-		if (DIFF_OPT_TST(&sb->revs->diffopt, ALLOW_TEXTCONV) &&
-		    textconv_object(path, o->mode, &o->blob_oid, 1, (char **) &sb->final_buf,
-				    &sb->final_buf_size))
-			;
-		else
-			sb->final_buf = read_sha1_file(o->blob_oid.hash, &type,
-						       &sb->final_buf_size);
-
-		if (!sb->final_buf)
-			die(_("cannot read blob %s for path %s"),
-			    oid_to_hex(&o->blob_oid),
-			    path);
-	}
-	sb->num_read_blob++;
-	prepare_lines(sb);
-
-	if (orig)
-		*orig = o;
-}
-
 struct blame_entry *blame_entry_prepend(struct blame_entry *head,
 					long start, long end,
 					struct blame_origin *o)
-- 
2.9.3


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

* [PATCH 29/29] blame: move entry prepend to libgit
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (27 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 28/29] blame: move scoreboard setup " Jeff Smith
@ 2017-05-24  5:15 ` Jeff Smith
  2017-05-24  7:08 ` [PATCH 00/29] Add blame " Junio C Hamano
  2017-05-25  5:55 ` Junio C Hamano
  30 siblings, 0 replies; 39+ messages in thread
From: Jeff Smith @ 2017-05-24  5:15 UTC (permalink / raw)
  To: git; +Cc: gitster, peff, Jeff Smith

Signed-off-by: Jeff Smith <whydoubt@gmail.com>
---
 blame.c         | 16 ++++++++++++++++
 blame.h         |  2 ++
 builtin/blame.c | 14 --------------
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/blame.c b/blame.c
index f6c9cb7..00404b9 100644
--- a/blame.c
+++ b/blame.c
@@ -1845,3 +1845,19 @@ void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blam
 	if (orig)
 		*orig = o;
 }
+
+
+
+struct blame_entry *blame_entry_prepend(struct blame_entry *head,
+					long start, long end,
+					struct blame_origin *o)
+{
+	struct blame_entry *new_head = xcalloc(1, sizeof(struct blame_entry));
+	new_head->lno = start;
+	new_head->num_lines = end - start;
+	new_head->suspect = o;
+	new_head->s_lno = start;
+	new_head->next = head;
+	blame_origin_incref(o);
+	return new_head;
+}
diff --git a/blame.h b/blame.h
index 76fd8ef..a6c915c 100644
--- a/blame.h
+++ b/blame.h
@@ -170,4 +170,6 @@ extern const char *blame_nth_line(struct blame_scoreboard *sb, long lno);
 extern void init_scoreboard(struct blame_scoreboard *sb);
 extern void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blame_origin **orig);
 
+extern struct blame_entry *blame_entry_prepend(struct blame_entry *head, long start, long end, struct blame_origin *o);
+
 #endif /* BLAME_H */
diff --git a/builtin/blame.c b/builtin/blame.c
index 7d9e322..08f35bd 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -648,20 +648,6 @@ static int blame_move_callback(const struct option *option, const char *arg, int
 	return 0;
 }
 
-struct blame_entry *blame_entry_prepend(struct blame_entry *head,
-					long start, long end,
-					struct blame_origin *o)
-{
-	struct blame_entry *new_head = xcalloc(1, sizeof(struct blame_entry));
-	new_head->lno = start;
-	new_head->num_lines = end - start;
-	new_head->suspect = o;
-	new_head->s_lno = start;
-	new_head->next = head;
-	blame_origin_incref(o);
-	return new_head;
-}
-
 int cmd_blame(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
-- 
2.9.3


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

* Re: [PATCH 00/29] Add blame to libgit
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (28 preceding siblings ...)
  2017-05-24  5:15 ` [PATCH 29/29] blame: move entry prepend " Jeff Smith
@ 2017-05-24  7:08 ` Junio C Hamano
  2017-05-25  5:55 ` Junio C Hamano
  30 siblings, 0 replies; 39+ messages in thread
From: Junio C Hamano @ 2017-05-24  7:08 UTC (permalink / raw)
  To: Jeff Smith; +Cc: git, peff

I haven't had a chance to take a deeper look, but what I saw in
merge conflicts with the rest of the system made all sense to me ;-)
Will take a deeper look tomorrow.

Thanks.

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

* Re: [PATCH 18/29] blame: move progess updates to a scoreboard callback
  2017-05-24  5:15 ` [PATCH 18/29] blame: move progess updates to a scoreboard callback Jeff Smith
@ 2017-05-25  4:16   ` Junio C Hamano
  0 siblings, 0 replies; 39+ messages in thread
From: Junio C Hamano @ 2017-05-25  4:16 UTC (permalink / raw)
  To: Jeff Smith; +Cc: git, peff

Jeff Smith <whydoubt@gmail.com> writes:

> Subject: Re: [PATCH 18/29] blame: move progess updates to a scoreboard callback

s/progess/progress/ (I can do this on my end).

> Allow the interface user to decide how to handle a progress update.
>
> Signed-off-by: Jeff Smith <whydoubt@gmail.com>
> ---
>  builtin/blame.c | 27 +++++++++++++++++----------
>  1 file changed, 17 insertions(+), 10 deletions(-)
>
> -static void found_guilty_entry(struct blame_entry *ent,
> -			   struct progress_info *pi)
> +static void found_guilty_entry(struct blame_entry *ent, void *data)
>  {
> +	struct progress_info *pi = (struct progress_info *)data;
> +
>  	if (incremental) {
>  		struct blame_origin *suspect = ent->suspect;
>  

This hunk is interesting.  The function not only does the "progress"
eye candy, but it actually handles the "--incremental" option.
Anybody who wants to do something similar using the libified blame
needs to implement it in their found_guilty callback like this one
does, which is probably a good division of labor between the blame
lib and the front-end (which builtin/blame.c is one instance of).

> @@ -1754,11 +1758,6 @@ static void assign_blame(struct blame_scoreboard *sb, int opt)
>  {
>  	struct rev_info *revs = sb->revs;
>  	struct commit *commit = prio_queue_get(&sb->commits);
> -	struct progress_info pi = { NULL, 0 };
> -
> -	if (show_progress)
> -		pi.progress = start_progress_delay(_("Blaming lines"),
> -						   sb->num_lines, 50, 1);

And this piece (and matching "stop progress" at the end of the
function) is now the responsibility of the caller, which makes
sense.

>  
> +	sb.found_guilty_entry = &found_guilty_entry;
> +	sb.found_guilty_entry_data = &pi;
> +	if (show_progress)
> +		pi.progress = start_progress_delay(_("Blaming lines"),
> +						   sb.num_lines, 50, 1);
> +
>  	assign_blame(&sb, opt);
>  
> +	stop_progress(&pi.progress);
> +
>  	if (!incremental)
>  		setup_pager();


Very nicely done so far.

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

* Re: [PATCH 20/29] blame: rework methods that determine 'final' commit
  2017-05-24  5:15 ` [PATCH 20/29] blame: rework methods that determine 'final' commit Jeff Smith
@ 2017-05-25  4:59   ` Junio C Hamano
  0 siblings, 0 replies; 39+ messages in thread
From: Junio C Hamano @ 2017-05-25  4:59 UTC (permalink / raw)
  To: Jeff Smith; +Cc: git, peff

Jeff Smith <whydoubt@gmail.com> writes:

> Either prepare_initial or prepare_final is used to determine which
> commit is marked as 'final'. Call the underlying methods directly to
> make this more clear.
>
> Signed-off-by: Jeff Smith <whydoubt@gmail.com>
> ---
>  builtin/blame.c | 49 +++++++++++++++++++++++--------------------------
>  1 file changed, 23 insertions(+), 26 deletions(-)

I do not necessarily agree with the log message in that this change
makes the contrast between prepare_{initial,final} more clear, but I
like it nevertheless because it makes the interface into the helpers
that prepare "initial" and "final" very similar.

The lifetime rule of final_commit_name used to be that these helpers
make a copy to be owned by the caller, and it seems that this change
makes all codepaths borrow it from entries in the revs.pending list.
I think that conversion is also done correctly in this patch.

So far, nicely done.

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

* Re: [PATCH 22/29] blame: create scoreboard setup function
  2017-05-24  5:15 ` [PATCH 22/29] blame: create scoreboard setup function Jeff Smith
@ 2017-05-25  5:15   ` Junio C Hamano
  0 siblings, 0 replies; 39+ messages in thread
From: Junio C Hamano @ 2017-05-25  5:15 UTC (permalink / raw)
  To: Jeff Smith; +Cc: git, peff

Jeff Smith <whydoubt@gmail.com> writes:

> Create function that completes setting up blame_scoreboard structure.
>
> Signed-off-by: Jeff Smith <whydoubt@gmail.com>
> ---
>  builtin/blame.c | 190 ++++++++++++++++++++++++++++++--------------------------
>  1 file changed, 101 insertions(+), 89 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index f839571..fd41551 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> ...
> @@ -2759,92 +2855,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
> ...
> -	sb.num_read_blob++;
> -	lno = prepare_lines(&sb);
> +	setup_scoreboard(&sb, path, &o);
> +	lno = sb.num_lines;
>  
>  	if (lno && !range_list.nr)
>  		string_list_append(&range_list, "1");

After this change, nobody uses the value returned from
prepare_ilnes(), but the function is still returning an "int".
Perhaps change it to return nothing?

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

* Re: [PATCH 23/29] blame: create entry prepend function
  2017-05-24  5:15 ` [PATCH 23/29] blame: create entry prepend function Jeff Smith
@ 2017-05-25  5:21   ` Junio C Hamano
  0 siblings, 0 replies; 39+ messages in thread
From: Junio C Hamano @ 2017-05-25  5:21 UTC (permalink / raw)
  To: Jeff Smith; +Cc: git, peff

Jeff Smith <whydoubt@gmail.com> writes:

> Create function that populates a blame_entry and prepends it to a list.
>
> Signed-off-by: Jeff Smith <whydoubt@gmail.com>
> ---
>  builtin/blame.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index fd41551..29771b7 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2643,6 +2643,20 @@ void setup_scoreboard(struct blame_scoreboard *sb, const char *path, struct blam
>  		*orig = o;
>  }
>  
> +struct blame_entry *blame_entry_prepend(struct blame_entry *head,
> +					long start, long end,
> +					struct blame_origin *o)
> +{
> +	struct blame_entry *new_head = xcalloc(1, sizeof(struct blame_entry));

We have a slight tendency to favour sizeof(*pointer_to_thing) over
sizeof(type_of_thing), which is why the original is written that
way.  The tendency is so slight that if this were a new code, I do
not think we mind it written either way, but a patch whose goal is
to move existing code around does not have a justification to change
between the two.

> +	new_head->lno = start;
> +	new_head->num_lines = end - start;
> +	new_head->suspect = o;
> +	new_head->s_lno = start;
> +	new_head->next = head;

On the other hand, naming the variables that receive start/end
anything but start/end was a stupidity in the original code (I can
badmouth the original because it is all my code ;-).  Thanks for
fixing the sloppy naming.

> +	blame_origin_incref(o);
> +	return new_head;
> +}
> +
>  int cmd_blame(int argc, const char **argv, const char *prefix)
>  {
>  	struct rev_info revs;
> @@ -2885,16 +2899,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
>  
>  	for (range_i = ranges.nr; range_i > 0; --range_i) {
>  		const struct range *r = &ranges.ranges[range_i - 1];
> -		long bottom = r->start;
> -		long top = r->end;
> -		struct blame_entry *next = ent;
> -		ent = xcalloc(1, sizeof(*ent));
> -		ent->lno = bottom;
> -		ent->num_lines = top - bottom;
> -		ent->suspect = o;
> -		ent->s_lno = bottom;
> -		ent->next = next;
> -		blame_origin_incref(o);
> +		ent = blame_entry_prepend(ent, r->start, r->end, o);
>  	}
>  
>  	o->suspects = ent;

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

* Re: [PATCH 24/29] blame: move core structures to header
  2017-05-24  5:15 ` [PATCH 24/29] blame: move core structures to header Jeff Smith
@ 2017-05-25  5:25   ` Junio C Hamano
  0 siblings, 0 replies; 39+ messages in thread
From: Junio C Hamano @ 2017-05-25  5:25 UTC (permalink / raw)
  To: Jeff Smith; +Cc: git, peff

Jeff Smith <whydoubt@gmail.com> writes:

> The origin, entry, and scoreboard structures are core to the blame
> interface and need to be exposed for blame functionality.
>
> Signed-off-by: Jeff Smith <whydoubt@gmail.com>
> ---

Looks good.  

Thanks to "prepare everything in place before bulk movement"
approach this round takes, unlike the previous one, reviewing this
is just the matter of running

	$ git blame -C -b HEAD^..HEAD -- blame.h

after applying this patch.  Anything that is shown as "introduced"
by HEAD needs careful examination; everything else we already know
are good because we know it came from existing parts.

Thanks.

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

* Re: [PATCH 28/29] blame: move scoreboard setup to libgit
  2017-05-24  5:15 ` [PATCH 28/29] blame: move scoreboard setup " Jeff Smith
@ 2017-05-25  5:53   ` Junio C Hamano
  2017-05-25 12:56     ` Jeffrey Smith
  0 siblings, 1 reply; 39+ messages in thread
From: Junio C Hamano @ 2017-05-25  5:53 UTC (permalink / raw)
  To: Jeff Smith; +Cc: git, peff

Jeff Smith <whydoubt@gmail.com> writes:

> Signed-off-by: Jeff Smith <whydoubt@gmail.com>
> ---
>  blame.c         | 279 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  blame.h         |  10 +-
>  builtin/blame.c | 276 -------------------------------------------------------
>  3 files changed, 281 insertions(+), 284 deletions(-)
>
> ...
> +static struct commit *find_single_initial(struct rev_info *revs,
> +					  const char **name_p)
> +{
> +	int i;
> +	struct commit *found = NULL;
> +	const char *name = NULL;
> +
> +	/*
> +	 * There must be one and only one negative commit, and it must be
> +	 * the boundary.
> +	 */
> +	for (i = 0; i < revs->pending.nr; i++) {
> +		struct object *obj = revs->pending.objects[i].item;
> +		if (!(obj->flags & UNINTERESTING))
> +			continue;
> +		obj = deref_tag(obj, NULL, 0);
> +		if (obj->type != OBJ_COMMIT)
> +			die("Non commit %s?", revs->pending.objects[i].name);
> +		if (found)
> +			die("More than one commit to dig up from, %s and %s?",
> +			    revs->pending.objects[i].name, name);
> +		found = (struct commit *) obj;
> +		name = revs->pending.objects[i].name;
> +	}
> +
> +	if (!name)
> +		found = dwim_reverse_initial(revs, &name);
> +	if (!name)
> +		die("No commit to dig up from?");
> +
> +	if (name_p)
> +		*name_p = name;
> +	return found;
> +}
> +...
> -static struct commit *find_single_initial(struct rev_info *revs,
> -					  const char **name_p)
> -{
> -	int i;
> -	const char *final_commit_name = NULL;
> -	struct commit *found = NULL;
> -
> -...
> -
> -	if (!final_commit_name)
> -		found = dwim_reverse_initial(revs, &final_commit_name);
> -	if (!final_commit_name)
> -		die("No commit to dig up from?");
> -
> -	if (name_p)
> -		*name_p = final_commit_name;
> -	return found;
> -}


In a patch whose primary purpose is to move code between files,
making what used to be public to static and vice versa is an
integral part of moving code.  That is why we want to see a patch
organized in such a way that comparing the lines that are lost from
builtin/blame.c and the lines that are added to blame.[ch] is made
easy.

And from that point of view, it was somewhat irritating to find this
kind of meaningless change.  If you didn't like the name of the
variable "final-commit-name", that shold have been renamed while the
code was still in builtin/blame.c

The end result looks OK anyway (I've checked 29/29 as well).

Thanks.



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

* Re: [PATCH 00/29] Add blame to libgit
  2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
                   ` (29 preceding siblings ...)
  2017-05-24  7:08 ` [PATCH 00/29] Add blame " Junio C Hamano
@ 2017-05-25  5:55 ` Junio C Hamano
  30 siblings, 0 replies; 39+ messages in thread
From: Junio C Hamano @ 2017-05-25  5:55 UTC (permalink / raw)
  To: Jeff Smith; +Cc: git, peff

Jeff Smith <whydoubt@gmail.com> writes:

> Rather than duplicate large portions of builtin/blame.c in cgit, it
> would be better to shift its core functionality into libgit.a.  The
> functionality left in builtin/blame.c mostly relates to terminal
> presentation.

This was a lot more pleasant to review compared to the last round.
I made a few small comments here and there, but I do not think there
was anything major that needs to be corrected by another round.

Thanks.

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

* Re: [PATCH 28/29] blame: move scoreboard setup to libgit
  2017-05-25  5:53   ` Junio C Hamano
@ 2017-05-25 12:56     ` Jeffrey Smith
  0 siblings, 0 replies; 39+ messages in thread
From: Jeffrey Smith @ 2017-05-25 12:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Jeff King

I had meant to change the name to match what is in find_single_final.
While the intent was for it to change while in builtin/blame.c,
apparently I missed that in the shuffle.

On Thu, May 25, 2017 at 12:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff Smith <whydoubt@gmail.com> writes:
>
>> Signed-off-by: Jeff Smith <whydoubt@gmail.com>
>> ---
>>  blame.c         | 279 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  blame.h         |  10 +-
>>  builtin/blame.c | 276 -------------------------------------------------------
>>  3 files changed, 281 insertions(+), 284 deletions(-)
>>
>> ...
>> +static struct commit *find_single_initial(struct rev_info *revs,
>> +                                       const char **name_p)
>> +{
>> +     int i;
>> +     struct commit *found = NULL;
>> +     const char *name = NULL;
>> +
>> +     /*
>> +      * There must be one and only one negative commit, and it must be
>> +      * the boundary.
>> +      */
>> +     for (i = 0; i < revs->pending.nr; i++) {
>> +             struct object *obj = revs->pending.objects[i].item;
>> +             if (!(obj->flags & UNINTERESTING))
>> +                     continue;
>> +             obj = deref_tag(obj, NULL, 0);
>> +             if (obj->type != OBJ_COMMIT)
>> +                     die("Non commit %s?", revs->pending.objects[i].name);
>> +             if (found)
>> +                     die("More than one commit to dig up from, %s and %s?",
>> +                         revs->pending.objects[i].name, name);
>> +             found = (struct commit *) obj;
>> +             name = revs->pending.objects[i].name;
>> +     }
>> +
>> +     if (!name)
>> +             found = dwim_reverse_initial(revs, &name);
>> +     if (!name)
>> +             die("No commit to dig up from?");
>> +
>> +     if (name_p)
>> +             *name_p = name;
>> +     return found;
>> +}
>> +...
>> -static struct commit *find_single_initial(struct rev_info *revs,
>> -                                       const char **name_p)
>> -{
>> -     int i;
>> -     const char *final_commit_name = NULL;
>> -     struct commit *found = NULL;
>> -
>> -...
>> -
>> -     if (!final_commit_name)
>> -             found = dwim_reverse_initial(revs, &final_commit_name);
>> -     if (!final_commit_name)
>> -             die("No commit to dig up from?");
>> -
>> -     if (name_p)
>> -             *name_p = final_commit_name;
>> -     return found;
>> -}
>
>
> In a patch whose primary purpose is to move code between files,
> making what used to be public to static and vice versa is an
> integral part of moving code.  That is why we want to see a patch
> organized in such a way that comparing the lines that are lost from
> builtin/blame.c and the lines that are added to blame.[ch] is made
> easy.
>
> And from that point of view, it was somewhat irritating to find this
> kind of meaningless change.  If you didn't like the name of the
> variable "final-commit-name", that shold have been renamed while the
> code was still in builtin/blame.c
>
> The end result looks OK anyway (I've checked 29/29 as well).
>
> Thanks.
>
>

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

end of thread, other threads:[~2017-05-25 12:57 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-24  5:15 [PATCH 00/29] Add blame to libgit Jeff Smith
2017-05-24  5:15 ` [PATCH 01/29] blame: remove unneeded dependency on blob.h Jeff Smith
2017-05-24  5:15 ` [PATCH 02/29] blame: move textconv_object with related functions Jeff Smith
2017-05-24  5:15 ` [PATCH 03/29] blame: remove unused parameters Jeff Smith
2017-05-24  5:15 ` [PATCH 04/29] blame: rename origin structure to blame_origin Jeff Smith
2017-05-24  5:15 ` [PATCH 05/29] blame: rename scoreboard structure to blame_scoreboard Jeff Smith
2017-05-24  5:15 ` [PATCH 06/29] blame: rename origin-related functions Jeff Smith
2017-05-24  5:15 ` [PATCH 07/29] blame: rename coalesce function Jeff Smith
2017-05-24  5:15 ` [PATCH 08/29] blame: rename ent_score function Jeff Smith
2017-05-24  5:15 ` [PATCH 09/29] blame: rename nth_line function Jeff Smith
2017-05-24  5:15 ` [PATCH 10/29] blame: move stat counters to scoreboard Jeff Smith
2017-05-24  5:15 ` [PATCH 11/29] blame: move copy/move thresholds " Jeff Smith
2017-05-24  5:15 ` [PATCH 12/29] blame: move contents_from " Jeff Smith
2017-05-24  5:15 ` [PATCH 13/29] blame: move reverse flag " Jeff Smith
2017-05-24  5:15 ` [PATCH 14/29] blame: move show_root " Jeff Smith
2017-05-24  5:15 ` [PATCH 15/29] blame: move xdl_opts flags " Jeff Smith
2017-05-24  5:15 ` [PATCH 16/29] blame: move no_whole_file_rename flag " Jeff Smith
2017-05-24  5:15 ` [PATCH 17/29] blame: make sanity_check use a callback in scoreboard Jeff Smith
2017-05-24  5:15 ` [PATCH 18/29] blame: move progess updates to a scoreboard callback Jeff Smith
2017-05-25  4:16   ` Junio C Hamano
2017-05-24  5:15 ` [PATCH 19/29] blame: wrap blame_sort and compare_blame_final Jeff Smith
2017-05-24  5:15 ` [PATCH 20/29] blame: rework methods that determine 'final' commit Jeff Smith
2017-05-25  4:59   ` Junio C Hamano
2017-05-24  5:15 ` [PATCH 21/29] blame: create scoreboard init function Jeff Smith
2017-05-24  5:15 ` [PATCH 22/29] blame: create scoreboard setup function Jeff Smith
2017-05-25  5:15   ` Junio C Hamano
2017-05-24  5:15 ` [PATCH 23/29] blame: create entry prepend function Jeff Smith
2017-05-25  5:21   ` Junio C Hamano
2017-05-24  5:15 ` [PATCH 24/29] blame: move core structures to header Jeff Smith
2017-05-25  5:25   ` Junio C Hamano
2017-05-24  5:15 ` [PATCH 25/29] blame: move origin-related methods to libgit Jeff Smith
2017-05-24  5:15 ` [PATCH 26/29] blame: move fake-commit-related " Jeff Smith
2017-05-24  5:15 ` [PATCH 27/29] blame: move scoreboard-related " Jeff Smith
2017-05-24  5:15 ` [PATCH 28/29] blame: move scoreboard setup " Jeff Smith
2017-05-25  5:53   ` Junio C Hamano
2017-05-25 12:56     ` Jeffrey Smith
2017-05-24  5:15 ` [PATCH 29/29] blame: move entry prepend " Jeff Smith
2017-05-24  7:08 ` [PATCH 00/29] Add blame " Junio C Hamano
2017-05-25  5:55 ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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