-
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
Upgrade to 3.2924.1575.g97389a9 / Add support for Visual Studio 2017 #46
Conversation
…_wrapper with the same settings.
…change - the locales have moved to a directory of their own, it seems.
… leave out the Debug/libcef.dll, since it was huge - around 180 MiB, which is bigger than the maximum limit enforced by GitHub. We can hopefully use the release/libcef.dll both for release and debug builds, since we aren't really debugging CEF so much in this use case…
… don't want to include either.
…unning in Debug, since the Debug libcef.dll is huge (180 MiB) and can hence not be version-controlled using GitHub. This works OK for now.
….a 1453, instead, since it gives us the x64 goodness we've all been longing for...
…n, I owe you :) so that the solution can be compiled with both VS2010 and VS2012, hopefully.
…r .lib files using MDd/MD (dynamic runtime linkage), not MTd/MT (static), which is still the default for Chromium.
…ttings (to make the projects compilable on both versions).
…t, though, because of problems upstream (http://www.magpcss.org/ceforum/viewtopic.php?f=6&t=11094).
…10/2012 (and soon 2013) to cater for.
…mpile stuff without this?
…les for x64 somehow...
Pristine apart from one place where we cheat/save some megabytes by reusing the /Release/libcef.dll in the /Debug folder too. Per did the same trick back in the CEF3 1413 and 1547 daysv- see SHA:d966747aba61397f918761073616a9868e0b1448
Same storry as previous commit for x64: Pristine apart from one place where we cheat/save some megabytes by reusing the /Release/libcef.dll in the /Debug folder too. Per did the same trick back in the CEF3 1413 and 1547 days- see SHA: d966747
- includes the .props include to handle builds for VS201x variants - and the /wd4275 compiler warning disable All this should give us four libcef_dll_wrapper.lib files ready for CefSharp consumption for both x86/x64/Debug/Release when running build.bat twice - for each compiler ... phew...
Add a short delay before moving the newly unzipped files - allow for 7z to release handles
Upgrade to cef.sdk.3.2785.1486
Exclude d3dcompiler_43.dll
Example for vs2015 and nupkg-only with local custom build of Cef branch 2924 .\build.ps1 -Target vs2015 -DownloadBinary local -CefVersion 3.2924.1538.gbfdeccd -CefBinaryDir "../../cefsource/chromium/src/cef/binary_distrib/" .\build.ps1 -Target nupkg-only -DownloadBinary none -CefVersion 3.2924.1538.gbfdeccd
Added command line support for using local CEF builds
… longer a common tools environment variable available, you have to use vswhere (https://github.com/Microsoft/vswhere). The SHA512 checksum has been included in the source code so that the executable may be independently verified.
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.
Thanks, comments inline 👍
build.ps1
Outdated
@@ -178,6 +182,10 @@ function Msvs | |||
$VXXCommonTools = $null | |||
$CmakeGenerator = $null | |||
|
|||
# https://github.com/Microsoft/vswhere/commit/a8c90e3218d6c4774f196d0400a8805038aa13b1 (Release mode / VS 2015 Update 3) |
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.
My target is to strip all the binaries from this repository eventually (even pruning the old ones aka #35).
I'd prefer to download vswhere
if required. Looks like there is a official release at https://github.com/Microsoft/vswhere/releases/tag/1.0.47 (same version was pushed to nuget.org
).
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.
Some code to check and download vswhere
from a different script I wrote. Someone can adapt this if they wanted.
$programFilesDir = (${env:ProgramFiles(x86)}, ${env:ProgramFiles} -ne $null)[0]
$client = New-Object System.Net.WebClient;
$vswherePath = Join-Path $programFilesDir 'Microsoft Visual Studio\Installer\vswhere.exe'
#Check if we already have vswhere which is included in newer versions of VS2017 installer
if(-not (Test-Path $vswherePath))
{
# Download vswhere if we don't have a copy
$vswherePath = Join-Path $WorkingDir \vswhere.exe
# TODO: Check hash and download if hash differs
if(-not (Test-Path $vswherePath))
{
$client.DownloadFile('https://github.com/Microsoft/vswhere/releases/download/2.2.7/vswhere.exe', $vswherePath);
}
}
build.ps1
Outdated
@@ -197,6 +205,22 @@ function Msvs | |||
$VXXCommonTools = Join-Path $env:VS140COMNTOOLS '..\..\vc' | |||
$CmakeGenerator = 'Visual Studio 14' | |||
} | |||
'v141' { |
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.
Formatting - indentation is out (I know the whole ps
file needs a reformat)
build.ps1
Outdated
@@ -565,6 +596,7 @@ switch -Exact ($Target) { | |||
#VSX v110 | |||
VSX v120 | |||
VSX v140 | |||
VSX v141 |
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.
How critical is it to include VS2017
binaries in cef.sdk
? VS2017
has only just been released and as I'm sure your well aware they're only required when compiling CefSharp
from source using VS2017
. Official builds are still done using VS2013
. Previously I'd stuck to just supporting two versions, current
and current - 1
. I'm a little reluctant to upgrade from VC++2013
at this time. This also means a build machine would require three different VS
versions.
How big are the new packages compared to the previous ones?
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.
VS2017 is ABI (binary) compatible with VS2015. This change is mainly for those that only have VS2017 installed.
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.
I can comment this one if thats what you prefer. Probably not that many people have adopted VS2017 yet.
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.
VS2017 is ABI (binary) compatible with VS2015. This change is mainly for those that only have VS2017 installed.
Do you mean VC++
?
I can comment this one if thats what you prefer. Probably not that many people have adopted VS2017 yet.
Yes please.
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.
I'm quite certain that VC++ 2015 and 2017 are not compatible with each other. That's never been the case before, there's always been that requirement to upgrade with each new release.
I get the impression that this is still the case:
@peters I think we'll wait with VS2017 until CEF moves up to it. That's probably the easiest approach for now.
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 just saw the comment in cefsharp/CefSharp#1983 - seems like you're right @peters! My bad.
build.ps1
Outdated
@@ -14,7 +14,7 @@ param( | |||
[string] $CefBinaryDir = "../cefsource/chromium/src/cef/binary_distrib/", | |||
|
|||
[Parameter(Position = 3)] | |||
$CefVersion = "3.2883.1552.g88ff29a" | |||
$CefVersion = "3.2924.1575.g97389a9" |
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.
Upgrading of versions would ideally be in a separate PR
as it's unrelated to adding VS2017
support.
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.
Hehe, sorry about that! I was in a hurry. I can rebase and send a new PR.
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.
No problem 👍 I'm hopeful a 2924
build will be available on http://opensource.spotify.com/cefbuilds/index.html in the next few days, so will upgrade to that ASAP
.
https://bitbucket.org/chromiumembedded/cef/commits/d3e47f59e9d3ba780cba5a77f90adf4f7ab5dc03
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.
Correction, I mean 2987
56f6e70
to
259397a
Compare
Hi, just wondering what the state of this PR is/what still needs be done to close this? (Can I help?) |
Agree, I would also find it interesting. @amaitland suggested this is the preferred start rather than #50, but this really needs a rebase (since the repository was cleaned up to get rid of old binaries.) @peters Are you still around, would you have a chance to rebase this off of current master? |
There are so many conflicts now that it's hard to really get an overview of this. @peters, some of what this PR aimed to do (upgrade CEF) has already been merged as of #53. I will close this now. I think we will add VS2017 support using the approach suggested in #50. If there are still things that should be done, please do so and submit a new PR. Thanks for your effort nonetheless! |
No description provided.