[PATCH] tools/rtla: Explicitly list libtraceevent dependency

[PATCH] tools/rtla: Explicitly list libtraceevent dependency

[Daniel Wagner]

The current libtracefs.pkg file lists the dependency on
libtraceevent ("pkg-config --libs libtracefs" -> "-ltracefs
-ltraceevent").

Dan Nicholson's Guide to pkg-config[1] stats that "Libs: The link
flags specific to this package and any required libraries that don't
support pkg-config". Thus the current libtracefs.pkg is not correct.

rtla is depending on libtraceevent but it doesn't express this in
'pkg-config' part to retrieve the correct build flags.

In order to be able to update the "Libs:" section in the libtracefs
project we need to list the dependency explicitly to avoid future linker
failures.

[1] https://people.freedesktop.org/~dbn/pkg-config-guide.html

Signed-off-by: Daniel Wagner <dwagner@suse.de>
---

I've got this fallout with because I am using libtraceevent and libtracefs build
with Meson. Meson generates different pkg files which seems to align with Dan's
Guide.

 tools/tracing/rtla/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/tracing/rtla/Makefile b/tools/tracing/rtla/Makefile
index 22e28b76f800..0664e2db22c1 100644
--- a/tools/tracing/rtla/Makefile
+++ b/tools/tracing/rtla/Makefile
@@ -32,7 +32,7 @@ TRACEFS_HEADERS	:= $$($(PKG_CONFIG) --cflags libtracefs)
 
 CFLAGS	:=	-O -g -DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(TRACEFS_HEADERS) $(EXTRA_CFLAGS)
 LDFLAGS	:=	-ggdb $(EXTRA_LDFLAGS)
-LIBS	:=	$$($(PKG_CONFIG) --libs libtracefs)
+LIBS	:=	$$($(PKG_CONFIG) --libs libtracefs libtraceevent)
 
 SRC	:=	$(wildcard src/*.c)
 HDR	:=	$(wildcard src/*.h)
-- 
2.39.0

Re: [PATCH] tools/rtla: Explicitly list libtraceevent dependency

[Daniel Bristot de Oliveira]

On 1/10/23 14:18, Daniel Wagner wrote:
> The current libtracefs.pkg file lists the dependency on
> libtraceevent ("pkg-config --libs libtracefs" -> "-ltracefs
> -ltraceevent").
> 
> Dan Nicholson's Guide to pkg-config[1] stats that "Libs: The link
> flags specific to this package and any required libraries that don't
> support pkg-config". Thus the current libtracefs.pkg is not correct.
> 
> rtla is depending on libtraceevent but it doesn't express this in
> 'pkg-config' part to retrieve the correct build flags.
> 
> In order to be able to update the "Libs:" section in the libtracefs
> project we need to list the dependency explicitly to avoid future linker
> failures.

I am ok with it. Steve?

-- Daniel

Re: [PATCH] tools/rtla: Explicitly list libtraceevent dependency

[Daniel Wagner]

On Tue, Jan 10, 2023 at 02:55:03PM +0100, Daniel Bristot de Oliveira wrote:
> On 1/10/23 14:18, Daniel Wagner wrote:
> > The current libtracefs.pkg file lists the dependency on
> > libtraceevent ("pkg-config --libs libtracefs" -> "-ltracefs
> > -ltraceevent").
> > 
> > Dan Nicholson's Guide to pkg-config[1] stats that "Libs: The link
> > flags specific to this package and any required libraries that don't
> > support pkg-config". Thus the current libtracefs.pkg is not correct.
> > 
> > rtla is depending on libtraceevent but it doesn't express this in
> > 'pkg-config' part to retrieve the correct build flags.
> > 
> > In order to be able to update the "Libs:" section in the libtracefs
> > project we need to list the dependency explicitly to avoid future linker
> > failures.
> 
> I am ok with it. Steve?

FWIW, this is change is also backwards compatible, meaning if you have system
which has a libtracefs.pkg installed which lists libtraceevent in its Libs:
section the 'pkg-config --libs libtracefs libtraceevent' command will return the
identically string which is '-ltracefs -ltraceevent'.

Re: [PATCH] tools/rtla: Explicitly list libtraceevent dependency

[Daniel Bristot de Oliveira]

On 1/10/23 15:08, Daniel Wagner wrote:
> On Tue, Jan 10, 2023 at 02:55:03PM +0100, Daniel Bristot de Oliveira wrote:
>> On 1/10/23 14:18, Daniel Wagner wrote:
>>> The current libtracefs.pkg file lists the dependency on
>>> libtraceevent ("pkg-config --libs libtracefs" -> "-ltracefs
>>> -ltraceevent").
>>>
>>> Dan Nicholson's Guide to pkg-config[1] stats that "Libs: The link
>>> flags specific to this package and any required libraries that don't
>>> support pkg-config". Thus the current libtracefs.pkg is not correct.
>>>
>>> rtla is depending on libtraceevent but it doesn't express this in
>>> 'pkg-config' part to retrieve the correct build flags.
>>>
>>> In order to be able to update the "Libs:" section in the libtracefs
>>> project we need to list the dependency explicitly to avoid future linker
>>> failures.
>>
>> I am ok with it. Steve?
> 
> FWIW, this is change is also backwards compatible, meaning if you have system
> which has a libtracefs.pkg installed which lists libtraceevent in its Libs:
> section the 'pkg-config --libs libtracefs libtraceevent' command will return the
> identically string which is '-ltracefs -ltraceevent'.

Yeah, we know it. I've added both in the initial implementation, but Steven suggested
using only libtracefs because it depends on libtraceevent anyways. That is why
I am re-checking with him.

-- Daniel

Re: [PATCH] tools/rtla: Explicitly list libtraceevent dependency

[Daniel Wagner]

On Tue, Jan 10, 2023 at 03:19:25PM +0100, Daniel Bristot de Oliveira wrote:
 > FWIW, this is change is also backwards compatible, meaning if you have system
> > which has a libtracefs.pkg installed which lists libtraceevent in its Libs:
> > section the 'pkg-config --libs libtracefs libtraceevent' command will return the
> > identically string which is '-ltracefs -ltraceevent'.
> 
> Yeah, we know it. I've added both in the initial implementation, but Steven suggested
> using only libtracefs because it depends on libtraceevent anyways. That is why
> I am re-checking with him.

Just to clarify, the generated pkg file by Meson is adding the libtraceevent
dependency in the private section. So this part should be okay. I would be
surprised if Meson would get this wrong at this point.

$ cat .build/meson-private/libtracefs.pc
prefix=/tmp/trace-cmd
includedir=${prefix}/include
libdir=${prefix}/lib64

Name: libtracefs
Description: Manage trace fs
URL: https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/
Version: 1.6.3
Requires.private: libtraceevent >=  1.7.0
Libs: -L${libdir} -ltracefs
Cflags: -I${includedir}/libtracefs

Re: [PATCH] tools/rtla: Explicitly list libtraceevent dependency

[Steven Rostedt]

On Tue, 10 Jan 2023 15:45:36 +0100
Daniel Wagner <dwagner@suse.de> wrote:

> Just to clarify, the generated pkg file by Meson is adding the libtraceevent
> dependency in the private section. So this part should be okay. I would be
> surprised if Meson would get this wrong at this point.

No that's incorrect.

There's many interfaces that require the libtraceevent header files to work
with libtracefs. Anything that uses libtracefs must also use libtraceevent,
as libtracefs is really just an extension of libtraceevent.

> 
> $ cat .build/meson-private/libtracefs.pc
> prefix=/tmp/trace-cmd
> includedir=${prefix}/include
> libdir=${prefix}/lib64
> 
> Name: libtracefs
> Description: Manage trace fs
> URL: https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/
> Version: 1.6.3
> Requires.private: libtraceevent >=  1.7.0

This is incorrect.

-- Steve

> Libs: -L${libdir} -ltracefs
> Cflags: -I${includedir}/libtracefs

Re: [PATCH] tools/rtla: Explicitly list libtraceevent dependency

[Steven Rostedt]

On Tue, 10 Jan 2023 09:51:37 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> There's many interfaces that require the libtraceevent header files to work
> with libtracefs. Anything that uses libtracefs must also use libtraceevent,
> as libtracefs is really just an extension of libtraceevent.

Although, I will say, since I hate the libtraceevent interface so much, I
may like to hide it via a fully functional libtracefs interface that hides
it :-)  In which case, this would be correct!

-- Steve

Re: [PATCH] tools/rtla: Explicitly list libtraceevent dependency

[Daniel Wagner]

On Tue, Jan 10, 2023 at 09:53:47AM -0500, Steven Rostedt wrote:
> On Tue, 10 Jan 2023 09:51:37 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > There's many interfaces that require the libtraceevent header files to work
> > with libtracefs. Anything that uses libtracefs must also use libtraceevent,
> > as libtracefs is really just an extension of libtraceevent.

Okay, in this case I am going update the Meson build to add the necessary
explicit dependency:

++ b/src/meson.build
@@ -50,6 +50,7 @@ libtracefs_static = static_library(
 pkg = import('pkgconfig')
 pkg.generate(
     libtracefs,
+    libraries: [libtraceevent_dep],
     subdirs: 'libtracefs',
     filebase: meson.project_name(),
     name: meson.project_name(),


$ cat /tmp/trace-cmd/lib64/pkgconfig/libtracefs.pc
prefix=/tmp/trace-cmd
includedir=${prefix}/include
libdir=${prefix}/lib64

Name: libtracefs
Description: Manage trace fs
URL: https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/
Version: 1.6.3
Requires: libtraceevent >=  1.7.0
Libs: -L${libdir} -ltracefs
Cflags: -I${includedir}/libtracefs

$ PKG_CONFIG_PATH=/tmp/trace-cmd
$ pkg-config --libs libtracefs
-L/tmp/trace-cmd/lib64 -ltracefs -ltraceevent

Re: [PATCH] tools/rtla: Explicitly list libtraceevent dependency

[Steven Rostedt]

On Tue, 10 Jan 2023 14:18:05 +0100
Daniel Wagner <dwagner@suse.de> wrote:

> The current libtracefs.pkg file lists the dependency on
> libtraceevent ("pkg-config --libs libtracefs" -> "-ltracefs
> -ltraceevent").
> 
> Dan Nicholson's Guide to pkg-config[1] stats that "Libs: The link
> flags specific to this package and any required libraries that don't
> support pkg-config". Thus the current libtracefs.pkg is not correct.
> 
> rtla is depending on libtraceevent but it doesn't express this in
> 'pkg-config' part to retrieve the correct build flags.
> 
> In order to be able to update the "Libs:" section in the libtracefs
> project we need to list the dependency explicitly to avoid future linker
> failures.
> 
> [1] https://people.freedesktop.org/~dbn/pkg-config-guide.html

The Libs: field in tracefs only shows the -ltracefs and not -ltraceevent.
It follows this rule. It's the "Requires:" tag that pulls in -ltraceevent,
correctly.

> 
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> ---
> 
> I've got this fallout with because I am using libtraceevent and libtracefs build
> with Meson. Meson generates different pkg files which seems to align with Dan's
> Guide.
> 
>  tools/tracing/rtla/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/tracing/rtla/Makefile b/tools/tracing/rtla/Makefile
> index 22e28b76f800..0664e2db22c1 100644
> --- a/tools/tracing/rtla/Makefile
> +++ b/tools/tracing/rtla/Makefile
> @@ -32,7 +32,7 @@ TRACEFS_HEADERS	:= $$($(PKG_CONFIG) --cflags libtracefs)
>  
>  CFLAGS	:=	-O -g -DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(TRACEFS_HEADERS) $(EXTRA_CFLAGS)
>  LDFLAGS	:=	-ggdb $(EXTRA_LDFLAGS)
> -LIBS	:=	$$($(PKG_CONFIG) --libs libtracefs)
> +LIBS	:=	$$($(PKG_CONFIG) --libs libtracefs libtraceevent)

I'm still confused as to why this is needed.

According to Dan's document:

Requires: A list of packages required by this package. The versions of these packages may be specified using the comparison operators =, <, >, <= or >=.
Requires.private: A list of private packages required by this package but not exposed to applications. The version specific rules from the Requires field also apply here.

The "Requires" is exported to other applications. It's the private that is
not.

What is this trying to fix?

-- Steve


>  
>  SRC	:=	$(wildcard src/*.c)
>  HDR	:=	$(wildcard src/*.h)