Closed Bug 1427840 Opened 2 years ago Closed 2 years ago

Add documentation for GN support


(Firefox Build System :: General, enhancement)

Not set


(firefox59 fixed)

Tracking Status
firefox59 --- fixed


(Reporter: chmanchester, Assigned: chmanchester)




(1 file)

No description provided.
I couldn't get the docs to build locally for some reason, but the output can be verified from this try push:
Comment on attachment 8939727 [details]
Bug 1427840 - Add in-tree docs for GN support in the build system.

::: build/docs/gn.rst:13
(Diff revision 1)
> +some related projects that are vendored in mozilla-central. Rather than
> +requiring ``GN`` to build or writing our own build definitions for these projects,
> +we have support in the build system for translating GN configuration
> +files into files. In most cases these files will be like any
> +others in the tree (except that they shouldn't be modified by hand), however
> +those updating vendored code or building on platforms not exercised in

maybe s/exercised/supported/

::: build/docs/gn.rst:39
(Diff revision 1)
> +
> +If updating upstream sources or vendoring a new project, this step must be
> +performed for each supported build configuration. If adding support for a
> +specific configuration, the generated configuration may be added to existing
> +configs before re-generating the ```` files, which should be found under
> +the ``gn-configs`` directory under the project's top-level directory.

This might confuse people, we should make it explicit that this is the top level directory of the vendored project.

Are we ok with checking in the generated json files? I had broken them out of my patch and was considering hosting them on a user repo. This is this definitely less convienent and makes matching the version of the json files used to generate a particular set of files more error prone, but there were concerns about the size of the files.
Attachment #8939727 - Flags: review?(dminor) → review+
Leaving a needinfo to check whether or not I should commit the generated json files in tree.
Flags: needinfo?(cmanchester)
Bloat in the tree is certainly a concern, but I think it's far preferable to have them checked in. What's the total size of the json files?
Flags: needinfo?(cmanchester) → needinfo?(dminor)
Average size for a generated json file is: 364140, total size for the current set is: 5826239.

I don't anticipate the files changing very often and I agree it would be better to have them checked in.

We could potentially post-process the generated json to remove things like compiler flags that will be ignored during generation anyway if size is a concern.
Flags: needinfo?(dminor)
That sounds pretty big but on the order of other third party imports we routinely perform. If it becomes a problem we can move them or find ways to slim them down, as you suggest.
Since we'll wind up updating these every time we update from upstream webrtc, it would be nice to make sure we're generating the JSON files in a reproducible way so that diffs will be minimal.
Pushed by
Add in-tree docs for GN support in the build system. r=dminor
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
The gn page is working here: but seems to be missing from the index page here: Sorry if I missed a bad link during review. Or maybe I'm getting a stale or cached version of that page.
Flags: needinfo?(cmanchester)
It's altogether possible I got this wrong somewhere, but I see the link when I visit that page:
Flags: needinfo?(cmanchester)
Yep, works for me now too. I guess I got a stale version of the page this morning. Thanks!
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.