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

Issue 2452473004: Fix a large number of missing dependencies in the blink gn build (Closed)

Created:
4 years, 1 month ago by tsniatowski
Modified:
4 years, 1 month ago
Reviewers:
Dirk Pranke, Nico
CC:
chromium-reviews, caseq+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, blink-reviews, kozyatinskiy+blink_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix a large number of missing dependencies in the blink gn build Make all blink_core_sources targets public_dep on all the code generators in core to ensure required headers are always generated first and a successful build does not depend on lucky ordering. Manually fix similar dep issues in core/inspector. There are now more dependencies than strictly necessary, but they will only trigger the generators with no effect on build commands (tested by checking that the patch doesn't trigger a rebuild of any c++ code). The end result is that the total number of targets that don't have proper deps in the 'chrome' target build goes down from over 1800 to about 40, and no missing dependencies on gen/blink files exist. BUG=655123 R=dpranke@chromium.org Committed: https://crrev.com/c50ed22545a79b7947f3b3fc2efe602c63db3d8d Cr-Commit-Position: refs/heads/master@{#427856}

Patch Set 1 #

Patch Set 2 : rebase (make_core_generated_input_mode_names was added meanwhile) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+61 lines, -45 lines) Patch
M third_party/WebKit/Source/core/BUILD.gn View 1 1 chunk +51 lines, -45 lines 0 comments Download
M third_party/WebKit/Source/core/core.gni View 1 chunk +8 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/inspector/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 19 (10 generated)
tsniatowski
4 years, 1 month ago (2016-10-25 19:48:52 UTC) #1
Dirk Pranke
lgtm
4 years, 1 month ago (2016-10-26 17:53:18 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2452473004/1
4 years, 1 month ago (2016-10-26 17:57:21 UTC) #6
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/303317) ios-device on master.tryserver.chromium.mac (JOB_FAILED, ...
4 years, 1 month ago (2016-10-26 18:00:02 UTC) #8
tsniatowski
On 2016/10/26 18:00:02, commit-bot: I haz the power wrote: > Try jobs failed on following ...
4 years, 1 month ago (2016-10-26 21:24:55 UTC) #11
Dirk Pranke
On 2016/10/26 21:24:55, tsniatowski wrote: > rebased; dpranke: should I usually ask for a new ...
4 years, 1 month ago (2016-10-26 21:37:07 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2452473004/20001
4 years, 1 month ago (2016-10-26 21:39:24 UTC) #15
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 1 month ago (2016-10-26 22:41:49 UTC) #17
commit-bot: I haz the power
4 years, 1 month ago (2016-10-26 22:45:17 UTC) #19
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/c50ed22545a79b7947f3b3fc2efe602c63db3d8d
Cr-Commit-Position: refs/heads/master@{#427856}

Powered by Google App Engine
This is Rietveld 408576698