uboot: fix custom patch dir legacy handling

PATCH New Danomi Manchego 4 participants 11 messages Dec 20, 2016

Share & tools

Danomi Manchego

author

2016-12-20 03:10 UTC #0 lore raw

PATCH 1/1 uboot: fix custom patch dir legacy handling

Commit 3e3fef39e71ed0426130b36aa00e4630ebb537ae added new and improved patch handling, with BR2_TARGET_UBOOT_PATCH. This was in addition to the existing BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR option, that only handled directories.

Later, commit 21b25d28fc7af3bb1b2c55e4a46e0d067ebc0081 moved the old BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR to legacy options. But on the way, bad things happened:

* The original option was a string, while the one added to Config.in.legacy is a bool. This results in a warning from defconfigs that actually define the old BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR (because a string is not a valid value for a bool), but it does not result in the legacy option being selected. Consequently, BR2_LEGACY is not selected either.

* The advice at the top of Config.in.legacy to add a hidden WRAP option to select BR2_LEGACY was not heeded.

* The advice at the top of Config.in.legacy to use the old string option as the default for the new string option was not heeded. In this case, the variable was not just renamed, as the old option supported directories only, while the new one supports files too. But since the old option is a subset of the new option, it can still be used to set a useful default.

So, this mod turns the legacy option back to a string, adds a hidden bool WRAP option to set BR2_LEGACY when the string is non emoty, and uses the legacy option to set the default for the new option.

Signed-off-by Danomi Manchego <[email protected]>

---
 Config.in.legacy     | 11 +++++++++--
 boot/uboot/Config.in |  1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

Config.in.legacy +9 -2

index b42eb41..b0ca2da 100644
@@ -551,12 +551,19 @@ config BR2_PACKAGE_QT5QUICK1
 	  from upstream starting from Qt 5.6.
 
 config BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR
-	bool "uboot custom patch dir removed"
-	select BR2_LEGACY
+	string "uboot custom patch dir has been removed"
 	help
 	  The uboot custom patch directory option has been removed. Use
 	  the improved BR2_TARGET_UBOOT_PATCH option instead.
 
+config BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR_WRAP
+	bool
+	default y if BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR != ""
+	select BR2_LEGACY
+
+# Note: BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR is still referenced from
+# boot/uboot/Config.in
+
 config BR2_PACKAGE_XDRIVER_XF86_INPUT_VOID
 	bool "xf86-input-void removed"
 	select BR2_LEGACY

boot/uboot/Config.in +1 -0

index 3b4bfac..2a9f14a 100644
@@ -95,6 +95,7 @@ config BR2_TARGET_UBOOT_VERSION
 
 config BR2_TARGET_UBOOT_PATCH
 	string "Custom U-Boot patches"
+	default BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR if BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR != ""  # legacy
 	help
 	  A space-separated list of patches to apply to U-Boot.
 	  Each patch can be described as an URL, a local file path,

Signature · 1 line

2.7.4

Peter Korsgaard

2016-12-20 09:06 UTC #1 lore raw

PATCH 1/1 uboot: fix custom patch dir legacy handling

Quoted 27 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Danomi" == Danomi Manchego <[email protected]> writes:
Commit 3e3fef39e71ed0426130b36aa00e4630ebb537ae added new and improved patch handling, with BR2_TARGET_UBOOT_PATCH. This was in addition to the existing BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR option, that only handled directories.
Later, commit 21b25d28fc7af3bb1b2c55e4a46e0d067ebc0081 moved the old BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR to legacy options. But on the way, bad things happened:
* The original option was a string, while the one added to Config.in.legacy is a bool. This results in a warning from defconfigs that actually define the old BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR (because a string is not a valid value for a bool), but it does not result in the legacy option being selected. Consequently, BR2_LEGACY is not selected either.
* The advice at the top of Config.in.legacy to add a hidden WRAP option to select BR2_LEGACY was not heeded.
* The advice at the top of Config.in.legacy to use the old string option as the default for the new string option was not heeded. In this case, the variable was not just renamed, as the old option supported directories only, while the new one supports files too. But since the old option is a subset of the new option, it can still be used to set a useful default.

Quoted 5 lines

So, this mod turns the legacy option back to a string, adds a hidden bool WRAP option to set BR2_LEGACY when the string is non emoty, and uses the legacy option to set the default for the new option.
Signed-off-by Danomi Manchego <[email protected]>

Nice detailed description! Committed, thanks.

Signature · 1 line

Bye, Peter Korsgaard

Arnout Vandecappelle

2016-12-20 20:53 UTC #2 lore raw

PATCH 1/1 uboot: fix custom patch dir legacy handling

Quoted 27 lines · Peter Korsgaard

Quoted 26 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Danomi" == Danomi Manchego <[email protected]> writes:
Commit 3e3fef39e71ed0426130b36aa00e4630ebb537ae added new and improved patch handling, with BR2_TARGET_UBOOT_PATCH. This was in addition to the existing BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR option, that only handled directories.
Later, commit 21b25d28fc7af3bb1b2c55e4a46e0d067ebc0081 moved the old BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR to legacy options. But on the way, bad things happened:
* The original option was a string, while the one added to Config.in.legacy is a bool. This results in a warning from defconfigs that actually define the old BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR (because a string is not a valid value for a bool), but it does not result in the legacy option being selected. Consequently, BR2_LEGACY is not selected either.
* The advice at the top of Config.in.legacy to add a hidden WRAP option to select BR2_LEGACY was not heeded.
* The advice at the top of Config.in.legacy to use the old string option as the default for the new string option was not heeded. In this case, the variable was not just renamed, as the old option supported directories only, while the new one supports files too. But since the old option is a subset of the new option, it can still be used to set a useful default.

Quoted 7 lines

Quoted 5 lines
So, this mod turns the legacy option back to a string, adds a hidden bool WRAP option to set BR2_LEGACY when the string is non emoty, and uses the legacy option to set the default for the new option.
Signed-off-by Danomi Manchego <[email protected]>
Nice detailed description! Committed, thanks.

This is 2016.11.x material.

Regards,
Arnout

Signature · 6 lines

Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

Peter Korsgaard

2016-12-20 21:04 UTC #3 lore raw

PATCH 1/1 uboot: fix custom patch dir legacy handling

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Arnout" == Arnout Vandecappelle <[email protected]> writes:

Hi,

Quoted 3 lines

Quoted 1 line
Nice detailed description! Committed, thanks.
This is 2016.11.x material.

It has been broken since 2016.05-rc1, but OK - We can indeed add it for 2016.11.x

Signature · 1 line

Bye, Peter Korsgaard

Danomi Manchego

author

2016-12-20 21:22 UTC #4 lore raw

PATCH 1/1 uboot: fix custom patch dir legacy handling

Quoted 10 lines · Peter Korsgaard

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Arnout" == Arnout Vandecappelle <[email protected]> writes:
Hi,
Quoted 3 lines
Quoted 1 line
Nice detailed description! Committed, thanks.
This is 2016.11.x material.
It has been broken since 2016.05-rc1, but OK - We can indeed add it for 2016.11.x

I noticed it now because we are upgrading from 2014.05 to 2016.11. We use both the uboot custom patch dir feature (replaced) and the linux custom path feature (dropped, frowny face), so I'm hitting those right now.

Danomi -

Peter Korsgaard

2016-12-20 21:45 UTC #5 lore raw

PATCH 1/1 uboot: fix custom patch dir legacy handling

Quoted 11 lines

Quoted 6 lines
Quoted 3 lines
Quoted 1 line
Quoted 1 line
"Danomi" == Danomi Manchego <[email protected]> writes:
This is 2016.11.x material.
It has been broken since 2016.05-rc1, but OK - We can indeed add it for 2016.11.x
I noticed it now because we are upgrading from 2014.05 to 2016.11. We use both the uboot custom patch dir feature (replaced) and the linux custom path feature (dropped, frowny face), so I'm hitting those right now.

Ok. The replacement for both is simply BR2_GLOBAL_PATCH_DIR (with linux patches under linux/ and u-boot patches under uboot/).

I've applied the fix to the the 2016.11.x branch and will include it in the upcoming 2016.11.1 release.

Signature · 1 line

Bye, Peter Korsgaard

Thomas Petazzoni

2016-12-20 22:00 UTC #6 lore raw

PATCH 1/1 uboot: fix custom patch dir legacy handling

Hello,

Quoted 7 lines · Peter Korsgaard

Quoted 4 lines
I noticed it now because we are upgrading from 2014.05 to 2016.11. We use both the uboot custom patch dir feature (replaced) and the linux custom path feature (dropped, frowny face), so I'm hitting those right now.
Ok. The replacement for both is simply BR2_GLOBAL_PATCH_DIR (with linux patches under linux/ and u-boot patches under uboot/).

Not completely true: the "linux custom path" feature is not replaced by BR2_GLOBAL_PATCH_DIR. If one wants to get the same functionality, i.e telling Buildroot to use the Linux kernel source code from an available local directory, then the only solution is to use LINUX_OVERRIDE_SRCDIR.

Best regards,

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Peter Korsgaard

2016-12-20 22:22 UTC #7 lore raw

PATCH 1/1 uboot: fix custom patch dir legacy handling

Quoted 10 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Thomas" == Thomas Petazzoni <[email protected]> writes:
Hello,
Quoted 5 lines · Peter Korsgaard
Quoted 4 lines
I noticed it now because we are upgrading from 2014.05 to 2016.11. We use both the uboot custom patch dir feature (replaced) and the linux custom path feature (dropped, frowny face), so I'm hitting those right now.

Quoted 7 lines

Quoted 2 lines
Ok. The replacement for both is simply BR2_GLOBAL_PATCH_DIR (with linux patches under linux/ and u-boot patches under uboot/).
Not completely true: the "linux custom path" feature is not replaced by BR2_GLOBAL_PATCH_DIR. If one wants to get the same functionality, i.e telling Buildroot to use the Linux kernel source code from an available local directory, then the only solution is to use LINUX_OVERRIDE_SRCDIR.

Sorry, I read it as 'custom patch' feature, not 'custom path'.

Signature · 1 line

Bye, Peter Korsgaard

Danomi Manchego

author

2016-12-20 22:55 UTC #8 lore raw

PATCH 1/1 uboot: fix custom patch dir legacy handling

In reply to Thomas Petazzoni

Quoted 9 lines · Thomas Petazzoni

Hello,
Quoted 5 lines · Peter Korsgaard
Quoted 4 lines
I noticed it now because we are upgrading from 2014.05 to 2016.11. We use both the uboot custom patch dir feature (replaced) and the linux custom path feature (dropped, frowny face), so I'm hitting those right now.

Quoted 7 lines

Quoted 2 lines
Ok. The replacement for both is simply BR2_GLOBAL_PATCH_DIR (with linux patches under linux/ and u-boot patches under uboot/).
Not completely true: the "linux custom path" feature is not replaced by BR2_GLOBAL_PATCH_DIR. If one wants to get the same functionality, i.e telling Buildroot to use the Linux kernel source code from an available local directory, then the only solution is to use LINUX_OVERRIDE_SRCDIR.

Understood. It's just that using a custom path for boot loaders and the kernel is the norm for us, not an exception. In fact, I've never been on a project where we were able to use a main line kernel or uboot. Sometimes, we'll go through the exercise of making patches against an initial state of a vendor's git repo, but many of out project leaders prefer to just source control everything locally. Under those circumstances, it's nicer to have the local path in the defconfig than in a secondary local.mk or in the .mk file itself.

Danomi -

Thomas Petazzoni

2016-12-20 22:56 UTC #9 lore raw

PATCH 1/1 uboot: fix custom patch dir legacy handling

Hello,

Quoted 8 lines · Danomi Manchego

Understood. It's just that using a custom path for boot loaders and the kernel is the norm for us, not an exception. In fact, I've never been on a project where we were able to use a main line kernel or uboot. Sometimes, we'll go through the exercise of making patches against an initial state of a vendor's git repo, but many of out project leaders prefer to just source control everything locally. Under those circumstances, it's nicer to have the local path in the defconfig than in a secondary local.mk or in the .mk file itself.

If your U-Boot / Linux kernel source code is under version control, why don't you ask Buildroot to pull it from your Git/SVN/Mercurial/whatever repository ?

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Danomi Manchego

author

2016-12-20 23:12 UTC #10 lore raw

PATCH 1/1 uboot: fix custom patch dir legacy handling

Quoted 13 lines · Thomas Petazzoni

Hello,
Quoted 8 lines · Danomi Manchego
Understood. It's just that using a custom path for boot loaders and the kernel is the norm for us, not an exception. In fact, I've never been on a project where we were able to use a main line kernel or uboot. Sometimes, we'll go through the exercise of making patches against an initial state of a vendor's git repo, but many of out project leaders prefer to just source control everything locally. Under those circumstances, it's nicer to have the local path in the defconfig than in a secondary local.mk or in the .mk file itself.

Quoted 3 lines

If your U-Boot / Linux kernel source code is under version control, why don't you ask Buildroot to pull it from your Git/SVN/Mercurial/whatever repository ?

Perforce integration with Makefiles can be - tricky. Our current setup is to use buildroot as the core for our build system, with all the proprietary apps and customized kernel and bootloader(s) at a level parallel to buildroot, but built using override source directories. So the whole thing (buildroot, proprietary apps, kernel, linux) are all checked at at once, not on a package by package basis. We keep a local cache of the package tarballs, so nothing is downloaded or pulled on-the-fly, buildroot-wise.