linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] tools/resolve_btfids: Install subcmd headers
@ 2023-01-12  0:40 Ian Rogers
  2023-01-16 11:22 ` Jiri Olsa
  0 siblings, 1 reply; 5+ messages in thread
From: Ian Rogers @ 2023-01-12  0:40 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Connor OBrien,
	Nathan Chancellor, Ian Rogers, Kumar Kartikeya Dwivedi, bpf,
	linux-kernel

Previously tools/lib/subcmd was added to the include path, switch to
installing the headers and then including from that directory. This
avoids dependencies on headers internal to tools/lib/subcmd. Add the
missing subcmd directory to the affected #include.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/bpf/resolve_btfids/Makefile | 19 ++++++++++++++-----
 tools/bpf/resolve_btfids/main.c   |  2 +-
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
index 19a3112e271a..de7d29cf43d6 100644
--- a/tools/bpf/resolve_btfids/Makefile
+++ b/tools/bpf/resolve_btfids/Makefile
@@ -35,21 +35,29 @@ SUBCMD_SRC := $(srctree)/tools/lib/subcmd/
 BPFOBJ     := $(OUTPUT)/libbpf/libbpf.a
 LIBBPF_OUT := $(abspath $(dir $(BPFOBJ)))/
 SUBCMDOBJ  := $(OUTPUT)/libsubcmd/libsubcmd.a
+SUBCMD_OUT := $(abspath $(dir $(SUBCMDOBJ)))/
 
 LIBBPF_DESTDIR := $(LIBBPF_OUT)
 LIBBPF_INCLUDE := $(LIBBPF_DESTDIR)include
 
+SUBCMD_DESTDIR := $(SUBCMD_OUT)
+SUBCMD_INCLUDE := $(SUBCMD_DESTDIR)include
+
 BINARY     := $(OUTPUT)/resolve_btfids
 BINARY_IN  := $(BINARY)-in.o
 
 all: $(BINARY)
 
+prepare: $(SUBCMDOBJ)
+
 $(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):
 	$(call msg,MKDIR,,$@)
 	$(Q)mkdir -p $(@)
 
 $(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
-	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
+	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(SUBCMD_OUT) \
+		    DESTDIR=$(SUBCMD_DESTDIR) prefix= \
+		    $(abspath $@) install_headers
 
 $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUT)
 	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT)    \
@@ -60,14 +68,14 @@ CFLAGS += -g \
           -I$(srctree)/tools/include \
           -I$(srctree)/tools/include/uapi \
           -I$(LIBBPF_INCLUDE) \
-          -I$(SUBCMD_SRC)
+          -I$(SUBCMD_INCLUDE)
 
 LIBS = -lelf -lz
 
 export srctree OUTPUT CFLAGS Q
 include $(srctree)/tools/build/Makefile.include
 
-$(BINARY_IN): $(BPFOBJ) fixdep FORCE | $(OUTPUT)
+$(BINARY_IN): $(BPFOBJ) fixdep FORCE prepare | $(OUTPUT)
 	$(Q)$(MAKE) $(build)=resolve_btfids
 
 $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
@@ -79,7 +87,8 @@ clean_objects := $(wildcard $(OUTPUT)/*.o                \
                             $(OUTPUT)/.*.o.d             \
                             $(LIBBPF_OUT)                \
                             $(LIBBPF_DESTDIR)            \
-                            $(OUTPUT)/libsubcmd          \
+                            $(SUBCMD_OUT)                \
+                            $(SUBCMD_DESTDIR)            \
                             $(OUTPUT)/resolve_btfids)
 
 ifneq ($(clean_objects),)
@@ -96,4 +105,4 @@ tags:
 
 FORCE:
 
-.PHONY: all FORCE clean tags
+.PHONY: all FORCE clean tags prepare
diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
index 80cd7843c677..77058174082d 100644
--- a/tools/bpf/resolve_btfids/main.c
+++ b/tools/bpf/resolve_btfids/main.c
@@ -75,7 +75,7 @@
 #include <linux/err.h>
 #include <bpf/btf.h>
 #include <bpf/libbpf.h>
-#include <parse-options.h>
+#include <subcmd/parse-options.h>
 
 #define BTF_IDS_SECTION	".BTF_ids"
 #define BTF_ID		"__BTF_ID__"
-- 
2.39.0.314.g84b9a713c41-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] tools/resolve_btfids: Install subcmd headers
  2023-01-12  0:40 [PATCH v1] tools/resolve_btfids: Install subcmd headers Ian Rogers
@ 2023-01-16 11:22 ` Jiri Olsa
  2023-01-16 17:20   ` Ian Rogers
  0 siblings, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2023-01-16 11:22 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Connor OBrien,
	Nathan Chancellor, Kumar Kartikeya Dwivedi, bpf, linux-kernel

On Wed, Jan 11, 2023 at 04:40:24PM -0800, Ian Rogers wrote:
> Previously tools/lib/subcmd was added to the include path, switch to
> installing the headers and then including from that directory. This
> avoids dependencies on headers internal to tools/lib/subcmd. Add the
> missing subcmd directory to the affected #include.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/bpf/resolve_btfids/Makefile | 19 ++++++++++++++-----
>  tools/bpf/resolve_btfids/main.c   |  2 +-
>  2 files changed, 15 insertions(+), 6 deletions(-)

this depends on changes that went to Arnaldo's tree right?
I can't apply this on bpf-next/master

> 
> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> index 19a3112e271a..de7d29cf43d6 100644
> --- a/tools/bpf/resolve_btfids/Makefile
> +++ b/tools/bpf/resolve_btfids/Makefile
> @@ -35,21 +35,29 @@ SUBCMD_SRC := $(srctree)/tools/lib/subcmd/
>  BPFOBJ     := $(OUTPUT)/libbpf/libbpf.a
>  LIBBPF_OUT := $(abspath $(dir $(BPFOBJ)))/
>  SUBCMDOBJ  := $(OUTPUT)/libsubcmd/libsubcmd.a
> +SUBCMD_OUT := $(abspath $(dir $(SUBCMDOBJ)))/
>  
>  LIBBPF_DESTDIR := $(LIBBPF_OUT)
>  LIBBPF_INCLUDE := $(LIBBPF_DESTDIR)include
>  
> +SUBCMD_DESTDIR := $(SUBCMD_OUT)
> +SUBCMD_INCLUDE := $(SUBCMD_DESTDIR)include
> +
>  BINARY     := $(OUTPUT)/resolve_btfids
>  BINARY_IN  := $(BINARY)-in.o
>  
>  all: $(BINARY)
>  
> +prepare: $(SUBCMDOBJ)

do we need special target for that? we already have BPFOBJ dependency
placed in the BINARY_IN as prereq

why not place both as BINARY_IN prereq, or is there some other reason
for new 'prepare' target?

jirka

> +
>  $(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):
>  	$(call msg,MKDIR,,$@)
>  	$(Q)mkdir -p $(@)
>  
>  $(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
> -	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
> +	$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(SUBCMD_OUT) \
> +		    DESTDIR=$(SUBCMD_DESTDIR) prefix= \
> +		    $(abspath $@) install_headers
>  
>  $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUT)
>  	$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT)    \
> @@ -60,14 +68,14 @@ CFLAGS += -g \
>            -I$(srctree)/tools/include \
>            -I$(srctree)/tools/include/uapi \
>            -I$(LIBBPF_INCLUDE) \
> -          -I$(SUBCMD_SRC)
> +          -I$(SUBCMD_INCLUDE)
>  
>  LIBS = -lelf -lz
>  
>  export srctree OUTPUT CFLAGS Q
>  include $(srctree)/tools/build/Makefile.include
>  
> -$(BINARY_IN): $(BPFOBJ) fixdep FORCE | $(OUTPUT)
> +$(BINARY_IN): $(BPFOBJ) fixdep FORCE prepare | $(OUTPUT)
>  	$(Q)$(MAKE) $(build)=resolve_btfids
>  
>  $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
> @@ -79,7 +87,8 @@ clean_objects := $(wildcard $(OUTPUT)/*.o                \
>                              $(OUTPUT)/.*.o.d             \
>                              $(LIBBPF_OUT)                \
>                              $(LIBBPF_DESTDIR)            \
> -                            $(OUTPUT)/libsubcmd          \
> +                            $(SUBCMD_OUT)                \
> +                            $(SUBCMD_DESTDIR)            \
>                              $(OUTPUT)/resolve_btfids)
>  
>  ifneq ($(clean_objects),)
> @@ -96,4 +105,4 @@ tags:
>  
>  FORCE:
>  
> -.PHONY: all FORCE clean tags
> +.PHONY: all FORCE clean tags prepare
> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> index 80cd7843c677..77058174082d 100644
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c
> @@ -75,7 +75,7 @@
>  #include <linux/err.h>
>  #include <bpf/btf.h>
>  #include <bpf/libbpf.h>
> -#include <parse-options.h>
> +#include <subcmd/parse-options.h>
>  
>  #define BTF_IDS_SECTION	".BTF_ids"
>  #define BTF_ID		"__BTF_ID__"
> -- 
> 2.39.0.314.g84b9a713c41-goog
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] tools/resolve_btfids: Install subcmd headers
  2023-01-16 11:22 ` Jiri Olsa
@ 2023-01-16 17:20   ` Ian Rogers
  2023-01-16 18:20     ` Jiri Olsa
  0 siblings, 1 reply; 5+ messages in thread
From: Ian Rogers @ 2023-01-16 17:20 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Connor OBrien,
	Nathan Chancellor, Kumar Kartikeya Dwivedi, bpf, linux-kernel

On Mon, Jan 16, 2023 at 3:22 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Wed, Jan 11, 2023 at 04:40:24PM -0800, Ian Rogers wrote:
> > Previously tools/lib/subcmd was added to the include path, switch to
> > installing the headers and then including from that directory. This
> > avoids dependencies on headers internal to tools/lib/subcmd. Add the
> > missing subcmd directory to the affected #include.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/bpf/resolve_btfids/Makefile | 19 ++++++++++++++-----
> >  tools/bpf/resolve_btfids/main.c   |  2 +-
> >  2 files changed, 15 insertions(+), 6 deletions(-)
>
> this depends on changes that went to Arnaldo's tree right?
> I can't apply this on bpf-next/master

Hmm.. sorry for that. I did the work on the master branch of
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

> >
> > diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> > index 19a3112e271a..de7d29cf43d6 100644
> > --- a/tools/bpf/resolve_btfids/Makefile
> > +++ b/tools/bpf/resolve_btfids/Makefile
> > @@ -35,21 +35,29 @@ SUBCMD_SRC := $(srctree)/tools/lib/subcmd/
> >  BPFOBJ     := $(OUTPUT)/libbpf/libbpf.a
> >  LIBBPF_OUT := $(abspath $(dir $(BPFOBJ)))/
> >  SUBCMDOBJ  := $(OUTPUT)/libsubcmd/libsubcmd.a
> > +SUBCMD_OUT := $(abspath $(dir $(SUBCMDOBJ)))/
> >
> >  LIBBPF_DESTDIR := $(LIBBPF_OUT)
> >  LIBBPF_INCLUDE := $(LIBBPF_DESTDIR)include
> >
> > +SUBCMD_DESTDIR := $(SUBCMD_OUT)
> > +SUBCMD_INCLUDE := $(SUBCMD_DESTDIR)include
> > +
> >  BINARY     := $(OUTPUT)/resolve_btfids
> >  BINARY_IN  := $(BINARY)-in.o
> >
> >  all: $(BINARY)
> >
> > +prepare: $(SUBCMDOBJ)
>
> do we need special target for that? we already have BPFOBJ dependency
> placed in the BINARY_IN as prereq

BPFOBJ is $(OUTPUT)/libbpf/libbpf.a which is needed at link time. The
prepare step is one we have elsewhere and it creates things like the
header files necessary to compile the C code, so we need it earlier is
the answer.

> why not place both as BINARY_IN prereq, or is there some other reason
> for new 'prepare' target?

I was trying to follow the convention elsewhere in the tree of having
a prepare target that does things like get the necessary header files
ready. Having prepare is useful because if an additional dependency is
added, then it just needs to be added to prepare. It could be tedious
to list all the dependencies for every C file, although Makefile.build
handles most of that. It isn't clear to me why $(BPFOBJ) is a
dependency of $(BINARY_IN) as it is already a dependency of $(BINARY).

Thanks,
Ian

> jirka
>
> > +
> >  $(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):
> >       $(call msg,MKDIR,,$@)
> >       $(Q)mkdir -p $(@)
> >
> >  $(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
> > -     $(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
> > +     $(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(SUBCMD_OUT) \
> > +                 DESTDIR=$(SUBCMD_DESTDIR) prefix= \
> > +                 $(abspath $@) install_headers
> >
> >  $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUT)
> >       $(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT)    \
> > @@ -60,14 +68,14 @@ CFLAGS += -g \
> >            -I$(srctree)/tools/include \
> >            -I$(srctree)/tools/include/uapi \
> >            -I$(LIBBPF_INCLUDE) \
> > -          -I$(SUBCMD_SRC)
> > +          -I$(SUBCMD_INCLUDE)
> >
> >  LIBS = -lelf -lz
> >
> >  export srctree OUTPUT CFLAGS Q
> >  include $(srctree)/tools/build/Makefile.include
> >
> > -$(BINARY_IN): $(BPFOBJ) fixdep FORCE | $(OUTPUT)
> > +$(BINARY_IN): $(BPFOBJ) fixdep FORCE prepare | $(OUTPUT)
> >       $(Q)$(MAKE) $(build)=resolve_btfids
> >
> >  $(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
> > @@ -79,7 +87,8 @@ clean_objects := $(wildcard $(OUTPUT)/*.o                \
> >                              $(OUTPUT)/.*.o.d             \
> >                              $(LIBBPF_OUT)                \
> >                              $(LIBBPF_DESTDIR)            \
> > -                            $(OUTPUT)/libsubcmd          \
> > +                            $(SUBCMD_OUT)                \
> > +                            $(SUBCMD_DESTDIR)            \
> >                              $(OUTPUT)/resolve_btfids)
> >
> >  ifneq ($(clean_objects),)
> > @@ -96,4 +105,4 @@ tags:
> >
> >  FORCE:
> >
> > -.PHONY: all FORCE clean tags
> > +.PHONY: all FORCE clean tags prepare
> > diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> > index 80cd7843c677..77058174082d 100644
> > --- a/tools/bpf/resolve_btfids/main.c
> > +++ b/tools/bpf/resolve_btfids/main.c
> > @@ -75,7 +75,7 @@
> >  #include <linux/err.h>
> >  #include <bpf/btf.h>
> >  #include <bpf/libbpf.h>
> > -#include <parse-options.h>
> > +#include <subcmd/parse-options.h>
> >
> >  #define BTF_IDS_SECTION      ".BTF_ids"
> >  #define BTF_ID               "__BTF_ID__"
> > --
> > 2.39.0.314.g84b9a713c41-goog
> >

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] tools/resolve_btfids: Install subcmd headers
  2023-01-16 17:20   ` Ian Rogers
@ 2023-01-16 18:20     ` Jiri Olsa
  2023-01-16 21:15       ` Ian Rogers
  0 siblings, 1 reply; 5+ messages in thread
From: Jiri Olsa @ 2023-01-16 18:20 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Connor OBrien,
	Nathan Chancellor, Kumar Kartikeya Dwivedi, bpf, linux-kernel

On Mon, Jan 16, 2023 at 09:20:39AM -0800, Ian Rogers wrote:
> On Mon, Jan 16, 2023 at 3:22 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> >
> > On Wed, Jan 11, 2023 at 04:40:24PM -0800, Ian Rogers wrote:
> > > Previously tools/lib/subcmd was added to the include path, switch to
> > > installing the headers and then including from that directory. This
> > > avoids dependencies on headers internal to tools/lib/subcmd. Add the
> > > missing subcmd directory to the affected #include.
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > >  tools/bpf/resolve_btfids/Makefile | 19 ++++++++++++++-----
> > >  tools/bpf/resolve_btfids/main.c   |  2 +-
> > >  2 files changed, 15 insertions(+), 6 deletions(-)
> >
> > this depends on changes that went to Arnaldo's tree right?
> > I can't apply this on bpf-next/master
> 
> Hmm.. sorry for that. I did the work on the master branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> 
> > >
> > > diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> > > index 19a3112e271a..de7d29cf43d6 100644
> > > --- a/tools/bpf/resolve_btfids/Makefile
> > > +++ b/tools/bpf/resolve_btfids/Makefile
> > > @@ -35,21 +35,29 @@ SUBCMD_SRC := $(srctree)/tools/lib/subcmd/
> > >  BPFOBJ     := $(OUTPUT)/libbpf/libbpf.a
> > >  LIBBPF_OUT := $(abspath $(dir $(BPFOBJ)))/
> > >  SUBCMDOBJ  := $(OUTPUT)/libsubcmd/libsubcmd.a
> > > +SUBCMD_OUT := $(abspath $(dir $(SUBCMDOBJ)))/
> > >
> > >  LIBBPF_DESTDIR := $(LIBBPF_OUT)
> > >  LIBBPF_INCLUDE := $(LIBBPF_DESTDIR)include
> > >
> > > +SUBCMD_DESTDIR := $(SUBCMD_OUT)
> > > +SUBCMD_INCLUDE := $(SUBCMD_DESTDIR)include
> > > +
> > >  BINARY     := $(OUTPUT)/resolve_btfids
> > >  BINARY_IN  := $(BINARY)-in.o
> > >
> > >  all: $(BINARY)
> > >
> > > +prepare: $(SUBCMDOBJ)
> >
> > do we need special target for that? we already have BPFOBJ dependency
> > placed in the BINARY_IN as prereq
> 
> BPFOBJ is $(OUTPUT)/libbpf/libbpf.a which is needed at link time. The
> prepare step is one we have elsewhere and it creates things like the
> header files necessary to compile the C code, so we need it earlier is
> the answer.
> 
> > why not place both as BINARY_IN prereq, or is there some other reason
> > for new 'prepare' target?
> 
> I was trying to follow the convention elsewhere in the tree of having
> a prepare target that does things like get the necessary header files
> ready. Having prepare is useful because if an additional dependency is
> added, then it just needs to be added to prepare. It could be tedious
> to list all the dependencies for every C file, although Makefile.build

ok, could we maybe add the BPFOBJ in prepare target as well?

> handles most of that. It isn't clear to me why $(BPFOBJ) is a
> dependency of $(BINARY_IN) as it is already a dependency of $(BINARY).

I think that if you specify OUTPUT then we need the libbpf headers
to be created before we go to compile resolve_btfids objects

thanks,
jirka

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] tools/resolve_btfids: Install subcmd headers
  2023-01-16 18:20     ` Jiri Olsa
@ 2023-01-16 21:15       ` Ian Rogers
  0 siblings, 0 replies; 5+ messages in thread
From: Ian Rogers @ 2023-01-16 21:15 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Connor OBrien,
	Nathan Chancellor, Kumar Kartikeya Dwivedi, bpf, linux-kernel

On Mon, Jan 16, 2023 at 10:20 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Mon, Jan 16, 2023 at 09:20:39AM -0800, Ian Rogers wrote:
> > On Mon, Jan 16, 2023 at 3:22 AM Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > On Wed, Jan 11, 2023 at 04:40:24PM -0800, Ian Rogers wrote:
> > > > Previously tools/lib/subcmd was added to the include path, switch to
> > > > installing the headers and then including from that directory. This
> > > > avoids dependencies on headers internal to tools/lib/subcmd. Add the
> > > > missing subcmd directory to the affected #include.
> > > >
> > > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > > ---
> > > >  tools/bpf/resolve_btfids/Makefile | 19 ++++++++++++++-----
> > > >  tools/bpf/resolve_btfids/main.c   |  2 +-
> > > >  2 files changed, 15 insertions(+), 6 deletions(-)
> > >
> > > this depends on changes that went to Arnaldo's tree right?
> > > I can't apply this on bpf-next/master
> >
> > Hmm.. sorry for that. I did the work on the master branch of
> > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> >
> > > >
> > > > diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> > > > index 19a3112e271a..de7d29cf43d6 100644
> > > > --- a/tools/bpf/resolve_btfids/Makefile
> > > > +++ b/tools/bpf/resolve_btfids/Makefile
> > > > @@ -35,21 +35,29 @@ SUBCMD_SRC := $(srctree)/tools/lib/subcmd/
> > > >  BPFOBJ     := $(OUTPUT)/libbpf/libbpf.a
> > > >  LIBBPF_OUT := $(abspath $(dir $(BPFOBJ)))/
> > > >  SUBCMDOBJ  := $(OUTPUT)/libsubcmd/libsubcmd.a
> > > > +SUBCMD_OUT := $(abspath $(dir $(SUBCMDOBJ)))/
> > > >
> > > >  LIBBPF_DESTDIR := $(LIBBPF_OUT)
> > > >  LIBBPF_INCLUDE := $(LIBBPF_DESTDIR)include
> > > >
> > > > +SUBCMD_DESTDIR := $(SUBCMD_OUT)
> > > > +SUBCMD_INCLUDE := $(SUBCMD_DESTDIR)include
> > > > +
> > > >  BINARY     := $(OUTPUT)/resolve_btfids
> > > >  BINARY_IN  := $(BINARY)-in.o
> > > >
> > > >  all: $(BINARY)
> > > >
> > > > +prepare: $(SUBCMDOBJ)
> > >
> > > do we need special target for that? we already have BPFOBJ dependency
> > > placed in the BINARY_IN as prereq
> >
> > BPFOBJ is $(OUTPUT)/libbpf/libbpf.a which is needed at link time. The
> > prepare step is one we have elsewhere and it creates things like the
> > header files necessary to compile the C code, so we need it earlier is
> > the answer.
> >
> > > why not place both as BINARY_IN prereq, or is there some other reason
> > > for new 'prepare' target?
> >
> > I was trying to follow the convention elsewhere in the tree of having
> > a prepare target that does things like get the necessary header files
> > ready. Having prepare is useful because if an additional dependency is
> > added, then it just needs to be added to prepare. It could be tedious
> > to list all the dependencies for every C file, although Makefile.build
>
> ok, could we maybe add the BPFOBJ in prepare target as well?

Agreed, added to v2.

> > handles most of that. It isn't clear to me why $(BPFOBJ) is a
> > dependency of $(BINARY_IN) as it is already a dependency of $(BINARY).
>
> I think that if you specify OUTPUT then we need the libbpf headers
> to be created before we go to compile resolve_btfids objects
>
> thanks,
> jirka

Yep. I also noticed that this code is doing the "CC=$(HOSTCC)" thing.
The problem with that is Makefile.include (line 2) will set things
like CFLAGS based on CC and then you change it. I sent out some
objtool build cleanup for this:
https://lore.kernel.org/lkml/20230105090155.357604-4-irogers@google.com/
I'll add a patch for this in v2.

Thanks,
Ian

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-01-16 21:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-12  0:40 [PATCH v1] tools/resolve_btfids: Install subcmd headers Ian Rogers
2023-01-16 11:22 ` Jiri Olsa
2023-01-16 17:20   ` Ian Rogers
2023-01-16 18:20     ` Jiri Olsa
2023-01-16 21:15       ` Ian Rogers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).