Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(463)

Issue 385603003: Split targets with external dependencies out of mojo_public.gypi (Closed)

Created:
6 years, 5 months ago by Chris Masone
Modified:
6 years, 5 months ago
Reviewers:
DaveMoore
CC:
chromium-reviews, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), ben+mojo_chromium.org
Project:
chromium
Visibility:
Public.

Description

Split targets with external dependencies out of mojo_public.gypi The non-testing code in src/mojo/public should be buildable without depending on anything outside of that directory. It's fine for test code to depend on base/ and gtest/, though. Due to the way GYP works, having targets for both production and test code in the same .gypi file winds up trying to resolve the dependencies for both kinds of targets, breaking the ability to build the public mojo code hermetically. Splitting the targets out will enable consumers of the mojo code to build the public code standalone using GYP. BUG=chromium:388412 TEST=build and run all mojo unittests Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=282699

Patch Set 1 #

Patch Set 2 : Added copyright messages to .gypi files #

Patch Set 3 : Remove dummy target causing build problems #

Patch Set 4 : get rid of mojo_public_java.gypi, per qsr #

Unified diffs Side-by-side diffs Delta from patch set Stats (+234 lines, -221 lines) Patch
M mojo/mojo.gyp View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M mojo/mojo_public.gypi View 1 2 3 3 chunks +4 lines, -221 lines 0 comments Download
A mojo/mojo_public_tests.gypi View 1 1 chunk +193 lines, -0 lines 0 comments Download
M mojo/mojo_services.gypi View 1 chunk +36 lines, -0 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Chris Masone
PTAL. I tried the standard bots. Any more I should look at?
6 years, 5 months ago (2014-07-10 22:19:49 UTC) #1
qsr
The java changed should not be needed: https://codereview.chromium.org/385113005
6 years, 5 months ago (2014-07-11 12:01:15 UTC) #2
Chris Masone
On 2014/07/11 12:01:15, qsr wrote: > The java changed should not be needed: https://codereview.chromium.org/385113005 THanks! ...
6 years, 5 months ago (2014-07-11 18:51:51 UTC) #3
DaveMoore
lgtm
6 years, 5 months ago (2014-07-11 19:33:23 UTC) #4
Chris Masone
The CQ bit was checked by cmasone@chromium.org
6 years, 5 months ago (2014-07-11 19:35:15 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cmasone@chromium.org/385603003/60001
6 years, 5 months ago (2014-07-11 19:36:52 UTC) #6
commit-bot: I haz the power
6 years, 5 months ago (2014-07-11 21:11:48 UTC) #7
Message was sent while issue was closed.
Change committed as 282699

Powered by Google App Engine
This is Rietveld 408576698