Add PNaCl support for VS addin.

Add PNaCl support for VS addin.

The primary changes here are due to the fact that the
link phase for PNaCl differs significantly from from
NaCl one.   Also, since PNaCl toolchains is a bat file
that calls python script, we need to ensure that python
is in the path for all PNaCl builds.

The link phase was using automatic depenedacy tracking
previously but this process fails to launch the python
process successfully, so we now write the .tlog manually
in the same way we do for the compiler phase.

BUG=

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

[Sam Clegg, 2012-10-25 19:47:33]

Initial PNaCL support for VS Addin.

This version only produces .pexe files.  I'm currently adding support for
creating nexes as part of the link phase.

[Sam Clegg, 2012-10-26 23:05:17]

Now with pnacl-translate to generate 3 nexes.

[binji, 2012-10-29 18:08:59]

lgtm

http://codereview.chromium.org/11266051/diff/2001/visual_studio/NativeClientV...
File
visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/hello_world_gles.vcxproj
(right):

http://codereview.chromium.org/11266051/diff/2001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/hello_world_gles.vcxproj:121:
<ReportUndefinedSymbols>false</ReportUndefinedSymbols>
remove

http://codereview.chromium.org/11266051/diff/2001/visual_studio/NativeClientV...
File visual_studio/NativeClientVSAddIn/create_package.py (right):

http://codereview.chromium.org/11266051/diff/2001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/create_package.py:172: replacements = {
nit: probably should inline this below or rename to pnacl_replacements.

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
File
visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/hello_world_gles.vcxproj
(right):

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/hello_world_gles.vcxproj:119:
<ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|PNaCl'">
what about release?

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
File visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs
(right):

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs:160:
//File.Delete(depFilePath);
Why is this removed?

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs:281:
MessageBox.Show("Multi-processor Compilation with NaCl requires that python
available in the visual stduio executable path.\n" +
sp: visual studio

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs:281:
MessageBox.Show("Multi-processor Compilation with NaCl requires that python
available in the visual stduio executable path.\n" +
Also, "requires that python be available"

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs:370:
returnCode = base.ExecuteTool("python.exe", cmd, "\"" + pythonScript + "\"");
what if we found python.bat instead?

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs:418:
List<ITaskItem> outOfDateSourcesFromCommandLineChanges =
this.GetOutOfDateSourcesFromCommandLineChanges();
nit: wrap these lines at 100

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs:422:
this.TrackedInputFiles = new CanonicalTrackedInputFiles(this,
this.TLogReadFiles, this.Sources, this.ExcludedInputPaths, trackedOutputFiles,
true, false);
nit: specify what true/false are here?

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs:438:
trackedOutputFiles.SaveTlog();
not necessary to set SkippedExecution to false anymore?

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
File visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs (right):

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs:19: ///
Property set only in PNaCl builds to signal that
nit: comment fits on one line?

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs:58:
CanonicalTrackedOutputFiles trackedFiles = new CanonicalTrackedOutputFiles(new
TaskItem[] { item });
nit: wrap at 100, here and elsewhere

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs:101: //write
tlog
nit: remove unnecessary comment

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs:188: if
(TranslateX64 == "true" && !Translate("x86-64"))
maybe should only try translating when platform is pnacl?

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
File visual_studio/NativeClientVSAddIn/create_package.py (right):

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/create_package.py:52: r'\.swp', # Exclude .svn
directories.
is this comment correct? swp is svn?

[binji, 2012-10-29 18:09:29]

btw, any tests for pnacl...?

[Sam Clegg, 2012-10-29 21:34:17]

On 2012/10/29 18:09:29, binji wrote:
> btw, any tests for pnacl...?

Adding tests now.

[Sam Clegg, 2012-10-29 22:42:50]

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
File
visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/hello_world_gles.vcxproj
(right):

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/hello_world_gles.vcxproj:119:
<ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|PNaCl'">
On 2012/10/29 18:08:59, binji wrote:
> what about release?

Done.

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
File visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs
(right):

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs:160:
//File.Delete(depFilePath);
On 2012/10/29 18:08:59, binji wrote:
> Why is this removed?

Done.

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs:281:
MessageBox.Show("Multi-processor Compilation with NaCl requires that python
available in the visual stduio executable path.\n" +
On 2012/10/29 18:08:59, binji wrote:
> sp: visual studio

Done.

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs:281:
MessageBox.Show("Multi-processor Compilation with NaCl requires that python
available in the visual stduio executable path.\n" +
On 2012/10/29 18:08:59, binji wrote:
> Also, "requires that python be available"

Done.

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs:370:
returnCode = base.ExecuteTool("python.exe", cmd, "\"" + pythonScript + "\"");
On 2012/10/29 18:08:59, binji wrote:
> what if we found python.bat instead?

Done.

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs:418:
List<ITaskItem> outOfDateSourcesFromCommandLineChanges =
this.GetOutOfDateSourcesFromCommandLineChanges();
On 2012/10/29 18:08:59, binji wrote:
> nit: wrap these lines at 100

Done.

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClCompile.cs:438:
trackedOutputFiles.SaveTlog();
On 2012/10/29 18:08:59, binji wrote:
> not necessary to set SkippedExecution to false anymore?

I set it to false at the start of each compile in the super class.

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
File visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs (right):

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs:19: ///
Property set only in PNaCl builds to signal that
On 2012/10/29 18:08:59, binji wrote:
> nit: comment fits on one line?

Done.

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs:58:
CanonicalTrackedOutputFiles trackedFiles = new CanonicalTrackedOutputFiles(new
TaskItem[] { item });
On 2012/10/29 18:08:59, binji wrote:
> nit: wrap at 100, here and elsewhere

Done.

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs:101: //write
tlog
On 2012/10/29 18:08:59, binji wrote:
> nit: remove unnecessary comment

Done.

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs:188: if
(TranslateX64 == "true" && !Translate("x86-64"))
On 2012/10/29 18:08:59, binji wrote:
> maybe should only try translating when platform is pnacl?

None of the other platforms will set these Translate flags,
but I guess we could a second check?

[binji, 2012-10-29 23:14:02]

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
File visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs (right):

http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs:188: if
(TranslateX64 == "true" && !Translate("x86-64"))
On 2012/10/29 22:42:51, Sam Clegg wrote:
> On 2012/10/29 18:08:59, binji wrote:
> > maybe should only try translating when platform is pnacl?
> 
> None of the other platforms will set these Translate flags,
> but I guess we could a second check?

Probably not a big deal -- I was just thinking you would get weird errors if
somehow it tried to translate a nexe instead of a pexe.

http://codereview.chromium.org/11266051/diff/17027/visual_studio/NativeClient...
File visual_studio/NativeClientVSAddIn/UnitTests/StyleCop.Cache (right):

http://codereview.chromium.org/11266051/diff/17027/visual_studio/NativeClient...
visual_studio/NativeClientVSAddIn/UnitTests/StyleCop.Cache:1:
<stylecopresultscache>
is this supposed to be checked in?

[Sam Clegg, 2012-10-29 23:27:01]

On 2012/10/29 23:14:02, binji wrote:
>
http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
> File visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs
(right):
> 
>
http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
> visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs:188: if
> (TranslateX64 == "true" && !Translate("x86-64"))
> On 2012/10/29 22:42:51, Sam Clegg wrote:
> > On 2012/10/29 18:08:59, binji wrote:
> > > maybe should only try translating when platform is pnacl?
> > 
> > None of the other platforms will set these Translate flags,
> > but I guess we could a second check?
> 
> Probably not a big deal -- I was just thinking you would get weird errors if
> somehow it tried to translate a nexe instead of a pexe.

Will fix.
> 
>
http://codereview.chromium.org/11266051/diff/17027/visual_studio/NativeClient...
> File visual_studio/NativeClientVSAddIn/UnitTests/StyleCop.Cache (right):
> 
>
http://codereview.chromium.org/11266051/diff/17027/visual_studio/NativeClient...
> visual_studio/NativeClientVSAddIn/UnitTests/StyleCop.Cache:1:
> <stylecopresultscache>
> is this supposed to be checked in?

[Sam Clegg, 2012-10-29 23:34:21]

On 2012/10/29 23:27:01, Sam Clegg wrote:
> On 2012/10/29 23:14:02, binji wrote:
> >
>
http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
> > File visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs
> (right):
> > 
> >
>
http://codereview.chromium.org/11266051/diff/6001/visual_studio/NativeClientV...
> > visual_studio/NativeClientVSAddIn/NaCl.Build.CPPTasks/NaClLink.cs:188: if
> > (TranslateX64 == "true" && !Translate("x86-64"))
> > On 2012/10/29 22:42:51, Sam Clegg wrote:
> > > On 2012/10/29 18:08:59, binji wrote:
> > > > maybe should only try translating when platform is pnacl?
> > > 
> > > None of the other platforms will set these Translate flags,
> > > but I guess we could a second check?
> > 
> > Probably not a big deal -- I was just thinking you would get weird errors if
> > somehow it tried to translate a nexe instead of a pexe.
> 
> Will fix.

Actually it its OK with you I'm going to fix this in my next CL which adds
createnmf as another optional post-link.
> > 
> >
>
http://codereview.chromium.org/11266051/diff/17027/visual_studio/NativeClient...
> > File visual_studio/NativeClientVSAddIn/UnitTests/StyleCop.Cache (right):
> > 
> >
>
http://codereview.chromium.org/11266051/diff/17027/visual_studio/NativeClient...
> > visual_studio/NativeClientVSAddIn/UnitTests/StyleCop.Cache:1:
> > <stylecopresultscache>
> > is this supposed to be checked in?

[Sam Clegg, 2012-10-30 20:30:28]

http://codereview.chromium.org/11266051/diff/17027/visual_studio/NativeClient...
File visual_studio/NativeClientVSAddIn/UnitTests/StyleCop.Cache (right):

http://codereview.chromium.org/11266051/diff/17027/visual_studio/NativeClient...
visual_studio/NativeClientVSAddIn/UnitTests/StyleCop.Cache:1:
<stylecopresultscache>
On 2012/10/29 23:14:02, binji wrote:
> is this supposed to be checked in?

No.  Done.

[binji, 2012-10-30 20:37:40]

http://codereview.chromium.org/11266051/diff/10008/visual_studio/NativeClient...
File
visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/PNaCl/hello_world_gles.nmf
(right):

http://codereview.chromium.org/11266051/diff/10008/visual_studio/NativeClient...
visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/PNaCl/hello_world_gles.nmf:4:
"x86-64": {
64-bit only?

http://codereview.chromium.org/11266051/diff/10008/visual_studio/NativeClient...
File visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/PropertyManager.cs
(right):

http://codereview.chromium.org/11266051/diff/10008/visual_studio/NativeClient...
visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/PropertyManager.cs:29:
public VCConfiguration configuration_;
why does this need to be public? Also, I think the style guide says to remove
trailing underscore for public members.

http://codereview.chromium.org/11266051/diff/10008/visual_studio/NativeClient...
visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/PropertyManager.cs:251:
return configuration_.Platform.Name.Equals(Strings.PNaClPlatformName,
StringComparison.OrdinalIgnoreCase);
nit: wrap at 100

[Sam Clegg, 2012-10-30 21:01:44]

http://codereview.chromium.org/11266051/diff/10008/visual_studio/NativeClient...
File
visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/PNaCl/hello_world_gles.nmf
(right):

http://codereview.chromium.org/11266051/diff/10008/visual_studio/NativeClient...
visual_studio/NativeClientVSAddIn/InstallerResources/examples/hello_world_gles/hello_world_gles/PNaCl/hello_world_gles.nmf:4:
"x86-64": {
On 2012/10/30 20:37:40, binji wrote:
> 64-bit only?

For now.  The next change is to introduce automatic nmf generation which will
fix this.

http://codereview.chromium.org/11266051/diff/10008/visual_studio/NativeClient...
File visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/PropertyManager.cs
(right):

http://codereview.chromium.org/11266051/diff/10008/visual_studio/NativeClient...
visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/PropertyManager.cs:29:
public VCConfiguration configuration_;
On 2012/10/30 20:37:40, binji wrote:
> why does this need to be public? Also, I think the style guide says to remove
> trailing underscore for public members.

Done.

http://codereview.chromium.org/11266051/diff/10008/visual_studio/NativeClient...
visual_studio/NativeClientVSAddIn/NativeClientVSAddIn/PropertyManager.cs:251:
return configuration_.Platform.Name.Equals(Strings.PNaClPlatformName,
StringComparison.OrdinalIgnoreCase);
On 2012/10/30 20:37:40, binji wrote:
> nit: wrap at 100

Done.

[binji, 2012-10-30 23:52:26]

lgtm