Re: [PATCH] git-pickaxe: blame rewritten.

From: Junio C Hamano <junkio@cox.net>
Date: Oct 13, 2006, 20:31 UTC
Message-ID: <7vd58wc5zs.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <7viripnyfh.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

Show 30 quoted lines

>> (I'd actually also like to have a range-modifier, so that I could do
>>
>> 	git annotate --since=2.weeks.ago v2.6.18.. <path>
>>
>> that didn't go back beyond a certain point,...
>
> I am not sure about revision bottom (v.2.6.18..) offhand, but
> the age limit (--since=2.weeks) should be trivial.
>
> Inside pass_blame() while we iterate over the parents of the
> suspect we are looking at, you can skip the parent if it is
> older than the age limit, or an ancestor of revision bottom,
> like this:
>
> --- l/builtin-pickaxe.c
> +++ k/builtin-pickaxe.c
> @@ -450,6 +450,12 @@ static void pass_blame(struct scoreboard
>  	     parent = parent->next, parent_ix++) {
>  		if (parse_commit(parent->item))
>  			continue;
> +
> +		if (parent is older than age limit)
> +			continue;
> +		if (parent is an ancestor of revision bottom)
> +			continue;
> +
>  		porigin = find_origin(sb, parent->item, origin->path);
>  		if (!porigin)
>  			porigin = find_rename(sb, parent->item, origin);
>

This is not quite right. The above code tries to avoid passing blames to the boundary revisions, so "v2.6.18.." makes every old line to be blamed on one revision after v2.6.18, which might be technically correct but not what we want.

Instead, we should pass blame, and then prevent boundary revisions to pass blame further down. The code in "pu" today has seen slight restructure of this part and this change should be easier to implement there, something along the lines of...

--- l/builtin-pickaxe.c +++ k/builtin-pickaxe.c

@@ -450,8 +450,7 @@ static int pass_blame_to_parent(struct s
 }
 
 #define MAXPARENT 16
-static void pass_blame(struct scoreboard *sb, struct origin *origin,
-		       struct rev_info *revs)
+static void pass_blame(struct scoreboard *sb, struct origin *origin)
 {
 	int i, parent_ix;
 	struct commit *commit = origin->commit;
@@ -469,10 +468,6 @@ static void pass_blame(struct scoreboard
 
 		if (parse_commit(p))
 			continue;
-		if (p->object.flags & UNINTERESTING)
-			continue;
-		if (revs->max_age != -1 && p->date  < revs->max_age)
-			continue;
 
 		porigin = find_origin(sb, parent->item, origin->path);
 		if (!porigin)
@@ -516,6 +511,7 @@ static void assign_blame(struct scoreboa
 	while (1) {
 		int i;
 		struct origin *suspect = NULL;
+		struct commit *commit;
 
 		/* find one suspect to break down */
 		for (i = 0; !suspect && i < sb->num_entries; i++) {
@@ -525,7 +521,10 @@ static void assign_blame(struct scoreboa
 		if (!suspect)
 			return; /* all done */
 
-		pass_blame(sb, suspect, revs);
+		commit = suspect->commit;
+		if (!(commit->object.flags & UNINTERESTING) &&
+		    !(revs->max_age != -1 && commit->date  < revs->max_age))
+			pass_blame(sb, suspect, revs);
 
 		/* Take responsibility for the remaining entries */
 		for (i = 0; i < sb->num_entries; i++)