Patchwork [1/6] pkg-infra: log current message

login
register
mail settings
Submitter Yann E. MORIN
Date Jan. 16, 2013, 11:41 p.m.
Message ID <7d5b13e61ca5941293be6d1402c87ca990510e1b.1358379198.git.yann.morin.1998@free.fr>
Download mbox | patch
Permalink /patch/213093/
State Changes Requested
Headers show

Comments

Yann E. MORIN - Jan. 16, 2013, 11:41 p.m.
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 files changed, 8 insertions(+), 1 deletions(-)
Markos Chandras - Jan. 17, 2013, 3:22 p.m.
On 16 January 2013 23:41, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> 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 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> index 835c588..477be57 100644
> --- a/package/pkg-utils.mk
> +++ b/package/pkg-utils.mk
> @@ -64,7 +64,14 @@ INFLATE.xz   = $(XZCAT)
>  INFLATE.tar  = cat
>
>  # 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.7.2.5
>
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Looks good to me

Reviewed-by: Markos Chandras <markos.chandras@imgtec.com>
Arnout Vandecappelle - Jan. 20, 2013, 1:49 p.m.
On 01/17/13 00:41, Yann E. MORIN wrote:
> 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

  I'm a bit concerned because this is another blocker for top-level 
parallel build (the contents of last-action will be incorrect).

  Also:

> 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.

  I think this one should be sufficient for the autobuilder. It's exactly 
what is displayed now. You could make it more structured and easily 
parseable with an extra colon:

echo "$$(date '+%s'): $($(PKG)_NAME) $($(PKG)_VERSION): $(1)"


  Note that this one is not an issue for parallel builds, because it will 
be done in a single write() call.


  Regards,
  Arnout

> 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 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> index 835c588..477be57 100644
> --- a/package/pkg-utils.mk
> +++ b/package/pkg-utils.mk
> @@ -64,7 +64,14 @@ INFLATE.xz   = $(XZCAT)
>   INFLATE.tar  = cat
>
>   # 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)
>
Yann E. MORIN - Jan. 20, 2013, 2:10 p.m.
Arnout, All,

On Sunday 20 January 2013 Arnout Vandecappelle wrote:
> On 01/17/13 00:41, Yann E. MORIN wrote:
> > 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
[--SNIP--]
>   I'm a bit concerned because this is another blocker for top-level 
> parallel build (the contents of last-action will be incorrect).

Do we really want to go for top-level parallel builds?
This has been discussed at length many times, and the drawbacks far
outweight any potential advantages.

> > 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.
> 
>   I think this one should be sufficient for the autobuilder. It's exactly 
> what is displayed now.

*IF* we were to implement top-level // build, then that log file would
not be of any use either, as the last one to fail would not necessarily
be the last entry:

  - one job starts configuring package foo  --> logged
  - a second jobs starts building package bar --> logged
  - foo configuration fails

In this case, the last log is not the failed action.

If we do really want top-level // build, then instrumenting the build in
the MESSAGE function is not adequate.

But I think top-level // builds should be avoided...

> You could make it more structured and easily 
> parseable with an extra colon:
> echo "$$(date '+%s'): $($(PKG)_NAME) $($(PKG)_VERSION): $(1)"

An extra colon will not make it easier to parse. With the current
format, it's already easy to extract each fields:

sed -r -e 's/^([[:digit:]]+) ([^[:space:]]*) ([^[:space:]]*) (.+)$/date: \1\npackage: \2\nversion: \3\nmesage: \4\n/;'

Regards,
Yann E. MORIN.
Thomas Petazzoni - Jan. 20, 2013, 2:38 p.m.
Dear Arnout Vandecappelle,

On Sun, 20 Jan 2013 14:49:30 +0100, Arnout Vandecappelle wrote:

>   I'm a bit concerned because this is another blocker for top-level 
> parallel build (the contents of last-action will be incorrect).

I agree that top-level parallel build would be nice, but I'm wondering
whether it is realistic to think we will support it one day. The main
problem I'm seeing with top-level parallel build is that we need to
create a per-package sysroot in order to get reproducible builds.

For the moment, we install all the libraries and headers in
$(STAGING_DIR), and we point all packages to $(STAGING_DIR) as the
toolchain sysroot, so that they find their required dependencies.

For now, the fact that the sysroot is unique and global is not a
problem as the build is completely serialized, and therefore
reproducible. If package A has an optional dependency on package B (not
expressed in Buildroot dependencies, but package A detects if B is
available in its configure script, and if it is, then uses B), then
either B is built before A, and A has B support, or B is built after A,
and A doesn't have B support.

If you switch to top-level parallel build, then sometimes, B will be
built before A, sometimes after, leading to A sometimes having B
support, sometimes B.

Of course, one way of seeing this is "that's a Buildroot bug, package A
should explicit its optional dependency on package B in its .mk file".
Unfortunately, in practice, it's going to be very hard to track *all*
those optional dependencies, especially when we bump package versions
and those bumps introduce need optional dependencies.

The only way to solve this is to create a per-package sysroot, in which
we install only the headers and libraries that the package explicitly
list as part of its dependencies. I know OpenBricks is doing that. And
I remember discussing with the OE-lite maintainer as this problem being
in his opinion the #1 reliability problem in OpenEmbedded.

Do we want to implement this per-package sysroot thing?

Best regards,

Thomas
Yann E. MORIN - Jan. 20, 2013, 2:59 p.m.
Thomas, Arnout, All,

On Sunday 20 January 2013 Thomas Petazzoni wrote:
> Dear Arnout Vandecappelle,
> 
> On Sun, 20 Jan 2013 14:49:30 +0100, Arnout Vandecappelle wrote:
> 
> >   I'm a bit concerned because this is another blocker for top-level 
> > parallel build (the contents of last-action will be incorrect).
> 
> I agree that top-level parallel build would be nice, but I'm wondering
> whether it is realistic to think we will support it one day.
[--SNIP--]
> Do we want to implement this per-package sysroot thing?

My stance on the subject is: no.

Although top-level // builds would be nice, as it would have the potential
to greatly speed up builds (expecially with many packages enabled), this
would complexifies Buildroot to a point where things will be much harder
to follow, understand, debug and fix.

Regards,
Yann E. MORIN.

Patch

diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index 835c588..477be57 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -64,7 +64,14 @@  INFLATE.xz   = $(XZCAT)
 INFLATE.tar  = cat
 
 # 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)