| Message ID | 20230110131805.16242-1-dwagner@suse.de (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | tools/rtla: Explicitly list libtraceevent dependency | expand |
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
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'.
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
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
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)
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
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
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
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)
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(-)