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

deps: Move from winapi to windows-sys #1595

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

poliorcetics
Copy link
Contributor

winapi is in maintenance mode and the new blessed way to access Windows APIs are the windows
and windows-sys crates. I don't think any types of winapi were exposed in the public API so
I used windows-sys since it has much faster compile times.

@zh-jq-b zh-jq-b mentioned this pull request Sep 13, 2023
@briansmith
Copy link
Owner

@poliorcetics Fair enough. What's the MSRV policy for windows-sys?

@faern
Copy link

faern commented Sep 20, 2023

Current MSRV seems to be 1.48. This is the closest to a "policy" I find: microsoft/windows-rs#2300

@poliorcetics
Copy link
Contributor Author

poliorcetics commented Sep 20, 2023

windows-sys tries to not do releases (the rythm is ask-based, there will not be a release if no one asks for one, and there needs to be a reason), and same for the MSRV: unless needed (and given the crate is "just" bindings to the Windows API, it shouldn't often need it), windows-sys won't move much, if at all from what we saw in the last few months

EDIT: negative is hard

@briansmith
Copy link
Owner

I can't get the GitHub Actions to let me run this in CI. Could you please rebase this on top of the latest main and push -f it to this branch? then I think I can run the CI and merge it.

Cargo.toml Outdated Show resolved Hide resolved
`winapi` is in maintenance mode and the new blessed way to access Windows APIs are the `windows`
and `windows-sys` crates. I don't think any types of `winapi` were exposed in the public API so
I used `windows-sys` since it has much faster compile times.
@codecov
Copy link

codecov bot commented Sep 21, 2023

Codecov Report

Merging #1595 (f493f89) into main (20ba41d) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1595   +/-   ##
=======================================
  Coverage   92.24%   92.24%           
=======================================
  Files         132      132           
  Lines       18845    18845           
  Branches      196      196           
=======================================
+ Hits        17383    17384    +1     
  Misses       1426     1426           
+ Partials       36       35    -1     
Files Changed Coverage Δ
src/cpu/arm.rs 92.39% <ø> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@poliorcetics
Copy link
Contributor Author

Also of note: in the past the Windows-rs team has reduced the MSRV if it proved important for a crate and it was feasible: GuillaumeGomez/sysinfo#950 (comment) :)

@briansmith briansmith merged commit 7bbc307 into briansmith:main Sep 27, 2023
257 checks passed
@briansmith
Copy link
Owner

Thanks!

@briansmith
Copy link
Owner

Friends, I merged this but I am hoping to immediately replace it by PR #1542, which removes the windows-sys dependency. I would appreciate people taking a look at #1542 and testing it out on Aarch64 Windows systems.

@poliorcetics poliorcetics deleted the winapi-to-windows-sys branch September 27, 2023 06:45
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