diff mbox series

[2/2] allow build infrastructure to pick up installed ninja tool

Message ID 20190503131851.12315-2-norbert.lange@andritz.com
State Rejected
Headers show
Series [1/2] allow build infrastructure to pick up installed meson tool | expand

Commit Message

Norbert Lange May 3, 2019, 1:18 p.m. UTC
From: Norbert Lange <nolange79@gmail.com>

If a fitting ninja tool is detected it will be used,
otherwise the tool will be built from source.

Replace the fixed dependencies to host-ninja,
notably from the meson infrastructure.

Signed-off-by: Norbert Lange <nolange79@gmail.com>
---
 package/pkg-meson.mk                     |  2 +-
 support/dependencies/check-host-meson.mk |  4 +--
 support/dependencies/check-host-ninja.mk | 14 ++++++++
 support/dependencies/check-host-ninja.sh | 45 ++++++++++++++++++++++++
 4 files changed, 62 insertions(+), 3 deletions(-)
 create mode 100644 support/dependencies/check-host-ninja.mk
 create mode 100755 support/dependencies/check-host-ninja.sh

--
2.20.1

Comments

Thomas Petazzoni May 7, 2019, 9 p.m. UTC | #1
Hello Norbert,

On Fri,  3 May 2019 15:18:51 +0200
Norbert Lange <nolange79@gmail.com> wrote:

> From: Norbert Lange <nolange79@gmail.com>
> 
> If a fitting ninja tool is detected it will be used,
> otherwise the tool will be built from source.
> 
> Replace the fixed dependencies to host-ninja,
> notably from the meson infrastructure.
> 
> Signed-off-by: Norbert Lange <nolange79@gmail.com>

Since host-meson cannot be changed to use the system-provided meson, as
discussed with Peter Seiderer on PATCH 1/2, I don't think it's really
worth doing the effort for host-ninja, which requires the same thing as
host-meson: a host Python interpreter.

I think the approaches to reduce the build time are:

 - Make sure we only need to build one of host-python or host-python3,
   and not both. This is what Jörg Krause is proposing in
   http://patchwork.ozlabs.org/patch/1047119/.

 - Or perhaps, try to go one step further, and see if we can use the
   system-provided Python instead of a Python built by Buildroot. But
   that would only work if the system provides Python 3, since Meson
   requires Python 3.

> diff --git a/support/dependencies/check-host-ninja.mk b/support/dependencies/check-host-ninja.mk
> new file mode 100644
> index 0000000000..6d89255ee5
> --- /dev/null
> +++ b/support/dependencies/check-host-ninja.mk
> @@ -0,0 +1,14 @@
> +# Set this to either 1.8.2 or higher, depending on the highest minimum
> +# version required by any of the packages bundled in Buildroot. If a
> +# package is bumped or a new one added, and it requires a higher
> +# version, our ninja infra will catch it and build its own.

I know this comment is in other .mk files in the same directory, but it
doesn't make any sense. There is no such thing as a "ninja infra" in
Buildroot, and there is nothing that will "catch it" and build its own.
If we forget to update BR2_NINJA_VERSION_MIN to the oldest Ninja
version that is acceptable by all packages using Ninja in Buildroot, we
will never notice until a user hits the problem.

Best regards,

Thomas
Norbert Lange May 8, 2019, 9:49 a.m. UTC | #2
Am Di., 7. Mai 2019 um 23:00 Uhr schrieb Thomas Petazzoni
<thomas.petazzoni@bootlin.com>:
>
> Hello Norbert,
>
> On Fri,  3 May 2019 15:18:51 +0200
> Norbert Lange <nolange79@gmail.com> wrote:
>
> > From: Norbert Lange <nolange79@gmail.com>
> >
> > If a fitting ninja tool is detected it will be used,
> > otherwise the tool will be built from source.
> >
> > Replace the fixed dependencies to host-ninja,
> > notably from the meson infrastructure.
> >
> > Signed-off-by: Norbert Lange <nolange79@gmail.com>
>
> Since host-meson cannot be changed to use the system-provided meson, as
> discussed with Peter Seiderer on PATCH 1/2, I don't think it's really
> worth doing the effort for host-ninja, which requires the same thing as
> host-meson: a host Python interpreter.

The effort is already done in the patch and not really a big one. Is
there anything bad about
having some additional requirements matched?
But yes, most of the time is spent building python (twice).

> I think the approaches to reduce the build time are:
>
>  - Make sure we only need to build one of host-python or host-python3,
>    and not both. This is what Jörg Krause is proposing in
>    http://patchwork.ozlabs.org/patch/1047119/.
>
>  - Or perhaps, try to go one step further, and see if we can use the
>    system-provided Python instead of a Python built by Buildroot. But
>    that would only work if the system provides Python 3, since Meson
>    requires Python 3.

maybe add some python-any dependency (resolves to either 2 or 3),
which is enough for bootstrapping host-ninja?

>
> > diff --git a/support/dependencies/check-host-ninja.mk b/support/dependencies/check-host-ninja.mk
> > new file mode 100644
> > index 0000000000..6d89255ee5
> > --- /dev/null
> > +++ b/support/dependencies/check-host-ninja.mk
> > @@ -0,0 +1,14 @@
> > +# Set this to either 1.8.2 or higher, depending on the highest minimum
> > +# version required by any of the packages bundled in Buildroot. If a
> > +# package is bumped or a new one added, and it requires a higher
> > +# version, our ninja infra will catch it and build its own.
>
> I know this comment is in other .mk files in the same directory, but it
> doesn't make any sense. There is no such thing as a "ninja infra" in
> Buildroot, and there is nothing that will "catch it" and build its own.
> If we forget to update BR2_NINJA_VERSION_MIN to the oldest Ninja
> version that is acceptable by all packages using Ninja in Buildroot, we
> will never notice until a user hits the problem.

That's something what your CI builder could catch.
packages break all the time with new compilers and libc's and I think
ninja should be pretty much painless in comparison.
The meta-buildsystems like mesons should be able to check for a matching version
(which might depend on the build, like C++20 modules will need some
features in ninja)

Norbert.
diff mbox series

Patch

diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk
index 25e9bb814e..c4cf682ee8 100644
--- a/package/pkg-meson.mk
+++ b/package/pkg-meson.mk
@@ -26,7 +26,7 @@ 
 # $HOME/.local/lib/python3.x/site-packages
 #
 MESON		= PYTHONNOUSERSITE=y $(BR2_MESON)
-NINJA		= PYTHONNOUSERSITE=y $(HOST_DIR)/bin/ninja
+NINJA		= PYTHONNOUSERSITE=y $(BR2_NINJA)
 NINJA_OPTS	= $(if $(VERBOSE),-v) -j$(PARALLEL_JOBS)

 ################################################################################
diff --git a/support/dependencies/check-host-meson.mk b/support/dependencies/check-host-meson.mk
index 6c779b3e6d..36fc9129d1 100644
--- a/support/dependencies/check-host-meson.mk
+++ b/support/dependencies/check-host-meson.mk
@@ -10,7 +10,7 @@  BR2_MESON ?= $(call suitable-host-package,meson,\
 	$(BR2_MESON_VERSION_MIN) $(BR2_MESON_CANDIDATES))
 ifeq ($(BR2_MESON),)
 BR2_MESON = $(HOST_DIR)/bin/meson
-BR2_MESON_HOST_DEPENDENCY = host-meson host-ninja
+BR2_MESON_HOST_DEPENDENCY = host-meson $(BR2_NINJA_HOST_DEPENDENCY)
 else
-BR2_MESON_HOST_DEPENDENCY = host-ninja
+BR2_MESON_HOST_DEPENDENCY = $(BR2_NINJA_HOST_DEPENDENCY)
 endif
diff --git a/support/dependencies/check-host-ninja.mk b/support/dependencies/check-host-ninja.mk
new file mode 100644
index 0000000000..6d89255ee5
--- /dev/null
+++ b/support/dependencies/check-host-ninja.mk
@@ -0,0 +1,14 @@ 
+# Set this to either 1.8.2 or higher, depending on the highest minimum
+# version required by any of the packages bundled in Buildroot. If a
+# package is bumped or a new one added, and it requires a higher
+# version, our ninja infra will catch it and build its own.
+#
+BR2_NINJA_VERSION_MIN = 1.8.0
+
+BR2_NINJA_CANDIDATES ?= ninja
+BR2_NINJA ?= $(call suitable-host-package,ninja,\
+	$(BR2_NINJA_VERSION_MIN) $(BR2_NINJA_CANDIDATES))
+ifeq ($(BR2_NINJA),)
+BR2_NINJA = $(HOST_DIR)/bin/ninja
+BR2_NINJA_HOST_DEPENDENCY = host-ninja
+endif
diff --git a/support/dependencies/check-host-ninja.sh b/support/dependencies/check-host-ninja.sh
new file mode 100755
index 0000000000..7f531da98e
--- /dev/null
+++ b/support/dependencies/check-host-ninja.sh
@@ -0,0 +1,45 @@ 
+#!/bin/sh
+
+# prevent shift error
+[ $# -lt 2 ] && exit 1
+
+split_version() {
+    local VARPREFIX
+    local NUMBERS
+    local major
+    local minor
+
+    VARPREFIX=$1
+    NUMBERS=$2
+
+    major=${NUMBERS%%\.*}
+    NUMBERS=${NUMBERS#$major}; NUMBERS=${NUMBERS#\.}
+    minor=${NUMBERS%%\.*}
+    NUMBERS=${NUMBERS#$minor}; NUMBERS=${NUMBERS#\.}
+
+    # ensure that missing values are 0
+    eval "${VARPREFIX}_major=\$major; ${VARPREFIX}_minor=\$((minor + 0)); ${VARPREFIX}_bugfix=\$((NUMBERS + 0));"
+}
+
+split_version req "$1"
+
+shift
+
+for candidate; do
+
+    # Try to locate the candidate. Discard it if not located.
+    ninja=$(which "${candidate}" 2>/dev/null)
+    [ -n "${ninja}" ] || continue
+
+    split_version cur "$("${ninja}" --version)"
+
+    [ -n "${cur_major}" -a "${cur_major}" -ge "${req_major}" ] || continue
+    [ "${cur_major}" -gt "${req_major}" ] || [ "${cur_minor}" -ge "${req_minor}" ] || continue
+    [ "${cur_minor}" -gt "${req_minor}" ] || [ "${cur_bugfix}" -ge "${req_bugfix}" ] || continue
+
+    echo "${ninja}"
+    exit
+done
+
+# echo nothing: no suitable ninja found
+exit 1