Message ID | 1398105498-7055-1-git-send-email-thomas.petazzoni@free-electrons.com |
---|---|
State | Rejected |
Headers | show |
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. >
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 + +$(2)_POST_PATCH_HOOKS += CHECK_PKG_CONFIG_HOOK + # # Build step. Only define it if not already defined by the package .mk # file.