Patchwork unionfs: add missing dependency on host-pkg-config

login
register
mail settings
Submitter Arnout Vandecappelle
Date Sept. 21, 2012, 10:45 a.m.
Message ID <1348224356-20104-1-git-send-email-arnout@mind.be>
Download mbox | patch
Permalink /patch/185680/
State Accepted
Commit e1aee0638f5a3657f27f5de18a33b2c0a15d839c
Headers show

Comments

Arnout Vandecappelle - Sept. 21, 2012, 10:45 a.m.
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
How was this never caught by the autobuilders?  Do they happen to
build host-pkg-config most of the time?  If so, maybe we should make
the randpackageconfig select a smaller number of packages to increase
the chance of finding this kind of issue...

 package/unionfs/unionfs.mk |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Gustavo Zacarias - Sept. 21, 2012, 11:03 a.m.
On 09/21/12 07:45, Arnout Vandecappelle (Essensium/Mind) wrote:

> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Acked-by: Gustavo Zacarias <gustavo@zacarias.com.ar>

> How was this never caught by the autobuilders?  Do they happen to
> build host-pkg-config most of the time?  If so, maybe we should make
> the randpackageconfig select a smaller number of packages to increase
> the chance of finding this kind of issue...

Because it uses any pkg-config around, it doesn't honour PKG_CONFIG nor
check for it as a dependency.

Makefile:CPPFLAGS += $(shell pkg-config --cflags fuse)
Makefile:LIB = $(shell pkg-config --libs fuse)

As long as there's one on PATH it'll just work.
Regards.
Arnout Vandecappelle - Sept. 21, 2012, 11:14 a.m.
On 09/21/12 13:03, Gustavo Zacarias wrote:
> On 09/21/12 07:45, Arnout Vandecappelle (Essensium/Mind) wrote:
>
>> Signed-off-by: Arnout Vandecappelle (Essensium/Mind)<arnout@mind.be>
> Acked-by: Gustavo Zacarias<gustavo@zacarias.com.ar>
>
>> How was this never caught by the autobuilders?  Do they happen to
>> build host-pkg-config most of the time?  If so, maybe we should make
>> the randpackageconfig select a smaller number of packages to increase
>> the chance of finding this kind of issue...
>
> Because it uses any pkg-config around, it doesn't honour PKG_CONFIG nor
> check for it as a dependency.
>
> Makefile:CPPFLAGS += $(shell pkg-config --cflags fuse)
> Makefile:LIB = $(shell pkg-config --libs fuse)
>
> As long as there's one on PATH it'll just work.

  No, unless libfuse-dev is installed on the autobuilders.  /usr/bin/pkg-config
doesn't look in $(STAGING_DIR)...

  Regards,
  Arnout
Gustavo Zacarias - Sept. 21, 2012, 11:15 a.m.
On 09/21/12 08:14, Arnout Vandecappelle wrote:
>  No, unless libfuse-dev is installed on the autobuilders. 
> /usr/bin/pkg-config
> doesn't look in $(STAGING_DIR)...

I'm on gentoo so -dev doesn't exist in my world, it's builtin :)
Regards.
Gustavo Zacarias - Sept. 21, 2012, 11:21 a.m.
On 09/21/12 08:14, Arnout Vandecappelle wrote:

>> Makefile:CPPFLAGS += $(shell pkg-config --cflags fuse)
>> Makefile:LIB = $(shell pkg-config --libs fuse)
>>
>> As long as there's one on PATH it'll just work.
> 
>  No, unless libfuse-dev is installed on the autobuilders. 
> /usr/bin/pkg-config
> doesn't look in $(STAGING_DIR)...

Also on that Makefile expression the result will fail with an error
message passing by (missing pkg-config or fuse.pc) but won't make make
(sic) bailout, try it...
Regards.
Arnout Vandecappelle - Sept. 21, 2012, 11:23 a.m.
On 09/21/12 13:21, Gustavo Zacarias wrote:
> On 09/21/12 08:14, Arnout Vandecappelle wrote:
>
>>> Makefile:CPPFLAGS += $(shell pkg-config --cflags fuse)
>>> Makefile:LIB = $(shell pkg-config --libs fuse)
>>>
>>> As long as there's one on PATH it'll just work.
>>
>>   No, unless libfuse-dev is installed on the autobuilders.
>> /usr/bin/pkg-config
>> doesn't look in $(STAGING_DIR)...
>
> Also on that Makefile expression the result will fail with an error
> message passing by (missing pkg-config or fuse.pc) but won't make make
> (sic) bailout, try it...

  But the final link will fail because of undefined fuse_xxx symbols,
because there is no -lfuse added by pkg-config.

  Regards,
  Arnout
Gustavo Zacarias - Sept. 21, 2012, 11:37 a.m.
On 09/21/12 08:23, Arnout Vandecappelle wrote:

>  But the final link will fail because of undefined fuse_xxx symbols,
> because there is no -lfuse added by pkg-config.

Yes, that's why i Acked your patch, it IS a problem.
It's just not the problem stated in the commit msg about the
autobuilders not detecting it (host-pkg-config almost always present,
that changes nothing as long as there's a pkg-config around) :)
Regards.
Thomas Petazzoni - Sept. 21, 2012, 12:02 p.m.
Dear Arnout Vandecappelle (Essensium/Mind),

On Fri, 21 Sep 2012 12:45:56 +0200, Arnout Vandecappelle
(Essensium/Mind) wrote:

> How was this never caught by the autobuilders?  Do they happen to
> build host-pkg-config most of the time?  If so, maybe we should make
> the randpackageconfig select a smaller number of packages to increase
> the chance of finding this kind of issue...

We can configure the probability for each given package to be selected
in randpackageconfig, and my autobuilder script itself make this value
a random value between (IIRC) 2 and 35%. So depending on the build,
each package has between 2% and 35% of chances of being selected. And I
certainly see builds with very few packages selected. So I'm also
surprised it wasn't found until now.

Thomas
Thomas Petazzoni - Sept. 21, 2012, 12:03 p.m.
Dear Gustavo Zacarias,

On Fri, 21 Sep 2012 08:03:05 -0300, Gustavo Zacarias wrote:

> Because it uses any pkg-config around, it doesn't honour PKG_CONFIG nor
> check for it as a dependency.

My autobuild stuff runs inside a chroot that does not have pkg-config
installed. The chroot in question has the strict minimal number of
dependencies that Buildroot requires on the host system.

Thomas
Arnout Vandecappelle - Sept. 21, 2012, 12:11 p.m.
On 09/21/12 13:37, Gustavo Zacarias wrote:
> host-pkg-config almost always present

  Maybe we should make that _always_ present, and add host-pkg-config to
the toolchain build...  It is more or less part of the toolchain, and it
doesn't hurt to build it if it's not needed.

  Regards,
  Arnout
Gustavo Zacarias - Sept. 21, 2012, 12:27 p.m.
On 09/21/12 09:11, Arnout Vandecappelle wrote:
>> host-pkg-config almost always present
> 
>  Maybe we should make that _always_ present, and add host-pkg-config to
> the toolchain build...  It is more or less part of the toolchain, and it
> doesn't hurt to build it if it's not needed.

Possible, host variant doesn't use anything that we don't have around
and it's pretty straightforward.
Also adding a helper script for new packages would be neat, at least to
check for the usual suspects, with some magic grepping of the unpacked
tarballs.
Regards.
Peter Korsgaard - Sept. 21, 2012, 2:11 p.m.
>>>>> "Arnout" == Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> writes:

 Arnout> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
 Arnout> ---
 Arnout> How was this never caught by the autobuilders?  Do they happen to
 Arnout> build host-pkg-config most of the time?  If so, maybe we should make
 Arnout> the randpackageconfig select a smaller number of packages to increase
 Arnout> the chance of finding this kind of issue...

Committed, thanks.

Lots of stuff indeed pulls in host-pkgconfig, and as 'u' is pretty late
in the alphabet the chance is quite high that it is already available:

ls package | cat -n | grep unionfs
   520	unionfs

ls package | wc -l
559

We do have KCONFIG_PROBABILITY (see 3435c1afb583) that I sometimes play
with, but you need both low and high probability builds as they catch
different issues.
Peter Korsgaard - Sept. 21, 2012, 2:20 p.m.
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

 Arnout> On 09/21/12 13:37, Gustavo Zacarias wrote:
 >> host-pkg-config almost always present

 Arnout>  Maybe we should make that _always_ present, and add
 Arnout> host-pkg-config to the toolchain build...  It is more or less
 Arnout> part of the toolchain, and it doesn't hurt to build it if it's
 Arnout> not needed.

That's an option, but it's fairly slow to build (mainly because of the
embedded glib), so people might not like it (especially if using an
external toolchain). Perhaps it would be better to move to pkgconf
instead then (I think Gentoo uses it)?

https://github.com/pkgconf/pkgconf
Gustavo Zacarias - Sept. 21, 2012, 2:26 p.m.
On 09/21/12 11:20, Peter Korsgaard wrote:

> That's an option, but it's fairly slow to build (mainly because of the
> embedded glib), so people might not like it (especially if using an
> external toolchain). Perhaps it would be better to move to pkgconf
> instead then (I think Gentoo uses it)?
> 
> https://github.com/pkgconf/pkgconf

It's not a default in Gentoo yet, though it's available.
Anybody run some tests switching to it?
Regards.
Lionel Orry - Sept. 21, 2012, 2:30 p.m.
On Fri, Sep 21, 2012 at 4:26 PM, Gustavo Zacarias
<gustavo@zacarias.com.ar> wrote:
> On 09/21/12 11:20, Peter Korsgaard wrote:
>
>> That's an option, but it's fairly slow to build (mainly because of the
>> embedded glib), so people might not like it (especially if using an
>> external toolchain). Perhaps it would be better to move to pkgconf
>> instead then (I think Gentoo uses it)?
>>
>> https://github.com/pkgconf/pkgconf
>
> It's not a default in Gentoo yet, though it's available.
> Anybody run some tests switching to it?
> Regards.

Funtoo Linux (gentoo-based) uses pkgconf as default now :
http://jira.funtoo.org/browse/FL-15
I think this switch is quite safe and you can trust Funtoo maintainers...

My 2 cents
Lionel

>
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Thomas Petazzoni - Sept. 21, 2012, 6:51 p.m.
Dear Arnout Vandecappelle,

On Fri, 21 Sep 2012 14:11:42 +0200, Arnout Vandecappelle wrote:

>   Maybe we should make that _always_ present, and add host-pkg-config to
> the toolchain build...  It is more or less part of the toolchain, and it
> doesn't hurt to build it if it's not needed.

host-pkg-config depends on host-automake, which depends on
host-autoconf and host-libtool, themselves needing host-m4.

Are we going to become OpenEmebdded, and rebuild the entire host
distribution before building anything useful for the target?

Sorry, but the fact that when building a Busybox system, Buildroot
builds Busybox and nothing else is a very good property of Buildroot.
Let's keep it this way, thanks :-)

Best regards,

Thomas
Thomas Petazzoni - Sept. 21, 2012, 6:52 p.m.
Dear Peter Korsgaard,

On Fri, 21 Sep 2012 16:11:12 +0200, Peter Korsgaard wrote:

> Lots of stuff indeed pulls in host-pkgconfig, and as 'u' is pretty late
> in the alphabet the chance is quite high that it is already available:

Can we somehow fight this alphabet thing and make things even more
random? Something that would shuffle the make targets or something, but
I don't know if it is possible.

Thomas
Peter Korsgaard - Sept. 22, 2012, 7:46 p.m.
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 Thomas> Dear Peter Korsgaard,
 Thomas> On Fri, 21 Sep 2012 16:11:12 +0200, Peter Korsgaard wrote:

 >> Lots of stuff indeed pulls in host-pkgconfig, and as 'u' is pretty late
 >> in the alphabet the chance is quite high that it is already available:

 Thomas> Can we somehow fight this alphabet thing and make things even more
 Thomas> random? Something that would shuffle the make targets or something, but
 Thomas> I don't know if it is possible.

I don't know of any easy/clean way, but it could probably be done by
some $(wildcard) / $(shell unsort ..) stuff instead of include
package/*/*.mk, but it won't be pretty.

Patch

diff --git a/package/unionfs/unionfs.mk b/package/unionfs/unionfs.mk
index e2ffd72..eb8109e 100644
--- a/package/unionfs/unionfs.mk
+++ b/package/unionfs/unionfs.mk
@@ -7,7 +7,7 @@ 
 UNIONFS_VERSION = 0.26
 UNIONFS_SITE = http://podgorny.cz/unionfs-fuse/releases
 UNIONFS_SOURCE = unionfs-fuse-$(UNIONFS_VERSION).tar.xz
-UNIONFS_DEPENDENCIES = libfuse
+UNIONFS_DEPENDENCIES = libfuse host-pkg-config
 UNIONFS_LICENSE = BSD-3c
 UNIONFS_LICENSE_FILES = LICENSE