| 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
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.
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
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.
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
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.
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
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 \