-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Add support for compiling with Visual studio 2019 #2924
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
Add support for compiling with Visual studio 2019 #2924
Conversation
…o 18362 as CEF 77>= require it Closes cefsharp#2628
|
✅ Build CefSharp 77.1.30-CI3328 completed (commit c4d89287d9 by @mitchcapper) |
| <VisualStudioProductVersion Condition="'$(VisualStudioVersion)'=='14.0'">2015</VisualStudioProductVersion> | ||
| <VisualStudioProductVersion Condition="'$(VisualStudioVersion)'=='15.0'">2017</VisualStudioProductVersion> | ||
| <VisualStudioProductVersion Condition="'$(VisualStudioVersion)'=='16.0'">2017</VisualStudioProductVersion> | ||
| <VisualStudioProductVersion Condition="'$(VisualStudioVersion)'=='16.0'">2019</VisualStudioProductVersion> |
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.
This is where things get complicated, the default cef.sdk package only has VS2015 and VS2017 versions of the wrapper by default. With this change anyone using the packages from Nuget.org would be unable to build using VS2019
The VisualStudioProductVersion variable is used to determine which directory to load the .lib file from.
CefSharp/CefSharp.Core/CefSharp.Core.vcxproj
Line 136 in a843407
| <AdditionalLibraryDirectories>$(SolutionDir)packages\$(CefSdkVer)\CEF\$(Platform)\$(Configuration);$(SolutionDir)packages\$(CefSdkVer)\CEF\$(Platform)\$(Configuration)\VS$(VisualStudioProductVersion)</AdditionalLibraryDirectories> |
Maybe it's possible to have AppVeyor build VS2015, VS2017 and VS2019 versions of the wrapper. See https://www.appveyor.com/docs/windows-images-software/ for list of software installed by image.
Current image is https://github.com/cefsharp/cef-binary/blob/master/appveyor.yml#L1
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.
Ah, I could add the v142 to the default for cef-binary building but yeah we would have to change the appveyor default to vs2019 version. Now it makes sense why you had the newer vs versions targeting the older option.
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.
VS2019 appears perfectly capable of compiling with a v141 lib file (as was promised by Microsoft that they'd be compatible).
I understand from your Docker build scripts you'd be looking to build everything with VS2019.
Open to suggestions here.
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.
Targeting v141 is no problem. With docker could even install V2017 and V2019 side by side without issue. In addition can easily patch in any changes to the code instead to manually change to v142. Keeping in on V141 for now would seem the best solution.
|
I've been thinking about how to best support both use cases, it's probably worth investigating if we can use a File exists check. Check if the package contains the vs2019 compiled lib, set VisualStudioProductVersion to 2019 otherwise fallback to using 2017. |
…ll_wrapper.lib otherwise fallback to VS2017 Follow up to #2924
|
Additional check added in 12edb67 I've not tested with a |
…ll_wrapper.lib otherwise fallback to VS2017 Follow up to cefsharp#2924
Bumped sdk to 18362 CEF 77 now requires that version but we could lower it is desired.
Does not default to building with it.
Resolves #2628