diff mbox series

core/pkg-generic: fix instrumentation for packages that install nowhere

Message ID 20210613143207.2445691-1-yann.morin.1998@free.fr
State Accepted
Headers show
Series core/pkg-generic: fix instrumentation for packages that install nowhere | expand

Commit Message

Yann E. MORIN June 13, 2021, 2:32 p.m. UTC
A (target [0]) package can independently declare installing in various
locations: target, staging, or images. The default is to only install
in target.

When a package opts out from installing to target, but does not opts
in to install in any other location, the package is not downloaded,
extracted, patched, configured, nor built at all. As a consequence, none
of the per-step instrumentation is not executed, specifically the
listing of files before/after the package sequence.

Down the line, the package infra does not cope well with that situation,
because the gathering-install step, the one that synchronises all the
optional target, staging, or images install steps, still gets run.

And as #13836 shows, this does not go well:

    /bin/sh: /home/tbuild/myboard/build/foo/.files-list.after: No such file or directory
    make[1]: *** [/home/tbuild/myboard/build/foo/.stamp_installed] Error 1
    make: *** [_all] Error 2

So, we should have ensured that the gathering-install step itself
depends on the build step, which would have solved the issue.

However, this bug really illustrates a more fundamental issue: does it
even make sense to have a package that installs nothing in any location?
Indeed, why even bother with that package to begin with if it will not
provide anything at all?

It turns out that yes, this makes sense. We have some packages, that
do not install anything at all, and do not even build anything; they are
there just to ensure that we can download something that will ultimately
be used by another package. This is the case for example for packages
that provide linux extensions, like aufs [1].

Additionally, some ugly out-of-tree packages could conceivably install
things during the build (or even configure!) steps. That's not unheard
of... [2]

So, the solution is to ensure that the gathering-install step does
depend on the build step, to trigger the proper dependency chain and
have the instrumentation hooks properly run even in that degenerate
case.

Fixes: #13836

[0] a host package can't opt out of installing anything.
[1] that one is actually missing AUFS_INSTALL_TARGET = NO, so this
    hides the issue.
[2] even us are not 100% clean on that topic: gcc will install files in
    staging and target as part of the same step (not the build, granted,
    but still...)

Reported-by: "Weber, Matthew L Collins" <Matthew.Weber@collins.com>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 package/pkg-generic.mk | 1 +
 1 file changed, 1 insertion(+)

Comments

Voss, Samuel M Collins via buildroot June 15, 2021, 7:40 p.m. UTC | #1
Yann,

> From: Yann E. MORIN <yann.morin.1998@gmail.com> on behalf of Yann E. MORIN <yann.morin.1998@free.fr>
> Sent: Sunday, June 13, 2021 9:32 AM
> To: buildroot@buildroot.org <buildroot@buildroot.org>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>; Weber, Matthew L Collins <Matthew.Weber@collins.com>; Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Subject: [External] [PATCH] core/pkg-generic: fix instrumentation for packages that install nowhere 
>  
> A (target [0]) package can independently declare installing in various
> locations: target, staging, or images. The default is to only install
> in target.
> 

[snip]

>
> Fixes: #13836
>
> [0] a host package can't opt out of installing anything.
> [1] that one is actually missing AUFS_INSTALL_TARGET = NO, so this
>     hides the issue.
> [2] even us are not 100% clean on that topic: gcc will install files in
>     staging and target as part of the same step (not the build, granted,
>     but still...)
>
> Reported-by: "Weber, Matthew L Collins" <Matthew.Weber@collins.com>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Tested-by: Matthew Weber <matthew.weber@collins.com
Yann E. MORIN June 16, 2021, 7:59 p.m. UTC | #2
Matthew, All,

On 2021-06-15 19:40 +0000, Weber, Matthew L                            Collins spake thusly:
> > From: Yann E. MORIN <yann.morin.1998@gmail.com> on behalf of Yann E. MORIN <yann.morin.1998@free.fr>
> > Sent: Sunday, June 13, 2021 9:32 AM
> > To: buildroot@buildroot.org <buildroot@buildroot.org>
> > Cc: Yann E. MORIN <yann.morin.1998@free.fr>; Weber, Matthew L Collins <Matthew.Weber@collins.com>; Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > Subject: [External] [PATCH] core/pkg-generic: fix instrumentation for packages that install nowhere 
> >  
> > A (target [0]) package can independently declare installing in various
> > locations: target, staging, or images. The default is to only install
> > in target.
[--SNIP--]
> > Fixes: #13836
> >
> > [0] a host package can't opt out of installing anything.
> > [1] that one is actually missing AUFS_INSTALL_TARGET = NO, so this
> >     hides the issue.
> > [2] even us are not 100% clean on that topic: gcc will install files in
> >     staging and target as part of the same step (not the build, granted,
> >     but still...)
> >
> > Reported-by: "Weber, Matthew L Collins" <Matthew.Weber@collins.com>
> > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Tested-by: Matthew Weber <matthew.weber@collins.com

Applied to master, thanks.

Regards,
Yann E. MORIN.
Peter Korsgaard June 21, 2021, 8:18 p.m. UTC | #3
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > A (target [0]) package can independently declare installing in various
 > locations: target, staging, or images. The default is to only install
 > in target.

 > When a package opts out from installing to target, but does not opts
 > in to install in any other location, the package is not downloaded,
 > extracted, patched, configured, nor built at all. As a consequence, none
 > of the per-step instrumentation is not executed, specifically the
 > listing of files before/after the package sequence.

 > Down the line, the package infra does not cope well with that situation,
 > because the gathering-install step, the one that synchronises all the
 > optional target, staging, or images install steps, still gets run.

 > And as #13836 shows, this does not go well:

 >     /bin/sh: /home/tbuild/myboard/build/foo/.files-list.after: No such file or directory
 >     make[1]: *** [/home/tbuild/myboard/build/foo/.stamp_installed] Error 1
 >     make: *** [_all] Error 2

 > So, we should have ensured that the gathering-install step itself
 > depends on the build step, which would have solved the issue.

 > However, this bug really illustrates a more fundamental issue: does it
 > even make sense to have a package that installs nothing in any location?
 > Indeed, why even bother with that package to begin with if it will not
 > provide anything at all?

 > It turns out that yes, this makes sense. We have some packages, that
 > do not install anything at all, and do not even build anything; they are
 > there just to ensure that we can download something that will ultimately
 > be used by another package. This is the case for example for packages
 > that provide linux extensions, like aufs [1].

 > Additionally, some ugly out-of-tree packages could conceivably install
 > things during the build (or even configure!) steps. That's not unheard
 > of... [2]

 > So, the solution is to ensure that the gathering-install step does
 > depend on the build step, to trigger the proper dependency chain and
 > have the instrumentation hooks properly run even in that degenerate
 > case.

 > Fixes: #13836

 > [0] a host package can't opt out of installing anything.
 > [1] that one is actually missing AUFS_INSTALL_TARGET = NO, so this
 >     hides the issue.
 > [2] even us are not 100% clean on that topic: gcc will install files in
 >     staging and target as part of the same step (not the build, granted,
 >     but still...)

 > Reported-by: "Weber, Matthew L Collins" <Matthew.Weber@collins.com>
 > Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
 > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Committed to 2021.02.x and 2021.05.x, thanks.
diff mbox series

Patch

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 9fbc63d19e..45589bcbb4 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -826,6 +826,7 @@  endif
 # human-friendly targets and target sequencing
 $(1):			$(1)-install
 $(1)-install:		$$($(2)_TARGET_INSTALL)
+$$($(2)_TARGET_INSTALL): $$($(2)_TARGET_BUILD)
 
 ifeq ($$($(2)_TYPE),host)
 $$($(2)_TARGET_INSTALL): $$($(2)_TARGET_INSTALL_HOST)