[Buildroot] [PATCH v4 1/2] Support for multiple BR2_GLOBAL_PATCH_DIR

[Buildroot] [PATCH v4 1/2] Support for multiple BR2_GLOBAL_PATCH_DIR

[Ryan Barnett]

Adding support for specifying multiple directories in
BR2_GLOBAL_PATCH_DIR. This will allow for a layered approach for the
patching of a package.

Signed-off-by: Ryan Barnett <rjbarnet@rockwellcollins.com>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>

---
Changes v3 -> v4:
  - None

Changes v2 -> v3:
  - changed the generation of patch directories to use 'addsuffix'
    instead of a foreach loop. (suggested by Arnout)

Changes v1 -> v2:
  - change wording in Config.in help (suggested by Thomas D)
---
 Config.in              |   20 ++++++++++++--------
 package/pkg-generic.mk |    5 ++++-
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/Config.in b/Config.in
index 2b401cb..d55e57c 100644
--- a/Config.in
+++ b/Config.in
@@ -461,18 +461,22 @@ config BR2_PACKAGE_OVERRIDE_FILE
 	  Buildroot documentation for more details on this feature.
 
 config BR2_GLOBAL_PATCH_DIR
-	string "global patch directory"
+	string "global patch directories"
 	help
-	  You may specify a directory containing global package patches.
-	  For a specific version <packageversion> of a specific package
-	  <packagename>, patches are applied as follows.
+	  You may specify a space separated list of one or more directories
+	  containing global package patches. For a specific version
+	  <packageversion> of a specific package <packagename>, patches are
+	  applied as follows:
 
-	  First, the default Buildroot patch set for the package is applied.
+	  First, the default Buildroot patch set for the package is applied
+	  from the package's directory in Buildroot.
 
-	  If the directory $(BR2_GLOBAL_PATCH_DIR)/<packagename>/<packageversion>
-	  exists, then all *.patch files in the directory will be applied.
+	  Then for every directory - <global-patch-dir> - that exists in
+	  BR2_GLOBAL_PATCH_DIR, if the directory
+	  <global-patch-dir>/<packagename>/<packageversion>/ exists, then all
+	  *.patch files in this directory will be applied.
 
-	  Otherwise, if the directory $(BR2_GLOBAL_PATCH_DIR)/<packagename> exists,
+	  Otherwise, if the directory <global-patch-dir>/<packagename> exists,
 	  then all *.patch files in the directory will be applied.
 
 endmenu
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 45b808a..66034ba 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -134,8 +134,11 @@ endif
 # The RAWNAME variable is the lowercased package name, which allows to
 # find the package directory (typically package/<pkgname>) and the
 # prefix of the patches
+#
+# For BR2_GLOBAL_PATCH_DIR, only generate if it is defined
 $(BUILD_DIR)/%/.stamp_patched: NAMEVER = $(RAWNAME)-$($(PKG)_VERSION)
-$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS = $($(PKG)_DIR_PREFIX)/$(RAWNAME) $(call qstrip,$(BR2_GLOBAL_PATCH_DIR))/$(RAWNAME)
+$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS =  $($(PKG)_DIR_PREFIX)/$(RAWNAME)
+$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS += $(addsuffix /$(RAWNAME),$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)))
 $(BUILD_DIR)/%/.stamp_patched:
 	@$(call step_start,patch)
 	@$(call MESSAGE,"Patching")
-- 
1.7.9.5

[Buildroot] [PATCH v4 2/2] manual: update for multiple global patch dirs

[Ryan Barnett]

Updating the documentation to reflect that multiple directories can
now be specified for BR2_GLOBAL_PATCH_DIR. Along with giving an
example use case of how to use multiple global patch directories.

Signed-off-by: Ryan Barnett <rjbarnet@rockwellcollins.com>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>

---
Changes v3 -> v4:
  - Fixed minor spelling mistakes and wording (suggested by Arnout)
  - Reword section about order that patches are applied along with
    making it clearer about when to use BR2_GLOBAL_PATCH_DIR
    (suggested by Arnout)

Changes v2 -> v3:
  - None

Changes v1 -> v2:
  - Fixed minor spelling mistakes and wording (suggested by Thomas D)
---
 docs/manual/customize-packages.txt |   91 ++++++++++++++++++++++++++++++++----
 docs/manual/patch-policy.txt       |   20 +++++---
 2 files changed, 95 insertions(+), 16 deletions(-)

diff --git a/docs/manual/customize-packages.txt b/docs/manual/customize-packages.txt
index 1820c54..43ccecd 100644
--- a/docs/manual/customize-packages.txt
+++ b/docs/manual/customize-packages.txt
@@ -8,16 +8,89 @@ It is sometimes useful to apply 'extra' patches to packages - over and
 above those provided in Buildroot. This might be used to support custom
 features in a project, for example, or when working on a new architecture.
 
-The +BR2_GLOBAL_PATCH_DIR+ configuration file option can be
-used to specify a directory containing global package patches.
+The +BR2_GLOBAL_PATCH_DIR+ configuration option can be used to specify
+a space separated list of one or more directories containing package
+patches. By specifying multiple global patch directories, a user could
+implement a layered approach to patches. This could be useful when a
+user has multiple boards that share a common processor architecture.
+It is often the case that a subset of patches for a package need to be
+shared between the different boards a user has. However, each board
+may require specific patches for the package that build on top of the
+common subset of patches.
 
-For a specific version <packageversion> of a specific package <packagename>,
-patches are applied as follows.
+For a specific version +<packageversion>+ of a specific package
++<packagename>+, patches are applied from +BR2_GLOBAL_PATCH_DIR+ as
+follows:
 
-First, the default Buildroot patch set for the package is applied.
+. For every directory - +<global-patch-dir>+ - that exists in
+  +BR2_GLOBAL_PATCH_DIR+, a +<package-patch-dir>+ will be determined as
+  follows:
++
+* If the directory
+  +<global-patch-dir>/<packagename>/<packageversion>/+ exists.
++
+* Otherwise, if the directory +<global-patch-dir>/<packagename>+ exists.
 
-If the directory +$(BR2_GLOBAL_PATCH_DIR)/<packagename>/<packageversion>+
-exists, then all +*.patch+ files in the directory will be applied.
+. Patches will then be applied from a +<package-patch-dir>+ as
+  follows:
++
+* If a +series+ file exists in the package directory, then patches are
+  applied according to the +series+ file;
++
+* Otherwise, patch files matching +<packagename>-*.patch+
+  are applied in alphabetical order.
+  So, to ensure they are applied in the right order, it is highly
+  recommended to name the patch files like this:
+  +<packagename>-<number>-<description>.patch+, where +<number>+
+  refers to the 'apply order'.
 
-Otherwise, if the directory +$(BR2_GLOBAL_PATCH_DIR)/<packagename>+
-exists, then all +*.patch+ files in the directory will be applied.
+For information about how patches are applied for a package, see
+xref:patch-apply-order[]
+
+The +BR2_GLOBAL_PATCH_DIR+ option is the preferred method for
+specifying a custom patch directory for packages. It can be used to
+specify a patch directory for any package in buildroot. It should also
+be used in place of the custom patch directory options that are
+available for packages such as U-Boot and Barebox. By doing this, it
+will allow a user to manage their patches from one top-level
+directory.
+
+The exception to +BR2_GLOBAL_PATCH_DIR+ being the preferred method for
+specifying custom patches is +BR2_LINUX_KERNEL_PATCH+.
++BR2_LINUX_KERNEL_PATCH+ should be used to specify kernel patches that
+are available at an URL. *Note:* +BR2_LINUX_KERNEL_PATCHES+ are applied
+after patches available in +BR2_GLOBAL_PATCH_DIR+ as it is a patch
+post-hook step for the Linux package.
+
+An example directory structure for where a user has multiple
+directories specified for +BR2_GLOBAL_PATCH_DIR+ may look like this:
+
+-----
+board/
++-- common-fooarch
+|   +-- patches
+|       +-- linux
+|       |   +-- linux-patch1.patch
+|       |   +-- linux-patch2.patch
+|       +-- u-boot
+|       +-- foopkg
++-- fooarch-board
+    +-- patches
+        +-- linux
+        |   +-- linux-patch3.patch
+        +-- u-boot
+        +-- foopkg
+-----
+
+If the user has the +BR2_GLOBAL_PATCH_DIR+ configuration option set as
+follows:
+
+-----
+BR2_GLOBAL_PATCH_DIR="board/common-fooarch board/fooarch-board"
+-----
+
+Then the patches would applied as follows for the Linux kernel:
+
+. board/common-fooarch/patches/linux/linux-patch1.patch
+. board/common-fooarch/patches/linux/linux-patch2.patch
+. board/fooarch-board/patches/linux/linux-patch3.patch
diff --git a/docs/manual/patch-policy.txt b/docs/manual/patch-policy.txt
index d9bc8ca..9041081 100644
--- a/docs/manual/patch-policy.txt
+++ b/docs/manual/patch-policy.txt
@@ -50,10 +50,11 @@ Global patch directory
 ^^^^^^^^^^^^^^^^^^^^^^
 
 The +BR2_GLOBAL_PATCH_DIR+ configuration file option can be
-used to specify a directory containing global package patches. See
-xref:packages-custom[] for details.
-
+used to specify a space separated list of one or more directories
+containing global package patches. See xref:packages-custom[] for
+details.
 
+[[patch-apply-order]]
 How patches are applied
 ~~~~~~~~~~~~~~~~~~~~~~~
 
@@ -64,19 +65,24 @@ How patches are applied
 . If +<packagename>_PATCH+ is defined, then patches from these
   tarballs are applied;
 
-. If there are some +*.patch+ files in the package directory or in the
-  a package subdirectory named +<packageversion>+, then:
+. If there are some +*.patch+ files in the package's Buildroot
+  directory or in a package subdirectory named +<packageversion>+,
+  then:
 +
 * If a +series+ file exists in the package directory, then patches are
   applied according to the +series+ file;
 +
 * Otherwise, patch files matching +<packagename>-*.patch+
   are applied in alphabetical order.
-  So, to ensure they are applied in the right order, it is hightly
-  recommended to named the patch files like this:
+  So, to ensure they are applied in the right order, it is highly
+  recommended to name the patch files like this:
   +<packagename>-<number>-<description>.patch+, where +<number>+
   refers to the 'apply order'.
 
+. If +BR2_GLOABL_PATCH_DIR+ is defined, the directories will be
+  enumerated in the order they are specified. The patches are applied
+  as described in the previous step.
+
 . Run the +<packagename>_POST_PATCH_HOOKS+ commands if defined.
 
 If something goes wrong in the steps _3_ or _4_, then the build fails.
-- 
1.7.9.5

[Buildroot] [PATCH v4 2/2] manual: update for multiple global patch dirs

[Thomas De Schampheleire]

On Tue, Dec 17, 2013 at 10:00 AM, Ryan Barnett
<rjbarnet@rockwellcollins.com> wrote:
> Updating the documentation to reflect that multiple directories can
> now be specified for BR2_GLOBAL_PATCH_DIR. Along with giving an
> example use case of how to use multiple global patch directories.
>
> Signed-off-by: Ryan Barnett <rjbarnet@rockwellcollins.com>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
>
> ---
> Changes v3 -> v4:
>   - Fixed minor spelling mistakes and wording (suggested by Arnout)
>   - Reword section about order that patches are applied along with
>     making it clearer about when to use BR2_GLOBAL_PATCH_DIR
>     (suggested by Arnout)
>
> Changes v2 -> v3:
>   - None
>
> Changes v1 -> v2:
>   - Fixed minor spelling mistakes and wording (suggested by Thomas D)
> ---
>  docs/manual/customize-packages.txt |   91 ++++++++++++++++++++++++++++++++----
>  docs/manual/patch-policy.txt       |   20 +++++---
>  2 files changed, 95 insertions(+), 16 deletions(-)
>
[..]
>
> -For a specific version <packageversion> of a specific package <packagename>,
> -patches are applied as follows.
> +For a specific version +<packageversion>+ of a specific package
> ++<packagename>+, patches are applied from +BR2_GLOBAL_PATCH_DIR+ as
> +follows:
>
> -First, the default Buildroot patch set for the package is applied.
> +. For every directory - +<global-patch-dir>+ - that exists in
> +  +BR2_GLOBAL_PATCH_DIR+, a +<package-patch-dir>+ will be determined as
> +  follows:
> ++
> +* If the directory
> +  +<global-patch-dir>/<packagename>/<packageversion>/+ exists.
> ++
> +* Otherwise, if the directory +<global-patch-dir>/<packagename>+ exists.

I find this wording strange:
'.... will be determined as follows: if the directory A exists.
Otherwise, if the directory B exists.'

What about:
'.... will be determined as follows: A, if it exists. Otherwise, B, if
it exists.'

[..]

> +
> +The exception to +BR2_GLOBAL_PATCH_DIR+ being the preferred method for
> +specifying custom patches is +BR2_LINUX_KERNEL_PATCH+.
> ++BR2_LINUX_KERNEL_PATCH+ should be used to specify kernel patches that
> +are available at an URL. *Note:* +BR2_LINUX_KERNEL_PATCHES+ are applied
> +after patches available in +BR2_GLOBAL_PATCH_DIR+ as it is a patch
> +post-hook step for the Linux package.

'a patch post-hook step for the Linux package' sounds odd.
What about:
..., as it is done from a post-patch hook of the Linux package.

[..]

>
> +[[patch-apply-order]]
>  How patches are applied
>  ~~~~~~~~~~~~~~~~~~~~~~~
>
> @@ -64,19 +65,24 @@ How patches are applied
>  . If +<packagename>_PATCH+ is defined, then patches from these
>    tarballs are applied;
>
> -. If there are some +*.patch+ files in the package directory or in the
> -  a package subdirectory named +<packageversion>+, then:
> +. If there are some +*.patch+ files in the package's Buildroot
> +  directory or in a package subdirectory named +<packageversion>+,
> +  then:
>  +
>  * If a +series+ file exists in the package directory, then patches are
>    applied according to the +series+ file;
>  +
>  * Otherwise, patch files matching +<packagename>-*.patch+
>    are applied in alphabetical order.
> -  So, to ensure they are applied in the right order, it is hightly
> -  recommended to named the patch files like this:
> +  So, to ensure they are applied in the right order, it is highly
> +  recommended to name the patch files like this:
>    +<packagename>-<number>-<description>.patch+, where +<number>+
>    refers to the 'apply order'.
>
> +. If +BR2_GLOABL_PATCH_DIR+ is defined, the directories will be

GLOBAL

> +  enumerated in the order they are specified. The patches are applied
> +  as described in the previous step.
> +
>  . Run the +<packagename>_POST_PATCH_HOOKS+ commands if defined.
>
>  If something goes wrong in the steps _3_ or _4_, then the build fails.

I think it's a pity that there is duplication between this section and
the one on BR2_GLOBAL_PATCH_DIR.
However, it seems this was an explicit request made by Arnout.

Arnout, would it not be better to remove the duplication, but rather
use hyperlinks to refer from one section to the other, with the
detailed explanation about patch order being in this 'How patches are
applied' section?

Best regards,
Thomas

[Buildroot] [PATCH v4 2/2] manual: update for multiple global patch dirs

[Arnout Vandecappelle]

On 17/12/13 13:58, Thomas De Schampheleire wrote:
> On Tue, Dec 17, 2013 at 10:00 AM, Ryan Barnett
> <rjbarnet@rockwellcollins.com> wrote:
>> Updating the documentation to reflect that multiple directories can
>> now be specified for BR2_GLOBAL_PATCH_DIR. Along with giving an
>> example use case of how to use multiple global patch directories.
>>
>> Signed-off-by: Ryan Barnett <rjbarnet@rockwellcollins.com>
>> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
>> Cc: Arnout Vandecappelle <arnout@mind.be>
>>
>> ---
>> Changes v3 -> v4:
>>    - Fixed minor spelling mistakes and wording (suggested by Arnout)
>>    - Reword section about order that patches are applied along with
>>      making it clearer about when to use BR2_GLOBAL_PATCH_DIR
>>      (suggested by Arnout)
>>
>> Changes v2 -> v3:
>>    - None
>>
>> Changes v1 -> v2:
>>    - Fixed minor spelling mistakes and wording (suggested by Thomas D)
>> ---
>>   docs/manual/customize-packages.txt |   91 ++++++++++++++++++++++++++++++++----
>>   docs/manual/patch-policy.txt       |   20 +++++---
>>   2 files changed, 95 insertions(+), 16 deletions(-)
>>
> [..]
>>
>> -For a specific version <packageversion> of a specific package <packagename>,
>> -patches are applied as follows.
>> +For a specific version +<packageversion>+ of a specific package
>> ++<packagename>+, patches are applied from +BR2_GLOBAL_PATCH_DIR+ as
>> +follows:
>>
>> -First, the default Buildroot patch set for the package is applied.
>> +. For every directory - +<global-patch-dir>+ - that exists in
>> +  +BR2_GLOBAL_PATCH_DIR+, a +<package-patch-dir>+ will be determined as
>> +  follows:
>> ++
>> +* If the directory
>> +  +<global-patch-dir>/<packagename>/<packageversion>/+ exists.
>> ++
>> +* Otherwise, if the directory +<global-patch-dir>/<packagename>+ exists.
>
> I find this wording strange:
> '.... will be determined as follows: if the directory A exists.
> Otherwise, if the directory B exists.'
>
> What about:
> '.... will be determined as follows: A, if it exists. Otherwise, B, if
> it exists.'

  Actually for me, Ryan's formulation sounds more natural: if ... else if 
... else ....


[snip]

>> +  enumerated in the order they are specified. The patches are applied
>> +  as described in the previous step.
>> +
>>   . Run the +<packagename>_POST_PATCH_HOOKS+ commands if defined.
>>
>>   If something goes wrong in the steps _3_ or _4_, then the build fails.
>
> I think it's a pity that there is duplication between this section and
> the one on BR2_GLOBAL_PATCH_DIR.
> However, it seems this was an explicit request made by Arnout.
>
> Arnout, would it not be better to remove the duplication, but rather
> use hyperlinks to refer from one section to the other, with the
> detailed explanation about patch order being in this 'How patches are
> applied' section?

  I meant to say that the remark about numbering the patches should be 
duplicated, so that's just one sentence.

  I do agree that it would be better to move the whole discussion about 
the order in which patches are applied to this section (including a 
specific comment about the linux patches), with a crossref from the 
global patch dir section. However, Ryan didn't really change that 
structure (there was already some amount of duplication), so I think it 
can safely be done in a separate patch.

  IOW: I'm OK with committing this one as is, except for the gloabl typo.


  Regards,
  Arnout

>
> Best regards,
> Thomas
>
>


-- 
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

[Buildroot] [PATCH v4 2/2] manual: update for multiple global patch dirs

[Thomas De Schampheleire]

On Tue, Dec 17, 2013 at 2:19 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
[..]
>>>
>>>
>>> -For a specific version <packageversion> of a specific package
>>> <packagename>,
>>> -patches are applied as follows.
>>> +For a specific version +<packageversion>+ of a specific package
>>> ++<packagename>+, patches are applied from +BR2_GLOBAL_PATCH_DIR+ as
>>> +follows:
>>>
>>> -First, the default Buildroot patch set for the package is applied.
>>> +. For every directory - +<global-patch-dir>+ - that exists in
>>> +  +BR2_GLOBAL_PATCH_DIR+, a +<package-patch-dir>+ will be determined as
>>> +  follows:
>>> ++
>>> +* If the directory
>>> +  +<global-patch-dir>/<packagename>/<packageversion>/+ exists.
>>> ++
>>> +* Otherwise, if the directory +<global-patch-dir>/<packagename>+ exists.
>>
>>
>> I find this wording strange:
>> '.... will be determined as follows: if the directory A exists.
>> Otherwise, if the directory B exists.'
>>
>> What about:
>> '.... will be determined as follows: A, if it exists. Otherwise, B, if
>> it exists.'
>
>
>  Actually for me, Ryan's formulation sounds more natural: if ... else if ...
> else ....


The order of if/else are both fine for me, but I was more referring to
something else. The intro sentence says: "The order will be determined
as follows". When I read this, I expect to get a summary of items (the
'order'). However, what follows is a list of conditionals ("if A")
without 'then' statement.

It's a bit like this to me:
"On lazy days, I do only two things: if I am hungry, and if I am sleepy."
while I expect more something like:
"On lazy days, I do only two things: if I am hungry, I eat, and if I
am sleepy, I sleep."

[..]

>> I think it's a pity that there is duplication between this section and
>> the one on BR2_GLOBAL_PATCH_DIR.
>> However, it seems this was an explicit request made by Arnout.
>>
>> Arnout, would it not be better to remove the duplication, but rather
>> use hyperlinks to refer from one section to the other, with the
>> detailed explanation about patch order being in this 'How patches are
>> applied' section?
>
>
>  I meant to say that the remark about numbering the patches should be
> duplicated, so that's just one sentence.
>
>  I do agree that it would be better to move the whole discussion about the
> order in which patches are applied to this section (including a specific
> comment about the linux patches), with a crossref from the global patch dir
> section. However, Ryan didn't really change that structure (there was
> already some amount of duplication), so I think it can safely be done in a
> separate patch.
>

Fair enough, but is Ryan prepared to make that follow-up patch, or
should we wait until someone takes it up?

Best regards,
Thomas

[Buildroot] [PATCH v4 2/2] manual: update for multiple global patch dirs

[Arnout Vandecappelle]

On 17/12/13 14:30, Thomas De Schampheleire wrote:
> On Tue, Dec 17, 2013 at 2:19 PM, Arnout Vandecappelle<arnout@mind.be>  wrote:
> [..]
>>>> >>>
>>>> >>>
>>>> >>>-For a specific version <packageversion> of a specific package
>>>> >>><packagename>,
>>>> >>>-patches are applied as follows.
>>>> >>>+For a specific version +<packageversion>+ of a specific package
>>>> >>>++<packagename>+, patches are applied from +BR2_GLOBAL_PATCH_DIR+ as
>>>> >>>+follows:
>>>> >>>
>>>> >>>-First, the default Buildroot patch set for the package is applied.
>>>> >>>+. For every directory - +<global-patch-dir>+ - that exists in
>>>> >>>+  +BR2_GLOBAL_PATCH_DIR+, a +<package-patch-dir>+ will be determined as
>>>> >>>+  follows:
>>>> >>>++
>>>> >>>+* If the directory
>>>> >>>+  +<global-patch-dir>/<packagename>/<packageversion>/+ exists.
>>>> >>>++
>>>> >>>+* Otherwise, if the directory +<global-patch-dir>/<packagename>+ exists.
>>> >>
>>> >>
>>> >>I find this wording strange:
>>> >>'.... will be determined as follows: if the directory A exists.
>>> >>Otherwise, if the directory B exists.'
>>> >>
>>> >>What about:
>>> >>'.... will be determined as follows: A, if it exists. Otherwise, B, if
>>> >>it exists.'
>> >
>> >
>> >  Actually for me, Ryan's formulation sounds more natural: if ... else if ...
>> >else ....
>
> The order of if/else are both fine for me, but I was more referring to
> something else. The intro sentence says: "The order will be determined
> as follows". When I read this, I expect to get a summary of items (the
> 'order'). However, what follows is a list of conditionals ("if A")
> without 'then' statement.
>
> It's a bit like this to me:
> "On lazy days, I do only two things: if I am hungry, and if I am sleepy."
> while I expect more something like:
> "On lazy days, I do only two things: if I am hungry, I eat, and if I
> am sleepy, I sleep."

  Oh, yes, you're right, I didn't read it carefully. The 'then' is 
missing, and it can indeed be avoided by putting the 'if' behind it.

  Regards,
  Arnout


-- 
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

[Buildroot] [PATCH v4 2/2] manual: update for multiple global patch dirs

[Ryan Barnett]

Thomas D, Arnout,

Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote on 12/17/2013 
07:30:01 AM:

> On Tue, Dec 17, 2013 at 2:19 PM, Arnout Vandecappelle <arnout@mind.be> 
wrote:
>[..]
>>>>
>>>>
>>>> -For a specific version <packageversion> of a specific package
>>>> <packagename>,
>>>> -patches are applied as follows.
>>>> +For a specific version +<packageversion>+ of a specific package
>>>> ++<packagename>+, patches are applied from +BR2_GLOBAL_PATCH_DIR+ as
>>>> +follows:
>>>>
>>>> -First, the default Buildroot patch set for the package is applied.
>>>> +. For every directory - +<global-patch-dir>+ - that exists in
>>>> +  +BR2_GLOBAL_PATCH_DIR+, a +<package-patch-dir>+ will be determined 
as
>>>> +  follows:
>>>> ++
>>>> +* If the directory
>>>> +  +<global-patch-dir>/<packagename>/<packageversion>/+ exists.
>>>> ++
>>>> +* Otherwise, if the directory +<global-patch-dir>/<packagename>+ 
exists.
>>>
>>>
>>> I find this wording strange:
>>> '.... will be determined as follows: if the directory A exists.
>>> Otherwise, if the directory B exists.'
>>>
>>> What about:
>>> '.... will be determined as follows: A, if it exists. Otherwise, B, if
>>> it exists.'
>>
>>
>>  Actually for me, Ryan's formulation sounds more natural: if ... else 
if 
>>  ... else ....

> The order of if/else are both fine for me, but I was more referring to
> something else. The intro sentence says: "The order will be determined
> as follows". When I read this, I expect to get a summary of items (the
> 'order'). However, what follows is a list of conditionals ("if A")
> without 'then' statement.
> 
> It's a bit like this to me:
> "On lazy days, I do only two things: if I am hungry, and if I am 
sleepy."
> while I expect more something like:
> "On lazy days, I do only two things: if I am hungry, I eat, and if I

Is alright to keep the way it is? I prefer the way this looks when the 
html manual is generated.

>
>>> I think it's a pity that there is duplication between this section and
>>> the one on BR2_GLOBAL_PATCH_DIR.
>>> However, it seems this was an explicit request made by Arnout.
>>>
>>> Arnout, would it not be better to remove the duplication, but rather
>>> use hyperlinks to refer from one section to the other, with the
>>> detailed explanation about patch order being in this 'How patches are
>>> applied' section?
>>
>>
>>  I meant to say that the remark about numbering the patches should be
>> duplicated, so that's just one sentence.
>>
>>  I do agree that it would be better to move the whole discussion about 
the
>> order in which patches are applied to this section (including a 
specific
>> comment about the linux patches), with a crossref from the global patch 
dir
>> section. However, Ryan didn't really change that structure (there was
>> already some amount of duplication), so I think it can safely be done 
in a
>> separate patch.
>>
> 
> Fair enough, but is Ryan prepared to make that follow-up patch, or
> should we wait until someone takes it up?

I was going to follow-up this patch with another series that removes the 
options PATCH_DIR options for individual packages besides the kernel. When 
I do that, I was planning on cleaning up the documentation to avoid this 
duplication. It was easier for right now, to duplicate the section Arnout 
requested rather than to try to figure out the best way possible for 
avoiding duplication.

Thomas D - are you ok with this patch series besides the minor spelling 
mistake?

Thanks,
-Ryan

[Buildroot] [PATCH v4 2/2] manual: update for multiple global patch dirs

[Thomas Petazzoni]

Dear Ryan Barnett,

On Tue, 17 Dec 2013 08:24:47 -0600, Ryan Barnett wrote:

> I was going to follow-up this patch with another series that removes
> the options PATCH_DIR options for individual packages besides the
> kernel. When I do that, I was planning on cleaning up the
> documentation to avoid this duplication. It was easier for right now,
> to duplicate the section Arnout requested rather than to try to
> figure out the best way possible for avoiding duplication.

Hmm, for which packages do you want to remove the PATCH_DIR options? If
they exist for some packages, it's quite hard to remove them, as you're
breaking compatibility. Not sure what should be our backward
compatibility guarantee in this kind of cases.

And why "besides the kernel" ?

Best regards,

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

[Buildroot] [PATCH v4 2/2] manual: update for multiple global patch dirs

[Arnout Vandecappelle]

On 17/12/13 16:02, Thomas Petazzoni wrote:
> Dear Ryan Barnett,
>
> On Tue, 17 Dec 2013 08:24:47 -0600, Ryan Barnett wrote:
>
>> I was going to follow-up this patch with another series that removes
>> the options PATCH_DIR options for individual packages besides the
>> kernel. When I do that, I was planning on cleaning up the
>> documentation to avoid this duplication. It was easier for right now,
>> to duplicate the section Arnout requested rather than to try to
>> figure out the best way possible for avoiding duplication.
>
> Hmm, for which packages do you want to remove the PATCH_DIR options? If
> they exist for some packages, it's quite hard to remove them, as you're
> breaking compatibility. Not sure what should be our backward
> compatibility guarantee in this kind of cases.
>
> And why "besides the kernel" ?

  Read the rest of the thread :-)

  In summary:

- For U-Boot, Barebox, maybe others there are config options which 
provide exactly the same functionality as GLOBAL_PATCH_DIR. So these 
options can be removed IMHO (with Config.in.legacy pointing to the 
GLOBAL_PATCH_DIR option).

- For the kernel, there is an option that provides a slightly different 
functionality: it allows to download patches in addition to using patches 
on the local system.


  Regards,
  Arnout


-- 
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

[Buildroot] [PATCH v4 2/2] manual: update for multiple global patch dirs

[Thomas De Schampheleire]

On Tue, Dec 17, 2013 at 3:24 PM, Ryan Barnett
<rjbarnet@rockwellcollins.com> wrote:
> Thomas D, Arnout,
>
> Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote on 12/17/2013
> 07:30:01 AM:
>
>> On Tue, Dec 17, 2013 at 2:19 PM, Arnout Vandecappelle <arnout@mind.be>
> wrote:
>>[..]
>>>>>
>>>>>
>>>>> -For a specific version <packageversion> of a specific package
>>>>> <packagename>,
>>>>> -patches are applied as follows.
>>>>> +For a specific version +<packageversion>+ of a specific package
>>>>> ++<packagename>+, patches are applied from +BR2_GLOBAL_PATCH_DIR+ as
>>>>> +follows:
>>>>>
>>>>> -First, the default Buildroot patch set for the package is applied.
>>>>> +. For every directory - +<global-patch-dir>+ - that exists in
>>>>> +  +BR2_GLOBAL_PATCH_DIR+, a +<package-patch-dir>+ will be determined
> as
>>>>> +  follows:
>>>>> ++
>>>>> +* If the directory
>>>>> +  +<global-patch-dir>/<packagename>/<packageversion>/+ exists.
>>>>> ++
>>>>> +* Otherwise, if the directory +<global-patch-dir>/<packagename>+
> exists.
>>>>
>>>>
>>>> I find this wording strange:
>>>> '.... will be determined as follows: if the directory A exists.
>>>> Otherwise, if the directory B exists.'
>>>>
>>>> What about:
>>>> '.... will be determined as follows: A, if it exists. Otherwise, B, if
>>>> it exists.'
>>>
>>>
>>>  Actually for me, Ryan's formulation sounds more natural: if ... else
> if
>>>  ... else ....
>
>> The order of if/else are both fine for me, but I was more referring to
>> something else. The intro sentence says: "The order will be determined
>> as follows". When I read this, I expect to get a summary of items (the
>> 'order'). However, what follows is a list of conditionals ("if A")
>> without 'then' statement.
>>
>> It's a bit like this to me:
>> "On lazy days, I do only two things: if I am hungry, and if I am
> sleepy."
>> while I expect more something like:
>> "On lazy days, I do only two things: if I am hungry, I eat, and if I
>
> Is alright to keep the way it is? I prefer the way this looks when the
> html manual is generated.

I'm not sure we fully understand each other. My comment is not really
about the looks of it, but rather about a missing part of the
sentence. I don't really care about "if A then B" or "B, if A", it was
just a suggestion. My main point is that there is no 'then' part on
the if, making the whole sentence incomplete.

Unless you can convince me that this is correct English, I think this
should be fixed in this patch and not in a followup.


Best regards,
Thomas

[Buildroot] [PATCH v4 2/2] manual: update for multiple global patch dirs

[Thomas De Schampheleire]

On Tue, Dec 17, 2013 at 5:07 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> On Tue, Dec 17, 2013 at 3:24 PM, Ryan Barnett
> <rjbarnet@rockwellcollins.com> wrote:
>> Thomas D, Arnout,
>>
>> Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote on 12/17/2013
>> 07:30:01 AM:
>>
>>> On Tue, Dec 17, 2013 at 2:19 PM, Arnout Vandecappelle <arnout@mind.be>
>> wrote:
>>>[..]
>>>>>>
>>>>>>
>>>>>> -For a specific version <packageversion> of a specific package
>>>>>> <packagename>,
>>>>>> -patches are applied as follows.
>>>>>> +For a specific version +<packageversion>+ of a specific package
>>>>>> ++<packagename>+, patches are applied from +BR2_GLOBAL_PATCH_DIR+ as
>>>>>> +follows:
>>>>>>
>>>>>> -First, the default Buildroot patch set for the package is applied.
>>>>>> +. For every directory - +<global-patch-dir>+ - that exists in
>>>>>> +  +BR2_GLOBAL_PATCH_DIR+, a +<package-patch-dir>+ will be determined
>> as
>>>>>> +  follows:
>>>>>> ++
>>>>>> +* If the directory
>>>>>> +  +<global-patch-dir>/<packagename>/<packageversion>/+ exists.
>>>>>> ++
>>>>>> +* Otherwise, if the directory +<global-patch-dir>/<packagename>+
>> exists.
>>>>>
>>>>>
>>>>> I find this wording strange:
>>>>> '.... will be determined as follows: if the directory A exists.
>>>>> Otherwise, if the directory B exists.'
>>>>>
>>>>> What about:
>>>>> '.... will be determined as follows: A, if it exists. Otherwise, B, if
>>>>> it exists.'
>>>>
>>>>
>>>>  Actually for me, Ryan's formulation sounds more natural: if ... else
>> if
>>>>  ... else ....
>>
>>> The order of if/else are both fine for me, but I was more referring to
>>> something else. The intro sentence says: "The order will be determined
>>> as follows". When I read this, I expect to get a summary of items (the
>>> 'order'). However, what follows is a list of conditionals ("if A")
>>> without 'then' statement.
>>>
>>> It's a bit like this to me:
>>> "On lazy days, I do only two things: if I am hungry, and if I am
>> sleepy."
>>> while I expect more something like:
>>> "On lazy days, I do only two things: if I am hungry, I eat, and if I
>>
>> Is alright to keep the way it is? I prefer the way this looks when the
>> html manual is generated.
>
> I'm not sure we fully understand each other. My comment is not really
> about the looks of it, but rather about a missing part of the
> sentence. I don't really care about "if A then B" or "B, if A", it was
> just a suggestion. My main point is that there is no 'then' part on
> the if, making the whole sentence incomplete.
>
> Unless you can convince me that this is correct English, I think this
> should be fixed in this patch and not in a followup.

Nevermind, I just see your v5. I will re-read it tomorrow.

Thanks,
Thomas

[Buildroot] [PATCH v4 2/2] manual: update for multiple global patch dirs

[Ryan Barnett]

Thomas D, Arnout,

Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote on 12/17/2013 
10:07:56 AM:

[...]
 
> I'm not sure we fully understand each other. My comment is not really
> about the looks of it, but rather about a missing part of the
> sentence. I don't really care about "if A then B" or "B, if A", it was
> just a suggestion. My main point is that there is no 'then' part on
> the if, making the whole sentence incomplete.
> 
> Unless you can convince me that this is correct English, I think this
> should be fixed in this patch and not in a followup.

I'm sorry I sent that email before Arnout said you were correct. Once he 
said you were correct, a light blub also went off and I corrected the 
issue for v5. The thread was a bit confusing there so by hopefully by 
sending a V5 of this patch all outstanding issues have been resolved.

As I stated previously, I intend to submit a follow-up series where I 
clean the patching up even further. For now though, I really want to get 
the ability to specify multiple BR2_GLOBAL_PATCH_DIR into mainline.

Thanks,
-Ryan

[Buildroot] [PATCH v4 1/2] Support for multiple BR2_GLOBAL_PATCH_DIR

[Thomas De Schampheleire]

On Tue, Dec 17, 2013 at 10:00 AM, Ryan Barnett
<rjbarnet@rockwellcollins.com> wrote:
> Adding support for specifying multiple directories in
> BR2_GLOBAL_PATCH_DIR. This will allow for a layered approach for the
> patching of a package.
>
> Signed-off-by: Ryan Barnett <rjbarnet@rockwellcollins.com>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
>
> ---
> Changes v3 -> v4:
>   - None
>
> Changes v2 -> v3:
>   - changed the generation of patch directories to use 'addsuffix'
>     instead of a foreach loop. (suggested by Arnout)
>
> Changes v1 -> v2:
>   - change wording in Config.in help (suggested by Thomas D)
> ---
>  Config.in              |   20 ++++++++++++--------
>  package/pkg-generic.mk |    5 ++++-
>  2 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/Config.in b/Config.in
> index 2b401cb..d55e57c 100644
> --- a/Config.in
> +++ b/Config.in
> @@ -461,18 +461,22 @@ config BR2_PACKAGE_OVERRIDE_FILE
>           Buildroot documentation for more details on this feature.
>
>  config BR2_GLOBAL_PATCH_DIR
> -       string "global patch directory"
> +       string "global patch directories"
>         help
> -         You may specify a directory containing global package patches.
> -         For a specific version <packageversion> of a specific package
> -         <packagename>, patches are applied as follows.
> +         You may specify a space separated list of one or more directories
> +         containing global package patches. For a specific version
> +         <packageversion> of a specific package <packagename>, patches are
> +         applied as follows:
>
> -         First, the default Buildroot patch set for the package is applied.
> +         First, the default Buildroot patch set for the package is applied
> +         from the package's directory in Buildroot.
>
> -         If the directory $(BR2_GLOBAL_PATCH_DIR)/<packagename>/<packageversion>
> -         exists, then all *.patch files in the directory will be applied.
> +         Then for every directory - <global-patch-dir> - that exists in
> +         BR2_GLOBAL_PATCH_DIR, if the directory
> +         <global-patch-dir>/<packagename>/<packageversion>/ exists, then all
> +         *.patch files in this directory will be applied.
>
> -         Otherwise, if the directory $(BR2_GLOBAL_PATCH_DIR)/<packagename> exists,
> +         Otherwise, if the directory <global-patch-dir>/<packagename> exists,
>           then all *.patch files in the directory will be applied.
>
>  endmenu
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 45b808a..66034ba 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -134,8 +134,11 @@ endif
>  # The RAWNAME variable is the lowercased package name, which allows to
>  # find the package directory (typically package/<pkgname>) and the
>  # prefix of the patches
> +#
> +# For BR2_GLOBAL_PATCH_DIR, only generate if it is defined
>  $(BUILD_DIR)/%/.stamp_patched: NAMEVER = $(RAWNAME)-$($(PKG)_VERSION)
> -$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS = $($(PKG)_DIR_PREFIX)/$(RAWNAME) $(call qstrip,$(BR2_GLOBAL_PATCH_DIR))/$(RAWNAME)
> +$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS =  $($(PKG)_DIR_PREFIX)/$(RAWNAME)
> +$(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS += $(addsuffix /$(RAWNAME),$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)))
>  $(BUILD_DIR)/%/.stamp_patched:
>         @$(call step_start,patch)
>         @$(call MESSAGE,"Patching")

Reviewed-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>