[libvirt] [PATCH hooks 1/1] Add check for Signed-off-by in commit messages

[Daniel P. Berrange]

On Mon, Jan 22, 2018 at 02:20:52PM +0100, Peter Krempa wrote:
> On Mon, Jan 22, 2018 at 13:06:28 +0000, Daniel Berrange wrote:
> > On Mon, Jan 22, 2018 at 01:20:12PM +0100, Peter Krempa wrote:
> > > On Mon, Jan 22, 2018 at 12:05:19 +0000, Daniel Berrange wrote:
> > > > This extends the update hook so that it enforces a requirement to have a
> > > > Signed-off-by line in every commit message. This can be optionally
> > > > turned off in individual repos by setting the "hooks.allowmissingsob"
> > > > git config variable.
> > > > 
> > > > Signed-off-by: Daniel P. Berrange <berrange at redhat.com>
> > > > ---
> > > >  update | 16 +++++++++++++++-
> > > >  1 file changed, 15 insertions(+), 1 deletion(-)
> > > 
> > > NACK, I don't like signoffs and I don't really think they achieve
> > > anything.
> > 
> > A signoff with no documented meaning attached by the project is fairly
> > weak, as you would have to argue there was some commonly accepted signifance
> > to it across the community. A signoff which is explicitly associated with a
> > statement of intent has benefit, hence why I also sent a patch to clarify
> > that the signoff asserts compliance with the DCO. Adding these signoffs has
> 
> You can do the same by declaring that all patches have to comply to that
> and not mandating adding a line which will become eventually pointless
> since every patch will need to have it.
> 
> Also since there's no way to check that it's actually true, anybody can
> declare anything. Also the check itself can be fooled easily, so I think
> it's pointless altogether.

The git hook isn't expected to be flawless. Whomever is pushing the patches
to git is still actually reviewing the submissions. If someone puts garbage
there it will still likely be caught just as if someone puts garbage in the
actual code.

> > little to no time burden on long term developers own work on a daily basis.
> > Just needs adding -s to "git commit" which quickly becomes engrained in
> > muscle memory such that you'll end up doing it for every project you find
> 
> Or you can defeat it entirely by adding it into your commit message
> template. I don't care about the added burden though. I don't really
> think it achieves anything.

So we can weigh up the possible outcomes. It it adopt it and it turns
out to not do anything, then we've merely suffered insigifincant burden
of adding it.  If we don't adopt it, and it does turn out to be important
the consequences may we quite severe.  It makes no prudent sense to
not adopt it, given the minimal burden it has and the potentially significant
upside.

> > yourself contributing to. Many of our "drive by" contributors already do
> > this as habit, we'll just need to remind those that forget periodically.
> 
> I presonally signed-off < 5 commits in libvirt. So the last statement
> is untrue. Some people don't do it on purpose.
> 
> The sign-off by itself (whithout cryptographic signature) is just
> pointless. Validity with a cryptographic signature from drive-by
> contributors can still be unproven, but at least you don't get
> impersonation.

I think these are two different axis. The sob isn't trying to address the
question of impersonation. It obviously has as a starting point that you
accept the identity of the submitter to some degree. I accept that if you
have cryptographically signed patches, that would give a stronger
validation of identity, but there's never any absolutes. So not having
a crypto signature doesn't make the sob invalid.

> If everything is signed off, nothing really is.

I don't really see that.

> NACK still stands.

You are nacking something that you've accepted above will have no negative
impact on your work, but has potentially significant upside to the project.
That is very disappointing.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|