Comments on Patching GCC

Comments on Patching GCC

[Scott Robert Ladd]

Hello.

I've been working on patches for GCC, both inside the general community 
and for specific users.

None of what follows is a criticism; rather, I hope this will be 
accepted as an honest commentary on the challenges posed to "GCC patch" 
newbies.

Some people find the "contributing" process intimidating, starting with 
the requirements for copyright assignments. Many people have unsettled 
feelings about legal paperwork, especially when it appears rather 
mysterious. I'm not certain how this necessary process could be made 
friendlier, especially for those who want to submit one or two patches 
specific to their domain.

Once paperwork is in order (mine is), it's difficult to know where to 
start. I've picked a few simple code generation bugs, and started 
patching those; the choices were semi-random, based on self-interest, 
available hardware, and (in one case) a request from a Linux x86_64 
distribution maintainer.

It's often difficult to know where to begin work on GCC, especially 
after reading comments in bugzilla. Many bugs are in what appears to be 
a "dangling" state, where a conversation has ended with an unanswered 
question or statement that "this is complicated." Some comment streams 
leave me wondering if a bug is actually patched, and in what version.

Many projects, private or free, have a clear process; in the realm of 
GCC, it's a sink-or-swim process, where its possible to step on toes you 
didn't even know were there... ;) I've chosen the "better to beg 
forgiveness than ask permission" approach in some cases.

Once a patch is created and posted to gcc-patches, there comes The Long 
Wait. The submitter asks: Was my patch accepted or not? Was it even noticed?

Perhaps we need a way of queuing patches, letting submitters know that 
their work is at least being acknowledged. Otherwise, new developers get 
disillusioned, and feel like they're lost in the wilderness or forgotten.

I think many people look at the size of the GCC codebase, and run away 
before realizing that it really isn't *that* arcane. Documentation for 
GCC's internals is imperfect, but is certainly better than what I've 
seen for other projects. I'm not a compiler hacker by nature, but have 
had little trouble gleaning the basics of GCC's architecture.

Perhaps we need a "patch newbies" mailing list?

Anything we can do to improve the process is likely to be a Good 
Thing(tm) for GCC.

..Scott

-- 
Scott Robert Ladd
site: http://www.coyotegulch.com
blog: http://chaoticcoyote.blogspot.com

Re: Comments on Patching GCC

[Clifford Wolf]

Hi,

On Wed, Sep 22, 2004 at 01:49:05PM -0400, Scott Robert Ladd wrote:
> Once a patch is created and posted to gcc-patches, there comes The Long 
> Wait. The submitter asks: Was my patch accepted or not? Was it even noticed?
> 
> Perhaps we need a way of queuing patches, letting submitters know that 
> their work is at least being acknowledged. Otherwise, new developers get 
> disillusioned, and feel like they're lost in the wilderness or forgotten.

for ROCK Linux I've developed SubMaster to solve this problem. Right now
it's more or less a proof-of-concept hack and in fact I'm looking for
someone who is willing to implement it "the right way". Also, I've only
implemented those features which where required for the specific needs and
size of ROCK Linux. (this month we had >300 patches going over SubMaster so
far).

	http://www.rocklinux.org/submaster.html

I'll do a presentration about SubMaster at this years SANE conference in
amsterdam next week.

	http://www.nluug.nl/events/sane2004/

Sorry, the homepage isn't very verbose right now. A more detailed
description of the system can be found in my paper for the conference which
can also be found on my homepage:

	http://www.clifford.at/papers/2004/sm/

yours,
 - clifford

--
 ____   ___   ____ _  __  _     _ www.rocklinux.org
|  _ \ / _ \ / ___| |/ / | |   (_)_ __  _   ___  __
| |_) | | | | |   | ' /  | |   | | '_ \| | | \ \/ /
|  _ <| |_| | |___| . \  | |___| | | | | |_| |>  <         Clifford Wolf
|_| \_\\___/ \____|_|\_\ |_____|_|_| |_|\__,_/_/\_\      www.clifford.at
 
Real Programmers always confuse Christmas and Halloween because Oct31 == Dec25.
 

Re: Comments on Patching GCC

[Ben Elliston]

Scott Robert Ladd <coyote@coyotegulch.com> writes:

> Perhaps we need a way of queuing patches, letting submitters know
> that their work is at least being acknowledged. Otherwise, new
> developers get disillusioned, and feel like they're lost in the
> wilderness or forgotten.

This was discussed to some degree at the GCC Summit during the panel
session.  There was a recognition that newbies need more guidance at
patch submission and that there might be a larger group of "pseudo
maintainers" that don't have the means to approve patches, but could
help newbies to knock their patches into shape.

I'm a bit undecided about what needs to happen next, but I agree that
sending mail to gcc-patches is really hit and miss.  A good amount of
traffic on gcc-patches consists of "ping?" messages.  Patch authors
have no idea on the state of their patches, nor do other interested
parties.  Maintainers may easily miss patches that are due their
attention.

I have to wonder if a patch tracking system wouldn't be a better way
of handling patches.

Ben

Re: Comments on Patching GCC

[Paul Koning]

>>>>> "Ben" == Ben Elliston <bje@au.ibm.com> writes:

 Ben> I'm a bit undecided about what needs to happen next, but I agree
 Ben> that sending mail to gcc-patches is really hit and miss.  A good
 Ben> amount of traffic on gcc-patches consists of "ping?" messages.
 Ben> Patch authors have no idea on the state of their patches, nor do
 Ben> other interested parties.  Maintainers may easily miss patches
 Ben> that are due their attention.

 Ben> I have to wonder if a patch tracking system wouldn't be a better
 Ben> way of handling patches.

Good point, especially for those of us that are maintainers for
components with low rates of traffic.  Trying to find the message that
I actually have to act on in a flood of 300+ gcc messages per day is
very difficult.

     paul

Re: Comments on Patching GCC

[Ian Lance Taylor]

Ben Elliston <bje@au.ibm.com> writes:

> I have to wonder if a patch tracking system wouldn't be a better way
> of handling patches.

I think a patch tracking system would be quite useful for gcc.  It
would of course need some manual guidance.

An interim approach would be a to have a few "patch shepherds" who
would manage the patch queue, direct the attention of appropriate
maintainers to new patches, and ping about patches which were being
ignored.  I think the project has done surprisingly well with the
volunteer bugmasters.  Would it be possible to have volunteer
patchmasters as well?

Ian

Re: Comments on Patching GCC

[Brad Roberts]

On 23 Sep 2004, Ian Lance Taylor wrote:

> Date: 23 Sep 2004 20:34:26 -0400
> From: Ian Lance Taylor <ian@wasabisystems.com>
> To: gcc@gcc.gnu.org
> Subject: Re: Comments on Patching GCC
>
> Ben Elliston <bje@au.ibm.com> writes:
>
> > I have to wonder if a patch tracking system wouldn't be a better way
> > of handling patches.
>
> I think a patch tracking system would be quite useful for gcc.  It
> would of course need some manual guidance.
>
> An interim approach would be a to have a few "patch shepherds" who
> would manage the patch queue, direct the attention of appropriate
> maintainers to new patches, and ping about patches which were being
> ignored.  I think the project has done surprisingly well with the
> volunteer bugmasters.  Would it be possible to have volunteer
> patchmasters as well?
>
> Ian

Bugzilla itself has a fairly nice attachment mechanism which has a lot of
special handling for patchs, review requests, etc.  Given that gcc
developers already use bugzilla, why not use more of it's features?  The
mozilla development community uses it for every change going into all of
the various apps they produce.

Later,
Brad

Re: Comments on Patching GCC

[Giovanni Bajo]

Brad Roberts wrote:

> Bugzilla itself has a fairly nice attachment mechanism which has a
> lot of special handling for patchs, review requests, etc.  Given that
> gcc developers already use bugzilla, why not use more of it's
> features?

Because it is hard to change habit, or maybe people just do not want to.

I have been working on and off on a little script to extract unreviewed patches
from Bugzilla (using the patch keyword + links to gcc-patches in the comments)
that can be used as a robitic patchmaster (as suggested by Ian). It can
generate mails to ping patches, and could be cron'd every 3-4 days (my direct
experience is that more often than not a patch is either reviewed in the next
72 hrs or simply is not until next ping).

It still needs a couple of refinements plus fixing some bugs with xml
validation for which I have been working with Danny B.

Giovanni Bajo

Re: Comments on Patching GCC

[Daniel Berlin]

On Fri, 2004-09-24 at 04:21 +0200, Giovanni Bajo wrote:
> Brad Roberts wrote:
> 
> > Bugzilla itself has a fairly nice attachment mechanism which has a
> > lot of special handling for patchs, review requests, etc.  Given that
> > gcc developers already use bugzilla, why not use more of it's
> > features?
> 
> Because it is hard to change habit, or maybe people just do not want to.
> 

I think we can script most of this, actually.
I'm pretty sure i can easily make a script that you hand a patch title,
possibly an existing bug id, a patch, and a changelog, and it'll handle
submission and give you a bug id and a patch id.

I can also make a script that will list the outstanding submissions by
bug and patch id, and let you retrieve patches by id.

Of course, this would *only* work for patches submitted using the
script.


> I have been working on and off on a little script to extract unreviewed patches
> from Bugzilla (using the patch keyword + links to gcc-patches in the comments)
> that can be used as a robitic patchmaster (as suggested by Ian). It can
> generate mails to ping patches, and could be cron'd every 3-4 days (my direct
> experience is that more often than not a patch is either reviewed in the next
> 72 hrs or simply is not until next ping).
> 
> It still needs a couple of refinements plus fixing some bugs with xml
> validation for which I have been working with Danny B.
> 
> Giovanni Bajo
> 
> 
-- 

Re: Comments on Patching GCC

[Giovanni Bajo]

Daniel Berlin wrote:

> I think we can script most of this, actually.
> I'm pretty sure i can easily make a script that you hand a patch
> title,
> possibly an existing bug id, a patch, and a changelog, and it'll
> handle submission and give you a bug id and a patch id.
>
> I can also make a script that will list the outstanding submissions by
> bug and patch id, and let you retrieve patches by id.
>
> Of course, this would *only* work for patches submitted using the
> script.

Yes. My script can instead be retrofitted for any posted patch: it's just a
matter of setting the 'patch' keyword in Bugzilla, and adding a comment with a
link to gcc-patches. And the good news is that we already do this for most of
the patches. The only thing is that we would need to keep the patch keyword in
sync: if the patch is reviewed asking for changes, the patch keyword needs to
be disabled until a new patch is posted (if it's taking too long), otherwise
it'll keep pinging.

Giovanni Bajo

Re: Comments on Patching GCC

[Daniel Berlin]

On Fri, 2004-09-24 at 04:43 +0200, Giovanni Bajo wrote:
> Daniel Berlin wrote:
> 
> > I think we can script most of this, actually.
> > I'm pretty sure i can easily make a script that you hand a patch
> > title,
> > possibly an existing bug id, a patch, and a changelog, and it'll
> > handle submission and give you a bug id and a patch id.
> >
> > I can also make a script that will list the outstanding submissions by
> > bug and patch id, and let you retrieve patches by id.
> >
> > Of course, this would *only* work for patches submitted using the
> > script.
> 
> Yes. My script can instead be retrofitted for any posted patch: it's just a
> matter of setting the 'patch' keyword in Bugzilla, and adding a comment with a
> link to gcc-patches. And the good news is that we already do this for most of
> the patches. The only thing is that we would need to keep the patch keyword in
> sync: if the patch is reviewed asking for changes, the patch keyword needs to
> be disabled until a new patch is posted (if it's taking too long), otherwise
> it'll keep pinging.
> 

Right, which is why i was going to do it based on attachments, the patch
keyword and attachment flag, and keep track of the obsoletes flag you
can set on attachments :)

> Giovanni Bajo
> 
> 
-- 

Re: Comments on Patching GCC

[Caroline Tice]

In general I think having a better way to track submitted patches so 
that they don't get lost is
a very good idea.  My one concern about attaching it to Bugzilla is: 
What if someone wants to
submit a patch for a (new) feature or extension, rather than a bug?  Is 
Bugzilla supposed to be used
to track new feature development as well as existing bugs?  (I really 
don't know; I'm just asking).

-- Caroline Tice
ctice@apple.com

On Sep 23, 2004, at 8:04 PM, Daniel Berlin wrote:

> On Fri, 2004-09-24 at 04:43 +0200, Giovanni Bajo wrote:
>> Daniel Berlin wrote:
>>
>>> I think we can script most of this, actually.
>>> I'm pretty sure i can easily make a script that you hand a patch
>>> title,
>>> possibly an existing bug id, a patch, and a changelog, and it'll
>>> handle submission and give you a bug id and a patch id.
>>>
>>> I can also make a script that will list the outstanding submissions 
>>> by
>>> bug and patch id, and let you retrieve patches by id.
>>>
>>> Of course, this would *only* work for patches submitted using the
>>> script.
>>
>> Yes. My script can instead be retrofitted for any posted patch: it's 
>> just a
>> matter of setting the 'patch' keyword in Bugzilla, and adding a 
>> comment with a
>> link to gcc-patches. And the good news is that we already do this for 
>> most of
>> the patches. The only thing is that we would need to keep the patch 
>> keyword in
>> sync: if the patch is reviewed asking for changes, the patch keyword 
>> needs to
>> be disabled until a new patch is posted (if it's taking too long), 
>> otherwise
>> it'll keep pinging.
>>
>
> Right, which is why i was going to do it based on attachments, the 
> patch
> keyword and attachment flag, and keep track of the obsoletes flag you
> can set on attachments :)
>
>> Giovanni Bajo
>>
>>
> -- 
>

Re: Comments on Patching GCC

[Andrew Pinski]

On Sep 24, 2004, at 2:16 PM, Caroline Tice wrote:

>
> In general I think having a better way to track submitted patches so 
> that they don't get lost is
> a very good idea.  My one concern about attaching it to Bugzilla is: 
> What if someone wants to
> submit a patch for a (new) feature or extension, rather than a bug?  
> Is Bugzilla supposed to be used
> to track new feature development as well as existing bugs?  (I really 
> don't know; I'm just asking).

Yes Bugzilla is used to keep track of feature development and other
enhancements like optimization passes (there is one assigned to me
right now which I have been meaning to work on).

Thanks,
Andrew Pinski

Re: Comments on Patching GCC

[Devang Patel]

On Sep 24, 2004, at 11:23 AM, Andrew Pinski wrote:

>
> On Sep 24, 2004, at 2:16 PM, Caroline Tice wrote:
>
>>
>> In general I think having a better way to track submitted patches so 
>> that they don't get lost is
>> a very good idea.  My one concern about attaching it to Bugzilla is: 
>> What if someone wants to
>> submit a patch for a (new) feature or extension, rather than a bug?  
>> Is Bugzilla supposed to be used
>> to track new feature development as well as existing bugs?  (I really 
>> don't know; I'm just asking).
>
> Yes Bugzilla is used to keep track of feature development and other
> enhancements like optimization passes

I am not sure this is true in reality. Look at the last 100 patches 
posted on gcc-patch. How many of them has bugzilla reference ?

http://gcc.gnu.org/contribute.html#patches does not ask bugzilla 
reference for new features.

-
Devang

Re: Comments on Patching GCC

[Joseph S. Myers]

On Thu, 23 Sep 2004, Daniel Berlin wrote:

> Right, which is why i was going to do it based on attachments, the patch
> keyword and attachment flag, and keep track of the obsoletes flag you
> can set on attachments :)

If patch attachments are to be used as a standard means of patch 
submission for review, then Bugzilla should automatically send those 
attachments - in plain text and in the body of the message, not a separate 
MIME part - to gcc-patches, so they can readily be quoted in replies etc..

-- 
Joseph S. Myers               http://www.srcf.ucam.org/~jsm28/gcc/
  http://www.srcf.ucam.org/~jsm28/gcc/#c90status - status of C90 for GCC 4.0
    jsm@polyomino.org.uk (personal mail)
    jsm28@gcc.gnu.org (Bugzilla assignments and CCs)

Re: Comments on Patching GCC

[Andrew Pinski]

On Sep 24, 2004, at 2:43 PM, Devang Patel wrote:
> I am not sure this is true in reality. Look at the last 100 patches 
> posted on gcc-patch. How many of them has bugzilla reference ?

Actually at this point, more than 50% of them do because users are 
starting
to use 4.0.0 and file bug reports against it.  But before stage3, there 
were
less than 50% of the patches were to file a bug report.  Another reason
why people don't file a bug report before submitting the patch is 
because
they have a patch already to fix the bug they were looking into.

> http://gcc.gnu.org/contribute.html#patches does not ask bugzilla 
> reference for new features.

But that is because the main use for bugzilla is for users rather than
developers for filing bugs/enhancements.  There are a huge amount of
enhancements filed in bugzilla already, almost 500 bugs.

The main reason why I file a bug report is either I don't have time to
look into the problem or I know I am going to take a long time to fix
the bug in the first place.

-- Pinski

Re: Comments on Patching GCC

[Daniel Berlin]

On Sep 24, 2004, at 2:50 PM, Joseph S. Myers wrote:

> On Thu, 23 Sep 2004, Daniel Berlin wrote:
>
>> Right, which is why i was going to do it based on attachments, the 
>> patch
>> keyword and attachment flag, and keep track of the obsoletes flag you
>> can set on attachments :)
>
> If patch attachments are to be used as a standard means of patch
> submission for review, then Bugzilla should automatically send those
> attachments - in plain text and in the body of the message, not a 
> separate
> MIME part - to gcc-patches, so they can readily be quoted in replies 
> etc..
>

See, this is the whole problem.
You guys want both something that tracks patches, *and* integrates well 
with email.
Nobody makes such a beast, and it's very non-trivial :)
> -- 
> Joseph S. Myers               http://www.srcf.ucam.org/~jsm28/gcc/
>   http://www.srcf.ucam.org/~jsm28/gcc/#c90status - status of C90 for 
> GCC 4.0
>     jsm@polyomino.org.uk (personal mail)
>     jsm28@gcc.gnu.org (Bugzilla assignments and CCs)

Re: Comments on Patching GCC

[Devang Patel]

On Sep 24, 2004, at 11:52 AM, Andrew Pinski wrote:

>
> On Sep 24, 2004, at 2:43 PM, Devang Patel wrote:
>> I am not sure this is true in reality. Look at the last 100 patches 
>> posted on gcc-patch. How many of them has bugzilla reference ?
>
> Actually at this point, more than 50% of them do because users are 
> starting
> to use 4.0.0 and file bug reports against it.  But before stage3, 
> there were
> less than 50% of the patches were to file a bug report.

Which means, it is OK to post patches without bugzilla reference.

> Another reason
> why people don't file a bug report before submitting the patch is 
> because
> they have a patch already to fix the bug they were looking into.

same here.

>
>> http://gcc.gnu.org/contribute.html#patches does not ask bugzilla 
>> reference for new features.
>
> But that is because the main use for bugzilla is for users rather than
> developers for filing bugs/enhancements.

And this says that following is not 100% true! :-)

" Yes Bugzilla is used to keep track of feature development and other
enhancements like optimization passes "

Caroline raised good question as part of this discussion and it is 
still unanswered.

-
Devang

[Note: I am not arguing whether each patch should include bugzilla 
reference or not.]

Re: Comments on Patching GCC

[Laurent GUERBY]

On Fri, 2004-09-24 at 20:50, Joseph S. Myers wrote:
> If patch attachments are to be used as a standard means of patch 
> submission for review, then Bugzilla should automatically send those 
> attachments - in plain text and in the body of the message, not a separate 
> MIME part - to gcc-patches, so they can readily be quoted in replies etc..

While I agree that bugzilla should offer the option to the submitter to
flag a patch and comment so it is sent to gcc-patches, I do not see any
reason not to use attachments if it makes the live of bugzilla
developpers easier. 

There was lots of discussion of email clients, I assume that a great
majority of people on this list use clients with decent facilities
for handling attachments (which hopefully also cure the
line wrapping stuff that often mangles patch in body).

With a few patch tracking keywords (eg: patch-testing-requested,
patch-review-requested, patch-needs-improvement, patch-accepted,
patch-approved-with-trivial-change, patch-rejected), it might also
help the reviewer job and may be provide a few helpful statistics
for the GCC project as a whole.

Laurent

Re: Comments on Patching GCC

[Daniel Berlin]

On Fri, 24 Sep 2004, Laurent GUERBY wrote:

> On Fri, 2004-09-24 at 20:50, Joseph S. Myers wrote:
>> If patch attachments are to be used as a standard means of patch
>> submission for review, then Bugzilla should automatically send those
>> attachments - in plain text and in the body of the message, not a separate
>> MIME part - to gcc-patches, so they can readily be quoted in replies etc..
>
> While I agree that bugzilla should offer the option to the submitter to
> flag a patch and comment so it is sent to gcc-patches, I do not see any
> reason not to use attachments if it makes the live of bugzilla
> developpers easier.
>
> There was lots of discussion of email clients, I assume that a great
> majority of people on this list use clients with decent facilities
> for handling attachments (which hopefully also cure the
> line wrapping stuff that often mangles patch in body).
>
> With a few patch tracking keywords (eg: patch-testing-requested,
> patch-review-requested, patch-needs-improvement, patch-accepted,
> patch-approved-with-trivial-change, patch-rejected),

YOu don't need keywords for these, you can actually add flags to 
attachments, request that a flag be set, etc.

< it might also
> help the reviewer job and may be provide a few helpful statistics
> for the GCC project as a whole.
>
> Laurent
>
>
>

Re: Comments on Patching GCC

[Scott Robert Ladd]

Devang Patel wrote:
> 
> On Sep 24, 2004, at 11:23 AM, Andrew Pinski wrote:
> 
> On Sep 24, 2004, at 2:16 PM, Caroline Tice wrote:
>>> Is Bugzilla supposed to be used
>>> to track new feature development as well as existing bugs?  (I really 
>>> don't know; I'm just asking).
>>
>> Yes Bugzilla is used to keep track of feature development and other
>> enhancements like optimization passes
> 
> I am not sure this is true in reality. Look at the last 100 patches 
> posted on gcc-patch. How many of them has bugzilla reference ?

There's also the question of tying gcc-patches threads to bugzilla 
entries. For example: I posted a patch for a bug, and after a 
conversation with Richard Henderson, realized that my solution was too 
simple, and the problem broader than the single issue addressed by the PR.

While I've added a link to the conversation in a bugzilla comment, I'd 
guess that is not done with most patches and debates.

Somehow, we need to integrate discussions of issues with bugzilla 
tracking; otherwise, people are forced to search two places, and they 
may miss important information.

A further question: As I've noted, the PR I addressed has turned up a 
somewhat bigger problem; should the existing PR be expended, or should I 
create a new PR for the new problem domain?

-- 
Scott Robert Ladd
site: http://www.coyotegulch.com
blog: http://chaoticcoyote.blogspot.com