search

cefsharp / cef-binary

NuGet repackaging of the CEF binaries provided by Chromium Embedded Framework
https://cef-builds.spotifycdn.com/index.html
Other
110 stars 87 forks source link

initial VS2017 build support with VS2015 toolset #50

Closed japj closed 6 years ago

japj commented 7 years ago

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.

perlun commented 7 years ago

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?

amaitland commented 7 years ago

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.

amaitland commented 7 years ago

I personally would finish the work started in #46

See https://github.com/cefsharp/cef-binary/pull/46/commits/12dede8ecf8a9b29a0eaf1e3f1d0eed9b26675e9 my comments are inline.

@perlun I'm not actively working on the project, so just go with whatever works for you. Nuget.org says your Pending approval for the cef.redist.x64 and cef.redist.x86 packages, if you need me to remove and re-add you let me know. That should be the last piece you require in terms of permissions, then you're free to go nuts 😄

japj commented 7 years ago

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?

amaitland commented 7 years ago

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.

Three versions is impractical size wise, I've never shipped a set of packages with more than two versions due to this. Adding VS2017 support doesn't necessarily mean it will be enabled by default.

Are people still using vs2013?

Removing VS2013 support essentially forces an upgrade to VC++2015. I would expect the number of support requests to double, maybe even triple directly after that upgrade is made. Nobody ever reads the release notes...

It's all in @perlun's capable hands now.

perlun commented 6 years ago

@perlun I'm not actively working on the project, so just go with whatever works for you. Nuget.org says your Pending approval for the cef.redist.x64 and cef.redist.x86 packages, if you need me to remove and re-add you let me know. That should be the last piece you require in terms of permissions, then you're free to go nuts 😄

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

perlun commented 6 years ago

@japj Since we added VS2017 support in CefSharp in https://github.com/cefsharp/CefSharp/pull/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 master and update the PR?

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

japj commented 6 years ago

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

japj commented 6 years ago

@perlun is there anything that I still need to do to get this finished?

perlun commented 6 years ago

@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!

perlun commented 6 years ago

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

amaitland commented 6 years ago

Support for building with v141 added in commit https://github.com/cefsharp/cef-binary/commit/568d1f5cb1d0685ad623b20329007a3d27099661