Fix javac --incremental by using jmake for dependency analysis

Fix javac --incremental by using jmake for dependency analysis

BUG=536817
Committed: https://crrev.com/fef292dbfd9b6cea6ae67ad1e939f8efc489b039
Cr-Commit-Position: refs/heads/master@{#352065}

[agrieve, 2015-09-29 14:35:03]

agrieve@chromium.org changed reviewers:
+ jam@chromium.org, jochen@chromium.org, yfriedman@chromium.org

[agrieve, 2015-09-29 14:35:04]

yfriedman@: Please review build/ changes.
jochen@ or jam@: Please review changes in third_party

[Daniel Berlin, 2015-09-29 15:41:25]

dannyb@google.com changed reviewers:
+ dannyb@google.com

[Daniel Berlin, 2015-09-29 15:41:25]

LGTM for OSTPR

[Yaron, 2015-09-30 02:19:25]

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/javac.py
File build/android/gyp/javac.py (right):

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/javac.py#...
build/android/gyp/javac.py:137: new_args = ['-C' + arg for arg in javac_cmd]
I feel like these lines would be a little clearer as:
new_args = ['bin/jmake', '-pdb', pdb_path] +  ['-C' + arg for arg in
javac_cmd[1:]]
followed by the conditionals

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/javac.py#...
build/android/gyp/javac.py:149: # Unprefix classpath args.
Yuck :(

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/javac.py#...
build/android/gyp/javac.py:163: # The .pdb records absolute paths. Fix up paths
witin /tmp (srcjars).
within

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/javac.py#...
build/android/gyp/javac.py:165: with open(pdb_path) as fileobj:
use fileinput.input:
http://stackoverflow.com/questions/5453267/is-it-possible-to-modify-lines-in-...
?

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/javac.py#...
build/android/gyp/javac.py:221: # files to tell jmake which files are stale.
how does this impact non-incremental builds which don't use jmake?

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/util/md5_...
File build/android/gyp/util/md5_check.py (right):

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/util/md5_...
build/android/gyp/util/md5_check.py:15: PRINT_EXPLANATIONS =
int(os.environ.get('PRINT_MD5_DIFFS', 0))
update actually env variable too?

https://codereview.chromium.org/1373723003/diff/1/build/config/android/intern...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/1373723003/diff/1/build/config/android/intern...
build/config/android/internal_rules.gni:1117: deps += [ "//third_party/jmake" ]
this is purely a build-rule dependency and doesn't affect classpath, right?

https://codereview.chromium.org/1373723003/diff/1/third_party/jmake/src/org/p...
File third_party/jmake/src/org/pantsbuild/jmake/Base64.java (right):

https://codereview.chromium.org/1373723003/diff/1/third_party/jmake/src/org/p...
third_party/jmake/src/org/pantsbuild/jmake/Base64.java:7: package
org.pantsbuild.jmake;
Add to tools/android/eclipse/.classpath ?

[agrieve, 2015-10-01 16:26:12]

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/javac.py
File build/android/gyp/javac.py (right):

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/javac.py#...
build/android/gyp/javac.py:137: new_args = ['-C' + arg for arg in javac_cmd]
On 2015/09/30 02:19:25, Yaron wrote:
> I feel like these lines would be a little clearer as:
> new_args = ['bin/jmake', '-pdb', pdb_path] +  ['-C' + arg for arg in
> javac_cmd[1:]]
> followed by the conditionals

Done.

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/javac.py#...
build/android/gyp/javac.py:149: # Unprefix classpath args.
On 2015/09/30 02:19:25, Yaron wrote:
> Yuck :(

Done.

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/javac.py#...
build/android/gyp/javac.py:163: # The .pdb records absolute paths. Fix up paths
witin /tmp (srcjars).
On 2015/09/30 02:19:25, Yaron wrote:
> within

Done.

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/javac.py#...
build/android/gyp/javac.py:163: # The .pdb records absolute paths. Fix up paths
witin /tmp (srcjars).
On 2015/09/30 02:19:25, Yaron wrote:
> within

Done.

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/javac.py#...
build/android/gyp/javac.py:165: with open(pdb_path) as fileobj:
On 2015/09/30 02:19:25, Yaron wrote:
> use fileinput.input:
>
http://stackoverflow.com/questions/5453267/is-it-possible-to-modify-lines-in-...
> ?

It's a binary file. Added a comment.

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/javac.py#...
build/android/gyp/javac.py:221: # files to tell jmake which files are stale.
On 2015/09/30 02:19:25, Yaron wrote:
> how does this impact non-incremental builds which don't use jmake?

It won't be hit since changed_paths == None for non-incremental.

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/util/md5_...
File build/android/gyp/util/md5_check.py (right):

https://codereview.chromium.org/1373723003/diff/1/build/android/gyp/util/md5_...
build/android/gyp/util/md5_check.py:15: PRINT_EXPLANATIONS =
int(os.environ.get('PRINT_MD5_DIFFS', 0))
On 2015/09/30 02:19:25, Yaron wrote:
> update actually env variable too?

Done.

https://codereview.chromium.org/1373723003/diff/1/build/config/android/intern...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/1373723003/diff/1/build/config/android/intern...
build/config/android/internal_rules.gni:1117: deps += [ "//third_party/jmake" ]
On 2015/09/30 02:19:25, Yaron wrote:
> this is purely a build-rule dependency and doesn't affect classpath, right?

correct. Would need to be a dep of write_build_config to affect classpath.

https://codereview.chromium.org/1373723003/diff/1/third_party/jmake/src/org/p...
File third_party/jmake/src/org/pantsbuild/jmake/Base64.java (right):

https://codereview.chromium.org/1373723003/diff/1/third_party/jmake/src/org/p...
third_party/jmake/src/org/pantsbuild/jmake/Base64.java:7: package
org.pantsbuild.jmake;
On 2015/09/30 02:19:25, Yaron wrote:
> Add to tools/android/eclipse/.classpath ?

Done.

[Yaron, 2015-10-01 17:21:20]

lgtm

https://codereview.chromium.org/1373723003/diff/20001/build/android/gyp/javac.py
File build/android/gyp/javac.py (right):

https://codereview.chromium.org/1373723003/diff/20001/build/android/gyp/javac...
build/android/gyp/javac.py:142: do_not_prefix = ('-classpath', '-bootclasspath')
make list?

[agrieve, 2015-10-01 18:39:37]

On 2015/10/01 17:21:20, Yaron wrote:
> lgtm
> 
>
https://codereview.chromium.org/1373723003/diff/20001/build/android/gyp/javac.py
> File build/android/gyp/javac.py (right):
> 
>
https://codereview.chromium.org/1373723003/diff/20001/build/android/gyp/javac...
> build/android/gyp/javac.py:142: do_not_prefix = ('-classpath',
'-bootclasspath')
> make list?

Tuple is probably better since it doesn't need to be mutable.

[jochen (gone - plz use gerrit), 2015-10-01 18:43:50]

lgtm

[agrieve, 2015-10-01 19:44:38]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2015-10-01 19:44:39]

The patchset sent to the CQ was uploaded after l-g-t-m from dannyb@google.com
Link to the patchset: https://codereview.chromium.org/1373723003/#ps20001
(title: "review comments")

[commit-bot: I haz the power, 2015-10-01 19:45:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1373723003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1373723003/20001

[commit-bot: I haz the power, 2015-10-01 19:51:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-01 19:51:21]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[agrieve, 2015-10-01 20:45:03]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2015-10-01 20:45:04]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
dannyb@google.com, yfriedman@chromium.org
Link to the patchset: https://codereview.chromium.org/1373723003/#ps80001
(title: "Nevermind, I just don't know how to use codereview.")

[commit-bot: I haz the power, 2015-10-01 20:46:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1373723003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1373723003/80001

[commit-bot: I haz the power, 2015-10-01 20:52:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-01 20:52:33]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[agrieve, 2015-10-02 01:11:33]

agrieve@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[agrieve, 2015-10-02 01:11:34]

phajdan.jr@chromium.org: Please review changes in license_checker

[agrieve, 2015-10-02 01:11:53]

The CQ bit was checked by agrieve@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-02 01:12:30]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1373723003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1373723003/100001

[commit-bot: I haz the power, 2015-10-02 01:17:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-02 01:17:56]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[agrieve, 2015-10-02 16:03:59]

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

[agrieve, 2015-10-02 16:04:36]

On 2015/10/02 01:17:56, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
>   win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
>   win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

thestig@ - please look at  tools/checklicenses/checklicenses.py

[Lei Zhang, 2015-10-02 17:07:01]

lgtm

[agrieve, 2015-10-02 17:19:45]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2015-10-02 17:19:46]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
dannyb@google.com, yfriedman@chromium.org
Link to the patchset: https://codereview.chromium.org/1373723003/#ps100001
(title: "fix license check")

[commit-bot: I haz the power, 2015-10-02 17:21:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1373723003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1373723003/100001

[commit-bot: I haz the power, 2015-10-02 17:28:06]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-10-02 17:29:03]

Patchset 6 (id:??) landed as
https://crrev.com/fef292dbfd9b6cea6ae67ad1e939f8efc489b039
Cr-Commit-Position: refs/heads/master@{#352065}