[v2,08/14] tools lib perf: Add missing install headers

Message ID 20221109184914.1357295-9-irogers@google.com
State New
Headers
Series Fix perf tools/lib includes |

Commit Message

Ian Rogers Nov. 9, 2022, 6:49 p.m. UTC
  Headers necessary for the perf build. Note, internal headers are also
installed as these are necessary for the build.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/lib/perf/Makefile | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
  

Comments

Namhyung Kim Nov. 9, 2022, 8:12 p.m. UTC | #1
On Wed, Nov 9, 2022 at 10:50 AM Ian Rogers <irogers@google.com> wrote:
>
> Headers necessary for the perf build. Note, internal headers are also
> installed as these are necessary for the build.

Yeah, it's sad we are using those internal headers in perf.
Ideally libperf provides callbacks to associate private data
to each public data structure (e.g. evsel, evlist, etc).  And
external users just use public APIs only.

But that would be a major change.

Thanks,
Namhyung


>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/lib/perf/Makefile | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/perf/Makefile b/tools/lib/perf/Makefile
> index 21df023a2103..1badc0a04676 100644
> --- a/tools/lib/perf/Makefile
> +++ b/tools/lib/perf/Makefile
> @@ -189,13 +189,21 @@ install_lib: libs
>
>  install_headers:
>         $(call QUIET_INSTALL, headers) \
> +               $(call do_install,include/perf/bpf_perf.h,$(prefix)/include/perf,644); \
>                 $(call do_install,include/perf/core.h,$(prefix)/include/perf,644); \
>                 $(call do_install,include/perf/cpumap.h,$(prefix)/include/perf,644); \
>                 $(call do_install,include/perf/threadmap.h,$(prefix)/include/perf,644); \
>                 $(call do_install,include/perf/evlist.h,$(prefix)/include/perf,644); \
>                 $(call do_install,include/perf/evsel.h,$(prefix)/include/perf,644); \
>                 $(call do_install,include/perf/event.h,$(prefix)/include/perf,644); \
> -               $(call do_install,include/perf/mmap.h,$(prefix)/include/perf,644);
> +               $(call do_install,include/perf/mmap.h,$(prefix)/include/perf,644); \
> +               $(call do_install,include/internal/cpumap.h,$(prefix)/include/internal,644); \
> +               $(call do_install,include/internal/evlist.h,$(prefix)/include/internal,644); \
> +               $(call do_install,include/internal/evsel.h,$(prefix)/include/internal,644); \
> +               $(call do_install,include/internal/lib.h,$(prefix)/include/internal,644); \
> +               $(call do_install,include/internal/mmap.h,$(prefix)/include/internal,644); \
> +               $(call do_install,include/internal/threadmap.h,$(prefix)/include/internal,644); \
> +               $(call do_install,include/internal/xyarray.h,$(prefix)/include/internal,644);
>
>  install_pkgconfig: $(LIBPERF_PC)
>         $(call QUIET_INSTALL, $(LIBPERF_PC)) \
> --
> 2.38.1.431.g37b22c650d-goog
>
  
Arnaldo Carvalho de Melo Nov. 10, 2022, 5:35 p.m. UTC | #2
Em Wed, Nov 09, 2022 at 12:12:16PM -0800, Namhyung Kim escreveu:
> On Wed, Nov 9, 2022 at 10:50 AM Ian Rogers <irogers@google.com> wrote:
> > Headers necessary for the perf build. Note, internal headers are also
> > installed as these are necessary for the build.
 
> Yeah, it's sad we are using those internal headers in perf.

The plan is for perf to eventually be a libperf tool, but what was done
so far was make available classes and methods that were asked for by
libperf users.

Completely untangling tools/perf/ from those internal bits will needs
lots more work, so doing it the way Ian is doing now seems ok.

- Arnaldo

> Ideally libperf provides callbacks to associate private data
> to each public data structure (e.g. evsel, evlist, etc).  And
> external users just use public APIs only.
> 
> But that would be a major change.
  
Namhyung Kim Nov. 10, 2022, 6:08 p.m. UTC | #3
Hi Arnaldo,

On Thu, Nov 10, 2022 at 9:35 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Wed, Nov 09, 2022 at 12:12:16PM -0800, Namhyung Kim escreveu:
> > On Wed, Nov 9, 2022 at 10:50 AM Ian Rogers <irogers@google.com> wrote:
> > > Headers necessary for the perf build. Note, internal headers are also
> > > installed as these are necessary for the build.
>
> > Yeah, it's sad we are using those internal headers in perf.
>
> The plan is for perf to eventually be a libperf tool, but what was done
> so far was make available classes and methods that were asked for by
> libperf users.
>
> Completely untangling tools/perf/ from those internal bits will needs
> lots more work, so doing it the way Ian is doing now seems ok.

Agreed.  I'm happy to see this work going on. :)

Thanks,
Namhyung
  

Patch

diff --git a/tools/lib/perf/Makefile b/tools/lib/perf/Makefile
index 21df023a2103..1badc0a04676 100644
--- a/tools/lib/perf/Makefile
+++ b/tools/lib/perf/Makefile
@@ -189,13 +189,21 @@  install_lib: libs
 
 install_headers:
 	$(call QUIET_INSTALL, headers) \
+		$(call do_install,include/perf/bpf_perf.h,$(prefix)/include/perf,644); \
 		$(call do_install,include/perf/core.h,$(prefix)/include/perf,644); \
 		$(call do_install,include/perf/cpumap.h,$(prefix)/include/perf,644); \
 		$(call do_install,include/perf/threadmap.h,$(prefix)/include/perf,644); \
 		$(call do_install,include/perf/evlist.h,$(prefix)/include/perf,644); \
 		$(call do_install,include/perf/evsel.h,$(prefix)/include/perf,644); \
 		$(call do_install,include/perf/event.h,$(prefix)/include/perf,644); \
-		$(call do_install,include/perf/mmap.h,$(prefix)/include/perf,644);
+		$(call do_install,include/perf/mmap.h,$(prefix)/include/perf,644); \
+		$(call do_install,include/internal/cpumap.h,$(prefix)/include/internal,644); \
+		$(call do_install,include/internal/evlist.h,$(prefix)/include/internal,644); \
+		$(call do_install,include/internal/evsel.h,$(prefix)/include/internal,644); \
+		$(call do_install,include/internal/lib.h,$(prefix)/include/internal,644); \
+		$(call do_install,include/internal/mmap.h,$(prefix)/include/internal,644); \
+		$(call do_install,include/internal/threadmap.h,$(prefix)/include/internal,644); \
+		$(call do_install,include/internal/xyarray.h,$(prefix)/include/internal,644);
 
 install_pkgconfig: $(LIBPERF_PC)
 	$(call QUIET_INSTALL, $(LIBPERF_PC)) \