[Buildroot] [PATCH] pkg-infra: log current message

[Buildroot] [PATCH] pkg-infra: log current message

[Yann E. MORIN]

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

In order to help the auto-builders (our own, or in-house ones),
just log the current message in a file that contains:
    Package: $($(PKG)_NAME)
    Version: $($(PKG)_VERSION)
    Action : Last '>>>' message displayed

For example:
    Package: host-fakeroot
    Version: 1.18.2
    Action : Configuring

If there is no package, then the package name and version are
empty, eg.:
    Package:
    Version:
    Action : Generating root filesystem image rootfs.tar

Also, all messages are logged to a file, one per line, with the
date each message was generated at, as the number of seconds
elapsed since Epoch.

This will hopefully help autobuilders extract the real reason for
a failure, and take appropriate action (eg. bug-mail the last
git-author of a package...)

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

diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index 5930f2c..ad6bd7c 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -66,7 +66,14 @@ INFLATE.tar  = cat
 suitable-extractor = $(INFLATE$(suffix $(1)))
 
 # MESSAGE Macro -- display a message in bold type
-MESSAGE     = echo "$(TERM_BOLD)>>> $($(PKG)_NAME) $($(PKG)_VERSION) $(1)$(TERM_RESET)"
+define MESSAGE
+	echo "$(TERM_BOLD)>>> $($(PKG)_NAME) $($(PKG)_VERSION) $(1)$(TERM_RESET)";   \
+	echo "$$(date '+%s'): $($(PKG)_NAME) $($(PKG)_VERSION) $(1)"                 \
+	     >>$(BUILD_DIR)/actions.log;                                             \
+	printf "Package: %s\nVersion: %s\nAction : %s\n"                             \
+	       "$($(PKG)_NAME)" "$($(PKG)_VERSION)" $(1)                             \
+	       >$(BUILD_DIR)/last-action
+endef
 TERM_BOLD  := $(shell tput smso)
 TERM_RESET := $(shell tput rmso)
 
-- 
1.8.1.2

[Buildroot] [PATCH] pkg-infra: log current message

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Sun,  6 Oct 2013 13:19:48 +0200, Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> In order to help the auto-builders (our own, or in-house ones),
> just log the current message in a file that contains:
>     Package: $($(PKG)_NAME)
>     Version: $($(PKG)_VERSION)
>     Action : Last '>>>' message displayed
> 
> For example:
>     Package: host-fakeroot
>     Version: 1.18.2
>     Action : Configuring
> 
> If there is no package, then the package name and version are
> empty, eg.:
>     Package:
>     Version:
>     Action : Generating root filesystem image rootfs.tar
> 
> Also, all messages are logged to a file, one per line, with the
> date each message was generated at, as the number of seconds
> elapsed since Epoch.
> 
> This will hopefully help autobuilders extract the real reason for
> a failure, and take appropriate action (eg. bug-mail the last
> git-author of a package...)
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

This looks interesting, but I'm wondering if this shouldn't be
conditional under a BR2_<something> configuration option that the
autobuilders would enable?

I'm thinking of other things this option could enable, such as some
sanity checks at the end of the build, that we want to do in the
autobuilders, but not necessarily in normal builds.

Best regards,

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

[Buildroot] [PATCH] pkg-infra: log current message

[Jérôme Pouiller]

Hello Yann,

On Sunday 06 October 2013 13:19:48 Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> In order to help the auto-builders (our own, or in-house ones),
> just log the current message in a file that contains:
>     Package: $($(PKG)_NAME)
>     Version: $($(PKG)_VERSION)
>     Action : Last '>>>' message displayed
I have also in my local git some branches modified in a similar fashion. 
Some weeks ago, Francois Perrad also suggested a patch with a similar 
modification (http://patchwork.ozlabs.org/patch/265214). Thomas also 
wrote a patch to get build time statistics   
(http://lists.busybox.net/pipermail/buildroot/2011-October/046513.html). 

IMHO, these patchs are too much specific and should not be mainlined. 
However, Buildroot may provide a generic framework to make this kind 
instrumentation easier. 

For example, we may add a (another) new hooks at beginning and end of 
each action of pkg-generic.mk. It will give opportunity to user to 
implement its instrumentation in local.mk.

Another need would be to call external scripts from these hooks. In this 
case, it may be difficult to pass all needed information to an external 
script (I mean all $($(PKG)_*) variables). Buildroot may provide a 
generic way to to it (passing $($(PKG)_*) in environment for example).


-- 
J?r?me Pouiller, Sysmic
Embedded Linux specialist
http://www.sysmic.fr

[Buildroot] [PATCH] pkg-infra: log current message

[Thomas Petazzoni]

Dear J?r?me Pouiller,

On Mon, 07 Oct 2013 12:09:50 +0200, J?r?me Pouiller wrote:

> I have also in my local git some branches modified in a similar fashion. 
> Some weeks ago, Francois Perrad also suggested a patch with a similar 
> modification (http://patchwork.ozlabs.org/patch/265214). Thomas also 
> wrote a patch to get build time statistics   
> (http://lists.busybox.net/pipermail/buildroot/2011-October/046513.html). 
> 
> IMHO, these patchs are too much specific and should not be mainlined. 

I am not sure I agree here. The fact that several of us have such
patches, and that they would indeed be useful to produce build time
statistics, or help the autobuilders provide better diagnostics could
be useful. I believe that it's the real strength of the common package
infrastructure: we can add such additional features on all packages at
once, very easily. Of course, it should be implemented in a nice and
clean way that doesn't complexity too much the core package
infrastructure, but it's very likely easy to achieve with some hooks.

Best regards,

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

[Buildroot] [PATCH] pkg-infra: log current message

[Yann E. MORIN]

Thomas, J?r?me, All,

On 2013-10-07 14:07 +0200, Thomas Petazzoni spake thusly:
> Dear J?r?me Pouiller,
> 
> On Mon, 07 Oct 2013 12:09:50 +0200, J?r?me Pouiller wrote:
> 
> > I have also in my local git some branches modified in a similar fashion. 
> > Some weeks ago, Francois Perrad also suggested a patch with a similar 
> > modification (http://patchwork.ozlabs.org/patch/265214). Thomas also 
> > wrote a patch to get build time statistics   
> > (http://lists.busybox.net/pipermail/buildroot/2011-October/046513.html). 
> > 
> > IMHO, these patchs are too much specific and should not be mainlined. 
> 
> I am not sure I agree here. The fact that several of us have such
> patches, and that they would indeed be useful to produce build time
> statistics, or help the autobuilders provide better diagnostics could
> be useful. I believe that it's the real strength of the common package
> infrastructure: we can add such additional features on all packages at
> once, very easily. Of course, it should be implemented in a nice and
> clean way that doesn't complexity too much the core package
> infrastructure, but it's very likely easy to achieve with some hooks.

What about adding a new Kconfig option like:

    config BR2_BUILD_INFRA_STEP_SCRIPT
        bool "script to run before and after each step"
        help
          Buildroot will call this script before executing any single
          step in the build process. The arguments to this script are:
              $1: either "pre" or "post", resp. meaning "before" or
                  "after" the step
              $2: name of the package
              $3: version of the package
              $4: action to be done

          Leave empty (the default) to not run any script.

It would then be the responsibility of the user to provide such a
script. We could provide a simple script that just do this logging as an
example.

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] pkg-infra: log current message

[Yann E. MORIN]

Thomas, All,

On 2013-10-07 18:48 +0200, Yann E. MORIN spake thusly:
> Thomas, J?r?me, All,
> 
> On 2013-10-07 14:07 +0200, Thomas Petazzoni spake thusly:
> > Dear J?r?me Pouiller,
> > 
> > On Mon, 07 Oct 2013 12:09:50 +0200, J?r?me Pouiller wrote:
> > 
> > > I have also in my local git some branches modified in a similar fashion. 
> > > Some weeks ago, Francois Perrad also suggested a patch with a similar 
> > > modification (http://patchwork.ozlabs.org/patch/265214). Thomas also 
> > > wrote a patch to get build time statistics   
> > > (http://lists.busybox.net/pipermail/buildroot/2011-October/046513.html). 
> > > 
> > > IMHO, these patchs are too much specific and should not be mainlined. 
> > 
> > I am not sure I agree here. The fact that several of us have such
> > patches, and that they would indeed be useful to produce build time
> > statistics, or help the autobuilders provide better diagnostics could
> > be useful. I believe that it's the real strength of the common package
> > infrastructure: we can add such additional features on all packages at
> > once, very easily. Of course, it should be implemented in a nice and
> > clean way that doesn't complexity too much the core package
> > infrastructure, but it's very likely easy to achieve with some hooks.
> 
> What about adding a new Kconfig option like:
> 
>     config BR2_BUILD_INFRA_STEP_SCRIPT
>         bool "script to run before and after each step"
>         help
>           Buildroot will call this script before executing any single
>           step in the build process. The arguments to this script are:
>               $1: either "pre" or "post", resp. meaning "before" or
>                   "after" the step
>               $2: name of the package
>               $3: version of the package
>               $4: action to be done
> 
>           Leave empty (the default) to not run any script.
> 
> It would then be the responsibility of the user to provide such a
> script. We could provide a simple script that just do this logging as an
> example.

I have another interesting idea about such a script: detect whether a
package overwrites or otherwise modifies a file already existing in
staging/ and/or target/ to assess for the sanity of the packaging.

We could also add another sanity-check to look at build issues (eg.
build-path in RPATH, build-path in scripts...).

Another not-so-much-interesting, wild idea is that this *could* be used
to prepare binary packages by looking at the list of files added by a
package, and creating an archive of just those files.

Really, I believe one could do such nice or nasty things with this! :-)
But that's no reason not to include it, mind you!. ;-)

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] pkg-infra: log current message

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Mon, 7 Oct 2013 18:59:56 +0200, Yann E. MORIN wrote:

> > It would then be the responsibility of the user to provide such a
> > script. We could provide a simple script that just do this logging as an
> > example.
> 
> I have another interesting idea about such a script: detect whether a
> package overwrites or otherwise modifies a file already existing in
> staging/ and/or target/ to assess for the sanity of the packaging.

I believe there are cases where we are doing this today, and where it
is valid. Think for example the full-blown versions of some commands
that overwrite the symbolic link installed by Busybox.

> We could also add another sanity-check to look at build issues (eg.
> build-path in RPATH, build-path in scripts...).

Right! However, I don't see why those checks should be in some external
scripts that the user should install. I believe those checks should be
part of the Buildroot core, possibly with a Config.in option to decide
whether a check failure should result to a global build failure or just
a warning (default value would be a warning, a global build failure
could be used in the autobuilders, for example).

Best regards,

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

[Buildroot] [PATCH] pkg-infra: log current message

[Jérôme Pouiller]

On Monday 07 October 2013 18:48:29 Yann E. MORIN wrote:
> Thomas, J?r?me, All,
> 
> On 2013-10-07 14:07 +0200, Thomas Petazzoni spake thusly:
> > Dear J?r?me Pouiller,
> > 
> > On Mon, 07 Oct 2013 12:09:50 +0200, J?r?me Pouiller wrote:
> > > I have also in my local git some branches modified in a similar
> > > fashion. Some weeks ago, Francois Perrad also suggested a patch
> > > with a similar modification
> > > (http://patchwork.ozlabs.org/patch/265214). Thomas also wrote a
> > > patch to get build time statistics
> > > (http://lists.busybox.net/pipermail/buildroot/2011-October/046513.
> > > html).
> > > 
> > > IMHO, these patchs are too much specific and should not be
> > > mainlined.
> > 
> > I am not sure I agree here. The fact that several of us have such
> > patches, and that they would indeed be useful to produce build time
> > statistics, or help the autobuilders provide better diagnostics
> > could
> > be useful. I believe that it's the real strength of the common
> > package infrastructure: we can add such additional features on all
> > packages at once, very easily. Of course, it should be implemented
> > in a nice and clean way that doesn't complexity too much the core
> > package
> > infrastructure, but it's very likely easy to achieve with some
> > hooks.
> 
> What about adding a new Kconfig option like:
> 
>     config BR2_BUILD_INFRA_STEP_SCRIPT
>         bool "script to run before and after each step"
>         help
>           Buildroot will call this script before executing any single
>           step in the build process. The arguments to this script are:
> $1: either "pre" or "post", resp. meaning "before" or "after" the
> step
>               $2: name of the package
>               $3: version of the package
Let's try to find a way to pass all VARIABLE relative to the package 
(version, dependencies, url, buildir, etc...). We may also want to pass 
TARGETDIR, HOSTDIR, etc... I thought to pass them using environment.

Else, it's good for me.

>               $4: action to be done
> 
>           Leave empty (the default) to not run any script.
> 
> It would then be the responsibility of the user to provide such a
> script. We could provide a simple script that just do this logging as
> an example.


-- 
J?r?me Pouiller, Sysmic
Embedded Linux specialist
http://www.sysmic.fr

[Buildroot] [PATCH] pkg-infra: log current message

[Jérôme Pouiller]

On Monday 07 October 2013 18:59:56 Yann E. MORIN wrote:
[...]
> I have another interesting idea about such a script: detect whether a
> package overwrites or otherwise modifies a file already existing in
> staging/ and/or target/ to assess for the sanity of the packaging.
I wrote something like this. It is able to track owner of each files of 
target/ and host/. It generates list of owners[1][2]. These list may be 
usefull to generate binary packages. 

I am also able to generate list of overwritten files[3][4]. 

Once I have list of owners in host/, I can track which files are 
accessed during build and compute dependencies between packages[5]. 

As you notice, it is only a proof of concept. There is plenty of false 
positives and I noticed many problems. Especially, current script is not 
very reliable when I launch compilation from a dirty directory. 

It is currently done by launching a daemon which watch target and host 
(using inotify) during each step a built. This daemon log access to 
files and another script post-process it. (I think, I would have far 
better results if I maintain in real time a database of owners of all 
installed files). I may send patches if you are interested (but they are 
a little ugly).


[1] http://sysmic.org/~jezz/instrument/host_owners.gz
[2] http://sysmic.org/~jezz/instrument/target_owners.gz
[3] http://sysmic.org/~jezz/instrument/host_owners.err.gz
[4] http://sysmic.org/~jezz/instrument/target_owners.err.gz
[5] http://sysmic.org/~jezz/instrument/depends

-- 
J?r?me Pouiller, Sysmic
Embedded Linux specialist
http://www.sysmic.fr

[Buildroot] [PATCH] pkg-infra: log current message

[Arnout Vandecappelle]

On 10/07/13 19:19, Thomas Petazzoni wrote:
> Dear Yann E. MORIN,
>
> On Mon, 7 Oct 2013 18:59:56 +0200, Yann E. MORIN wrote:
>
>>> It would then be the responsibility of the user to provide such a
>>> script. We could provide a simple script that just do this logging as an
>>> example.
>>
>> I have another interesting idea about such a script: detect whether a
>> package overwrites or otherwise modifies a file already existing in
>> staging/ and/or target/ to assess for the sanity of the packaging.
>
> I believe there are cases where we are doing this today, and where it
> is valid. Think for example the full-blown versions of some commands
> that overwrite the symbolic link installed by Busybox.
>
>> We could also add another sanity-check to look at build issues (eg.
>> build-path in RPATH, build-path in scripts...).
>
> Right! However, I don't see why those checks should be in some external
> scripts that the user should install. I believe those checks should be
> part of the Buildroot core, possibly with a Config.in option to decide
> whether a check failure should result to a global build failure or just
> a warning (default value would be a warning, a global build failure
> could be used in the autobuilders, for example).

  I agree with that. Any check or whatever that makes life easier for a 
contributor should be part of the mainline.

  That said, it still makes sense to give the possibility to add custom 
pre/post hooks. And autobuilder-specific things that are _not_ relevant 
to the generic contributor would fit there.

  Regards,
  Arnout


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH] pkg-infra: log current message

[Arnout Vandecappelle]

On 10/07/13 18:48, Yann E. MORIN wrote:
> Thomas, J?r?me, All,
> 
> On 2013-10-07 14:07 +0200, Thomas Petazzoni spake thusly:
>> Dear J?r?me Pouiller,
>>
>> On Mon, 07 Oct 2013 12:09:50 +0200, J?r?me Pouiller wrote:
>>
>>> I have also in my local git some branches modified in a similar fashion.
>>> Some weeks ago, Francois Perrad also suggested a patch with a similar
>>> modification (http://patchwork.ozlabs.org/patch/265214). Thomas also
>>> wrote a patch to get build time statistics
>>> (http://lists.busybox.net/pipermail/buildroot/2011-October/046513.html).
>>>
>>> IMHO, these patchs are too much specific and should not be mainlined.
>>
>> I am not sure I agree here. The fact that several of us have such
>> patches, and that they would indeed be useful to produce build time
>> statistics, or help the autobuilders provide better diagnostics could
>> be useful. I believe that it's the real strength of the common package
>> infrastructure: we can add such additional features on all packages at
>> once, very easily. Of course, it should be implemented in a nice and
>> clean way that doesn't complexity too much the core package
>> infrastructure, but it's very likely easy to achieve with some hooks.
> 
> What about adding a new Kconfig option like:
> 
>      config BR2_BUILD_INFRA_STEP_SCRIPT
>          bool "script to run before and after each step"
>          help
>            Buildroot will call this script before executing any single
>            step in the build process. The arguments to this script are:
>                $1: either "pre" or "post", resp. meaning "before" or
>                    "after" the step
>                $2: name of the package
>                $3: version of the package
>                $4: action to be done
> 
>            Leave empty (the default) to not run any script.
> 
> It would then be the responsibility of the user to provide such a
> script. We could provide a simple script that just do this logging as an
> example.

 I think that this kind of stuff is really orthogonal to the configuration. 
It is rather something local to the developer's environment. Therefore, I 
think it should be something that is defined in the local.mk.

 For instance:

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index bfc4dc1..ac44565 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -108,10 +108,12 @@ $(BUILD_DIR)/%/.stamp_patched:
 
 # Configure
 $(BUILD_DIR)/%/.stamp_configured:
+       $(foreach hook,$(GENERIC_HOOKS),$(call $(hook),pre,configure)$(sep))
        $(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
        @$(call MESSAGE,"Configuring")
        $($(PKG)_CONFIGURE_CMDS)
        $(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))
+       $(foreach hook,$(GENERIC_HOOKS),$(call $(hook),post,configure)$(sep))
        $(Q)touch $@
 
 # Build


 This also makes all make variables accessible in the hook, avoiding the 
problem that J?r?me pointed out.


 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F