Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1549543002:
GN(android): Allow android_instrumentation_test_apk() to depend on Java files in android_apk() (Closed)
|
Created:
5 years ago by agrieve Modified:
5 years ago Reviewers:
pkotwicz CC:
chromium-reviews, klundberg+watch_chromium.org, mikecase+watch_chromium.org, yfriedman+watch_chromium.org, jbudorick+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionGN(android): Add the dependant test apk to the javac classpath
The work-around is to use an intermediate java_library() that both the
test apk and apk-under-test depend on. This change removes the need for
the work-around.
BUG=571678, 532905
Committed: https://crrev.com/c23d3b97765e04a997e6602e1b23c11c8721d2a2
Cr-Commit-Position: refs/heads/master@{#366751}
Patch Set 1 #
Total comments: 2
Patch Set 2 : address comment #Messages
Total messages: 16 (7 generated)
Description was changed from ========== GN(android): Add the dependant test apk to the javac classpath The work-around is to use an intermediate java_library() that both the test apk and apk-under-test depend on. This change removes the need for the work-around. BUG=532905 ========== to ========== GN(android): Add the dependant test apk to the javac classpath The work-around is to use an intermediate java_library() that both the test apk and apk-under-test depend on. This change removes the need for the work-around. BUG=532905 ==========
agrieve@chromium.org changed reviewers: + pkotwicz@chromium.org
On 2015/12/22 16:37:44, agrieve wrote: > mailto:agrieve@chromium.org changed reviewers: > + mailto:pkotwicz@chromium.org 🎅
Can you add a more relevant bug?
Description was changed from ========== GN(android): Add the dependant test apk to the javac classpath The work-around is to use an intermediate java_library() that both the test apk and apk-under-test depend on. This change removes the need for the work-around. BUG=532905 ========== to ========== GN(android): Add the dependant test apk to the javac classpath The work-around is to use an intermediate java_library() that both the test apk and apk-under-test depend on. This change removes the need for the work-around. BUG=571678,532905 ==========
On 2015/12/22 20:40:38, pkotwicz wrote: > Can you add a more relevant bug? Done
LGTM https://codereview.chromium.org/1549543002/diff/1/build/android/gyp/write_bui... File build/android/gyp/write_build_config.py (right): https://codereview.chromium.org/1549543002/diff/1/build/android/gyp/write_bui... build/android/gyp/write_build_config.py:398: tested_apk_deps.All('java_library')) For the sake of general knowledge, why are you making the change on line 398
LGTM
https://codereview.chromium.org/1549543002/diff/1/build/android/gyp/write_bui... File build/android/gyp/write_build_config.py (right): https://codereview.chromium.org/1549543002/diff/1/build/android/gyp/write_bui... build/android/gyp/write_build_config.py:398: tested_apk_deps.All('java_library')) On 2015/12/23 02:15:29, pkotwicz wrote: > For the sake of general knowledge, why are you making the change on line 398 Thanks Peter! Confirmed that this part of the change didn't actually do anything. tested_apk_config['dex_path'] will never appear in deps_dex_files. Reverted this line.
The CQ bit was checked by agrieve@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from pkotwicz@chromium.org Link to the patchset: https://codereview.chromium.org/1549543002/#ps20001 (title: "address comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1549543002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1549543002/20001
Message was sent while issue was closed.
Description was changed from ========== GN(android): Add the dependant test apk to the javac classpath The work-around is to use an intermediate java_library() that both the test apk and apk-under-test depend on. This change removes the need for the work-around. BUG=571678,532905 ========== to ========== GN(android): Add the dependant test apk to the javac classpath The work-around is to use an intermediate java_library() that both the test apk and apk-under-test depend on. This change removes the need for the work-around. BUG=571678,532905 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== GN(android): Add the dependant test apk to the javac classpath The work-around is to use an intermediate java_library() that both the test apk and apk-under-test depend on. This change removes the need for the work-around. BUG=571678,532905 ========== to ========== GN(android): Add the dependant test apk to the javac classpath The work-around is to use an intermediate java_library() that both the test apk and apk-under-test depend on. This change removes the need for the work-around. BUG=571678,532905 Committed: https://crrev.com/c23d3b97765e04a997e6602e1b23c11c8721d2a2 Cr-Commit-Position: refs/heads/master@{#366751} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/c23d3b97765e04a997e6602e1b23c11c8721d2a2 Cr-Commit-Position: refs/heads/master@{#366751} |
