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

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

[Yicong Yang]

From: Yicong Yang <yangyicong@hisilicon.com>

When building with custom libtraceevent, below errors occur:
$ make -C tools/perf NO_LIBPYTHON=1 PKG_CONFIG_PATH=<custom libtraceevent>
In file included from util/session.h:5,
                 from builtin-buildid-list.c:17:
util/trace-event.h:153:10: fatal error: traceevent/event-parse.h: No such file or directory
  153 | #include <traceevent/event-parse.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~
<snip similar errors of missing headers>

This is because the include path is missed in the cflags. Add it.

Fixes: 0f0e1f445690 ("perf build: Use pkg-config for feature check for libtrace{event,fs}")
Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
---
Another problem occurs when building on my old Ubuntu 18.04.1 (4.15 kernel)
is that linux/mount.h (which is introduced in the later kernel version) is
missing. It's included in tools/include/uapi/linux/fs.h which header is
introduced recently. Does it make sense to have a copy of mount.h in tools/include?

 tools/perf/Makefile.config | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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)
     LDFLAGS += $(shell $(PKG_CONFIG) --libs-only-L libtraceevent)
     EXTLIBS += $(shell $(PKG_CONFIG) --libs-only-l libtraceevent)
     LIBTRACEEVENT_VERSION := $(shell $(PKG_CONFIG) --modversion libtraceevent).0.0
-- 
2.24.0

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

[Namhyung Kim]

Hello,

On Thu, Oct 24, 2024 at 09:32:36PM +0800, Yicong Yang wrote:
> From: Yicong Yang <yangyicong@hisilicon.com>
> 
> When building with custom libtraceevent, below errors occur:
> $ make -C tools/perf NO_LIBPYTHON=1 PKG_CONFIG_PATH=<custom libtraceevent>
> In file included from util/session.h:5,
>                  from builtin-buildid-list.c:17:
> util/trace-event.h:153:10: fatal error: traceevent/event-parse.h: No such file or directory
>   153 | #include <traceevent/event-parse.h>
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~
> <snip similar errors of missing headers>
> 
> This is because the include path is missed in the cflags. Add it.
> 
> Fixes: 0f0e1f445690 ("perf build: Use pkg-config for feature check for libtrace{event,fs}")
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> ---
> Another problem occurs when building on my old Ubuntu 18.04.1 (4.15 kernel)
> is that linux/mount.h (which is introduced in the later kernel version) is
> missing. It's included in tools/include/uapi/linux/fs.h which header is
> introduced recently. Does it make sense to have a copy of mount.h in tools/include?
> 
>  tools/perf/Makefile.config | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 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.

Thanks,
Namhyung


>      LDFLAGS += $(shell $(PKG_CONFIG) --libs-only-L libtraceevent)
>      EXTLIBS += $(shell $(PKG_CONFIG) --libs-only-l libtraceevent)
>      LIBTRACEEVENT_VERSION := $(shell $(PKG_CONFIG) --modversion libtraceevent).0.0
> -- 
> 2.24.0
> 

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

[Yicong Yang]

On 2024/10/30 8:12, Namhyung Kim wrote:
> Hello,
> 
> On Thu, Oct 24, 2024 at 09:32:36PM +0800, Yicong Yang wrote:
>> From: Yicong Yang <yangyicong@hisilicon.com>
>>
>> When building with custom libtraceevent, below errors occur:
>> $ make -C tools/perf NO_LIBPYTHON=1 PKG_CONFIG_PATH=<custom libtraceevent>
>> In file included from util/session.h:5,
>>                  from builtin-buildid-list.c:17:
>> util/trace-event.h:153:10: fatal error: traceevent/event-parse.h: No such file or directory
>>   153 | #include <traceevent/event-parse.h>
>>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~
>> <snip similar errors of missing headers>
>>
>> This is because the include path is missed in the cflags. Add it.
>>
>> Fixes: 0f0e1f445690 ("perf build: Use pkg-config for feature check for libtrace{event,fs}")
>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
>> ---
>> Another problem occurs when building on my old Ubuntu 18.04.1 (4.15 kernel)
>> is that linux/mount.h (which is introduced in the later kernel version) is
>> missing. It's included in tools/include/uapi/linux/fs.h which header is
>> introduced recently. Does it make sense to have a copy of mount.h in tools/include?
>>
>>  tools/perf/Makefile.config | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> 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)

Thanks.

> Thanks,
> Namhyung
> 
> 
>>      LDFLAGS += $(shell $(PKG_CONFIG) --libs-only-L libtraceevent)
>>      EXTLIBS += $(shell $(PKG_CONFIG) --libs-only-l libtraceevent)
>>      LIBTRACEEVENT_VERSION := $(shell $(PKG_CONFIG) --modversion libtraceevent).0.0
>> -- 
>> 2.24.0
>>
> .
> 

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

[Leo Yan]

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.

Thanks,
Leo

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

[Namhyung Kim]

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

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

[Leo Yan]

On Fri, Nov 01, 2024 at 11:23:27PM -0700, Namhyung Kim wrote:

[...]

> > > 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?

Yes.

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

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

[Guilherme Amadio]

Hi Namhyung,

On Mon, Nov 04, 2024 at 10:47:41AM +0000, Leo Yan wrote:
> On Fri, Nov 01, 2024 at 11:23:27PM -0700, Namhyung Kim wrote:
> 
> [...]
> 
> > > > 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?
> 
> Yes.
> 
> Reviewed-by: Leo Yan <leo.yan@arm.com>

I agree that this was a mistake I made in my patch. I added the cflags
to the feature check in tools/build/Makefile.feature, but in Makefile.config
I added it only for libtracefs and missed it for libtraceevent. The fixes tag
is correct. This patch fixes my mistake.

Reviewed-by: Guilherme Amadio <amadio@gentoo.org>

Best regards,
-Guilherme

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

[Yicong Yang]

On 2024/11/4 20:51, Guilherme Amadio wrote:
> Hi Namhyung,
> 
> On Mon, Nov 04, 2024 at 10:47:41AM +0000, Leo Yan wrote:
>> On Fri, Nov 01, 2024 at 11:23:27PM -0700, Namhyung Kim wrote:
>>
>> [...]
>>
>>>>> 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?
>>
>> Yes.
>>
>> Reviewed-by: Leo Yan <leo.yan@arm.com>
> 
> I agree that this was a mistake I made in my patch. I added the cflags
> to the feature check in tools/build/Makefile.feature, but in Makefile.config
> I added it only for libtracefs and missed it for libtraceevent. The fixes tag
> is correct. This patch fixes my mistake.
> 
> Reviewed-by: Guilherme Amadio <amadio@gentoo.org>
> 

Thanks for the review by Leo and Guilherme!

Hi Namhyung,

As you noticed we should use the include path from pkg-config [1]. Would you like
to post a patch yourself or want me to post this as well? It's correct and we also
need this. (I didn't found this may because I changed my custom libtraceevent.pc when
debugging and currently approach is ok if libtraceevent's installed system wide).

[1] https://lore.kernel.org/linux-perf-users/ZyF5_Hf1iL01kldE@google.com/

Thanks.

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

[Namhyung Kim]

On Tue, Nov 05, 2024 at 10:06:34AM +0800, Yicong Yang wrote:
> On 2024/11/4 20:51, Guilherme Amadio wrote:
> > Hi Namhyung,
> > 
> > On Mon, Nov 04, 2024 at 10:47:41AM +0000, Leo Yan wrote:
> >> On Fri, Nov 01, 2024 at 11:23:27PM -0700, Namhyung Kim wrote:
> >>
> >> [...]
> >>
> >>>>> 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?
> >>
> >> Yes.
> >>
> >> Reviewed-by: Leo Yan <leo.yan@arm.com>
> > 
> > I agree that this was a mistake I made in my patch. I added the cflags
> > to the feature check in tools/build/Makefile.feature, but in Makefile.config
> > I added it only for libtracefs and missed it for libtraceevent. The fixes tag
> > is correct. This patch fixes my mistake.
> > 
> > Reviewed-by: Guilherme Amadio <amadio@gentoo.org>
> > 
> 
> Thanks for the review by Leo and Guilherme!
> 
> Hi Namhyung,
> 
> As you noticed we should use the include path from pkg-config [1]. Would you like
> to post a patch yourself or want me to post this as well? It's correct and we also
> need this. (I didn't found this may because I changed my custom libtraceevent.pc when
> debugging and currently approach is ok if libtraceevent's installed system wide).

Please do that as a follow-up.

Thanks,
Namhyung

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

[Namhyung Kim]

On Thu, 24 Oct 2024 21:32:36 +0800, Yicong Yang wrote:

> When building with custom libtraceevent, below errors occur:
> $ make -C tools/perf NO_LIBPYTHON=1 PKG_CONFIG_PATH=<custom libtraceevent>
> In file included from util/session.h:5,
>                  from builtin-buildid-list.c:17:
> util/trace-event.h:153:10: fatal error: traceevent/event-parse.h: No such file or directory
>   153 | #include <traceevent/event-parse.h>
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~
> <snip similar errors of missing headers>
> 
> [...]

Applied to perf-tools-next, thanks!

Best regards,
Namhyung