-
Notifications
You must be signed in to change notification settings - Fork 270
build(core): allow windows-targets
0.53.0
#694
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
build(core): allow windows-targets
0.53.0
#694
Conversation
6b3e731
to
43e7db9
Compare
This PR is effectively ready for review, minus dependencies. A local |
Looks like CI is indeed happy? Is this still a draft? poking @ChrisDenton |
43e7db9
to
ce6b657
Compare
|
|
||
[target.'cfg(any(windows, target_os = "cygwin"))'.dependencies] | ||
windows-targets = "0.52.6" | ||
windows-targets = ">=0.52.6,<0.54.0" |
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.
Driveby note; this is a pretty unusual version spec, it should come with a comment explaining what's going on
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.
Added a comment in the latest rebased commit.
TBH, I'm not sure what commentary to add besides "this is what the version spec. does". Did you have more in mind?
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 think we should consider switching to the windows-link
crate here. It's much more stable because it uses raw-dylib
to generate import libs as-needed rather than needing to pre-compile import libs as windows-targets
needs to.
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.
Is the suggestion blocking, or for a follow-up?
ce6b657
to
3ae9df5
Compare
The Test (windows-latest, stable-i686-msvc) job is failing, but I'm not sure why, or how exactly to debug this. I don't have any 32-bit machines. 😅 |
Neither does CI. It cross-compiles to 32-bit. But it might just be that test is flaky again (32-bit struggles with symbolization). I'll try to look into it. |
Apologies for not getting to this expediently. I will likely accept #727 instead as it seems to implement the change preferred by @ChrisDenton |
Presuming this was on account of bandwidth, I feel your pain. 🫡 At least there's good news for movement! |
Yep. backtrace 0.3.76 is out and that will be the version that rust-lang/rust uses shortlyish, assuming CI passes after std integration. |
Hard dependency on nagisa/rust_libloading#169.