After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 778784 - Allow using Ninja with CMake modules
Allow using Ninja with CMake modules
Status: RESOLVED FIXED
Product: jhbuild
Classification: Infrastructure
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Jhbuild maintainers
Jhbuild QA
Depends on:
Blocks:
 
 
Reported: 2017-02-16 18:45 UTC by Emmanuele Bassi (:ebassi)
Modified: 2018-01-06 22:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Allow telling CMake modules to use Ninja (4.89 KB, patch)
2017-02-16 18:45 UTC, Emmanuele Bassi (:ebassi)
accepted-commit_now Details | Review
apps-3.24: Use Ninja to build libgit2 (854 bytes, patch)
2017-02-16 18:45 UTC, Emmanuele Bassi (:ebassi)
rejected Details | Review
cmake: Use Ninja by default (1.84 KB, patch)
2017-02-16 18:54 UTC, Emmanuele Bassi (:ebassi)
reviewed Details | Review
Make CMake modules use Ninja instead of Make (5.08 KB, patch)
2017-02-16 22:18 UTC, Emmanuele Bassi (:ebassi)
committed Details | Review

Description Emmanuele Bassi (:ebassi) 2017-02-16 18:45:32 UTC
Ninja is on average faster and more efficient than Make, especially on non-Unix platforms.

CMake can generate Ninja build rules; we should allow modules to tell us they prefer using Ninja.
Comment 1 Emmanuele Bassi (:ebassi) 2017-02-16 18:45:37 UTC
Created attachment 345988 [details] [review]
Allow telling CMake modules to use Ninja

Ninja is a replacement for Make; it's faster, simpler, and by default
tries to take advantage of multiple cores and platform-specific
efficient paths.
Comment 2 Emmanuele Bassi (:ebassi) 2017-02-16 18:45:43 UTC
Created attachment 345989 [details] [review]
apps-3.24: Use Ninja to build libgit2
Comment 3 Michael Catanzaro 2017-02-16 18:48:34 UTC
Thanks for working on this.

I think we should make this the default behavior for all modules and just not use the use-ninja flag at all. If there's some module that's really broken with ninja, which I guess is unlikely, then we can use a use-make flag instead.
Comment 4 Emmanuele Bassi (:ebassi) 2017-02-16 18:54:18 UTC
The default is conservative because I wanted more testing to happen.

In general, I think `use-ninja=yes` should be the default, and we should allow `use-ninja=no` as an escape hatch.
Comment 5 Emmanuele Bassi (:ebassi) 2017-02-16 18:54:38 UTC
Created attachment 345991 [details] [review]
cmake: Use Ninja by default

Allow specifying 'use-ninja="no"' as an escape hatch.
Comment 6 Michael Catanzaro 2017-02-16 19:20:36 UTC
Review of attachment 345988 [details] [review]:

OK, squash the third patch into this one please.
Comment 7 Michael Catanzaro 2017-02-16 19:20:50 UTC
Review of attachment 345989 [details] [review]:

So this isn't needed anymore.
Comment 8 Michael Catanzaro 2017-02-16 19:21:28 UTC
Review of attachment 345991 [details] [review]:

Squash into the first patch please.

This also avoids breaking current users of JHBuild, who won't have to upgrade immediately as the modulesets won't use use_ninja yet.
Comment 9 Emmanuele Bassi (:ebassi) 2017-02-16 22:18:07 UTC
Created attachment 346013 [details] [review]
Make CMake modules use Ninja instead of Make

Ninja is a replacement for Make; it's faster, simpler, and by default
tries to take advantage of multiple cores and platform-specific
efficient paths.

We use the Ninja generator by default, but we allow using the attribute
'use-ninja' as an escape hatch for the modules that prefer the Make
generator.
Comment 10 Emmanuele Bassi (:ebassi) 2017-02-16 22:37:27 UTC
Attachment 346013 [details] pushed as cb64cee - Make CMake modules use Ninja instead of Make
Comment 11 Sébastien Wilmet 2017-11-23 14:19:05 UTC
I'm trying to build WebKit, but I have only 4GB of RAM, and it swaps like hell, my computer is unusable during many hours.

I want to pass -j1 to ninja, but it doesn't seem possible with jhbuild, there are no ninjaargs.

With make, I can add this to my jhbuildrc:
module_makeargs['WebKit'] = '-j1'

I don't want to modify the moduleset, I prefer a solution where I can just configure my jhbuildrc. Do you have a solution?
Comment 12 Emmanuele Bassi (:ebassi) 2017-11-23 14:46:56 UTC
(In reply to Sébastien Wilmet from comment #11)
> I'm trying to build WebKit, but I have only 4GB of RAM, and it swaps like
> hell, my computer is unusable during many hours.

You really should not build WebKit with just 4GB of RAM, sorry. It's a lost cause.

> I want to pass -j1 to ninja, but it doesn't seem possible with jhbuild,
> there are no ninjaargs.

A new issue to add a ninjaargs/module_ninjaargs configuration option would be okay.
 
> I don't want to modify the moduleset, I prefer a solution where I can just
> configure my jhbuildrc. Do you have a solution?

Please, open a new issue. This issue was for adding the ability to build with CMake+Ninja modules.
Comment 13 Sébastien Wilmet 2017-11-23 15:00:16 UTC
I reopened this bug because the solution was incomplete IMHO.

Anyway, there is already bug #782320 to add ninjaargs.
Comment 14 BrunoP 2017-12-11 09:19:40 UTC
The provided patch did not work when Ninja is not installed with jhbuild version jhbuild-3.15.92+20171014~ed1297d

Please consider to add "self.ensure_ninja_binary()" before testing self.use_ninja in all functions

diff --git a/cmake.py b/cmake_mod.py
index 71654a4..3bef0b3 100644
--- a/cmake.py
+++ b/cmake_mod.py
@@ -122,8 +122,8 @@ class CMakeModule(MakeModule, DownloadableModule):
     def do_clean(self, buildscript):
         buildscript.set_action(_('Cleaning'), self)
         builddir = self.get_builddir(buildscript)
-        self.ensure_ninja_binary()
         if self.use_ninja:
+            self.ensure_ninja_binary()
             buildscript.execute(self.ninja_binary + ' clean', cwd=builddir, extra_env=self.extra_env)
         else:
             self.make(buildscript, 'clean')
@@ -133,8 +133,8 @@ class CMakeModule(MakeModule, DownloadableModule):
     def do_build(self, buildscript):
         buildscript.set_action(_('Building'), self)
         builddir = self.get_builddir(buildscript)
-        self.ensure_ninja_binary()
         if self.use_ninja:
+            self.ensure_ninja_binary()
             buildscript.execute(self.ninja_binary, cwd=builddir, extra_env=self.extra_env)
         else:
             self.make(buildscript)
@@ -157,8 +157,8 @@ class CMakeModule(MakeModule, DownloadableModule):
         buildscript.set_action(_('Installing'), self)
         builddir = self.get_builddir(buildscript)
         destdir = self.prepare_installroot(buildscript)
-        self.ensure_ninja_binary()
         if self.use_ninja:
+            self.ensure_ninja_binary()
             extra_env = self.extra_env or {}
             extra_env['DESTDIR'] = destdir
             buildscript.execute(self.ninja_binary + ' install', cwd=builddir, extra_env=extra_env)
Comment 15 BrunoP 2017-12-11 09:22:34 UTC
Corrected patch

diff --git a/cmake.py b/cmake_mod.py
index 3ea88b0..3bef0b3 100644
--- a/cmake.py
+++ b/cmake_mod.py
@@ -105,6 +105,7 @@ class CMakeModule(MakeModule, DownloadableModule):
             raise CommandError(_('%s not found') % 'cmake')
         baseargs = '-DCMAKE_INSTALL_PREFIX=%s -DCMAKE_INSTALL_LIBDIR=lib' % prefix
         cmakeargs = self.get_cmakeargs()
+        self.ensure_ninja_binary()
         if self.use_ninja:
             baseargs += ' -G Ninja'
         # CMake on Windows generates VS projects or NMake makefiles by default.
@@ -121,8 +122,8 @@ class CMakeModule(MakeModule, DownloadableModule):
     def do_clean(self, buildscript):
         buildscript.set_action(_('Cleaning'), self)
         builddir = self.get_builddir(buildscript)
-        self.ensure_ninja_binary()
         if self.use_ninja:
+            self.ensure_ninja_binary()
             buildscript.execute(self.ninja_binary + ' clean', cwd=builddir, extra_env=self.extra_env)
         else:
             self.make(buildscript, 'clean')
@@ -132,8 +133,8 @@ class CMakeModule(MakeModule, DownloadableModule):
     def do_build(self, buildscript):
         buildscript.set_action(_('Building'), self)
         builddir = self.get_builddir(buildscript)
-        self.ensure_ninja_binary()
         if self.use_ninja:
+            self.ensure_ninja_binary()
             buildscript.execute(self.ninja_binary, cwd=builddir, extra_env=self.extra_env)
         else:
             self.make(buildscript)
@@ -156,8 +157,8 @@ class CMakeModule(MakeModule, DownloadableModule):
         buildscript.set_action(_('Installing'), self)
         builddir = self.get_builddir(buildscript)
         destdir = self.prepare_installroot(buildscript)
-        self.ensure_ninja_binary()
         if self.use_ninja:
+            self.ensure_ninja_binary()
             extra_env = self.extra_env or {}
             extra_env['DESTDIR'] = destdir
             buildscript.execute(self.ninja_binary + ' install', cwd=builddir, extra_env=extra_env)
Comment 16 Emmanuele Bassi (:ebassi) 2017-12-11 10:01:25 UTC
Thanks!

Could you please:

 - open a new bug, instead of re-purposing closed ones
 - describe your environment, i.e. how you ended up with jhbuild but without ninja
 - attach the patch instead of putting it into a comment
 - generate the patch using `git format-patch` or, better yet, through the `git-bz` tool
Comment 17 Michael Catanzaro 2017-12-11 16:01:41 UTC
(In reply to Emmanuele Bassi (:ebassi) from comment #16)
> Thanks!
> 
> Could you please:
> 
>  - open a new bug, instead of re-purposing closed ones
>  - describe your environment, i.e. how you ended up with jhbuild but without
> ninja

Well it's not a build dependency, so the only way JHBuild can enforce the presence of ninja is to check for it as a sysdep. I've broken this in the past myself.

Let's move discussion to bug #791476.
Comment 18 Sébastien Wilmet 2018-01-06 21:10:08 UTC
(In reply to Emmanuele Bassi (:ebassi) from comment #12)
> You really should not build WebKit with just 4GB of RAM, sorry. It's a lost
> cause.

FYI, it worked well with -j1, run during the night (I don't contribute to WebKit, it was just to install a development version).
Comment 19 Michael Catanzaro 2018-01-06 22:17:17 UTC
(In reply to Sébastien Wilmet from comment #18)
> (In reply to Emmanuele Bassi (:ebassi) from comment #12)
> > You really should not build WebKit with just 4GB of RAM, sorry. It's a lost
> > cause.
> 
> FYI, it worked well with -j1, run during the night (I don't contribute to
> WebKit, it was just to install a development version).

I think 2 GB should be enough, at least for release builds. Less than that would be pretty iffy.