New issue
Advanced search Search tips

Issue 623012 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

Remove IntDef from android_support_v13_java.

Project Member Reported by peconn@chromium.org, Jun 24 2016

Issue description

From [1], there is a conflict between android_support_v13_java and android_support_annotations_java because our android_support_v13_java defines IntDef. The means that adding IntDef to code that does not already depend on either of the above is painful.

[1] - https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/lb4-SR1XKT4/WdojZ2-UBQAJ
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 8 2016

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

commit dd8383aa3af51d5c067e445c1390dc9eadc6a220
Author: peconn <peconn@chromium.org>
Date: Fri Jul 08 14:22:53 2016

Remove android_support_annotations target.

android-support-annotations.jar and android-support-v13.jar both define android.support.annotations. Since we use both, this can produce errors building when we try to use android.support.annotations in a new target (eg com.android.dex.DexException: Multiple dex files define Landroid/support/annotation/AnimRes).

android-support-annotations.jar contains a subset of android-support-v13.jar, so I am removing it and updating all dependencies to point to the latter.

BUG= 623012 

Review-Url: https://codereview.chromium.org/2137433002
Cr-Commit-Position: refs/heads/master@{#404371}

[modify] https://crrev.com/dd8383aa3af51d5c067e445c1390dc9eadc6a220/build/secondary/third_party/android_tools/BUILD.gn
[modify] https://crrev.com/dd8383aa3af51d5c067e445c1390dc9eadc6a220/components/cronet.gypi
[modify] https://crrev.com/dd8383aa3af51d5c067e445c1390dc9eadc6a220/components/cronet/android/BUILD.gn
[modify] https://crrev.com/dd8383aa3af51d5c067e445c1390dc9eadc6a220/third_party/android_async_task/BUILD.gn
[modify] https://crrev.com/dd8383aa3af51d5c067e445c1390dc9eadc6a220/third_party/android_async_task/README.chromium
[modify] https://crrev.com/dd8383aa3af51d5c067e445c1390dc9eadc6a220/third_party/espresso/BUILD.gn

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 8 2016

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

commit 2ccd50c3c7f17e770e511096af7d62e36f13135e
Author: peconn <peconn@chromium.org>
Date: Fri Jul 08 15:10:19 2016

Revert of Remove android_support_annotations target. (patchset #2 id:20001 of https://codereview.chromium.org/2137433002/ )

Reason for revert:
Broke the tree

Original issue's description:
> Remove android_support_annotations target.
>
> android-support-annotations.jar and android-support-v13.jar both define android.support.annotations. Since we use both, this can produce errors building when we try to use android.support.annotations in a new target (eg com.android.dex.DexException: Multiple dex files define Landroid/support/annotation/AnimRes).
>
> android-support-annotations.jar contains a subset of android-support-v13.jar, so I am removing it and updating all dependencies to point to the latter.
>
> BUG= 623012 
>
> Committed: https://crrev.com/dd8383aa3af51d5c067e445c1390dc9eadc6a220
> Cr-Commit-Position: refs/heads/master@{#404371}

TBR=jochen@chromium.org,bauerb@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 623012 

Review-Url: https://codereview.chromium.org/2132033002
Cr-Commit-Position: refs/heads/master@{#404390}

[modify] https://crrev.com/2ccd50c3c7f17e770e511096af7d62e36f13135e/build/secondary/third_party/android_tools/BUILD.gn
[modify] https://crrev.com/2ccd50c3c7f17e770e511096af7d62e36f13135e/components/cronet.gypi
[modify] https://crrev.com/2ccd50c3c7f17e770e511096af7d62e36f13135e/components/cronet/android/BUILD.gn
[modify] https://crrev.com/2ccd50c3c7f17e770e511096af7d62e36f13135e/third_party/android_async_task/BUILD.gn
[modify] https://crrev.com/2ccd50c3c7f17e770e511096af7d62e36f13135e/third_party/android_async_task/README.chromium
[modify] https://crrev.com/2ccd50c3c7f17e770e511096af7d62e36f13135e/third_party/espresso/BUILD.gn

It turns out that an easier way to fix the issue is to remove android_support_annotations and replace all of its uses with android_support_v13_java.
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 11 2016

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

commit 3196a901443858f34e9c765f32a83eed978a9393
Author: peconn <peconn@chromium.org>
Date: Mon Jul 11 14:44:51 2016

Reland of move android_support_annotations target. (patchset #1 id:1 of https://codereview.chromium.org/2132033002/ )

Reason for revert:
Attempting to fix cronet build issues.

Original issue's description:
> Revert of Remove android_support_annotations target. (patchset #2 id:20001 of https://codereview.chromium.org/2137433002/ )
>
> Reason for revert:
> Broke the tree
>
> Original issue's description:
> > Remove android_support_annotations target.
> >
> > android-support-annotations.jar and android-support-v13.jar both define android.support.annotations. Since we use both, this can produce errors building when we try to use android.support.annotations in a new target (eg com.android.dex.DexException: Multiple dex files define Landroid/support/annotation/AnimRes).
> >
> > android-support-annotations.jar contains a subset of android-support-v13.jar, so I am removing it and updating all dependencies to point to the latter.
> >
> > BUG= 623012 
> >
> > Committed: https://crrev.com/dd8383aa3af51d5c067e445c1390dc9eadc6a220
> > Cr-Commit-Position: refs/heads/master@{#404371}
>
> TBR=jochen@chromium.org,bauerb@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 623012 
>
> Committed: https://crrev.com/2ccd50c3c7f17e770e511096af7d62e36f13135e
> Cr-Commit-Position: refs/heads/master@{#404390}

TBR=jochen@chromium.org,bauerb@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 623012 

Review-Url: https://codereview.chromium.org/2140543002
Cr-Commit-Position: refs/heads/master@{#404656}

[modify] https://crrev.com/3196a901443858f34e9c765f32a83eed978a9393/build/secondary/third_party/android_tools/BUILD.gn
[modify] https://crrev.com/3196a901443858f34e9c765f32a83eed978a9393/components/cronet.gypi
[modify] https://crrev.com/3196a901443858f34e9c765f32a83eed978a9393/components/cronet/android/BUILD.gn
[modify] https://crrev.com/3196a901443858f34e9c765f32a83eed978a9393/components/cronet/android/proguard.cfg
[modify] https://crrev.com/3196a901443858f34e9c765f32a83eed978a9393/third_party/android_async_task/BUILD.gn
[modify] https://crrev.com/3196a901443858f34e9c765f32a83eed978a9393/third_party/android_async_task/README.chromium
[modify] https://crrev.com/3196a901443858f34e9c765f32a83eed978a9393/third_party/espresso/BUILD.gn

Cc: torne@chromium.org
Labels: OS-Android
re #3: I believe webview doesn't want to depend on code that includes support-v13.
Having support-annotations be separate would allow this (e.g. base_java is used by webview and would benefit from these annotations).

+torne in case I'm misremembering.

Comment 6 by torne@chromium.org, Aug 2 2016

Proguard is in better shape for webview now, so we *might* consider allowing a support library dependency. Someone would have to keep a very careful eye on the .dex size change for the webview apk though, to make sure that proguard really is removing everything it should.
Well it looks like someone readded

//third_party/android_tools:android_support_annotations_java

since I committed this CL, and may have possibly removed IntDef (and presumably all the other annotations) from android_support_v13_java.

Comment 8 by peconn@chromium.org, Aug 16 2016

Status: Fixed (was: Assigned)

Sign in to add a comment