Added support for boards with only private overlays.

Added support for boards with only private overlays.

If a board has a public overlay it will be used as the primary overlay.
However, with this change, if a board only has a private overlay it will
be used as the primary.

BUG=7339
TEST=Ran setup_board, build_packages, and build_image on a board with only
a private overlay (locally created). Ran setup_board, build_packages, and
build_imge on a board with both public and private overlays.

Change-Id: Ib5de567ba1c62047969a20f3bea5e1db2bda020e

Related Issues:
http://codereview.chromium.org/3544009
http://codereview.chromium.org/3622003

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=01fed21

[Dave Parker, 2010-10-05 20:37:01]

Related Issues:
http://codereview.chromium.org/3544009
http://codereview.chromium.org/3622003

[robotboy, 2010-10-05 20:41:12]

It might make sense to put this functionality in the cros_overlay_list utility
and add a --primary-only option to it.  That would reduce code duplication and
centralize the logic of finding the overlays even further.

[kliegs, 2010-10-05 20:49:10]

I would agree that this should have a distinct flag for specifying which.

I'd probably be ok with a fatal error being printed that "Two possible
overlays exist for board XXX" and requiring disambiguation.

If this change goes forward I'd like to see the logic moved into a common
function however.  Having this logic split will likely cause confusion
lately.

And as a nit, I'd recommend the error message to occur outside the main if
clause and just check if BOARD_OVERLAY was set.  This way the error doesn't
risk being lost if more cases are added to the if statement.

On Tue, Oct 5, 2010 at 4:41 PM, <robotboy@chromium.org> wrote:

> It might make sense to put this functionality in the cros_overlay_list
> utility
> and add a --primary-only option to it.  That would reduce code duplication
> and
> centralize the logic of finding the overlays even further.
>
>
> http://codereview.chromium.org/3571015/show
>

[robotboy, 2010-10-05 21:06:00]

On 2010/10/05 20:49:10, kliegs wrote:
> I would agree that this should have a distinct flag for specifying which.
> 
> I'd probably be ok with a fatal error being printed that "Two possible
> overlays exist for board XXX" and requiring disambiguation.
> 

I actually meant for the --primary-only flag to cros_overlay_list to cause that
utility to only output a single overlay, the one that will be used as the
primary.  That would let both of these changes just call that utility with that
flag.  The error message could also be generated from cros_overlay_list.  I
didn't make that change when I wrote cros_overlay_list because I didn't
understand exactly what build_packages and setup_board were assuming.

> If this change goes forward I'd like to see the logic moved into a common
> function however.  Having this logic split will likely cause confusion
> lately.
> 

That's what I meant to say with my second sentence above.  :)

> And as a nit, I'd recommend the error message to occur outside the main if
> clause and just check if BOARD_OVERLAY was set.  This way the error doesn't
> risk being lost if more cases are added to the if statement.
>

[dparker, 2010-10-06 04:57:39]

Thanks for the feedback. I agree that the same logic to identify the primary
board overlay is repeated in too many places. I'll add the --primary-only
flag to cros_overlay_list, as suggested, and alter setup_board and
build_packages to use it. However, unless there are objections, I'll leave
parallel_emerge (python) alone. I don't like the idea of python invoking a
shell script unless it is absolutely necessary.

-Dave

On Tue, Oct 5, 2010 at 2:06 PM, <robotboy@chromium.org> wrote:

> On 2010/10/05 20:49:10, kliegs wrote:
>
>> I would agree that this should have a distinct flag for specifying which.
>>
>
>  I'd probably be ok with a fatal error being printed that "Two possible
>> overlays exist for board XXX" and requiring disambiguation.
>>
>
>
> I actually meant for the --primary-only flag to cros_overlay_list to cause
> that
> utility to only output a single overlay, the one that will be used as the
> primary.  That would let both of these changes just call that utility with
> that
> flag.  The error message could also be generated from cros_overlay_list.  I
> didn't make that change when I wrote cros_overlay_list because I didn't
> understand exactly what build_packages and setup_board were assuming.
>
>
>  If this change goes forward I'd like to see the logic moved into a common
>> function however.  Having this logic split will likely cause confusion
>> lately.
>>
>
>
> That's what I meant to say with my second sentence above.  :)
>
>
>  And as a nit, I'd recommend the error message to occur outside the main if
>> clause and just check if BOARD_OVERLAY was set.  This way the error
>> doesn't
>> risk being lost if more cases are added to the if statement.
>>
>
>
>
> http://codereview.chromium.org/3571015/show
>

[Dave Parker, 2010-10-06 23:19:37]

I've centralized logic for selecting a board's primary overlay into
cros_overlay_list (per feedback).

Related CL: http://codereview.chromium.org/3622003

[robotboy, 2010-10-06 23:24:59]

LGTM, assuming the check for the existence of the toolchain.conf file made it's
way to cros_overlay_list.

[Dave Parker, 2010-12-01 19:18:33]

On 2010/10/06 23:24:59, robotboy wrote:
> LGTM, assuming the check for the existence of the toolchain.conf file made
it's
> way to cros_overlay_list.

It does. A board overlay must have a make.conf and a toolchain.conf to be
considered as a primary overlay by cros_overlay_list.

[robotboy, 2010-12-01 19:21:20]

On 2010/12/01 19:18:33, Dave Parker wrote:
> On 2010/10/06 23:24:59, robotboy wrote:
> > LGTM, assuming the check for the existence of the toolchain.conf file made
> it's
> > way to cros_overlay_list.
> 
> It does. A board overlay must have a make.conf and a toolchain.conf to be
> considered as a primary overlay by cros_overlay_list.

LGTM, thanks.