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
Document non-functional no-std for bitcoinconsensus #1352
Conversation
concept ACK, though we should try to investigate upstream what's wrong |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 3988e4a
What's the error message? Are you sure it's not some ARM issue instead of |
Its a build problem with building the C code, I did not do more than a cursory read of the output but here it is:
|
3988e4a
to
dc0ae1a
Compare
Rebased on master, no other changes. |
Then I guess it's a problem with the specific toolchain, not But this message looks quite weird anyway:
|
My guess is that the g++ there is just too old and the code is using some magic C++ craziness that the old version can't handle. If you could compare the versions that'd help investigating. Also I think we should print them out in CI for quicker access. |
I think you could be right but without building with |
If we can prove it then it's probably not even ARM build but specific version of GCC. If you have the tie to try out on your machine first that'd be great. |
Cool, leave it with me. |
Putting on ice till after release of 0.30.0 |
There is no "no-std" feature any more. |
There is a no-std build (by disabling default features). Does it no longer apply? |
Gee wiz, I was on fire yesterday. |
dc0ae1a
to
2c1cc66
Compare
I don't have clock cycles to dig into this at the moment. We learned yesterday that LDK is using |
CI fail is unrelated to this PR. |
Can we fix it in this PR though? I think it's a pretty small fix, it's just that some import needs to be feature gated. I think it's a bad interaction between #1225 ( |
I was going to rebase everything when #2279 merges. |
Oh this was the first PR I did this morning before I tackled #2279. I just wrote that comment to save you clicking on the CI failing job and investigating for me. |
The `rust-bitcoinconsensus` dependency is not able to be built with `--thumbv7m-none-eabi`. This means, at least as far as verifying our `no-std` build, `no-std` does not work when the `bitcoinconsensus` dependency is enabled. Instead of fixing the problem, document that it is broken.
2c1cc66
to
9955756
Compare
Rebased to pick up #2279, no other changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 9955756
bitcoinconsensus = { version = "0.20.2-0.5.0", default-features = false, optional = true } | ||
|
||
# Please note, bitocinconsensus does not currently work with no-std. | ||
bitcoinconsensus = { version = "0.20.2-0.5.0", optional = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not documenting anything and as I understand it, bitcoinconsensus
may be able to compile on some no_std
platforms but not all of them?
There are bigger fish to fry. |
The
rust-bitcoinconsensus
dependency is not able to be built with--thumbv7m-none-eabi
. This means, at least as far as verifying ourno-std
build,no-std
does not work when thebitcoinconsensus
dependency is enabled.Instead of fixing the problem, document that it is broken.
Context
cargo +nightly build --target thumbv7m-none-eabi
fails on every tag inrust-bitcoinconsensus
back to v0.17.1