All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnout Vandecappelle <arnout@mind.be>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] axel: bump to version 2.14.1
Date: Sun, 24 Sep 2017 16:09:13 +0200	[thread overview]
Message-ID: <cf484d3b-ef26-1829-3a09-e98a4253cc16@mind.be> (raw)
In-Reply-To: <20170922044545.18813-1-ismael@iodev.co.uk>

 Hi Ismael,

On 22-09-17 06:45, Ismael Luceno wrote:
> Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>

 Please make a longer commit message that explains a bit what changes you make:
- change upstream URL;
- add optional support for OpenSSL/LibreSSL;
...

> ---
>  package/axel/Config.in | 14 +++++++++++++-
>  package/axel/axel.hash |  3 +--
>  package/axel/axel.mk   | 17 ++++++++---------
>  3 files changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/package/axel/Config.in b/package/axel/Config.in
> index 6e709cfb1351..c7dcbe71c179 100644
> --- a/package/axel/Config.in
> +++ b/package/axel/Config.in
> @@ -4,7 +4,19 @@ config BR2_PACKAGE_AXEL
>  	help
>  	  HTTP/FTP download accelerator.
>  
> -	  http://axel.alioth.debian.org/
> +	  https://github.com/axel-download-accelerator/

 Better refer to axel itself, that has a nice README.md:

https://github.com/axel-download-accelerator/axel

> +
> +if BR2_PACKAGE_AXEL
> +
> +config BR2_PACKAGE_AXEL_SSL
> +	bool "SSL/TLS support"
> +	default y
> +	depends on BR2_PACKAGE_OPENSSL || BR2_PACKAGE_LIBRESSL
> +
> +comment "SSL/TLS support requires openssl or libressl"
> +	depends on !(BR2_PACKAGE_OPENSSL || BR2_PACKAGE_LIBRESSL)

 Unless if there is a very good reason for it, we make this kind of dependency
automatic, i.e. don't add a Config.in option. Instead, add conditions to the .mk
file.

[snip]
>  
> -AXEL_VERSION = 2.4
> -AXEL_SITE = http://sources.buildroot.net
> +AXEL_VERSION = 2.14.1
> +AXEL_SITE = $(call github,axel-download-accelerator,axel,v$(AXEL_VERSION))

 If upstream has a v in their version, you should also have it. So

AXEL_VERSION = v2.14.1

 Note that as of today, the github helper no longer requires the third argument.

>  AXEL_LICENSE = GPL-2.0+
>  AXEL_LICENSE_FILES = COPYING
> -AXEL_DEPENDENCIES = $(TARGET_NLS_DEPENDENCIES)
> +AXEL_DEPENDENCIES = $(TARGET_NLS_DEPENDENCIES) \
> +	$(if $(BR2_PACKAGE_AXEL_SSL),\
> +		$(if $(BR2_PACKAGE_LIBRESSL),libressl,openssl))

 Remove this part. Instead, add something like this:

ifeq ($(BR2_PACKAGE_OPENSSL),y)
AXEL_CONF_OPTS += --with-ssl
AXEL_DEPENDENCIES += openssl
else ifeq ($(BR2_PACKAGE_LIBRESSL),y)
AXEL_CONF_OPTS += --with-ssl
AXEL_DEPENDENCIES += libressl
else
AXEL_CONF_OPTS += --without-ssl
endif

 It's more lines, but it is easier to understand and therefore easier to maintain.

>  AXEL_LDFLAGS = -lpthread $(TARGET_NLS_LIBS)
>  
> -ifeq ($(BR2_SYSTEM_ENABLE_NLS),)
> -AXEL_DISABLE_I18N = --i18n=0
> -endif
> -
>  define AXEL_CONFIGURE_CMDS
>  	(cd $(@D); \
> +		./autogen.sh; \
>  		./configure \
>  			--prefix=/usr \
> -			--debug=1 \
> -			$(AXEL_DISABLE_I18N) \
> +			$(if $(BR2_PACKAGE_AXEL_SSL),,--without-ssl) \
> +			$(if $(BR2_SYSTEM_ENABLE_NLS),,--disable-nls) \

 While you're at it, perhaps you can switch to the autotools infrastructure.
You'll have to set AXEL_AUTORECONF = YES because there is no configure script
included. But then the configure, build and install commands can just be removed.

 Regards,
 Arnout

>  	)
>  endef
>  
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

  reply	other threads:[~2017-09-24 14:09 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-22  4:45 [Buildroot] [PATCH] axel: bump to version 2.14.1 Ismael Luceno
2017-09-24 14:09 ` Arnout Vandecappelle [this message]
2017-09-25 14:27   ` Ismael Luceno
2017-09-25 16:44     ` Arnout Vandecappelle
2017-09-25 17:41       ` Ismael Luceno
2017-09-25 19:50         ` Arnout Vandecappelle
2017-09-25 14:36   ` [Buildroot] [PATCH v2] " Ismael Luceno
2017-09-25 15:17     ` Ismael Luceno

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cf484d3b-ef26-1829-3a09-e98a4253cc16@mind.be \
    --to=arnout@mind.be \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.