Skip to content

Conversation

@kumulynja
Copy link

Description

This PR just adds "lib" as a crate-type besides "cdylib" and "staticlib". In this way bdk-ffi can be more easily wrapped and reused for other bindings like the dart bindings in bdk-dart. Without it, more code from bdk-ffi has to be duplicated to bdk-dart and the process becomes more complex and hacky.

Notes to the reviewers

I tested this PR in the following bdk-dart PR already: bitcoindevkit/bdk-dart#12
It would be good to get this merged here first to be able to remove the patch to my fork of bdk-ffi there.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing
  • I've added a changelog in the next release tracking issue (see example)
  • I've linked the relevant upstream docs or specs above

Copy link
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

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

I'm not against this change per se, but I'd like to understand better why the plugin for dart needs it when others don't (JVM, Android, Swift, Python, React Native).

I took some time to clean this up a while back and my understanding was that the lib type is required in situations where your Rust code is used as a dependency itself, whereas for us here it's the compiled artifacts that are used by the plugins.

Again I'm not against it because it doesn't impact us negatively other than build times, but I want to make sure I know why each of those crate types are required (cdylib is for Android/JVM/Python, staticlib is for iOS).


[lib]
crate-type = ["cdylib", "staticlib"]
crate-type = ["lib", "cdylib", "staticlib"]
Copy link
Member

Choose a reason for hiding this comment

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

This is the only line that needs to be on this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants