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

winbuild: Fix manifest embedding for proper OS version querying #4399

Closed
wants to merge 1 commit into from

Conversation

@JDepooter
Copy link
Contributor

commented Sep 22, 2019

I believe this will fix #4391. I don't have nghttp built on this machine to test with, so I cannot be sure. I can verify that building without this fix results in no manifest in the exe, and there is a manifest after this change. Without a manifest the OS version checks in the SChannel code will not be correct, and libcurl will not attempt to use ALPN in the TLS handshake.

This fixes commit ebd2132 in pull request #1221. That commit added the CURL_EMBED_MANIFEST flag to CURL_RC_FLAGS (line 361). However, later in the file CURL_RC_FLAGS is overwritten. The fix is to append values to CURL_RC_FLAGS instead of overwriting.

This is a small fix to commit ebd2132 in pull request #1221. That commit added the CURL_EMBED_MANIFEST flag to CURL_RC_FLAGS. However, later in the file CURL_RC_FLAGS is overwritten. The fix is to append values to CURL_RC_FLAGS instead of overwriting
@bagder bagder closed this in 96a3ab7 Sep 23, 2019
@bagder

This comment has been minimized.

Copy link
Member

commented Sep 23, 2019

Thanks!

@bennyAv10

This comment has been minimized.

Copy link

commented Sep 23, 2019

I tested this fix with nghttp2 on curl 7.64.1 and it's working. Thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.