2023-01-19 18:52:07

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v3] perf script flamegraph: Avoid d3-flame-graph package dependency

Currently flame graph generation requires a d3-flame-graph template to
be installed. Unfortunately this is hard to come by for things like
Debian [1]. If the template isn't installed then ask if it should be
downloaded from jsdelivr CDN. The downloaded HTML file is validated
against an md5sum. If the download fails, generate a minimal flame
graph with the javascript coming from links to jsdelivr CDN.

v3. Adds a warning message and quits before download in live mode.
v2. Change the warning to a prompt about downloading and add the
--allow-download command line flag. Add an md5sum check for the
downloaded HTML.

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996839

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/scripts/python/flamegraph.py | 107 +++++++++++++++++++-----
1 file changed, 85 insertions(+), 22 deletions(-)

diff --git a/tools/perf/scripts/python/flamegraph.py b/tools/perf/scripts/python/flamegraph.py
index b6af1dd5f816..cf7ce8229a6c 100755
--- a/tools/perf/scripts/python/flamegraph.py
+++ b/tools/perf/scripts/python/flamegraph.py
@@ -19,12 +19,34 @@
# pylint: disable=missing-function-docstring

from __future__ import print_function
-import sys
-import os
-import io
import argparse
+import hashlib
+import io
import json
+import os
import subprocess
+import sys
+import urllib.request
+
+minimal_html = """<head>
+ <link rel="stylesheet" type="text/css" href="https://cdn.jsdelivr.net/npm/[email protected]/dist/d3-flamegraph.css">
+</head>
+<body>
+ <div id="chart"></div>
+ <script type="text/javascript" src="https://d3js.org/d3.v7.js"></script>
+ <script type="text/javascript" src="https://cdn.jsdelivr.net/npm/[email protected]/dist/d3-flamegraph.min.js"></script>
+ <script type="text/javascript">
+ const stacks = [/** @flamegraph_json **/];
+ // Note, options is unused.
+ const options = [/** @options_json **/];
+
+ var chart = flamegraph();
+ d3.select("#chart")
+ .datum(stacks[0])
+ .call(chart);
+ </script>
+</body>
+"""

# pylint: disable=too-few-public-methods
class Node:
@@ -50,16 +72,6 @@ class FlameGraphCLI:
self.args = args
self.stack = Node("all", "root")

- if self.args.format == "html" and \
- not os.path.isfile(self.args.template):
- print("Flame Graph template {} does not exist. Please install "
- "the js-d3-flame-graph (RPM) or libjs-d3-flame-graph (deb) "
- "package, specify an existing flame graph template "
- "(--template PATH) or another output format "
- "(--format FORMAT).".format(self.args.template),
- file=sys.stderr)
- sys.exit(1)
-
@staticmethod
def get_libtype_from_dso(dso):
"""
@@ -128,16 +140,63 @@ class FlameGraphCLI:
}
options_json = json.dumps(options)

+ template_md5sum = None
+ if self.args.format == "html":
+ if os.path.isfile(self.args.template):
+ template = f"file://{self.args.template}"
+ else:
+ if not self.args.allow_download:
+ print(f"""Warning: Flame Graph template '{self.args.template}'
+does not exist. To avoid this please install a package such as the
+js-d3-flame-graph or libjs-d3-flame-graph, specify an existing flame
+graph template (--template PATH) or use another output format (--format
+FORMAT).""",
+ file=sys.stderr)
+ if self.args.input == "-":
+ print("""Not attempting to download Flame Graph template as script command line
+input is disabled due to using live mode. If you want to download the
+template retry without live mode. For example, use 'perf record -a -g
+-F 99 sleep 60' and 'perf script report flamegraph'. Alternatively,
+download the template from:
+https://cdn.jsdelivr.net/npm/[email protected]/dist/templates/d3-flamegraph-base.html
+and place it at:
+/usr/share/d3-flame-graph/d3-flamegraph-base.html""",
+ file=sys.stderr)
+ quit()
+ s = None
+ while s != "y" and s != "n":
+ s = input("Do you wish to download a template from cdn.jsdelivr.net? (this warning can be suppressed with --allow-download) [yn] ").lower()
+ if s == "n":
+ quit()
+ template = "https://cdn.jsdelivr.net/npm/[email protected]/dist/templates/d3-flamegraph-base.html"
+ template_md5sum = "143e0d06ba69b8370b9848dcd6ae3f36"
+
try:
- with io.open(self.args.template, encoding="utf-8") as template:
- output_str = (
- template.read()
- .replace("/** @options_json **/", options_json)
- .replace("/** @flamegraph_json **/", stacks_json)
- )
- except IOError as err:
- print("Error reading template file: {}".format(err), file=sys.stderr)
- sys.exit(1)
+ with urllib.request.urlopen(template) as template:
+ output_str = "".join([
+ l.decode("utf-8") for l in template.readlines()
+ ])
+ except Exception as err:
+ print(f"Error reading template {template}: {err}\n"
+ "a minimal flame graph will be generated", file=sys.stderr)
+ output_str = minimal_html
+ template_md5sum = None
+
+ if template_md5sum:
+ download_md5sum = hashlib.md5(output_str.encode("utf-8")).hexdigest()
+ if download_md5sum != template_md5sum:
+ s = None
+ while s != "y" and s != "n":
+ s = input(f"""Unexpected template md5sum.
+{download_md5sum} != {template_md5sum}, for:
+{output_str}
+continue?[yn] """).lower()
+ if s == "n":
+ quit()
+
+ output_str = output_str.replace("/** @options_json **/", options_json)
+ output_str = output_str.replace("/** @flamegraph_json **/", stacks_json)
+
output_fn = self.args.output or "flamegraph.html"
else:
output_str = stacks_json
@@ -172,6 +231,10 @@ if __name__ == "__main__":
choices=["blue-green", "orange"])
parser.add_argument("-i", "--input",
help=argparse.SUPPRESS)
+ parser.add_argument("--allow-download",
+ default=False,
+ action="store_true",
+ help="allow unprompted downloading of HTML template")

cli_args = parser.parse_args()
cli = FlameGraphCLI(cli_args)
--
2.39.0.314.g84b9a713c41-goog


2023-01-19 18:54:57

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v3 2/2] tools/resolve_btfids: Alter how HOSTCC is forced

HOSTCC is always wanted when building. Setting CC to HOSTCC happens
after tools/scripts/Makefile.include is included, meaning flags are
set assuming say CC is gcc, but then it can be later set to HOSTCC
which may be clang. tools/scripts/Makefile.include is needed for host
set up and common macros in objtool's Makefile. Rather than override
CC to HOSTCC, just pass CC as HOSTCC to Makefile.build, the libsubcmd
builds and the linkage step. This means the Makefiles don't see things
like CC changing and tool flag determination, and similar, work
properly.

Also, clear the passed subdir as otherwise an outer build may break by
inadvertently passing an inappropriate value.

Signed-off-by: Ian Rogers <[email protected]>
---
tools/bpf/resolve_btfids/Makefile | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
index 76b737b2560d..ac8e302babc6 100644
--- a/tools/bpf/resolve_btfids/Makefile
+++ b/tools/bpf/resolve_btfids/Makefile
@@ -18,14 +18,11 @@ else
endif

# always use the host compiler
-AR = $(HOSTAR)
-CC = $(HOSTCC)
-LD = $(HOSTLD)
-ARCH = $(HOSTARCH)
+HOST_OVERRIDES := AR="$(HOSTAR)" CC="$(HOSTCC)" LD="$(HOSTLD)" ARCH="$(HOSTARCH)" \
+ EXTRA_CFLAGS="$(HOSTCFLAGS) $(KBUILD_HOSTCFLAGS)"
+
RM ?= rm
CROSS_COMPILE =
-CFLAGS := $(KBUILD_HOSTCFLAGS)
-LDFLAGS := $(KBUILD_HOSTLDFLAGS)

OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/

@@ -56,12 +53,12 @@ $(OUTPUT) $(OUTPUT)/libsubcmd $(LIBBPF_OUT):

$(SUBCMDOBJ): fixdep FORCE | $(OUTPUT)/libsubcmd
$(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(SUBCMD_OUT) \
- DESTDIR=$(SUBCMD_DESTDIR) prefix= \
+ DESTDIR=$(SUBCMD_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
$(abspath $@) install_headers

$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(LIBBPF_OUT)
$(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(LIBBPF_OUT) \
- DESTDIR=$(LIBBPF_DESTDIR) prefix= EXTRA_CFLAGS="$(CFLAGS)" \
+ DESTDIR=$(LIBBPF_DESTDIR) $(HOST_OVERRIDES) prefix= subdir= \
$(abspath $@) install_headers

CFLAGS += -g \
@@ -76,11 +73,11 @@ export srctree OUTPUT CFLAGS Q
include $(srctree)/tools/build/Makefile.include

$(BINARY_IN): fixdep FORCE prepare | $(OUTPUT)
- $(Q)$(MAKE) $(build)=resolve_btfids
+ $(Q)$(MAKE) $(build)=resolve_btfids $(HOST_OVERRIDES)

$(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
$(call msg,LINK,$@)
- $(Q)$(CC) $(BINARY_IN) $(LDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
+ $(Q)$(HOSTCC) $(BINARY_IN) $(KBUILD_HOSTLDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)

clean_objects := $(wildcard $(OUTPUT)/*.o \
$(OUTPUT)/.*.o.cmd \
--
2.39.0.246.g2a6d74b583-goog

2023-01-19 19:10:46

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v3 1/2] tools/resolve_btfids: Install subcmd headers

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 <[email protected]>
---
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..76b737b2560d 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: $(BPFOBJ) $(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): 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.246.g2a6d74b583-goog

2023-01-19 21:00:42

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH v3] perf script flamegraph: Avoid d3-flame-graph package dependency

On Thu, Jan 19, 2023 at 10:31 AM Ian Rogers <[email protected]> wrote:
>
> Currently flame graph generation requires a d3-flame-graph template to
> be installed. Unfortunately this is hard to come by for things like
> Debian [1]. If the template isn't installed then ask if it should be
> downloaded from jsdelivr CDN. The downloaded HTML file is validated
> against an md5sum. If the download fails, generate a minimal flame
> graph with the javascript coming from links to jsdelivr CDN.
>
> v3. Adds a warning message and quits before download in live mode.
> v2. Change the warning to a prompt about downloading and add the
> --allow-download command line flag. Add an md5sum check for the
> downloaded HTML.
>
> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=996839
>
> Signed-off-by: Ian Rogers <[email protected]>


Apologies, please ignore this patch. I accidentally picked this merged
change up with a wild card.

Thanks,
Ian

> ---
> tools/perf/scripts/python/flamegraph.py | 107 +++++++++++++++++++-----
> 1 file changed, 85 insertions(+), 22 deletions(-)
>
> diff --git a/tools/perf/scripts/python/flamegraph.py b/tools/perf/scripts/python/flamegraph.py
> index b6af1dd5f816..cf7ce8229a6c 100755
> --- a/tools/perf/scripts/python/flamegraph.py
> +++ b/tools/perf/scripts/python/flamegraph.py
> @@ -19,12 +19,34 @@
> # pylint: disable=missing-function-docstring
>
> from __future__ import print_function
> -import sys
> -import os
> -import io
> import argparse
> +import hashlib
> +import io
> import json
> +import os
> import subprocess
> +import sys
> +import urllib.request
> +
> +minimal_html = """<head>
> + <link rel="stylesheet" type="text/css" href="https://cdn.jsdelivr.net/npm/[email protected]/dist/d3-flamegraph.css">
> +</head>
> +<body>
> + <div id="chart"></div>
> + <script type="text/javascript" src="https://d3js.org/d3.v7.js"></script>
> + <script type="text/javascript" src="https://cdn.jsdelivr.net/npm/[email protected]/dist/d3-flamegraph.min.js"></script>
> + <script type="text/javascript">
> + const stacks = [/** @flamegraph_json **/];
> + // Note, options is unused.
> + const options = [/** @options_json **/];
> +
> + var chart = flamegraph();
> + d3.select("#chart")
> + .datum(stacks[0])
> + .call(chart);
> + </script>
> +</body>
> +"""
>
> # pylint: disable=too-few-public-methods
> class Node:
> @@ -50,16 +72,6 @@ class FlameGraphCLI:
> self.args = args
> self.stack = Node("all", "root")
>
> - if self.args.format == "html" and \
> - not os.path.isfile(self.args.template):
> - print("Flame Graph template {} does not exist. Please install "
> - "the js-d3-flame-graph (RPM) or libjs-d3-flame-graph (deb) "
> - "package, specify an existing flame graph template "
> - "(--template PATH) or another output format "
> - "(--format FORMAT).".format(self.args.template),
> - file=sys.stderr)
> - sys.exit(1)
> -
> @staticmethod
> def get_libtype_from_dso(dso):
> """
> @@ -128,16 +140,63 @@ class FlameGraphCLI:
> }
> options_json = json.dumps(options)
>
> + template_md5sum = None
> + if self.args.format == "html":
> + if os.path.isfile(self.args.template):
> + template = f"file://{self.args.template}"
> + else:
> + if not self.args.allow_download:
> + print(f"""Warning: Flame Graph template '{self.args.template}'
> +does not exist. To avoid this please install a package such as the
> +js-d3-flame-graph or libjs-d3-flame-graph, specify an existing flame
> +graph template (--template PATH) or use another output format (--format
> +FORMAT).""",
> + file=sys.stderr)
> + if self.args.input == "-":
> + print("""Not attempting to download Flame Graph template as script command line
> +input is disabled due to using live mode. If you want to download the
> +template retry without live mode. For example, use 'perf record -a -g
> +-F 99 sleep 60' and 'perf script report flamegraph'. Alternatively,
> +download the template from:
> +https://cdn.jsdelivr.net/npm/[email protected]/dist/templates/d3-flamegraph-base.html
> +and place it at:
> +/usr/share/d3-flame-graph/d3-flamegraph-base.html""",
> + file=sys.stderr)
> + quit()
> + s = None
> + while s != "y" and s != "n":
> + s = input("Do you wish to download a template from cdn.jsdelivr.net? (this warning can be suppressed with --allow-download) [yn] ").lower()
> + if s == "n":
> + quit()
> + template = "https://cdn.jsdelivr.net/npm/[email protected]/dist/templates/d3-flamegraph-base.html"
> + template_md5sum = "143e0d06ba69b8370b9848dcd6ae3f36"
> +
> try:
> - with io.open(self.args.template, encoding="utf-8") as template:
> - output_str = (
> - template.read()
> - .replace("/** @options_json **/", options_json)
> - .replace("/** @flamegraph_json **/", stacks_json)
> - )
> - except IOError as err:
> - print("Error reading template file: {}".format(err), file=sys.stderr)
> - sys.exit(1)
> + with urllib.request.urlopen(template) as template:
> + output_str = "".join([
> + l.decode("utf-8") for l in template.readlines()
> + ])
> + except Exception as err:
> + print(f"Error reading template {template}: {err}\n"
> + "a minimal flame graph will be generated", file=sys.stderr)
> + output_str = minimal_html
> + template_md5sum = None
> +
> + if template_md5sum:
> + download_md5sum = hashlib.md5(output_str.encode("utf-8")).hexdigest()
> + if download_md5sum != template_md5sum:
> + s = None
> + while s != "y" and s != "n":
> + s = input(f"""Unexpected template md5sum.
> +{download_md5sum} != {template_md5sum}, for:
> +{output_str}
> +continue?[yn] """).lower()
> + if s == "n":
> + quit()
> +
> + output_str = output_str.replace("/** @options_json **/", options_json)
> + output_str = output_str.replace("/** @flamegraph_json **/", stacks_json)
> +
> output_fn = self.args.output or "flamegraph.html"
> else:
> output_str = stacks_json
> @@ -172,6 +231,10 @@ if __name__ == "__main__":
> choices=["blue-green", "orange"])
> parser.add_argument("-i", "--input",
> help=argparse.SUPPRESS)
> + parser.add_argument("--allow-download",
> + default=False,
> + action="store_true",
> + help="allow unprompted downloading of HTML template")
>
> cli_args = parser.parse_args()
> cli = FlameGraphCLI(cli_args)
> --
> 2.39.0.314.g84b9a713c41-goog
>

2023-01-23 21:11:41

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] tools/resolve_btfids: Install subcmd headers

On 1/19/23 7:31 PM, 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 <[email protected]>
> ---
> 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..76b737b2560d 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: $(BPFOBJ) $(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
>

This series needs a rebase against bpf-next, given it results in merge conflict
e.g. see commit 0e43662e61f2 ("tools/resolve_btfids: Use pkg-config to locate
libelf") from Dec 15th.

Thanks,
Daniel

2023-01-24 06:48:03

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] tools/resolve_btfids: Install subcmd headers

On Mon, Jan 23, 2023 at 12:37 PM Daniel Borkmann <[email protected]> wrote:
>
> On 1/19/23 7:31 PM, 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 <[email protected]>
> > ---
> > 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..76b737b2560d 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: $(BPFOBJ) $(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
> >
>
> This series needs a rebase against bpf-next, given it results in merge conflict
> e.g. see commit 0e43662e61f2 ("tools/resolve_btfids: Use pkg-config to locate
> libelf") from Dec 15th.
>
> Thanks,
> Daniel

Thanks, rebased onto bpf-next/master in v4:
https://lore.kernel.org/lkml/[email protected]/

Ian