Skip to contentSkip to conversation

upmpdcli: new package

PATCH New Joerg Krause 3 participants 6 messages Jul 14, 2014 – Jul 17, 2014

lore
Share & tools
lore

Joerg Krause

author
#0 lore raw

PATCH 1/1 upmpdcli: new package

This patch adds support for the package upmpdcli.

Signed-off-by Joerg Krause <[email protected]>
---
 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
package/upmpdcli/Config.in new file +16 -0
new file mode 100644
index 0000000..d878bfc
@@ -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
package/upmpdcli/S99upmpdcli new file +40 -0
new file mode 100644
index 0000000..314ea32
@@ -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
package/upmpdcli/upmpdcli.mk new file +22 -0
new file mode 100644
index 0000000..fa03fd4
@@ -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))
Signature · 1 line
2.0.1
#1 lore raw

PATCH 1/1 upmpdcli: new package

Joerg, All,

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

On 2014-07-14 22:52 +0200, Joerg Krause spake thusly:

Quoted 1 line

This patch adds support for the package upmpdcli.

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

[--SNIP--]

package/upmpdcli/Config.in line 6

Quoted 12 lines
package/upmpdcli/Config.in new file +6 -0
new file mode 100644
index 0000000..d878bfc
@@ -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

package/upmpdcli/S99upmpdcli line 5

Quoted 12 lines
package/upmpdcli/Config.in new file +1 -0
+	depends on !BR2_INSTALL_LIBSTDCPP
package/upmpdcli/S99upmpdcli new file +5 -0
new file mode 100644
index 0000000..314ea32
@@ -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--]

Quoted 11 lines
package/upmpdcli/upmpdcli.mk new file +5 -0
new file mode 100644
index 0000000..fa03fd4
@@ -0,0 +1,22 @@
+################################################################################
+#
+# upmpdcli
+#
+################################################################################

package/upmpdcli/upmpdcli.mk line 15

Quoted 10 lines
package/upmpdcli/upmpdcli.mk new file +10 -0
+
+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

package/upmpdcli/upmpdcli.mk line 22

Quoted 7 lines
package/upmpdcli/upmpdcli.mk new file +7 -0
+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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#2 lore raw

PATCH 1/1 upmpdcli: new package

Dear Yann E. MORIN,

Quoted 6 lines · Yann E. MORIN

package/upmpdcli/upmpdcli.mk line 15

Quoted 3 lines
package/upmpdcli/upmpdcli.mk new file +3 -0
+# 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

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#3 lore raw

PATCH 1/1 upmpdcli: new package

Thomas, Joerg, All,

On 2014-07-16 20:35 +0200, Thomas Petazzoni spake thusly:

Quoted 11 lines · Yann E. MORIN
Quoted 7 lines

package/upmpdcli/upmpdcli.mk line 15

Quoted 4 lines
package/upmpdcli/upmpdcli.mk new file +3 -0
+# 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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#4 lore raw

PATCH 1/1 upmpdcli: new package

In reply to Joerg Krause

Dear Joerg Krause,

Quoted 3 lines · Joerg Krause

This patch adds support for the package upmpdcli.

Signed-off-by Joerg Krause <[email protected]>

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

Quoted 2 lines
---
 package/upmpdcli/Config.in   | 16 ++++++++++++++++

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

Quoted 13 lines
 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
package/upmpdcli/Config.in new file +0 -0
new file mode 100644
index 0000000..d878bfc

package/upmpdcli/Config.in line 6

Quoted 6 lines
package/upmpdcli/Config.in new file +6 -0
@@ -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.

Quoted 6 lines
package/upmpdcli/upmpdcli.mk new file +0 -0
new file mode 100644
index 0000000..fa03fd4

package/upmpdcli/upmpdcli.mk line 9

Quoted 9 lines
package/upmpdcli/upmpdcli.mk new file +9 -0
@@ -0,0 +1,22 @@
+################################################################################
+#
+# upmpdcli
+#
+################################################################################
+
+UPMPDCLI_VERSION = 0.7.1
+UPMPDCLI_SITE = http://www.lesbonscomptes.com/upmpdcli/downloads
+UPMPDCLI_LICENSE = GPLv2

License was GPLv2+.

package/upmpdcli/upmpdcli.mk line 15

Quoted 6 lines
package/upmpdcli/upmpdcli.mk new file +6 -0
+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.

package/upmpdcli/upmpdcli.mk line 19

Quoted 4 lines
package/upmpdcli/upmpdcli.mk new file +4 -0
+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.

package/upmpdcli/upmpdcli.mk line 20

Quoted 1 line
package/upmpdcli/upmpdcli.mk new file +1 -0
+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

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
author
#5 lore raw

PATCH 1/1 upmpdcli: new package

Quoted 73 lines · Thomas Petazzoni

Dear Joerg Krause,

Quoted 3 lines · Joerg Krause

This patch adds support for the package upmpdcli.

Signed-off-by Joerg Krause <[email protected]>

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

Quoted 2 lines
---
  package/upmpdcli/Config.in   | 16 ++++++++++++++++

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

Quoted 13 lines
  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

package/upmpdcli/Config.in new file +0 -0
new file mode 100644
index 0000000..d878bfc

package/upmpdcli/Config.in line 6

Quoted 6 lines
package/upmpdcli/Config.in new file +6 -0
@@ -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.

Quoted 6 lines
package/upmpdcli/upmpdcli.mk new file +0 -0
new file mode 100644
index 0000000..fa03fd4

package/upmpdcli/upmpdcli.mk line 9

Quoted 9 lines
package/upmpdcli/upmpdcli.mk new file +9 -0
@@ -0,0 +1,22 @@
+################################################################################
+#
+# upmpdcli
+#
+################################################################################
+
+UPMPDCLI_VERSION = 0.7.1
+UPMPDCLI_SITE = http://www.lesbonscomptes.com/upmpdcli/downloads
+UPMPDCLI_LICENSE = GPLv2

License was GPLv2+.

package/upmpdcli/upmpdcli.mk line 15

Quoted 6 lines
package/upmpdcli/upmpdcli.mk new file +6 -0
+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.

package/upmpdcli/upmpdcli.mk line 19

Quoted 4 lines
package/upmpdcli/upmpdcli.mk new file +4 -0
+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.

package/upmpdcli/upmpdcli.mk line 20

Quoted 1 line
package/upmpdcli/upmpdcli.mk new file +1 -0
+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.

Quoted 6 lines

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