[PATCH v3 0/5] perf build: libtraceevent, libtracefs feature check with pkg-config

Guilherme Amadio posted 5 patches 1 year, 5 months ago
There is a newer version of this series
[PATCH v3 0/5] perf build: libtraceevent, libtracefs feature check with pkg-config
Posted by Guilherme Amadio 1 year, 5 months ago
Hi Namhyung, Arnaldo,

Here is version 3 of the patchset. I see the change to send output to devnull
has already been applied, so I am submitting the remaining work only.

The difference with previous changes is that in v3 rather than add tests with
pkg-config to check if dependencies are actually installed, we just set the
flags and send any warning to devnull. The change that remains in this patchset
is the fix for the other tools, which were inadvertently broken when the include
for libtracefs changed from #include <tracefs/tracefs.h> to #include <tracefs.h>
since the flags for the feature check are not set in the other tools Makefiles,
it currently only works for perf. I recommend to either take at least patch 2/5
that moves setting the flags to tools/build/Makefile.feature or to revert the two
patches that have been applied. You can easily test the fix with the commands below:

make -B -C tools/verification/rv VF=1
make -B -C tools/tracing/latency VF=1
make -B -C tools/tracing/rtla VF=1

from the root of the repository. Only after the feature check flags are moved to
Makefile.feature that it also fixes the other tools. Apologies for the breakage
there.

Best regards,
-Guilherme
Re: [PATCH v3 0/5] perf build: libtraceevent, libtracefs feature check with pkg-config
Posted by Namhyung Kim 1 year, 4 months ago
On Fri, 12 Jul 2024 21:40:45 +0200, Guilherme Amadio wrote:

> Here is version 3 of the patchset. I see the change to send output to devnull
> has already been applied, so I am submitting the remaining work only.
> 
> The difference with previous changes is that in v3 rather than add tests with
> pkg-config to check if dependencies are actually installed, we just set the
> flags and send any warning to devnull. The change that remains in this patchset
> is the fix for the other tools, which were inadvertently broken when the include
> for libtracefs changed from #include <tracefs/tracefs.h> to #include <tracefs.h>
> since the flags for the feature check are not set in the other tools Makefiles,
> it currently only works for perf. I recommend to either take at least patch 2/5
> that moves setting the flags to tools/build/Makefile.feature or to revert the two
> patches that have been applied. You can easily test the fix with the commands below:
> 
> [...]

Sorry, it was a bit late to send the message but it's applied to perf-tools
and the mainline Linux now, thanks!

Best regards,
Namhyung
Re: [PATCH v3 0/5] perf build: libtraceevent, libtracefs feature check with pkg-config
Posted by Namhyung Kim 1 year, 5 months ago
Hello,

On Fri, Jul 12, 2024 at 12:45 PM Guilherme Amadio <amadio@gentoo.org> wrote:
>
> Hi Namhyung, Arnaldo,
>
> Here is version 3 of the patchset. I see the change to send output to devnull
> has already been applied, so I am submitting the remaining work only.
>
> The difference with previous changes is that in v3 rather than add tests with
> pkg-config to check if dependencies are actually installed, we just set the
> flags and send any warning to devnull. The change that remains in this patchset
> is the fix for the other tools, which were inadvertently broken when the include
> for libtracefs changed from #include <tracefs/tracefs.h> to #include <tracefs.h>
> since the flags for the feature check are not set in the other tools Makefiles,
> it currently only works for perf. I recommend to either take at least patch 2/5
> that moves setting the flags to tools/build/Makefile.feature or to revert the two
> patches that have been applied. You can easily test the fix with the commands below:
>
> make -B -C tools/verification/rv VF=1
> make -B -C tools/tracing/latency VF=1
> make -B -C tools/tracing/rtla VF=1
>
> from the root of the repository. Only after the feature check flags are moved to
> Makefile.feature that it also fixes the other tools. Apologies for the breakage
> there.

Steve, are you ok with having this patchset in the perf-tools tree?

Thanks,
Namhyung
Re: [PATCH v3 0/5] perf build: libtraceevent, libtracefs feature check with pkg-config
Posted by Steven Rostedt 1 year, 5 months ago
On Wed, 17 Jul 2024 09:27:58 -0700
Namhyung Kim <namhyung@kernel.org> wrote:

> Hello,
> 
> On Fri, Jul 12, 2024 at 12:45 PM Guilherme Amadio <amadio@gentoo.org> wrote:
> >
> > Hi Namhyung, Arnaldo,
> >
> > Here is version 3 of the patchset. I see the change to send output to devnull
> > has already been applied, so I am submitting the remaining work only.
> >
> > The difference with previous changes is that in v3 rather than add tests with
> > pkg-config to check if dependencies are actually installed, we just set the
> > flags and send any warning to devnull. The change that remains in this patchset
> > is the fix for the other tools, which were inadvertently broken when the include
> > for libtracefs changed from #include <tracefs/tracefs.h> to #include <tracefs.h>
> > since the flags for the feature check are not set in the other tools Makefiles,
> > it currently only works for perf. I recommend to either take at least patch 2/5
> > that moves setting the flags to tools/build/Makefile.feature or to revert the two
> > patches that have been applied. You can easily test the fix with the commands below:
> >
> > make -B -C tools/verification/rv VF=1
> > make -B -C tools/tracing/latency VF=1
> > make -B -C tools/tracing/rtla VF=1
> >
> > from the root of the repository. Only after the feature check flags are moved to
> > Makefile.feature that it also fixes the other tools. Apologies for the breakage
> > there.  
> 
> Steve, are you ok with having this patchset in the perf-tools tree?
> 

I don't know as this is the first I've seen it. If you could have this
resend with me Cc'd and/or Cc linux-trace-devel@vger.kernel.org then I
can review and possibly ack it.

-- Steve
[PATCH v3 0/5] perf build: libtraceevent, libtracefs feature check with pkg-config
Posted by Guilherme Amadio 1 year, 5 months ago
This patch series is a continuation of recent work to use pkg-config for feature
checks for libtraceevent and libtracefs.

The original intention of the first patches was to fix build issues with
libtracefs installed with its meson build system (used in Gentoo), which
installs headers in a different directory as the make build system. This has
been reported here: https://github.com/rostedt/libtracefs/issues/3

In order to fix the feature check, instead of using #include <tracefs/tracefs.h>
in tools/build/feature/test-libtracefs.c, we need to use #include <tracefs.h>
and add the appropriate -I flag for the include directory, which is what is now
done with pkg-config. An unintended consequence of this (reported by Thorsten)
was that other tools which also perform the same feature check for libtracefs
became broken, because the initial patch only set the include flag for perf in
tools/perf/Makefile.config.

This patch series fixes this issue by moving the feature check flags to the file
tools/build/Makefile.feature, which ensures the flags get set for all tools that
need them. I tried to write the new code in a way that would be easily extensible
to other dependencies that may be used via pkg-config as well.

There is no change in this new submission relative to the last (which has been
tested by Thorsten to fix the build issue in Fedora). The only change has been
to add in CC Steven Rostedt and the list linux-trace-devel@vger.kernel.org.

Best regards,
-Guilherme

Guilherme Amadio (5):
  perf build: Warn if libtracefs is not found
  tools: Make pkg-config dependency checks usable by other tools
  tools/verification: Use pkg-config in lib_setup of Makefile.config
  tools/rtla: Use pkg-config in lib_setup of Makefile.config
  tools/latency: Use pkg-config in lib_setup of Makefile.config

 tools/build/Makefile.feature          | 18 ++++++++++++++++++
 tools/perf/Makefile.config            | 13 +++++--------
 tools/tracing/latency/Makefile.config |  3 ++-
 tools/tracing/rtla/Makefile.config    |  3 ++-
 tools/verification/rv/Makefile.config |  3 ++-
 5 files changed, 29 insertions(+), 11 deletions(-)

-- 
2.39.3
Re: [PATCH v3 0/5] perf build: libtraceevent, libtracefs feature check with pkg-config
Posted by Leo Yan 1 year, 5 months ago
On 7/17/2024 6:47 PM, Guilherme Amadio wrote:
> 
> This patch series is a continuation of recent work to use pkg-config for feature
> checks for libtraceevent and libtracefs.

Tested this series for building x86_64 target and cross building aarch64 target:

Tested-by: Leo Yan <leo.yan@arm.com>
Re: [PATCH v3 0/5] perf build: libtraceevent, libtracefs feature check with pkg-config
Posted by Steven Rostedt 1 year, 5 months ago
On Wed, 17 Jul 2024 19:47:34 +0200
Guilherme Amadio <amadio@gentoo.org> wrote:

> There is no change in this new submission relative to the last (which has been
> tested by Thorsten to fix the build issue in Fedora). The only change has been
> to add in CC Steven Rostedt and the list linux-trace-devel@vger.kernel.org.

Thanks,

For the whole series:

Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>

Feel free to take it through the perf tree.

-- Steve

> 
> Best regards,
> -Guilherme
> 
> Guilherme Amadio (5):
>   perf build: Warn if libtracefs is not found
>   tools: Make pkg-config dependency checks usable by other tools
>   tools/verification: Use pkg-config in lib_setup of Makefile.config
>   tools/rtla: Use pkg-config in lib_setup of Makefile.config
>   tools/latency: Use pkg-config in lib_setup of Makefile.config
> 
>  tools/build/Makefile.feature          | 18 ++++++++++++++++++
>  tools/perf/Makefile.config            | 13 +++++--------
>  tools/tracing/latency/Makefile.config |  3 ++-
>  tools/tracing/rtla/Makefile.config    |  3 ++-
>  tools/verification/rv/Makefile.config |  3 ++-
>  5 files changed, 29 insertions(+), 11 deletions(-)
>
Re: [PATCH v3 0/5] perf build: libtraceevent, libtracefs feature check with pkg-config
Posted by Namhyung Kim 1 year, 5 months ago
On Wed, Jul 17, 2024 at 10:58 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 17 Jul 2024 19:47:34 +0200
> Guilherme Amadio <amadio@gentoo.org> wrote:
>
> > There is no change in this new submission relative to the last (which has been
> > tested by Thorsten to fix the build issue in Fedora). The only change has been
> > to add in CC Steven Rostedt and the list linux-trace-devel@vger.kernel.org.
>
> Thanks,
>
> For the whole series:
>
> Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
>
> Feel free to take it through the perf tree.

Thanks, I'll add it to perf-tools and send out to v6.11 later.

Namhyung
[PATCH v3 1/5] perf build: Warn if libtracefs is not found
Posted by Guilherme Amadio 1 year, 5 months ago
Signed-off-by: Guilherme Amadio <amadio@gentoo.org>
---
 tools/perf/Makefile.config | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index a4829b6532d8..44f010b9f562 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -1206,6 +1206,8 @@ ifneq ($(NO_LIBTRACEEVENT),1)
     LIBTRACEFS_VERSION_3 := $(word 3, $(subst ., ,$(LIBTRACEFS_VERSION)))
     LIBTRACEFS_VERSION_CPP := $(shell expr $(LIBTRACEFS_VERSION_1) \* 255 \* 255 + $(LIBTRACEFS_VERSION_2) \* 255 + $(LIBTRACEFS_VERSION_3))
     CFLAGS += -DLIBTRACEFS_VERSION=$(LIBTRACEFS_VERSION_CPP)
+  else
+    $(warning libtracefs is missing. Please install libtracefs-dev/libtracefs-devel)
   endif
 endif
 
-- 
2.39.3
[PATCH v3 2/5] tools: Make pkg-config dependency checks usable by other tools
Posted by Guilherme Amadio 1 year, 5 months ago
Other tools, in tools/verification and tools/tracing, make use of
libtraceevent and libtracefs as dependencies. This allows setting
up the feature check flags for them as well.

Signed-off-by: Guilherme Amadio <amadio@gentoo.org>
---
 tools/build/Makefile.feature | 18 ++++++++++++++++++
 tools/perf/Makefile.config   | 11 +++--------
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 1e2ab148d5db..e1900abd44f6 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -149,6 +149,24 @@ FEATURE_DISPLAY ?=              \
 #
 FEATURE_GROUP_MEMBERS-libbfd = libbfd-liberty libbfd-liberty-z
 
+#
+# Declare list of feature dependency packages that provide pkg-config files.
+#
+FEATURE_PKG_CONFIG ?=           \
+         libtraceevent          \
+         libtracefs
+
+feature_pkg_config = $(eval $(feature_pkg_config_code))
+define feature_pkg_config_code
+  FEATURE_CHECK_CFLAGS-$(1) := $(shell $(PKG_CONFIG) --cflags $(1) 2>/dev/null)
+  FEATURE_CHECK_LDFLAGS-$(1) := $(shell $(PKG_CONFIG) --libs $(1) 2>/dev/null)
+endef
+
+# Set FEATURE_CHECK_(C|LD)FLAGS-$(package) for packages using pkg-config.
+ifneq ($(PKG_CONFIG),)
+  $(foreach package,$(FEATURE_PKG_CONFIG),$(call feature_pkg_config,$(package)))
+endif
+
 # Set FEATURE_CHECK_(C|LD)FLAGS-all for all FEATURE_TESTS features.
 # If in the future we need per-feature checks/flags for features not
 # mentioned in this list we need to refactor this ;-).
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index 44f010b9f562..c896babf7a74 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -182,20 +182,15 @@ endif
 FEATURE_CHECK_CFLAGS-libzstd := $(LIBZSTD_CFLAGS)
 FEATURE_CHECK_LDFLAGS-libzstd := $(LIBZSTD_LDFLAGS)
 
+# for linking with debug library, run like:
+# make DEBUG=1 PKG_CONFIG_PATH=/opt/libtraceevent/(lib|lib64)/pkgconfig
+
 ifneq ($(NO_LIBTRACEEVENT),1)
   ifeq ($(call get-executable,$(PKG_CONFIG)),)
     $(error Error: $(PKG_CONFIG) needed by libtraceevent is missing on this system, please install it)
   endif
 endif
 
-# for linking with debug library, run like:
-# make DEBUG=1 PKG_CONFIG_PATH=/opt/libtraceevent/(lib|lib64)/pkgconfig
-FEATURE_CHECK_CFLAGS-libtraceevent := $(shell $(PKG_CONFIG) --cflags libtraceevent 2>/dev/null)
-FEATURE_CHECK_LDFLAGS-libtraceevent := $(shell $(PKG_CONFIG) --libs libtraceevent 2>/dev/null)
-
-FEATURE_CHECK_CFLAGS-libtracefs := $(shell $(PKG_CONFIG) --cflags libtracefs 2>/dev/null)
-FEATURE_CHECK_LDFLAGS-libtracefs := $(shell $(PKG_CONFIG) --libs libtracefs 2>/dev/null)
-
 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
 -include $(src-perf)/arch/$(SRCARCH)/Makefile
-- 
2.39.3
[PATCH v3 3/5] tools/verification: Use pkg-config in lib_setup of Makefile.config
Posted by Guilherme Amadio 1 year, 5 months ago
This allows to build against libtraceevent and libtracefs installed
in non-standard locations.

Signed-off-by: Guilherme Amadio <amadio@gentoo.org>
---
 tools/verification/rv/Makefile.config | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/verification/rv/Makefile.config b/tools/verification/rv/Makefile.config
index 6d4ba77847b6..066302230eb2 100644
--- a/tools/verification/rv/Makefile.config
+++ b/tools/verification/rv/Makefile.config
@@ -7,7 +7,8 @@ LIBTRACEFS_MIN_VERSION = 1.3
 
 define lib_setup
   $(eval LIB_INCLUDES += $(shell sh -c "$(PKG_CONFIG) --cflags lib$(1)"))
-  $(eval EXTLIBS += -l$(1))
+  $(eval LDFLAGS += $(shell sh -c "$(PKG_CONFIG) --libs-only-L lib$(1)"))
+  $(eval EXTLIBS += $(shell sh -c "$(PKG_CONFIG) --libs-only-l lib$(1)"))
 endef
 
 $(call feature_check,libtraceevent)
-- 
2.39.3
[PATCH v3 4/5] tools/rtla: Use pkg-config in lib_setup of Makefile.config
Posted by Guilherme Amadio 1 year, 5 months ago
This allows to build against libtraceevent and libtracefs installed
in non-standard locations.

Signed-off-by: Guilherme Amadio <amadio@gentoo.org>
---
 tools/tracing/rtla/Makefile.config | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/tracing/rtla/Makefile.config b/tools/tracing/rtla/Makefile.config
index 0b7ecfb30d19..5f8c286712d4 100644
--- a/tools/tracing/rtla/Makefile.config
+++ b/tools/tracing/rtla/Makefile.config
@@ -7,7 +7,8 @@ LIBTRACEFS_MIN_VERSION = 1.6
 
 define lib_setup
   $(eval LIB_INCLUDES += $(shell sh -c "$(PKG_CONFIG) --cflags lib$(1)"))
-  $(eval EXTLIBS += -l$(1))
+  $(eval LDFLAGS += $(shell sh -c "$(PKG_CONFIG) --libs-only-L lib$(1)"))
+  $(eval EXTLIBS += $(shell sh -c "$(PKG_CONFIG) --libs-only-l lib$(1)"))
 endef
 
 $(call feature_check,libtraceevent)
-- 
2.39.3
[PATCH v3 5/5] tools/latency: Use pkg-config in lib_setup of Makefile.config
Posted by Guilherme Amadio 1 year, 5 months ago
This allows to build against libtraceevent and libtracefs installed
in non-standard locations.

Signed-off-by: Guilherme Amadio <amadio@gentoo.org>
---
 tools/tracing/latency/Makefile.config | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/tracing/latency/Makefile.config b/tools/tracing/latency/Makefile.config
index b25e531a1f95..0fe6b50f029b 100644
--- a/tools/tracing/latency/Makefile.config
+++ b/tools/tracing/latency/Makefile.config
@@ -3,8 +3,9 @@
 STOP_ERROR :=
 
 define lib_setup
-  $(eval EXTLIBS += -l$(1))
   $(eval LIB_INCLUDES += $(shell sh -c "$(PKG_CONFIG) --cflags lib$(1)"))
+  $(eval LDFLAGS += $(shell sh -c "$(PKG_CONFIG) --libs-only-L lib$(1)"))
+  $(eval EXTLIBS += $(shell sh -c "$(PKG_CONFIG) --libs-only-l lib$(1)"))
 endef
 
 $(call feature_check,libtraceevent)
-- 
2.39.3
Re: [PATCH v3 0/5] perf build: libtraceevent, libtracefs feature check with pkg-config
Posted by Guilherme Amadio 1 year, 5 months ago
On Wed, Jul 17, 2024 at 12:31:47PM -0400, Steven Rostedt wrote:
> On Wed, 17 Jul 2024 09:27:58 -0700
> Namhyung Kim <namhyung@kernel.org> wrote:
> 
> > Hello,
> > 
> > On Fri, Jul 12, 2024 at 12:45 PM Guilherme Amadio <amadio@gentoo.org> wrote:
> > >
> > > Hi Namhyung, Arnaldo,
> > >
> > > Here is version 3 of the patchset. I see the change to send output to devnull
> > > has already been applied, so I am submitting the remaining work only.
> > >
> > > The difference with previous changes is that in v3 rather than add tests with
> > > pkg-config to check if dependencies are actually installed, we just set the
> > > flags and send any warning to devnull. The change that remains in this patchset
> > > is the fix for the other tools, which were inadvertently broken when the include
> > > for libtracefs changed from #include <tracefs/tracefs.h> to #include <tracefs.h>
> > > since the flags for the feature check are not set in the other tools Makefiles,
> > > it currently only works for perf. I recommend to either take at least patch 2/5
> > > that moves setting the flags to tools/build/Makefile.feature or to revert the two
> > > patches that have been applied. You can easily test the fix with the commands below:
> > >
> > > make -B -C tools/verification/rv VF=1
> > > make -B -C tools/tracing/latency VF=1
> > > make -B -C tools/tracing/rtla VF=1
> > >
> > > from the root of the repository. Only after the feature check flags are moved to
> > > Makefile.feature that it also fixes the other tools. Apologies for the breakage
> > > there.  
> > 
> > Steve, are you ok with having this patchset in the perf-tools tree?
> > 
> 
> I don't know as this is the first I've seen it. If you could have this
> resend with me Cc'd and/or Cc linux-trace-devel@vger.kernel.org then I
> can review and possibly ack it.

I will resend with the extra CC suggestions shortly and a cover letter
explaining everything.

Best regards,
-Guilherme
Re: [PATCH v3 0/5] perf build: libtraceevent, libtracefs feature check with pkg-config
Posted by Thorsten Leemhuis 1 year, 5 months ago
On 12.07.24 21:40, Guilherme Amadio wrote:
> 
> Here is version 3 of the patchset. I see the change to send output to devnull
> has already been applied, so I am submitting the remaining work only.
> [...]

Applied the series and it fixed my build problems with Fedora's RPM spec
file adjusted for -next[1], hence:

Tested-by: Thorsten Leemhuis <linux@leemhuis.info>

[1]
https://lore.kernel.org/all/569626ee-4aca-418b-9cfc-10cf5e2747a9@leemhuis.info/

Ciao, THorsten