Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2141153002:
Fix java_group() handling in write_build_config.py. (Closed)
|
Created:
4 years, 5 months ago by agrieve Modified:
4 years, 5 months ago Reviewers:
Ian Wen CC:
chromium-reviews, jbudorick+watch_chromium.org, mikecase+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix java_group() handling in write_build_config.py.
Ran into this when trying to add support for android_aar() template.
The existing logic did not play nice with the filtering done by
_FilterUnwantedDepsPaths.
It now just resolves groups at the same time as _FilterUnwantedDepsPaths
so that no other logic within the file should need to know about them.
BUG=611171
Committed: https://crrev.com/29078463073171cf034c7c5e8e9fdd18d5022fec
Cr-Commit-Position: refs/heads/master@{#405536}
Patch Set 1 #
Total comments: 4
Patch Set 2 : added some comments #
Total comments: 1
Messages
Total messages: 22 (13 generated)
The CQ bit was checked by agrieve@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Fix java_group() handling in write_build_config.py. Ran into this when trying to add support for android_aar() template. The existing logic did not play nice with the filtering done by _FilterUnwantedDepsPaths. It now just resolves groups at the same time as _FilterUnwantedDepsPaths so that no other logic within the file should need to know about them. BUG=611171 ========== to ========== Fix java_group() handling in write_build_config.py. Ran into this when trying to add support for android_aar() template. The existing logic did not play nice with the filtering done by _FilterUnwantedDepsPaths. It now just resolves groups at the same time as _FilterUnwantedDepsPaths so that no other logic within the file should need to know about them. BUG=611171 ==========
agrieve@chromium.org changed reviewers: + ianwen@chromium.org
On 2016/07/12 19:01:36, agrieve wrote: > mailto:agrieve@chromium.org changed reviewers: > + mailto:ianwen@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org to run a CQ dry run
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by agrieve@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/07/12 19:01:42, agrieve wrote: > On 2016/07/12 19:01:36, agrieve wrote: > > mailto:agrieve@chromium.org changed reviewers: > > + mailto:ianwen@chromium.org bump
https://codereview.chromium.org/2141153002/diff/1/build/android/gyp/write_bui... File build/android/gyp/write_build_config.py (right): https://codereview.chromium.org/2141153002/diff/1/build/android/gyp/write_bui... build/android/gyp/write_build_config.py:175: ret[index:index + 1] = expanded_configs IIUC, this removes "group" from "deps_config" of a target and inserts the "deps_config" of the "group" to the target, right? https://codereview.chromium.org/2141153002/diff/1/build/android/gyp/write_bui... build/android/gyp/write_build_config.py:186: configs = [c for c in configs if c['type'] in _RESOURCE_TYPES] Sorry I have a million questions... :P Doesn't this mean that a resource target still cannot reference a java target? Also, why was it designed that android resources target cannot depend on java libraries?
Good questions! https://codereview.chromium.org/2141153002/diff/1/build/android/gyp/write_bui... File build/android/gyp/write_build_config.py (right): https://codereview.chromium.org/2141153002/diff/1/build/android/gyp/write_bui... build/android/gyp/write_build_config.py:175: ret[index:index + 1] = expanded_configs On 2016/07/14 04:04:55, Ian Wen wrote: > IIUC, this removes "group" from "deps_config" of a target and inserts the > "deps_config" of the "group" to the target, right? Exactly. Certainly seems deserving of a comment, doesn't it! Added one. https://codereview.chromium.org/2141153002/diff/1/build/android/gyp/write_bui... build/android/gyp/write_build_config.py:186: configs = [c for c in configs if c['type'] in _RESOURCE_TYPES] On 2016/07/14 04:04:55, Ian Wen wrote: > Sorry I have a million questions... :P > > Doesn't this mean that a resource target still cannot reference a java target? > > Also, why was it designed that android resources target cannot depend on java > libraries? Added a comment to this function as well. That is still the case, and it's desirable. However, an aar (which is a java_group) can depend on another aar, and this will result in its resources depending on the other one's resources, and the java_prebuilt depending on both the resources and the other java_prebuilt.
The CQ bit was checked by ianwen@chromium.org
lgtm https://codereview.chromium.org/2141153002/diff/20001/build/android/gyp/write... File build/android/gyp/write_build_config.py (right): https://codereview.chromium.org/2141153002/diff/20001/build/android/gyp/write... build/android/gyp/write_build_config.py:183: include the .apk as a resource/asset, not to have the apk's classpath added. This makes so much sense! Thanks for the comment!
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Fix java_group() handling in write_build_config.py. Ran into this when trying to add support for android_aar() template. The existing logic did not play nice with the filtering done by _FilterUnwantedDepsPaths. It now just resolves groups at the same time as _FilterUnwantedDepsPaths so that no other logic within the file should need to know about them. BUG=611171 ========== to ========== Fix java_group() handling in write_build_config.py. Ran into this when trying to add support for android_aar() template. The existing logic did not play nice with the filtering done by _FilterUnwantedDepsPaths. It now just resolves groups at the same time as _FilterUnwantedDepsPaths so that no other logic within the file should need to know about them. BUG=611171 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== Fix java_group() handling in write_build_config.py. Ran into this when trying to add support for android_aar() template. The existing logic did not play nice with the filtering done by _FilterUnwantedDepsPaths. It now just resolves groups at the same time as _FilterUnwantedDepsPaths so that no other logic within the file should need to know about them. BUG=611171 ========== to ========== Fix java_group() handling in write_build_config.py. Ran into this when trying to add support for android_aar() template. The existing logic did not play nice with the filtering done by _FilterUnwantedDepsPaths. It now just resolves groups at the same time as _FilterUnwantedDepsPaths so that no other logic within the file should need to know about them. BUG=611171 Committed: https://crrev.com/29078463073171cf034c7c5e8e9fdd18d5022fec Cr-Commit-Position: refs/heads/master@{#405536} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/29078463073171cf034c7c5e8e9fdd18d5022fec Cr-Commit-Position: refs/heads/master@{#405536} |
