Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(38)

Issue 74690043: Fix GYP rules for MASM to allow Ninja build. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 2 months ago by etienneb
Modified:
10 years, 2 months ago
CC:
sawbuck-changes_googlegroups.com
Base URL:
http://sawbuck.googlecode.com/svn/trunk
Visibility:
Public.

Description

Fix GYP rules for MASM to allow Ninja build. By looking at https://codereview.chromium.org/11738002 file: nss.gyp they use 'process_outputs_as_sources': 1 I looked at the generated ninja file, the option adds the asm.obj file to the link command-line. Which solve the missing symbols when compiling with ninja: [35/35] LINK_EMBED core_unittests.exe FAILED: D:\src\syzygy\src\third_party\python_26\python.exe gyp-win-tool link-with-manifests environment.x86 True core_unittests.exe "D:\src\ syzygy\src\third_party\python_26\python.exe gyp-win-tool link-wrapper environment.x86 False link.exe /nologo /OUT:core_unittests.exe @core_u nittests.exe.rsp" 1 mt.exe rc.exe "obj\syzygy\core\core_unittests.core_unittests.exe.intermediate.manifest" obj\syzygy\core\core_unittests.c ore_unittests.exe.generated.manifest core_unittests.disassembler_unittest.obj : error LNK2019: unresolved external symbol _assembly_func referenced in function "private: virtual void __thiscall core::DisassemblerTest_DisassembleFull_Test::TestBody(void)" (?TestBody@DisassemblerTest_DisassembleFull_Test@core@@EAEXXZ) It seems to work with MSVS in both cases. So, if somebody knows a reason to keep this to '0', please speak now or stay quiet forever. R=chrisha@chromium.org, rogerm@chromium.org, chrisha, rogerm BUG= Committed: https://code.google.com/p/sawbuck/source/detail?r=2071

Patch Set 1 #

Patch Set 2 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M syzygy/build/masm.gypi View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 4
etienneb
PTAL.
10 years, 2 months ago (2014-03-12 13:13:28 UTC) #1
chrisha
lgtm
10 years, 2 months ago (2014-03-12 14:30:11 UTC) #2
Roger McFarlane
LGTM This didn't work when I originally wrote that rule, but if it does now, ...
10 years, 2 months ago (2014-03-12 14:52:46 UTC) #3
etienneb
10 years, 2 months ago (2014-03-12 14:53:29 UTC) #4
Message was sent while issue was closed.
Committed patchset #2 manually as r2071 (presubmit successful).
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b