New issue
Advanced search Search tips

Issue 853801 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

cros-config-host: doesn't parse single value for firmware/extra

Project Member Reported by la...@chromium.org, Jun 18 2018

Issue description

From YH:

---
extra = "bcs://Fizz.10139.129.0.tbz2";

 * Validating 6 files:
/build/fizz//usr/share/models/baseboard/model.dtsi:
/chromeos/family/firmware/fizz_129ro_129rw: 'extra' value 'b' does not match pattern '^(\${(FILESDIR|SYSROOT)}/[a-z/]+)|(bcs://[A-Za-z0-9\.]+\.tbz2)$'
/chromeos/family/firmware/fizz_129ro_129rw: 'extra' value 'c' does not match pattern '^(\${(FILESDIR|SYSROOT)}/[a-z/]+)|(bcs://[A-Za-z0-9\.]+\.tbz2)$'
/chromeos/family/firmware/fizz_129ro_129rw: 'extra' value 's' does not match pattern '^(\${(FILESDIR|SYSROOT)}/[a-z/]+)|(bcs://[A-Za-z0-9\.]+\.tbz2)$'
/chromeos/family/firmware/fizz_129ro_129rw: 'extra' value ':' does not match pattern '^(\${(FILESDIR|SYSROOT)}/[a-z/]+)|(bcs://[A-Za-z0-9\.]+\.tbz2)$'
/chromeos/family/firmware/fizz_129ro_129rw: 'extra' value '/' does not match pattern '^(\${(FILESDIR|SYSROOT)}/[a-z/]+)|(bcs://[A-Za-z0-9\.]+\.tbz2)$'
---

It appears that it is iterating the string characters instead of the single list string.
 
Cc: dnojiri@chromium.org
Since we are using only one directory, a workaround is to specify that directory twice. As example, https://chrome-internal-review.googlesource.com/c/chromeos/overlays/baseboard-fizz-private/+/641808.

RE: #1

We updated the CL, the workaround was there in patchset 4.
Project Member

Comment 4 by bugdroid1@chromium.org, Jun 21 2018

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

commit 711050c0e4b227bf15f88b4df80f4dc5b9517f2a
Author: Simon Glass <sjg@chromium.org>
Date: Thu Jun 21 23:31:31 2018

chromeos-config: Correct impl of single-element string list

At present a string list with a single element is treated as a single
string in the device-tree implementation. This is not correct. It should
be a list with a single element.

Correct this and add a test to cover this case for both FDT and yaml
impls. The test uses the GetFirmwareInfo() API since this is the only one
that currently uses string lists.

BUG= chromium:853801 
TEST=FEATURES=test sudo -E emerge chromeos-config-tools

Change-Id: Icb255fbddc64eb763daf3aaa8f32e4679fd0ca57
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1107844
Reviewed-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: YH Lin <yueherngl@chromium.org>

[modify] https://crrev.com/711050c0e4b227bf15f88b4df80f4dc5b9517f2a/chromeos-config/libcros_config/test.dts
[modify] https://crrev.com/711050c0e4b227bf15f88b4df80f4dc5b9517f2a/chromeos-config/libcros_config/test_build.json
[modify] https://crrev.com/711050c0e4b227bf15f88b4df80f4dc5b9517f2a/chromeos-config/cros_config_host/fdt_unittest.py
[modify] https://crrev.com/711050c0e4b227bf15f88b4df80f4dc5b9517f2a/chromeos-config/cros_config_host/libcros_config_host_fdt.py
[modify] https://crrev.com/711050c0e4b227bf15f88b4df80f4dc5b9517f2a/chromeos-config/cros_config_host/libcros_config_host_unittest.py
[modify] https://crrev.com/711050c0e4b227bf15f88b4df80f4dc5b9517f2a/chromeos-config/libcros_config/test.yaml

Comment 5 by sjg@chromium.org, Jun 25 2018

Status: Fixed (was: Untriaged)
This bug is fixed now. Is there any config change needed to drop duplicate 'extra' properties?
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 19

Labels: merge-merged-factory-nami-10715.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/ed753597535b2c5634d855e0e8ee0999896070ac

commit ed753597535b2c5634d855e0e8ee0999896070ac
Author: Simon Glass <sjg@chromium.org>
Date: Thu Jul 19 02:33:13 2018

chromeos-config: Correct impl of single-element string list

At present a string list with a single element is treated as a single
string in the device-tree implementation. This is not correct. It should
be a list with a single element.

Correct this and add a test to cover this case for both FDT and yaml
impls. The test uses the GetFirmwareInfo() API since this is the only one
that currently uses string lists.

BUG= chromium:853801 
TEST=FEATURES=test sudo -E emerge chromeos-config-tools

Change-Id: Icb255fbddc64eb763daf3aaa8f32e4679fd0ca57
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1107844
Reviewed-by: Jason Clinton <jclinton@chromium.org>
Reviewed-by: YH Lin <yueherngl@chromium.org>
(cherry picked from commit 711050c0e4b227bf15f88b4df80f4dc5b9517f2a)
Reviewed-on: https://chromium-review.googlesource.com/1142124
Reviewed-by: Marco Chen <marcochen@chromium.org>
Commit-Queue: Marco Chen <marcochen@chromium.org>
Tested-by: Marco Chen <marcochen@chromium.org>

[modify] https://crrev.com/ed753597535b2c5634d855e0e8ee0999896070ac/chromeos-config/libcros_config/test.dts
[modify] https://crrev.com/ed753597535b2c5634d855e0e8ee0999896070ac/chromeos-config/libcros_config/test_build.json
[modify] https://crrev.com/ed753597535b2c5634d855e0e8ee0999896070ac/chromeos-config/cros_config_host/fdt_unittest.py
[modify] https://crrev.com/ed753597535b2c5634d855e0e8ee0999896070ac/chromeos-config/cros_config_host/libcros_config_host_fdt.py
[modify] https://crrev.com/ed753597535b2c5634d855e0e8ee0999896070ac/chromeos-config/cros_config_host/libcros_config_host_unittest.py
[modify] https://crrev.com/ed753597535b2c5634d855e0e8ee0999896070ac/chromeos-config/libcros_config/test.yaml

Sign in to add a comment