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

tests/server/util.c: fix support for Windows Unicode builds #6070

Closed
wants to merge 1 commit into from

Conversation

@mback2k
Copy link
Member

@mback2k mback2k commented Oct 13, 2020

Detected via #6066

@mback2k mback2k requested a review from MarcelRaad Oct 13, 2020
@mback2k mback2k self-assigned this Oct 13, 2020
@mback2k
Copy link
Member Author

@mback2k mback2k commented Oct 13, 2020

@bagder would you mind waiting with the release tomorrow till this and #6066 is merged to make sure we have a good state of curl on Windows before release?

@mback2k mback2k added the regression label Oct 13, 2020
Detected via #6066
@mback2k mback2k force-pushed the mback2k:fix-test-util-unicode branch from 0fdd80a to 2a887c2 Oct 13, 2020
@bagder
Copy link
Member

@bagder bagder commented Oct 13, 2020

Sorry, but no. There's no delaying of the release, I can't deal with that this late. Both these changes are also in testing/CI and don't actually change the product we ship tomorrow.

@mback2k
Copy link
Member Author

@mback2k mback2k commented Oct 13, 2020

Yes, I get that. Unfortunately these issues mean that we have no way to know if Unicode curl is working or not on Windows at the moment. Unless someone performs a manual test with the changes applied. cc @MarcelRaad @vszakats @jay

@jay
Copy link
Member

@jay jay commented Oct 13, 2020

It looks correct as long as the TEXT macro is available (windows.h included). I don't know much about cmake and #6066. If there is some command you need me to run here let me know, I have cmake version 3.18.1 installed in Windows.

@mback2k
Copy link
Member Author

@mback2k mback2k commented Oct 14, 2020

Thanks. Since the non-Unicode CI completed successfully, I will merge this in a few minutes since this means the macro is available as assumed.

@mback2k mback2k closed this in 1124180 Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.