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

Issue 823605 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

build_image: udevadm hwdb does not report errors

Project Member Reported by drinkcat@chromium.org, Mar 20 2018

Issue description

When doing build_image, we run this command to update /etc/udev/hwdb.bin (build_library/build_image_util.sh, run_udevadm_hwdb):
sudo udevadm hwdb --update -r /build/poppy

Howver, when there is a syntax error in any of the /etc/udev/hwdb.d/ files, the command just prints an error, but still returns an exit code 0, and the build_image process continues (see https://b.corp.google.com/issues/63982780#comment10, for example).

Ideally, we should make sure that udevadm returns a non-zero exit code in this case. Alternatively, we could just just make sure udevadm does not print anything to stderr.
 
Upstream (https://github.com/systemd/systemd/pull/8520) suggests using util/parse_hwdb.py:

- Cherry-pick https://chromium-review.googlesource.com/#/c/chromiumos/overlays/portage-stable/+/974721
- Then run:
PYTHON_TARGETS="python2_7 python3_3" sudo emerge pyparsing
python3.3 parse_hwdb.py /build/poppy/lib/udev/hwdb.d/*

The problem is that:
 1. parse_hwdb.py is only available after udev v225 that we use
 2. It fails on many hwdb files (too strict?), for example, it even fails on the upstream version of 20-acpi-vendor.hwdb:
Cannot parse 20-acpi-vendor.hwdb: Expected stringEnd (at char 188), (line:9, col:1)
20-acpi-vendor.hwdb: 0 match groups, 0 matches, 0 properties

I'm still tempted to make udevadm hwdb --update more strict, I'll refine the patch and update upstream.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/portage-stable/+/7137da63ea85978f33c1d5e38f97177f542e23b5

commit 7137da63ea85978f33c1d5e38f97177f542e23b5
Author: Nicolas Boichat <drinkcat@google.com>
Date: Thu Mar 22 06:23:39 2018

pyparsing: upgraded package to upstream

Upgraded dev-python/pyparsing to version 2.2.0 on amd64.

Added back PYTHON_TARGETS="python3_3" that was dropped upstream.

BUG= chromium:823605 
TEST=PYTHON_TARGETS="python2_7 python3_3" sudo emerge pyparsing
     python3.3 parse_hwdb.py \
          /build/poppy/etc/udev/hwdb.d/61-hammer-keyboard.hwdb
     works

Change-Id: I062ce3b9a21b2c3affc916431f272ccad7d24ae3
Reviewed-on: https://chromium-review.googlesource.com/974721
Commit-Ready: Nicolas Boichat <drinkcat@chromium.org>
Tested-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[add] https://crrev.com/7137da63ea85978f33c1d5e38f97177f542e23b5/dev-python/pyparsing/pyparsing-2.2.0.ebuild
[add] https://crrev.com/7137da63ea85978f33c1d5e38f97177f542e23b5/dev-python/pyparsing/files/pyparsing-2.2.0-distutils.patch
[add] https://crrev.com/7137da63ea85978f33c1d5e38f97177f542e23b5/metadata/md5-cache/dev-python/pyparsing-2.2.0
[modify] https://crrev.com/7137da63ea85978f33c1d5e38f97177f542e23b5/dev-python/pyparsing/Manifest
[modify] https://crrev.com/7137da63ea85978f33c1d5e38f97177f542e23b5/dev-python/pyparsing/metadata.xml
[delete] https://crrev.com/13d7ceadae341f6d53e71f1bafa3a3cda6ee759d/metadata/md5-cache/dev-python/pyparsing-2.0.1
[delete] https://crrev.com/13d7ceadae341f6d53e71f1bafa3a3cda6ee759d/dev-python/pyparsing/pyparsing-2.0.1.ebuild

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/1f7d049a7af8f783e93c757d992e7ff78f336e4a

commit 1f7d049a7af8f783e93c757d992e7ff78f336e4a
Author: Nicolas Boichat <drinkcat@google.com>
Date: Thu Mar 22 10:01:21 2018

udev: Make udevadm hwdb --update return non-zero exit code on error

When doing build_image, we run this command to update /etc/udev/hwdb.bin
(build_library/build_image_util.sh, run_udevadm_hwdb):
sudo udevadm hwdb --update -r /build/poppy

However, this does not detect syntax errors in the hwdb file (as it
should), as udevadm hwdb always returns a zero error code.

CQ-DEPEND=CL:970053
BUG= chromium:823605 
TEST=Using invalid hwdb file on poppy:
     sudo udevadm hwdb --update -r /build/poppy; echo $?
     shows 1 (and 0 on soraka where there is extra hwdb file)

Change-Id: I87ac62dc30b8391f262f2957f1b149cce4dec3ef
Reviewed-on: https://chromium-review.googlesource.com/970051
Commit-Ready: Nicolas Boichat <drinkcat@chromium.org>
Tested-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[rename] https://crrev.com/1f7d049a7af8f783e93c757d992e7ff78f336e4a/sys-fs/udev/udev-225-r9.ebuild
[add] https://crrev.com/1f7d049a7af8f783e93c757d992e7ff78f336e4a/sys-fs/udev/files/udev-225-udevadm-hwdb-Return-non-zero-exit-code-on-error.patch

Status: Fixed (was: Assigned)
Labels: -Pri-1 Pri-2
Status: Started (was: Fixed)
Upstream has requested to add a --strict flag, and the patch was updated. We should refresh the patch and modify our build_image script accordingly.

I'll follow up once the upstream PR is accepted (so I do not need to do multiple refreshes).
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/2d8db016d3526d9c26018dfbc0b1e6331889531b

commit 2d8db016d3526d9c26018dfbc0b1e6331889531b
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Tue Apr 24 13:22:23 2018

udev: Update udevadm hwdb --strict patch from upstream

When doing build_image, we run this command to update /etc/udev/hwdb.bin
(build_library/build_image_util.sh, run_udevadm_hwdb):
sudo udevadm hwdb --update -r /build/poppy

However, this does not detect syntax errors in the hwdb file (as it
should), as udevadm hwdb always returns a zero error code.

Upstream requested to only change the return code when a new --strict
parameter is passed to the command.

CQ-DEPEND=If4c36cf0c71ae1d0b19d50fcbdae55cf6ce95e3e
BUG= chromium:823605 
TEST=Using invalid hwdb file on poppy:
         sudo udevadm hwdb --strict --update -r /build/poppy; echo $?
         shows 1 (and 0 on soraka where there is no extra hwdb file)

Change-Id: I1244690397b7e812a514454b640cfecb261c1dd8
Reviewed-on: https://chromium-review.googlesource.com/1023356
Commit-Ready: Nicolas Boichat <drinkcat@chromium.org>
Tested-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[rename] https://crrev.com/2d8db016d3526d9c26018dfbc0b1e6331889531b/sys-fs/udev/udev-225-r10.ebuild
[modify] https://crrev.com/2d8db016d3526d9c26018dfbc0b1e6331889531b/sys-fs/udev/files/udev-225-udevadm-hwdb-Return-non-zero-exit-code-on-error.patch

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/crosutils/+/95277f59f82bcf2b194ee477672e61978dbcaeed

commit 95277f59f82bcf2b194ee477672e61978dbcaeed
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Tue Apr 24 13:22:21 2018

build_image: Add --strict parameter to udevadm hwdb update

This makes sure that build_image fails if the hwdb rules are not
formed properly.

CQ-DEPEND=I1244690397b7e812a514454b640cfecb261c1dd8
BUG= chromium:823605 
TEST=build_image succeeds on all boards

Change-Id: If4c36cf0c71ae1d0b19d50fcbdae55cf6ce95e3e
Reviewed-on: https://chromium-review.googlesource.com/1023773
Commit-Ready: Nicolas Boichat <drinkcat@chromium.org>
Tested-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/95277f59f82bcf2b194ee477672e61978dbcaeed/build_library/build_image_util.sh

Status: Verified (was: Started)

Sign in to add a comment