All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] ympd: new package
@ 2014-09-23 19:53 Eric Limpens
  2014-09-23 20:02 ` Thomas Petazzoni
  2014-09-23 21:02 ` Samuel Martin
  0 siblings, 2 replies; 6+ messages in thread
From: Eric Limpens @ 2014-09-23 19:53 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Eric Limpens <limpens@gmail.com>
---
 package/Config.in                                       |  1 +
 package/ympd/Config.in                                  |  9 +++++++++
 package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch | 12 ++++++++++++
 package/ympd/ympd.mk                                    | 10 ++++++++++
 4 files changed, 32 insertions(+)
 create mode 100644 package/ympd/Config.in
 create mode 100644 package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch
 create mode 100644 package/ympd/ympd.mk

diff --git a/package/Config.in b/package/Config.in
index 2eefc3f..1578404 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -35,6 +35,7 @@ menu "Audio and video applications"
 	source "package/wavpack/Config.in"
 	source "package/xbmc/Config.in"
 	source "package/yavta/Config.in"
+	source "package/ympd/Config.in"
 endmenu
 
 menu "Compressors and decompressors"
diff --git a/package/ympd/Config.in b/package/ympd/Config.in
new file mode 100644
index 0000000..53f128e
--- /dev/null
+++ b/package/ympd/Config.in
@@ -0,0 +1,9 @@
+config BR2_PACKAGE_YMPD
+	bool "ympd"
+	select BR2_PACKAGE_LIBMPDCLIENT #libmpdclient
+	depends on BR2_TOOLCHAIN_HAS_THREADS
+	help
+	  ympd, Standalone MPD Web GUI written in C, utilizing Websockets and Bootstrap/JS
+
+comment "ympd needs a toolchain w/ threads"
+	depends on !(BR2_TOOLCHAIN_HAS_THREADS)
diff --git a/package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch b/package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch
new file mode 100644
index 0000000..55a29ee
--- /dev/null
+++ b/package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch
@@ -0,0 +1,12 @@
+--- ympd-a98d760dddff4b0dd595d977c9077f34beff05c5/CMakeLists.txt	2014-05-24 15:25:47.000000000 +0200
++++ ympd-a98d760dddff4b0dd595d977c9077f34beff05c5-1/CMakeLists.txt	2014-09-23 20:04:55.491997850 +0200
+@@ -29,6 +29,7 @@
+ )
+ 
+-add_executable(mkdata htdocs/mkdata.c)
+-get_target_property(MKDATA_EXE mkdata LOCATION)
++#add_executable(mkdata htdocs/mkdata.c)
++#get_target_property(MKDATA_EXE mkdata LOCATION)
++set(MKDATA_EXE "./mkdata")
+ 
+ add_custom_command(OUTPUT ${PROJECT_BINARY_DIR}/assets.c
diff --git a/package/ympd/ympd.mk b/package/ympd/ympd.mk
new file mode 100644
index 0000000..22727a9
--- /dev/null
+++ b/package/ympd/ympd.mk
@@ -0,0 +1,10 @@
+YMPD_VERSION = a98d760dddff4b0dd595d977c9077f34beff05c5
+YMPD_SITE = $(call github,notandy,ympd,$(YMPD_VERSION))
+
+define YMPD_MAKE_HOST_TOOL
+        $(CC) -O2 $(@D)/htdocs/mkdata.c -o $(@D)/mkdata
+endef
+
+YMPD_PRE_BUILD_HOOKS += YMPD_MAKE_HOST_TOOL
+
+$(eval $(cmake-package))
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] ympd: new package
  2014-09-23 19:53 Eric Limpens
@ 2014-09-23 20:02 ` Thomas Petazzoni
       [not found]   ` <CABGWzsQ5K=n4Cobck__oRkppK9Aq0uyV2Nn03tO1J2kYOqjqaQ@mail.gmail.com>
  2014-09-23 21:02 ` Samuel Martin
  1 sibling, 1 reply; 6+ messages in thread
From: Thomas Petazzoni @ 2014-09-23 20:02 UTC (permalink / raw)
  To: buildroot

Dear Eric Limpens,

Thanks for your contribution! Below a few comments. Could you take them
into account and resend an updated version? Thanks!

On Tue, 23 Sep 2014 21:53:32 +0200, Eric Limpens wrote:

> diff --git a/package/ympd/Config.in b/package/ympd/Config.in
> new file mode 100644
> index 0000000..53f128e
> --- /dev/null
> +++ b/package/ympd/Config.in
> @@ -0,0 +1,9 @@
> +config BR2_PACKAGE_YMPD
> +	bool "ympd"
> +	select BR2_PACKAGE_LIBMPDCLIENT #libmpdclient

Comment not needed.

> +	depends on BR2_TOOLCHAIN_HAS_THREADS
> +	help
> +	  ympd, Standalone MPD Web GUI written in C, utilizing Websockets and Bootstrap/JS

Help text should be wrapped to a short length, and maybe made more
explicit, like:

	  ympd is a standalone MPD Web GUI written in C, using
	  Websockets and Bootstrap/JS.

> diff --git a/package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch b/package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch
> new file mode 100644
> index 0000000..55a29ee
> --- /dev/null
> +++ b/package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch
> @@ -0,0 +1,12 @@

All patches must have a Signed-off-by line and a description. See
http://buildroot.org/downloads/manual/manual.html#_format_and_licensing_of_the_package_patches.

> +--- ympd-a98d760dddff4b0dd595d977c9077f34beff05c5/CMakeLists.txt	2014-05-24 15:25:47.000000000 +0200
> ++++ ympd-a98d760dddff4b0dd595d977c9077f34beff05c5-1/CMakeLists.txt	2014-09-23 20:04:55.491997850 +0200
> +@@ -29,6 +29,7 @@
> + )
> + 
> +-add_executable(mkdata htdocs/mkdata.c)
> +-get_target_property(MKDATA_EXE mkdata LOCATION)
> ++#add_executable(mkdata htdocs/mkdata.c)
> ++#get_target_property(MKDATA_EXE mkdata LOCATION)

We generally like to have a solution that can potentially be submitted
upstream.

> ++set(MKDATA_EXE "./mkdata")
> + 
> + add_custom_command(OUTPUT ${PROJECT_BINARY_DIR}/assets.c
> diff --git a/package/ympd/ympd.mk b/package/ympd/ympd.mk
> new file mode 100644
> index 0000000..22727a9
> --- /dev/null
> +++ b/package/ympd/ympd.mk
> @@ -0,0 +1,10 @@

Missing comment header like all other packages have.

> +YMPD_VERSION = a98d760dddff4b0dd595d977c9077f34beff05c5
> +YMPD_SITE = $(call github,notandy,ympd,$(YMPD_VERSION))
> +
> +define YMPD_MAKE_HOST_TOOL
> +        $(CC) -O2 $(@D)/htdocs/mkdata.c -o $(@D)/mkdata
> +endef

Use $(HOSTCC) $(HOST_CFLAGS) $(@D)/htdocs/mkdata.c -o $(@D)/mkdata

Thanks,

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] ympd: new package
       [not found]   ` <CABGWzsQ5K=n4Cobck__oRkppK9Aq0uyV2Nn03tO1J2kYOqjqaQ@mail.gmail.com>
@ 2014-09-23 20:46     ` Thomas Petazzoni
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2014-09-23 20:46 UTC (permalink / raw)
  To: buildroot

Dear Eric Limpens,

Please keep the Buildroot list in Cc.

On Tue, 23 Sep 2014 22:28:10 +0200, Eric Limpens wrote:

>  should I set the current patch to 'superseded' in patchwork? I'll send a
> new/revised version.

We usually set the state to "Superseded" once a new patch has been
sent. If we have requested some changes and we are fairly sure the
submitter is going to submit an updated version, then we usually set
the state to "Changes requested". But in fact, it doesn't make much
difference, as in both cases, the patch is no longer in the "New"
state, which means we no longer consider it as to be merged.

Thanks,

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] ympd: new package
  2014-09-23 19:53 Eric Limpens
  2014-09-23 20:02 ` Thomas Petazzoni
@ 2014-09-23 21:02 ` Samuel Martin
  1 sibling, 0 replies; 6+ messages in thread
From: Samuel Martin @ 2014-09-23 21:02 UTC (permalink / raw)
  To: buildroot

Hi Eric,

On Tue, Sep 23, 2014 at 9:53 PM, Eric Limpens <limpens@gmail.com> wrote:
> Signed-off-by: Eric Limpens <limpens@gmail.com>
> ---
>  package/Config.in                                       |  1 +
>  package/ympd/Config.in                                  |  9 +++++++++
>  package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch | 12 ++++++++++++
>  package/ympd/ympd.mk                                    | 10 ++++++++++
>  4 files changed, 32 insertions(+)
>  create mode 100644 package/ympd/Config.in
>  create mode 100644 package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch
>  create mode 100644 package/ympd/ympd.mk
>
> diff --git a/package/Config.in b/package/Config.in
> index 2eefc3f..1578404 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -35,6 +35,7 @@ menu "Audio and video applications"
>         source "package/wavpack/Config.in"
>         source "package/xbmc/Config.in"
>         source "package/yavta/Config.in"
> +       source "package/ympd/Config.in"
>  endmenu
>
>  menu "Compressors and decompressors"
> diff --git a/package/ympd/Config.in b/package/ympd/Config.in
> new file mode 100644
> index 0000000..53f128e
> --- /dev/null
> +++ b/package/ympd/Config.in
> @@ -0,0 +1,9 @@
> +config BR2_PACKAGE_YMPD
> +       bool "ympd"
> +       select BR2_PACKAGE_LIBMPDCLIENT #libmpdclient
> +       depends on BR2_TOOLCHAIN_HAS_THREADS
> +       help
> +         ympd, Standalone MPD Web GUI written in C, utilizing Websockets and Bootstrap/JS
> +
> +comment "ympd needs a toolchain w/ threads"
> +       depends on !(BR2_TOOLCHAIN_HAS_THREADS)
> diff --git a/package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch b/package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch
> new file mode 100644
> index 0000000..55a29ee
> --- /dev/null
> +++ b/package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch
> @@ -0,0 +1,12 @@
> +--- ympd-a98d760dddff4b0dd595d977c9077f34beff05c5/CMakeLists.txt       2014-05-24 15:25:47.000000000 +0200
> ++++ ympd-a98d760dddff4b0dd595d977c9077f34beff05c5-1/CMakeLists.txt     2014-09-23 20:04:55.491997850 +0200
> +@@ -29,6 +29,7 @@
> + )
> +
> +-add_executable(mkdata htdocs/mkdata.c)
> +-get_target_property(MKDATA_EXE mkdata LOCATION)
> ++#add_executable(mkdata htdocs/mkdata.c)
> ++#get_target_property(MKDATA_EXE mkdata LOCATION)
> ++set(MKDATA_EXE "./mkdata")

This usually won't work.
The tricky thing is that the tool comes with the project itself, and
is used during the build process of the project itself.

This kind of programs is usually not necessary on the target, but
useful to generate source code and/or data for the target.

Building a tool, then trying to using it in the same build is a common
mistake when the developers do not have cross-development in mind.
So, this is not an issue specific to CMake (or any orther build-system).

To workaround this, there is a couple of solutions, depending on
whether you can acheive the same job using standard programs (e.g.:
zip, dd ... whatever is needed/done) or not:
1) if the job can be done somehow else using standard programs, then
you can try doing the job using these standard tools. It may require
writing a script;
2) if the (host-)tool is simple enough, so that it can easily be built
for the host, then you can build it manually (i.e. by by-passing the
original build-system), before configuring the project for the target
build, and feed its path in the configure phase of the target build;
3) otherwise, you can build a the package for the host, so you'll get
the host tools installed in the Buildroot host directory, and feed its
path in the configure phase of the target build.

In case you choose option 3), you may want to limit the host-build of
the project to the needed host-tools. This may require patching the
build system to support that, and/or patching the build-system to use
a predefined tool when cross-compiling.


> +
> + add_custom_command(OUTPUT ${PROJECT_BINARY_DIR}/assets.c
> diff --git a/package/ympd/ympd.mk b/package/ympd/ympd.mk
> new file mode 100644
> index 0000000..22727a9
> --- /dev/null
> +++ b/package/ympd/ympd.mk
> @@ -0,0 +1,10 @@
> +YMPD_VERSION = a98d760dddff4b0dd595d977c9077f34beff05c5
> +YMPD_SITE = $(call github,notandy,ympd,$(YMPD_VERSION))
> +
> +define YMPD_MAKE_HOST_TOOL
> +        $(CC) -O2 $(@D)/htdocs/mkdata.c -o $(@D)/mkdata
> +endef
> +

Instead of adding a set(MKDATA_EXE "....") in the cmake code itself,
you can override it through the command line:

YMPD_CONF_OPT += -DMKDATA_EXE="$(@D)/mkdata"

(But you'll still need to disable the mkdata add_executable call).

> +YMPD_PRE_BUILD_HOOKS += YMPD_MAKE_HOST_TOOL
> +
> +$(eval $(cmake-package))
> --
> 1.9.1
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

Regards,

-- 
Samuel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] ympd: new package
@ 2014-09-28 12:04 Eric Limpens
  2014-10-04 12:42 ` Thomas Petazzoni
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Limpens @ 2014-09-28 12:04 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Eric Limpens <limpens@gmail.com>
---
 package/ympd/Config.in                              | 10 ++++++++++
 .../ympd/ympd-0001-CMake-cross-compile-mkdata.patch | 15 +++++++++++++++
 package/ympd/ympd.mk                                | 21 +++++++++++++++++++++
 3 files changed, 46 insertions(+)
 create mode 100644 package/ympd/Config.in
 create mode 100644 package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch
 create mode 100644 package/ympd/ympd.mk

diff --git a/package/ympd/Config.in b/package/ympd/Config.in
new file mode 100644
index 0000000..dadb897
--- /dev/null
+++ b/package/ympd/Config.in
@@ -0,0 +1,10 @@
+config BR2_PACKAGE_YMPD
+	bool "ympd"
+	select BR2_PACKAGE_LIBMPDCLIENT
+	depends on BR2_TOOLCHAIN_HAS_THREADS
+	help
+	  ympd, Standalone MPD Web GUI written in C,
+	  utilizing Websockets and Bootstrap/JS
+
+comment "ympd needs a toolchain w/ threads"
+	depends on !(BR2_TOOLCHAIN_HAS_THREADS)
diff --git a/package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch b/package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch
new file mode 100644
index 0000000..2ac27d5
--- /dev/null
+++ b/package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch
@@ -0,0 +1,15 @@
+CMakeLists.txt: make it cross-compile friendly. Upstream is looking into a proper solution. 
+
+Signed-Off-by: Eric Limpens <Limpens@Gmail.com>
+diff -U2 -r ympd-a98d760dddff4b0dd595d977c9077f34beff05c5~/CMakeLists.txt ympd-a98d760dddff4b0dd595d977c9077f34beff05c5/CMakeLists.txt
+--- ympd-a98d760dddff4b0dd595d977c9077f34beff05c5~/CMakeLists.txt	2014-05-24 15:25:47.000000000 +0200
++++ ympd-a98d760dddff4b0dd595d977c9077f34beff05c5/CMakeLists.txt	2014-09-27 12:51:48.958358400 +0200
+@@ -29,6 +29,6 @@
+ )
+ 
+-add_executable(mkdata htdocs/mkdata.c)
+-get_target_property(MKDATA_EXE mkdata LOCATION)
++#add_executable(mkdata htdocs/mkdata.c)
++#get_target_property(MKDATA_EXE mkdata LOCATION)
+ 
+ add_custom_command(OUTPUT ${PROJECT_BINARY_DIR}/assets.c
diff --git a/package/ympd/ympd.mk b/package/ympd/ympd.mk
new file mode 100644
index 0000000..fc4be4d
--- /dev/null
+++ b/package/ympd/ympd.mk
@@ -0,0 +1,21 @@
+################################################################################
+#
+# ympd
+#
+################################################################################
+
+YMPD_VERSION = a98d760dddff4b0dd595d977c9077f34beff05c5
+YMPD_SITE = $(call github,notandy,ympd,$(YMPD_VERSION))
+YMPD_LICENSE = GPLv2
+YMPD_LICENSE_FILE = LICENSE
+YMPD_DEPENDENCIES = libmpdclient
+
+define YMPD_MAKE_HOST_TOOL
+        $(HOSTCC) $(HOST_CFLAGS) $(@D)/htdocs/mkdata.c -o $(@D)/mkdata
+endef
+
+YMPD_PRE_BUILD_HOOKS += YMPD_MAKE_HOST_TOOL
+
+YMPD_CONF_OPT += -DMKDATA_EXE=$(@D)/mkdata
+
+$(eval $(cmake-package))
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH 1/1] ympd: new package
  2014-09-28 12:04 [Buildroot] [PATCH 1/1] ympd: new package Eric Limpens
@ 2014-10-04 12:42 ` Thomas Petazzoni
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2014-10-04 12:42 UTC (permalink / raw)
  To: buildroot

Dear Eric Limpens,

On Sun, 28 Sep 2014 14:04:02 +0200, Eric Limpens wrote:
> Signed-off-by: Eric Limpens <limpens@gmail.com>
> ---
>  package/ympd/Config.in                              | 10 ++++++++++
>  .../ympd/ympd-0001-CMake-cross-compile-mkdata.patch | 15 +++++++++++++++
>  package/ympd/ympd.mk                                | 21 +++++++++++++++++++++
>  3 files changed, 46 insertions(+)
>  create mode 100644 package/ympd/Config.in
>  create mode 100644 package/ympd/ympd-0001-CMake-cross-compile-mkdata.patch
>  create mode 100644 package/ympd/ympd.mk

I've applied your patch, after making a good number of changes:

    [Thomas:
     - Add the package to package/Config.in
     - Add dependency on largefile
     - Add new patch to CMakeLists.txt to remove the unused C++
    dependency
     - Change the existing CMakeLists.txt patch to simply remove the
       problematic code, and reword the explanation.]

Thanks,

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
end of thread, other threads:[~2014-10-04 12:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-28 12:04 [Buildroot] [PATCH 1/1] ympd: new package Eric Limpens
2014-10-04 12:42 ` Thomas Petazzoni
  -- strict thread matches above, loose matches on Subject: below --
2014-09-23 19:53 Eric Limpens
2014-09-23 20:02 ` Thomas Petazzoni
     [not found]   ` <CABGWzsQ5K=n4Cobck__oRkppK9Aq0uyV2Nn03tO1J2kYOqjqaQ@mail.gmail.com>
2014-09-23 20:46     ` Thomas Petazzoni
2014-09-23 21:02 ` Samuel Martin

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.