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

Switch to windows-sys on 0.4.x branch #1190

Closed
pitdicker opened this issue Jul 17, 2023 · 4 comments
Closed

Switch to windows-sys on 0.4.x branch #1190

pitdicker opened this issue Jul 17, 2023 · 4 comments

Comments

@pitdicker
Copy link
Collaborator

#1003 (comment) mentioned we can switch from winapi to windows-sys on the 0.4.x branch now that our MSRV exceeds 1.48.

But I don't know if there are other concerns.

@pitdicker
Copy link
Collaborator Author

See #712 for the original PR to switch to windows-sys.

@djc
Copy link
Member

djc commented Jul 24, 2023

I'm open to this -- an alternative approach which I think would be preferrable would be to use the windows-bindgen crate to generate more minimal API surface in an integration test instead of using windows-sys. The problem with windows-sys is that it adds lots of download size, so it's not a great fit for low-level crates like chrono.

@pitdicker
Copy link
Collaborator Author

Documentation on windows-bindgen:
https://github.com/microsoft/windows-rs/blob/0.48.0/docs/readme.md#windows-bindgen
https://kennykerr.ca/rust-getting-started/standalone-code-generation.html

It does come with a note:

Warning: Standalone code generation should only be used as a last resort for the most demanding scenarios. It is much simpler to use the windows-sys crate and let Cargo manage this dependency. This windows-sys crate provides raw bindings, is heavily tested and widely used, and should not meaningfully impact your build time.

I'll have a look at it.

@djc
Copy link
Member

djc commented Jul 24, 2023

I think chrono would qualify as a fairly demanding scenario, and I think the warning is maybe a bit overblown.

See more context in microsoft/windows-rs#1720.

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

No branches or pull requests

2 participants