All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] e2fsprogs: add packageconfig for -file-
@ 2016-09-17  1:37 Randy MacLeod
  2016-09-17  5:09 ` Christopher Larson
  0 siblings, 1 reply; 8+ messages in thread
From: Randy MacLeod @ 2016-09-17  1:37 UTC (permalink / raw)
  To: openembedded-core

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@windriver.com>
---
 meta/recipes-devtools/e2fsprogs/e2fsprogs_1.43.bb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.43.bb b/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.43.bb
index f4855bc..1707cb9 100644
--- a/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.43.bb
+++ b/meta/recipes-devtools/e2fsprogs/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'
 PACKAGECONFIG[fuse] = '--enable-fuse2fs,--disable-fuse2fs,fuse'
 
 do_configure_prepend () {
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH] e2fsprogs: add packageconfig for -file-
  2016-09-17  1:37 [PATCH] e2fsprogs: add packageconfig for -file- Randy MacLeod
@ 2016-09-17  5:09 ` Christopher Larson
  2016-09-19  2:32   ` Randy MacLeod
  0 siblings, 1 reply; 8+ messages in thread
From: Christopher Larson @ 2016-09-17  5:09 UTC (permalink / raw)
  To: Randy MacLeod; +Cc: Patches and discussions about the oe-core layer

[-- Attachment #1: Type: text/plain, Size: 1787 bytes --]

On Fri, Sep 16, 2016 at 6:37 PM, Randy MacLeod <Randy.MacLeod@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@windriver.com>
> ---
>  meta/recipes-devtools/e2fsprogs/e2fsprogs_1.43.bb | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.43.bb
> b/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.43.bb
> index f4855bc..1707cb9 100644
> --- a/meta/recipes-devtools/e2fsprogs/e2fsprogs_1.43.bb
> +++ b/meta/recipes-devtools/e2fsprogs/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.
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 2744 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] e2fsprogs: add packageconfig for -file-
  2016-09-17  5:09 ` Christopher Larson
@ 2016-09-19  2:32   ` Randy MacLeod
  2016-09-19  2:37     ` Christopher Larson
  0 siblings, 1 reply; 8+ messages in thread
From: Randy MacLeod @ 2016-09-19  2:32 UTC (permalink / raw)
  To: Christopher Larson; +Cc: Patches and discussions about the oe-core layer

On 2016-09-17 01:09 AM, Christopher Larson wrote:
>
> On Fri, Sep 16, 2016 at 6:37 PM, Randy MacLeod
> <Randy.MacLeod@windriver.com <mailto:Randy.MacLeod@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@windriver.com
>     <mailto:Randy.MacLeod@windriver.com>>
>     ---
>      meta/recipes-devtools/e2fsprogs/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>
>     b/meta/recipes-devtools/e2fsprogs/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>
>     +++ b/meta/recipes-devtools/e2fsprogs/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.



The details:

Note, that I thought this was a rare race condition but
it showed up again yesterday in a work-related build.

It seems to work when I tested it and there are
several other examples of using:
    PACKAGECONFIG[foo] = ",,foo,"
such as:
    meta/recipes-devtools/python/python-smartpm_git.bb

    GTK_RDEP = "${PN}-interface-gtk"
    ...
    PACKAGECONFIG[gtk] = ",,gtk+,${GTK_RDEP}"

and 38 others:
$ grep -r PACKAGECONFIG meta/recipes* | \
      grep ",,"| grep -v with | grep -v ",,," | wc -l
38

Also from:
    meta/classes/base.bbclass
The PACKAGEDEPENDS handling code looks at:

         for flag, flagval in sorted(pkgconfigflags.items()):
             items = flagval.split(",")
             num = len(items)
             if num > 4:
                 ...

             if flag in pkgconfig:
                 if num >= 3 and items[2]:
                     extradeps.append(items[2])

In my case, num = 3 and file gets added to the depends
and therefore built first. Then when e2fsprogs is built,
the configure script auto-detects that file and libmagic
are present (without risk of a race) and sets the magic flags
as shown below. If there were a configure flag, I'd be happy
to use it but from e2fsprogs's configure.ac:

dnl
dnl See if libmagic exists
dnl
AC_CHECK_LIB(magic, magic_file, [MAGIC_LIB=-lmagic
AC_CHECK_HEADERS([magic.h])])
if test "$ac_cv_func_dlopen" = yes ; then
    MAGIC_LIB=$DLOPEN_LIB
fi
AC_SUBST(MAGIC_LIB)
dnl

so even with my limited exposure to the autotools syntax,
it seems like there's no: --with[out]-libmagic=yes option, right?

Some logs:

# No patch:
$ grep -i magic /tmp/e2fsprogs-no-patch-cleanall-file-config.log

configure:13035: checking for magic_file in -lmagic
configure:13060: x86_64-oe-linux-gcc  <flags> \
    conftest.c -lmagic  -lblkid  >&5
/.../tmp-glibc/sysroots/x86_64-linux/usr/libexec/x86_64-oe-linux/gcc/x86_64-oe-linux/6.2.0/ld: 
cannot find -lmagic
| char magic_file ();
| return magic_file ();
ac_cv_lib_magic_magic_file=no
MAGIC_LIB='-ldl'

# with patch:
$ grep -i magic /tmp/e2fsprogs-with-patch-cim-config.log
g'
configure:13035: checking for magic_file in -lmagic
configure:13060: x86_64-oe-linux-gcc <flags> \
     conftest.c -lmagic  -lblkid  >&5
configure:13075: checking magic.h usability
configure:13075: checking magic.h presence
configure:13075: checking for magic.h
| #define HAVE_MAGIC_H 1
ac_cv_header_magic_h=yes
ac_cv_lib_magic_magic_file=yes
MAGIC_LIB='-ldl'
#define HAVE_MAGIC_H 1

Granted this doens't prove that the race went away.
[1]


$ cd .../e2fsprogs.git
$ git pull
$ grep magic ./configure
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for magic_file in 
-lmagic" >&5
$as_echo_n "checking for magic_file in -lmagic... " >&6; }
if ${ac_cv_lib_magic_magic_file+:} false; then :
LIBS="-lmagic  $LIBS"
char magic_file ();
return magic_file ();
   ac_cv_lib_magic_magic_file=yes
   ac_cv_lib_magic_magic_file=no
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: 
$ac_cv_lib_magic_magic_file" >&5
$as_echo "$ac_cv_lib_magic_magic_file" >&6; }
if test "x$ac_cv_lib_magic_magic_file" = xyes; then :
   MAGIC_LIB=-lmagic
for ac_header in magic.h
   ac_fn_c_check_header_mongrel "$LINENO" "magic.h" 
"ac_cv_header_magic_h" "$ac_includes_default"
if test "x$ac_cv_header_magic_h" = xyes; then :


Thanks,
../Randy 'often knows how to spell bitbake' MacLeod

> --
> Christopher Larson
> clarson at kergoth dot com
> Founder - BitBake, OpenEmbedded, OpenZaurus
> Maintainer - Tslib
> Senior Software Engineer, Mentor Graphics


-- 
# Randy MacLeod. SMTS, Linux, Wind River
Direct: 613.963.1350 | 350 Terry Fox Drive, Suite 200, Ottawa, ON, 
Canada, K2K 2W5


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] e2fsprogs: add packageconfig for -file-
  2016-09-19  2:32   ` Randy MacLeod
@ 2016-09-19  2:37     ` Christopher Larson
  2016-09-19  3:32       ` Randy MacLeod
  0 siblings, 1 reply; 8+ messages in thread
From: Christopher Larson @ 2016-09-19  2:37 UTC (permalink / raw)
  To: Randy MacLeod; +Cc: Patches and discussions about the oe-core layer

[-- Attachment #1: Type: text/plain, Size: 3195 bytes --]

On Sun, Sep 18, 2016 at 7:32 PM, Randy MacLeod <randy.macleod@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@windriver.com <mailto:Randy.MacLeod@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@windriver.com
>>     <mailto:Randy.MacLeod@windriver.com>>
>>     ---
>>      meta/recipes-devtools/e2fsprogs/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>
>>     b/meta/recipes-devtools/e2fsprogs/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>
>>     +++ b/meta/recipes-devtools/e2fsprogs/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.
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 5109 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] e2fsprogs: add packageconfig for -file-
  2016-09-19  2:37     ` Christopher Larson
@ 2016-09-19  3:32       ` Randy MacLeod
  2016-09-19  3:42         ` Robert Yang
  0 siblings, 1 reply; 8+ messages in thread
From: Randy MacLeod @ 2016-09-19  3:32 UTC (permalink / raw)
  To: Christopher Larson; +Cc: Patches and discussions about the oe-core layer

On 2016-09-18 10:37 PM, Christopher Larson wrote:
>
> On Sun, Sep 18, 2016 at 7:32 PM, Randy MacLeod
> <randy.macleod@windriver.com <mailto:randy.macleod@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@windriver.com
>         <mailto:Randy.MacLeod@windriver.com>
>         <mailto:Randy.MacLeod@windriver.com
>         <mailto:Randy.MacLeod@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@windriver.com
>         <mailto:Randy.MacLeod@windriver.com>
>             <mailto:Randy.MacLeod@windriver.com
>         <mailto:Randy.MacLeod@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"
since 'fuse' is in the meta-filesystems layer but
I don't understand how the fuse dependency gets fixed since
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


-- 
# Randy MacLeod. SMTS, Linux, Wind River
Direct: 613.963.1350 | 350 Terry Fox Drive, Suite 200, Ottawa, ON, 
Canada, K2K 2W5


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] e2fsprogs: add packageconfig for -file-
  2016-09-19  3:32       ` Randy MacLeod
@ 2016-09-19  3:42         ` Robert Yang
  2016-09-19  5:00           ` Ulf Magnusson
  0 siblings, 1 reply; 8+ messages in thread
From: Robert Yang @ 2016-09-19  3:42 UTC (permalink / raw)
  To: Randy MacLeod, Christopher Larson
  Cc: Patches and discussions about the oe-core layer



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@windriver.com <mailto:randy.macleod@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@windriver.com
>>         <mailto:Randy.MacLeod@windriver.com>
>>         <mailto:Randy.MacLeod@windriver.com
>>         <mailto:Randy.MacLeod@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@windriver.com
>>         <mailto:Randy.MacLeod@windriver.com>
>>             <mailto:Randy.MacLeod@windriver.com
>>         <mailto:Randy.MacLeod@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
>
>


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] e2fsprogs: add packageconfig for -file-
  2016-09-19  3:42         ` Robert Yang
@ 2016-09-19  5:00           ` Ulf Magnusson
  2016-09-19  7:02             ` Ulf Magnusson
  0 siblings, 1 reply; 8+ messages in thread
From: Ulf Magnusson @ 2016-09-19  5:00 UTC (permalink / raw)
  To: Robert Yang
  Cc: Christopher Larson, Patches and discussions about the oe-core layer

On Mon, Sep 19, 2016 at 5:42 AM, Robert Yang <liezhi.yang@windriver.com> wrote:
>
>
> 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@windriver.com <mailto:randy.macleod@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@windriver.com
>>>         <mailto:Randy.MacLeod@windriver.com>
>>>         <mailto:Randy.MacLeod@windriver.com
>>>         <mailto:Randy.MacLeod@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@windriver.com
>>>         <mailto:Randy.MacLeod@windriver.com>
>>>             <mailto:Randy.MacLeod@windriver.com
>>>         <mailto:Randy.MacLeod@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.
>>http://www.yoctoproject.org/docs/2.2/ref-manual/ref-manual.html#checking-for-missing-build-time-dependencies
>>
>> 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

There is a relevant new section in the 2.2 reference manual by the
way, that explains what floating dependencies are and why they are
problematic:

http://www.yoctoproject.org/docs/2.2/ref-manual/ref-manual.html#checking-for-missing-build-time-dependencies

Cheers,
Ulf

>
>> 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
>>
>>
>>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH] e2fsprogs: add packageconfig for -file-
  2016-09-19  5:00           ` Ulf Magnusson
@ 2016-09-19  7:02             ` Ulf Magnusson
  0 siblings, 0 replies; 8+ messages in thread
From: Ulf Magnusson @ 2016-09-19  7:02 UTC (permalink / raw)
  To: Robert Yang
  Cc: Christopher Larson, Patches and discussions about the oe-core layer

On Mon, Sep 19, 2016 at 7:00 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
> On Mon, Sep 19, 2016 at 5:42 AM, Robert Yang <liezhi.yang@windriver.com> wrote:
>>
>>
>> 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@windriver.com <mailto:randy.macleod@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@windriver.com
>>>>         <mailto:Randy.MacLeod@windriver.com>
>>>>         <mailto:Randy.MacLeod@windriver.com
>>>>         <mailto:Randy.MacLeod@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@windriver.com
>>>>         <mailto:Randy.MacLeod@windriver.com>
>>>>             <mailto:Randy.MacLeod@windriver.com
>>>>         <mailto:Randy.MacLeod@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.
>>>http://www.yoctoproject.org/docs/2.2/ref-manual/ref-manual.html#checking-for-missing-build-time-dependencies
>>>
>>> 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
>
> There is a relevant new section in the 2.2 reference manual by the
> way, that explains what floating dependencies are and why they are
> problematic:
>
> http://www.yoctoproject.org/docs/2.2/ref-manual/ref-manual.html#checking-for-missing-build-time-dependencies

Heh... just noticed that it had already been mentioned. It would be a
good idea to also mention PACKAGECONFIG and the importance of
explicitly disabling/enabling features that would otherwise be
floating, to make things deterministic.

Cheers,
Ulf

>
>>
>>> 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
>>>
>>>
>>>
>> --
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core


^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-09-19  7:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-17  1:37 [PATCH] e2fsprogs: add packageconfig for -file- Randy MacLeod
2016-09-17  5:09 ` Christopher Larson
2016-09-19  2:32   ` Randy MacLeod
2016-09-19  2:37     ` Christopher Larson
2016-09-19  3:32       ` Randy MacLeod
2016-09-19  3:42         ` Robert Yang
2016-09-19  5:00           ` Ulf Magnusson
2016-09-19  7:02             ` Ulf Magnusson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.