Patchwork [2/2] tclibc-uclibc.inc: Append -uclibc only to target recipes

login
register
mail settings
Submitter Richard Purdie
Date May 17, 2011, 12:13 a.m.
Message ID <1305591231.3424.152.camel@rex>
Download mbox | patch
Permalink /patch/4213/
State New, archived
Headers show

Comments

Richard Purdie - May 17, 2011, 12:13 a.m.
On Sun, 2011-05-15 at 23:04 -0700, Khem Raj wrote:
> Do not define DEPLOY_DIR_IMAGE
> Append -uclibc to STAGING_DIR_TARGET only for target recipe and cross
> recipes
> Append -uclibc to STAGING_DIR_HOST only for target recipes.
> 
> These changes make sure that we still share the native sysroot
> 
> Signed-off-by: Khem Raj <raj.khem@gmail.com>

The more I looked at that patch, the more holes I could see in what we
were doing (and what Angstrom currently does). I started playing around
with the patch below which tried to improve on that idea.

I then concluded that we might be able to do something like:

MACHINEOVERRIDES := "${MACHINE}"
MACHINE_append = "-uclibc"

since what we're really trying to do in the uclibc case is replace
anything MACHINE specific with something containing uclibc?

Thoughts?

Cheers,

Richard
Phil Blundell - May 17, 2011, 9:17 a.m.
On Tue, 2011-05-17 at 01:13 +0100, Richard Purdie wrote:
> The more I looked at that patch, the more holes I could see in what we
> were doing (and what Angstrom currently does). I started playing around
> with the patch below which tried to improve on that idea.
> 
> I then concluded that we might be able to do something like:
> 
> MACHINEOVERRIDES := "${MACHINE}"
> MACHINE_append = "-uclibc"
> 
> since what we're really trying to do in the uclibc case is replace
> anything MACHINE specific with something containing uclibc?

I think that particular change would be a bad idea for several reasons.
Firstly, and perhaps most importantly, MACHINE is a primary
configuration variable and having it magically change under the hood
seems like it would violate the principle of least surprise.  Secondly,
at a practical level, this would make it hard to use MACHINE for
anything other than overrides.  And thirdly, at a conceptual level, the
choice of libc is really nothing to do with the MACHINE.

> diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
> index 553c6a2..354668f 100644
> --- a/meta/classes/sstate.bbclass
> +++ b/meta/classes/sstate.bbclass

This patch looks good to me (and I would definitely prefer this to the
solution above).  The lines that you have deleted from tclibc-uclibc.inc
were causing my uclibc builds to fail so I would certainly be glad to
have them removed.

p.
Richard Purdie - May 17, 2011, 9:34 a.m.
On Tue, 2011-05-17 at 10:17 +0100, Phil Blundell wrote:
> On Tue, 2011-05-17 at 01:13 +0100, Richard Purdie wrote:
> > The more I looked at that patch, the more holes I could see in what we
> > were doing (and what Angstrom currently does). I started playing around
> > with the patch below which tried to improve on that idea.
> > 
> > I then concluded that we might be able to do something like:
> > 
> > MACHINEOVERRIDES := "${MACHINE}"
> > MACHINE_append = "-uclibc"
> > 
> > since what we're really trying to do in the uclibc case is replace
> > anything MACHINE specific with something containing uclibc?
> 
> I think that particular change would be a bad idea for several reasons.
> Firstly, and perhaps most importantly, MACHINE is a primary
> configuration variable and having it magically change under the hood
> seems like it would violate the principle of least surprise.  Secondly,
> at a practical level, this would make it hard to use MACHINE for
> anything other than overrides.  And thirdly, at a conceptual level, the
> choice of libc is really nothing to do with the MACHINE.
> 
> > diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
> > index 553c6a2..354668f 100644
> > --- a/meta/classes/sstate.bbclass
> > +++ b/meta/classes/sstate.bbclass
> 
> This patch looks good to me (and I would definitely prefer this to the
> solution above).  The lines that you have deleted from tclibc-uclibc.inc
> were causing my uclibc builds to fail so I would certainly be glad to
> have them removed.

No question those lines are "wrong" but the solutions various people are
using out there are "wrong" in various ways too (including what is
'working' in ansgtrom at the moment).

A solution where you replace 80% of uses of a variable with a new
variable as per my proposed patch doesn't quite feel right either.

How about this idea:

TMPDIR_append = "-uclibc"

since sstate cache is deliberately outside TMPDIR so anything that can
be reused between builds will automatically.

Simple, effective and addresses a lot of the bugs I can see in the other
approaches...

Cheers,

Richard
Phil Blundell - May 17, 2011, 9:44 a.m.
On Tue, 2011-05-17 at 10:34 +0100, Richard Purdie wrote:
> How about this idea:
> 
> TMPDIR_append = "-uclibc"

Hm, I'm not totally sure what this really buys us.  

If the whole issue boils down to saying that you just can't share a
TMPDIR between builds with competing C libraries (which sounds
reasonable, since it's probably about the same thing as saying that libc
selection is a DISTRO property) then it seems like something that can be
fixed in the documentation.  Users can just select a different TMPDIR by
hand, same as they would when changing DISTROs, and it doesn't seem that
there is any real need for the build system to try to work around it for
them.

I'm also slightly uncomfortable with automagic TMPDIR frobbing for the
same reason as MACHINE; if I set TMPDIR="foo" in my local.conf then I
would have an (IMHO reasonable) expectation that the build artifacts
would actually go into that directory and not some variation on the
theme.  I guess you could ameliorate that slightly by appending
"/uclibc" so that at least you ended up using a subfolder of the chosen
path, but it still doesn't seem very wholesome to me.

p.
Richard Purdie - May 17, 2011, 2:31 p.m.
On Tue, 2011-05-17 at 10:44 +0100, Phil Blundell wrote:
> On Tue, 2011-05-17 at 10:34 +0100, Richard Purdie wrote:
> > How about this idea:
> > 
> > TMPDIR_append = "-uclibc"
> 
> Hm, I'm not totally sure what this really buys us.  
> 
> If the whole issue boils down to saying that you just can't share a
> TMPDIR between builds with competing C libraries (which sounds
> reasonable, since it's probably about the same thing as saying that libc
> selection is a DISTRO property) then it seems like something that can be
> fixed in the documentation.  Users can just select a different TMPDIR by
> hand, same as they would when changing DISTROs, and it doesn't seem that
> there is any real need for the build system to try to work around it for
> them.

Some distros want to be able to build multiple libc in the same tree in
the same way multiple machines work without requiring the user to switch
settings. I don't think its an unreasonable request but the
implementation shouldn't impact the system too adversely. I think this
fits that requirement.

> I'm also slightly uncomfortable with automagic TMPDIR frobbing for the
> same reason as MACHINE; if I set TMPDIR="foo" in my local.conf then I
> would have an (IMHO reasonable) expectation that the build artifacts
> would actually go into that directory and not some variation on the
> theme.  I guess you could ameliorate that slightly by appending
> "/uclibc" so that at least you ended up using a subfolder of the chosen
> path, but it still doesn't seem very wholesome to me.

Its not ideal but its nicer than the MACHINE workarounds IMO. We could
default to turning it off and let distros choose to turn it on if they
desire it too...

Cheers,

Richard
Phil Blundell - May 17, 2011, 2:40 p.m.
On Tue, 2011-05-17 at 15:31 +0100, Richard Purdie wrote:
> Its not ideal but its nicer than the MACHINE workarounds IMO. We could
> default to turning it off and let distros choose to turn it on if they
> desire it too...

That sounds like a good plan to me.  Or if it's literally just
"TMPDIR_append = ...", maybe we could leave it up to the distros to set
in their own config files.

p.
Khem Raj - May 18, 2011, 5:36 a.m.
On Tue, May 17, 2011 at 2:44 AM, Phil Blundell <pb@pbcl.net> wrote:
> On Tue, 2011-05-17 at 10:34 +0100, Richard Purdie wrote:
>> How about this idea:
>>
>> TMPDIR_append = "-uclibc"
>
> Hm, I'm not totally sure what this really buys us.
>
> If the whole issue boils down to saying that you just can't share a
> TMPDIR between builds with competing C libraries (which sounds
> reasonable, since it's probably about the same thing as saying that libc
> selection is a DISTRO property) then it seems like something that can be
> fixed in the documentation.  Users can just select a different TMPDIR by
> hand, same as they would when changing DISTROs, and it doesn't seem that
> there is any real need for the build system to try to work around it for
> them.

there are certain parts of build process which could be shared e.g.
native sysroot
and nativesdk package builds across target system library selections.
and we build
a lot of native packages it will be good if they could be shared
secondly the all-*-* package builds could be shared as well. Similarly
multimachine build is another aspect which could share builds too
having selected same C library.

Many users of oe would be development tool vendors who have BSPs for
many processor boards and C libraries and they would like to have this
possibility
Khem Raj - May 18, 2011, 5:49 a.m.
On Mon, May 16, 2011 at 5:13 PM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Sun, 2011-05-15 at 23:04 -0700, Khem Raj wrote:
>> Do not define DEPLOY_DIR_IMAGE
>> Append -uclibc to STAGING_DIR_TARGET only for target recipe and cross
>> recipes
>> Append -uclibc to STAGING_DIR_HOST only for target recipes.
>>
>> These changes make sure that we still share the native sysroot
>>
>> Signed-off-by: Khem Raj <raj.khem@gmail.com>
>
> The more I looked at that patch, the more holes I could see in what we
> were doing (and what Angstrom currently does). I started playing around
> with the patch below which tried to improve on that idea.
>
> I then concluded that we might be able to do something like:
>
> MACHINEOVERRIDES := "${MACHINE}"
> MACHINE_append = "-uclibc"
>
> since what we're really trying to do in the uclibc case is replace
> anything MACHINE specific with something containing uclibc?
>
> Thoughts?
>
> Cheers,
>
> Richard
>
>
> diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
> index 553c6a2..354668f 100644
> --- a/meta/classes/sstate.bbclass
> +++ b/meta/classes/sstate.bbclass
> @@ -23,7 +23,7 @@ python () {
>         bb.data.setVar('SSTATE_PKGARCH', bb.data.getVar('BUILD_ARCH', d), d)
>     elif bb.data.inherits_class('cross', d):
>         bb.data.setVar('SSTATE_PKGARCH', bb.data.expand("${BUILD_ARCH}_${BASE_PACKAGE_ARCH}", d), d)
> -        bb.data.setVar('SSTATE_MANMACH', bb.data.expand("${BUILD_ARCH}_${MACHINE}", d), d)
> +        bb.data.setVar('SSTATE_MANMACH', bb.data.expand("${BUILD_ARCH}_${STAGING_MACHNAME}", d), d)
>     elif bb.data.inherits_class('crosssdk', d):
>         bb.data.setVar('SSTATE_PKGARCH', bb.data.expand("${BUILD_ARCH}_${BASE_PACKAGE_ARCH}", d), d)
>     elif bb.data.inherits_class('nativesdk', d):
> @@ -31,7 +31,7 @@ python () {
>     elif bb.data.inherits_class('cross-canadian', d):
>         bb.data.setVar('SSTATE_PKGARCH', bb.data.expand("${SDK_ARCH}_${BASE_PACKAGE_ARCH}", d), d)
>     else:
> -        bb.data.setVar('SSTATE_MANMACH', bb.data.expand("${MACHINE}", d), d)
> +        bb.data.setVar('SSTATE_MANMACH', bb.data.expand("${STAGING_MACHNAME}", d), d)
>
>     # These classes encode staging paths into their scripts data so can only be
>     # reused if we manipulate the paths
> diff --git a/meta/classes/staging.bbclass b/meta/classes/staging.bbclass
> index fef6457..c2e4e50 100644
> --- a/meta/classes/staging.bbclass
> +++ b/meta/classes/staging.bbclass
> @@ -91,7 +91,7 @@ SSTATETASKS += "do_populate_sysroot"
>  do_populate_sysroot[sstate-name] = "populate-sysroot"
>  do_populate_sysroot[sstate-inputdirs] = "${SYSROOT_DESTDIR}"
>  do_populate_sysroot[sstate-outputdirs] = "${STAGING_DIR_HOST}/"
> -do_populate_sysroot[stamp-extra-info] = "${MACHINE}"
> +do_populate_sysroot[stamp-extra-info] = "${STAGING_MACHNAME}"
>
>  python do_populate_sysroot_setscene () {
>        sstate_setscene(d)
> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> index 8b6236e..141b942 100644
> --- a/meta/conf/bitbake.conf
> +++ b/meta/conf/bitbake.conf
> @@ -300,7 +300,8 @@ STAGING_DATADIR_NATIVE = "${STAGING_DIR_NATIVE}${datadir_native}"
>
>  # This should really be MULTIMACH_HOST_SYS but that breaks "all" and machine
>  # specific packages - hack around it for now.
> -STAGING_DIR_HOST = "${STAGING_DIR}/${MACHINE}"
> +STAGING_MACHNAME ??= "${MACHINE}"
> +STAGING_DIR_HOST = "${STAGING_DIR}/${STAGING_MACHNAME}"
>  STAGING_BINDIR = "${STAGING_DIR_HOST}${bindir}"
>  STAGING_LIBDIR = "${STAGING_DIR_HOST}${libdir}"
>  STAGING_INCDIR = "${STAGING_DIR_HOST}${includedir}"
> @@ -312,7 +313,7 @@ STAGING_PYDIR = "${STAGING_DIR}/lib/python2.4"
>
>  # This should really be MULTIMACH_TARGET_SYS but that breaks "all" and machine
>  # specific packages - hack around it for now.
> -STAGING_DIR_TARGET = "${STAGING_DIR}/${MACHINE}"
> +STAGING_DIR_TARGET = "${STAGING_DIR}/${STAGING_MACHNAME}"
>  STAGING_DIR_TCBOOTSTRAP = "${STAGING_DIR_TARGET}-tcbootstrap"
>
>  # Setting DEPLOY_DIR outside of TMPDIR is helpful, when you are using
> @@ -344,8 +345,8 @@ STAGING_KERNEL_DIR = "${STAGING_DIR_HOST}/kernel"
>
>  IMAGE_ROOTFS = "${WORKDIR}/rootfs"
>  IMAGE_BASENAME = "${PN}"
> -IMAGE_NAME = "${IMAGE_BASENAME}-${MACHINE}-${DATETIME}"
> -IMAGE_LINK_NAME = "${IMAGE_BASENAME}-${MACHINE}"
> +IMAGE_NAME = "${IMAGE_BASENAME}-${STAGING_MACHNAME}-${DATETIME}"
> +IMAGE_LINK_NAME = "${IMAGE_BASENAME}-${STAGING_MACHNAME}"
>
>  # This option allows for a precentage overage of the actaul image size rather than a
>  # fixed extra space
> diff --git a/meta/conf/distro/include/tclibc-uclibc.inc b/meta/conf/distro/include/tclibc-uclibc.inc
> index 27f6ec6..e84da81 100644
> --- a/meta/conf/distro/include/tclibc-uclibc.inc
> +++ b/meta/conf/distro/include/tclibc-uclibc.inc
> @@ -20,11 +20,7 @@ CXXFLAGS += "-fvisibility-inlines-hidden"
>
>  IMAGE_LINGUAS = ""
>
> -DEPLOY_DIR_IMAGE = "${TMPDIR}/deploy/images"
> -DEPLOY_DIR_append = "-uclibc"
> -STAGING_DIR_TARGET_append = "-uclibc"
> -STAGING_DIR_HOST_append = "-uclibc"
> -SSTATE_MANIFESTS_append = "-uclibc"
> +STAGING_MACHNAME = "${MACHINE}-uclibc"

should we use ${MACHINE}/${TCLIBC} in all those places instead of
defining STAGING_MACHNAME

>
>  LIBC_DEPENDENCIES = "\
>     uclibc \
>
>

Patch

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index 553c6a2..354668f 100644
--- a/meta/classes/sstate.bbclass
+++ b/meta/classes/sstate.bbclass
@@ -23,7 +23,7 @@  python () {
         bb.data.setVar('SSTATE_PKGARCH', bb.data.getVar('BUILD_ARCH', d), d)
     elif bb.data.inherits_class('cross', d):
         bb.data.setVar('SSTATE_PKGARCH', bb.data.expand("${BUILD_ARCH}_${BASE_PACKAGE_ARCH}", d), d)
-        bb.data.setVar('SSTATE_MANMACH', bb.data.expand("${BUILD_ARCH}_${MACHINE}", d), d)
+        bb.data.setVar('SSTATE_MANMACH', bb.data.expand("${BUILD_ARCH}_${STAGING_MACHNAME}", d), d)
     elif bb.data.inherits_class('crosssdk', d):
         bb.data.setVar('SSTATE_PKGARCH', bb.data.expand("${BUILD_ARCH}_${BASE_PACKAGE_ARCH}", d), d)
     elif bb.data.inherits_class('nativesdk', d):
@@ -31,7 +31,7 @@  python () {
     elif bb.data.inherits_class('cross-canadian', d):
         bb.data.setVar('SSTATE_PKGARCH', bb.data.expand("${SDK_ARCH}_${BASE_PACKAGE_ARCH}", d), d)
     else:
-        bb.data.setVar('SSTATE_MANMACH', bb.data.expand("${MACHINE}", d), d)
+        bb.data.setVar('SSTATE_MANMACH', bb.data.expand("${STAGING_MACHNAME}", d), d)
 
     # These classes encode staging paths into their scripts data so can only be
     # reused if we manipulate the paths
diff --git a/meta/classes/staging.bbclass b/meta/classes/staging.bbclass
index fef6457..c2e4e50 100644
--- a/meta/classes/staging.bbclass
+++ b/meta/classes/staging.bbclass
@@ -91,7 +91,7 @@  SSTATETASKS += "do_populate_sysroot"
 do_populate_sysroot[sstate-name] = "populate-sysroot"
 do_populate_sysroot[sstate-inputdirs] = "${SYSROOT_DESTDIR}"
 do_populate_sysroot[sstate-outputdirs] = "${STAGING_DIR_HOST}/"
-do_populate_sysroot[stamp-extra-info] = "${MACHINE}"
+do_populate_sysroot[stamp-extra-info] = "${STAGING_MACHNAME}"
 
 python do_populate_sysroot_setscene () {
 	sstate_setscene(d)
diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index 8b6236e..141b942 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -300,7 +300,8 @@  STAGING_DATADIR_NATIVE = "${STAGING_DIR_NATIVE}${datadir_native}"
 
 # This should really be MULTIMACH_HOST_SYS but that breaks "all" and machine 
 # specific packages - hack around it for now.
-STAGING_DIR_HOST = "${STAGING_DIR}/${MACHINE}"
+STAGING_MACHNAME ??= "${MACHINE}"
+STAGING_DIR_HOST = "${STAGING_DIR}/${STAGING_MACHNAME}"
 STAGING_BINDIR = "${STAGING_DIR_HOST}${bindir}"
 STAGING_LIBDIR = "${STAGING_DIR_HOST}${libdir}"
 STAGING_INCDIR = "${STAGING_DIR_HOST}${includedir}"
@@ -312,7 +313,7 @@  STAGING_PYDIR = "${STAGING_DIR}/lib/python2.4"
 
 # This should really be MULTIMACH_TARGET_SYS but that breaks "all" and machine 
 # specific packages - hack around it for now.
-STAGING_DIR_TARGET = "${STAGING_DIR}/${MACHINE}"
+STAGING_DIR_TARGET = "${STAGING_DIR}/${STAGING_MACHNAME}"
 STAGING_DIR_TCBOOTSTRAP = "${STAGING_DIR_TARGET}-tcbootstrap"
 
 # Setting DEPLOY_DIR outside of TMPDIR is helpful, when you are using
@@ -344,8 +345,8 @@  STAGING_KERNEL_DIR = "${STAGING_DIR_HOST}/kernel"
 
 IMAGE_ROOTFS = "${WORKDIR}/rootfs"
 IMAGE_BASENAME = "${PN}"
-IMAGE_NAME = "${IMAGE_BASENAME}-${MACHINE}-${DATETIME}"
-IMAGE_LINK_NAME = "${IMAGE_BASENAME}-${MACHINE}"
+IMAGE_NAME = "${IMAGE_BASENAME}-${STAGING_MACHNAME}-${DATETIME}"
+IMAGE_LINK_NAME = "${IMAGE_BASENAME}-${STAGING_MACHNAME}"
 
 # This option allows for a precentage overage of the actaul image size rather than a
 # fixed extra space
diff --git a/meta/conf/distro/include/tclibc-uclibc.inc b/meta/conf/distro/include/tclibc-uclibc.inc
index 27f6ec6..e84da81 100644
--- a/meta/conf/distro/include/tclibc-uclibc.inc
+++ b/meta/conf/distro/include/tclibc-uclibc.inc
@@ -20,11 +20,7 @@  CXXFLAGS += "-fvisibility-inlines-hidden"
 
 IMAGE_LINGUAS = ""
 
-DEPLOY_DIR_IMAGE = "${TMPDIR}/deploy/images"
-DEPLOY_DIR_append = "-uclibc"
-STAGING_DIR_TARGET_append = "-uclibc"
-STAGING_DIR_HOST_append = "-uclibc"
-SSTATE_MANIFESTS_append = "-uclibc"
+STAGING_MACHNAME = "${MACHINE}-uclibc"
 
 LIBC_DEPENDENCIES = "\
     uclibc \