Removing tools:ignore="GradleOverride"

[EdbertChan]

This issue is primarily due to Bazel.

When compiling a module, Bazel will bring in libraries and their transitive dependencies. When resolving manifest merger actions, Accompanist is brought in via the compose library.

Per this GH issue (mixpanel/mixpanel-android#298):

"By ignoring the GradleOverrides, it ignores all overrides for the entire project"

Therefore, as long as an app brings in Accompanist, the app will be forced to use API 21 minimum. They cannot use the tools:overrideLibrary to override Accompanist because Accompanist will be brought in transitively.

It also seems wholly unnecessary to have this tag to begin with as well.

[bentrengrove]

I remember we added this to help non-gradle environments so it seems odd that we need to remove it for Bazel.

Compose doesn't bring in Accompanist as well, it's the other way around. Accompanist depends on Compose. What exactly are you trying to do that currently isn't possible? Compose has a minSdk of 21 so why do you need to override that?

[EdbertChan]

Ah yes you are right. Accompanist appears to be reliant on compose. Sorry about that!

But that does not change the core nature of the problem we have.

The way we use compose in our codebase is via a common utility module. We create ui-compose views in a ui-compose module for the rest of the codebase to consume. That ui-compose module has a direct dependency on accompanist and makes that an exported dependency.

The goal is to be able to compile downstream dependent modules and Robolectric tests. The use case is that we are bumping our app's min SDK version to 24. We also have other 3rd party libraries that have their own min SDK version 28.

Ordinarily, we would a Robolectric manifest that has tools:overrideLibrary to get around this. But as long as the ManifestMerger in Bazel consumes and respects the tools:ignore="GradleOverride" tag from Accompanist, we will always get an error:

com.google.devtools.build.android.AndroidManifestProcessor$ManifestProcessingException: Manifest merger failed : uses-sdk:minSdkVersion 21 cannot be smaller than version 28 declared in library

[bentrengrove]

It seems strange to me that Bazel would require us removing a gradle lint warning. We have the minSDKs defined in the manifest because build systems like Bazel need to know the minSDK of a library and we added it to support them, but in Gradle (our primary build system we support) this is overriden by the gradle build file and Studio shows this as a warning. Without this lint suppress, we would have a warning in every AndroidManifest which isn't acceptable.

To land this, we would need to add the suppression somewhere else, most likely in a global lint config file, so the warning doesn't show. I will try and get this implemented for you but I can't say when I will have time unfortunately. If you want to merge this in, I would need that change as well please.

[EdbertChan]

@bentrengrove I have updated the diff. There should be no more lints on each of the corresponding modified AndroidManifests.

But yes, perhaps there should be an internal ticket since the actual manifest merge action is done by ManifestMerger2. The code blindly adds all xml tags upstream (including the GradleOverrides) which is why our issue exists.

[bentrengrove]

Thanks for making that change! This looks good to me