[Buildroot] [PATCH] pkg-generic: improve incorrectly used package detection

[Romain Naour]

Hi Yann, All,

Le 01/01/2016 01:01, Yann E. MORIN a écrit :
> From: Thomas Petazzoni <thomas.petazzoni at free-electrons.com>
> 
> Currently, the check that packages we build are indeed enabled is done
> at the time a package is configured.
> 
> This can come quite late in the build process, and does not provide
> direct knowledge of the real culprit for the incorrect dependency.
> 
> However, we can improve these two issues quite easily, albeit at the
> expense of a very slightly more complicated make code.
> 
> First, the check can not be done at the time we define the package, i.e.
> in the inner-generic-pacakge, because all its dependencies might have
> not been parsed yet, so we can't yet know whether it is enabled or not
> (because we can't match the package name of the dependency to its
> Kconfig variable yet).
> 
> But then, we know we have all packages definitions after we scanned the
> the bundled packages, kernel, bootloaders and toolchains, as well as the
> br2-external tree (if any).
> 
> So, at this location, we iterate through the list of enabled packages,
> and check that the packages they each depend on are indeed enabled.
> 
> This allows us to:
>  1- do the check very early, before any build action,
>  2- report on the exact offending package very easily.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998 at free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni at free-electrons.com>

While testing this patch with some packages enabled (efl + enlightenment), I get:

make[1]: execvp: /bin/sh: Liste d'arguments trop longue
package/pkg-generic.mk:200: recipe for target
'output/build/skeleton-undefined/.stamp_configured' failed

Any idea ?

Best regards,
Romain
> 
> ---
> Changes v2 -> v3:  (Yann)
>   - completely drop the reverse dependency list
>   - drop the pre-configure check
>   - make it Makefile code
>   - print the offending package
>   - drop Thomas' SoB, as I rewrote it entirely and completely
>     differently
> 
> Changes v1 -> v2:  (Yann)
>   - recursively scan the reverse list of dependencies to find all the
>     potential cuplrits, not only the first-level ones
> ---
>  Makefile               | 28 ++++++++++++++++++++++++++++
>  package/pkg-generic.mk | 14 --------------
>  2 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 71ace68..9139e81 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -434,6 +434,34 @@ include fs/common.mk
>  
>  include $(BR2_EXTERNAL)/external.mk
>  
> +# Now we are sure we have all the packages scanned and defined. We now
> +# check for each package in the list of enabled packages, that all its
> +# dependencies are indeed enabled.
> +#
> +# Only trigger the check for default builds. If the user forces building
> +# a package, even if not enabled in the configuration, we want to accept
> +# it.
> +#
> +ifeq ($(MAKECMDGOALS),)
> +
> +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
> +
>  dirs: $(BUILD_DIR) $(STAGING_DIR) $(TARGET_DIR) \
>  	$(HOST_DIR) $(BINARIES_DIR)
>  
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 9e88423..1e024d3 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -182,20 +182,6 @@ $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\
>  
>  # Configure
>  $(BUILD_DIR)/%/.stamp_configured:
> -# Only trigger the check for default builds. If the user forces
> -# building a package, even if not enabled in the configuration, we
> -# want to accept it.
> -ifeq ($(MAKECMDGOALS),)
> -	@if test "$($(PKG)_TYPE)" = "target" \
> -		-a "$($(PKG)_IS_VIRTUAL)" != "YES" \
> -		-a -z "$($($(PKG)_KCONFIG_VAR))" ; \
> -	then \
> -		echo "ERROR: $($(PKG)_NAME) is in the dependency chain of a package that has" ; \
> -		echo "added it to its _DEPENDENCIES variable (directly or indirectly)" ; \
> -		echo "without selecting it from Config.in." ; \
> -		exit 1 ; \
> -	fi
> -endif
>  	@$(call step_start,configure)
>  	@$(call MESSAGE,"Configuring")
>  	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
>