[Buildroot] [PATCH v2 1/2] package/waf: add a blind Config.in.host

[Buildroot] [PATCH v2 1/2] package/waf: add a blind Config.in.host

[Carlos Santos]

Quoting Thomas Petazzoni, the idea we have for the future is:

 * All host packages have a Config.in.host option.

 * The host packages that are only build dependencies of other packages
   have a blind Config.in.host option

 * The host packages that are useful by themselves continue to have a
   visible Config.in.host option.

host-waf gets a blind Config.in.host, because it exists only to build
Waf-based packages that set <PKG>_NEEDS_EXTERNAL_WAF to YES.

A help text is included to document the package, only, since it is not
shown in the configuration menu.

Signed-off-by: Carlos Santos <casantos@datacom.com.br>
---
Changes v1->v2:
  - Explain the motivation in the commit message
  - Make the Config.in.host blind
---
 package/Config.in.host     | 1 +
 package/waf/Config.in.host | 6 ++++++
 2 files changed, 7 insertions(+)
 create mode 100644 package/waf/Config.in.host

diff --git a/package/Config.in.host b/package/Config.in.host
index 16b474fc9d..3644436fe3 100644
--- a/package/Config.in.host
+++ b/package/Config.in.host
@@ -64,6 +64,7 @@ menu "Host utilities"
 	source "package/uboot-tools/Config.in.host"
 	source "package/util-linux/Config.in.host"
 	source "package/vboot-utils/Config.in.host"
+	source "package/waf/Config.in.host"
 	source "package/xorriso/Config.in.host"
 	source "package/zip/Config.in.host"
 	source "package/zstd/Config.in.host"
diff --git a/package/waf/Config.in.host b/package/waf/Config.in.host
new file mode 100644
index 0000000000..81a5c5cc27
--- /dev/null
+++ b/package/waf/Config.in.host
@@ -0,0 +1,6 @@
+config BR2_PACKAGE_HOST_WAF
+	bool
+	help
+	  WAF, the meta build system
+
+	  https://waf.io/
-- 
2.19.2

[Buildroot] [PATCH v2 2/2] [RFC] package/pkg-waf.mk: mimic selection of host-waf

[Carlos Santos]

Since host-waf's Config.in.host is blind, it is not possible to select
it in the configuration menu. There are two possible solutions for this
problem:

- Add "select BR2_PACKAGE_HOST_WAF" to package/mpv/Config.in (currently
  only mpv requires host-waf). Also, document that BR2_PACKAGE_HOST_WAF
  must be selected when <PKG>_NEEDS_EXTERNAL_WAF is YES.

- Set BR2_PACKAGE_HOST_WAF to 'y' in the waf-package macro. This one is
  simpler because it does not require changing anything else.

Signed-off-by: Carlos Santos <casantos@datacom.com.br>
---
 package/pkg-waf.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/pkg-waf.mk b/package/pkg-waf.mk
index 3288dd63a0..fe5995250c 100644
--- a/package/pkg-waf.mk
+++ b/package/pkg-waf.mk
@@ -43,6 +43,7 @@ $(2)_NEEDS_EXTERNAL_WAF ?= NO
 
 # If the package does not have its own waf, use our own.
 ifeq ($$($(2)_NEEDS_EXTERNAL_WAF),YES)
+BR2_PACKAGE_HOST_WAF = y
 $(2)_DEPENDENCIES += host-waf
 $(2)_WAF = $$(HOST_DIR)/bin/waf
 else
-- 
2.19.2

[Buildroot] [PATCH v2 2/2] [RFC] package/pkg-waf.mk: mimic selection of host-waf

[Thomas Petazzoni]

Hello,

On Thu, 20 Dec 2018 22:22:21 -0200, Carlos Santos wrote:
> Since host-waf's Config.in.host is blind, it is not possible to select
> it in the configuration menu. There are two possible solutions for this
> problem:
> 
> - Add "select BR2_PACKAGE_HOST_WAF" to package/mpv/Config.in (currently
>   only mpv requires host-waf). Also, document that BR2_PACKAGE_HOST_WAF
>   must be selected when <PKG>_NEEDS_EXTERNAL_WAF is YES.
> 
> - Set BR2_PACKAGE_HOST_WAF to 'y' in the waf-package macro. This one is
>   simpler because it does not require changing anything else.
> 
> Signed-off-by: Carlos Santos <casantos@datacom.com.br>

I understand the idea that doing this avoids repeating a "select
BR2_PACKAGE_HOST_WAF" in all packages using host-waf. However, I don't
think we want to go down the route of setting those variables from the
make logic. I don't have a very strong argument to defend this
position, but to me it looks more logical that we continue to use
kconfig-level "select" to enable those config options.

Best regards,

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

[Buildroot] [PATCH v2 2/2] [RFC] package/pkg-waf.mk: mimic selection of host-waf

[Carlos Santos]

> From: "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
> To: "Carlos Santos" <casantos@datacom.com.br>
> Cc: "buildroot" <buildroot@buildroot.org>, "Yann Morin" <yann.morin.1998@free.fr>
> Sent: Sexta-feira, 21 de dezembro de 2018 13:34:30
> Subject: Re: [Buildroot] [PATCH v2 2/2] [RFC] package/pkg-waf.mk: mimic selection of host-waf

> Hello,
> 
> On Thu, 20 Dec 2018 22:22:21 -0200, Carlos Santos wrote:
>> Since host-waf's Config.in.host is blind, it is not possible to select
>> it in the configuration menu. There are two possible solutions for this
>> problem:
>> 
>> - Add "select BR2_PACKAGE_HOST_WAF" to package/mpv/Config.in (currently
>>   only mpv requires host-waf). Also, document that BR2_PACKAGE_HOST_WAF
>>   must be selected when <PKG>_NEEDS_EXTERNAL_WAF is YES.
>> 
>> - Set BR2_PACKAGE_HOST_WAF to 'y' in the waf-package macro. This one is
>>   simpler because it does not require changing anything else.
>> 
>> Signed-off-by: Carlos Santos <casantos@datacom.com.br>
> 
> I understand the idea that doing this avoids repeating a "select
> BR2_PACKAGE_HOST_WAF" in all packages using host-waf. However, I don't
> think we want to go down the route of setting those variables from the
> make logic. I don't have a very strong argument to defend this
> position, but to me it looks more logical that we continue to use
> kconfig-level "select" to enable those config options.

Thanks for reviewing this.

So we need to make mpv select BR2_PACKAGE_HOST_WAF, right?

-- 
Carlos Santos (Casantos) - DATACOM, P&D

[Buildroot] [PATCH v2 2/2] [RFC] package/pkg-waf.mk: mimic selection of host-waf

[Thomas Petazzoni]

Hello,

On Fri, 21 Dec 2018 14:04:15 -0200 (BRST), Carlos Santos wrote:

> > I understand the idea that doing this avoids repeating a "select
> > BR2_PACKAGE_HOST_WAF" in all packages using host-waf. However, I don't
> > think we want to go down the route of setting those variables from the
> > make logic. I don't have a very strong argument to defend this
> > position, but to me it looks more logical that we continue to use
> > kconfig-level "select" to enable those config options.  
> 
> Thanks for reviewing this.
> 
> So we need to make mpv select BR2_PACKAGE_HOST_WAF, right?

Yes, all packages that set <pkg>_NEEDS_EXTERNAL_WAF would need to
select BR2_PACKAGE_HOST_WAF.

Long term, we would add a check like we have for the target packages
that verifies that if a target package is being built, its Config.in
option is enabled:

define CHECK_ONE_DEPENDENCY
ifeq ($$($(2)_TYPE),target)
ifeq ($$($(2)_IS_VIRTUAL),)
ifneq ($$($$($(2)_KCONFIG_VAR)),y)
$$(error $$($(2)_NAME) is in the dependency chain of $$($(1)_NAME) that \
has added it to its _DEPENDENCIES variable without selecting it or \
depending on it from Config.in)
endif
endif
endif
endef

$(foreach pkg,$(call UPPERCASE,$(PACKAGES)),\
        $(foreach dep,$(call UPPERCASE,$($(pkg)_FINAL_ALL_DEPENDENCIES)),\
                $(eval $(call CHECK_ONE_DEPENDENCY,$(pkg),$(dep))$(sep))))

endif

This will ensure that all host packages have a Config.in option, and
that it is properly selected in all situations. However, this obviously
requires that all host packages are converted, and all reverse
dependencies have the appropriate selects. This is going to take a long
time :-)

Best regards,

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