Better customization of Chrome command lines for Chrome-OS-based boards |
|||||
Issue description
we've wanted people to not check for board values when building the chrome command line, but rather push for feature flags. that's fine.
sometimes though, you really have board-specific junk, and feature flags don't quite cut it. the code reflects this:
libchromeos-ui/chromeos/ui/chromium_command_builder.cc: const bool is_atom = IsBoard("x86-alex") || IsBoard("x86-alex_he") ||
libchromeos-ui/chromeos/ui/chromium_command_builder.cc: IsBoard("x86-mario") || IsBoard("x86-zgb") || IsBoard("x86-zgb_he");
libchromeos-ui/chromeos/ui/chromium_command_builder.cc: if (IsBoard("link") || IsBoard("link_freon")) {
login_manager/chrome_setup.cc: if (builder->IsBoard("kevin") || builder->IsBoard("kevin-tpm2")) {
login_manager/chrome_setup.cc: if (builder->IsBoard("veyron_minnie"))
login_manager/chrome_setup.cc: if (builder->IsBoard("veyron_rialto")) {
login_manager/chrome_setup.cc: if (builder->IsBoard("veyron_mickey"))
maybe we should capitulate slightly here and add support for an /etc/chrome_board.conf file that boards can install. then we can purge the IsBoard func entirely from the source tree.
,
Sep 1 2016
is_atom is really just a proxy for "the CPU in this board is slow". we could change it to something like "disable_low_latency_audio", but those boards are EOL and dying hopefully soonish.
link_freon is dead and can be cleaned.
renaming the rialto board check to USE=rialto is semantics. the rialto board is the only one who wants that specific set of USE flags ... it's not like it's controlling one or two.
however, we can add as many/few USE flags as we want, but it still won't help -- the starting point for this was that we have people using CrOS to do their own builds and they don't have a way of customizing things cleanly. the recommendation is to do it dirty and have the overlay's scripts/board_specific_setup.sh:board_finalize_base_image() function hack up the "${root_fs_dir}/etc/chrome_dev.conf" file. at least one board in the tree is already doing this.
,
Sep 1 2016
Re rialto, their intent (as I understand it) is to remove this customization and use kiosk mode -- see https://docs.google.com/document/d/1oj2vZA3smu2yCXAluHnDdqap2791MTW-Sedv9l6DCL8/edit#heading=h.8f5nq2mi1oza. Re checked-in chrome_dev.conf files: yay. My favorite copy is the one that adds switches immediately below the comment saying not to check in modified versions of the file. If this is a frequent use case, I'm fine with finding a cleaner way to support it. I want to do it from the perspective of identifying developers' needs and coming up with a good solution, though. chrome_dev.conf is basically "this thing here looks sort of like what I need so I'll use it." chrome_dev.conf (or renamed files using the same parser) just hack the standard Chrome command line, which can change at any time. Are developers using a stock Chrome binary or their own customized versions? If it's the stock one, why are they okay with landing changes to it upstream but not with editing chrome_setup.cc? Should I start an internal doc to enumerate the existing use cases?
,
Sep 1 2016
i grok that rialto wants to drop the code entirely and use the standard enterprise/kiosk modes. but i think that is a good example of every once in a while, a board needs an escape hatch. i don't think inserting that logic directly into platform2/ makes sense, regardless of it being a board or USE flag. we allowed it because we have closeish ties with the rialto people, but it's not the situation we want to be in forever. we're talking about external people using existing flags -- they won't generally be landing any code into CrOS. i don't think we want to force them to modify the platform2 code just to pass in their own flags. yes, Chromium changes, and that might cause them problems eventually, but that's not our problem :).
,
Sep 2 2016
I've convinced myself that I'm okay with this approach as long as it's only used for external, not-checked-in overlays. I don't want our internal command lines to end up scattered across many different repositories. I'm tempted to enforce that with a presubmit check, if I can think of a way to do so... :-P
,
Sep 2 2016
we can make an autotest to enforce it let's consider the rialto case though ... i'm not convinced it's better to have the list hardcoded in the common source
,
Sep 2 2016
You mean something rialto-like that continues using custom flags over the long term, right? I guess I'm still hung up on the suspicion that it's rare to need to set a bunch of custom flags but not touch the Chrome code that implements those flags. I suspect that a more common scenario is that the flags are being implemented in Chrome specifically for these boards.
,
Sep 2 2016
i mean even in the short to mid term for rialto. if a Google board wants to spin up quickly, having to throw hacks into common code is ... not ideal. we're still cleaning up hacks from tegra2 boards (i just posted a CL today for it).
,
Sep 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/08b3532995d999c14e49a871b49daadb52ef4ffe commit 08b3532995d999c14e49a871b49daadb52ef4ffe Author: Daniel Erat <derat@chromium.org> Date: Thu Sep 01 20:25:48 2016 Add pineview USE flag to x86-{alex,mario,zgb}. Introduce a pineview USE flag for systems with early Atom CPUs, intended to prevent the need for a hardcoded board list in libchromeos-ui. BUG= chromium:643323 TEST=none Change-Id: I1caa238ff7517cc0cf9d8badea8efa32bf9629c7 Reviewed-on: https://chromium-review.googlesource.com/380255 Commit-Ready: Dan Erat (ooo friday) <derat@chromium.org> Tested-by: Dan Erat (ooo friday) <derat@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/08b3532995d999c14e49a871b49daadb52ef4ffe/overlay-x86-mario/make.conf [modify] https://crrev.com/08b3532995d999c14e49a871b49daadb52ef4ffe/overlay-x86-alex/make.conf [modify] https://crrev.com/08b3532995d999c14e49a871b49daadb52ef4ffe/overlay-x86-zgb/make.conf
,
Sep 4 2017
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. If you change it back, also remove the "Hotlist-Recharge-Cold" label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 1
,
Sep 14
i think we've fixed this now |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by derat@chromium.org
, Sep 1 2016