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

copy only the libraries that were built #135

Merged
merged 1 commit into from
May 13, 2024
Merged

Conversation

mightyguava
Copy link
Contributor

@mightyguava mightyguava commented May 10, 2024

As in #89, we build multiple ffi libraries in the same cargo workspace. cargo-ndk ends up copying all libraries in the target directory instead of only the ones built for that invocation. Gradle then will complain with something like 2 files found with path 'lib/arm64-v8a/libffi.so' from inputs in the merge JNI libraries step.

This change parses the library name from Cargo.toml and copies only the library (per target) to the output directory. I'm not sure if this will end up missing some libraries in copy. As far as I can tell, every rust crate can have 0 or 1 libraries, and package.name is required in Cargo.toml so we always have a name.

The library name is derived from lib.name if set, or from package.name if not, per https://doc.rust-lang.org/cargo/reference/cargo-targets.html#the-name-field

@bbqsrc bbqsrc merged commit 5045be6 into bbqsrc:main May 13, 2024
@mightyguava
Copy link
Contributor Author

@bbqsrc how do I get this new change? Do I wait for you to get a new tag? Not sure how cargo plugins versioning works

@bbqsrc
Copy link
Owner

bbqsrc commented May 15, 2024

Published in 3.5.5. 😄

package: Option<Package>,
package: Package,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is breaking the parsing of Cargo.toml files which define a [workspace], without defining a [package]: https://github.com/rust-mobile/ndk/actions/runs/9149848316/job/25153929797

@bbqsrc is it possible to yank and fix this?

@bbqsrc
Copy link
Owner

bbqsrc commented May 20, 2024

done

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.

3 participants