Patchwork New package: freerdp

login
register
mail settings
Submitter Julian Lunz
Date May 23, 2012, 9:45 p.m.
Message ID <1337809509-29526-1-git-send-email-git@jlunz.de>
Download mbox | patch
Permalink /patch/161038/
State Superseded
Headers show

Comments

Julian Lunz - May 23, 2012, 9:45 p.m.
Signed-off-by: Julian Lunz <git@jlunz.de>
---
 package/Config.in          |    1 +
 package/freerdp/Config.in  |   38 ++++++++++++++++++++++++++++++++++++++
 package/freerdp/freerdp.mk |   41 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+), 0 deletions(-)
 create mode 100644 package/freerdp/Config.in
 create mode 100644 package/freerdp/freerdp.mk
Julian Lunz - May 29, 2012, 6:34 p.m.
Hi,

Cared to fix, looking good?

On Wed, 23 May 2012 23:45:09 +0200
Julian Lunz <git@jlunz.de> wrote:

> Signed-off-by: Julian Lunz <git@jlunz.de>
> ---
>  package/Config.in          |    1 +
>  package/freerdp/Config.in  |   38
> ++++++++++++++++++++++++++++++++++++++ package/freerdp/freerdp.mk |
> 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 80
> insertions(+), 0 deletions(-) create mode 100644
> package/freerdp/Config.in create mode 100644
> package/freerdp/freerdp.mk
> 
> diff --git a/package/Config.in b/package/Config.in
> index fb1b08f..dc683aa 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -144,6 +144,7 @@ comment "X applications"
>  source "package/alsamixergui/Config.in"
>  source "package/docker/Config.in"
>  source "package/feh/Config.in"
> +source "package/freerdp/Config.in"
>  source "package/gqview/Config.in"
>  source "package/gmpc/Config.in"
>  source "package/gob2/Config.in"
> diff --git a/package/freerdp/Config.in b/package/freerdp/Config.in
> new file mode 100644
> index 0000000..b452141
> --- /dev/null
> +++ b/package/freerdp/Config.in
> @@ -0,0 +1,38 @@
> +menuconfig BR2_PACKAGE_FREERDP
> +	bool "freerdp"
> +	select BR2_PACKAGE_OPENSSL
> +	select BR2_PACKAGE_XLIB_LIBX11
> +	select BR2_PACKAGE_XLIB_LIBXT
> +	select BR2_PACKAGE_ZLIB
> +	depends on BR2_PACKAGE_XORG7
> +	help
> +	  FreeRDPFreeRDP is a free implementation of the 
> +	  Remote Desktop Protocol (RDP), released under the Apache
> license +
> +	  http://www.freerdp.com/
> +	  
> +
> +if BR2_PACKAGE_FREERDP
> +
> +config BR2_PACKAGE_FREERDP_WITHCUPS
> +	bool "CUPS support"
> +	help
> +	  Compile with CUPS support.
> +	  
> +config BR2_PACKAGE_FREERDP_WITHFFMPEG
> +	bool "FFmpeg support"
> +	help
> +	  Compile with FFmpeg support.
> +	  
> +config BR2_PACKAGE_FREERDP_WITHALSA
> +	bool "Alsa support"
> +	help
> +	  Compile with Alsa support.
> +	  
> +config BR2_PACKAGE_FREERDP_WITHXINERAMA
> +	bool "Xinerama support"
> +	help
> +	  Compile with Xinerama support.
> +
> +endif # BR2_PACKAGE_FREERDP
> +
> diff --git a/package/freerdp/freerdp.mk b/package/freerdp/freerdp.mk
> new file mode 100644
> index 0000000..b947628
> --- /dev/null
> +++ b/package/freerdp/freerdp.mk
> @@ -0,0 +1,41 @@
> +#############################################################
> +#
> +# FreeRDP
> +#
> +#############################################################
> +
> +FREERDP_VERSION = 1.0.1
> +FREERDP_SOURCE = freerdp-$(FREERDP_VERSION).tar.gz
> +FREERDP_SITE = https://github.com/downloads/FreeRDP/FreeRDP
> +
> +ifeq ($(BR2_PACKAGE_FREERDP_WITHCUPS),y)
> +FREERDP_CONF_OPT += -DWITH_CUPS=ON
> +FREERDP_DEPENDENCIES += cups
> +else
> +FREERDP_CONF_OPT += -DWITH_CUPS=OFF
> +endif
> +
> +ifeq ($(BR2_PACKAGE_FREERDP_WITHFFMPEG),y)
> +FREERDP_CONF_OPT += -DWITH_FFMPEG=ON
> +FREERDP_DEPENDENCIES += ffmpeg
> +else
> +FREERDP_CONF_OPT += -DWITH_FFMPEG=OFF
> +endif
> +
> +ifeq ($(BR2_PACKAGE_FREERDP_WITHALSA),y)
> +FREERDP_CONF_OPT += -DWITH_ALSA=ON
> +FREERDP_DEPENDENCIES += alsa-utils
> +else
> +FREERDP_CONF_OPT += -DWITH_ALSA=OFF
> +endif
> +
> +ifeq ($(BR2_PACKAGE_FREERDP_WITHXINERAMA),y)
> +FREERDP_CONF_OPT += -DWITH_XINERAMA=ON
> +FREERDP_DEPENDENCIES += xlib_libXinerama
> +else
> +FREERDP_CONF_OPT += -DWITH_XINERAMA=OFF
> +endif
> +
> +FREERDP_DEPENDENCIES += openssl xlib_libX11 xlib_libXt zlib
> +
> +$(eval $(call CMAKETARGETS))
Arnout Vandecappelle - May 29, 2012, 7:12 p.m.
On 05/23/12 23:45, Julian Lunz wrote:
[snip]
> +if BR2_PACKAGE_FREERDP
> +
> +config BR2_PACKAGE_FREERDP_WITHCUPS
> +    bool "CUPS support"
  You need to add 'select BR2_PACKAGE_CUPS'.

  But in fact, in this case I think it could be turned around: compile with
cups support if BR2_PACKAGE_CUPS is selected.  I.e., remove these
config options and in the .mk file:
[snip]
> +ifeq ($(BR2_PACKAGE_FREERDP_WITHCUPS),y)
  replace with
ifeq ($(BR2_PACKAGE_CUPS),y)


  On the other hand, for some of these (particularly alsa and Xinerama) it's
probably more user-friendly to have a config option like you have now.

> +FREERDP_CONF_OPT += -DWITH_CUPS=ON
> +FREERDP_DEPENDENCIES += cups
> +else
> +FREERDP_CONF_OPT += -DWITH_CUPS=OFF
> +endif
> +
> +ifeq ($(BR2_PACKAGE_FREERDP_WITHFFMPEG),y)
> +FREERDP_CONF_OPT += -DWITH_FFMPEG=ON
> +FREERDP_DEPENDENCIES += ffmpeg
> +else
> +FREERDP_CONF_OPT += -DWITH_FFMPEG=OFF
> +endif
> +
> +ifeq ($(BR2_PACKAGE_FREERDP_WITHALSA),y)
> +FREERDP_CONF_OPT += -DWITH_ALSA=ON
> +FREERDP_DEPENDENCIES += alsa-utils
  Shouldn't this be alsa-lib, rather than alsa-utils?

[snip]


  Regards,
  Arnout
Julian Lunz - May 30, 2012, 5:53 p.m.
Hi Arnout,

thanks for your input!


On Tue, 29 May 2012 21:12:29 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

> On 05/23/12 23:45, Julian Lunz wrote:
> [snip]
> > +if BR2_PACKAGE_FREERDP
> > +
> > +config BR2_PACKAGE_FREERDP_WITHCUPS
> > +    bool "CUPS support"
>   You need to add 'select BR2_PACKAGE_CUPS'.
Yep, missing for the others as well.

> 
>   But in fact, in this case I think it could be turned around:
> compile with cups support if BR2_PACKAGE_CUPS is selected.  I.e.,
> remove these config options and in the .mk file:
> [snip]
> > +ifeq ($(BR2_PACKAGE_FREERDP_WITHCUPS),y)
>   replace with
> ifeq ($(BR2_PACKAGE_CUPS),y)
> 
True, but I tend to have it stated more explicitly that these options
could but mustn't be used.
So it is more a question of how would I buildroot like to behave.
A lot of other packages use the mechanism you describe, so what about
merging both like that:

(make "logical or")

ifneq (,$(filter y, $(BR2_PACKAGE_CUPS) \
$(BR2_PACKAGE_FREERDP_WITHCUPS)))
...
endif

> 
>   On the other hand, for some of these (particularly alsa and
> Xinerama) it's probably more user-friendly to have a config option
> like you have now.
> 
> > +FREERDP_CONF_OPT += -DWITH_CUPS=ON
> > +FREERDP_DEPENDENCIES += cups
> > +else
> > +FREERDP_CONF_OPT += -DWITH_CUPS=OFF
> > +endif
> > +
> > +ifeq ($(BR2_PACKAGE_FREERDP_WITHFFMPEG),y)
> > +FREERDP_CONF_OPT += -DWITH_FFMPEG=ON
> > +FREERDP_DEPENDENCIES += ffmpeg
> > +else
> > +FREERDP_CONF_OPT += -DWITH_FFMPEG=OFF
> > +endif
> > +
> > +ifeq ($(BR2_PACKAGE_FREERDP_WITHALSA),y)
> > +FREERDP_CONF_OPT += -DWITH_ALSA=ON
> > +FREERDP_DEPENDENCIES += alsa-utils
>   Shouldn't this be alsa-lib, rather than alsa-utils?
Indeed, alsa-lib here.
> 
> [snip]
> 
> 
>   Regards,
>   Arnout
> 

Best regards,
Julian
Thomas Petazzoni - May 30, 2012, 7:04 p.m.
Le Wed, 30 May 2012 19:53:34 +0200,
Julian Lunz <git@jlunz.de> a écrit :

> True, but I tend to have it stated more explicitly that these options
> could but mustn't be used.
> So it is more a question of how would I buildroot like to behave.
> A lot of other packages use the mechanism you describe, so what about
> merging both like that:
> 
> (make "logical or")
> 
> ifneq (,$(filter y, $(BR2_PACKAGE_CUPS) \
> $(BR2_PACKAGE_FREERDP_WITHCUPS)))
> ...
> endif

No, please do like we do for all other packages in Buildroot: simply
use them if available, i.e:

ifeq ($(BR2_PACKAGE_CUPS),y)
... do whatever is needed in your package to use cups
yourpkg_DEPENDENCIES += cups
else
... do whatever is needed in your package to not use cups
endif

Best regards,

Thomas

Patch

diff --git a/package/Config.in b/package/Config.in
index fb1b08f..dc683aa 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -144,6 +144,7 @@  comment "X applications"
 source "package/alsamixergui/Config.in"
 source "package/docker/Config.in"
 source "package/feh/Config.in"
+source "package/freerdp/Config.in"
 source "package/gqview/Config.in"
 source "package/gmpc/Config.in"
 source "package/gob2/Config.in"
diff --git a/package/freerdp/Config.in b/package/freerdp/Config.in
new file mode 100644
index 0000000..b452141
--- /dev/null
+++ b/package/freerdp/Config.in
@@ -0,0 +1,38 @@ 
+menuconfig BR2_PACKAGE_FREERDP
+	bool "freerdp"
+	select BR2_PACKAGE_OPENSSL
+	select BR2_PACKAGE_XLIB_LIBX11
+	select BR2_PACKAGE_XLIB_LIBXT
+	select BR2_PACKAGE_ZLIB
+	depends on BR2_PACKAGE_XORG7
+	help
+	  FreeRDPFreeRDP is a free implementation of the 
+	  Remote Desktop Protocol (RDP), released under the Apache license
+
+	  http://www.freerdp.com/
+	  
+
+if BR2_PACKAGE_FREERDP
+
+config BR2_PACKAGE_FREERDP_WITHCUPS
+	bool "CUPS support"
+	help
+	  Compile with CUPS support.
+	  
+config BR2_PACKAGE_FREERDP_WITHFFMPEG
+	bool "FFmpeg support"
+	help
+	  Compile with FFmpeg support.
+	  
+config BR2_PACKAGE_FREERDP_WITHALSA
+	bool "Alsa support"
+	help
+	  Compile with Alsa support.
+	  
+config BR2_PACKAGE_FREERDP_WITHXINERAMA
+	bool "Xinerama support"
+	help
+	  Compile with Xinerama support.
+
+endif # BR2_PACKAGE_FREERDP
+
diff --git a/package/freerdp/freerdp.mk b/package/freerdp/freerdp.mk
new file mode 100644
index 0000000..b947628
--- /dev/null
+++ b/package/freerdp/freerdp.mk
@@ -0,0 +1,41 @@ 
+#############################################################
+#
+# FreeRDP
+#
+#############################################################
+
+FREERDP_VERSION = 1.0.1
+FREERDP_SOURCE = freerdp-$(FREERDP_VERSION).tar.gz
+FREERDP_SITE = https://github.com/downloads/FreeRDP/FreeRDP
+
+ifeq ($(BR2_PACKAGE_FREERDP_WITHCUPS),y)
+FREERDP_CONF_OPT += -DWITH_CUPS=ON
+FREERDP_DEPENDENCIES += cups
+else
+FREERDP_CONF_OPT += -DWITH_CUPS=OFF
+endif
+
+ifeq ($(BR2_PACKAGE_FREERDP_WITHFFMPEG),y)
+FREERDP_CONF_OPT += -DWITH_FFMPEG=ON
+FREERDP_DEPENDENCIES += ffmpeg
+else
+FREERDP_CONF_OPT += -DWITH_FFMPEG=OFF
+endif
+
+ifeq ($(BR2_PACKAGE_FREERDP_WITHALSA),y)
+FREERDP_CONF_OPT += -DWITH_ALSA=ON
+FREERDP_DEPENDENCIES += alsa-utils
+else
+FREERDP_CONF_OPT += -DWITH_ALSA=OFF
+endif
+
+ifeq ($(BR2_PACKAGE_FREERDP_WITHXINERAMA),y)
+FREERDP_CONF_OPT += -DWITH_XINERAMA=ON
+FREERDP_DEPENDENCIES += xlib_libXinerama
+else
+FREERDP_CONF_OPT += -DWITH_XINERAMA=OFF
+endif
+
+FREERDP_DEPENDENCIES += openssl xlib_libX11 xlib_libXt zlib
+
+$(eval $(call CMAKETARGETS))