Skip to content

Conversation

@mitchcapper
Copy link
Contributor

@mitchcapper mitchcapper commented Oct 6, 2019

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

@amaitland amaitland changed the title Visual studio 2019 support Closes #2628 Add support for compiling with Visual studio 2019 Oct 6, 2019
@AppVeyorBot
Copy link

<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>
Copy link
Member

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.

<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

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

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.

@amaitland
Copy link
Member

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.

@amaitland amaitland merged commit f83a375 into cefsharp:master Nov 3, 2019
amaitland added a commit that referenced this pull request Nov 3, 2019
…ll_wrapper.lib otherwise fallback to VS2017

Follow up to #2924
@amaitland
Copy link
Member

Additional check added in 12edb67

I've not tested with a VS2019 build of the libcefwrapper`, if you experience any problems please let me know.

kkwpsv pushed a commit to kkwpsv/CefSharp that referenced this pull request Dec 24, 2019
…ll_wrapper.lib otherwise fallback to VS2017

Follow up to cefsharp#2924
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.

Feature Request - Add Support for compiling with VS2019

3 participants