New issue
Advanced search Search tips

Issue 832770 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Jan 2
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Need a long term solution to detect unwanted dependencies in cronet_impl_native_java.jar

Project Member Reported by xunji...@chromium.org, Apr 13 2018

Issue description

Context and discussion are at internal CL 192618432.

https://chromium-review.googlesource.com/c/chromium/src/+/930332 added //third_party/android_tools:android_support_v4_java to dependencies of //base:base_java. This caused cronet_impl_native_java.jar to package those dependencies, and triggered the one-definition failure in gmscore. 

This issue is not detected until Cronet binary drop. We should add a check somewhere (maybe in build step or in test step) to detect such added dependencies and not export those through our impl jar.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c7c6e319228203aca51b61917f1b439dd0894ea5

commit c7c6e319228203aca51b61917f1b439dd0894ea5
Author: Helen Li <xunjieli@chromium.org>
Date: Fri Apr 13 23:12:42 2018

Excludes android support libraries from cronet_impl_native_java.jar

Bug:  832770 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: Ifb3474a2b250c5a1ac8606e3a9cffbfaa9aa92aa
Reviewed-on: https://chromium-review.googlesource.com/1012564
Reviewed-by: Andrei Kapishnikov <kapishnikov@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550788}
[modify] https://crrev.com/c7c6e319228203aca51b61917f1b439dd0894ea5/components/cronet/android/BUILD.gn

Labels: -Pri-3 Merge-Request-67 Pri-2
Adding merge request label. The CL in #1 only affects Cronet. Per discussion with TPMs, we can go ahead with the merge.
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 16 2018

Labels: merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/81f114f2e2a915bd304d723e3e7085e897f6636e

commit 81f114f2e2a915bd304d723e3e7085e897f6636e
Author: Helen Li <xunjieli@chromium.org>
Date: Mon Apr 16 17:18:28 2018

Excludes android support libraries from cronet_impl_native_java.jar

Bug:  832770 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: Ifb3474a2b250c5a1ac8606e3a9cffbfaa9aa92aa
Reviewed-on: https://chromium-review.googlesource.com/1012564
Reviewed-by: Andrei Kapishnikov <kapishnikov@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#550788}(cherry picked from commit c7c6e319228203aca51b61917f1b439dd0894ea5)
Reviewed-on: https://chromium-review.googlesource.com/1013680
Cr-Commit-Position: refs/branch-heads/3396@{#19}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/81f114f2e2a915bd304d723e3e7085e897f6636e/components/cronet/android/BUILD.gn

Project Member

Comment 4 by sheriffbot@chromium.org, Apr 16 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c7c6e319228203aca51b61917f1b439dd0894ea5

commit c7c6e319228203aca51b61917f1b439dd0894ea5
Author: Helen Li <xunjieli@chromium.org>
Date: Fri Apr 13 23:12:42 2018

Excludes android support libraries from cronet_impl_native_java.jar

Bug:  832770 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: Ifb3474a2b250c5a1ac8606e3a9cffbfaa9aa92aa
Reviewed-on: https://chromium-review.googlesource.com/1012564
Reviewed-by: Andrei Kapishnikov <kapishnikov@chromium.org>
Commit-Queue: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550788}
[modify] https://crrev.com/c7c6e319228203aca51b61917f1b439dd0894ea5/components/cronet/android/BUILD.gn

Project Member

Comment 6 by sheriffbot@chromium.org, Apr 20 2018

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 7 by sheriffbot@chromium.org, Apr 25 2018

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 8 by cmasso@google.com, May 16 2018

Labels: -merge-merged-testbranch
Owner: xunji...@chromium.org
Status: Assigned (was: Untriaged)
Please confirm there is nothing else to merge here
Labels: -Hotlist-Merge-Approved -Merge-Approved-67
Owner: ----
Status: Available (was: Assigned)
I wasn't able to get to this.
Status: Fixed (was: Available)
It may already be fixed by build of sample app using combined jar. 

Sign in to add a comment