Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH v5] espeak: new package
Date: Mon, 14 Oct 2013 19:40:50 +0200	[thread overview]
Message-ID: <20131014194050.6f0db457@skate> (raw)
In-Reply-To: <1381770929-13754-1-git-send-email-arnaud.aujon@gmail.com>

Dear Arnaud Aujon,

> +if BR2_PACKAGE_ESPEAK
> +choice
> +	prompt "choose audio backend"
> +	default BR2_PACKAGE_ESPEAK_AUDIO_BACKEND_NONE
> +
> +	config BR2_PACKAGE_ESPEAK_AUDIO_BACKEND_NONE
> +		bool "No sound backend, only produce wav files"
> +
> +	comment "ALSA backend requires a toolchain with threads support"
> +	depends on !BR2_TOOLCHAIN_HAS_THREADS
> +
> +	config BR2_PACKAGE_ESPEAK_AUDIO_BACKEND_ALSA
> +		bool "ALSA via Portaudio"
> +		select BR2_PACKAGE_PORTAUDIO
> +		select BR2_PACKAGE_PORTAUDIO_CXX
> +		depends on BR2_TOOLCHAIN_HAS_THREADS #portaudio
> +
> +	comment "Pulseaudio backend requires a toolchain with WCHAR, LARGEFILE and threads support"
> +	depends on !(BR2_TOOLCHAIN_HAS_THREADS && BR2_USE_WCHAR && BR2_LARGEFILE)
> +
> +	config BR2_PACKAGE_ESPEAK_AUDIO_BACKEND_PULSEAUDIO
> +		bool "pulseaudio"
> +		select BR2_PACKAGE_PULSEAUDIO
> +		depends on BR2_TOOLCHAIN_HAS_THREADS # pulseaudio
> +		depends on BR2_USE_WCHAR # pulseaudio
> +		depends on BR2_LARGEFILE # pulseaudio

Throughout the Buildroot tree, I don't think we have the habit of
indenting things inside a choice, so I would remove one tab from this
entire block. However, the "depends on" for the comments should be
intended.

> +
> +endchoice
> +endif #BR2_PACKAGE_ESPEAK

Nit pick: one space between # and BR2_...

> diff --git a/package/espeak/espeak-1-do-not-compil-when-install.patch b/package/espeak/espeak-1-do-not-compil-when-install.patch

Even we have agreed on the number of digits we should use, I believe 1
digit is really too short. We don't even notice it's the patch number.
So if you could replace that by espeak-01-....patch it would be great.

> +ESPEAK_VERSION = 1.47.11
> +ESPEAK_SOURCE = espeak-$(ESPEAK_VERSION)-source.zip
> +ESPEAK_SITE = http://downloads.sourceforge.net/project/espeak/espeak/espeak-1.47
> +ESPEAK_LICENSE = GPLv3

License is actually GPLv3+, as the source code states:

 *   This program is free software; you can redistribute it and/or modify  *
 *   it under the terms of the GNU General Public License as published by  *
 *   the Free Software Foundation; either version 3 of the License, or     *
 *   (at your option) any later version.                                   *


> +define ESPEAK_BUILD_CMDS
> +	$(MAKE) $(TARGET_CONFIGURE_OPTS)\
> +		AUDIO="$(ESPEAK_AUDIO_BACKEND)"\

One space before backslashes.

> +		-C $(@D)/src all
> +endef
> +
> +define ESPEAK_INSTALL_TARGET_CMDS
> +	$(MAKE) install DESTDIR="$(TARGET_DIR)" -C $(@D)/src
> +endef

Other than that, looks good to me. Thanks a lot for the quick
turnaround of new versions!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

  parent reply	other threads:[~2013-10-14 17:40 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-14 17:15 [Buildroot] [PATCH v5] espeak: new package Arnaud Aujon
2013-10-14 17:19 ` Ryan Barnett
2013-10-14 17:40 ` Thomas Petazzoni [this message]
2013-10-14 17:49   ` arnaud aujon
2013-10-14 17:58     ` Thomas Petazzoni
2013-10-14 18:07       ` arnaud aujon

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=20131014194050.6f0db457@skate \
    --to=thomas.petazzoni@free-electrons.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox