android: Convert java_deobfuscate.py to Java

android: Convert java_deobfuscate.py to Java

This allows it to not buffer output lines, which is essential when
piping logcat through it.

BUG=713710
Review-Url: https://codereview.chromium.org/2956153003
Cr-Commit-Position: refs/heads/master@{#484281}
Committed: https://chromium.googlesource.com/chromium/src/+/a350dbdbfe91f9f2be20b8946d4352194e839d9e

[agrieve, 2017-06-28 01:13:33]

agrieve@chromium.org changed reviewers:
+ jbudorick@chromium.org

[agrieve, 2017-06-28 01:13:33]

🔭

[agrieve, 2017-06-28 01:14:53]

agrieve@chromium.org changed reviewers:
+ yfriedman@chromium.org

[agrieve, 2017-06-28 01:14:53]

yfriedman@chromium.org: Please review changes in OWNERS

[Yaron, 2017-06-28 15:55:25]

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
File build/android/stacktrace/BUILD.gn (right):

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
build/android/stacktrace/BUILD.gn:1: # Copyright 2017 The Chromium Authors. All
rights reserved.
Move to tools/android

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
build/android/stacktrace/BUILD.gn:14:
"$root_build_dir/lib.java/build/android/stacktrace/java_deobfuscate.jar",
weird that you have to re-list jars and they aren't picked up by classpath?

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
File build/android/stacktrace/java/org/chromium/build/FlushingReTrace.java
(right):

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
build/android/stacktrace/java/org/chromium/build/FlushingReTrace.java:58:
System.exit(0);
unnecessary?

[jbudorick, 2017-06-28 16:11:39]

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
File build/android/stacktrace/BUILD.gn (right):

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
build/android/stacktrace/BUILD.gn:15:
"$root_build_dir/lib.java/third_party/proguard/proguard.jar",
Shouldn't these two be handled by //third_party/proguard?

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
File build/android/stacktrace/java/org/chromium/build/FlushingReTrace.java
(right):

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
build/android/stacktrace/java/org/chromium/build/FlushingReTrace.java:35: if
(args.length != 1) {
I'm surprised that the java standard library doesn't have some sort of
command-line parser.

[agrieve, 2017-06-29 01:59:20]

Also added a README for good measure.

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
File build/android/stacktrace/BUILD.gn (right):

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
build/android/stacktrace/BUILD.gn:1: # Copyright 2017 The Chromium Authors. All
rights reserved.
On 2017/06/28 15:55:25, Yaron wrote:
> Move to tools/android

My intention is to have test_runner.py use this tool. We try to keep all
dependencies of build/ within build/ when possible so chrome sub-projects can
DEP in build/ and have it work without also having to track down other
dependencies.

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
build/android/stacktrace/BUILD.gn:14:
"$root_build_dir/lib.java/build/android/stacktrace/java_deobfuscate.jar",
On 2017/06/28 15:55:25, Yaron wrote:
> weird that you have to re-list jars and they aren't picked up by classpath?

This is so that they will be available to swarming. There actually doesn't look
to be any existing precedent for running java_binaries on swarming that I can
see, and it's certainly arguable how it should work.

My thoughts:
- Don't want to have all java_library() declare themselves as data, because the
libraries don't know if they'll be needed at runtime
- Can't add classpath as data at the java_binary level because GN has no way to
express that
- We could use dist_jar() instead to package them all up as one jar, but there's
no existing logic for that, and I figured with just a few files it wasn't worth
it.

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
build/android/stacktrace/BUILD.gn:15:
"$root_build_dir/lib.java/third_party/proguard/proguard.jar",
On 2017/06/28 16:11:38, jbudorick wrote:
> Shouldn't these two be handled by //third_party/proguard?

If someone wanted to go the dist_jar() route, then no. Given there's no other
uses currently, and we may want the option of swapping out another proguard
version, probably does make sense to change... so, done!

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
File build/android/stacktrace/java/org/chromium/build/FlushingReTrace.java
(right):

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
build/android/stacktrace/java/org/chromium/build/FlushingReTrace.java:35: if
(args.length != 1) {
On 2017/06/28 16:11:38, jbudorick wrote:
> I'm surprised that the java standard library doesn't have some sort of
> command-line parser.

:( indeed.

https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
build/android/stacktrace/java/org/chromium/build/FlushingReTrace.java:58:
System.exit(0);
On 2017/06/28 15:55:25, Yaron wrote:
> unnecessary?

It's necessary if there are any non-daemon threads still running. I have no idea
if this is the case, and ReTrace's main() has this exit(), so best to keep it in
I think.

[agrieve, 2017-06-30 15:49:38]

On 2017/06/29 01:59:20, agrieve wrote:
> Also added a README for good measure.
> 
>
https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
> File build/android/stacktrace/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
> build/android/stacktrace/BUILD.gn:1: # Copyright 2017 The Chromium Authors.
All
> rights reserved.
> On 2017/06/28 15:55:25, Yaron wrote:
> > Move to tools/android
> 
> My intention is to have test_runner.py use this tool. We try to keep all
> dependencies of build/ within build/ when possible so chrome sub-projects can
> DEP in build/ and have it work without also having to track down other
> dependencies.
> 
>
https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
> build/android/stacktrace/BUILD.gn:14:
> "$root_build_dir/lib.java/build/android/stacktrace/java_deobfuscate.jar",
> On 2017/06/28 15:55:25, Yaron wrote:
> > weird that you have to re-list jars and they aren't picked up by classpath?
> 
> This is so that they will be available to swarming. There actually doesn't
look
> to be any existing precedent for running java_binaries on swarming that I can
> see, and it's certainly arguable how it should work.
> 
> My thoughts:
> - Don't want to have all java_library() declare themselves as data, because
the
> libraries don't know if they'll be needed at runtime
> - Can't add classpath as data at the java_binary level because GN has no way
to
> express that
> - We could use dist_jar() instead to package them all up as one jar, but
there's
> no existing logic for that, and I figured with just a few files it wasn't
worth
> it.
> 
>
https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
> build/android/stacktrace/BUILD.gn:15:
> "$root_build_dir/lib.java/third_party/proguard/proguard.jar",
> On 2017/06/28 16:11:38, jbudorick wrote:
> > Shouldn't these two be handled by //third_party/proguard?
> 
> If someone wanted to go the dist_jar() route, then no. Given there's no other
> uses currently, and we may want the option of swapping out another proguard
> version, probably does make sense to change... so, done!
> 
>
https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
> File build/android/stacktrace/java/org/chromium/build/FlushingReTrace.java
> (right):
> 
>
https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
> build/android/stacktrace/java/org/chromium/build/FlushingReTrace.java:35: if
> (args.length != 1) {
> On 2017/06/28 16:11:38, jbudorick wrote:
> > I'm surprised that the java standard library doesn't have some sort of
> > command-line parser.
> 
> :( indeed.
> 
>
https://codereview.chromium.org/2956153003/diff/20001/build/android/stacktrac...
> build/android/stacktrace/java/org/chromium/build/FlushingReTrace.java:58:
> System.exit(0);
> On 2017/06/28 15:55:25, Yaron wrote:
> > unnecessary?
> 
> It's necessary if there are any non-daemon threads still running. I have no
idea
> if this is the case, and ReTrace's main() has this exit(), so best to keep it
in
> I think.

🏓

[Yaron, 2017-06-30 16:55:15]

whoops - sorry for the delay. lgtm

[jbudorick, 2017-07-05 13:22:37]

lgtm

[agrieve, 2017-07-05 14:14:49]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2017-07-05 14:15:00]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-07-05 15:27:19]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1499264089317300,
"parent_rev": "bd29c48c5736cf06601aa1c207e4dbba13becc8b", "commit_rev":
"a350dbdbfe91f9f2be20b8946d4352194e839d9e"}

[commit-bot: I haz the power, 2017-07-05 15:27:34]

Description was changed from

==========
android: Convert java_deobfuscate.py to Java

This allows it to not buffer output lines, which is essential when
piping logcat through it.

BUG=713710
==========

to

==========
android: Convert java_deobfuscate.py to Java

This allows it to not buffer output lines, which is essential when
piping logcat through it.

BUG=713710

Review-Url: https://codereview.chromium.org/2956153003
Cr-Commit-Position: refs/heads/master@{#484281}
Committed:
https://chromium.googlesource.com/chromium/src/+/a350dbdbfe91f9f2be20b8946d43...
==========

[commit-bot: I haz the power, 2017-07-05 15:27:35]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/a350dbdbfe91f9f2be20b8946d43...