add unittest for overlay include order, use flags, rather than unsetting use flags |
|||
Issue descriptionCurrently "target-chrome-os" overrides a bunch of use flags which are not required to be disabled. Historically, it seems the intention was to "test" that the use flags were set in baseboard rather than overlay. However they frequently cause unexpected build output and unnecessary debugging, since test "failure" simply reports success, but causes defective build output. This has wasted many hours of debug time across a number of projects. If a test is desired, an actual CQ or presubit test is needed, with an obvious failure message. USE flags shouldn't be unset silently for "test" purposes. See: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/master/profiles/targets/chromeos/make.defaults * We should remove all unsets from this file. 03:30PM Component: ChromeOS > Packages
,
May 14 2018
As in the initial bug description, the request is to drop USE=-eclog in make.defaults associated with this target. Sounds like we agree.
,
May 14 2018
nowhere in this bug or in b/79598191 did you mention "eclog". you only ever talked in large generalities, none of which we wanted. Bob already posted https://chromium-review.googlesource.com/1053594 to drop -eclog from the make.defaults, but he kept trying to make unnecessary ebuild changes at the same time. also, i think i've been pretty clear both in the CLs and in the bugs: dropping -eclog from make.defaults is a workaround, pure and simple. the meowth overlays are broken and must be fixed. treating CL:1053594 as the "correct" fix is ignoring the fundamental breakage the boards continue to have.
,
May 14 2018
The generalized point of this bug is that setting USE flags in one overlay vs another is not "broken", but rather didn't conform to your style preferences. If you want to enforce coding style, adding silent overrides in profiles/targets/chromeos/make.defaults is not the correct way to do it.
,
May 15 2018
if you don't understand how the CrOS system profiles come together, and aren't familiar with the history (and who actually wrote these things), then don't deflect behind claims that i'm enforcing arbitrary style preferences. it has nothing to do with that, and all you're doing is pointing fingers and wasting everyone's time. in fact, i've never once set eclog variable or told anyone where to set them. in fact, when that line was added to the profile 2 years ago, i commented at that time "the make.defaults line isn't really needed, but it also doesn't hurt". but the author opted to go that route and i didn't tell them to do it one way or the other. on the other hand, this issue does have to do with the right preference/stacking order so that we have a shared baseline architecture, a shared baseline OS (linux), a shared baseline distro (CrOS), a shared chipset, a shared baseboard, and finally the leaf overlays. it avoids a ton of duplication of code and so people, when they want to update something at say the distro level, they don't have to copy & paste the same change to well over 100 files. however, because the meowth overlays are written wrong, they directly declare that they want the chromeos settings to have a higher precedence. the authors of those overlays made a mistake, and none of it has to do with "style". as for "being silent", i'm not really sure what you reasonably expect here. you want every flag/variable that gets overridden in every single file to generate a log warning on every emerge command ? that's ridiculous. so please stop these pointless blame games.
,
May 15 2018
Mike, please chat with me directly if you don't understand what this bug is about. My understanding is that you have a preference for overlay stacking order and where use flags are set. My request is that you write an explicit unittest for this in the chromeos pre-CQ or somewhere appropriate. This will keep the overlays clean and ordered the way you like (and I generally agree with you on this) and easily convey this message to anyone working on the code. I've updated the bug title to hopefully be clearer.
,
May 21 2018
this bug is not salvageable. CrOS has settled on a design with profile stacks. much of it predates me, while the rest has come from ddocs reviewed by the CrOS team. so while it's true "I have a preference", it's one backed with technical reasons, existing art, and what the project has settled on. this bug wasn't filed requesting better tooling, it was to break from all established routes to support a technically unsound board setup. you can star issue 845071 and issue 845072 for tooling improvements. |
|||
►
Sign in to add a comment |
|||
Comment 1 by vapier@chromium.org
, May 14 2018Owner: vapier@chromium.org
Status: WontFix (was: Unconfirmed)