Bug 38926

Summary: [chromium] Chromium window build system does not rebuild correctly when enabling/disabling a feature
Product: WebKit Reporter: Jian Li <jianli>
Component: Tools / TestsAssignee: Tony Chang <tony>
Status: RESOLVED FIXED    
Severity: Normal CC: bradnelson, bruno.abinader, dglazkov, eric, mark, thakis, tkent, tony, tonyg, webkit.review.bot, yaar
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
Bug Depends on:    
Bug Blocks: 39054    
Attachments:
Description Flags
A workaround patch
none
Patch none

Jian Li
Reported 2010-05-11 12:31:43 PDT
When a feature is turned on, Chromium window build bot cannot build it successfully. The clean build has been forced. We need to find a solution for this.
Attachments
A workaround patch (1.89 KB, patch)
2010-05-17 00:08 PDT, Kent Tamura 		no flags
DetailsFormatted DiffDiff
Patch (2.10 KB, patch)
2012-10-23 12:53 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2010-05-16 10:22:32 PDT
Claimed another victim: bug 39054.
Kent Tamura
Comment 2 2010-05-17 00:08:30 PDT
Created attachment 56214 [details] A workaround patch
Yaar Schnitman
Comment 3 2010-05-17 09:23:35 PDT
(In reply to comment #2) > Created an attachment (id=56214) [details] > A workaround patch Smart! LGTM, though I'm not a reviewer.
Eric Seidel (no email)
Comment 4 2010-05-17 10:39:56 PDT
Comment on attachment 56214 [details] A workaround patch I think that either the log or a comment should mention that this is a hack, and what bug we're working around. I'm told this is a bug in visual studio?
Eric Seidel (no email)
Comment 5 2010-05-17 10:40:51 PDT
Is there a better fix to be done? Or is this a permanent work-around? The comments in the code should make that clear. If this is permanent workaround, is there a way to add this hack into gyp directly?
Kent Tamura
Comment 6 2010-05-17 17:12:55 PDT
I think my patch is a temporal workaround. I'll commit the patch with an additional comment. I'm not sure what is the real fix. Can we solve this by adding dependencies in .gyp files? Does GYP has no capability solve this? Should we remove build directories in GYP, gyp_webkit or gyp_chromium?
Kent Tamura
Comment 7 2010-05-17 21:06:07 PDT
I landed "A workaround patch" as r59646. I won't close this bug.
Kent Tamura
Comment 8 2010-05-17 22:07:34 PDT
Unfortunately, the workaround didn't work. Windows buildbot doesn't use update-webkit. It run "svn update", then update-webkit-chromium.
Eric Seidel (no email)
Comment 9 2010-05-17 22:53:55 PDT
It should call update-webkit --chromium. WE need to fix the master.cfg file.
Eric Seidel (no email)
Comment 10 2012-05-28 03:24:32 PDT
Is this still an issue?
Tony Chang
Comment 11 2012-05-29 10:22:12 PDT
(In reply to comment #10) > Is this still an issue? Kent's workaround is still checked in and still works around the problem. I think msbuild (part of VS2010) doesn't have this problem, but we're not using VS2010 on build.webkit.org yet (we just started the rollout on the build.chromium.org bots). ninja doesn't have this problem either. Once the bots switch to VS2010 or ninja, we can test to verify that rebuilds work properly, remove Kent's work around and close this bug.
Nico Weber
Comment 12 2012-05-29 10:23:58 PDT
(In reply to comment #11) > Once the bots switch to VS2010 or ninja, we can test to verify that rebuilds work properly, remove Kent's work around and close this bug. ninja/win won't be ready for bot use for a while; I think it's more likely we'll switch the windows bots to vs2010 first. (ist that blocked on anything?)
Tony Chang
Comment 13 2012-05-29 11:31:16 PDT
(In reply to comment #12) > (In reply to comment #11) > > Once the bots switch to VS2010 or ninja, we can test to verify that rebuilds work properly, remove Kent's work around and close this bug. > > ninja/win won't be ready for bot use for a while; I think it's more likely we'll switch the windows bots to vs2010 first. (ist that blocked on anything?) We can probably switch the bot to VS2010 now. Someone just has to do the work.
Bruno Abinader (history only)
Comment 14 2012-08-24 06:11:21 PDT
Adding bug 94094 as depending on this (EWS test runs with compile flag enabled are failing on win, gtk and mac ports).
Nico Weber
Comment 15 2012-10-22 06:22:47 PDT
Bruno: I think win is Apple's windows port, gtk is the gtk port, and mac is Apple's mac port. Neither of those use chromium's build system. The chromium bots all start with 'cr-'. I think you can remove this dependency again. Since I'm posting this anyway: We deprecated msvc2008 in chromium and don't have any bots for it anymore upstream. If the webkit chromium bots still use msvc2008, you should move away from that (upstream uses both msvs2010 and ninja now)
Bruno Abinader (history only)
Comment 16 2012-10-22 06:50:01 PDT
(In reply to comment #15) > Bruno: I think win is Apple's windows port, gtk is the gtk port, and mac is Apple's mac port. Neither of those use chromium's build system. The chromium bots all start with 'cr-'. I think you can remove this dependency again. > > Since I'm posting this anyway: We deprecated msvc2008 in chromium and don't have any bots for it anymore upstream. If the webkit chromium bots still use msvc2008, you should move away from that (upstream uses both msvs2010 and ninja now) Thank you for the clarification, Nico! I'm going to update the bug depedencies accordingly.
Tony Chang
Comment 17 2012-10-22 11:07:30 PDT
The build.webkit.org Chromium Win bot has been using VS2010 for a few months now. I think it's safe to remove this hack.
Tony Chang
Comment 18 2012-10-23 12:53:37 PDT
Created attachment 170209 [details] Patch
Tony Chang
Comment 19 2012-10-23 13:23:05 PDT
(In reply to comment #18) > Created an attachment (id=170209) [details] > Patch I manually verified that VS2010 properly rebuilds after changing features.gypi.
Nico Weber
Comment 20 2012-10-23 13:34:17 PDT
Looks good, but I'm no reviewer.
Eric Seidel (no email)
Comment 21 2012-10-23 13:42:15 PDT
Comment on attachment 170209 [details] Patch LGTM.
WebKit Review Bot
Comment 22 2012-10-23 14:10:08 PDT
Comment on attachment 170209 [details] Patch Clearing flags on attachment: 170209 Committed r132262: <http://trac.webkit.org/changeset/132262>
WebKit Review Bot
Comment 23 2012-10-23 14:10:13 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.