diff mbox series

[1/1] package/apparmor: fix per-package build with apache

Message ID 20200901201046.2757475-1-fontaine.fabrice@gmail.com
State Superseded
Headers show
Series [1/1] package/apparmor: fix per-package build with apache | expand

Commit Message

Fabrice Fontaine Sept. 1, 2020, 8:10 p.m. UTC
Per-package build of apparmor with apache fails on:

/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apparmor/host/x86_64-buildroot-linux-musl/sysroot/usr/bin/apxs  -c mod_apparmor.c -L/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apparmor/host/bin/../x86_64-buildroot-linux-musl/sysroot/usr/lib -lapparmor

/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/bin/../../usr/build-1/libtool --silent --mode=compile /usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/bin/x86_64-linux-gcc -prefer-pic -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -g2    -I/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/include  -I/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/bin/../../usr/include/apr-1   -I/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/bin/../../usr/include/apr-1 -I/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/bin/../../../../x86_64-buildroot-lin
 ux-musl/sysroot/usr/include  -c -o mod_apparmor.lo mod_apparmor.c && touch mod_apparmor.slo
mod_apparmor.c:28:10: fatal error: sys/apparmor.h: No such file or directory
 #include <sys/apparmor.h>
          ^~~~~~~~~~~~~~~~

The issue is that sys/appamor.h is not installed in the apache
per-package directory which is mangled by
APACHE_FIX_STAGING_APACHE_CONFIG, i.e.
/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/include

So help apxs to find sys/apparmor.h by passing
-I$(STAGING_DIR)/usr/include

Fixes:
 - http://autobuild.buildroot.org/results/ef1fcd57e0c09a2806bf2272bb21df6d3300b45b

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 package/apparmor/apparmor.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Yann E. MORIN Sept. 5, 2020, 12:50 p.m. UTC | #1
Fabrice, All,

+Thomas, +Peter, +Arnout: please read in full, then there is a question
for you toward the end...

On 2020-09-01 22:10 +0200, Fabrice Fontaine spake thusly:
> Per-package build of apparmor with apache fails on:
> 
> /usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apparmor/host/x86_64-buildroot-linux-musl/sysroot/usr/bin/apxs  -c mod_apparmor.c -L/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apparmor/host/bin/../x86_64-buildroot-linux-musl/sysroot/usr/lib -lapparmor
> 
> /usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/bin/../../usr/build-1/libtool --silent --mode=compile /usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/bin/x86_64-linux-gcc -prefer-pic -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -g2    -I/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/include  -I/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/bin/../../usr/include/apr-1   -I/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/bin/../../usr/include/apr-1 -I/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/bin/../../../../x86_64-buildroot-lin
>  ux-musl/sysroot/usr/include  -c -o mod_apparmor.lo mod_apparmor.c && touch mod_apparmor.slo
> mod_apparmor.c:28:10: fatal error: sys/apparmor.h: No such file or directory
>  #include <sys/apparmor.h>
>           ^~~~~~~~~~~~~~~~
> 
> The issue is that sys/appamor.h is not installed in the apache
> per-package directory which is mangled by
> APACHE_FIX_STAGING_APACHE_CONFIG, i.e.
> /usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/include
> 
> So help apxs to find sys/apparmor.h by passing
> -I$(STAGING_DIR)/usr/include

For a short time, I was a bit puzzled as to why adding the current
sysroot explicitly would fix the issue. That's in fact because
sys/apparmor.h comes from the libapparmor package, not from the apparmor
pacakge itself.

So, indeed, this technically fixes the issue.

However, see below, I think there is a more fundamental issue with apxs
and how we "fix" and use it.

> Fixes:
>  - http://autobuild.buildroot.org/results/ef1fcd57e0c09a2806bf2272bb21df6d3300b45b
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
>  package/apparmor/apparmor.mk | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/package/apparmor/apparmor.mk b/package/apparmor/apparmor.mk
> index f667966082..d5794b9585 100644
> --- a/package/apparmor/apparmor.mk
> +++ b/package/apparmor/apparmor.mk
> @@ -52,7 +52,8 @@ endif
>  ifeq ($(BR2_PACKAGE_APACHE),y)
>  APPARMOR_DEPENDENCIES += apache
>  APPARMOR_TOOLS += changehat/mod_apparmor
> -APPARMOR_MAKE_OPTS += APXS=$(STAGING_DIR)/usr/bin/apxs
> +APPARMOR_MAKE_OPTS += \
> +	APXS="$(STAGING_DIR)/usr/bin/apxs -I$(STAGING_DIR)/usr/include"

So, APXS contains hard-coded path to the apache's sysroot, but also host
directory. This can be seen in the traces you included in your commit
log (lines manually splitted for (relative) readability):

    /usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apparmor/host/x86_64-buildroot-linux-musl/sysroot/usr/bin/apxs \
        -c mod_apparmor.c \
        -L/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apparmor/host/bin/../x86_64-buildroot-linux-musl/sysroot/usr/lib \
        -lapparmor
    /usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/bin/../../usr/build-1/libtool \
        --silent --mode=compile \
        /usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/bin/x86_64-linux-gcc \
        -prefer-pic -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -g2 \
        -I/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/include \
        -I/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/bin/../../usr/include/apr-1 \
        -I/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/bin/../../usr/include/apr-1 \
        -I/usr/lfs/hdd_v1/rc-buildroot-test/scripts/instance-0/output-1/per-package/apache/host/x86_64-buildroot-linux-musl/sysroot/usr/bin/../../../../x86_64-buildroot-linux-musl/sysroot/usr/include \
        -c -o mod_apparmor.lo mod_apparmor.c && touch mod_apparmor.slo

So, it is calling libtool from apache's host dir, and consequently, it
is calling gcc from apache's host dir, and uses include paths that point
to apache's sysroot.

This is no good.

In theory, we would want to fix apxs so we can only use path to the
current package's sysroot. This could probably be achievable, because:

  - apxs is a perl script, so we could either sed it to replace paths,
    but this is not noce, or we could fix it to introduce a variable
    that contains the base directory to look from;

  - there is a config_vars.mk file that contains hard-coded paths, which
    we could probably modify to also include a base directory to look
    from. For example, we could introduce "APXS_BASE_DIR = blabla", and
    then, right before the condifure step of dependant packages, fixup
    that file to inject the correct PPD base directory.

This would not be very different from the fixups we apply to the meson's
cross-compilation.conf configuration file, but there we do have an edge,
in that this is done by an infra.

However, for this apxs configs_var.mk, it is not so trivial, because
this is not part of an infra, and we do not have a way for packages to
export a hook that other packages would have to run automatically.

We could probably have the apache package provide this as a set of
macros:

    define APACHE_APXS_FIXUPS
        $(SED) blablaba $(STAGING_DIR)/usr/build/config_vars.mk
    endef

and then packages would do something like:

    APPARMOR_PRE_CONFIGURE_HOOKS += APACHE_APXS_FIXUPS

But:

 1. this is not so clean, because of the cross-package reference,

 2. we so far have only two packages using apxs, and that is probably
    too-big a hammer to introduce this generic tweaking...

So, I am still a bit undecided whether to apply this quick workaround
your patch is, or to require a full-blown, complete and correct fix...

Thomas, Peter, Arnout, thoughts?

Regards,
Yann E. MORIN.

>  endif
>  
>  define APPARMOR_BUILD_CMDS
> -- 
> 2.28.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Thomas Petazzoni Sept. 5, 2020, 1:16 p.m. UTC | #2
Hello,

On Sat, 5 Sep 2020 14:50:58 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> So, it is calling libtool from apache's host dir, and consequently, it
> is calling gcc from apache's host dir, and uses include paths that point
> to apache's sysroot.
> 
> This is no good.
> 
> In theory, we would want to fix apxs so we can only use path to the
> current package's sysroot. This could probably be achievable, because:
> 
>   - apxs is a perl script, so we could either sed it to replace paths,
>     but this is not noce, or we could fix it to introduce a variable
>     that contains the base directory to look from;
> 
>   - there is a config_vars.mk file that contains hard-coded paths, which
>     we could probably modify to also include a base directory to look
>     from. For example, we could introduce "APXS_BASE_DIR = blabla", and
>     then, right before the condifure step of dependant packages, fixup
>     that file to inject the correct PPD base directory.
> 
> This would not be very different from the fixups we apply to the meson's
> cross-compilation.conf configuration file, but there we do have an edge,
> in that this is done by an infra.
> 
> However, for this apxs configs_var.mk, it is not so trivial, because
> this is not part of an infra, and we do not have a way for packages to
> export a hook that other packages would have to run automatically.
> 
> We could probably have the apache package provide this as a set of
> macros:
> 
>     define APACHE_APXS_FIXUPS
>         $(SED) blablaba $(STAGING_DIR)/usr/build/config_vars.mk
>     endef
> 
> and then packages would do something like:
> 
>     APPARMOR_PRE_CONFIGURE_HOOKS += APACHE_APXS_FIXUPS
> 
> But:
> 
>  1. this is not so clean, because of the cross-package reference,
> 
>  2. we so far have only two packages using apxs, and that is probably
>     too-big a hammer to introduce this generic tweaking...
> 
> So, I am still a bit undecided whether to apply this quick workaround
> your patch is, or to require a full-blown, complete and correct fix...
> 
> Thomas, Peter, Arnout, thoughts?

We're already fixing a similar issue in the apache package as follows:

ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
define APACHE_FIXUP_APR_LIBTOOL
	$(SED) "s@$(PER_PACKAGE_DIR)/[^/]\+/@$(PER_PACKAGE_DIR)/apache/@g" \
		$(STAGING_DIR)/usr/build-1/libtool
endef
APACHE_POST_CONFIGURE_HOOKS += APACHE_FIXUP_APR_LIBTOOL
endif

Any reason not to do the same here ?

Thomas
Fabrice Fontaine Sept. 5, 2020, 9:36 p.m. UTC | #3
Hello,

Le sam. 5 sept. 2020 à 15:16, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> a écrit :
>
> Hello,
>
> On Sat, 5 Sep 2020 14:50:58 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
>
> > So, it is calling libtool from apache's host dir, and consequently, it
> > is calling gcc from apache's host dir, and uses include paths that point
> > to apache's sysroot.
> >
> > This is no good.
> >
> > In theory, we would want to fix apxs so we can only use path to the
> > current package's sysroot. This could probably be achievable, because:
> >
> >   - apxs is a perl script, so we could either sed it to replace paths,
> >     but this is not noce, or we could fix it to introduce a variable
> >     that contains the base directory to look from;
> >
> >   - there is a config_vars.mk file that contains hard-coded paths, which
> >     we could probably modify to also include a base directory to look
> >     from. For example, we could introduce "APXS_BASE_DIR = blabla", and
> >     then, right before the condifure step of dependant packages, fixup
> >     that file to inject the correct PPD base directory.
> >
> > This would not be very different from the fixups we apply to the meson's
> > cross-compilation.conf configuration file, but there we do have an edge,
> > in that this is done by an infra.
> >
> > However, for this apxs configs_var.mk, it is not so trivial, because
> > this is not part of an infra, and we do not have a way for packages to
> > export a hook that other packages would have to run automatically.
> >
> > We could probably have the apache package provide this as a set of
> > macros:
> >
> >     define APACHE_APXS_FIXUPS
> >         $(SED) blablaba $(STAGING_DIR)/usr/build/config_vars.mk
> >     endef
> >
> > and then packages would do something like:
> >
> >     APPARMOR_PRE_CONFIGURE_HOOKS += APACHE_APXS_FIXUPS
> >
> > But:
> >
> >  1. this is not so clean, because of the cross-package reference,
> >
> >  2. we so far have only two packages using apxs, and that is probably
> >     too-big a hammer to introduce this generic tweaking...
> >
> > So, I am still a bit undecided whether to apply this quick workaround
> > your patch is, or to require a full-blown, complete and correct fix...
> >
> > Thomas, Peter, Arnout, thoughts?
>
> We're already fixing a similar issue in the apache package as follows:
>
> ifeq ($(BR2_PER_PACKAGE_DIRECTORIES),y)
> define APACHE_FIXUP_APR_LIBTOOL
>         $(SED) "s@$(PER_PACKAGE_DIR)/[^/]\+/@$(PER_PACKAGE_DIR)/apache/@g" \
>                 $(STAGING_DIR)/usr/build-1/libtool
> endef
> APACHE_POST_CONFIGURE_HOOKS += APACHE_FIXUP_APR_LIBTOOL
> endif
>
> Any reason not to do the same here ?
Thanks for spotting this solution, I'll send a v2.
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Best Regards,

Fabrice
diff mbox series

Patch

diff --git a/package/apparmor/apparmor.mk b/package/apparmor/apparmor.mk
index f667966082..d5794b9585 100644
--- a/package/apparmor/apparmor.mk
+++ b/package/apparmor/apparmor.mk
@@ -52,7 +52,8 @@  endif
 ifeq ($(BR2_PACKAGE_APACHE),y)
 APPARMOR_DEPENDENCIES += apache
 APPARMOR_TOOLS += changehat/mod_apparmor
-APPARMOR_MAKE_OPTS += APXS=$(STAGING_DIR)/usr/bin/apxs
+APPARMOR_MAKE_OPTS += \
+	APXS="$(STAGING_DIR)/usr/bin/apxs -I$(STAGING_DIR)/usr/include"
 endif
 
 define APPARMOR_BUILD_CMDS