[PATCH 0/2] Consolidate _FOR_BUILD variables

[PATCH 0/2] Consolidate _FOR_BUILD variables

[Ross Burton]

Hi,

It's autotools best practise[1] to use the variables CC_FOR_BUILD etc when
building tools that are required to build the rest of the package.  A lot of
packages do this, and they all repeat the same variable assignements, so
consolidate these assignments in autotools.bbclass.

Not every instance of _FOR_BUILD has been removed from the metadata, as there
are several cases where the assignment isn't the same (added flags, mainly).

Tested with a world build of oe-core, zero buildhistory differences.

Ross

[1] http://www.sourceware.org/autobook/autobook/autobook_270.html#SEC270

The following changes since commit 78983e939ab17f02f8911c8b0d0e326b419856b9:

  lib/oe/classextend: Ensure we don't extend expressions more than once (2012-11-06 09:34:47 +0100)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib ross/cross

for you to fetch changes up to 154a30032b03a9ce269752adbf5bbec85205720e:

  meta: remove redundant _FOR_BUILD variables (2012-11-09 10:00:05 +0000)

----------------------------------------------------------------
Ross Burton (2):
      autotools: set _FOR_BUILD variables here
      meta: remove redundant _FOR_BUILD variables

 meta/classes/autotools.bbclass                 |   15 +++++++++++++++
 meta/recipes-devtools/binutils/binutils.inc    |    4 ++--
 meta/recipes-devtools/rpm/rpm_5.4.9.bb         |    1 -
 meta/recipes-extended/bash/bash.inc            |    1 -
 meta/recipes-gnome/gnome/metacity_2.30.3.bb    |    5 ++---
 meta/recipes-graphics/mutter/mutter.inc        |    5 ++---
 meta/recipes-graphics/xorg-lib/libx11.inc      |    8 ++------
 meta/recipes-kernel/systemtap/systemtap_git.bb |    4 ----
 meta/recipes-support/gmp/gmp_4.2.1.bb          |    2 --
 meta/recipes-support/gmp/gmp_5.0.5.bb          |    2 --
 10 files changed, 23 insertions(+), 24 deletions(-)

Ross Burton (2):
  autotools: set _FOR_BUILD variables here
  meta: remove redundant _FOR_BUILD variables

 meta/classes/autotools.bbclass                 |   15 +++++++++++++++
 meta/recipes-devtools/binutils/binutils.inc    |    4 ++--
 meta/recipes-devtools/rpm/rpm_5.4.9.bb         |    1 -
 meta/recipes-extended/bash/bash.inc            |    1 -
 meta/recipes-gnome/gnome/metacity_2.30.3.bb    |    5 ++---
 meta/recipes-graphics/mutter/mutter.inc        |    5 ++---
 meta/recipes-graphics/xorg-lib/libx11.inc      |    8 ++------
 meta/recipes-kernel/systemtap/systemtap_git.bb |    4 ----
 meta/recipes-support/gmp/gmp_4.2.1.bb          |    2 --
 meta/recipes-support/gmp/gmp_5.0.5.bb          |    2 --
 10 files changed, 23 insertions(+), 24 deletions(-)

-- 
1.7.10

[PATCH 1/2] autotools: set _FOR_BUILD variables here

[Ross Burton]

Signed-off-by: Ross Burton <ross.burton@intel.com>
---
 meta/classes/autotools.bbclass |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/meta/classes/autotools.bbclass b/meta/classes/autotools.bbclass
index b97d74b..ca981ec 100644
--- a/meta/classes/autotools.bbclass
+++ b/meta/classes/autotools.bbclass
@@ -34,6 +34,21 @@ EXTRA_AUTORECONF = "--exclude=autopoint"
 
 export lt_cv_sys_lib_dlsearch_path_spec = "${libdir} ${base_libdir}"
 
+# When building tools for use at build-time it's recommended for the build
+# system to use these variables when cross-compiling.
+# (http://sources.redhat.com/autobook/autobook/autobook_270.html)
+export CPP_FOR_BUILD = "${BUILD_CPP}"
+export CPPFLAGS_FOR_BUILD = "${BUILD_CPPFLAGS}"
+
+export CC_FOR_BUILD = "${BUILD_CC}"
+export CFLAGS_FOR_BUILD = "${BUILD_CFLAGS}"
+
+export CXX_FOR_BUILD = "${BUILD_CXX}"
+export CXXFLAGS_FOR_BUILD="${BUILD_CXXFLAGS}"
+
+export LD_FOR_BUILD = "${BUILD_LD}"
+export LDFLAGS_FOR_BUILD = "${BUILD_LDFLAGS}"
+
 def autotools_set_crosscompiling(d):
     if not bb.data.inherits_class('native', d):
         return " cross_compiling=yes"
-- 
1.7.10

[PATCH 2/2] meta: remove redundant _FOR_BUILD variables

[Ross Burton]

Signed-off-by: Ross Burton <ross.burton@intel.com>
---
 meta/recipes-devtools/binutils/binutils.inc    |    4 ++--
 meta/recipes-devtools/rpm/rpm_5.4.9.bb         |    1 -
 meta/recipes-extended/bash/bash.inc            |    1 -
 meta/recipes-gnome/gnome/metacity_2.30.3.bb    |    5 ++---
 meta/recipes-graphics/mutter/mutter.inc        |    5 ++---
 meta/recipes-graphics/xorg-lib/libx11.inc      |    8 ++------
 meta/recipes-kernel/systemtap/systemtap_git.bb |    4 ----
 meta/recipes-support/gmp/gmp_4.2.1.bb          |    2 --
 meta/recipes-support/gmp/gmp_5.0.5.bb          |    2 --
 9 files changed, 8 insertions(+), 24 deletions(-)

diff --git a/meta/recipes-devtools/binutils/binutils.inc b/meta/recipes-devtools/binutils/binutils.inc
index ff64882..1e59296 100644
--- a/meta/recipes-devtools/binutils/binutils.inc
+++ b/meta/recipes-devtools/binutils/binutils.inc
@@ -76,9 +76,9 @@ export RANLIB_FOR_TARGET = "${TARGET_PREFIX}ranlib"
 export CC_FOR_HOST = "${CCACHE}${HOST_PREFIX}gcc ${HOST_CC_ARCH}"
 export CXX_FOR_HOST = "${CCACHE}${HOST_PREFIX}gcc ${HOST_CC_ARCH}"
 
+# autotools.bbclass sets the _FOR_BUILD variables, but for some reason we need
+# to unset LD_LIBRARY_PATH.
 export CC_FOR_BUILD = "LD_LIBRARY_PATH= ${BUILD_CC}"
-export CPP_FOR_BUILD = "${BUILD_CPP}"
-export CFLAGS_FOR_BUILD = "${BUILD_CFLAGS}"
 
 MULTIARCH := "${@bb.utils.contains("DISTRO_FEATURES", "multiarch", "yes", "no", d)}"
 do_configure[vardeps] += "MULTIARCH"
diff --git a/meta/recipes-devtools/rpm/rpm_5.4.9.bb b/meta/recipes-devtools/rpm/rpm_5.4.9.bb
index 3268160..0721284 100644
--- a/meta/recipes-devtools/rpm/rpm_5.4.9.bb
+++ b/meta/recipes-devtools/rpm/rpm_5.4.9.bb
@@ -350,7 +350,6 @@ do_configure() {
 	    -e 's/pkg-config uuid/pkg-config ossp-uuid/g' -i configure
 
 	export varprefix=${localstatedir}
-	export CC_FOR_BUILD="${BUILD_CC}"
 	oe_runconf
 }
 
diff --git a/meta/recipes-extended/bash/bash.inc b/meta/recipes-extended/bash/bash.inc
index ae4b681..f1de931 100644
--- a/meta/recipes-extended/bash/bash.inc
+++ b/meta/recipes-extended/bash/bash.inc
@@ -9,7 +9,6 @@ inherit autotools gettext update-alternatives
 PARALLEL_MAKE = ""
 
 EXTRA_OECONF = "--enable-job-control"
-export CC_FOR_BUILD = "${BUILD_CC}"
 
 ALTERNATIVE_${PN} = "sh"
 ALTERNATIVE_LINK_NAME[sh] = "${base_bindir}/sh"
diff --git a/meta/recipes-gnome/gnome/metacity_2.30.3.bb b/meta/recipes-gnome/gnome/metacity_2.30.3.bb
index 3524f0e..5b996d2 100644
--- a/meta/recipes-gnome/gnome/metacity_2.30.3.bb
+++ b/meta/recipes-gnome/gnome/metacity_2.30.3.bb
@@ -23,9 +23,8 @@ EXTRA_OECONF += "--disable-verbose \
 do_configure_prepend() {
 	sed -i -e 's:$ZENITY:$NOZENITY:g' -e 's:-Werror::g' ${S}/configure.in
 }
-export CC_FOR_BUILD = "${BUILD_CC}"
-export CFLAGS_FOR_BUILD = "${BUILD_CFLAGS} -I${STAGING_LIBDIR_NATIVE}/glib-2.0/include -I${STAGING_INCDIR_NATIVE}/glib-2.0 -I${STAGING_INCDIR_NATIVE}/glib-2.0/include -I${STAGING_INCDIR_NATIVE}"
-export LDFLAGS_FOR_BUILD = "${BUILD_LDFLAGS} -L${STAGING_LIBDIR_NATIVE} -lglib-2.0"
+CFLAGS_FOR_BUILD += "-I${STAGING_LIBDIR_NATIVE}/glib-2.0/include -I${STAGING_INCDIR_NATIVE}/glib-2.0 -I${STAGING_INCDIR_NATIVE}/glib-2.0/include -I${STAGING_INCDIR_NATIVE}"
+LDFLAGS_FOR_BUILD += "-L${STAGING_LIBDIR_NATIVE} -lglib-2.0"
 
 FILES_${PN} += "${datadir}/themes ${datadir}/gnome-control-center ${datadir}/gnome"
 
diff --git a/meta/recipes-graphics/mutter/mutter.inc b/meta/recipes-graphics/mutter/mutter.inc
index da78827..942349f 100644
--- a/meta/recipes-graphics/mutter/mutter.inc
+++ b/meta/recipes-graphics/mutter/mutter.inc
@@ -20,9 +20,8 @@ EXTRA_OECONF += "--disable-verbose	\
 FILES_${PN} += "${datadir}/themes ${libdir}/mutter/plugins/*.so ${datadir}/gnome/wm-properties/"
 FILES_${PN}-dbg += "${libdir}/mutter/plugins/.debug/*"
 
-export CC_FOR_BUILD = "${BUILD_CC}"
-export CFLAGS_FOR_BUILD = "${BUILD_CFLAGS} -I${STAGING_INCDIR_NATIVE}/glib-2.0 -I${STAGING_INCDIR_NATIVE}/glib-2.0/include"
-export LDFLAGS_FOR_BUILD = "${BUILD_LDFLAGS} -L${STAGING_LIBDIR_NATIVE} -lglib-2.0"
+CFLAGS_FOR_BUILD += "-I${STAGING_INCDIR_NATIVE}/glib-2.0 -I${STAGING_INCDIR_NATIVE}/glib-2.0/include"
+LDFLAGS_FOR_BUILD += "-L${STAGING_LIBDIR_NATIVE} -lglib-2.0"
 
 do_configure_prepend () {
         echo "EXTRA_DIST=" > ${S}/gnome-doc-utils.make
diff --git a/meta/recipes-graphics/xorg-lib/libx11.inc b/meta/recipes-graphics/xorg-lib/libx11.inc
index 71e74f7..e0fce3e 100644
--- a/meta/recipes-graphics/xorg-lib/libx11.inc
+++ b/meta/recipes-graphics/xorg-lib/libx11.inc
@@ -30,12 +30,8 @@ EXTRA_OEMAKE += 'CWARNFLAGS=""'
 PACKAGECONFIG ??= ""
 PACKAGECONFIG[xcms] = "--enable-xcms,--disable-xcms"
 
-# src/util/makekeys needs to be compiled natively, so tell it what compiler to
-# use.
-export CC_FOR_BUILD = "${BUILD_CC}"
-export CFLAGS_FOR_BUILD = "${BUILD_CFLAGS} -D_GNU_SOURCE"
-export CPPFLAGS_FOR_BUILD = "${BUILD_CPPFLAGS}"
-export LDFLAGS_FOR_BUILD = "${BUILD_LDFLAGS}"
+# src/util/makekeys is built natively but needs -D_GNU_SOURCE defined.
+CPPFLAGS_FOR_BUILD += "-D_GNU_SOURCE"
 
 PACKAGES =+ "${PN}-xcb"
 
diff --git a/meta/recipes-kernel/systemtap/systemtap_git.bb b/meta/recipes-kernel/systemtap/systemtap_git.bb
index 9e88ee9..a490a2e 100644
--- a/meta/recipes-kernel/systemtap/systemtap_git.bb
+++ b/meta/recipes-kernel/systemtap/systemtap_git.bb
@@ -8,10 +8,6 @@ DEPENDS_class-nativesdk = "nativesdk-elfutils nativesdk-sqlite3 nativesdk-gettex
 
 PR = "r0"
 
-export CC_FOR_BUILD = "${BUILD_CC}"
-export CFLAGS_FOR_BUILD = "${BUILD_CFLAGS}"
-export LDFLAGS_FOR_BUILD = "${BUILD_LDFLAGS}"
-
 EXTRA_OECONF += "--with-libelf=${STAGING_DIR_TARGET} --without-rpm \
             --without-nss --without-avahi \
             --disable-server --disable-grapher "
diff --git a/meta/recipes-support/gmp/gmp_4.2.1.bb b/meta/recipes-support/gmp/gmp_4.2.1.bb
index 1f86a65..49ee59b 100644
--- a/meta/recipes-support/gmp/gmp_4.2.1.bb
+++ b/meta/recipes-support/gmp/gmp_4.2.1.bb
@@ -16,6 +16,4 @@ SRC_URI = "${GNU_MIRROR}/gmp/${BP}.tar.bz2 \
 SRC_URI[md5sum] = "091c56e0e1cca6b09b17b69d47ef18e3"
 SRC_URI[sha256sum] = "d07ffcb37eecec35c5ec72516d10b35fdf6e6fef1fcf1dcd37e30b8cbf8bf941"
 
-export CC_FOR_BUILD = "${BUILD_CC}"
-
 inherit autotools
diff --git a/meta/recipes-support/gmp/gmp_5.0.5.bb b/meta/recipes-support/gmp/gmp_5.0.5.bb
index 2058868..d91340c 100644
--- a/meta/recipes-support/gmp/gmp_5.0.5.bb
+++ b/meta/recipes-support/gmp/gmp_5.0.5.bb
@@ -8,7 +8,5 @@ SRC_URI_append = " file://use-includedir.patch \
                    file://gmp_fix_for_automake-1.12.patch \
                    "
 
-export CC_FOR_BUILD = "${BUILD_CC}"
-
 SRC_URI[md5sum] = "041487d25e9c230b0c42b106361055fe"
 SRC_URI[sha256sum] = "1f588aaccc41bb9aed946f9fe38521c26d8b290d003c5df807f65690f2aadec9"
-- 
1.7.10

Re: [PATCH 2/2] meta: remove redundant _FOR_BUILD variables

[Otavio Salvador]

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

On Fri, Nov 9, 2012 at 8:09 AM, Ross Burton <ross.burton@intel.com> wrote:

> Signed-off-by: Ross Burton <ross.burton@intel.com>
>

Please bump PR. Even it might not change the end result we need to force a
rebuild as we did change the meta-data and we risk having a subtle bug
somewhere.

-- 
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

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

Re: [PATCH 2/2] meta: remove redundant _FOR_BUILD variables

[Richard Purdie]

On Fri, 2012-11-09 at 20:32 -0200, Otavio Salvador wrote:
> On Fri, Nov 9, 2012 at 8:09 AM, Ross Burton <ross.burton@intel.com>
> wrote:
>         Signed-off-by: Ross Burton <ross.burton@intel.com>

>
> Please bump PR. Even it might not change the end result we need to
> force a rebuild as we did change the meta-data and we risk having a
> subtle bug somewhere.

This patch set adds a number of exports to autotools.bbclass. In doing
so, the sstate checksums will change for pretty much everything and
hence pretty much everything will rebuild. Using PR specifically to
force a rebuild is therefore unnecessary.

Cheers,

Richard

Re: [PATCH 2/2] meta: remove redundant _FOR_BUILD variables

[Otavio Salvador]

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

On Fri, Nov 9, 2012 at 8:57 PM, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Fri, 2012-11-09 at 20:32 -0200, Otavio Salvador wrote:
> > On Fri, Nov 9, 2012 at 8:09 AM, Ross Burton <ross.burton@intel.com>
> > wrote:
> >         Signed-off-by: Ross Burton <ross.burton@intel.com>
>
> >
> > Please bump PR. Even it might not change the end result we need to
> > force a rebuild as we did change the meta-data and we risk having a
> > subtle bug somewhere.
>
> This patch set adds a number of exports to autotools.bbclass. In doing
> so, the sstate checksums will change for pretty much everything and
> hence pretty much everything will rebuild. Using PR specifically to
> force a rebuild is therefore unnecessary.
>

If it rebuilds it might get a different binary package and for preserve the
upgrade path PR bumps are need, aren't they?

-- 
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

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

Re: [PATCH 2/2] meta: remove redundant _FOR_BUILD variables

[Richard Purdie]

On Fri, 2012-11-09 at 21:23 -0200, Otavio Salvador wrote:
> On Fri, Nov 9, 2012 at 8:57 PM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>         On Fri, 2012-11-09 at 20:32 -0200, Otavio Salvador wrote:
>         > On Fri, Nov 9, 2012 at 8:09 AM, Ross Burton
>         <ross.burton@intel.com>
>         > wrote:
>         >         Signed-off-by: Ross Burton <ross.burton@intel.com>
>         >
>         > Please bump PR. Even it might not change the end result we
>         need to
>         > force a rebuild as we did change the meta-data and we risk
>         having a
>         > subtle bug somewhere.
>
>         This patch set adds a number of exports to autotools.bbclass.
>         In doing
>         so, the sstate checksums will change for pretty much
>         everything and
>         hence pretty much everything will rebuild. Using PR
>         specifically to
>         force a rebuild is therefore unnecessary.
>
> If it rebuilds it might get a different binary package and for
> preserve the upgrade path PR bumps are need, aren't they?

Technically you are correct (I took your comment above to refer to
rebuilding the recipes). Technically we should bump PR for every recipe
using the autotools class.

We don't however bump PR of every recipe every time we change the core
classes and there is plenty of precedent for not doing that.

This is basically why I think the whole "PR bump" thing is flawed and we
should stop doing it in favour of some automated mechanism like PR
service. This is why its high on the 1.4 feature list since I've had
enough of this madness.

I'm not going to ask Ross to spend the hour it would take to manually
bump PR for each recipe using autotools and even that won't account for
PR in other layers.

Cheers,

Richard

Re: [PATCH 2/2] meta: remove redundant _FOR_BUILD variables

[Phil Blundell]

On Fri, 2012-11-09 at 21:23 -0200, Otavio Salvador wrote:
> If it rebuilds it might get a different binary package and for
> preserve the upgrade path PR bumps are need, aren't they?

Binary upgrade paths have always been a DISTRO issue.  If you want to
bump PR on everything in your distro config then you certainly can do
that, though from the point of view of an end-user I don't think that
performing a phony "upgrade" of every autotools-using package without
any functional change would necessarily be a desirable thing.

The whole PR-bumping thing is just a massive pain in the backside and,
now that all the metadata is hashed into the signature files anyway, I
think we should consider just ditching PR from oe-core altogether.
Distros that want to go on using PR can do so, either by maintaining it
manually or by using some sort of automated PR server.  Distros that
don't use binary feeds and/or support online upgrades will generally
have less of a requirement for PR and might prefer just to do without it
altogether.

p.

Re: [PATCH 2/2] meta: remove redundant _FOR_BUILD variables

[Tomas Frydrych]

On 10/11/12 11:11, Phil Blundell wrote:
> The whole PR-bumping thing is just a massive pain in the backside and,
> now that all the metadata is hashed into the signature files anyway, I
> think we should consider just ditching PR from oe-core altogether.

+1

Tomas