[Buildroot] [pull request] [RFC] Pull request for branch yem/instrument-build

[Buildroot] [pull request] [RFC] Pull request for branch yem/instrument-build

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Hello All!

This is an RFC about instrumenting the build process.

It works as thus:
  - we register a set of hooks
  - hooks are called before and after each steps

There are currently two hooks defined:
  - log steps with timings
  - call a user-supplied script

This is just a RFC for now, new hooks can be defined later on.

I'm planning on adding at least one other hook, that runs all scripts
present in support/step-hooks/ (or whatever its name). Eventually,
the user-supplied hook could be made to accept a directory instead of
a file, and run all scripts in that directory, too.

Open for comments! ;-)

Regards,
Yann E. MORIN.


The following changes since commit 9f7e8f120dfeddd8b49eb4d232253cc3f9ac8efa:

  package/weston: add RPi compositor (2013-10-11 08:56:53 +0200)

are available in the git repository at:

  git://gitorious.org/buildroot/buildroot.git yem/instrument-build

for you to fetch changes up to 3f8ce54570cde2f93d5f61c80a1b28af046b4926:

  pkg-infra: add user-supplied step-hooks (2013-10-14 00:58:46 +0200)

----------------------------------------------------------------
Yann E. MORIN (4):
      pkg-infra: introduce pre/post-step hooks
      pkg-infra: add hook to log timing of steps
      Makefile: export BUILD_DIR
      pkg-infra: add user-supplied step-hooks

 Makefile               |  1 +
 package/pkg-generic.mk | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+)

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/4] pkg-infra: introduce pre/post-step hooks

[Yann E. MORIN]

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 fine-grain, and all have a 'start' and a 'end' hooks.
Here is the list of available steps (19 total):
  - extract, post-extract
  - pre-patch, patch, post-patch
  - pre-configure, configure, post-configure
  - build, post-build
  - install-host, post-install-host
  - install-staging, post-install-staging, pkg-config-staging
  - install-image, post-install-image
  - install-target, post-install-target

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

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/pkg-generic.mk | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index a46457c..c4c7f8a 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -21,6 +21,22 @@
 ################################################################################
 
 ################################################################################
+# Helper functions to catch start/end of each steps
+################################################################################
+
+# Start step
+# $1: step name
+define step_start
+	$(foreach hook,$(STEP_HOOKS),$(call $(hook),start,$(1));)
+endef
+
+# End step
+# $1: step name
+define step_end
+	$(foreach hook,$(STEP_HOOKS),$(call $(hook),end,$(1));)
+endef
+
+################################################################################
 # Implicit targets -- produce a stamp file for each step of a package build
 ################################################################################
 
@@ -56,11 +72,15 @@ endif
 # Unpack the archive
 $(BUILD_DIR)/%/.stamp_extracted:
 	@$(call MESSAGE,"Extracting")
+	@$(call step_start,extract)
 	$(Q)mkdir -p $(@D)
 	$($(PKG)_EXTRACT_CMDS)
 # some packages have messed up permissions inside
 	$(Q)chmod -R +rw $(@D)
+	@$(call step_end,extract)
+	@$(call step_start,post-extract)
 	$(foreach hook,$($(PKG)_POST_EXTRACT_HOOKS),$(call $(hook))$(sep))
+	@$(call step_end,post-extract)
 	$(Q)touch $@
 
 # Rsync the source directory if the <pkg>_OVERRIDE_SRCDIR feature is
@@ -91,7 +111,10 @@ $(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 MESSAGE,"Patching $($(PKG)_DIR_PREFIX)/$(RAWNAME)")
+	@$(call step_start,pre-patch)
 	$(foreach hook,$($(PKG)_PRE_PATCH_HOOKS),$(call $(hook))$(sep))
+	@$(call step_end,pre-patch)
+	@$(call step_start,patch)
 	$(foreach p,$($(PKG)_PATCH),support/scripts/apply-patches.sh $(@D) $(DL_DIR) $(notdir $(p))$(sep))
 	$(Q)( \
 	for D in $(PATCH_BASE_DIRS); do \
@@ -104,36 +127,58 @@ $(BUILD_DIR)/%/.stamp_patched:
 	  fi; \
 	done; \
 	)
+	@$(call step_end,patch)
+	@$(call step_start,post-patch)
 	$(foreach hook,$($(PKG)_POST_PATCH_HOOKS),$(call $(hook))$(sep))
+	@$(call step_end,patch)
 	$(Q)touch $@
 
 # Configure
 $(BUILD_DIR)/%/.stamp_configured:
+	@$(call step_start,pre-configure)
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
+	@$(call step_end,pre-configure)
 	@$(call MESSAGE,"Configuring")
+	@$(call step_start,configure)
 	$($(PKG)_CONFIGURE_CMDS)
+	@$(call step_end,configure)
+	@$(call step_start,post-configure)
 	$(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))
+	@$(call step_end,post-configure)
 	$(Q)touch $@
 
 # Build
 $(BUILD_DIR)/%/.stamp_built::
 	@$(call MESSAGE,"Building")
+	@$(call step_start,build)
 	$($(PKG)_BUILD_CMDS)
+	@$(call step_end,build)
+	@$(call step_start,post-build)
 	$(foreach hook,$($(PKG)_POST_BUILD_HOOKS),$(call $(hook))$(sep))
+	@$(call step_end,post-build)
 	$(Q)touch $@
 
 # Install to host dir
 $(BUILD_DIR)/%/.stamp_host_installed:
 	@$(call MESSAGE,"Installing to host directory")
+	@$(call step_start,install-host)
 	$($(PKG)_INSTALL_CMDS)
+	@$(call step_end,install-host)
+	@$(call step_start,post-install-host)
 	$(foreach hook,$($(PKG)_POST_INSTALL_HOOKS),$(call $(hook))$(sep))
+	@$(call step_end,post-install-host)
 	$(Q)touch $@
 
 # Install to staging dir
 $(BUILD_DIR)/%/.stamp_staging_installed:
 	@$(call MESSAGE,"Installing to staging directory")
+	@$(call step_start,install-staging)
 	$($(PKG)_INSTALL_STAGING_CMDS)
+	@$(call step_end,install-staging)
+	@$(call step_start,post-install-staging)
 	$(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
+	@$(call step_end,post-install-staging)
+	@$(call step_start,pkg-config-staging)
 	$(Q)if test -n "$($(PKG)_CONFIG_SCRIPTS)" ; then \
 		$(call MESSAGE,"Fixing package configuration files") ;\
 			$(SED)  "s,^\(exec_\)\?prefix=.*,\1prefix=$(STAGING_DIR)/usr,g" \
@@ -141,27 +186,36 @@ $(BUILD_DIR)/%/.stamp_staging_installed:
 				-e "s,-L/usr/,-L$(STAGING_DIR)/usr/,g" \
 				$(addprefix $(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_SCRIPTS)) ;\
 	fi
+	@$(call step_end,pkg-config-staging)
 	$(Q)touch $@
 
 # Install to images dir
 $(BUILD_DIR)/%/.stamp_images_installed:
 	@$(call MESSAGE,"Installing to images directory")
+	@$(call step_start,install-image)
 	$($(PKG)_INSTALL_IMAGES_CMDS)
+	@$(call step_end,install-image)
+	@$(call step_start,post-install-image)
 	$(foreach hook,$($(PKG)_POST_INSTALL_IMAGES_HOOKS),$(call $(hook))$(sep))
+	@$(call step_end,post-install-image)
 	$(Q)touch $@
 
 # Install to target dir
 $(BUILD_DIR)/%/.stamp_target_installed:
 	@$(call MESSAGE,"Installing to target")
+	@$(call step_start,install-target)
 	$(if $(BR2_INIT_SYSTEMD),\
 		$($(PKG)_INSTALL_INIT_SYSTEMD))
 	$(if $(BR2_INIT_SYSV)$(BR2_INIT_BUSYBOX),\
 		$($(PKG)_INSTALL_INIT_SYSV))
 	$($(PKG)_INSTALL_TARGET_CMDS)
+	@$(call step_end,install-target)
+	@$(call step_start,post-install-target)
 	$(foreach hook,$($(PKG)_POST_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep))
 	$(Q)if test -n "$($(PKG)_CONFIG_SCRIPTS)" ; then \
 		$(RM) -f $(addprefix $(TARGET_DIR)/usr/bin/,$($(PKG)_CONFIG_SCRIPTS)) ; \
 	fi
+	@$(call step_end,post-install-target)
 	$(Q)touch $@
 
 # Clean package
-- 
1.8.1.2

[Buildroot] [PATCH 1/4] pkg-infra: introduce pre/post-step hooks

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Mon, 14 Oct 2013 01:11:25 +0200, Yann E. MORIN 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 fine-grain, and all have a 'start' and a 'end' hooks.
> Here is the list of available steps (19 total):
>   - extract, post-extract
>   - pre-patch, patch, post-patch
>   - pre-configure, configure, post-configure
>   - build, post-build
>   - install-host, post-install-host
>   - install-staging, post-install-staging, pkg-config-staging
>   - install-image, post-install-image
>   - install-target, post-install-target
> 
> The download, clean, uninstall steps are not instrumented on purpose.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

I am not sure to follow why we're introducing additional hooks here.
Why don't we generalize the existing pre/post hooks mechanism to *all*
steps (by all I mean the steps you are interested in instrumenting),
and use that to hook the different things you introduce in patch 2, 3,
4 ?

Also, do we really need to have hook points for the pre-hooks and
post-hooks each time?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/4] pkg-infra: introduce pre/post-step hooks

[Yann E. MORIN]

On 2013-10-14 09:16 +0200, Thomas Petazzoni spake thusly:
> Dear Yann E. MORIN,
> 
> On Mon, 14 Oct 2013 01:11:25 +0200, Yann E. MORIN 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 fine-grain, and all have a 'start' and a 'end' hooks.
> > Here is the list of available steps (19 total):
> >   - extract, post-extract
> >   - pre-patch, patch, post-patch
> >   - pre-configure, configure, post-configure
> >   - build, post-build
> >   - install-host, post-install-host
> >   - install-staging, post-install-staging, pkg-config-staging
> >   - install-image, post-install-image
> >   - install-target, post-install-target
> > 
> > The download, clean, uninstall steps are not instrumented on purpose.
> > 
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> I am not sure to follow why we're introducing additional hooks here.
> Why don't we generalize the existing pre/post hooks mechanism to *all*
> steps (by all I mean the steps you are interested in instrumenting),
> and use that to hook the different things you introduce in patch 2, 3,
> 4 ?

I'm not sure I understand what you suggest, so I'll try to explain what
I understood (in a series-like order):

  - add missing pre/post hooks to all steps
  - have the pkg-infra internally register pre/post hooks for each
    package
  - add and register the time/user hooks

Is that what you meant?

> Also, do we really need to have hook points for the pre-hooks and
> post-hooks each time?

Yes, I wan't to be able to instrument them. Especially, I want to be
able to check that pre/post hook are not messing with target/ in crazy
way. I also want to be able to time them (heck, I've seen a hook takes
orders of magnitude longer than the corresponding action).

With your suggestion, I don't know how we can instrument the pre/post
hooks.

Note: However, I agree that we could reduce the number of steps by
squashing, for example, end-pre-configure with start-configure, or
end-configure with start-post-configure (and so on). But having both
start/end be separate barriers is cleaner and more systematic; it helps
reviewing a log of the build without constantly wondering what frontier
a specific step is.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 2/4] pkg-infra: add hook to log timing of steps

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/pkg-generic.mk | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index c4c7f8a..162c9ed 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -36,6 +36,12 @@ define step_end
 	$(foreach hook,$(STEP_HOOKS),$(call $(hook),end,$(1));)
 endef
 
+# Time steps
+define step_time
+	printf "%s:%-5.5s:%-20.20s: %s\n" "$$(date +%s)" "$(1)" "$(2)" "$($(PKG)_NAME)" >>"$(BUILD_DIR)/build.log"
+endef
+STEP_HOOKS += step_time
+
 ################################################################################
 # Implicit targets -- produce a stamp file for each step of a package build
 ################################################################################
-- 
1.8.1.2

[Buildroot] [PATCH 3/4] Makefile: export BUILD_DIR

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

$(BUILD_DIR) is a nice place to put files generated during the build.
With the advent of user-supplied step-hooks, they may want to store
some information on the build.

Export BUILD_DIR to that effect.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index f266e2d..9528703 100644
--- a/Makefile
+++ b/Makefile
@@ -291,6 +291,7 @@ export STAGING_DIR
 export HOST_DIR
 export BINARIES_DIR
 export BASE_DIR
+export BUILD_DIR
 
 ################################################################################
 #
-- 
1.8.1.2

[Buildroot] [PATCH 3/4] Makefile: export BUILD_DIR

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 Yann> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
 Yann> $(BUILD_DIR) is a nice place to put files generated during the build.
 Yann> With the advent of user-supplied step-hooks, they may want to store
 Yann> some information on the build.

 Yann> Export BUILD_DIR to that effect.

Looks good, but please also update the documentation in
docs/manual/customize-rootfs.txt.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 3/4] Makefile: export BUILD_DIR

[Yann E. MORIN]

Peter, All,

On 2013-10-14 12:54 +0200, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
> 
>  Yann> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>  Yann> $(BUILD_DIR) is a nice place to put files generated during the build.
>  Yann> With the advent of user-supplied step-hooks, they may want to store
>  Yann> some information on the build.
> 
>  Yann> Export BUILD_DIR to that effect.
> 
> Looks good, but please also update the documentation in
> docs/manual/customize-rootfs.txt.

Ah, yes. Done. Thanks!

Don't forget this pull-request is only an RFC from my point-of-view,
aimed at a starting point for further discussion.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 4/4] pkg-infra: add user-supplied step-hooks

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Allow user to supply their own step-hooks by passing a variable
on the make command-line:
    make BR2_STEP_USER_HOOK=/path/to/my/script

This can be usefull to run site-specific actions at each step of the
build process, such as logging installed, removed or modified files,
do sanity checks on installed files...

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/pkg-generic.mk | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 162c9ed..f5c33b2 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -42,6 +42,14 @@ define step_time
 endef
 STEP_HOOKS += step_time
 
+# User-supplied script
+define step_user
+	"$(BR2_STEP_USER_HOOK)" "$(1)" "$(2)" "$($(PKG)_NAME)"
+endef
+ifneq ($(BR2_STEP_USER_HOOK),)
+STEP_HOOKS += step_user
+endif
+
 ################################################################################
 # Implicit targets -- produce a stamp file for each step of a package build
 ################################################################################
-- 
1.8.1.2