Add flag to disable output library.ap_ #11253

jongerrish · 2020-04-29T18:07:41Z

android_library ResourceLinkAction exports generated library.ap_ files. These files grow exponentially in size as their contents are merged in to depending libraries, but library.ap_ files are in the format consumable by devices only. Since android_binary generated the final library.ap_ file these intermediates are not needed as outputs.

For a build the size of ours this saves around 2GB of unnecessary outputs.

jin · 2020-04-30T07:37:49Z

src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java

+            defaultValue = "true",
+            documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+            effectTags = {OptionEffectTag.UNKNOWN},
+            help = "If disabled, does not provide library.ap_ outputs for library targets")


Could you also include the PR description here? The PR description has great context, e.g. reducing download overhead and devices not requiring these files.

Done - thanks for the review!

jin · 2020-05-01T02:51:16Z

Update: looking into failing tests internally with blaze.

jongerrish · 2020-05-01T02:56:01Z

Thanks Jin!

…

On Thu, Apr 30, 2020, 7:51 PM Jin ***@***.***> wrote: Update: looking into failing tests internally with blaze. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#11253 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQFU6GIJFHIAQEPDJ4JWCTRPI2LDANCNFSM4MT6K52A> .

jongerrish · 2020-05-06T14:22:46Z

I think bazelbuild/rules_android#10 is a better solution

artem-zinnatullin · 2020-05-12T02:25:10Z

I think bazelbuild/rules_android#10 is a better solution

But do you think there is any use case in library.ap_ files in general?

I agree that disabling R class merging is generally what we should aim for but wouldn't it still be producing useless library.ap_ files, just smaller?

jongerrish · 2020-05-12T12:06:09Z

I can't see a usecase for library.ap_ - I think it was just a side affect (-o is a mandatory parameter) from calling aapt2 link which is used to generate the R.java + R.txt. However, AndroidResourceMerge now generates an R.class + R.txt directly from the intermediate compilation outputs, so really linking is no longer necessary at all. That's why I prefer bazelbuild/rules_android#10 Also both is behind a flag, just in case there is a strange usecase I haven't thought of.

transitive_static_lib should be removed in coordination with #11253 PiperOrigin-RevId: 311406837

ulfjack · 2020-05-20T12:57:06Z

@jin, can you provide an update here, please? This is part of a batch of changes that are necessary to get Bazel's Android builds on par with Buck wrt. build performance. This is a blocker for some people for migrating from Buck to Bazel.

jin · 2020-05-21T17:05:18Z

I'm currently adding tests to add coverage for our decoupled @android_tools infra, which the resource processing pipeline is part of, so @jongerrish's PRs can be properly tested on our Buildkite CI and not run into failures internally.

ulfjack · 2020-05-27T09:09:59Z

Cool, glad to hear it. Have you been able to make progress? Anything we can help with?

jin · 2020-05-27T09:13:51Z

Blocking on internal review.

jin · 2020-05-28T14:46:02Z

Could you rebase your PRs onto f3d1683 please?

jongerrish · 2020-05-28T15:29:21Z

@jin Just resolved conflicts is that enough?

jin · 2020-05-29T16:51:46Z

src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java

+
+  boolean outputLibraryLinkedResources() {
+    return outputLibraryLinkedResources;
+


You have a missing closing brace here

aiuto · 2021-01-06T05:13:52Z

Friendly ping. The tests are failing. Are you going to fix or drop this.

Bencodes · 2021-01-22T19:53:46Z

@jin any updates on getting this merged?

nkoroste · 2021-02-19T17:42:23Z

Any updates on this?

oquenchil · 2021-07-14T13:19:20Z

@jin It looks like this PR was approved and it doesn't need any more changes. Can it be merged? Or does it not make sense any more and it should be closed?

ajanuar · 2021-07-28T10:28:59Z

Any update on this?

Bencodes · 2022-02-10T22:38:44Z

This is still something that would be really nice to merge so I opened the PR back up with the merge conflicts resolved. Mind taking another quick look @jin @aiuto?

#14786

aiuto · 2022-02-10T22:56:12Z

I tried to get a better reviewer than myself. Let's follow up in the other PR.

…

On Thu, Feb 10, 2022 at 5:38 PM Ben Lee ***@***.***> wrote: This is still something that would be really nice to merge so I opened the PR back up with the merge conflicts resolved. Mind taking another quick look @jin <https://github.com/jin> @aiuto <https://github.com/aiuto>? #14786 <#14786> — Reply to this email directly, view it on GitHub <#11253 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXHHHADOLIS33DYCY5CAYLU2Q477ANCNFSM4MT6K52A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

Bencodes · 2022-02-11T00:31:11Z

Awesome thanks @aiuto!

sgowroji · 2022-08-11T07:26:36Z

Hello @jongerrish, Can you please check build failures and code conflicts Thanks!

transitive_static_lib should be removed in coordination with bazelbuild/bazel#11253 PiperOrigin-RevId: 311406837

keertk · 2022-12-07T13:01:26Z

Hi there! Thank you for contributing to the Bazel repository. We appreciate your time and effort. We're doing a clean up of old PRs and will be closing this one since it seems to have stalled. Please feel free to reopen if you’re still interested in pursuing this or if you'd like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so.

ted-xie · 2022-12-08T17:59:18Z

This PR is still in our backlog, sorry for the confusion. I'll mark this as "P3" so it doesn't get closed again.

Bencodes · 2022-12-08T18:03:36Z

This PR was reopened over here with the conflicts resolved.

#14786

ted-xie · 2022-12-08T18:05:02Z

Oh, thanks Ben! Closing this PR then.

Add flag to desiable output library.ap_

37b7630

jongerrish requested review from ahumesky, jin and lberki as code owners April 29, 2020 18:07

googlebot added the cla: yes label Apr 29, 2020

jongerrish mentioned this pull request Apr 29, 2020

Permit disabling resource merging bazelbuild/rules_android#10

Open

Fix test broken by removed parameter

3143d59

jin approved these changes Apr 30, 2020

View reviewed changes

jin removed the request for review from lberki April 30, 2020 07:39

Improve help text

43b44f4

jin added the team-Android Issues for Android team label Apr 30, 2020

jongerrish changed the title ~~Add flag to desiable output library.ap_~~ Add flag to disable output library.ap_ May 12, 2020

nkoroste mentioned this pull request May 13, 2020

Bazel remote cache is not a clear win #7664

Open

bazel-io pushed a commit that referenced this pull request May 13, 2020

Add defaults to facilitate removing deprecated struct fields.

c7dc024

transitive_static_lib should be removed in coordination with #11253 PiperOrigin-RevId: 311406837

Merge branch 'master' into jg_dont_create_linked_resource_outputs

130a0d7

jin reviewed May 29, 2020

View reviewed changes

aiuto assigned jongerrish Feb 19, 2021

oquenchil assigned jin Jul 14, 2021

philwo force-pushed the master branch from 26cb401 to 168b89b Compare December 2, 2021 18:07

Bencodes mentioned this pull request Feb 10, 2022

Add output_library_linked_resources flag to disable library.ap_ outputs #14786

Open

jongerrish requested a review from ted-xie as a code owner April 22, 2022 11:45

sgowroji added the awaiting-user-response Awaiting a response from the author label May 13, 2022

sgowroji added awaiting-review PR is awaiting review from an assigned reviewer awaiting-user-response Awaiting a response from the author and removed awaiting-user-response Awaiting a response from the author awaiting-review PR is awaiting review from an assigned reviewer labels Aug 11, 2022

keertk closed this Dec 7, 2022

ted-xie reopened this Dec 8, 2022

ted-xie added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed awaiting-user-response Awaiting a response from the author labels Dec 8, 2022

ted-xie closed this Dec 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add flag to disable output library.ap_ #11253

Add flag to disable output library.ap_ #11253

jongerrish commented Apr 29, 2020

jin Apr 30, 2020

jongerrish Apr 30, 2020

jin commented May 1, 2020

jongerrish commented May 1, 2020 via email

jongerrish commented May 6, 2020

artem-zinnatullin commented May 12, 2020

jongerrish commented May 12, 2020

ulfjack commented May 20, 2020

jin commented May 21, 2020 •

edited

Loading

ulfjack commented May 27, 2020

jin commented May 27, 2020

jin commented May 28, 2020

jongerrish commented May 28, 2020

jin May 29, 2020

aiuto commented Jan 6, 2021

Bencodes commented Jan 22, 2021

nkoroste commented Feb 19, 2021

oquenchil commented Jul 14, 2021

ajanuar commented Jul 28, 2021

Bencodes commented Feb 10, 2022

aiuto commented Feb 10, 2022 via email

Bencodes commented Feb 11, 2022

sgowroji commented Aug 11, 2022

keertk commented Dec 7, 2022

ted-xie commented Dec 8, 2022

Bencodes commented Dec 8, 2022

ted-xie commented Dec 8, 2022


		boolean outputLibraryLinkedResources() {
		return outputLibraryLinkedResources;

Add flag to disable output library.ap_ #11253

Add flag to disable output library.ap_ #11253

Conversation

jongerrish commented Apr 29, 2020

jin Apr 30, 2020

Choose a reason for hiding this comment

jongerrish Apr 30, 2020

Choose a reason for hiding this comment

jin commented May 1, 2020

jongerrish commented May 1, 2020 via email

jongerrish commented May 6, 2020

artem-zinnatullin commented May 12, 2020

jongerrish commented May 12, 2020

ulfjack commented May 20, 2020

jin commented May 21, 2020 • edited Loading

ulfjack commented May 27, 2020

jin commented May 27, 2020

jin commented May 28, 2020

jongerrish commented May 28, 2020

jin May 29, 2020

Choose a reason for hiding this comment

aiuto commented Jan 6, 2021

Bencodes commented Jan 22, 2021

nkoroste commented Feb 19, 2021

oquenchil commented Jul 14, 2021

ajanuar commented Jul 28, 2021

Bencodes commented Feb 10, 2022

aiuto commented Feb 10, 2022 via email

Bencodes commented Feb 11, 2022

sgowroji commented Aug 11, 2022

keertk commented Dec 7, 2022

ted-xie commented Dec 8, 2022

Bencodes commented Dec 8, 2022

ted-xie commented Dec 8, 2022

jin commented May 21, 2020 •

edited

Loading