diff options
author | primiano <primiano@chromium.org> | 2016-01-21 06:38:37 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-01-21 14:40:06 +0000 |
commit | e97b9201e99ad1a0c22b631762d0bb5f3fdd3d83 (patch) | |
tree | c8f3fbf06bea239b2e1f1f35f159be96ab8496ce /base/BUILD.gn | |
parent | e46533afc22085c91b2ee05102c90208b029d6a9 (diff) | |
download | chromium_src-e97b9201e99ad1a0c22b631762d0bb5f3fdd3d83.zip chromium_src-e97b9201e99ad1a0c22b631762d0bb5f3fdd3d83.tar.gz chromium_src-e97b9201e99ad1a0c22b631762d0bb5f3fdd3d83.tar.bz2 |
Reland (2) of: Allow base to depend on allocator
Reason for reland:
Some targets (clearkeycdmadapter, osmesa) now fell in the state where
they depend indirectly on base (which causes the removal of the default
clib) but via a shared_library boundary (which does not propagate the
replacement allocator / shim layer code).
Fixing those targets by adding a direct base dependency.
This causes a breakage in the main waterfall:
https://build.chromium.org/p/chromium.win/builders/Win%20x64%20GN/builds/10231
Original CL description:
> A smaller, yet key, step to move
>
> From: a situation where mainly executables (but not really) depend on
> allocator, and base needs dependencies (to tcmalloc) to be injected
> from content (which violates the ODR in component buids).
>
> To: a situation where only base depends on allocator and the other
> targets get recursively the required linked flags.
>
> In essence this CL is a more gradual approach to the bigger
> unreviewable crrev.com/1528013002.
>
> How is the transition handled?
> ------------------------------
> After this CL, the situation will be as follows:
> From a build time perspective base will also depend on allocator.
> This will not change anything substantial in static builds and introduce
> yet another (temporary) ODR violation in Linux component builds.
> The big change introduced by this CL is the fact that all the executable
> targets that depend on base (virtualy all) will also get another
> indirect dependency to allocator.
>
> In other words, after this CL executable targets will depend on
> allocator for two reasons:
> - Because they have an explicit dependency to it (the one I am going to
> get rid of in the immediate future).
> - Because this new transitive path I am introducing in base.
>
> Rationale of this approach
> --------------------------
> This allows to restrict the critical changes in a smaller CL easier to
> review, at the cost of the temporary double dependency on base.
> The good things are:
> - If something will break, this CL will be very easy to revert.
> - The next cleanups will be straightforward.
> - We have now smoke tests (crrev.com/1577883002) that will help us
> realize if something goes wrong.
>
> Next steps
> ----------
> In the next CLs I will:
> - Remove the content -> base injection layer, and let base directly use
> the tcmalloc functions it needs.
> - Remove all the traces of USE_TCMALLOC outside of base.
> - Start cleaning up the hundreds use_allocator conditionals in the gyp
> files in a way which is easier to review and produce zero ninja diffs
> (see crrev.com/1583973002 as an example)
>
> Ninja diffs caused by this change
> ---------------------------------
> ### Win, static build, GN: https://paste.ee/p/hvcRp
> The missing targets (mostly tests) that previously were
> not depending on allocator, now get that by virtue of the transitive
> dependency.
>
> ### Win, static build, GYP: https://paste.ee/p/AGuKR
> As above. Just GYP seems to emit the ninja files in a different,
> inlined, format.
>
> ### linux static build, GYP: https://paste.ee/p/kmD7U
> As above. Plus the new targets also get the -Wl,-u (keep symbol)
> args as expected by allocator.gyp for the tcmalloc heap profiler.
>
> ### linux shared build, GYP: https://paste.ee/p/FHHNR
> Nothing relevant. Just I moved the dependency to allocator from
> base_unittests to base, and that is the only thing that reflects in the
> ninja files.
>
> BUG=564618
TBR=wfh@chromium.org,brettw@chromium.org,kbr@chromium.org
BUG=564618
Review URL: https://codereview.chromium.org/1616793003
Cr-Commit-Position: refs/heads/master@{#370694}
Diffstat (limited to 'base/BUILD.gn')
-rw-r--r-- | base/BUILD.gn | 5 |
1 files changed, 4 insertions, 1 deletions
diff --git a/base/BUILD.gn b/base/BUILD.gn index fe8ad86..3f8380c 100644 --- a/base/BUILD.gn +++ b/base/BUILD.gn @@ -953,6 +953,7 @@ component("base") { ] deps = [ + "//base/allocator", "//base/third_party/dynamic_annotations", "//third_party/modp_b64", ] @@ -1861,7 +1862,6 @@ test("base_unittests") { ":message_loop_tests", ":prefs", ":prefs_test_support", - "//base/allocator", "//base/test:run_all_unittests", "//base/test:test_support", "//base/third_party/dynamic_annotations", @@ -1870,6 +1870,9 @@ test("base_unittests") { "//third_party/icu", ] + # Some unittests depend on the ALLOCATOR_SHIM macro. + configs += [ "//base/allocator:allocator_shim_define" ] + data = [ "test/data/", ] |