| Message ID | 20190225211147.14947-2-patrickdepinguin@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | misc fixes for 2019.02 | expand |
>>>>> "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?
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
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
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
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
El dom., 17 mar. 2019 a las 16:23, Thomas Petazzoni (<thomas.petazzoni@bootlin.com>) escribió: > > 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. When Buildroot 'make' has completed successfully, and you run 'make' without changes a second time, the only thing currently happening again is target-finalize. As the name suggests, it should only touch the TARGET_DIR. This is a principle I think we should keep. The generation of the toolchain file for meson violated this, by touching a host file from target-finalize, and thus also causing changes to something else than the TARGET_DIR after a 'make' without changes. In fact, there is totally no need to regenerate this 'static' file more than once. The solution I proposed fixes the described problem, but indeed uses another apparent violation (present before in other places): touching a host file from a staging step. Strictly speaking STAGING_DIR is inside HOST_DIR, but conceptually they are different indeed. This is actually caused by the fact that the toolchain step does not actually have a 'host' installation step, currently (because it is not a 'host' package. Therefore, here and in other places, the 'staging' step is used instead. > > Is it just for consistency for how the corresponding cmake file is > created ? I used the cmake thing as an example because it is very similar. It makes sense to align, but alignment itself is not the goal. Thanks, Thomas
On 25/02/2019 22:11, Thomas De Schampheleire wrote: > 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> Applied to master (with a bit more text as proposed), thanks. > --- > 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 check-package now complains that you're setting a variable that doesn't start with MESON_, so I moved this to pkg-meson.mk (like for cmake). > +TOOLCHAIN_INSTALL_STAGING = YES It would make sense to move this to toolchain.mk. I'll send a patch for that. Regards, Arnout
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))