build-v8.cmd - Issue 29451722: Issue 4907 - Update v8 to 5.7.278 in libadblockplus

Delta Between Two Patch Sets: build-v8.cmd

Issue 29451722: Issue 4907 - Update v8 to 5.7.278 in libadblockplus (Closed) Base URL: https://github.com/adblockplus/libadblockplus.git

Left Patch Set: add Xcode into readme Created June 2, 2017, 2:24 p.m.

Right Patch Set: independence from file mode of make_gyp_wrapper.py Created July 4, 2017, 11:11 a.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

Left: Side by side diff | Download
Right: Side by side diff | Download

LEFT	RIGHT
1 @echo off	1 @echo off

2 SETLOCAL	2 SETLOCAL

3	3

4 rem %1 - MSBuildBinPath, e.g. c:\Program Files (x86)\MSBuild\14.0\Bin	4 rem %1 - MSBuildBinPath, e.g. c:\Program Files (x86)\MSBuild\14.0\Bin
Oleksandr 2017/06/03 00:40:13 How about making the first parameter optional and How about making the first parameter optional and using msbuild from PATH if it isn't provided? sergei 2017/06/14 16:05:01 Currently it's not needed because that script is n Show quoted text On 2017/06/03 00:40:13, Oleksandr wrote: > How about making the first parameter optional and using msbuild from PATH if it > isn't provided? Currently it's not needed because that script is not manually called, so I would like to not complicate it.
5 rem %2 - arch {ia32, x64}	5 rem %2 - arch {ia32, x64}

6 rem %3 - configuration {Release, Debug}	6 rem %3 - configuration {Release, Debug}

	7 rem %4 - platform toolset, e.g. v140

7 set MsBuildBinPath=%~1	8 set MsBuildBinPath=%~1

8 set ARCH=%~2	9 set ARCH=%~2

9 set CONFIGURATION=%~3	10 set CONFIGURATION=%~3

	11 set PlatformToolset=%~4

10	12

11 pushd "%~dp0"	13 pushd "%~dp0"

12	14

13 @python msvs_gyp_wrapper.py --depth=build\%ARCH%\v8 -f msvs -I v8.gypi --generat or-output=build\%ARCH%\v8 -G msvs_version=2015 -Dtarget_arch=%ARCH% -Dhost_arch= %ARCH% third_party/v8/src/v8.gyp	15 @python msvs_gyp_wrapper.py --depth=build\%ARCH%\v8 -f msvs -I v8.gypi --generat or-output=build\%ARCH%\v8 -G msvs_version=2015 -Dtarget_arch=%ARCH% -Dhost_arch= %ARCH% third_party/v8/src/v8.gyp
Oleksandr 2017/06/03 00:40:13 I have tried building using VS2017, and it works s I have tried building using VS2017, and it works somewhat fine, once the projects are converted. However problem is that I had to manually upgrade the project files produced here, which are overwritten every time this script runs. Other option is to use something like /p:PlatformToolset=v141 (https://stackoverflow.com/questions/33380128/visual-studio-2015-command-line-...) in the line below, but that doesn't work for some reason. How about making the solution generation optional here? Better yet, why not move this into createsolutions.bat? sergei 2017/06/14 16:05:01 V8 project files don't require conversion when one Show quoted text On 2017/06/03 00:40:13, Oleksandr wrote: > I have tried building using VS2017, and it works somewhat fine, once the > projects are converted. However problem is that I had to manually upgrade the > project files produced here, which are overwritten every time this script runs. V8 project files don't require conversion when one uses MSVS2017. It does not show any dialog or what ever, why did you have to convert them? Show quoted text > Other option is to use something like /p:PlatformToolset=v141 > (https://stackoverflow.com/questions/33380128/visual-studio-2015-command-line-...) > in the line below, but that doesn't work for some reason. I'm not sure how V8 is working with v141 because the used version of V8 explicitly supports v140. However it seems it does work, so instead of adding "/p:PlatformToolset=v141" with a hardcoded value I have add a command line argument for it, so currently the value is taken from project setting for the toolset. Namely, if one uses MSVS 2017 and converts libadblockplus project files then toolset for build-v8 project is v141, otherwise v140 is used. Show quoted text > > How about making the solution generation optional here? Better yet, why not move > this into createsolutions.bat? I agree that generating of v8 project files each time we build libadblockplus looks bad, fortunately it does not cause a full rebuilding of v8, so it should not hurt here. If the solution generation is in createsolutions.bat then it should be done for all projects using libadblockplus.gyp but not that createsolution.bat, e.g. ABP for IE. Beside it complicates things, it looks a bit strange that one has to generate a solution for dependency (v8) of a dependency (libadblockplus). If it does not hurt, what should not, I would rather keep it as is for present. There is a plan to stop building of V8 in libadblockplus and use prebuilt binaries, so I consider this script rather as a temporary solution. Oleksandr 2017/07/03 21:50:38 Acknowledged. Show quoted text On 2017/06/14 16:05:01, sergei wrote: > On 2017/06/03 00:40:13, Oleksandr wrote: > > I have tried building using VS2017, and it works somewhat fine, once the > > projects are converted. However problem is that I had to manually upgrade the > > project files produced here, which are overwritten every time this script > runs. > V8 project files don't require conversion when one uses MSVS2017. It does not > show any dialog or what ever, why did you have to convert them? > > Other option is to use something like /p:PlatformToolset=v141 > > > (https://stackoverflow.com/questions/33380128/visual-studio-2015-command-line-...) > > in the line below, but that doesn't work for some reason. > I'm not sure how V8 is working with v141 because the used version of V8 > explicitly supports v140. However it seems it does work, so instead of adding > "/p:PlatformToolset=v141" with a hardcoded value I have add a command line > argument for it, so currently the value is taken from project setting for the > toolset. Namely, if one uses MSVS 2017 and converts libadblockplus project files > then toolset for build-v8 project is v141, otherwise v140 is used. > > > > How about making the solution generation optional here? Better yet, why not > move > > this into createsolutions.bat? > I agree that generating of v8 project files each time we build libadblockplus > looks bad, fortunately it does not cause a full rebuilding of v8, so it should > not hurt here. > If the solution generation is in createsolutions.bat then it should be done for > all projects using libadblockplus.gyp but not that createsolution.bat, e.g. ABP > for IE. Beside it complicates things, it looks a bit strange that one has to > generate a solution for dependency (v8) of a dependency (libadblockplus). If it > does not hurt, what should not, I would rather keep it as is for present. There > is a plan to stop building of V8 in libadblockplus and use prebuilt binaries, so > I consider this script rather as a temporary solution. > Acknowledged.
14	16

15 @"%MSBuildBinPath%/msbuild.exe" /m build/%ARCH%/v8/third_party/v8/src/v8.sln /p: Configuration=%CONFIGURATION% /target:v8_snapshot,v8_libplatform,v8_libsampler	17 @"%MSBuildBinPath%/msbuild.exe" /m build/%ARCH%/v8/third_party/v8/src/v8.sln /p: PlatformToolset=%PlatformToolset% /p:Configuration=%CONFIGURATION% /target:v8_sn apshot,v8_libplatform,v8_libsampler
Oleksandr 2017/06/03 00:40:13 It's a lot of building. How about adding /maxcpuco It's a lot of building. How about adding /maxcpucount:3, for example? https://msdn.microsoft.com/en-us/library/ms164311.aspx sergei 2017/06/14 16:05:01 What do you mean under a lot of building? Does /m Show quoted text On 2017/06/03 00:40:13, Oleksandr wrote: > It's a lot of building. How about adding /maxcpucount:3, for example? > https://msdn.microsoft.com/en-us/library/ms164311.aspx What do you mean under a lot of building? Does /m slow down the overall performance and you want to limit it to use only three CPU? I'm not sure that we should do it. Oleksandr 2017/07/03 21:50:38 Looks like I have missed the /m switch in this lin Show quoted text On 2017/06/14 16:05:01, sergei wrote: > On 2017/06/03 00:40:13, Oleksandr wrote: > > It's a lot of building. How about adding /maxcpucount:3, for example? > > https://msdn.microsoft.com/en-us/library/ms164311.aspx > > What do you mean under a lot of building? Does /m slow down the overall > performance and you want to limit it to use only three CPU? I'm not sure that we > should do it. Looks like I have missed the /m switch in this line. Please nevermind this.
16	18

17 popd	19 popd

18	20

19 ENDLOCAL	21 ENDLOCAL

20	22

21 exit /b	23 exit /b

LEFT	RIGHT