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

Issue 321323004: Add yasm to the GN build (Closed)

Created:
6 years, 6 months ago by brettw
Modified:
6 years, 6 months ago
Reviewers:
jamesr, awong
CC:
chromium-reviews, darin-cc_chromium.org, jam, tfarina
Visibility:
Public.

Description

Add yasm to the GN build. This is forked off of https://codereview.chromium.org/266613002 The code to compile yasm itself is mostly from that patch, with a few updates for the other changes. Adds a template for running compiled binaries. Compared to Albert's patch above, this assumes the binary is generated by the source tree so can have a cleaner interface (just specify the label of the tool you use). The yasm rule is new compared to Albert's patch. It uses a special wrapper script instead of the new compiled_action templates so it can properly support depfiles. This also adds convenient support for defines and include dirs. This adds some trivial ios changes to the content/public/browser. This should be a NOP now. Fixes a bug in GN depfile creation. Previously, the .d file was emitted as an output of the script, but this confuses ninja. This change just removes that. BUG= R=ajwong@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=276772

Patch Set 1 #

Patch Set 2 : #

Total comments: 4

Patch Set 3 : Now with less disaster #

Patch Set 4 : y #

Patch Set 5 : #

Patch Set 6 : x86/x64 only #

Patch Set 7 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+835 lines, -40 lines) Patch
M BUILD.gn View 1 2 3 4 5 1 chunk +5 lines, -0 lines 0 comments Download
A build/compiled_action.gni View 1 2 3 4 1 chunk +148 lines, -0 lines 0 comments Download
A build/gn_run_binary.py View 1 1 chunk +22 lines, -0 lines 0 comments Download
M content/public/browser/BUILD.gn View 1 chunk +16 lines, -2 lines 0 comments Download
A third_party/yasm/BUILD.gn View 1 chunk +402 lines, -0 lines 0 comments Download
A third_party/yasm/run_yasm.py View 1 2 3 4 5 6 1 chunk +51 lines, -0 lines 0 comments Download
A third_party/yasm/yasm_assemble.gni View 1 2 3 1 chunk +185 lines, -0 lines 0 comments Download
M tools/gn/function_rebase_path.cc View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M tools/gn/ninja_action_target_writer.cc View 2 chunks +0 lines, -10 lines 0 comments Download
M tools/gn/ninja_action_target_writer_unittest.cc View 1 2 3 chunks +4 lines, -28 lines 0 comments Download

Messages

Total messages: 11 (0 generated)
brettw
6 years, 6 months ago (2014-06-11 20:21:05 UTC) #1
awong
2 small nits https://codereview.chromium.org/321323004/diff/20001/build/compiled_action.gni File build/compiled_action.gni (right): https://codereview.chromium.org/321323004/diff/20001/build/compiled_action.gni#newcode63 build/compiled_action.gni:63: # need a target build of ...
6 years, 6 months ago (2014-06-11 20:34:03 UTC) #2
brettw
https://codereview.chromium.org/321323004/diff/20001/third_party/yasm/run_yasm.py File third_party/yasm/run_yasm.py (right): https://codereview.chromium.org/321323004/diff/20001/third_party/yasm/run_yasm.py#newcode33 third_party/yasm/run_yasm.py:33: # Assemble. On 2014/06/11 20:34:03, awong wrote: > nit: ...
6 years, 6 months ago (2014-06-11 20:45:08 UTC) #3
awong
On 2014/06/11 20:45:08, brettw wrote: > https://codereview.chromium.org/321323004/diff/20001/third_party/yasm/run_yasm.py > File third_party/yasm/run_yasm.py (right): > > https://codereview.chromium.org/321323004/diff/20001/third_party/yasm/run_yasm.py#newcode33 > ...
6 years, 6 months ago (2014-06-11 22:05:45 UTC) #4
brettw
I guess I can change this, but it make me nervous. If yasm outputs anything ...
6 years, 6 months ago (2014-06-11 22:21:38 UTC) #5
awong
On 2014/06/11 22:21:38, brettw wrote: > I guess I can change this, but it make ...
6 years, 6 months ago (2014-06-11 22:22:43 UTC) #6
brettw
On 2014/06/11 22:22:43, awong wrote: > On 2014/06/11 22:21:38, brettw wrote: > > I guess ...
6 years, 6 months ago (2014-06-11 22:36:25 UTC) #7
awong
Regarding doing deps for or second, you're right that the deps file timestamp is irrelevant. ...
6 years, 6 months ago (2014-06-11 22:42:37 UTC) #8
brettw
PTAL
6 years, 6 months ago (2014-06-12 18:06:56 UTC) #9
awong
LGTM ship it.
6 years, 6 months ago (2014-06-12 18:42:10 UTC) #10
brettw
6 years, 6 months ago (2014-06-12 19:36:04 UTC) #11
Message was sent while issue was closed.
Committed patchset #7 manually as r276772 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698