* [Buildroot] [PATCH] pkg-generic: improve incorrectly used package detection
@ 2016-01-01 0:01 Yann E. MORIN
2016-01-01 16:49 ` Romain Naour
2016-01-03 21:15 ` Peter Korsgaard
0 siblings, 2 replies; 6+ messages in thread
From: Yann E. MORIN @ 2016-01-01 0:01 UTC (permalink / raw)
To: buildroot
From: Thomas Petazzoni <thomas.petazzoni@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@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
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))
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Buildroot] [PATCH] pkg-generic: improve incorrectly used package detection
2016-01-01 0:01 [Buildroot] [PATCH] pkg-generic: improve incorrectly used package detection Yann E. MORIN
@ 2016-01-01 16:49 ` Romain Naour
2016-01-01 16:58 ` Yann E. MORIN
2016-01-03 21:15 ` Peter Korsgaard
1 sibling, 1 reply; 6+ messages in thread
From: Romain Naour @ 2016-01-01 16:49 UTC (permalink / raw)
To: buildroot
Hi Yann, All,
Le 01/01/2016 01:01, Yann E. MORIN a ?crit :
> From: Thomas Petazzoni <thomas.petazzoni@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@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@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))
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] pkg-generic: improve incorrectly used package detection
2016-01-01 16:49 ` Romain Naour
@ 2016-01-01 16:58 ` Yann E. MORIN
2016-01-01 17:01 ` Yann E. MORIN
0 siblings, 1 reply; 6+ messages in thread
From: Yann E. MORIN @ 2016-01-01 16:58 UTC (permalink / raw)
To: buildroot
Romain, All,
On 2016-01-01 17:49 +0100, Romain Naour spake thusly:
> Le 01/01/2016 01:01, Yann E. MORIN a ?crit :
> > From: Thomas Petazzoni <thomas.petazzoni@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@free.fr>
> > Cc: Thomas Petazzoni <thomas.petazzoni@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 ?
Yes, this patch is not good. I've sent an updated version:
https://patchwork.ozlabs.org/patch/561998/
This new version is entirely using Makefile code, so it should not
suffer from this limitation.
Regards,
Yann E. MORIN.
> 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))
> >
>
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 6+ messages in thread* [Buildroot] [PATCH] pkg-generic: improve incorrectly used package detection
2016-01-01 16:58 ` Yann E. MORIN
@ 2016-01-01 17:01 ` Yann E. MORIN
0 siblings, 0 replies; 6+ messages in thread
From: Yann E. MORIN @ 2016-01-01 17:01 UTC (permalink / raw)
To: buildroot
Romain, All,
On 2016-01-01 17:58 +0100, Yann E. MORIN spake thusly:
> On 2016-01-01 17:49 +0100, Romain Naour spake thusly:
> > Le 01/01/2016 01:01, Yann E. MORIN a ?crit :
> > > From: Thomas Petazzoni <thomas.petazzoni@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@free.fr>
> > > Cc: Thomas Petazzoni <thomas.petazzoni@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 ?
>
> Yes, this patch is not good. I've sent an updated version:
> https://patchwork.ozlabs.org/patch/561998/
The patch in this thread *is* the latest version I referred to above.
As discussed with Romain on IRC, he was in fact using the previous
version of that patch, not the one he replied to, hence the confusion.
Regards,
Yann E. MORIN.
> This new version is entirely using Makefile code, so it should not
> suffer from this limitation.
>
> Regards,
> Yann E. MORIN.
>
> > 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))
> > >
> >
>
> --
> .-----------------.--------------------.------------------.--------------------.
> | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
> | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
> '------------------------------^-------^------------------^--------------------'
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] pkg-generic: improve incorrectly used package detection
2016-01-01 0:01 [Buildroot] [PATCH] pkg-generic: improve incorrectly used package detection Yann E. MORIN
2016-01-01 16:49 ` Romain Naour
@ 2016-01-03 21:15 ` Peter Korsgaard
1 sibling, 0 replies; 6+ messages in thread
From: Peter Korsgaard @ 2016-01-03 21:15 UTC (permalink / raw)
To: buildroot
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
> From: Thomas Petazzoni <thomas.petazzoni@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@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Committed, thanks.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] pkg-generic: improve incorrectly used package detection
@ 2015-12-30 21:27 Yann E. MORIN
0 siblings, 0 replies; 6+ messages in thread
From: Yann E. MORIN @ 2015-12-30 21:27 UTC (permalink / raw)
To: buildroot
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
The package infrastructure now detects when a target package is being
built even if its corresponding Config.in option is not enabled, and
aborts with an error. However, it does not indicate *which* package is
improperly depending on the current package without selecting it at
the kconfig level.
So, in this commit, in addition to displaying an error, we try to help
the user by saying which packages could be the culprit.
To achieve this, we first register the reverse dependencies of each
package in a variable called <pkg>_DEPENDENT_OF.
Then, we add two macros:
- the first will recursively expand all the dependency chain of
potential cuplrits,
- the second will filter only enabled packages
Eventually, we display the list of potential culprits, which more often
than not will be a singleton.
We do the check at the configure time, because we can't do it while
parsing the packages. That's because, at the moment we scan the
problematic package, the offending package might not yet be parsed, so
it is not yet in the <pkg>_DEPENDENT_OF list.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
[yann.morin.1998 at free.fr:
- only dump enabled culprits
- allow dumping culprits that are two-level-or-deeper in the depenency
chain
]
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
package/pkg-generic.mk | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 9e88423..bc4e487 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -180,6 +180,17 @@ $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\
$(if $(wildcard $(dir)),,\
$(error BR2_GLOBAL_PATCH_DIR contains nonexistent directory $(dir))))
+define EXPAND_DEPENDENCY_CULPRITS
+ $(foreach c,$(1),\
+ $(c) \
+ $(call EXPAND_DEPENDENCY_CULPRITS,$($(call UPPERCASE,$(c))_DEPENDENT_OF)))
+endef
+
+define DEPENDENCY_CULPRITS
+ $(foreach c,$(call EXPAND_DEPENDENCY_CULPRITS,$(1)),\
+ $(if $($($(call UPPERCASE,$(c))_KCONFIG_VAR)),$(c)))
+endef
+
# Configure
$(BUILD_DIR)/%/.stamp_configured:
# Only trigger the check for default builds. If the user forces
@@ -193,6 +204,8 @@ ifeq ($(MAKECMDGOALS),)
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." ; \
+ echo "Potential culprits:" ; \
+ printf ' - %s\n' $(call DEPENDENCY_CULPRITS,$($(PKG)_DEPENDENT_OF)); \
exit 1 ; \
fi
endif
@@ -828,6 +841,12 @@ endif # other packages
endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
$$(foreach hook,$$($(2)_POST_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))
+# Store reverse build-dependency information: we add the name of the
+# current package to the <pkg>_DEPENDENT_OF variable of all packages
+# the current package depends on.
+$$(eval $$(foreach d,$$($(2)_FINAL_ALL_DEPENDENCIES),\
+ $$(call UPPERCASE,$$(d))_DEPENDENT_OF += $(1)$$(sep)))
+
# add package to the general list of targets if requested by the buildroot
# configuration
ifeq ($$($$($(2)_KCONFIG_VAR)),y)
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-03 21:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-01 0:01 [Buildroot] [PATCH] pkg-generic: improve incorrectly used package detection Yann E. MORIN
2016-01-01 16:49 ` Romain Naour
2016-01-01 16:58 ` Yann E. MORIN
2016-01-01 17:01 ` Yann E. MORIN
2016-01-03 21:15 ` Peter Korsgaard
-- strict thread matches above, loose matches on Subject: below --
2015-12-30 21:27 Yann E. MORIN
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox