Bug 104136 – WebKit2.def seems not regenerated when WebKit2.def.in is changed

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Xianzhu Wang]

My patch in bug 102543 added a method called from Internals to WebCore. I added the symbol in WebKit2.def.in but the build still reported unresolved external of the symbol on ews-win. So I guessed that WebKit2.def is not regenerated from changed WebKit2.def.in. To prove that, I created a experimental patch https://bugs.webkit.org/attachment.cgi?id=177603 in which I removed some other symbols like Page::scrollingTreeStateAstext() etc. to see if ews-win would report unresolved externals for these symbols. ews-win still only reported one unresolved external of my added symbol (http://queues.webkit.org/results/15147552). So it seems to me that WebKit2.def is not regenerated. I also can't find anything about WebKit2ExportGenerator or "Generating export definitions" in the log.

Please verify if it is a bug, or I did sth wrong adding the symbol. Thanks.

[Roger Fong]

I think this could be related to that caching issue where VS likes to keep its old version of the export definitions somewhere. 
A clean rebuild will always properly sync up the new export file but it seems an incremental build will not.

[Brent Fulgham]

This is happening because the WebKit2ExportGenerator does not automatically rebuild itself when the WebKit2.def.in file changes.  I'm working on a change to correct that oversight.

As Roger points out, clearing the build directory will resolve the problem because that forces the generator to be rebuild, at which point it will reflect the new definitions.

[Brent Fulgham]

Sorry this took so long, I was a little swamped today.

[Brent Fulgham]

Created attachment 177834 [details]
Patch

[Brent Fulgham]

Created attachment 177835 [details]
Patch

[Tim Horton]

Comment on attachment 177835 [details]
Patch

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

> Source/WebKit2/win/WebKit2ExportGenerator.vcproj:396
> +					CommandLine="REM Do not edit from the Visual Studio IDE! Customize via a $(ProjectName)BuildCmd.cmd file.
if not exist "$(ProjectDir)$(ProjectName)BuildCmd.cmd" exit /b

set CONFIGURATIONBUILDDIR=$(ConfigurationBuildDir)
set CONFIGURATIONNAME=$(ConfigurationName)
set INPUTDIR=$(InputDir)
set INPUTFILENAME=$(InputFileName)
set INPUTPATH=$(InputPath)
set INTDIR=$(IntDir)
set LIBRARYCONFIGSUFFIX=$(LibraryConfigSuffix)
set OUTDIR=$(OutDir)
set PLATFORMNAME=$(PlatformName)
set PROJECTDIR=$(ProjectDir)
set PROJECTFILENAME=$(ProjectFileName)
set PROJECTNAME=$(ProjectName)
set PROJECTPATH=$(ProjectPath)
set SOLUTIONDIR=$(SolutionDir)
set SOLUTIONFILENAME=$(SolutionFileName)
set SOLUTIONNAME=$(SolutionName)
set SOLUTIONPATH=$(SolutionPath)
set TARGETDIR=$(TargetDir)
set TARGETEXT=$(TargetExt)
set TARGETFILENAME=$(TargetFileName)
set TARGETPATH=$(TargetPath)
set WEBKITCONFIGSUFFIX=$(WebKitConfigSuffix)
set WEBKITDLLCONFIGSUFFIX=$(WebKitDLLConfigSuffix)

REM If any of the above variables didn't exist previously and
REM were set to an empty string, set will set the errorlevel to 1,
REM which will cause the project-specific script to think the build
REM has failed. This cmd /c call will clear the errorlevel.
cmd /c

"$(ProjectDir)$(ProjectName)BuildCmd.cmd"
"

What's going on here?

[Roger Fong]

Trying to understand the patch.
Where's the part where you add WebKit2.def.in into the sources in the WebkitExportGenerator vcproj file?

[Brent Fulgham]

Comment on attachment 177835 [details]
Patch

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

>> Source/WebKit2/win/WebKit2ExportGenerator.vcproj:396
>> +					CommandLine="REM Do not edit from the Visual Studio IDE! Customize via a $(ProjectName)BuildCmd.cmd file.
if not exist "$(ProjectDir)$(ProjectName)BuildCmd.cmd" exit /b

set CONFIGURATIONBUILDDIR=$(ConfigurationBuildDir)
set CONFIGURATIONNAME=$(ConfigurationName)
set INPUTDIR=$(InputDir)
set INPUTFILENAME=$(InputFileName)
set INPUTPATH=$(InputPath)
set INTDIR=$(IntDir)
set LIBRARYCONFIGSUFFIX=$(LibraryConfigSuffix)
set OUTDIR=$(OutDir)
set PLATFORMNAME=$(PlatformName)
set PROJECTDIR=$(ProjectDir)
set PROJECTFILENAME=$(ProjectFileName)
set PROJECTNAME=$(ProjectName)
set PROJECTPATH=$(ProjectPath)
set SOLUTIONDIR=$(SolutionDir)
set SOLUTIONFILENAME=$(SolutionFileName)
set SOLUTIONNAME=$(SolutionName)
set SOLUTIONPATH=$(SolutionPath)
set TARGETDIR=$(TargetDir)
set TARGETEXT=$(TargetExt)
set TARGETFILENAME=$(TargetFileName)
set TARGETPATH=$(TargetPath)
set WEBKITCONFIGSUFFIX=$(WebKitConfigSuffix)
set WEBKITDLLCONFIGSUFFIX=$(WebKitDLLConfigSuffix)

REM If any of the above variables didn't exist previously and
REM were set to an empty string, set will set the errorlevel to 1,
REM which will cause the project-specific script to think the build
REM has failed. This cmd /c call will clear the errorlevel.
cmd /c

"$(ProjectDir)$(ProjectName)BuildCmd.cmd"
"
> 
> What's going on here?

That's the same block of code we use for the Pre/Pre-link/Post-build steps. It sets up various environment variables before calling an external shell script. 

Here, I'm changing the WebKit2.def.in file from being excluded from the build to having a build rule that regenerates the WebKit2ExportGenerator.cpp file.

[Brent Fulgham]

(In reply to comment #7)
> Trying to understand the patch.
> Where's the part where you add WebKit2.def.in into the sources in the WebkitExportGenerator vcproj file?

It's that big block of junk that looks like a hex-encoded binary (see Tim's review question).  Previously, the WebKit2.def.in file was marked as "excluded from build", since the pre-build step was responsible for generating the source file.

As Xianzhu pointed out, however, this prevented Visual Studio from recognizing changes to the WebKit2.def.in file from triggering a rebuild of the generator (and therefore the output to the definition file).

[WebKit Review Bot]

Comment on attachment 177835 [details]
Patch

Clearing flags on attachment: 177835

Committed r136792: <http://trac.webkit.org/changeset/136792>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.