[Buildroot] [PATCH/next 1/2] package/libcap: drop host-gperf dependency

[Buildroot] [PATCH/next 1/2] package/libcap: drop host-gperf dependency

[Fabrice Fontaine]

host-gperf dependency was added in commit
5d8926add5da1b0bdfb90a41f4d7f857864c5524 without any explanation in the
commit message but gperf can be disabled through BUILD_GPERF since
https://git.kernel.org/pub/scm/linux/kernel/git/morgan/libcap.git/commit/?id=3c22870c762f7925b5ff143d76f9affbade275ba

So use this variable and drop this unneeded dependency

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 package/libcap/libcap.mk | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/package/libcap/libcap.mk b/package/libcap/libcap.mk
index 08c1bc9deb..208f8eade1 100644
--- a/package/libcap/libcap.mk
+++ b/package/libcap/libcap.mk
@@ -10,11 +10,9 @@ LIBCAP_SOURCE = libcap-$(LIBCAP_VERSION).tar.xz
 LIBCAP_LICENSE = GPL-2.0 or BSD-3-Clause
 LIBCAP_LICENSE_FILES = License
 
-LIBCAP_DEPENDENCIES = host-libcap host-gperf
+LIBCAP_DEPENDENCIES = host-libcap
 LIBCAP_INSTALL_STAGING = YES
 
-HOST_LIBCAP_DEPENDENCIES = host-gperf
-
 ifeq ($(BR2_STATIC_LIBS),y)
 LIBCAP_MAKE_TARGET = libcap.a libcap.pc
 LIBCAP_MAKE_INSTALL_TARGET = install-static
@@ -28,7 +26,8 @@ endif
 
 LIBCAP_MAKE_FLAGS = \
 	BUILD_CC="$(HOSTCC)" \
-	BUILD_CFLAGS="$(HOST_CFLAGS)"
+	BUILD_CFLAGS="$(HOST_CFLAGS)" \
+	BUILD_GPERF=no
 
 ifeq ($(BR2_PACKAGE_LIBCAP_TOOLS),y)
 define LIBCAP_BUILD_TOOLS_CMDS
@@ -62,12 +61,12 @@ endef
 
 define HOST_LIBCAP_BUILD_CMDS
 	$(HOST_MAKE_ENV) $(HOST_CONFIGURE_OPTS) $(MAKE) -C $(@D)\
-		RAISE_SETFCAP=no
+		BUILD_GPERF=no RAISE_SETFCAP=no
 endef
 
 define HOST_LIBCAP_INSTALL_CMDS
 	$(HOST_MAKE_ENV) $(MAKE) -C $(@D) prefix=$(HOST_DIR) \
-		RAISE_SETFCAP=no lib=lib install
+		BUILD_GPERF=no RAISE_SETFCAP=no lib=lib install
 endef
 
 $(eval $(generic-package))
-- 
2.28.0

[Buildroot] [PATCH/next 2/2] package/libcap: drop host-libcap dependency

[Fabrice Fontaine]

host-libcap was added as a dependency of libcap in commit
efae605c88deb15d9c50fd67bdd80f46a78b06c1 without any explanation in the
commit message. Drop this dependency that does seem to be needed.

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

diff --git a/package/libcap/libcap.mk b/package/libcap/libcap.mk
index 208f8eade1..ead5a17dcc 100644
--- a/package/libcap/libcap.mk
+++ b/package/libcap/libcap.mk
@@ -10,7 +10,6 @@ LIBCAP_SOURCE = libcap-$(LIBCAP_VERSION).tar.xz
 LIBCAP_LICENSE = GPL-2.0 or BSD-3-Clause
 LIBCAP_LICENSE_FILES = License
 
-LIBCAP_DEPENDENCIES = host-libcap
 LIBCAP_INSTALL_STAGING = YES
 
 ifeq ($(BR2_STATIC_LIBS),y)
-- 
2.28.0

Re: [Buildroot] [PATCH/next 2/2] package/libcap: drop host-libcap dependency

[Romain Naour]

Hello Fabrice,

Le 30/08/2020 à 12:00, Fabrice Fontaine a écrit :
> host-libcap was added as a dependency of libcap in commit
> efae605c88deb15d9c50fd67bdd80f46a78b06c1 without any explanation in the
> commit message. Drop this dependency that does seem to be needed.

I dont see why host-libcap would be needed to build libcap for the target.

libcap in Yocto doesn't add libcap-native in dependency:

https://github.com/openembedded/openembedded-core/blob/master/meta/recipes-support/libcap/libcap_2.62.bb

This patch needs to be rebased on master since host-gperf has been added rencently.

Best regards,
Romain


> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
>  package/libcap/libcap.mk | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/package/libcap/libcap.mk b/package/libcap/libcap.mk
> index 208f8eade1..ead5a17dcc 100644
> --- a/package/libcap/libcap.mk
> +++ b/package/libcap/libcap.mk
> @@ -10,7 +10,6 @@ LIBCAP_SOURCE = libcap-$(LIBCAP_VERSION).tar.xz
>  LIBCAP_LICENSE = GPL-2.0 or BSD-3-Clause
>  LIBCAP_LICENSE_FILES = License
>  
> -LIBCAP_DEPENDENCIES = host-libcap
>  LIBCAP_INSTALL_STAGING = YES
>  
>  ifeq ($(BR2_STATIC_LIBS),y)
> 

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH/next 2/2] package/libcap: drop host-libcap dependency

[Thomas Petazzoni]

On Sat, 8 Jan 2022 21:44:49 +0100
Romain Naour <romain.naour@smile.fr> wrote:

> Hello Fabrice,
> 
> Le 30/08/2020 à 12:00, Fabrice Fontaine a écrit :
> > host-libcap was added as a dependency of libcap in commit
> > efae605c88deb15d9c50fd67bdd80f46a78b06c1 without any explanation in the
> > commit message. Drop this dependency that does seem to be needed.  
> 
> I dont see why host-libcap would be needed to build libcap for the target.
> 
> libcap in Yocto doesn't add libcap-native in dependency:
> 
> https://github.com/openembedded/openembedded-core/blob/master/meta/recipes-support/libcap/libcap_2.62.bb
> 
> This patch needs to be rebased on master since host-gperf has been added rencently.

See https://git.buildroot.org/buildroot/commit/?id=0651b22d638faecb3688f1fd4e0760c31bfba893

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH/next 2/2] package/libcap: drop host-libcap dependency

[Thomas Petazzoni]

On Sun, 30 Aug 2020 12:00:58 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> host-libcap was added as a dependency of libcap in commit
> efae605c88deb15d9c50fd67bdd80f46a78b06c1 without any explanation in the
> commit message. Drop this dependency that does seem to be needed.
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
>  package/libcap/libcap.mk | 1 -
>  1 file changed, 1 deletion(-)

We did some research, and figured out why the dependency was needed
back then, and is no longer now. I've expanded the commit log with the
explanation, and applied. See the final commit at:

  https://git.buildroot.org/buildroot/commit/?id=0651b22d638faecb3688f1fd4e0760c31bfba893

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH/next 1/2] package/libcap: drop host-gperf dependency

[Thomas Petazzoni]

Hello Fabrice,

On Sun, 30 Aug 2020 12:00:57 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> host-gperf dependency was added in commit
> 5d8926add5da1b0bdfb90a41f4d7f857864c5524 without any explanation in the
> commit message but gperf can be disabled through BUILD_GPERF since
> https://git.kernel.org/pub/scm/linux/kernel/git/morgan/libcap.git/commit/?id=3c22870c762f7925b5ff143d76f9affbade275ba
> 
> So use this variable and drop this unneeded dependency
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>

Thanks for looking into this. I agree that BUILD_GPERF=no will avoid
the host-gperf dependency, however I'm a bit confused how this works in
libcap and whether it has any drawback.

When gperf is detected, it will be used to generate some C code that
contains a hash function:

ifeq ($(BUILD_GPERF),yes)
USE_GPERF_OUTPUT = $(GPERF_OUTPUT)
INCLUDE_GPERF_OUTPUT = -DINCLUDE_GPERF_OUTPUT='"$(GPERF_OUTPUT)"'
endif

$(GPERF_OUTPUT): cap_names.list.h
        perl -e 'print "struct __cap_token_s { const char *name; int index; };\n%{\nconst struct __cap_token_s *__cap_lookup_name(const char *, size_t);\n%}\n%%\n"; while ($$l = <>) { $$l =~ s/[\{\"]//g; $$l =~ s/\}.*// ; print $$l; }' < $< | gperf --ignore-case --language=ANSI-C --readonly --null-strings --global-table --hash-function-name=__cap_hash_name --lookup-function-name="__cap_lookup_name" -c -t -m20 $(INDENT) > $@
        sed -e 's/unsigned int len/size_t len/' -i $@

cap_text.o: cap_text.c $(USE_GPERF_OUTPUT) $(INCLS)
        $(CC) $(CFLAGS) $(IPATH) $(INCLUDE_GPERF_OUTPUT) -c $< -o $@

Then in cap_text.c, this gets used like this:

#ifdef INCLUDE_GPERF_OUTPUT
/* we need to include it after #define _GNU_SOURCE is set */
#include INCLUDE_GPERF_OUTPUT
#endif

So the gperf-generated source file, if it exists is included. But I
don't see any conditional code that makes use of it when available.

Am I missing something? I wanted to understand the impact of not having
gperf. Perhaps not having a hash function makes libcap significantly
slower ?

Initially, I was expected libcap to have a pre-generated version of the
file, and having gperf available would only allow to re-generate the
file, but that doesn't seem to be what's happening.

Do you understand a bit better what is going on here ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH/next 1/2] package/libcap: drop host-gperf dependency

[Fabrice Fontaine]

Hello Thomas,

Le jeu. 3 sept. 2020 ? 22:47, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> a ?crit :
>
> Hello Fabrice,
>
> On Sun, 30 Aug 2020 12:00:57 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>
> > host-gperf dependency was added in commit
> > 5d8926add5da1b0bdfb90a41f4d7f857864c5524 without any explanation in the
> > commit message but gperf can be disabled through BUILD_GPERF since
> > https://git.kernel.org/pub/scm/linux/kernel/git/morgan/libcap.git/commit/?id=3c22870c762f7925b5ff143d76f9affbade275ba
> >
> > So use this variable and drop this unneeded dependency
> >
> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
>
> Thanks for looking into this. I agree that BUILD_GPERF=no will avoid
> the host-gperf dependency, however I'm a bit confused how this works in
> libcap and whether it has any drawback.
>
> When gperf is detected, it will be used to generate some C code that
> contains a hash function:
>
> ifeq ($(BUILD_GPERF),yes)
> USE_GPERF_OUTPUT = $(GPERF_OUTPUT)
> INCLUDE_GPERF_OUTPUT = -DINCLUDE_GPERF_OUTPUT='"$(GPERF_OUTPUT)"'
> endif
>
> $(GPERF_OUTPUT): cap_names.list.h
>         perl -e 'print "struct __cap_token_s { const char *name; int index; };\n%{\nconst struct __cap_token_s *__cap_lookup_name(const char *, size_t);\n%}\n%%\n"; while ($$l = <>) { $$l =~ s/[\{\"]//g; $$l =~ s/\}.*// ; print $$l; }' < $< | gperf --ignore-case --language=ANSI-C --readonly --null-strings --global-table --hash-function-name=__cap_hash_name --lookup-function-name="__cap_lookup_name" -c -t -m20 $(INDENT) > $@
>         sed -e 's/unsigned int len/size_t len/' -i $@
>
> cap_text.o: cap_text.c $(USE_GPERF_OUTPUT) $(INCLS)
>         $(CC) $(CFLAGS) $(IPATH) $(INCLUDE_GPERF_OUTPUT) -c $< -o $@
>
> Then in cap_text.c, this gets used like this:
>
> #ifdef INCLUDE_GPERF_OUTPUT
> /* we need to include it after #define _GNU_SOURCE is set */
> #include INCLUDE_GPERF_OUTPUT
> #endif
>
> So the gperf-generated source file, if it exists is included. But I
> don't see any conditional code that makes use of it when available.
gperf code has been added with
https://git.kernel.org/pub/scm/linux/kernel/git/morgan/libcap.git/commit/?id=e57378c88b6144ff9c06777ff0e0c9d722eeefd3

From my understanding, gperf code is used in libcap/cap_text.c by the
lookupname function:

#ifdef GPERF_DOWNCASE
 const struct __cap_token_s *token_info;
 int c;
 unsigned len;

 for (len=0; (c = str.constp[len]); ++len) {
 if (!(isalpha(c) || (c == '_'))) {
 break;
 }
 }
 token_info = __cap_lookup_name(str.constp, len);
 if (token_info != NULL) {
 *strp = str.constp + len;
 return token_info->index;
 }
#else /* ie., ndef GPERF_DOWNCASE */

lookupname is used by cap_from_name which is a part of the public API
and is also used by capsh program.
>
> Am I missing something? I wanted to understand the impact of not having
> gperf. Perhaps not having a hash function makes libcap significantly
> slower ?
Yes, you're right, not having gperf will have an unknown (but probably
small?) performance impact.
I didn't take time to measure it on an embedded target but we should
probably keep building host-gperf for the target (and probably also
for the host as host-gperf is not a "huge" dependency).
>
> Initially, I was expected libcap to have a pre-generated version of the
> file, and having gperf available would only allow to re-generate the
> file, but that doesn't seem to be what's happening.
>
> Do you understand a bit better what is going on here ?
>
> Thanks,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Best Regards,

Fabrice

Re: [Buildroot] [PATCH/next 1/2] package/libcap: drop host-gperf dependency

[Thomas Petazzoni]

On Sun, 30 Aug 2020 12:00:57 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> host-gperf dependency was added in commit
> 5d8926add5da1b0bdfb90a41f4d7f857864c5524 without any explanation in the
> commit message but gperf can be disabled through BUILD_GPERF since
> https://git.kernel.org/pub/scm/linux/kernel/git/morgan/libcap.git/commit/?id=3c22870c762f7925b5ff143d76f9affbade275ba
> 
> So use this variable and drop this unneeded dependency
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
>  package/libcap/libcap.mk | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)

So we discussed this with Yann, Peter and Arnout and found that
host-gperf is really quick to build, and provides a better way for
libcap to lookup symbols, thanks to a hash table. So we prefer to keep
things as they are with the host-gperf dependency.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot