[v5,07/34] package/kodi-texturepacker: new host package

Needed for upcoming kodi version bump to 17.1-Krypton which will also
switch the kodi build system to CMake.

Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
---
 package/kodi-texturepacker/kodi-texturepacker.hash |  1 +
 package/kodi-texturepacker/kodi-texturepacker.mk   | 34 ++++++++++++++++++++++
 package/kodi/kodi.hash                             |  1 +
 3 files changed, 36 insertions(+)
 create mode 120000 package/kodi-texturepacker/kodi-texturepacker.hash
 create mode 100644 package/kodi-texturepacker/kodi-texturepacker.mk

Bernd, All,

On 2017-04-17 21:54 +0200, Bernd Kuhls spake thusly:
> Needed for upcoming kodi version bump to 17.1-Krypton which will also
> switch the kodi build system to CMake.

And now I see why you did not introduce a host-variant of Kodi: you need
two tools from it, and they are in different subdirs.

I still find it a bit of disapointing that the Kodi buildsystem can not
build its own tools... :-(

> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> ---
>  package/kodi-texturepacker/kodi-texturepacker.hash |  1 +
>  package/kodi-texturepacker/kodi-texturepacker.mk   | 34 ++++++++++++++++++++++
>  package/kodi/kodi.hash                             |  1 +
>  3 files changed, 36 insertions(+)
>  create mode 120000 package/kodi-texturepacker/kodi-texturepacker.hash
>  create mode 100644 package/kodi-texturepacker/kodi-texturepacker.mk
> 
> diff --git a/package/kodi-texturepacker/kodi-texturepacker.hash b/package/kodi-texturepacker/kodi-texturepacker.hash
> new file mode 120000
> index 000000000..92a75949b
> --- /dev/null
> +++ b/package/kodi-texturepacker/kodi-texturepacker.hash
> @@ -0,0 +1 @@
> +/home/bernd/buildroot/br8_ffmpeg3_kodi17_github/package/kodi/kodi.hash
> \ No newline at end of file

Uh-Oh, bad symlink again... :-/

> diff --git a/package/kodi-texturepacker/kodi-texturepacker.mk b/package/kodi-texturepacker/kodi-texturepacker.mk
> new file mode 100644
> index 000000000..bce73f7f0
> --- /dev/null
> +++ b/package/kodi-texturepacker/kodi-texturepacker.mk
> @@ -0,0 +1,34 @@
> +################################################################################
> +#
> +# kodi-texturepacker
> +#
> +################################################################################
> +
> +# Not possible to directly refer to kodi variables, because of
> +# first/second expansion trickery...
> +HOST_KODI_TEXTUREPACKER_VERSION = 17.1-Krypton
> +HOST_KODI_TEXTUREPACKER_SITE = $(call github,xbmc,xbmc,$(HOST_KODI_TEXTUREPACKER_VERSION))
> +HOST_KODI_TEXTUREPACKER_LICENSE = GPLv2
> +HOST_KODI_TEXTUREPACKER_LICENSE_FILES = LICENSE.GPL

As for the previous package: we usually do not prefix "generic"
variables with 'HOST_', even for host-only packages.

> +HOST_KODI_TEXTUREPACKER_SUBDIR = tools/depends/native/TexturePacker
> +HOST_KODI_TEXTUREPACKER_DEPENDENCIES += \
> +	host-giflib \
> +	host-libjpeg \
> +	host-libpng \
> +	host-lzo
> +
> +HOST_KODI_TEXTUREPACKER_HOST_CXXFLAGS = "$(HOST_CXXFLAGS) -std=c++0x \
> +	-DTARGET_POSIX -DTARGET_LINUX -D_LINUX -I$(@D)/xbmc/linux"

When lines are really too long and contains many definitiions-or-stuff,
we usually make it a multi-line with one item per line, like you did for
_DEPENDENCIES, above. But in this case, you are setting CXXFLAGS and the
string is split on two lines, which is ugly.

    HOST_KODI_TEXTUREPACKER_CXXFLAGS = \
        $(HOST_CXXFLAGS) \
        -DTARGET_POSIX \
        -DTARGET_LINUX \
        -D_LINUX \
        -I$(@D)/xbmc/linux

(Note also that the variable lost the second 'HOST_' in its name,
because there is no 'host-cxxflags' when building for the host; there
are only 'cxxflags'.)

> +HOST_KODI_TEXTUREPACKER_CONF_OPTS += \
> +	-DCMAKE_CXX_FLAGS=$(HOST_KODI_TEXTUREPACKER_HOST_CXXFLAGS) \

    -DCMAKE_CXX_FLAGS=$(HOST_KODI_TEXTUREPACKER_CXXFLAGS)

> +	-DCMAKE_MODULE_PATH=$(@D)/project/cmake/modules \
> +	-DCORE_SOURCE_DIR=$(@D) \
> +	-Wno-dev
> +
> +HOST_KODI_TEXTUREPACKER_INSTALL_CMDS = \
> +	$(INSTALL) -m 755 -D \
> +		$(@D)/tools/depends/native/TexturePacker/TexturePacker \
> +		$(HOST_DIR)/usr/bin/TexturePacker

Please use a define block here too.

Regards,
Yann E. MORIN.

> +$(eval $(host-cmake-package))
> diff --git a/package/kodi/kodi.hash b/package/kodi/kodi.hash
> index b910af2d8..ae47c70c9 100644
> --- a/package/kodi/kodi.hash
> +++ b/package/kodi/kodi.hash
> @@ -1,3 +1,4 @@
>  # Locally computed
>  sha256	7d82c8aff2715c83deecdf10c566e26105bec0473af530a1356d4c747ebdfd10	kodi-16.1-Jarvis.tar.gz
>  sha256 303f3903cbb57ccc2961f09cf3746505542bcb129a464f0687d7ca8601cebbee  kodi-jsonschemabuilder-17.1-Krypton.tar.gz
> +sha256 303f3903cbb57ccc2961f09cf3746505542bcb129a464f0687d7ca8601cebbee  kodi-texturepacker-17.1-Krypton.tar.gz
> -- 
> 2.11.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

On 22-04-17 11:26, Yann E. MORIN wrote:
> Bernd, All,
> 
> On 2017-04-17 21:54 +0200, Bernd Kuhls spake thusly:
>> Needed for upcoming kodi version bump to 17.1-Krypton which will also
>> switch the kodi build system to CMake.
> And now I see why you did not introduce a host-variant of Kodi: you need
> two tools from it, and they are in different subdirs.

 ... and you don't want to build all of Kodi for the host.

 Still, I think it might be easier to treat the host version as a generic
package, and call cmake and make explicitly, once for each tool.


> I still find it a bit of disapointing that the Kodi buildsystem can not
> build its own tools... :-(

 Or indeed, patch Kodi's CMakeLists.txt so it can build its own build tools.

 Regards,
 Arnout

rnout, All,

On 2017-04-23 00:19 +0200, Arnout Vandecappelle spake thusly:
> On 22-04-17 11:26, Yann E. MORIN wrote:
> > Bernd, All,
> > 
> > On 2017-04-17 21:54 +0200, Bernd Kuhls spake thusly:
> >> Needed for upcoming kodi version bump to 17.1-Krypton which will also
> >> switch the kodi build system to CMake.
> > And now I see why you did not introduce a host-variant of Kodi: you need
> > two tools from it, and they are in different subdirs.
> 
>  ... and you don't want to build all of Kodi for the host.

I was not about building the whole Kodi for the host, but to have thinks
like:

    HOST_KODI_SUBDIR = tools/blabla

    define HOST_KODI_INSTALL_CMDS
        $(INSTALL) -D -m 0755 $(@D)/tools/blabla/foo $(HOST_DIR)/usr/bin/foo
    endef

    $(eval $(host-cmake-package))

>  Still, I think it might be easier to treat the host version as a generic
> package, and call cmake and make explicitly, once for each tool.

I would like we avoid duplicating the infra. When we fix something in
the infra, we have to hunt down all dupes in the code...

Unless we change the infra to have the common code in variables, like:

1- outside the -inner define:

    # $(1): uppercase package name
    define CMAKE_CONFIGURE_CMD
        (mkdir -p $($(1)_BUILDDIR) && \
        cd $($(1)_BUILDDIR) && \
        rm -f CMakeCache.txt && \
        PATH=$$(BR_PATH) \
        $($(1)_CONF_ENV) $(BR2_CMAKE) $($(1)_SRCDIR) \
            -DCMAKE_TOOLCHAIN_FILE="$(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake" \
            -DCMAKE_INSTALL_PREFIX="/usr" \
            -DCMAKE_COLOR_MAKEFILE=OFF \
            -DBUILD_DOC=OFF \
            -DBUILD_DOCS=OFF \
            -DBUILD_EXAMPLE=OFF \
            -DBUILD_EXAMPLES=OFF \
            -DBUILD_TEST=OFF \
            -DBUILD_TESTS=OFF \
            -DBUILD_TESTING=OFF \
            -DBUILD_SHARED_LIBS=$$(if $$(BR2_STATIC_LIBS),OFF,ON) \
            $$(CMAKE_QUIET) \
            $($(1)_CONF_OPTS) \
        )
    endef

And then inside the -inner define:

    ifndef $(2)_CONFIGURE_CMDS
    define $(2)CONFIGURE_CMDS
        $$(call CMAKE_CONFIGURE_CMD,$(2))
    endef
    endif

Or something along those lines, which would allow us to re-use the
infras and be sure that we can fix them in a single location.

Of course, is it worth the effort?

> > I still find it a bit of disapointing that the Kodi buildsystem can not
> > build its own tools... :-(
> 
>  Or indeed, patch Kodi's CMakeLists.txt so it can build its own build tools.

Meh...

Regards,
Yann E. MORIN.