Add per build type cargoBuild${buildType} tasks

[dcsommer]

The goal of this change is to support explicitly building particular Cargo
profiles per Android build type. This change makes it possible to build
both release and debug tasks in a single gradle invocation without editing
the use of the cargo extension.

There are some backwards incompatible changes present:
profile is deprecated and is replaced with the required map buildTypeToProfile.
This map controls which cargoBuild${buildType} tasks are created and
what Cargo profile is used for each. Once
rust-lang/cargo#6988 is resolved and stabilized,
we should switch the implementation to use cargo build --profile=$x
explicitly rather than --release.

[ncalexan]

@dcsommer many apologies for the long delay reviewing.

There's no significant technical issue with this commit. But I do have two concerns that I want to discuss.

First and foremost, I don't think handling this per Android build type is the correct level. Android manages splits per "variant" -- product flavor and build type. We should too. (There's a tiny bit of discussion at #38.)

Second, I don't like that we lose sensible (if arbitrary) defaults and therefore that we're not backwards compatible. Since we eventually want to handle variants, I'd like to lose backwards compatibility at most once. If we make this change, we would probably want to rev to semver 1.x, since it's a major break with 0.8 (and a potential 0.9, too).

I am aware that my time for this plugin is extremely limited. I could be convinced that it's better to just fix the nits on this commit and call it a day. Let me know your thoughts.

[ncalexan]

Might as well include the profile name here. And nit: capitalize Android.

[ncalexan]

This link is broken. s/second-edition// to fix it.

[ncalexan]

Shouldn't it be "debug": "dev", since the Rust profiles are release and dev?

[ncalexan]

nit: no trailing semi-colon.

[dcsommer]

Thanks for the review and starting the discussion.

First and foremost, I don't think handling this per Android build type is the correct level. Android manages splits per "variant" -- product flavor and build type. We should too. (There's a tiny bit of discussion at #38.)

Good point. I was modelling this after how externalNativeBuild DSL works within the Android Gradle plugin. When I run tasks --all I saw externalNativeBuild${buildType} but no flavor dimension. I believe that's because I declare my externalNativeBuild within a defaultConfig block, which as it happens is a flavor.

Since cargo is declared outside the android block, I agree we shouldn't elide the flavor. I think the user of the cargo plugin should provide the flavors as an input. The question is how to organize this all. We could try to go fully parallel to externalNativeBuild with nested blocks. It gets complicated quickly though :(

Second, I don't like that we lose sensible (if arbitrary) defaults and therefore that we're not backwards compatible.

Understandable, and I also feel bad about the API breakage. If we go with API breakage, we should get this right so we don't have to rev a second time. I'm not in a rush to push this. I have an internal fork (that I won't be publishing), and I'm happy to keep it in sync with this upstream as the design evolves.

[ncalexan]

Thanks for the review and starting the discussion.

First and foremost, I don't think handling this per Android build type is the correct level. Android manages splits per "variant" -- product flavor and build type. We should too. (There's a tiny bit of discussion at #38.)

Good point. I was modelling this after how externalNativeBuild DSL works within the Android Gradle plugin. When I run tasks --all I saw externalNativeBuild${buildType} but no flavor dimension. I believe that's because I declare my externalNativeBuild within a defaultConfig block, which as it happens is a flavor.

If you did have flavours, you'd probably see externalNativeBuild${variant} as variant ranges over flavours and build types.

Since cargo is declared outside the android block, I agree we shouldn't elide the flavor. I think the user of the cargo plugin should provide the flavors as an input. The question is how to organize this all. We could try to go fully parallel to externalNativeBuild with nested blocks. It gets complicated quickly though :(

I agree -- I was stunned when I discovered how hard Gradle makes nested configuration of this manner. I have been assuming we would ape something like https://github.com/usefulness/easylauncher-gradle-plugin, which provides a pretty slick model for doing per-variant configuration.

Second, I don't like that we lose sensible (if arbitrary) defaults and therefore that we're not backwards compatible.

Understandable, and I also feel bad about the API breakage. If we go with API breakage, we should get this right so we don't have to rev a second time. I'm not in a rush to push this. I have an internal fork (that I won't be publishing), and I'm happy to keep it in sync with this upstream as the design evolves.

It seems like we can avoid API breakage by continuing to support top-level profile and having it continue to default to debug, and having the other options override.

The thing that I think will be harder than getting the variants sorted is actually updating the task graph for all these variants. Right now we kick the responsibility to the consumer, which is not very pleasant; if cargoBuild* tasks proliferate, we really need to do this in the plugin. I'm not aware of any blockers to this, it just needs to be wired up.

[dcsommer]

have been assuming we would ape something like https://github.com/usefulness/easylauncher-gradle-plugin, which provides a pretty slick model for doing per-variant configuration.

Very slick. That looks like a good target.

The thing that I think will be harder than getting the variants sorted is actually updating the task graph for all these variants. Right now we kick the responsibility to the consumer, which is not very pleasant; if cargoBuild* tasks proliferate, we really need to do this in the plugin. I'm not aware of any blockers to this, it just needs to be wired up.

Yeah, I have a couple nested loops in my build.gradle to do this dependency management. I do it slightly differently than recommended in this project's README.md so as to integrate with the CMake plugin tasks which do stripping and build static libraries. We could try to depend on some high level android plugin tasks and let users customize the dependencies further if tighter integrations (e.g. with CMake) are needed.

[SergioRibera]

Any Update? 😢