Re: Write after approval additions

Re: Write after approval additions

[Frank Ch. Eigler]

cagney wrote:

: [...]
: Someone gets to add themselves to the write after
: approval list of the MAINTAINERS file when they:
: 
:         o       have recently demonstated an ability
:                 to submit good quality patches to
: 		  gdb-patches.
: 		  (Hint, fix something trivial)
: 
:         o       have an assignment in place
: 
:         o       have OpenSSH and can reach the
:                 repository (so an account can be
:                 created).


It may be instructive to contrast this policy with that of binutils:

#                --------- Write After Approval ---------
# 
# Individuals with "write after approval" have the ability to check in
# changes, but they must get approval for each change from someone in
# one of the above lists (blanket write or maintainers).
# 
# [It's a huge list, folks.  You know who you are.  If you have the
#  *ability* to do binutils checkins, you're in this group.  Just remember
#  to get approval before checking anything in.]


I've always wondered what this specific colour along the power
spectrum was supposed to accomplish.  In what way is it a *useful*
middle point between maintainers and ordinary contributors?

It has the unintended consequence of making more work for maintainers.
They have to check in approved patches themselves instead of letting a
contributor do it.

- FChE

Re: Write after approval additions

[DJ Delorie]

fche@redhat.com (Frank Ch. Eigler) writes:
> It may be instructive to contrast this policy with that of binutils:
> 
> #                --------- Write After Approval ---------
> # 
> # Individuals with "write after approval" have the ability to check in
> # changes, but they must get approval for each change from someone in
> # one of the above lists (blanket write or maintainers).
> # 
> # [It's a huge list, folks.  You know who you are.  If you have the
> #  *ability* to do binutils checkins, you're in this group.  Just remember
> #  to get approval before checking anything in.]
> 
> 
> I've always wondered what this specific colour along the power
> spectrum was supposed to accomplish.

As the author of those paragraphs, I can say with authority that the
reason for the wording is simple:  laziness.

There was *no* list of write-after-approval people for binutils.  All
I had to go on was the cvs permissions list and a long history of
random people checking things in all over the place.

I wouldn't recommend using the binutils wording as an example of the
right way to do it.  I prefer Andrew's policy better, but I wouldn't
be able to implement it for binutils.  Perhaps Nick could/would.

> In what way is it a *useful* middle point between maintainers and
> ordinary contributors?

The maintainers just have to reply with the word "approved" and don't
have to apply the patch, build, test, and fight with cvs to get it in
the repository.  Also helps when a patch covers multiple maintainers'
territories; once both approve it the submitter can check it in to
both places.

Re: Write after approval additions

[Frank Ch. Eigler]

DJ Delorie <dj@redhat.com> writes:

: [...]
: There was *no* list of write-after-approval people for binutils.  All
: I had to go on was the cvs permissions list and a long history of
: random people checking things in all over the place. [...]

Right, but if you need to police CVS commits after the fact, then you
have the same problem.  In other words, the effects of
write-after-approval in the binutils vs gdb senses are hard to
distinguish: if someone checks in approved patches, there is no
problem in either case; if someone checks in unapproved patches, the
same badness occurs in both cases.


: > In what way is it a *useful* middle point between maintainers and
: > ordinary contributors?
: [...]

Sorry, I meant to wonder aloud about the spectrum between maintainers
and those potential contributors who already have CVS access, but are
not on the GDB list, or equivalently, between the gdb vs binutils
senses.


- FChE

Re: Write after approval additions

[Andrew Cagney]

"Frank Ch. Eigler" wrote:

> #                --------- Write After Approval ---------
> #
> # Individuals with "write after approval" have the ability to check in
> # changes, but they must get approval for each change from someone in
> # one of the above lists (blanket write or maintainers).
> #
> # [It's a huge list, folks.  You know who you are.  If you have the
> #  *ability* to do binutils checkins, you're in this group.  Just remember
> #  to get approval before checking anything in.]
> 
> I've always wondered what this specific colour along the power
> spectrum was supposed to accomplish.  In what way is it a *useful*
> middle point between maintainers and ordinary contributors?
> 
> It has the unintended consequence of making more work for maintainers.
> They have to check in approved patches themselves instead of letting a
> contributor do it.

Kind of :-)

Once someones name is in the MAINTAINERS file the people giving approval
can be sure of the above all hold.  Significantly, they can be sure that
the person has a valid assignment.

To turn the question around, how to BFD maintainers know if someone
submitting a patch has a valid GDB assignment in place?

	Andrew

Re: Write after approval additions

[Frank Ch. Eigler]

Hi -

cagney wrote:
: [...]
: Once someones name is in the MAINTAINERS file the people giving approval
: can be sure of the above all hold.  Significantly, they can be sure that
: the person has a valid assignment.
: 
: To turn the question around, how to BFD maintainers know if someone
: submitting a patch has a valid GDB assignment in place?

But that's a separate issue!  Vetting a patch based on whether
the contributor has a filed copyright assignment occurs before 
the patch is approved.  The MAINTAINERS file does not list all
people with assignments.


- FChE
-- 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.4 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iD8DBQE6inj1VZbdDOm/ZT0RAsgUAJ9bWl6ci5rT0PECgtikwm+EdCMeeACfR1t5
vClLhvypau8xkf9AiaKnf3I=
=9NzN
-----END PGP SIGNATURE-----
From cgf@redhat.com Wed Feb 14 06:53:00 2001
From: Christopher Faylor <cgf@redhat.com>
To: gdb-patches@sources.redhat.com
Subject: Re: [RFA] sources.redhat.com -> sources.redhat.com
Date: Wed, 14 Feb 2001 06:53:00 -0000
Message-id: <20010214095356.I16327@redhat.com>
References: <200102132300.SAA17489@panix2.panix.com> <3A89C50B.48405663@cygnus.com>
X-SW-Source: 2001-02/msg00205.html
Content-length: 615

On Tue, Feb 13, 2001 at 06:36:43PM -0500, Andrew Cagney wrote:
>jkingdon@engr.sgi.com wrote:
>> 
>> At first I was going to call this an "obvious fix" as described in
>> MAINTAINERS, but in case I'm wrong, any objections?
>> 
>> 2001-02-13  Jim Kingdon  <jkingdon@engr.sgi.com>
>> 
>>         * CONTRIBUTE, README, TODO: sourceware.cygnus.com ->
>>         sources.redhat.com.  The nice thing about company names is that
>>         there are so many to choose from (don't get me started on logos :-)).
>
>I don't see any real point in the patch.  Why not just leave it until
>gdb.gnu.org is in place.

Ditto.

cgf

Re: Write after approval additions

[Andrew Cagney]

"Frank Ch. Eigler" wrote:
> 
> Hi -
> 
> cagney wrote:
> : [...]
> : Once someones name is in the MAINTAINERS file the people giving approval
> : can be sure of the above all hold.  Significantly, they can be sure that
> : the person has a valid assignment.
> :
> : To turn the question around, how to BFD maintainers know if someone
> : submitting a patch has a valid GDB assignment in place?
> 
> But that's a separate issue!  Vetting a patch based on whether
> the contributor has a filed copyright assignment occurs before
> the patch is approved.  The MAINTAINERS file does not list all
> people with assignments.

So?  It doesn't need to.  It just needs to reflect the current status of
people activly contributing to GDB.

	enjoy,
		Andrew

Re: Write after approval additions

[Frank Ch. Eigler]

Hi -

On Wed, Feb 14, 2001 at 10:46:33AM -0500, Andrew Cagney wrote:
: [...]
: > But that's a separate issue!  Vetting a patch based on whether
: > the contributor has a filed copyright assignment occurs before
: > the patch is approved.  The MAINTAINERS file does not list all
: > people with assignments.
: 
: So?  It doesn't need to.  It just needs to reflect the current status of
: people activly contributing to GDB.

But again, the question was about the value of the various restrictions
on write-after-approval status.  What happens *prior* to patch approval
(e.g., copyright assignment checking) is irrelevant to that question.


- FChE
-- 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.4 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iD8DBQE6iqo8VZbdDOm/ZT0RAhtdAJwJUMsTRrbhrUbdFwQgJcDPDY/FlQCfcpkm
KAYNF8ITTc8hpFxOlDbHL1w=
=S4+z
-----END PGP SIGNATURE-----
From keiths@cygnus.com Wed Feb 14 08:25:00 2001
From: Keith Seitz <keiths@cygnus.com>
To: gdb-patches@sources.redhat.com
Subject: [RFA] Assuming malloc exists in callfwmall.exp
Date: Wed, 14 Feb 2001 08:25:00 -0000
Message-id: <Pine.SOL.3.91.1010214082014.13194C-100000@ryobi.cygnus.com>
X-SW-Source: 2001-02/msg00209.html
Content-length: 4509

(let me try to get the right mailing list this time :-)

Hi,

The problem: When doing an inferior function call with a 
struct/array/string argument (aka 'print foo({2,1})' or 'print 
foo("bar")', gdb requires "malloc" in the executable... This leads to 
some trivial testsuite failures.

2001-02-13  Keith Seitz  <kseitz@nwlink.com>

	* gdb.base/callfwmall.exp: Check for the existence of
	malloc.
	(do_function_calls): Do not do an inferior function
	call which requires malloc if malloc doesn't exist.

Patch:
Index: testsuite/gdb.base/callfwmall.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/callfwmall.exp,v
retrieving revision 1.1.1.3
diff -p -r1.1.1.3 callfwmall.exp
*** testsuite/gdb.base/callfwmall.exp	1999/09/09 00:00:21	1.1.1.3
--- testsuite/gdb.base/callfwmall.exp	2001/02/14 01:11:52
*************** proc set_lang_c {} {
*** 99,105 ****
  proc do_function_calls {} {
      global prototypes
      global gcc_compiled
!     global gdb_prompt
  
      # We need to up this because this can be really slow on some boards.
      set timeout 60;
--- 99,105 ----
  proc do_function_calls {} {
      global prototypes
      global gcc_compiled
!     global gdb_prompt have_malloc_p
  
      # We need to up this because this can be really slow on some boards.
      set timeout 60;
*************** proc do_function_calls {} {
*** 169,183 ****
  
      gdb_test "p t_string_values(string_val2,string_val1)" " = 0"
      gdb_test "p t_string_values(string_val1,string_val2)" " = 1"
!     gdb_test "p t_string_values(\"string 1\",\"string 2\")" " = 1"
!     gdb_test "p t_string_values(\"string 1\",string_val2)" " = 1"
!     gdb_test "p t_string_values(string_val1,\"string 2\")" " = 1"
  
      gdb_test "p t_char_array_values(char_array_val2,char_array_val1)" " = 0"
      gdb_test "p t_char_array_values(char_array_val1,char_array_val2)" " = 1"
!     gdb_test "p t_char_array_values(\"carray 1\",\"carray 2\")" " = 1"
!     gdb_test "p t_char_array_values(\"carray 1\",char_array_val2)" " = 1"
!     gdb_test "p t_char_array_values(char_array_val1,\"carray 2\")" " = 1"
  
      gdb_test "p doubleit(4)" " = 8"
      gdb_test "p add(4,5)" " = 9"
--- 169,187 ----
  
      gdb_test "p t_string_values(string_val2,string_val1)" " = 0"
      gdb_test "p t_string_values(string_val1,string_val2)" " = 1"
!     if {$have_malloc_p} {
!       gdb_test "p t_string_values(\"string 1\",\"string 2\")" " = 1"
!       gdb_test "p t_string_values(\"string 1\",string_val2)" " = 1"
!       gdb_test "p t_string_values(string_val1,\"string 2\")" " = 1"
!     }
  
      gdb_test "p t_char_array_values(char_array_val2,char_array_val1)" " = 0"
      gdb_test "p t_char_array_values(char_array_val1,char_array_val2)" " = 1"
!     if {$have_malloc_p} {
!       gdb_test "p t_char_array_values(\"carray 1\",\"carray 2\")" " = 1"
!       gdb_test "p t_char_array_values(\"carray 1\",char_array_val2)" " = 1"
!       gdb_test "p t_char_array_values(char_array_val1,\"carray 2\")" " = 1"
!     }
  
      gdb_test "p doubleit(4)" " = 8"
      gdb_test "p add(4,5)" " = 9"
*************** proc do_function_calls {} {
*** 222,231 ****
      gdb_test "p t_enum_value2(enum_val2)" " = 1"
      gdb_test "p t_enum_value2(enum_val1)" " = 0"
  
!     gdb_test "p sum_args(1,{2})" " = 2"
!     gdb_test "p sum_args(2,{2,3})" " = 5"
!     gdb_test "p sum_args(3,{2,3,4})" " = 9"
!     gdb_test "p sum_args(4,{2,3,4,5})" " = 14"
      gdb_test "p sum10 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10)" " = 55"
  
      gdb_test "p t_structs_c(struct_val1)" "= 120 'x'" \
--- 226,237 ----
      gdb_test "p t_enum_value2(enum_val2)" " = 1"
      gdb_test "p t_enum_value2(enum_val1)" " = 0"
  
!     if {$have_malloc_p} {
!       gdb_test "p sum_args(1,{2})" " = 2"
!       gdb_test "p sum_args(2,{2,3})" " = 5"
!       gdb_test "p sum_args(3,{2,3,4})" " = 9"
!       gdb_test "p sum_args(4,{2,3,4,5})" " = 14"
!     }
      gdb_test "p sum10 (1, 2, 3, 4, 5, 6, 7, 8, 9, 10)" " = 55"
  
      gdb_test "p t_structs_c(struct_val1)" "= 120 'x'" \
*************** gdb_load ${binfile}
*** 255,260 ****
--- 261,273 ----
  gdb_test "set print sevenbit-strings" ""
  gdb_test "set print address off" ""
  gdb_test "set width 0" ""
+ 
+ # Note whether malloc exists
+ set have_malloc_p 1
+ send_gdb "p malloc\n"
+ gdb_expect {
+   "No symbol \"malloc\""  { set have_malloc_p 0 }
+ }
  
  if { $hp_aCC_compiler } {
      # Do not set language explicitly to 'C'.  This will cause aCC