Switch SimpleChrome builders to use GN |
||||||||
Issue descriptionNow that issue 558623 and related issues are largely addressed allowing GN to be used for SimpleChrome, we should switch the builders to GN: https://build.chromium.org/p/chromium.chromiumos/builders/ChromiumOS%20amd64-generic%20Compile https://build.chromium.org/p/chromium.chromiumos/builders/ChromiumOS%20daisy%20Compile https://build.chromium.org/p/chromium.chromiumos/builders/ChromiumOS%20x86-generic%20Compile
,
Apr 20 2016
,
Apr 20 2016
'gn gen' is currently failing for daisy, issue 604971 , so we need to wait for that to be resolved before converting the daisy builder. I've confirmed that 'gn gen' at least apepars to work for amd64-generic and x86-generic.
,
Apr 20 2016
there's a couple of issues to consider in how we want to do this: 1) Do you want to invoke GYP or GN directly, or use the MB wrapper? the latter may provide some benefits, but not a lot, since you'll be passing the list of defines in. 2) Where do you want to control whether the bot uses GYP or GN? via a config in the .ebuild, or a setting on the bot, or a setting in the chrome checkout? Most chromium bots do the last thing, but this may be less convenient for CrOS infra.
,
Apr 20 2016
1) I don't know what the 'MB wrapper' is. 2) I think we need to make it a setting in the bot, or change to the bot configuration file. I would like to switch a single SimpleChrome builder to GN initially. (Plus we can't switch daisy over until issue 605971 is resolved).
,
Apr 20 2016
in comment #5, stevenjb wrote: > 1) I don't know what the 'MB wrapper' is. Good question (statement?) :). The MB wrapper is a new python tool I wrote to help w/ the migration from GYP to GN in Chromium. https://chromium.googlesource.com/chromium/src/+/master/tools/mb It's designed to hide the details of whether a given bot is using GYP or GN from the rest of the build infrastructure, and to handle the mapping between GYP and GN flags via entries in: https://chromium.googlesource.com/chromium/src/+/master/tools/mb/mb_config.pyl Nearly all of the non-CrOS bots have been updated so that they do not invoke build/gyp_chromium during `gclient runhooks`, and invoke mb in a separate step instead: https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Builder/builds/38916/steps/generate_build_files/logs/stdio This wrapper also implements various other things that the build infrastructure needs, like generating runtime dependencies for swarming from GN, etc. It's possible we don't need much of that functionality in the simplechrome builds. If we can assume that both GYP_DEFINES and GN_ARGS are set in the environment by cros, then if we can also get a third env var that says whether we want to use GYP or GN, then we can modify MB to just look at the env vars and do the right thing in this case, and this will fit most seamlessly into the rest of the chromium build recipes, so I'd probably suggest we start with that.
,
Apr 22 2016
So... I spent a little bit of time browsing through mb.py, mb_config.pyl, and builders.pyl, attempting to tease out how the recipe for "ChromiumOS x86-generic Compile" is generated, and I confess it was... daunting. I will probably need to be walked through this. I think, what I would like is for us to be able to is: 1. Replace 'cros_chrome_sdk_gyp' in the mb_config.pyl entry for "ChromiumOS x86-generic Compile" (initially just that one) with 'cros_chrome_sdk_gn'. 2. Create a 'cros_chrome_sdk_gn' entry, which I guess would look like ['gn', 'error'] ? 3. Make that "just work". GN_ARGS should be defined correctly in the 'cros chrome-sdk' environment. We will need to run 'gn gen $BUILDIR --args=$GN_ARGS'. Everything should then 'just work'. dpranke@ - will you have time to put something together or walk me through it? I'd love to get x86-generic switch over sooner than later. Cheers, Steven
,
Apr 22 2016
Yup, I can put together something for you later today. From comment #7, it sounds like you're thinking that whether to use GYP or GN would be controlled by the entry in the mb_config.pyl file, rather than by a separate env var that was set in the ebuild. Is that correct?
,
Apr 22 2016
That looked to be reasonable to me? I don't see us going back and forth, this will hopefully be a one way trip. We just need to be able to switch them over one at a time because: a) I'd rather just risk breaking one initially :) b) issue 604971 will prevent us from switching over "ChromiumOS daisy Compile" until fixed.
,
Apr 22 2016
Okay. I'm less certain how this is going to play out on the downstream bots; maybe we won't even call MB there ...
,
Apr 22 2016
What do you mean by "downstream bots"?
,
Apr 22 2016
actual cros-managed bots like (I think) https://build.chromium.org/p/chromiumos/waterfall that are doing full ebuilds.
,
Apr 22 2016
Ah. Those (builders using the chroot) are a separate beast altogether. I will create a separate issue for converting those builders once we get the ebuilds working, but as I understand it we still have a fair bit of work to do there.
,
Apr 23 2016
Here's what I have in mind; I think it should more-or-less Just Work: https://codereview.chromium.org/1913703002/ We can try landing this and enabling MB on 'ChromiumOS x86-generic Compile' and see what happens, and go from there, if you like.
,
Apr 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/73ed0d6aa6f1c9c9a045090574d92493dc12307d commit 73ed0d6aa6f1c9c9a045090574d92493dc12307d Author: dpranke <dpranke@chromium.org> Date: Mon Apr 25 19:18:34 2016 Add support for cros chrome_sdk (simplechrome) to MB. When we build Chrome for ChromeOS using the simplechrome workflow, all of the GYP_DEFINES and gn args that are needed are determined by the cros wrapper scripts, and rather than duplicate that logic in the MB mixins, it's easier to just pass the data through. However, we don't want this passthrough mechanism to be abused, so we implement so that this only works if the bot is configured to allow for passthrough *and* the GYP_DEFINES or GN_ARGS env vars contain chromeos=1 or target_os="chromeos" as appropriate. If we need to be less strict later we can alway change things. R=stevenjb@chromium.org BUG= 605154 Review URL: https://codereview.chromium.org/1913703002 Cr-Commit-Position: refs/heads/master@{#389532} [modify] https://crrev.com/73ed0d6aa6f1c9c9a045090574d92493dc12307d/tools/mb/mb.py [modify] https://crrev.com/73ed0d6aa6f1c9c9a045090574d92493dc12307d/tools/mb/mb_config.pyl
,
Apr 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/73ed0d6aa6f1c9c9a045090574d92493dc12307d commit 73ed0d6aa6f1c9c9a045090574d92493dc12307d Author: dpranke <dpranke@chromium.org> Date: Mon Apr 25 19:18:34 2016 Add support for cros chrome_sdk (simplechrome) to MB. When we build Chrome for ChromeOS using the simplechrome workflow, all of the GYP_DEFINES and gn args that are needed are determined by the cros wrapper scripts, and rather than duplicate that logic in the MB mixins, it's easier to just pass the data through. However, we don't want this passthrough mechanism to be abused, so we implement so that this only works if the bot is configured to allow for passthrough *and* the GYP_DEFINES or GN_ARGS env vars contain chromeos=1 or target_os="chromeos" as appropriate. If we need to be less strict later we can alway change things. R=stevenjb@chromium.org BUG= 605154 Review URL: https://codereview.chromium.org/1913703002 Cr-Commit-Position: refs/heads/master@{#389532} [modify] https://crrev.com/73ed0d6aa6f1c9c9a045090574d92493dc12307d/tools/mb/mb.py [modify] https://crrev.com/73ed0d6aa6f1c9c9a045090574d92493dc12307d/tools/mb/mb_config.pyl
,
Apr 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/78685fb274d05ee48e5425dc2ce4f5ba7b2dce81 commit 78685fb274d05ee48e5425dc2ce4f5ba7b2dce81 Author: stevenjb <stevenjb@chromium.org> Date: Tue Apr 26 22:17:53 2016 Use GN in simple chrome builders BUG= 605154 Review URL: https://codereview.chromium.org/1918713005 Cr-Commit-Position: refs/heads/master@{#389914} [modify] https://crrev.com/78685fb274d05ee48e5425dc2ce4f5ba7b2dce81/tools/mb/mb_config.pyl
,
Apr 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/570d6f823fd119974967652479bf570e5002a362 commit 570d6f823fd119974967652479bf570e5002a362 Author: dpranke <dpranke@chromium.org> Date: Wed Apr 27 06:07:18 2016 Revert of Use GN in simple chrome builders (patchset #1 id:1 of https://codereview.chromium.org/1918713005/ ) Reason for revert: Well, I fixed a bunch of things, but it got late and I'm still hitting bugs, so I'm going to try to flip this back to GYP and see if that works, at least (through MB). https://build.chromium.org/p/chromium.chromiumos/builders/ChromiumOS%20amd64-generic%20Compile/builds/17407 The last error makes it look like we might be missing a few targets that chromiumos_preflight needs, but I might've also just gotten the build labels wrong in the GN build. I suggest we try again tomorrow and flip the trybots at the same time so we can be consistent and see the trybots working first. Original issue's description: > Use GN in simple chrome builders > > BUG= 605154 > > Committed: https://crrev.com/78685fb274d05ee48e5425dc2ce4f5ba7b2dce81 > Cr-Commit-Position: refs/heads/master@{#389914} TBR=stevenjb@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 605154 Review URL: https://codereview.chromium.org/1921003009 Cr-Commit-Position: refs/heads/master@{#389998} [modify] https://crrev.com/570d6f823fd119974967652479bf570e5002a362/tools/mb/mb_config.pyl
,
Apr 28 2016
,
May 2 2016
,
Jun 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/f67895d75bae253dda0f2c08beabdb3bcdc03252 commit f67895d75bae253dda0f2c08beabdb3bcdc03252 Author: Steven Bennetts <stevenjb@chromium.org> Date: Mon May 16 19:09:19 2016 Use gn instead of GYP in SimpleChrome stage BUG= chromium:605154 TEST=SimpleChromeWorkflow stage succeeds and uses GN Change-Id: I925d3bdfb251fbd63888d7122ca4e04009609fce Reviewed-on: https://chromium-review.googlesource.com/344831 Reviewed-by: David James <davidjames@chromium.org> Tested-by: Steven Bennetts <stevenjb@chromium.org> [modify] https://crrev.com/f67895d75bae253dda0f2c08beabdb3bcdc03252/cbuildbot/stages/chrome_stages.py
,
Jun 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/3ebb69aba9d917e130a5f10d365454e4f06d2f8e commit 3ebb69aba9d917e130a5f10d365454e4f06d2f8e Author: Steven Bennetts <stevenjb@chromium.org> Date: Tue Jun 21 16:56:37 2016 Use gn instead of GYP in SimpleChrome stage (take 2) BUG= chromium:605154 TEST=SimpleChromeWorkflow stage succeeds and uses GN Change-Id: Iafcf8174fa013a7f10755f6c20db23118e4378b8 Reviewed-on: https://chromium-review.googlesource.com/354212 Tested-by: Steven Bennetts <stevenjb@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Reviewed-by: Steven Bennetts <stevenjb@chromium.org> [modify] https://crrev.com/3ebb69aba9d917e130a5f10d365454e4f06d2f8e/cbuildbot/stages/chrome_stages.py
,
Jun 23 2016
This is fixed for real now - we are using GN in the SimpleChrome stage of the PFQ builders also now, hooray!
,
Jun 23 2016
Awesome!
,
Jul 1 2016
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by steve...@chromium.org
, Apr 20 2016