[VS AddIn] Fix default library paths.

[VS AddIn] Fix default library paths.

Also, add pnacl-finalize step to PNaCl build.

The default library path for all platforms was wrong and
did not include the location of the SDK libraries such
as ppapi_cpp.  There was a bug in the test code that meant
that we didn't catch this since we where not actually linking
any of the test projects to .nexe or .pexe files.

R=binji@chromium.org

Committed: https://code.google.com/p/nativeclient-sdk/source/detail?r=1567

[Sam Clegg, 2013-11-19 21:44:18]

I'm really not sure how this went unnoticed for so long :(
I mean I found out why the unittests didn't catch it but
I really should have found this through manual testing at
an earlier date.

[binji, 2013-11-19 21:57:16]

lgtm, I think

https://codereview.chromium.org/69343013/diff/1/visual_studio/NativeClientVSA...
File
visual_studio/NativeClientVSAddIn/InstallerResources/NaCl/NaCl.Paths.default.props
(right):

https://codereview.chromium.org/69343013/diff/1/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/InstallerResources/NaCl/NaCl.Paths.default.props:25:
<LibraryPath Condition="'$(LibraryPath)' == '' AND
'$(TargetArchitecture)'=='i686'">$(VSNaClSDKRoot)lib\$(ToolchainName)_x86_32\$(Configuration);</LibraryPath>
I don't fully understand this, but it looks like it is only added if LibraryPath
is not defined. Is that what we want? I figured we would always want to add that
path.

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

https://codereview.chromium.org/69343013/diff/1/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs:104: if
(ExecuteTool(finalize, cmd, string.Empty) != 0)
You could do just

return ExecuteTool(...) == 0;

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

https://codereview.chromium.org/69343013/diff/1/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/XamlParser.cs:52: public
string Parse(ITaskItem taskItem, bool fullOutputName, string extension)
this looks like it only changes the output file's extension, maybe change the
variable name to reflect that?

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

https://codereview.chromium.org/69343013/diff/1/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs:116:
SetProjectType("Application", platform);
you changed Executable -> Application... what's the difference?

[Sam Clegg, 2013-11-19 23:21:24]

https://codereview.chromium.org/69343013/diff/1/visual_studio/NativeClientVSA...
File
visual_studio/NativeClientVSAddIn/InstallerResources/NaCl/NaCl.Paths.default.props
(right):

https://codereview.chromium.org/69343013/diff/1/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/InstallerResources/NaCl/NaCl.Paths.default.props:25:
<LibraryPath Condition="'$(LibraryPath)' == '' AND
'$(TargetArchitecture)'=='i686'">$(VSNaClSDKRoot)lib\$(ToolchainName)_x86_32\$(Configuration);</LibraryPath>
On 2013/11/19 21:57:16, binji wrote:
> I don't fully understand this, but it looks like it is only added if
LibraryPath
> is not defined. Is that what we want? I figured we would always want to add
that
> path.

This is just setting the default library path, yes.

This is how VS does this generally.  For example the default include
and lib paths point to the visual studio libs and headers.  If you
remove these paths that is your prerogative.  I think it makes sense
to mimic them.

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

https://codereview.chromium.org/69343013/diff/1/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs:104: if
(ExecuteTool(finalize, cmd, string.Empty) != 0)
On 2013/11/19 21:57:16, binji wrote:
> You could do just
> 
> return ExecuteTool(...) == 0;

Done.

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

https://codereview.chromium.org/69343013/diff/1/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/XamlParser.cs:52: public
string Parse(ITaskItem taskItem, bool fullOutputName, string extension)
On 2013/11/19 21:57:16, binji wrote:
> this looks like it only changes the output file's extension, maybe change the
> variable name to reflect that?

Done.

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

https://codereview.chromium.org/69343013/diff/1/visual_studio/NativeClientVSA...
visual_studio/NativeClientVSAddIn/UnitTests/BaseCompileTest.cs:116:
SetProjectType("Application", platform);
On 2013/11/19 21:57:16, binji wrote:
> you changed Executable -> Application... what's the difference?

This was the main bug.  "Executable" is not a valid value for project type so we
were never building executables.

[Sam Clegg, 2013-11-19 23:21:29]

Committed patchset #3 manually as r1567 (presubmit successful).