Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 1/6] package/alure: new package
Date: Sun, 12 Jan 2020 21:31:31 +0100	[thread overview]
Message-ID: <20200112213131.6154c8f9@windsurf> (raw)
In-Reply-To: <20200105113204.30356-1-romain.naour@gmail.com>

Hello,

On Sun,  5 Jan 2020 12:31:59 +0100
Romain Naour <romain.naour@gmail.com> wrote:

> Use the latest commit since there is no release since 1.2 (8 years ago).
> 
> While testing with test-pkg, the last build issue was
> due to the gcc 4.8.3 compiler missing C++11 feature:
> "std::list.erase(const_iterator pos) not implemented" [1]
> 
> arm-none-linux-gnueabi/include/c++/4.8.3/bits/vector.tcc:134:5: note:
> no known conversion for argument 1 from '__gnu_cxx::__normal_iterator<const alure::Source*, std::vector<alure::Source> >'
> to 'std::vector<alure::Source>::iterator {aka __gnu_cxx::__normal_iterator<alure::Source*, std::vector<alure::Source> >}
> 
> https://github.com/kcat/alure/blob/14beed2a86d5a36030e907b21c46614d505f07cd/src/context.cpp#L1357
> 
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57158
> 
> Signed-off-by: Romain Naour <romain.naour@gmail.com>

I've applied the entire series, but with a few changes. I'll comment
only on the patches where changes were needed, including this one.

> ---
>  package/Config.in        |  1 +
>  package/alure/Config.in  | 27 +++++++++++++++++++++++++++
>  package/alure/alure.hash |  2 ++
>  package/alure/alure.mk   | 34 ++++++++++++++++++++++++++++++++++
>  4 files changed, 64 insertions(+)

Missing entry in the DEVELOPERS file.


> diff --git a/package/alure/alure.hash b/package/alure/alure.hash
> new file mode 100644
> index 0000000000..3387f76f9d
> --- /dev/null
> +++ b/package/alure/alure.hash
> @@ -0,0 +1,2 @@
> +# Locally computed
> +sha256 9b808e5b5a54d156348221dc86704c7831115047d5238691e6088f8c4814b2ca  alure-14beed2a86d5a36030e907b21c46614d505f07cd.tar.gz

Hash of the license file is missing... and I forgot to add it as well,
I'm only seeing this now that I review the patch once again. A
follow-up patch to add it would be good.

> +ALURE_VERSION = 14beed2a86d5a36030e907b21c46614d505f07cd
> +ALURE_SITE = $(call github,kcat,alure,$(ALURE_VERSION))
> +ALURE_LICENSE = MIT, Public Domain (src/decoders/dr_flac.h)

The license is not MIT, but Zlib. Make sure to compare the license text
when determining the license type.

> +ifeq ($(BR2_STATIC_LIBS),y)
> +ALURE_CONF_OPTS += -DALURE_BUILD_SHARED=OFF \
> +	-DALURE_BUILD_STATIC=ON
> +else
> +ALURE_CONF_OPTS += -DALURE_BUILD_SHARED=ON \
> +	-DALURE_BUILD_STATIC=OFF
> +endif

You forgot to handle the BR2_STATIC_SHARED_LIBS=y case, where both the
static and the shared libraries must be built.

That being said, the static library support of alure is quite broken:
it installs the library as libalure_s.a, i.e with a different name than
the shared library. This means that any application/library using alure
must now whether it should use -lalure or -lalure_s.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

      parent reply	other threads:[~2020-01-12 20:31 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-05 11:31 [Buildroot] [PATCH 1/6] package/alure: new package Romain Naour
2020-01-05 11:32 ` [Buildroot] [PATCH 2/6] package/alure: add flac decoder option Romain Naour
2020-01-05 11:32 ` [Buildroot] [PATCH 3/6] package/alure: add opus " Romain Naour
2020-01-05 11:32 ` [Buildroot] [PATCH 4/6] package/alure: add libvorbis " Romain Naour
2020-01-12 20:32   ` Thomas Petazzoni
2020-01-05 11:32 ` [Buildroot] [PATCH 5/6] package/alure: add libsndfile " Romain Naour
2020-01-05 11:32 ` [Buildroot] [PATCH 6/6] package/alure: add minimp3 " Romain Naour
2020-01-12 20:31 ` Thomas Petazzoni [this message]

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=20200112213131.6154c8f9@windsurf \
    --to=thomas.petazzoni@bootlin.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