-
Notifications
You must be signed in to change notification settings - Fork 88
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
initial VS2017 build support with VS2015 toolset #50
Conversation
Looks good to me. @amaitland or @jornh, do you see any objections to getting this merged? @japj - what would be required to get it building with VS2017 using the native (VS2017) toolset? It's not doable at the moment because CEF is VS2015 only? |
I personally would finish the work started in #46 If you are going ahead with this, which is totally up to you then the toolchain version is incorrect and comments should be added explaining what's going on. |
See 12dede8 my comments are inline. @perlun I'm not actively working on the project, so just go with whatever works for you. |
Unfortunately, appveyor does not have build machines with VS 2013, 2015 and 2017 installed (see https://www.appveyor.com/docs/build-environment/#pre-installed-software ). So building nuget packages with all binaries is a bit tricky I think. VS2017 c++ toolset is abi compatible with 2015 as far as I understand. Are people still using vs2013? |
Three versions is impractical size wise, I've never shipped a set of packages with more than two versions due to this. Adding
Removing It's all in @perlun's capable hands now. |
Ah, thanks! I accepted these requests now. If/when I have time to work on this, I'll let you all know. (It will probably not happen in an extremely near future, unless someone is willing to step in and pay for the work - just get in touch with me if you're reading this and want to discuss the terms.) |
@japj Since we added VS2017 support in CefSharp in cefsharp/CefSharp#2179, it would be nice to have a new go at this now as well. Would you have a chance to rebase it on top off the latest (We can still build the AppVeyor builds using VC2013 until we are ready to finally move over to VC2015++ redist, but I think we'll perhaps postpone that until CEF 63 or later.) |
ok, not sure if everything went ok now (bit of git uncertainty), first did the conflict resolution through the github web interface, which resulted in an additional merge commit. Redid it with a rebase upon master, hopefully it is ok now |
@perlun is there anything that I still need to do to get this finished? |
Sorry, just hasn't had time to look at it yet. Will give it a look ASAP! |
@japj Sorry, some more merge conflicts came via #61. Would I be asking for too much if you try rebasing this once more? (If you are unable, just let us know and someone else can give it a try instead.) (In the long run, we should perhaps drop all other versions than VS2015 and VS2017, and use the VC++2015 redistributable - I filed #64 about the latter part now.) |
Support for building with |
Enable cef-binary build with VS2017 using the VS2015 toolset.
Please note that the VS2017 provided cmake is not automatically put in the path, so the detection might fail. For now I manually put it in my path.