[PATCH V2] tools/perf: Add includes for detected configs in Makefile.perf

[PATCH V2] tools/perf: Add includes for detected configs in Makefile.perf

[Athira Rajeev]

Makefile.perf uses "CONFIG_*" checks in the code. Example the config
for libtraceevent is used to set PYTHON_EXT_SRCS

	ifeq ($(CONFIG_LIBTRACEEVENT),y)
	  PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
	else
	  PYTHON_EXT_SRCS := $(shell grep -v '^\#\|util/trace-event.c' util/python-ext-sources)
	endif

But this is not picking the value for CONFIG_LIBTRACEEVENT that is
set using the settings in Makefile.config. Include the file
".config-detected" so that make will use the system detected
configuration in the CONFIG checks. This will fix isues that
could arise when other "CONFIG_*" checks are added to Makefile.perf
in future as well.

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
Changelog:
 v1 -> v2:
 Added $(OUTPUT) prefix to config-detected as pointed
 out by Ian

 tools/perf/Makefile.perf | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 37af6df7b978..66b9dc61c32f 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -351,6 +351,9 @@ export PYTHON_EXTBUILD_LIB PYTHON_EXTBUILD_TMP
 
 python-clean := $(call QUIET_CLEAN, python) $(RM) -r $(PYTHON_EXTBUILD) $(OUTPUT)python/perf*.so
 
+# Use the detected configuration
+include $(OUTPUT).config-detected
+
 ifeq ($(CONFIG_LIBTRACEEVENT),y)
   PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
 else
-- 
2.31.1

Re: [PATCH V2] tools/perf: Add includes for detected configs in Makefile.perf

[Ian Rogers]

On Fri, Sep 8, 2023 at 7:51 AM Athira Rajeev
<atrajeev@linux.vnet.ibm.com> wrote:
>
> Makefile.perf uses "CONFIG_*" checks in the code. Example the config
> for libtraceevent is used to set PYTHON_EXT_SRCS
>
>         ifeq ($(CONFIG_LIBTRACEEVENT),y)
>           PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
>         else
>           PYTHON_EXT_SRCS := $(shell grep -v '^\#\|util/trace-event.c' util/python-ext-sources)
>         endif
>
> But this is not picking the value for CONFIG_LIBTRACEEVENT that is
> set using the settings in Makefile.config. Include the file
> ".config-detected" so that make will use the system detected
> configuration in the CONFIG checks. This will fix isues that
> could arise when other "CONFIG_*" checks are added to Makefile.perf
> in future as well.
>
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> Changelog:
>  v1 -> v2:
>  Added $(OUTPUT) prefix to config-detected as pointed
>  out by Ian
>
>  tools/perf/Makefile.perf | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 37af6df7b978..66b9dc61c32f 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -351,6 +351,9 @@ export PYTHON_EXTBUILD_LIB PYTHON_EXTBUILD_TMP
>
>  python-clean := $(call QUIET_CLEAN, python) $(RM) -r $(PYTHON_EXTBUILD) $(OUTPUT)python/perf*.so
>
> +# Use the detected configuration
> +include $(OUTPUT).config-detected

The Makefile.build version also has a "-include" rather than "include"
in case the .config-detected file is missing. In Makefile.perf
including Makefile.config is optional:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/Makefile.perf?h=perf-tools-next#n253

and there are certain targets that where we don't include it:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/Makefile.perf?h=perf-tools-next#n200

So playing devil's advocate, if we ran "make clean" we'd remove
.config-detected:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/Makefile.perf?h=perf-tools-next#n1131

If we then ran "make tags" then we wouldn't include Makefile.config
and so .config-detected wouldn't be generated and I think the build
would fail due to a missing include here. So I think this should be
-include or perhaps:

ifeq ($(config),1)
include $(OUTPUT).config-detected
endif

Thanks,
Ian

> +
>  ifeq ($(CONFIG_LIBTRACEEVENT),y)
>    PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
>  else
> --
> 2.31.1
>

Re: [PATCH V2] tools/perf: Add includes for detected configs in Makefile.perf

[Athira Rajeev]

> On 08-Sep-2023, at 9:45 PM, Ian Rogers <irogers@google.com> wrote:
> 
> On Fri, Sep 8, 2023 at 7:51 AM Athira Rajeev
> <atrajeev@linux.vnet.ibm.com> wrote:
>> 
>> Makefile.perf uses "CONFIG_*" checks in the code. Example the config
>> for libtraceevent is used to set PYTHON_EXT_SRCS
>> 
>>        ifeq ($(CONFIG_LIBTRACEEVENT),y)
>>          PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
>>        else
>>          PYTHON_EXT_SRCS := $(shell grep -v '^\#\|util/trace-event.c' util/python-ext-sources)
>>        endif
>> 
>> But this is not picking the value for CONFIG_LIBTRACEEVENT that is
>> set using the settings in Makefile.config. Include the file
>> ".config-detected" so that make will use the system detected
>> configuration in the CONFIG checks. This will fix isues that
>> could arise when other "CONFIG_*" checks are added to Makefile.perf
>> in future as well.
>> 
>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>> ---
>> Changelog:
>> v1 -> v2:
>> Added $(OUTPUT) prefix to config-detected as pointed
>> out by Ian
>> 
>> tools/perf/Makefile.perf | 3 +++
>> 1 file changed, 3 insertions(+)
>> 
>> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
>> index 37af6df7b978..66b9dc61c32f 100644
>> --- a/tools/perf/Makefile.perf
>> +++ b/tools/perf/Makefile.perf
>> @@ -351,6 +351,9 @@ export PYTHON_EXTBUILD_LIB PYTHON_EXTBUILD_TMP
>> 
>> python-clean := $(call QUIET_CLEAN, python) $(RM) -r $(PYTHON_EXTBUILD) $(OUTPUT)python/perf*.so
>> 
>> +# Use the detected configuration
>> +include $(OUTPUT).config-detected
> 
> The Makefile.build version also has a "-include" rather than "include"
> in case the .config-detected file is missing. In Makefile.perf
> including Makefile.config is optional:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/Makefile.perf?h=perf-tools-next#n253
> 
> and there are certain targets that where we don't include it:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/Makefile.perf?h=perf-tools-next#n200
> 
> So playing devil's advocate, if we ran "make clean" we'd remove
> .config-detected:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/Makefile.perf?h=perf-tools-next#n1131
> 
> If we then ran "make tags" then we wouldn't include Makefile.config
> and so .config-detected wouldn't be generated and I think the build
> would fail due to a missing include here. So I think this should be
> -include or perhaps:

Hi Ian

Thanks for checking in detail. Yes, make clean in perf fails with just “include”

# make clean
Makefile.perf:355: .config-detected: No such file or directory
make[1]: *** No rule to make target '.config-detected'.  Stop.
make: *** [Makefile:90: clean] Error 2


Below change will be correct as you pointed:

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 66b9dc61c32f..f6fdc2d5a92f 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -352,7 +352,7 @@ export PYTHON_EXTBUILD_LIB PYTHON_EXTBUILD_TMP
 python-clean := $(call QUIET_CLEAN, python) $(RM) -r $(PYTHON_EXTBUILD) $(OUTPUT)python/perf*.so
   # Use the detected configuration
-include $(OUTPUT).config-detected
+-include $(OUTPUT).config-detected
   ifeq ($(CONFIG_LIBTRACEEVENT),y)
   PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)


I could test to make sure it includes the file when it is present and picks the detected configs correctly as well with this change.
Adding this change in V3 

Thanks
Athira
> 
> ifeq ($(config),1)
> include $(OUTPUT).config-detected
> endif
> 
> Thanks,
> Ian
> 
>> +
>> ifeq ($(CONFIG_LIBTRACEEVENT),y)
>>   PYTHON_EXT_SRCS := $(shell grep -v ^\# util/python-ext-sources)
>> else
>> --
>> 2.31.1