Bug 132599

Summary: [CMake][GTK] CMake Error: Could not create named generator Eclipse CDT4 - Ninja
Product: WebKit Reporter: Nikos Andronikos <nikos.andronikos>
Component: Tools / TestsAssignee: 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 Flags
cmake help output
none
Patch
none
Patch
none
Patch
none
Patch none

Description Nikos Andronikos 2014-05-05 21:40:53 PDT
Running the command line:
Tools/Scripts/build-webkit --gtk

Yields the following error:
CMake Error: Could not create named generator "Eclipse CDT4 - Ninja"
Not searching for unused variables given on the command line.


The problem seems to be related to this bug fix:
https://bugs.webkit.org/show_bug.cgi?id=132190

I have ninja installed and eclipse installed but cmake does not
list the 'Eclipse CDT4 - Ninja' or the 'Ninja' generators (see attachment
cmake_help_output.txt of output for "cmake --help" command).
Comment 1 Nikos Andronikos 2014-05-05 21:41:46 PDT
Created attachment 230885 [details]
cmake help output
Comment 2 Nikos Andronikos 2014-05-05 21:46:54 PDT
Created attachment 230886 [details]
Patch
Comment 3 Nikos Andronikos 2014-05-08 23:28:00 PDT
(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 4 Martin Robinson 2014-09-16 10:16:25 PDT
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.
Comment 5 Nikos Andronikos 2014-09-16 17:28:39 PDT
(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)
Comment 6 Nikos Andronikos 2014-09-16 17:59:34 PDT
Created attachment 238231 [details]
Patch
Comment 7 Michael Catanzaro 2016-01-03 17:12:58 PST
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.
Comment 8 Nikos Andronikos 2016-01-10 19:00:06 PST
Created attachment 268668 [details]
Patch
Comment 9 Nikos Andronikos 2016-01-10 19:02:02 PST
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 10 Michael Catanzaro 2016-01-13 08:28:28 PST
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. :(
Comment 11 Nikos Andronikos 2016-01-13 21:57:01 PST
Created attachment 268941 [details]
Patch
Comment 12 Michael Catanzaro 2016-01-14 07:44:01 PST
Comment on attachment 268941 [details]
Patch

Thanks!
Comment 13 WebKit Commit Bot 2016-01-14 08:32:13 PST
Comment on attachment 268941 [details]
Patch

Clearing flags on attachment: 268941

Committed r195057: <http://trac.webkit.org/changeset/195057>
Comment 14 WebKit Commit Bot 2016-01-14 08:32:15 PST
All reviewed patches have been landed.  Closing bug.