Re: [PATCH] libperf: Don't remove -g when EXTRA_CFLAGS are used

[Ian Rogers]

On Wed, Mar 19, 2025 at 4:40 AM James Clark <james.clark@xxxxxxxxxx> wrote:
>
> When using EXTRA_CFLAGS, for example "EXTRA_CFLAGS=-DREFCNT_CHECKING=1",
> this construct stops setting -g which you'd expect would not be affected
> by adding extra flags. Additionally, EXTRA_CFLAGS should be the last
> thing to be appended so that it can be used to undo any defaults. And no
> condition is required, just += appends to any existing CFLAGS and also
> appends or doesn't append EXTRA_CFLAGS if they are or aren't set.
>
> It's not clear why DEBUG=1 is required for -g in Perf when in libperf
> it's always on, but I don't think we need to change that behavior now
> because someone may be depending on it.
>
> Signed-off-by: James Clark <james.clark@xxxxxxxxxx>
> ---
>  tools/lib/perf/Makefile | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/tools/lib/perf/Makefile b/tools/lib/perf/Makefile
> index 3a9b2140aa04..478fe57bf8ce 100644
> --- a/tools/lib/perf/Makefile
> +++ b/tools/lib/perf/Makefile
> @@ -54,13 +54,6 @@ endif
>
>  TEST_ARGS := $(if $(V),-v)
>
> -# Set compile option CFLAGS
> -ifdef EXTRA_CFLAGS
> -  CFLAGS := $(EXTRA_CFLAGS)
> -else
> -  CFLAGS := -g -Wall
> -endif

I suspect this was cargo culted from:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/lib/bpf/Makefile?h=perf-tools-next#n81
I'm not clear why it was ever that way, but perhaps a wider cleanup is
necessary than just this one patch.

Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks,
Ian

>  INCLUDES = \
>  -I$(srctree)/tools/lib/perf/include \
>  -I$(srctree)/tools/lib/ \
> @@ -70,11 +63,12 @@ INCLUDES = \
>  -I$(srctree)/tools/include/uapi
>
>  # Append required CFLAGS
> -override CFLAGS += $(EXTRA_WARNINGS)
> -override CFLAGS += -Werror -Wall
> +override CFLAGS += -g -Werror -Wall
>  override CFLAGS += -fPIC
>  override CFLAGS += $(INCLUDES)
>  override CFLAGS += -fvisibility=hidden
> +override CFLAGS += $(EXTRA_WARNINGS)
> +override CFLAGS += $(EXTRA_CFLAGS)
>
>  all:
>
> --
> 2.34.1
>