-
Notifications
You must be signed in to change notification settings - Fork 442
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
Regenerate windows sys bindings #1132
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
f9c9193
Regenerate windows sys bindings
invalid-email-address c407c89
Add new dep windows-targets v0.52 on windows
NobodyXu 6dfd7f6
Replace `windows_targets::link!` with homebrew `link_macro!`
NobodyXu 0b408d3
Fix fmt
NobodyXu f6ed3de
Rm accidentally commited files
NobodyXu f394c9f
Refactor `gen-windows-sys-binding`
NobodyXu cbefede
Simplify gen-windows-sys-binding
NobodyXu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,3 +7,4 @@ publish = false | |
[dependencies] | ||
windows-bindgen = "0.58" | ||
tempfile = "3" | ||
regex = "1" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,19 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
//! Provides the `link!` macro used by the generated windows bindings. | ||||||||||||||||||||||||||||||||||||||||||||||
//! | ||||||||||||||||||||||||||||||||||||||||||||||
//! This is a simple wrapper around an `extern` block with a `#[link]` attribute. | ||||||||||||||||||||||||||||||||||||||||||||||
//! It's very roughly equivalent to the windows-targets crate. | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
macro_rules! link_macro { | ||||||||||||||||||||||||||||||||||||||||||||||
($library:literal $abi:literal $($link_name:literal)? $(#[$doc:meta])? fn $($function:tt)*) => ( | ||||||||||||||||||||||||||||||||||||||||||||||
// Note: the windows-targets crate uses a pre-built Windows.lib import library which we don't | ||||||||||||||||||||||||||||||||||||||||||||||
// have in this repo. So instead we always link kernel32.lib and add the rest of the import | ||||||||||||||||||||||||||||||||||||||||||||||
// libraries below by using an empty extern block. This works because extern blocks are not | ||||||||||||||||||||||||||||||||||||||||||||||
// connected to the library given in the #[link] attribute. | ||||||||||||||||||||||||||||||||||||||||||||||
#[link(name = "kernel32")] | ||||||||||||||||||||||||||||||||||||||||||||||
extern $abi { | ||||||||||||||||||||||||||||||||||||||||||||||
$(#[link_name=$link_name])? | ||||||||||||||||||||||||||||||||||||||||||||||
pub fn $($function)*; | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+7
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For example:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
pub(crate) use link_macro as link; |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
While the import library is often named the same as the DLL, this isn't always true (e.g. there's a Synchronization.lib but not synchronization.dll). This doesn't affect cc-rs at the moment but it could do in the future. I wonder if it would be better to maintain a manual mapping, even if this is more verbose?
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.
How many of these cases are there?
If there're only a few, then I could hard code it here, so that people don't have to modify the
macro_rules!
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.
The Windows API is sprawling so it's hard to answer definitely but it seems like there are a lot. Complicating matters further are functions like
GetFileVersionInfoExA
which is exported from version.dll and there is version.lib but the lib doesn't contain that function. I don't know how many of them there are but cases like that would need to be handled specially by matching on the function name.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.
Alternatively we could bump the msrv up to 1.65 and just use raw-dylib. That would completely side-step any issues.
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'm ok with raw-dynlib, but the problem is that we can't enable it just cc-rs, it's not a feature but a cfg.
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.
Hmmm, I think it's fine for now, it's not like we add new windows sys binding very often.
Thank you so much, and I will merge it as now.
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.
It's not a cfg if we implement in a macro ourselves. Then it just a
#[link ... kind="raw-dylib"]
.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.
Hmmm I didn't know we could do that, would open another PR.