-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revert "Remove libbcc-no-libbpf shared library, change libbcc linkage… #3225
Conversation
[buildbot, test this please] |
I do see @bluca approach is to static link libbpf package libbpf.a into shared libbcc.so on the system. libbcc-no-libbpf.so already became an ABI, so agree that we need to revert it. Sorry about this. I guess either debian can also use libbcc-no-libbpf.so or they want libbcc.so to statically link with host libbpf.a, a cmake |
@olsajiri looks like you already reverted commit version for libbpf submodule, could you checkout latest master, do a revert and resubmit? |
[buildbot, ok to test] |
This is not true though? There's the actual option to have the real library, with the real soname, dynamically linked - CMAKE_USE_LIBBPF_PACKAGE |
I mean if you have a dependency on the elf library with the wrong soname, that's fine I guess, but please add it as an option. It doesn't match the pkg-config interface, the package names, nothing.
Mmh what we need is the real library, with the real soname, dynamically linked. If an extra option is needed that's fine. |
yes, but people expect libbcc.so without libbpf.so dependency so we had to add libbcc-no-libbpf.so to be able to use it with libbpf.so |
… instead" This reverts commit 1cb5026. bpftrace in fedora is using it: -DLIBBCC_LIBRARIES:PATH=/usr/lib64/libbcc-no-libbpf.so It's the only way to have libcc together with dynamicaly linked libbpf.so. Signed-off-by: Jiri Olsa <jolsa@kernel.org>
1459fc2
to
ab4ea6e
Compare
I might be missing something, but if someone wants libbcc statically linked, the option can be turned off? It's off by default in fact, one has to opt-in
This is not really usable, I'm afraid. The soname is wrong. pkg-config doesn't match. I mean it's fine as an option I guess, but there needs to be a way to do the right thing as well. |
sorry, re-pushed |
I need to check new bcc code, but at the time we were adding this, it was the only way d4b3bf0 Add libbcc-no-libbpf.so library |
Right, the commit you are reverting makes libbcc dynamically link with libbpf when CMAKE_USE_LIBBPF_PACKAGE is used. Isn't that what you want? It's opt-in, and users who don't want that simply don't set it and get the static linking. |
@olsajiri could you check whether bpftrace works properly with |
it probably will work, I'll try but the point is that I was told libbcc needs to stay as is with libbpf compiled in, I think I won't be able to persuade libbcc fedora maintainer to change that, |
And that will keep happening with CMAKE_USE_LIBBPF_PACKAGE=off which is the default, as mentioned, so it shouldn't be a problem? Unless I'm missing something. Again, if you also want at the same time to keep the separate elf shared object with the broken soname that doesn't match pkg-config, I don't have any problem with that - but please add a separate option that can be disabled independently so that we can also do the right thing as well. Thanks! |
well, we need libbcc (with libbpf compiled in) and libbcc-no-libbpf at the same time and that was the case until this commit:
what's wrong with the soname? $ readelf -a /usr/lib64/libbcc.so.0 | grep SONAME $ readelf -a /usr/lib64/libbcc-no-libbpf.so.0 | grep SONAME thanks |
Sorry, maybe I misunderstood - I thought the different cases were needed by different people in different situations? Are you saying that you need that at the exact same time for the exact same build ? What's the use case, if you don't mind me asking?
It's mangled with the no-libbpf suffix, so it doesn't match the pkg-config metadata. It also makes it seem like it's a separate interface with a separate ABI, when in reality it's exactly the same thing, which is a bit weird. |
bcc package providing libraries is the use case it provides libbcc (with libbpf compiled in) when we compile bpftrace with libbcc, the dynamic linker gets confused, my first fix was to have libcc dynamically linked with libbpf (like you did),
while 1) and 2) might not be a big issue anymore, I'm worried about 3)
I guess we can just fix pkg-config metadata then? |
Those only applies to local builds though, it doesn't apply to a distribution (where in fact it's the opposite - vendoring means out of date libbpf that doesn't track latest changes and always falls behind). The existing option allows users to do this, and it's the default so there should be no surprise nor regressions for direct users and developers, right?
How would that happen? The breakages, as you said, come from the linker getting confused because of multiple built-in and dynamic objects. What cases are there where the opposite is true?
By all means feel free to add an entirely separate metadata file, but that doesn't work for me at all. The real thing being the real thing is required. This is just one library out of millions, it's not special in any way, and having special casing and special handling and special everything beyond a build-time config flag is just unsustainable, sorry. The build flag can be opt-in and disabled by default, that's not an issue. |
missing libbpf dependency but as you mentioned above that's probably not big deal for distributions, I tested bpftrace with libbcc and it seems to work |
It's packaged, so the libbc package will depend on libbpf - it cannot be missing. It can only be missing if it wasn't used at build time because the option was disabled, but then it would be statically linked so no issue either. As far as I can see, missing libbpf is only a problem if someone installs the libbpf-dev package and does a manual, local build of bcc and enables the option (disabled by default) and separately, manually ships the result somewhere else. I mean, it's possible, but there's a lot of opt-in steps that need to be voluntarily taken to get there, and one can just not opt-in...
Nice! |
Great. Thanks for good news :-) |
Great news, thanks for double checking! |
I'm surprised you'd remove library that's being used.
bpftrace in fedora is using it:
-DLIBBCC_LIBRARIES:PATH=/usr/lib64/libbcc-no-libbpf.so
It's the only way to have libcc together with dynamicaly linked libbpf.so.
Please let's keep it in.