allarch: disable allarch when multilib is used

== Series Details ==

Series: allarch: disable allarch when multilib is used (rev4)
Revision: 4
URL : https://patchwork.openembedded.org/series/13457/
State : failure

== Summary ==

Thank you for submitting this patch series to OpenEmbedded Core. This is an automated response. Several tests have been executed on the proposed series by patchtest resulting in the following failures:

* Issue             Series does not apply on top of target branch [test_series_merge_on_head]
   Suggested fix    Rebase your series on top of targeted branch
   Targeted branch  master (currently at 0791c4d39e)

If you believe any of these test results are incorrect, please reply to the mailing list ([email protected]) raising your concerns. Otherwise we would appreciate you correcting the issues and submitting a new version of the patchset if applicable. Please ensure you add/increment the version number when sending the new version (i.e. [PATCH] -> [PATCH v2] -> [PATCH v3] -> ...).

---

Guidelines:     https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe

Some allarch packages rdepends non-allarch packages. When multilib is used, it doesn't expand the dependency chain correctly, e.g.

core-image-sato -> ca-certificates(allarch) -> openssl

we expect dependency chain for lib32-core-image-sato:

lib32-core-image-sato -> ca-certificates(allarch) -> lib32-openssl

it should install lib32-openssl for ca-certificates but openssl is still wrongly required.

Copy allarch.bbclass to allarch-enabled.bbclass and only inherit allarch-enabled.bbclass when multilib is not used.

Signed-off-by Kai Kang <[email protected]>

---
 meta/classes/allarch-enabled.bbclass | 52 ++++++++++++++++++++++++++++++++++++
 meta/classes/allarch.bbclass         | 51 ++---------------------------------
 meta/classes/icecc.bbclass           |  2 +-
 meta/classes/multilib.bbclass        |  2 +-
 meta/classes/multilib_global.bbclass |  2 +-
 meta/classes/package.bbclass         |  6 ++---
 6 files changed, 60 insertions(+), 55 deletions(-)
 create mode 100644 meta/classes/allarch-enabled.bbclass

meta/classes/allarch.bbclass +1 -40

index 1eebe0bf2e7..0eca076df0b 100644
@@ -1,52 +1,5 @@
 #
-# This class is used for architecture independent recipes/data files (usually scripts)
+# This class enables allarch only when multilib is not used.
 #
 
-PACKAGE_ARCH = "all"
-
-python () {
-    # Allow this class to be included but overridden - only set
-    # the values if we're still "all" package arch.
-    if d.getVar("PACKAGE_ARCH") == "all":
-        # No need for virtual/libc or a cross compiler
-        d.setVar("INHIBIT_DEFAULT_DEPS","1")
-
-        # Set these to a common set of values, we shouldn't be using them other that for WORKDIR directory
-        # naming anyway
-        d.setVar("baselib", "lib")
-        d.setVar("TARGET_ARCH", "allarch")
-        d.setVar("TARGET_OS", "linux")
-        d.setVar("TARGET_CC_ARCH", "none")
-        d.setVar("TARGET_LD_ARCH", "none")
-        d.setVar("TARGET_AS_ARCH", "none")
-        d.setVar("TARGET_FPU", "")
-        d.setVar("TARGET_PREFIX", "")
-        # Expand PACKAGE_EXTRA_ARCHS since the staging code needs this
-        # (this removes any dependencies from the hash perspective)
-        d.setVar("PACKAGE_EXTRA_ARCHS", d.getVar("PACKAGE_EXTRA_ARCHS"))
-        d.setVar("SDK_ARCH", "none")
-        d.setVar("SDK_CC_ARCH", "none")
-        d.setVar("TARGET_CPPFLAGS", "none")
-        d.setVar("TARGET_CFLAGS", "none")
-        d.setVar("TARGET_CXXFLAGS", "none")
-        d.setVar("TARGET_LDFLAGS", "none")
-        d.setVar("POPULATESYSROOTDEPS", "")
-
-        # Avoid this being unnecessarily different due to nuances of
-        # the target machine that aren't important for "all" arch
-        # packages.
-        d.setVar("LDFLAGS", "")
-
-        # No need to do shared library processing or debug symbol handling
-        d.setVar("EXCLUDE_FROM_SHLIBS", "1")
-        d.setVar("INHIBIT_PACKAGE_DEBUG_SPLIT", "1")
-        d.setVar("INHIBIT_PACKAGE_STRIP", "1")

Hi,

I think this is close and it passes the tests on the autobuilder however I did spot a couple of potential issues after thinking about this for a bit.

Quoted 76 lines · Kai Kang

Some allarch packages rdepends non-allarch packages. When multilib is used, it doesn't expand the dependency chain correctly, e.g.

core-image-sato -> ca-certificates(allarch) -> openssl

we expect dependency chain for lib32-core-image-sato:

lib32-core-image-sato -> ca-certificates(allarch) -> lib32-openssl

it should install lib32-openssl for ca-certificates but openssl is still wrongly required.

Copy allarch.bbclass to allarch-enabled.bbclass and only inherit allarch-enabled.bbclass when multilib is not used.

Signed-off-by Kai Kang <[email protected]>

---
  meta/classes/allarch-enabled.bbclass | 52 ++++++++++++++++++++++++++++++++++++
  meta/classes/allarch.bbclass         | 51 ++---------------------------------
  meta/classes/icecc.bbclass           |  2 +-
  meta/classes/multilib.bbclass        |  2 +-
  meta/classes/multilib_global.bbclass |  2 +-
  meta/classes/package.bbclass         |  6 ++---
  6 files changed, 60 insertions(+), 55 deletions(-)

create mode 100644 meta/classes/allarch-enabled.bbclass

meta/classes/allarch.bbclass +1 -40

index 1eebe0bf2e7..0eca076df0b 100644
@@ -1,52 +1,5 @@
  #
-# This class is used for architecture independent recipes/data files (usually scripts)
+# This class enables allarch only when multilib is not used.
  #
 
-PACKAGE_ARCH = "all"
-
-python () {
-    # Allow this class to be included but overridden - only set
-    # the values if we're still "all" package arch.
-    if d.getVar("PACKAGE_ARCH") == "all":
-        # No need for virtual/libc or a cross compiler
-        d.setVar("INHIBIT_DEFAULT_DEPS","1")
-
-        # Set these to a common set of values, we shouldn't be using them other that for WORKDIR directory
-        # naming anyway
-        d.setVar("baselib", "lib")
-        d.setVar("TARGET_ARCH", "allarch")
-        d.setVar("TARGET_OS", "linux")
-        d.setVar("TARGET_CC_ARCH", "none")
-        d.setVar("TARGET_LD_ARCH", "none")
-        d.setVar("TARGET_AS_ARCH", "none")
-        d.setVar("TARGET_FPU", "")
-        d.setVar("TARGET_PREFIX", "")
-        # Expand PACKAGE_EXTRA_ARCHS since the staging code needs this
-        # (this removes any dependencies from the hash perspective)
-        d.setVar("PACKAGE_EXTRA_ARCHS", d.getVar("PACKAGE_EXTRA_ARCHS"))
-        d.setVar("SDK_ARCH", "none")
-        d.setVar("SDK_CC_ARCH", "none")
-        d.setVar("TARGET_CPPFLAGS", "none")
-        d.setVar("TARGET_CFLAGS", "none")
-        d.setVar("TARGET_CXXFLAGS", "none")
-        d.setVar("TARGET_LDFLAGS", "none")
-        d.setVar("POPULATESYSROOTDEPS", "")
-
-        # Avoid this being unnecessarily different due to nuances of
-        # the target machine that aren't important for "all" arch
-        # packages.
-        d.setVar("LDFLAGS", "")
-
-        # No need to do shared library processing or debug symbol handling
-        d.setVar("EXCLUDE_FROM_SHLIBS", "1")
-        d.setVar("INHIBIT_PACKAGE_DEBUG_SPLIT", "1")
-        d.setVar("INHIBIT_PACKAGE_STRIP", "1")

meta/classes/allarch.bbclass line 5

Quoted 10 lines

meta/classes/allarch.bbclass +1 -9

-
-        # These multilib values shouldn't change allarch packages so exclude them
-        d.appendVarFlag("emit_pkgdata", "vardepsexclude", " MULTILIB_VARIANTS")
-        d.appendVarFlag("write_specfile", "vardepsexclude", " MULTILIBS")
-        d.appendVarFlag("do_package", "vardepsexclude", " package_do_shlibs")
-    elif bb.data.inherits_class('packagegroup', d) and not bb.data.inherits_class('nativesdk', d):
-        bb.error("Please ensure recipe %s sets PACKAGE_ARCH before inherit packagegroup" % d.getVar("FILE"))
-}
-
+inherit ${@oe.utils.ifelse(d.getVar('MULTILIB_VARIANTS'), '', 'allarch-enabled')}

Firstly, whilst I do understand why you've made this change like this, I don't really like one line classes which clutter up the classes directory. Using inherit like this does force the expansion of the variable early and is very prone to "race" conditions. The above is safe but see below.

FWIW the original code tried to switch off the value of the PACKAGE_ARCH variable. If we change the way we enable/disable the code, we should consider whether we can consistently use one mechanism for both.

Secondly, does this change affect the behaviour of nativesdk?

I think this will disable allarch for nativesdk in any multilib build and we don't want to do that, we only have a problem with target multilib allarch.

At a guess you probably need to check class-target is in overrides and multilib_variants is set? The problem could be that class-target is set comparatively late and then we're into ordering issues.

I do want to get the nativesdk issue fixed before we merge this though.

Cheers,

Richard

Packages which inherit fontcache.bbclass call postinstall script update_font_cache. And in update_font_cache, it calls ${bindir}/fc- cache
by qemuwrapper. When multilib is enabled, both packages foo and lib32-foo
will call ${bindir}/fc-cache and one of them will fail to run obviously.

Duplicate install file fc-cache to ${libexecdir} with ${MLPREFIX} and call proper fc-cache in update_font_cache.

Signed-off-by Kai Kang <[email protected]>

---
 meta/recipes-graphics/fontconfig/fontconfig_2.12.6.bb | 8 +++++++-
 scripts/postinst-intercepts/update_font_cache         | 2 +-
 2 files changed, 8 insertions(+), 2 deletions(-)

meta/recipes-graphics/fontconfig/fontconfig_2.12.6.bb +0 -0

index d4cbce80b45..db36c867741 100644

Fix install files conflicts between multlib packages:

Quoted 8 lines

Error: Transaction check error:
file /usr/bin/g-ir-annotation-tool conflicts between attempted installs of lib32-gobject-introspection-1.56.1-r0.x86 and gobject-introspection-1.56.1-r0.core2_64 file /usr/bin/g-ir-scanner conflicts between attempted installs of lib32-gobject-introspection-1.56.1-r0.x86 and gobject-introspection-1.56.1-r0.core2_64 file /usr/bin/cairo-trace conflicts between attempted installs of lib32-libcairo-perf-utils-1.14.12-r0.x86 and libcairo-perf-utils-1.14.12-r0.core2_64 file /usr/bin/icu-config conflicts between attempted installs of lib32-icu-dev-62.1-r0.x86 and icu-dev-62.1-r0.core2_64
file /usr/share/gir-1.0/GLib-2.0.gir conflicts between attempted installs of gobject-introspection-dev-1.56.1-r0.core2_64 and lib32-gobject-introspection-dev-1.56.1-r0.x86 file /usr/bin/gpgrt-config conflicts between attempted installs of lib32-libgpg-error-dev-1.32-r0.x86 and libgpg-error-dev-1.32-r0.core2_64 file /usr/share/pkgconfig/udev.pc conflicts between attempted installs of eudev-dev-3.2.5-r0.core2_64 and lib32-eudev-dev-3.2.5-r0.x86

Quoted 26 lines · Kai Kang

Packages which inherit fontcache.bbclass call postinstall script update_font_cache. And in update_font_cache, it calls ${bindir}/fc- cache
by qemuwrapper. When multilib is enabled, both packages foo and lib32-foo
will call ${bindir}/fc-cache and one of them will fail to run obviously.

Duplicate install file fc-cache to ${libexecdir} with ${MLPREFIX} and call proper fc-cache in update_font_cache.

Signed-off-by Kai Kang <[email protected]>

---
  meta/recipes-graphics/fontconfig/fontconfig_2.12.6.bb | 8 +++++++-
  scripts/postinst-intercepts/update_font_cache         | 2 +-
  2 files changed, 8 insertions(+), 2 deletions(-)

meta/recipes-graphics/fontconfig/fontconfig_2.12.6.bb +0 -0

index d4cbce80b45..db36c867741 100644
@@ -35,9 +35,15 @@ do_configure_prepend() {
      rm -f ${S}/src/fcobjshash.h ${S}/src/fcobjshash.gperf
  }
 

meta/recipes-graphics/fontconfig/fontconfig_2.12.6.bb do_configure_prepend() { · line 41

Quoted 4 lines

meta/recipes-graphics/fontconfig/fontconfig_2.12.6.bb +4 -0

+do_install_append_class-target() {
+    # duplicate fc-cache for postinstall script
+    mkdir -p ${D}${libexecdir}
+    cp ${D}${bindir}/fc-cache ${D}${libexecdir}/${MLPREFIX}fc-cache

We may as well hardlink this, same for the following patch too.

Cheers,

Richard

Quoted 10 lines · Kai Kang

Fix install files conflicts between multlib packages:

Quoted 8 lines

Error: Transaction check error:
file /usr/bin/g-ir-annotation-tool conflicts between attempted installs of lib32-gobject-introspection-1.56.1-r0.x86 and gobject-introspection-1.56.1-r0.core2_64 file /usr/bin/g-ir-scanner conflicts between attempted installs of lib32-gobject-introspection-1.56.1-r0.x86 and gobject-introspection-1.56.1-r0.core2_64 file /usr/bin/cairo-trace conflicts between attempted installs of lib32-libcairo-perf-utils-1.14.12-r0.x86 and libcairo-perf-utils-1.14.12-r0.core2_64 file /usr/bin/icu-config conflicts between attempted installs of lib32-icu-dev-62.1-r0.x86 and icu-dev-62.1-r0.core2_64
file /usr/share/gir-1.0/GLib-2.0.gir conflicts between attempted installs of gobject-introspection-dev-1.56.1-r0.core2_64 and lib32-gobject-introspection-dev-1.56.1-r0.x86 file /usr/bin/gpgrt-config conflicts between attempted installs of lib32-libgpg-error-dev-1.32-r0.x86 and libgpg-error-dev-1.32-r0.core2_64 file /usr/share/pkgconfig/udev.pc conflicts between attempted installs of eudev-dev-3.2.5-r0.core2_64 and lib32-eudev-dev-3.2.5-r0.x86

.pc files installed into /usr/share are architecture independent. Either it is arch dependent in which case its in the wrong place, or it needs to be fixed.

We are not putting these under control of update-alternatives!

It would be helpful to understand the kinds of differences in some of these files. gir files in /usr/share/gir-1.0/ should also really be arch independent...

Cheers,

Richard

Hi,

I think this is close and it passes the tests on the autobuilder however I did spot a couple of potential issues after thinking about this for a bit.

Quoted 76 lines · Kai Kang

Some allarch packages rdepends non-allarch packages. When multilib is used, it doesn't expand the dependency chain correctly, e.g.

core-image-sato -> ca-certificates(allarch) -> openssl

we expect dependency chain for lib32-core-image-sato:

lib32-core-image-sato -> ca-certificates(allarch) -> lib32-openssl

it should install lib32-openssl for ca-certificates but openssl is still wrongly required.

Copy allarch.bbclass to allarch-enabled.bbclass and only inherit allarch-enabled.bbclass when multilib is not used.

Signed-off-by Kai Kang <[email protected]>

---
  meta/classes/allarch-enabled.bbclass | 52 ++++++++++++++++++++++++++++++++++++
  meta/classes/allarch.bbclass         | 51 ++---------------------------------
  meta/classes/icecc.bbclass           |  2 +-
  meta/classes/multilib.bbclass        |  2 +-
  meta/classes/multilib_global.bbclass |  2 +-
  meta/classes/package.bbclass         |  6 ++---
  6 files changed, 60 insertions(+), 55 deletions(-)

create mode 100644 meta/classes/allarch-enabled.bbclass

meta/classes/allarch.bbclass +1 -40

index 1eebe0bf2e7..0eca076df0b 100644
@@ -1,52 +1,5 @@
  #
-# This class is used for architecture independent recipes/data files (usually scripts)
+# This class enables allarch only when multilib is not used.
  #
 
-PACKAGE_ARCH = "all"
-
-python () {
-    # Allow this class to be included but overridden - only set
-    # the values if we're still "all" package arch.
-    if d.getVar("PACKAGE_ARCH") == "all":
-        # No need for virtual/libc or a cross compiler
-        d.setVar("INHIBIT_DEFAULT_DEPS","1")
-
-        # Set these to a common set of values, we shouldn't be using them other that for WORKDIR directory
-        # naming anyway
-        d.setVar("baselib", "lib")
-        d.setVar("TARGET_ARCH", "allarch")
-        d.setVar("TARGET_OS", "linux")
-        d.setVar("TARGET_CC_ARCH", "none")
-        d.setVar("TARGET_LD_ARCH", "none")
-        d.setVar("TARGET_AS_ARCH", "none")
-        d.setVar("TARGET_FPU", "")
-        d.setVar("TARGET_PREFIX", "")
-        # Expand PACKAGE_EXTRA_ARCHS since the staging code needs this
-        # (this removes any dependencies from the hash perspective)
-        d.setVar("PACKAGE_EXTRA_ARCHS", d.getVar("PACKAGE_EXTRA_ARCHS"))
-        d.setVar("SDK_ARCH", "none")
-        d.setVar("SDK_CC_ARCH", "none")
-        d.setVar("TARGET_CPPFLAGS", "none")
-        d.setVar("TARGET_CFLAGS", "none")
-        d.setVar("TARGET_CXXFLAGS", "none")
-        d.setVar("TARGET_LDFLAGS", "none")
-        d.setVar("POPULATESYSROOTDEPS", "")
-
-        # Avoid this being unnecessarily different due to nuances of
-        # the target machine that aren't important for "all" arch
-        # packages.
-        d.setVar("LDFLAGS", "")
-
-        # No need to do shared library processing or debug symbol handling
-        d.setVar("EXCLUDE_FROM_SHLIBS", "1")
-        d.setVar("INHIBIT_PACKAGE_DEBUG_SPLIT", "1")
-        d.setVar("INHIBIT_PACKAGE_STRIP", "1")

meta/classes/allarch.bbclass line 5

Quoted 10 lines

meta/classes/allarch.bbclass +1 -9

-
-        # These multilib values shouldn't change allarch packages so exclude them
-        d.appendVarFlag("emit_pkgdata", "vardepsexclude", " MULTILIB_VARIANTS")
-        d.appendVarFlag("write_specfile", "vardepsexclude", " MULTILIBS")
-        d.appendVarFlag("do_package", "vardepsexclude", " package_do_shlibs")
-    elif bb.data.inherits_class('packagegroup', d) and not bb.data.inherits_class('nativesdk', d):
-        bb.error("Please ensure recipe %s sets PACKAGE_ARCH before inherit packagegroup" % d.getVar("FILE"))
-}
-
+inherit ${@oe.utils.ifelse(d.getVar('MULTILIB_VARIANTS'), '', 'allarch-enabled')}

Firstly, whilst I do understand why you've made this change like this, I don't really like one line classes which clutter up the classes directory. Using inherit like this does force the expansion of the variable early and is very prone to "race" conditions. The above is safe but see below.

FWIW the original code tried to switch off the value of the PACKAGE_ARCH variable. If we change the way we enable/disable the code, we should consider whether we can consistently use one mechanism for both.

Secondly, does this change affect the behaviour of nativesdk?

I think this will disable allarch for nativesdk in any multilib build and we don't want to do that, we only have a problem with target multilib allarch.

At a guess you probably need to check class-target is in overrides and multilib_variants is set? The problem could be that class-target is set comparatively late and then we're into ordering issues.

I do want to get the nativesdk issue fixed before we merge this though.

Cheers,

Richard

I searched oe-core for which inherits allarch and bbextend nativesdk by:

$ rgrep 'inherit.*allarch' -l meta | xargs grep -l 
'BBCLASSEXTEND.*nativesdk'
meta/recipes-devtools/autoconf-archive/autoconf-archive_2018.03.13.bb
meta/recipes-support/ca-certificates/ca-certificates_20180409.bb
meta/recipes-core/update-rc.d/update-rc.d_0.8.bb

And I checked one of them:

$ bitbake -e nativesdk-autoconf-archive > be.nativesdk-autoconf-
archive

The var PACKAGE_ARCH will be set with value from nativesdk.bbclass and
overwrite the value from allarch.bbclass:

# $PACKAGE_ARCH [4 operations]
# set /buildarea/kkang/yocto/poky/meta/conf/bitbake.conf:150
# [_defaultval] "${TUNE_PKGARCH}"
# set /buildarea/kkang/yocto/poky/meta/conf/documentation.conf:304
# [doc] "The architecture of the resulting package or packages."
# set /buildarea/kkang/yocto/poky/meta/classes/allarch.bbclass:5
# "all"
# set /buildarea/kkang/yocto/poky/meta/classes/nativesdk.bbclass:18
# "${SDK_ARCH}-${SDKPKGSUFFIX}"

# pre-expansion value:
# "${SDK_ARCH}-${SDKPKGSUFFIX}"
PACKAGE_ARCH="x86_64-nativesdk"

It seems update of allarch.bbclass doesn't affect nativesdk. Or did I miss something? Thanks.