diff mbox series

[RFC,1/1] package/pkg-generic.mk: introduce pre/post target finalize hooks

Message ID CYYP221MB1140ACF0E0092617169683ECA0BC2@CYYP221MB1140.NAMP221.PROD.OUTLOOK.COM
State New
Headers show
Series [RFC,1/1] package/pkg-generic.mk: introduce pre/post target finalize hooks | expand

Commit Message

James Knight Aug. 3, 2024, 5:38 p.m. UTC
Provides the ability for packages to register hooks in the finalization
process before and after Buildroot performs its own final target
changes. The pre-hook (`LIBFOO_TARGET_PRE_FINALIZE_HOOKS`) operates in
the same manner as `LIBFOO_TARGET_FINALIZE_HOOKS` did before. The newly
added post-hook (`LIBFOO_TARGET_POST_FINALIZE_HOOKS`) allows packages
to perform target modifications after Buildroot applies changes such as
rootfs overlay updates.

Signed-off-by: James Knight <james.d.knight@live.com>
---
The following is a proposal for adding support for post-finalize hooks
into the Buildroot framework. Been playing with using external package
definitions to perform some post-build tweaks to some board
configurations. While post-build scripts are a great way to perform any
late-stage target manipulation, it would be nice to be able to define
packages and register them into the menu system to make it easier to
re-use shared tweaks across multiple board configuration (versus trying
to add/stack post-build script paths into each configuration).

While `LIBFOO_TARGET_FINALIZE_HOOKS` worked for most cases, this occurs
before Buildroot performs additional finalization changes (e.g. merged
usr/). It would be nice to allow packages to perform their own
finalization changes after Buildroot does the bulk of it's own
finalization, but before post-build scripts are invoked.

If this change is considered, it appears the `luarocks.mk` package would
also need to be updated (in addition to manual changes).
---
 Makefile               | 7 +++++--
 package/pkg-generic.mk | 5 ++++-
 2 files changed, 9 insertions(+), 3 deletions(-)

Comments

Thomas Petazzoni Aug. 3, 2024, 8:57 p.m. UTC | #1
Hello James,

On Sat,  3 Aug 2024 13:38:32 -0400
James Knight <james.d.knight@live.com> wrote:

> Provides the ability for packages to register hooks in the finalization
> process before and after Buildroot performs its own final target
> changes. The pre-hook (`LIBFOO_TARGET_PRE_FINALIZE_HOOKS`) operates in
> the same manner as `LIBFOO_TARGET_FINALIZE_HOOKS` did before. The newly
> added post-hook (`LIBFOO_TARGET_POST_FINALIZE_HOOKS`) allows packages
> to perform target modifications after Buildroot applies changes such as
> rootfs overlay updates.
> 
> Signed-off-by: James Knight <james.d.knight@live.com>
> ---
> The following is a proposal for adding support for post-finalize hooks
> into the Buildroot framework. Been playing with using external package
> definitions to perform some post-build tweaks to some board
> configurations. While post-build scripts are a great way to perform any
> late-stage target manipulation, it would be nice to be able to define
> packages and register them into the menu system to make it easier to
> re-use shared tweaks across multiple board configuration (versus trying
> to add/stack post-build script paths into each configuration).
> 
> While `LIBFOO_TARGET_FINALIZE_HOOKS` worked for most cases, this occurs
> before Buildroot performs additional finalization changes (e.g. merged
> usr/). It would be nice to allow packages to perform their own
> finalization changes after Buildroot does the bulk of it's own
> finalization, but before post-build scripts are invoked.
> 
> If this change is considered, it appears the `luarocks.mk` package would
> also need to be updated (in addition to manual changes).

Thanks for proposing this. Obviously it raises a number of
questions/comments:

- My initial though was "but pre/post hooks only make sense when they
  are around a well-defined step, like download, extract, build, etc.,
  the target-finalize step is much more loosely defined". But in the
  end, perhaps we can consider target-finalize like a step.

- If we were to have PRE_FINALIZE_HOOKS and POST_FINALIZE_HOOKS, we
  should drop the TARGET_FINALIZE_HOOKS from all packages, otherwise
  the semantic is really unclear.

- Could give a specific example of something that cannot be using the
  existing TARGET_FINALIZE_HOOKS? I'm not talking about a theoretical
  example like "a user may want to...", but a really specific practical
  example you've encountered.

- Also, do we have some evidence that what we're doing as
  TARGET_FINALIZE_HOOKS today cannot be done at the point of the
  POST_FINALIZE_HOOKS that you are adding today?

Basically, I would like some really compelling evidence that we need
both hook points.

Thoughts?

Thomas
James Knight Aug. 4, 2024, 2:04 a.m. UTC | #2
Hey Thomas,

On Sat, Aug 3, 2024 at 4:57 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
> ... Could give a specific example of something that cannot be using the existing TARGET_FINALIZE_HOOKS? ...

One example where we have been trying this newly suggested hook is
defining a br2-external package dedicated for tailoring a generated
`/usr/lib/os-release` file. Originally, I have been using a post-build
script to populate entries into `os-release` (e.g. adding a
`SUPPORT_URL` entry and more). While this works, it does require
populating a script in `BR2_ROOTFS_POST_BUILD_SCRIPTS`. A different
approach I was looking for was having a menu option that other board
configurations can easily use to perform os-release tailoring. With
the proposed hook, I can now define a package that performs the
changes after Buildroot generates an os-release, and there is no
requirement for existing and new board configurations to set or append
their post-build script option.

From what I can tell, there was no other means to invoke a
package-specific hook right before Buildroot triggers any custom
post-build script. While the mentioned example is somewhat of a
trivial use case, I figured I would propose this hook to have
developers have flexibility in their target tailoring.

It would be fine if the proposal is not preferred. The example
mentioned may be too trivial and having just a `TARGET_FINALIZE_HOOKS`
over a pre/post pair is simpler to promote/use. It would not be too
much of a hassle to move back to using post-build scripts.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index f51e8f690fcb208c021b8169cd9db287d1e3e0ec..d97b5d7e492b8a4ba29103ba4ba6fbfeec1775d3 100644
--- a/Makefile
+++ b/Makefile
@@ -668,7 +668,7 @@  define GENERATE_GLIBC_LOCALES
 		LOCALES="$(GLIBC_GENERATE_LOCALES)" \
 		Q=$(Q)
 endef
-TARGET_FINALIZE_HOOKS += GENERATE_GLIBC_LOCALES
+TARGET_PRE_FINALIZE_HOOKS += GENERATE_GLIBC_LOCALES
 endif
 endif
 
@@ -706,7 +706,7 @@  define PURGE_LOCALES
 		done > $(TARGET_DIR)/usr/share/X11/locale/locale.dir; \
 	fi
 endef
-TARGET_FINALIZE_HOOKS += PURGE_LOCALES
+TARGET_PRE_FINALIZE_HOOKS += PURGE_LOCALES
 endif
 
 $(TARGETS_ROOTFS): target-finalize
@@ -731,6 +731,7 @@  target-finalize: $(PACKAGES) $(TARGET_DIR) host-finalize
 	@$(call MESSAGE,"Finalizing target directory")
 	$(call per-package-rsync,$(sort $(PACKAGES)),target,$(TARGET_DIR),copy)
 	$(foreach hook,$(TARGET_FINALIZE_HOOKS),$($(hook))$(sep))
+	$(foreach hook,$(TARGET_PRE_FINALIZE_HOOKS),$($(hook))$(sep))
 	rm -rf $(TARGET_DIR)/usr/include $(TARGET_DIR)/usr/share/aclocal \
 		$(TARGET_DIR)/usr/lib/pkgconfig $(TARGET_DIR)/usr/share/pkgconfig \
 		$(TARGET_DIR)/usr/lib/cmake $(TARGET_DIR)/usr/share/cmake \
@@ -798,6 +799,8 @@  endif # merged /usr
 		@$(call MESSAGE,"Copying overlay $(d)")$(sep) \
 		$(Q)$(call SYSTEM_RSYNC,$(d),$(TARGET_DIR))$(sep))
 
+	$(foreach hook,$(TARGET_POST_FINALIZE_HOOKS),$($(hook))$(sep))
+
 	$(Q)$(if $(TARGET_DIR_FILES_LISTS), \
 		cat $(TARGET_DIR_FILES_LISTS)) > $(BUILD_DIR)/packages-file-list.txt
 	$(Q)$(if $(HOST_DIR_FILES_LISTS), \
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 171163dcb4401683bf2b8c7751fe261efa811081..17ddfcc4084ef522c693115623e20e14f5574fcb 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -1240,7 +1240,10 @@  endif
 ifneq ($$($(2)_LINUX_CONFIG_FIXUPS),)
 PACKAGES_LINUX_CONFIG_FIXUPS += $$($(2)_LINUX_CONFIG_FIXUPS)$$(sep)
 endif
-TARGET_FINALIZE_HOOKS += $$($(2)_TARGET_FINALIZE_HOOKS)
+# note: 'LIBFOO_TARGET_FINALIZE_HOOKS' deprecated
+TARGET_PRE_FINALIZE_HOOKS += $$($(2)_TARGET_FINALIZE_HOOKS)   
+TARGET_PRE_FINALIZE_HOOKS += $$($(2)_TARGET_PRE_FINALIZE_HOOKS)
+TARGET_POST_FINALIZE_HOOKS += $$($(2)_TARGET_POST_FINALIZE_HOOKS)
 ROOTFS_PRE_CMD_HOOKS += $$($(2)_ROOTFS_PRE_CMD_HOOKS)
 KEEP_PYTHON_PY_FILES += $$($(2)_KEEP_PY_FILES)