From: Junio C Hamano <gitster@pobox•com>
To: Jeff King <peff@peff•net>
Cc: "brian m. carlson" <sandals@crustytoothpaste•net>,
git@vger•kernel.org,
Christian Couder <christian.couder@gmail•com>,
Bruce Korb <bruce.korb@gmail•com>
Subject: Re: [PATCH] format-patch: dereference tags with --ignore-if-in-upstream
Date: Mon, 01 Jun 2015 13:35:17 -0700 [thread overview]
Message-ID: <xmqq4mmr5fqy.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20150601174712.GA18364@peff.net> (Jeff King's message of "Mon, 1 Jun 2015 13:47:12 -0400")
Jeff King <peff@peff•net> writes:
> On Mon, Jun 01, 2015 at 10:44:21AM -0700, Junio C Hamano wrote:
>
>> > 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.
>
> Yeah, I think this matches the recommendation I gave in the last round.
>
> I do still think we could get rid of this "second" traversal entirely in
> favor of using "--cherry", but that is a much larger topic. Even if
> somebody wants to pursue that, the immediate fix should look like this.
>
> -Peff
Thanks.
-- >8 --
From: Junio C Hamano <gitster@pobox•com>
Date: Mon, 1 Jun 2015 10:44:21 -0700
Subject: [PATCH] format-patch: do not feed tags to clear_commit_marks()
"git format-patch --ignore-if-in-upstream A..B", when either A or B
is a tag, failed miserably.
This is because the code passes the tips it used for traversal to
clear_commit_marks(), after running a temporary revision traversal
to enumerate the commits on both branches to find if they have
commits that make equivalent changes. The revision traversal
machinery knows how to enumerate commits reachable starting from a
tag, but clear_commit_marks() wants to take nothing but a commit.
In the longer term, it might be a more correct fix to teach
clear_commit_marks() to do the same "committish to commit"
dereferncing that is done in the revision traversal machinery, but
for now this fix should suffice.
Reported-by: Bruce Korb <bruce.korb@gmail•com>
Helped-by: Christian Couder <christian.couder@gmail•com>
Helped-by: brian m. carlson <sandals@crustytoothpaste•net>
Helped-by: Jeff King <peff@peff•net>
Signed-off-by: Junio C Hamano <gitster@pobox•com>
---
builtin/log.c | 12 ++++++------
t/t4014-format-patch.sh | 8 ++++++++
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 734aab3..39181e2 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 256affc..2ea12dd 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 &&
--
2.4.2-558-g3ddf4bb
next prev parent reply other threads:[~2015-06-01 20:35 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-31 19:13 seg fault in "git format-patch" Bruce Korb
2015-05-31 20:26 ` Christian Couder
2015-05-31 20:41 ` Bruce Korb
2015-05-31 20:45 ` Bruce Korb
2015-05-31 23:14 ` Christian Couder
2015-05-31 23:53 ` Christian Couder
2015-06-01 0:01 ` Christian Couder
2015-06-01 1:03 ` [PATCH] format-patch: dereference tags with --ignore-if-in-upstream brian m. carlson
2015-06-01 10:20 ` Jeff King
2015-06-01 11:22 ` brian m. carlson
2015-06-01 11:47 ` Jeff King
2015-06-01 14:56 ` Junio C Hamano
2015-06-01 17:44 ` Junio C Hamano
2015-06-01 17:47 ` Jeff King
2015-06-01 20:35 ` Junio C Hamano [this message]
2015-06-01 22:34 ` brian m. carlson
2015-06-01 22:46 ` Junio C Hamano
2015-06-01 17:58 ` Junio C Hamano
2015-06-01 13:44 ` seg fault in "git format-patch" Christian Couder
2015-06-01 14:17 ` Christian Couder
2015-06-01 14:47 ` Bruce Korb
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq4mmr5fqy.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=bruce.korb@gmail$(echo .)com \
--cc=christian.couder@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=peff@peff$(echo .)net \
--cc=sandals@crustytoothpaste$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox