Narrow libbcc dependency from llvm-dev to just libllvm #4997
Conversation
It works with list libllvm, so no need to pull in the whole thing, which is 600MiB worth of packages.
bobrik
requested review from
drzaeus77,
goldshtn,
yonghong-song,
4ast,
brendangregg and
davemarchevsky
as code owners
|
@shunghsiyu, what exactly broke for you prior to #4921? |
The problem for me is that using bcc header files (packaged as debian/control does not have a separate package to host the bcc header files so I ended up placing such requirement on the main bcc package, which is admittedly not ideal. |
|
From a packaging point of view I'd much prefer we revert 6f11bf7 to keep the dependency at a minimum. That said I only know two users of @chantra what do you think? |
|
On Sun, May 19, 2024 at 11:02 PM Shung-Hsi Yu ***@***.***> wrote:
From a packaging point of view I'd much prefer we revert 6f11bf7
<6f11bf7>
to keep the dependency at a minimum.
For packaging, this should only be a build dependency right:
https://www.debian.org/doc/debian-policy/ch-relationships.html ?
The package itself has a pretty minimal footprint (26kb on Ubuntu per
https://packages.ubuntu.com/noble/llvm-dev). Probably this pulls more deps,
but only on the build machine.
Quickly checking the issue, I think moving llvm-dev to a “Build-Depends”
would solve the problem for machines where bcc gets installed.
Would that be a good enough middle ground?
That said I only know two users of bcc-devel: one is
… Sysinternals/ProcMon-for-Linux
<https://github.com/Sysinternals/ProcMon-for-Linux> mentioned above, and
the other is bpftrace/bpftrace <https://github.com/bpftrace/bpftrace>.
bpftrace already pulls llvm-devel anyway, while ProcMon-for-Linux doesn't
get packaged everywhere; so this is perhaps a bit of bikeshedding.
@chantra <https://github.com/chantra> what do you think?
—
Reply to this email directly, view it on GitHub
<#4997 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAF77DROQGITXS5HX7OYKTZDGGWXAVCNFSM6AAAAABHPIRQ7CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJZG4ZDONRQGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
If bcc-devel requires llvm-devel, I suppose the problem is because we pull llvm-config.h in bcc_module.h which is public? Moving llvm-config.h to private only headers should do the job. Can you list the public headers and we can work on moving this include out of the way. |
Not sure about Debian, but on the RPM side, even though The follow graph depicts the dependency chain: |
I think that will work
The public headers I founds are:
So bpf_module.h should be the one that need modification. |
In iovisor#4737 I added depedency on llvm-config.h in order to discover what LLVM version is being used. This was only meant to be used at build time. By adding llvm-config.h in bpf_module.h, a public header of libbcc, I introduced a dependency on llvm-dev to libbcc which is unnecessary. This change removes the include from `bpf_module.h`, which should be enough. It does not seem I need to add the include in any other files. Fixes iovisor#4997 Signed-off-by: Manu Bretelle <chantr4@gmail.com>
In iovisor#4737 I added depedency on llvm-config.h in order to discover what LLVM version is being used. This was only meant to be used at build time. By adding llvm-config.h in bpf_module.h, a public header of libbcc, I introduced a dependency on llvm-dev to libbcc which is unnecessary. This change removes the include from `bpf_module.h`, which should be enough. It does not seem I need to add the include in any other files. Fixes iovisor#4997 Signed-off-by: Manu Bretelle <chantr4@gmail.com>
In iovisor#4737 I added depedency on llvm-config.h in order to discover what LLVM version is being used. This was only meant to be used at build time. By adding llvm-config.h in bpf_module.h, a public header of libbcc, I introduced a dependency on llvm-dev to libbcc which is unnecessary. This change removes the include from `bpf_module.h`, which should be enough, and add it to `bpf_module.cc` instead. Fixes iovisor#4997 Signed-off-by: Manu Bretelle <chantr4@gmail.com>
|
#5018 should fix this issue. @shunghsiyu could you confirm the package change are fine? cc @yonghong-song |
It works with list libllvm, so no need to pull in the whole thing, which is 600MiB worth of packages.