diff mbox

[1/1] package/libarchive: add optional support for bzip2

Message ID 1455137950-18164-2-git-send-email-bernd.kuhls@t-online.de
State Superseded
Headers show

Commit Message

Bernd Kuhls Feb. 10, 2016, 8:59 p.m. UTC
When bzip2 was compiled before, libarchive will use it as optional
dependency:

$ output/host/usr/bin/i586-buildroot-linux-uclibc-readelf -a output/target/usr/lib/libarchive.so.13.1.2 | grep NEEDED
 0x00000001 (NEEDED)                     Shared library: [liblzma.so.5]
 0x00000001 (NEEDED)                     Shared library: [libbz2.so.1.0]
 0x00000001 (NEEDED)                     Shared library: [libz.so.1]
 0x00000001 (NEEDED)                     Shared library: [libc.so.1]

Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
---
 package/libarchive/libarchive.mk | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Arnout Vandecappelle Feb. 10, 2016, 9:19 p.m. UTC | #1
On 10-02-16 21:59, Bernd Kuhls wrote:
> When bzip2 was compiled before, libarchive will use it as optional
> dependency:
> 
> $ output/host/usr/bin/i586-buildroot-linux-uclibc-readelf -a output/target/usr/lib/libarchive.so.13.1.2 | grep NEEDED
>  0x00000001 (NEEDED)                     Shared library: [liblzma.so.5]
>  0x00000001 (NEEDED)                     Shared library: [libbz2.so.1.0]
>  0x00000001 (NEEDED)                     Shared library: [libz.so.1]
>  0x00000001 (NEEDED)                     Shared library: [libc.so.1]
> 
> Signed-off-by: Bernd Kuhls <bernd.kuhls@t-online.de>
> ---
>  package/libarchive/libarchive.mk | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/package/libarchive/libarchive.mk b/package/libarchive/libarchive.mk
> index 206de3f..f438d6e 100644
> --- a/package/libarchive/libarchive.mk
> +++ b/package/libarchive/libarchive.mk
> @@ -42,6 +42,12 @@ else
>  LIBARCHIVE_CONF_OPTS += --disable-xattr
>  endif
>  
> +ifeq ($(BR2_PACKAGE_BZIP2),y)
> +LIBARCHIVE_DEPENDENCIES += bzip2

 No --with-bz2lib? If there is a good reason, mention it in the commit log.

 Regards,
 Arnout

> +else
> +LIBARCHIVE_CONF_OPTS += --without-bz2lib
> +endif
> +
>  ifeq ($(BR2_PACKAGE_EXPAT),y)
>  LIBARCHIVE_DEPENDENCIES += expat
>  else
>
Thomas Petazzoni Feb. 16, 2016, 10:15 p.m. UTC | #2
Bernd,

On Wed, 10 Feb 2016 22:19:07 +0100, Arnout Vandecappelle wrote:

> > +ifeq ($(BR2_PACKAGE_BZIP2),y)
> > +LIBARCHIVE_DEPENDENCIES += bzip2
> 
>  No --with-bz2lib? If there is a good reason, mention it in the commit log.

Could you give some feedback on this?

Thanks!

Thomas
Bernd Kuhls Feb. 20, 2016, 1:49 p.m. UTC | #3
Am Wed, 10 Feb 2016 22:19:07 +0100 schrieb Arnout Vandecappelle:

>  No --with-bz2lib? If there is a good reason, mention it in the commit
>  log.

Hi Arnout,

libarchive always checks for bzip2 unless --without-bz2lib is used:
https://github.com/libarchive/libarchive/blob/master/configure.ac#L300

Therefore I see no need for --with-bz2lib and I also should not have 
added --with-lzma as well, I will send a patch removing it along with v2 
of this patch explaining why --with-bz2lib is not needed.

Regards, Bernd
Arnout Vandecappelle Feb. 20, 2016, 11:45 p.m. UTC | #4
On 02/20/16 14:49, Bernd Kuhls wrote:
> Am Wed, 10 Feb 2016 22:19:07 +0100 schrieb Arnout Vandecappelle:
> 
>>  No --with-bz2lib? If there is a good reason, mention it in the commit
>>  log.
> 
> Hi Arnout,
> 
> libarchive always checks for bzip2 unless --without-bz2lib is used:
> https://github.com/libarchive/libarchive/blob/master/configure.ac#L300
> 
> Therefore I see no need for --with-bz2lib and I also should not have 
> added --with-lzma as well, I will send a patch removing it along with v2 
> of this patch explaining why --with-bz2lib is not needed.

 We prefer to have both an explicit enable and disable (or with and without)
because:

- it makes it simpler in case the default changes after a version bump;

- it makes it easier for other people to understand that the right thing is done
(no need to check in configure what the default is);

- AFAIK the typical --enable/--with will give an error if the dependency is not
found, which gives an extra safety net;

- we do it like this in other places, it's nice to see the same pattern everywhere.


 Regards,
 Arnout
diff mbox

Patch

diff --git a/package/libarchive/libarchive.mk b/package/libarchive/libarchive.mk
index 206de3f..f438d6e 100644
--- a/package/libarchive/libarchive.mk
+++ b/package/libarchive/libarchive.mk
@@ -42,6 +42,12 @@  else
 LIBARCHIVE_CONF_OPTS += --disable-xattr
 endif
 
+ifeq ($(BR2_PACKAGE_BZIP2),y)
+LIBARCHIVE_DEPENDENCIES += bzip2
+else
+LIBARCHIVE_CONF_OPTS += --without-bz2lib
+endif
+
 ifeq ($(BR2_PACKAGE_EXPAT),y)
 LIBARCHIVE_DEPENDENCIES += expat
 else