* [Buildroot] [PATCHv5 0/2] Detect incorrect dependencies @ 2015-10-09 9:15 Thomas Petazzoni 2015-10-09 9:15 ` [Buildroot] [PATCHv5 1/2] pkg-generic: detect incorrectly used package Thomas Petazzoni 2015-10-09 9:15 ` [Buildroot] [PATCHv5 2/2] pkg-generic: improve incorrectly used package detection Thomas Petazzoni 0 siblings, 2 replies; 6+ messages in thread From: Thomas Petazzoni @ 2015-10-09 9:15 UTC (permalink / raw) To: buildroot Hello, This is the fifth version of the small patch series to detect incorrect dependencies (i.e a package that depends on another in its .mk file, but that forgets to do so in its Config.in). Compared to the previous version, this new version: - Use $($(PKG)_TYPE) and $($(PKG)_KCONFIG)_VAR) instead of target-specific variables, as suggested by Arnout. - Move the "culprit listing" logic into a separate patch, so that it can be considered separately, suggested by Yann E. Morin. - Makes some other trivial adjustements suggested by Yann and Arnout. - Is rebased on top of master. Thanks, Thomas Thomas Petazzoni (2): pkg-generic: detect incorrectly used package pkg-generic: improve incorrectly used package detection package/pkg-generic.mk | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) -- 2.6.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCHv5 1/2] pkg-generic: detect incorrectly used package 2015-10-09 9:15 [Buildroot] [PATCHv5 0/2] Detect incorrect dependencies Thomas Petazzoni @ 2015-10-09 9:15 ` Thomas Petazzoni 2015-12-29 18:19 ` Yann E. MORIN 2015-12-29 18:36 ` Thomas Petazzoni 2015-10-09 9:15 ` [Buildroot] [PATCHv5 2/2] pkg-generic: improve incorrectly used package detection Thomas Petazzoni 1 sibling, 2 replies; 6+ messages in thread From: Thomas Petazzoni @ 2015-10-09 9:15 UTC (permalink / raw) To: buildroot In Buildroot, the selection of a package from a Config.in level and from a Makefile level are completely disconnected. This can lead to issues where the build of a package is triggered at the Makefile level due to the package being listed in another package <pkg>_DEPENDENCIES variable, even if that package is not enabled in the configuration. This has for example been the case recently with python-can having 'python' in its <pkg>_DEPENDENCIES, while python-can could be enabled when Python 3.x is used, in which case the 'python' package should not be built. To detect such issues more easily, this patch adds a check in the package infrastructure. When the build process of a package is being triggered, we verify that the package is enabled in the configuration. We do this check in the "configure" step, since this step is the first common step between the normal download case and the "local site method" / "package override" case. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- Changes since v4: - Use $($(PKG)_KCONFIG_VAR) and $($(PKG)_TYPE) as suggested by Arnout. - Split the "potential culprit" mechanism into a separate commit, as suggested by Yann. Changes since v3: - Add the DEPENDENT_OF mechanism to display which packages are mistakenly depending on a package without selecting it. Suggested and implement by Yann E. Morin. Changes since v2: - Only do the check if MAKECMDGOALS is empty, i.e if a "default" build is being done. This allows advanced users to continue doing "make <pkg>" to forcefully build a package even if not enabled in the configuration. Suggested by Peter Korsgaard. - Add @ in front of the test command so that it doesn't get displayed. Suggested by Peter Korsgaard. - Improve error message, as suggested by Peter Korsgaard. Changes since v1: - Use KCONFIG_VAR in order to make the thing work for toolchain packages, bootloaders and Linux. Issue reported by Vicente. --- package/pkg-generic.mk | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk index 76ec295..a831199 100644 --- a/package/pkg-generic.mk +++ b/package/pkg-generic.mk @@ -143,6 +143,16 @@ $(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 -z "$($($(PKG)_KCONFIG_VAR))" ; then \ + echo "ERROR: A package must have added $($(PKG)_NAME) to its _DEPENDENCIES line but" ; \ + echo "forgot to add the corresponding select / depends on $($(PKG)_KCONFIG_VAR)." ; \ + exit 1 ; \ + fi +endif @$(call step_start,configure) @$(call MESSAGE,"Configuring") $(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep)) -- 2.6.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [PATCHv5 1/2] pkg-generic: detect incorrectly used package 2015-10-09 9:15 ` [Buildroot] [PATCHv5 1/2] pkg-generic: detect incorrectly used package Thomas Petazzoni @ 2015-12-29 18:19 ` Yann E. MORIN 2015-12-29 18:36 ` Thomas Petazzoni 1 sibling, 0 replies; 6+ messages in thread From: Yann E. MORIN @ 2015-12-29 18:19 UTC (permalink / raw) To: buildroot Thomas, All, On 2015-10-09 11:15 +0200, Thomas Petazzoni spake thusly: > In Buildroot, the selection of a package from a Config.in level and > from a Makefile level are completely disconnected. This can lead to > issues where the build of a package is triggered at the Makefile level > due to the package being listed in another package <pkg>_DEPENDENCIES > variable, even if that package is not enabled in the configuration. > > This has for example been the case recently with python-can having > 'python' in its <pkg>_DEPENDENCIES, while python-can could be enabled > when Python 3.x is used, in which case the 'python' package should not > be built. > > To detect such issues more easily, this patch adds a check in the > package infrastructure. When the build process of a package is being > triggered, we verify that the package is enabled in the > configuration. We do this check in the "configure" step, since this > step is the first common step between the normal download case and the > "local site method" / "package override" case. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- [--SNIP--] > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index 76ec295..a831199 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -143,6 +143,16 @@ $(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 -z "$($($(PKG)_KCONFIG_VAR))" ; then \ > + echo "ERROR: A package must have added $($(PKG)_NAME) to its _DEPENDENCIES line but" ; \ As I reported in the second patch of this series, the package being build might second-or-deeper in the dependency chain. So, if we have the existing _correct_ dependencies: foo -> bar bar -> buz buz -> daz and we then add an incorrect dependency; alpha -> foo then the build order guranatees that daz is built first, and the we report that something is incorrectly selecting daz, while the reall issue is that something is incorrectly selecting foo. So, we'd probably need to state something like: ERROR: $(1) is in the dependency chain of a package that has added it to is _DEPENDENCIES line (directly or indirectly) without selecting it from Config.in. I'm not too happy with that phrasing, because it is still obscur for a new-comer. But we would instantly recognise the pattern and it would be relatively easy to spot the real culprit. Thoughts? Regards, Yann E. MORIN. > + echo "forgot to add the corresponding select / depends on $($(PKG)_KCONFIG_VAR)." ; \ > + exit 1 ; \ > + fi > +endif > @$(call step_start,configure) > @$(call MESSAGE,"Configuring") > $(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep)) > -- > 2.6.1 > > _______________________________________________ > 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] [PATCHv5 1/2] pkg-generic: detect incorrectly used package 2015-10-09 9:15 ` [Buildroot] [PATCHv5 1/2] pkg-generic: detect incorrectly used package Thomas Petazzoni 2015-12-29 18:19 ` Yann E. MORIN @ 2015-12-29 18:36 ` Thomas Petazzoni 1 sibling, 0 replies; 6+ messages in thread From: Thomas Petazzoni @ 2015-12-29 18:36 UTC (permalink / raw) To: buildroot Hello, On Fri, 9 Oct 2015 11:15:05 +0200, Thomas Petazzoni wrote: > In Buildroot, the selection of a package from a Config.in level and > from a Makefile level are completely disconnected. This can lead to > issues where the build of a package is triggered at the Makefile level > due to the package being listed in another package <pkg>_DEPENDENCIES > variable, even if that package is not enabled in the configuration. > > This has for example been the case recently with python-can having > 'python' in its <pkg>_DEPENDENCIES, while python-can could be enabled > when Python 3.x is used, in which case the 'python' package should not > be built. > > To detect such issues more easily, this patch adds a check in the > package infrastructure. When the build process of a package is being > triggered, we verify that the package is enabled in the > configuration. We do this check in the "configure" step, since this > step is the first common step between the normal download case and the > "local site method" / "package override" case. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > Changes since v4: > - Use $($(PKG)_KCONFIG_VAR) and $($(PKG)_TYPE) as suggested by Arnout. > - Split the "potential culprit" mechanism into a separate commit, as > suggested by Yann. I've applied, after taking into account the comment from Yann (and fixing a minor issue with it: $(1) cannot be used, it should have been $($(PKG)_NAME)). Thanks! Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCHv5 2/2] pkg-generic: improve incorrectly used package detection 2015-10-09 9:15 [Buildroot] [PATCHv5 0/2] Detect incorrect dependencies Thomas Petazzoni 2015-10-09 9:15 ` [Buildroot] [PATCHv5 1/2] pkg-generic: detect incorrectly used package Thomas Petazzoni @ 2015-10-09 9:15 ` Thomas Petazzoni 2015-12-29 18:06 ` Yann E. MORIN 1 sibling, 1 reply; 6+ messages in thread From: Thomas Petazzoni @ 2015-10-09 9:15 UTC (permalink / raw) To: buildroot 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 register the reverse dependencies of each package in a variable called <pkg>_DEPENDENT_OF, and display this variable for the problematic package when the error is detected. Many thanks to Yann E. Morin for the idea and implementation! Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- package/pkg-generic.mk | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk index a831199..1266a47 100644 --- a/package/pkg-generic.mk +++ b/package/pkg-generic.mk @@ -150,6 +150,8 @@ ifeq ($(MAKECMDGOALS),) @if test "$($(PKG)_TYPE)" = "target" -a -z "$($($(PKG)_KCONFIG_VAR))" ; then \ echo "ERROR: A package must have added $($(PKG)_NAME) to its _DEPENDENCIES line but" ; \ echo "forgot to add the corresponding select / depends on $($(PKG)_KCONFIG_VAR)." ; \ + echo "Potential culprits:" ; \ + for p in $($(PKG)_DEPENDENT_OF) ; do echo " - $$p" ; done ; \ exit 1 ; \ fi endif @@ -777,6 +779,12 @@ endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),) # configuration ifeq ($$($$($(2)_KCONFIG_VAR)),y) +# 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))) + # Ensure the calling package is the declared provider for all the virtual # packages it claims to be an implementation of. ifneq ($$($(2)_PROVIDES),) -- 2.6.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [PATCHv5 2/2] pkg-generic: improve incorrectly used package detection 2015-10-09 9:15 ` [Buildroot] [PATCHv5 2/2] pkg-generic: improve incorrectly used package detection Thomas Petazzoni @ 2015-12-29 18:06 ` Yann E. MORIN 0 siblings, 0 replies; 6+ messages in thread From: Yann E. MORIN @ 2015-12-29 18:06 UTC (permalink / raw) To: buildroot Thomas, All, On 2015-10-09 11:15 +0200, Thomas Petazzoni spake thusly: > 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 register the reverse dependencies of each package in a > variable called <pkg>_DEPENDENT_OF, and display this variable for the > problematic package when the error is detected. Many thanks to Yann > E. Morin for the idea and implementation! > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > package/pkg-generic.mk | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index a831199..1266a47 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -150,6 +150,8 @@ ifeq ($(MAKECMDGOALS),) > @if test "$($(PKG)_TYPE)" = "target" -a -z "$($($(PKG)_KCONFIG_VAR))" ; then \ > echo "ERROR: A package must have added $($(PKG)_NAME) to its _DEPENDENCIES line but" ; \ > echo "forgot to add the corresponding select / depends on $($(PKG)_KCONFIG_VAR)." ; \ > + echo "Potential culprits:" ; \ > + for p in $($(PKG)_DEPENDENT_OF) ; do echo " - $$p" ; done ; \ This does only work for first-level culprits. For example, I added; BUSYBOX_DEPENDENCIES += exim And I get: >>> berkeleydb 5.3.28 Patching libtool ERROR: A package must have added berkeleydb to its _DEPENDENCIES line but forgot to add the corresponding select / depends on BR2_PACKAGE_BERKELEYDB. Potential culprits: make[1]: *** [.../berkeleydb-5.3.28/.stamp_configured] Error 1 make: *** [_all] Error 2 That's because the real culprit is not directly the cause of the berkeleydb dependency; the depenency chain is two-or-more levels deep. So, NAK on this patch (even though that was my idea originally, IIRC). I'll (later!) to find a solution... Regards, Yann E. MORIN. > exit 1 ; \ > fi > endif > @@ -777,6 +779,12 @@ endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),) > # configuration > ifeq ($$($$($(2)_KCONFIG_VAR)),y) > > +# 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))) > + > # Ensure the calling package is the declared provider for all the virtual > # packages it claims to be an implementation of. > ifneq ($$($(2)_PROVIDES),) > -- > 2.6.1 > > _______________________________________________ > 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
end of thread, other threads:[~2015-12-29 18:36 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-10-09 9:15 [Buildroot] [PATCHv5 0/2] Detect incorrect dependencies Thomas Petazzoni 2015-10-09 9:15 ` [Buildroot] [PATCHv5 1/2] pkg-generic: detect incorrectly used package Thomas Petazzoni 2015-12-29 18:19 ` Yann E. MORIN 2015-12-29 18:36 ` Thomas Petazzoni 2015-10-09 9:15 ` [Buildroot] [PATCHv5 2/2] pkg-generic: improve incorrectly used package detection Thomas Petazzoni 2015-12-29 18:06 ` 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