Re: [PATCH] perf build: Add missing cflags when building with custom libtraceevent

[Namhyung Kim <namhyung@kernel.org>]

On Wed, Oct 30, 2024 at 11:04:58AM +0000, Leo Yan wrote:
> On Wed, Oct 30, 2024 at 10:45:47AM +0800, Yicong Yang wrote:
> 
> [...]
> 
> > >> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > >> index 4ddb27a48eed..53fc5f787a77 100644
> > >> --- a/tools/perf/Makefile.config
> > >> +++ b/tools/perf/Makefile.config
> > >> @@ -1194,7 +1194,7 @@ endif
> > >>  ifneq ($(NO_LIBTRACEEVENT),1)
> > >>    $(call feature_check,libtraceevent)
> > >>    ifeq ($(feature-libtraceevent), 1)
> > >> -    CFLAGS += -DHAVE_LIBTRACEEVENT
> > >> +    CFLAGS += -DHAVE_LIBTRACEEVENT $(shell $(PKG_CONFIG) --cflags libtraceevent)
> > >
> > > Does this solve the problem?  On my machine, it returns this:
> > >
> > >   $ pkg-config --cflags libtraceevent
> > >   -I/usr/include/traceevent
> > >
> > >   $ ls /usr/include/traceevent/
> > >   event-parse.h  event-utils.h  kbuffer.h  trace-seq.h
> > >
> > > So the include path should be "/usr/include" or we should include
> > > <event-parse.h> directly.  We may update the every include statements
> > > but just curious how it solved your problem.
> > >
> > 
> > I think you're right, we should use the path according to the pkg-config outputs.
> > But it's a different problem, for me it's due to the missing of the include path
> > (my libtraceevent is not under the system path). With the fix you proposed and drop
> > the change in my patch:
> > 
> > $ PKG_CONFIG_PATH=/home/yangyicong/Community/libtraceevent/install/usr/local/lib/aarch64-linux-gnu/pkgconfig/ pkg-config --cflags-only-I libtraceevent
> > -I/home/yangyicong/Community/libtraceevent/install/usr/local/include/traceevent
> > $ PKG_CONFIG_PATH=/home/yangyicong/Community/libtraceevent/install/usr/local/lib/aarch64-linux-gnu/pkgconfig/ make -C tools/perf NO_LIBPYTHON=1
> > In file included from bench/../util/session.h:5,
> >                  from bench/synthesize.c:12:
> > bench/../util/trace-event.h:153:10: fatal error: event-parse.h: No such file or directory
> >   153 | #include <event-parse.h>
> >       |
> > [...]
> > 
> > Checked how we handle with libtracefs currently:
> >  ifeq ($(feature-libtracefs), 1)
> >     CFLAGS +=  $(shell $(PKG_CONFIG) --cflags libtracefs) # we've added the cflags from pkg-config
> >     LDFLAGS += $(shell $(PKG_CONFIG) --libs-only-L libtracefs)
> 
> The libtracefs.pc file claims requesting libtraceevent, as a result, it
> returns the header paths for both libtraceevent and libtracefs:
> 
>   $ pkg-config --cflags libtracefs
>   -I/usr/local/include/tracefs -I/usr/local/include/traceevent
> 
> I checked the packages libtraceevent-dev and libtracefs-dev and either
> can be installed independently. Therefore, it seems to me that this
> patch is correct for finding the traceevent header path, with no
> dependency on tracefs.

I'm ok with the patch itself.  Can I get your Acked-by, Leo?

Thanks,
Namhyung