Skip to contentSkip to conversation

u-boot: Allow to specify a list of patches

PATCH New Ezequiel Garcia 5 participants 12 messages Jul 15, 2014 – Jul 21, 2014

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 u-boot: Allow to specify a list of patches

It's often desirable to specify a list of patches, in addition to the current support to pass a directory full of uboot-*.patch files. This matches the option currently available for specifying Linux patches, except this commit doesn't add the URL option. Relying on URLs is fragile and makes a build hard to reproduce.

U-Boot is as special as Linux, in the sense that it needs to be customized more often than not, so having a flexible mechanism is useful.

---
 Config.in.legacy     | 14 ++++++++++++++
 boot/uboot/Config.in | 11 ++++++-----
 boot/uboot/uboot.mk  | 12 ++++++++----
 3 files changed, 28 insertions(+), 9 deletions(-)
Config.in.legacy +14 -0
index 23962a2..7fd8b9c 100644
@@ -248,6 +248,20 @@ config BR2_PACKAGE_LIBV4L_V4L2_DBG
 	  This libv4l option has been deprecated and replaced by a single
 	  option to build all the libv4l utilities.
 
+config BR2_TARGET_UBOOT_PATCH
+	default BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR if BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR != ""
+
+config BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR
+	string "u-boot: the patches list option has been renamed"
+	help
+	  The option BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR has been renamed to
+	  BR2_TARGET_UBOOT_PATCH.
+
+config BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR_WRAP
+	bool
+	default y if BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR != ""
+	select BR2_LEGACY
+
 ###############################################################################
 comment "Legacy options removed in 2014.05"
 
boot/uboot/Config.in +6 -5
index 0f98d7f..ec7095a 100644
@@ -70,12 +70,13 @@ config BR2_TARGET_UBOOT_VERSION
 	default BR2_TARGET_UBOOT_CUSTOM_REPO_VERSION \
 		if BR2_TARGET_UBOOT_CUSTOM_GIT || BR2_TARGET_UBOOT_CUSTOM_HG
 
-config BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR
-	string "custom patch dir"
+config BR2_TARGET_UBOOT_PATCH
+	string "Custom U-Boot patches"
 	help
-	  If your board requires custom patches, add the path to the
-	  directory containing the patches here. The patches must be
-	  named uboot-<something>.patch.
+	  A space-separated list of patches to be applied.
+	  Each patch can be described as a local file path, or a directory.
+	  In the case of a directory, all files matching uboot-*.patch in
+	  the directory will be applied.
 
 	  Most users may leave this empty
 
boot/uboot/uboot.mk +8 -4
index 532ac8b..753bccb 100644
@@ -85,14 +85,18 @@ endef
 
 UBOOT_POST_EXTRACT_HOOKS += UBOOT_COPY_OLD_LICENSE_FILE
 
-ifneq ($(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR)),)
+UBOOT_PATCHES = $(call qstrip,$(BR2_TARGET_UBOOT_PATCH))
 define UBOOT_APPLY_CUSTOM_PATCHES
-	support/scripts/apply-patches.sh $(@D) $(BR2_TARGET_UBOOT_CUSTOM_PATCH_DIR) \
-		uboot-\*.patch
+	for p in $(UBOOT_PATCHES) ; do \
+		if test -d $$p ; then \
+			support/scripts/apply-patches.sh $(@D) $$p uboot-\*.patch ; \
+		else \
+			support/scripts/apply-patches.sh $(@D) `dirname $$p` `basename $$p` ; \
+		fi \
+	done
 endef
 
 UBOOT_POST_PATCH_HOOKS += UBOOT_APPLY_CUSTOM_PATCHES
-endif
 
 define UBOOT_CONFIGURE_CMDS
 	$(TARGET_CONFIGURE_OPTS) $(UBOOT_CONFIGURE_OPTS) 	\
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 u-boot: Allow to specify a list of patches

Dear Ezequiel Garcia,

Quoted 9 lines · Ezequiel Garcia

It's often desirable to specify a list of patches, in addition to the current support to pass a directory full of uboot-*.patch files. This matches the option currently available for specifying Linux patches, except this commit doesn't add the URL option. Relying on URLs is fragile and makes a build hard to reproduce.

U-Boot is as special as Linux, in the sense that it needs to be customized more often than not, so having a flexible mechanism is useful.

Quoted 6 lines
---
 Config.in.legacy     | 14 ++++++++++++++
 boot/uboot/Config.in | 11 ++++++-----
 boot/uboot/uboot.mk  | 12 ++++++++----
 3 files changed, 28 insertions(+), 9 deletions(-)

We now have BR2_GLOBAL_PATCH_DIR, so I believe that what we would recommend to use from now on. We have even been talking about removing the Linux-specific and U-Boot specific patching config options.

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#2 lore raw

PATCH 1/1 u-boot: Allow to specify a list of patches

Quoted 19 lines · Thomas Petazzoni

Dear Ezequiel Garcia,

Quoted 9 lines · Ezequiel Garcia

It's often desirable to specify a list of patches, in addition to the current support to pass a directory full of uboot-*.patch files. This matches the option currently available for specifying Linux patches, except this commit doesn't add the URL option. Relying on URLs is fragile and makes a build hard to reproduce.

U-Boot is as special as Linux, in the sense that it needs to be customized more often than not, so having a flexible mechanism is useful.

Quoted 6 lines
---
 Config.in.legacy     | 14 ++++++++++++++
 boot/uboot/Config.in | 11 ++++++-----
 boot/uboot/uboot.mk  | 12 ++++++++----
 3 files changed, 28 insertions(+), 9 deletions(-)
Quoted 4 lines

We now have BR2_GLOBAL_PATCH_DIR, so I believe that what we would recommend to use from now on. We have even been talking about removing the Linux-specific and U-Boot specific patching config options.

Yeah, Yann already said that on IRC when I asked him about this patch. You guys are really in sync! :)

The thing is that Linux and U-Boot are not the regular package, since almost every BR user using it for a custom product will need to customize the kernel and the bootloader to some extent.

At least from my perspective, it's nice to have the patches (instead of pointing to some git branch) and be forced to see what modifications I'm using.

And to produce the patches, I use git-format-patch (aren't we all?), which doesn't allow to specify a prefix for the patches. This has been discussed and rejected, so don't think the feature will ever be added.

Sure, I can use a script, but these options seemed natural enough and it allows for a simple "git-format-patch" run and dump the files to a directory.

Anyway... this is just my use case. I understand your desire to remove strange options and reduce the churn.

Signature · 2 lines
Ezequiel Garc?a, VanguardiaSur
www.vanguardiasur.com.ar
#3 lore raw

PATCH 1/1 u-boot: Allow to specify a list of patches

Ezequiel, Thomas, All,

On 2014-07-15 16:49 -0300, Ezequiel Garc?a spake thusly:

Quoted 33 lines
Quoted 19 lines · Thomas Petazzoni

Dear Ezequiel Garcia,

Quoted 9 lines · Ezequiel Garcia

It's often desirable to specify a list of patches, in addition to the current support to pass a directory full of uboot-*.patch files. This matches the option currently available for specifying Linux patches, except this commit doesn't add the URL option. Relying on URLs is fragile and makes a build hard to reproduce.

U-Boot is as special as Linux, in the sense that it needs to be customized more often than not, so having a flexible mechanism is useful.

Quoted 6 lines
---
 Config.in.legacy     | 14 ++++++++++++++
 boot/uboot/Config.in | 11 ++++++-----
 boot/uboot/uboot.mk  | 12 ++++++++----
 3 files changed, 28 insertions(+), 9 deletions(-)
Quoted 4 lines

We now have BR2_GLOBAL_PATCH_DIR, so I believe that what we would recommend to use from now on. We have even been talking about removing the Linux-specific and U-Boot specific patching config options.

Yeah, Yann already said that on IRC when I asked him about this patch. You guys are really in sync! :)

The thing is that Linux and U-Boot are not the regular package, since almost every BR user using it for a custom product will need to customize the kernel and the bootloader to some extent.

Quoted 7 lines

At least from my perspective, it's nice to have the patches (instead of pointing to some git branch) and be forced to see what modifications I'm using.

And to produce the patches, I use git-format-patch (aren't we all?), which doesn't allow to specify a prefix for the patches. This has been discussed and rejected, so don't think the feature will ever be added.

Now I remember that we talked about this on IRC the other day.

The more I think about it, the more I find our policy to require PKG-prefixed patches to be really cumbersome, since the patches already are in a subdir named PKG/

Of course, we're enforcing this naming scheme in BR2_GLOBAL_PATCH_DIR to be in sync with what we do for our bundled patches.

But still, if patches were just named NNNN-title.patch, that would be as efficient at sorting the patches. The PKG- prefix is not really required, and indeed can cause some troubles with some use-cases, such as yours.

Thomas, was there a specific reason we wanted the patches to be PKG-prefixed? If not, would it make sense to just accept patches without a PKG-prefix?

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#4 lore raw

PATCH 1/1 u-boot: Allow to specify a list of patches

Dear Yann E. MORIN,

Quoted 3 lines · Yann E. MORIN

The more I think about it, the more I find our policy to require PKG-prefixed patches to be really cumbersome, since the patches already are in a subdir named PKG/

Actually, me too. It prevents the output of "git format-patch" from being directly used as patches.

Quoted 11 lines

Of course, we're enforcing this naming scheme in BR2_GLOBAL_PATCH_DIR to be in sync with what we do for our bundled patches.

But still, if patches were just named NNNN-title.patch, that would be as efficient at sorting the patches. The PKG- prefix is not really required, and indeed can cause some troubles with some use-cases, such as yours.

Thomas, was there a specific reason we wanted the patches to be PKG-prefixed? If not, would it make sense to just accept patches without a PKG-prefix?

I honestly don't remember what was the reasoning behind that. Arnout maybe?

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#5 lore raw

PATCH 1/1 u-boot: Allow to specify a list of patches

Thomas Petazzoni <[email protected]> schreef:

Quoted 19 lines

Dear Yann E. MORIN,

Quoted 3 lines · Yann E. MORIN

The more I think about it, the more I find our policy to require PKG-prefixed patches to be really cumbersome, since the patches already are in a subdir named PKG/

Actually, me too. It prevents the output of "git format-patch" from being directly used as patches.

Quoted 8 lines

Of course, we're enforcing this naming scheme in BR2_GLOBAL_PATCH_DIR to be in sync with what we do for our bundled patches.

But still, if patches were just named NNNN-title.patch, that would be as efficient at sorting the patches. The PKG- prefix is not really required, and indeed can cause some troubles with some use-cases, such as yours.

Quoted 6 lines
Quoted 3 lines

Thomas, was there a specific reason we wanted the patches to be PKG-prefixed? If not, would it make sense to just accept patches without a PKG-prefix?

I honestly don't remember what was the reasoning behind that. Arnout maybe?

Not sure either...

#6 lore raw

PATCH 1/1 u-boot: Allow to specify a list of patches

In reply to Yann E. MORIN
Quoted 7 lines · Yann E. MORIN

The more I think about it, the more I find our policy to require PKG-prefixed patches to be really cumbersome, since the patches already are in a subdir named PKG/

Of course, we're enforcing this naming scheme in BR2_GLOBAL_PATCH_DIR to be in sync with what we do for our bundled patches.

Quoted 8 lines

But still, if patches were just named NNNN-title.patch, that would be as efficient at sorting the patches. The PKG- prefix is not really required, and indeed can cause some troubles with some use-cases, such as yours.

Thomas, was there a specific reason we wanted the patches to be PKG-prefixed? If not, would it make sense to just accept patches without a PKG-prefix?

Er, we don't... We require this specific naming scheme for contributed
packages, but the code itself just takes *.patch.

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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#7 lore raw

PATCH 1/1 u-boot: Allow to specify a list of patches

Arnout Vandecappelle <[email protected]> schreef:

Quoted 13 lines · Yann E. MORIN
Quoted 12 lines

The more I think about it, the more I find our policy to require PKG-prefixed patches to be really cumbersome, since the patches already are in a subdir named PKG/

Of course, we're enforcing this naming scheme in BR2_GLOBAL_PATCH_DIR to be in sync with what we do for our bundled patches.

But still, if patches were just named NNNN-title.patch, that would be as efficient at sorting the patches. The PKG- prefix is not really required, and indeed can cause some troubles with some use-cases, such as yours.

Quoted 6 lines
Quoted 3 lines

Thomas, was there a specific reason we wanted the patches to be PKG-prefixed? If not, would it make sense to just accept patches without a PKG-prefix?

Er, we don't... We require this specific naming scheme for contributed packages, but the code itself just takes *.patch.

Yes sure, it's a convention only, but the question is: why did we include the package name in the convention?

#8 lore raw

PATCH 1/1 u-boot: Allow to specify a list of patches

Quoted 14 lines · Thomas De Schampheleire

Arnout Vandecappelle <[email protected]> schreef:

Quoted 13 lines · Yann E. MORIN
Quoted 12 lines

The more I think about it, the more I find our policy to require PKG-prefixed patches to be really cumbersome, since the patches already are in a subdir named PKG/

Of course, we're enforcing this naming scheme in BR2_GLOBAL_PATCH_DIR to be in sync with what we do for our bundled patches.

But still, if patches were just named NNNN-title.patch, that would be as efficient at sorting the patches. The PKG- prefix is not really required, and indeed can cause some troubles with some use-cases, such as yours.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

Thomas, was there a specific reason we wanted the patches to be PKG-prefixed? If not, would it make sense to just accept patches without a PKG-prefix?

Er, we don't... We require this specific naming scheme for contributed packages, but the code itself just takes *.patch.

Yes sure, it's a convention only, but the question is: why did we include the package name in the convention?

I think it's purely historical. And I think it never was required for patches
in a <pkgname> subdir.

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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#9 lore raw

PATCH 1/1 u-boot: Allow to specify a list of patches

Arnout, All,

On 2014-07-17 09:48 +0200, Arnout Vandecappelle spake thusly:

Quoted 19 lines · Thomas De Schampheleire
Quoted 18 lines

Arnout Vandecappelle <[email protected]> schreef:

Quoted 17 lines · Yann E. MORIN
Quoted 15 lines

The more I think about it, the more I find our policy to require PKG-prefixed patches to be really cumbersome, since the patches already are in a subdir named PKG/

Of course, we're enforcing this naming scheme in BR2_GLOBAL_PATCH_DIR to be in sync with what we do for our bundled patches.

But still, if patches were just named NNNN-title.patch, that would be as efficient at sorting the patches. The PKG- prefix is not really required, and indeed can cause some troubles with some use-cases, such as yours.

Thomas, was there a specific reason we wanted the patches to be PKG-prefixed? If not, would it make sense to just accept patches without a PKG-prefix?

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Er, we don't... We require this specific naming scheme for contributed packages, but the code itself just takes *.patch.

Yes sure, it's a convention only, but the question is: why did we include the package name in the convention?

I think it's purely historical. And I think it never was required for patches
in a <pkgname> subdir.

So we could change the manual to not require patches to be PKG-prefixed?

As long as they are number-prefixed, that's all we need, right?

So, Ezequiel's patch is really no longer needed, and his use-case to use git-formatted patches is already covered, right?

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#10 lore raw

PATCH 1/1 u-boot: Allow to specify a list of patches

Quoted 30 lines · Yann E. MORIN

Arnout, All,

On 2014-07-17 09:48 +0200, Arnout Vandecappelle spake thusly:

Quoted 19 lines · Thomas De Schampheleire
Quoted 18 lines

Arnout Vandecappelle <[email protected]> schreef:

Quoted 17 lines · Yann E. MORIN
Quoted 15 lines

The more I think about it, the more I find our policy to require PKG-prefixed patches to be really cumbersome, since the patches already are in a subdir named PKG/

Of course, we're enforcing this naming scheme in BR2_GLOBAL_PATCH_DIR to be in sync with what we do for our bundled patches.

But still, if patches were just named NNNN-title.patch, that would be as efficient at sorting the patches. The PKG- prefix is not really required, and indeed can cause some troubles with some use-cases, such as yours.

Thomas, was there a specific reason we wanted the patches to be PKG-prefixed? If not, would it make sense to just accept patches without a PKG-prefix?

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Er, we don't... We require this specific naming scheme for contributed packages, but the code itself just takes *.patch.

Yes sure, it's a convention only, but the question is: why did we include the package name in the convention?

I think it's purely historical. And I think it never was required for patches
in a <pkgname> subdir.

Quoted 6 lines

So we could change the manual to not require patches to be PKG-prefixed?

As long as they are number-prefixed, that's all we need, right?

So, Ezequiel's patch is really no longer needed, and his use-case to use git-formatted patches is already covered, right?

Yep.

Untested, of course :-)

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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#11 lore raw

PATCH 1/1 u-boot: Allow to specify a list of patches

Hi everyone,

On 18 Jul 01:20 AM, Arnout Vandecappelle wrote:
[..]

Quoted 15 lines
Quoted 15 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines

Thomas, was there a specific reason we wanted the patches to be PKG-prefixed? If not, would it make sense to just accept patches without a PKG-prefix?

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Er, we don't... We require this specific naming scheme for contributed packages, but the code itself just takes *.patch.

Yes sure, it's a convention only, but the question is: why did we include the package name in the convention?

I think it's purely historical. And I think it never was required for patches
in a <pkgname> subdir.

So we could change the manual to not require patches to be PKG-prefixed?

Quoted 9 lines
Quoted 4 lines

As long as they are number-prefixed, that's all we need, right?

So, Ezequiel's patch is really no longer needed, and his use-case to use git-formatted patches is already covered, right?

Yep.

Untested, of course :-)

I did a quick test in here. It seems GLOBAL_PATCH_DIR works perfectly, and it's a lot cleaner than having N options, one for each supposedly special package, as it's keeps all the modification to vanilla in one place.

So I strongly support removing all the UBOOT_PATCH, LINUX_PATCH, and any other FOO_PATCH options out there.

Any brave hacker stepping up? :)

Signature · 2 lines
Ezequiel Garcia, VanguardiaSur
www.vanguardiasur.com.ar