-
Notifications
You must be signed in to change notification settings - Fork 27
Add build script #34
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
Add build script #34
Conversation
c685b9c to
5e37237
Compare
5e37237 to
fabe07b
Compare
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.
I don't think using build scripts is the best way to do this: our goal is to install hwi on the user's machine, not the builder's.
I think the best way is to expose a couple of functions, one to check if hwi is available (also checking its version) and one to install it. Then I would leave it up to the user of this library to call those functions whenever they want.
Most of the time I would expect hwi to be packaged together with the rust binaries in some ways: when using package managers like apt or rpm you could just add the python hwi as a dependency of your package. Alternatively when building a wallet into an AppImage you can probably bundle hwi in there as well.
fabe07b to
3571997
Compare
d0a9a06 to
6ba0b17
Compare
6ba0b17 to
231b9da
Compare
|
Once this is updated, I can ACK, merge and tag the new version (needed for bitcoindevkit/bdk#682) - no rush tho :) We can totally start reviewing #682 before rust-hwi is published, you can just use rust-hwi master in the meantime |
79a99a6 to
6d801a9
Compare
6d801a9 to
589c3cf
Compare
|
utACK 589c3cf |
589c3cf7cb163a583f84018670df3e9dfb7d4a9a Add functions for checking and installing hwi (wszdexdrf)
Pull request description:
I believe this should be all that is needed. I don't think we can install OS-level dependencies. For udev rules, it is possible but then we would have to duplicate/include_str the `install_udev_rules()` function in `build.rs`
ACKs for top commit:
danielabrozzoni:
utACK 589c3cf7cb163a583f84018670df3e9dfb7d4a9a
Tree-SHA512: e7738f38f2d566ee2d62dc1152baa5c3d4c4415e61244b7c626b51d1dd8ad68871bece16fabc898ed8d25c2008354f4477de4e0bb4f96052621534c5642be17c
I believe this should be all that is needed. I don't think we can install OS-level dependencies. For udev rules, it is possible but then we would have to duplicate/include_str the
install_udev_rules()function inbuild.rs