| Summary: | [chromium] Chromium window build system does not rebuild correctly when enabling/disabling a feature | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Jian Li <jianli> | ||||||
| Component: | Tools / Tests | Assignee: | 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
Jian Li
2010-05-11 12:31:43 PDT
Created attachment 56214 [details]
A workaround patch
(In reply to comment #2) > Created an attachment (id=56214) [details] > A workaround patch Smart! LGTM, though I'm not a reviewer. 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?
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? 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? I landed "A workaround patch" as r59646. I won't close this bug. Unfortunately, the workaround didn't work. Windows buildbot doesn't use update-webkit. It run "svn update", then update-webkit-chromium. It should call update-webkit --chromium. WE need to fix the master.cfg file. Is this still an issue? (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. (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?) (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. Adding bug 94094 as depending on this (EWS test runs with compile flag enabled are failing on win, gtk and mac ports). 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) (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. 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. Created attachment 170209 [details]
Patch
(In reply to comment #18) > Created an attachment (id=170209) [details] > Patch I manually verified that VS2010 properly rebuilds after changing features.gypi. Looks good, but I'm no reviewer. Comment on attachment 170209 [details]
Patch
LGTM.
Comment on attachment 170209 [details] Patch Clearing flags on attachment: 170209 Committed r132262: <http://trac.webkit.org/changeset/132262> All reviewed patches have been landed. Closing bug. |