Re: [PATCH v2 1/2] CMakeLists.txt: enable SHARED and STATIC lib creation

[Luca Boccassi <bluca@debian.org>]

[-- Attachment #1: Type: text/plain, Size: 6556 bytes --]

On Fri, 2021-06-11 at 13:08 -0700, Andrii Nakryiko wrote:
On Fri, Jun 11, 2021 at 1:00 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
> 
> Em Fri, Jun 11, 2021 at 12:34:13PM -0700, Andrii Nakryiko escreveu:
> > On Thu, Jun 10, 2021 at 10:31 AM Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:
> > > 
> > > Em Tue, Jun 08, 2021 at 12:50:13AM +0530, Deepak Kumar Mishra
> > > escreveu:
> > > > CMakeLists.txt does not allow creation of static library and
> > > > link applications
> > > > accordingly.
> > > > 
> > > > Creation of SHARED and STATIC should be allowed using -
> > > > DBUILD_SHARED_LIBS
> > > > If -DBUILD_SHARED_LIBS option is not supplied, CMakeLists.txt
> > > > sets it to ON.
> > > > 
> > > > Ex:
> > > > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
> > > > cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..
> > > 
> > > Had to do some fixups due to a previous patch touching
> > > CMakeLists.txt,
> > > please check below.
> > > 
> > > I tested it and added some performance notes.
> > 
> > Hey Arnaldo, Deepak,
> > 
> > I think this commit actually breaks libbpf's CI (see [0]) and my
> > local
> > setup as well (see output below). It seems like now we are using
> > system-wide libbpf headers, while still building local libbpf
> > sources.
> > This is pretty bad because system-wide headers might be too old
> > or
> > just missing.
> 
> I can't check this right now, but isn't this related to this one
> instead?

Heh, I beat you by 5 minutes ;)


Hi,

This should not be the case - the local paths are added to CMake and
should win, unless something is going wrong - which is of course
possible. A quick build of the current tip of the master branch would
seem to confirm things are working - building with -
DLIBBPF_EMBEDDED=off (which does force to use the system library, and
defaults to on) the build fails, while building without any options on
a new tree the build succeeds.

I'll fetch the script and try to reproduce, as it might be using other
options - I assume it's this one, right?

https://github.com/libbpf/libbpf/blob/master/travis-ci/vmtest/build_pahole.sh

> > 
> > commit ae2581647e84948810ba209f3891359dd4540110 (quaco/master,
> > quaco/HEAD, acme/tmp.master)
> > Author: Luca Boccassi <bluca@debian.org>
> > Date:   Mon Jan 4 22:16:22 2021 +0000
> > 
> >     libbpf: Allow to use packaged version
> > 
> >     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 embedded version as the default.
> > 
> >  -------
> > 
> > I can't look at this right now, will try probably tomorrow.
> > 
> > Andrii, I would love to be able to stage this somewhere, like I did
> > with
> > tmp.master, so that it could go thru your CI before I moved to
> > master,
> > is that possible?
> 
> Yes, absolutely, we can pick whatever branch and use that to checkout
> and build pahole. It would be great, though, if you can keep an eye
> on
> kernel CI and/or libbpf CI breakages when you are pushing new changes
> to pahole. That would save everyone time and will shorten the
> downtime
> for our CIs.
> 
> Here are the links where all the builds can be seen in real-time:
> 
>   - kernel CI: 
> https://travis-ci.com/github/kernel-patches/bpf/pull_requests
>   - libbpf CI: https://travis-ci.com/github/libbpf/libbpf
> 
> 
> Let me know which branch we should hard-code for staging.
> 
> > 
> > - Arnaldo
> > 
> > > Is it possible to make sure that we always use local libbpf
> > > headers
> > > when building pahole with libbpf built from sources (the default
> > > case,
> > > right?). It's also important to use UAPI headers distributed with
> > > libbpf when building libbpf itself, I don't know if that's what
> > > is
> > > done right now or not.
> > > 
> > > Note how libbpf CI case shows that system-wide bpf/btf.h is not
> > > available at all because we don't have system-wide libbpf
> > > installed.
> > > In my local case, you can see that my system-wide header is
> > > outdated
> > > and doesn't have BTF_LITTLE_ENDIAN/BTF_BIG_ENDIAN constants
> > > defined in
> > > libbpf.h.
> > > 
> > > BTW, I tried -D__LIB=lib -DBUILD_SHARED_LIBS=OFF options and they
> > > didn't help. Maybe I'm doing something wrong.
> > > 
> > >   [0] 
> > > https://travis-ci.com/github/kernel-patches/bpf/builds/228673352
> > > 
> > > 
> > > $ make -j60
> > > -- Setting BUILD_SHARED_LIBS = ON
> > > -- Checking availability of DWARF and ELF development libraries
> > > -- Checking availability of DWARF and ELF development libraries -
> > > done
> > > -- Configuring done
> > > -- Generating done
> > > -- Build files have been written to:
> > > /home/andriin/local/pahole/build
> > > 
> > > ....
> > > 
> > > /home/andriin/local/pahole/btf_encoder.c:900:28: error:
> > > ‘BTF_LITTLE_ENDIAN’ undeclared (first use in this function)
> > >    btf__set_endianness(btf, BTF_LITTLE_ENDIAN);
> > >                             ^
> > > /home/andriin/local/pahole/btf_encoder.c:900:28: note: each
> > > undeclared
> > > identifier is reported only once for each function it appears in
> > > /home/andriin/local/pahole/btf_encoder.c:903:28: error:
> > > ‘BTF_BIG_ENDIAN’ undeclared (first use in this function)
> > >    btf__set_endianness(btf, BTF_BIG_ENDIAN);
> > >                             ^
> > > ...
> > > 
> > > 
> > > > 
> > > > Thanks!
> > > > 
> > > > - Arnaldo
> > > > 
> > > > commit aa2027708659f172780f85698f14303c7de6a1d2
> > > > Author: Deepak Kumar Mishra <deepakkumar.mishra@arm.com>
> > > > Date:   Tue Jun 8 00:50:13 2021 +0530
> > > > 
> > > >     CMakeLists.txt: Enable SHARED and STATIC lib creation
> > > > 
> > > >     CMakeLists.txt does not allow creation of static library
> > > > and link applications
> > > >     accordingly.
> > > > 
> > > >     Creation of SHARED and STATIC should be allowed using -
> > > > DBUILD_SHARED_LIBS
> > > >     If -DBUILD_SHARED_LIBS option is not supplied,
> > > > CMakeLists.txt sets it to ON.
> > > > 
> > > >     Ex:
> > > > 
> > > >       $ cmake -D__LIB=lib -DBUILD_SHARED_LIBS=OFF ..
> > > >       $ cmake -D__LIB=lib -DBUILD_SHARED_LIBS=ON ..
> > > > 
> > > 
> > > [...]
> > 
> > --
> > 
> > - Arnaldo

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]