* [PATCH 1/3] btrfs-tools: Update to 5.11.1
@ 2021-04-16 5:37 Robert Joslyn
2021-04-16 5:37 ` [PATCH 2/3] btrfs-tools: Add PACKAGECONFIG options Robert Joslyn
2021-04-16 5:37 ` [PATCH 3/3] btrfs-tools: Try to follow style guide Robert Joslyn
0 siblings, 2 replies; 6+ messages in thread
From: Robert Joslyn @ 2021-04-16 5:37 UTC (permalink / raw)
To: openembedded-core; +Cc: Robert Joslyn
Update licensing, as libtrfsutil is under LGPLv3+. Note that libtrfsutil
is in the process of being relicensed to LGPLv2.1+:
https://github.com/kdave/btrfs-progs/issues/323
Signed-off-by: Robert Joslyn <robert.joslyn@redrectangle.org>
---
.../{btrfs-tools_5.10.1.bb => btrfs-tools_5.11.1.bb} | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
rename meta/recipes-devtools/btrfs-tools/{btrfs-tools_5.10.1.bb => btrfs-tools_5.11.1.bb} (88%)
diff --git a/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.10.1.bb b/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
similarity index 88%
rename from meta/recipes-devtools/btrfs-tools/btrfs-tools_5.10.1.bb
rename to meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
index fca010d4ae..2ab476a88e 100644
--- a/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.10.1.bb
+++ b/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
@@ -7,14 +7,17 @@ btrfs and an utility (btrfs-convert) to make a btrfs filesystem from an ext3."
HOMEPAGE = "https://btrfs.wiki.kernel.org"
-LICENSE = "GPLv2"
-LIC_FILES_CHKSUM = "file://COPYING;md5=fcb02dc552a041dee27e4b85c7396067"
+LICENSE = "GPLv2 & LGPLv3+"
+LIC_FILES_CHKSUM = " \
+ file://COPYING;md5=fcb02dc552a041dee27e4b85c7396067 \
+ file://libbtrfsutil/COPYING.LESSER;md5=3000208d539ec061b899bce1d9ce9404 \
+"
SECTION = "base"
DEPENDS = "util-linux attr e2fsprogs lzo acl"
DEPENDS_append_class-target = " udev"
RDEPENDS_${PN} = "libgcc"
-SRCREV = "f2ffce38b9c1477a7350bfe165f0e34b9bde40f5"
+SRCREV = "8d5051f279f7994fb80536ef8f846f06d121d898"
SRC_URI = "git://git.kernel.org/pub/scm/linux/kernel/git/kdave/btrfs-progs.git \
file://0001-Add-a-possibility-to-specify-where-python-modules-ar.patch \
"
--
2.26.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] btrfs-tools: Add PACKAGECONFIG options
2021-04-16 5:37 [PATCH 1/3] btrfs-tools: Update to 5.11.1 Robert Joslyn
@ 2021-04-16 5:37 ` Robert Joslyn
2021-04-16 7:17 ` [OE-core] " Andre McCurdy
2021-04-16 5:37 ` [PATCH 3/3] btrfs-tools: Try to follow style guide Robert Joslyn
1 sibling, 1 reply; 6+ messages in thread
From: Robert Joslyn @ 2021-04-16 5:37 UTC (permalink / raw)
To: openembedded-core; +Cc: Robert Joslyn
Add options to make it easier to control which features are enabled. All
of these default to enabled by upstream, so keep them enabled to
maintain previous behavior.
The convert option also supports reiserfs, but no recipes exist in the
layer index. Limit the option to ext filesystems until someone cares
enough to make reiserfs recipes.
Remove acl and attr from DEPENDS, as they do not apper to be needed. Add
zlib since it is required.
Signed-off-by: Robert Joslyn <robert.joslyn@redrectangle.org>
---
.../btrfs-tools/btrfs-tools_5.11.1.bb | 26 ++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb b/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
index 2ab476a88e..b875ea1aa1 100644
--- a/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
+++ b/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
@@ -13,7 +13,7 @@ LIC_FILES_CHKSUM = " \
file://libbtrfsutil/COPYING.LESSER;md5=3000208d539ec061b899bce1d9ce9404 \
"
SECTION = "base"
-DEPENDS = "util-linux attr e2fsprogs lzo acl"
+DEPENDS = "lzo util-linux zlib"
DEPENDS_append_class-target = " udev"
RDEPENDS_${PN} = "libgcc"
@@ -22,17 +22,37 @@ SRC_URI = "git://git.kernel.org/pub/scm/linux/kernel/git/kdave/btrfs-progs.git \
file://0001-Add-a-possibility-to-specify-where-python-modules-ar.patch \
"
-PACKAGECONFIG ??= "python"
+PACKAGECONFIG ??= " \
+ ${@bb.utils.filter('DISTRO_FEATURES', 'largefile', d)} \
+ backtrace \
+ programs \
+ shared \
+ convert \
+ python \
+ crypto-builtin \
+"
+PACKAGECONFIG[largefile] = "--enable-largefile,--disable-largefile"
+PACKAGECONFIG[backtrace] = "--enable-backtrace,--disable-backtrace"
PACKAGECONFIG[manpages] = "--enable-documentation, --disable-documentation, asciidoc-native xmlto-native"
+PACKAGECONFIG[programs] = "--enable-programs,--disable-programs"
+PACKAGECONFIG[shared] = "--enable-shared,--disable-shared"
+PACKAGECONFIG[static] = "--enable-static,--disable-static"
+PACKAGECONFIG[convert] = "--enable-convert --with-convert=ext2,--disable-convert --without-convert,e2fsprogs"
PACKAGECONFIG[python] = "--enable-python,--disable-python,python3-setuptools-native"
PACKAGECONFIG[zstd] = "--enable-zstd,--disable-zstd,zstd"
+# Pick only one crypto provider
+PACKAGECONFIG[crypto-builtin] = "--with-crypto=builtin"
+PACKAGECONFIG[crypto-libgcrypt] = "--with-crypto=libgcrypt,,libgcrypt"
+PACKAGECONFIG[crypto-libsodium] = "--with-crypto=libsodium,,libsodium"
+PACKAGECONFIG[crypto-libkcapi] = "--with-crypto=libkcapi,,libkcapi"
+
inherit autotools-brokensep pkgconfig manpages
inherit ${@bb.utils.contains('PACKAGECONFIG', 'python', 'distutils3-base', '', d)}
CLEANBROKEN = "1"
-EXTRA_OECONF_append_libc-musl = " --disable-backtrace "
+PACKAGECONFIG_remove_libc-musl = "backtrace"
EXTRA_PYTHON_CFLAGS = "${DEBUG_PREFIX_MAP}"
EXTRA_PYTHON_CFLAGS_class-native = ""
EXTRA_PYTHON_LDFLAGS = "${LDFLAGS}"
--
2.26.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] btrfs-tools: Try to follow style guide
2021-04-16 5:37 [PATCH 1/3] btrfs-tools: Update to 5.11.1 Robert Joslyn
2021-04-16 5:37 ` [PATCH 2/3] btrfs-tools: Add PACKAGECONFIG options Robert Joslyn
@ 2021-04-16 5:37 ` Robert Joslyn
1 sibling, 0 replies; 6+ messages in thread
From: Robert Joslyn @ 2021-04-16 5:37 UTC (permalink / raw)
To: openembedded-core; +Cc: Robert Joslyn
Cosmetic changes to bettor follow the style guide.
Signed-off-by: Robert Joslyn <robert.joslyn@redrectangle.org>
---
meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb b/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
index b875ea1aa1..0b8e1d22a2 100644
--- a/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
+++ b/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
@@ -15,12 +15,12 @@ LIC_FILES_CHKSUM = " \
SECTION = "base"
DEPENDS = "lzo util-linux zlib"
DEPENDS_append_class-target = " udev"
-RDEPENDS_${PN} = "libgcc"
-SRCREV = "8d5051f279f7994fb80536ef8f846f06d121d898"
SRC_URI = "git://git.kernel.org/pub/scm/linux/kernel/git/kdave/btrfs-progs.git \
file://0001-Add-a-possibility-to-specify-where-python-modules-ar.patch \
"
+SRCREV = "8d5051f279f7994fb80536ef8f846f06d121d898"
+S = "${WORKDIR}/git"
PACKAGECONFIG ??= " \
${@bb.utils.filter('DISTRO_FEATURES', 'largefile', d)} \
@@ -64,7 +64,6 @@ do_configure_prepend() {
cp -f $(automake --print-libdir)/install-sh ${S}/config/
}
-S = "${WORKDIR}/git"
do_install_append() {
if [ "${@bb.utils.filter('PACKAGECONFIG', 'python', d)}" ]; then
@@ -72,4 +71,6 @@ do_install_append() {
fi
}
+RDEPENDS_${PN} = "libgcc"
+
BBCLASSEXTEND = "native nativesdk"
--
2.26.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [OE-core] [PATCH 2/3] btrfs-tools: Add PACKAGECONFIG options
2021-04-16 5:37 ` [PATCH 2/3] btrfs-tools: Add PACKAGECONFIG options Robert Joslyn
@ 2021-04-16 7:17 ` Andre McCurdy
2021-04-16 14:28 ` Robert Joslyn
0 siblings, 1 reply; 6+ messages in thread
From: Andre McCurdy @ 2021-04-16 7:17 UTC (permalink / raw)
To: Robert Joslyn; +Cc: OE Core mailing list
On Thu, Apr 15, 2021 at 10:38 PM Robert Joslyn
<robert.joslyn@redrectangle.org> wrote:
>
> Add options to make it easier to control which features are enabled. All
> of these default to enabled by upstream, so keep them enabled to
> maintain previous behavior.
>
> The convert option also supports reiserfs, but no recipes exist in the
> layer index. Limit the option to ext filesystems until someone cares
> enough to make reiserfs recipes.
>
> Remove acl and attr from DEPENDS, as they do not apper to be needed. Add
> zlib since it is required.
>
> Signed-off-by: Robert Joslyn <robert.joslyn@redrectangle.org>
> ---
> .../btrfs-tools/btrfs-tools_5.11.1.bb | 26 ++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb b/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
> index 2ab476a88e..b875ea1aa1 100644
> --- a/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
> +++ b/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
> @@ -13,7 +13,7 @@ LIC_FILES_CHKSUM = " \
> file://libbtrfsutil/COPYING.LESSER;md5=3000208d539ec061b899bce1d9ce9404 \
> "
> SECTION = "base"
> -DEPENDS = "util-linux attr e2fsprogs lzo acl"
> +DEPENDS = "lzo util-linux zlib"
> DEPENDS_append_class-target = " udev"
> RDEPENDS_${PN} = "libgcc"
>
> @@ -22,17 +22,37 @@ SRC_URI = "git://git.kernel.org/pub/scm/linux/kernel/git/kdave/btrfs-progs.git \
> file://0001-Add-a-possibility-to-specify-where-python-modules-ar.patch \
> "
>
> -PACKAGECONFIG ??= "python"
> +PACKAGECONFIG ??= " \
> + ${@bb.utils.filter('DISTRO_FEATURES', 'largefile', d)} \
The largefile distro feature is for historical reference only. Recipes
should always enable largefile support regardless of the distro
feature (which will eventually be removed).
> + backtrace \
> + programs \
> + shared \
> + convert \
> + python \
> + crypto-builtin \
This is not conventional formatting for a list of PACKAGECONFIG
options. Maybe have a look at some other recipes in oe-core to get a
feel for the typical style.
> +"
> +PACKAGECONFIG[largefile] = "--enable-largefile,--disable-largefile"
> +PACKAGECONFIG[backtrace] = "--enable-backtrace,--disable-backtrace"
> PACKAGECONFIG[manpages] = "--enable-documentation, --disable-documentation, asciidoc-native xmlto-native"
> +PACKAGECONFIG[programs] = "--enable-programs,--disable-programs"
> +PACKAGECONFIG[shared] = "--enable-shared,--disable-shared"
> +PACKAGECONFIG[static] = "--enable-static,--disable-static"
The choice to build shared and/or static libs is not something which
is typically controlled by PACKAGECONFIG options.
> +PACKAGECONFIG[convert] = "--enable-convert --with-convert=ext2,--disable-convert --without-convert,e2fsprogs"
> PACKAGECONFIG[python] = "--enable-python,--disable-python,python3-setuptools-native"
> PACKAGECONFIG[zstd] = "--enable-zstd,--disable-zstd,zstd"
>
> +# Pick only one crypto provider
> +PACKAGECONFIG[crypto-builtin] = "--with-crypto=builtin"
> +PACKAGECONFIG[crypto-libgcrypt] = "--with-crypto=libgcrypt,,libgcrypt"
> +PACKAGECONFIG[crypto-libsodium] = "--with-crypto=libsodium,,libsodium"
> +PACKAGECONFIG[crypto-libkcapi] = "--with-crypto=libkcapi,,libkcapi"
> +
> inherit autotools-brokensep pkgconfig manpages
> inherit ${@bb.utils.contains('PACKAGECONFIG', 'python', 'distutils3-base', '', d)}
>
> CLEANBROKEN = "1"
>
> -EXTRA_OECONF_append_libc-musl = " --disable-backtrace "
> +PACKAGECONFIG_remove_libc-musl = "backtrace"
Use of _remove in core recipes is discouraged.
> EXTRA_PYTHON_CFLAGS = "${DEBUG_PREFIX_MAP}"
> EXTRA_PYTHON_CFLAGS_class-native = ""
> EXTRA_PYTHON_LDFLAGS = "${LDFLAGS}"
> --
> 2.26.3
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [OE-core] [PATCH 2/3] btrfs-tools: Add PACKAGECONFIG options
2021-04-16 7:17 ` [OE-core] " Andre McCurdy
@ 2021-04-16 14:28 ` Robert Joslyn
2021-04-16 21:17 ` Andre McCurdy
0 siblings, 1 reply; 6+ messages in thread
From: Robert Joslyn @ 2021-04-16 14:28 UTC (permalink / raw)
To: Andre McCurdy; +Cc: OE Core mailing list
> On Apr 16, 2021, at 12:17 AM, Andre McCurdy <armccurdy@gmail.com> wrote:
>
> On Thu, Apr 15, 2021 at 10:38 PM Robert Joslyn
> <robert.joslyn@redrectangle.org> wrote:
>>
>> Add options to make it easier to control which features are enabled. All
>> of these default to enabled by upstream, so keep them enabled to
>> maintain previous behavior.
>>
>> The convert option also supports reiserfs, but no recipes exist in the
>> layer index. Limit the option to ext filesystems until someone cares
>> enough to make reiserfs recipes.
>>
>> Remove acl and attr from DEPENDS, as they do not apper to be needed. Add
>> zlib since it is required.
>>
>> Signed-off-by: Robert Joslyn <robert.joslyn@redrectangle.org>
>> ---
>> .../btrfs-tools/btrfs-tools_5.11.1.bb | 26 ++++++++++++++++---
>> 1 file changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb b/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
>> index 2ab476a88e..b875ea1aa1 100644
>> --- a/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
>> +++ b/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
>> @@ -13,7 +13,7 @@ LIC_FILES_CHKSUM = " \
>> file://libbtrfsutil/COPYING.LESSER;md5=3000208d539ec061b899bce1d9ce9404 \
>> "
>> SECTION = "base"
>> -DEPENDS = "util-linux attr e2fsprogs lzo acl"
>> +DEPENDS = "lzo util-linux zlib"
>> DEPENDS_append_class-target = " udev"
>> RDEPENDS_${PN} = "libgcc"
>>
>> @@ -22,17 +22,37 @@ SRC_URI = "git://git.kernel.org/pub/scm/linux/kernel/git/kdave/btrfs-progs.git \
>> file://0001-Add-a-possibility-to-specify-where-python-modules-ar.patch \
>> "
>>
>> -PACKAGECONFIG ??= "python"
>> +PACKAGECONFIG ??= " \
>> + ${@bb.utils.filter('DISTRO_FEATURES', 'largefile', d)} \
>
> The largefile distro feature is for historical reference only. Recipes
> should always enable largefile support regardless of the distro
> feature (which will eventually be removed).
Is it worth making it a PACKAGECONFIG at all, or is it preferred to just set —enable-largefile in EXTRA_OECONF? The upstream configure script does enable this by default so it’s not strictly needed at all either, but I usually prefer to be more explicit with configure options (as you can probably tell with this patch!).
>
>> + backtrace \
>> + programs \
>> + shared \
>> + convert \
>> + python \
>> + crypto-builtin \
>
> This is not conventional formatting for a list of PACKAGECONFIG
> options. Maybe have a look at some other recipes in oe-core to get a
> feel for the typical style.
I’m not sure what you mean. Would you prefer them on one line rather than split? In a specific order? There seems to be a lot of different styles in use, so I tried to follow the OE style guide, which suggests splitting lists like this, but maybe I’m misunderstanding.
>
>> +"
>> +PACKAGECONFIG[largefile] = "--enable-largefile,--disable-largefile"
>> +PACKAGECONFIG[backtrace] = "--enable-backtrace,--disable-backtrace"
>> PACKAGECONFIG[manpages] = "--enable-documentation, --disable-documentation, asciidoc-native xmlto-native"
>> +PACKAGECONFIG[programs] = "--enable-programs,--disable-programs"
>> +PACKAGECONFIG[shared] = "--enable-shared,--disable-shared"
>> +PACKAGECONFIG[static] = "--enable-static,--disable-static"
>
> The choice to build shared and/or static libs is not something which
> is typically controlled by PACKAGECONFIG options.
Some recipes do use PACKAGECONFIG, such as ffmpeg, but in grepping the repo it seems more common to use EXTRA_OECONF. Is there a reason not to use PACKAGECONFIG here? Just that building the static libraries is uncommon? I don’t mind changing it, just curious.
>
>> +PACKAGECONFIG[convert] = "--enable-convert --with-convert=ext2,--disable-convert --without-convert,e2fsprogs"
>> PACKAGECONFIG[python] = "--enable-python,--disable-python,python3-setuptools-native"
>> PACKAGECONFIG[zstd] = "--enable-zstd,--disable-zstd,zstd"
>>
>> +# Pick only one crypto provider
>> +PACKAGECONFIG[crypto-builtin] = "--with-crypto=builtin"
>> +PACKAGECONFIG[crypto-libgcrypt] = "--with-crypto=libgcrypt,,libgcrypt"
>> +PACKAGECONFIG[crypto-libsodium] = "--with-crypto=libsodium,,libsodium"
>> +PACKAGECONFIG[crypto-libkcapi] = "--with-crypto=libkcapi,,libkcapi"
>> +
>> inherit autotools-brokensep pkgconfig manpages
>> inherit ${@bb.utils.contains('PACKAGECONFIG', 'python', 'distutils3-base', '', d)}
>>
>> CLEANBROKEN = "1"
>>
>> -EXTRA_OECONF_append_libc-musl = " --disable-backtrace "
>> +PACKAGECONFIG_remove_libc-musl = "backtrace"
>
> Use of _remove in core recipes is discouraged.
I wasn’t sure the best way to handle this. I can put it back the way it was, where the configure script enables the feature by default and it’s disabled only for musl. I could keep the packagconfig and append only for glibc:
PACKAGECONFIG_append_libc-glibc = “ backtrace”
I could also disable the feature by default. I do disable this in my product layer, but I don’t know if that is a good default.
Thanks for the review!
Robert
>
>> EXTRA_PYTHON_CFLAGS = "${DEBUG_PREFIX_MAP}"
>> EXTRA_PYTHON_CFLAGS_class-native = ""
>> EXTRA_PYTHON_LDFLAGS = "${LDFLAGS}"
>> --
>> 2.26.3
>>
>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [OE-core] [PATCH 2/3] btrfs-tools: Add PACKAGECONFIG options
2021-04-16 14:28 ` Robert Joslyn
@ 2021-04-16 21:17 ` Andre McCurdy
0 siblings, 0 replies; 6+ messages in thread
From: Andre McCurdy @ 2021-04-16 21:17 UTC (permalink / raw)
To: Robert Joslyn; +Cc: OE Core mailing list
On Fri, Apr 16, 2021 at 7:28 AM Robert Joslyn
<robert.joslyn@redrectangle.org> wrote:
> > On Apr 16, 2021, at 12:17 AM, Andre McCurdy <armccurdy@gmail.com> wrote:
> > On Thu, Apr 15, 2021 at 10:38 PM Robert Joslyn
> > <robert.joslyn@redrectangle.org> wrote:
> >>
> >> Add options to make it easier to control which features are enabled. All
> >> of these default to enabled by upstream, so keep them enabled to
> >> maintain previous behavior.
> >>
> >> The convert option also supports reiserfs, but no recipes exist in the
> >> layer index. Limit the option to ext filesystems until someone cares
> >> enough to make reiserfs recipes.
> >>
> >> Remove acl and attr from DEPENDS, as they do not apper to be needed. Add
> >> zlib since it is required.
> >>
> >> Signed-off-by: Robert Joslyn <robert.joslyn@redrectangle.org>
> >> ---
> >> .../btrfs-tools/btrfs-tools_5.11.1.bb | 26 ++++++++++++++++---
> >> 1 file changed, 23 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb b/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
> >> index 2ab476a88e..b875ea1aa1 100644
> >> --- a/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
> >> +++ b/meta/recipes-devtools/btrfs-tools/btrfs-tools_5.11.1.bb
> >> @@ -13,7 +13,7 @@ LIC_FILES_CHKSUM = " \
> >> file://libbtrfsutil/COPYING.LESSER;md5=3000208d539ec061b899bce1d9ce9404 \
> >> "
> >> SECTION = "base"
> >> -DEPENDS = "util-linux attr e2fsprogs lzo acl"
> >> +DEPENDS = "lzo util-linux zlib"
> >> DEPENDS_append_class-target = " udev"
> >> RDEPENDS_${PN} = "libgcc"
> >>
> >> @@ -22,17 +22,37 @@ SRC_URI = "git://git.kernel.org/pub/scm/linux/kernel/git/kdave/btrfs-progs.git \
> >> file://0001-Add-a-possibility-to-specify-where-python-modules-ar.patch \
> >> "
> >>
> >> -PACKAGECONFIG ??= "python"
> >> +PACKAGECONFIG ??= " \
> >> + ${@bb.utils.filter('DISTRO_FEATURES', 'largefile', d)} \
> >
> > The largefile distro feature is for historical reference only. Recipes
> > should always enable largefile support regardless of the distro
> > feature (which will eventually be removed).
>
> Is it worth making it a PACKAGECONFIG at all, or is it preferred to just set —enable-largefile in EXTRA_OECONF? The upstream configure script does enable this by default so it’s not strictly needed at all either, but I usually prefer to be more explicit with configure options (as you can probably tell with this patch!).
Adding —enable-largefile to EXTRA_OECONF would be fine.
In general the goal of PACKAGECONFIG is not to expose every possible
configure option, but only the ones which we want to encourage people
to experiment with or use to control high level functionality. An
option to break support for large files doesn't fit into that
category.
> >> + backtrace \
> >> + programs \
> >> + shared \
> >> + convert \
> >> + python \
> >> + crypto-builtin \
> >
> > This is not conventional formatting for a list of PACKAGECONFIG
> > options. Maybe have a look at some other recipes in oe-core to get a
> > feel for the typical style.
>
> I’m not sure what you mean. Would you prefer them on one line rather than split? In a specific order? There seems to be a lot of different styles in use, so I tried to follow the OE style guide, which suggests splitting lists like this, but maybe I’m misunderstanding.
My preference doesn't matter much :-) I was suggesting that you look
in oe-core to get a feel for the typical style. Note that the OE style
guide mostly predates PACKAGECONFIG and hasn't been updated to cover
it in any detail.
> >> +"
> >> +PACKAGECONFIG[largefile] = "--enable-largefile,--disable-largefile"
> >> +PACKAGECONFIG[backtrace] = "--enable-backtrace,--disable-backtrace"
> >> PACKAGECONFIG[manpages] = "--enable-documentation, --disable-documentation, asciidoc-native xmlto-native"
> >> +PACKAGECONFIG[programs] = "--enable-programs,--disable-programs"
> >> +PACKAGECONFIG[shared] = "--enable-shared,--disable-shared"
> >> +PACKAGECONFIG[static] = "--enable-static,--disable-static"
> >
> > The choice to build shared and/or static libs is not something which
> > is typically controlled by PACKAGECONFIG options.
>
> Some recipes do use PACKAGECONFIG, such as ffmpeg, but in grepping the repo it seems more common to use EXTRA_OECONF. Is there a reason not to use PACKAGECONFIG here? Just that building the static libraries is uncommon? I don’t mind changing it, just curious.
See the comment above about the goal of PACKAGECONFIG not being to
expose every possible option. However, there are no well defined rules
about it (and in general in OE, what rules there are are not
consistently enforced) so patches like the one to ffmpeg which
converted a lot of inappropriate configure options to PACKAGECONFIG
options do sometimes get merged.
Specifically for shared and static libs, shared libs are generally
enabled by default but if a particular component didn't do that then
the normal approach would be to add a configure option to enable them
to EXTRA_OECONF. Static libs are generally unused but harmless, so
don't justify a PACKAGECONFIG option to control them. The only real
downside of enabling static libs is the wasted build time. If that
concerns you then you can try including
conf/distro/include/no-static-libs.inc in your distro config file.
> >> +PACKAGECONFIG[convert] = "--enable-convert --with-convert=ext2,--disable-convert --without-convert,e2fsprogs"
> >> PACKAGECONFIG[python] = "--enable-python,--disable-python,python3-setuptools-native"
> >> PACKAGECONFIG[zstd] = "--enable-zstd,--disable-zstd,zstd"
> >>
> >> +# Pick only one crypto provider
> >> +PACKAGECONFIG[crypto-builtin] = "--with-crypto=builtin"
> >> +PACKAGECONFIG[crypto-libgcrypt] = "--with-crypto=libgcrypt,,libgcrypt"
> >> +PACKAGECONFIG[crypto-libsodium] = "--with-crypto=libsodium,,libsodium"
> >> +PACKAGECONFIG[crypto-libkcapi] = "--with-crypto=libkcapi,,libkcapi"
> >> +
> >> inherit autotools-brokensep pkgconfig manpages
> >> inherit ${@bb.utils.contains('PACKAGECONFIG', 'python', 'distutils3-base', '', d)}
> >>
> >> CLEANBROKEN = "1"
> >>
> >> -EXTRA_OECONF_append_libc-musl = " --disable-backtrace "
> >> +PACKAGECONFIG_remove_libc-musl = "backtrace"
> >
> > Use of _remove in core recipes is discouraged.
>
> I wasn’t sure the best way to handle this. I can put it back the way it was, where the configure script enables the feature by default and it’s disabled only for musl. I could keep the packagconfig and append only for glibc:
> PACKAGECONFIG_append_libc-glibc = “ backtrace”
> I could also disable the feature by default. I do disable this in my product layer, but I don’t know if that is a good default.
I don't think backtrace justifies a PACKAGECONFIG option, especially
if the default value for the option is problematic (as it seems to
be). The original approach of appending --disable-backtrace to
EXTRA_OECONF for musl builds looks fine.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-16 21:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-16 5:37 [PATCH 1/3] btrfs-tools: Update to 5.11.1 Robert Joslyn
2021-04-16 5:37 ` [PATCH 2/3] btrfs-tools: Add PACKAGECONFIG options Robert Joslyn
2021-04-16 7:17 ` [OE-core] " Andre McCurdy
2021-04-16 14:28 ` Robert Joslyn
2021-04-16 21:17 ` Andre McCurdy
2021-04-16 5:37 ` [PATCH 3/3] btrfs-tools: Try to follow style guide Robert Joslyn
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.