Patchwork [1/2] u-boot: remove UBOOT_MACHINE and COMPATIBLE_MACHINES

login
register
mail settings
Submitter Darren Hart
Date May 26, 2011, 9:12 p.m.
Message ID <e5f5d48165bcb5dc3ccf160a29e16ca700f85395.1306444161.git.dvhart@linux.intel.com>
Download mbox | patch
Permalink /patch/4843/
State New, archived
Headers show

Comments

Darren Hart - May 26, 2011, 9:12 p.m.
oe-core does not define any machines, so it does not make sense to
add machine specific information in the oe-core u-boot recipe and
infrastructure. Also note that COMPATIBLE_MACHINES is not easily extended due to
its regex syntax: "(machine_a|machine_b)", making it difficult to extend the
u-boot recipe in bbappend files without resorting to machine specific overrides.

Remove COMPATIBLE_MACHINES and the default UBOOT_MACHINE from the recipe and
insert some anonymous python into u-boot.inc to raise SkipPackage if
UBOOT_MACHINE is not set (this ensures 'world' still works for machines that
can't build u-boot).

UBOOT_MACHINE must now be specified in each machine config that requires u-boot.
This is an improvement over requiring machine specific overrides in every BSP
layer's u-boot_git.bbappend file. For example, a beagleboard machine config
currently contains:

UBOOT_ENTRYPOINT = "0x80008000"
UBOOT_LOADADDRESS = "0x80008000"

With this change, it must now contain:

UBOOT_MACHINE = "omap3_beagle_config"
UBOOT_ENTRYPOINT = "0x80008000"
UBOOT_LOADADDRESS = "0x80008000"

So long as the SRC_URI in the base recipe can build a working u-boot for a given
machine, there is no need to create a u-boot_git.bbappend file. If additional
patches are deemed necessary, a BSP layer creates a u-boot_git.bbappend file and
extends the SRC_URI to include general or machine specific backports.

Note: I used bb.note() instead of bb.debug() to ensure the message at least
      makes it to the console. From what I could gather, bb.debug() doesn't
      go anywhere during recipe parsing.

Signed-off-by: Darren Hart <dvhart@linux.intel.com>
Cc: Richard Purdie <richard.purdie@linuxfoundation.org>
Cc: Koen Kooi <koen@dominion.thruhere.net>
Cc: Jason Kridner <jkridner@beagleboard.org>
Cc: Chris Larson <clarson@kergoth.com>
---
 meta/recipes-bsp/uboot/u-boot.inc    |   10 +++++++++-
 meta/recipes-bsp/uboot/u-boot_git.bb |   11 ++++++-----
 2 files changed, 15 insertions(+), 6 deletions(-)
Richard Purdie - May 26, 2011, 11:24 p.m.
On Thu, 2011-05-26 at 14:12 -0700, Darren Hart wrote:
> Note: I used bb.note() instead of bb.debug() to ensure the message at least
>       makes it to the console. From what I could gather, bb.debug() doesn't
>       go anywhere during recipe parsing.

Why?

We exclude about 30 different recipes when parsing and would you really
want to see a usability message from each one when you likely don't care
about it?

A bb.debug is fine and the user can see it if they run with -D to get
more info. A bb.note is just irritating.

Cheers,

Richard
Chris Larson - May 26, 2011, 11:31 p.m.
On Thu, May 26, 2011 at 4:24 PM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Thu, 2011-05-26 at 14:12 -0700, Darren Hart wrote:
>> Note: I used bb.note() instead of bb.debug() to ensure the message at least
>>       makes it to the console. From what I could gather, bb.debug() doesn't
>>       go anywhere during recipe parsing.
>
> Why?
>
> We exclude about 30 different recipes when parsing and would you really
> want to see a usability message from each one when you likely don't care
> about it?
>
> A bb.debug is fine and the user can see it if they run with -D to get
> more info. A bb.note is just irritating.

As alternative, *If* there's a need for a one time message when
building, which is questionable, and assuming the variable is
configuration, not recipe bound, one could always install a
ConfigParsed event handler and warn if e.g. UBOOT_ENTRYPOINT or
UBOOT_LOADADDRESS are set and UBOOT_MACHINE is not.
Darren Hart - May 27, 2011, 3:43 a.m.
On 05/26/2011 04:24 PM, Richard Purdie wrote:
> On Thu, 2011-05-26 at 14:12 -0700, Darren Hart wrote:
>> Note: I used bb.note() instead of bb.debug() to ensure the message at least
>>       makes it to the console. From what I could gather, bb.debug() doesn't
>>       go anywhere during recipe parsing.
> 
> Why?
> 

My thinking was that the only time you would legitimately try and build
this package when you can't is during a "world" build, which is likely
an unattended sort of build anyway. The rest of the time you might hit
this error would be when you intended to build u-boot but are missing
the requisite configuration bits in your machine config.

Since the debug lines don't get logged anywhere, and you have to clear
tmp/cache in order to retrigger the SkipPackage event with a new bitbake
command (even with -D), I thought it the most user friendly to ensure
the message made it out somewhere where it wouldn't get lost.

> We exclude about 30 different recipes

I didn't realize it was so many, it's difficult to tell just grepping
for SkipPackage.

> when parsing and would you really
> want to see a usability message from each one when you likely don't care
> about it?

See above for my rationale on when you "care about it".

> 
> A bb.debug is fine and the user can see it if they run with -D to get
> more info.

The user won't see it unless they clear tmp/cache, which isn't very
intuitive (or at least it wasn't to me).

> A bb.note is just irritating.

I can resend with bb.debug() if you feel strongly about it, as
apparently you do. I've answered your "why" question, but I don't feel
strongly about it. If you want to use bb.debug() I can resend as such
(or just repush with that single change).

Thanks,
Richard Purdie - May 27, 2011, 8:40 a.m.
On Thu, 2011-05-26 at 20:43 -0700, Darren Hart wrote:
> On 05/26/2011 04:24 PM, Richard Purdie wrote:
> > On Thu, 2011-05-26 at 14:12 -0700, Darren Hart wrote:
> >> Note: I used bb.note() instead of bb.debug() to ensure the message at least
> >>       makes it to the console. From what I could gather, bb.debug() doesn't
> >>       go anywhere during recipe parsing.
> > 
> > Why?
> > 
> 
> My thinking was that the only time you would legitimately try and build
> this package when you can't is during a "world" build, which is likely
> an unattended sort of build anyway. The rest of the time you might hit
> this error would be when you intended to build u-boot but are missing
> the requisite configuration bits in your machine config.

You've inserted the note at parsing time though so every time anyone
builds anything this will show up.

In case you wonder why bitbake does this its because it has no idea what
the recipe provides until it attempts to parse it. Yes, you can make
guesses from filenames but those are just a convenience and
BBCLASSEXTEND can make one recipe provide multiple things for example.

foo_git.bb

containing:

PN = "bar"

or

PROVIDES += "something-else"

are both valid things to do too if potentially misleading.

> Since the debug lines don't get logged anywhere, and you have to clear
> tmp/cache in order to retrigger the SkipPackage event with a new bitbake
> command (even with -D), I thought it the most user friendly to ensure
> the message made it out somewhere where it wouldn't get lost.
> 
> > We exclude about 30 different recipes
> 
> I didn't realize it was so many, it's difficult to tell just grepping
> for SkipPackage.

COMPATIBLE_MACHINE and COMPATIBLE_HOST will show more.

> > when parsing and would you really
> > want to see a usability message from each one when you likely don't care
> > about it?
> 
> See above for my rationale on when you "care about it".
> 
> > 
> > A bb.debug is fine and the user can see it if they run with -D to get
> > more info.
> 
> The user won't see it unless they clear tmp/cache, which isn't very
> intuitive (or at least it wasn't to me).
> 
> > A bb.note is just irritating.
> 
> I can resend with bb.debug() if you feel strongly about it, as
> apparently you do. I've answered your "why" question, but I don't feel
> strongly about it. If you want to use bb.debug() I can resend as such
> (or just repush with that single change).

Given my comment above (the note appears all the time, not just for
world), I do feel strongly about this.

Yes, we could do with a better way of showing up which packages were
skipped if the user needs to know but this isn't the way to do it and it
won't scale.

Cheers,

Richard
Darren Hart - May 27, 2011, 1:56 p.m.
On 05/27/2011 01:40 AM, Richard Purdie wrote:
> On Thu, 2011-05-26 at 20:43 -0700, Darren Hart wrote:
>> On 05/26/2011 04:24 PM, Richard Purdie wrote:
>>> On Thu, 2011-05-26 at 14:12 -0700, Darren Hart wrote:
>>>> Note: I used bb.note() instead of bb.debug() to ensure the message at least
>>>>       makes it to the console. From what I could gather, bb.debug() doesn't
>>>>       go anywhere during recipe parsing.
>>>
>>> Why?
>>>
>>
>> My thinking was that the only time you would legitimately try and build
>> this package when you can't is during a "world" build, which is likely
>> an unattended sort of build anyway. The rest of the time you might hit
>> this error would be when you intended to build u-boot but are missing
>> the requisite configuration bits in your machine config.
> 
> You've inserted the note at parsing time though so every time anyone
> builds anything this will show up.

Aha! Got it. I changed it and ran a test:

bb.note("DEBUG TO FOLLOW")
bb.debug(1, "To build %s, see %s for instructions on setting \
	     up your machine config" % (PN, FILE))


$ rm -rf tmp/cache; bitbake -DDDD u-boot | tee log
Pseudo is not present but is required, building this first before the
main build
Loading cache...done.
Loaded 998 entries from dependency cache.
Parsing recipes...NOTE: DEBUG TO FOLLOW
done.
Parsing of 783 .bb files complete (780 cached, 3 parsed). 1000 targets,
11 skipped, 0 masked, 0 errors.

As you can see, even with -DDDD, the message never makes it to the
console. So I agree that bb.note() is inappropriate, unfortunately,
bb.debug doesn't appear to be working. I believe it was yesterday...

I pushed the bb.debug version to the same contrib branch in case I'm
just running on too little sleep and I did something stupid in the above
command which I'm not seeing.

--
Darren

> 
> In case you wonder why bitbake does this its because it has no idea what
> the recipe provides until it attempts to parse it. Yes, you can make
> guesses from filenames but those are just a convenience and
> BBCLASSEXTEND can make one recipe provide multiple things for example.
> 
> foo_git.bb
> 
> containing:
> 
> PN = "bar"
> 
> or
> 
> PROVIDES += "something-else"
> 
> are both valid things to do too if potentially misleading.
> 
>> Since the debug lines don't get logged anywhere, and you have to clear
>> tmp/cache in order to retrigger the SkipPackage event with a new bitbake
>> command (even with -D), I thought it the most user friendly to ensure
>> the message made it out somewhere where it wouldn't get lost.
>>
>>> We exclude about 30 different recipes
>>
>> I didn't realize it was so many, it's difficult to tell just grepping
>> for SkipPackage.
> 
> COMPATIBLE_MACHINE and COMPATIBLE_HOST will show more.
> 
>>> when parsing and would you really
>>> want to see a usability message from each one when you likely don't care
>>> about it?
>>
>> See above for my rationale on when you "care about it".
>>
>>>
>>> A bb.debug is fine and the user can see it if they run with -D to get
>>> more info.
>>
>> The user won't see it unless they clear tmp/cache, which isn't very
>> intuitive (or at least it wasn't to me).
>>
>>> A bb.note is just irritating.
>>
>> I can resend with bb.debug() if you feel strongly about it, as
>> apparently you do. I've answered your "why" question, but I don't feel
>> strongly about it. If you want to use bb.debug() I can resend as such
>> (or just repush with that single change).
> 
> Given my comment above (the note appears all the time, not just for
> world), I do feel strongly about this.
> 
> Yes, we could do with a better way of showing up which packages were
> skipped if the user needs to know but this isn't the way to do it and it
> won't scale.
> 
> Cheers,
> 
> Richard
> 
>
Richard Purdie - May 27, 2011, 2:46 p.m.
On Fri, 2011-05-27 at 06:56 -0700, Darren Hart wrote:
> 
> On 05/27/2011 01:40 AM, Richard Purdie wrote:
> > On Thu, 2011-05-26 at 20:43 -0700, Darren Hart wrote:
> >> On 05/26/2011 04:24 PM, Richard Purdie wrote:
> >>> On Thu, 2011-05-26 at 14:12 -0700, Darren Hart wrote:
> >>>> Note: I used bb.note() instead of bb.debug() to ensure the message at least
> >>>>       makes it to the console. From what I could gather, bb.debug() doesn't
> >>>>       go anywhere during recipe parsing.
> >>>
> >>> Why?
> >>>
> >>
> >> My thinking was that the only time you would legitimately try and build
> >> this package when you can't is during a "world" build, which is likely
> >> an unattended sort of build anyway. The rest of the time you might hit
> >> this error would be when you intended to build u-boot but are missing
> >> the requisite configuration bits in your machine config.
> > 
> > You've inserted the note at parsing time though so every time anyone
> > builds anything this will show up.
> 
> Aha! Got it. I changed it and ran a test:
> 
> bb.note("DEBUG TO FOLLOW")
> bb.debug(1, "To build %s, see %s for instructions on setting \
> 	     up your machine config" % (PN, FILE))
> 
> 
> $ rm -rf tmp/cache; bitbake -DDDD u-boot | tee log
> Pseudo is not present but is required, building this first before the
> main build
> Loading cache...done.
> Loaded 998 entries from dependency cache.
> Parsing recipes...NOTE: DEBUG TO FOLLOW
> done.
> Parsing of 783 .bb files complete (780 cached, 3 parsed). 1000 targets,
> 11 skipped, 0 masked, 0 errors.
> 
> As you can see, even with -DDDD, the message never makes it to the
> console. So I agree that bb.note() is inappropriate, unfortunately,
> bb.debug doesn't appear to be working. I believe it was yesterday...

The result was cached (780 cached). Try that command after "touch
conf/local.conf" and it will be in there.

Cheers,

Richard
Richard Purdie - May 27, 2011, 2:48 p.m.
On Fri, 2011-05-27 at 15:46 +0100, Richard Purdie wrote:
> On Fri, 2011-05-27 at 06:56 -0700, Darren Hart wrote:
> > 
> > On 05/27/2011 01:40 AM, Richard Purdie wrote:
> > > On Thu, 2011-05-26 at 20:43 -0700, Darren Hart wrote:
> > >> On 05/26/2011 04:24 PM, Richard Purdie wrote:
> > >>> On Thu, 2011-05-26 at 14:12 -0700, Darren Hart wrote:
> > >>>> Note: I used bb.note() instead of bb.debug() to ensure the message at least
> > >>>>       makes it to the console. From what I could gather, bb.debug() doesn't
> > >>>>       go anywhere during recipe parsing.
> > >>>
> > >>> Why?
> > >>>
> > >>
> > >> My thinking was that the only time you would legitimately try and build
> > >> this package when you can't is during a "world" build, which is likely
> > >> an unattended sort of build anyway. The rest of the time you might hit
> > >> this error would be when you intended to build u-boot but are missing
> > >> the requisite configuration bits in your machine config.
> > > 
> > > You've inserted the note at parsing time though so every time anyone
> > > builds anything this will show up.
> > 
> > Aha! Got it. I changed it and ran a test:
> > 
> > bb.note("DEBUG TO FOLLOW")
> > bb.debug(1, "To build %s, see %s for instructions on setting \
> > 	     up your machine config" % (PN, FILE))
> > 
> > 
> > $ rm -rf tmp/cache; bitbake -DDDD u-boot | tee log
> > Pseudo is not present but is required, building this first before the
> > main build
> > Loading cache...done.
> > Loaded 998 entries from dependency cache.
> > Parsing recipes...NOTE: DEBUG TO FOLLOW
> > done.
> > Parsing of 783 .bb files complete (780 cached, 3 parsed). 1000 targets,
> > 11 skipped, 0 masked, 0 errors.
> > 
> > As you can see, even with -DDDD, the message never makes it to the
> > console. So I agree that bb.note() is inappropriate, unfortunately,
> > bb.debug doesn't appear to be working. I believe it was yesterday...
> 
> The result was cached (780 cached). Try that command after "touch
> conf/local.conf" and it will be in there.

Sorry, the note did appear though.

That needs looking into and is possibly a logging problem. Needs fixing
as a separate issue though and use of bb.note there isn't the right
approach.

Cheers,

Richard
Darren Hart - May 27, 2011, 3:13 p.m.
On 05/27/2011 07:48 AM, Richard Purdie wrote:
> On Fri, 2011-05-27 at 15:46 +0100, Richard Purdie wrote:
>> On Fri, 2011-05-27 at 06:56 -0700, Darren Hart wrote:
>>>
>>> On 05/27/2011 01:40 AM, Richard Purdie wrote:
>>>> On Thu, 2011-05-26 at 20:43 -0700, Darren Hart wrote:
>>>>> On 05/26/2011 04:24 PM, Richard Purdie wrote:
>>>>>> On Thu, 2011-05-26 at 14:12 -0700, Darren Hart wrote:
>>>>>>> Note: I used bb.note() instead of bb.debug() to ensure the message at least
>>>>>>>       makes it to the console. From what I could gather, bb.debug() doesn't
>>>>>>>       go anywhere during recipe parsing.
>>>>>>
>>>>>> Why?
>>>>>>
>>>>>
>>>>> My thinking was that the only time you would legitimately try and build
>>>>> this package when you can't is during a "world" build, which is likely
>>>>> an unattended sort of build anyway. The rest of the time you might hit
>>>>> this error would be when you intended to build u-boot but are missing
>>>>> the requisite configuration bits in your machine config.
>>>>
>>>> You've inserted the note at parsing time though so every time anyone
>>>> builds anything this will show up.
>>>
>>> Aha! Got it. I changed it and ran a test:
>>>
>>> bb.note("DEBUG TO FOLLOW")
>>> bb.debug(1, "To build %s, see %s for instructions on setting \
>>> 	     up your machine config" % (PN, FILE))
>>>
>>>
>>> $ rm -rf tmp/cache; bitbake -DDDD u-boot | tee log
>>> Pseudo is not present but is required, building this first before the
>>> main build
>>> Loading cache...done.
>>> Loaded 998 entries from dependency cache.
>>> Parsing recipes...NOTE: DEBUG TO FOLLOW
>>> done.
>>> Parsing of 783 .bb files complete (780 cached, 3 parsed). 1000 targets,
>>> 11 skipped, 0 masked, 0 errors.
>>>
>>> As you can see, even with -DDDD, the message never makes it to the
>>> console. So I agree that bb.note() is inappropriate, unfortunately,
>>> bb.debug doesn't appear to be working. I believe it was yesterday...
>>
>> The result was cached (780 cached). Try that command after "touch
>> conf/local.conf" and it will be in there.
> 
> Sorry, the note did appear though.
> 
> That needs looking into and is possibly a logging problem. Needs fixing
> as a separate issue though and use of bb.note there isn't the right
> approach.


OK, I haven't completely lost it then :-) In that case, are you looking
for any additional changes/dialog/acks here? If not, the version with
bb.debug() is now in the contrib branch:

  git://git.pokylinux.org/poky-contrib dvhart/oe/u-boot
  http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=dvhart/oe/u-boot

I can resend as V3 if more discussion is desirable.
Richard Purdie - May 27, 2011, 3:37 p.m.
On Fri, 2011-05-27 at 08:13 -0700, Darren Hart wrote:
> OK, I haven't completely lost it then :-) In that case, are you looking
> for any additional changes/dialog/acks here? If not, the version with
> bb.debug() is now in the contrib branch:
> 
>   git://git.pokylinux.org/poky-contrib dvhart/oe/u-boot
>   http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=dvhart/oe/u-boot
> 
> I can resend as V3 if more discussion is desirable.

No, we're good.

Both patches merged to master.

Cheers,

Richard

Patch

diff --git a/meta/recipes-bsp/uboot/u-boot.inc b/meta/recipes-bsp/uboot/u-boot.inc
index 058e3ba..3ffec21 100644
--- a/meta/recipes-bsp/uboot/u-boot.inc
+++ b/meta/recipes-bsp/uboot/u-boot.inc
@@ -11,7 +11,15 @@  PARALLEL_MAKE=""
 # GCC 4.5.1 builds unusable binaries using -Os, remove it from OPTFLAGS
 EXTRA_OEMAKE = "CROSS_COMPILE=${TARGET_PREFIX} OPTFLAGS='-O2'"
 
-UBOOT_MACHINE ?= "${MACHINE}_config"
+python () {
+	if not d.getVar("UBOOT_MACHINE", True):
+		PN = d.getVar("PN", True)
+		FILE = os.path.basename(d.getVar("FILE", True))
+		bb.note("To build %s, see %s for instructions on setting up \
+			 your machine config" % (PN, FILE))
+		raise bb.parse.SkipPackage("because UBOOT_MACHINE is not set")
+}
+
 UBOOT_IMAGE ?= "u-boot-${MACHINE}-${PV}-${PR}.bin"
 UBOOT_SYMLINK ?= "u-boot-${MACHINE}.bin"
 UBOOT_MAKE_TARGET ?= "all"
diff --git a/meta/recipes-bsp/uboot/u-boot_git.bb b/meta/recipes-bsp/uboot/u-boot_git.bb
index 4c8f5df..0fbb9ba 100644
--- a/meta/recipes-bsp/uboot/u-boot_git.bb
+++ b/meta/recipes-bsp/uboot/u-boot_git.bb
@@ -1,5 +1,11 @@ 
 require u-boot.inc
 
+# To build u-boot for your machine, provide the following lines in your machine
+# config, replacing the assignments as appropriate for your machine.
+# UBOOT_MACHINE = "omap3_beagle_config"
+# UBOOT_ENTRYPOINT = "0x80008000"
+# UBOOT_LOADADDRESS = "0x80008000"
+
 LICENSE = "GPLv2+"
 LIC_FILES_CHKSUM = "file://COPYING;md5=1707d6db1d42237583f50183a5651ecb \
                     file://README;beginline=1;endline=22;md5=3a00ef51d3fc96e9d6c1bc4708ccd3b5"
@@ -12,11 +18,6 @@  PR="r3"
 
 SRC_URI = "git://git.denx.de/u-boot.git;branch=master;protocol=git"
 
-UBOOT_MACHINE_beagleboard = "omap3_beagle_config"
-UBOOT_MACHINE_overo = "omap3_overo_config"
-
 S = "${WORKDIR}/git"
 
 PACKAGE_ARCH = "${MACHINE_ARCH}"
-
-COMPATIBLE_MACHINE = "(beagleboard|overo)"