Bug 51937 – Windows build should use per-configuration build output directories

• Component: New Bugs
• Status: RESOLVED DUPLICATE

[Steve Falkenburg]

Windows build should use per-configuration build output directories

[Steve Falkenburg]

Created attachment 78020 [details]
Patch

[Adam Roben (:aroben)]

Comment on attachment 78020 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=78020&action=review

> Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCoreGenerated.vcproj:110
> +				BuildCommandLine="%SystemDrive%\cygwin\bin\which.exe bash
if errorlevel 1 set PATH=%SystemDrive%\cygwin\bin;%PATH%
cmd /c

set WEBKITCONFIGURATIONNAME=$(ConfigurationName)
nmake /nologo -f JavaScriptCoreGenerated.make"
> +				ReBuildCommandLine="%SystemDrive%\cygwin\bin\which.exe bash
if errorlevel 1 set PATH=%SystemDrive%\cygwin\bin;%PATH%
cmd /c

set WEBKITCONFIGURATIONNAME=$(ConfigurationName)
nmake /nologo -f JavaScriptCoreGenerated.make clean
nmake -f JavaScriptCoreGenerated.make"
> +				CleanCommandLine="%SystemDrive%\cygwin\bin\which.exe bash
if errorlevel 1 set PATH=%SystemDrive%\cygwin\bin;%PATH%
cmd /c

set WEBKITCONFIGURATIONNAME=$(ConfigurationName)
nmake /nologo -f JavaScriptCoreGenerated.make clean"
> +				Output=""
> +				PreprocessorDefinitions=""
> +				IncludeSearchPath=""
> +				ForcedIncludes=""
> +				AssemblySearchPath=""
> +				ForcedUsingAssemblies=""
> +				CompileAsManaged=""

I don't think you meant to do this. VS helpfully does this for you.

[Adam Roben (:aroben)]

*** This bug has been marked as a duplicate of bug 49185 ***

[Build Bot]

Attachment 78020 [details] did not build on win:
Build output: http://queues.webkit.org/results/7252446

[Adam Roben (:aroben)]

Comment on attachment 78020 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=78020&action=review

Overall: You seem to inconsistently use $ConfigurationName vs. $WebKitConfigurationName, but I think in almost all cases a $ConfigurationBuildDir variable would be clearer.

> Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore.make:18
> +    xcopy "$(SRCROOT)\AppleInternal\tests\SunSpider\*" "$(OBJROOT)\$(BUILDSTYLE)\tests\SunSpider" /e/v/i/h/y
> +    cd "$(OBJROOT)\$(BUILDSTYLE)\tests\SunSpider"

Maybe a new variable would be better than repeating this over and over? Xcode calls this CONFIGURATION_BUILD_DIR. Maybe we could use that name elsewhere, too.

> Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore.sln:27
> -		Release_PGOInstrument|Win32 = Release_PGOInstrument|Win32
> -		Release_PGOOptimize|Win32 = Release_PGOOptimize|Win32
> +		Release_PGO_Optimize|Win32 = Release_PGO_Optimize|Win32
> +		Release_PGO|Win32 = Release_PGO|Win32

Why the renames? Can we do this separately?

> Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.vcproj:147
> -			WholeProgramOptimization="2"
> +      WholeProgramOptimization="2"
>  			>
> -			<Tool
> +      <Tool

This doesn't look right.

> Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.vcproj:1921
> -				RelativePath="$(WebKitOutputDir)\obj\$(ProjectName)\DerivedSources\ArrayPrototype.lut.h"
> +				RelativePath="$(WebKitOutputDir)\$(WebKitConfigurationName)\obj\$(ProjectName)\DerivedSources\ArrayPrototype.lut.h"

Why not just use $(ConfigurationName) here?

> Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCoreCommon.vsprops:9
> -		AdditionalIncludeDirectories="&quot;$(WebKitOutputDir)\obj\JavaScriptCore\DerivedSources\&quot;;../../;../../API/;../../pcre/;../../parser/;../../bytecompiler/;../../jit/;../../runtime/;../../bytecode/;../../interpreter/;../../wtf/;../../profiler;../../assembler/;../../debugger/;&quot;$(WebKitLibrariesDir)\include&quot;;&quot;$(WebKitLibrariesDir)\include\private&quot;;&quot;$(WebKitOutputDir)\include&quot;;&quot;$(WebKitOutputDir)\include\JavaScriptCore&quot;;&quot;$(WebKitOutputDir)\include\private&quot;;&quot;$(WebKitLibrariesDir)\include\pthreads&quot;"
> +		AdditionalIncludeDirectories="&quot;$(WebKitOutputDir)\$(WebKitConfigurationName)\obj\JavaScriptCore\DerivedSources\&quot;;../../;../../API/;../../pcre/;../../parser/;../../bytecompiler/;../../jit/;../../runtime/;../../bytecode/;../../interpreter/;../../wtf/;../../profiler;../../assembler/;../../debugger/;&quot;$(WebKitLibrariesDir)\include&quot;;&quot;$(WebKitLibrariesDir)\include\private&quot;;&quot;$(WebKitOutputDir)\$(WebKitConfigurationName)\include&quot;;&quot;$(WebKitOutputDir)\$(WebKitConfigurationName)\include\JavaScriptCore&quot;;&quot;$(WebKitOutputDir)\$(WebKitConfigurationName)\include\private&quot;;&quot;$(WebKitLibrariesDir)\include\pthreads&quot;"

...and here?

> Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCoreCommon.vsprops:17
> -		AdditionalLibraryDirectories="&quot;$(IntDir)\lib&quot;"
> +		AdditionalLibraryDirectories="&quot;$(WebKitOutputDir)\$(WebKitConfigurationName)\lib&quot;"

...and here?

> Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCorePostBuild.cmd:1
> -if exist "%WEBKITOUTPUTDIR%\buildfailed" del "%WEBKITOUTPUTDIR%\buildfailed"
> +if exist "%WEBKITOUTPUTDIR%\%WEBKITCONFIGURATIONNAME%\buildfailed" del "%WEBKITOUTPUTDIR%\%WEBKITCONFIGURATIONNAME%\buildfailed"

If we set up a CONFIGURATION_BUILD_DIR variable in the global pre-build event (etc.), we could use it here.

> Source/JavaScriptCore/JavaScriptCore.vcproj/WTF/WTFPreBuild.cmd:4
> +if exist "%WEBKITOUTPUTDIR%\%CONFIGURATIONNAME%\buildfailed" grep XX%PROJECTNAME%XX "%WEBKITOUTPUTDIR%\%CONFIGURATIONNAME%\buildfailed"

I thought it was %WEBKITCONFIGURATIONNAME%? (Though using %CONFIGURATIONNAME% instead seems fine, as long as it actually works. We should just be sure to be consistent.)

> Tools/FindSafari/FindSafari.vcproj:20
> -			InheritedPropertySheets="$(WebKitVSPropsRedirectionDir)..\..\WebKitLibraries\win\tools\vsprops\common.vsprops;$(WebKitVSPropsRedirectionDir)..\..\WebKitLibraries\win\tools\vsprops\debug.vsprops;.\FindSafariCommon.vsprops"
> +			InheritedPropertySheets="..\..\WebKitLibraries\win\tools\vsprops\common.vsprops;..\..\WebKitLibraries\win\tools\vsprops\debug.vsprops;.\FindSafariCommon.vsprops"

Whoopsie!

> WebCore/WebCore.vcproj/WebCore.vcproj:351
> -				RelativePath="$(WebKitOutputDir)\obj\$(ProjectName)\DerivedSources\ColorData.cpp"
> +				RelativePath="$(WebKitOutputDir)\$(ConfigurationName)\obj\$(ProjectName)\DerivedSources\ColorData.cpp"

It would be nicer to just use $(ConfigurationBuildDir) instead of $(WebKitOutputDir)\$(ConfigurationName).

> WebCore/dom/Document.cpp:1610
> -    ASSERT(!view() || (!view()->isInLayout() && !view()->isPainting()));
> +//    ASSERT(!view() || (!view()->isInLayout() && !view()->isPainting()));

I don't think you meant to leave this in.