Update patterns for breaking circular dependencies and usage of sources.gni
4.5 KiB
sources.gni
Background
The use of sources.gni was initially added to help deal with the issue of
circular dependencies. We often subclass upstream code like
BraveContentBrowserClient -> ChromeContentBrowserClient, but in
//brave/browser instead of //chrome/browser. This created a circular
dependency because:
BraveContentBrowserClientdepends on//chrome/brave.//chrome/bravedepends on//chrome/bravebecause it now was to instantiateBraveContentBrowserClientinstead ofChromeContentBrowserClient.
This became a problem when we started running gn check and checkdeps so
worked around it by using sources.gni to add
sources += brave_chrome_browser_sources so both BraveContentBrowserClient
and ChromeContentBrowserClient were in the same target. This is mainly an
issue for //chrome/browser and //chrome/browser/ui and upstream is actively
working to break these up so we need to make sure we are not adding to the
problem on our side.
Usage of sources.gni
Use of sources.gni to include sources in //chrome/browser and
//chrome/browser/ui should be avoided, see Circular
dependencies. Adding deps through sources.gni is
generally ok. Use of sources.gni to include sources in other targets can be used
if the there is no reasonable way to avoid it using the options below.
This does not mean that you cannot ever use sources.gni. For instance it may be appropriate when adding a very small number of sources to an existing upstream target, but please consider other approaches below first. Using sources.gni to add dependencies and other non-source configuration to upstream targets is generally ok.
Methods to avoid circular dependencies
Whenever possible try to break circular dependencies see Recipes for Breaking Chrome Dependencies and Dependency Inversion for examples.
An interface/impl pattern can also often be used where header files and possibly
some cc files are included in the direct dependency and the code that would
cause the circular dependency is included in a higher level target like
//brave/browser to ensure that the implementation code is always linked into
the final output. See tabs:tabs_public and tabs:impl and //chrome/browser impl
dependency.
The chromium ios code is also a good model for separating out dependencies and sometimes makes use of interface/implementation patterns.
Another technique to avoid circular dependencies is to use a template so the subclass does not need a dependency on the base class.
brave_class.h
template <typename ChromeClass>
class BraveClass : public ChromeClass {
...
}
The chrome target that we override will need a dependency on the brave target, but there is no circular dependency some_chromium_source.cc
chrome_class_ = std::make_unique<ChromeClass>();
chromium_src/some_chromium_source.cc
#define ChromeClass BraveClass<ChromeClass>()
Circular dependencies
Circular dependencies can sometimes (temporarily) use
allow_circular_includes_from to split sources up into smaller targets so they
can be more easily resolved down the road. This is the technique we should use
for //chrome/browser and //chrome/browserui if the circular dependencies
cannot be removed through the methods above. It may be appropriate in other
cases, check in slack if you are unsure. See //chrome/browser and
//chrome/browser/ui for examples. Also
https://github.com/brave/brave-core/pull/25892/files for an example in brave-core
of converting from sources.gni.
Do not use check_includes = false to suppress errors about circular includes.