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

Issue 22290010: Add support for data deps. (Closed)

Created:
7 years, 4 months ago by brettw
Modified:
7 years, 4 months ago
Reviewers:
scottmg
CC:
chromium-reviews
Visibility:
Public.

Description

Add support for data deps. Data deps are non-linked dependencies of a target. They are built in parallel (they are not input dependencies). I redefined "data" to mean data file dependencies, and added a new "datadeps" contept for non-linked target dependencies. Fix a bug to make it not crash if there's nothing to generate. Add variable documentation for some vars. Removed support for some builtin vars "root output dir name" and related. These had changed definition from when I originally wrote them, and I don't think there's any use for these values. We can add them back if we need. I moved the variable name constant declarations from scope_per_file_provider to the new variables file which includes documentation. I added support for getting the name of the current toolchain via a builtin variable. I removed support for solink_module which is not necessary. This was a way to express a .dll target that isn't linked to its dependees, but that's no longer necessary for datadeps. BUG= R=scottmg@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=215976

Patch Set 1 #

Patch Set 2 : fixes #

Patch Set 3 : Clarify deps documentation. #

Total comments: 2

Patch Set 4 : remove switch #

Unified diffs Side-by-side diffs Delta from patch set Stats (+715 lines, -207 lines) Patch
M tools/gn/BUILD.gn View 1 chunk +2 lines, -0 lines 0 comments Download
M tools/gn/command_desc.cc View 1 2 3 6 chunks +16 lines, -3 lines 0 comments Download
M tools/gn/command_gen.cc View 1 chunk +1 line, -4 lines 0 comments Download
M tools/gn/command_help.cc View 5 chunks +44 lines, -5 lines 0 comments Download
M tools/gn/gn.gyp View 1 chunk +2 lines, -0 lines 0 comments Download
M tools/gn/ninja_helper.cc View 1 chunk +5 lines, -5 lines 0 comments Download
M tools/gn/ninja_target_writer.h View 1 chunk +5 lines, -0 lines 0 comments Download
M tools/gn/ninja_target_writer.cc View 3 chunks +87 lines, -36 lines 0 comments Download
M tools/gn/ninja_writer.h View 1 chunk +1 line, -0 lines 0 comments Download
M tools/gn/ninja_writer.cc View 2 chunks +17 lines, -3 lines 0 comments Download
M tools/gn/scope_per_file_provider.h View 2 chunks +2 lines, -28 lines 0 comments Download
M tools/gn/scope_per_file_provider.cc View 4 chunks +16 lines, -69 lines 0 comments Download
M tools/gn/secondary/base/BUILD.gn View 1 1 chunk +10 lines, -6 lines 0 comments Download
M tools/gn/secondary/build/config/mac/BUILD.gn View 1 2 chunks +4 lines, -10 lines 0 comments Download
M tools/gn/target.h View 2 chunks +6 lines, -0 lines 0 comments Download
M tools/gn/target_generator.h View 2 chunks +5 lines, -2 lines 0 comments Download
M tools/gn/target_generator.cc View 5 chunks +36 lines, -22 lines 0 comments Download
M tools/gn/toolchain.h View 1 2 chunks +0 lines, -2 lines 0 comments Download
M tools/gn/toolchain.cc View 1 3 chunks +0 lines, -3 lines 0 comments Download
M tools/gn/toolchain_manager.cc View 1 chunk +0 lines, -9 lines 0 comments Download
A tools/gn/variables.h View 1 chunk +122 lines, -0 lines 0 comments Download
A tools/gn/variables.cc View 1 2 1 chunk +334 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
brettw
7 years, 4 months ago (2013-08-06 20:12:01 UTC) #1
scottmg
lgtm https://codereview.chromium.org/22290010/diff/5001/tools/gn/command_desc.cc File tools/gn/command_desc.cc (right): https://codereview.chromium.org/22290010/diff/5001/tools/gn/command_desc.cc#newcode103 tools/gn/command_desc.cc:103: if (cmdline->HasSwitch(kIncludeDataDeps)) { i feel like you probably ...
7 years, 4 months ago (2013-08-06 20:27:58 UTC) #2
brettw
https://codereview.chromium.org/22290010/diff/5001/tools/gn/command_desc.cc File tools/gn/command_desc.cc (right): https://codereview.chromium.org/22290010/diff/5001/tools/gn/command_desc.cc#newcode103 tools/gn/command_desc.cc:103: if (cmdline->HasSwitch(kIncludeDataDeps)) { Okay, I removed this and included ...
7 years, 4 months ago (2013-08-06 21:10:22 UTC) #3
brettw
7 years, 4 months ago (2013-08-06 21:11:21 UTC) #4
Message was sent while issue was closed.
Committed patchset #4 manually as r215976 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698