Fix building packages with bin targets

[parasyte]

I have a cross-platform application that needs to build a bin target on Windows, macOS, and Linux (at minimum). cargo-apk attempts to build my bin target as a cdylib, and explodes after signing the first artifact:

thread 'main' panicked at /home/jay/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cargo-subcommand-0.12.0/src/artifact.rs:51:23:
Bin is not compatible with Cdylib
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: cargo_subcommand::artifact::Artifact::file_name
   3: cargo_subcommand::subcommand::Subcommand::artifact
   4: cargo_apk::apk::ApkBuilder::build
   5: cargo_apk::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

This fixes the panic by filtering artifacts to only build libraries.

I couldn't find an open issue for this, but there is a related comment in #23 (comment)

[MarijnS95]

I couldn't find an open issue for this, but there is a related comment in #23 (comment)

This should strictly be unrelated? That user/comment is trying to reuse the same target as multiple crate types, which is not valid. It sounds like you have multiple targets of differing types defined in the same crate instead?

---

How are you invoking cargo apk to cause this? If I had to guess, --lib was forgotten to disambiguate whether it should build just the library target and not any binary, which we're scraping up now via autobins support from rust-mobile/cargo-subcommand#17.

A few more thoughts:

1. Despite cargo/rustc not providing a clean way to override this, we've been working on some ugly yet functional hacks to override the crate-type from a bin to a cdylib to support APK building without Cargo.toml changes;
2. How does your patch deal with library and binary types detected from i.e. [examples]? Was this tested?
3. Why is your patch only affecting cargo build, but not any of the other commands that deal with artifacts?

[parasyte]

It sounds like you have multiple targets of differing types defined in the same crate instead?

Yes, this is what happens when you have both main.rs and lib.rs in the same package. Which is probably the most common setup for packages that build executables.

If I had to guess, --lib was forgotten

I wasn't aware of this argument. Can I ask the corollary question to understand the purpose of --bin for Android? My limited understanding is that binary artifacts can't work on Android at all. Which is why cdylib is needed in the first place. If the crate type is cdylib then it is explicitly not a bin. Both can be used together, but to my knowledge again, the bin doesn't mean anything for Android.

I cannot answer your remaining questions because of this confusion:

• cdylib is required for Android builds.
• cdylib and bin need to be built separately (this PR, fixing the error in OP).
• bin is used for building the desktop apps targeting macOS, Windows, Linux.

The error makes it clear that passing the bin artifacts to cargo as CrateType::Cdylib is invalid. The latter is hardcoded here:

cargo-apk/cargo-apk/src/apk.rs

Line 223 in caa8062

let artifact = self.cmd.artifact(artifact, Some(triple), CrateType::Cdylib);

Furthermore, only ApkBuilder::build() is affected, which the other subcommands do not call.

Given this, I cannot fathom how --bin would ever work at all.

[MarijnS95]

--bin is implemented by the generic cargo-subcommand library, which intentionally is not limited to the library-only scope of cargo-apk. A generic solution involves adding a configuration to cargo-subcommand to limit its output to cdylib targets only, by skipping non-library autodetected targets and explicitly erroring on user-specified (on the cmdline) non-library targets.

That is however no longer an issue when (not if, because we've already proven this is possible) cargo-apk translates bin targets to cdylib, requiring minimal user/crate changes.

---

Furthermore, only ApkBuilder::build() is affected, which the other subcommands do not call.

Both Run and Gdb call ApkBuilder::build() internally. See also how the top-level command collects artifacts for those. And while at it, ApkBuilder::check() should have been adequately limited as well.

[parasyte]

I think I see. You are arguing to move the filtering into cargo-subcommand. I am completely indifferent to how the bug is fixed, so long as it is fixed.

I still don’t know what it means to “translates bin targets to cdylib” but that sounds like something I want. It looks like ndk-examples are technically supposed to be bins, but they are cdylibs. Where can I find examples of real bins that build for Android?

[MarijnS95]

I don't really care where the filtering happens - just explained why cargo-subcommand supports bin targets - as long as it is consistent for every cargo-apk subcommand. And with that, as long as we also error out appropriately (or find a way to disable this) when the user requests a --bin or a binary --example on the command line.

[parasyte]

The reason examples are filtered is because artifact.r#type == ArtifactType::Example. That's a good thing to get fixed. The ndk-examples are not binary examples (they can't be due to the weird cdylib requirement for building native Android code). They build after refining the filter to artifact.r#type != ArtifactType::Bin.

Can you provide any existing code bases where --bin or "binary --example" needs to be passed to cargo-apk? I asked this in my last comment and it's crucial that I have a reproducible test case to address the concern for binary artifacts. At this time, I'm inclined to believe that binary artifacts are not supported at all in the entire ecosystem for Android native builds because every code sample I have found uses cdylib.