| Message ID | 20220722060936.566534-1-james.hilliard1@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v3,1/1] package/pkg-meson: use meson to build/install packages | expand |
James, All, On 2022-07-22 00:09 -0600, James Hilliard spake thusly: > As of version 0.54.0 meson has had the ability to build and install > packages rather than having to run ninja directly as before. > > This will allow us to use features such as meson install tags in > the future which require meson to be used for the installation. > > It appears we need to ensure the cmake prefix path is set for the > meson generated relocatable pkg-config format to work properly. > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > --- [--SNIP--] > diff --git a/docs/manual/adding-packages-meson.txt b/docs/manual/adding-packages-meson.txt > index 029c8c2488..c8aed65c2f 100644 > --- a/docs/manual/adding-packages-meson.txt > +++ b/docs/manual/adding-packages-meson.txt > @@ -125,8 +125,8 @@ will therefore only use a few of them. > +c_link_args+, +cpp_args+, +cpp_link_args+, +sys_root+, and > +pkg_config_libdir+. > > -* +FOO_NINJA_ENV+, to specify additional environment variables to pass to > - +ninja+, meson companion tool in charge of the build operations. By default, > +* +FOO_MESON_ENV+, to specify additional environment variables to pass to > + +meson+, meson tool in charge of the build/install operations. By default, > empty. IIRC, the topic of what to do about external packages that set their FOO_NINJA_ENV, was raised in a previous iteration. I understand that the conclusion was basically "the probability is low, we don't handle it". This should have been noted in the commit log. > * +FOO_NINJA_OPTS+, to specify a space-separated list of targets to build. By Now that we use meson to build and instal, does it still make sense to have FOO_NINJA_OPTS to be the list of targets to install? I mean, isn't there a way to tell meson what to install, instead? [--SNIP--] > diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk > index 3b1db35fb6..4fa620080d 100644 > --- a/package/pkg-cmake.mk > +++ b/package/pkg-cmake.mk > @@ -90,6 +90,8 @@ define $(2)_CONFIGURE_CMDS > $$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \ > -DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/share/buildroot/toolchainfile.cmake" \ > -DCMAKE_INSTALL_PREFIX="/usr" \ > + -DCMAKE_PREFIX_PATH="$$(STAGING_DIR)/usr" \ > + -DPKG_CONFIG_USE_CMAKE_PREFIX_PATH=ON \ But doesn't that needs to be in a separate patch? I mean, does it not make sense opn its own? [--SNIP--] > diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk > index 0835e08e3a..de5817d5da 100644 > --- a/package/pkg-meson.mk > +++ b/package/pkg-meson.mk [--SNIP--] > @@ -138,6 +138,7 @@ define $(2)_CONFIGURE_CMDS > $$(MESON) \ > --prefix=/usr \ > --libdir=lib \ > + --pkgconfig.relocatable \ Please explain what that means and why this is needed in the context of a target package. > --default-library=$(if $(BR2_STATIC_LIBS),static,shared) \ > --buildtype=$(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release) \ > --cross-file=$$($$(PKG)_SRCDIR)/build/cross-compilation.conf \ > @@ -156,10 +157,11 @@ define $(2)_CONFIGURE_CMDS > mkdir -p $$($$(PKG)_SRCDIR)/build > $$(HOST_CONFIGURE_OPTS) \ > $$($$(PKG)_CONF_ENV) $$(MESON) \ > - --prefix=$$(HOST_DIR) \ > + --prefix=/ \ It is very weird that host packages are configured with / as prefix, and then installed in HOST_DIR set as DEST_DIR. I guess this is working thanks to --pkgconfig.relocatable a few lines below? This must be explained in the commit log (and probably a little comment above $(2)_CONFIGURE_CMDS for host packages. > --libdir=lib \ > - --sysconfdir=$$(HOST_DIR)/etc \ > - --localstatedir=$$(HOST_DIR)/var \ > + --pkgconfig.relocatable \ > + --sysconfdir=etc \ > + --localstatedir=var \ Ditto for etc and var? So, to make it easier to follow, I'd change the ordering: --pkgconfig.relocatable \ --prefix=/ \ --sysconfdir=etc \ --localstatedir=var \ Regards, Yann E. MORIN.
On 24/07/2022 17:37, Yann E. MORIN wrote: > James, All, > > On 2022-07-22 00:09 -0600, James Hilliard spake thusly: >> As of version 0.54.0 meson has had the ability to build and install >> packages rather than having to run ninja directly as before. >> >> This will allow us to use features such as meson install tags in >> the future which require meson to be used for the installation. >> >> It appears we need to ensure the cmake prefix path is set for the >> meson generated relocatable pkg-config format to work properly. >> >> Signed-off-by: James Hilliard <james.hilliard1@gmail.com> >> --- > [--SNIP--] >> diff --git a/docs/manual/adding-packages-meson.txt b/docs/manual/adding-packages-meson.txt >> index 029c8c2488..c8aed65c2f 100644 >> --- a/docs/manual/adding-packages-meson.txt >> +++ b/docs/manual/adding-packages-meson.txt >> @@ -125,8 +125,8 @@ will therefore only use a few of them. >> +c_link_args+, +cpp_args+, +cpp_link_args+, +sys_root+, and >> +pkg_config_libdir+. >> >> -* +FOO_NINJA_ENV+, to specify additional environment variables to pass to >> - +ninja+, meson companion tool in charge of the build operations. By default, >> +* +FOO_MESON_ENV+, to specify additional environment variables to pass to >> + +meson+, meson tool in charge of the build/install operations. By default, >> empty. > > IIRC, the topic of what to do about external packages that set their > FOO_NINJA_ENV, was raised in a previous iteration. > > I understand that the conclusion was basically "the probability is low, > we don't handle it". This should have been noted in the commit log. > >> * +FOO_NINJA_OPTS+, to specify a space-separated list of targets to build. By > > Now that we use meson to build and instal, does it still make sense to > have FOO_NINJA_OPTS to be the list of targets to install? > > I mean, isn't there a way to tell meson what to install, instead? FOO_NINJA_OPTS could theoretically be used to control other things about ninja, e.g. warnings and debugging. Though I don't really see why you'd ever want to do that in a buildroot package. > > [--SNIP--] >> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk >> index 3b1db35fb6..4fa620080d 100644 >> --- a/package/pkg-cmake.mk >> +++ b/package/pkg-cmake.mk >> @@ -90,6 +90,8 @@ define $(2)_CONFIGURE_CMDS >> $$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \ >> -DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/share/buildroot/toolchainfile.cmake" \ >> -DCMAKE_INSTALL_PREFIX="/usr" \ >> + -DCMAKE_PREFIX_PATH="$$(STAGING_DIR)/usr" \ Wouldn't it be better to put this in the toolchainfile? >> + -DPKG_CONFIG_USE_CMAKE_PREFIX_PATH=ON \ This one as well, I guess. > > But doesn't that needs to be in a separate patch? I mean, does it not > make sense opn its own? +1. The commit message of that patch could also explain better what it does exactly. > > [--SNIP--] >> diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk >> index 0835e08e3a..de5817d5da 100644 >> --- a/package/pkg-meson.mk >> +++ b/package/pkg-meson.mk > [--SNIP--] >> @@ -138,6 +138,7 @@ define $(2)_CONFIGURE_CMDS >> $$(MESON) \ >> --prefix=/usr \ >> --libdir=lib \ >> + --pkgconfig.relocatable \ > > Please explain what that means and why this is needed in the context of > a target package. And also it should probably be a separate patch - pkgconfig stuff should be done at configure time, not at build time, so using meson to start the build shouldn't really be related to it. > >> --default-library=$(if $(BR2_STATIC_LIBS),static,shared) \ >> --buildtype=$(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release) \ >> --cross-file=$$($$(PKG)_SRCDIR)/build/cross-compilation.conf \ >> @@ -156,10 +157,11 @@ define $(2)_CONFIGURE_CMDS >> mkdir -p $$($$(PKG)_SRCDIR)/build >> $$(HOST_CONFIGURE_OPTS) \ >> $$($$(PKG)_CONF_ENV) $$(MESON) \ >> - --prefix=$$(HOST_DIR) \ >> + --prefix=/ \ > > It is very weird that host packages are configured with / as prefix, and > then installed in HOST_DIR set as DEST_DIR. It's not weird, it's wrong! There is no reasonable explanation how this can be correct. We are really installing stuff in that specific location, any paths should refer to that location and not a path beginning with /. If for some reason meson doesn't work when the prefix is not / or /usr or /usr/local, it's a bug in meson, really. Regards, Arnout > > I guess this is working thanks to --pkgconfig.relocatable a few lines > below? > > This must be explained in the commit log (and probably a little comment > above $(2)_CONFIGURE_CMDS for host packages. > >> --libdir=lib \ >> - --sysconfdir=$$(HOST_DIR)/etc \ >> - --localstatedir=$$(HOST_DIR)/var \ >> + --pkgconfig.relocatable \ >> + --sysconfdir=etc \ >> + --localstatedir=var \ These two could be correct, however, since I think they're taken relative to prefix (the default values are absolute because the default prefix is /usr and sysconfdir is /etc, not /usr/etc - but in our case a relative path is fine). Regards, Arnout > > Ditto for etc and var? > > So, to make it easier to follow, I'd change the ordering: > > --pkgconfig.relocatable \ > --prefix=/ \ > --sysconfdir=etc \ > --localstatedir=var \ > > Regards, > Yann E. MORIN. >
On 22/07/2022 08:09, James Hilliard wrote: > As of version 0.54.0 meson has had the ability to build and install > packages rather than having to run ninja directly as before. > > This will allow us to use features such as meson install tags in > the future which require meson to be used for the installation. > > It appears we need to ensure the cmake prefix path is set for the > meson generated relocatable pkg-config format to work properly. > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > --- > Changes v2 -> v3: > - set pkgconfig to relocatable > - set cmake pkg-config prefix path > Changes v1 -> v2: > - update docs > --- > docs/manual/adding-packages-meson.txt | 4 +- > .../gobject-introspection.mk | 4 +- > package/pkg-cmake.mk | 4 ++ > package/pkg-meson.mk | 61 +++++++++++++------ > package/systemd/systemd.mk | 4 +- > 5 files changed, 53 insertions(+), 24 deletions(-) > > diff --git a/docs/manual/adding-packages-meson.txt b/docs/manual/adding-packages-meson.txt > index 029c8c2488..c8aed65c2f 100644 > --- a/docs/manual/adding-packages-meson.txt > +++ b/docs/manual/adding-packages-meson.txt > @@ -125,8 +125,8 @@ will therefore only use a few of them. > +c_link_args+, +cpp_args+, +cpp_link_args+, +sys_root+, and > +pkg_config_libdir+. > > -* +FOO_NINJA_ENV+, to specify additional environment variables to pass to > - +ninja+, meson companion tool in charge of the build operations. By default, > +* +FOO_MESON_ENV+, to specify additional environment variables to pass to If you're anyway respinning and anyway changing this, I have one more small nit: I think it would be better to name this FOO_BUILD_ENV and also add separate FOO_INSTALL_TARGET_ENV etc. This is different from how we currently do it in other infras, but (some) other infras *do* use e.g. FOO_BUILD_OPTS. The thing with FOO_MESON_ENV is that it suggests that it would also (and mainly) be applied to the configure step, and that is not the case. Regards, Arnout > + +meson+, meson tool in charge of the build/install operations. By default, > empty. > > * +FOO_NINJA_OPTS+, to specify a space-separated list of targets to build. By > diff --git a/package/gobject-introspection/gobject-introspection.mk b/package/gobject-introspection/gobject-introspection.mk > index 41d64171a7..ea5100247d 100644 > --- a/package/gobject-introspection/gobject-introspection.mk > +++ b/package/gobject-introspection/gobject-introspection.mk > @@ -30,14 +30,14 @@ HOST_GOBJECT_INTROSPECTION_DEPENDENCIES = \ > host-python3 > > # g-ir-scanner will default to /usr/bin/ld for linking if this is not set. > -GOBJECT_INTROSPECTION_NINJA_ENV += \ > +GOBJECT_INTROSPECTION_MESON_ENV += \ > CC="$(TARGET_CC)" > > # When building, gobject-introspection uses tools/g-ir-scanner to build several > # .gir and .typelib files. g-ir-scanner does not use LDFLAGS, and by default, > # links to the system-installed libglib2 path. To remedy this issue, defining > # LD_LIBRARY_PATH forces g-ir-scanner to use our host installed libglib2 files. > -HOST_GOBJECT_INTROSPECTION_NINJA_ENV += \ > +HOST_GOBJECT_INTROSPECTION_MESON_ENV += \ > LD_LIBRARY_PATH="$(if $(LD_LIBRARY_PATH),$(LD_LIBRARY_PATH):)$(HOST_DIR)/lib" > > # Use the host gi-scanner to prevent the scanner from generating incorrect > diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk > index 3b1db35fb6..4fa620080d 100644 > --- a/package/pkg-cmake.mk > +++ b/package/pkg-cmake.mk > @@ -90,6 +90,8 @@ define $(2)_CONFIGURE_CMDS > $$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \ > -DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/share/buildroot/toolchainfile.cmake" \ > -DCMAKE_INSTALL_PREFIX="/usr" \ > + -DCMAKE_PREFIX_PATH="$$(STAGING_DIR)/usr" \ > + -DPKG_CONFIG_USE_CMAKE_PREFIX_PATH=ON \ > -DCMAKE_COLOR_MAKEFILE=OFF \ > -DBUILD_DOC=OFF \ > -DBUILD_DOCS=OFF \ > @@ -123,6 +125,8 @@ define $(2)_CONFIGURE_CMDS > -DCMAKE_FIND_ROOT_PATH_MODE_LIBRARY="BOTH" \ > -DCMAKE_FIND_ROOT_PATH_MODE_INCLUDE="BOTH" \ > -DCMAKE_INSTALL_PREFIX="$$(HOST_DIR)" \ > + -DCMAKE_PREFIX_PATH="$$(HOST_DIR)" \ > + -DPKG_CONFIG_USE_CMAKE_PREFIX_PATH=ON \ > -DCMAKE_C_FLAGS="$$(HOST_CFLAGS)" \ > -DCMAKE_CXX_FLAGS="$$(HOST_CXXFLAGS)" \ > -DCMAKE_EXE_LINKER_FLAGS="$$(HOST_LDFLAGS)" \ > diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk > index 0835e08e3a..de5817d5da 100644 > --- a/package/pkg-meson.mk > +++ b/package/pkg-meson.mk > @@ -21,13 +21,13 @@ > ################################################################################ > > # > -# Pass PYTHONNOUSERSITE environment variable when invoking Meson or Ninja, so > +# Pass PYTHONNOUSERSITE environment variable when invoking Meson, so > # $(HOST_DIR)/bin/python3 will not look for Meson modules in > # $HOME/.local/lib/python3.x/site-packages > # > -MESON = PYTHONNOUSERSITE=y $(HOST_DIR)/bin/meson > -NINJA = PYTHONNOUSERSITE=y $(HOST_DIR)/bin/ninja > -NINJA_OPTS = $(if $(VERBOSE),-v) > +MESON = PYTHONNOUSERSITE=y $(HOST_DIR)/bin/meson > +MESON_BUILD_OPTS = $(if $(VERBOSE),-v) > +MESON_INSTALL_OPTS = --no-rebuild > > # https://mesonbuild.com/Reference-tables.html#cpu-families > ifeq ($(BR2_arcle)$(BR2_arceb),y) > @@ -138,6 +138,7 @@ define $(2)_CONFIGURE_CMDS > $$(MESON) \ > --prefix=/usr \ > --libdir=lib \ > + --pkgconfig.relocatable \ > --default-library=$(if $(BR2_STATIC_LIBS),static,shared) \ > --buildtype=$(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release) \ > --cross-file=$$($$(PKG)_SRCDIR)/build/cross-compilation.conf \ > @@ -156,10 +157,11 @@ define $(2)_CONFIGURE_CMDS > mkdir -p $$($$(PKG)_SRCDIR)/build > $$(HOST_CONFIGURE_OPTS) \ > $$($$(PKG)_CONF_ENV) $$(MESON) \ > - --prefix=$$(HOST_DIR) \ > + --prefix=/ \ > --libdir=lib \ > - --sysconfdir=$$(HOST_DIR)/etc \ > - --localstatedir=$$(HOST_DIR)/var \ > + --pkgconfig.relocatable \ > + --sysconfdir=etc \ > + --localstatedir=var \ > --default-library=shared \ > --buildtype=release \ > --wrap-mode=nodownload \ > @@ -179,13 +181,23 @@ $(2)_DEPENDENCIES += host-meson > ifndef $(2)_BUILD_CMDS > ifeq ($(4),target) > define $(2)_BUILD_CMDS > - $$(TARGET_MAKE_ENV) $$($$(PKG)_NINJA_ENV) \ > - $$(NINJA) $$(NINJA_OPTS) $$($$(PKG)_NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build > + $$(TARGET_MAKE_ENV) \ > + $$($$(PKG)_MESON_ENV) \ > + $$(MESON) \ > + compile \ > + $$(MESON_BUILD_OPTS) \ > + $$(if $$($$(PKG)_NINJA_OPTS),--ninja-args $$($$(PKG)_NINJA_OPTS)) \ > + -C $$($$(PKG)_SRCDIR)/build > endef > else > define $(2)_BUILD_CMDS > - $$(HOST_MAKE_ENV) $$($$(PKG)_NINJA_ENV) \ > - $$(NINJA) $$(NINJA_OPTS) $$($$(PKG)_NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build > + $$(HOST_MAKE_ENV) \ > + $$($$(PKG)_MESON_ENV) \ > + $$(MESON) \ > + compile \ > + $$(MESON_BUILD_OPTS) \ > + $$(if $$($$(PKG)_NINJA_OPTS),--ninja-args $$($$(PKG)_NINJA_OPTS)) \ > + -C $$($$(PKG)_SRCDIR)/build > endef > endif > endif > @@ -196,8 +208,13 @@ endif > # > ifndef $(2)_INSTALL_CMDS > define $(2)_INSTALL_CMDS > - $$(HOST_MAKE_ENV) $$($$(PKG)_NINJA_ENV) \ > - $$(NINJA) $$(NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build install > + $$(HOST_MAKE_ENV) \ > + $$($$(PKG)_MESON_ENV) \ > + $$(MESON) \ > + install \ > + $$(MESON_INSTALL_OPTS) \ > + --destdir $$(HOST_DIR) \ > + -C $$($$(PKG)_SRCDIR)/build > endef > endif > > @@ -207,8 +224,13 @@ endif > # > ifndef $(2)_INSTALL_STAGING_CMDS > define $(2)_INSTALL_STAGING_CMDS > - $$(TARGET_MAKE_ENV) $$($$(PKG)_NINJA_ENV) DESTDIR=$$(STAGING_DIR) \ > - $$(NINJA) $$(NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build install > + $$(TARGET_MAKE_ENV) \ > + $$($$(PKG)_MESON_ENV) \ > + $$(MESON) \ > + install \ > + $$(MESON_INSTALL_OPTS) \ > + --destdir $$(STAGING_DIR) \ > + -C $$($$(PKG)_SRCDIR)/build > endef > endif > > @@ -218,8 +240,13 @@ endif > # > ifndef $(2)_INSTALL_TARGET_CMDS > define $(2)_INSTALL_TARGET_CMDS > - $$(TARGET_MAKE_ENV) $$($$(PKG)_NINJA_ENV) DESTDIR=$$(TARGET_DIR) \ > - $$(NINJA) $$(NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build install > + $$(TARGET_MAKE_ENV) \ > + $$($$(PKG)_MESON_ENV) \ > + $$(MESON) \ > + install \ > + $$(MESON_INSTALL_OPTS) \ > + --destdir $$(TARGET_DIR) \ > + -C $$($$(PKG)_SRCDIR)/build > endef > endif > > diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk > index 46a4e8de2c..9bb87f8e39 100644 > --- a/package/systemd/systemd.mk > +++ b/package/systemd/systemd.mk > @@ -746,7 +746,7 @@ endef > SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_PRESET_ALL > > SYSTEMD_CONF_ENV = $(HOST_UTF8_LOCALE_ENV) > -SYSTEMD_NINJA_ENV = $(HOST_UTF8_LOCALE_ENV) > +SYSTEMD_MESON_ENV = $(HOST_UTF8_LOCALE_ENV) > > define SYSTEMD_LINUX_CONFIG_FIXUPS > $(call KCONFIG_ENABLE_OPT,CONFIG_DEVTMPFS) > @@ -849,8 +849,6 @@ HOST_SYSTEMD_DEPENDENCIES = \ > host-gperf \ > host-python-jinja2 > > -HOST_SYSTEMD_NINJA_ENV = DESTDIR=$(HOST_DIR) > - > # Fix RPATH After installation > # * systemd provides a install_rpath instruction to meson because the binaries > # need to link with libsystemd which is not in a standard path
On Sun, Jul 24, 2022 at 1:30 PM Arnout Vandecappelle <arnout@mind.be> wrote: > > > > On 24/07/2022 17:37, Yann E. MORIN wrote: > > James, All, > > > > On 2022-07-22 00:09 -0600, James Hilliard spake thusly: > >> As of version 0.54.0 meson has had the ability to build and install > >> packages rather than having to run ninja directly as before. > >> > >> This will allow us to use features such as meson install tags in > >> the future which require meson to be used for the installation. > >> > >> It appears we need to ensure the cmake prefix path is set for the > >> meson generated relocatable pkg-config format to work properly. > >> > >> Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > >> --- > > [--SNIP--] > >> diff --git a/docs/manual/adding-packages-meson.txt b/docs/manual/adding-packages-meson.txt > >> index 029c8c2488..c8aed65c2f 100644 > >> --- a/docs/manual/adding-packages-meson.txt > >> +++ b/docs/manual/adding-packages-meson.txt > >> @@ -125,8 +125,8 @@ will therefore only use a few of them. > >> +c_link_args+, +cpp_args+, +cpp_link_args+, +sys_root+, and > >> +pkg_config_libdir+. > >> > >> -* +FOO_NINJA_ENV+, to specify additional environment variables to pass to > >> - +ninja+, meson companion tool in charge of the build operations. By default, > >> +* +FOO_MESON_ENV+, to specify additional environment variables to pass to > >> + +meson+, meson tool in charge of the build/install operations. By default, > >> empty. > > > > IIRC, the topic of what to do about external packages that set their > > FOO_NINJA_ENV, was raised in a previous iteration. > > > > I understand that the conclusion was basically "the probability is low, > > we don't handle it". This should have been noted in the commit log. Added a note in v4 commit log. > > > >> * +FOO_NINJA_OPTS+, to specify a space-separated list of targets to build. By > > > > Now that we use meson to build and instal, does it still make sense to > > have FOO_NINJA_OPTS to be the list of targets to install? > > > > I mean, isn't there a way to tell meson what to install, instead? > > FOO_NINJA_OPTS could theoretically be used to control other things about > ninja, e.g. warnings and debugging. Though I don't really see why you'd ever > want to do that in a buildroot package. There might be some use cases in which it makes sense to use it to control what gets built...I figured best just keep it for now. > > > > > [--SNIP--] > >> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk > >> index 3b1db35fb6..4fa620080d 100644 > >> --- a/package/pkg-cmake.mk > >> +++ b/package/pkg-cmake.mk > >> @@ -90,6 +90,8 @@ define $(2)_CONFIGURE_CMDS > >> $$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \ > >> -DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/share/buildroot/toolchainfile.cmake" \ > >> -DCMAKE_INSTALL_PREFIX="/usr" \ > >> + -DCMAKE_PREFIX_PATH="$$(STAGING_DIR)/usr" \ > > Wouldn't it be better to put this in the toolchainfile? So it seems we just need to remove this from the toolchainfile instead actually: https://patchwork.ozlabs.org/project/buildroot/patch/20220726044425.72570-1-james.hilliard1@gmail.com/ > > >> + -DPKG_CONFIG_USE_CMAKE_PREFIX_PATH=ON \ > > This one as well, I guess. > > > > > But doesn't that needs to be in a separate patch? I mean, does it not > > make sense opn its own? > > +1. > > The commit message of that patch could also explain better what it does exactly. > > > > > [--SNIP--] > >> diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk > >> index 0835e08e3a..de5817d5da 100644 > >> --- a/package/pkg-meson.mk > >> +++ b/package/pkg-meson.mk > > [--SNIP--] > >> @@ -138,6 +138,7 @@ define $(2)_CONFIGURE_CMDS > >> $$(MESON) \ > >> --prefix=/usr \ > >> --libdir=lib \ > >> + --pkgconfig.relocatable \ > > > > Please explain what that means and why this is needed in the context of > > a target package. > > And also it should probably be a separate patch - pkgconfig stuff should be > done at configure time, not at build time, so using meson to start the build > shouldn't really be related to it. This is inside of $(2)_CONFIGURE_CMDS so it is done at configure time AFAIU. > > > > >> --default-library=$(if $(BR2_STATIC_LIBS),static,shared) \ > >> --buildtype=$(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release) \ > >> --cross-file=$$($$(PKG)_SRCDIR)/build/cross-compilation.conf \ > >> @@ -156,10 +157,11 @@ define $(2)_CONFIGURE_CMDS > >> mkdir -p $$($$(PKG)_SRCDIR)/build > >> $$(HOST_CONFIGURE_OPTS) \ > >> $$($$(PKG)_CONF_ENV) $$(MESON) \ > >> - --prefix=$$(HOST_DIR) \ > >> + --prefix=/ \ > > > > It is very weird that host packages are configured with / as prefix, and > > then installed in HOST_DIR set as DEST_DIR. > > It's not weird, it's wrong! There is no reasonable explanation how this can be > correct. We are really installing stuff in that specific location, any paths > should refer to that location and not a path beginning with /. Well from my understanding when using relocatable mode/meson install the prefix is appended to the destdir similar to how it works for target/staging directories. > > If for some reason meson doesn't work when the prefix is not / or /usr or > /usr/local, it's a bug in meson, really. I think the issue is in how prefix and destdir get combined. > > Regards, > Arnout > > > > > I guess this is working thanks to --pkgconfig.relocatable a few lines > > below? > > > > This must be explained in the commit log (and probably a little comment > > above $(2)_CONFIGURE_CMDS for host packages. > > > >> --libdir=lib \ > >> - --sysconfdir=$$(HOST_DIR)/etc \ > >> - --localstatedir=$$(HOST_DIR)/var \ > >> + --pkgconfig.relocatable \ > >> + --sysconfdir=etc \ > >> + --localstatedir=var \ > > These two could be correct, however, since I think they're taken relative to > prefix (the default values are absolute because the default prefix is /usr and > sysconfdir is /etc, not /usr/etc - but in our case a relative path is fine). > > Regards, > Arnout > > > > > Ditto for etc and var? > > > > So, to make it easier to follow, I'd change the ordering: > > > > --pkgconfig.relocatable \ > > --prefix=/ \ > > --sysconfdir=etc \ > > --localstatedir=var \ Reordered in v4. > > > > Regards, > > Yann E. MORIN. > >
On Sun, Jul 24, 2022 at 1:42 PM Arnout Vandecappelle <arnout@mind.be> wrote: > > > > On 22/07/2022 08:09, James Hilliard wrote: > > As of version 0.54.0 meson has had the ability to build and install > > packages rather than having to run ninja directly as before. > > > > This will allow us to use features such as meson install tags in > > the future which require meson to be used for the installation. > > > > It appears we need to ensure the cmake prefix path is set for the > > meson generated relocatable pkg-config format to work properly. > > > > Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > > --- > > Changes v2 -> v3: > > - set pkgconfig to relocatable > > - set cmake pkg-config prefix path > > Changes v1 -> v2: > > - update docs > > --- > > docs/manual/adding-packages-meson.txt | 4 +- > > .../gobject-introspection.mk | 4 +- > > package/pkg-cmake.mk | 4 ++ > > package/pkg-meson.mk | 61 +++++++++++++------ > > package/systemd/systemd.mk | 4 +- > > 5 files changed, 53 insertions(+), 24 deletions(-) > > > > diff --git a/docs/manual/adding-packages-meson.txt b/docs/manual/adding-packages-meson.txt > > index 029c8c2488..c8aed65c2f 100644 > > --- a/docs/manual/adding-packages-meson.txt > > +++ b/docs/manual/adding-packages-meson.txt > > @@ -125,8 +125,8 @@ will therefore only use a few of them. > > +c_link_args+, +cpp_args+, +cpp_link_args+, +sys_root+, and > > +pkg_config_libdir+. > > > > -* +FOO_NINJA_ENV+, to specify additional environment variables to pass to > > - +ninja+, meson companion tool in charge of the build operations. By default, > > +* +FOO_MESON_ENV+, to specify additional environment variables to pass to > > If you're anyway respinning and anyway changing this, I have one more small > nit: I think it would be better to name this FOO_BUILD_ENV and also add separate > FOO_INSTALL_TARGET_ENV etc. This is different from how we currently do it in > other infras, but (some) other infras *do* use e.g. FOO_BUILD_OPTS. The thing > with FOO_MESON_ENV is that it suggests that it would also (and mainly) be > applied to the configure step, and that is not the case. Hmm, maybe we want FOO_MESON_ENV to apply to all steps so that we don't have to set things separately between build/conf like this?: https://github.com/buildroot/buildroot/blob/2022.05.1/package/systemd/systemd.mk#L748-L749 > > Regards, > Arnout > > > > + +meson+, meson tool in charge of the build/install operations. By default, > > empty. > > > > * +FOO_NINJA_OPTS+, to specify a space-separated list of targets to build. By > > diff --git a/package/gobject-introspection/gobject-introspection.mk b/package/gobject-introspection/gobject-introspection.mk > > index 41d64171a7..ea5100247d 100644 > > --- a/package/gobject-introspection/gobject-introspection.mk > > +++ b/package/gobject-introspection/gobject-introspection.mk > > @@ -30,14 +30,14 @@ HOST_GOBJECT_INTROSPECTION_DEPENDENCIES = \ > > host-python3 > > > > # g-ir-scanner will default to /usr/bin/ld for linking if this is not set. > > -GOBJECT_INTROSPECTION_NINJA_ENV += \ > > +GOBJECT_INTROSPECTION_MESON_ENV += \ > > CC="$(TARGET_CC)" > > > > # When building, gobject-introspection uses tools/g-ir-scanner to build several > > # .gir and .typelib files. g-ir-scanner does not use LDFLAGS, and by default, > > # links to the system-installed libglib2 path. To remedy this issue, defining > > # LD_LIBRARY_PATH forces g-ir-scanner to use our host installed libglib2 files. > > -HOST_GOBJECT_INTROSPECTION_NINJA_ENV += \ > > +HOST_GOBJECT_INTROSPECTION_MESON_ENV += \ > > LD_LIBRARY_PATH="$(if $(LD_LIBRARY_PATH),$(LD_LIBRARY_PATH):)$(HOST_DIR)/lib" > > > > # Use the host gi-scanner to prevent the scanner from generating incorrect > > diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk > > index 3b1db35fb6..4fa620080d 100644 > > --- a/package/pkg-cmake.mk > > +++ b/package/pkg-cmake.mk > > @@ -90,6 +90,8 @@ define $(2)_CONFIGURE_CMDS > > $$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \ > > -DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/share/buildroot/toolchainfile.cmake" \ > > -DCMAKE_INSTALL_PREFIX="/usr" \ > > + -DCMAKE_PREFIX_PATH="$$(STAGING_DIR)/usr" \ > > + -DPKG_CONFIG_USE_CMAKE_PREFIX_PATH=ON \ > > -DCMAKE_COLOR_MAKEFILE=OFF \ > > -DBUILD_DOC=OFF \ > > -DBUILD_DOCS=OFF \ > > @@ -123,6 +125,8 @@ define $(2)_CONFIGURE_CMDS > > -DCMAKE_FIND_ROOT_PATH_MODE_LIBRARY="BOTH" \ > > -DCMAKE_FIND_ROOT_PATH_MODE_INCLUDE="BOTH" \ > > -DCMAKE_INSTALL_PREFIX="$$(HOST_DIR)" \ > > + -DCMAKE_PREFIX_PATH="$$(HOST_DIR)" \ > > + -DPKG_CONFIG_USE_CMAKE_PREFIX_PATH=ON \ > > -DCMAKE_C_FLAGS="$$(HOST_CFLAGS)" \ > > -DCMAKE_CXX_FLAGS="$$(HOST_CXXFLAGS)" \ > > -DCMAKE_EXE_LINKER_FLAGS="$$(HOST_LDFLAGS)" \ > > diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk > > index 0835e08e3a..de5817d5da 100644 > > --- a/package/pkg-meson.mk > > +++ b/package/pkg-meson.mk > > @@ -21,13 +21,13 @@ > > ################################################################################ > > > > # > > -# Pass PYTHONNOUSERSITE environment variable when invoking Meson or Ninja, so > > +# Pass PYTHONNOUSERSITE environment variable when invoking Meson, so > > # $(HOST_DIR)/bin/python3 will not look for Meson modules in > > # $HOME/.local/lib/python3.x/site-packages > > # > > -MESON = PYTHONNOUSERSITE=y $(HOST_DIR)/bin/meson > > -NINJA = PYTHONNOUSERSITE=y $(HOST_DIR)/bin/ninja > > -NINJA_OPTS = $(if $(VERBOSE),-v) > > +MESON = PYTHONNOUSERSITE=y $(HOST_DIR)/bin/meson > > +MESON_BUILD_OPTS = $(if $(VERBOSE),-v) > > +MESON_INSTALL_OPTS = --no-rebuild > > > > # https://mesonbuild.com/Reference-tables.html#cpu-families > > ifeq ($(BR2_arcle)$(BR2_arceb),y) > > @@ -138,6 +138,7 @@ define $(2)_CONFIGURE_CMDS > > $$(MESON) \ > > --prefix=/usr \ > > --libdir=lib \ > > + --pkgconfig.relocatable \ > > --default-library=$(if $(BR2_STATIC_LIBS),static,shared) \ > > --buildtype=$(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release) \ > > --cross-file=$$($$(PKG)_SRCDIR)/build/cross-compilation.conf \ > > @@ -156,10 +157,11 @@ define $(2)_CONFIGURE_CMDS > > mkdir -p $$($$(PKG)_SRCDIR)/build > > $$(HOST_CONFIGURE_OPTS) \ > > $$($$(PKG)_CONF_ENV) $$(MESON) \ > > - --prefix=$$(HOST_DIR) \ > > + --prefix=/ \ > > --libdir=lib \ > > - --sysconfdir=$$(HOST_DIR)/etc \ > > - --localstatedir=$$(HOST_DIR)/var \ > > + --pkgconfig.relocatable \ > > + --sysconfdir=etc \ > > + --localstatedir=var \ > > --default-library=shared \ > > --buildtype=release \ > > --wrap-mode=nodownload \ > > @@ -179,13 +181,23 @@ $(2)_DEPENDENCIES += host-meson > > ifndef $(2)_BUILD_CMDS > > ifeq ($(4),target) > > define $(2)_BUILD_CMDS > > - $$(TARGET_MAKE_ENV) $$($$(PKG)_NINJA_ENV) \ > > - $$(NINJA) $$(NINJA_OPTS) $$($$(PKG)_NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build > > + $$(TARGET_MAKE_ENV) \ > > + $$($$(PKG)_MESON_ENV) \ > > + $$(MESON) \ > > + compile \ > > + $$(MESON_BUILD_OPTS) \ > > + $$(if $$($$(PKG)_NINJA_OPTS),--ninja-args $$($$(PKG)_NINJA_OPTS)) \ > > + -C $$($$(PKG)_SRCDIR)/build > > endef > > else > > define $(2)_BUILD_CMDS > > - $$(HOST_MAKE_ENV) $$($$(PKG)_NINJA_ENV) \ > > - $$(NINJA) $$(NINJA_OPTS) $$($$(PKG)_NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build > > + $$(HOST_MAKE_ENV) \ > > + $$($$(PKG)_MESON_ENV) \ > > + $$(MESON) \ > > + compile \ > > + $$(MESON_BUILD_OPTS) \ > > + $$(if $$($$(PKG)_NINJA_OPTS),--ninja-args $$($$(PKG)_NINJA_OPTS)) \ > > + -C $$($$(PKG)_SRCDIR)/build > > endef > > endif > > endif > > @@ -196,8 +208,13 @@ endif > > # > > ifndef $(2)_INSTALL_CMDS > > define $(2)_INSTALL_CMDS > > - $$(HOST_MAKE_ENV) $$($$(PKG)_NINJA_ENV) \ > > - $$(NINJA) $$(NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build install > > + $$(HOST_MAKE_ENV) \ > > + $$($$(PKG)_MESON_ENV) \ > > + $$(MESON) \ > > + install \ > > + $$(MESON_INSTALL_OPTS) \ > > + --destdir $$(HOST_DIR) \ > > + -C $$($$(PKG)_SRCDIR)/build > > endef > > endif > > > > @@ -207,8 +224,13 @@ endif > > # > > ifndef $(2)_INSTALL_STAGING_CMDS > > define $(2)_INSTALL_STAGING_CMDS > > - $$(TARGET_MAKE_ENV) $$($$(PKG)_NINJA_ENV) DESTDIR=$$(STAGING_DIR) \ > > - $$(NINJA) $$(NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build install > > + $$(TARGET_MAKE_ENV) \ > > + $$($$(PKG)_MESON_ENV) \ > > + $$(MESON) \ > > + install \ > > + $$(MESON_INSTALL_OPTS) \ > > + --destdir $$(STAGING_DIR) \ > > + -C $$($$(PKG)_SRCDIR)/build > > endef > > endif > > > > @@ -218,8 +240,13 @@ endif > > # > > ifndef $(2)_INSTALL_TARGET_CMDS > > define $(2)_INSTALL_TARGET_CMDS > > - $$(TARGET_MAKE_ENV) $$($$(PKG)_NINJA_ENV) DESTDIR=$$(TARGET_DIR) \ > > - $$(NINJA) $$(NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build install > > + $$(TARGET_MAKE_ENV) \ > > + $$($$(PKG)_MESON_ENV) \ > > + $$(MESON) \ > > + install \ > > + $$(MESON_INSTALL_OPTS) \ > > + --destdir $$(TARGET_DIR) \ > > + -C $$($$(PKG)_SRCDIR)/build > > endef > > endif > > > > diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk > > index 46a4e8de2c..9bb87f8e39 100644 > > --- a/package/systemd/systemd.mk > > +++ b/package/systemd/systemd.mk > > @@ -746,7 +746,7 @@ endef > > SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_PRESET_ALL > > > > SYSTEMD_CONF_ENV = $(HOST_UTF8_LOCALE_ENV) > > -SYSTEMD_NINJA_ENV = $(HOST_UTF8_LOCALE_ENV) > > +SYSTEMD_MESON_ENV = $(HOST_UTF8_LOCALE_ENV) > > > > define SYSTEMD_LINUX_CONFIG_FIXUPS > > $(call KCONFIG_ENABLE_OPT,CONFIG_DEVTMPFS) > > @@ -849,8 +849,6 @@ HOST_SYSTEMD_DEPENDENCIES = \ > > host-gperf \ > > host-python-jinja2 > > > > -HOST_SYSTEMD_NINJA_ENV = DESTDIR=$(HOST_DIR) > > - > > # Fix RPATH After installation > > # * systemd provides a install_rpath instruction to meson because the binaries > > # need to link with libsystemd which is not in a standard path
On 26/07/2022 06:59, James Hilliard wrote: > On Sun, Jul 24, 2022 at 1:30 PM Arnout Vandecappelle <arnout@mind.be> wrote: >> >> >> >> On 24/07/2022 17:37, Yann E. MORIN wrote: >>> James, All, >>> >>> On 2022-07-22 00:09 -0600, James Hilliard spake thusly: >>>> As of version 0.54.0 meson has had the ability to build and install >>>> packages rather than having to run ninja directly as before. >>>> >>>> This will allow us to use features such as meson install tags in >>>> the future which require meson to be used for the installation. >>>> >>>> It appears we need to ensure the cmake prefix path is set for the >>>> meson generated relocatable pkg-config format to work properly. >>>> >>>> Signed-off-by: James Hilliard <james.hilliard1@gmail.com> [snip] >>>> * +FOO_NINJA_OPTS+, to specify a space-separated list of targets to build. By >>> >>> Now that we use meson to build and instal, does it still make sense to >>> have FOO_NINJA_OPTS to be the list of targets to install? >>> >>> I mean, isn't there a way to tell meson what to install, instead? >> >> FOO_NINJA_OPTS could theoretically be used to control other things about >> ninja, e.g. warnings and debugging. Though I don't really see why you'd ever >> want to do that in a buildroot package. > > There might be some use cases in which it makes sense to use it to control > what gets built...I figured best just keep it for now. If it is used to control what gets built, then Yann's remark applies - it should be passed to meson itself, not to ninja. That is, assuming that "meson build" does have a way to specify what gets built. >>>> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk >>>> index 3b1db35fb6..4fa620080d 100644 >>>> --- a/package/pkg-cmake.mk >>>> +++ b/package/pkg-cmake.mk >>>> @@ -90,6 +90,8 @@ define $(2)_CONFIGURE_CMDS >>>> $$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \ >>>> -DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/share/buildroot/toolchainfile.cmake" \ >>>> -DCMAKE_INSTALL_PREFIX="/usr" \ >>>> + -DCMAKE_PREFIX_PATH="$$(STAGING_DIR)/usr" \ >> >> Wouldn't it be better to put this in the toolchainfile? > > So it seems we just need to remove this from the toolchainfile instead actually: > https://patchwork.ozlabs.org/project/buildroot/patch/20220726044425.72570-1-james.hilliard1@gmail.com/ The problem is, the toolchainfile is not only meant to be used in Buildroot itself, but also by external builds (using the SDK). The idea is that it should be sufficient to call cmake with an option pointing to the toolchain file to do a build externally. That's why we have the pkgconfig setting in there. >>>> + -DPKG_CONFIG_USE_CMAKE_PREFIX_PATH=ON \ >> >> This one as well, I guess. >> >>> >>> But doesn't that needs to be in a separate patch? I mean, does it not >>> make sense opn its own? >> >> +1. >> >> The commit message of that patch could also explain better what it does exactly. >> >>> >>> [--SNIP--] >>>> diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk >>>> index 0835e08e3a..de5817d5da 100644 >>>> --- a/package/pkg-meson.mk >>>> +++ b/package/pkg-meson.mk >>> [--SNIP--] >>>> @@ -138,6 +138,7 @@ define $(2)_CONFIGURE_CMDS >>>> $$(MESON) \ >>>> --prefix=/usr \ >>>> --libdir=lib \ >>>> + --pkgconfig.relocatable \ >>> >>> Please explain what that means and why this is needed in the context of >>> a target package. >> >> And also it should probably be a separate patch - pkgconfig stuff should be >> done at configure time, not at build time, so using meson to start the build >> shouldn't really be related to it. > > This is inside of $(2)_CONFIGURE_CMDS so it is done at configure time AFAIU. The patch is about "use meson to build/install packages". It is very weird that calling meson instead of ninja in the build step would require to do something different in the configure step. >>>> --default-library=$(if $(BR2_STATIC_LIBS),static,shared) \ >>>> --buildtype=$(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release) \ >>>> --cross-file=$$($$(PKG)_SRCDIR)/build/cross-compilation.conf \ >>>> @@ -156,10 +157,11 @@ define $(2)_CONFIGURE_CMDS >>>> mkdir -p $$($$(PKG)_SRCDIR)/build >>>> $$(HOST_CONFIGURE_OPTS) \ >>>> $$($$(PKG)_CONF_ENV) $$(MESON) \ >>>> - --prefix=$$(HOST_DIR) \ >>>> + --prefix=/ \ >>> >>> It is very weird that host packages are configured with / as prefix, and >>> then installed in HOST_DIR set as DEST_DIR. >> >> It's not weird, it's wrong! There is no reasonable explanation how this can be >> correct. We are really installing stuff in that specific location, any paths >> should refer to that location and not a path beginning with /. > > Well from my understanding when using relocatable mode/meson install the > prefix is appended to the destdir similar to how it works for target/staging > directories. As Thomas explained in the v4, it is not at all similar to target/staging. Regards, Arnout >> If for some reason meson doesn't work when the prefix is not / or /usr or >> /usr/local, it's a bug in meson, really. > > I think the issue is in how prefix and destdir get combined. > >> >> Regards, >> Arnout >> >>> >>> I guess this is working thanks to --pkgconfig.relocatable a few lines >>> below? >>> >>> This must be explained in the commit log (and probably a little comment >>> above $(2)_CONFIGURE_CMDS for host packages. >>> >>>> --libdir=lib \ >>>> - --sysconfdir=$$(HOST_DIR)/etc \ >>>> - --localstatedir=$$(HOST_DIR)/var \ >>>> + --pkgconfig.relocatable \ >>>> + --sysconfdir=etc \ >>>> + --localstatedir=var \ >> >> These two could be correct, however, since I think they're taken relative to >> prefix (the default values are absolute because the default prefix is /usr and >> sysconfdir is /etc, not /usr/etc - but in our case a relative path is fine). >> >> Regards, >> Arnout >> >>> >>> Ditto for etc and var? >>> >>> So, to make it easier to follow, I'd change the ordering: >>> >>> --pkgconfig.relocatable \ >>> --prefix=/ \ >>> --sysconfdir=etc \ >>> --localstatedir=var \ > > Reordered in v4. > >>> >>> Regards, >>> Yann E. MORIN. >>>
On Tue, Jul 26, 2022 at 1:31 AM Arnout Vandecappelle <arnout@mind.be> wrote: > > > > On 26/07/2022 06:59, James Hilliard wrote: > > On Sun, Jul 24, 2022 at 1:30 PM Arnout Vandecappelle <arnout@mind.be> wrote: > >> > >> > >> > >> On 24/07/2022 17:37, Yann E. MORIN wrote: > >>> James, All, > >>> > >>> On 2022-07-22 00:09 -0600, James Hilliard spake thusly: > >>>> As of version 0.54.0 meson has had the ability to build and install > >>>> packages rather than having to run ninja directly as before. > >>>> > >>>> This will allow us to use features such as meson install tags in > >>>> the future which require meson to be used for the installation. > >>>> > >>>> It appears we need to ensure the cmake prefix path is set for the > >>>> meson generated relocatable pkg-config format to work properly. > >>>> > >>>> Signed-off-by: James Hilliard <james.hilliard1@gmail.com> > [snip] > >>>> * +FOO_NINJA_OPTS+, to specify a space-separated list of targets to build. By > >>> > >>> Now that we use meson to build and instal, does it still make sense to > >>> have FOO_NINJA_OPTS to be the list of targets to install? > >>> > >>> I mean, isn't there a way to tell meson what to install, instead? > >> > >> FOO_NINJA_OPTS could theoretically be used to control other things about > >> ninja, e.g. warnings and debugging. Though I don't really see why you'd ever > >> want to do that in a buildroot package. > > > > There might be some use cases in which it makes sense to use it to control > > what gets built...I figured best just keep it for now. > > If it is used to control what gets built, then Yann's remark applies - it > should be passed to meson itself, not to ninja. That is, assuming that "meson > build" does have a way to specify what gets built. Well it's being passed by meson to ninja directly I think. > > > >>>> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk > >>>> index 3b1db35fb6..4fa620080d 100644 > >>>> --- a/package/pkg-cmake.mk > >>>> +++ b/package/pkg-cmake.mk > >>>> @@ -90,6 +90,8 @@ define $(2)_CONFIGURE_CMDS > >>>> $$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \ > >>>> -DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/share/buildroot/toolchainfile.cmake" \ > >>>> -DCMAKE_INSTALL_PREFIX="/usr" \ > >>>> + -DCMAKE_PREFIX_PATH="$$(STAGING_DIR)/usr" \ > >> > >> Wouldn't it be better to put this in the toolchainfile? > > > > So it seems we just need to remove this from the toolchainfile instead actually: > > https://patchwork.ozlabs.org/project/buildroot/patch/20220726044425.72570-1-james.hilliard1@gmail.com/ > > The problem is, the toolchainfile is not only meant to be used in Buildroot > itself, but also by external builds (using the SDK). The idea is that it should > be sufficient to call cmake with an option pointing to the toolchain file to do > a build externally. That's why we have the pkgconfig setting in there. This seems to work: https://patchwork.ozlabs.org/project/buildroot/patch/20220728014402.142320-1-james.hilliard1@gmail.com/ > > > >>>> + -DPKG_CONFIG_USE_CMAKE_PREFIX_PATH=ON \ > >> > >> This one as well, I guess. > >> > >>> > >>> But doesn't that needs to be in a separate patch? I mean, does it not > >>> make sense opn its own? > >> > >> +1. > >> > >> The commit message of that patch could also explain better what it does exactly. > >> > >>> > >>> [--SNIP--] > >>>> diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk > >>>> index 0835e08e3a..de5817d5da 100644 > >>>> --- a/package/pkg-meson.mk > >>>> +++ b/package/pkg-meson.mk > >>> [--SNIP--] > >>>> @@ -138,6 +138,7 @@ define $(2)_CONFIGURE_CMDS > >>>> $$(MESON) \ > >>>> --prefix=/usr \ > >>>> --libdir=lib \ > >>>> + --pkgconfig.relocatable \ > >>> > >>> Please explain what that means and why this is needed in the context of > >>> a target package. > >> > >> And also it should probably be a separate patch - pkgconfig stuff should be > >> done at configure time, not at build time, so using meson to start the build > >> shouldn't really be related to it. > > > > This is inside of $(2)_CONFIGURE_CMDS so it is done at configure time AFAIU. > > The patch is about "use meson to build/install packages". It is very weird > that calling meson instead of ninja in the build step would require to do > something different in the configure step. Well meson does do some extra stuff when using meson's install compared with just ninja, it also supports additional features like tag filtering during install. > > > >>>> --default-library=$(if $(BR2_STATIC_LIBS),static,shared) \ > >>>> --buildtype=$(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release) \ > >>>> --cross-file=$$($$(PKG)_SRCDIR)/build/cross-compilation.conf \ > >>>> @@ -156,10 +157,11 @@ define $(2)_CONFIGURE_CMDS > >>>> mkdir -p $$($$(PKG)_SRCDIR)/build > >>>> $$(HOST_CONFIGURE_OPTS) \ > >>>> $$($$(PKG)_CONF_ENV) $$(MESON) \ > >>>> - --prefix=$$(HOST_DIR) \ > >>>> + --prefix=/ \ > >>> > >>> It is very weird that host packages are configured with / as prefix, and > >>> then installed in HOST_DIR set as DEST_DIR. > >> > >> It's not weird, it's wrong! There is no reasonable explanation how this can be > >> correct. We are really installing stuff in that specific location, any paths > >> should refer to that location and not a path beginning with /. > > > > Well from my understanding when using relocatable mode/meson install the > > prefix is appended to the destdir similar to how it works for target/staging > > directories. > > As Thomas explained in the v4, it is not at all similar to target/staging. Well meson AFAIU is designed to make things functional even when run from the build tree which may have implications here, meson tends to do stuff differently from other build systems in general I think. > > > Regards, > Arnout > > >> If for some reason meson doesn't work when the prefix is not / or /usr or > >> /usr/local, it's a bug in meson, really. > > > > I think the issue is in how prefix and destdir get combined. > > > >> > >> Regards, > >> Arnout > >> > >>> > >>> I guess this is working thanks to --pkgconfig.relocatable a few lines > >>> below? > >>> > >>> This must be explained in the commit log (and probably a little comment > >>> above $(2)_CONFIGURE_CMDS for host packages. > >>> > >>>> --libdir=lib \ > >>>> - --sysconfdir=$$(HOST_DIR)/etc \ > >>>> - --localstatedir=$$(HOST_DIR)/var \ > >>>> + --pkgconfig.relocatable \ > >>>> + --sysconfdir=etc \ > >>>> + --localstatedir=var \ > >> > >> These two could be correct, however, since I think they're taken relative to > >> prefix (the default values are absolute because the default prefix is /usr and > >> sysconfdir is /etc, not /usr/etc - but in our case a relative path is fine). > >> > >> Regards, > >> Arnout > >> > >>> > >>> Ditto for etc and var? > >>> > >>> So, to make it easier to follow, I'd change the ordering: > >>> > >>> --pkgconfig.relocatable \ > >>> --prefix=/ \ > >>> --sysconfdir=etc \ > >>> --localstatedir=var \ > > > > Reordered in v4. > > > >>> > >>> Regards, > >>> Yann E. MORIN. > >>>
diff --git a/docs/manual/adding-packages-meson.txt b/docs/manual/adding-packages-meson.txt index 029c8c2488..c8aed65c2f 100644 --- a/docs/manual/adding-packages-meson.txt +++ b/docs/manual/adding-packages-meson.txt @@ -125,8 +125,8 @@ will therefore only use a few of them. +c_link_args+, +cpp_args+, +cpp_link_args+, +sys_root+, and +pkg_config_libdir+. -* +FOO_NINJA_ENV+, to specify additional environment variables to pass to - +ninja+, meson companion tool in charge of the build operations. By default, +* +FOO_MESON_ENV+, to specify additional environment variables to pass to + +meson+, meson tool in charge of the build/install operations. By default, empty. * +FOO_NINJA_OPTS+, to specify a space-separated list of targets to build. By diff --git a/package/gobject-introspection/gobject-introspection.mk b/package/gobject-introspection/gobject-introspection.mk index 41d64171a7..ea5100247d 100644 --- a/package/gobject-introspection/gobject-introspection.mk +++ b/package/gobject-introspection/gobject-introspection.mk @@ -30,14 +30,14 @@ HOST_GOBJECT_INTROSPECTION_DEPENDENCIES = \ host-python3 # g-ir-scanner will default to /usr/bin/ld for linking if this is not set. -GOBJECT_INTROSPECTION_NINJA_ENV += \ +GOBJECT_INTROSPECTION_MESON_ENV += \ CC="$(TARGET_CC)" # When building, gobject-introspection uses tools/g-ir-scanner to build several # .gir and .typelib files. g-ir-scanner does not use LDFLAGS, and by default, # links to the system-installed libglib2 path. To remedy this issue, defining # LD_LIBRARY_PATH forces g-ir-scanner to use our host installed libglib2 files. -HOST_GOBJECT_INTROSPECTION_NINJA_ENV += \ +HOST_GOBJECT_INTROSPECTION_MESON_ENV += \ LD_LIBRARY_PATH="$(if $(LD_LIBRARY_PATH),$(LD_LIBRARY_PATH):)$(HOST_DIR)/lib" # Use the host gi-scanner to prevent the scanner from generating incorrect diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk index 3b1db35fb6..4fa620080d 100644 --- a/package/pkg-cmake.mk +++ b/package/pkg-cmake.mk @@ -90,6 +90,8 @@ define $(2)_CONFIGURE_CMDS $$($$(PKG)_CONF_ENV) $$(BR2_CMAKE) $$($$(PKG)_SRCDIR) \ -DCMAKE_TOOLCHAIN_FILE="$$(HOST_DIR)/share/buildroot/toolchainfile.cmake" \ -DCMAKE_INSTALL_PREFIX="/usr" \ + -DCMAKE_PREFIX_PATH="$$(STAGING_DIR)/usr" \ + -DPKG_CONFIG_USE_CMAKE_PREFIX_PATH=ON \ -DCMAKE_COLOR_MAKEFILE=OFF \ -DBUILD_DOC=OFF \ -DBUILD_DOCS=OFF \ @@ -123,6 +125,8 @@ define $(2)_CONFIGURE_CMDS -DCMAKE_FIND_ROOT_PATH_MODE_LIBRARY="BOTH" \ -DCMAKE_FIND_ROOT_PATH_MODE_INCLUDE="BOTH" \ -DCMAKE_INSTALL_PREFIX="$$(HOST_DIR)" \ + -DCMAKE_PREFIX_PATH="$$(HOST_DIR)" \ + -DPKG_CONFIG_USE_CMAKE_PREFIX_PATH=ON \ -DCMAKE_C_FLAGS="$$(HOST_CFLAGS)" \ -DCMAKE_CXX_FLAGS="$$(HOST_CXXFLAGS)" \ -DCMAKE_EXE_LINKER_FLAGS="$$(HOST_LDFLAGS)" \ diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk index 0835e08e3a..de5817d5da 100644 --- a/package/pkg-meson.mk +++ b/package/pkg-meson.mk @@ -21,13 +21,13 @@ ################################################################################ # -# Pass PYTHONNOUSERSITE environment variable when invoking Meson or Ninja, so +# Pass PYTHONNOUSERSITE environment variable when invoking Meson, so # $(HOST_DIR)/bin/python3 will not look for Meson modules in # $HOME/.local/lib/python3.x/site-packages # -MESON = PYTHONNOUSERSITE=y $(HOST_DIR)/bin/meson -NINJA = PYTHONNOUSERSITE=y $(HOST_DIR)/bin/ninja -NINJA_OPTS = $(if $(VERBOSE),-v) +MESON = PYTHONNOUSERSITE=y $(HOST_DIR)/bin/meson +MESON_BUILD_OPTS = $(if $(VERBOSE),-v) +MESON_INSTALL_OPTS = --no-rebuild # https://mesonbuild.com/Reference-tables.html#cpu-families ifeq ($(BR2_arcle)$(BR2_arceb),y) @@ -138,6 +138,7 @@ define $(2)_CONFIGURE_CMDS $$(MESON) \ --prefix=/usr \ --libdir=lib \ + --pkgconfig.relocatable \ --default-library=$(if $(BR2_STATIC_LIBS),static,shared) \ --buildtype=$(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release) \ --cross-file=$$($$(PKG)_SRCDIR)/build/cross-compilation.conf \ @@ -156,10 +157,11 @@ define $(2)_CONFIGURE_CMDS mkdir -p $$($$(PKG)_SRCDIR)/build $$(HOST_CONFIGURE_OPTS) \ $$($$(PKG)_CONF_ENV) $$(MESON) \ - --prefix=$$(HOST_DIR) \ + --prefix=/ \ --libdir=lib \ - --sysconfdir=$$(HOST_DIR)/etc \ - --localstatedir=$$(HOST_DIR)/var \ + --pkgconfig.relocatable \ + --sysconfdir=etc \ + --localstatedir=var \ --default-library=shared \ --buildtype=release \ --wrap-mode=nodownload \ @@ -179,13 +181,23 @@ $(2)_DEPENDENCIES += host-meson ifndef $(2)_BUILD_CMDS ifeq ($(4),target) define $(2)_BUILD_CMDS - $$(TARGET_MAKE_ENV) $$($$(PKG)_NINJA_ENV) \ - $$(NINJA) $$(NINJA_OPTS) $$($$(PKG)_NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build + $$(TARGET_MAKE_ENV) \ + $$($$(PKG)_MESON_ENV) \ + $$(MESON) \ + compile \ + $$(MESON_BUILD_OPTS) \ + $$(if $$($$(PKG)_NINJA_OPTS),--ninja-args $$($$(PKG)_NINJA_OPTS)) \ + -C $$($$(PKG)_SRCDIR)/build endef else define $(2)_BUILD_CMDS - $$(HOST_MAKE_ENV) $$($$(PKG)_NINJA_ENV) \ - $$(NINJA) $$(NINJA_OPTS) $$($$(PKG)_NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build + $$(HOST_MAKE_ENV) \ + $$($$(PKG)_MESON_ENV) \ + $$(MESON) \ + compile \ + $$(MESON_BUILD_OPTS) \ + $$(if $$($$(PKG)_NINJA_OPTS),--ninja-args $$($$(PKG)_NINJA_OPTS)) \ + -C $$($$(PKG)_SRCDIR)/build endef endif endif @@ -196,8 +208,13 @@ endif # ifndef $(2)_INSTALL_CMDS define $(2)_INSTALL_CMDS - $$(HOST_MAKE_ENV) $$($$(PKG)_NINJA_ENV) \ - $$(NINJA) $$(NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build install + $$(HOST_MAKE_ENV) \ + $$($$(PKG)_MESON_ENV) \ + $$(MESON) \ + install \ + $$(MESON_INSTALL_OPTS) \ + --destdir $$(HOST_DIR) \ + -C $$($$(PKG)_SRCDIR)/build endef endif @@ -207,8 +224,13 @@ endif # ifndef $(2)_INSTALL_STAGING_CMDS define $(2)_INSTALL_STAGING_CMDS - $$(TARGET_MAKE_ENV) $$($$(PKG)_NINJA_ENV) DESTDIR=$$(STAGING_DIR) \ - $$(NINJA) $$(NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build install + $$(TARGET_MAKE_ENV) \ + $$($$(PKG)_MESON_ENV) \ + $$(MESON) \ + install \ + $$(MESON_INSTALL_OPTS) \ + --destdir $$(STAGING_DIR) \ + -C $$($$(PKG)_SRCDIR)/build endef endif @@ -218,8 +240,13 @@ endif # ifndef $(2)_INSTALL_TARGET_CMDS define $(2)_INSTALL_TARGET_CMDS - $$(TARGET_MAKE_ENV) $$($$(PKG)_NINJA_ENV) DESTDIR=$$(TARGET_DIR) \ - $$(NINJA) $$(NINJA_OPTS) -C $$($$(PKG)_SRCDIR)/build install + $$(TARGET_MAKE_ENV) \ + $$($$(PKG)_MESON_ENV) \ + $$(MESON) \ + install \ + $$(MESON_INSTALL_OPTS) \ + --destdir $$(TARGET_DIR) \ + -C $$($$(PKG)_SRCDIR)/build endef endif diff --git a/package/systemd/systemd.mk b/package/systemd/systemd.mk index 46a4e8de2c..9bb87f8e39 100644 --- a/package/systemd/systemd.mk +++ b/package/systemd/systemd.mk @@ -746,7 +746,7 @@ endef SYSTEMD_ROOTFS_PRE_CMD_HOOKS += SYSTEMD_PRESET_ALL SYSTEMD_CONF_ENV = $(HOST_UTF8_LOCALE_ENV) -SYSTEMD_NINJA_ENV = $(HOST_UTF8_LOCALE_ENV) +SYSTEMD_MESON_ENV = $(HOST_UTF8_LOCALE_ENV) define SYSTEMD_LINUX_CONFIG_FIXUPS $(call KCONFIG_ENABLE_OPT,CONFIG_DEVTMPFS) @@ -849,8 +849,6 @@ HOST_SYSTEMD_DEPENDENCIES = \ host-gperf \ host-python-jinja2 -HOST_SYSTEMD_NINJA_ENV = DESTDIR=$(HOST_DIR) - # Fix RPATH After installation # * systemd provides a install_rpath instruction to meson because the binaries # need to link with libsystemd which is not in a standard path
As of version 0.54.0 meson has had the ability to build and install packages rather than having to run ninja directly as before. This will allow us to use features such as meson install tags in the future which require meson to be used for the installation. It appears we need to ensure the cmake prefix path is set for the meson generated relocatable pkg-config format to work properly. Signed-off-by: James Hilliard <james.hilliard1@gmail.com> --- Changes v2 -> v3: - set pkgconfig to relocatable - set cmake pkg-config prefix path Changes v1 -> v2: - update docs --- docs/manual/adding-packages-meson.txt | 4 +- .../gobject-introspection.mk | 4 +- package/pkg-cmake.mk | 4 ++ package/pkg-meson.mk | 61 +++++++++++++------ package/systemd/systemd.mk | 4 +- 5 files changed, 53 insertions(+), 24 deletions(-)