From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21015 invoked by alias); 24 Feb 2003 20:36:03 -0000 Mailing-List: contact gdb-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sources.redhat.com Received: (qmail 20995 invoked from network); 24 Feb 2003 20:35:54 -0000 Received: from unknown (HELO jackfruit.Stanford.EDU) (171.64.38.136) by 172.16.49.205 with SMTP; 24 Feb 2003 20:35:54 -0000 Received: (from carlton@localhost) by jackfruit.Stanford.EDU (8.11.6/8.11.6) id h1OJm4M18361; Mon, 24 Feb 2003 11:48:04 -0800 X-Authentication-Warning: jackfruit.Stanford.EDU: carlton set sender to carlton@math.stanford.edu using -f To: gdb Subject: [maint] GDB needs more local maintainers From: David Carlton Date: Mon, 24 Feb 2003 20:36:00 -0000 Message-ID: User-Agent: Gnus/5.0808 (Gnus v5.8.8) XEmacs/21.4 (Common Lisp) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2003-02/txt/msg00528.txt.bz2 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 , 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@math.stanford.edu