Re: [B.A.T.M.A.N.] [PATCH 00/26] staging:batman-adv

[Sven Eckelmann <sven.eckelmann@gmx.de>]

[-- Attachment #1: Type: Text/Plain, Size: 6499 bytes --]

Marek Lindner wrote:
> On Friday 07 May 2010 05:44:06 Sven Eckelmann wrote:
> > Am I the only one who thinks that Marek takes credits for too many
> > patches? Yes, we all have to praise him for his work, but just noticed
> > those minor
> > 
> > glitches:
> >  * fix whitespace style issues
> >  
> >    - work from Luis de Bethencourt <luisbg@ubuntu.com>
> >  
> >  * Fix whitespace problems criticized by
> >  
> >    Reduce max characters on a line to 80
> >    - work from me... somewhat
> > 
> > Not that I really want credit for those patches (maybe I should have
> > submitted them under a different name.....), but that didn't happen in
> > the past and now more than one time in a row. What happened here?
> > git-am's fault?
> 
> It seems there are a couple of patches wrong but I don't know how this
> initial credit list gets created. I had the impression the first
> "Signed-off-by" is the owner of a patch ?

Ok, there seems to be a problem misunderstanding in what our "infrastructure" 
does. Let's see what svn does:
 * svn does not have a field to store the complex information
   "Full Name <email@address>" for committer and author
 * svn stores the username of the committer

So we get a commit in svn it must get somehow to our repository. We have a 
"translation and synchronisation" repository hidden in a private directory. We 
use git-svn there to fetch our data from svn and then push it to our public 
git repository into the master branch.

If you look at a specific commit using something like
`git cat-file -p v0.2-59-g23a0a47` then you will see the committer and author 
information which is part of the metadata of a git commit and not really part 
of the actual commit message. An example would be:

tree 359ee2bfbc9ad702b9f6ef1649833793090244fe
parent 0024f65f2c6cc1765bc7034cf82b9dc2b7626630
author Daniel Seither <post@tiwoc.de> 1266335052 +0000
committer Marek Lindner <lindner_marek@yahoo.de> 1266335052 +0000
....

author is obviously the author of a patch/change. This is something which 
should not be changed by the person who applies the patch. Thereof we have 
also the committer part which shows who has applied the patch. The committer 
information is changed in many situations. A common one would be that someone 
cherry picks a patch from master to maint. In other words: we have enough 
information to thank the person for their contribution and to blame the person 
who applied in on the wrong branch. :)

So where do we get that information svn only stores the committer username? 
git-svn uses different information to gather such data. The first one is the a 
translation table statically defined by us. We use it to translate the svn 
committer to a something like "Marek Lindner <lindner_marek@yahoo.de>" in the 
git committer field. We still have the problem that the author isn't stored 
inside the svn commit... normally.

We defined that all commits in trunk/batman-adv-kernelland must have a valid 
'Signed-off-by: ' line and must be submitted by an eligible committer (Simon 
or Marek). git-svn can now extract the first Signed-off-by-line to get the 
actual author information from a (special) svn commit..... Problem solved for 
svn to git translation.

So what does git-commit do here? Nothing real exciting. When a new commit is 
created (and not otherwise specified) then both author and committer fields 
get the same information fetched from ~/.gitconfig or an other configuration 
file. When we change a commit then only the committer field is updated.

So we have nothing like signed-off-by parsing in git-commit. There are some 
"tricks" described in the git-commit-tree manpage to supply such information 
directly - but that is nothing you really want to do for each patch.

The way patches get applied are using git-am. This program use(s|d?) 
internally git-mailinfo to extract important information from the patch. Lets 
use
"[PATCH 21/26] staging:batman-adv: kfree_skb() in interface_tx() in error  
case" for further explanations. We would normally just save that as file or in 
a mbox. Now just use `git am FILENAME` and everything should be parsed and 
committed. If not than you can try `git am -3 FILENAME` to try a 3-way merge - 
if it still not work than maybe it is the best idea to inform the patch author 
that he should resubmit the patch.

So what information is now extract? Lets call git-mailinfo with patch21.mbox 
(the patch mentioned above saved as mbox file).

git mailinfo msg patch < patch21.mbox > info

We have 3 new files. "msg" is the commit message, "patch" is a patch which can 
be applied using a normal patch program or git-apply and "info" - which is the 
extracted info we are interested in.

Author: Simon Wunderlich
Email: siwu@hrz.tu-chemnitz.de
Subject: staging:batman-adv: kfree_skb() in interface_tx() in error case
Date: Thu, 6 May 2010 22:18:47 +0200

mailinfo has now the possibility to extract the From: line in the mail header 
- this is the usual way. But Andrew submitted the patch instead of Simon. 
Thereof there is a second way to extract that information - the first 'From: ' 
line in the mail body. git-format-patch added this line because Andrew was not 
the author of the patch and git-mailinfo will strip this line again from the 
commit message.

We can see that the Signed-off-by line is not used at all in git-am or git-
commit and that we must tools like git-am to get the correct author field in a 
commit when applying a patch. If you apply your patch with another tool than 
you must use git-commit differently (not explained here because I don't want 
to encourage people to it). So even if the Signed-off-by thing is more a 
social thing (read linux-2.6/Documentation/SubmittingPatches), it is still 
used in different scripts to find out who is responsible for a specific part 
of the kernel or similar things. We only use it here when getting patches from 
svn to translate them into git commits.

This "credit" list you saw here is more or less the thing you get when running 
git-shortlog. It just gathers the commits of all authors and shows a summary 
commit list for each author. "author" means here the author mentioned in the 
author field of the git commit. So it is not Andrews fault, but the committer 
has done something different than the above mentioned steps to get the commit 
applied in maint.

Best regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]