This is the mail archive of the gdb@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[maint] GDB needs more local maintainers


For various reasons, I agree with Daniel that GDB's current
maintenance process isn't satisfactory in ways that won't be fixed by
faster patch review.  (I'll explain why at the end of this message.)
I don't like his proposed fix of letting global maintainers make
changes to any parts of the code: I'm not sure that it's desirable,
but I also have the purely selfish objection that, as a non-global
maintainer, it won't help me one bit!  It seems to me, however, that
there is one fix that might help significantly and that won't require
any formal changes to the way GDB is run.  Namely:

* Add more local maintainers.  Make it a normal part of the GDB
  process for people who are working actively on an area of GDB to
  become a local maintainer for that area.


Look at the MAINTAINERS file right now: there are 12 global
maintainers, yet there isn't any local area with more than 3
maintainers, and 1 or 2 maintainers is much more common!  This seems,
on the surface of it, to be bizarre: could it really be the case that
there are 12 people whose judgment we trust on any random piece of GDB
code, but that there aren't any little areas of code where we believe
that more than 3 people have good enough judgment to be able to work
with it?  That doesn't make any sense; and a quick scan of the list of
local maintainers reveals many areas where there are knowledgeable
people who aren't local maintainers.


Specifically, I think that anybody who meets these criteria:

* Has worked enough on a piece of code to have a good feel for the
  broad issues involved.

* Has demonstrated a willingness to take responsibility for a piece of
  code.

* Has demonstrated a willingness to ask others for advice when
  appropriate, and to listen to that advice, either modifying or
  withdrawing their suggestions in response to it

be named a local maintainer of that piece of code.

Let me clarify what I mean by these bullet points.  By the first one,
I don't mean an expert on every single line of code (that's a very
high standard, one which current maintainers frequently couldn't
reach).  Rather, I mean something along the lines of "having completed
a large-scale project in that area, and not having required too much
hand-holding to do so".  As for the second bullet point, some ways of
demonstrating responsibility might be either to comment on others'
patches in that area or to have demonstrated some sort of vision of
how the code should look, and what changes might need to be made to
the code to get it there.  I think the third bullet point is pretty
self-explanatory.

The process of being made a local maintainer would be the same for
global maintainers as for random stiffs.  (But, in practice, it would
be easier for global maintainers, because they're more likely to have
the given knowledge and because, if they wouldn't already meet the
third bullet-point above, they wouldn't be global maintainers.)  It
doesn't solve the problem of "global maintainers would like to be able
to make changes that touch lots of code without having to get approval
from every single affected local maintainer"; I think/hope that issue
is less urgent.


I expect that, under this proposal, people would be made local
maintainers is two ways.

* People could get tired of needing approval for their simple patches
  in an area; they could then post a message to gdb@ asking to be made
  a local maintainer, outlining their qualifications.  Other people
  might comment on this application; the local maintainers for that
  area and the global maintainers could then secretly discuss the
  proposal, and approve or deny it (hopefully with constructive
  feedback in the latter case).

* The local maintainers in an area could get tired of always having to
  approve somebody's patches, after observing that that person
  typically submits patches that require little or no corrections.  In
  that case, the local maintainers could privately discuss with each
  other and with the global maintainers the possibility of naming that
  person a local maintainer; if the discussion is positive, and if the
  person agrees, then that person would become a local maintainer.


What would some effects be?  More people would be able to get their
patches through with less effort.  Having more maintainers would also
help non-maintainers, because it would increase the chances that
patches would get reviewed more quickly.

One thing to worry about would be if this would lead to people
committing bad patches that would then have to be reverted; that's why
I included the third qualification above.  Local maintainers would be
expected to put up large changes for comment just like everybody else
would (albeit with less formal approval needed), as they do currently;
if other people disagree with those changes, then they would rework or
withdraw the changes as a matter of course.  We'd just trust the local
maintainers to have good judgment about when to ask for comments.


Some concrete examples.  (If your name isn't listed here, please don't
be offended: I just happen to be listing names of people whom I've
been working with, but I'm sure there are many other equally valuable
candidates in other areas of GDB!)

* Michael Chastain spends more time looking after the testsuite than
  anybody else does, and he certainly knows how to analyze and write
  tests: I think he's an obvious candidate to be added to the list of
  testsuite maintainers.

* Daniel Jacobowitz has just committed a large DWARF 2 patch, and has
  also done good work with general symtab stuff (demangling partial
  symbols, and I seem to recall that he was the person who got blocks
  hashed).  So he seems like a good candidate both as a DWARF
  maintainer and a "generic symtabs" maintainer.  (I suspect he'd be a
  good candidate for a threads maintainer, too, but I haven't followed
  that area so closely.)

* I've read through every single line of linespec.c, and a quick grep
  through ChangeLogs suggests that I've been doing the lion's share of
  recent maintenance to that file.  So I think I would make sense as a
  local maintainer there.  (In fact, after finishing this e-mail, I'll
  send out another e-mail nominating myself as a linespec maintainer.)


So that's my proposal.  The rest of this message is just an aside as
to why I think that there are problems with GDB's approval process
that won't be solved by improved patch tracking.

In an ideal world, the time necessary to approve a patch would be
proportional to how much they would affect GDB and to their size.  But
that's not at all case for the current approval process (if the
submitter is a non-maintainer), even if we had a system that prevented
patches from getting lost through the cracks.  Right now, there are
two classes of patch: obvious and non-obvious.  If a patch is obvious,
you just commit it.  If a patch is non-obvious, you have to wait for
approval; this is a sizeable time cost (even in an ideal
patch-tracking situation, I don't see how to get the average approval
time under a week or so with the current number of local maintainers)
that is independendent of how big your patch is and how much of GDB it
affects.  Of course approval typically takes longer for large patches
than it does for small patches, but the time difference isn't all that
great: the primary factor is still waiting for a maintainer to get
around to looking at your patch at all, unless your patch is really
large and needs a lot of reworking.

This seems to me to create a selection bias in two ways, neither of
which is good:

* It encourages people to write large patches instead of small
  patches.  You'll get your patches approved more quickly if you
  bundle them together as much as possible.

* It encourages patches that add functionality to GDB over patches
  that clean up GDB's existing structure.  Most of us have as our
  primary goal to teach GDB how to do things that it doesn't know how
  to do: if cleanups make that easier, then we're happy to do cleanup
  patches as well, but not if waiting for approval on the cleanup
  patches will significantly delay getting the functional patches in.

I think you can see the effects of this on GDB's code: there are lots
of big functions that have grown by accretion, with the result that
(frequently, at least in my experience) nobody, not even the
maintainers, understands the assumptions that the code is making or
the purpose of all of the code.  The code just gets more and more
unmaintainable.

I don't see how to fix this while still requiring approval for basic
cleanup patches.  The sorts of patches I have in mind are patches that
break up large functions into multiple small functions, just to make
it easier to understand what the functions are doing; these patches
aren't obvious enough that a non-maintainer would feel comfortable
committing them without approval, but they really shouldn't need to go
through the full approval process.  For example, at the end of
<http://sources.redhat.com/ml/gdb-patches/2003-02/msg00504.html>, I
outlined a program for cleaning up the various functions that GDB has
to gather lists of symbols; I'm pretty sure that GDB could be improved
in this area without too much effort, with significant speedups and/or
bug fixes to be found, but it's not going to happen unless a symtab
maintainer does it, because nobody else is likely to be willing to
wait for a week (or maybe a day, but maybe a month) for approval just
to extract a chunk of code into its own function.

Also, I don't want random people to just start "cleaning up" GDB's
code: their idea of what's a cleanup may be misguided.  You definitely
need to build up some level of trustworthiness before being permitted
to do this; being required to be a local maintainers seems like a good
compromise to me, as long as it isn't too difficult to become a local
maintainer.

David Carlton
carlton at math dot stanford dot edu


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]