Patchwork [2/4] pkg-infra: introduce pre/post-step hooks

login
register
mail settings
Submitter Yann E. MORIN
Date Oct. 27, 2013, 5:24 p.m.
Message ID <a89033d9a1639d8899191cae05b772b1646b6ddd.1382894622.git.yann.morin.1998@free.fr>
Download mbox | patch
Permalink /patch/286355/
State Changes Requested
Headers show

Comments

Yann E. MORIN - Oct. 27, 2013, 5:24 p.m.
From: "Yann E. MORIN" <yann.morin.1998@free.fr>

This hooks will let us instrument the build process in many ways:
  - log current step to see what broke
  - time each step to see what is worth optimising
  - sanity-check installed files (rpath, overwritten files...)
  - call user-provided script
  - ...

The steps are coarse-grain, and all have a 'start' and a 'end' hooks.
Here is the list of available steps (8 total):
  - extract
  - patch
  - configure
  - build
  - install-host
  - install-staging
  - install-image
  - install-target

The download, clean and uninstall steps are not instrumented on purpose.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/pkg-generic.mk | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
Thomas De Schampheleire - Oct. 27, 2013, 9:40 p.m.
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
>From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
>This hooks will let us instrument the build process in many ways:
>  - log current step to see what broke
>  - time each step to see what is worth optimising
>  - sanity-check installed files (rpath, overwritten files...)
>  - call user-provided script
>  - ...
>
>The steps are coarse-grain, and all have a 'start' and a 'end' hooks.
>Here is the list of available steps (8 total):
>  - extract
>  - patch
>  - configure
>  - build
>  - install-host
>  - install-staging
>  - install-image
>  - install-target
>
>The download, clean and uninstall steps are not instrumented on purpose.
>
>Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>---
> package/pkg-generic.mk | 39 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
>diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>index 4bba4b5..799555c 100644
>--- a/package/pkg-generic.mk
>+++ b/package/pkg-generic.mk
>@@ -21,6 +21,29 @@
> ################################################################################
> 
> ################################################################################
>+# Helper functions to catch start/end of each steps

each step

>+################################################################################
>+
>+# Those two functions are called by each steps below.

each step

>+# They are responsible for calling all hooks defined in $(STEP_HOOKS)

I personally find the name STEP_HOOKS pretty vague, if you don't know the context you have no clue what it could be. The other hooks POST_EXTRACT etc are clear from their name.
Maybe, GLOBAL_INSTRUMENTATION_HOOKS?

Best regards,
Thomas
Ryan Barnett - Oct. 28, 2013, 2:22 p.m.
Thomas D, Yann,

Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote on 10/27/2013 
04:40:09 PM:

> > 
################################################################################
> > 
> > 
################################################################################
> >+# Helper functions to catch start/end of each steps
> 
> each step
> 
> 
>+################################################################################
> >+
> >+# Those two functions are called by each steps below.
> 
> each step
> 
> >+# They are responsible for calling all hooks defined in $(STEP_HOOKS)
> 
> I personally find the name STEP_HOOKS pretty vague, if you don't know 
the
> context you have no clue what it could be. The other hooks POST_EXTRACT 
etc
> are clear from their name.
>
> Maybe, GLOBAL_INSTRUMENTATION_HOOKS?

But that is so long to type :)
 
Thanks,
-Ryan
Yann E. MORIN - Oct. 29, 2013, 6:43 p.m.
On 2013-10-27 22:40 +0100, Thomas De Schampheleire spake thusly:
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> >From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >This hooks will let us instrument the build process in many ways:
[--SNIP--]
> >diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> >index 4bba4b5..799555c 100644
> >--- a/package/pkg-generic.mk
> >+++ b/package/pkg-generic.mk
> >@@ -21,6 +21,29 @@
> > ################################################################################
> > 
> > ################################################################################
> >+# Helper functions to catch start/end of each steps
> 
> each step
> 
> >+################################################################################
> >+
> >+# Those two functions are called by each steps below.
> 
> each step

Both: OK.

> >+# They are responsible for calling all hooks defined in $(STEP_HOOKS)
> 
> I personally find the name STEP_HOOKS pretty vague, if you don't know
> the context you have no clue what it could be. The other hooks
> POST_EXTRACT etc are clear from their name.
> Maybe, GLOBAL_INSTRUMENTATION_HOOKS?

I'm on the side of Ryan here: it is way too long. :-p
I'll try to see if I can come with a better name, though.

Thanks for the review!

Regards,
Yann E. MORIN.
Thomas De Schampheleire - Oct. 29, 2013, 8:03 p.m.
Hi Yann, Ryan,

On Tue, Oct 29, 2013 at 7:43 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> On 2013-10-27 22:40 +0100, Thomas De Schampheleire spake thusly:

>> I personally find the name STEP_HOOKS pretty vague, if you don't know
>> the context you have no clue what it could be. The other hooks
>> POST_EXTRACT etc are clear from their name.
>> Maybe, GLOBAL_INSTRUMENTATION_HOOKS?
>
> I'm on the side of Ryan here: it is way too long. :-p
> I'll try to see if I can come with a better name, though.
>

I'm fine with looking at other names, of course, but I don't see how
GLOBAL_INSTRUMENTATION_HOOKS
is longer than
FOO_POST_INSTALL_STAGING_HOOKS
FOO_POST_INSTALL_TARGET_HOOKS
or
CONNTRACK_TOOLS_PRE_CONFIGURE_HOOKS

Besides, it's not that you're writing this so regularly that the
length is really so important, IMHO.

Best regards,
Thomas
Yann E. MORIN - Oct. 29, 2013, 8:56 p.m.
Thomas DS, All,

On 2013-10-29 21:03 +0100, Thomas De Schampheleire spake thusly:
> Hi Yann, Ryan,
> 
> On Tue, Oct 29, 2013 at 7:43 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > On 2013-10-27 22:40 +0100, Thomas De Schampheleire spake thusly:
> 
> >> I personally find the name STEP_HOOKS pretty vague, if you don't know
> >> the context you have no clue what it could be. The other hooks
> >> POST_EXTRACT etc are clear from their name.
> >> Maybe, GLOBAL_INSTRUMENTATION_HOOKS?
> >
> > I'm on the side of Ryan here: it is way too long. :-p
> > I'll try to see if I can come with a better name, though.
> >
> 
> I'm fine with looking at other names, of course, but I don't see how
> GLOBAL_INSTRUMENTATION_HOOKS
> is longer than
> FOO_POST_INSTALL_STAGING_HOOKS
> FOO_POST_INSTALL_TARGET_HOOKS
> or
> CONNTRACK_TOOLS_PRE_CONFIGURE_HOOKS
> 
> Besides, it's not that you're writing this so regularly that the
> length is really so important, IMHO.

Yeah, yeah, right. :-)

Regards,
Yann E. MORIN.

Patch

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 4bba4b5..799555c 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -21,6 +21,29 @@ 
 ################################################################################
 
 ################################################################################
+# Helper functions to catch start/end of each steps
+################################################################################
+
+# Those two functions are called by each steps below.
+# They are responsible for calling all hooks defined in $(STEP_HOOKS)
+# and pass each of them two arguments:
+#   $1: either 'start' or 'end'
+#   $2: the name of the step
+#   $3: the name of the package
+
+# Start step
+# $1: step name
+define step_start
+	$(foreach hook,$(STEP_HOOKS),$(call $(hook),start,$(1),$($(PKG)_NAME))$(sep))
+endef
+
+# End step
+# $1: step name
+define step_end
+	$(foreach hook,$(STEP_HOOKS),$(call $(hook),end,$(1),$($(PKG)_NAME))$(sep))
+endef
+
+################################################################################
 # Implicit targets -- produce a stamp file for each step of a package build
 ################################################################################
 
@@ -55,6 +78,7 @@  endif
 
 # Unpack the archive
 $(BUILD_DIR)/%/.stamp_extracted:
+	@$(call step_start,extract)
 	@$(call MESSAGE,"Extracting")
 	$(Q)mkdir -p $(@D)
 	$($(PKG)_EXTRACT_CMDS)
@@ -62,6 +86,7 @@  $(BUILD_DIR)/%/.stamp_extracted:
 	$(Q)chmod -R +rw $(@D)
 	$(foreach hook,$($(PKG)_POST_EXTRACT_HOOKS),$(call $(hook))$(sep))
 	$(Q)touch $@
+	@$(call step_end,extract)
 
 # Rsync the source directory if the <pkg>_OVERRIDE_SRCDIR feature is
 # used.
@@ -90,6 +115,7 @@  endif
 $(BUILD_DIR)/%/.stamp_patched: NAMEVER = $(RAWNAME)-$($(PKG)_VERSION)
 $(BUILD_DIR)/%/.stamp_patched: PATCH_BASE_DIRS = $($(PKG)_DIR_PREFIX)/$(RAWNAME) $(call qstrip,$(BR2_GLOBAL_PATCH_DIR))/$(RAWNAME)
 $(BUILD_DIR)/%/.stamp_patched:
+	@$(call step_start,patch)
 	@$(call MESSAGE,"Patching $($(PKG)_DIR_PREFIX)/$(RAWNAME)")
 	$(foreach hook,$($(PKG)_PRE_PATCH_HOOKS),$(call $(hook))$(sep))
 	$(foreach p,$($(PKG)_PATCH),support/scripts/apply-patches.sh $(@D) $(DL_DIR) $(notdir $(p))$(sep))
@@ -106,31 +132,39 @@  $(BUILD_DIR)/%/.stamp_patched:
 	)
 	$(foreach hook,$($(PKG)_POST_PATCH_HOOKS),$(call $(hook))$(sep))
 	$(Q)touch $@
+	@$(call step_end,patch)
 
 # Configure
 $(BUILD_DIR)/%/.stamp_configured:
+	@$(call step_start,configure)
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	@$(call MESSAGE,"Configuring")
 	$($(PKG)_CONFIGURE_CMDS)
 	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$(Q)touch $@
+	@$(call step_end,configure)
 
 # Build
 $(BUILD_DIR)/%/.stamp_built::
+	@$(call step_start,build)
 	@$(call MESSAGE,"Building")
 	$($(PKG)_BUILD_CMDS)
 	$(foreach hook,$($(PKG)_POST_BUILD_HOOKS),$(call $(hook))$(sep))
 	$(Q)touch $@
+	@$(call step_end,build)
 
 # Install to host dir
 $(BUILD_DIR)/%/.stamp_host_installed:
+	@$(call step_start,install-host)
 	@$(call MESSAGE,"Installing to host directory")
 	$($(PKG)_INSTALL_CMDS)
 	$(foreach hook,$($(PKG)_POST_INSTALL_HOOKS),$(call $(hook))$(sep))
 	$(Q)touch $@
+	@$(call step_end,install-host)
 
 # Install to staging dir
 $(BUILD_DIR)/%/.stamp_staging_installed:
+	@$(call step_start,install-staging)
 	@$(call MESSAGE,"Installing to staging directory")
 	$($(PKG)_INSTALL_STAGING_CMDS)
 	$(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
@@ -142,16 +176,20 @@  $(BUILD_DIR)/%/.stamp_staging_installed:
 				$(addprefix $(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_SCRIPTS)) ;\
 	fi
 	$(Q)touch $@
+	@$(call step_end,install-staging)
 
 # Install to images dir
 $(BUILD_DIR)/%/.stamp_images_installed:
+	@$(call step_start,install-image)
 	@$(call MESSAGE,"Installing to images directory")
 	$($(PKG)_INSTALL_IMAGES_CMDS)
 	$(foreach hook,$($(PKG)_POST_INSTALL_IMAGES_HOOKS),$(call $(hook))$(sep))
 	$(Q)touch $@
+	@$(call step_end,install-image)
 
 # Install to target dir
 $(BUILD_DIR)/%/.stamp_target_installed:
+	@$(call step_start,install-target)
 	@$(call MESSAGE,"Installing to target")
 	$(if $(BR2_INIT_SYSTEMD),\
 		$($(PKG)_INSTALL_INIT_SYSTEMD))
@@ -163,6 +201,7 @@  $(BUILD_DIR)/%/.stamp_target_installed:
 		$(RM) -f $(addprefix $(TARGET_DIR)/usr/bin/,$($(PKG)_CONFIG_SCRIPTS)) ; \
 	fi
 	$(Q)touch $@
+	@$(call step_end,install-target)
 
 # Clean package
 $(BUILD_DIR)/%/.stamp_cleaned: