Revert: Set default-features = false for rust-bitcoin and rust-miniscript#897
Revert: Set default-features = false for rust-bitcoin and rust-miniscript#897notmandatory wants to merge 2 commits intobitcoindevkit:release/0.27from
Conversation
This reverts commit 87c558c.
notmandatory
force-pushed
the
revert_bitcoin-default-features
branch
from
e8f92c2 to
3466393
Compare
notmandatory
force-pushed
the
revert_bitcoin-default-features
branch
3 times, most recently
from
74a39c0 to
6ffed05
Compare
rajarshimaitra
left a comment
rajarshimaitra
left a comment
There was a problem hiding this comment.
ACK.. One comment..
| miniscript = { version = "9.0", default-features = false, features = ["serde"] } | ||
| bitcoin = { version = "0.29.2", default-features = false, features = ["serde", "base64", "rand"] } | ||
| miniscript = { version = "9.0", features = ["serde"] } | ||
| bitcoin = { version = "0.29.1", features = ["serde", "base64", "rand"] } |
There was a problem hiding this comment.
Is it intentional to downgrade bitcoin to 0.29.1? I ran some tests with latest version and all seemed to work well with it. Am I missing something..
There was a problem hiding this comment.
This was part of the original commit and since I used the git revert command undid the bitcoin 0.29.2 change too. To keep it simple I'd rather not make any other changes in this PR but to revert the original commit (and corresponding changelog entry). Also even if we leave this at 0.29.1 anyone who does a cargo update will still get the latest 0.29.2 version.
| [dependencies] | ||
| bdk-macros = "^0.6" | ||
| log = "^0.4" | ||
| miniscript = { version = "9.0", default-features = false, features = ["serde"] } |
There was a problem hiding this comment.
Mh, rather than outright reverting it, could we maybe leave default-features = false and set miniscript/std in BDK's default feature? This way downstream projects could opt-out of std by setting default-features = false for BDK.
@benthecarman Would this work for you?
There was a problem hiding this comment.
This should work but will still force any downstream projects that use bdk and have default-features = false to now have to also enable a new std feature. That's the kind of breaking change I want to avoid on a patch release. Ideally I want to defer all breaking changes to the 1.0 release.
Besides Mutiny are there any other downstream projects that need to disable std for bitcoin and miniscript?
notmandatory
force-pushed
the
revert_bitcoin-default-features
branch
from
6ffed05 to
751ba3c
Compare
notmandatory
force-pushed
the
revert_bitcoin-default-features
branch
from
751ba3c to
9bc8cf9
Compare
|
In another PR I mentioned that we should always just enable the |
|
I'm closing this PR in favor of bumping the maintenance release version to 0.28.0 and add a default |
5 participants
Description
Since the
bitcoinandminiscriptdependencies require enabling eitherstdorno-stdthe only way we can make this change is to add a correspondingstdandno-stdfeatures and requiring our downstream users to enable one. But BDK doesn't really supportno-stdyet and we don't want to break downstream projects on a patch release. So unfortunately I need to revert #882.See also: rust-bitcoin/rust-miniscript#533
Notes to the reviewers
I've also yanked release
0.27.2on crates.io and will be sure to test future releases with downstream bdk-cli and bdk-ffi projects to catch any issues like this.Changelog notice
None
Checklists
All Submissions:
cargo fmtandcargo clippybefore committing