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

Automate asio sys build #804

Merged
merged 6 commits into from
Oct 17, 2023
Merged

Conversation

alisomay
Copy link
Contributor

@alisomay alisomay commented Oct 16, 2023

Hello all,

Having a rather manual pre build preparation process for the windows ASIO support could be cumbersome for users who'd like to install applications we publish to crates.io which depends on cpal by cargo install method because cargo install builds from source.

This PR is a suggestion to automate the two steps which is laid out in the readme..

  • Automate the finding and retrieval of ASIO SDK.
  • Set the build environment by invoking vcvarsall.bat

Trying to install LLVM would be out of scope for this script in my opinion because that is more general and essential to have a development environment. If the build fails due to that, normally the user can easily derive the reason from the error message.

I didn't update the README.md yet but if you find this PR welcome, I'd gladly do so.

Looking forward to your review!

*I've tested all the code paths on a x86_64 machine running Windows 10.
*The PR is an adaptation of a ps1 script I use for my project to automate the process in #801

@alisomay
Copy link
Contributor Author

I can also develop it further if more robustness is required.

Copy link
Member

@est31 est31 left a comment

Choose a reason for hiding this comment

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

in general it's a good idea to have a fallback mode where we download the asio SDK, but it should not cause additional dependencies for users who already have the SDK installed.

and it should also be mentioned in README :).

walkdir = "2"
cc = "1.0.25"
cc = "1.0.83"
reqwest = { version = "0.11" , features = ["blocking"] }
Copy link
Member

Choose a reason for hiding this comment

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

we should not include reqwest for a tiny task like a build script. it's better to invoke some available windows-specific command that downloads a file from an url. same goes for the zip crate, if possible

@alisomay
Copy link
Contributor Author

alisomay commented Oct 17, 2023

in general it's a good idea to have a fallback mode where we download the asio SDK, but it should not cause additional dependencies for users who already have the SDK installed.

and it should also be mentioned in README :).

Thanks for the quick look 🎉

I agree, the PR is meant to be a vehicle to show my intention not the complete implementation.
I'll re-work it in a way which it avoids dependencies and uses windows apis.

Also update the README.

Please don't hesitate to add any other ideas.

@alisomay
Copy link
Contributor Author

  • README.md updated.
  • Build script uses powershell commands now.
  • Build script detects if vcvarsall.bat is invoked in the session or not.

@alisomay alisomay requested a review from est31 October 17, 2023 09:53
@est31 est31 merged commit 3a44953 into RustAudio:master Oct 17, 2023
16 checks passed
@alisomay alisomay mentioned this pull request Oct 17, 2023
@chipnertkj
Copy link

When is this going to be released to crates.io? I've noticed that the cargo-publish job was skipped.

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