Add support for building the new analyzer using gyp.

Add support for building the new analyzer using gyp.

Do we have a name for the new analyzer? I don't really like the new prefix I use for building and naming.

Also, I don't particularly like the way I handle the jar files, but I
did not find a way to easily make a new string out of a list of
strings using a non space separator.

Additionally, I currently copy in the dependent on jar files so that
we can easily wrap this up, I could make the manifest just refer to
the the files relative to tbe build dir, but that is just asking for
trouble in the long term IMHO.
Committed: https://code.google.com/p/dart/source/detail?r=18804

[ahe, 2013-02-14 14:28:16]

https://codereview.chromium.org/12262040/diff/7/editor/analyzer.gyp
File editor/analyzer.gyp (right):

https://codereview.chromium.org/12262040/diff/7/editor/analyzer.gyp#newcode42
editor/analyzer.gyp:42: 'destination': '<(PRODUCT_DIR)/<(analyzer_name)',
I don't think this is correct. I don't know how GYP tracks time stamps on
"copies", but if it uses the directory, you change that timestamp later.

My suggested fix is to place all dependent JAR files in a subdirectory which
contains no other outputs.

https://codereview.chromium.org/12262040/diff/7/editor/analyzer.gyp#newcode64
editor/analyzer.gyp:64: 'message': 'Creating classpath file.',
I'd say:

'Creating <(_outputs)'

https://codereview.chromium.org/12262040/diff/7/editor/analyzer.gyp#newcode76
editor/analyzer.gyp:76: 'javac',
Specify -sourcepath, -source 6, -target 6, and -implicit:none

https://codereview.chromium.org/12262040/diff/7/editor/analyzer.gyp#newcode82
editor/analyzer.gyp:82: 'message': 'Creating analyzer.',
I'd use the same message as above.

[ahe, 2013-02-14 14:52:34]

A few Python suggestions.

https://codereview.chromium.org/12262040/diff/7001/editor/tools/write_class_p...
File editor/tools/write_class_path_file.py (right):

https://codereview.chromium.org/12262040/diff/7001/editor/tools/write_class_p...
editor/tools/write_class_path_file.py:17: output = open(sys.argv[1], 'w')
I'd use:

with open(...) as output:

https://codereview.chromium.org/12262040/diff/7001/editor/tools/write_class_p...
editor/tools/write_class_path_file.py:18: output.write('Class-Path:')
Here I would use:

print >> output, 'Class-Path:',

[ricow1, 2013-02-14 15:40:20]

PTAL

https://codereview.chromium.org/12262040/diff/7/editor/analyzer.gyp
File editor/analyzer.gyp (right):

https://codereview.chromium.org/12262040/diff/7/editor/analyzer.gyp#newcode42
editor/analyzer.gyp:42: 'destination': '<(PRODUCT_DIR)/<(analyzer_name)',
On 2013/02/14 14:28:16, ahe wrote:
> I don't think this is correct. I don't know how GYP tracks time stamps on
> "copies", but if it uses the directory, you change that timestamp later.
> 
> My suggested fix is to place all dependent JAR files in a subdirectory which
> contains no other outputs. 
Do you mean as in this will make it non-incremental?
From the gyp spec:
The copies will be created in destination and have the same file name as the
file they are copied from. Even if the files are from multiple directories they
will all be copied into the destination directory. Each destination file has an
implicit build dependency on the file it is copied from.

So I don't think that is a problem.

https://codereview.chromium.org/12262040/diff/7/editor/analyzer.gyp#newcode64
editor/analyzer.gyp:64: 'message': 'Creating classpath file.',
On 2013/02/14 14:28:16, ahe wrote:
> I'd say:
> 
> 'Creating <(_outputs)'

Done.

https://codereview.chromium.org/12262040/diff/7/editor/analyzer.gyp#newcode76
editor/analyzer.gyp:76: 'javac',
On 2013/02/14 14:28:16, ahe wrote:
> Specify -sourcepath, -source 6, -target 6, and -implicit:none
Done

https://codereview.chromium.org/12262040/diff/7/editor/analyzer.gyp#newcode76
editor/analyzer.gyp:76: 'javac',
On 2013/02/14 14:28:16, ahe wrote:
> Specify -sourcepath, -source 6, -target 6, and -implicit:none
As discussed offline, done and moved to a python script

https://codereview.chromium.org/12262040/diff/7/editor/analyzer.gyp#newcode82
editor/analyzer.gyp:82: 'message': 'Creating analyzer.',
On 2013/02/14 14:28:16, ahe wrote:
> I'd use the same message as above.

Done.

https://codereview.chromium.org/12262040/diff/7001/editor/tools/write_class_p...
File editor/tools/write_class_path_file.py (right):

https://codereview.chromium.org/12262040/diff/7001/editor/tools/write_class_p...
editor/tools/write_class_path_file.py:17: output = open(sys.argv[1], 'w')
On 2013/02/14 14:52:34, ahe wrote:
> I'd use:
> 
> with open(...) as output:

Done.

https://codereview.chromium.org/12262040/diff/7001/editor/tools/write_class_p...
editor/tools/write_class_path_file.py:18: output.write('Class-Path:')
On 2013/02/14 14:52:34, ahe wrote:
> Here I would use:
> 
> print >> output, 'Class-Path:',

Done.

[ahe, 2013-02-14 16:34:09]

Rico, I'm so sorry. I keep finding problems with this approach. It takes me a
while to fully switch to GYP mode.

https://codereview.chromium.org/12262040/diff/7/editor/analyzer.gyp
File editor/analyzer.gyp (right):

https://codereview.chromium.org/12262040/diff/7/editor/analyzer.gyp#newcode42
editor/analyzer.gyp:42: 'destination': '<(PRODUCT_DIR)/<(analyzer_name)',
On 2013/02/14 15:40:20, ricow1 wrote:
> On 2013/02/14 14:28:16, ahe wrote:
> > I don't think this is correct. I don't know how GYP tracks time stamps on
> > "copies", but if it uses the directory, you change that timestamp later.
> > 
> > My suggested fix is to place all dependent JAR files in a subdirectory which
> > contains no other outputs. 
> Do you mean as in this will make it non-incremental?
> From the gyp spec:
> The copies will be created in destination and have the same file name as the
> file they are copied from. Even if the files are from multiple directories
they
> will all be copied into the destination directory. Each destination file has
an
> implicit build dependency on the file it is copied from.
> 
> So I don't think that is a problem.

I agree. Sorry for the confusion.

However, I still think this feature of GYP is flawed. What happens if you delete
a file? I don't think it will get deleted. That wouldn't be a big deal if we
regularly did full builds. So we should prioritize that.

https://codereview.chromium.org/12262040/diff/3004/editor/analyzer.gyp
File editor/analyzer.gyp (right):

https://codereview.chromium.org/12262040/diff/3004/editor/analyzer.gyp#newcode28
editor/analyzer.gyp:28: 'compilation_command': [
Unused.

https://codereview.chromium.org/12262040/diff/3004/editor/analyzer.gyp#newcode76
editor/analyzer.gyp:76: '<(PRODUCT_DIR)/<(analyzer_name)/com',
This is a directory dependency. I'm not convinced they work. See my comments
below.

https://codereview.chromium.org/12262040/diff/3004/editor/analyzer.gyp#newcode99
editor/analyzer.gyp:99: 'jar',
Probably needs to be wrapped in a python program. I think I can find a python
program that compiles and creates JAR files in one go.

[ricow1, 2013-02-15 12:01:36]

https://codereview.chromium.org/12262040/diff/3004/editor/analyzer.gyp
File editor/analyzer.gyp (right):

https://codereview.chromium.org/12262040/diff/3004/editor/analyzer.gyp#newcode28
editor/analyzer.gyp:28: 'compilation_command': [
On 2013/02/14 16:34:10, ahe wrote:
> Unused.

Done.

https://codereview.chromium.org/12262040/diff/3004/editor/analyzer.gyp#newcode76
editor/analyzer.gyp:76: '<(PRODUCT_DIR)/<(analyzer_name)/com',
On 2013/02/14 16:34:10, ahe wrote:
> This is a directory dependency. I'm not convinced they work. See my comments
> below.
I changed this and below to rely on the main file

https://codereview.chromium.org/12262040/diff/3004/editor/analyzer.gyp#newcode99
editor/analyzer.gyp:99: 'jar',
On 2013/02/14 16:34:10, ahe wrote:
> Probably needs to be wrapped in a python program. I think I can find a python
> program that compiles and creates JAR files in one go.

Done.

[ricow1, 2013-02-19 10:33:17]

PTAL

[ahe, 2013-02-19 10:38:38]

I think you forgot the stuff we discussed in person. Please chat me with
follow-up questions if it isn't clear.

https://codereview.chromium.org/12262040/diff/16001/editor/analyzer.gyp
File editor/analyzer.gyp (right):

https://codereview.chromium.org/12262040/diff/16001/editor/analyzer.gyp#newco...
editor/analyzer.gyp:30: 'compilation_classpath':
'<(PRODUCT_DIR)/<(analyzer_name)/*',
This means that you expect that the files have been copied here first, but
you're not guaranteeing that.

https://codereview.chromium.org/12262040/diff/16001/editor/analyzer.gyp#newco...
editor/analyzer.gyp:38: '<@(java_source_files)',
I don't understand this input.

[ricow1, 2013-02-19 10:44:48]

https://codereview.chromium.org/12262040/diff/16001/editor/analyzer.gyp
File editor/analyzer.gyp (right):

https://codereview.chromium.org/12262040/diff/16001/editor/analyzer.gyp#newco...
editor/analyzer.gyp:30: 'compilation_classpath':
'<(PRODUCT_DIR)/<(analyzer_name)/*',
On 2013/02/19 10:38:38, ahe wrote:
> This means that you expect that the files have been copied here first, but
> you're not guaranteeing that.
I don't expect that, it does not matter, we just reference these files from the
Manifest, not when we compile

https://codereview.chromium.org/12262040/diff/16001/editor/analyzer.gyp#newco...
editor/analyzer.gyp:38: '<@(java_source_files)',
On 2013/02/19 10:38:38, ahe wrote:
> I don't understand this input.
Done, that is _not_ a input for this

[ricow1, 2013-02-19 10:45:58]

https://codereview.chromium.org/12262040/diff/16001/editor/analyzer.gyp
File editor/analyzer.gyp (right):

https://codereview.chromium.org/12262040/diff/16001/editor/analyzer.gyp#newco...
editor/analyzer.gyp:30: 'compilation_classpath':
'<(PRODUCT_DIR)/<(analyzer_name)/*',
On 2013/02/19 10:44:48, ricow1 wrote:
> On 2013/02/19 10:38:38, ahe wrote:
> > This means that you expect that the files have been copied here first, but
> > you're not guaranteeing that.
> I don't expect that, it does not matter, we just reference these files from
the
> Manifest, not when we compile
Yeah, I see why, sorry, will fix

[ricow1, 2013-02-19 13:50:39]

unified in one python file

[ahe, 2013-02-20 12:52:03]

LGTM!

[ricow1, 2013-02-21 08:25:24]

Committed patchset #10 manually as r18804 (presubmit successful).

[aam-me, 2013-03-16 04:27:36]

As far as I can see there are problems with this script on Windows - build of
analyzer fails there:

1>------ Build started: Project: analyzer, Configuration: ReleaseIA32 Win32
------
1>  Creating "..\build\ReleaseIA32\/new_analyzer/new_analyzer.jar".
1>  The system cannot execute the specified program.
1>C:\Program Files
(x86)\MSBuild\Microsoft.Cpp\v4.0\V110\Microsoft.CppCommon.targets(172,5): error
MSB6006: "cmd.exe" exited with code 9020.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

There are two problems as far as I can see:
1) generated command line that invokes compile_analyzer.py lists all java
sources and is over 44k characters long. Windows supports only 8192 characters
long command lines. That I believe results in "The system cannot execute the
specified program." error message.
2) If you fix first problem(by using tempfile with java filenames),
compile_analyzer.py still will fail because javac on windows doesn't handle
wildcard expansion properly in class path
argument(http://stackoverflow.com/questions/219585/setting-multiple-jars-in-java-classpath,
http://stackoverflow.com/questions/11607873/escape-wildcard-processing-in-jav...)

Anyway, in case I'm still making sense, to solve first problem I would move java
source list creation into compile_analyzer.py from gyp and  create temporary
file with all java source listed in there. 
To solve second problem(per second stackoverflow article) on Windows I would
have '*;' rather than '*' at the end of class path.

Hope it helps!