[OE-core] [PATCH] e2fsprogs: add packageconfig for -file-

[Robert Yang]

On 09/19/2016 11:32 AM, Randy MacLeod wrote:
> On 2016-09-18 10:37 PM, Christopher Larson wrote:
>>
>> On Sun, Sep 18, 2016 at 7:32 PM, Randy MacLeod
>> <randy.macleod at windriver.com <mailto:randy.macleod at windriver.com>> wrote:
>>
>>     On 2016-09-17 01:09 AM, Christopher Larson wrote:
>>
>>
>>         On Fri, Sep 16, 2016 at 6:37 PM, Randy MacLeod
>>         <Randy.MacLeod at windriver.com
>>         <mailto:Randy.MacLeod at windriver.com>
>>         <mailto:Randy.MacLeod at windriver.com
>>         <mailto:Randy.MacLeod at windriver.com>>> wrote:
>>
>>             Without a packageconfig dependency for the file utility, there's
>>             a very rare compile faiure caused by a race where the magic.h
>>             header file is not found:
>>
>>              ../../../git/lib/support/plausible.c:33:19: fatal error:
>>         magic.h:
>>             No such file or directory
>>
>>             This file, plausible.c, is part of libsupport.a which is used by
>>             many binaries produced by the e2fsprogs package. plausible.c
>>         attempts
>>             to dynamically load libmagic.so if the e2fsprogs configure
>>         detects
>>             that magic was available. Adding the packageconfig will
>>         eliminate
>>             the build as well as the possible configure-time race condition.
>>
>>             Signed-off-by: Randy MacLeod <Randy.MacLeod at windriver.com
>>         <mailto:Randy.MacLeod at windriver.com>
>>             <mailto:Randy.MacLeod at windriver.com
>>         <mailto:Randy.MacLeod at windriver.com>>>
>>             ---
>>              meta/recipes-devtools/e2fsprogs/e2fsprogs_1.43.bb
>>         <http://e2fsprogs_1.43.bb>
>>             <http://e2fsprogs_1.43.bb> | 1 +
>>              1 file changed, 1 insertion(+)
>>
>>             diff --git
>>         a/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.43.bb
>>         <http://e2fsprogs_1.43.bb>
>>             <http://e2fsprogs_1.43.bb>
>>             b/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.43.bb
>>         <http://e2fsprogs_1.43.bb>
>>             <http://e2fsprogs_1.43.bb>
>>             index f4855bc..1707cb9 100644
>>             --- a/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.43.bb
>>         <http://e2fsprogs_1.43.bb>
>>             <http://e2fsprogs_1.43.bb>
>>             +++ b/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.43.bb
>>         <http://e2fsprogs_1.43.bb>
>>             <http://e2fsprogs_1.43.bb>
>>             @@ -23,6 +23,7 @@ EXTRA_OECONF += "--libdir=${base_libdir}
>>             --sbindir=${base_sbindir} \
>>              EXTRA_OECONF_darwin = "--libdir=${base_libdir}
>>             --sbindir=${base_sbindir} --enable-bsd-shlibs"
>>
>>              PACKAGECONFIG ??= ""
>>             +PACKAGECONFIG[file] = ',,file'
>>
>>
>>         This isn’t going to be good enough. You aren’t explicitly
>>         enabling/disabling the option, so it’ll still detect based on
>>         what’s in
>>         the sysroot, so recipe build order will result in non-deterministic
>>         behavior.
>>
>>
>>     Hi Christopher,
>>
>>     Thanks for the response but I don't understand.
>>     I'm probably wrong so please explain.
>>     It seems that you're suggesting that I add:
>>      --with[out]-magic to e2fsprogs configure, and while
>>     I could do that, it does not seem to be needed.
>>
>>
>>     Short response:
>>
>>       This change forces 'file' to be built before e2fsprogs if 'file'
>>       is present in the image or world build. Then e2fsprogs will
>>       detect magic.h,libmagic.so and  the race is eliminated.
>>
>>
>> That’s only the case if ‘file’ is in PACKAGECONFIG, if not, it’ll still
>> behave the way it did before, and change its deps based on build order.
>> Our policy is to avoid floating deps period, regardless of
>> PACKAGECONFIG. If a given packageconfig is not enabled, then explicitly
>> disable the dependency, don’t detect and use it anyway.
>
> Ah right and now I see some uses where there would not be a
> package dependency required.
>
> I'll likely send a v2 adding just:
>    PACKAGECONFIG ??= "file"

I'm afraid that this can't fix the floating deps, if file is a must
and can't be disabled, you can set:

DEPENDS += "file"

rather than use a PACKAGECONFIG.

> since 'fuse' is in the meta-filesystems layer but

For fuse:
PACKAGECONFIG ??= ""
PACKAGECONFIG[fuse] = '--enable-fuse2fs,--disable-fuse2fs,fuse'

It is disabled by default, but other layers use a e2fsprogs.bbappend
or PACKAGECONFIG_pn-e2fsprogs = "fuse" in conf file to enable it.

> I don't understand how the fuse dependency gets fixed since

It is disabled by default (--disable-fuse2fs), this can fix the
floating deps issue.

// Robert

> it's not changed in meta-oe/meta-filesystems AFAIKT. Should the
> fuse dependency be added as well so that builds that add
> the meta-filesystems layer don't have this potential race?
> I could do some testing to find out but it's late here.
> Robert likely knows all about this so I'm
> CCing him since he also committed the fuse change.
>
> It seems unlikely but we could have a pile of clean-up
> to do...
>
> Thanks,
> ../Randy
>
>> --
>> Christopher Larson
>> clarson at kergoth dot com
>> Founder - BitBake, OpenEmbedded, OpenZaurus
>> Maintainer - Tslib
>> Senior Software Engineer, Mentor Graphics
>
>