* [Buildroot] [RFC] pkg-autotools: check if host-pkgconf should be part of the dependencies
2014-04-21 18:38 [Buildroot] [RFC] pkg-autotools: check if host-pkgconf should be part of the dependencies Thomas Petazzoni
@ 2014-04-21 20:16 ` Yann E. MORIN
2014-04-21 20:29 ` Thomas Petazzoni
2014-04-22 16:33 ` Arnout Vandecappelle
1 sibling, 1 reply; 4+ messages in thread
From: Yann E. MORIN @ 2014-04-21 20:16 UTC (permalink / raw)
To: buildroot
Thomas, All,
On 2014-04-21 20:38 +0200, Thomas Petazzoni spake thusly:
> There is a good number of autotools-based packages that use the
> PKG_CHECK_MODULES() in their configure.{ac,in} file, and the presence
> of this macro indicates that the package should depend on
> host-pkgconf. However, we very often fail at adding this dependency,
> and discover later that it is necessary.
>
> In order to prevent that from happening, this commit proposes to add a
> post-patch hook that looks if PKG_CHECK_MODULES is used in the
> configure.{ac,in} file, and if it is, it verifies that host-pkgconf is
> part of the current package dependencies. If not, it aborts the build
> with an error message.
>
> Note that adding this dependency cannot be done automatically, because
> by the time the makefiles are parsed, the source code for the packages
> are not extracted, so we can't look at configure.{in,ac} to
> automatically add the host-pkgconf dependency. The only thing we can
> do is what this patch does: a check during the build itself.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> This is purely an RFC patch, I just tested it on one package (the
> recently added 'smack' package), which was lacking this host-pkgconf
> dependency.
Well, I rather like it, except for a little nit, see below.
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> package/pkg-autotools.mk | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/package/pkg-autotools.mk b/package/pkg-autotools.mk
> index a646612..842a7c3 100644
> --- a/package/pkg-autotools.mk
> +++ b/package/pkg-autotools.mk
> @@ -230,6 +230,17 @@ $(2)_PRE_CONFIGURE_HOOKS += AUTORECONF_HOOK
> $(2)_DEPENDENCIES += host-automake host-autoconf host-libtool
> endif
>
> +define CHECK_PKG_CONFIG_HOOK
> + $(Q)if grep -q PKG_CHECK_MODULES $$($$(PKG)_SRCDIR)/configure.{ac,in}; then \
I know we do not support building on anything but a GNU userland.
However, 'grep -q' is not portable. Can we just use the more poprtable
constrcut 'grep ... >/dev/null 2>&1' instead?
Regards,
Yann E. MORIN.
> + if test -z "$$(filter host-pkgconf,$$($$(PKG)_DEPENDENCIES))" ; then \
> + echo "ERROR: package $$(PKG) uses PKG_CHECK_MODULES but does not depend on host-pkgconf" ; \
> + exit 1 ; \
> + fi ; \
> + fi
> +endef
> +
> +$(2)_POST_PATCH_HOOKS += CHECK_PKG_CONFIG_HOOK
> +
> #
> # Build step. Only define it if not already defined by the package .mk
> # file.
> --
> 1.9.2
>
> _______________________________________________
> 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] 4+ messages in thread* [Buildroot] [RFC] pkg-autotools: check if host-pkgconf should be part of the dependencies
2014-04-21 18:38 [Buildroot] [RFC] pkg-autotools: check if host-pkgconf should be part of the dependencies Thomas Petazzoni
2014-04-21 20:16 ` Yann E. MORIN
@ 2014-04-22 16:33 ` Arnout Vandecappelle
1 sibling, 0 replies; 4+ messages in thread
From: Arnout Vandecappelle @ 2014-04-22 16:33 UTC (permalink / raw)
To: buildroot
On 21/04/14 20:38, Thomas Petazzoni wrote:
> There is a good number of autotools-based packages that use the
> PKG_CHECK_MODULES() in their configure.{ac,in} file, and the presence
> of this macro indicates that the package should depend on
> host-pkgconf. However, we very often fail at adding this dependency,
> and discover later that it is necessary.
>
> In order to prevent that from happening, this commit proposes to add a
> post-patch hook that looks if PKG_CHECK_MODULES is used in the
> configure.{ac,in} file, and if it is, it verifies that host-pkgconf is
> part of the current package dependencies. If not, it aborts the build
> with an error message.
Instead of post-patch, I think that pre-configure is more appropriate.
Also, perhaps this could be done as a BR2_INSTRUMENTATION_SCRIPTS
instead of a hard-coded check. The autobuilders would obviously have to
enable that script. It's nicer to have such tests of buildroot itself
isolated from the user's build. We can probably invent a lot more checks
like that, which may potentially have an important impact on build time.
That said, it's a lot more difficult as an instrumentation script
because then you don't have access to the make variables. You'd have to
run a recursive 'make printvars' to get them.
>
> Note that adding this dependency cannot be done automatically, because
> by the time the makefiles are parsed, the source code for the packages
> are not extracted, so we can't look at configure.{in,ac} to
> automatically add the host-pkgconf dependency. The only thing we can
> do is what this patch does: a check during the build itself.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> This is purely an RFC patch, I just tested it on one package (the
> recently added 'smack' package), which was lacking this host-pkgconf
> dependency.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> package/pkg-autotools.mk | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/package/pkg-autotools.mk b/package/pkg-autotools.mk
> index a646612..842a7c3 100644
> --- a/package/pkg-autotools.mk
> +++ b/package/pkg-autotools.mk
> @@ -230,6 +230,17 @@ $(2)_PRE_CONFIGURE_HOOKS += AUTORECONF_HOOK
> $(2)_DEPENDENCIES += host-automake host-autoconf host-libtool
> endif
>
> +define CHECK_PKG_CONFIG_HOOK
> + $(Q)if grep -q PKG_CHECK_MODULES $$($$(PKG)_SRCDIR)/configure.{ac,in}; then \
> + if test -z "$$(filter host-pkgconf,$$($$(PKG)_DEPENDENCIES))" ; then \
> + echo "ERROR: package $$(PKG) uses PKG_CHECK_MODULES but does not depend on host-pkgconf" ; \
> + exit 1 ; \
> + fi ; \
> + fi
> +endef
If defined like this, the CHECK_PKG_CONFIG_HOOK variable will be
redefined for every package... So I think it's better to move it outside
of the inner-generic-package macro (removing the double dollars).
Yes, the same is true of UPDATE_CONFIG_HOOK, AUTORECONF_HOOK and
LIBTOOL_PATCH_HOOK - but that's historical accident :-)
Finally, perhaps it would be better to search for 'pkg-config' in the
configure script itself, rather than configure.{ac,in}. The
PKG_CHECK_MODULES may be hidden in one of the m4 files, or pkg-config may
be used by some custom code instead. Obviously, that means it must be
done after AUTORECONF_HOOK so it _must_ be in a pre-configure hook (not
post-patch and not instrumentation).
Regards,
Arnout
> +
> +$(2)_POST_PATCH_HOOKS += CHECK_PKG_CONFIG_HOOK
> +
> #
> # Build step. Only define it if not already defined by the package .mk
> # file.
>
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
^ permalink raw reply [flat|nested] 4+ messages in thread