Bug 38926 – [chromium] Chromium window build system does not rebuild correctly when enabling/disabling a feature

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Jian Li]

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.

[Eric Seidel (no email)]

Claimed another victim: bug 39054.

[Kent Tamura]

Created attachment 56214 [details]
A workaround patch

[Yaar Schnitman]

(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 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)]

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]

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]

I landed "A workaround patch" as r59646.
I won't close this bug.

[Kent Tamura]

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)]

It should call update-webkit --chromium.  WE need to fix the master.cfg file.

[Eric Seidel (no email)]

Is this still an issue?

[Tony Chang]

(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]

(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]

(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)]

Adding bug 94094 as depending on this (EWS test runs with compile flag enabled are failing on win, gtk and mac ports).

[Nico Weber]

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)]

(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]

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]

Created attachment 170209 [details]
Patch

[Tony Chang]

(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]

Looks good, but I'm no reviewer.

[Eric Seidel (no email)]

Comment on attachment 170209 [details]
Patch

LGTM.

[WebKit Review Bot]

Comment on attachment 170209 [details]
Patch

Clearing flags on attachment: 170209

Committed r132262: <http://trac.webkit.org/changeset/132262>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.