| Message ID | 20191026145151.17749-1-jeremy.rosen@smile.fr |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] Include makefiles from GLOBAL_PATCH_DIR | expand |
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.
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. | > > '------------------------------^-------^------------------^--------------------' >
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
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@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot Regards, Vadym Kochan
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.
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.
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. | > > '------------------------------^-------^------------------^--------------------' >
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
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(-)