[U-Boot] [PATCH] patman: Only apply patches when we know the original HEAD
When patman applies the patches it checks out a new branch, uses 'git am' to apply the patches one by one, and then tries to go back to the old branch. If you try this when the branch is 'undefined', this doesn't work as patman cannot restore the correct branch after applying the patches. It seems that 'undefined' is created by git and is persistent after it is created, so that you can end up on quite an old branch.
Add a check for the 'undefined' branch to avoid this.
Reported-by: Masahiro Yamada yamada.m@jp.panasonic.com Signed-off-by: Simon Glass sjg@chromium.org ---
tools/patman/gitutil.py | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py index 3ea256d..7b75c83 100644 --- a/tools/patman/gitutil.py +++ b/tools/patman/gitutil.py @@ -232,6 +232,10 @@ def ApplyPatches(verbose, args, start_point): print stdout return False old_head = stdout.splitlines()[0] + if old_head == 'undefined': + str = "Invalid HEAD '%s'" % stdout.strip() + print col.Color(col.RED, str) + return False
# Checkout the required start point cmd = ['git', 'checkout', 'HEAD~%d' % start_point]
Hi Masahiro,
On 11 June 2014 23:27, Simon Glass sjg@chromium.org wrote:
When patman applies the patches it checks out a new branch, uses 'git am' to apply the patches one by one, and then tries to go back to the old branch. If you try this when the branch is 'undefined', this doesn't work as patman cannot restore the correct branch after applying the patches. It seems that 'undefined' is created by git and is persistent after it is created, so that you can end up on quite an old branch.
Add a check for the 'undefined' branch to avoid this.
Reported-by: Masahiro Yamada yamada.m@jp.panasonic.com Signed-off-by: Simon Glass sjg@chromium.org
Can you please check if this fixes the problem you reported? If so I would like to get it into this release.
Regards, Simon
Hi Simon,
On Sat, 21 Jun 2014 10:27:06 -0600 Simon Glass sjg@chromium.org wrote:
Hi Masahiro,
On 11 June 2014 23:27, Simon Glass sjg@chromium.org wrote:
When patman applies the patches it checks out a new branch, uses 'git am' to apply the patches one by one, and then tries to go back to the old branch. If you try this when the branch is 'undefined', this doesn't work as patman cannot restore the correct branch after applying the patches. It seems that 'undefined' is created by git and is persistent after it is created, so that you can end up on quite an old branch.
Add a check for the 'undefined' branch to avoid this.
Reported-by: Masahiro Yamada yamada.m@jp.panasonic.com Signed-off-by: Simon Glass sjg@chromium.org
Can you please check if this fixes the problem you reported? If so I would like to get it into this release.
I cannot understand why patman needs to apply the patches with 'git am'.
Why isn't patman like this ? [1] Generate patches with 'git format-patch' [2] Parse the patman-tags in the generated patches and edit them. [3] Send the patches with 'git send-email'
Best Regards Masahiro Yamada
Hi Masahiro,
On 22 June 2014 21:19, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Hi Simon,
On Sat, 21 Jun 2014 10:27:06 -0600 Simon Glass sjg@chromium.org wrote:
Hi Masahiro,
On 11 June 2014 23:27, Simon Glass sjg@chromium.org wrote:
When patman applies the patches it checks out a new branch, uses 'git am' to apply the patches one by one, and then tries to go back to the old branch. If you try this when the branch is 'undefined', this doesn't work as patman cannot restore the correct branch after applying the patches. It seems that 'undefined' is created by git and is persistent after it is created, so that you can end up on quite an old branch.
Add a check for the 'undefined' branch to avoid this.
Reported-by: Masahiro Yamada yamada.m@jp.panasonic.com Signed-off-by: Simon Glass sjg@chromium.org
Can you please check if this fixes the problem you reported? If so I would like to get it into this release.
I cannot understand why patman needs to apply the patches with 'git am'.
Why isn't patman like this ? [1] Generate patches with 'git format-patch' [2] Parse the patman-tags in the generated patches and edit them.
2a. Apply the patches to make sure there are no whitespace errors.
[3] Send the patches with 'git send-email'
The whitespace problems are not common but they do happen sometimes - or at least I have seen it at times. Unless perhaps checkpatch has got smarter?
Best Regards Masahiro Yamada
Regards, Simon
Hi Simon,
On Sun, 22 Jun 2014 21:39:26 -0600 Simon Glass sjg@chromium.org wrote:
Hi Masahiro,
On 22 June 2014 21:19, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Hi Simon,
On Sat, 21 Jun 2014 10:27:06 -0600 Simon Glass sjg@chromium.org wrote:
Hi Masahiro,
On 11 June 2014 23:27, Simon Glass sjg@chromium.org wrote:
When patman applies the patches it checks out a new branch, uses 'git am' to apply the patches one by one, and then tries to go back to the old branch. If you try this when the branch is 'undefined', this doesn't work as patman cannot restore the correct branch after applying the patches. It seems that 'undefined' is created by git and is persistent after it is created, so that you can end up on quite an old branch.
Add a check for the 'undefined' branch to avoid this.
Reported-by: Masahiro Yamada yamada.m@jp.panasonic.com Signed-off-by: Simon Glass sjg@chromium.org
Can you please check if this fixes the problem you reported? If so I would like to get it into this release.
I cannot understand why patman needs to apply the patches with 'git am'.
Why isn't patman like this ? [1] Generate patches with 'git format-patch' [2] Parse the patman-tags in the generated patches and edit them.
2a. Apply the patches to make sure there are no whitespace errors.
[3] Send the patches with 'git send-email'
The whitespace problems are not common but they do happen sometimes - or at least I have seen it at times. Unless perhaps checkpatch has got smarter?
I still don't understand. Anyway, Patman invokes checkpatch.pl, right? Isn't whitespace error checking of checkpatch nice?
Best Regards Masahiro Yamada
Hi Masahiro,
On Jun 22, 2014 9:51 PM, "Masahiro Yamada" yamada.m@jp.panasonic.com wrote:
Hi Simon,
On Sun, 22 Jun 2014 21:39:26 -0600 Simon Glass sjg@chromium.org wrote:
Hi Masahiro,
On 22 June 2014 21:19, Masahiro Yamada yamada.m@jp.panasonic.com
wrote:
Hi Simon,
On Sat, 21 Jun 2014 10:27:06 -0600 Simon Glass sjg@chromium.org wrote:
Hi Masahiro,
On 11 June 2014 23:27, Simon Glass sjg@chromium.org wrote:
When patman applies the patches it checks out a new branch, uses
'git am'
to apply the patches one by one, and then tries to go back to the
old
branch. If you try this when the branch is 'undefined', this
doesn't work
as patman cannot restore the correct branch after applying the
patches.
It seems that 'undefined' is created by git and is persistent
after it is
created, so that you can end up on quite an old branch.
Add a check for the 'undefined' branch to avoid this.
Reported-by: Masahiro Yamada yamada.m@jp.panasonic.com Signed-off-by: Simon Glass sjg@chromium.org
Can you please check if this fixes the problem you reported? If so I would like to get it into this release.
I cannot understand why patman needs to apply the patches with 'git
am'.
Why isn't patman like this ? [1] Generate patches with 'git format-patch' [2] Parse the patman-tags in the generated patches and edit them.
2a. Apply the patches to make sure there are no whitespace errors.
[3] Send the patches with 'git send-email'
The whitespace problems are not common but they do happen sometimes - or at least I have seen it at times. Unless perhaps checkpatch has got smarter?
I still don't understand. Anyway, Patman invokes checkpatch.pl, right? Isn't whitespace error checking of checkpatch nice?
Not when I wrote patman, no. Maybe it has changed? I could perhaps invert the meaning of the -a flag :-)
Regards, Simon
Best Regards Masahiro Yamada
On Mon, Jun 23, 2014 at 12:39 AM, Simon Glass sjg@chromium.org wrote:
The whitespace problems are not common but they do happen sometimes - or at least I have seen it at times. Unless perhaps checkpatch has got smarter?
Couldn't ./scripts/cleanpatch be used to fix the whitespace issues?
Hi,
On Jun 22, 2014 10:23 PM, "Fabio Estevam" festevam@gmail.com wrote:
On Mon, Jun 23, 2014 at 12:39 AM, Simon Glass sjg@chromium.org wrote:
The whitespace problems are not common but they do happen sometimes - or at least I have seen it at times. Unless perhaps checkpatch has got smarter?
Couldn't ./scripts/cleanpatch be used to fix the whitespace issues?
Maybe. If you know how to test this please give it a try.
Regards, Simon
Hi Simon,
On Sun, 22 Jun 2014 22:54:43 -0600 Simon Glass sjg@chromium.org wrote:
Hi,
On Jun 22, 2014 10:23 PM, "Fabio Estevam" festevam@gmail.com wrote:
On Mon, Jun 23, 2014 at 12:39 AM, Simon Glass sjg@chromium.org wrote:
The whitespace problems are not common but they do happen sometimes - or at least I have seen it at times. Unless perhaps checkpatch has got smarter?
Couldn't ./scripts/cleanpatch be used to fix the whitespace issues?
Maybe. If you know how to test this please give it a try.
Does Patman fix the whitespace issues?
I'm getting confused.
To make things clearer, please let me ask you some questions.
[1] What does "whitespace errors" mean here?
- Trailing whitespaces ? - Spaces before TAB indent ? - Or what else ?
[2] What do you expect Patman to do when it finds those errors? - Just display an error message ? or - Fix the issues automatically ?
[3] Which features were missing from scripts/checkpatch.pl when you wrote Patman first ?
Best Regards Masahiro Yamada
+Doug who may have some thoughts here.
HI Masahiro,
On 23 June 2014 00:28, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Hi Simon,
On Sun, 22 Jun 2014 22:54:43 -0600 Simon Glass sjg@chromium.org wrote:
Hi,
On Jun 22, 2014 10:23 PM, "Fabio Estevam" festevam@gmail.com wrote:
On Mon, Jun 23, 2014 at 12:39 AM, Simon Glass sjg@chromium.org
wrote:
The whitespace problems are not common but they do happen sometimes - or at least I have seen it at times. Unless perhaps checkpatch has
got
smarter?
Couldn't ./scripts/cleanpatch be used to fix the whitespace issues?
Maybe. If you know how to test this please give it a try.
Note, I do understand all the questions, but the problem is I am not
completely sure how this happens.
Does Patman fix the whitespace issues?
No, it just runs 'git am' which will then report them.
I'm getting confused.
To make things clearer, please let me ask you some questions.
[1] What does "whitespace errors" mean here?
- Trailing whitespaces ?
- Spaces before TAB indent ?
- Or what else ?
Both of those, plus a blank line at EOF as I understand it.
[2] What do you expect Patman to do when it finds those errors?
- Just display an error message ?
Yes (it displays an error from 'git am')
or
- Fix the issues automatically ?
No - apart from removing tags and adding notes patman avoids changing patches as a matter of policy.
[3] Which features were missing from scripts/checkpatch.pl when you wrote Patman first ?
It's really a belts-and-braces check that nothing will go wrong when the patches are applied. I have had it report errors to me on my own patches, although not recently. I'm not sure if it is still needed though.
Regards, Simon
On Mon, 23 Jun 2014 06:11:28 -0600 Simon Glass sjg@chromium.org wrote:
+Doug who may have some thoughts here.
HI Masahiro,
On 23 June 2014 00:28, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Hi Simon,
On Sun, 22 Jun 2014 22:54:43 -0600 Simon Glass sjg@chromium.org wrote:
Hi,
On Jun 22, 2014 10:23 PM, "Fabio Estevam" festevam@gmail.com wrote:
On Mon, Jun 23, 2014 at 12:39 AM, Simon Glass sjg@chromium.org
wrote:
The whitespace problems are not common but they do happen sometimes - or at least I have seen it at times. Unless perhaps checkpatch has
got
smarter?
Couldn't ./scripts/cleanpatch be used to fix the whitespace issues?
Maybe. If you know how to test this please give it a try.
Note, I do understand all the questions, but the problem is I am not
completely sure how this happens.
Does Patman fix the whitespace issues?
No, it just runs 'git am' which will then report them.
I'm getting confused.
To make things clearer, please let me ask you some questions.
[1] What does "whitespace errors" mean here?
- Trailing whitespaces ?
- Spaces before TAB indent ?
- Or what else ?
Both of those, plus a blank line at EOF as I understand it.
[2] What do you expect Patman to do when it finds those errors?
- Just display an error message ?
Yes (it displays an error from 'git am')
or
- Fix the issues automatically ?
No - apart from removing tags and adding notes patman avoids changing patches as a matter of policy.
[3] Which features were missing from scripts/checkpatch.pl when you wrote Patman first ?
It's really a belts-and-braces check that nothing will go wrong when the patches are applied. I have had it report errors to me on my own patches, although not recently. I'm not sure if it is still needed though.
OK. It seems better to check.
I added some patches which include whitespace errors and ran Patman.
It looks like nothing changes with/without -a option.
$ tools/patman/patman -a -c 3 1 warnings for 0002-arm-commit-2.patch: Line 21/0 has space before tab
1 warnings for 0003-arm-commit-3.patch: Found possible blank line(s) at end of file 'None'
Cleaned 3 patches 1 errors, 0 warnings, 0 checks for 0001-arm-commit-1.patch: error: common/cmd_mmc.c,22: trailing whitespace
1 errors, 2 warnings, 0 checks for 0002-arm-commit-2.patch: error: common/cmd_mmc.c,26: code indent should use tabs where possible warning: common/cmd_mmc.c,26: please, no space before tabs warning: common/cmd_mmc.c,26: please, no spaces at the start of a line
checkpatch.pl found 2 error(s), 2 warning(s), 0 checks(s)
$ tools/patman/patman -c 3 1 warnings for 0002-arm-commit-2.patch: Line 21/0 has space before tab
1 warnings for 0003-arm-commit-3.patch: Found possible blank line(s) at end of file 'None'
Cleaned 3 patches 1 errors, 0 warnings, 0 checks for 0001-arm-commit-1.patch: error: common/cmd_mmc.c,22: trailing whitespace
1 errors, 2 warnings, 0 checks for 0002-arm-commit-2.patch: error: common/cmd_mmc.c,26: code indent should use tabs where possible warning: common/cmd_mmc.c,26: please, no space before tabs warning: common/cmd_mmc.c,26: please, no spaces at the start of a line
checkpatch.pl found 2 error(s), 2 warning(s), 0 checks(s)
Best Regards Masahiro Yamada
Hi,
On Tue, Jun 24, 2014 at 3:24 AM, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
OK. It seems better to check.
I added some patches which include whitespace errors and ran Patman.
It looks like nothing changes with/without -a option.
$ tools/patman/patman -a -c 3 1 warnings for 0002-arm-commit-2.patch: Line 21/0 has space before tab
1 warnings for 0003-arm-commit-3.patch: Found possible blank line(s) at end of file 'None'
Cleaned 3 patches 1 errors, 0 warnings, 0 checks for 0001-arm-commit-1.patch: error: common/cmd_mmc.c,22: trailing whitespace
1 errors, 2 warnings, 0 checks for 0002-arm-commit-2.patch: error: common/cmd_mmc.c,26: code indent should use tabs where possible warning: common/cmd_mmc.c,26: please, no space before tabs warning: common/cmd_mmc.c,26: please, no spaces at the start of a line
checkpatch.pl found 2 error(s), 2 warning(s), 0 checks(s)
$ tools/patman/patman -c 3 1 warnings for 0002-arm-commit-2.patch: Line 21/0 has space before tab
1 warnings for 0003-arm-commit-3.patch: Found possible blank line(s) at end of file 'None'
Cleaned 3 patches 1 errors, 0 warnings, 0 checks for 0001-arm-commit-1.patch: error: common/cmd_mmc.c,22: trailing whitespace
1 errors, 2 warnings, 0 checks for 0002-arm-commit-2.patch: error: common/cmd_mmc.c,26: code indent should use tabs where possible warning: common/cmd_mmc.c,26: please, no space before tabs warning: common/cmd_mmc.c,26: please, no spaces at the start of a line
checkpatch.pl found 2 error(s), 2 warning(s), 0 checks(s)
It sure looks like checkpatch now catches things so it never gets to "git am". Making "-a" off by default seems reasonable to me.
-Doug
Hi,
On 24 June 2014 14:40, Doug Anderson dianders@chromium.org wrote:
Hi,
On Tue, Jun 24, 2014 at 3:24 AM, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
OK. It seems better to check.
I added some patches which include whitespace errors and ran Patman.
It looks like nothing changes with/without -a option.
$ tools/patman/patman -a -c 3 1 warnings for 0002-arm-commit-2.patch: Line 21/0 has space before tab
1 warnings for 0003-arm-commit-3.patch: Found possible blank line(s) at end of file 'None'
Cleaned 3 patches 1 errors, 0 warnings, 0 checks for 0001-arm-commit-1.patch: error: common/cmd_mmc.c,22: trailing whitespace
1 errors, 2 warnings, 0 checks for 0002-arm-commit-2.patch: error: common/cmd_mmc.c,26: code indent should use tabs where possible warning: common/cmd_mmc.c,26: please, no space before tabs warning: common/cmd_mmc.c,26: please, no spaces at the start of a line
checkpatch.pl found 2 error(s), 2 warning(s), 0 checks(s)
$ tools/patman/patman -c 3 1 warnings for 0002-arm-commit-2.patch: Line 21/0 has space before tab
1 warnings for 0003-arm-commit-3.patch: Found possible blank line(s) at end of file 'None'
Cleaned 3 patches 1 errors, 0 warnings, 0 checks for 0001-arm-commit-1.patch: error: common/cmd_mmc.c,22: trailing whitespace
1 errors, 2 warnings, 0 checks for 0002-arm-commit-2.patch: error: common/cmd_mmc.c,26: code indent should use tabs where possible warning: common/cmd_mmc.c,26: please, no space before tabs warning: common/cmd_mmc.c,26: please, no spaces at the start of a line
checkpatch.pl found 2 error(s), 2 warning(s), 0 checks(s)
It sure looks like checkpatch now catches things so it never gets to "git am". Making "-a" off by default seems reasonable to me.
Well, good.
Still, we need this patch to fix the bug I thihnk.
I'll do a new patch to invert the flag too.
Regards, Simon
Hi Masahiro,
On 24 June 2014 17:56, Simon Glass sjg@chromium.org wrote:
Hi,
On 24 June 2014 14:40, Doug Anderson dianders@chromium.org wrote:
It sure looks like checkpatch now catches things so it never gets to "git am". Making "-a" off by default seems reasonable to me.
Well, good.
Still, we need this patch to fix the bug I thihnk.
Aside from the discussion about apply patches, does this fix the bug you reported?
I'll do a new patch to invert the flag too.
Regards, Simon
Regards, Simon
Hi Simon,
On Wed, 25 Jun 2014 10:53:08 -0600 Simon Glass sjg@chromium.org wrote:
Hi Masahiro,
On 24 June 2014 17:56, Simon Glass sjg@chromium.org wrote:
Hi,
On 24 June 2014 14:40, Doug Anderson dianders@chromium.org wrote:
It sure looks like checkpatch now catches things so it never gets to "git am". Making "-a" off by default seems reasonable to me.
Well, good.
Still, we need this patch to fix the bug I thihnk.
Aside from the discussion about apply patches, does this fix the bug you reported?
Yes, that fixes the issue. Thanks! But, is '-a' option still necessary? Shoud we rip it off someday?
Best Regards Masahiro Yamada
Hi Masahiro,
On 25 June 2014 19:03, Masahiro Yamada yamada.m@jp.panasonic.com wrote:
Hi Simon,
On Wed, 25 Jun 2014 10:53:08 -0600 Simon Glass sjg@chromium.org wrote:
Hi Masahiro,
On 24 June 2014 17:56, Simon Glass sjg@chromium.org wrote:
Hi,
On 24 June 2014 14:40, Doug Anderson dianders@chromium.org wrote:
It sure looks like checkpatch now catches things so it never gets to "git am". Making "-a" off by default seems reasonable to me.
Well, good.
Still, we need this patch to fix the bug I thihnk.
Aside from the discussion about apply patches, does this fix the bug you reported?
Yes, that fixes the issue. Thanks! But, is '-a' option still necessary? Shoud we rip it off someday?
I'm in favour of this, perhaps for the next release.
Regards, Simon
Applied via x86 tree.
participants (4)
-
Doug Anderson -
Fabio Estevam -
Masahiro Yamada -
Simon Glass