-
-
Notifications
You must be signed in to change notification settings - Fork 617
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
Fix missing CA certs on Windows #707
Conversation
Also added a test that fails on the previous commit
I'm OK with this change, but then I think we also want to only build/include/link against mbedtls on linux only... so more changes to the premake5.lua script would be needed. |
FWIW, the only reasons I initially made the change from using OpenSSL to mbedtls, was that I could link against it statically, and don't get a premake binary on linux that only works on one specific distribution of linux. On Mac & Windows that isn't really an issue, so using their internal 'tls' libraries is perfectly fine with me. |
contrib/mbedtls/premake5.lua
Outdated
@@ -16,4 +16,5 @@ project "mbedtls-lib" | |||
"library/*.c" | |||
} | |||
|
|||
|
|||
filter { "system:windows or macosx" } |
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.
Let's revert this file, since it would still create a .lib file that is empty...
I think just not linking against it is enough, since the dependency tree will not include it (at least in visual studio that will work)... Thank you for those modifications...
It apparently doesn't skip the project anymore now. I guess it's caused by building the entire solution instead of only the Premake5 project. |
No that is fine.... the alternative would be to not include the library script at all on mac and windows... which is on this line: https://github.com/premake/premake-core/blob/master/premake5.lua#L195 an |
Hello, is there anything to fix in this pull before merging it? This bug prevents https://github.com/mono/CppSharp from gaining support for VS 2017 because we're forced to update premake to get its support of VS 2017 but at the same time premake now has this bug which prevents us from downloading some dependencies. |
Thanks everyone! |
Thank you for merging this. When will there be a new release of premake 5 with this fix? |
Yes, it will be included in the next alpha release. |
This pull request fixes #706 (http.* with SSL doesn't work on Windows) and partially reverts the mbedtls transition.
The main reason for using Microsoft's own TLS implementation (Schannel) is that it doesn't explicitly require us to bundle the CA certs, but uses the the OS' built-in certs instead (see the last section for details: https://curl.haxx.se/docs/sslcerts.html)