Re: [PATCH] format-patch: dereference tags with --ignore-if-in-upstream

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
>> diff --git a/builtin/log.c b/builtin/log.c
>> index dd8f3fc..e0465ba 100644
>> --- a/builtin/log.c
>> +++ b/builtin/log.c
>> @@ -807,6 +807,12 @@ static void get_patch_ids(struct rev_info *rev,
>> struct patch_ids *ids)
>>  	o2 = rev->pending.objects[1].item;
>>  	flags2 = o2->flags;
>>  
>> +	o1 = deref_tag(o1, NULL, 0);
>> +	o2 = deref_tag(o2, NULL, 0);
>> +
>> +	if (!o1 || !o2)
>> +		die(_("Invalid tag."));
>
> Shouldn't you ensure o1 and o2 are commits here?

Heh, I should have read the remainder of the thread before
responding.

How about doing it this way?  We know and trust that existing
revision traversal machinery is doing the right thing, and it is
only that the clear_commit_marks() calls are botched.

diff --git a/builtin/log.c b/builtin/log.c
index dd8f3fc..23a42fa 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -795,7 +795,7 @@ static int reopen_stdout(struct commit *commit, const char *subject,
 static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)
 {
 	struct rev_info check_rev;
-	struct commit *commit;
+	struct commit *commit, *c1, *c2;
 	struct object *o1, *o2;
 	unsigned flags1, flags2;
 
@@ -803,9 +803,11 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)
 		die(_("Need exactly one range."));
 
 	o1 = rev->pending.objects[0].item;
-	flags1 = o1->flags;
 	o2 = rev->pending.objects[1].item;
+	flags1 = o1->flags;
 	flags2 = o2->flags;
+	c1 = lookup_commit_reference(o1->sha1);
+	c2 = lookup_commit_reference(o2->sha1);
 
 	if ((flags1 & UNINTERESTING) == (flags2 & UNINTERESTING))
 		die(_("Not a range."));
@@ -827,10 +829,8 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)
 	}
 
 	/* reset for next revision walk */
-	clear_commit_marks((struct commit *)o1,
-			SEEN | UNINTERESTING | SHOWN | ADDED);
-	clear_commit_marks((struct commit *)o2,
-			SEEN | UNINTERESTING | SHOWN | ADDED);
+	clear_commit_marks(c1, SEEN | UNINTERESTING | SHOWN | ADDED);
+	clear_commit_marks(c2, SEEN | UNINTERESTING | SHOWN | ADDED);
 	o1->flags = flags1;
 	o2->flags = flags2;
 }
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index c39e500..890db11 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -57,6 +57,14 @@ test_expect_success "format-patch --ignore-if-in-upstream" '
 
 '
 
+test_expect_success "format-patch --ignore-if-in-upstream handles tags" '
+	git tag -a v1 -m tag side &&
+	git tag -a v2 -m tag master &&
+	git format-patch --stdout --ignore-if-in-upstream v2..v1 >patch1 &&
+	cnt=$(grep "^From " patch1 | wc -l) &&
+	test $cnt = 2
+'
+
 test_expect_success "format-patch doesn't consider merge commits" '
 
 	git checkout -b slave master &&