| Message ID | 20221020141412.346992-4-tobias@waldekranz.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | package/kmemd: new package | expand |
Hello Tobias, On Thu, 20 Oct 2022 16:14:11 +0200 Tobias Waldekranz <tobias@waldekranz.com> wrote: > This makes it obvious that the comment is just the negation of the > dependency requirements. It also makes it easier for other packages to > depend on libbpf. > > Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> > --- > package/libbpf/Config.in | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/package/libbpf/Config.in b/package/libbpf/Config.in > index 1465366c9e..7e816c193d 100644 > --- a/package/libbpf/Config.in > +++ b/package/libbpf/Config.in > @@ -1,11 +1,16 @@ > -config BR2_PACKAGE_LIBBPF > - bool "libbpf" > +config BR2_PACKAGE_LIBBPF_SUPPORTED > + bool > depends on BR2_TOOLCHAIN_HAS_SYNC_4 > depends on BR2_USE_WCHAR # elfutils > depends on !BR2_STATIC_LIBS # elfutils > depends on BR2_TOOLCHAIN_HAS_THREADS # elfutils > depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_13 > depends on BR2_TOOLCHAIN_USES_UCLIBC || BR2_TOOLCHAIN_USES_GLIBC # elfutils I'm afraid, but we don't do this in Buildroot, in none of our packages. I think our rationale is that by duplicating these dependencies, it is easier to maintain the Config.in comment in the packages that select BR2_PACKAGE_LIBBPF. I agree it's a very arbitrary choice, but we really strongly want things to be consistent between all Buildroot packages, so we cannot change this for just one package. If we want to change this, it needs to be discussed (advantages vs. drawbacks), then the documentation needs to be updated, and all packages changed. Best regards, Thomas
On ons, okt 26, 2022 at 22:15, Thomas Petazzoni via buildroot <buildroot@buildroot.org> wrote: > Hello Tobias, > > On Thu, 20 Oct 2022 16:14:11 +0200 > Tobias Waldekranz <tobias@waldekranz.com> wrote: > >> This makes it obvious that the comment is just the negation of the >> dependency requirements. It also makes it easier for other packages to >> depend on libbpf. >> >> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> >> --- >> package/libbpf/Config.in | 15 ++++++++------- >> 1 file changed, 8 insertions(+), 7 deletions(-) >> >> diff --git a/package/libbpf/Config.in b/package/libbpf/Config.in >> index 1465366c9e..7e816c193d 100644 >> --- a/package/libbpf/Config.in >> +++ b/package/libbpf/Config.in >> @@ -1,11 +1,16 @@ >> -config BR2_PACKAGE_LIBBPF >> - bool "libbpf" >> +config BR2_PACKAGE_LIBBPF_SUPPORTED >> + bool >> depends on BR2_TOOLCHAIN_HAS_SYNC_4 >> depends on BR2_USE_WCHAR # elfutils >> depends on !BR2_STATIC_LIBS # elfutils >> depends on BR2_TOOLCHAIN_HAS_THREADS # elfutils >> depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_13 >> depends on BR2_TOOLCHAIN_USES_UCLIBC || BR2_TOOLCHAIN_USES_GLIBC # elfutils > > I'm afraid, but we don't do this in Buildroot, in none of our packages. > I think our rationale is that by duplicating these dependencies, it is > easier to maintain the Config.in comment in the packages that select > BR2_PACKAGE_LIBBPF. Fair enough, I will drop this change in v2 and copy the information to kmemd.mk instead. Is there a convention for how to comment dependencies that stem from grandparent packages? E.g. v2, kmemd.mk will declare: depends on BR2_USE_WCHAR # <comment> as libbpf requires it, but only because of its dependency on elfutils. So what is the proper <comment> in this case? "elfutils" or "libbpf"? Or maybe something like "elfutils->libbpf"? > I agree it's a very arbitrary choice, but we really strongly want > things to be consistent between all Buildroot packages, so we cannot > change this for just one package. If we want to change this, it needs > to be discussed (advantages vs. drawbacks), then the documentation > needs to be updated, and all packages changed. Completely understand. As a Buildroot user, I really appreciate that consistency! > Best regards, > > Thomas > -- > Thomas Petazzoni, co-owner and CEO, Bootlin > Embedded Linux and Kernel engineering and training > https://bootlin.com > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
Hello Tobias, On Wed, 26 Oct 2022 23:14:08 +0200 Tobias Waldekranz <tobias@waldekranz.com> wrote: > Fair enough, I will drop this change in v2 and copy the information to > kmemd.mk instead. Is there a convention for how to comment dependencies > that stem from grandparent packages? E.g. v2, kmemd.mk will declare: No need to send a v2, I already applied the patches locally, did the few fixes, I will push them soon. Thanks a lot! Thomas
diff --git a/package/libbpf/Config.in b/package/libbpf/Config.in index 1465366c9e..7e816c193d 100644 --- a/package/libbpf/Config.in +++ b/package/libbpf/Config.in @@ -1,11 +1,16 @@ -config BR2_PACKAGE_LIBBPF - bool "libbpf" +config BR2_PACKAGE_LIBBPF_SUPPORTED + bool depends on BR2_TOOLCHAIN_HAS_SYNC_4 depends on BR2_USE_WCHAR # elfutils depends on !BR2_STATIC_LIBS # elfutils depends on BR2_TOOLCHAIN_HAS_THREADS # elfutils depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_13 depends on BR2_TOOLCHAIN_USES_UCLIBC || BR2_TOOLCHAIN_USES_GLIBC # elfutils + default y + +config BR2_PACKAGE_LIBBPF + bool "libbpf" + depends on BR2_PACKAGE_LIBBPF_SUPPORTED select BR2_PACKAGE_ELFUTILS select BR2_PACKAGE_ZLIB help @@ -17,8 +22,4 @@ config BR2_PACKAGE_LIBBPF https://github.com/libbpf/libbpf comment "libbpf needs a uClibc or glibc toolchain w/ wchar, dynamic library, threads, headers >= 4.13" - depends on BR2_TOOLCHAIN_HAS_SYNC_4 - depends on !BR2_USE_WCHAR || BR2_STATIC_LIBS \ - || !BR2_TOOLCHAIN_HAS_THREADS \ - || !BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_13 \ - || !(BR2_TOOLCHAIN_USES_UCLIBC || BR2_TOOLCHAIN_USES_GLIBC) + depends on !BR2_PACKAGE_LIBBPF_SUPPORTED
This makes it obvious that the comment is just the negation of the dependency requirements. It also makes it easier for other packages to depend on libbpf. Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> --- package/libbpf/Config.in | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)