MAINTAINERS policy question

MAINTAINERS policy question

[Bernd Schmidt]

I have a question about how the MAINTAINERS file should be interpreted.
Suppose you have somebody listed as maintaining port X.  Does this grant
the right to check in any patch for a problem that affects port X, or
does it only grant the right to modify config/X/*?

I've had someone claim in private mail that it means the former, and
this surprised me, since I always assumed it meant the latter.


Bernd

Re: MAINTAINERS policy question

[Andreas Jaeger]

Bernd Schmidt <bernds@redhat.com> writes:

> I have a question about how the MAINTAINERS file should be interpreted.
> Suppose you have somebody listed as maintaining port X.  Does this grant
> the right to check in any patch for a problem that affects port X, or
> does it only grant the right to modify config/X/*?
>
> I've had someone claim in private mail that it means the former, and
> this surprised me, since I always assumed it meant the latter.

The MAINTAINERS file states:

  Note individuals who maintain parts of the compiler need approval to
  check in changes outside of the parts of the compiler they maintain.

The cvswrite.html page states under "Write access policies":
   Localized write permission.
          This is for people who have primary responsibility for ports,
          front ends, or significant hunks of code in the compiler. These
          folks are allowed to make changes in code they maintain without
          going through any kind of approval process.

I interpret this so that if somebody maintains port X it only gives
check-in rights for config/X/*,

Andreas
-- 
 Andreas Jaeger
  SuSE Labs aj@suse.de
   private aj@arthur.inka.de
    http://www.suse.de/~aj

Re: MAINTAINERS policy question

[Graham Stott]

Andreas Jaeger wrote:
> 
> Bernd Schmidt <bernds@redhat.com> writes:
> 
> > I have a question about how the MAINTAINERS file should be interpreted.
> > Suppose you have somebody listed as maintaining port X.  Does this grant
> > the right to check in any patch for a problem that affects port X, or
> > does it only grant the right to modify config/X/*?
> >
> > I've had someone claim in private mail that it means the former, and
> > this surprised me, since I always assumed it meant the latter.

> 
> I interpret this so that if somebody maintains port X it only gives
> check-in rights for config/X/*,
Yes I agree with this interpretation.

I'm surprised that anyone is claiming otherwise.
 

> 
> Andreas
> --
>  Andreas Jaeger
>   SuSE Labs aj@suse.de
>    private aj@arthur.inka.de
>     http://www.suse.de/~aj

Graham

Re: MAINTAINERS policy question

[Mark Mitchell]

> I interpret this so that if somebody maintains port X it only gives
> check-in rights for config/X/*,

That has been my interpretation as well.

(I would extend it to documentation that affects X, config.foo files
that affect X, and so forth, but certainly not, say, to the register
allocator code just because the register allocator happens to have
a bug that affects X.)

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: MAINTAINERS policy question

[Andreas Jaeger]

Mark Mitchell <mark@codesourcery.com> writes:

>> I interpret this so that if somebody maintains port X it only gives
>> check-in rights for config/X/*,
>
> That has been my interpretation as well.
>
> (I would extend it to documentation that affects X, config.foo files
> that affect X, and so forth, but certainly not, say, to the register
> allocator code just because the register allocator happens to have
> a bug that affects X.)

Based on your and Joseph's comments, I've made the appended patch for
our web pages.

If there is consensus on this, I'd like to get permission to commit
it.

I think the MAINTAINERS file needs no update - but we could add the
paragraph there also if people think it's needed,

Andreas

--- cvswrite.html.~1~	Thu Jul 26 16:10:30 2001
+++ cvswrite.html	Sun Sep 16 20:13:24 2001
@@ -98,7 +98,13 @@
   primary responsibility for ports, front ends, or significant hunks
   of code in the compiler.  These folks are allowed to make changes in
   code they maintain without going through any kind of approval
-  process.</dd>
+  process.
+
+  <p>
+  Maintainers of a port maintain the files in config/&lt;port&gt;/,
+  the configure fragments for the port, documentation for the port and
+  test cases for features or bugs specific to this port.  Port
+  maintainers do not have approval writes in other files.  </dd>
 
   <dt>Write after approval.</dt> <dd>This is folks that make regular
   contributions, but do not fall into one of the two previous

-- 
 Andreas Jaeger
  SuSE Labs aj@suse.de
   private aj@arthur.inka.de
    http://www.suse.de/~aj

Re: MAINTAINERS policy question

[Mark Mitchell]

> If there is consensus on this, I'd like to get permission to commit
> it.

It's fine by me, but you should probably get approval from a couple
more SC members to make sure that I am not speaking nonsense.

Thanks,

--
Mark Mitchell                   mark@codesourcery.com
CodeSourcery, LLC               http://www.codesourcery.com

Re: MAINTAINERS policy question

[Toon Moene]

Mark Mitchell wrote:

> > If there is consensus on this, I'd like to get permission to commit
> > it.
> 
> It's fine by me, but you should probably get approval from a couple
> more SC members to make sure that I am not speaking nonsense.

It's OK with me.

-- 
Toon Moene - mailto:toon@moene.indiv.nluug.nl - phoneto: +31 346 214290
Saturnushof 14, 3738 XG  Maartensdijk, The Netherlands
Maintainer, GNU Fortran 77: http://gcc.gnu.org/onlinedocs/g77_news.html
Join GNU Fortran 95: http://g95.sourceforge.net/ (under construction)

Re: MAINTAINERS policy question

[Gerald Pfeifer]

On Sun, 16 Sep 2001, Andreas Jaeger wrote:
> If there is consensus on this, I'd like to get permission to commit
> it.

I also understood it like that, and as now (at least) three maintainers
with write privilege for wwwdocs/htdocs/* agree, I'd say go ahead and
commit the patch.

(Please replace the single <p> by two <p>block1</p><p>block2</p>,
though.)

Thanks,
Gerald
-- 
Gerald "Jerry" pfeifer@dbai.tuwien.ac.at http://www.dbai.tuwien.ac.at/~pfeifer/

Re: MAINTAINERS policy question

[David Edelsohn]

>>>>> Mark Mitchell writes:

Mark> I would extend it to documentation that affects X, config.foo files
Mark> that affect X, and so forth...

	What about collect2.c?  What about parts of libstdc++?  What about
libtool?  What about config.gcc?  There is no clear definition of "so
forth".  We only have obscured the ambiguity which remains.

	Remember that there are a lot of machine-dependent infrastructure
pieces distributed throughout the common files in GCC which were
implemented for just one target or only a few targets.  The register
allocator clearly is global, common infrastructure and the config files
clearly are local to a port, but some of GCC falls in the gray area
inbetween.  It is fallacious reasoning to generalize from specific
components like the register allocator to all components throughout the
entire compiler.

	There is a difference between policy and practice.  I propose that
the policy should remain liberal while continuing to be implemented more
narrowly in practice.  In other words, one waits for approval from the
maintainer of a component or someone with global write privilege as a
courtesy.

	I think Bernd's original question is ill-formed and is generating
an inaccurate response.  GCC is not implemented with the clear dichotomy
that the question of "any patch affecting port X versus config/X/*"
implies.  Simplistic, hasty answers to a complex question intertwined with
GCC's design diverse target support will not help GCC development, IMHO.

David

Re: MAINTAINERS policy question

[Phil Edwards]

On Sun, Sep 16, 2001 at 05:13:44PM -0400, David Edelsohn wrote:
> >>>>> Mark Mitchell writes:
> 
> Mark> I would extend it to documentation that affects X, config.foo files
> Mark> that affect X, and so forth...
> 
> 	What about collect2.c?  What about parts of libstdc++?  What about
> libtool?  What about config.gcc?  There is no clear definition of "so
> forth".  We only have obscured the ambiguity which remains.

Well, when Mark switched libstdc++-v3 to be the official C++ library,
he spoke to this point:

    http://gcc.gnu.org/ml/gcc/2000-11/msg00522.html

Look about seven paragraphs down, the one with "Port maintainers w/o global
write access," etc.

It's been libstdc++'s informal policy to treat those config pieces,
and the relevent chunks of configure.{host,target}, as part of a port,
and therefore under a port maintainer's control.  Most of the maintainers
run patches past Benjamin anyhow as an extra sanity check, which is good.


Phil

-- 
Would I had phrases that are not known, utterances that are strange, in
new language that has not been used, free from repetition, not an utterance
which has grown stale, which men of old have spoken.
                                     - anonymous Egyptian scribe, c.1700 BC

Re: MAINTAINERS policy question

[Joseph S. Myers]

On Sun, 16 Sep 2001, Bernd Schmidt wrote:

> I have a question about how the MAINTAINERS file should be interpreted.
> Suppose you have somebody listed as maintaining port X.  Does this grant
> the right to check in any patch for a problem that affects port X, or
> does it only grant the right to modify config/X/*?

I'd say it means config/X/*, plus the entries for X in config.gcc, plus
documentation for X (options in invoke.texi, attributes in extend.texi,
specific issues in install.texi, ...), plus testcases for any feature or
bug specific to X (probably plus configuration for X in libstdc++-v3 /
libffi / ...).  (But not anything X-specific in config.* or libtool, since
those should be imported unmodified where possible.)  But not changes to 
generic files for bugs that affect X.

-- 
Joseph S. Myers
jsm28@cam.ac.uk

Re: MAINTAINERS policy question

[Joe Buck]

> I have a question about how the MAINTAINERS file should be interpreted.
> Suppose you have somebody listed as maintaining port X.  Does this grant
> the right to check in any patch for a problem that affects port X, or
> does it only grant the right to modify config/X/*?
> 
> I've had someone claim in private mail that it means the former, and
> this surprised me, since I always assumed it meant the latter.

The port maintainer will also need to deal with other port-specific
components, for example the processor-specific atomicity code in libstdc++, 

Re: MAINTAINERS policy question

[Ulrich Weigand]

Joseph S. Myers wrote:

>On Sun, 16 Sep 2001, Bernd Schmidt wrote:
>
>> I have a question about how the MAINTAINERS file should be interpreted.
>> Suppose you have somebody listed as maintaining port X.  Does this grant
>> the right to check in any patch for a problem that affects port X, or
>> does it only grant the right to modify config/X/*?
>
>I'd say it means config/X/*, plus the entries for X in config.gcc, plus
>documentation for X (options in invoke.texi, attributes in extend.texi,
>specific issues in install.texi, ...), plus testcases for any feature or
>bug specific to X (probably plus configuration for X in libstdc++-v3 /
>libffi / ...).

Is there consensus on this policy?  I'm asking becausing I'm still waiting
for approval for some changes to the S/390-specific config.gcc entries
(which I thought I needed approval for ...).  See

http://gcc.gnu.org/ml/gcc-patches/2001-08/msg00683.html



Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
  Dr. Ulrich Weigand
  Linux for S/390 Design & Development
  IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
  Phone: +49-7031/16-3727   ---   Email: Ulrich.Weigand@de.ibm.com

Re: MAINTAINERS policy question

[DJ Delorie]

> Is there consensus on this policy?  I'm asking becausing I'm still
> waiting for approval for some changes to the S/390-specific
> config.gcc entries (which I thought I needed approval for ...).  See
> http://gcc.gnu.org/ml/gcc-patches/2001-08/msg00683.html

Insofaras config.gcc changes are part of configure.in for which I can
approve changes, consider the config.gcc changes approved, and in the
future, please consider the platform-specific entries in config.gcc to
be one of the things the platform maintainers maintain (provided
you're careful, you test before committing, and you don't break anyone
else, of course ;)

And a month is a long time to wait.  I'd think two weeks with no
response at all would be long enough before posting an "unreviewed
patch" note to the list.  If it's my area, a private ping after a week
just to make I know about the patch is OK by me too.

Re: MAINTAINERS policy question

[Richard Kenner]

    >I'd say it means config/X/*, plus the entries for X in config.gcc, plus
    >documentation for X (options in invoke.texi, attributes in extend.texi,
    >specific issues in install.texi, ...), plus testcases for any feature or
    >bug specific to X (probably plus configuration for X in libstdc++-v3 /
    >libffi / ...).

    Is there consensus on this policy?  

That's how I always interpreted it.

Re: MAINTAINERS policy question

[Daniel Berlin]

On Sunday, September 16, 2001, at 09:45 AM, Richard Kenner wrote:

>> I'd say it means config/X/*, plus the entries for X in config.gcc, plus
>> documentation for X (options in invoke.texi, attributes in extend.texi,
>> specific issues in install.texi, ...), plus testcases for any feature 
>> or
>> bug specific to X (probably plus configuration for X in libstdc++-v3 /
>> libffi / ...).
>
>     Is there consensus on this policy?
>
> That's how I always interpreted it.
Just to present a differing viewpoint (though i'm not the one who 
emailed Bernd), i've always interpreted it to be the way that surprised 
Bernd.
After all, anyone with enough experience to be maintaining a port, it's 
likely they know enough to be able to make changes to other parts of the 
compiler that affect their platform.
But I could see how the viewpoint you guys have is perfectly reasonable.
Maybe we should write down whatever the consensus is?

Re: MAINTAINERS policy question

[Richard Kenner]

    After all, anyone with enough experience to be maintaining a port, it's 
    likely they know enough to be able to make changes to other parts of the 
    compiler that affect their platform.

Somewhat.  But one point of a reviewer is to look at a change and see if
it is "reasonable".  You have to have a global knowlege of the compiler
to do that if the change is in a common area.  Local knowlege of that area
isn't enough since the issue may be that the proper place for the change
is elsewhere.  It is possible to do a port by just doing a very small number
(or even no) changes to the rest of the compiler, so there's no reason to
assume that a port maintainer would had gained such expertese.

Re: MAINTAINERS policy question

[Richard Kenner]

    +  Maintainers of a port maintain the files in config/<port>/,
    +  the configure fragments for the port, documentation for the port and
    +  test cases for features or bugs specific to this port.  Port
    +  maintainers do not have approval writes in other files.  </dd>

Typo: "... approval rights in ...

Re: MAINTAINERS policy question

[Andreas Jaeger]

kenner@vlsi1.ultra.nyu.edu (Richard Kenner) writes:

>     +  Maintainers of a port maintain the files in config/<port>/,
>     +  the configure fragments for the port, documentation for the port and
>     +  test cases for features or bugs specific to this port.  Port
>     +  maintainers do not have approval writes in other files.  </dd>
>
> Typo: "... approval rights in ...

Thanks, I've fixed this.

Andreas
-- 
 Andreas Jaeger
  SuSE Labs aj@suse.de
   private aj@arthur.inka.de
    http://www.suse.de/~aj

Re: MAINTAINERS policy question

[Ulrich Weigand]

DJ Delorie wrote:

>> Is there consensus on this policy?  I'm asking becausing I'm still
>> waiting for approval for some changes to the S/390-specific
>> config.gcc entries (which I thought I needed approval for ...).  See
>> http://gcc.gnu.org/ml/gcc-patches/2001-08/msg00683.html
>
>Insofaras config.gcc changes are part of configure.in for which I can
>approve changes, consider the config.gcc changes approved, and in the
>future, please consider the platform-specific entries in config.gcc to
>be one of the things the platform maintainers maintain (provided
>you're careful, you test before committing, and you don't break anyone
>else, of course ;)

OK, thanks.

>And a month is a long time to wait.  I'd think two weeks with no
>response at all would be long enough before posting an "unreviewed
>patch" note to the list.  If it's my area, a private ping after a week
>just to make I know about the patch is OK by me too.

I'll do it that way in the future.  Thanks again ...


Mit freundlichen Gruessen / Best Regards

Ulrich Weigand

--
  Dr. Ulrich Weigand
  Linux for S/390 Design & Development
  IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
  Phone: +49-7031/16-3727   ---   Email: Ulrich.Weigand@de.ibm.com

Re: MAINTAINERS policy question

[mike stump]

> From: Andreas Jaeger <aj@suse.de>
> Date: Sun, 16 Sep 2001 20:14:57 +0200

> --- cvswrite.html.~1~	Thu Jul 26 16:10:30 2001
> +++ cvswrite.html	Sun Sep 16 20:13:24 2001
> @@ -98,7 +98,13 @@
>    primary responsibility for ports, front ends, or significant hunks
>    of code in the compiler.  These folks are allowed to make changes in
>    code they maintain without going through any kind of approval
> -  process.</dd>
> +  process.
> +
> +  <p>
> +  Maintainers of a port maintain the files in config/&lt;port&gt;/,
> +  the configure fragments for the port, documentation for the port and
> +  test cases for features or bugs specific to this port.  Port
> +  maintainers do not have approval writes in other files.  </dd>

I mostly agree as well.

The part about testcases and bugs specific to a port is a little to
general.  Basically, a port maintainer has privs for all those bytes
that _only_ affect ports under his control, if they are normally done
in such a manner.

This would mean that a bug fix to libgcc2.c in:

#if defined (sony_news) && defined (SYSTYPE_BSD)

#include <stdio.h>
#include <sys/types.h>
#include <sys/param.h>
#include <syscall.h>
#include <machine/sysnews.h>

/* cacheflush function for NEWS-OS 4.2.
   This function is called from trampoline-initialize code
   defined in config/mips/mips.h.  */

void
cacheflush (char *beg, int size, int flag)
{
  if (syscall (SYS_sysnews, NEWS_CACHEFLUSH, beg, size, FLUSH_BCACHE))
    {
      perror ("cache_flush");
      fflush (stderr);
      abort ();
    }
}

#endif /* sony_news */

would be OK by the sony_news/mips maintainer.  My wording also makes
it clear that an FreeBSD maintainer can change the FreeBSD make file
fragments, but not the Linux parts of the x86 files.  Adding a

#ifdef __mips
[ hack around ]
#endif

into MI code, would not be acceptable, unless signed off by a
maintainer of the MI code.

Testcases are potentially hard to decide.  A testcase _should_ be done
in a fairly MI way.  A port maintainer can miss ways in which a
testcase is machine dependent.  Having someone else review the
testcase is reasonably a good idea.  A port maintainer that can
generate a MI testcase I think should be allowed to just check it in
directly.

I am in favor of having things loosely defined, and leaving a little
wiggle room.  Those people that are new/inexperienced should caution
to the conservative side, those that are very experienced should be
granted just a little bit more leeway in what parts they change.

Re: MAINTAINERS policy question

[Marc Espie]

In article < 200109180038.RAA27823@kankakee.wrs.com > you write:
>would be OK by the sony_news/mips maintainer.  My wording also makes
>it clear that an FreeBSD maintainer can change the FreeBSD make file
>fragments, but not the Linux parts of the x86 files.  Adding a

I'm looking at the 3.0.1 MAINTAINER list. There is no FreeBSD maintainer,
nor a Linux maintainer, nor an OpenBSD maintainer.

In fact, considering the current discussion, what's the interpretation of
`port' ? I don't think it's unnatural to consider it might mean `processor'
(current interpretation) but may also mean `os' or `processor+os'.

The area is fairly simple to define.

If I don't presume too much, I would like to be relisted a maintainer for
the OpenBSD ports (the OpenBSD specific part, of course, by your rules).