unibuild firmware packaging handles cros_ec flag incorrectly |
||||||||
Issue descriptionSeen on the scarlet-paladin CQ. There were no changes picked up by this build that look like they directly affect scarlet, so this appears to be a bug on ToT/Cloud Storage. It appears that the binary package contents were changed in such a way that the ebuild can no longer pick them up. https://luci-milo.appspot.com/buildbot/chromeos/scarlet-paladin/4326 Log excerpts: === Start output for job chromeos-firmware-scarlet-0.0.1-r106 (0m8.2s) === ... chromeos-firmware-scarlet-0.0.1-r106: INFO:root:RunCommand: sh /build/scarlet/tmp/portage/chromeos-base/chromeos-firmware-scarlet-0.0.1-r106/work/chromeos-firmware-scarlet-0.0.1/chromeos-firmwareupdate --sb_repack /build/scarlet/tmp/portage/chromeos-base/chromeos-firmware-scarlet-0.0.1-r106/temp/tmpaSFQ68.pack_firmware-25319 chromeos-firmware-scarlet-0.0.1-r106: Package Content: ... chromeos-firmware-scarlet-0.0.1-r106: 16809f21e7a566a8ef855bd75f24fba1 *./models/dru/ec.bin ... chromeos-firmware-scarlet-0.0.1-r106: IOError: [Errno 2] No such file or directory: '/build/scarlet/firmware/scarlet/ec.bin'
,
Jun 20 2018
It looks like scarlet does not define USE=cros_ec
Perhaps add this:
USE="${USE} cros_ec"
to:
src/overlays/overlay-scarlet/profiles/base/make.defaults
This should probably be done by the scarlet SIE
,
Jun 20 2018
->Philip who I think is the right person
,
Jun 20 2018
Looks like scarlet was surprise-unibuilded incorrectly without letting the SIE team know. Passing to the build team.
,
Jun 20 2018
Build team TL is vapier@ but I'm pretty sure this has nothing to do with them or unibuild. AFAIK, no one is actively converting boards. Please find a root cause.
,
Jun 20 2018
Looks like scarlet was migrated by Charles before he left. As per previous discussion w/ Charles he owns build maintenance on migrated systems.
,
Jun 21 2018
After switching to unibuild, why would chromeos-firmware-scarlet depend on locally-built firmware artifacts? chromeos-firmware-scarlet-0.0.1-r106: IOError: [Errno 2] No such file or directory: '/build/scarlet/firmware/scarlet/ec.bin' Actually, I've asked the same question before but didn't get answered: https://b.corp.google.com/issues/80497318#comment5 I can probably resolve this by adding the USE flag to build chromeos-ec. But please let me know why we need this dependency. chromeos-firmware-${BOARD} should only pack the firmware binaries downloaded from BCS.
,
Jun 21 2018
ec build targets get removed after awhile in TOT so the BCS based firmware packager must not rely on the presence of a locally built ec.
,
Jun 21 2018
Look like the problem is in https://cs.corp.google.com/chromeos_public/src/third_party/chromiumos-overlay/eclass/cros-firmware.eclass?l=309 unibuild implementation, which does not handle cros_ec USE flag correctly
,
Jun 21 2018
This issue will mostly be 'fixed' after this patch lands: https://chromium-review.googlesource.com/c/chromiumos/overlays/board-overlays/+/1109378 But in the long run, we'll need a real fix as #9 described.
,
Jun 21 2018
bumping priority down as the builder should be fixed with the above workaround. vapier@ to fix or reassign build support for the root cause. +cc sjg@ as well as he wrote the code.
,
Jun 21 2018
So I gather that the fix is to skip EC and PD from the local firmware build if USE=cros_ec is not set. I can do that.
,
Jun 21 2018
I think that will do it, yes, but I'm not too familiar with that code. Thanks!
,
Jun 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/7c2e9229eb139d218e8eab06d7ab81f0ee741c16 commit 7c2e9229eb139d218e8eab06d7ab81f0ee741c16 Author: Nick Sanders <nsanders@chromium.org> Date: Thu Jun 21 08:05:51 2018 overlay-scarlet: add cros_ec USE flag. scarlet was unibuilded incorrectly. This temporarily works around an apparent bug in the unibuild implementation of chromeos-firmware that requires a source built ec binary in order to package the one from bcs. BUG= chromium:854764 TEST=./build_packages Change-Id: I142042296be4ddd9e36fafe1af48ac59b7b38c30 Reviewed-on: https://chromium-review.googlesource.com/1109378 Commit-Ready: Philip Chen <philipchen@chromium.org> Tested-by: Nick Sanders <nsanders@chromium.org> Reviewed-by: Philip Chen <philipchen@chromium.org> [modify] https://crrev.com/7c2e9229eb139d218e8eab06d7ab81f0ee741c16/overlay-scarlet/profiles/base/make.defaults
,
Jun 21 2018
,
Jun 21 2018
,
Jun 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/firmware/+/cab05a6fc1b2fa84169ae9e423884807c4001d85 commit cab05a6fc1b2fa84169ae9e423884807c4001d85 Author: Simon Glass <sjg@chromium.org> Date: Fri Jun 22 02:56:19 2018 Allow building local firmware without EC and PD At present we require the EC and PD to be included in a local firmware build. But at the eclass level this is actually controlled by a USE flag (cros_ec). In that case we don't want to include these binaries. Update the logic and comments to support this possibility. There is no test coverage for this case. BUG= chromium:854764 TEST=with a change to cros-firmware.eclass: emerge-scarlet --unmerge chromeos-ec FEATURES=test USE=-cros_ec emerge-scarlet chromeos-firmware-scarlet See that this happily builds a local firmware image now and tests pass Also: FEATURES=test USE=-bootimage emerge-scarlet chromeos-firmware-scarlet See that it does not build a local image, tests still pass Change-Id: Ie59ca201906dbd947a0fb6db1380675d8a357fa4 Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1110025 Reviewed-by: Philip Chen <philipchen@chromium.org> Reviewed-by: Nick Sanders <nsanders@chromium.org> [modify] https://crrev.com/cab05a6fc1b2fa84169ae9e423884807c4001d85/pack_firmware.py
,
Jun 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/b3af86b8283221d72ce08926b2eb4fc90845ee26 commit b3af86b8283221d72ce08926b2eb4fc90845ee26 Author: Simon Glass <sjg@chromium.org> Date: Fri Jun 22 21:13:21 2018 cros-firmware: Support local firmware package without EC At present with unibuild we require that the EC and PD iamges be present in order to build a firmware image based on locally built binaries (as opposed to 'official' ones downloaded from BCS). Prior to unibuild the cros_ec USE flag was used to control this. Add this same behaviour for unibuild. This allows chromeos-firmware-scarlet to build. The problem was not noticed for a long time since EC binaries were still present from before it was converted to unibuild. Note that we provide both EC and PD filenames to pack_firmware.py but it checks the master config to see which ones are needed. For example, it will only include the PD binary if the config has a 'pd-image' property. If there is no 'pd-image' then the PD filename provided will be ignored, and it is fine to provide is anyway. BUG= chromium:854764 TEST=with go/croscl/1110025: emerge-scarlet --unmerge chromeos-ec FEATURES=test USE=-cros_ec emerge-scarlet chromeos-firmware-scarlet See that this happily builds a local firmware image now and tests pass Change-Id: Ideab9531c91b7c23f00d857d38ffdc09f870c2eb Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1110029 Reviewed-by: Nick Sanders <nsanders@chromium.org> [modify] https://crrev.com/b3af86b8283221d72ce08926b2eb4fc90845ee26/eclass/cros-firmware.eclass
,
Jun 25 2018
,
Jun 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/30593cb1185ba36c9f6a85cdbebdb67002188910 commit 30593cb1185ba36c9f6a85cdbebdb67002188910 Author: Simon Glass <sjg@chromium.org> Date: Tue Jun 26 19:52:00 2018 Revert "overlay-scarlet: add cros_ec USE flag." This reverts commit 7c2e9229eb139d218e8eab06d7ab81f0ee741c16. Reason for revert: Now that crbug.com/854764 is fixed, we should not need this? Original change's description: > overlay-scarlet: add cros_ec USE flag. > > scarlet was unibuilded incorrectly. This temporarily works around > an apparent bug in the unibuild implementation of chromeos-firmware > that requires a source built ec binary in order to package the one > from bcs. > > BUG= chromium:854764 > TEST=./build_packages > > Change-Id: I142042296be4ddd9e36fafe1af48ac59b7b38c30 > Reviewed-on: https://chromium-review.googlesource.com/1109378 > Commit-Ready: Philip Chen <philipchen@chromium.org> > Tested-by: Nick Sanders <nsanders@chromium.org> > Reviewed-by: Philip Chen <philipchen@chromium.org> Bug: chromium:854764 Change-Id: I9f8a99d42864c86577bc5047a6a6bb4df5d90504 Reviewed-on: https://chromium-review.googlesource.com/1113778 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Tested-by: Simon Glass <sjg@chromium.org> Reviewed-by: Nick Sanders <nsanders@chromium.org> [modify] https://crrev.com/30593cb1185ba36c9f6a85cdbebdb67002188910/overlay-scarlet/profiles/base/make.defaults
,
Jun 26 2018
thanks Simon! |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by sjg@chromium.org
, Jun 20 2018