* [Buildroot] [PATCH 2/2] Update documentation of CMake infrastructure
2017-01-06 22:37 [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option Cédric Marie
@ 2017-01-06 22:37 ` Cédric Marie
2017-01-25 3:27 ` Thomas Petazzoni
2017-01-21 22:25 ` [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option Romain Naour
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Cédric Marie @ 2017-01-06 22:37 UTC (permalink / raw)
To: buildroot
Describe BR2_CMAKE_USE_NINJA_BACKEND option, and update all the
variables that have changed with this evolution.
Replace LIBFOO_INSTALL_STAGING_OPTS and LIBFOO_INSTALL_TARGET_OPTS with
LIBFOO_INSTALL_STAGING_ENV, LIBFOO_INSTALL_TARGET_ENV and
LIBFOO_INSTALL_OPTS.
Signed-off-by: C?dric Marie <cedric.marie@openmailbox.org>
---
docs/manual/adding-packages-cmake.txt | 37 +++++++++++++++++++++++------------
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/docs/manual/adding-packages-cmake.txt b/docs/manual/adding-packages-cmake.txt
index 6ccf390..06331dd 100644
--- a/docs/manual/adding-packages-cmake.txt
+++ b/docs/manual/adding-packages-cmake.txt
@@ -117,25 +117,38 @@ typical packages will therefore only use a few of them.
the given package, for one reason or another. By default, set to
+$(MAKE)+. If parallel building is not supported by the package,
then it should be set to +LIBFOO_MAKE=$(MAKE1)+.
+ When +BR2_CMAKE_USE_NINJA_BACKEND+ is enabled, the default value is
+ +$(HOST_DIR)/usr/bin/ninja+. If parallel building is not supported by
+ the package, then it should be set to
+ +LIBFOO_MAKE=$(HOST_DIR)/usr/bin/ninja -j1+.
* +LIBFOO_MAKE_ENV+, to specify additional environment variables to
pass to make in the build step. These are passed before the +make+
command. By default, empty.
* +LIBFOO_MAKE_OPTS+, to specify additional variables to pass to make
- in the build step. These are passed after the +make+ command. By
- default, empty.
-
-* +LIBFOO_INSTALL_STAGING_OPTS+ contains the make options used to
- install the package to the staging directory. By default, the value
- is +DESTDIR=$(STAGING_DIR) install+, which is correct for most
- CMake packages. It is still possible to override it.
-
-* +LIBFOO_INSTALL_TARGET_OPTS+ contains the make options used to
+ in the build step. These are passed after the +make+ or +ninja+
+ command. If +BR2_CMAKE_USE_NINJA_BACKEND+ is enabled and +VERBOSE+ is
+ set, the default value is -v. Otherwise it is empty. With Ninja
+ backend, the verbose mode is not set in the configure step. Instead,
+ ninja -v option must be used in the following steps.
+
+* +LIBFOO_INSTALL_STAGING_ENV+ contains environment variables to pass
+ to make or ninja, to install the package to the staging directory. By
+ default, the value is +DESTDIR=$(STAGING_DIR)+, which is correct for
+ most CMake packages. It is still possible to override it.
+
+* +LIBFOO_INSTALL_TARGET_ENV+ contains environment variables to pass
+ to make or ninja, to install the package to the target directory. By
+ default, the value is +DESTDIR=$(TARGET_DIR)+. The default value is
+ correct for most CMake packages, but it is still possible to override
+ it if needed.
+
+* +LIBFOO_INSTALL_OPTS+ contains the make or ninja options used to
install the package to the target directory. By default, the value
- is +DESTDIR=$(TARGET_DIR) install+. The default value is correct
- for most CMake packages, but it is still possible to override it if
- needed.
+ is +install/fast+ for make and +install+ for ninja. The default value
+ is correct for most CMake packages, but it is still possible to
+ override it if needed.
With the CMake infrastructure, all the steps required to build and
install the packages are already defined, and they generally work well
--
2.9.3
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Buildroot] [PATCH 2/2] Update documentation of CMake infrastructure
2017-01-06 22:37 ` [Buildroot] [PATCH 2/2] Update documentation of CMake infrastructure Cédric Marie
@ 2017-01-25 3:27 ` Thomas Petazzoni
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Petazzoni @ 2017-01-25 3:27 UTC (permalink / raw)
To: buildroot
Hello,
On Fri, 6 Jan 2017 23:37:48 +0100, C?dric Marie wrote:
> diff --git a/docs/manual/adding-packages-cmake.txt b/docs/manual/adding-packages-cmake.txt
> index 6ccf390..06331dd 100644
> --- a/docs/manual/adding-packages-cmake.txt
> +++ b/docs/manual/adding-packages-cmake.txt
> @@ -117,25 +117,38 @@ typical packages will therefore only use a few of them.
> the given package, for one reason or another. By default, set to
> +$(MAKE)+. If parallel building is not supported by the package,
> then it should be set to +LIBFOO_MAKE=$(MAKE1)+.
> + When +BR2_CMAKE_USE_NINJA_BACKEND+ is enabled, the default value is
> + +$(HOST_DIR)/usr/bin/ninja+. If parallel building is not supported by
> + the package, then it should be set to
> + +LIBFOO_MAKE=$(HOST_DIR)/usr/bin/ninja -j1+.
That's a bit annoying, because it means packages would have to do:
ifeq ($(BR2_CMAKE_USE_NINJA_BACKEND),y)
LIBFOO_MAKE = $(HOST_DIR)/usr/bin/ninja -j1
else
LIBFOO_MAKE = $(MAKE1)
endif
So perhaps in this case we instead need a variable like
LIBFOO_SUPPORTS_PARALLEL_BUILD so that the infrastructure can do the
right thing.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option
2017-01-06 22:37 [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option Cédric Marie
2017-01-06 22:37 ` [Buildroot] [PATCH 2/2] Update documentation of CMake infrastructure Cédric Marie
@ 2017-01-21 22:25 ` Romain Naour
2017-01-23 13:39 ` Cédric Marie
2017-01-25 1:37 ` Thomas Petazzoni
2017-07-11 11:56 ` Thomas Petazzoni
3 siblings, 1 reply; 16+ messages in thread
From: Romain Naour @ 2017-01-21 22:25 UTC (permalink / raw)
To: buildroot
Hi C?dric,
Le 06/01/2017 ? 23:37, C?dric Marie a ?crit :
> CMake provides several backends. The default one is Make, but Ninja is
> also supported. Ninja is a small build system with a focus on speed.
>
> If the new option BR2_CMAKE_USE_NINJA_BACKEND is enabled, CMake will
> use Ninja backend.
>
> In CMake package infrastructure, when this option is set:
> - Add host-ninja dependency
> - Add "-G Ninja" option in CMake configure step
> - Use ninja command instead of make ($(MAKE))
>
> Most of make arguments are compatible with ninja command. But there are
> a few differences:
> - Environment variables such as DESTDIR cannot be given in arguments.
> Instead they must be set before the command (which is compatible with
> make)
> - CMake does not handle VERBOSE variable with Ninja backend. Instead,
> we must add -v option when VERBOSE is set.
> - install/fast target is specific to make backend. With ninja backend,
> install target must be used (and it will not try to compile as make
> backend does).
>
> Tested on following packages: cannelloni, graphite2, libcuefile,
> libubox, rabbitmq-c, ubus.
There are currently 146 packages using CMake in Buildroot, I'm not sure all of
them support ninja backend yet.
I would suggest to enable Ninja backend package by package when it has been tested.
>
> Signed-off-by: C?dric Marie <cedric.marie@openmailbox.org>
> ---
> Config.in | 6 ++++++
> package/pkg-cmake.mk | 26 ++++++++++++++++++++------
> 2 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/Config.in b/Config.in
> index ccd777e..26ebcf6 100644
> --- a/Config.in
> +++ b/Config.in
> @@ -656,6 +656,12 @@ config BR2_SHARED_STATIC_LIBS
>
> endchoice
>
> +config BR2_CMAKE_USE_NINJA_BACKEND
> + bool "Compile CMake packages with Ninja backend"
> + help
Indent with one tab.
> + CMake provides several backends. The default one is Make, but
> + Ninja is also supported. Ninja is a small build system with a
> + focus on speed.
Indent with one tab and 2 spaces.
>
> config BR2_PACKAGE_OVERRIDE_FILE
> string "location of a package override file"
> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
> index 4e0e838..83bf79e 100644
> --- a/package/pkg-cmake.mk
> +++ b/package/pkg-cmake.mk
> @@ -53,24 +53,35 @@ define inner-cmake-package
>
> $(2)_CONF_ENV ?=
> $(2)_CONF_OPTS ?=
> +$(2)_INSTALL_STAGING_ENV ?= DESTDIR=$$(STAGING_DIR)
> +$(2)_INSTALL_TARGET_ENV ?= DESTDIR=$$(TARGET_DIR)
> +
> +ifeq ($(BR2_CMAKE_USE_NINJA_BACKEND),y)
> +$(2)_MAKE ?= $$(HOST_DIR)/usr/bin/ninja
> +$(2)_MAKE_ENV ?=
> +$(2)_MAKE_OPTS ?= $(if $(VERBOSE),-v)
> +$(2)_INSTALL_OPTS ?= install
> +else
> $(2)_MAKE ?= $$(MAKE)
> $(2)_MAKE_ENV ?=
> $(2)_MAKE_OPTS ?=
> -$(2)_INSTALL_OPTS ?= install
> -$(2)_INSTALL_STAGING_OPTS ?= DESTDIR=$$(STAGING_DIR) install/fast
> -$(2)_INSTALL_TARGET_OPTS ?= DESTDIR=$$(TARGET_DIR) install/fast
> +$(2)_INSTALL_OPTS ?= install/fast
I think these changes should be in a preparatory patch before adding Ninja
backend support.
Best regards,
Romain
> +endif
>
> $(2)_SRCDIR = $$($(2)_DIR)/$$($(2)_SUBDIR)
>
> $(3)_SUPPORTS_IN_SOURCE_BUILD ?= YES
>
> -
> ifeq ($$($(3)_SUPPORTS_IN_SOURCE_BUILD),YES)
> $(2)_BUILDDIR = $$($(2)_SRCDIR)
> else
> $(2)_BUILDDIR = $$($(2)_SRCDIR)/buildroot-build
> endif
>
> +ifeq ($(BR2_CMAKE_USE_NINJA_BACKEND),y)
> +$(2)_CONF_OPTS += -G Ninja
> +endif
> +
> #
> # Configure step. Only define it if not already defined by the package
> # .mk file. And take care of the differences between host and target
> @@ -146,6 +157,9 @@ endif
> $(2)_DEPENDENCIES += host-pkgconf
>
> $(2)_DEPENDENCIES += $(BR2_CMAKE_HOST_DEPENDENCY)
> +ifeq ($(BR2_CMAKE_USE_NINJA_BACKEND),y)
> +$(2)_DEPENDENCIES += host-ninja
> +endif
>
> #
> # Build step. Only define it if not already defined by the package .mk
> @@ -179,7 +193,7 @@ endif
> #
> ifndef $(2)_INSTALL_STAGING_CMDS
> define $(2)_INSTALL_STAGING_CMDS
> - $$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPTS) $$($$(PKG)_INSTALL_STAGING_OPTS) -C $$($$(PKG)_BUILDDIR)
> + $$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_INSTALL_STAGING_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPTS) $$($$(PKG)_INSTALL_OPTS) -C $$($$(PKG)_BUILDDIR)
> endef
> endif
>
> @@ -189,7 +203,7 @@ endif
> #
> ifndef $(2)_INSTALL_TARGET_CMDS
> define $(2)_INSTALL_TARGET_CMDS
> - $$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPTS) $$($$(PKG)_INSTALL_TARGET_OPTS) -C $$($$(PKG)_BUILDDIR)
> + $$(TARGET_MAKE_ENV) $$($$(PKG)_MAKE_ENV) $$($$(PKG)_INSTALL_TARGET_ENV) $$($$(PKG)_MAKE) $$($$(PKG)_MAKE_OPTS) $$($$(PKG)_INSTALL_OPTS) -C $$($$(PKG)_BUILDDIR)
> endef
> endif
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option
2017-01-21 22:25 ` [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option Romain Naour
@ 2017-01-23 13:39 ` Cédric Marie
2017-01-24 21:48 ` Romain Naour
0 siblings, 1 reply; 16+ messages in thread
From: Cédric Marie @ 2017-01-23 13:39 UTC (permalink / raw)
To: buildroot
Hi,
Le 2017-01-21 23:25, Romain Naour a ?crit?:
> There are currently 146 packages using CMake in Buildroot, I'm not sure
> all of
> them support ninja backend yet.
I don't think there is anything specific to a package that can make it
support ninja backend or not.
CMake provides several backends, and the result is expected to be the
same whatever the backend. If not, this is a bug in CMake.
> I would suggest to enable Ninja backend package by package when it has
> been tested.
What do you mean exactly "enable Ninja backend package by package"?
- Add an option for each package?
- Switch to ninja backend if supported by the package, without option
(i.e. force ninja backend)?
- Keep a global option - the one in my patch - but only apply it on
packages that will define a specific package variable to say "OK I can
do it"?
My point of view is that we should keep a global option - that could be
described as experimental - and never enable it in an official
defconfig.
If someone enables it, and find a package that does not compile with
ninja backend, he can either disable the option (he knows that it is
experimental), or/and send a bug to either CMake or the package
maintainer.
Moreover, I think we can't just say "this package is ninja compliant".
Maybe if we change a single option in configure step, it will not
compile anymore.
Let me know what you're expecting exactly...
> Indent with one tab.
> Indent with one tab and 2 spaces.
OK, sorry for that, didn't pay enough attention.
> I think these changes should be in a preparatory patch before adding
> Ninja
> backend support.
Sorry I don't understand what you mean with "preparatory patch".
Regards,
--
C?dric
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option
2017-01-23 13:39 ` Cédric Marie
@ 2017-01-24 21:48 ` Romain Naour
2017-01-25 1:27 ` Thomas Petazzoni
0 siblings, 1 reply; 16+ messages in thread
From: Romain Naour @ 2017-01-24 21:48 UTC (permalink / raw)
To: buildroot
Hi C?dric,
Le 23/01/2017 ? 14:39, C?dric Marie a ?crit :
> Hi,
>
> Le 2017-01-21 23:25, Romain Naour a ?crit :
>> There are currently 146 packages using CMake in Buildroot, I'm not sure all of
>> them support ninja backend yet.
>
> I don't think there is anything specific to a package that can make it support
> ninja backend or not.
> CMake provides several backends, and the result is expected to be the same
> whatever the backend. If not, this is a bug in CMake.
Ok.
>
>> I would suggest to enable Ninja backend package by package when it has
>> been tested.
>
> What do you mean exactly "enable Ninja backend package by package"?
> - Add an option for each package?
no
> - Switch to ninja backend if supported by the package, without option (i.e.
> force ninja backend)?
no
> - Keep a global option - the one in my patch - but only apply it on packages
> that will define a specific package variable to say "OK I can do it"?
Yes, something like that but It's just a proposal...
<package>_SUPPORT_NINJA_BACKEND = YES in the .mk
(with default to N0)
>
> My point of view is that we should keep a global option - that could be
> described as experimental - and never enable it in an official defconfig.
> If someone enables it, and find a package that does not compile with ninja
> backend, he can either disable the option (he knows that it is experimental),
> or/and send a bug to either CMake or the package maintainer.
Keep in mind that this new option will certainly be enabled in the generated
defconfigs for the autobuilders. So it may break several packages quickly.
For example, the openpowerlink package break with Ninja backend but I think it's
a bug in the CMakeLists.txt. I don't think openpowerlink maintainers ever used
Ninja and me neither until now. I'll take a look.
>
> Moreover, I think we can't just say "this package is ninja compliant". Maybe if
> we change a single option in configure step, it will not compile anymore.
Maybe something like this (untested):
ifeq ($(BR2_CMAKE_USE_NINJA_BACKEND),y)
ifeq ($$($(2)_SUPPORT_NINJA_BACKEND),YES)
$(2)_CONF_OPTS += -G Ninja
$(2)_DEPENDENCIES += host-ninja
endif
endif
It's really a proposal, If you're not convinced let's see other comments/reviews.
Also, did you do some time measurements with/without Ninja ?
Here are some results on my host:
make menuconfig (enable/disable the Ninja option)
make <package> (first build from scratch)
make <package>-dirclean
time make <package>-build
w/ Ninja
cannelloni
real 0m10.009s
user 0m8.771s
sys 0m0.885s
bullet
real 0m43.750s
user 1m4.464s
sys 0m4.356s
w/ Make
cannelloni
real 0m11.478s
user 0m9.282s
sys 0m1.127s
bullet
real 0m45.234s
user 1m4.213s
sys 0m5.501s
So, yes Ninja is slightly faster than Make.
It would be great if you can add some test results in the commit log.
>
> Let me know what you're expecting exactly...
>
>> Indent with one tab.
>> Indent with one tab and 2 spaces.
>
> OK, sorry for that, didn't pay enough attention.
No problem.
>
>
>> I think these changes should be in a preparatory patch before adding Ninja
>> backend support.
>
> Sorry I don't understand what you mean with "preparatory patch".
Sorry, I meant you can do a first patch with these changes in pkg-cmake.mk
(refactoring) and add the Ninja in a second patch.
I suggest to do this since it impact the cmake with Make backend build.
Best regards,
Romain
>
> Regards,
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option
2017-01-24 21:48 ` Romain Naour
@ 2017-01-25 1:27 ` Thomas Petazzoni
2017-01-26 17:27 ` Cédric Marie
0 siblings, 1 reply; 16+ messages in thread
From: Thomas Petazzoni @ 2017-01-25 1:27 UTC (permalink / raw)
To: buildroot
Hello,
On Tue, 24 Jan 2017 22:48:08 +0100, Romain Naour wrote:
> > - Keep a global option - the one in my patch - but only apply it on packages
> > that will define a specific package variable to say "OK I can do it"?
>
> Yes, something like that but It's just a proposal...
>
> <package>_SUPPORT_NINJA_BACKEND = YES in the .mk
> (with default to N0)
But then that's a bit annoying because we would have to explicitly
set this option to "YES" on all packages that support the Ninja backend
(most likely the majority).
I think it would make more sense to default the other way around, i.e
default to YES, and set it to NO on the few packages that do not
properly support the Ninja backend.
Or maybe better: do not introduce a per-package option for the moment,
have only the global one, see in practice how many packages work /
don't work and decide if we need a per-package option, and what default
value it should have.
> w/ Ninja
>
> cannelloni
> real 0m10.009s
> user 0m8.771s
> sys 0m0.885s
>
> bullet
> real 0m43.750s
> user 1m4.464s
> sys 0m4.356s
>
> w/ Make
>
> cannelloni
> real 0m11.478s
> user 0m9.282s
> sys 0m1.127s
>
> bullet
> real 0m45.234s
> user 1m4.213s
> sys 0m5.501s
So we're saving between 1 and 2 seconds of build time, while host-ninja
requires building host-python or host-python3 ?
Seeing those numbers, the whole thing seems really pointless to me.
You're saving 1-2 seconds of build time, but you've got a full Python
interpreter to build.
Would it be possible to get numbers on the overall build time showing
what Ninja is improving?
> >> I think these changes should be in a preparatory patch before adding Ninja
> >> backend support.
> >
> > Sorry I don't understand what you mean with "preparatory patch".
>
> Sorry, I meant you can do a first patch with these changes in pkg-cmake.mk
> (refactoring) and add the Ninja in a second patch.
> I suggest to do this since it impact the cmake with Make backend build.
I looked again at the patch, and I don't really see where it can be
split in two parts. Which changes do you see as "preparatory" ?
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option
2017-01-25 1:27 ` Thomas Petazzoni
@ 2017-01-26 17:27 ` Cédric Marie
2017-01-30 9:23 ` Thomas Petazzoni
0 siblings, 1 reply; 16+ messages in thread
From: Cédric Marie @ 2017-01-26 17:27 UTC (permalink / raw)
To: buildroot
Hi Romain, Thomas, and all,
Le 2017-01-24 22:48, Romain Naour a ?crit :
> Yes, something like that but It's just a proposal...
>
> <package>_SUPPORT_NINJA_BACKEND = YES in the .mk
> (with default to N0)
>
> ifeq ($(BR2_CMAKE_USE_NINJA_BACKEND),y)
> ifeq ($$($(2)_SUPPORT_NINJA_BACKEND),YES)
> $(2)_CONF_OPTS += -G Ninja
> $(2)_DEPENDENCIES += host-ninja
> endif
> endif
Yes, that could be a good solution to use ninja whenever possible and
keep other few packages compiling with make backend.
> So, yes Ninja is slightly faster than Make.
> It would be great if you can add some test results in the commit log.
OK, I can do that.
Le 2017-01-25 02:27, Thomas Petazzoni a ?crit?:
> But then that's a bit annoying because we would have to explicitly
> set this option to "YES" on all packages that support the Ninja backend
> (most likely the majority).
>
> I think it would make more sense to default the other way around, i.e
> default to YES, and set it to NO on the few packages that do not
> properly support the Ninja backend.
>
> Or maybe better: do not introduce a per-package option for the moment,
> have only the global one, see in practice how many packages work /
> don't work and decide if we need a per-package option, and what default
> value it should have.
OK, but it seems that Romain has already noticed a problem with
openpowerlink, so it is likely that this option will be necessary - and
hopefully default to YES (supported).
> So we're saving between 1 and 2 seconds of build time, while host-ninja
> requires building host-python or host-python3 ?
>
> Seeing those numbers, the whole thing seems really pointless to me.
> You're saving 1-2 seconds of build time, but you've got a full Python
> interpreter to build.
>
> Would it be possible to get numbers on the overall build time showing
> what Ninja is improving?
Yes, you're right. Unless we use python and ninja from the host if
available, instead of compiling them, but this is not the right solution
I suppose (although this is done for CMake!...).
I agree that a few seconds are not much!... And I agree that we are
loosing much more time to compile host packages.
So, why did I want to introduce this option?
Because I'm not using Buildroot in one-shot mode. I'm developping in a
Buildroot environment. So I often make foo-rebuild my package, and I'm
interested in saving time for each rebuild.
I know this is not what Buildroot is designed for. (Yet, there are a few
things that seem to be present for dev mode (rsync with OVERRIDE_SRCDIR
for example), so this is not "pure one-shot" either.)
In conclusion, I would say that this option is useful to me - not much,
just for a few seconds - but maybe makes no sense for buildroot
upstream, since it increases build time.
It's up to you. I won't be offended if the patch is refused. I don't
want to introduce something that will break Buildroot philosophy.
Regards,
--
C?dric
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option
2017-01-26 17:27 ` Cédric Marie
@ 2017-01-30 9:23 ` Thomas Petazzoni
2017-02-01 17:01 ` Cédric Marie
0 siblings, 1 reply; 16+ messages in thread
From: Thomas Petazzoni @ 2017-01-30 9:23 UTC (permalink / raw)
To: buildroot
Hello,
On Thu, 26 Jan 2017 18:27:36 +0100, C?dric Marie wrote:
> Le 2017-01-25 02:27, Thomas Petazzoni a ?crit?:
> > But then that's a bit annoying because we would have to explicitly
> > set this option to "YES" on all packages that support the Ninja backend
> > (most likely the majority).
> >
> > I think it would make more sense to default the other way around, i.e
> > default to YES, and set it to NO on the few packages that do not
> > properly support the Ninja backend.
> >
> > Or maybe better: do not introduce a per-package option for the moment,
> > have only the global one, see in practice how many packages work /
> > don't work and decide if we need a per-package option, and what default
> > value it should have.
>
> OK, but it seems that Romain has already noticed a problem with
> openpowerlink, so it is likely that this option will be necessary - and
> hopefully default to YES (supported).
Indeed.
> So, why did I want to introduce this option?
>
> Because I'm not using Buildroot in one-shot mode. I'm developping in a
> Buildroot environment. So I often make foo-rebuild my package, and I'm
> interested in saving time for each rebuild.
OK, understood. But do you have numbers showing that "make foo-rebuild"
is actually faster with Ninja ?
In the tests done by Romain, it's only saving 1-2 seconds on the
total build of a package. So I would suspect that the savings on a
partial build are even smaller.
Even if your CMake packages are private, can you give us some numbers
that show the time benefits of Ninja?
> I know this is not what Buildroot is designed for. (Yet, there are a few
> things that seem to be present for dev mode (rsync with OVERRIDE_SRCDIR
> for example), so this is not "pure one-shot" either.)
Well, if we do have features such as OVERRIDE_SRCDIR, it's precisely to
make Buildroot usable/useful also during the development. So if there
are things we can improve in this area, it's good to have.
Thanks,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option
2017-01-30 9:23 ` Thomas Petazzoni
@ 2017-02-01 17:01 ` Cédric Marie
2017-02-01 20:12 ` Thomas Petazzoni
0 siblings, 1 reply; 16+ messages in thread
From: Cédric Marie @ 2017-02-01 17:01 UTC (permalink / raw)
To: buildroot
Hi,
Le 2017-01-30 10:23, Thomas Petazzoni a ?crit?:
> OK, understood. But do you have numbers showing that "make foo-rebuild"
> is actually faster with Ninja ?
>
> In the tests done by Romain, it's only saving 1-2 seconds on the
> total build of a package. So I would suspect that the savings on a
> partial build are even smaller.
>
> Even if your CMake packages are private, can you give us some numbers
> that show the time benefits of Ninja?
You're right, it is a private package, that's why I had to test on other
packages I'm not particularly using, to demonstrate.
The benefit is rather small for my package too.
Make:
real 1m1.517s
user 2m23.996s
sys 0m36.197s
Ninja:
real 0m56.312s
user 2m19.078s
sys 0m29.119s
NB: It includes the whole "time make foo-rebuild" command, because time
output is "strange" when inserted in pkg-cmake.mk (... time
$$($$(PKG)_MAKE) ...), don't know why...
Looks like:
130.50user 19.88system 0:42.32elapsed 355%CPU (0avgtext+0avgdata
24680maxresident)k
0inputs+50416outputs (0major+7602195minor)pagefaults 0swaps
To be honest, when I started to add ninja possibility, I expected much
bigger savings :)
Yet I have the option, so even if the benefit is small, I keep using it.
But in the end, I don't know whether it's worth pushing upstream or
not...
Besides Buildroot, I also switched another package from CMake/Make to
Meson (based on Ninja), and the benefit was much more interesting -
although I have not kept any measure to give here.
I expected the improvement to be caused by Ninja, not Meson. But it
seems that Meson makes the difference, rather than Make vs Ninja.
--
C?dric
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option
2017-02-01 17:01 ` Cédric Marie
@ 2017-02-01 20:12 ` Thomas Petazzoni
2017-02-03 10:44 ` Cédric Marie
0 siblings, 1 reply; 16+ messages in thread
From: Thomas Petazzoni @ 2017-02-01 20:12 UTC (permalink / raw)
To: buildroot
Hello,
On Wed, 01 Feb 2017 18:01:31 +0100, C?dric Marie wrote:
> You're right, it is a private package, that's why I had to test on other
> packages I'm not particularly using, to demonstrate.
>
> The benefit is rather small for my package too.
>
> Make:
> real 1m1.517s
> user 2m23.996s
> sys 0m36.197s
>
> Ninja:
> real 0m56.312s
> user 2m19.078s
> sys 0m29.119s
Indeed the benefit is quite small here.
> NB: It includes the whole "time make foo-rebuild" command, because time
> output is "strange" when inserted in pkg-cmake.mk (... time
> $$($$(PKG)_MAKE) ...), don't know why...
> Looks like:
> 130.50user 19.88system 0:42.32elapsed 355%CPU (0avgtext+0avgdata
> 24680maxresident)k
> 0inputs+50416outputs (0major+7602195minor)pagefaults 0swaps
In the first case you're using the shell built-in "time" program, in
the later you're using the separate /usr/bin/time program. Try:
/usr/bin/time make foo-rebuild
> To be honest, when I started to add ninja possibility, I expected much
> bigger savings :)
> Yet I have the option, so even if the benefit is small, I keep using it.
>
> But in the end, I don't know whether it's worth pushing upstream or
> not...
I'm indeed not sure it's really worth the effort for such a small
saving.
> Besides Buildroot, I also switched another package from CMake/Make to
> Meson (based on Ninja), and the benefit was much more interesting -
> although I have not kept any measure to give here.
> I expected the improvement to be caused by Ninja, not Meson. But it
> seems that Meson makes the difference, rather than Make vs Ninja.
We had some contribution some months ago to add support for Meson in
Buildroot. The problem was that the contribution was only composed of
host packages, not used by any target package in Buildroot itself.
Which means it was only adding dead code, that wasn't used at all by
Buildroot unless you have some in-house package that uses Meson.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option
2017-02-01 20:12 ` Thomas Petazzoni
@ 2017-02-03 10:44 ` Cédric Marie
0 siblings, 0 replies; 16+ messages in thread
From: Cédric Marie @ 2017-02-03 10:44 UTC (permalink / raw)
To: buildroot
Le 2017-02-01 21:12, Thomas Petazzoni a ?crit?:
> In the first case you're using the shell built-in "time" program, in
> the later you're using the separate /usr/bin/time program. Try:
Thank you, I was not aware of that.
> I'm indeed not sure it's really worth the effort for such a small
> saving.
OK, so let's forget it. Moreover, as you have mentioned in your reply to
the second patch (on documentation), there are some problems with
$(2)_INSTALL_STAGING/TARGET_OPTS being changed to
$(2)_INSTALL_STAGING/TARGET_ENV, and also with parallel build support
being not supported for some packages.
Regards.
--
C?dric
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option
2017-01-06 22:37 [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option Cédric Marie
2017-01-06 22:37 ` [Buildroot] [PATCH 2/2] Update documentation of CMake infrastructure Cédric Marie
2017-01-21 22:25 ` [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option Romain Naour
@ 2017-01-25 1:37 ` Thomas Petazzoni
2017-07-11 11:56 ` Thomas Petazzoni
3 siblings, 0 replies; 16+ messages in thread
From: Thomas Petazzoni @ 2017-01-25 1:37 UTC (permalink / raw)
To: buildroot
Hello,
On Fri, 6 Jan 2017 23:37:47 +0100, C?dric Marie wrote:
> $(2)_CONF_ENV ?=
> $(2)_CONF_OPTS ?=
> +$(2)_INSTALL_STAGING_ENV ?= DESTDIR=$$(STAGING_DIR)
> +$(2)_INSTALL_TARGET_ENV ?= DESTDIR=$$(TARGET_DIR)
We don't have such options in any other package infrastructure. So I
think I'd prefer if you kept just INSTALL_STAGING_OPTS and
INSTALL_TARGET_OPTS, and define them to the appropriate value depending
on whether you're using Ninja or Make.
Are there any existing CMake package that overrides
INSTALL_STAGING_OPTS or INSTALL_TARGET_OPTS ? If that's the case, then
we cannot change its semantic like this without taking care of those
packages first.
But as I said in my other reply on this thread, I'd really like to get
some justification of why adding support for the Ninja backend is
useful.
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option
2017-01-06 22:37 [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option Cédric Marie
` (2 preceding siblings ...)
2017-01-25 1:37 ` Thomas Petazzoni
@ 2017-07-11 11:56 ` Thomas Petazzoni
2017-07-11 13:25 ` Cédric Marie
3 siblings, 1 reply; 16+ messages in thread
From: Thomas Petazzoni @ 2017-07-11 11:56 UTC (permalink / raw)
To: buildroot
Hello,
Reviving a fairly old thread.
On Fri, 6 Jan 2017 23:37:47 +0100, C?dric Marie wrote:
> Tested on following packages: cannelloni, graphite2, libcuefile,
> libubox, rabbitmq-c, ubus.
I ended up needing this patch to experiment for a project, so I took it
and tested. It generally works fine, but I found one Buildroot CMake
package that doesn't behave properly when the Ninja backend is used:
the "bullet" package.
With the "make" backend, it builds and installs fine, and everything
you expect is install to staging and target.
With the "ninja" backend, it builds and installs fine, but in fact the
only thing it installs to staging and target are two .cmake files. None
of the shared libraries or headers are installed.
The build log with the "make" backend clearly shows that lots of stuff
are being installed: http://code.bulix.org/sx2yb8-160329.
The build log with the "ninja" backend shows just two .cmake files
being installed to staging and target:
http://code.bulix.org/u145dq-160328.
Do you know why a CMake package could be working with the make backend,
but not with the ninja backend ?
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 16+ messages in thread* [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option
2017-07-11 11:56 ` Thomas Petazzoni
@ 2017-07-11 13:25 ` Cédric Marie
2017-07-11 13:35 ` Thomas Petazzoni
0 siblings, 1 reply; 16+ messages in thread
From: Cédric Marie @ 2017-07-11 13:25 UTC (permalink / raw)
To: buildroot
Hi,
Le 2017-07-11 13:56, Thomas Petazzoni a ?crit?:
> Do you know why a CMake package could be working with the make backend,
> but not with the ninja backend ?
In file CMakeLists.txt, in the root directory of the bullet package, I
have found:
IF("${CMAKE_GENERATOR}" MATCHES "Unix Makefiles")
OPTION(INSTALL_LIBS "Set when you want to install libraries" ON)
ELSE()
IF(APPLE AND FRAMEWORK)
OPTION(INSTALL_LIBS "Set when you want to install libraries" ON)
ELSE()
#by default, don't enable the 'INSTALL' option for Xcode and MSVC
projectfiles
OPTION(INSTALL_LIBS "Set when you want to install libraries" OFF)
ENDIF()
ENDIF()
I believe that's the reason :)
I suppose the test was expecting to detect the type of machine, while it
is based on the type of backend...
This should be fixed in "bullet" package, either by testing Ninja
backend as well, or changing the test (I don't know exactly what he's
trying to test...)
Regards,
--
C?dric
^ permalink raw reply [flat|nested] 16+ messages in thread* [Buildroot] [PATCH 1/2] Add BR2_CMAKE_USE_NINJA_BACKEND option
2017-07-11 13:25 ` Cédric Marie
@ 2017-07-11 13:35 ` Thomas Petazzoni
0 siblings, 0 replies; 16+ messages in thread
From: Thomas Petazzoni @ 2017-07-11 13:35 UTC (permalink / raw)
To: buildroot
Hello,
On Tue, 11 Jul 2017 15:25:46 +0200, C?dric Marie wrote:
> Le 2017-07-11 13:56, Thomas Petazzoni a ?crit?:
> > Do you know why a CMake package could be working with the make backend,
> > but not with the ninja backend ?
>
> In file CMakeLists.txt, in the root directory of the bullet package, I
> have found:
>
> IF("${CMAKE_GENERATOR}" MATCHES "Unix Makefiles")
> OPTION(INSTALL_LIBS "Set when you want to install libraries" ON)
> ELSE()
> IF(APPLE AND FRAMEWORK)
> OPTION(INSTALL_LIBS "Set when you want to install libraries" ON)
> ELSE()
> #by default, don't enable the 'INSTALL' option for Xcode and MSVC
> projectfiles
> OPTION(INSTALL_LIBS "Set when you want to install libraries" OFF)
> ENDIF()
> ENDIF()
>
>
> I believe that's the reason :)
Indeed! I skimmed through the CMakeLists.txt file, but didn't spot this
obvious thing.
> I suppose the test was expecting to detect the type of machine, while it
> is based on the type of backend...
> This should be fixed in "bullet" package, either by testing Ninja
> backend as well, or changing the test (I don't know exactly what he's
> trying to test...)
I think it's just a bug on their side. I've filed an issue
(https://github.com/bulletphysics/bullet3/issues/1225). Though with 286
open issues, I'm not sure how long it will take for this issue to be
looked at :)
But anyway, it is a very package-specific issue here, unlikely to
affect many other packages. Thanks for having investigated this!
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 16+ messages in thread