Bug 104136 - WebKit2.def seems not regenerated when WebKit2.def.in is changed
Summary: WebKit2.def seems not regenerated when WebKit2.def.in is changed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-05 10:18 PST by Xianzhu Wang
Modified: 2012-12-05 19:45 PST (History)
4 users (show)

See Also:


Attachments
Patch (16.71 KB, patch)
2012-12-05 14:51 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch (16.19 KB, patch)
2012-12-05 14:53 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Xianzhu Wang 2012-12-05 10:18:01 PST
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.
Comment 1 Roger Fong 2012-12-05 10:39:44 PST
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.
Comment 2 Brent Fulgham 2012-12-05 10:52:27 PST
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.
Comment 3 Brent Fulgham 2012-12-05 14:50:57 PST
Sorry this took so long, I was a little swamped today.
Comment 4 Brent Fulgham 2012-12-05 14:51:11 PST
Created attachment 177834 [details]
Patch
Comment 5 Brent Fulgham 2012-12-05 14:53:31 PST
Created attachment 177835 [details]
Patch
Comment 6 Tim Horton 2012-12-05 16:16:24 PST
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?
Comment 7 Roger Fong 2012-12-05 17:27:35 PST
Trying to understand the patch.
Where's the part where you add WebKit2.def.in into the sources in the WebkitExportGenerator vcproj file?
Comment 8 Brent Fulgham 2012-12-05 19:37:34 PST
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.
Comment 9 Brent Fulgham 2012-12-05 19:44:04 PST
(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).
Comment 10 WebKit Review Bot 2012-12-05 19:45:03 PST
Comment on attachment 177835 [details]
Patch

Clearing flags on attachment: 177835

Committed r136792: <http://trac.webkit.org/changeset/136792>
Comment 11 WebKit Review Bot 2012-12-05 19:45:06 PST
All reviewed patches have been landed.  Closing bug.