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

Issue 1134753011: Fix missing GN dependencies. (Closed)

Created:
5 years, 7 months ago by brettw
Modified:
5 years, 6 months ago
Reviewers:
Dirk Pranke
CC:
chromium-reviews, extensions-reviews_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, samuong+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, oshima+watch_chromium.org, chromium-apps-reviews_chromium.org, stgao, ben+mojo_chromium.org, darin (slow to review)
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix missing GN dependencies. Fixes some cases where a dependency between a target's inputs and outputs exists without an explicit dependency. json_schema_api.gni was generating wrong names, this was fixed. mojo_application_package.gni was fixed to generate unique names (just a bug I noticed in passing) and support the testonly flag, which is necessary when I added the correct dependency. The rest of the cases are just adding deps or making existing deps public so that the ability to depend on the target's outputs is passed to dependents. BUG=487897 TBR=dpranke CQ_EXTRA_TRYBOTS=tryserver.chromium.linux:android_chromium_gn_compile_dbg,android_chromium_gn_compile_rel;tryserver.chromium.win:win8_chromium_gn_rel,win8_chromium_gn_dbg;tryserver.chromium.mac:mac_chromium_gn_rel,mac_chromium_gn_dbg Previously landed as https://codereview.chromium.org/1128163007/ the issue there should have been fixed by https://codereview.chromium.org/1148173002/ which has already been landed. Committed: https://crrev.com/de262b0acc538d307f35b85f4ebf395357d1a4db Cr-Commit-Position: refs/heads/master@{#331625}

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+61 lines, -21 lines) Patch
M build/json_schema_api.gni View 2 chunks +6 lines, -4 lines 0 comments Download
M build/secondary/tools/grit/grit_rule.gni View 1 chunk +5 lines, -3 lines 0 comments Download
M chrome/BUILD.gn View 1 2 8 chunks +21 lines, -3 lines 0 comments Download
M chrome/app/BUILD.gn View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/BUILD.gn View 1 2 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/libgtk2ui/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/test/chromedriver/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M components/html_viewer/BUILD.gn View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M extensions/BUILD.gn View 1 2 3 2 chunks +4 lines, -3 lines 0 comments Download
M gin/BUILD.gn View 2 chunks +6 lines, -4 lines 0 comments Download
M mojo/mojo_application_package.gni View 1 chunk +7 lines, -2 lines 0 comments Download
M ui/resources/BUILD.gn View 1 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 18 (9 generated)
brettw
The original patch is patchset 1. Patchset 2 is identical, I don't anticipate any changes ...
5 years, 7 months ago (2015-05-21 19:53:03 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1134753011/40001
5 years, 7 months ago (2015-05-21 20:23:22 UTC) #5
Dirk Pranke
lgtm
5 years, 7 months ago (2015-05-21 21:04:54 UTC) #6
commit-bot: I haz the power
Try jobs failed on following builders: win8_chromium_gn_rel on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_gn_rel/builds/7071)
5 years, 7 months ago (2015-05-21 21:26:53 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1134753011/60001
5 years, 6 months ago (2015-05-27 16:58:37 UTC) #11
commit-bot: I haz the power
Try jobs failed on following builders: win8_chromium_gn_rel on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_gn_rel/builds/7152)
5 years, 6 months ago (2015-05-27 17:41:50 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1134753011/80001
5 years, 6 months ago (2015-05-27 18:23:43 UTC) #16
commit-bot: I haz the power
Committed patchset #4 (id:80001)
5 years, 6 months ago (2015-05-27 19:41:54 UTC) #17
commit-bot: I haz the power
5 years, 6 months ago (2015-05-27 19:43:51 UTC) #18
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/de262b0acc538d307f35b85f4ebf395357d1a4db
Cr-Commit-Position: refs/heads/master@{#331625}

Powered by Google App Engine
This is Rietveld 408576698