-
Notifications
You must be signed in to change notification settings - Fork 909
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(windows): fix typo in build.rs
#3955
Conversation
It doesn't look like we want to merge this as is? The commits here are a little strange to me. |
@djc There is a typo in the build script that actually invalidated the check that @ChrisDenton wanted to add. If we're linking against |
Hm, I don't think that's a typo? It's just that |
@ChrisDenton You accidentally wrote |
Ah thanks! I'm confused because when I clicked the "Files Changed" tab I don't see that change. Edit: so I was looking at completely the wrong thing, apparently 😅 |
e543987
to
d053583
Compare
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.
These code changes look good to me, though I'll happily defer to @ChrisDenton on whether these changes are correct (in particular whether we still need delay loading for powrprof
).
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.
We don't seem to be using powrprof for anything and haven't been for at least a long while (I checked some old rustup-init versions). So I'm good with this.
Cherry-picked from #3898.
The first commit is deliberately failing to demonstrate that the fix is actually working.
See aws/aws-lc-rs#453 (comment) for the whole context: