[Buildroot] [PATCH 1/1] Include makefiles from GLOBAL_PATCH_DIR

[Buildroot] [PATCH 1/1] Include makefiles from GLOBAL_PATCH_DIR

[Jérémy Rosen]

Including a file named <pkgname>.mk from the GLOBAL_PATCH_DIR allows a user
to tweak a package provided by buildroot to his own need.

* add a function to pkg-utils.mk to include makefiles from GLOBAL_PATCH_DIR
* modify all pkg-* to use it as their first action

Signed-off-by: J?r?my Rosen <jeremy.rosen@smile.fr>
---
I am not very proficient in makefiles, so feel free to criticize my
coding style.

I am using GLOBAL_PATCH_DIR to search for .mk because this directory
already has a PACKAGE/VERSION layout with priority rules, but this can
be changed if there is a consensus for another location
---
 package/pkg-autotools.mk     |  1 +
 package/pkg-cmake.mk         |  1 +
 package/pkg-generic.mk       |  8 ++++++--
 package/pkg-golang.mk        |  1 +
 package/pkg-kconfig.mk       |  1 +
 package/pkg-kernel-module.mk |  1 +
 package/pkg-luarocks.mk      |  1 +
 package/pkg-meson.mk         |  1 +
 package/pkg-perl.mk          |  1 +
 package/pkg-python.mk        |  1 +
 package/pkg-rebar.mk         |  1 +
 package/pkg-utils.mk         | 14 ++++++++++++++
 package/pkg-virtual.mk       |  1 +
 package/pkg-waf.mk           |  1 +
 14 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/package/pkg-autotools.mk b/package/pkg-autotools.mk
index daa688dab6..b18ee4edf5 100644
--- a/package/pkg-autotools.mk
+++ b/package/pkg-autotools.mk
@@ -119,6 +119,7 @@ endef
 ################################################################################
 
 define inner-autotools-package
+$(call global-patchdir-include,$(1),$(2))
 
 ifndef $(2)_LIBTOOL_PATCH
  ifdef $(3)_LIBTOOL_PATCH
diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
index b9ce8ff622..adbcf6a850 100644
--- a/package/pkg-cmake.mk
+++ b/package/pkg-cmake.mk
@@ -50,6 +50,7 @@ endif
 ################################################################################
 
 define inner-cmake-package
+$(call global-patchdir-include,$(1),$(2))
 
 $(2)_CONF_ENV			?=
 $(2)_CONF_OPTS			?=
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 1f24b52a69..86024aebd5 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -414,6 +414,10 @@ endef
 #   undesired behavior occurs with respect to these variables and functions.
 #
 ################################################################################
+define intermediate-generic-package
+$(call global-patchdir-include,$(1),$(2))
+$(call inner-generic-package,$(1),$(2),$(3),$(4)) 
+endef
 
 define inner-generic-package
 
@@ -1126,8 +1130,8 @@ endef # inner-generic-package
 ################################################################################
 
 # In the case of target packages, keep the package name "pkg"
-generic-package = $(call inner-generic-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
+generic-package = $(call intermediate-generic-package,$(pkgname),$(call UPPERCASE,$(pkgname)),$(call UPPERCASE,$(pkgname)),target)
 # In the case of host packages, turn the package name "pkg" into "host-pkg"
-host-generic-package = $(call inner-generic-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
+host-generic-package = $(call intermediate-generic-package,host-$(pkgname),$(call UPPERCASE,host-$(pkgname)),$(call UPPERCASE,$(pkgname)),host)
 
 # :mode=makefile:
diff --git a/package/pkg-golang.mk b/package/pkg-golang.mk
index e47de17aba..916645c52d 100644
--- a/package/pkg-golang.mk
+++ b/package/pkg-golang.mk
@@ -55,6 +55,7 @@ GO_HOST_ENV = \
 ################################################################################
 
 define inner-golang-package
+$(call global-patchdir-include,$(1),$(2))
 
 $(2)_WORKSPACE ?= _gopath
 
diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index 86d7c14fdb..5e06adaa0d 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -77,6 +77,7 @@ endef
 ################################################################################
 
 define inner-kconfig-package
+$(call global-patchdir-include,$(1),$(2))
 
 # Register the kconfig dependencies as regular dependencies, so that
 # they are also accounted for in the generated graphs.
diff --git a/package/pkg-kernel-module.mk b/package/pkg-kernel-module.mk
index fcd6b8bc29..b9e211aac1 100644
--- a/package/pkg-kernel-module.mk
+++ b/package/pkg-kernel-module.mk
@@ -43,6 +43,7 @@
 ################################################################################
 
 define inner-kernel-module
+$(call global-patchdir-include,$(1),$(2))
 
 # If the package is enabled, ensure the kernel will support modules
 ifeq ($$(BR2_PACKAGE_$(2)),y)
diff --git a/package/pkg-luarocks.mk b/package/pkg-luarocks.mk
index 78d6c325f8..9a2b508532 100644
--- a/package/pkg-luarocks.mk
+++ b/package/pkg-luarocks.mk
@@ -32,6 +32,7 @@
 ################################################################################
 
 define inner-luarocks-package
+$(call global-patchdir-include,$(1),$(2))
 
 $(2)_BUILD_OPTS		?=
 $(2)_NAME_UPSTREAM	?= $(1)
diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk
index 184a22a44a..0e00a45065 100644
--- a/package/pkg-meson.mk
+++ b/package/pkg-meson.mk
@@ -44,6 +44,7 @@ NINJA_OPTS	= $(if $(VERBOSE),-v) -j$(PARALLEL_JOBS)
 ################################################################################
 
 define inner-meson-package
+$(call global-patchdir-include,$(1),$(2))
 
 $(2)_CONF_ENV		?=
 $(2)_CONF_OPTS		?=
diff --git a/package/pkg-perl.mk b/package/pkg-perl.mk
index 1ecf31eff9..88cc00c5f6 100644
--- a/package/pkg-perl.mk
+++ b/package/pkg-perl.mk
@@ -38,6 +38,7 @@ PERL_RUN = PERL5LIB= PERL_USE_UNSAFE_INC=1 $(HOST_DIR)/bin/perl
 ################################################################################
 
 define inner-perl-package
+$(call global-patchdir-include,$(1),$(2))
 
 # Target packages need both the perl interpreter on the target (for
 # runtime) and the perl interpreter on the host (for
diff --git a/package/pkg-python.mk b/package/pkg-python.mk
index be1ce071df..26391e09ba 100644
--- a/package/pkg-python.mk
+++ b/package/pkg-python.mk
@@ -104,6 +104,7 @@ HOST_PKG_PYTHON_SETUPTOOLS_INSTALL_OPTS = \
 ################################################################################
 
 define inner-python-package
+$(call global-patchdir-include,$(1),$(2))
 
 $(2)_ENV         ?=
 $(2)_BUILD_OPTS   ?=
diff --git a/package/pkg-rebar.mk b/package/pkg-rebar.mk
index e4e3f3bb6c..282f7e600f 100644
--- a/package/pkg-rebar.mk
+++ b/package/pkg-rebar.mk
@@ -118,6 +118,7 @@ endef
 ################################################################################
 
 define inner-rebar-package
+$(call global-patchdir-include,$(1),$(2))
 
 # Extract just the raw package name, lowercase without the leading
 # erlang- or host- prefix, as this is used by rebar to find the
diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index 63b19e812b..28bcb93386 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -175,3 +175,17 @@ legal-deps = \
         $(filter-out $(if $(1:host-%=),host-%),\
             $(call non-virtual-deps,\
                 $($(call UPPERCASE,$(1))_FINAL_RECURSIVE_DEPENDENCIES))),$(p) [$($(call UPPERCASE,$(p))_LICENSE)])
+# Include a makefile from GLOBAL_PATCH_DIR
+# This follows the namind and directory layout of GLOBAL_PATCH_DIR
+#  argument 1 is the lowercase package name
+#  argument 2 is the uppercase package name, including a HOST_ prefix
+#             for host packages
+define global-patchdir-include
+$(foreach dir,$(addsuffix /$(patsubst host-%,%,$(1)),$(call qstrip,$(BR2_GLOBAL_PATCH_DIR))),\
+	$(if $(wildcard $(dir)/$($(2)_VERSION)/$(1).mk),\
+		include $(dir)/$($(2)_VERSION)/$(1).mk  \
+	,$(if $(wildcard $(dir)/$(1).mk),\
+		include $(dir)/$(1).mk \
+	))\
+)
+endef
diff --git a/package/pkg-virtual.mk b/package/pkg-virtual.mk
index 05bd63eb18..c34834c884 100644
--- a/package/pkg-virtual.mk
+++ b/package/pkg-virtual.mk
@@ -33,6 +33,7 @@
 # so it is not evaluated now, but as part of the generated make code.
 
 define inner-virtual-package
+$(call global-patchdir-include,$(1),$(2))
 
 # Ensure the virtual package has an implementation defined.
 ifeq ($$(BR2_PACKAGE_HAS_$(2)),y)
diff --git a/package/pkg-waf.mk b/package/pkg-waf.mk
index a32d5dab33..05bd90e463 100644
--- a/package/pkg-waf.mk
+++ b/package/pkg-waf.mk
@@ -35,6 +35,7 @@
 ################################################################################
 
 define inner-waf-package
+$(call global-patchdir-include,$(1),$(2))
 
 # We need host-python to run waf
 $(2)_DEPENDENCIES += host-python
-- 
2.23.0

[Buildroot] [PATCH 1/1] Include makefiles from GLOBAL_PATCH_DIR

[Yann E. MORIN]

J?r?my, All,

Adding some core devs to the Cc list.

On 2019-10-26 16:51 +0200, J?r?my Rosen spake thusly:
> Including a file named <pkgname>.mk from the GLOBAL_PATCH_DIR allows a user
> to tweak a package provided by buildroot to his own need.

Thanks for sharing your work with this proposal.

> * add a function to pkg-utils.mk to include makefiles from GLOBAL_PATCH_DIR
> * modify all pkg-* to use it as their first action

So I finally took the time to think about this patch. This is not really
a review per-se, because the core of the changes are pretty trivial
overall.

Rather, it is mostly my point of view on the subject, and why I think
an override-based solution is a bad idea. We discussed this during the
last dev-days, but for posterity, I'll recap all here to spur more
comments.


*
First, this solution does not cover all use-cases. One major drawback it
has, is that it can't allow overriding the version of a package. The
reason for this limitation is that he .hash file is not overridden, so it
is not possible to provide an alternate version. However, when exposed
with your proposal, the very first thing most users thought about using
it for, was *exactly* for that: changing the version of the package.

So, the most obvious limitations of this solution, is also the most
obvious use-case users would expect to use it for. Big drawback...


*
Second, the excuse for such an override is to be able to claim not
touching the Buildroot tree, and to be able to update the Buildroot tree
with just a simple "git pull".

I believe this is broken by design, because this actually hides breakage
when updating Buildroot. When you update Buildroot, you have to account
for the changes in the new version: new config options, new variables,
new values in existing variables, or even changes deep in the various
infrastructures. A proper override would have to go so far as to unset
all the variables of a package before redefining it entirely.

Hiding local changes away in an override actually is a missed opportunity
to notice these changes with actual merge (or rebase) conflicts, instead
leading to issues much later down the road, which means a lot of time
lost. That would not show semantic conflicts, true, but actual
code-level conflicts would show, and they would probably be the most
common.

So, I am afraid that these overrides are a poor excuse for not wanting to
do actual VCS work.


*
Third, since the override is outside the Buildroot tree (if it were in,
there would be no point in the override to begin with), it means it is
easy to miss it in critical times. Some packages in Buildroot are
licensed under copyleft licenses (GPL, LGPL...), and those have 
requirements reading something along the lines of "complete source code
means [...], plus the scripts used to control compilation and
installation of the executable." Scripts which happens to be Buildroot
in our case, and the stuff in your override.

Since the override would live in a separate tree with private, non
public stuff, it would be very easy to miss it when coming into
compliance for such packages, as one would be tempted to only distribute
their Buildroot tree.

So, such overrides actually makes one's life more difficult in this
already complex topic.


*
In conclusion, my position on this topic is pretty straightforward: if
you want to modify the package recipes, then do so, and do so your the
Buildroot tree. Use a branch that contains your changes, and when you
need, merge a new master (or stable, or LTS) into your branch.

This is probably not a pleasing answer or review, I am sorry for that,
but for all the reasons expressed above, I think an override-based
solution is not a correct solution.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/1] Include makefiles from GLOBAL_PATCH_DIR

[Jérémy ROSEN]

Thanks Yann, this is not really a surprise with the discussion we had. I'm
still a bit disappointed since this patch has always been a first step in
being able to change the version and one of the most requested feature for
a very long time, but that's the way it goes, I guess...

At least the people reading this mailing list will be able to use the
feature for their needs...

Cheers
Jeremy

Le mer. 6 nov. 2019 ? 17:38, Yann E. MORIN <yann.morin.1998@free.fr> a
?crit :

> J?r?my, All,
>
> Adding some core devs to the Cc list.
>
> On 2019-10-26 16:51 +0200, J?r?my Rosen spake thusly:
> > Including a file named <pkgname>.mk from the GLOBAL_PATCH_DIR allows a
> user
> > to tweak a package provided by buildroot to his own need.
>
> Thanks for sharing your work with this proposal.
>
> > * add a function to pkg-utils.mk to include makefiles from
> GLOBAL_PATCH_DIR
> > * modify all pkg-* to use it as their first action
>
> So I finally took the time to think about this patch. This is not really
> a review per-se, because the core of the changes are pretty trivial
> overall.
>
> Rather, it is mostly my point of view on the subject, and why I think
> an override-based solution is a bad idea. We discussed this during the
> last dev-days, but for posterity, I'll recap all here to spur more
> comments.
>
>
> *
> First, this solution does not cover all use-cases. One major drawback it
> has, is that it can't allow overriding the version of a package. The
> reason for this limitation is that he .hash file is not overridden, so it
> is not possible to provide an alternate version. However, when exposed
> with your proposal, the very first thing most users thought about using
> it for, was *exactly* for that: changing the version of the package.
>
> So, the most obvious limitations of this solution, is also the most
> obvious use-case users would expect to use it for. Big drawback...
>
>
> *
> Second, the excuse for such an override is to be able to claim not
> touching the Buildroot tree, and to be able to update the Buildroot tree
> with just a simple "git pull".
>
> I believe this is broken by design, because this actually hides breakage
> when updating Buildroot. When you update Buildroot, you have to account
> for the changes in the new version: new config options, new variables,
> new values in existing variables, or even changes deep in the various
> infrastructures. A proper override would have to go so far as to unset
> all the variables of a package before redefining it entirely.
>
> Hiding local changes away in an override actually is a missed opportunity
> to notice these changes with actual merge (or rebase) conflicts, instead
> leading to issues much later down the road, which means a lot of time
> lost. That would not show semantic conflicts, true, but actual
> code-level conflicts would show, and they would probably be the most
> common.
>
> So, I am afraid that these overrides are a poor excuse for not wanting to
> do actual VCS work.
>
>
> *
> Third, since the override is outside the Buildroot tree (if it were in,
> there would be no point in the override to begin with), it means it is
> easy to miss it in critical times. Some packages in Buildroot are
> licensed under copyleft licenses (GPL, LGPL...), and those have
> requirements reading something along the lines of "complete source code
> means [...], plus the scripts used to control compilation and
> installation of the executable." Scripts which happens to be Buildroot
> in our case, and the stuff in your override.
>
> Since the override would live in a separate tree with private, non
> public stuff, it would be very easy to miss it when coming into
> compliance for such packages, as one would be tempted to only distribute
> their Buildroot tree.
>
> So, such overrides actually makes one's life more difficult in this
> already complex topic.
>
>
> *
> In conclusion, my position on this topic is pretty straightforward: if
> you want to modify the package recipes, then do so, and do so your the
> Buildroot tree. Use a branch that contains your changes, and when you
> need, merge a new master (or stable, or LTS) into your branch.
>
> This is probably not a pleasing answer or review, I am sorry for that,
> but for all the reasons expressed above, I think an override-based
> solution is not a correct solution.
>
> Regards,
> Yann E. MORIN.
>
> --
>
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
> conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___
>      |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is
> no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v
>  conspiracy.  |
>
> '------------------------------^-------^------------------^--------------------'
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20191106/51c0296e/attachment.html>

[Buildroot] [PATCH 1/1] Include makefiles from GLOBAL_PATCH_DIR

[Thomas Petazzoni]

Hello,

On Wed, 6 Nov 2019 17:37:48 +0100
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> > * add a function to pkg-utils.mk to include makefiles from GLOBAL_PATCH_DIR
> > * modify all pkg-* to use it as their first action  
> 
> So I finally took the time to think about this patch. This is not really
> a review per-se, because the core of the changes are pretty trivial
> overall.
> 
> Rather, it is mostly my point of view on the subject, and why I think
> an override-based solution is a bad idea. We discussed this during the
> last dev-days, but for posterity, I'll recap all here to spur more
> comments.

Here is also my point of view. I will contradict Yann on several
points, but this does not necessarily mean that I strongly support the
patch. It's just an attempt at providing a different point of view in
the discussion.

> First, this solution does not cover all use-cases. One major drawback it
> has, is that it can't allow overriding the version of a package. The
> reason for this limitation is that he .hash file is not overridden, so it
> is not possible to provide an alternate version. However, when exposed
> with your proposal, the very first thing most users thought about using
> it for, was *exactly* for that: changing the version of the package.
> 
> So, the most obvious limitations of this solution, is also the most
> obvious use-case users would expect to use it for. Big drawback...

True, but could we also allow overriding the .hash file ? Perhaps we
could specify that in a BR2_EXTERNAL tree, package/foo/foo.mk gets
included at the end of package/foo/foo.mk of the main Buildroot tree,
right before the $(eval $(something-package)), and that
package/foo/foo.hash in the BR2_EXTERNAL takes precedence over
package/foo/foo.hash in the Buildroot tree. I agree it starts to be
messy and complicated, but probably not impossible either.

> Second, the excuse for such an override is to be able to claim not
> touching the Buildroot tree, and to be able to update the Buildroot tree
> with just a simple "git pull".
> 
> I believe this is broken by design, because this actually hides breakage
> when updating Buildroot. When you update Buildroot, you have to account
> for the changes in the new version: new config options, new variables,
> new values in existing variables, or even changes deep in the various
> infrastructures. A proper override would have to go so far as to unset
> all the variables of a package before redefining it entirely.
> 
> Hiding local changes away in an override actually is a missed opportunity
> to notice these changes with actual merge (or rebase) conflicts, instead
> leading to issues much later down the road, which means a lot of time
> lost. That would not show semantic conflicts, true, but actual
> code-level conflicts would show, and they would probably be the most
> common.
> 
> So, I am afraid that these overrides are a poor excuse for not wanting to
> do actual VCS work.

Well, you are the one who introduced BR2_EXTERNAL in the first place!
And what we can put today in a BR2_EXTERNAL is in fact what is the
easiest to maintain using a VCS inside the Buildroot tree: new
defconfigs, new packages, board/ artefacts. These are always new files,
they will never conflict in any way when rebasing, etc. And still, we
agreed on adding BR2_EXTERNAL, even if at the time there was the same
concern: why don't people simply use the capabilities of their VCS.

> Third, since the override is outside the Buildroot tree (if it were in,
> there would be no point in the override to begin with), it means it is
> easy to miss it in critical times. Some packages in Buildroot are
> licensed under copyleft licenses (GPL, LGPL...), and those have 
> requirements reading something along the lines of "complete source code
> means [...], plus the scripts used to control compilation and
> installation of the executable." Scripts which happens to be Buildroot
> in our case, and the stuff in your override.
> 
> Since the override would live in a separate tree with private, non
> public stuff, it would be very easy to miss it when coming into
> compliance for such packages, as one would be tempted to only distribute
> their Buildroot tree.

The problem already exists today with BR2_EXTERNAL: one can easily mess
up and create a BR2_EXTERNAL with a mix of GPL-licensed packages and
proprietary-licensed packages, and forget to redistribute the source
code of the BR2_EXTERNAL. So that doesn't really seem like a good
argument against Jeremy's patch: the problem already exists today, and
we can't avoid it. Jeremy's patch doesn't really make it better or
worse.

A reasonably sane person would probably use two BR2_EXTERNAL: a first
one with the additional GPL-licensed packages + the package overrides
to existing Buildroot packages, and a second one with all the
proprietary/product-specific bits. The first BR2_EXTERNAL would be
published, not the second one.

> In conclusion, my position on this topic is pretty straightforward: if
> you want to modify the package recipes, then do so, and do so your the
> Buildroot tree. Use a branch that contains your changes, and when you
> need, merge a new master (or stable, or LTS) into your branch.

So we can remove the BR2_EXTERNAL feature ? :-)

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] Include makefiles from GLOBAL_PATCH_DIR

[Vadim Kochan]

Hi All,

Regarding the package replacing by one from the BR2_EXTERNAL, I really
did not tried this
just in case may be it may help.

On Thu, Nov 7, 2019 at 12:31 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello,
>
> On Wed, 6 Nov 2019 17:37:48 +0100
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
>
> > > * add a function to pkg-utils.mk to include makefiles from GLOBAL_PATCH_DIR
> > > * modify all pkg-* to use it as their first action
> >
> > So I finally took the time to think about this patch. This is not really
> > a review per-se, because the core of the changes are pretty trivial
> > overall.
> >
> > Rather, it is mostly my point of view on the subject, and why I think
> > an override-based solution is a bad idea. We discussed this during the
> > last dev-days, but for posterity, I'll recap all here to spur more
> > comments.
>
> Here is also my point of view. I will contradict Yann on several
> points, but this does not necessarily mean that I strongly support the
> patch. It's just an attempt at providing a different point of view in
> the discussion.
>
> > First, this solution does not cover all use-cases. One major drawback it
> > has, is that it can't allow overriding the version of a package. The
> > reason for this limitation is that he .hash file is not overridden, so it
> > is not possible to provide an alternate version. However, when exposed
> > with your proposal, the very first thing most users thought about using
> > it for, was *exactly* for that: changing the version of the package.
> >
> > So, the most obvious limitations of this solution, is also the most
> > obvious use-case users would expect to use it for. Big drawback...
>
> True, but could we also allow overriding the .hash file ? Perhaps we
> could specify that in a BR2_EXTERNAL tree, package/foo/foo.mk gets
> included at the end of package/foo/foo.mk of the main Buildroot tree,
> right before the $(eval $(something-package)), and that
> package/foo/foo.hash in the BR2_EXTERNAL takes precedence over
> package/foo/foo.hash in the Buildroot tree. I agree it starts to be
> messy and complicated, but probably not impossible either.
>

Well, I am not a makefile expert but what if BR2_EXTERNAL package might set
XXX_OVERRIDE_PKG and replace the original package with its content and
the following
code in common Makefile (which is in Buildroot tree) will just skip
including the original package:

    include $(sort $(foreach p,$(wildcard package/*/*.mk),$(if
$($(call UPPERCASE,$(p))_OVERRIDE_PKG),,$(p))))

> > Second, the excuse for such an override is to be able to claim not
> > touching the Buildroot tree, and to be able to update the Buildroot tree
> > with just a simple "git pull".
> >
> > I believe this is broken by design, because this actually hides breakage
> > when updating Buildroot. When you update Buildroot, you have to account
> > for the changes in the new version: new config options, new variables,
> > new values in existing variables, or even changes deep in the various
> > infrastructures. A proper override would have to go so far as to unset
> > all the variables of a package before redefining it entirely.
> >
> > Hiding local changes away in an override actually is a missed opportunity
> > to notice these changes with actual merge (or rebase) conflicts, instead
> > leading to issues much later down the road, which means a lot of time
> > lost. That would not show semantic conflicts, true, but actual
> > code-level conflicts would show, and they would probably be the most
> > common.
> >
> > So, I am afraid that these overrides are a poor excuse for not wanting to
> > do actual VCS work.
>
> Well, you are the one who introduced BR2_EXTERNAL in the first place!
> And what we can put today in a BR2_EXTERNAL is in fact what is the
> easiest to maintain using a VCS inside the Buildroot tree: new
> defconfigs, new packages, board/ artefacts. These are always new files,
> they will never conflict in any way when rebasing, etc. And still, we
> agreed on adding BR2_EXTERNAL, even if at the time there was the same
> concern: why don't people simply use the capabilities of their VCS.
>
> > Third, since the override is outside the Buildroot tree (if it were in,
> > there would be no point in the override to begin with), it means it is
> > easy to miss it in critical times. Some packages in Buildroot are
> > licensed under copyleft licenses (GPL, LGPL...), and those have
> > requirements reading something along the lines of "complete source code
> > means [...], plus the scripts used to control compilation and
> > installation of the executable." Scripts which happens to be Buildroot
> > in our case, and the stuff in your override.
> >
> > Since the override would live in a separate tree with private, non
> > public stuff, it would be very easy to miss it when coming into
> > compliance for such packages, as one would be tempted to only distribute
> > their Buildroot tree.
>
> The problem already exists today with BR2_EXTERNAL: one can easily mess
> up and create a BR2_EXTERNAL with a mix of GPL-licensed packages and
> proprietary-licensed packages, and forget to redistribute the source
> code of the BR2_EXTERNAL. So that doesn't really seem like a good
> argument against Jeremy's patch: the problem already exists today, and
> we can't avoid it. Jeremy's patch doesn't really make it better or
> worse.
>
> A reasonably sane person would probably use two BR2_EXTERNAL: a first
> one with the additional GPL-licensed packages + the package overrides
> to existing Buildroot packages, and a second one with all the
> proprietary/product-specific bits. The first BR2_EXTERNAL would be
> published, not the second one.
>
> > In conclusion, my position on this topic is pretty straightforward: if
> > you want to modify the package recipes, then do so, and do so your the
> > Buildroot tree. Use a branch that contains your changes, and when you
> > need, merge a new master (or stable, or LTS) into your branch.
>
> So we can remove the BR2_EXTERNAL feature ? :-)
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Regards,
Vadym Kochan

[Buildroot] [PATCH 1/1] Include makefiles from GLOBAL_PATCH_DIR

[Yann E. MORIN]

Thomas, All,

On 2019-11-06 23:31 +0100, Thomas Petazzoni spake thusly:
> On Wed, 6 Nov 2019 17:37:48 +0100
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
[--SNIP--]
> > So, I am afraid that these overrides are a poor excuse for not wanting to
> > do actual VCS work.
> Well, you are the one who introduced BR2_EXTERNAL in the first place!

Ahem... git show a4239f7fd10 ;-)

I did introduce support for multi br2-external, though.

Anyway, this patch, as on IRC by J?r?my, has a designe flaw: since the
override files are included too early, they hcamge the way pkgdir is
computed, and this breaks completely.

This can be corrected, though, and it might even not be too complex to
do.

But if we're going to do the override stuff, I think we must think it
thouroughly, and not use an half-baked solution.

If at all, I'd favour looking at the aternate solution that was floated
around during the last devdays: postpone evaluation of the infras to
after all the packages hav actually been scanned. This would even allow
overriding the type of a package (e.g. autotools-package instead of
meson-package, in case the version was changed to use another
buildsystem).

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/1] Include makefiles from GLOBAL_PATCH_DIR

[Yann E. MORIN]

All,

On 2019-11-07 19:09 +0100, Yann E. MORIN spake thusly:
> On 2019-11-06 23:31 +0100, Thomas Petazzoni spake thusly:
> > On Wed, 6 Nov 2019 17:37:48 +0100
> > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> [--SNIP--]
> > > So, I am afraid that these overrides are a poor excuse for not wanting to
> > > do actual VCS work.
> > Well, you are the one who introduced BR2_EXTERNAL in the first place!
> 
> Ahem... git show a4239f7fd10 ;-)
> 
> I did introduce support for multi br2-external, though.
> 
> Anyway, this patch, as on IRC by J?r?my, has a designe flaw: since the

*as discussed on IRC with ... a design flaw...

> override files are included too early, they hcamge the way pkgdir is

* change

> computed, and this breaks completely.
> 
> This can be corrected, though, and it might even not be too complex to
> do.
> 
> But if we're going to do the override stuff, I think we must think it
> thouroughly, and not use an half-baked solution.
> 
> If at all, I'd favour looking at the aternate solution that was floated
> around during the last devdays: postpone evaluation of the infras to
> after all the packages hav actually been scanned. This would even allow
> overriding the type of a package (e.g. autotools-package instead of
> meson-package, in case the version was changed to use another
> buildsystem).

... and thus we could drop the check that a package is not redefined,
and so a package could be redefined in a br2-external tree, and that
would not need any new override mechanism at all, presumably.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/1] Include makefiles from GLOBAL_PATCH_DIR

[Jérémy ROSEN]

Yes I was about to post that

for units that have both a target and a host  mode, the .mk is included
twice.

This is rather easy to fix (probably just  add a flag to avoid double
include) but since the patch is still being discussed on a philosophical
ground, I'll wait before submitting a V2

Le jeu. 7 nov. 2019 ? 19:15, Yann E. MORIN <yann.morin.1998@free.fr> a
?crit :

> All,
>
> On 2019-11-07 19:09 +0100, Yann E. MORIN spake thusly:
> > On 2019-11-06 23:31 +0100, Thomas Petazzoni spake thusly:
> > > On Wed, 6 Nov 2019 17:37:48 +0100
> > > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> > [--SNIP--]
> > > > So, I am afraid that these overrides are a poor excuse for not
> wanting to
> > > > do actual VCS work.
> > > Well, you are the one who introduced BR2_EXTERNAL in the first place!
> >
> > Ahem... git show a4239f7fd10 ;-)
> >
> > I did introduce support for multi br2-external, though.
> >
> > Anyway, this patch, as on IRC by J?r?my, has a designe flaw: since the
>
> *as discussed on IRC with ... a design flaw...
>
> > override files are included too early, they hcamge the way pkgdir is
>
> * change
>
> > computed, and this breaks completely.
> >
> > This can be corrected, though, and it might even not be too complex to
> > do.
> >
> > But if we're going to do the override stuff, I think we must think it
> > thouroughly, and not use an half-baked solution.
> >
> > If at all, I'd favour looking at the aternate solution that was floated
> > around during the last devdays: postpone evaluation of the infras to
> > after all the packages hav actually been scanned. This would even allow
> > overriding the type of a package (e.g. autotools-package instead of
> > meson-package, in case the version was changed to use another
> > buildsystem).
>
> ... and thus we could drop the check that a package is not redefined,
> and so a package could be redefined in a br2-external tree, and that
> would not need any new override mechanism at all, presumably.
>
> Regards,
> Yann E. MORIN.
>
> --
>
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
> conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___
>      |
> | +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is
> no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v
>  conspiracy.  |
>
> '------------------------------^-------^------------------^--------------------'
>


-- 
[image: SMILE]  <http://www.smile.eu/>

20 rue des Jardins
92600 Asni?res-sur-Seine
*J?r?my ROSEN*
Architecte technique

[image: email] jeremy.rosen at smile.fr
[image: phone]  +33 6 88 25 87 42
[image: url] http://www.smile.eu

[image: Twitter] <https://twitter.com/GroupeSmile> [image: Facebook]
<https://www.facebook.com/smileopensource> [image: LinkedIn]
<https://www.linkedin.com/company/smile> [image: Github]
<https://github.com/Smile-SA>

[image: D?couvrez l?univers Smile, rendez-vous sur smile.eu]
<https://www.smile.eu/fr/publications/livres-blancs/yocto?utm_source=signature&utm_medium=email&utm_campaign=signature>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20191107/804b85af/attachment.html>