Closed Bug 1386782 Opened 7 years ago Closed 7 years ago

Build GN in automation

Categories

(Firefox Build System :: General, enhancement)

Product:
Firefox Build System
Component:
General
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox57 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: chmanchester, Assigned: chmanchester)

References

Details

Attachments

(1 file)

Bug 1386782 - Add toolchain tasks to build gn in automation.
59 bytes, text/x-review-board-request
ted
: review+
Details
Assignee: nobody → cmanchester
Comment on attachment 8893132 [details] Bug 1386782 - Add toolchain tasks to build gn in automation. https://reviewboard.mozilla.org/r/164130/#review172820 ::: taskcluster/ci/toolchain/macosx.yml:74 (Diff revision 1) > tooltool-downloads: internal > resources: > - 'taskcluster/scripts/misc/tooltool-download.sh' > toolchain-artifact: public/build/cctools.tar.bz2 > + > +macosx64-gn: Do we need a different gn binary for OS X builds? I feel like I remember you saying something about this but I can't remember the specifics. Seems like they ought to be able to use the Linux binaries since we're not running builds on OS X anymore. Or is this purely for the use of `mach toolchain` for developers to build locally? ::: taskcluster/scripts/misc/build-gn-common.sh:39 (Diff revision 1) > +tar -xvzf base.tar.gz > +cd .. > + > +mkdir build > +cd build > +wget https://chromium.googlesource.com/chromium/src/+archive/${CHROMIUM_SRC_REV}/build.tar.gz Would it just be simpler to clone the whole chromium/src repo? ::: taskcluster/scripts/misc/build-gn-common.sh:60 (Diff revision 1) > +STAGE=gn > +mkdir -p $UPLOAD_DIR $STAGE > + > +# At this point, the resulting binary is at: > +# $WORKSPACE/out/Release/gn > +cp out/Release/gn $STAGE You might also want to strip the binary before packaging it. ::: taskcluster/scripts/misc/gn.patch:9 (Diff revision 1) > ++++ b/bootstrap/bootstrap.py > +@@ -29,9 +29,10 @@ BOOTSTRAP_DIR = os.path.dirname(os.path.abspath(__file__)) > + GN_ROOT = os.path.dirname(BOOTSTRAP_DIR) > + SRC_ROOT = os.path.dirname(os.path.dirname(GN_ROOT)) > + > ++is_mac_cross = os.environ.get('MAC_CROSS', False) Ugh. Do you think we can get this into a state where it can be upstreamed? I don't think cross-compilation is that ridiculous of a use case.
Attachment #8893132 - Flags: review?(ted) → review+
Comment on attachment 8893132 [details] Bug 1386782 - Add toolchain tasks to build gn in automation. https://reviewboard.mozilla.org/r/164130/#review172998 ::: taskcluster/ci/toolchain/linux.yml:154 (Diff revision 1) > + symbol: TL(gn) > + tier: 1 > + worker-type: aws-provisioner-v1/gecko-{level}-b-linux > + worker: > + docker-image: {in-tree: desktop-build} > + max-run-time: 36000 This is 10 hours. Please stop the cargo cult and switch to something reasonable. ::: taskcluster/scripts/misc/build-gn-common.sh:15 (Diff revision 1) > +wget https://chromium.googlesource.com/chromium/src/+archive/${CHROMIUM_SRC_REV}/tools/gn.tar.gz > +tar -xvzf gn.tar.gz We need some kind of content based verification here to protect us from tampering. At the very least, a sha-256 or stronger hash should be hard coded in this file. You can also supplement that with a GPG verification. Keep in mind that GPG verification only verifies the signer was valid: it doesn't offer protection against compromise of the signing key leading to rogue signatures. In other toolchain tasks, we `git clone`. This is also acceptable. But due to the recent Git vulnerability, it is unsafe to `git clone` because it will check out the default branch (`master`) by default. But if you force it to check out a known good revision first, we're protected.
Comment on attachment 8893132 [details] Bug 1386782 - Add toolchain tasks to build gn in automation. https://reviewboard.mozilla.org/r/164130/#review172820 > Do we need a different gn binary for OS X builds? I feel like I remember you saying something about this but I can't remember the specifics. Seems like they ought to be able to use the Linux binaries since we're not running builds on OS X anymore. Or is this purely for the use of `mach toolchain` for developers to build locally? Right, this is for the benefit of mach bootstrap once we're requiring gn to build.
Comment on attachment 8893132 [details] Bug 1386782 - Add toolchain tasks to build gn in automation. https://reviewboard.mozilla.org/r/164130/#review172820 > Would it just be simpler to clone the whole chromium/src repo? chromium/src is pretty big, but I think I can stitch this together from multiple git mirrors.
Comment on attachment 8893132 [details] Bug 1386782 - Add toolchain tasks to build gn in automation. https://reviewboard.mozilla.org/r/164130/#review172998 > We need some kind of content based verification here to protect us from tampering. At the very least, a sha-256 or stronger hash should be hard coded in this file. You can also supplement that with a GPG verification. Keep in mind that GPG verification only verifies the signer was valid: it doesn't offer protection against compromise of the signing key leading to rogue signatures. > > In other toolchain tasks, we `git clone`. This is also acceptable. But due to the recent Git vulnerability, it is unsafe to `git clone` because it will check out the default branch (`master`) by default. But if you force it to check out a known good revision first, we're protected. I ended up using git and checking out specific revisions.
Comment on attachment 8893132 [details] Bug 1386782 - Add toolchain tasks to build gn in automation. https://reviewboard.mozilla.org/r/164130/#review172820 > Ugh. Do you think we can get this into a state where it can be upstreamed? I don't think cross-compilation is that ridiculous of a use case. I'll file a follow-up about this. I guess the bootstrap.py file isn't officially maintained by google, so they might just take patches like these.
Comment on attachment 8893132 [details] Bug 1386782 - Add toolchain tasks to build gn in automation. A few changes here, asking for re-review.
Attachment #8893132 - Flags: review+ → review?(ted)
Comment on attachment 8893132 [details] Bug 1386782 - Add toolchain tasks to build gn in automation. https://reviewboard.mozilla.org/r/164130/#review178712 ::: taskcluster/scripts/misc/build-gn-common.sh:16 (Diff revision 3) > + > +mkdir tools > +cd tools > +git clone --no-checkout https://chromium.googlesource.com/chromium/src/tools/gn > +cd gn > +git checkout 2786e523ece02a0b9c6c4c58a9a74346e2f05c8d It feels a little odd that you have two named revisions at the top of the script, then a bunch of other ones inline. Not sure it's worth having every single one of these given a name, though. How did you derive all these changeset values? Presumably you need a consistent set to make everything build reliably? I would guess that these all get linked together with gclient and a `DEPS` file. Would it make things more future-proof to scrape the specific revisions out of that?
Attachment #8893132 - Flags: review?(ted) → review+
Comment on attachment 8893132 [details] Bug 1386782 - Add toolchain tasks to build gn in automation. https://reviewboard.mozilla.org/r/164130/#review178712 > It feels a little odd that you have two named revisions at the top of the script, then a bunch of other ones inline. Not sure it's worth having every single one of these given a name, though. > > How did you derive all these changeset values? Presumably you need a consistent set to make everything build reliably? I would guess that these all get linked together with gclient and a `DEPS` file. Would it make things more future-proof to scrape the specific revisions out of that? Yeah, this ended up pretty messy... these are just convenience mirrors for subsets of the chromium/src repo rather than proper deps as far as I can tell. I just got them from each repo's master at a specific point, I'll see if there's actually an automatic way to correlate them.
Comment on attachment 8893132 [details] Bug 1386782 - Add toolchain tasks to build gn in automation. https://reviewboard.mozilla.org/r/164130/#review178712 > Yeah, this ended up pretty messy... these are just convenience mirrors for subsets of the chromium/src repo rather than proper deps as far as I can tell. I just got them from each repo's master at a specific point, I'll see if there's actually an automatic way to correlate them. I don't see a way to achieve this, so I think we're going to be better off just cloning all of chromium/src after all. It's large, but it doesn't take a crazy amount of time in automation, and we shouldn't be building these often.
Pushed by cmanchester@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/4228b24f2559 Add toolchain tasks to build gn in automation. r=ted
https://hg.mozilla.org/mozilla-central/rev/4228b24f2559
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
(In reply to Chris Manchester (:chmanchester) from comment #13) > I don't see a way to achieve this, so I think we're going to be better off > just cloning all of chromium/src after all. It's large, but it doesn't take > a crazy amount of time in automation, and we shouldn't be building these > often. Thanks for following through with this. I just want to ensure that we can maintain this for the future without jumping through hoops. :)
FWIW it can take ~20 minutes to do the Git clones. Oy. [task 2017-08-31T03:33:15.399225Z] git clone --no-checkout https://chromium.googlesource.com/chromium/src $WORKSPACE/gn-standalone [task 2017-08-31T03:33:15.413021Z] Cloning into '/builds/worker/workspace/gn-standalone'... [task 2017-08-31T03:50:22.425363Z] cd $WORKSPACE/gn-standalone [task 2017-08-31T03:50:22.427358Z] git checkout $CHROMIUM_SRC_REV [task 2017-08-31T03:50:43.565801Z] Note: checking out 'c338d43f49c0d72e69cd6e40eeaf4c0597dbdda1'. [task 2017-08-31T03:50:43.565841Z] [task 2017-08-31T03:50:43.565860Z] You are in 'detached HEAD' state. You can look around, make experimental [task 2017-08-31T03:50:43.565876Z] changes and commit them, and you can discard any commits you make in this [task 2017-08-31T03:50:43.565892Z] state without impacting any branches by performing another checkout. [task 2017-08-31T03:50:43.565896Z] [task 2017-08-31T03:50:43.565912Z] If you want to create a new branch to retain commits you create, you may [task 2017-08-31T03:50:43.565928Z] do so (now or later) by using -b with the checkout command again. Example: [task 2017-08-31T03:50:43.565932Z] [task 2017-08-31T03:50:43.565941Z] git checkout -b <new-branch-name> [task 2017-08-31T03:50:43.565945Z] [task 2017-08-31T03:50:43.565956Z] HEAD is now at c338d43... Fix GN bootstrap [task 2017-08-31T03:50:43.621469Z] [task 2017-08-31T03:50:43.621524Z] git clone --no-checkout https://chromium.googlesource.com/chromium/chromium chromium_checkout [task 2017-08-31T03:50:43.622875Z] Cloning into 'chromium_checkout'... [task 2017-08-31T03:53:54.402356Z] cd chromium_checkout [task 2017-08-31T03:53:54.402418Z] git checkout $CHROMIUM_REV [task 2017-08-31T03:53:59.236924Z] Note: checking out 'e6ba81e00ae835946e069e5bd80bd533b11d8442'. [task 2017-08-31T03:53:59.236961Z] [task 2017-08-31T03:53:59.236980Z] You are in 'detached HEAD' state. You can look around, make experimental [task 2017-08-31T03:53:59.236997Z] changes and commit them, and you can discard any commits you make in this [task 2017-08-31T03:53:59.237013Z] state without impacting any branches by performing another checkout. [task 2017-08-31T03:53:59.237016Z] [task 2017-08-31T03:53:59.237032Z] If you want to create a new branch to retain commits you create, you may [task 2017-08-31T03:53:59.237049Z] do so (now or later) by using -b with the checkout command again. Example: [task 2017-08-31T03:53:59.237052Z] [task 2017-08-31T03:53:59.237062Z] git checkout -b <new-branch-name> [task 2017-08-31T03:53:59.237066Z] [task 2017-08-31T03:53:59.237082Z] HEAD is now at e6ba81e... Set 'gcc_version' to 0 for Clang/ASan/TSan builds.
Product: Core → Firefox Build System

Chris, do you remember why the Windows version was made 32-bits?

Flags: needinfo?(cmanchester)

No, I do not.

Flags: needinfo?(cmanchester)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: