Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Yann E. MORIN <yann.morin.1998@free.fr>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCHv5 1/2] pkg-generic: detect incorrectly used package
Date: Tue, 29 Dec 2015 19:19:16 +0100	[thread overview]
Message-ID: <20151229181916.GD3445@free.fr> (raw)
In-Reply-To: <1444382106-16019-2-git-send-email-thomas.petazzoni@free-electrons.com>

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.  |
'------------------------------^-------^------------------^--------------------'

  reply	other threads:[~2015-12-29 18:19 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151229181916.GD3445@free.fr \
    --to=yann.morin.1998@free.fr \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox