2007-08-13 10:48:17

by Joe Perches

[permalink] [raw]
Subject: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

Add file pattern to MAINTAINER entry

Signed-off-by: Joe Perches <[email protected]>

diff --git a/MAINTAINERS b/MAINTAINERS
index 29a2179..5ae90a2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4215,6 +4215,9 @@ P: Rafael J. Wysocki
M: [email protected]
L: [email protected]
S: Supported
+F: Documentation/power/swsusp*
+F: arch/i386/power/swsusp.S
+F: kernel/power/

SUSPEND TO RAM:
P: Pavel Machek


2007-08-13 20:41:28

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

On Monday, 13 August 2007 08:36, [email protected] wrote:
> Add file pattern to MAINTAINER entry
>
> Signed-off-by: Joe Perches <[email protected]>
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 29a2179..5ae90a2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4215,6 +4215,9 @@ P: Rafael J. Wysocki
> M: [email protected]
> L: [email protected]
> S: Supported
> +F: Documentation/power/swsusp*
> +F: arch/i386/power/swsusp.S
> +F: kernel/power/

Well, the list isn't complete. Please add

arch/i386/power/
arch/x86_64/kernel/suspend_asm.S
arch/x86_64/kernel/suspend.c
drivers/base/power/

There also is a couple of headers. Do you want to list them too?

>
> SUSPEND TO RAM:
> P: Pavel Machek
>
>

Greetings,
Rafael

--
"Premature optimization is the root of all evil." - Donald Knuth

2007-08-13 20:57:49

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

On Mon, 2007-08-13 at 22:50 +0200, Rafael J. Wysocki wrote:
> Please add
> arch/i386/power/
> arch/x86_64/kernel/suspend_asm.S
> arch/x86_64/kernel/suspend.c
> drivers/base/power/
> There also is a couple of headers. Do you want to list them too?

Your choice. Just let me know.

HIBERNATION (aka Software Suspend, aka swsusp):
P: Pavel Machek
M: [email protected]
P: Rafael J. Wysocki
M: [email protected]
L: [email protected]
S: Supported
F: Documentation/power/swsusp*
F: arch/i386/power/
F: arch/x86_64/kernel/suspend*
F: drivers/base/power/
F: kernel/power/



2007-08-13 21:15:42

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

On Monday, 13 August 2007 22:54, Joe Perches wrote:
> On Mon, 2007-08-13 at 22:50 +0200, Rafael J. Wysocki wrote:
> > Please add
> > arch/i386/power/
> > arch/x86_64/kernel/suspend_asm.S
> > arch/x86_64/kernel/suspend.c
> > drivers/base/power/
> > There also is a couple of headers. Do you want to list them too?
>
> Your choice. Just let me know.

+ include/linux/suspend.h
+ include/linux/freezer.h
+ include/linux/pm.h
+ include/asm-*/suspend.h

Hm, they are also relevant for suspend to RAM, so perhaps one common
list of files for suspend to RAM and hibernation would be better?

> HIBERNATION (aka Software Suspend, aka swsusp):
> P: Pavel Machek
> M: [email protected]
> P: Rafael J. Wysocki
> M: [email protected]
> L: [email protected]
> S: Supported
> F: Documentation/power/swsusp*
> F: arch/i386/power/
> F: arch/x86_64/kernel/suspend*
> F: drivers/base/power/
> F: kernel/power/
>
>
>
>
>

--
"Premature optimization is the root of all evil." - Donald Knuth

2007-08-13 21:23:42

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

On Mon, 2007-08-13 at 23:25 +0200, Rafael J. Wysocki wrote:
> + include/linux/suspend.h
> + include/linux/freezer.h
> + include/linux/pm.h
> + include/asm-*/suspend.h
>
> Hm, they are also relevant for suspend to RAM, so perhaps one common
> list of files for suspend to RAM and hibernation would be better?

Maybe just collapse the 2 maintainers blocks into one?

HIBERNATION (aka Software Suspend, aka swsusp) and SUSPEND TO RAM:
P: Pavel Machek
M: [email protected]
P: Rafael J. Wysocki
M: [email protected]
L: [email protected]
S: Supported
F: Documentation/power/swsusp*
F: arch/i386/kernel/acpi/
F: arch/i386/power/
F: arch/x86_64/kernel/acpi/
F: arch/x86_64/kernel/suspend_asm.S
F: arch/x86_64/kernel/suspend.c
F: drivers/base/power/
F: kernel/power/
F: include/linux/suspend.h
F: include/linux/freezer.h
F: include/linux/pm.h
F: include/asm-*/suspend.h


2007-08-13 21:29:21

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

On Monday, 13 August 2007 23:21, Joe Perches wrote:
> On Mon, 2007-08-13 at 23:25 +0200, Rafael J. Wysocki wrote:
> > + include/linux/suspend.h
> > + include/linux/freezer.h
> > + include/linux/pm.h
> > + include/asm-*/suspend.h
> >
> > Hm, they are also relevant for suspend to RAM, so perhaps one common
> > list of files for suspend to RAM and hibernation would be better?
>
> Maybe just collapse the 2 maintainers blocks into one?
>
> HIBERNATION (aka Software Suspend, aka swsusp) and SUSPEND TO RAM:
> P: Pavel Machek
> M: [email protected]
> P: Rafael J. Wysocki
> M: [email protected]
> L: [email protected]
> S: Supported

Then I think it should be "Maintained".

> F: Documentation/power/swsusp*

F: Documentation/power/

> F: arch/i386/kernel/acpi/
> F: arch/i386/power/
> F: arch/x86_64/kernel/acpi/
> F: arch/x86_64/kernel/suspend_asm.S
> F: arch/x86_64/kernel/suspend.c
> F: drivers/base/power/
> F: kernel/power/
> F: include/linux/suspend.h
> F: include/linux/freezer.h
> F: include/linux/pm.h
> F: include/asm-*/suspend.h

Pavel, what do you think?


--
"Premature optimization is the root of all evil." - Donald Knuth

2007-08-14 11:43:47

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

Hi!

> > > + include/linux/suspend.h
> > > + include/linux/freezer.h
> > > + include/linux/pm.h
> > > + include/asm-*/suspend.h
> > >
> > > Hm, they are also relevant for suspend to RAM, so perhaps one common
> > > list of files for suspend to RAM and hibernation would be better?
> >
> > Maybe just collapse the 2 maintainers blocks into one?

No, please don't.

> > HIBERNATION (aka Software Suspend, aka swsusp) and SUSPEND TO RAM:
> > P: Pavel Machek
> > M: [email protected]
> > P: Rafael J. Wysocki
> > M: [email protected]
> > L: [email protected]
> > S: Supported
>
> Then I think it should be "Maintained".

No, supported is still right :-).
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2007-08-14 17:27:48

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

On Tuesday, 14 August 2007 09:05, Pavel Machek wrote:
> Hi!
>
> > > > + include/linux/suspend.h
> > > > + include/linux/freezer.h
> > > > + include/linux/pm.h
> > > > + include/asm-*/suspend.h
> > > >
> > > > Hm, they are also relevant for suspend to RAM, so perhaps one common
> > > > list of files for suspend to RAM and hibernation would be better?
> > >
> > > Maybe just collapse the 2 maintainers blocks into one?
>
> No, please don't.

So, I think the F: entries should be the following:

(1) hibernation
F:??????Documentation/power/
F:??????arch/i386/power/
F:??????arch/x86_64/kernel/suspend_asm.S
F:??????arch/x86_64/kernel/suspend.c
F:??????drivers/base/power/
F:??????kernel/power/
F:??????include/linux/suspend.h
F:??????include/linux/freezer.h
F:??????include/linux/pm.h
F:??????include/asm-*/suspend.h

(2) suspend
F:??????Documentation/power/
F:??????arch/i386/kernel/acpi/
F:??????arch/x86_64/kernel/acpi/
F:??????arch/x86_64/kernel/suspend.c
F:??????drivers/base/power/
F:??????kernel/power/
F:??????include/linux/suspend.h
F:??????include/linux/freezer.h
F:??????include/linux/pm.h
F:??????include/asm-*/suspend.h

> > > HIBERNATION (aka Software Suspend, aka swsusp) and SUSPEND TO RAM:
> > > P: Pavel Machek
> > > M: [email protected]
> > > P: Rafael J. Wysocki
> > > M: [email protected]
> > > L: [email protected]
> > > S: Supported
> >
> > Then I think it should be "Maintained".
>
> No, supported is still right :-).

OK

Greetings,
Rafael

2007-08-14 17:58:10

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

On Tue, 2007-08-14 at 19:37 +0200, Rafael J. Wysocki wrote:
> (1) hibernation
> F: Documentation/power/
> F: arch/i386/power/
> F: arch/x86_64/kernel/suspend_asm.S
> F: arch/x86_64/kernel/suspend.c
> F: drivers/base/power/
> F: kernel/power/
> F: include/linux/suspend.h
> F: include/linux/freezer.h
> F: include/linux/pm.h
> F: include/asm-*/suspend.h
>
> (2) suspend
> F: Documentation/power/
> F: arch/i386/kernel/acpi/
> F: arch/x86_64/kernel/acpi/
> F: arch/x86_64/kernel/suspend.c
> F: drivers/base/power/
> F: kernel/power/
> F: include/linux/suspend.h
> F: include/linux/freezer.h
> F: include/linux/pm.h
> F: include/asm-*/suspend.h

SUSPEND TO RAM:
P: Pavel Machek
M: [email protected]
P: Rafael J. Wysocki
M: [email protected]
L: [email protected]
S: Maintained
F: Documentation/power/
F: arch/i386/kernel/acpi/
F: arch/x86_64/kernel/acpi/
F: arch/x86_64/kernel/suspend.c
F: drivers/base/power/
F: kernel/power/
F: include/linux/suspend.h
F: include/linux/freezer.h
F: include/linux/pm.h
F: include/asm-*/suspend.h

HIBERNATION (aka Software Suspend, aka swsusp)
P: Pavel Machek
M: [email protected]
P: Rafael J. Wysocki
M: [email protected]
L: [email protected]
S: Supported
F: Documentation/power/
F: arch/i386/power/
F: arch/x86_64/kernel/suspend_asm.S
F: arch/x86_64/kernel/suspend.c
F: drivers/base/power/
F: kernel/power/
F: include/linux/suspend.h
F: include/linux/freezer.h
F: include/linux/pm.h
F: include/asm-*/suspend.h


2007-08-14 18:05:05

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

On Tuesday, 14 August 2007 19:57, Joe Perches wrote:
> On Tue, 2007-08-14 at 19:37 +0200, Rafael J. Wysocki wrote:
> > (1) hibernation
> > F: Documentation/power/
> > F: arch/i386/power/
> > F: arch/x86_64/kernel/suspend_asm.S
> > F: arch/x86_64/kernel/suspend.c
> > F: drivers/base/power/
> > F: kernel/power/
> > F: include/linux/suspend.h
> > F: include/linux/freezer.h
> > F: include/linux/pm.h
> > F: include/asm-*/suspend.h
> >
> > (2) suspend
> > F: Documentation/power/
> > F: arch/i386/kernel/acpi/
> > F: arch/x86_64/kernel/acpi/
> > F: arch/x86_64/kernel/suspend.c
> > F: drivers/base/power/
> > F: kernel/power/
> > F: include/linux/suspend.h
> > F: include/linux/freezer.h
> > F: include/linux/pm.h
> > F: include/asm-*/suspend.h
>
> SUSPEND TO RAM:
> P: Pavel Machek
> M: [email protected]
> P: Rafael J. Wysocki
> M: [email protected]
> L: [email protected]
> S: Maintained
> F: Documentation/power/
> F: arch/i386/kernel/acpi/
> F: arch/x86_64/kernel/acpi/
> F: arch/x86_64/kernel/suspend.c
> F: drivers/base/power/
> F: kernel/power/
> F: include/linux/suspend.h
> F: include/linux/freezer.h
> F: include/linux/pm.h
> F: include/asm-*/suspend.h
>
> HIBERNATION (aka Software Suspend, aka swsusp)
> P: Pavel Machek
> M: [email protected]
> P: Rafael J. Wysocki
> M: [email protected]
> L: [email protected]
> S: Supported
> F: Documentation/power/
> F: arch/i386/power/
> F: arch/x86_64/kernel/suspend_asm.S
> F: arch/x86_64/kernel/suspend.c
> F: drivers/base/power/
> F: kernel/power/
> F: include/linux/suspend.h
> F: include/linux/freezer.h
> F: include/linux/pm.h
> F: include/asm-*/suspend.h

ACK

2007-08-14 18:16:56

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):



On Tue, 14 Aug 2007, Joe Perches wrote:
>
> SUSPEND TO RAM:
> P: Pavel Machek
> M: [email protected]
> P: Rafael J. Wysocki
> M: [email protected]
> L: [email protected]
> S: Maintained
> F: Documentation/power/
> F: arch/i386/kernel/acpi/
> F: arch/x86_64/kernel/acpi/
> F: arch/x86_64/kernel/suspend.c
> F: drivers/base/power/
> F: kernel/power/
> F: include/linux/suspend.h
> F: include/linux/freezer.h
> F: include/linux/pm.h
> F: include/asm-*/suspend.h

Quite frankly, I think the MAINTAINERS file gets a whole lot uglier this
way.

There's also a rather fundamental issue: this will likely make people
touch the MAINTAINERS file *more*, and from a maintenance standpoint, that
is exactly the wrong thing to have (one central file that everybody
touches). It just tends to generate unnecessary merge conflicts etc.

(We used to have that issue with the central configuration file, for
example).

So the more I look at these things, the more convinced I am that this is
not the right thing to do. These things should *not* be in one huge file,
and I'd much much rather have the maintainership information be carried
along with the subsystem itself, or the files it contains.

In other words, it would be much better to just have per-file markers,
along with some per-subdirectory stuff or similar.

Linus

2007-08-14 18:24:19

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

On Tue, 14 Aug 2007 11:15:41 -0700 (PDT)
Linus Torvalds <[email protected]> wrote:

> In other words, it would be much better to just have per-file markers,
> along with some per-subdirectory stuff or similar.

And a `make maintainers' target to pull it all together..

(perhaps we could add a

maintainer <keyword>

record to Kconfig, then `make maintainers' goes and looks up <keyword>
somewhere and does something with it)

2007-08-14 18:30:07

by Rene Herman

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

On 08/14/2007 08:15 PM, Linus Torvalds wrote:

> Quite frankly, I think the MAINTAINERS file gets a whole lot uglier this
> way.
>
> There's also a rather fundamental issue: this will likely make people
> touch the MAINTAINERS file *more*, and from a maintenance standpoint, that
> is exactly the wrong thing to have (one central file that everybody
> touches). It just tends to generate unnecessary merge conflicts etc.
>
> (We used to have that issue with the central configuration file, for
> example).
>
> So the more I look at these things, the more convinced I am that this is
> not the right thing to do. These things should *not* be in one huge file,
> and I'd much much rather have the maintainership information be carried
> along with the subsystem itself, or the files it contains.
>
> In other words, it would be much better to just have per-file markers,
> along with some per-subdirectory stuff or similar.

Joe just now convinced me that rather than the per-file markers, the marker
is meta-information that could just be stored in GIT, with the MAINTAINERS
file turning into something generated.

git info --maintainer and such (for many possible kinds of --flags) would
throw out the information.

Rene.

2007-08-14 18:44:28

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

On Tue, 2007-08-14 at 11:15 -0700, Linus Torvalds wrote:
> Quite frankly, I think the MAINTAINERS file gets a whole lot uglier this
> way.

Me too.
Chopping up the current file is simple.

How about keeping the whole thing in git?
Please look at thread:
[PATCH] [1/2many] - Find the maintainer(s) for a patch - scripts/get_maintainer.pl

> In other words, it would be much better to just have per-file markers,
> along with some per-subdirectory stuff or similar.

Perhaps a single top-level MAINTAINERS subdirectory with
500+ files or MAINTAINERS per subdirectory.

Doesn't matter to me.

I'd prefer no per-file external markers,
nor embedded per-file keywords.

2007-08-14 23:25:29

by Dave Jones

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

On Tue, Aug 14, 2007 at 11:22:37AM -0700, Andrew Morton wrote:
> On Tue, 14 Aug 2007 11:15:41 -0700 (PDT)
> Linus Torvalds <[email protected]> wrote:
>
> > In other words, it would be much better to just have per-file markers,
> > along with some per-subdirectory stuff or similar.
>
> And a `make maintainers' target to pull it all together..
>
> (perhaps we could add a
>
> maintainer <keyword>
>
> record to Kconfig, then `make maintainers' goes and looks up <keyword>
> somewhere and does something with it)

Not everything that's in MAINTAINERS has a Kconfig entry though,
so it really needs to live in the .c/.h files.

Dave


--
http://www.codemonkey.org.uk

2007-08-15 01:45:46

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

On Tue, 2007-08-14 at 11:15 -0700, Linus Torvalds wrote:
> In other words, it would be much better to just have per-file markers,
> along with some per-subdirectory stuff or similar.

So that there would be no hot single file, I cut the
MAINTAINER file into single file segments in maintainers/*

00_descriptions
3c359_network_driver
3c505_network_driver
3c59x_network_driver
3cr990_network_driver
...
zd1211rw_wireless_driver
zf_machz_watchdog
zr36067_video_for_linux_driver
zs_decstation_z85c30_serial_driver
zz_the_rest

611 files.

How could "make" make a single MAINTAINERS?

"cat [0-9a-z]* > ../MAINTAINERS"?

Would it need to?
Anyone have suggestions for Makefile/Kconfig support?

2007-08-15 02:51:49

by Kyle Moffett

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

On Aug 14, 2007, at 19:24:30, Dave Jones wrote:
> On Tue, Aug 14, 2007 at 11:22:37AM -0700, Andrew Morton wrote:
>> On Tue, 14 Aug 2007 11:15:41 -0700 (PDT) Linus Torvalds
>> <[email protected]> wrote:
>>> In other words, it would be much better to just have per-file
>>> markers, along with some per-subdirectory stuff or similar.
>>
>> And a `make maintainers' target to pull it all together..
>>
>> (perhaps we could add a
>>
>> maintainer <keyword>
>>
>> record to Kconfig, then `make maintainers' goes and looks up
>> <keyword> somewhere and does something with it)
>
> Not everything that's in MAINTAINERS has a Kconfig entry though, so
> it really needs to live in the .c/.h files.

How about making "MAINTAINERS" operate vaguely similar
to .gitignore? You would need 4 pieces

(a) A set of "Maintainers" files sprinkled around the source tree
where they make sense. Any file references would be done using
relative paths and patterns. For example, there would be one in the
root directory which has:

[EVERYTHING ELSE]
P: Various Linux Kernel Developers
L: [email protected]
F: *


Then (using the earlier SUSPEND TO RAM example) in the kernel/power/
Maintainers file, you would have:

[SUSPEND TO RAM]
P: Pavel Machek
M: [email protected]
P: Rafael J. Wysocki
M: [email protected]
L: [email protected]
S: Maintained
F: *


Now that at least *one* of the maintainers files has the info for
Pavel and Rafael, you could just use this simpler form in any other
Maintainers file and still have it find their entries:

[SUSPEND TO RAM]
F: linux/suspend.h
F: linux/freezer.h
F: linux/pm.h
F: asm-*/suspend.h


(b) You would need a little tool which generates a combined
MAINTAINERS file when "make maintainers" is run. It would iterate
over the directory tree and combine entries with the same names.
This also allows you to group people with their associated files even
if they work on the same subsystem/driver; they would be listed in
the respective sub-Maintainer-files, but when built it would mention
both of them. The intent would not be a MAINTAINERS file which is
perfectly human-readable, it would be one which can be efficiently
grepped by a helper tool to find the necessary information. When the
resulting MAINTAINERS file is built it would include the source
"Maintainers" file for each chunk right before said chunk, for example:

[SUSPEND TO RAM]

Origin: kernel/power/Maintainers
P: Pavel Machek
M: [email protected]
P: Rafael J. Wysocki
M: [email protected]
L: [email protected]
S: Maintained
F: kernel/power/*

Origin: include/Maintainers
F: include/linux/suspend.h
F: include/linux/freezer.h
F: include/linux/pm.h
F: include/asm-*/suspend.h



(c) You would need a tool to go digging through the built
MAINTAINERS file based on a file, an email address, a subsystem-name-
regexp, etc. It would return all matching entries, with the desired
fields user-selectable.


(d) You would need a little tool to poke at git similar to the shell
script Linus posted which dug through the recent commit history
looking for people doing significant *original* modifications (IE:
first person to sign-off) on code for which they aren't a maintainer,
as well as "Maintainers" who haven't recently signed off at all on
code for which they are responsible. The output might be something
like this:

> ## Historical significance: 6 months
> ## Uncategorized file threshold: >10 changes or >20 sign-offs
> ## New maintainer threshold: >20 changes or >40 sign-offs
> ## Neglectful-maintainer threshold: <5 changes and <10 sign-offs
>
> [SUSPEND TO RAM]
> Nigel Cunningham should probably be a maintainer:
> kernel/power/* (130 changes, 132 sign-offs)
> include/linux/suspend.h (29 changes, 29 sign-offs)
>
> [RANDOM UNMAINTAINED DRIVER]
> J. Random Hacker has neglected his maintainership:
> drivers/random/unmaintained.c (0 changes, 0 sign-offs)
>
> [UNCATEGORIZED]
> John Linville should probably add/update a Maintainers entry:
> drivers/wireless/newly_added_driver.c (142 changes and 453 sign-
> offs)


Hopefully that kind of tool would make it a hundred times easier to
keep an eye out for out-dated entries as well as missing new
entries. The obsolescence-verification would be based on the rapidly-
moving git changelog, so it would pick up new maintainers quite
quickly, as well as noticing uncategorized source files and
neglectful maintainers.

Soon as I get moved into my new place I'll see what kind of Perl
scripts I can hack up


Cheers,
Kyle Moffett

2007-08-15 11:12:11

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] [443/2many] MAINTAINERS - HIBERNATION (aka Software Suspend, aka swsusp):

On Wednesday, 15 August 2007 04:51, Kyle Moffett wrote:
> On Aug 14, 2007, at 19:24:30, Dave Jones wrote:
> > On Tue, Aug 14, 2007 at 11:22:37AM -0700, Andrew Morton wrote:
> >> On Tue, 14 Aug 2007 11:15:41 -0700 (PDT) Linus Torvalds
> >> <[email protected]> wrote:
> >>> In other words, it would be much better to just have per-file
> >>> markers, along with some per-subdirectory stuff or similar.
> >>
> >> And a `make maintainers' target to pull it all together..
> >>
> >> (perhaps we could add a
> >>
> >> maintainer <keyword>
> >>
> >> record to Kconfig, then `make maintainers' goes and looks up
> >> <keyword> somewhere and does something with it)
> >
> > Not everything that's in MAINTAINERS has a Kconfig entry though, so
> > it really needs to live in the .c/.h files.
>
> How about making "MAINTAINERS" operate vaguely similar
> to .gitignore? You would need 4 pieces
>
> (a) A set of "Maintainers" files sprinkled around the source tree
> where they make sense. Any file references would be done using
> relative paths and patterns. For example, there would be one in the
> root directory which has:
>
> [EVERYTHING ELSE]
> P: Various Linux Kernel Developers
> L: [email protected]
> F: *
>
>
> Then (using the earlier SUSPEND TO RAM example) in the kernel/power/
> Maintainers file, you would have:
>
> [SUSPEND TO RAM]
> P: Pavel Machek
> M: [email protected]
> P: Rafael J. Wysocki
> M: [email protected]
> L: [email protected]
> S: Maintained
> F: *
>
>
> Now that at least *one* of the maintainers files has the info for
> Pavel and Rafael, you could just use this simpler form in any other
> Maintainers file and still have it find their entries:
>
> [SUSPEND TO RAM]
> F: linux/suspend.h
> F: linux/freezer.h
> F: linux/pm.h
> F: asm-*/suspend.h
>
>
> (b) You would need a little tool which generates a combined
> MAINTAINERS file when "make maintainers" is run. It would iterate
> over the directory tree and combine entries with the same names.
> This also allows you to group people with their associated files even
> if they work on the same subsystem/driver; they would be listed in
> the respective sub-Maintainer-files, but when built it would mention
> both of them. The intent would not be a MAINTAINERS file which is
> perfectly human-readable, it would be one which can be efficiently
> grepped by a helper tool to find the necessary information. When the
> resulting MAINTAINERS file is built it would include the source
> "Maintainers" file for each chunk right before said chunk, for example:
>
> [SUSPEND TO RAM]
>
> Origin: kernel/power/Maintainers
> P: Pavel Machek
> M: [email protected]
> P: Rafael J. Wysocki
> M: [email protected]
> L: [email protected]
> S: Maintained
> F: kernel/power/*
>
> Origin: include/Maintainers
> F: include/linux/suspend.h
> F: include/linux/freezer.h
> F: include/linux/pm.h
> F: include/asm-*/suspend.h
>
>
>
> (c) You would need a tool to go digging through the built
> MAINTAINERS file based on a file, an email address, a subsystem-name-
> regexp, etc. It would return all matching entries, with the desired
> fields user-selectable.
>
>
> (d) You would need a little tool to poke at git similar to the shell
> script Linus posted which dug through the recent commit history
> looking for people doing significant *original* modifications (IE:
> first person to sign-off) on code for which they aren't a maintainer,
> as well as "Maintainers" who haven't recently signed off at all on
> code for which they are responsible. The output might be something
> like this:
>
> > ## Historical significance: 6 months
> > ## Uncategorized file threshold: >10 changes or >20 sign-offs
> > ## New maintainer threshold: >20 changes or >40 sign-offs
> > ## Neglectful-maintainer threshold: <5 changes and <10 sign-offs
> >
> > [SUSPEND TO RAM]
> > Nigel Cunningham should probably be a maintainer:
> > kernel/power/* (130 changes, 132 sign-offs)
> > include/linux/suspend.h (29 changes, 29 sign-offs)
> >
> > [RANDOM UNMAINTAINED DRIVER]
> > J. Random Hacker has neglected his maintainership:
> > drivers/random/unmaintained.c (0 changes, 0 sign-offs)
> >
> > [UNCATEGORIZED]
> > John Linville should probably add/update a Maintainers entry:
> > drivers/wireless/newly_added_driver.c (142 changes and 453 sign-
> > offs)

I like this idea. :-)

Greetings,
Rafael

2007-08-16 13:05:31

by Kyle Moffett

[permalink] [raw]
Subject: Storing Maintainers info around the kernel tree

Merging a couple related threads here:

On Aug 16, 2007, at 07:57:23, Rene Herman wrote:
> On 08/16/2007 01:26 PM, Salikh Zakirov wrote:
>> Rene Herman wrote:
>>> Perhaps that immediately suggests an implementation to someone
>>> already familiar with git internals?
>> perhaps http://www.kernel.org/pub/software/scm/git/docs/
>> gitattributes.html and http://www.kernel.org/pub/software/scm/git/
>> docs/git-check-attr.html can help you?
>
> No, thanks, saw them, but .gitattributes is in fact in the same
> category as .gitignore, which would _be_ a property.
>
> If you do this stuff in files scattered around the tree, updating
> and moving stuff becomes a pain -- the tool would need to go edit
> files.

From a practical standpoint we don't want to duplicate someone's
maintainer information in the attributes of every file they
maintain. It would be much easier to put in the "kernel/somesubsys"
directory a Maintainers file which has:

[SOME RANDOM SUBSYSTEM]
P: J. Random Hacker
M: j.random.hacker@localhost
L: [email protected]
F: *

Anywhere else you had files that you wanted to associate with J.
Random Hacker's maintainership, you would just use:

[SOME RANDOM SUBSYSTEM]
F: somesubsys.h


I posted a comment describing a mechanism like this a couple days ago:
http://lkml.org/lkml/2007/8/14/488

Executive overview:
On Aug 15, 2007, at 07:21:04, Rafael J. Wysocki wrote:
> On Wednesday, 15 August 2007 04:51, Kyle Moffett wrote:
>> (a) "Maintainers" files sprinkled around the source tree with
>> relative pathnames and other data
>>
>> (b) Tool to generate a combined "MAINTAINERS" file from the ones
>> sprinkled around the source tree
>>
>> (c) Tool to search through the generated "MAINTAINERS" file with
>> all sorts of useful command-line options
>>
>> (d) Tool to check the generated "MAINTAINERS" file against recent
>> git history and make suggestions
>
> I like this idea. :-)

Well, to back up this idea with some code, I'm attaching a little
perl script which does part (b). Basically you call it as:
./maint-combine $(find . -name Maintainers)

It will print any syntax errors on stderr during parsing. Once it's
done it will dump to stdout its combined "MAINTAINERS" text. A
couple notes:

* It uses a little "priority" system to figure out what order to
print the data from each origin in. For example, the "F:" tag is
given a score of 0, to force data consisting of just files towards
the end. The "P:", "M:", and "L:" tags are given scores of 5, since
people are generally interesting to know about. Everything else is
given a score of "1". The scores are added up per ($file,
$subsystem) pair and then during printing each subsystem's data is
ordered by score (highest comes first).

* It generally allows any field at all; eventually we might want to
limit it to a fixed list to help avoid typos.

* It has a little bit of magic logic for the "F:" field so that it
figures out the relative directory for each field when generating the
output. For example, an entry of "asm-*/suspend.h" in a file
"include/Maintainers" will produce the output file entry: "F: include/
asm-*/suspend.h"

* The format isn't quite the same as the current MAINTAINERS file,
to make parsing easier and more dummy-proof I changed the syntax for
a subsystem-name to use square brackets (IE: "[SUSPEND TO RAM]").
The samples I gave in my previous email are what I used to test it
with, plus a little dummy file with some syntax errors to check out
the error messages:

Maintainers:
> [EVERYTHING ELSE]
> P: Various Linux Kernel Developers
> L: [email protected]
> F: *

kernel/power/Maintainers:
> [SUSPEND TO RAM]
> P: Pavel Machek
> M: [email protected]
> P: Rafael J. Wysocki
> M: [email protected]
> L: [email protected]
> S: Maintained
> F: *

include/Maintainers:
> [SUSPEND TO RAM]
> F: linux/suspend.h
> F: linux/freezer.h
> F: linux/pm.h
> F: asm-*/suspend.h



If you have any other questions, the perl script is pretty self-
explanatory and I'll be completely back online this weekend. With
any luck I'll have some time in a hotel tomorrow (mmm, slow-as-dirt
hotel wireless, what fun) to work on parts (c) and (d).

Cheers,
Kyle Moffett


Attachments:
maint-combine.txt (1.99 kB)

2007-08-16 15:18:28

by Rene Herman

[permalink] [raw]
Subject: Re: Storing Maintainers info around the kernel tree

On 08/16/2007 03:04 PM, Kyle Moffett wrote:

> On Aug 16, 2007, at 07:57:23, Rene Herman wrote:

>> category as .gitignore, which would _be_ a property.
>>
>> If you do this stuff in files scattered around the tree, updating and
>> moving stuff becomes a pain -- the tool would need to go edit files.
>
> From a practical standpoint we don't want to duplicate someone's
> maintainer information in the attributes of every file they maintain.

In a tree structure, you don't have to. As described earlier, the tool
(git-prop) looks for the requested property being set first on the file
itself, then on the directory in which it resides, then its parent, and so
on. If I read things right, this is also how properties work in subversion
in fact.

So after

$ git prop --set --name maintainer --value \
"Bartlomiej Zolnierkiewicz <[email protected]>" drivers/ide/
and

$ git prop --set --name maintainer --value \
"Alan Cox <[email protected]>" drivers/ide/ide-cd.*

we get:

$ git prop --get --name maintainer drivers/ide/ide-cd.c
Alan Cox <[email protected]>

$ git prop --get --name maintainer drivers/ide/ide-generic.c
Bartlomiej Zolnierkiewicz <[email protected]>

Now, this override behaviour needs a tree structure ofcourse, but notice I
set the "maintainer" property only to the name/address. The other
information from the MAINTAINERS file would be using their own properties:

$ git prop --set --name tree --value \
"quilt kernel.org/pub/linux/kernel/people/bart/pata-2.6/" drivers/ide/

and nothing under drivers/ide/ would override this value nor would it be
repeated anywhere. Alan takes care of more than ide-cd but only the actual
"maintainer" value string would be set on the others as well and repeating
that much for different "maintenance units" is no different from the current
MAINTAINERS file where it also is (well, would be, Alan is in fact only
listed for ide-cd it seems...) repeated in different entries.

(as a slight difference -- in the above example, Alan's information _is_
repeated over ide-cd.c and ide-cd.h where the current MAINTAINERS file just
says "IDE/ATAPI CDROM DRIVER", but that's a bit of an oddbal situation since
you normally have either single files or a tree that make up a "maintenance
unit" -- and is in fact just a human versus tool difference).

> It would be much easier to put in the "kernel/somesubsys" directory a
> Maintainers file which has:

It's ofcourse possible, but note that if we want this stuff to be minimally
manual, moving files around (and deleting them) then requires editing these
actual in-tree files via a tool.

With the properties deleting files just requires deleting any file-specific
properties alongside which is trivial since those are linked from the file.

Moving stuff works by building a list of all properties that are set on the
source starting at the source and destination's highest shared parent
directory and then reconstructing this list at the destination, striking
properties off the list that are already set at the destination.

Adding properties, alongside added files or after the fact, could be done
via standard patch submissals via the kind of "meta-diff" that already
exists for "git move".

I really believe this stuff should be meta-data -- and these properties as
outlined work well it seems.

$ git prop --set --name git.ignore -V ./.gitignore .
$ rm .gitignore

This is something I saw subversion also uses properties for. Takes the value
from a file instead of the command line. .gitattributes are also easily
incorporated into the property-system directly.

$ git prop --set --name git.executable scripts/Lindent

Must say I'm not particularly sure if this one has much value over the
current executable bit storage,but also from svn and example of a boolean
property.

$ git prop --set --name license --value "GPL v2" .
$ git prop --set --name license --value "GPL" sound/alsa/

and so on. The GPL v2 on the source root only works if you set the property
on everything that's not, so you may not want to but as a "wouldn't it be
nice if" kinda thing. Makes for easy license analysis at least...

$ git prop --set --name FIXME drivers/block/floppy.c

Okay, that's probably overdoing it a bit, but as long as I'm having fun here...

Note -- the properties would be versioned themselves ofcourse so that you'd
always have a tree where data and meta-data matched. Basically, I believe
you'd view the properties as just more data files, one per property, with
the exception that they'd not actually live in the working tree and are
linked from data (files/directories) that do.

Long "letters of intent" this, but I'm by now in love with these things.
More comments (or implementations obviously :-) welcome. Any significant
misses in this?

Rene.

2007-08-16 15:31:41

by Alan Stern

[permalink] [raw]
Subject: Re: [linux-pm] Re: Storing Maintainers info around the kernel tree

On Thu, 16 Aug 2007, Rene Herman wrote:

> > It would be much easier to put in the "kernel/somesubsys" directory a
> > Maintainers file which has:
>
> It's ofcourse possible, but note that if we want this stuff to be minimally
> manual, moving files around (and deleting them) then requires editing these
> actual in-tree files via a tool.
>
> With the properties deleting files just requires deleting any file-specific
> properties alongside which is trivial since those are linked from the file.
>
> Moving stuff works by building a list of all properties that are set on the
> source starting at the source and destination's highest shared parent
> directory and then reconstructing this list at the destination, striking
> properties off the list that are already set at the destination.
>
> Adding properties, alongside added files or after the fact, could be done
> via standard patch submissals via the kind of "meta-diff" that already
> exists for "git move".
>
> I really believe this stuff should be meta-data -- and these properties as
> outlined work well it seems.

Please remember that not everybody uses git. The MAINTAINERS data
should be available in the kernel source itself.

(Maybe your suggestion is consistent with this -- I simply wanted to
raise the point.)

Alan Stern

2007-08-16 15:54:58

by Rene Herman

[permalink] [raw]
Subject: Re: [linux-pm] Re: Storing Maintainers info around the kernel tree

On 08/16/2007 05:31 PM, Alan Stern wrote:

> Please remember that not everybody uses git. The MAINTAINERS data
> should be available in the kernel source itself.

It may be useful to generate a MAINTAINERS file into releases yes.

I must say though that "why?" would also be a question. I personally don't
think there's a whole lot wrong with more and more expecting people who
submit patches (for whom this automation is intended) to be using git. Back
in the BK days there were lots of reasons for resisting any and all
dependency on the source code management tool but there don't seem to be too
many left today as far as I'm concerned.

If it's about non-developer users, I suspect it would to a fairly large
degree be an "in theory" thing to expect that said user does want the
information in a downloaded releases, but not in git, and not online where
git-web could also easily display all the information right alongside the files.

But yes, sure, anything can be generated...

Rene.

2007-08-16 21:45:45

by Stefan Richter

[permalink] [raw]
Subject: Re: [linux-pm] Re: Storing Maintainers info around the kernel tree

Rene Herman wrote:
> I personally don't think there's a whole lot wrong with more and more
> expecting people who submit patches (for whom this automation is
> intended) to be using git.

You mean "people who frequently submit patches for various different
subsystems".
--
Stefan Richter
-=====-=-=== =--- =----
http://arcgraph.de/sr/

2007-08-17 01:48:23

by Rene Herman

[permalink] [raw]
Subject: Re: [linux-pm] Re: Storing Maintainers info around the kernel tree

On 08/16/2007 11:39 PM, Stefan Richter wrote:
> Rene Herman wrote:

>> I personally don't think there's a whole lot wrong with more and more
>> expecting people who submit patches (for whom this automation is
>> intended) to be using git.
>
> You mean "people who frequently submit patches for various different
> subsystems".

Erm, I guess. Is that agreeing or disagreeing with me?

Rene.

2007-08-17 01:59:21

by Alan Stern

[permalink] [raw]
Subject: Re: [linux-pm] Re: Storing Maintainers info around the kernel tree

On Fri, 17 Aug 2007, Rene Herman wrote:

> On 08/16/2007 11:39 PM, Stefan Richter wrote:
> > Rene Herman wrote:
>
> >> I personally don't think there's a whole lot wrong with more and more
> >> expecting people who submit patches (for whom this automation is
> >> intended) to be using git.
> >
> > You mean "people who frequently submit patches for various different
> > subsystems".
>
> Erm, I guess. Is that agreeing or disagreeing with me?

Don't forget also that the MAINTAINERS information is (or should be!)
used by people who want to submit bug reports, not just by people who
submit patches. Bug reporters shouldn't need to use Git.

Alan Stern

2007-08-17 02:23:19

by Rene Herman

[permalink] [raw]
Subject: Re: [linux-pm] Re: Storing Maintainers info around the kernel tree

On 08/17/2007 03:58 AM, Alan Stern wrote:

> On Fri, 17 Aug 2007, Rene Herman wrote:
>
>> On 08/16/2007 11:39 PM, Stefan Richter wrote:
>>> Rene Herman wrote:

>>>> I personally don't think there's a whole lot wrong with more and
>>>> more expecting people who submit patches (for whom this automation
>>>> is intended) to be using git.
>>>
>>> You mean "people who frequently submit patches for various different
>>> subsystems".
>>
>> Erm, I guess. Is that agreeing or disagreeing with me?
>
> Don't forget also that the MAINTAINERS information is (or should be!)
> used by people who want to submit bug reports, not just by people who
> submit patches. Bug reporters shouldn't need to use Git.

Like I said:

>> If it's about non-developer users, I suspect it would to a fairly large
>> degree be an "in theory" thing to expect that said user does want the
>> information in a downloaded releases, but not in git, and not online
>> where git-web could also easily display all the information right
>> alongside the files.

And again, generating the MAINTAINERS file/info into releases is fine as well.

Rene.

2007-08-17 06:31:21

by Stefan Richter

[permalink] [raw]
Subject: Re: [linux-pm] Re: Storing Maintainers info around the kernel tree

Rene Herman wrote:
> On 08/17/2007 03:58 AM, Alan Stern wrote:
>
>> On Fri, 17 Aug 2007, Rene Herman wrote:
>>
>>> On 08/16/2007 11:39 PM, Stefan Richter wrote:
>>>> Rene Herman wrote:
>
>>>>> I personally don't think there's a whole lot wrong with more and
>>>>> more expecting people who submit patches (for whom this automation
>>>>> is intended) to be using git.
>>>>
>>>> You mean "people who frequently submit patches for various different
>>>> subsystems".
>>>
>>> Erm, I guess. Is that agreeing or disagreeing with me?
>>
>> Don't forget also that the MAINTAINERS information is (or should be!)
>> used by people who want to submit bug reports, not just by people who
>> submit patches. Bug reporters shouldn't need to use Git.

Yes, problem reporters and people who infrequently (or for their first
time) submit patches, and even people who frequently submit patches but
most of the time only to the same one or two subsystems need an obvious,
tool-independent way to get contact information.

> Like I said:
>
>>> If it's about non-developer users, I suspect it would to a fairly large
>>> degree be an "in theory" thing to expect that said user does want the
>>> information in a downloaded releases, but not in git, and not online
>>> where git-web could also easily display all the information right
>>> alongside the files.
>
> And again, generating the MAINTAINERS file/info into releases is fine as
> well.

Good. This generated data will be used by almost everyone except for a
certain special group of submitters.
--
Stefan Richter
-=====-=-=== =--- =---=
http://arcgraph.de/sr/