[PATCH-for-master,1/2] package/meson: don't install cross-compilation.conf during target-finalize

Message ID 20190225211147.14947-2-patrickdepinguin@gmail.com
State New
Headers show
Series
  • misc fixes for 2019.02
Related show

Commit Message

Thomas De Schampheleire Feb. 25, 2019, 9:11 p.m.
From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

package/meson installs a cross-compilation.conf file in
$(HOST_DIR)/etc/meson, via TARGET_FINALIZE_HOOKS.

package/pkg-cmake.mk installs a toolchainfile.cmake in
$(HOST_DIR)/share/buildroot, via TOOLCHAIN_POST_INSTALL_STAGING_HOOKS.

Both files have a similar concept, they describe some flags/paths needed for
compilation using respective build systems. One difference is that the meson
file is added for external compilation, from the SDK, while the cmake file
is used internally in Buildroot.

The 'problem' of using TARGET_FINALIZE_HOOKS for the meson file, is that it
installs a 'host' file from target-finalize, which is conceptually incorrect
and breaks the invariant that only TARGET_DIR is changed on a subsequent
'make' when everything was already built (i.e. only target-finalize is run).

This can easily be fixed, by using the same hook as cmake uses, i.e.
TOOLCHAIN_POST_INSTALL_STAGING_HOOKS.

Note that actually even for cmake, TOOLCHAIN_POST_INSTALL_STAGING_HOOKS is
not the best hook to install a host file. A better hook would have been
TOOLCHAIN_POST_INSTALL_HOOKS, but this triggers only for 'host' packages,
and 'toolchain' is treated as a 'target' package.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/meson/meson.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Peter Korsgaard Feb. 26, 2019, 6:52 p.m. | #1
>>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:

 > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
 > package/meson installs a cross-compilation.conf file in
 > $(HOST_DIR)/etc/meson, via TARGET_FINALIZE_HOOKS.

 > package/pkg-cmake.mk installs a toolchainfile.cmake in
 > $(HOST_DIR)/share/buildroot, via TOOLCHAIN_POST_INSTALL_STAGING_HOOKS.

 > Both files have a similar concept, they describe some flags/paths needed for
 > compilation using respective build systems. One difference is that the meson
 > file is added for external compilation, from the SDK, while the cmake file
 > is used internally in Buildroot.

 > The 'problem' of using TARGET_FINALIZE_HOOKS for the meson file, is that it
 > installs a 'host' file from target-finalize, which is conceptually incorrect
 > and breaks the invariant that only TARGET_DIR is changed on a subsequent
 > 'make' when everything was already built (i.e. only target-finalize is run).

 > This can easily be fixed, by using the same hook as cmake uses, i.e.
 > TOOLCHAIN_POST_INSTALL_STAGING_HOOKS.

 > Note that actually even for cmake, TOOLCHAIN_POST_INSTALL_STAGING_HOOKS is
 > not the best hook to install a host file. A better hook would have been
 > TOOLCHAIN_POST_INSTALL_HOOKS, but this triggers only for 'host' packages,
 > and 'toolchain' is treated as a 'target' package.

 > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
 > ---
 >  package/meson/meson.mk | 3 ++-
 >  1 file changed, 2 insertions(+), 1 deletion(-)

 > diff --git a/package/meson/meson.mk b/package/meson/meson.mk
 > index d76541cc93..aba7382cc9 100644
 > --- a/package/meson/meson.mk
 > +++ b/package/meson/meson.mk
 > @@ -65,6 +65,7 @@ define HOST_MESON_INSTALL_CROSS_CONF
 >> $(HOST_DIR)/etc/meson/cross-compilation.conf
 >  endef
 
 > -TARGET_FINALIZE_HOOKS += HOST_MESON_INSTALL_CROSS_CONF
 > +TOOLCHAIN_POST_INSTALL_STAGING_HOOKS += HOST_MESON_INSTALL_CROSS_CONF

Is this then going to use the correct (final) paths for per-package
build directories?
Thomas De Schampheleire Feb. 26, 2019, 7:48 p.m. | #2
El mar., 26 feb. 2019 a las 19:52, Peter Korsgaard
(<peter@korsgaard.com>) escribió:
>
> >>>>> "Thomas" == Thomas De Schampheleire <patrickdepinguin@gmail.com> writes:
>
>  > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>  > package/meson installs a cross-compilation.conf file in
>  > $(HOST_DIR)/etc/meson, via TARGET_FINALIZE_HOOKS.
>
>  > package/pkg-cmake.mk installs a toolchainfile.cmake in
>  > $(HOST_DIR)/share/buildroot, via TOOLCHAIN_POST_INSTALL_STAGING_HOOKS.
>
>  > Both files have a similar concept, they describe some flags/paths needed for
>  > compilation using respective build systems. One difference is that the meson
>  > file is added for external compilation, from the SDK, while the cmake file
>  > is used internally in Buildroot.
>
>  > The 'problem' of using TARGET_FINALIZE_HOOKS for the meson file, is that it
>  > installs a 'host' file from target-finalize, which is conceptually incorrect
>  > and breaks the invariant that only TARGET_DIR is changed on a subsequent
>  > 'make' when everything was already built (i.e. only target-finalize is run).
>
>  > This can easily be fixed, by using the same hook as cmake uses, i.e.
>  > TOOLCHAIN_POST_INSTALL_STAGING_HOOKS.
>
>  > Note that actually even for cmake, TOOLCHAIN_POST_INSTALL_STAGING_HOOKS is
>  > not the best hook to install a host file. A better hook would have been
>  > TOOLCHAIN_POST_INSTALL_HOOKS, but this triggers only for 'host' packages,
>  > and 'toolchain' is treated as a 'target' package.
>
>  > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>  > ---
>  >  package/meson/meson.mk | 3 ++-
>  >  1 file changed, 2 insertions(+), 1 deletion(-)
>
>  > diff --git a/package/meson/meson.mk b/package/meson/meson.mk
>  > index d76541cc93..aba7382cc9 100644
>  > --- a/package/meson/meson.mk
>  > +++ b/package/meson/meson.mk
>  > @@ -65,6 +65,7 @@ define HOST_MESON_INSTALL_CROSS_CONF
>  >> $(HOST_DIR)/etc/meson/cross-compilation.conf
>  >  endef
>
>  > -TARGET_FINALIZE_HOOKS += HOST_MESON_INSTALL_CROSS_CONF
>  > +TOOLCHAIN_POST_INSTALL_STAGING_HOOKS += HOST_MESON_INSTALL_CROSS_CONF
>
> Is this then going to use the correct (final) paths for per-package
> build directories?
>

I haven't played with / investigated the per-package feature yet, so I
may miss some specifics.
Below is the content of the hook:

define HOST_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@%$(HOST_MESON_SED_CFLAGS)%g" \
        -e "s%@TARGET_LDFLAGS@%$(HOST_MESON_SED_LDFLAGS)%g" \
        -e "s%@TARGET_CXXFLAGS@%$(HOST_MESON_SED_CXXFLAGS)%g" \
        -e "s%@HOST_DIR@%$(HOST_DIR)%g" \
        $(HOST_MESON_PKGDIR)/cross-compilation.conf.in \
        > $(HOST_DIR)/etc/meson/cross-compilation.conf
endef

Are there any variables in there that would change value?

In sequential build, the file before and after this change is identical.

Best regards,
Thomas
Thomas Petazzoni Feb. 27, 2019, 5:18 p.m. | #3
Hello,

On Tue, 26 Feb 2019 20:48:49 +0100
Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote:

> I haven't played with / investigated the per-package feature yet, so I
> may miss some specifics.
> Below is the content of the hook:
> 
> define HOST_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@%$(HOST_MESON_SED_CFLAGS)%g" \
>         -e "s%@TARGET_LDFLAGS@%$(HOST_MESON_SED_LDFLAGS)%g" \
>         -e "s%@TARGET_CXXFLAGS@%$(HOST_MESON_SED_CXXFLAGS)%g" \
>         -e "s%@HOST_DIR@%$(HOST_DIR)%g" \

With per-package enabled, this $(HOST_DIR) here will point to the
per-package host directory of host-meson instead of pointing to the
global host directory.

>         $(HOST_MESON_PKGDIR)/cross-compilation.conf.in \
>         > $(HOST_DIR)/etc/meson/cross-compilation.conf  

Here as well, but this one is not an issue: the per-package host
directory of host-meson will be rsync'ed into the global host directory
at the end of the build.

> Are there any variables in there that would change value?

With per-package enabled, HOST_DIR, STAGING_DIR and TARGET_DIR have a
different value when building each package (they point to the current
package per-package directories) or when building stuff outside of a
package (they point to the global directories).

> In sequential build, the file before and after this change is identical.

It's not so much sequential vs. parallel, but per-package directories
or not. Per-package directory can be used while doing a sequential
build.

Best regards,

Thomas
Thomas De Schampheleire Feb. 27, 2019, 7:57 p.m. | #4
El mié., 27 feb. 2019 a las 18:18, Thomas Petazzoni
(<thomas.petazzoni@bootlin.com>) escribió:
>
> Hello,
>
> On Tue, 26 Feb 2019 20:48:49 +0100
> Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote:
>
> > I haven't played with / investigated the per-package feature yet, so I
> > may miss some specifics.
> > Below is the content of the hook:
> >
> > define HOST_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@%$(HOST_MESON_SED_CFLAGS)%g" \
> >         -e "s%@TARGET_LDFLAGS@%$(HOST_MESON_SED_LDFLAGS)%g" \
> >         -e "s%@TARGET_CXXFLAGS@%$(HOST_MESON_SED_CXXFLAGS)%g" \
> >         -e "s%@HOST_DIR@%$(HOST_DIR)%g" \
>
> With per-package enabled, this $(HOST_DIR) here will point to the
> per-package host directory of host-meson instead of pointing to the
> global host directory.
>
> >         $(HOST_MESON_PKGDIR)/cross-compilation.conf.in \
> >         > $(HOST_DIR)/etc/meson/cross-compilation.conf
>
> Here as well, but this one is not an issue: the per-package host
> directory of host-meson will be rsync'ed into the global host directory
> at the end of the build.
>
> > Are there any variables in there that would change value?
>
> With per-package enabled, HOST_DIR, STAGING_DIR and TARGET_DIR have a
> different value when building each package (they point to the current
> package per-package directories) or when building stuff outside of a
> package (they point to the global directories).
>
> > In sequential build, the file before and after this change is identical.
>
> It's not so much sequential vs. parallel, but per-package directories
> or not. Per-package directory can be used while doing a sequential
> build.
>

Thanks for explaining everything.

If I understand it correctly, since the patch is hooking into
TOOLCHAIN_POST_INSTALL_STAGING_HOOKS, HOST_DIR will be the per-package
directory of 'toolchain', not of 'host-meson', right?

To solve it, we could change:
-        -e "s%@HOST_DIR@%$(HOST_DIR)%g" \
+        -e "s%@HOST_DIR@%$(call qstrip,$(BR2_HOST_DIR))%g" \

Is that correct?
And do you consider this acceptable?

Thanks,
Thomas
Thomas Petazzoni March 17, 2019, 3:23 p.m. | #5
Hello,

On Mon, 25 Feb 2019 22:11:46 +0100
Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote:
> Both files have a similar concept, they describe some flags/paths needed for
> compilation using respective build systems. One difference is that the meson
> file is added for external compilation, from the SDK, while the cmake file
> is used internally in Buildroot.
> 
> The 'problem' of using TARGET_FINALIZE_HOOKS for the meson file, is that it
> installs a 'host' file from target-finalize, which is conceptually incorrect
> and breaks the invariant that only TARGET_DIR is changed on a subsequent
> 'make' when everything was already built (i.e. only target-finalize is run).

In fact, I don't quite get what this commit is fixing. I don't really
understand what you call the "invariant that only TARGET_DIR is changed
on a subsequent make when everything was already built".

The current situation is not ideal because we create a file in
$(HOST_DIR) from target-finalize, but the proposed situation is also
not ideal because we create a file in $(HOST_DIR) from the staging
installation of a package. So I'd need to understand why the second
option has some advantages over the first, and that is currently
unclear.

Is it just for consistency for how the corresponding cmake file is
created ?

Best regards,

Thomas

Patch

diff --git a/package/meson/meson.mk b/package/meson/meson.mk
index d76541cc93..aba7382cc9 100644
--- a/package/meson/meson.mk
+++ b/package/meson/meson.mk
@@ -65,6 +65,7 @@  define HOST_MESON_INSTALL_CROSS_CONF
 	    > $(HOST_DIR)/etc/meson/cross-compilation.conf
 endef
 
-TARGET_FINALIZE_HOOKS += HOST_MESON_INSTALL_CROSS_CONF
+TOOLCHAIN_POST_INSTALL_STAGING_HOOKS += HOST_MESON_INSTALL_CROSS_CONF
+TOOLCHAIN_INSTALL_STAGING = YES
 
 $(eval $(host-python-package))