On Thu, Oct 22, 2015 at 12:11:23PM -0700, Junio C Hamano wrote:
> Max Kirillov <max@xxxxxxxxxx> writes:
>> On Wed, Oct 21, 2015 at 09:25:37PM -0700, Junio C Hamano wrote:
>>
>>> The logic you implemented feels solid to me, at least at a first
>>> glance. What kind of gotchas are you worried about?
>>
>> The fact than arbitrary commit's children are unknown does
>> not seem reliable to me. It more fits the "works by chance"
>> description.
>
> That's sad.
How about this then? I could collect only children
along the first-parent chain. This makes sure that
no extra child entry appear in revs->children. Since the revs
structure is exclusive for cmd_blame() it is guaranteed
that no other command will affect or be affected by this
behavior.
This additionally forbids having several end commits
(git blame --reverse --first-parent ^A B C ...), but
this does not seem to have much practical use.
--
Max
8< ------------------
diff --git a/builtin/blame.c b/builtin/blame.c
index eb348f0..f66d0ae 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2401,16 +2401,11 @@ static struct commit *fake_working_tree_commit(struct diff_options *opt,
return commit;
}
-static char *prepare_final(struct scoreboard *sb)
+static struct object_array_entry *find_single_final(struct rev_info *revs)
{
int i;
- const char *final_commit_name = NULL;
- struct rev_info *revs = sb->revs;
+ struct object_array_entry *found = NULL;
- /*
- * There must be one and only one positive commit in the
- * revs->pending array.
- */
for (i = 0; i < revs->pending.nr; i++) {
struct object *obj = revs->pending.objects[i].item;
if (obj->flags & UNINTERESTING)
@@ -2419,14 +2414,24 @@ static char *prepare_final(struct 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 from %s and %s?",
revs->pending.objects[i].name,
- final_commit_name);
- sb->final = (struct commit *) obj;
- final_commit_name = revs->pending.objects[i].name;
+ found->name);
+ found = &(revs->pending.objects[i]);
+ }
+ return found;
+}
+
+static char *prepare_final(struct scoreboard *sb)
+{
+ struct object_array_entry *found = find_single_final(sb->revs);
+ if (found) {
+ sb->final = (struct commit *) found->item;
+ return xstrdup(found->name);
+ } else {
+ return NULL;
}
- return xstrdup_or_null(final_commit_name);
}
static char *prepare_initial(struct scoreboard *sb)
@@ -2502,6 +2507,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
long dashdash_pos, lno;
char *final_commit_name = NULL;
enum object_type type;
+ struct commit *firstparent_chain_initial = NULL;
static struct string_list range_list;
static int output_option = 0, opt = 0;
@@ -2695,6 +2701,8 @@ parse_done:
else {
final_commit_name = prepare_initial(&sb);
sb.commits.compare = compare_commits_by_reverse_commit_date;
+ if (revs.first_parent_only)
+ revs.children.name = NULL;
}
if (!sb.final) {
@@ -2711,6 +2719,14 @@ parse_done:
else if (contents_from)
die("Cannot use --contents with final commit object name");
+ if (reverse && revs.first_parent_only) {
+ struct object_array_entry *entry = find_single_final(sb.revs);
+ if (!entry)
+ die("--reverse and --first-parent together require specified latest commit");
+ else
+ firstparent_chain_initial = (struct commit*) entry->item;
+ }
+
/*
* If we have bottom, this will mark the ancestors of the
* bottom commits we would reach while traversing as
@@ -2720,11 +2736,21 @@ parse_done:
die(_("revision walk setup failed"));
if (reverse && revs.first_parent_only) {
- struct commit_list *final_children = lookup_decoration(&revs.children,
- &sb.final->object);
- if (!final_children ||
- hashcmp(final_children->item->parents->item->object.sha1,
- sb.final->object.sha1))
+ struct commit *c = firstparent_chain_initial;
+
+ sb.revs->children.name = "children";
+ while (c->parents &&
+ hashcmp(c->object.sha1, sb.final->object.sha1)) {
+ 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 (hashcmp(c->object.sha1, sb.final->object.sha1))
die("--reverse --first-parent together require range along first-parent chain");
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html