Starting to track patches through bugzilla

Starting to track patches through bugzilla

[Daniel Berlin]

After discussing it on IRC (the words "This is trivial" were uttered, 
the gauntlet laid down, and the challenge taken up), i'm starting to 
work on tracking patches +followup discussion through bugzilla, so that 
they don't get lost, and so we can associate bugs as being blocked on 
the patches that fix them (and whatnot).


An example can be seen at 
http://dberlin.org/bugzilla/show_bug.cgi?id=11757


Userids it can't determine (due to non-existent bugzilla accounts or 
not writing from the same email address you use in your bugzilla mail) 
are currently set to me, rather than creating them new accounts.  It'll 
likely just be set to an "unknown commenter" account with the from 
pasted into the text of their followup.

The bugs from 11750 onwards are patches processed from the gcc-patches 
archive since June.

The idea is to get something that is "good enough", but not in the way 
or annoying to deal with.

An example of this theory is that patches with subjects not starting 
with [PATCH] don't get processed (and any followup referencing them 
won't get processed as a result).  This is good enough, since people 
are supposed to do it. Patches that don't do it have the same chance as 
before of being lost in the ether.

I'll probably make a patch component/version so that they don't get the 
defaults (2.95/pending), since their is no way to tell either, and 
trying to make people put this info in would probably not actually 
happen, and just cause the patch tracker to be less used.

The only real thing left is a way to note that patches should be closed.

This will require some special keyword in the followups that we can 
notice, like a single line containing "NOPE or APPROVED".
(we've been discussing trying to use the cvs commit messages in some 
way to do this automatically, but it's really non-trivial)

It already handles attachments.

It doesn't attempt to seperate the inline diffs from the messages, 
though i'm sure this could be added later on.

Thoughts?

Trivial feature requests?

Note that people with mailers that don't set references/in-reply-to 
properly, of course, won't get pasted into the bug as a followup.
This of course, only affects a few well-known people using EVIL 
MAILERS(TM), and this feature should be considered part of the ongoing 
worldwide conspiracy to get them to move to a reasonable mailer.


--Dan

Re: Starting to track patches through bugzilla

[Daniel Berlin]

>
> An example of this theory is that patches with subjects not starting 
> with [PATCH] don't get processed (and any followup referencing them 
> won't get processed as a result).

Just to clarify this, followups don't have to keep the subject the 
same, or have [PATCH]. Only the original patch message does.
>

Re: Starting to track patches through bugzilla

[Carlo Wood]

It could happen that someone starts a mail with an RFC or
bug report - and then later someone replies to that thread
with a PATCH proposal - shouldn't that be prosessed as a
patch then?

Perhaps it is better to scan the body for attachments, and
detect the existance of a patch based on typical 'diff'-like
output.

Also, if I understand you right - this describes how mails
to gcc-patches@ will be filtered/detected and processed/included
on GNATs.  But the other way around is also needed imho:
When someone creates an attachment on GNAT for an existing
PR he can mark that attachment as a patch - if he does that,
then I think a mail should go out to patches@, including
the patch (or when the patch is very large - with just a
link to it).

Just ideas,

-- 
Carlo Wood <carlo@alinoe.com>

Re: Starting to track patches through bugzilla

[Daniel Berlin]

On Sep 25, 2003, at 5:16 PM, Carlo Wood wrote:

> It could happen that someone starts a mail with an RFC or
> bug report - and then later someone replies to that thread
> with a PATCH proposal - shouldn't that be prosessed as a
> patch then?
The patch will.

>
> Perhaps it is better to scan the body for attachments, and
> detect the existance of a patch based on typical 'diff'-like
> output.
This is not easy, and is somewhat more prone to false positives.
> Also, if I understand you right - this describes how mails
> to gcc-patches@ will be filtered/detected and processed/included
> on GNATs.  But the other way around is also needed imho:
> When someone creates an attachment on GNAT for an existing
> PR he can mark that attachment as a patch - if he does that,
> then I think a mail should go out to patches@, including
> the patch (or when the patch is very large - with just a
> link to it).

This is being worked on, if it can be done reasonably.

>
> Just ideas,
>
> -- 
> Carlo Wood <carlo@alinoe.com>

Re: Starting to track patches through bugzilla

[Tom Tromey]

>>>>> "Dan" == Daniel Berlin <dberlin@dberlin.org> writes:

Dan> An example can be seen at
Dan> http://dberlin.org/bugzilla/show_bug.cgi?id=11757

Dan> I'll probably make a patch component/version so that they don't get
Dan> the defaults (2.95/pending), since their is no way to tell either, and
Dan> trying to make people put this info in would probably not actually
Dan> happen, and just cause the patch tracker to be less used.

Dan> Trivial feature requests?

How about following the java-patches list as well?

While we're at it, it would be convenient if the patches were sorted
into components according to what they touch.  E.g., a patch that
touches gcc/java, libjava, libffi (etc) could be sorted into the "java
patches" component.  That would make it easier for me to see what
I've forgotten...  Failing that, just putting any patch that was sent
to java-patches into a special category would be nice.

It would be cool if the PR had a link to the mailing list archive
holding the article.  That way if a patch shows up mid-thread, you can
easily jump to the archive and read the history.

Tom

Re: Starting to track patches through bugzilla

[Hans-Peter Nilsson]

Great.

On Thu, 25 Sep 2003, Daniel Berlin wrote:
> Userids it can't determine (due to non-existent bugzilla accounts or
> not writing from the same email address you use in your bugzilla mail)
> are currently set to me, rather than creating them new accounts.  It'll
> likely just be set to an "unknown commenter" account with the from
> pasted into the text of their followup.

I don't want to send patches from hp gcc gnu org but I want to
be that my "official" bugzilla account.  Can I make that and
this addresses aliases (and more addresses)?

> An example of this theory is that patches with subjects not starting
> with [PATCH] don't get processed (and any followup referencing them
> won't get processed as a result).  This is good enough, since people
> are supposed to do it. Patches that don't do it have the same chance as
> before of being lost in the ether.

Please also [RFA] and [RFA:] (request for approval) as the
convention is, as was suggested last time this came up.  (Maybe
RFC too, but IMHO that's a misnomer.)

brgds, H-P

Re: Starting to track patches through bugzilla

[Daniel Berlin]

On Sep 27, 2003, at 1:53 AM, Hans-Peter Nilsson wrote:

> Great.
>
> On Thu, 25 Sep 2003, Daniel Berlin wrote:
>> Userids it can't determine (due to non-existent bugzilla accounts or
>> not writing from the same email address you use in your bugzilla mail)
>> are currently set to me, rather than creating them new accounts.  
>> It'll
>> likely just be set to an "unknown commenter" account with the from
>> pasted into the text of their followup.
>
> I don't want to send patches from hp gcc gnu org but I want to
> be that my "official" bugzilla account.  Can I make that and
> this addresses aliases (and more addresses)?

I'm working on this functionality, actually.
It'll probably be trivial, like a list of extra incoming email 
addresses for a given account.

>> An example of this theory is that patches with subjects not starting
>> with [PATCH] don't get processed (and any followup referencing them
>> won't get processed as a result).  This is good enough, since people
>> are supposed to do it. Patches that don't do it have the same chance 
>> as
>> before of being lost in the ether.
>
> Please also [RFA] and [RFA:] (request for approval) as the
> convention is, as was suggested last time this came up.  (Maybe
> RFC too, but IMHO that's a misnomer.)
>
The problem is that nobody uses that.
In the past three months (IE since june), it's been used
[dberlin@dberlin dberlin]$ pcregrep "Subject:\s*\[RFA.*\]" 
gcc-patches|wc
      18     131    1080

18 times.

I'd rather we standardize on [PATCH] than RFA.

> brgds, H-P
>

Re: Starting to track patches through bugzilla

[Hans-Peter Nilsson]

On Sat, 27 Sep 2003, Daniel Berlin wrote:
> On Sep 27, 2003, at 1:53 AM, Hans-Peter Nilsson wrote:
> > Please also [RFA] and [RFA:] (request for approval) as the
> > convention is, as was suggested last time this came up.  (Maybe
> > RFC too, but IMHO that's a misnomer.)
> >
> The problem is that nobody uses that.
> In the past three months (IE since june), it's been used
> [dberlin@dberlin dberlin]$ pcregrep "Subject:\s*\[RFA.*\]"
> gcc-patches|wc
>       18     131    1080
>
> 18 times.

I haven't sent patches in a while. :-)
...still it's a whopping 18 times that people used this
convention!  Anyway, I'm not going to change the nice convention
that I've been using since I don't know when.

> I'd rather we standardize on [PATCH] than RFA.

*No thanks*.  If you start making these kind of
help-the-robot-by-making-standardization-rules requests rather
than throwing in all and any convention you see, then this
feature will quickly be getting in the way.  I'm just not going
to mark a patch sent to gcc-patches with "patch".  It's
redundant: non-referencing messages that go there are usually
new patches, but patches are not always requests for approval,
a.k.a. requesting other maintainer attention.

So, RFA or whatever convention with the same meaning is
basically for human consumption, but of course it'd be nice if
bugzilla could recognize it.

brgds, H-P

Re: Starting to track patches through bugzilla

[Daniel Berlin]

>
> I haven't sent patches in a while. :-)
> ...still it's a whopping 18 times that people used this
> convention!  Anyway, I'm not going to change the nice convention
> that I've been using since I don't know when.
>

Okay.

>> I'd rather we standardize on [PATCH] than RFA.
>
> *No thanks*.  If you start making these kind of
> help-the-robot-by-making-standardization-rules requests rather
> than throwing in all and any convention you see, then this
> feature will quickly be getting in the way.  I'm just not going
> to mark a patch sent to gcc-patches with "patch".  It's
> redundant: non-referencing messages that go there are usually
> new patches, but patches are not always requests for approval,
> a.k.a. requesting other maintainer attention.

Sorry, but RFA has a very high false positive rate.

Why, just today, there were three [RFA]'s with no patches in them.
There have been *0* messages with [PATCH] in them with no patches in 
them.

The tool will be more in the way if it has a high false positive rate 
than if it  misses the occasional patch.

>
> So, RFA or whatever convention with the same meaning is
> basically for human consumption, but of course it'd be nice if
> bugzilla could recognize it.
>
> brgds, H-P
>

Re: Starting to track patches through bugzilla

[Hans-Peter Nilsson]

On Sat, 27 Sep 2003, Daniel Berlin wrote:
> Sorry, but RFA has a very high false positive rate.

I see the opposite.

> Why, just today, there were three [RFA]'s with no patches in them.

No, there's been no original [RFA] or [RFA:] messages today to
gcc-patches.  Going by the archive, that is.  I don't know where
you look; please clarify.

There's been one message marked RFA: that *was* a patch.  There
was another, in a thread marked "Re: [RFA/RFT] libffi reorg
(take 3)" (and similar) in response to a patch sent earlier.

> There have been *0* messages with [PATCH] in them with no patches in
> them.

What's that supposed to mean?  That people like marking their
patches really loud?

Oh well, if you just don't like it, then ignore RFA.  If you
think I'm the only one using it as "request for approval", you
won't lose much.

Anyway, why not have bugzilla look at *all messages* to
gcc-patches that don't reference another instead of telling
people how to mark their patches to please bugzilla?

brgds, H-P

Re: Starting to track patches through bugzilla

[Daniel Berlin]

On Sep 27, 2003, at 4:13 PM, Hans-Peter Nilsson wrote:

> On Sat, 27 Sep 2003, Daniel Berlin wrote:
>> Sorry, but RFA has a very high false positive rate.
>
> I see the opposite.
>
>> Why, just today, there were three [RFA]'s with no patches in them.
>
> No, there's been no original [RFA] or [RFA:] messages today to
> gcc-patches.  Going by the archive, that is.  I don't know where
> you look; please clarify.
>

All my gcc mail goes to one mailbox, so it's quite possible it was to 
gcc@
> There's been one message marked RFA: that *was* a patch.  There
> was another, in a thread marked "Re: [RFA/RFT] libffi reorg
> (take 3)" (and similar) in response to a patch sent earlier.
>
>> There have been *0* messages with [PATCH] in them with no patches in
>> them.
>
> What's that supposed to mean?  That people like marking their
> patches really loud?
>
That regardless of whether you think RFA is the standard, it appears 
people think PATCH is.

> Oh well, if you just don't like it, then ignore RFA.  If you
> think I'm the only one using it as "request for approval", you
> won't lose much.
>
> Anyway, why not have bugzilla look at *all messages* to
> gcc-patches that don't reference another instead of telling
> people how to mark their patches to please bugzilla?

Because i'm starting simple?
Right now I'm not going to try to match all text in the message and all 
the text in the <possibly compressed> attachments against a regular 
expression to detect diffs.
Maybe later.

>
> brgds, H-P
>

Re: Starting to track patches through bugzilla

[Hans-Peter Nilsson]

On Sat, 27 Sep 2003, Daniel Berlin wrote:
> On Sep 27, 2003, at 4:13 PM, Hans-Peter Nilsson wrote:
> > Anyway, why not have bugzilla look at *all messages* to
> > gcc-patches that don't reference another instead of telling
> > people how to mark their patches to please bugzilla?
>
> Because i'm starting simple?
> Right now I'm not going to try to match all text in the message and all
> the text in the <possibly compressed> attachments against a regular
> expression to detect diffs.
> Maybe later.

That's not what I suggested.  Like matching Subject with a tag,
you just match the absence of In-Reply-To in the header before
looking closer.  For email to gcc-patches, that is.  You don't
look at email going elsewhere, I presume.  False positives, you
know.

brgds, H-P

Re: Starting to track patches through bugzilla

[Daniel Berlin]

On Sep 27, 2003, at 4:35 PM, Hans-Peter Nilsson wrote:

> On Sat, 27 Sep 2003, Daniel Berlin wrote:
>> On Sep 27, 2003, at 4:13 PM, Hans-Peter Nilsson wrote:
>>> Anyway, why not have bugzilla look at *all messages* to
>>> gcc-patches that don't reference another instead of telling
>>> people how to mark their patches to please bugzilla?
>>
>> Because i'm starting simple?
>> Right now I'm not going to try to match all text in the message and 
>> all
>> the text in the <possibly compressed> attachments against a regular
>> expression to detect diffs.
>> Maybe later.
>
> That's not what I suggested.  Like matching Subject with a tag,
> you just match the absence of In-Reply-To in the header before
> looking closer.

But we don't *look* closer (IE the only validation we currently perform 
is the matching of [PATCH]).
Not all new threads on gcc-patches contain patches, so until i'm 
looking for diffs in mails, doing what you suggest would track more 
than just patches, it would track random started discussions without 
patches.
>  For email to gcc-patches, that is.  You don't
> look at email going elsewhere, I presume.  False positives, you
> know.
>
> brgds, H-P
>

Re: Starting to track patches through bugzilla

[Jeff Sturm]

On Sat, 27 Sep 2003, Hans-Peter Nilsson wrote:
> (Maybe RFC too, but IMHO that's a misnomer.)

Curious... why?

Jeff

Re: Starting to track patches through bugzilla

[Hans-Peter Nilsson]

On Sat, 27 Sep 2003, Jeff Sturm wrote:
> On Sat, 27 Sep 2003, Hans-Peter Nilsson wrote:
> > (Maybe RFC too, but IMHO that's a misnomer.)
>
> Curious... why?

Just taste, feel free to ignore.  Methinks a "request for
comments" is more of a patch in progress, not ripe for approval.
It seems used in both senses, though.  Huh, well ok then, that
means bugzilla should trig on it.

brgds, H-P

Re: Starting to track patches through bugzilla

[Gerald Pfeifer]

On Thu, 25 Sep 2003, Daniel Berlin wrote:
> After discussing it on IRC (the words "This is trivial" were uttered,
> the gauntlet laid down, and the challenge taken up), i'm starting to
> work on tracking patches +followup discussion through bugzilla, so that
> they don't get lost, and so we can associate bugs as being blocked on
> the patches that fix them (and whatnot).

I think it would be preferrable to have such a patch tracker (that
tracks every single patch) separate from the bug-tracking system.

Also, I'm afraid we'll change from one extreme (no patch tracking)
to the other (over-zealous patch tracking).

A middle-ground I'd prefer is having patches explicitly submitted
by contributors, perhaps after they haven't been reviewed upon first
submission to the list.  I think this will be much more manageable
for us in the long or even mid term.

Gerald

> Note that people with mailers that don't set references/in-reply-to
> properly, of course, won't get pasted into the bug as a followup.
> This of course, only affects a few well-known people using EVIL
> MAILERS(TM), and this feature should be considered part of the ongoing
> worldwide conspiracy to get them to move to a reasonable mailer.

Good luck! :-)
-- 
Gerald Pfeifer (Jerry)   gerald@pfeifer.com   http://www.pfeifer.com/gerald/

Re: Starting to track patches through bugzilla

[Nathanael Nerode]

Daniel Berlin wrote:
>An example of this theory is that patches with subjects not starting with
>[PATCH] don't get processed (and any followup referencing them won't get
>processed as a result).
...
>Trivial feature requests?

Please make it non-case-sensitive, so that [patch] and [Patch] work too.

-- 
Nathanael Nerode  <neroden at gcc.gnu.org>
http://home.twcny.rr.com/nerode/neroden/fdl.html

Re: Starting to track patches through bugzilla

[Joseph S. Myers]

On Thu, 25 Sep 2003, Nathanael Nerode wrote:

> Please make it non-case-sensitive, so that [patch] and [Patch] work too.

A variant on this is such notations as [C Patch] and [C++ Patch].

If the patch has already been applied when the mail is sent, there
probably isn't much point in opening a bug for it (given the point of
finding patches that fall through the cracks).  So that should have some
notation to indicate it's already been applied (whether in the subject, or
along the lines of the automatic closing notation).  Avoiding [PATCH] in
the subject if you maintain the relevant part of the compiler and so have
already applied the patch seems like too much of a kludge.

Arguably it should be possible to mark a patch as being for a particular
bug, [Patch] PR 98765, and then get the discussion attached to the
existing bug report rather than a new one.  But this would complicate
automatic closing (an applied patch might or might not *completely* fix
the original bug).

-- 
Joseph S. Myers
jsm@polyomino.org.uk

Re: Starting to track patches through bugzilla

[Daniel Berlin]

On Sep 26, 2003, at 5:56 PM, Joseph S. Myers wrote:

> On Thu, 25 Sep 2003, Nathanael Nerode wrote:
>
>> Please make it non-case-sensitive, so that [patch] and [Patch] work 
>> too.
>
> A variant on this is such notations as [C Patch] and [C++ Patch].
>
> If the patch has already been applied when the mail is sent, there
> probably isn't much point in opening a bug for it (given the point of
> finding patches that fall through the cracks).  So that should have 
> some
> notation to indicate it's already been applied (whether in the 
> subject, or
> along the lines of the automatic closing notation).  Avoiding [PATCH] 
> in
> the subject if you maintain the relevant part of the compiler and so 
> have
> already applied the patch seems like too much of a kludge.

How bout [committed] instead of [patch]?

>
> Arguably it should be possible to mark a patch as being for a 
> particular
> bug, [Patch] PR 98765, and then get the discussion attached to the
> existing bug report rather than a new one.  But this would complicate
> automatic closing (an applied patch might or might not *completely* fix
> the original bug).

Right.
I'm just trying to track patches, then we'll look at trying to 
associate patch bugs and non-patch bugs, etc.
>

Re: Starting to track patches through bugzilla

[Gabriel Dos Reis]

Daniel Berlin <dberlin@dberlin.org> writes:

[...]

| > along the lines of the automatic closing notation).  Avoiding
| > [PATCH] in
| > the subject if you maintain the relevant part of the compiler and so
| > have
| > already applied the patch seems like too much of a kludge.
| 
| How bout [committed] instead of [patch]?

would we have gcc-cvs or cvs-commit no longer send a mail?

-- Gaby

Re: Starting to track patches through bugzilla

[Daniel Berlin]

On Sep 26, 2003, at 8:20 PM, Gabriel Dos Reis wrote:

> Daniel Berlin <dberlin@dberlin.org> writes:
>
> [...]
>
> | > along the lines of the automatic closing notation).  Avoiding
> | > [PATCH] in
> | > the subject if you maintain the relevant part of the compiler and 
> so
> | > have
> | > already applied the patch seems like too much of a kludge.
> |
> | How bout [committed] instead of [patch]?

> would we have gcc-cvs or cvs-commit no longer send a mail?
>
The problem is that cvs-commit doesn't reference the original patch 
mail (it can't), while people who write 'committed' are usually 
following up to a patch mail.

I'm really trying to avoid having to have people know the patch pr 
number.

--Dan

Re: Starting to track patches through bugzilla

[Gabriel Dos Reis]

Daniel Berlin <dberlin@dberlin.org> writes:

| > would we have gcc-cvs or cvs-commit no longer send a mail?
| >
| The problem is that cvs-commit doesn't reference the original patch
| mail (it can't), while people who write 'committed' are usually
| following up to a patch mail.
| 
| I'm really trying to avoid having to have people know the patch pr
| number.

Thanks for the clarification.

-- Gaby

Re: Starting to track patches through bugzilla

[Tom Tromey]

>>>>> "Joseph" == Joseph S Myers <jsm@polyomino.org.uk> writes:

Joseph> A variant on this is such notations as [C Patch] and [C++ Patch].

On java-patches we frequently write simply "Patch:".

Joseph> If the patch has already been applied when the mail is sent,
Joseph> there probably isn't much point in opening a bug for it (given
Joseph> the point of finding patches that fall through the cracks).

For this I usually write "FYI" in the subject.

Tom

Re: Starting to track patches through bugzilla

[Wolfgang Bangerth]

I don't have much to say about the usefulness of tracking patches through 
bugzilla. However, I'd like to raise one concern:

Under no circumstances do I want patches to be tracked through the same 
component of bugzilla as bug reports. It took us about one year of hard work 
to clean out the database from old reports that are no longer valid, have 
never been looked at, etc. We would make all this effort useless if we 
started to open massive numbers of reports for patches that have been sent 
somewhere and that have subsequently been forgotten -- either because noone 
looked at them, because they were rejected, or because the submitter of the 
patch applied it without closing the bugzilla report. I don't want us 
bugmasters to be in a position where we have to start tracking down what 
happened to a certain patch just to keep the data base clean of reports that 
are no longer valid.

Remember that some of us (me including) do not know much about gcc's internals 
or its source code. So if I had a PR with a patch and had to decide what 
happened, I would either have to indulge into digging into code that I don't 
know, send emails to other people, or just give up. Neither is a particularly 
useful thing.

So I'd like to keep patches and bugs separate somehow for a while. If we're 
sure that the patch database works fine, then we can think about merging 
them, but I'd like to have a test phase first.

W.
 
-------------------------------------------------------------------------
Wolfgang Bangerth              email:            bangerth@ices.utexas.edu
                               www: http://www.ices.utexas.edu/~bangerth/