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

Issue 11547004: [NaCl SDK Addin] Add arm support in visual studio (Closed)

Created:
8 years ago by Sam Clegg
Modified:
8 years ago
Reviewers:
binji
CC:
native-client-reviews_googlegroups.com
Visibility:
Public.

Description

[NaCl SDK Addin] Add arm support in visual studio Also, don't pass -t to create_nmf on newer versions of the SDK. BUG= Committed: https://code.google.com/p/nativeclient-sdk/source/detail?r=1473

Patch Set 1 #

Total comments: 13

Patch Set 2 : fix nits #

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+243 lines, -131 lines) Patch
M .gitignore View 1 chunk +1 line, -0 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles.sln View 4 chunks +7 lines, -1 line 0 comments Download
M visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/hello_world_gles.vcxproj View 1 4 chunks +19 lines, -1 line 0 comments Download
M visual_studio/NativeClientVSAddIn/InstallerResources/install.py View 7 chunks +27 lines, -21 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs View 1 1 chunk +8 lines, -1 line 0 comments Download
M visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/SDKUtilities.cs View 1 2 1 chunk +22 lines, -4 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/PropertyManager.cs View 1 chunk +1 line, -0 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Strings.resx View 1 chunk +3 lines, -0 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/Strings.Designer.cs View 2 chunks +10 lines, -1 line 0 comments Download
M visual_studio/NativeClientVSAddIn/UnitTests/CompileTest.cs View 1 2 8 chunks +24 lines, -9 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/UnitTests/ProjectSettingsTest.cs View 13 chunks +39 lines, -44 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/UnitTests/PropertyManagerTest.cs View 10 chunks +48 lines, -28 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/UnitTests/TestUtilities.cs View 1 3 chunks +17 lines, -17 lines 0 comments Download
M visual_studio/NativeClientVSAddIn/create_package.py View 1 2 chunks +17 lines, -4 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Sam Clegg
8 years ago (2012-12-12 00:05:00 UTC) #1
binji
Few questions, but otherwise lgtm https://codereview.chromium.org/11547004/diff/1/visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/hello_world_gles.vcxproj File visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/hello_world_gles.vcxproj (right): https://codereview.chromium.org/11547004/diff/1/visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/hello_world_gles.vcxproj#newcode144 visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/hello_world_gles.vcxproj:144: <ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|NaClARM'"> only debug? ...
8 years ago (2012-12-12 00:25:45 UTC) #2
Sam Clegg
8 years ago (2012-12-12 00:51:30 UTC) #3
https://codereview.chromium.org/11547004/diff/1/visual_studio/NativeClientVSA...
File
visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/hello_world_gles.vcxproj
(right):

https://codereview.chromium.org/11547004/diff/1/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/hello_world_gles.vcxproj:144:
<ItemDefinitionGroup
Condition="'$(Configuration)|$(Platform)'=='Debug|NaClARM'">
On 2012/12/12 00:25:45, binji wrote:
> only debug?

Done.

https://codereview.chromium.org/11547004/diff/1/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/hello_world_gles.vcxproj:145:
<ClCompile />
On 2012/12/12 00:25:45, binji wrote:
> why does this one have ClCompile? None of the others do.

Done.

https://codereview.chromium.org/11547004/diff/1/visual_studio/NativeClientVSA...
File visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/SDKUtilities.cs
(right):

https://codereview.chromium.org/11547004/diff/1/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/SDKUtilities.cs:74: ///
Retrun true if the NaCl SDK at the given location is at the given
On 2012/12/12 00:25:45, binji wrote:
> s/Retrun/Return/

Done.

https://codereview.chromium.org/11547004/diff/1/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/SDKUtilities.cs:77: public
static bool CheckVersion(string root, int version, int revision)
On 2012/12/12 00:25:45, binji wrote:
> maybe CheckVersionAtLeast (or something), I thought you were checking for an
> exact version at first.

Done.

https://codereview.chromium.org/11547004/diff/1/visual_studio/NativeClientVSA...
File visual_studio/NativeClientVSAddIn/UnitTests/TestUtilities.cs (right):

https://codereview.chromium.org/11547004/diff/1/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/UnitTests/TestUtilities.cs:109: // Order
matters if copying from the other Native Client type.
On 2012/12/12 00:25:45, binji wrote:
> this seems like it needs a few more cases now that there are multiple nacl
> types, right?
> 
> What are valid values for pepperCopyFrom?

Done.

https://codereview.chromium.org/11547004/diff/1/visual_studio/NativeClientVSA...
File visual_studio/NativeClientVSAddIn/create_package.py (right):

https://codereview.chromium.org/11547004/diff/1/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/create_package.py:185: replacements = {
On 2012/12/12 00:25:45, binji wrote:
> maybe should rename this to pnacl_replacements now

Done.

Powered by Google App Engine
This is Rietveld 408576698