On Sat, Jan 2, 2021 at 10:25 AM Luca Boccassi <bluca@xxxxxxxxxx> wrote:
>
> Add a new CMake option, LIBBPF_EMBEDDED, to switch between the
> embedded version and the system version (searched via pkg-config)
> of libbpf. Set the system version as the default.
>
There's been a lot of arguments about libbpf as a submodule, so I
don't think we need to go about pros and cons again, but I just wanted
to cast my vote against doing this at all. Having pahole built with
libbpf statically (only) was a great thing for me so far with
iterating quickly and adopting new APIs without overcomplicating the
source code with all sorts of feature detection code. Without it,
adopting libbpf's faster string deduplication, BTF writing APIs,
module/split BTFs, etc, etc, would be much bigger PITA and/or would
prolong such changes. To the point that I personally wouldn't bother
with some of them at all. Distro maintainers obviously don't care
about such inconveniences for developers, but it's a real factor in
determining what kind of functionality is implemented in pahole, so I
hope Arnaldo won't dismiss this without thinking about this carefully.
> Signed-off-by: Luca Boccassi <bluca@xxxxxxxxxx>
> ---
> Note: I can switch the default around if you prefer.
With BCC you seem to go with the default preserving existing behavior
(using libbpf from a submodule), so if we do this, I think the default
should be flipped.
>
> CMakeLists.txt | 49 +++++++++++++++++++++++++++++++++++-------------
> btf_encoder.c | 5 +++++
> btf_loader.c | 4 ++++
> libbtf.c | 6 ++++++
> libbtf.h | 4 ++++
> pahole.c | 4 ++++
> pahole_strings.h | 4 ++++
> strings.c | 4 ++++
> 8 files changed, 67 insertions(+), 13 deletions(-)
>
[...]
> #include "dutil.h"
> +#ifdef LIBBPF_FOUND
> +#include <bpf/libbpf.h>
> +#else
> #include "lib/bpf/src/libbpf.h"
> +#endif
this is horrible, are you sure there is no way to make <bpf/libbpf.h>
work for both cases?
>
> struct strings *strings__new(void)
> {
> --
> 2.29.2
>
