Summary: | [CMake][GTK] CMake Error: Could not create named generator Eclipse CDT4 - Ninja | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nikos Andronikos <nikos.andronikos> | ||||||||||||
Component: | Tools / Tests | Assignee: | Nikos Andronikos <nikos.andronikos> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | commit-queue, dbates, mcatanzaro | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | PC | ||||||||||||||
OS: | Linux | ||||||||||||||
Attachments: |
|
Description
Nikos Andronikos
2014-05-05 21:40:53 PDT
Created attachment 230885 [details]
cmake help output
Created attachment 230886 [details]
Patch
(In reply to comment #2) > Created an attachment (id=230886) [details] > Patch I'm still getting my head around the build systems and such. Does the red on win indicate this patch broke the build? Or just that the build is broken (and this could have been caused by some other patch). I'm not currently able to reproduce the win breakage. Comment on attachment 230886 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=230886&action=review One issue here. It looks like the Windows failure is unrelated. > Tools/Scripts/webkitdirs.pm:1818 > - my $willUseNinja = isGtk() && canUseNinja(); > + my $willUseNinja = isGtk() && canUseNinja() && canUseNinjaGenerator(); It looks like you shouldn't change this line, because if Eclipse support is not available it should fall back to the typical ninja generator. (In reply to comment #4) > (From update of attachment 230886 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=230886&action=review > > One issue here. It looks like the Windows failure is unrelated. > > > Tools/Scripts/webkitdirs.pm:1818 > > - my $willUseNinja = isGtk() && canUseNinja(); > > + my $willUseNinja = isGtk() && canUseNinja() && canUseNinjaGenerator(); > > It looks like you shouldn't change this line, because if Eclipse support is not available it should fall back to the typical ninja generator. There are 2 functions: 1. canUseNinjaGenerator 2. canUseEclipseNinjaGenerator canUseNinjaGenerator performs an extra check to ensure that a Ninja generator is installed - so this is extra error checking to make sure that cmake supports the Ninja generator which we know is installed (from the canUseNinja check) Created attachment 238231 [details]
Patch
Comment on attachment 238231 [details]
Patch
Sorry this went unreviewed for so long.
This looks fine to me. If the patch still applies on trunk and works, please set cq? to request commit.
Created attachment 268668 [details]
Patch
Comment on attachment 268668 [details]
Patch
Thanks for the review - I've reapplied the patch to the trunk and updated the style to match the current code.
Comment on attachment 268668 [details]
Patch
Unfortunately what I did not realize the first time I reviewed this, is that we can't use grep, because these functions need to work on Windows without Cygwin. The Windows EWS is passing because (a) it currently has Cygwin installed, and (b) it probably doesn't have Ninja installed. So you should try rewriting this without grep. Sorry. :(
Created attachment 268941 [details]
Patch
Comment on attachment 268941 [details]
Patch
Thanks!
Comment on attachment 268941 [details] Patch Clearing flags on attachment: 268941 Committed r195057: <http://trac.webkit.org/changeset/195057> All reviewed patches have been landed. Closing bug. |