All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] upmpdcli: new package
@ 2014-07-14 20:52 Joerg Krause
  2014-07-14 21:51 ` Yann E. MORIN
  2014-07-16 18:37 ` Thomas Petazzoni
  0 siblings, 2 replies; 6+ messages in thread
From: Joerg Krause @ 2014-07-14 20:52 UTC (permalink / raw)
  To: buildroot

This patch adds support for the package upmpdcli. 

Signed-off-by: Joerg Krause <jkrause@posteo.de>
---
 package/upmpdcli/Config.in   | 16 ++++++++++++++++
 package/upmpdcli/S99upmpdcli | 40 ++++++++++++++++++++++++++++++++++++++++
 package/upmpdcli/upmpdcli.mk | 22 ++++++++++++++++++++++
 3 files changed, 78 insertions(+)
 create mode 100644 package/upmpdcli/Config.in
 create mode 100644 package/upmpdcli/S99upmpdcli
 create mode 100644 package/upmpdcli/upmpdcli.mk

diff --git a/package/upmpdcli/Config.in b/package/upmpdcli/Config.in
new file mode 100644
index 0000000..d878bfc
--- /dev/null
+++ b/package/upmpdcli/Config.in
@@ -0,0 +1,16 @@
+config BR2_PACKAGE_UPMPDCLI
+	bool "upmpdcli"
+	select BR2_PACKAGE_EXPAT
+	select BR2_PACKAGE_LIBUPNP
+	select BR2_PACKAGE_LIBMPDCLIENT
+	depends on BR2_INSTALL_LIBSTDCPP
+	help
+
+	  upmpdcli is a UPnP Media Renderer front-end for MPD, the Music
+	  Player Daemon. It supports UPnP gapless track transitions and
+	  the OpenHome ohMedia services.
+
+	  http://www.lesbonscomptes.com/upmpdcli/
+
+comment "upmpdcli needs a toolchain w/ C++ support"
+	depends on !BR2_INSTALL_LIBSTDCPP
diff --git a/package/upmpdcli/S99upmpdcli b/package/upmpdcli/S99upmpdcli
new file mode 100644
index 0000000..314ea32
--- /dev/null
+++ b/package/upmpdcli/S99upmpdcli
@@ -0,0 +1,40 @@
+#!/bin/sh
+
+NAME=upmpdcli
+DAEMON=/usr/bin/$NAME
+CONFFILE=/etc/$NAME.conf
+PIDFILE=/var/run/$NAME.pid
+DAEMON_ARGS="-D -c $CONFFILE"
+
+# Sanity checks
+test -f $DAEMON || exit 0
+
+do_start() {
+        echo -n "Starting $NAME: "
+        start-stop-daemon --start --quiet --background --exec $DAEMON \
+                -- $DAEMON_ARGS \
+                && echo "OK" || echo "FAIL"
+}
+
+do_stop() {
+        echo -n "Stopping $NAME: "
+        start-stop-daemon --stop --quiet --pidfile $PIDFILE \
+                && echo "OK" || echo "FAIL"
+}
+
+case "$1" in
+        start)
+                do_start
+                ;;
+        stop)
+                do_stop
+                ;;
+        restart)
+                do_stop
+                sleep 1
+                do_start
+                ;;
+        *)
+                echo "Usage: $0 {start|stop|restart}"
+                exit 1
+esac
diff --git a/package/upmpdcli/upmpdcli.mk b/package/upmpdcli/upmpdcli.mk
new file mode 100644
index 0000000..fa03fd4
--- /dev/null
+++ b/package/upmpdcli/upmpdcli.mk
@@ -0,0 +1,22 @@
+################################################################################
+#
+# upmpdcli
+#
+################################################################################
+
+UPMPDCLI_VERSION = 0.7.1
+UPMPDCLI_SITE = http://www.lesbonscomptes.com/upmpdcli/downloads
+UPMPDCLI_LICENSE = GPLv2
+UPMPDCLI_LICENSE_FILES = COPYING
+UPMPDCLI_DEPENDENCIES = expat libupnp libmpdclient
+
+# Upmpdcli only runs if user upmpdcli exists
+define UPMPDCLI_USERS
+	upmpdcli -1 upmpdcli -1 * - - - Upmpdcli: MPD UPnP Renderer Front-End
+endef
+
+define UPMPDCLI_INSTALL_INIT_SYSV
+	$(INSTALL) -m 0755 package/upmpdcli/S99upmpdcli $(TARGET_DIR)/etc/init.d/
+endef
+
+$(eval $(autotools-package))
-- 
2.0.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] upmpdcli: new package
  2014-07-14 20:52 [Buildroot] [PATCH 1/1] upmpdcli: new package Joerg Krause
@ 2014-07-14 21:51 ` Yann E. MORIN
  2014-07-16 18:35   ` Thomas Petazzoni
  2014-07-16 18:37 ` Thomas Petazzoni
  1 sibling, 1 reply; 6+ messages in thread
From: Yann E. MORIN @ 2014-07-14 21:51 UTC (permalink / raw)
  To: buildroot

Joerg, All,

BTW, is it Joerg, or J?rg? Ah, those multiple personalities! ;-)

On 2014-07-14 22:52 +0200, Joerg Krause spake thusly:
> This patch adds support for the package upmpdcli. 

Thanks for this new package! :-)
A few comments below...

[--SNIP--]
> diff --git a/package/upmpdcli/Config.in b/package/upmpdcli/Config.in
> new file mode 100644
> index 0000000..d878bfc
> --- /dev/null
> +++ b/package/upmpdcli/Config.in
> @@ -0,0 +1,16 @@
> +config BR2_PACKAGE_UPMPDCLI
> +	bool "upmpdcli"
> +	select BR2_PACKAGE_EXPAT
> +	select BR2_PACKAGE_LIBUPNP
> +	select BR2_PACKAGE_LIBMPDCLIENT
> +	depends on BR2_INSTALL_LIBSTDCPP

libupnp depends on LARGEFILES and HAS_THREADS, so you need to duplicate
these dependencies here, too (the comment is the name of the package(s)
that need this dependency):

    depends on BR2_LARGEFILE # libupnp
    depends on BR2_TOOLCHAIN_HAS_THREADS # libupnp

> +	depends on !BR2_INSTALL_LIBSTDCPP
> diff --git a/package/upmpdcli/S99upmpdcli b/package/upmpdcli/S99upmpdcli
> new file mode 100644
> index 0000000..314ea32
> --- /dev/null
> +++ b/package/upmpdcli/S99upmpdcli
> @@ -0,0 +1,40 @@
> +#!/bin/sh
> +
> +NAME=upmpdcli
> +DAEMON=/usr/bin/$NAME
> +CONFFILE=/etc/$NAME.conf

Who's responsible for installing this upmpdcli.conf file?

[--SNIP--]
> diff --git a/package/upmpdcli/upmpdcli.mk b/package/upmpdcli/upmpdcli.mk
> new file mode 100644
> index 0000000..fa03fd4
> --- /dev/null
> +++ b/package/upmpdcli/upmpdcli.mk
> @@ -0,0 +1,22 @@
> +################################################################################
> +#
> +# upmpdcli
> +#
> +################################################################################
> +
> +UPMPDCLI_VERSION = 0.7.1
> +UPMPDCLI_SITE = http://www.lesbonscomptes.com/upmpdcli/downloads
> +UPMPDCLI_LICENSE = GPLv2
> +UPMPDCLI_LICENSE_FILES = COPYING
> +UPMPDCLI_DEPENDENCIES = expat libupnp libmpdclient
> +
> +# Upmpdcli only runs if user upmpdcli exists
> +define UPMPDCLI_USERS
> +	upmpdcli -1 upmpdcli -1 * - - - Upmpdcli: MPD UPnP Renderer Front-End

You're adding this user, but nowhere do you seem to use it to run the
upmpdcli daemon. What am I missing?

Also, as stated in the manual, no filed should contain a colon ':', not
even the comment field, see the manual:
    http://buildroot.org/downloads/manual/manual.html#makeuser-syntax

since the colon is used to separate the fields in the passwd file:
    https://en.wikipedia.org/wiki/Passwd
    man 5 passwd

> +endef
> +
> +define UPMPDCLI_INSTALL_INIT_SYSV
> +	$(INSTALL) -m 0755 package/upmpdcli/S99upmpdcli $(TARGET_DIR)/etc/init.d/
> +endef
> +
> +$(eval $(autotools-package))

Otherwise, looks good.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] upmpdcli: new package
  2014-07-14 21:51 ` Yann E. MORIN
@ 2014-07-16 18:35   ` Thomas Petazzoni
  2014-07-16 20:09     ` Yann E. MORIN
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Petazzoni @ 2014-07-16 18:35 UTC (permalink / raw)
  To: buildroot

Dear Yann E. MORIN,

On Mon, 14 Jul 2014 23:51:13 +0200, Yann E. MORIN wrote:

> > +# Upmpdcli only runs if user upmpdcli exists
> > +define UPMPDCLI_USERS
> > +	upmpdcli -1 upmpdcli -1 * - - - Upmpdcli: MPD UPnP Renderer Front-End
> 
> You're adding this user, but nowhere do you seem to use it to run the
> upmpdcli daemon. What am I missing?

It's automatically used by the upmpdcli daemon, which expects this user
to exist, as explained by the comment above.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] upmpdcli: new package
  2014-07-14 20:52 [Buildroot] [PATCH 1/1] upmpdcli: new package Joerg Krause
  2014-07-14 21:51 ` Yann E. MORIN
@ 2014-07-16 18:37 ` Thomas Petazzoni
  2014-07-17  6:20   ` Jörg Krause
  1 sibling, 1 reply; 6+ messages in thread
From: Thomas Petazzoni @ 2014-07-16 18:37 UTC (permalink / raw)
  To: buildroot

Dear Joerg Krause,

On Mon, 14 Jul 2014 22:52:47 +0200, Joerg Krause wrote:
> This patch adds support for the package upmpdcli. 
> 
> Signed-off-by: Joerg Krause <jkrause@posteo.de>

Thanks, I've applied your patch, but there were a number of issues.

> ---
>  package/upmpdcli/Config.in   | 16 ++++++++++++++++

This file was never included in package/Config.in, so there was no way
to enable the package.

>  package/upmpdcli/S99upmpdcli | 40 ++++++++++++++++++++++++++++++++++++++++
>  package/upmpdcli/upmpdcli.mk | 22 ++++++++++++++++++++++
>  3 files changed, 78 insertions(+)
>  create mode 100644 package/upmpdcli/Config.in
>  create mode 100644 package/upmpdcli/S99upmpdcli
>  create mode 100644 package/upmpdcli/upmpdcli.mk
> 
> diff --git a/package/upmpdcli/Config.in b/package/upmpdcli/Config.in
> new file mode 100644
> index 0000000..d878bfc
> --- /dev/null
> +++ b/package/upmpdcli/Config.in
> @@ -0,0 +1,16 @@
> +config BR2_PACKAGE_UPMPDCLI
> +	bool "upmpdcli"
> +	select BR2_PACKAGE_EXPAT
> +	select BR2_PACKAGE_LIBUPNP
> +	select BR2_PACKAGE_LIBMPDCLIENT
> +	depends on BR2_INSTALL_LIBSTDCPP

As Yann pointed out, missing dependencies inherited from libupnp.


> diff --git a/package/upmpdcli/upmpdcli.mk b/package/upmpdcli/upmpdcli.mk
> new file mode 100644
> index 0000000..fa03fd4
> --- /dev/null
> +++ b/package/upmpdcli/upmpdcli.mk
> @@ -0,0 +1,22 @@
> +################################################################################
> +#
> +# upmpdcli
> +#
> +################################################################################
> +
> +UPMPDCLI_VERSION = 0.7.1
> +UPMPDCLI_SITE = http://www.lesbonscomptes.com/upmpdcli/downloads
> +UPMPDCLI_LICENSE = GPLv2

License was GPLv2+.

> +UPMPDCLI_LICENSE_FILES = COPYING
> +UPMPDCLI_DEPENDENCIES = expat libupnp libmpdclient
> +
> +# Upmpdcli only runs if user upmpdcli exists
> +define UPMPDCLI_USERS
> +	upmpdcli -1 upmpdcli -1 * - - - Upmpdcli: MPD UPnP Renderer Front-End

I removed the colon from the description, as suggested by Yann.

> +endef
> +
> +define UPMPDCLI_INSTALL_INIT_SYSV
> +	$(INSTALL) -m 0755 package/upmpdcli/S99upmpdcli $(TARGET_DIR)/etc/init.d/

We normally use the -D option of $(INSTALL) and use a complete path as
the destination.

> +endef

I've also added the installation of a default configuration file (from
the upmpdcli sources). And the most annoying thing is that you
apparently never tested building upmpdcli with uClibc, and there were a
number of build failures that I had to fix.

Anyway, thanks a lot for your contribution! Looking forward to see more
patches from you, maybe to update or improve the upmpdcli package!

Best regards,

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] upmpdcli: new package
  2014-07-16 18:35   ` Thomas Petazzoni
@ 2014-07-16 20:09     ` Yann E. MORIN
  0 siblings, 0 replies; 6+ messages in thread
From: Yann E. MORIN @ 2014-07-16 20:09 UTC (permalink / raw)
  To: buildroot

Thomas, Joerg, All,

On 2014-07-16 20:35 +0200, Thomas Petazzoni spake thusly:
> On Mon, 14 Jul 2014 23:51:13 +0200, Yann E. MORIN wrote:
> 
> > > +# Upmpdcli only runs if user upmpdcli exists
> > > +define UPMPDCLI_USERS
> > > +	upmpdcli -1 upmpdcli -1 * - - - Upmpdcli: MPD UPnP Renderer Front-End
> > 
> > You're adding this user, but nowhere do you seem to use it to run the
> > upmpdcli daemon. What am I missing?
> 
> It's automatically used by the upmpdcli daemon, which expects this user
> to exist, as explained by the comment above.

/me rubs his eyes...
Doh... Indeed...
/me hides... ;-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] upmpdcli: new package
  2014-07-16 18:37 ` Thomas Petazzoni
@ 2014-07-17  6:20   ` Jörg Krause
  0 siblings, 0 replies; 6+ messages in thread
From: Jörg Krause @ 2014-07-17  6:20 UTC (permalink / raw)
  To: buildroot


On 07/16/2014 08:37 PM, Thomas Petazzoni wrote:
> Dear Joerg Krause,
>
> On Mon, 14 Jul 2014 22:52:47 +0200, Joerg Krause wrote:
>> This patch adds support for the package upmpdcli.
>>
>> Signed-off-by: Joerg Krause <jkrause@posteo.de>
> Thanks, I've applied your patch, but there were a number of issues.
>
>> ---
>>   package/upmpdcli/Config.in   | 16 ++++++++++++++++
> This file was never included in package/Config.in, so there was no way
> to enable the package.
>
>>   package/upmpdcli/S99upmpdcli | 40 ++++++++++++++++++++++++++++++++++++++++
>>   package/upmpdcli/upmpdcli.mk | 22 ++++++++++++++++++++++
>>   3 files changed, 78 insertions(+)
>>   create mode 100644 package/upmpdcli/Config.in
>>   create mode 100644 package/upmpdcli/S99upmpdcli
>>   create mode 100644 package/upmpdcli/upmpdcli.mk
>>
>> diff --git a/package/upmpdcli/Config.in b/package/upmpdcli/Config.in
>> new file mode 100644
>> index 0000000..d878bfc
>> --- /dev/null
>> +++ b/package/upmpdcli/Config.in
>> @@ -0,0 +1,16 @@
>> +config BR2_PACKAGE_UPMPDCLI
>> +	bool "upmpdcli"
>> +	select BR2_PACKAGE_EXPAT
>> +	select BR2_PACKAGE_LIBUPNP
>> +	select BR2_PACKAGE_LIBMPDCLIENT
>> +	depends on BR2_INSTALL_LIBSTDCPP
> As Yann pointed out, missing dependencies inherited from libupnp.
>
>
>> diff --git a/package/upmpdcli/upmpdcli.mk b/package/upmpdcli/upmpdcli.mk
>> new file mode 100644
>> index 0000000..fa03fd4
>> --- /dev/null
>> +++ b/package/upmpdcli/upmpdcli.mk
>> @@ -0,0 +1,22 @@
>> +################################################################################
>> +#
>> +# upmpdcli
>> +#
>> +################################################################################
>> +
>> +UPMPDCLI_VERSION = 0.7.1
>> +UPMPDCLI_SITE = http://www.lesbonscomptes.com/upmpdcli/downloads
>> +UPMPDCLI_LICENSE = GPLv2
> License was GPLv2+.
>
>> +UPMPDCLI_LICENSE_FILES = COPYING
>> +UPMPDCLI_DEPENDENCIES = expat libupnp libmpdclient
>> +
>> +# Upmpdcli only runs if user upmpdcli exists
>> +define UPMPDCLI_USERS
>> +	upmpdcli -1 upmpdcli -1 * - - - Upmpdcli: MPD UPnP Renderer Front-End
> I removed the colon from the description, as suggested by Yann.
>
>> +endef
>> +
>> +define UPMPDCLI_INSTALL_INIT_SYSV
>> +	$(INSTALL) -m 0755 package/upmpdcli/S99upmpdcli $(TARGET_DIR)/etc/init.d/
> We normally use the -D option of $(INSTALL) and use a complete path as
> the destination.
>
>> +endef
> I've also added the installation of a default configuration file (from
> the upmpdcli sources). And the most annoying thing is that you
> apparently never tested building upmpdcli with uClibc, and there were a
> number of build failures that I had to fix.
>
> Anyway, thanks a lot for your contribution! Looking forward to see more
> patches from you, maybe to update or improve the upmpdcli package!
>
> Best regards,
>
> Thomas

Thanks a lot for the comments, for fixing the remaining issues and for 
applying the patch! You are all doing a really good work with buildroot!

I will take care of the package. And sorry for not testing against 
uClibc. Still learning :-)

Best regards
J?rg Krause

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-17  6:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-14 20:52 [Buildroot] [PATCH 1/1] upmpdcli: new package Joerg Krause
2014-07-14 21:51 ` Yann E. MORIN
2014-07-16 18:35   ` Thomas Petazzoni
2014-07-16 20:09     ` Yann E. MORIN
2014-07-16 18:37 ` Thomas Petazzoni
2014-07-17  6:20   ` Jörg Krause

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.