Re: Can't seem to find a maintainer for init/* files

[Valdis Klētnieks]

On Tue, 22 Oct 2019 01:37:33 +1300, Paulo Miguel Almeida said:

> In a second approach:
> I tried making "git log" to list all the commits this particular file was 
> involved in
> (so I could use --follow) but I ended up with loads of commits that change 
> other sections
> of the file (not the lines we were looking for) and because of that I didn't 
> feel like I
> was meeting the "'git blame' to show you the original 6 commits rather than 
> the
> cleanup" rule.

Hint 1:  'git log' supports --no-merges which can simplify things sometimes.

Hint 2:   When specifying a commit, there is a ~ operator that can be used to 
advantage here.

So once you figure out which commit did the whitespace patching, it's easy to 
get
git blame to cough up what the tree looked like just before.


pgpOioUjTV4ly.pgp
Description: PGP signature
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Can't seem to find a maintainer for init/* files

[Paulo Miguel Almeida]

On Fri, Oct 18, 2019 at 07:25:12PM -0400, Valdis Klētnieks wrote:
> On Sat, 19 Oct 2019 10:33:00 +1300, Paulo Almeida said:
> 
> > 1 - This specific code block has been around for quite some time and many
> > additions using the correct printk(KERN_* were made after it was written.
> > Does that mean that this code block is an exception and should be left
> > as-is for some technical reason? Or, people have somehow forgotten about it
> > and I finally found something to do? :)
> 
> There's a meta-consideration or two here to think about.
> 
> First, many maintainers are not thrilled with trivial patches to code,
> especially checkpatch cleanups.  That's because those patches fall into two
> major categories:
> 
> The patch is against code that's debugged and rock solid stable.  Most of
> do_mounts.c is close to a decade old, and it's only being changed when it's
> needed to add an actual feature (such as mounting by partition label in 2018
> or mounting a CIFS filesystem this year).  And we *have* had what looked like
> "trivial checkpatch cleanup" patches that were buggy and broke stuff.
> 
> The other category is "patches against code that's being worked on".  If it's
> something that somebody else is working on, it can cause merge conflicts, 
> which
> make maintainers grumpy.  So the maintainer only wants to see those cleanups 
> if
> they're by the person who's working on the code, at the front of the patch
> series, so that (presumably) they don't have merge commits and they've gotten
> some compile and run testing.
> 

That makes perfect sense. 

> The other big consideration is git.  Yes, git knows where and when every 
> single
> line of code came from.  That doesn't mean it's always easy to get it to cough
> up information.

I must confess that prior to this "challenge" of yours, I hadn't realised how 
tricky
git can become to get it to do certain things.  

> 
> For example:   'git blame init/do_mounts.c'.  That tells you where each line 
> came from.
> Now... imagine a commit that did a spaces-to-tabs cleanup on lines 249 to 257.
> git blame' now lists the cleanup commit, not the 6 commits that added the 
> original code.
> 
> Exercise for the reader:  Determine the easiest way to get 'git blame' to 
> show you
> the original 6 commits rather than the cleanup.

Considering that I'm technically a reader :) I'm gonna start with my solution 
and
if anyone else wants to add to it (or replace it altogether) feel free to do so.

What I got so far is:

L_START=249
L_FINISH=257
FILE=init/do_mounts.c; 

for commit in $(git log --format='%h' --no-patch -L$L_START,$L_FINISH:$FILE | 
cat | tail -n +2)
do  
echo "Showing commit: $commit of file: $FILE"
git blame -n -L$L_START,$L_FINISH $commit -- $FILE
done

PS.: (For future readers)  That may take a while.

I feel like this isn't right per se as the "git log -L" doesn't support 
"--follow" 
argument that would impact our ability to make it cough up the info we wanted 
in case the
file had been renamed. However, on the other hand, it would (to my 
understanding) make
its best effort to keep track of those lines in the log history. (Correct me if 
I'm wrong)

In a second approach:
I tried making "git log" to list all the commits this particular file was 
involved in
(so I could use --follow) but I ended up with loads of commits that change 
other sections 
of the file (not the lines we were looking for) and because of that I didn't 
feel like I 
was meeting the "'git blame' to show you the original 6 commits rather than the 
cleanup" rule.

@Valdis I hope I'm not going off-topic but could you shed some light on this?

Last but not least, I appriciate the meta-consideration points you brought up.

Best Regards,

Paulo Almeida



___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Can't seem to find a maintainer for init/* files

[Paulo Almeida]

> On 19/10/2019, at 11:32 AM, Greg KH  wrote:
> 
> Run scripts/get_maintainer.pl on any file and it will tell you where to
> send changes to:
>   $ ./scripts/get_maintainer.pl --file init/do_mounts.c
>   Al Viro mailto:v...@zeniv.linux.org.uk>> 
> (commit_signer:7/9=78%,authored:5/9=56%,added_lines:7/44=16%,removed_lines:27/32=84%)
>   David Howells mailto:dhowe...@redhat.com>> 
> (commit_signer:3/9=33%,authored:2/9=22%,added_lines:5/44=11%,removed_lines:5/32=16%)
>   Andrew Morton  > (commit_signer:2/9=22%)
>   Nikolaus Voss  > 
> (commit_signer:1/9=11%,authored:1/9=11%,added_lines:31/44=70%)
>   Thomas Gleixner mailto:t...@linutronix.de>> 
> (commit_signer:1/9=11%,authored:1/9=11%)
>   linux-ker...@vger.kernel.org  
> (open list)
> 
> But don't do janitorial cleanup on those files as a "first task".  That
> is what the drivers/staging/ directory is for, don't mess with core
> kernel files unless you have experience doing kernel changes, as the
> learning curve needs to be gotten over first.

Hi Greg,

I appreciate your reply and I will take your piece of advice on what to do as 
the first task.

Best regards,

Paulo Almeida___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Can't seem to find a maintainer for init/* files

[Valdis Klētnieks]

On Sat, 19 Oct 2019 10:33:00 +1300, Paulo Almeida said:

> 1 - This specific code block has been around for quite some time and many
> additions using the correct printk(KERN_* were made after it was written.
> Does that mean that this code block is an exception and should be left
> as-is for some technical reason? Or, people have somehow forgotten about it
> and I finally found something to do? :)

There's a meta-consideration or two here to think about.

First, many maintainers are not thrilled with trivial patches to code,
especially checkpatch cleanups.  That's because those patches fall into two
major categories:

The patch is against code that's debugged and rock solid stable.  Most of
do_mounts.c is close to a decade old, and it's only being changed when it's
needed to add an actual feature (such as mounting by partition label in 2018
or mounting a CIFS filesystem this year).  And we *have* had what looked like
"trivial checkpatch cleanup" patches that were buggy and broke stuff.

The other category is "patches against code that's being worked on".  If it's
something that somebody else is working on, it can cause merge conflicts, which
make maintainers grumpy.  So the maintainer only wants to see those cleanups if
they're by the person who's working on the code, at the front of the patch
series, so that (presumably) they don't have merge commits and they've gotten
some compile and run testing.

The other big consideration is git.  Yes, git knows where and when every single
line of code came from.  That doesn't mean it's always easy to get it to cough
up information.

For example:   'git blame init/do_mounts.c'.  That tells you where each line 
came from.
Now... imagine a commit that did a spaces-to-tabs cleanup on lines 249 to 257.
git blame' now lists the cleanup commit, not the 6 commits that added the 
original code.

Exercise for the reader:  Determine the easiest way to get 'git blame' to show 
you
the original 6 commits rather than the cleanup.


pgptKjwtCKMRP.pgp
Description: PGP signature
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Can't seem to find a maintainer for init/* files

[Greg KH]

On Sat, Oct 19, 2019 at 10:33:00AM +1300, Paulo Almeida wrote:
> Hi all,
> 
> I was reading the KernelJanitor/Todo webpage and found the printk-related
> task that had to be done.
> 
> I eventually came across this piece of code that led me to 2 questions that
> I couldn't answer myself
> https://github.com/torvalds/linux/blame/master/init/do_mounts.c#L434-L455
> 
> 1 - This specific code block has been around for quite some time and many
> additions using the correct printk(KERN_* were made after it was written.
> Does that mean that this code block is an exception and should be left
> as-is for some technical reason? Or, people have somehow forgotten about it
> and I finally found something to do? :)
> 
> 2 - I took a look at the https://www.kernel.org/doc/linux/MAINTAINERS list
> and their respective folders/repos and couldn't find out who would be the
> person who I should eventually send the patches to. Is there any
> "fallthrough" maintainer when there isn't one specified? (Please put me
> straight if I'm not seeing things from the right angle)

Run scripts/get_maintainer.pl on any file and it will tell you where to
send changes to:
$ ./scripts/get_maintainer.pl --file init/do_mounts.c
Al Viro  
(commit_signer:7/9=78%,authored:5/9=56%,added_lines:7/44=16%,removed_lines:27/32=84%)
David Howells  
(commit_signer:3/9=33%,authored:2/9=22%,added_lines:5/44=11%,removed_lines:5/32=16%)
Andrew Morton  (commit_signer:2/9=22%)
Nikolaus Voss  
(commit_signer:1/9=11%,authored:1/9=11%,added_lines:31/44=70%)
Thomas Gleixner  
(commit_signer:1/9=11%,authored:1/9=11%)
linux-ker...@vger.kernel.org (open list)

But don't do janitorial cleanup on those files as a "first task".  That
is what the drivers/staging/ directory is for, don't mess with core
kernel files unless you have experience doing kernel changes, as the
learning curve needs to be gotten over first.

good luck!

greg k-h

___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

Re: Can't seem to find a maintainer for init/* files

[Paulo Almeida]

I believe I just found the answer for point number 2:

THE REST
M: Linus Torvalds 
L: linux-ker...@vger.kernel.org
Q: http://patchwork.kernel.org/project/LKML/list/
T: git git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
S: Buried alive in reporters
F: *
F: */


On Sat, Oct 19, 2019 at 10:33 AM Paulo Almeida <
paulo.miguel.almeida.rode...@gmail.com> wrote:

> Hi all,
>
> I was reading the KernelJanitor/Todo webpage and found the printk-related
> task that had to be done.
>
> I eventually came across this piece of code that led me to 2 questions
> that I couldn't answer myself
> https://github.com/torvalds/linux/blame/master/init/do_mounts.c#L434-L455
>
> 1 - This specific code block has been around for quite some time and many
> additions using the correct printk(KERN_* were made after it was written.
> Does that mean that this code block is an exception and should be left
> as-is for some technical reason? Or, people have somehow forgotten about it
> and I finally found something to do? :)
>
> 2 - I took a look at the https://www.kernel.org/doc/linux/MAINTAINERS list
> and their respective folders/repos and couldn't find out who would be the
> person who I should eventually send the patches to. Is there any
> "fallthrough" maintainer when there isn't one specified? (Please put me
> straight if I'm not seeing things from the right angle)
>
> Best regards,
>
> Paulo Almeida
>
>
___
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies