All of 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 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.