Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Hans de Goede]

Hi,

On 4/14/21 3:51 PM, Stephen Rothwell wrote:
> Hi all,
> 
> Commit
> 
>   ff57cfaa3d68 ("platform/x86: pmc_atom: Match all Beckhoff Automation 
> baytrail boards with critclk_systems DMI table")
> 
> is missing a Signed-off-by from its committer.

My bad I somehow forgot to pass -s to "git am", this is fixed now.

Regards,

Hans

linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi all,

Commit

  ff57cfaa3d68 ("platform/x86: pmc_atom: Match all Beckhoff Automation baytrail 
boards with critclk_systems DMI table")

is missing a Signed-off-by from its committer.

-- 
Cheers,
Stephen Rothwell


pgpK7vO1nXM1y.pgp
Description: OpenPGP digital signature

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Hans de Goede]

Hi all,

On 4/8/21 2:13 PM, Stephen Rothwell wrote:
> Hi all,
> 
> Commit
> 
>   11cccec79c60 ("genirq: Add IRQF_NO_AUTOEN for request_irq/nmi()")
> 
> is missing a Signed-off-by from its committer.

Ugh, thanks for letting me know, this was supposed to come from a merge
from an immutable branch from the tip folks, but I did a rebase -i
to fixup a typo in a commit message and that seems to have flattened
the merge :|

I'll redo the last 3 commits in pdx/for-next to re-add the merge
and do a forced push.

Regards,

Hans

linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi all,

Commit

  11cccec79c60 ("genirq: Add IRQF_NO_AUTOEN for request_irq/nmi()")

is missing a Signed-off-by from its committer.

-- 
Cheers,
Stephen Rothwell


pgppsj7QakBNS.pgp
Description: OpenPGP digital signature

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Andy Shevchenko]

On Mon, May 6, 2019 at 4:22 PM Stephen Rothwell  wrote:
>
> Hi all,
>
> Commit
>
>   cc86bb923508 ("platform/x86: thinkpad_acpi: fix spelling mistake 
> "capabilites" -> "capabilities"")
>
> is missing a Signed-off-by from its committer.

It's fixed now, thanks!

>
> --
> Cheers,
> Stephen Rothwell



-- 
With Best Regards,
Andy Shevchenko

linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi all,

Commit

  cc86bb923508 ("platform/x86: thinkpad_acpi: fix spelling mistake 
"capabilites" -> "capabilities"")

is missing a Signed-off-by from its committer.

-- 
Cheers,
Stephen Rothwell


pgpaTqbRpnEU5.pgp
Description: OpenPGP digital signature

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi Darren,

On Sat, 23 Feb 2019 09:52:07 -0800 Darren Hart  wrote:
>
> Apologies if I've asked you this before... I didn't find it after some
> searching.
> 
> We should be catching errors like this before they hit next. First,
> there is no reason we can't catch them - unlike the integration failures
> only next can catch. Second, once they are in next, there is no "right"
> way to fix them.  Either rebase or send the bad patch to mainline - both
> are bad. Don't get me wrong, I'm glad next catches them but I'd like
> to get to the point where it doesn't trigger on our subsystem :-)
> 
> Are your patch mechanics tests available for us to integrate into our
> commit and prepublication checks?

I have attached the current version of my check scripts (I can't use
git hooks since I want to do these checks when I fetch trees).  They
both take a range of commits (I usually pass "^origin/master ..." (only 2 dots if the branch is just
fast forwarded).

These could obviously be simplified because they also generate most of
the appropriate mail messages ...
-- 
Cheers,
Stephen Rothwell


check_fixes
Description: application/shellscript


check_commits
Description: application/shellscript


pgpmkj_iEEIxP.pgp
Description: OpenPGP digital signature

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

On Sun, Feb 24, 2019 at 01:19:25AM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> Commit
> 
>   9bfe33ba29d0 ("x86/CPU: Add Icelake model number")
> 
> is missing a Signed-off-by from its committer.
> 

Hi Stephen,

Apologies if I've asked you this before... I didn't find it after some
searching.

We should be catching errors like this before they hit next. First,
there is no reason we can't catch them - unlike the integration failures
only next can catch. Second, once they are in next, there is no "right"
way to fix them.  Either rebase or send the bad patch to mainline - both
are bad. Don't get me wrong, I'm glad next catches them but I'd like
to get to the point where it doesn't trigger on our subsystem :-)

Are your patch mechanics tests available for us to integrate into our
commit and prepublication checks?

Thanks,

-- 
Darren Hart
VMware Open Source Technology Center

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

On Sun, Feb 24, 2019 at 01:19:25AM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> Commit
> 
>   9bfe33ba29d0 ("x86/CPU: Add Icelake model number")
> 
> is missing a Signed-off-by from its committer.

Thanks for the catch. Andy, please check your commit scripts as should
definitely be automated in our workflow.

I have corrected it.
-- 
Darren Hart
VMware Open Source Technology Center

linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi all,

Commit

  9bfe33ba29d0 ("x86/CPU: Add Icelake model number")

is missing a Signed-off-by from its committer.

-- 
Cheers,
Stephen Rothwell


pgpEWJcWekscX.pgp
Description: OpenPGP digital signature

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi Hans,

On Sun, 19 Aug 2018 10:21:40 +0200 Hans de Goede  wrote:
>
> On 18-08-18 16:35, Stephen Rothwell wrote:
> > Hi all,
> > 
> > Commit
> > 
> >cda5915d15d3 ("platform/x86: touchscreen_dmi: Add info for the Cube 
> > KNote i1101 tablet")  
> 
> For reasons which I do not know youling does not wish to
> use his real name.
> 
> So as with the "platform/x86: touchscreen_dmi: Add
> info for the ONDA V891W Dual OS tablet" patch,
> I've given my S-o-b for this, where the intent of my
> S-o-b is to certify point c. of the certificate
> of origin, quoting from:
> 
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#developer-s-certificate-of-origin-1-1
> 
> "c. The contribution was provided directly to me by some other person who 
> certified (a), (b) or (c) and I have not modified it."

Thats fine, thanks.

-- 
Cheers,
Stephen Rothwell


pgpejl5tGIsRF.pgp
Description: OpenPGP digital signature

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi Hans,

On Sun, 19 Aug 2018 10:21:40 +0200 Hans de Goede  wrote:
>
> On 18-08-18 16:35, Stephen Rothwell wrote:
> > Hi all,
> > 
> > Commit
> > 
> >cda5915d15d3 ("platform/x86: touchscreen_dmi: Add info for the Cube 
> > KNote i1101 tablet")  
> 
> For reasons which I do not know youling does not wish to
> use his real name.
> 
> So as with the "platform/x86: touchscreen_dmi: Add
> info for the ONDA V891W Dual OS tablet" patch,
> I've given my S-o-b for this, where the intent of my
> S-o-b is to certify point c. of the certificate
> of origin, quoting from:
> 
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#developer-s-certificate-of-origin-1-1
> 
> "c. The contribution was provided directly to me by some other person who 
> certified (a), (b) or (c) and I have not modified it."

Thats fine, thanks.

-- 
Cheers,
Stephen Rothwell


pgpejl5tGIsRF.pgp
Description: OpenPGP digital signature

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Hans de Goede]

Hi,

On 18-08-18 16:35, Stephen Rothwell wrote:

Hi all,

Commit

   cda5915d15d3 ("platform/x86: touchscreen_dmi: Add info for the Cube KNote i1101 
tablet")


For reasons which I do not know youling does not wish to
use his real name.

So as with the "platform/x86: touchscreen_dmi: Add
info for the ONDA V891W Dual OS tablet" patch,
I've given my S-o-b for this, where the intent of my
S-o-b is to certify point c. of the certificate
of origin, quoting from:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#developer-s-certificate-of-origin-1-1

"c. The contribution was provided directly to me by some other person who certified 
(a), (b) or (c) and I have not modified it."

Regards,

Hans

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Hans de Goede]

Hi,

On 18-08-18 16:35, Stephen Rothwell wrote:

Hi all,

Commit

   cda5915d15d3 ("platform/x86: touchscreen_dmi: Add info for the Cube KNote i1101 
tablet")


For reasons which I do not know youling does not wish to
use his real name.

So as with the "platform/x86: touchscreen_dmi: Add
info for the ONDA V891W Dual OS tablet" patch,
I've given my S-o-b for this, where the intent of my
S-o-b is to certify point c. of the certificate
of origin, quoting from:

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#developer-s-certificate-of-origin-1-1

"c. The contribution was provided directly to me by some other person who certified 
(a), (b) or (c) and I have not modified it."

Regards,

Hans

linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi all,

Commit

  cda5915d15d3 ("platform/x86: touchscreen_dmi: Add info for the Cube KNote 
i1101 tablet")

is missing a Signed-off-by from its author.

-- 
Cheers,
Stephen Rothwell


pgpo0vHZOCf_2.pgp
Description: OpenPGP digital signature

linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi all,

Commit

  cda5915d15d3 ("platform/x86: touchscreen_dmi: Add info for the Cube KNote 
i1101 tablet")

is missing a Signed-off-by from its author.

-- 
Cheers,
Stephen Rothwell


pgpo0vHZOCf_2.pgp
Description: OpenPGP digital signature

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

On Sat, Jun 02, 2018 at 01:26:46AM +1000, Stephen Rothwell wrote:
> Hi,
> 
> On Fri, 01 Jun 2018 07:38:42 -0700 dvh...@infradead.org wrote:
> >
> > Stephen, are the tests you use available publicly?
> 
> I use the script below when I am fetching trees.  You don't want the
> "gitk" invocation if you are doing this automatically, of course.  The
> script just takes a commit range.

Thanks for sharing Stephen.
-- 
Darren Hart
VMware Open Source Technology Center

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

On Sat, Jun 02, 2018 at 01:26:46AM +1000, Stephen Rothwell wrote:
> Hi,
> 
> On Fri, 01 Jun 2018 07:38:42 -0700 dvh...@infradead.org wrote:
> >
> > Stephen, are the tests you use available publicly?
> 
> I use the script below when I am fetching trees.  You don't want the
> "gitk" invocation if you are doing this automatically, of course.  The
> script just takes a commit range.

Thanks for sharing Stephen.
-- 
Darren Hart
VMware Open Source Technology Center

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi,

On Fri, 01 Jun 2018 07:38:42 -0700 dvh...@infradead.org wrote:
>
> Stephen, are the tests you use available publicly?

I use the script below when I am fetching trees.  You don't want the
"gitk" invocation if you are doing this automatically, of course.  The
script just takes a commit range.

-- 
Cheers,
Stephen Rothwell

-
#!/bin/bash

if [ "$#" -lt 1 ]; then
printf "Usage: %s \n", "$0" 1>&2
exit 1
fi

commits=$(git rev-list --no-merges "$@")
if [ -z "$commits" ]; then
printf "No commits\n"
exit 0
fi

for c in $commits; do
ae=$(git log -1 --format='%ae' "$c")
aE=$(git log -1 --format='%aE' "$c")
an=$(git log -1 --format='%an' "$c")
aN=$(git log -1 --format='%aN' "$c")
ce=$(git log -1 --format='%ce' "$c")
cE=$(git log -1 --format='%cE' "$c")
cn=$(git log -1 --format='%cn' "$c")
cN=$(git log -1 --format='%cN' "$c")
sob=$(git log -1 --format='%b' "$c" | grep -i 
'^[[:space:]]*Signed-off-by:')

am=false
cm=false
grep -i -q "<$ae>" <<<"$sob" ||
grep -i -q "<$aE>" <<<"$sob" ||
grep -i -q ":[[:space:]]*$an[[:space:]]*<" <<<"$sob" ||
grep -i -q ":[[:space:]]*$aN[[:space:]]*<" <<<"$sob" ||
am=true
grep -i -q "<$ce>" <<<"$sob" ||
grep -i -q "<$cE>" <<<"$sob" ||
grep -i -q ":[[:space:]]*$cn[[:space:]]*<" <<<"$sob" ||
grep -i -q ":[[:space:]]*$cN[[:space:]]*<" <<<"$sob" ||
cm=true

if "$am" || "$cm"; then
printf "Commit %s\n" "$c"
"$am" && printf "\tauthor SOB missing\n"
"$cm" && printf "\tcommitter SOB missing\n"
printf "%s %s\n%s\n" "$ae" "$ce" "$sob"
fi
done

exec gitk "$@"
-


pgpBPqjcYE3Km.pgp
Description: OpenPGP digital signature

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi,

On Fri, 01 Jun 2018 07:38:42 -0700 dvh...@infradead.org wrote:
>
> Stephen, are the tests you use available publicly?

I use the script below when I am fetching trees.  You don't want the
"gitk" invocation if you are doing this automatically, of course.  The
script just takes a commit range.

-- 
Cheers,
Stephen Rothwell

-
#!/bin/bash

if [ "$#" -lt 1 ]; then
printf "Usage: %s \n", "$0" 1>&2
exit 1
fi

commits=$(git rev-list --no-merges "$@")
if [ -z "$commits" ]; then
printf "No commits\n"
exit 0
fi

for c in $commits; do
ae=$(git log -1 --format='%ae' "$c")
aE=$(git log -1 --format='%aE' "$c")
an=$(git log -1 --format='%an' "$c")
aN=$(git log -1 --format='%aN' "$c")
ce=$(git log -1 --format='%ce' "$c")
cE=$(git log -1 --format='%cE' "$c")
cn=$(git log -1 --format='%cn' "$c")
cN=$(git log -1 --format='%cN' "$c")
sob=$(git log -1 --format='%b' "$c" | grep -i 
'^[[:space:]]*Signed-off-by:')

am=false
cm=false
grep -i -q "<$ae>" <<<"$sob" ||
grep -i -q "<$aE>" <<<"$sob" ||
grep -i -q ":[[:space:]]*$an[[:space:]]*<" <<<"$sob" ||
grep -i -q ":[[:space:]]*$aN[[:space:]]*<" <<<"$sob" ||
am=true
grep -i -q "<$ce>" <<<"$sob" ||
grep -i -q "<$cE>" <<<"$sob" ||
grep -i -q ":[[:space:]]*$cn[[:space:]]*<" <<<"$sob" ||
grep -i -q ":[[:space:]]*$cN[[:space:]]*<" <<<"$sob" ||
cm=true

if "$am" || "$cm"; then
printf "Commit %s\n" "$c"
"$am" && printf "\tauthor SOB missing\n"
"$cm" && printf "\tcommitter SOB missing\n"
printf "%s %s\n%s\n" "$ae" "$ce" "$sob"
fi
done

exec gitk "$@"
-


pgpBPqjcYE3Km.pgp
Description: OpenPGP digital signature

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Andy Shevchenko]

On Fri, Jun 1, 2018 at 5:33 PM,   wrote:
> On June 1, 2018 5:08:32 AM PDT, Stephen Rothwell  
> wrote:
>>On Fri, 1 Jun 2018 14:40:35 +0300 Andy Shevchenko
>> wrote:
>>>
>>> Oops. What is the proposed fix for that? It seems we can't rebase
>>> published branches.
>>
>>It's unfixable without a rebase, so you could instead consider it an
>>opportunity to improve your processes for the future. :-)
>
> Yeah, in this case we have to do a rebase.
>
> Andy, I've been wanting to look at using some simple bots and tags for this 
> kind of thing. GitHub makes this really easy, might be time to move. We'll 
> continue this offline.
>
> For now, please rebase next to correct the error as this change is on top of 
> any of my commits, we won't make things worse by you recommitting my commits.

Done.

Author has been informed.

-- 
With Best Regards,
Andy Shevchenko

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Andy Shevchenko]

On Fri, Jun 1, 2018 at 5:33 PM,   wrote:
> On June 1, 2018 5:08:32 AM PDT, Stephen Rothwell  
> wrote:
>>On Fri, 1 Jun 2018 14:40:35 +0300 Andy Shevchenko
>> wrote:
>>>
>>> Oops. What is the proposed fix for that? It seems we can't rebase
>>> published branches.
>>
>>It's unfixable without a rebase, so you could instead consider it an
>>opportunity to improve your processes for the future. :-)
>
> Yeah, in this case we have to do a rebase.
>
> Andy, I've been wanting to look at using some simple bots and tags for this 
> kind of thing. GitHub makes this really easy, might be time to move. We'll 
> continue this offline.
>
> For now, please rebase next to correct the error as this change is on top of 
> any of my commits, we won't make things worse by you recommitting my commits.

Done.

Author has been informed.

-- 
With Best Regards,
Andy Shevchenko

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Andy Shevchenko]

On Fri, Jun 1, 2018 at 3:08 PM, Stephen Rothwell  wrote:
> Hi Andy,
>
> On Fri, 1 Jun 2018 14:40:35 +0300 Andy Shevchenko  
> wrote:
>>
>> Oops. What is the proposed fix for that? It seems we can't rebase
>> published branches.
>
> It's unfixable without a rebase, so you could instead consider it an
> opportunity to improve your processes for the future. :-)

Do you have some already cooked scripts to perform such checks on
given repository?
Can you  share?

-- 
With Best Regards,
Andy Shevchenko

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Andy Shevchenko]

On Fri, Jun 1, 2018 at 3:08 PM, Stephen Rothwell  wrote:
> Hi Andy,
>
> On Fri, 1 Jun 2018 14:40:35 +0300 Andy Shevchenko  
> wrote:
>>
>> Oops. What is the proposed fix for that? It seems we can't rebase
>> published branches.
>
> It's unfixable without a rebase, so you could instead consider it an
> opportunity to improve your processes for the future. :-)

Do you have some already cooked scripts to perform such checks on
given repository?
Can you  share?

-- 
With Best Regards,
Andy Shevchenko

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[dvhart]

On June 1, 2018 5:08:32 AM PDT, Stephen Rothwell  wrote:
>Hi Andy,
>
>On Fri, 1 Jun 2018 14:40:35 +0300 Andy Shevchenko
> wrote:
>>
>> Oops. What is the proposed fix for that? It seems we can't rebase
>> published branches.
>
>It's unfixable without a rebase, so you could instead consider it an
>opportunity to improve your processes for the future. :-)

Stephen, are the tests you use available publicly?

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[dvhart]

On June 1, 2018 5:08:32 AM PDT, Stephen Rothwell  wrote:
>Hi Andy,
>
>On Fri, 1 Jun 2018 14:40:35 +0300 Andy Shevchenko
> wrote:
>>
>> Oops. What is the proposed fix for that? It seems we can't rebase
>> published branches.
>
>It's unfixable without a rebase, so you could instead consider it an
>opportunity to improve your processes for the future. :-)

Stephen, are the tests you use available publicly?

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[dvhart]

On June 1, 2018 5:08:32 AM PDT, Stephen Rothwell  wrote:
>Hi Andy,
>
>On Fri, 1 Jun 2018 14:40:35 +0300 Andy Shevchenko
> wrote:
>>
>> Oops. What is the proposed fix for that? It seems we can't rebase
>> published branches.
>
>It's unfixable without a rebase, so you could instead consider it an
>opportunity to improve your processes for the future. :-)

Yeah, in this case we have to do a rebase.

Andy, I've been wanting to look at using some simple bots and tags for this 
kind of thing. GitHub makes this really easy, might be time to move. We'll 
continue this offline.

For now, please rebase next to correct the error as this change is on top of 
any of my commits, we won't make things worse by you recommitting my commits.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[dvhart]

On June 1, 2018 5:08:32 AM PDT, Stephen Rothwell  wrote:
>Hi Andy,
>
>On Fri, 1 Jun 2018 14:40:35 +0300 Andy Shevchenko
> wrote:
>>
>> Oops. What is the proposed fix for that? It seems we can't rebase
>> published branches.
>
>It's unfixable without a rebase, so you could instead consider it an
>opportunity to improve your processes for the future. :-)

Yeah, in this case we have to do a rebase.

Andy, I've been wanting to look at using some simple bots and tags for this 
kind of thing. GitHub makes this really easy, might be time to move. We'll 
continue this offline.

For now, please rebase next to correct the error as this change is on top of 
any of my commits, we won't make things worse by you recommitting my commits.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi Andy,

On Fri, 1 Jun 2018 14:40:35 +0300 Andy Shevchenko  
wrote:
>
> Oops. What is the proposed fix for that? It seems we can't rebase
> published branches.

It's unfixable without a rebase, so you could instead consider it an
opportunity to improve your processes for the future. :-)

-- 
Cheers,
Stephen Rothwell


pgpREb03ZBEfB.pgp
Description: OpenPGP digital signature

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi Andy,

On Fri, 1 Jun 2018 14:40:35 +0300 Andy Shevchenko  
wrote:
>
> Oops. What is the proposed fix for that? It seems we can't rebase
> published branches.

It's unfixable without a rebase, so you could instead consider it an
opportunity to improve your processes for the future. :-)

-- 
Cheers,
Stephen Rothwell


pgpREb03ZBEfB.pgp
Description: OpenPGP digital signature

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Andy Shevchenko]

On Fri, Jun 1, 2018 at 2:36 PM, Stephen Rothwell  wrote:
> Hi all,
>
> Commit
>
>   bba074f926d1 ("platform/x86: silead_dmi: Add entry for Chuwi Hi8 S806_206 
> tablet touchscreen")
>
> is missing a Signed-off-by from its author.

Oops. What is the proposed fix for that? It seems we can't rebase
published branches.

-- 
With Best Regards,
Andy Shevchenko

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Andy Shevchenko]

On Fri, Jun 1, 2018 at 2:36 PM, Stephen Rothwell  wrote:
> Hi all,
>
> Commit
>
>   bba074f926d1 ("platform/x86: silead_dmi: Add entry for Chuwi Hi8 S806_206 
> tablet touchscreen")
>
> is missing a Signed-off-by from its author.

Oops. What is the proposed fix for that? It seems we can't rebase
published branches.

-- 
With Best Regards,
Andy Shevchenko

linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi all,

Commit

  bba074f926d1 ("platform/x86: silead_dmi: Add entry for Chuwi Hi8 S806_206 
tablet touchscreen")

is missing a Signed-off-by from its author.

-- 
Cheers,
Stephen Rothwell


pgpFtHG0Jixuv.pgp
Description: OpenPGP digital signature

linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi all,

Commit

  bba074f926d1 ("platform/x86: silead_dmi: Add entry for Chuwi Hi8 S806_206 
tablet touchscreen")

is missing a Signed-off-by from its author.

-- 
Cheers,
Stephen Rothwell


pgpFtHG0Jixuv.pgp
Description: OpenPGP digital signature

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

On Sat, Aug 05, 2017 at 02:58:29PM -0700, Darren Hart wrote:
> On Thu, Aug 03, 2017 at 08:50:06AM -0700, Darren Hart wrote:
> > On Wed, Aug 02, 2017 at 06:06:20PM -0700, Linus Torvalds wrote:
> > > On Wed, Aug 2, 2017 at 5:28 PM, Stephen Rothwell  
> > > wrote:
> > > >
> > > > I would say that if you rebase someone's commit(s), then you are on the
> > > > "patch's delivery path" and so should add a Signed-off-by tag.
> > > 
> > > Yeah, I agree. Rebasing really is pretty much the exact same thing as
> > > applying a patch.
> 
> I will be away for a few days, but will follow up on this when I return.
> In the meantime, my plan is to leave the current for-next branch alone
> rather than rebasing it to fix the previous rebase which resulted in the
> mixed committer/signoff issue Stephen's new test identified.
> 
> I just want it to be clear I'm not ignoring the issue, but rather
> planning on addressing it in commits going forward - based on the
> results of the discussion below.
> 

OK, with no additional feedback here, Andy and I have discussed and we will
adapt our process by using individual review branches which 0-day can pull from
which are considered transient and mutable. After this, the patches will be
added to the common testing branch, which will now be fast-forward only [1].
After a short period, testing will move to for-next and fixes branches in
preparation for pull-requests, just as before.

Thanks,

1. We may eliminate the testing branch as it may not offer any value over
   for-next, but we'll work through at least one release cycle before doing
   so.

-- 
Darren Hart
VMware Open Source Technology Center

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

On Sat, Aug 05, 2017 at 02:58:29PM -0700, Darren Hart wrote:
> On Thu, Aug 03, 2017 at 08:50:06AM -0700, Darren Hart wrote:
> > On Wed, Aug 02, 2017 at 06:06:20PM -0700, Linus Torvalds wrote:
> > > On Wed, Aug 2, 2017 at 5:28 PM, Stephen Rothwell  
> > > wrote:
> > > >
> > > > I would say that if you rebase someone's commit(s), then you are on the
> > > > "patch's delivery path" and so should add a Signed-off-by tag.
> > > 
> > > Yeah, I agree. Rebasing really is pretty much the exact same thing as
> > > applying a patch.
> 
> I will be away for a few days, but will follow up on this when I return.
> In the meantime, my plan is to leave the current for-next branch alone
> rather than rebasing it to fix the previous rebase which resulted in the
> mixed committer/signoff issue Stephen's new test identified.
> 
> I just want it to be clear I'm not ignoring the issue, but rather
> planning on addressing it in commits going forward - based on the
> results of the discussion below.
> 

OK, with no additional feedback here, Andy and I have discussed and we will
adapt our process by using individual review branches which 0-day can pull from
which are considered transient and mutable. After this, the patches will be
added to the common testing branch, which will now be fast-forward only [1].
After a short period, testing will move to for-next and fixes branches in
preparation for pull-requests, just as before.

Thanks,

1. We may eliminate the testing branch as it may not offer any value over
   for-next, but we'll work through at least one release cycle before doing
   so.

-- 
Darren Hart
VMware Open Source Technology Center

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

+Olof and Arnd,

I am curious how you handle the situation below as a maintainer team.

This problem arose from a new for-next test which triggers on the
committer not having a SOB tag in the patch (which can happen when a
shared branch is rebased to drop a patch).

Do you have a branch that you both use for testing and automated testing
which you occasionally need to drop patches from before moving them to a
publication branch like "for-next" or "fixes"? I understand you tend to
pull from sub-maintainers, so perhaps our contexts are fairly different.

Andy and I have brainstormed various approaches to addressing this, and
all of the cures appear worse than the disease from a scalability and/or
chance of error perspective (outlined below).

Linus has been clear he sees "rebase --signoff" to be the wrong thing to
do for "publicized branches" (see my comment below on published vs.
collaboration branches).

On Sat, Aug 05, 2017 at 02:58:29PM -0700, Darren Hart wrote:
> On Thu, Aug 03, 2017 at 08:50:06AM -0700, Darren Hart wrote:
> > On Wed, Aug 02, 2017 at 06:06:20PM -0700, Linus Torvalds wrote:
> > > On Wed, Aug 2, 2017 at 5:28 PM, Stephen Rothwell  
> > > wrote:
> > > >
> > > > I would say that if you rebase someone's commit(s), then you are on the
> > > > "patch's delivery path" and so should add a Signed-off-by tag.
> > > 
> > > Yeah, I agree. Rebasing really is pretty much the exact same thing as
> > > applying a patch.
> 
> I will be away for a few days, but will follow up on this when I return.
> In the meantime, my plan is to leave the current for-next branch alone
> rather than rebasing it to fix the previous rebase which resulted in the
> mixed committer/signoff issue Stephen's new test identified.
> 
> I just want it to be clear I'm not ignoring the issue, but rather
> planning on addressing it in commits going forward - based on the
> results of the discussion below.
> 
> Thanks,
> 
> > > 
> > > > "git rebase" does have a "--signoff" option.
> > > 
> > > I think you end up signing off twice using that. I don't think it's
> > > smart enough to say "oh, you already did it once".
> > > 
> > > But I didn't check. Sometimes git is a lot smarter than I remember it
> > > being, simply because I don't worry about it. Junio does a good job.
> > > 
> > > And in general, you simply should never rebase commits that have
> > > already been publicized. And the fact that you didn't commit them in
> > > the first place definitely means that they've been public somewhere.
> > 
> > For the platform driver x86 subsystem, Andy I have defined our "testing"
> > branch as mutable. It's the place where our CI pulls from, as well as
> > the first place 0day pulls from, and where we stage things prior to
> > going to the publication branches ("for-next" and then sometimes
> > "fixes"). We find it valuable to let the robots have a chance to catch
> > issues we may have missed before pushing patches to a publication
> > branch, but to do that, we need the testing branch to be accessible to
> > them.
> > 
> > The usual case that would land us in the situation here is we discover a
> > bug in a patch and revert it before going to a publication branch.
> > Generally, this will involve one file (most patches here are isolated),
> > which we drop via rebase, and the rest are entirely unaffected in terms
> > of code, but as the tree changed under them, they get "re-committed".
> > 
> > This seems like a reasonable way to handle a tree with more than one
> > maintainer and take advantage of some automation. Andy and I do need a
> > common tree to work from, and I prefer to sync with him as early in the
> > process as possible, rather than have him and I work with two private
> > testing branches and have to negotiate who takes which patches. It would
> > slow us down and wouldn't improve quality in any measurable way. Even if
> > we did this work in an access controlled repository, we would still have
> > this problem.
> > 
> > With more and more maintainer teams, I think we need to distinguish
> > between "published" branches and "collaboration" branches. I suspect
> > maintainer teams will expose this rebasing behavior, but I don't believe
> > it is new or unique to us. To collaborate, we need a common branch,
> > which a lone maintainer doesn't need, and the committer/sign-off delta
> > makes this discoverable, whereas it was invisible with a lone
> > maintainer.
> > 
> > Note: A guiding principle behind our process is that of not introducing
> > bugs into mainline. Rather than reverting bad patches in testing, we
> > drop them, and replace them with a fixed version. The idea being we
> > don't want to introduce git bisect breakage, and we don't want to open
> > the window for stable/distro maintainers to pull a bad patch and forget
> > the revert or the fixup. If we can correct it before it goes to Linus,
> > we do.
> > 
> > > So I would definitely suggest against the "git rebase --signoff"
> > > model, 

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

+Olof and Arnd,

I am curious how you handle the situation below as a maintainer team.

This problem arose from a new for-next test which triggers on the
committer not having a SOB tag in the patch (which can happen when a
shared branch is rebased to drop a patch).

Do you have a branch that you both use for testing and automated testing
which you occasionally need to drop patches from before moving them to a
publication branch like "for-next" or "fixes"? I understand you tend to
pull from sub-maintainers, so perhaps our contexts are fairly different.

Andy and I have brainstormed various approaches to addressing this, and
all of the cures appear worse than the disease from a scalability and/or
chance of error perspective (outlined below).

Linus has been clear he sees "rebase --signoff" to be the wrong thing to
do for "publicized branches" (see my comment below on published vs.
collaboration branches).

On Sat, Aug 05, 2017 at 02:58:29PM -0700, Darren Hart wrote:
> On Thu, Aug 03, 2017 at 08:50:06AM -0700, Darren Hart wrote:
> > On Wed, Aug 02, 2017 at 06:06:20PM -0700, Linus Torvalds wrote:
> > > On Wed, Aug 2, 2017 at 5:28 PM, Stephen Rothwell  
> > > wrote:
> > > >
> > > > I would say that if you rebase someone's commit(s), then you are on the
> > > > "patch's delivery path" and so should add a Signed-off-by tag.
> > > 
> > > Yeah, I agree. Rebasing really is pretty much the exact same thing as
> > > applying a patch.
> 
> I will be away for a few days, but will follow up on this when I return.
> In the meantime, my plan is to leave the current for-next branch alone
> rather than rebasing it to fix the previous rebase which resulted in the
> mixed committer/signoff issue Stephen's new test identified.
> 
> I just want it to be clear I'm not ignoring the issue, but rather
> planning on addressing it in commits going forward - based on the
> results of the discussion below.
> 
> Thanks,
> 
> > > 
> > > > "git rebase" does have a "--signoff" option.
> > > 
> > > I think you end up signing off twice using that. I don't think it's
> > > smart enough to say "oh, you already did it once".
> > > 
> > > But I didn't check. Sometimes git is a lot smarter than I remember it
> > > being, simply because I don't worry about it. Junio does a good job.
> > > 
> > > And in general, you simply should never rebase commits that have
> > > already been publicized. And the fact that you didn't commit them in
> > > the first place definitely means that they've been public somewhere.
> > 
> > For the platform driver x86 subsystem, Andy I have defined our "testing"
> > branch as mutable. It's the place where our CI pulls from, as well as
> > the first place 0day pulls from, and where we stage things prior to
> > going to the publication branches ("for-next" and then sometimes
> > "fixes"). We find it valuable to let the robots have a chance to catch
> > issues we may have missed before pushing patches to a publication
> > branch, but to do that, we need the testing branch to be accessible to
> > them.
> > 
> > The usual case that would land us in the situation here is we discover a
> > bug in a patch and revert it before going to a publication branch.
> > Generally, this will involve one file (most patches here are isolated),
> > which we drop via rebase, and the rest are entirely unaffected in terms
> > of code, but as the tree changed under them, they get "re-committed".
> > 
> > This seems like a reasonable way to handle a tree with more than one
> > maintainer and take advantage of some automation. Andy and I do need a
> > common tree to work from, and I prefer to sync with him as early in the
> > process as possible, rather than have him and I work with two private
> > testing branches and have to negotiate who takes which patches. It would
> > slow us down and wouldn't improve quality in any measurable way. Even if
> > we did this work in an access controlled repository, we would still have
> > this problem.
> > 
> > With more and more maintainer teams, I think we need to distinguish
> > between "published" branches and "collaboration" branches. I suspect
> > maintainer teams will expose this rebasing behavior, but I don't believe
> > it is new or unique to us. To collaborate, we need a common branch,
> > which a lone maintainer doesn't need, and the committer/sign-off delta
> > makes this discoverable, whereas it was invisible with a lone
> > maintainer.
> > 
> > Note: A guiding principle behind our process is that of not introducing
> > bugs into mainline. Rather than reverting bad patches in testing, we
> > drop them, and replace them with a fixed version. The idea being we
> > don't want to introduce git bisect breakage, and we don't want to open
> > the window for stable/distro maintainers to pull a bad patch and forget
> > the revert or the fixup. If we can correct it before it goes to Linus,
> > we do.
> > 
> > > So I would definitely suggest against the "git rebase --signoff"
> > > model, even if git were to do 

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

On Thu, Aug 03, 2017 at 08:50:06AM -0700, Darren Hart wrote:
> On Wed, Aug 02, 2017 at 06:06:20PM -0700, Linus Torvalds wrote:
> > On Wed, Aug 2, 2017 at 5:28 PM, Stephen Rothwell  
> > wrote:
> > >
> > > I would say that if you rebase someone's commit(s), then you are on the
> > > "patch's delivery path" and so should add a Signed-off-by tag.
> > 
> > Yeah, I agree. Rebasing really is pretty much the exact same thing as
> > applying a patch.

I will be away for a few days, but will follow up on this when I return.
In the meantime, my plan is to leave the current for-next branch alone
rather than rebasing it to fix the previous rebase which resulted in the
mixed committer/signoff issue Stephen's new test identified.

I just want it to be clear I'm not ignoring the issue, but rather
planning on addressing it in commits going forward - based on the
results of the discussion below.

Thanks,

> > 
> > > "git rebase" does have a "--signoff" option.
> > 
> > I think you end up signing off twice using that. I don't think it's
> > smart enough to say "oh, you already did it once".
> > 
> > But I didn't check. Sometimes git is a lot smarter than I remember it
> > being, simply because I don't worry about it. Junio does a good job.
> > 
> > And in general, you simply should never rebase commits that have
> > already been publicized. And the fact that you didn't commit them in
> > the first place definitely means that they've been public somewhere.
> 
> For the platform driver x86 subsystem, Andy I have defined our "testing"
> branch as mutable. It's the place where our CI pulls from, as well as
> the first place 0day pulls from, and where we stage things prior to
> going to the publication branches ("for-next" and then sometimes
> "fixes"). We find it valuable to let the robots have a chance to catch
> issues we may have missed before pushing patches to a publication
> branch, but to do that, we need the testing branch to be accessible to
> them.
> 
> The usual case that would land us in the situation here is we discover a
> bug in a patch and revert it before going to a publication branch.
> Generally, this will involve one file (most patches here are isolated),
> which we drop via rebase, and the rest are entirely unaffected in terms
> of code, but as the tree changed under them, they get "re-committed".
> 
> This seems like a reasonable way to handle a tree with more than one
> maintainer and take advantage of some automation. Andy and I do need a
> common tree to work from, and I prefer to sync with him as early in the
> process as possible, rather than have him and I work with two private
> testing branches and have to negotiate who takes which patches. It would
> slow us down and wouldn't improve quality in any measurable way. Even if
> we did this work in an access controlled repository, we would still have
> this problem.
> 
> With more and more maintainer teams, I think we need to distinguish
> between "published" branches and "collaboration" branches. I suspect
> maintainer teams will expose this rebasing behavior, but I don't believe
> it is new or unique to us. To collaborate, we need a common branch,
> which a lone maintainer doesn't need, and the committer/sign-off delta
> makes this discoverable, whereas it was invisible with a lone
> maintainer.
> 
> Note: A guiding principle behind our process is that of not introducing
> bugs into mainline. Rather than reverting bad patches in testing, we
> drop them, and replace them with a fixed version. The idea being we
> don't want to introduce git bisect breakage, and we don't want to open
> the window for stable/distro maintainers to pull a bad patch and forget
> the revert or the fixup. If we can correct it before it goes to Linus,
> we do.
> 
> > So I would definitely suggest against the "git rebase --signoff"
> > model, even if git were to do the "right thing". It's simply
> > fundamentally the wrong thing to do. Either you already committed them
> > (and hopefully signed off correctly the first time), or you didn't
> > (and you shouldn't be rebasing). So in neither case is "git rebase
> > --signoff" sensible.
> 
> So in light of the above, we can:
> 
> a) Keep doing what we're doing
> b) Sign off whenever we rebase
> c) Add our signoff whenever we move patches from testing to for-next
>(I hadn't considered this until now... this might be the most
> compatible with maintainer teams while strictly tracking the
> "patches" delivery path")
> d) Redefine testing as immutable and revert patches rather than drop
>them, introducing bugs into mainline.
> e) Make each maintainer work from a private set of branches (this just
>masks the problem by making the rebase invisible)
> 
> Whatever we decide, I'd like to add this to some documentation for
> maintainer teams (which I'm happy to prepare and submit).

-- 
Darren Hart
VMware Open Source Technology Center

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

On Thu, Aug 03, 2017 at 08:50:06AM -0700, Darren Hart wrote:
> On Wed, Aug 02, 2017 at 06:06:20PM -0700, Linus Torvalds wrote:
> > On Wed, Aug 2, 2017 at 5:28 PM, Stephen Rothwell  
> > wrote:
> > >
> > > I would say that if you rebase someone's commit(s), then you are on the
> > > "patch's delivery path" and so should add a Signed-off-by tag.
> > 
> > Yeah, I agree. Rebasing really is pretty much the exact same thing as
> > applying a patch.

I will be away for a few days, but will follow up on this when I return.
In the meantime, my plan is to leave the current for-next branch alone
rather than rebasing it to fix the previous rebase which resulted in the
mixed committer/signoff issue Stephen's new test identified.

I just want it to be clear I'm not ignoring the issue, but rather
planning on addressing it in commits going forward - based on the
results of the discussion below.

Thanks,

> > 
> > > "git rebase" does have a "--signoff" option.
> > 
> > I think you end up signing off twice using that. I don't think it's
> > smart enough to say "oh, you already did it once".
> > 
> > But I didn't check. Sometimes git is a lot smarter than I remember it
> > being, simply because I don't worry about it. Junio does a good job.
> > 
> > And in general, you simply should never rebase commits that have
> > already been publicized. And the fact that you didn't commit them in
> > the first place definitely means that they've been public somewhere.
> 
> For the platform driver x86 subsystem, Andy I have defined our "testing"
> branch as mutable. It's the place where our CI pulls from, as well as
> the first place 0day pulls from, and where we stage things prior to
> going to the publication branches ("for-next" and then sometimes
> "fixes"). We find it valuable to let the robots have a chance to catch
> issues we may have missed before pushing patches to a publication
> branch, but to do that, we need the testing branch to be accessible to
> them.
> 
> The usual case that would land us in the situation here is we discover a
> bug in a patch and revert it before going to a publication branch.
> Generally, this will involve one file (most patches here are isolated),
> which we drop via rebase, and the rest are entirely unaffected in terms
> of code, but as the tree changed under them, they get "re-committed".
> 
> This seems like a reasonable way to handle a tree with more than one
> maintainer and take advantage of some automation. Andy and I do need a
> common tree to work from, and I prefer to sync with him as early in the
> process as possible, rather than have him and I work with two private
> testing branches and have to negotiate who takes which patches. It would
> slow us down and wouldn't improve quality in any measurable way. Even if
> we did this work in an access controlled repository, we would still have
> this problem.
> 
> With more and more maintainer teams, I think we need to distinguish
> between "published" branches and "collaboration" branches. I suspect
> maintainer teams will expose this rebasing behavior, but I don't believe
> it is new or unique to us. To collaborate, we need a common branch,
> which a lone maintainer doesn't need, and the committer/sign-off delta
> makes this discoverable, whereas it was invisible with a lone
> maintainer.
> 
> Note: A guiding principle behind our process is that of not introducing
> bugs into mainline. Rather than reverting bad patches in testing, we
> drop them, and replace them with a fixed version. The idea being we
> don't want to introduce git bisect breakage, and we don't want to open
> the window for stable/distro maintainers to pull a bad patch and forget
> the revert or the fixup. If we can correct it before it goes to Linus,
> we do.
> 
> > So I would definitely suggest against the "git rebase --signoff"
> > model, even if git were to do the "right thing". It's simply
> > fundamentally the wrong thing to do. Either you already committed them
> > (and hopefully signed off correctly the first time), or you didn't
> > (and you shouldn't be rebasing). So in neither case is "git rebase
> > --signoff" sensible.
> 
> So in light of the above, we can:
> 
> a) Keep doing what we're doing
> b) Sign off whenever we rebase
> c) Add our signoff whenever we move patches from testing to for-next
>(I hadn't considered this until now... this might be the most
> compatible with maintainer teams while strictly tracking the
> "patches" delivery path")
> d) Redefine testing as immutable and revert patches rather than drop
>them, introducing bugs into mainline.
> e) Make each maintainer work from a private set of branches (this just
>masks the problem by making the rebase invisible)
> 
> Whatever we decide, I'd like to add this to some documentation for
> maintainer teams (which I'm happy to prepare and submit).

-- 
Darren Hart
VMware Open Source Technology Center

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

On Fri, Aug 04, 2017 at 10:44:31AM -0700, Junio C Hamano wrote:
> Linus Torvalds  writes:
> 
> > On Wed, Aug 2, 2017 at 5:28 PM, Stephen Rothwell  
> > wrote:
> >>
> >> I would say that if you rebase someone's commit(s), then you are on the
> >> "patch's delivery path" and so should add a Signed-off-by tag.
> >
> > Yeah, I agree. Rebasing really is pretty much the exact same thing as
> > applying a patch.
> >
> >> "git rebase" does have a "--signoff" option.
> >
> > I think you end up signing off twice using that. I don't think it's
> > smart enough to say "oh, you already did it once".
> 
> Git avoids duplication only when your SoB appears as the last
> existing one, so that we can capture a flow of a patch which you
> originally signed off, picked up and tweaked further by somebody
> else, which comes back to you and you sign it off again.
> 
> We may drop yours even when yours is not the last in the existing
> chain, but that would be a bug; at least the above is what we try to
> do.
> 
> > And in general, you simply should never rebase commits that have
> > already been publicized. And the fact that you didn't commit them in
> > the first place definitely means that they've been public somewhere.
> >
> > So I would definitely suggest against the "git rebase --signoff"
> > model, even if git were to do the "right thing". It's simply
> > fundamentally the wrong thing to do.
> 
> When those involved are using push/pull as a replacement for
> e-mailed patch exchange, then such a workflow should be OK.  There
> needs to be a shared understanding that the branch(es) used for such
> exchange are unstable and should not be built directly on to be
> merged, of course.
> 

Thanks Junio,

I don't think I correctly parsed "should not be built directly on to be
merged", can you rephrase?

-- 
Darren Hart
VMware Open Source Technology Center

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

On Fri, Aug 04, 2017 at 10:44:31AM -0700, Junio C Hamano wrote:
> Linus Torvalds  writes:
> 
> > On Wed, Aug 2, 2017 at 5:28 PM, Stephen Rothwell  
> > wrote:
> >>
> >> I would say that if you rebase someone's commit(s), then you are on the
> >> "patch's delivery path" and so should add a Signed-off-by tag.
> >
> > Yeah, I agree. Rebasing really is pretty much the exact same thing as
> > applying a patch.
> >
> >> "git rebase" does have a "--signoff" option.
> >
> > I think you end up signing off twice using that. I don't think it's
> > smart enough to say "oh, you already did it once".
> 
> Git avoids duplication only when your SoB appears as the last
> existing one, so that we can capture a flow of a patch which you
> originally signed off, picked up and tweaked further by somebody
> else, which comes back to you and you sign it off again.
> 
> We may drop yours even when yours is not the last in the existing
> chain, but that would be a bug; at least the above is what we try to
> do.
> 
> > And in general, you simply should never rebase commits that have
> > already been publicized. And the fact that you didn't commit them in
> > the first place definitely means that they've been public somewhere.
> >
> > So I would definitely suggest against the "git rebase --signoff"
> > model, even if git were to do the "right thing". It's simply
> > fundamentally the wrong thing to do.
> 
> When those involved are using push/pull as a replacement for
> e-mailed patch exchange, then such a workflow should be OK.  There
> needs to be a shared understanding that the branch(es) used for such
> exchange are unstable and should not be built directly on to be
> merged, of course.
> 

Thanks Junio,

I don't think I correctly parsed "should not be built directly on to be
merged", can you rephrase?

-- 
Darren Hart
VMware Open Source Technology Center

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Junio C Hamano]

Linus Torvalds  writes:

> On Wed, Aug 2, 2017 at 5:28 PM, Stephen Rothwell  
> wrote:
>>
>> I would say that if you rebase someone's commit(s), then you are on the
>> "patch's delivery path" and so should add a Signed-off-by tag.
>
> Yeah, I agree. Rebasing really is pretty much the exact same thing as
> applying a patch.
>
>> "git rebase" does have a "--signoff" option.
>
> I think you end up signing off twice using that. I don't think it's
> smart enough to say "oh, you already did it once".

Git avoids duplication only when your SoB appears as the last
existing one, so that we can capture a flow of a patch which you
originally signed off, picked up and tweaked further by somebody
else, which comes back to you and you sign it off again.

We may drop yours even when yours is not the last in the existing
chain, but that would be a bug; at least the above is what we try to
do.

> And in general, you simply should never rebase commits that have
> already been publicized. And the fact that you didn't commit them in
> the first place definitely means that they've been public somewhere.
>
> So I would definitely suggest against the "git rebase --signoff"
> model, even if git were to do the "right thing". It's simply
> fundamentally the wrong thing to do.

When those involved are using push/pull as a replacement for
e-mailed patch exchange, then such a workflow should be OK.  There
needs to be a shared understanding that the branch(es) used for such
exchange are unstable and should not be built directly on to be
merged, of course.

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Junio C Hamano]

Linus Torvalds  writes:

> On Wed, Aug 2, 2017 at 5:28 PM, Stephen Rothwell  
> wrote:
>>
>> I would say that if you rebase someone's commit(s), then you are on the
>> "patch's delivery path" and so should add a Signed-off-by tag.
>
> Yeah, I agree. Rebasing really is pretty much the exact same thing as
> applying a patch.
>
>> "git rebase" does have a "--signoff" option.
>
> I think you end up signing off twice using that. I don't think it's
> smart enough to say "oh, you already did it once".

Git avoids duplication only when your SoB appears as the last
existing one, so that we can capture a flow of a patch which you
originally signed off, picked up and tweaked further by somebody
else, which comes back to you and you sign it off again.

We may drop yours even when yours is not the last in the existing
chain, but that would be a bug; at least the above is what we try to
do.

> And in general, you simply should never rebase commits that have
> already been publicized. And the fact that you didn't commit them in
> the first place definitely means that they've been public somewhere.
>
> So I would definitely suggest against the "git rebase --signoff"
> model, even if git were to do the "right thing". It's simply
> fundamentally the wrong thing to do.

When those involved are using push/pull as a replacement for
e-mailed patch exchange, then such a workflow should be OK.  There
needs to be a shared understanding that the branch(es) used for such
exchange are unstable and should not be built directly on to be
merged, of course.

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

On Wed, Aug 02, 2017 at 06:06:20PM -0700, Linus Torvalds wrote:
> On Wed, Aug 2, 2017 at 5:28 PM, Stephen Rothwell  
> wrote:
> >
> > I would say that if you rebase someone's commit(s), then you are on the
> > "patch's delivery path" and so should add a Signed-off-by tag.
> 
> Yeah, I agree. Rebasing really is pretty much the exact same thing as
> applying a patch.
> 
> > "git rebase" does have a "--signoff" option.
> 
> I think you end up signing off twice using that. I don't think it's
> smart enough to say "oh, you already did it once".
> 
> But I didn't check. Sometimes git is a lot smarter than I remember it
> being, simply because I don't worry about it. Junio does a good job.
> 
> And in general, you simply should never rebase commits that have
> already been publicized. And the fact that you didn't commit them in
> the first place definitely means that they've been public somewhere.

For the platform driver x86 subsystem, Andy I have defined our "testing"
branch as mutable. It's the place where our CI pulls from, as well as
the first place 0day pulls from, and where we stage things prior to
going to the publication branches ("for-next" and then sometimes
"fixes"). We find it valuable to let the robots have a chance to catch
issues we may have missed before pushing patches to a publication
branch, but to do that, we need the testing branch to be accessible to
them.

The usual case that would land us in the situation here is we discover a
bug in a patch and revert it before going to a publication branch.
Generally, this will involve one file (most patches here are isolated),
which we drop via rebase, and the rest are entirely unaffected in terms
of code, but as the tree changed under them, they get "re-committed".

This seems like a reasonable way to handle a tree with more than one
maintainer and take advantage of some automation. Andy and I do need a
common tree to work from, and I prefer to sync with him as early in the
process as possible, rather than have him and I work with two private
testing branches and have to negotiate who takes which patches. It would
slow us down and wouldn't improve quality in any measurable way. Even if
we did this work in an access controlled repository, we would still have
this problem.

With more and more maintainer teams, I think we need to distinguish
between "published" branches and "collaboration" branches. I suspect
maintainer teams will expose this rebasing behavior, but I don't believe
it is new or unique to us. To collaborate, we need a common branch,
which a lone maintainer doesn't need, and the committer/sign-off delta
makes this discoverable, whereas it was invisible with a lone
maintainer.

Note: A guiding principle behind our process is that of not introducing
bugs into mainline. Rather than reverting bad patches in testing, we
drop them, and replace them with a fixed version. The idea being we
don't want to introduce git bisect breakage, and we don't want to open
the window for stable/distro maintainers to pull a bad patch and forget
the revert or the fixup. If we can correct it before it goes to Linus,
we do.

> So I would definitely suggest against the "git rebase --signoff"
> model, even if git were to do the "right thing". It's simply
> fundamentally the wrong thing to do. Either you already committed them
> (and hopefully signed off correctly the first time), or you didn't
> (and you shouldn't be rebasing). So in neither case is "git rebase
> --signoff" sensible.

So in light of the above, we can:

a) Keep doing what we're doing
b) Sign off whenever we rebase
c) Add our signoff whenever we move patches from testing to for-next
   (I hadn't considered this until now... this might be the most
compatible with maintainer teams while strictly tracking the
"patches" delivery path")
d) Redefine testing as immutable and revert patches rather than drop
   them, introducing bugs into mainline.
e) Make each maintainer work from a private set of branches (this just
   masks the problem by making the rebase invisible)

Whatever we decide, I'd like to add this to some documentation for
maintainer teams (which I'm happy to prepare and submit).

Thanks,

-- 
Darren Hart
VMware Open Source Technology Center

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

On Wed, Aug 02, 2017 at 06:06:20PM -0700, Linus Torvalds wrote:
> On Wed, Aug 2, 2017 at 5:28 PM, Stephen Rothwell  
> wrote:
> >
> > I would say that if you rebase someone's commit(s), then you are on the
> > "patch's delivery path" and so should add a Signed-off-by tag.
> 
> Yeah, I agree. Rebasing really is pretty much the exact same thing as
> applying a patch.
> 
> > "git rebase" does have a "--signoff" option.
> 
> I think you end up signing off twice using that. I don't think it's
> smart enough to say "oh, you already did it once".
> 
> But I didn't check. Sometimes git is a lot smarter than I remember it
> being, simply because I don't worry about it. Junio does a good job.
> 
> And in general, you simply should never rebase commits that have
> already been publicized. And the fact that you didn't commit them in
> the first place definitely means that they've been public somewhere.

For the platform driver x86 subsystem, Andy I have defined our "testing"
branch as mutable. It's the place where our CI pulls from, as well as
the first place 0day pulls from, and where we stage things prior to
going to the publication branches ("for-next" and then sometimes
"fixes"). We find it valuable to let the robots have a chance to catch
issues we may have missed before pushing patches to a publication
branch, but to do that, we need the testing branch to be accessible to
them.

The usual case that would land us in the situation here is we discover a
bug in a patch and revert it before going to a publication branch.
Generally, this will involve one file (most patches here are isolated),
which we drop via rebase, and the rest are entirely unaffected in terms
of code, but as the tree changed under them, they get "re-committed".

This seems like a reasonable way to handle a tree with more than one
maintainer and take advantage of some automation. Andy and I do need a
common tree to work from, and I prefer to sync with him as early in the
process as possible, rather than have him and I work with two private
testing branches and have to negotiate who takes which patches. It would
slow us down and wouldn't improve quality in any measurable way. Even if
we did this work in an access controlled repository, we would still have
this problem.

With more and more maintainer teams, I think we need to distinguish
between "published" branches and "collaboration" branches. I suspect
maintainer teams will expose this rebasing behavior, but I don't believe
it is new or unique to us. To collaborate, we need a common branch,
which a lone maintainer doesn't need, and the committer/sign-off delta
makes this discoverable, whereas it was invisible with a lone
maintainer.

Note: A guiding principle behind our process is that of not introducing
bugs into mainline. Rather than reverting bad patches in testing, we
drop them, and replace them with a fixed version. The idea being we
don't want to introduce git bisect breakage, and we don't want to open
the window for stable/distro maintainers to pull a bad patch and forget
the revert or the fixup. If we can correct it before it goes to Linus,
we do.

> So I would definitely suggest against the "git rebase --signoff"
> model, even if git were to do the "right thing". It's simply
> fundamentally the wrong thing to do. Either you already committed them
> (and hopefully signed off correctly the first time), or you didn't
> (and you shouldn't be rebasing). So in neither case is "git rebase
> --signoff" sensible.

So in light of the above, we can:

a) Keep doing what we're doing
b) Sign off whenever we rebase
c) Add our signoff whenever we move patches from testing to for-next
   (I hadn't considered this until now... this might be the most
compatible with maintainer teams while strictly tracking the
"patches" delivery path")
d) Redefine testing as immutable and revert patches rather than drop
   them, introducing bugs into mainline.
e) Make each maintainer work from a private set of branches (this just
   masks the problem by making the rebase invisible)

Whatever we decide, I'd like to add this to some documentation for
maintainer teams (which I'm happy to prepare and submit).

Thanks,

-- 
Darren Hart
VMware Open Source Technology Center

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi Andy,

On Thu, 03 Aug 2017 11:17:03 +0300 Andy Shevchenko 
 wrote:
>
> I just checked what we have in our for-next branch and mentioned commits
> have mine SoB. Should they have something else?

In Darren's response it became clear that even though you had initially
commited the patches, he had rebased them so he is now the committer.
So he should have added a Signed-off-by tag.


-- 
Cheers,
Stephen Rothwell

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi Andy,

On Thu, 03 Aug 2017 11:17:03 +0300 Andy Shevchenko 
 wrote:
>
> I just checked what we have in our for-next branch and mentioned commits
> have mine SoB. Should they have something else?

In Darren's response it became clear that even though you had initially
commited the patches, he had rebased them so he is now the committer.
So he should have added a Signed-off-by tag.


-- 
Cheers,
Stephen Rothwell

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Andy Shevchenko]

On Thu, 2017-08-03 at 06:37 +1000, Stephen Rothwell wrote:
> Hi Darren,
> 
> Commits
> 
>   890f658c101d ("platform/x86: peaq-wmi: silence a static checker
> warning")
>   6d8d55626296 ("platform/x86: msi-wmi: remove unnecessary static in
> msi_wmi_notify()")
>   cd0223c64c60 ("platform/x86: ibm_rtl: remove unnecessary static in
> ibm_rtl_write()")
> 
> are missing Signed-off-by's from their commiter.
> 

I just checked what we have in our for-next branch and mentioned commits
have mine SoB. Should they have something else?

-- 
Andy Shevchenko 
Intel Finland Oy

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Andy Shevchenko]

On Thu, 2017-08-03 at 06:37 +1000, Stephen Rothwell wrote:
> Hi Darren,
> 
> Commits
> 
>   890f658c101d ("platform/x86: peaq-wmi: silence a static checker
> warning")
>   6d8d55626296 ("platform/x86: msi-wmi: remove unnecessary static in
> msi_wmi_notify()")
>   cd0223c64c60 ("platform/x86: ibm_rtl: remove unnecessary static in
> ibm_rtl_write()")
> 
> are missing Signed-off-by's from their commiter.
> 

I just checked what we have in our for-next branch and mentioned commits
have mine SoB. Should they have something else?

-- 
Andy Shevchenko 
Intel Finland Oy

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Linus Torvalds]

On Wed, Aug 2, 2017 at 5:28 PM, Stephen Rothwell  wrote:
>
> I would say that if you rebase someone's commit(s), then you are on the
> "patch's delivery path" and so should add a Signed-off-by tag.

Yeah, I agree. Rebasing really is pretty much the exact same thing as
applying a patch.

> "git rebase" does have a "--signoff" option.

I think you end up signing off twice using that. I don't think it's
smart enough to say "oh, you already did it once".

But I didn't check. Sometimes git is a lot smarter than I remember it
being, simply because I don't worry about it. Junio does a good job.

And in general, you simply should never rebase commits that have
already been publicized. And the fact that you didn't commit them in
the first place definitely means that they've been public somewhere.

So I would definitely suggest against the "git rebase --signoff"
model, even if git were to do the "right thing". It's simply
fundamentally the wrong thing to do. Either you already committed them
(and hopefully signed off correctly the first time), or you didn't
(and you shouldn't be rebasing). So in neither case is "git rebase
--signoff" sensible.

  Linus

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Linus Torvalds]

On Wed, Aug 2, 2017 at 5:28 PM, Stephen Rothwell  wrote:
>
> I would say that if you rebase someone's commit(s), then you are on the
> "patch's delivery path" and so should add a Signed-off-by tag.

Yeah, I agree. Rebasing really is pretty much the exact same thing as
applying a patch.

> "git rebase" does have a "--signoff" option.

I think you end up signing off twice using that. I don't think it's
smart enough to say "oh, you already did it once".

But I didn't check. Sometimes git is a lot smarter than I remember it
being, simply because I don't worry about it. Junio does a good job.

And in general, you simply should never rebase commits that have
already been publicized. And the fact that you didn't commit them in
the first place definitely means that they've been public somewhere.

So I would definitely suggest against the "git rebase --signoff"
model, even if git were to do the "right thing". It's simply
fundamentally the wrong thing to do. Either you already committed them
(and hopefully signed off correctly the first time), or you didn't
(and you shouldn't be rebasing). So in neither case is "git rebase
--signoff" sensible.

  Linus

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi Darren,

On Wed, 2 Aug 2017 16:57:40 -0700 Darren Hart  wrote:
>
> Is this a new check Stephen?

Yes :-)

> Is there any statement regarding maintainer teams that we must abide by
> this?  e.g. any time a rebase in a testing branch is made, the
> maintainer must also ensure a SOB is on each patch?

I would say that if you rebase someone's commit(s), then you are on the
"patch's delivery path" and so should add a Signed-off-by tag. (cc'ing
Linus to see if he has an opinion.  Linus, the case here is a patch
originally committed by one maintainer and then rebased by the other.)

> I just want to get a clear picture of what the failure was so we can
> update our tooling so this doesn't repeat itself.

"git rebase" does have a "--signoff" option.

-- 
Cheers,
Stephen Rothwell

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi Darren,

On Wed, 2 Aug 2017 16:57:40 -0700 Darren Hart  wrote:
>
> Is this a new check Stephen?

Yes :-)

> Is there any statement regarding maintainer teams that we must abide by
> this?  e.g. any time a rebase in a testing branch is made, the
> maintainer must also ensure a SOB is on each patch?

I would say that if you rebase someone's commit(s), then you are on the
"patch's delivery path" and so should add a Signed-off-by tag. (cc'ing
Linus to see if he has an opinion.  Linus, the case here is a patch
originally committed by one maintainer and then rebased by the other.)

> I just want to get a clear picture of what the failure was so we can
> update our tooling so this doesn't repeat itself.

"git rebase" does have a "--signoff" option.

-- 
Cheers,
Stephen Rothwell

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

On Thu, Aug 03, 2017 at 06:37:43AM +1000, Stephen Rothwell wrote:
> Hi Darren,
> 
> Commits
> 
>   890f658c101d ("platform/x86: peaq-wmi: silence a static checker warning")
>   6d8d55626296 ("platform/x86: msi-wmi: remove unnecessary static in 
> msi_wmi_notify()")
>   cd0223c64c60 ("platform/x86: ibm_rtl: remove unnecessary static in 
> ibm_rtl_write()")
> 
> are missing Signed-off-by's from their commiter.

So each of these was originally committed by Andy... but appear as
committed by me. This must have occured as a rebase of our testing
branch I suppose. Nothing has been out of the ordinary this development
cycle, so I wonder that we haven't received such a report previously.
Hrm.

Is this a new check Stephen?

Is there any statement regarding maintainer teams that we must abide by
this?  e.g. any time a rebase in a testing branch is made, the
maintainer must also ensure a SOB is on each patch?

I just want to get a clear picture of what the failure was so we can
update our tooling so this doesn't repeat itself.

-- 
Darren Hart
VMware Open Source Technology Center

Re: linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Darren Hart]

On Thu, Aug 03, 2017 at 06:37:43AM +1000, Stephen Rothwell wrote:
> Hi Darren,
> 
> Commits
> 
>   890f658c101d ("platform/x86: peaq-wmi: silence a static checker warning")
>   6d8d55626296 ("platform/x86: msi-wmi: remove unnecessary static in 
> msi_wmi_notify()")
>   cd0223c64c60 ("platform/x86: ibm_rtl: remove unnecessary static in 
> ibm_rtl_write()")
> 
> are missing Signed-off-by's from their commiter.

So each of these was originally committed by Andy... but appear as
committed by me. This must have occured as a rebase of our testing
branch I suppose. Nothing has been out of the ordinary this development
cycle, so I wonder that we haven't received such a report previously.
Hrm.

Is this a new check Stephen?

Is there any statement regarding maintainer teams that we must abide by
this?  e.g. any time a rebase in a testing branch is made, the
maintainer must also ensure a SOB is on each patch?

I just want to get a clear picture of what the failure was so we can
update our tooling so this doesn't repeat itself.

-- 
Darren Hart
VMware Open Source Technology Center

linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi Darren,

Commits

  890f658c101d ("platform/x86: peaq-wmi: silence a static checker warning")
  6d8d55626296 ("platform/x86: msi-wmi: remove unnecessary static in 
msi_wmi_notify()")
  cd0223c64c60 ("platform/x86: ibm_rtl: remove unnecessary static in 
ibm_rtl_write()")

are missing Signed-off-by's from their commiter.

-- 
Cheers,
Stephen Rothwell

linux-next: Signed-off-by missing for commit in the drivers-x86 tree

[Stephen Rothwell]

Hi Darren,

Commits

  890f658c101d ("platform/x86: peaq-wmi: silence a static checker warning")
  6d8d55626296 ("platform/x86: msi-wmi: remove unnecessary static in 
msi_wmi_notify()")
  cd0223c64c60 ("platform/x86: ibm_rtl: remove unnecessary static in 
ibm_rtl_write()")

are missing Signed-off-by's from their commiter.

-- 
Cheers,
Stephen Rothwell