New issue
Advanced search Search tips

Issue 775141 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

cros_config_host_py get-firmware-uris returns incorrect result for coral

Project Member Reported by sjg@chromium.org, Oct 16 2017

Issue description

cros_config_host_py /build/coral/usr/share/chromeos-config/config.dtb --all-models get-firmware-uris
gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-coral/Coral_EC.9984.0.0.tbz2 gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-coral/Coral.9984.0.0.tbz2
gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-astronaut/Coral_EC.9984.0.0.tbz2 gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-astronaut/Coral.9984.0.0.tbz2
gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-blue/Coral_EC.9984.0.0.tbz2 gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-blue/Coral.9984.0.0.tbz2
gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-bruce/Coral_EC.9984.0.0.tbz2 gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-bruce/Coral.9984.0.0.tbz2
gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-lava/Coral_EC.9984.0.0.tbz2 gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-lava/Coral.9984.0.0.tbz2
gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-nasher/Coral_EC.9984.0.0.tbz2 gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-nasher/Coral.9984.0.0.tbz2
gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-nasher360/Coral_EC.9984.0.0.tbz2 gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-nasher360/Coral.9984.0.0.tbz2
gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-porbeagle/Coral_EC.9984.0.0.tbz2 gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-porbeagle/Coral.9984.0.0.tbz2
gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-robo/Coral_EC.9984.0.0.tbz2 gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-robo/Coral.9984.0.0.tbz2
gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-robo360/Coral_EC.9984.0.0.tbz2 gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-robo360/Coral.9984.0.0.tbz2
gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-santa/Coral_EC.9984.0.0.tbz2 gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-santa/Coral.9984.0.0.tbz2
gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-whitetip/Coral_EC.9984.0.0.tbz2 gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/chromeos-base/chromeos-firmware-whitetip/Coral.9984.0.0.tbz2

This should only return firmware for coral, which is the only valid firmware. I think it is incorrectly using the model name for the chromeos-firmware-xxx directory.

 
I think this is a problem with CrosConfig.Node.FollowPhandle, it's expanding out firmware phandles that shouldn't be expanded out because it's the wrong model.
Owner: sjg@chromium.org

Comment 3 by sjg@chromium.org, Oct 24 2017

Status: Started (was: Untriaged)
CL here:

https://chromium-review.googlesource.com/c/chromiumos/platform2/+/736509

The impact is cosmetic, but it does try to download non-existent files, which takes a few minutes, so worth fixing.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 29 2017

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

commit 642016411d0f39603e52d0a2bea14205235a81d2
Author: Simon Glass <sjg@chromium.org>
Date: Sun Oct 29 16:28:09 2017

chromeos-config: Correct URI list with shared firmware

This implementation is not actually correct at present. It assumes that
each model with shared firmware has it in its own location. In fact, the
images are truely shared, and exist in one place. Also we should not
return duplicates.

Fix these issues. Part of the problem comes from taking a model-centric
view of the URI list. In fact, the URI list is a global thing across all
models and it does not make much sense to request the URIs of a single
model. We never use that the firmware ebuild, and we are not allowed to
change the value of SRC_URI, so we have to have everything in the list
at all times.

Also tidy up the test data a little as it is confusing to have pyro
using an overlay named 'reef'.

BUG= chromium:775141 
TEST=./run_tests.sh
FEATURES=test sudo -E  emerge chromeos-config-tools
PYTHONPATH=~/c python cros_config_host.py  -c \
	~/c/chroot/build/coral/usr/share/chromeos-config/config.dtb \
	 get-firmware-uris
See that we get the corect set:
gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/\
	chromeos-base/chromeos-firmware-coral/Coral.9984.0.0.tbz2 \
gs://chromeos-binaries/HOME/bcs-coral-private/overlay-coral-private/\
	chromeos-base/chromeos-firmware-coral/Coral_EC.9984.0.0.tbz2
Signed-off-by: Simon Glass <sjg@chromium.org>

Change-Id: Ib691d30953ad7e21146901d90f8e7ff1ef81b596
Reviewed-on: https://chromium-review.googlesource.com/736509
Commit-Ready: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Jason Clinton <jclinton@chromium.org>

[modify] https://crrev.com/642016411d0f39603e52d0a2bea14205235a81d2/chromeos-config/libcros_config/test.dts
[modify] https://crrev.com/642016411d0f39603e52d0a2bea14205235a81d2/chromeos-config/libcros_config_host/libcros_config_host.py
[modify] https://crrev.com/642016411d0f39603e52d0a2bea14205235a81d2/chromeos-config/libcros_config_host/fdt_unittest.py
[modify] https://crrev.com/642016411d0f39603e52d0a2bea14205235a81d2/chromeos-config/libcros_config/cros_config_unittest.cc
[modify] https://crrev.com/642016411d0f39603e52d0a2bea14205235a81d2/chromeos-config/cros_config_host.py
[modify] https://crrev.com/642016411d0f39603e52d0a2bea14205235a81d2/chromeos-config/cros_config_main_unittest.cc
[modify] https://crrev.com/642016411d0f39603e52d0a2bea14205235a81d2/chromeos-config/libcros_config_host_unittest.py

Comment 5 by sjg@chromium.org, Nov 4 2017

Labels: Unibuild

Comment 6 by sjg@chromium.org, Nov 6 2017

Status: Fixed (was: Started)

Sign in to add a comment