briannorris@ can you please move this to a new component for ChromeOS Infra?

I don't understand this bug. Afaik the reason this broke the tree is that the ebuild change was pushed (chumped) to ToT without the CQ vetting it. So how would the CQ have helped?

The uprev stage made the ebuild rename, not any user-initiated push/chump.

I guess this is tricky still, b/c the CQ can't exactly reject the chump to the kernel repo...

But still, this problem can occur with no chumping whatsoever; just a new git-tag appears in the repo, and so the next commit to land (and uprev the ebuild) will generate a new name. This name may be invalid.

---

BTW, I didn't actually point to the bug that originated this request. It's here:

https://bugs.chromium.org/p/chromium/issues/detail?id=653672

Hey Aviv, could you find a suitable owner for this?

In what sense was the ebuild name invalid? I.e. where did it fail?

I'm not seeing how the CQ can protect us against git tags that are pushed entirely separately. Maybe what you're asking for is for the CQ failure to be more obvious in such cases?

The new ebuild name was chromeos-kernel-4_4-4.4.21_baseline-r384.ebuild. That violates the "version" aspect of this doc:

https://devmanual.gentoo.org/ebuild-writing/file-format/

What I'm asking is that the ebuild should not be created with an invalid name. I guess that could mean the uprev stage just rejects it; so the git ToT would be updated, but the non -9999 ebuild would not point to it. That would eventually result in CQ failures I suppose, and it would be obvious. (NB: I'm not an expert on Infra / CQ.) At any rate, I think rejecting the uprev and causing CQ failures is better than giving everyone a manifest that can't build.

BTW, if you want to know what the failure looks like, see:

https://groups.google.com/a/google.com/d/msg/chromeos-kernel/5k_7hU6MaxE/NxP6CC4CBQAJ

---

https://uberchromegw.corp.google.com/i/chromiumos.tryserver/builders/no_vmtest_pre_cq​ has started failing with this message:

​11:41:34: INFO: Running cidb query on pid 10170, repr(query) starts with <sqlalchemy.sql.expression.Update object at 0x7f3033202fd0>
Preconditions for the stage successfully met. Beginning to execute stage...
11:41:34: INFO: Running cidb query on pid 10170, repr(query) starts with <sqlalchemy.sql.expression.Update object at 0x7f3033202dd0>
11:41:34: INFO: RunCommand: /b/cbuild/internal_master/chromite/bin/cros_sdk 'PARALLEL_EMERGE_STATUS_FILE=/tmp/tmpKiIopX' 'USE=chrome_internal' 'FEATURES=separatedebug' -- emerge-reef -pegNvq '--with-bdeps=y' '--color=n' virtual/target-os virtual/target-os-dev virtual/target-os-test virtual/target-os-factory virtual/target-os-factory-shim chromeos-base/autotest-all in /b/cbuild/internal_master
11:42:49: ERROR: 
return code: 1; command: /b/cbuild/internal_master/chromite/bin/cros_sdk 'PARALLEL_EMERGE_STATUS_FILE=/tmp/tmpKiIopX' 'USE=chrome_internal' 'FEATURES=separatedebug' -- emerge-reef -pegNvq '--with-bdeps=y' '--color=n' virtual/target-os virtual/target-os-dev virtual/target-os-test virtual/target-os-factory virtual/target-os-factory-shim chromeos-base/autotest-all

Invalid ebuild name: /mnt/host/source/src/third_party/chromiumos-overlay/sys-kernel/chromeos-kernel-4_4/chromeos-kernel-4_4-4.4.21_baseline-r384.ebuild

How about changing chromeos_version or the kernel ebuild itself to enforce that the name is sane.

Why not a change to chromite/lib/portage_util.py to do extra validation on the output from chromeos-version.sh? We have a little bit there already.

Guenter, you happy to tackle this eventually?

I am taking a stab at this, but I don't want to change chromite/lib/portage_util.py - I don't know all valid patterns, and I don't think the risk is worth the gain. Changing the kernel version script is less risky and better anyway since we can use it to replace bad tags with better ones, at least for some bad tags.

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

commit 77938ad08d80f25e02b450780cae8ea2fe633400
Author: Guenter Roeck <groeck@chromium.org>
Date: Thu Nov 30 09:13:13 2017

sys-kernel: Only generate valid kernel versions

When generating a kernel version, the chromeos-version scripts
match any tag which starts with 'v[234].'. This is not really
what we want; only upstream version tags should be returned.
On top of that, a glob is used for both "git describe" and "egrep",
even though only "git describe" takes a glob as parameter and "egrep"
expects a regex pattern.

Provide a more specific glob as well as a regex pattern and use
accordingly. This ensures that the script returns a version number
which is valid for use as ebuild version.

BUG= chromium:653706 
TEST=Build with all supported kernels

Change-Id: I5a2881da4bb2c9dd6b5040bb5c68edf0cd760b11
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/794858
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/77938ad08d80f25e02b450780cae8ea2fe633400/sys-kernel/chromeos-kernel-3_8/files/chromeos-version.sh
[modify] https://crrev.com/77938ad08d80f25e02b450780cae8ea2fe633400/sys-kernel/chromeos-kernel-experimental/files/chromeos-version.sh
[modify] https://crrev.com/77938ad08d80f25e02b450780cae8ea2fe633400/sys-kernel/chromeos-kernel-3_18/files/chromeos-version.sh
[modify] https://crrev.com/77938ad08d80f25e02b450780cae8ea2fe633400/sys-kernel/chromeos-kernel-3_14/files/chromeos-version.sh
[modify] https://crrev.com/77938ad08d80f25e02b450780cae8ea2fe633400/sys-kernel/chromeos-kernel-4_4/files/chromeos-version.sh
[modify] https://crrev.com/77938ad08d80f25e02b450780cae8ea2fe633400/sys-kernel/chromeos-kernel-3_10/files/chromeos-version.sh
[modify] https://crrev.com/77938ad08d80f25e02b450780cae8ea2fe633400/sys-kernel/chromeos-kernel-4_14/files/chromeos-version.sh
[modify] https://crrev.com/77938ad08d80f25e02b450780cae8ea2fe633400/sys-kernel/chromeos-kernel-4_12/files/chromeos-version.sh