New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 903433 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
User never visited
Closed: Nov 16
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

cros_config_host get-firmware-build-combinations fails if 'ec' is not defined

Project Member Reported by dlaurie@chromium.org, Nov 8

Issue description

If the firmware  build-targets does not define 'ec':

  firmware: &firmware_common
    bcs-overlay: "overlay-sarien-private"
    build-targets:
      coreboot: "{{$fw-build-name}}"
      depthcharge: "sarien"
      libpayload: "sarien"

Then the coreboot ebuild fails because it will attempt to read both coreboot and ec at the same time.  This is difficult to debug within the coreboot build itself (no errors are printed) but you can see it if you run manually:

cros_config_host -c /mnt/host/source/src/private-overlays/overlay-sarien-private/chromeos-base/chromeos-config-bsp-sarien-private/files/model.yaml get-firmware-build-combinations coreboot,ec
Traceback (most recent call last):
  File "/usr/lib/python-exec/python2.7/cros_config_host", line 9, in <module>
    load_entry_point('cros-config-host==1', 'console_scripts', 'cros_config_host')()
  File "/usr/lib64/python2.7/site-packages/cros_config_host/cros_config_host.py", line 368, in main
    GetFirmwareBuildCombinations(config, opts.components.split(','))
  File "/usr/lib64/python2.7/site-packages/cros_config_host/cros_config_host.py", line 153, in GetFirmwareBuildCombinations
    d = config.GetFirmwareBuildCombinations(targets)
  File "/usr/lib64/python2.7/site-packages/cros_config_host/libcros_config_host_base.py", line 450, in GetFirmwareBuildCombinations
    targets = [device_targets[c] for c in components]
KeyError: 'ec'


The workaround is to define ec pointing to nothing:

  firmware: &firmware_common
    bcs-overlay: "overlay-sarien-private"
    build-targets:
      coreboot: "{{$fw-build-name}}"
      depthcharge: "sarien"
      libpayload: "sarien"
      ec: ""

 
Owner: shapiroc@chromium.org
Owner: evanhernandez@chromium.org
Status: Assigned (was: Unconfirmed)
It's clear cros config host is wrong (in part) because it is not validating that the requested build target exists.

Fixing with this change:
https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1327666

Just a note: there is a question of intended behavior here. You could argue that cros_config_host should die when it's asked for build targets that aren't in the config (albeit with a better error message). In that case, the problem is more with the ebuild than cros_config_host.

For now I think we should just skip the target when it does not exist.
I think the original issue was that the code was assuming there's always an ec build where this fix was to put in an empty string:

 ec: ""

ec field being non-existent should be handled as a proper configuration.
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/4c72f9905ee18301c625cd0bc752a4a14121a2bf

commit 4c72f9905ee18301c625cd0bc752a4a14121a2bf
Author: Evan Hernandez <evanhernandez@chromium.org>
Date: Fri Nov 16 13:01:57 2018

chromeos-config: Check build target exists before reading

GetFirmwareBuildCombinations currently fails when a device
does not specify the requested build target, when in reality
it should just ignore that target (as we do elsewhere in
cros_config_host).

BUG= chromium:903433 
TEST=./run_tests.sh

Change-Id: I3adf8ab8328da1120648aea0df5dcf902d7a097c
Reviewed-on: https://chromium-review.googlesource.com/1327666
Commit-Ready: Evan Hernandez <evanhernandez@chromium.org>
Tested-by: Evan Hernandez <evanhernandez@chromium.org>
Reviewed-by: C Shapiro <shapiroc@chromium.org>

[modify] https://crrev.com/4c72f9905ee18301c625cd0bc752a4a14121a2bf/chromeos-config/cros_config_host/libcros_config_host_base.py
[modify] https://crrev.com/4c72f9905ee18301c625cd0bc752a4a14121a2bf/chromeos-config/cros_config_host/libcros_config_host_unittest.py

Status: Fixed (was: Assigned)

Sign in to add a comment