Re: [Buildroot] [PATCH v3 2/2] package/pkg-meson: improve generation of cross-compilation file

[Arnout Vandecappelle <arnout@mind.be>]

On 03/12/2021 22:54, Norbert Lange wrote:
> Removed a few variables, as they were only used to communicate
> between the meson package and pkg-meson.mk and are not needed
> anymore.
> 
> Moved cross-compilation.conf.in out of meson package.
> 
> Creating the cross-compilation.conf files for packages is now
> using the original template.
> To avoid duplicate code, the common sed pattern is stored in
> a make variable.
> 
> Use explicit Buildroot variables for compiler tools,
> and some fixes. (TARGET_LDFLAGS and TARGET_CXXFLAGS
> were mixed up with PKG_TARGET_CFLAGS)
> 
> Signed-off-by: Norbert Lange <nolange79@gmail.com>

  Applied to master, but see below.

> ---
> v2->v3:
> -   rebase
> 
> v1->v2:
> -   squash 2 commits, really hard to separate as moving and renaming
>      variables from 2 to one location affects alot code.
> -   use TOOLCHAIN_POST_INSTALL_STAGING_HOOKS again.
> -   consistently use double quotes for sed patterns
> 
> Signed-off-by: Norbert Lange <nolange79@gmail.com>
> ---
>   package/pkg-meson.mk                          | 58 +++++++++----------
>   .../misc}/cross-compilation.conf.in           | 10 ++--
>   2 files changed, 33 insertions(+), 35 deletions(-)
>   rename {package/meson => support/misc}/cross-compilation.conf.in (84%)
> 
> diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk
> index 4914486665..77dc2b46b0 100644
> --- a/package/pkg-meson.mk
> +++ b/package/pkg-meson.mk
> @@ -67,19 +67,9 @@ $(2)_CXXFLAGS ?= $$(TARGET_CXXFLAGS)
>   define $(2)_CONFIGURE_CMDS
>   	rm -rf $$($$(PKG)_SRCDIR)/build
>   	mkdir -p $$($$(PKG)_SRCDIR)/build
> -	sed -e 's%@TARGET_CROSS@%$$(TARGET_CROSS)%g' \
> -	    -e 's%@TARGET_ARCH@%$$(HOST_MESON_TARGET_CPU_FAMILY)%g' \
> -	    -e 's%@TARGET_CPU@%$$(HOST_MESON_TARGET_CPU)%g' \
> -	    -e 's%@TARGET_ENDIAN@%$$(HOST_MESON_TARGET_ENDIAN)%g' \
> -	    -e "s%@TARGET_CFLAGS@%$$(call make-sq-comma-list,$$($(2)_CFLAGS))%g" \
> -	    -e "s%@TARGET_LDFLAGS@%$$(call make-sq-comma-list,$$($(2)_LDFLAGS))%g" \
> -	    -e "s%@TARGET_CXXFLAGS@%$$(call make-sq-comma-list,$$($(2)_CXXFLAGS))%g" \
> -	    -e 's%@HOST_DIR@%$$(HOST_DIR)%g' \
> -	    -e 's%@STAGING_DIR@%$$(STAGING_DIR)%g' \
> -	    -e 's%@STATIC@%$$(if $$(BR2_STATIC_LIBS),true,false)%g' \
> -	    -e "/^\[binaries\]$$$$/s:$$$$:$$(foreach x,$$($(2)_MESON_EXTRA_BINARIES),\n$$(x)):" \
> +	sed -e "/^\[binaries\]$$$$/s:$$$$:$$(foreach x,$$($(2)_MESON_EXTRA_BINARIES),\n$$(x)):" \
>   	    -e "/^\[properties\]$$$$/s:$$$$:$$(foreach x,$$($(2)_MESON_EXTRA_PROPERTIES),\n$$(x)):" \
> -	    package/meson/cross-compilation.conf.in \
> +	    $$(call PKG_MESON_CROSSCONFIG_SED,$(2)_CFLAGS,$(2)_CXXFLAGS,$(2)_LDFLAGS) \
>   	    > $$($$(PKG)_SRCDIR)/build/cross-compilation.conf
>   	PATH=$$(BR_PATH) \
>   	CC_FOR_BUILD="$$(HOSTCC)" \
> @@ -223,31 +213,39 @@ else
>   PKG_MESON_TARGET_CPU_FAMILY = $(ARCH)
>   endif
>   
> -HOST_MESON_TARGET_ENDIAN = $(call qstrip,$(call LOWERCASE,$(BR2_ENDIAN)))
> -HOST_MESON_TARGET_CPU = $(GCC_TARGET_CPU)
> +# Generates sed patterns for patching the cross-compilation.conf template,
> +# since Flags might contain commas the arguments are passed indirectly by
> +# variable name (stripped to deal with whitespaces).
> +# Arguments are variable containing cflags, cxxflags, ldflags.
> +define PKG_MESON_CROSSCONFIG_SED
> +        -e "s%@TARGET_CC@%$(TARGET_CC)%g" \
> +        -e "s%@TARGET_CXX@%$(TARGET_CXX)%g" \
> +        -e "s%@TARGET_AR@%$(TARGET_AR)%g" \
> +        -e "s%@TARGET_STRIP@%$(TARGET_STRIP)%g" \
> +        -e "s%@TARGET_ARCH@%$(PKG_MESON_TARGET_CPU_FAMILY)%g" \
> +        -e "s%@TARGET_CPU@%$(GCC_TARGET_CPU)%g" \
> +        -e "s%@TARGET_ENDIAN@%$(call qstrip,$(call LOWERCASE,$(BR2_ENDIAN)))%g" \
> +        -e "s%@TARGET_CFLAGS@%$(call make-sq-comma-list,$($(strip $(1))))%g" \
> +        -e "s%@TARGET_LDFLAGS@%$(call make-sq-comma-list,$($(strip $(3))))%g" \
> +        -e "s%@TARGET_CXXFLAGS@%$(call make-sq-comma-list,$($(strip $(2))))%g" \
> +        -e "s%@PKGCONF_HOST_BINARY@%$(HOST_DIR)/bin/pkgconf%g" \
> +        -e "s%@STAGING_DIR@%$(STAGING_DIR)%g" \
> +        -e "s%@STATIC@%$(if $(BR2_STATIC_LIBS),true,false)%g" \
> +        $(TOPDIR)/support/misc/cross-compilation.conf.in
> +endef
>   
>   # Generate a Meson cross-compilation.conf suitable for use with the
>   # SDK; also install the file as a template for users to add their
>   # own flags if they need to.
>   define PKG_MESON_INSTALL_CROSS_CONF
>   	mkdir -p $(HOST_DIR)/etc/meson
> -	sed -e 's%@TARGET_CROSS@%$(TARGET_CROSS)%g' \
> -	    -e 's%@TARGET_ARCH@%$(HOST_MESON_TARGET_CPU_FAMILY)%g' \
> -	    -e 's%@TARGET_CPU@%$(HOST_MESON_TARGET_CPU)%g' \
> -	    -e 's%@TARGET_ENDIAN@%$(HOST_MESON_TARGET_ENDIAN)%g' \
> -	    -e "s%@TARGET_CFLAGS@%$(call make-sq-comma-list,$(TARGET_CFLAGS))@PKG_TARGET_CFLAGS@%g" \
> -	    -e "s%@TARGET_LDFLAGS@%$(call make-sq-comma-list,$(TARGET_LDFLAGS))@PKG_TARGET_CFLAGS@%g" \
> -	    -e "s%@TARGET_CXXFLAGS@%$(call make-sq-comma-list,$(TARGET_CXXFLAGS))@PKG_TARGET_CFLAGS@%g" \
> -	    -e 's%@HOST_DIR@%$(HOST_DIR)%g' \
> -	    -e 's%@STAGING_DIR@%$(STAGING_DIR)%g' \
> -	    -e 's%@STATIC@%$(if $(BR2_STATIC_LIBS),true,false)%g' \
> -	    $(HOST_MESON_PKGDIR)/cross-compilation.conf.in \
> +	sed -e "s%@TARGET_CFLAGS@%$(call make-sq-comma-list,$(TARGET_CFLAGS))@PKG_TARGET_CFLAGS@%g" \
> +	    -e "s%@TARGET_LDFLAGS@%$(call make-sq-comma-list,$(TARGET_LDFLAGS))@PKG_TARGET_LDFLAGS@%g" \
> +	    -e "s%@TARGET_CXXFLAGS@%$(call make-sq-comma-list,$(TARGET_CXXFLAGS))@PKG_TARGET_CXXFLAGS@%g" \
> +	    $(call PKG_MESON_CROSSCONFIG_SED) \

  This could have been

$(call PKG_MESON_CROSSCONFIG_SED,$(call 
make-sq-comma-list,$(TARGET_CFLAGS))@PKG_TARGET_CFLAGS@,$(call 
make-sq-comma-list,$(TARGET_CXXFLAGS))@PKG_TARGET_CXXFLAGS@,$(call 
make-sq-comma-list,$(TARGET_LDFLAGS))@PKG_TARGET_LDFLAGS@)

which is of course completely unreadable.

  While looking at this, however, I noticed that there's something wrong with 
the cross-compilation.conf.in we generate. It contains this:

c_args = ['-D_LARGEFILE_SOURCE', ..., '-D_FORTIFY_SOURCE=1'@PKG_TARGET_CFLAGS@]
c_link_args = [@PKG_TARGET_CFLAGS@]

which means that @PKG_TARGET_CFLAGS@ can't be replaced with anything sane: in 
c_args, it must start with a comma, but in c_link_args it must not start with 
comma. But this was already wrong before this patch. In fact, this was already a 
problem since the very time that Peter introduced PKG_TARGET_CFLAGS.

  I'm not sure what to do about it.


>   	    > $(HOST_DIR)/etc/meson/cross-compilation.conf.in
> -	sed -e 's%@PKG_TARGET_CFLAGS@%%g' \
> -	    -e 's%@PKG_TARGET_LDFLAGS@%%g' \
> -	    -e 's%@PKG_TARGET_CXXFLAGS@%%g' \
> -	    $(HOST_DIR)/etc/meson/cross-compilation.conf.in \
> +	sed $(call PKG_MESON_CROSSCONFIG_SED,TARGET_CFLAGS,TARGET_CXXFLAGS,TARGET_LDFLAGS) \
>   	    > $(HOST_DIR)/etc/meson/cross-compilation.conf
>   endef
>   
> -TOOLCHAIN_TARGET_FINALIZE_HOOKS += PKG_MESON_INSTALL_CROSS_CONF
> +TOOLCHAIN_POST_INSTALL_STAGING_HOOKS += PKG_MESON_INSTALL_CROSS_CONF

  This change (do it in post-install-staging instead of target-finalize) is not 
explained in the commit message, and it reverts an earlier change [1]. So I 
reverted that part.

  Regards,
  Arnout

[1] 
https://lore.kernel.org/buildroot/20191204150225.33749-1-thomas.petazzoni@bootlin.com/



> diff --git a/package/meson/cross-compilation.conf.in b/support/misc/cross-compilation.conf.in
> similarity index 84%
> rename from package/meson/cross-compilation.conf.in
> rename to support/misc/cross-compilation.conf.in
> index 7a7ece97ae..18cf258a8e 100644
> --- a/package/meson/cross-compilation.conf.in
> +++ b/support/misc/cross-compilation.conf.in
> @@ -4,11 +4,11 @@
>   # - Buildroot's 'target' is Meson's 'host'
>   
>   [binaries]
> -c = '@TARGET_CROSS@gcc'
> -cpp = '@TARGET_CROSS@g++'
> -ar = '@TARGET_CROSS@ar'
> -strip = '@TARGET_CROSS@strip'
> -pkgconfig = '@HOST_DIR@/bin/pkgconf'
> +c = '@TARGET_CC@'
> +cpp = '@TARGET_CXX@'
> +ar = '@TARGET_AR@'
> +strip = '@TARGET_STRIP@'
> +pkgconfig = '@PKGCONF_HOST_BINARY@'
>   g-ir-compiler = '@STAGING_DIR@/usr/bin/g-ir-compiler'
>   g-ir-scanner = '@STAGING_DIR@/usr/bin/g-ir-scanner'
>   
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot