[Buildroot] [PATCH] mpdstate: add package

[Buildroot] [PATCH] mpdstate: add package

[Kurt Van Dijck]

Mpdstate listens for MPD state changes and outputs them
on stdout. It is easy to write a script that receives mpdstate's output
on stdin an act upon some of the events.

Signed-off-by: Kurt Van Dijck <dev.kurt@vandijck-laurijssen.be>
---
 package/Config.in            |  1 +
 package/mpdstate/Config.in   | 11 +++++++++++
 package/mpdstate/mpdstate.mk | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+)
 create mode 100644 package/mpdstate/Config.in
 create mode 100644 package/mpdstate/mpdstate.mk

diff --git a/package/Config.in b/package/Config.in
index 112e516..629a7d0 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -26,6 +26,7 @@ menu "Audio and video applications"
 	source "package/modplugtools/Config.in"
 	source "package/mpd/Config.in"
 	source "package/mpd-mpc/Config.in"
+	source "package/mpdstate/Config.in"
 	source "package/mpg123/Config.in"
 	source "package/mplayer/Config.in"
 	source "package/musepack/Config.in"
diff --git a/package/mpdstate/Config.in b/package/mpdstate/Config.in
new file mode 100644
index 0000000..e3a5688
--- /dev/null
+++ b/package/mpdstate/Config.in
@@ -0,0 +1,11 @@
+config BR2_PACKAGE_MPDSTATE
+	bool "mpdstate"
+	help
+	  mpdstate is a small C-program which outputs the state of an MPD
+	  (Music Player Daemon). The program keeps waiting for changes and
+	  outputs immediately the state changes that I found important.
+
+	  The output is easily parsed with shell scripting,
+	  and that is how I automate my MPD boxes to:
+	  - enable a LED when MPD is playing
+	  - activate the power amplifier's power when MPD is playing
diff --git a/package/mpdstate/mpdstate.mk b/package/mpdstate/mpdstate.mk
new file mode 100644
index 0000000..82436d4
--- /dev/null
+++ b/package/mpdstate/mpdstate.mk
@@ -0,0 +1,35 @@
+################################################################################
+#
+# mpdstate
+#
+################################################################################
+
+MPDSTATE_VERSION = r3
+MPDSTATE_SITE = git://github.com/kurt-vd/mpdstate.git
+MPDSTATE_LICENSE = GPLv3
+MPDSTATE_LICENSE_FILES = LICENSE
+
+define MPDSTATE_CONFIGURE_CMDS
+	echo "PREFIX=/" > $(@D)/config.mk
+	echo "CFLAGS=$(TARGET_CFLAGS)" >> $(@D)/config.mk
+	echo "CPPFLAGS=$(TARGET_CPPFLAGS)" >> $(@D)/config.mk
+	echo "CXXFLAGS=$(TARGET_CXXFLAGS)" >> $(@D)/config.mk
+	echo "LDFLAGS=$(TARGET_LDFLAGS)" >> $(@D)/config.mk
+	echo "LDLIBS+=$(TARGET_LDLIBS)" >> $(@D)/config.mk
+	echo "CC=$(TARGET_CC)" >> $(@D)/config.mk
+	echo "CXX=$(TARGET_CXX)" >> $(@D)/config.mk
+	echo "LD=$(TARGET_LD)" >> $(@D)/config.mk
+	echo "AS=$(TARGET_AS)" >> $(@D)/config.mk
+
+	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) clean
+endef
+
+define MPDSTATE_BUILD_CMDS
+	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)
+endef
+
+define MPDSTATE_INSTALL_TARGET_CMDS
+	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) install DESTDIR="$(TARGET_DIR)"
+endef
+
+$(eval $(generic-package))
-- 
1.8.5.rc3

[Buildroot] [PATCH] mpdstate: add package

[Thomas Petazzoni]

Hello,

On Mon, 30 May 2016 08:36:56 +0200, Kurt Van Dijck wrote:
> Mpdstate listens for MPD state changes and outputs them
> on stdout. It is easy to write a script that receives mpdstate's output
> on stdin an act upon some of the events.
> 
> Signed-off-by: Kurt Van Dijck <dev.kurt@vandijck-laurijssen.be>

Isn't the same thing achievable with the "mpc idle" feature, which
allows you to listen for events from mpd ?

Like your other package, I'd prefer to have some evidence of even some
limited adoption/usage of a software package before adding it to
Buildroot.

Nonetheless, here is a review below.

> diff --git a/package/mpdstate/Config.in b/package/mpdstate/Config.in
> new file mode 100644
> index 0000000..e3a5688
> --- /dev/null
> +++ b/package/mpdstate/Config.in
> @@ -0,0 +1,11 @@
> +config BR2_PACKAGE_MPDSTATE
> +	bool "mpdstate"
> +	help
> +	  mpdstate is a small C-program which outputs the state of an MPD
> +	  (Music Player Daemon). The program keeps waiting for changes and
> +	  outputs immediately the state changes that I found important.

Please avoid first person sentences. This strengthen the feeling of
"this is a personal project and nobody else but the original author is
using it".

> +	  The output is easily parsed with shell scripting,
> +	  and that is how I automate my MPD boxes to:
> +	  - enable a LED when MPD is playing
> +	  - activate the power amplifier's power when MPD is playing

Please add an empty line here, followed by the URL of the upstream
project homepage.

> diff --git a/package/mpdstate/mpdstate.mk b/package/mpdstate/mpdstate.mk
> new file mode 100644
> index 0000000..82436d4
> --- /dev/null
> +++ b/package/mpdstate/mpdstate.mk
> @@ -0,0 +1,35 @@
> +################################################################################
> +#
> +# mpdstate
> +#
> +################################################################################
> +
> +MPDSTATE_VERSION = r3
> +MPDSTATE_SITE = git://github.com/kurt-vd/mpdstate.git

Please use the Github helper, and add a hash file.

> +MPDSTATE_LICENSE = GPLv3
> +MPDSTATE_LICENSE_FILES = LICENSE
> +
> +define MPDSTATE_CONFIGURE_CMDS
> +	echo "PREFIX=/" > $(@D)/config.mk

We normally use /usr as the prefix, not /.

> +	echo "CFLAGS=$(TARGET_CFLAGS)" >> $(@D)/config.mk
> +	echo "CPPFLAGS=$(TARGET_CPPFLAGS)" >> $(@D)/config.mk
> +	echo "CXXFLAGS=$(TARGET_CXXFLAGS)" >> $(@D)/config.mk
> +	echo "LDFLAGS=$(TARGET_LDFLAGS)" >> $(@D)/config.mk
> +	echo "LDLIBS+=$(TARGET_LDLIBS)" >> $(@D)/config.mk
> +	echo "CC=$(TARGET_CC)" >> $(@D)/config.mk
> +	echo "CXX=$(TARGET_CXX)" >> $(@D)/config.mk
> +	echo "LD=$(TARGET_LD)" >> $(@D)/config.mk
> +	echo "AS=$(TARGET_AS)" >> $(@D)/config.mk

You can pass all those variables on the make command line, and
therefore use $(TARGET_CONFIGURE_OPTS) instead.

Thanks!

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

[Buildroot] [PATCH] mpdstate: add package

[Kurt Van Dijck]

> Hello,
> 
> On Mon, 30 May 2016 08:36:56 +0200, Kurt Van Dijck wrote:
> > Mpdstate listens for MPD state changes and outputs them
> > on stdout. It is easy to write a script that receives mpdstate's output
> > on stdin an act upon some of the events.
> > 
> > Signed-off-by: Kurt Van Dijck <dev.kurt@vandijck-laurijssen.be>
> 
> Isn't the same thing achievable with the "mpc idle" feature, which
> allows you to listen for events from mpd ?

'mpc idleloop' prints the category of events, certainly not the real
changes. 

> 
> Like your other package, I'd prefer to have some evidence of even some
> limited adoption/usage of a software package before adding it to
> Buildroot.

Like my other package, mpc is ideal for interactive use, but not for
small automation stuff.
I created a raspberry pi box that engages the audio amplifier whenever
mpd plays, and standby when mpd pauses/stops.

It's hard to build stable scripts around 'mpc' for that regards,
as you need several calls, and the state may change in between.

mpdstate fixes that flow.

Like my other package, I may need to add that kind of comments, I
learned that, and you're right in pointing me to that.

I'm yet puzzled by the version info that I take from the git repository,
that does not work in buildroot since it's buildroot's git repo there.
I haven't found a solution yet, and that's keeping me from updating my
patches :-)

Thanks for the review, I'll take that into account.

Kurt

[Buildroot] [PATCH] mpdstate: add package

[Thomas Petazzoni]

Hello,

On Fri, 17 Jun 2016 10:36:00 +0200, Kurt Van Dijck wrote:

> I'm yet puzzled by the version info that I take from the git repository,
> that does not work in buildroot since it's buildroot's git repo there.
> I haven't found a solution yet, and that's keeping me from updating my
> patches :-)

Your package build system should not call "git describe" or call git to
get the version. Or at least it should provide a way to override that.

Buildroot extracts the source code of packages in
output/build/<name>-<version>/, and those directories do *not* contain
the Git metadata (i.e the .git/ directory is not present). Due to this,
when you run "git" in output/build/<name>-<version>/, it searches in
the parent directory, up to the point where it finds the .git/
directory of Buildroot.

Best regards,

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

[Buildroot] [PATCH] mpdstate: add package

[Ben Boeckel]

On Fri, 17 Jun, 2016 at 08:41:31 GMT, Thomas Petazzoni wrote:
> On Fri, 17 Jun 2016 10:36:00 +0200, Kurt Van Dijck wrote:
>> I'm yet puzzled by the version info that I take from the git repository,
>> that does not work in buildroot since it's buildroot's git repo there.
>> I haven't found a solution yet, and that's keeping me from updating my
>> patches :-)
>
> Your package build system should not call "git describe" or call git to
> get the version. Or at least it should provide a way to override that.

The way to do this properly is to add to .gitattributes:

    <filespec> export-subst

and in <filespec>, any strings in the form of $Format:$ will be
expanded, the right side being a format string (see git-log(1)). I
usually do logic like:

    if ("$Format:$" == "") {
        // git archive tarball
        commit_short = "$Format:%h"
        commit = "$Format:%H"
    } else if (isdir(".git"))
        // Run "git describe"
    } else {
        commit_short = "<unknown>"
        commit = "<unknown>"
    }

--Ben