[9fans] proposal: a patch acceptance system

[9fans] proposal: a patch acceptance system

[andrey mirtchovski]

Hello,

Several people have discussed on IRC the need for a definitive way of
submitting patches (enhancements and fixes) to Plan 9 developers with write
privileges. Currently this happens in three different ways -- sending
patches to 9fans, sending patches (fixes mostly) to 9trouble, and via
personal communication with people who can do a push on sources.

What this mechanism lacks is an easy way to get feedback on rejected or
accepted patches, and a way for developers to discriminate good and bad
ones and to know which patches have been outstanding.

We think the following solution could be sufficiently simple and general to
accomodate our needs:

Enhancements and fixes by people lacking the ability to 'push' on sources
could be put (by the users themselves) in a world-writeable directory
"/patches" on sources.cs.bell-labs.com. A particular set of patched files
submitted by a user will be located in:

	/patches/MMDD.user/*

where MM is the month, DD is the day and 'user' is the username of the
person submitting the patch, as it appears on the sources auth server.

Along with the changed files the user must submit a short README describing
in detail what this patch accomplishes, and where those files appear
relevant to the root of a Plan 9 system tree (no need to create complex
directory structures just to put a couple of files there).

Whether an announcement is made to 9fans or not is left for the user to
decide.

Plan 9 developers can then examine /patch for outstanding patches and either
add them to the system, in which case the MMDD.user directory can disappear
(it will be available in the next pull) or reject them, in which case they
go to:

	/patches/rejected/MMDD.user/

with a small 'reason' file added, where the reason for the rejection is
given. This could be made private and viewable only by the person who
submitted the patch.

One major issue with this scheme is synchronization -- making sure the files
submitted patch the latest system is left as an exercise for the patch
submitter, and making sure there's no "push"-ing of files for which there
are patches outstanding is done by the person doing the "push" (it's just a
simple "ls /patches/*/file" before "push -c /path/to/file" is done on
sources)...

Any coments will be appreciated, as long as you have in mind that we're
trying to cut short of writing a new bugzilla :)

andrey

Re: [9fans] proposal: a patch acceptance system

[Russ Cox]

The main problem with the current system (email 9trouble) is that
the reading of 9trouble itself is spotty.  But if we end up with a
different patch submission system there's no reason to believe the
problem would go away.  I was reading 9trouble pretty regularly
before the summer, but then it got away from me.  Dave has been
keeping up pretty well otherwise.

When we don't like a patch, we reply and tell you.  If you're not
getting replies it means we're behind.  There's still a lot of good
stuff sitting in the 9trouble box that I just have never gotten to.
I think what you're trying to address is the not knowing whether
we're busy or we silently deleted your bad idea.  But we don't silently
delete bad ideas.  It just seems that way since we get behind sometimes.

I don't think the code drift is too big a concern.  If you include
in the files the copy you are changing, then it's no big deal to
generate the diffs and apply them to the current file.
Your solution of checking for patches every time you update
a file isn't accurate -- we make the changes in our local
tree and then eventually they make it to sources.

It sounds like more than anything else what you want is a way to
check on your submitted changes, which is entirely reasonable.

Sitting on the other side of the fence, I want an easy way to
accept patches, basically a button to push when I think it's good
that will apply the change.  And when they're not good, I want
an easy way to send people back to the drawing board with
constructive comments (like, "make the code style look like all the
other code so your patch doesn't stick out like a sore thumb",
though that hasn't happened recently).

I built a simple patch tracking system based on your proposal.
Let's see how it works.  See patch(1).

Russ

Re: [9fans] proposal: a patch acceptance system

[andrey mirtchovski]

that seems to be the right solution, thanx!

Re: [9fans] proposal: a patch acceptance system

[Michael Giegerich]

andrey mirtchovski:
> that seems to be the right solution, thanx!

I' afraid, I cannot concur :-(
I think this approach lacks transparency. You never know
if somebody is not up reading his/her mail/news or if
the article didn't make it to the relevant places or if
it's being ignored at purpose or or or...

I would welcome a patch submission, acknowledging and
feedbacking system...

--
Michael Giegerich

Re: [9fans] proposal: a patch acceptance system

[kazumi iwane]

On 2003.11.3, at 05:23 Japan, Russ Cox wrote:
> I built a simple patch tracking system based on your proposal.
> Let's see how it works.  See patch(1).

Thanks for a new set of tools. I tried it today (I found a minor
problem with
ki command) and got this:

term% patch/create ki-error /sys/src/cmd/ki/stats.c
mkdir: can't create /n/sources/patch/ki-error: read-only filesystem
mkdir /n/sources/patch/ki-error/ki-error failed
term%

Do I need to be in a special group on sources to be able to submit
patches?

-- kazumi

Re: [9fans] proposal: a patch acceptance system

[Russ Cox]

> term% patch/create ki-error /sys/src/cmd/ki/stats.c
> mkdir: can't create /n/sources/patch/ki-error: read-only filesystem
> mkdir /n/sources/patch/ki-error/ki-error failed
> term%
>
> Do I need to be in a special group on sources to be able to submit
> patches?

yes.  oops.  i'll have to rethink this.
i can't make a world-writable directory
on sources because then things like the
logs would also be world-writable, which
i definitely do not want.

the ki compile error is fixed anyway.
(pull again.)

Re: [9fans] proposal: a patch acceptance system

[Russ Cox]

> I' afraid, I cannot concur :-(
> I think this approach lacks transparency. You never know
> if somebody is not up reading his/her mail/news or if
> the article didn't make it to the relevant places or if
> it's being ignored at purpose or or or...
>
> I would welcome a patch submission, acknowledging and
> feedbacking system...

this is what andrey was talking about.
how does it not address these?

     PATCH(1)                                                 PATCH(1)

     NAME
          patch - simple patch creation and tracking system

     SYNOPSIS
          patch/create name files ... [ < description ]

          patch/list [ name ... ]

          patch/diff name

          patch/apply name

          patch/undo name

          patch/note name [ < note ]

     DESCRIPTION
          These scripts are a simple patch submission and tracking
          system used to propose additions or changes to Plan 9.  Each
          patch has a name and is stored in /n/sources/patch/name.

          Patch/create creates a new patch consisting of the changes
          to the listed files, reading a description of the patch from
          standard input.

          Patch/list displays information about the named patches, or
          all currently pending patches if none are specified.

          Patch/diff shows a patch as diffs between the original
          source files and the patched source files.

          Patch/apply applies the patch to the current source tree.
          It is intended to be run by the Plan 9 developers with
          emelie as their root file system.  If the source has changed
          since the patch was created, apply will report the conflict
          and not change any files.  Before changing any files,
          patch/apply makes backup copies of the current source tree's
          files.  The backups are stored in the patch directory.

          Patch/undo will copy the backups saved by patch/apply back
          into the source tree.  It will not restore a backup if the
          file being replaced is not byte-identical to the one created
          by patch/apply.

     EXAMPLES
          Propose a change to pwd, which you have modified locally:

               % patch/create pwd-errors /sys/src/cmd/pwd.c
               Fix pwd to print errors to fd 2 rather than 1.
               ^D
               %

          Then the developers at Bell Labs run

               patch/diff pwd-errors

          to inspect the change (possibly viewing
          /n/sources/patch/pwd-errors/pwd.c to see the larger con-
          text).  To make the change, they run

               patch/apply pwd-errors

          Otherwise they run

               % patch/note pwd-errors
               Pwd should definitely print errors to fd 1 because ...
               ^D
               %

          to add a note to the /n/sources/pwd-errors/notes file.

     FILES
          /n/sources/patch

     SOURCE
          /rc/bin/patch

     SEE ALSO
          diff(1)

Re: [9fans] proposal: a patch acceptance system

[Fco.J.Ballesteros]

[-- Attachment #1: Type: text/plain, Size: 71 bytes --]

Perhaps a tiny but different fs in the same machine could do the job.

[-- Attachment #2: Type: message/rfc822, Size: 2421 bytes --]

From: "Russ Cox" <rsc@swtch.com>
To: 9fans@cse.psu.edu
Subject: Re: [9fans] proposal: a patch acceptance system
Date: Mon, 03 Nov 2003 10:01:24 -0500
Message-ID: <E1AGgD0-000GoQ-3I@t40.swtch.com>

> term% patch/create ki-error /sys/src/cmd/ki/stats.c
> mkdir: can't create /n/sources/patch/ki-error: read-only filesystem
> mkdir /n/sources/patch/ki-error/ki-error failed
> term%
>
> Do I need to be in a special group on sources to be able to submit
> patches?

yes.  oops.  i'll have to rethink this.
i can't make a world-writable directory
on sources because then things like the
logs would also be world-writable, which
i definitely do not want.

the ki compile error is fixed anyway.
(pull again.)

Re: [9fans] proposal: a patch acceptance system

[Michael Giegerich]

Russ Cox:
> > I' afraid, I cannot concur :-(
> > I think this approach lacks transparency. You never know
> > if somebody is not up reading his/her mail/news or if
> > the article didn't make it to the relevant places or if
> > it's being ignored at purpose or or or...
> >
> > I would welcome a patch submission, acknowledging and
> > feedbacking system...
>
> this is what andrey was talking about.

You're right. Somehow I didn't get this right the 1st time.

--
Michael Giegerich

Re: [9fans] proposal: a patch acceptance system

[a]

i understand (and second, as far as it goes) presotto's concern
regarding matching patch versions. what the scheme andry described
would rely on in order to avoid that is some level of responsability
on the part of the patch submitter to make sure they're patching
current code, some care on the part of sources-pushers to see if
there's outstanding patches their changes are affecting (in which
case they should at least notify the submitter), and some promptness
on the part of whoever's responsable for the code being affected.
certainly this is a bit of attention by all parties, but i think it's
the best way (of suggestions i've heard) to make sure changes can get
put in or rejected quickly, and the reasons for rejections understood,
without putting a very large burdon on any one of the three above
parties.
ア

Re: [9fans] proposal: a patch acceptance system

[andrey mirtchovski]

One possibility is creating yet another group on sources -- one where patch
submitters can be added. It's easy then for a person not in any such group
to cat /adm/users and send a patch to one of the people already in, or if
they're more serious to ask to be added to the group.

If Plan 9's userbase suddenly starts growing exponentially this could solve
the problems, otherwise the simple restriction that one needs a Plan 9
installation running to access /patch is going to be enough :)

andrey

On Sun, 2 Nov 2003, David Presotto wrote:

> I'm of mixed minds.  If you send something to 9trouble, I will respond fairly
> quickly modulo not being around for a week every now and then.  If you send
> something to 9fans, I could take a while because I tend to miss things
> when a flame war is going on.  I would suggest that contributions at least
> get sent to 9trouble and, if you want wider immediate distribution, to
> 9fans.  I'm also not averse to leaving a patch area on sources that people
> can indescriminately dump into.  I don't understand how to avoid abuse problems,
> but I would draft a 9grid like policy (i.e. don't be stupid) and leave it in
> an obvious place.  However, I don't want to turn this into Linux where you need
> something like debian to find a set of patches (or CVS's) that actually make
> sense together.

Re: [9fans] proposal: a patch acceptance system

[David Presotto]

[-- Attachment #1: Type: text/plain, Size: 786 bytes --]

I'm of mixed minds.  If you send something to 9trouble, I will respond fairly
quickly modulo not being around for a week every now and then.  If you send
something to 9fans, I could take a while because I tend to miss things
when a flame war is going on.  I would suggest that contributions at least
get sent to 9trouble and, if you want wider immediate distribution, to
9fans.  I'm also not averse to leaving a patch area on sources that people
can indescriminately dump into.  I don't understand how to avoid abuse problems,
but I would draft a 9grid like policy (i.e. don't be stupid) and leave it in
an obvious place.  However, I don't want to turn this into Linux where you need
something like debian to find a set of patches (or CVS's) that actually make
sense together.

[-- Attachment #2: Type: message/rfc822, Size: 4591 bytes --]

From: andrey mirtchovski <mirtchov@cpsc.ucalgary.ca>
To: 9fans@cse.psu.edu
Subject: [9fans] proposal: a patch acceptance system
Date: Sun, 2 Nov 2003 10:01:54 -0700 (MST)
Message-ID: <Pine.LNX.4.44.0311020929200.11290-100000@fbsd.cpsc.ucalgary.ca>

Hello,

Several people have discussed on IRC the need for a definitive way of
submitting patches (enhancements and fixes) to Plan 9 developers with write
privileges. Currently this happens in three different ways -- sending
patches to 9fans, sending patches (fixes mostly) to 9trouble, and via
personal communication with people who can do a push on sources.

What this mechanism lacks is an easy way to get feedback on rejected or
accepted patches, and a way for developers to discriminate good and bad
ones and to know which patches have been outstanding.

We think the following solution could be sufficiently simple and general to
accomodate our needs:

Enhancements and fixes by people lacking the ability to 'push' on sources
could be put (by the users themselves) in a world-writeable directory
"/patches" on sources.cs.bell-labs.com. A particular set of patched files
submitted by a user will be located in:

	/patches/MMDD.user/*

where MM is the month, DD is the day and 'user' is the username of the
person submitting the patch, as it appears on the sources auth server.

Along with the changed files the user must submit a short README describing
in detail what this patch accomplishes, and where those files appear
relevant to the root of a Plan 9 system tree (no need to create complex
directory structures just to put a couple of files there).

Whether an announcement is made to 9fans or not is left for the user to
decide.

Plan 9 developers can then examine /patch for outstanding patches and either
add them to the system, in which case the MMDD.user directory can disappear
(it will be available in the next pull) or reject them, in which case they
go to:

	/patches/rejected/MMDD.user/

with a small 'reason' file added, where the reason for the rejection is
given. This could be made private and viewable only by the person who
submitted the patch.

One major issue with this scheme is synchronization -- making sure the files
submitted patch the latest system is left as an exercise for the patch
submitter, and making sure there's no "push"-ing of files for which there
are patches outstanding is done by the person doing the "push" (it's just a
simple "ls /patches/*/file" before "push -c /path/to/file" is done on
sources)...

Any coments will be appreciated, as long as you have in mind that we're
trying to cut short of writing a new bugzilla :)

andrey