Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apk: Use rustc --crate-type=cdylib to always force a lib to be compiled #28

Closed
wants to merge 1 commit into from

Conversation

MarijnS95
Copy link
Member

@MarijnS95 MarijnS95 commented Jun 16, 2023

This automatically turns crate-type = ["*lib"] and [lib] targets into a cdylib, and otherwise errors appropriately when encountering a binary instead of succeeding the build and having cargo-apk later complain when target/aarch64-linux-android/debug/lib<yourpacakge>.so is not found.

@MarijnS95
Copy link
Member Author

Will probably transplant this to xbuild as it seems quite useful.

@rib
Copy link
Contributor

rib commented Jun 16, 2023

Ah interesting, didn't know about cargo rustc - if you thought it wasn't possible to make configuring rustflags more complex... :)

I vaguely wonder what happens in a situation where you have a [bin] and a [lib] with the same name and then both end up mapping to the same target.so maybe?

Not exactly intentionally but I've probably had crates with overlapping [bin] and [lib] crates when I've wanted it to build for desktop and Android.

The docs say "If more than one target is available for the current package the filters of --lib, --bin, etc, must be used to select which target is compiled"

so would it error if it's ambiguous? does cargo apk already explicitly name the target to build?

@MarijnS95
Copy link
Member Author

Ah interesting, didn't know about cargo rustc - if you thought it wasn't possible to make configuring rustflags more complex... :)

I vaguely wonder what happens in a situation where you have a [bin] and a [lib] with the same name and then both end up mapping to the same target.so maybe?

A bin cannot be a .so.

Not exactly intentionally but I've probably had crates with overlapping [bin] and [lib] crates when I've wanted it to build for desktop and Android.

The docs say "If more than one target is available for the current package the filters of --lib, --bin, etc, must be used to select which target is compiled"

so would it error if it's ambiguous? does cargo apk already explicitly name the target to build?

Yes, it can, and cargo-apk might get confused or panic, otherwise it blindly calls cargo and then cargo will tell you that you have to be more specific. Only one way to find out.

Also keep in mind #26 and rust-mobile/cargo-subcommand#17 (latter could use some review) that messes around with target selection specifically.

@rib
Copy link
Contributor

rib commented Jun 16, 2023

good to know that cargo rustc might be another general option though for situation where you need to add rustflags without needing to read back any of the rustflags configured by the user

@rib
Copy link
Contributor

rib commented Jun 16, 2023

A bin cannot be a .so.

I just wasn't sure initially whether this was going to somehow force the compilation of a [bin] target as cdylib too, hadn't got as far as looking at the cargo rustc docs

@rib
Copy link
Contributor

rib commented Jun 16, 2023

A bin cannot be a .so.

I just wasn't sure initially whether this was going to somehow force the compilation of a [bin] target as cdylib too, hadn't got as far as looking at the cargo rustc docs

tbh even since I have read them I'm not 100% sure. I see you can use --bin or --lib to select a target if there is more than one but if you actually just had one [bin] target will --crate-type=cdylib actually force that [bin] to build as a cdylib. Potentially that could actually be kind of convenient in situations where you want a crate to build for desktop or Android and you just want to configure a single Cargo.toml target.

@MarijnS95
Copy link
Member Author

Oh I see now, yeah I covered this in the PR message already:

and otherwise errors appropriately when encountering a binary instead of succeeding the build

@MarijnS95
Copy link
Member Author

Potentially that could actually be kind of convenient in situations where you want a crate to build for desktop or Android and you just want to configure a single Cargo.toml target.

Yeah it would have been, except that it explicitly disallows this scenario. Same for [[test]] and [[example]] etc when they don't already set a crate-type to any of the library-ish forms (lib, rlib, dylib, cdylib, ...).

…iled

This automatically turns `crate-type = ["*lib"]` and `[lib]` targets
into a `cdylib`, and otherwise errors appropriately when encountering a
binary instead of succeeding the build and having `cargo-apk` later
complain when `target/aarch64-linux-android/debug/lib<yourpacakge>.so`
is not found.
@MarijnS95
Copy link
Member Author

The unfortunate downside of this, as shown by the last CI run is that we can no longer build multiple targets such as examples at once. Neither --examples nor explicitly specifying --example hello_world --example jni_audio --example looper is allowed, all result in error: crate types to rustc can only be passed to one target, consider filtering.

@MarijnS95
Copy link
Member Author

Unsure what to do with breaking multi-build arguments, so users will just have to read the documentation and provide + select a cdylib target. Closing.

@MarijnS95 MarijnS95 closed this Nov 30, 2023
@MarijnS95 MarijnS95 deleted the rustc-crate-type-cdylib branch November 30, 2023 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants