Convert java library javac/jar to python

Convert java library javac/jar to python

This is a fairly straightforward translation of the ant build script
into two python actions.

Two things have been moved into the gyp file: adding android.jar to the
classpath and specifying the jar_excludes (currently just R.class and
R$*.class).

BUG=158821
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=188594

[Yaron, 2013-03-14 23:03:50]

https://codereview.chromium.org/12853002/diff/6002/build/android/jar.py
File build/android/jar.py (right):

https://codereview.chromium.org/12853002/diff/6002/build/android/jar.py#newco...
build/android/jar.py:53: 
double trailing ws?

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py
File build/android/javac.py (right):

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:18: srcdirs = build_utils.ParseGypList(options.src_dirs)
nit:src_dirs

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:20: if len(options.javac_includes) > 0:
nit: omit "> 0"

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:30: # crash... Sorted order works, so use that.
nice

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:36: build_utils.DeleteDirectory(intermediate_dir)
Add comment about deleting .class files (similar to chromium-jars.xml)

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:41: '-g',
Shouldn't this indet match the ' above?

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:52: parser.add_option('--intermediate-dir')
Can you add the description fields to each of these

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:53: parser.add_option('--input-jars-paths')
Any reason not to rename this classpath?

https://codereview.chromium.org/12853002/diff/6002/build/android/pylib/build_...
File build/android/pylib/build_utils.py (right):

https://codereview.chromium.org/12853002/diff/6002/build/android/pylib/build_...
build/android/pylib/build_utils.py:6: import itertools
unused

https://codereview.chromium.org/12853002/diff/6002/build/android/pylib/build_...
build/android/pylib/build_utils.py:45: # With the ninja generator, some list
members are in double quotes,
Why would we have these charcters in strings? "$" for inner classes? Double
quote?

https://codereview.chromium.org/12853002/diff/6002/build/java.gypi
File build/java.gypi (right):

https://codereview.chromium.org/12853002/diff/6002/build/java.gypi#newcode179
build/java.gypi:179: '--intermediate-dir=<(classes_dir)',
nit: classes-dir (or output-dir)

https://codereview.chromium.org/12853002/diff/6002/build/java.gypi#newcode204
build/java.gypi:204: '--jar-excludes=<(jar_excludes)',
Nit: I think it's clearer as --excluded-clases

[newt (away), 2013-03-15 03:25:04]

I like the direction!

https://codereview.chromium.org/12853002/diff/6002/build/android/jar.py
File build/android/jar.py (right):

https://codereview.chromium.org/12853002/diff/6002/build/android/jar.py#newco...
build/android/jar.py:28: class_files = map(lambda f: os.path.relpath(f,
jar_cwd), class_files)
I think a list comprehension is preferred:

class_files = [os.path.relpath(f, jar_cwd) for f in class_files]

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py
File build/android/javac.py (right):

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:20: if len(options.javac_includes) > 0:
nit: omit "len()"

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:26: filtered_java_files.append(f)
break (so you don't end up with the same file included twice)

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:32: java_files = sorted(java_files)
or java_files.sort()

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:43: '-source', '1.5',
I thought we used 1.6, but I trust you here

https://codereview.chromium.org/12853002/diff/6002/build/android/pylib/build_...
File build/android/pylib/build_utils.py (right):

https://codereview.chromium.org/12853002/diff/6002/build/android/pylib/build_...
build/android/pylib/build_utils.py:11: def EnsureDirectoryExists(dir_path):
MakeDirIfNotExists?  the current name sounds assert-y... but maybe that's just
me.

https://codereview.chromium.org/12853002/diff/6002/build/android/pylib/build_...
build/android/pylib/build_utils.py:33: fnmatch.filter(filenames, filter)))
list comprehension?

    rel_files = fnmatch.filter(filenames, filter)
    files += [os.path.join(root, f) for f in rel_files]

https://codereview.chromium.org/12853002/diff/6002/build/android/pylib/build_...
build/android/pylib/build_utils.py:44: def ParseGypList(gyp_string):
gyp_string = gyp_string.replace('##','$')
return shlex.split(gyp_string)

https://codereview.chromium.org/12853002/diff/6002/build/android/pylib/build_...
build/android/pylib/build_utils.py:50: # The ninja generator doesn't support $
in strings, so use ## to
is there any way we can change the generator to support $?  it would be nice to
have this escaping built into gyp.

https://codereview.chromium.org/12853002/diff/6002/build/java.gypi
File build/java.gypi (right):

https://codereview.chromium.org/12853002/diff/6002/build/java.gypi#newcode186
build/java.gypi:186: '--ignore=>(_inputs)',
not related (at all), but...
we could pass the md5sum of the inputs as the ignore argument... that would make
the argument list *much* shorter, which is helpful in the case of errors

[cjhopman, 2013-03-15 22:44:39]

https://codereview.chromium.org/12853002/diff/6002/build/android/jar.py
File build/android/jar.py (right):

https://codereview.chromium.org/12853002/diff/6002/build/android/jar.py#newco...
build/android/jar.py:28: class_files = map(lambda f: os.path.relpath(f,
jar_cwd), class_files)
On 2013/03/15 03:25:04, newt wrote:
> I think a list comprehension is preferred:
> 
> class_files = [os.path.relpath(f, jar_cwd) for f in class_files]

Done. Not preferred by me, though :).

https://codereview.chromium.org/12853002/diff/6002/build/android/jar.py#newco...
build/android/jar.py:53: 
On 2013/03/14 23:03:51, Yaron wrote:
> double trailing ws?

Done.

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py
File build/android/javac.py (right):

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:18: srcdirs = build_utils.ParseGypList(options.src_dirs)
On 2013/03/14 23:03:51, Yaron wrote:
> nit:src_dirs

Done.

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:20: if len(options.javac_includes) > 0:
On 2013/03/14 23:03:51, Yaron wrote:
> nit: omit "> 0"

Done.

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:20: if len(options.javac_includes) > 0:
On 2013/03/15 03:25:04, newt wrote:
> nit: omit "len()"

Done.

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:26: filtered_java_files.append(f)
On 2013/03/15 03:25:04, newt wrote:
> break (so you don't end up with the same file included twice)

Done.

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:30: # crash... Sorted order works, so use that.
On 2013/03/14 23:03:51, Yaron wrote:
> nice
Yeah, isn't it...

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:32: java_files = sorted(java_files)
On 2013/03/15 03:25:04, newt wrote:
> or java_files.sort()

Done.

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:36: build_utils.DeleteDirectory(intermediate_dir)
On 2013/03/14 23:03:51, Yaron wrote:
> Add comment about deleting .class files (similar to chromium-jars.xml)

Done.

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:41: '-g',
On 2013/03/14 23:03:51, Yaron wrote:
> Shouldn't this indet match the ' above?
Actually, I think it should match the 'j' or the 'v'... but that looks funny.

How about this?

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:43: '-source', '1.5',
On 2013/03/15 03:25:04, newt wrote:
> I thought we used 1.6, but I trust you here

So, currrently, we use 1.6 for libraries and 1.5 for apks... :/

I think this should be changed to match apks (1.5 is the default from Android's
build scripts and the Android java libraries are based on 1.5).

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:52: parser.add_option('--intermediate-dir')
On 2013/03/14 23:03:51, Yaron wrote:
> Can you add the description fields to each of these

Done.

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:53: parser.add_option('--input-jars-paths')
On 2013/03/14 23:03:51, Yaron wrote:
> Any reason not to rename this classpath?

Done.

https://codereview.chromium.org/12853002/diff/6002/build/android/pylib/build_...
File build/android/pylib/build_utils.py (right):

https://codereview.chromium.org/12853002/diff/6002/build/android/pylib/build_...
build/android/pylib/build_utils.py:6: import itertools
On 2013/03/14 23:03:51, Yaron wrote:
> unused

Done.

https://codereview.chromium.org/12853002/diff/6002/build/android/pylib/build_...
build/android/pylib/build_utils.py:11: def EnsureDirectoryExists(dir_path):
On 2013/03/15 03:25:04, newt wrote:
> MakeDirIfNotExists?  the current name sounds assert-y... but maybe that's just
> me.
Yeah, I didn't really like this name either... Maybe MkDirDashP? That's what
this is really doing...

How about just MakeDirectory.

https://codereview.chromium.org/12853002/diff/6002/build/android/pylib/build_...
build/android/pylib/build_utils.py:33: fnmatch.filter(filenames, filter)))
On 2013/03/15 03:25:04, newt wrote:
> list comprehension?
> 
>     rel_files = fnmatch.filter(filenames, filter)
>     files += [os.path.join(root, f) for f in rel_files]

Done.

https://codereview.chromium.org/12853002/diff/6002/build/android/pylib/build_...
build/android/pylib/build_utils.py:44: def ParseGypList(gyp_string):
On 2013/03/15 03:25:04, newt wrote:
> gyp_string = gyp_string.replace('##','$')
> return shlex.split(gyp_string)
Oh, shlex... much nicer. Done.

https://codereview.chromium.org/12853002/diff/6002/build/android/pylib/build_...
build/android/pylib/build_utils.py:45: # With the ninja generator, some list
members are in double quotes,
On 2013/03/14 23:03:51, Yaron wrote:
> Why would we have these charcters in strings? "$" for inner classes? Double
> quote?

Precisely. I could find no way to escape/quote such a string to make it work.

https://codereview.chromium.org/12853002/diff/6002/build/android/pylib/build_...
build/android/pylib/build_utils.py:50: # The ninja generator doesn't support $
in strings, so use ## to
On 2013/03/15 03:25:04, newt wrote:
> is there any way we can change the generator to support $?  it would be nice
to
> have this escaping built into gyp.
I agree. Updated with a gyp bug tracking this.

https://codereview.chromium.org/12853002/diff/6002/build/java.gypi
File build/java.gypi (right):

https://codereview.chromium.org/12853002/diff/6002/build/java.gypi#newcode179
build/java.gypi:179: '--intermediate-dir=<(classes_dir)',
On 2013/03/14 23:03:51, Yaron wrote:
> nit: classes-dir (or output-dir)

Done.

https://codereview.chromium.org/12853002/diff/6002/build/java.gypi#newcode186
build/java.gypi:186: '--ignore=>(_inputs)',
On 2013/03/15 03:25:04, newt wrote:
> not related (at all), but...
> we could pass the md5sum of the inputs as the ignore argument... that would
make
> the argument list *much* shorter, which is helpful in the case of errors

Done.

https://codereview.chromium.org/12853002/diff/6002/build/java.gypi#newcode204
build/java.gypi:204: '--jar-excludes=<(jar_excludes)',
On 2013/03/14 23:03:51, Yaron wrote:
> Nit: I think it's clearer as --excluded-clases

Done.

[newt (away), 2013-03-15 23:01:19]

lgtm with a formatting nit

https://codereview.chromium.org/12853002/diff/25001/build/android/javac.py
File build/android/javac.py (right):

https://codereview.chromium.org/12853002/diff/25001/build/android/javac.py#ne...
build/android/javac.py:45: 'javac',
indent by 4

https://codereview.chromium.org/12853002/diff/25001/build/android/pylib/build...
File build/android/pylib/build_utils.py (right):

https://codereview.chromium.org/12853002/diff/25001/build/android/pylib/build...
build/android/pylib/build_utils.py:11: def MakeDirectory(dir_path):
Yea, that's clearer.

https://codereview.chromium.org/12853002/diff/25001/build/java.gypi
File build/java.gypi (right):

https://codereview.chromium.org/12853002/diff/25001/build/java.gypi#newcode186
build/java.gypi:186: '--ignore=>!(echo >(_inputs) | md5sum)',
sweet!

[Yaron, 2013-03-15 23:13:46]

lgtm

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py
File build/android/javac.py (right):

https://codereview.chromium.org/12853002/diff/6002/build/android/javac.py#new...
build/android/javac.py:41: '-g',
On 2013/03/15 22:44:39, cjhopman wrote:
> On 2013/03/14 23:03:51, Yaron wrote:
> > Shouldn't this indet match the ' above?
> Actually, I think it should match the 'j' or the 'v'... but that looks funny.
> 
> How about this?

My vi says it should match the quote, but this is fine.

[commit-bot: I haz the power, 2013-03-16 00:55:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/cjhopman@chromium.org/12853002/32001

[commit-bot: I haz the power, 2013-03-16 16:25:24]

Change committed as 188594