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

Fix missing CA certs on Windows #707

Merged
merged 4 commits into from
Mar 12, 2017
Merged

Fix missing CA certs on Windows #707

merged 4 commits into from
Mar 12, 2017

Conversation

jushar
Copy link
Contributor

@jushar jushar commented Mar 8, 2017

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)

@tvandijck
Copy link
Contributor

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.

@tvandijck
Copy link
Contributor

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.

@@ -16,4 +16,5 @@ project "mbedtls-lib"
"library/*.c"
}


filter { "system:windows or macosx" }
Copy link
Contributor

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...

@jushar
Copy link
Contributor Author

jushar commented Mar 9, 2017

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.
Shall I revert the revert or do we keep it like this?

@tvandijck
Copy link
Contributor

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 if not (os.is("windows") or os.is("macosx")) then around it would do it I think.

@ddobrev
Copy link

ddobrev commented Mar 11, 2017

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.

@starkos starkos merged commit ce49456 into premake:master Mar 12, 2017
@starkos
Copy link
Member

starkos commented Mar 12, 2017

Thanks everyone!

@ddobrev
Copy link

ddobrev commented Mar 13, 2017

Thank you for merging this. When will there be a new release of premake 5 with this fix?

@starkos
Copy link
Member

starkos commented Mar 13, 2017

Yes, it will be included in the next alpha release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

http.* with SSL doesn't work on Windows
5 participants