(no subject)

[amadio]

Hi Arnaldo,

I adapted my patches to build on Gentoo to apply on perf-tools-next.
This moves libtraceevent and libtracefs to rely on pkg-config, but is
still able to build against non-standard installations with PKG_CONFIG_PATH.
I needed this patch because libtracefs changes the include directory when
using the meson build system. I filed https://github.com/rostedt/libtracefs/issues/3
in the hope that this inconsistency will be addressed in the future.

Best regards,
-Guilherme

[PATCH 1/2] perf build: Use pkg-config for feature check for libtrace{event,fs}

[amadio]

From: Guilherme Amadio <amadio@gentoo.org>

Needed to add required include directories for the feature detection
to succeed. The header tracefs.h is installed either into the include
directory /usr/include/tracefs/tracefs.h when using the Makefile, or
into /usr/include/libtracefs/tracefs.h when using meson to build
libtracefs. The header tracefs.h uses #include <event-parse.h> from
libtraceevent, so pkg-config needs to pick the correct include directory
for libtracefs and add the one for libtraceevent to succeed.

Note that in baa2ca59ec1e31ccbe3f24ff0368152b36f68720 the variable
LIBTRACEEVENT_DIR was introduced, and now the method to compile against
non-standard locations requires PKG_CONFIG_PATH to be set instead, which
works for both libtraceevent and libtracefs.

Signed-off-by: Guilherme Amadio <amadio@gentoo.org>
Cc: Ian Rogers <irogers@google.com>
---
 tools/build/feature/test-libtracefs.c |  2 +-
 tools/perf/Makefile.config            | 27 +++++++++++++--------------
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/tools/build/feature/test-libtracefs.c b/tools/build/feature/test-libtracefs.c
index 8eff16c0c10b..29a757a7d848 100644
--- a/tools/build/feature/test-libtracefs.c
+++ b/tools/build/feature/test-libtracefs.c
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
-#include <tracefs/tracefs.h>
+#include <tracefs.h>
 
 int main(void)
 {
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 7f1e016a9253..54a6c6bf23c7 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -183,14 +183,12 @@ FEATURE_CHECK_CFLAGS-libzstd := $(LIBZSTD_CFLAGS)
 FEATURE_CHECK_LDFLAGS-libzstd := $(LIBZSTD_LDFLAGS)
 
 # for linking with debug library, run like:
-# make DEBUG=1 LIBTRACEEVENT_DIR=/opt/libtraceevent/
-TRACEEVENTLIBS := -ltraceevent
-ifdef LIBTRACEEVENT_DIR
-  LIBTRACEEVENT_CFLAGS  := -I$(LIBTRACEEVENT_DIR)/include
-  LIBTRACEEVENT_LDFLAGS := -L$(LIBTRACEEVENT_DIR)/lib
-endif
-FEATURE_CHECK_CFLAGS-libtraceevent := $(LIBTRACEEVENT_CFLAGS)
-FEATURE_CHECK_LDFLAGS-libtraceevent := $(LIBTRACEEVENT_LDFLAGS) $(TRACEEVENTLIBS)
+# make DEBUG=1 PKG_CONFIG_PATH=/opt/libtraceevent/(lib|lib64)/pkgconfig
+FEATURE_CHECK_CFLAGS-libtraceevent := $(shell $(PKG_CONFIG) --cflags libtraceevent)
+FEATURE_CHECK_LDFLAGS-libtraceevent := $(shell $(PKG_CONFIG) --libs libtraceevent)
+
+FEATURE_CHECK_CFLAGS-libtracefs := $(shell $(PKG_CONFIG) --cflags libtracefs)
+FEATURE_CHECK_LDFLAGS-libtracefs := $(shell $(PKG_CONFIG) --libs libtracefs)
 
 FEATURE_CHECK_CFLAGS-bpf = -I. -I$(srctree)/tools/include -I$(srctree)/tools/arch/$(SRCARCH)/include/uapi -I$(srctree)/tools/include/uapi
 # include ARCH specific config
@@ -1178,10 +1176,10 @@ endif
 ifneq ($(NO_LIBTRACEEVENT),1)
   $(call feature_check,libtraceevent)
   ifeq ($(feature-libtraceevent), 1)
-    CFLAGS += -DHAVE_LIBTRACEEVENT $(LIBTRACEEVENT_CFLAGS)
-    LDFLAGS += $(LIBTRACEEVENT_LDFLAGS)
-    EXTLIBS += ${TRACEEVENTLIBS}
-    LIBTRACEEVENT_VERSION := $(shell PKG_CONFIG_PATH=$(LIBTRACEEVENT_DIR) $(PKG_CONFIG) --modversion libtraceevent)
+    CFLAGS += -DHAVE_LIBTRACEEVENT
+    LDFLAGS += $(shell $(PKG_CONFIG) --libs-only-L libtraceevent)
+    EXTLIBS += $(shell $(PKG_CONFIG) --libs-only-l libtraceevent)
+    LIBTRACEEVENT_VERSION := $(shell $(PKG_CONFIG) --modversion libtraceevent)
     LIBTRACEEVENT_VERSION_1 := $(word 1, $(subst ., ,$(LIBTRACEEVENT_VERSION)))
     LIBTRACEEVENT_VERSION_2 := $(word 2, $(subst ., ,$(LIBTRACEEVENT_VERSION)))
     LIBTRACEEVENT_VERSION_3 := $(word 3, $(subst ., ,$(LIBTRACEEVENT_VERSION)))
@@ -1194,7 +1192,9 @@ ifneq ($(NO_LIBTRACEEVENT),1)
 
   $(call feature_check,libtracefs)
   ifeq ($(feature-libtracefs), 1)
-    EXTLIBS += -ltracefs
+    CFLAGS +=  $(shell $(PKG_CONFIG) --cflags libtracefs)
+    LDFLAGS += $(shell $(PKG_CONFIG) --libs-only-L libtracefs)
+    EXTLIBS += $(shell $(PKG_CONFIG) --libs-only-l libtracefs)
     LIBTRACEFS_VERSION := $(shell $(PKG_CONFIG) --modversion libtracefs)
     LIBTRACEFS_VERSION_1 := $(word 1, $(subst ., ,$(LIBTRACEFS_VERSION)))
     LIBTRACEFS_VERSION_2 := $(word 2, $(subst ., ,$(LIBTRACEFS_VERSION)))
@@ -1315,7 +1315,6 @@ ifeq ($(VF),1)
   $(call print_var,LIBUNWIND_DIR)
   $(call print_var,LIBDW_DIR)
   $(call print_var,JDIR)
-  $(call print_var,LIBTRACEEVENT_DIR)
 
   ifeq ($(dwarf-post-unwind),1)
     $(call feature_print_text,"DWARF post unwind library", $(dwarf-post-unwind-text)) $(info $(MSG))
-- 
2.45.1

[PATCH 2/2] perf build: Ensure libtraceevent and libtracefs versions have 3 components

[amadio]

From: Guilherme Amadio <amadio@gentoo.org>

When either of these have a shorter version, like 1.8, the expression
that computes the version has a syntax error that can be seen in the
output of make:

expr: syntax error: missing argument after +

Link: https://bugs.gentoo.org/917559
Reported-by: Peter Volkov <peter.volkov@gmail.com>
Signed-off-by: Guilherme Amadio <amadio@gentoo.org>
Cc: Ian Rogers <irogers@google.com>
---
 tools/perf/Makefile.config | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 54a6c6bf23c7..81f73f68d256 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -1179,7 +1179,7 @@ ifneq ($(NO_LIBTRACEEVENT),1)
     CFLAGS += -DHAVE_LIBTRACEEVENT
     LDFLAGS += $(shell $(PKG_CONFIG) --libs-only-L libtraceevent)
     EXTLIBS += $(shell $(PKG_CONFIG) --libs-only-l libtraceevent)
-    LIBTRACEEVENT_VERSION := $(shell $(PKG_CONFIG) --modversion libtraceevent)
+    LIBTRACEEVENT_VERSION := $(shell $(PKG_CONFIG) --modversion libtraceevent).0.0
     LIBTRACEEVENT_VERSION_1 := $(word 1, $(subst ., ,$(LIBTRACEEVENT_VERSION)))
     LIBTRACEEVENT_VERSION_2 := $(word 2, $(subst ., ,$(LIBTRACEEVENT_VERSION)))
     LIBTRACEEVENT_VERSION_3 := $(word 3, $(subst ., ,$(LIBTRACEEVENT_VERSION)))
@@ -1195,7 +1195,7 @@ ifneq ($(NO_LIBTRACEEVENT),1)
     CFLAGS +=  $(shell $(PKG_CONFIG) --cflags libtracefs)
     LDFLAGS += $(shell $(PKG_CONFIG) --libs-only-L libtracefs)
     EXTLIBS += $(shell $(PKG_CONFIG) --libs-only-l libtracefs)
-    LIBTRACEFS_VERSION := $(shell $(PKG_CONFIG) --modversion libtracefs)
+    LIBTRACEFS_VERSION := $(shell $(PKG_CONFIG) --modversion libtracefs).0.0
     LIBTRACEFS_VERSION_1 := $(word 1, $(subst ., ,$(LIBTRACEFS_VERSION)))
     LIBTRACEFS_VERSION_2 := $(word 2, $(subst ., ,$(LIBTRACEFS_VERSION)))
     LIBTRACEFS_VERSION_3 := $(word 3, $(subst ., ,$(LIBTRACEFS_VERSION)))
-- 
2.45.1

Re: [PATCH 1/2] perf build: Use pkg-config for feature check for libtrace{event,fs}

[Leo Yan]

On 6/6/24 16:33, amadio@gentoo.org wrote:
> From: Guilherme Amadio <amadio@gentoo.org>
>
> Needed to add required include directories for the feature detection
> to succeed. The header tracefs.h is installed either into the include
> directory /usr/include/tracefs/tracefs.h when using the Makefile, or
> into /usr/include/libtracefs/tracefs.h when using meson to build
> libtracefs. The header tracefs.h uses #include <event-parse.h> from
> libtraceevent, so pkg-config needs to pick the correct include directory
> for libtracefs and add the one for libtraceevent to succeed.
>
> Note that in baa2ca59ec1e31ccbe3f24ff0368152b36f68720 the variable
> LIBTRACEEVENT_DIR was introduced, and now the method to compile against
> non-standard locations requires PKG_CONFIG_PATH to be set instead, which
> works for both libtraceevent and libtracefs.

Should we advise users to use PKG_CONFIG_LIBDIR or PKG_CONFIG_PATH?

When I wrote the patch set [1], at the beginning I also used the env
PKG_CONFIG_PATH, but later I think PKG_CONFIG_LIBDIR is better for users
for only lib path setting.

[1]
https://lore.kernel.org/linux-perf-users/20240604093223.1934236-3-leo.yan@arm.com/T/#mec2e44d6091dc1cf5f827b6542592b2938f1b697

> Signed-off-by: Guilherme Amadio <amadio@gentoo.org>
> Cc: Ian Rogers <irogers@google.com>
> ---
>   tools/build/feature/test-libtracefs.c |  2 +-
>   tools/perf/Makefile.config            | 27 +++++++++++++--------------
>   2 files changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/tools/build/feature/test-libtracefs.c b/tools/build/feature/test-libtracefs.c
> index 8eff16c0c10b..29a757a7d848 100644
> --- a/tools/build/feature/test-libtracefs.c
> +++ b/tools/build/feature/test-libtracefs.c
> @@ -1,5 +1,5 @@
>   // SPDX-License-Identifier: GPL-2.0
> -#include <tracefs/tracefs.h>
> +#include <tracefs.h>
>
>   int main(void)
>   {
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index 7f1e016a9253..54a6c6bf23c7 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -183,14 +183,12 @@ FEATURE_CHECK_CFLAGS-libzstd := $(LIBZSTD_CFLAGS)
>   FEATURE_CHECK_LDFLAGS-libzstd := $(LIBZSTD_LDFLAGS)
>
>   # for linking with debug library, run like:
> -# make DEBUG=1 LIBTRACEEVENT_DIR=/opt/libtraceevent/
> -TRACEEVENTLIBS := -ltraceevent
> -ifdef LIBTRACEEVENT_DIR
> -  LIBTRACEEVENT_CFLAGS  := -I$(LIBTRACEEVENT_DIR)/include
> -  LIBTRACEEVENT_LDFLAGS := -L$(LIBTRACEEVENT_DIR)/lib
> -endif
> -FEATURE_CHECK_CFLAGS-libtraceevent := $(LIBTRACEEVENT_CFLAGS)
> -FEATURE_CHECK_LDFLAGS-libtraceevent := $(LIBTRACEEVENT_LDFLAGS) $(TRACEEVENTLIBS)
> +# make DEBUG=1 PKG_CONFIG_PATH=/opt/libtraceevent/(lib|lib64)/pkgconfig
> +FEATURE_CHECK_CFLAGS-libtraceevent := $(shell $(PKG_CONFIG) --cflags libtraceevent)
> +FEATURE_CHECK_LDFLAGS-libtraceevent := $(shell $(PKG_CONFIG) --libs libtraceevent)
> +
> +FEATURE_CHECK_CFLAGS-libtracefs := $(shell $(PKG_CONFIG) --cflags libtracefs)
> +FEATURE_CHECK_LDFLAGS-libtracefs := $(shell $(PKG_CONFIG) --libs libtracefs)
>
>   FEATURE_CHECK_CFLAGS-bpf = -I. -I$(srctree)/tools/include -I$(srctree)/tools/arch/$(SRCARCH)/include/uapi -I$(srctree)/tools/include/uapi
>   # include ARCH specific config
> @@ -1178,10 +1176,10 @@ endif
>   ifneq ($(NO_LIBTRACEEVENT),1)
>     $(call feature_check,libtraceevent)
>     ifeq ($(feature-libtraceevent), 1)
> -    CFLAGS += -DHAVE_LIBTRACEEVENT $(LIBTRACEEVENT_CFLAGS)
> -    LDFLAGS += $(LIBTRACEEVENT_LDFLAGS)
> -    EXTLIBS += ${TRACEEVENTLIBS}
> -    LIBTRACEEVENT_VERSION := $(shell PKG_CONFIG_PATH=$(LIBTRACEEVENT_DIR) $(PKG_CONFIG) --modversion libtraceevent)
> +    CFLAGS += -DHAVE_LIBTRACEEVENT
> +    LDFLAGS += $(shell $(PKG_CONFIG) --libs-only-L libtraceevent)
> +    EXTLIBS += $(shell $(PKG_CONFIG) --libs-only-l libtraceevent)
> +    LIBTRACEEVENT_VERSION := $(shell $(PKG_CONFIG) --modversion libtraceevent)
>       LIBTRACEEVENT_VERSION_1 := $(word 1, $(subst ., ,$(LIBTRACEEVENT_VERSION)))
>       LIBTRACEEVENT_VERSION_2 := $(word 2, $(subst ., ,$(LIBTRACEEVENT_VERSION)))
>       LIBTRACEEVENT_VERSION_3 := $(word 3, $(subst ., ,$(LIBTRACEEVENT_VERSION)))
> @@ -1194,7 +1192,9 @@ ifneq ($(NO_LIBTRACEEVENT),1)
>
>     $(call feature_check,libtracefs)
>     ifeq ($(feature-libtracefs), 1)
> -    EXTLIBS += -ltracefs
> +    CFLAGS +=  $(shell $(PKG_CONFIG) --cflags libtracefs)
> +    LDFLAGS += $(shell $(PKG_CONFIG) --libs-only-L libtracefs)
> +    EXTLIBS += $(shell $(PKG_CONFIG) --libs-only-l libtracefs)
>       LIBTRACEFS_VERSION := $(shell $(PKG_CONFIG) --modversion libtracefs)
>       LIBTRACEFS_VERSION_1 := $(word 1, $(subst ., ,$(LIBTRACEFS_VERSION)))
>       LIBTRACEFS_VERSION_2 := $(word 2, $(subst ., ,$(LIBTRACEFS_VERSION)))
> @@ -1315,7 +1315,6 @@ ifeq ($(VF),1)
>     $(call print_var,LIBUNWIND_DIR)
>     $(call print_var,LIBDW_DIR)
>     $(call print_var,JDIR)
> -  $(call print_var,LIBTRACEEVENT_DIR)

Except above comment, the change looks good to me:

Reviewed-by: Leo Yan <leo.yan@arm.com>

>
>     ifeq ($(dwarf-post-unwind),1)
>       $(call feature_print_text,"DWARF post unwind library", $(dwarf-post-unwind-text)) $(info $(MSG))
> --
> 2.45.1
>
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH 2/2] perf build: Ensure libtraceevent and libtracefs versions have 3 components

[Leo Yan]

On 6/6/24 16:33, amadio@gentoo.org wrote:
> From: Guilherme Amadio <amadio@gentoo.org>
> 
> When either of these have a shorter version, like 1.8, the expression
> that computes the version has a syntax error that can be seen in the
> output of make:
> 
> expr: syntax error: missing argument after +
> 
> Link: https://bugs.gentoo.org/917559
> Reported-by: Peter Volkov <peter.volkov@gmail.com>
> Signed-off-by: Guilherme Amadio <amadio@gentoo.org>
> Cc: Ian Rogers <irogers@google.com>

Reviewed-by: Leo Yan <leo.yan@arm.com>

Re: [PATCH 1/2] perf build: Use pkg-config for feature check for libtrace{event,fs}

[Guilherme Amadio]

Dear Leo,

On Thu, Jun 06, 2024 at 05:28:47PM +0100, Leo Yan wrote:
> 
> 
> On 6/6/24 16:33, amadio@gentoo.org wrote:
> > From: Guilherme Amadio <amadio@gentoo.org>
> >
> > Needed to add required include directories for the feature detection
> > to succeed. The header tracefs.h is installed either into the include
> > directory /usr/include/tracefs/tracefs.h when using the Makefile, or
> > into /usr/include/libtracefs/tracefs.h when using meson to build
> > libtracefs. The header tracefs.h uses #include <event-parse.h> from
> > libtraceevent, so pkg-config needs to pick the correct include directory
> > for libtracefs and add the one for libtraceevent to succeed.
> >
> > Note that in baa2ca59ec1e31ccbe3f24ff0368152b36f68720 the variable
> > LIBTRACEEVENT_DIR was introduced, and now the method to compile against
> > non-standard locations requires PKG_CONFIG_PATH to be set instead, which
> > works for both libtraceevent and libtracefs.
> 
> Should we advise users to use PKG_CONFIG_LIBDIR or PKG_CONFIG_PATH?
> 
> When I wrote the patch set [1], at the beginning I also used the env
> PKG_CONFIG_PATH, but later I think PKG_CONFIG_LIBDIR is better for users
> for only lib path setting.
> 
> [1]
> https://lore.kernel.org/linux-perf-users/20240604093223.1934236-3-leo.yan@arm.com/T/#mec2e44d6091dc1cf5f827b6542592b2938f1b697

Thank you for the review. The documentation says:

- PKG_CONFIG_PATH:        List of secondary directories where `.pc' files are looked up.
- PKG_CONFIG_LIBDIR:      List of primary directories where `.pc' files are looked up.
- PKG_CONFIG_SYSROOT_DIR: `sysroot' directory, will be prepended to every path defined in PKG_CONFIG_PATH.  Useful for cross compilation.

So I think what's best depends on what the user is doing. In my case, I
was not cross-compiling, just using a non-standard location from where
to pick up the dependencies in /opt. For cross-compilation, using
PKG_CONFIG_SYSROOT_DIR might make sense instead of either of the other
two options. I will update the comment accordingly.

The other question I have is about other dependencies that use a similar
style, like capstone, should we also update them in the same way to rely
on pkg-config?

Best regards,
-Guilherme

> 
> > Signed-off-by: Guilherme Amadio <amadio@gentoo.org>
> > Cc: Ian Rogers <irogers@google.com>
> > ---
> >   tools/build/feature/test-libtracefs.c |  2 +-
> >   tools/perf/Makefile.config            | 27 +++++++++++++--------------
> >   2 files changed, 14 insertions(+), 15 deletions(-)
> >
> > diff --git a/tools/build/feature/test-libtracefs.c b/tools/build/feature/test-libtracefs.c
> > index 8eff16c0c10b..29a757a7d848 100644
> > --- a/tools/build/feature/test-libtracefs.c
> > +++ b/tools/build/feature/test-libtracefs.c
> > @@ -1,5 +1,5 @@
> >   // SPDX-License-Identifier: GPL-2.0
> > -#include <tracefs/tracefs.h>
> > +#include <tracefs.h>
> >
> >   int main(void)
> >   {
> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index 7f1e016a9253..54a6c6bf23c7 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -183,14 +183,12 @@ FEATURE_CHECK_CFLAGS-libzstd := $(LIBZSTD_CFLAGS)
> >   FEATURE_CHECK_LDFLAGS-libzstd := $(LIBZSTD_LDFLAGS)
> >
> >   # for linking with debug library, run like:
> > -# make DEBUG=1 LIBTRACEEVENT_DIR=/opt/libtraceevent/
> > -TRACEEVENTLIBS := -ltraceevent
> > -ifdef LIBTRACEEVENT_DIR
> > -  LIBTRACEEVENT_CFLAGS  := -I$(LIBTRACEEVENT_DIR)/include
> > -  LIBTRACEEVENT_LDFLAGS := -L$(LIBTRACEEVENT_DIR)/lib
> > -endif
> > -FEATURE_CHECK_CFLAGS-libtraceevent := $(LIBTRACEEVENT_CFLAGS)
> > -FEATURE_CHECK_LDFLAGS-libtraceevent := $(LIBTRACEEVENT_LDFLAGS) $(TRACEEVENTLIBS)
> > +# make DEBUG=1 PKG_CONFIG_PATH=/opt/libtraceevent/(lib|lib64)/pkgconfig
> > +FEATURE_CHECK_CFLAGS-libtraceevent := $(shell $(PKG_CONFIG) --cflags libtraceevent)
> > +FEATURE_CHECK_LDFLAGS-libtraceevent := $(shell $(PKG_CONFIG) --libs libtraceevent)
> > +
> > +FEATURE_CHECK_CFLAGS-libtracefs := $(shell $(PKG_CONFIG) --cflags libtracefs)
> > +FEATURE_CHECK_LDFLAGS-libtracefs := $(shell $(PKG_CONFIG) --libs libtracefs)
> >
> >   FEATURE_CHECK_CFLAGS-bpf = -I. -I$(srctree)/tools/include -I$(srctree)/tools/arch/$(SRCARCH)/include/uapi -I$(srctree)/tools/include/uapi
> >   # include ARCH specific config
> > @@ -1178,10 +1176,10 @@ endif
> >   ifneq ($(NO_LIBTRACEEVENT),1)
> >     $(call feature_check,libtraceevent)
> >     ifeq ($(feature-libtraceevent), 1)
> > -    CFLAGS += -DHAVE_LIBTRACEEVENT $(LIBTRACEEVENT_CFLAGS)
> > -    LDFLAGS += $(LIBTRACEEVENT_LDFLAGS)
> > -    EXTLIBS += ${TRACEEVENTLIBS}
> > -    LIBTRACEEVENT_VERSION := $(shell PKG_CONFIG_PATH=$(LIBTRACEEVENT_DIR) $(PKG_CONFIG) --modversion libtraceevent)
> > +    CFLAGS += -DHAVE_LIBTRACEEVENT
> > +    LDFLAGS += $(shell $(PKG_CONFIG) --libs-only-L libtraceevent)
> > +    EXTLIBS += $(shell $(PKG_CONFIG) --libs-only-l libtraceevent)
> > +    LIBTRACEEVENT_VERSION := $(shell $(PKG_CONFIG) --modversion libtraceevent)
> >       LIBTRACEEVENT_VERSION_1 := $(word 1, $(subst ., ,$(LIBTRACEEVENT_VERSION)))
> >       LIBTRACEEVENT_VERSION_2 := $(word 2, $(subst ., ,$(LIBTRACEEVENT_VERSION)))
> >       LIBTRACEEVENT_VERSION_3 := $(word 3, $(subst ., ,$(LIBTRACEEVENT_VERSION)))
> > @@ -1194,7 +1192,9 @@ ifneq ($(NO_LIBTRACEEVENT),1)
> >
> >     $(call feature_check,libtracefs)
> >     ifeq ($(feature-libtracefs), 1)
> > -    EXTLIBS += -ltracefs
> > +    CFLAGS +=  $(shell $(PKG_CONFIG) --cflags libtracefs)
> > +    LDFLAGS += $(shell $(PKG_CONFIG) --libs-only-L libtracefs)
> > +    EXTLIBS += $(shell $(PKG_CONFIG) --libs-only-l libtracefs)
> >       LIBTRACEFS_VERSION := $(shell $(PKG_CONFIG) --modversion libtracefs)
> >       LIBTRACEFS_VERSION_1 := $(word 1, $(subst ., ,$(LIBTRACEFS_VERSION)))
> >       LIBTRACEFS_VERSION_2 := $(word 2, $(subst ., ,$(LIBTRACEFS_VERSION)))
> > @@ -1315,7 +1315,6 @@ ifeq ($(VF),1)
> >     $(call print_var,LIBUNWIND_DIR)
> >     $(call print_var,LIBDW_DIR)
> >     $(call print_var,JDIR)
> > -  $(call print_var,LIBTRACEEVENT_DIR)
> 
> Except above comment, the change looks good to me:
> 
> Reviewed-by: Leo Yan <leo.yan@arm.com>
> 
> >
> >     ifeq ($(dwarf-post-unwind),1)
> >       $(call feature_print_text,"DWARF post unwind library", $(dwarf-post-unwind-text)) $(info $(MSG))
> > --
> > 2.45.1
> >
> >
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH 1/2] perf build: Use pkg-config for feature check for libtrace{event,fs}

[Leo Yan]

Hi Guilherme,

On 6/7/24 13:04, Guilherme Amadio wrote:

[...]

>>> Note that in baa2ca59ec1e31ccbe3f24ff0368152b36f68720 the variable
>>> LIBTRACEEVENT_DIR was introduced, and now the method to compile against
>>> non-standard locations requires PKG_CONFIG_PATH to be set instead, which
>>> works for both libtraceevent and libtracefs.
>>
>> Should we advise users to use PKG_CONFIG_LIBDIR or PKG_CONFIG_PATH?
>>
>> When I wrote the patch set [1], at the beginning I also used the env
>> PKG_CONFIG_PATH, but later I think PKG_CONFIG_LIBDIR is better for users
>> for only lib path setting.
> 
> Thank you for the review. The documentation says:
> 
> - PKG_CONFIG_PATH:        List of secondary directories where `.pc' files are looked up.
> - PKG_CONFIG_LIBDIR:      List of primary directories where `.pc' files are looked up.
> - PKG_CONFIG_SYSROOT_DIR: `sysroot' directory, will be prepended to every path defined in PKG_CONFIG_PATH.  Useful for cross compilation.
> 
> So I think what's best depends on what the user is doing. In my case, I
> was not cross-compiling, just using a non-standard location from where
> to pick up the dependencies in /opt. For cross-compilation, using
> PKG_CONFIG_SYSROOT_DIR might make sense instead of either of the other
> two options. I will update the comment accordingly.

Thanks for the info. Just make clear for two things:

Based on my tested result, we cannot only set the PKG_CONFIG_SYSROOT_DIR 
flag for cross compilation. This env variable adds prefix for paths 
defined in PKG_CONFIG_PATH or PKG_CONFIG_LIBDIR.

Your patch shows a good practice - we do not need to set any variables 
for PKG_CONFIG_PATH / PKG_CONFIG_LIBDIR / PKG_CONFIG_SYSROOT_DIR in 
Linux Perf Makefile. These variables should be left to users or the 
build system (e.g. Yocto, Buildroot, etc) to pass.

Below is an example for using the make command:

   PKG_CONFIG_LIBDIR=/usr/lib/pkgconfigs \
   PKG_CONFIG_SYSROOT_DIR=/buildfarm/sysroot \
   make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- \
   -C tools/perf VF=1


> The other question I have is about other dependencies that use a similar
> style, like capstone, should we also update them in the same way to rely
> on pkg-config?

I think your proposed approach is doable, as it is more neat than 
current code.

But I am not familiar with capstone, other developers may confirm as 
well. At my side, I did a test with below command for building capstone:

   cmake -DCMAKE_INSTALL_PREFIX=/path/for/install ...

The .pc file can dynamically generated to reflect the package's 
installation. So it has a prerequisite - for self-built package, we also 
need to install libs (e.g. make install) so that the path included in 
.pc can match the libs installation path.

Thanks,
Leo

Re: [PATCH 1/2] perf build: Use pkg-config for feature check for libtrace{event,fs}

[Namhyung Kim]

On Thu, 06 Jun 2024 17:33:01 +0200, amadio@gentoo.org wrote:

> Needed to add required include directories for the feature detection
> to succeed. The header tracefs.h is installed either into the include
> directory /usr/include/tracefs/tracefs.h when using the Makefile, or
> into /usr/include/libtracefs/tracefs.h when using meson to build
> libtracefs. The header tracefs.h uses #include <event-parse.h> from
> libtraceevent, so pkg-config needs to pick the correct include directory
> for libtracefs and add the one for libtraceevent to succeed.
> 
> [...]

Applied both to perf-tools-next, thanks!

Best regards,
Namhyung

Re: [PATCH 1/2] perf build: Use pkg-config for feature check for libtrace{event,fs}

[Thorsten Leemhuis]

On 06.06.24 17:33, amadio@gentoo.org wrote:
> From: Guilherme Amadio <amadio@gentoo.org>
> 
> Needed to add required include directories for the feature detection
> to succeed. The header tracefs.h is installed either into the include
> directory /usr/include/tracefs/tracefs.h when using the Makefile, or
> into /usr/include/libtracefs/tracefs.h when using meson to build
> libtracefs. The header tracefs.h uses #include <event-parse.h> from
> libtraceevent, so pkg-config needs to pick the correct include directory
> for libtracefs and add the one for libtraceevent to succeed.
> 
> Note that in baa2ca59ec1e31ccbe3f24ff0368152b36f68720 the variable
> LIBTRACEEVENT_DIR was introduced, and now the method to compile against
> non-standard locations requires PKG_CONFIG_PATH to be set instead, which
> works for both libtraceevent and libtracefs.
> 
> Signed-off-by: Guilherme Amadio <amadio@gentoo.org>
> Cc: Ian Rogers <irogers@google.com>

Since this patch landed in Linux next as 0f0e1f44569061 ("perf build:
Use pkg-config for feature check for libtrace{event,fs}") my vanilla 
-next build using the RPM spec file from Fedora fails. I'm not sure if
it causes my problems, but it looks likely.

A log of a failed build can be found here:
https://download.copr.fedorainfracloud.org/results/@kernel-vanilla/next/fedora-rawhide-x86_64/07670271-next-next-all/builder-live.log.gz

The gist:
"""

Auto-detecting system features:
...                           libtraceevent: [ on  ]
...                              libtracefs: [ OFF ]

libtracefs is missing. Please install libtracefs-dev/libtracefs-devel
Makefile.config:46: *** Please, check the errors above..  Stop.
error: Bad exit status from /var/tmp/rpm-tmp.yNun6A (%build)
"""

But libtracefs-devel is installed and pkgconfig works (output from
another machine):

$ pkgconf --cflags libtracefs
-I/usr/include/tracefs -I/usr/include/traceevent

From a *quick* looks it seems those flags are not picked up when
building tools/verification/rv/feature/test-libtracefs.bin 

"""
+ pkg-config --cflags libtraceevent
gcc  -MD -Wall -Werror -o /home/thl/usr/tmp/linux-next/tools/verification/rv/feature/test-libtracefs.bin test-libtracefs.c -ggdb   > /home/thl/usr/tmp/linux-next/tools/verification/rv/feature/test-libtracefs.make.output 2>&1 -I/usr/include/traceevent  -ltracefs
+ gcc -MD -Wall -Werror -o /home/thl/usr/tmp/linux-next/tools/verification/rv/feature/test-libtracefs.bin test-libtracefs.c -ggdb -I/usr/include/traceevent -ltracefs
make: *** [Makefile:216: /home/thl/usr/tmp/linux-next/tools/verification/rv/feature/test-libtracefs.bin] Error 1
make: Leaving directory '/home/thl/usr/tmp/linux-next/tools/build/feature'
"""

And

"""
$ cat /home/thl/usr/tmp/linux-next/tools/verification/rv/feature/test-libtracefs.make.output 
test-libtracefs.c:2:10: fatal error: tracefs.h: No such file or directory
    2 | #include <tracefs.h>
      |          ^~~~~~~~~~~
compilation terminated.
"""

Then I ran out of time looking where it might missing and decided to write
this mail instead.

Ciao, Thorsten


> ---
>  tools/build/feature/test-libtracefs.c |  2 +-
>  tools/perf/Makefile.config            | 27 +++++++++++++--------------
>  2 files changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/build/feature/test-libtracefs.c b/tools/build/feature/test-libtracefs.c
> index 8eff16c0c10b..29a757a7d848 100644
> --- a/tools/build/feature/test-libtracefs.c
> +++ b/tools/build/feature/test-libtracefs.c
> @@ -1,5 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0
> -#include <tracefs/tracefs.h>
> +#include <tracefs.h>
>
> [...]

Re: [PATCH 1/2] perf build: Use pkg-config for feature check for libtrace{event,fs}

[Guilherme Amadio]

On Fri, Jun 28, 2024 at 09:13:43AM +0200, Thorsten Leemhuis wrote:
> On 06.06.24 17:33, amadio@gentoo.org wrote:
> > From: Guilherme Amadio <amadio@gentoo.org>
> > 
> > Needed to add required include directories for the feature detection
> > to succeed. The header tracefs.h is installed either into the include
> > directory /usr/include/tracefs/tracefs.h when using the Makefile, or
> > into /usr/include/libtracefs/tracefs.h when using meson to build
> > libtracefs. The header tracefs.h uses #include <event-parse.h> from
> > libtraceevent, so pkg-config needs to pick the correct include directory
> > for libtracefs and add the one for libtraceevent to succeed.
> > 
> > Note that in baa2ca59ec1e31ccbe3f24ff0368152b36f68720 the variable
> > LIBTRACEEVENT_DIR was introduced, and now the method to compile against
> > non-standard locations requires PKG_CONFIG_PATH to be set instead, which
> > works for both libtraceevent and libtracefs.
> > 
> > Signed-off-by: Guilherme Amadio <amadio@gentoo.org>
> > Cc: Ian Rogers <irogers@google.com>
> 
> Since this patch landed in Linux next as 0f0e1f44569061 ("perf build:
> Use pkg-config for feature check for libtrace{event,fs}") my vanilla 
> -next build using the RPM spec file from Fedora fails. I'm not sure if
> it causes my problems, but it looks likely.

Thank you for reporting. I think we need to use similar checks in the
tools/verification/rv/Makefile.config, as it seems the problem comes
from there:

+ pushd tools/verification/rv/
kernel.spec:3000: build rv
... (compile command lines removed)

Auto-detecting system features:
...                           libtraceevent: [ on  ]
...                              libtracefs: [ OFF ]

libtracefs is missing. Please install libtracefs-dev/libtracefs-devel
Makefile.config:46: *** Please, check the errors above..  Stop.
error: Bad exit status from /var/tmp/rpm-tmp.yNun6A (%build)

I will submit a patch to fix it.

Best regards,
-Guilherme

> 
> A log of a failed build can be found here:
> https://download.copr.fedorainfracloud.org/results/@kernel-vanilla/next/fedora-rawhide-x86_64/07670271-next-next-all/builder-live.log.gz
> 
> The gist:
> """
> 
> Auto-detecting system features:
> ...                           libtraceevent: [ on  ]
> ...                              libtracefs: [ OFF ]
> 
> libtracefs is missing. Please install libtracefs-dev/libtracefs-devel
> Makefile.config:46: *** Please, check the errors above..  Stop.
> error: Bad exit status from /var/tmp/rpm-tmp.yNun6A (%build)
> """
> 
> But libtracefs-devel is installed and pkgconfig works (output from
> another machine):
> 
> $ pkgconf --cflags libtracefs
> -I/usr/include/tracefs -I/usr/include/traceevent
> 
> From a *quick* looks it seems those flags are not picked up when
> building tools/verification/rv/feature/test-libtracefs.bin 
> 
> """
> + pkg-config --cflags libtraceevent
> gcc  -MD -Wall -Werror -o /home/thl/usr/tmp/linux-next/tools/verification/rv/feature/test-libtracefs.bin test-libtracefs.c -ggdb   > /home/thl/usr/tmp/linux-next/tools/verification/rv/feature/test-libtracefs.make.output 2>&1 -I/usr/include/traceevent  -ltracefs
> + gcc -MD -Wall -Werror -o /home/thl/usr/tmp/linux-next/tools/verification/rv/feature/test-libtracefs.bin test-libtracefs.c -ggdb -I/usr/include/traceevent -ltracefs
> make: *** [Makefile:216: /home/thl/usr/tmp/linux-next/tools/verification/rv/feature/test-libtracefs.bin] Error 1
> make: Leaving directory '/home/thl/usr/tmp/linux-next/tools/build/feature'
> """
> 
> And
> 
> """
> $ cat /home/thl/usr/tmp/linux-next/tools/verification/rv/feature/test-libtracefs.make.output 
> test-libtracefs.c:2:10: fatal error: tracefs.h: No such file or directory
>     2 | #include <tracefs.h>
>       |          ^~~~~~~~~~~
> compilation terminated.
> """
> 
> Then I ran out of time looking where it might missing and decided to write
> this mail instead.
> 
> Ciao, Thorsten
> 
> 
> > ---
> >  tools/build/feature/test-libtracefs.c |  2 +-
> >  tools/perf/Makefile.config            | 27 +++++++++++++--------------
> >  2 files changed, 14 insertions(+), 15 deletions(-)
> > 
> > diff --git a/tools/build/feature/test-libtracefs.c b/tools/build/feature/test-libtracefs.c
> > index 8eff16c0c10b..29a757a7d848 100644
> > --- a/tools/build/feature/test-libtracefs.c
> > +++ b/tools/build/feature/test-libtracefs.c
> > @@ -1,5 +1,5 @@
> >  // SPDX-License-Identifier: GPL-2.0
> > -#include <tracefs/tracefs.h>
> > +#include <tracefs.h>
> >
> > [...]

Re: [PATCH 1/2] perf build: Use pkg-config for feature check for libtrace{event,fs}

[Arnaldo Carvalho de Melo]

On Fri, Jun 28, 2024 at 12:27:11PM +0200, Guilherme Amadio wrote:
> On Fri, Jun 28, 2024 at 09:13:43AM +0200, Thorsten Leemhuis wrote:
> > On 06.06.24 17:33, amadio@gentoo.org wrote:
> > > From: Guilherme Amadio <amadio@gentoo.org>
> > > 
> > > Needed to add required include directories for the feature detection
> > > to succeed. The header tracefs.h is installed either into the include
> > > directory /usr/include/tracefs/tracefs.h when using the Makefile, or
> > > into /usr/include/libtracefs/tracefs.h when using meson to build
> > > libtracefs. The header tracefs.h uses #include <event-parse.h> from
> > > libtraceevent, so pkg-config needs to pick the correct include directory
> > > for libtracefs and add the one for libtraceevent to succeed.
> > > 
> > > Note that in baa2ca59ec1e31ccbe3f24ff0368152b36f68720 the variable
> > > LIBTRACEEVENT_DIR was introduced, and now the method to compile against
> > > non-standard locations requires PKG_CONFIG_PATH to be set instead, which
> > > works for both libtraceevent and libtracefs.
> > > 
> > > Signed-off-by: Guilherme Amadio <amadio@gentoo.org>
> > > Cc: Ian Rogers <irogers@google.com>
> > 
> > Since this patch landed in Linux next as 0f0e1f44569061 ("perf build:
> > Use pkg-config for feature check for libtrace{event,fs}") my vanilla 
> > -next build using the RPM spec file from Fedora fails. I'm not sure if
> > it causes my problems, but it looks likely.
> 
> Thank you for reporting. I think we need to use similar checks in the
> tools/verification/rv/Makefile.config, as it seems the problem comes
> from there:
> 
> + pushd tools/verification/rv/
> kernel.spec:3000: build rv
> ... (compile command lines removed)
> 
> Auto-detecting system features:
> ...                           libtraceevent: [ on  ]
> ...                              libtracefs: [ OFF ]
> 
> libtracefs is missing. Please install libtracefs-dev/libtracefs-devel
> Makefile.config:46: *** Please, check the errors above..  Stop.
> error: Bad exit status from /var/tmp/rpm-tmp.yNun6A (%build)
> 
> I will submit a patch to fix it.

I noticed something related, but in my case the build proceeded, giving
these warings:

Package libtracefs was not found in the pkg-config search path.
Perhaps you should add the directory containing `libtracefs.pc'
to the PKG_CONFIG_PATH environment variable
Package 'libtracefs', required by 'virtual:world', not found
Package libtracefs was not found in the pkg-config search path.
Perhaps you should add the directory containing `libtracefs.pc'
to the PKG_CONFIG_PATH environment variable
Package 'libtracefs', required by 'virtual:world', not found

Auto-detecting system features:
...                                   dwarf: [ on  ]
...                      dwarf_getlocations: [ on  ]
...                                   glibc: [ on  ]
...                                  libbfd: [ on  ]
...                          libbfd-buildid: [ on  ]
...                                  libcap: [ on  ]
...                                  libelf: [ on  ]
...                                 libnuma: [ on  ]
...                  numa_num_possible_cpus: [ on  ]
...                                 libperl: [ on  ]
...                               libpython: [ on  ]
...                               libcrypto: [ on  ]
...                               libunwind: [ on  ]
...                      libdw-dwarf-unwind: [ on  ]
...                             libcapstone: [ on  ]
...                                    zlib: [ on  ]
...                                    lzma: [ on  ]
...                               get_cpuid: [ on  ]
...                                     bpf: [ on  ]
...                                  libaio: [ on  ]
...                                 libzstd: [ on  ]

⬢[acme@toolbox perf-tools-next]$ rpm -qa | grep tracefs
⬢[acme@toolbox perf-tools-next]$ rpm -qa | grep traceevent
libtraceevent-1.7.2-3.fc39.x86_64
libtraceevent-devel-1.7.2-3.fc39.x86_64
⬢[acme@toolbox perf-tools-next]$

- Arnaldo

 
> Best regards,
> -Guilherme
> 
> > 
> > A log of a failed build can be found here:
> > https://download.copr.fedorainfracloud.org/results/@kernel-vanilla/next/fedora-rawhide-x86_64/07670271-next-next-all/builder-live.log.gz
> > 
> > The gist:
> > """
> > 
> > Auto-detecting system features:
> > ...                           libtraceevent: [ on  ]
> > ...                              libtracefs: [ OFF ]
> > 
> > libtracefs is missing. Please install libtracefs-dev/libtracefs-devel
> > Makefile.config:46: *** Please, check the errors above..  Stop.
> > error: Bad exit status from /var/tmp/rpm-tmp.yNun6A (%build)
> > """
> > 
> > But libtracefs-devel is installed and pkgconfig works (output from
> > another machine):
> > 
> > $ pkgconf --cflags libtracefs
> > -I/usr/include/tracefs -I/usr/include/traceevent
> > 
> > From a *quick* looks it seems those flags are not picked up when
> > building tools/verification/rv/feature/test-libtracefs.bin 
> > 
> > """
> > + pkg-config --cflags libtraceevent
> > gcc  -MD -Wall -Werror -o /home/thl/usr/tmp/linux-next/tools/verification/rv/feature/test-libtracefs.bin test-libtracefs.c -ggdb   > /home/thl/usr/tmp/linux-next/tools/verification/rv/feature/test-libtracefs.make.output 2>&1 -I/usr/include/traceevent  -ltracefs
> > + gcc -MD -Wall -Werror -o /home/thl/usr/tmp/linux-next/tools/verification/rv/feature/test-libtracefs.bin test-libtracefs.c -ggdb -I/usr/include/traceevent -ltracefs
> > make: *** [Makefile:216: /home/thl/usr/tmp/linux-next/tools/verification/rv/feature/test-libtracefs.bin] Error 1
> > make: Leaving directory '/home/thl/usr/tmp/linux-next/tools/build/feature'
> > """
> > 
> > And
> > 
> > """
> > $ cat /home/thl/usr/tmp/linux-next/tools/verification/rv/feature/test-libtracefs.make.output 
> > test-libtracefs.c:2:10: fatal error: tracefs.h: No such file or directory
> >     2 | #include <tracefs.h>
> >       |          ^~~~~~~~~~~
> > compilation terminated.
> > """
> > 
> > Then I ran out of time looking where it might missing and decided to write
> > this mail instead.
> > 
> > Ciao, Thorsten
> > 
> > 
> > > ---
> > >  tools/build/feature/test-libtracefs.c |  2 +-
> > >  tools/perf/Makefile.config            | 27 +++++++++++++--------------
> > >  2 files changed, 14 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/tools/build/feature/test-libtracefs.c b/tools/build/feature/test-libtracefs.c
> > > index 8eff16c0c10b..29a757a7d848 100644
> > > --- a/tools/build/feature/test-libtracefs.c
> > > +++ b/tools/build/feature/test-libtracefs.c
> > > @@ -1,5 +1,5 @@
> > >  // SPDX-License-Identifier: GPL-2.0
> > > -#include <tracefs/tracefs.h>
> > > +#include <tracefs.h>
> > >
> > > [...]