diff mbox

[v5] remmina: new package

Message ID 1451750166-10049-1-git-send-email-fancp2007@gmail.com
State Changes Requested
Headers show

Commit Message

Scott Fan Jan. 2, 2016, 3:56 p.m. UTC 
Remmina is a remote desktop client written in GTK+.
Currently RDP, VNC, NX, XDMCP and SSH are supported.

http://remmina.sourceforge.net/

Signed-off-by: Scott Fan <fancp2007@gmail.com>

---
Changes v4 -> v5:
1) rebase to master, select the latest 1.2.0 rcgit.7 release.
2) remove libssh dependency.

Changes v3 -> v4:
1) rebase to master
2) remove libssh patch which had merged to master

Changes v2 -> v3:
1) pick the first patch 'libssh' from the 'next' tree.
2) fix the dependencies of package remmina.

Changes v1 -> v2:
1) fix the dependencies of package remmina.
2) fix the license of package remmina.
3) wrap lines at 72 characters.

Signed-off-by: Scott Fan <fancp2007@gmail.com>
---
 package/Config.in          |  1 +
 package/remmina/Config.in  | 26 ++++++++++++++++++++++++++
 package/remmina/remmina.mk | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+)
 create mode 100644 package/remmina/Config.in
 create mode 100644 package/remmina/remmina.mk

Comments

Thomas Petazzoni May 31, 2016, 2:06 p.m. UTC | #1 
Hello Scott,

It's been a very long time since you posted this patch. It generally
looks good, but there's one thing that I don't really like.

On Sat,  2 Jan 2016 23:56:06 +0800, Scott Fan wrote:

> diff --git a/package/remmina/remmina.mk b/package/remmina/remmina.mk
> new file mode 100644
> index 0000000..8f333ae
> --- /dev/null
> +++ b/package/remmina/remmina.mk
> @@ -0,0 +1,36 @@
> +################################################################################
> +#
> +# remmina
> +#
> +################################################################################
> +
> +REMMINA_VERSION = e7f665f2634939aaa9bed114569cbd13eac10b82
> +REMMINA_SITE = $(call github,FreeRDP,Remmina,$(REMMINA_VERSION))

There has been new upstream releases since then, could you try to use a
more recent version? Also, we now add hash files for packages sourced
from Github, so it would be good to add one.

> +REMMINA_LICENSE = GPLv2+ with OpenSSL exception
> +REMMINA_LICENSE_FILES = COPYING LICENSE LICENSE.OpenSSL
> +
> +REMMINA_CONF_OPTS = \
> +	-DWITH_AVAHI=OFF \
> +	-DWITH_APPINDICATOR=OFF \
> +	-DWITH_GNOMEKEYRING=OFF \
> +	-DWITH_TELEPATHY=OFF \
> +	-DWITH_GCRYPT=OFF \
> +	-DWITH_LIBSSH=OFF \
> +	-DWITH_VTE=OFF
> +
> +REMMINA_DEPENDENCIES = \
> +	libgtk3 libvncserver freerdp \
> +	xlib_libX11 xlib_libXext xlib_libxkbfile
> +
> +ifeq ($(BR2_NEEDS_GETTEXT),y)
> +REMMINA_DEPENDENCIES += gettext
> +
> +define REMMINA_POST_PATCH_FIXINTL
> +	$(SED) 's/$${GTK_LIBRARIES}/$${GTK_LIBRARIES} -lintl/' \
> +		$(@D)/remmina/CMakeLists.txt
> +endef

The thing I dislike is this hook. Could you instead work on changing
the CMakeLists.txt so that it links with -lintl when needed? This way,
it would be a solution that we can contribute upstream to the Remmina
developers.

I've added in Cc: to this e-mail Samuel Martin, who is our CMake expert
in the Buildroot community. Hopefully he can help you achieve this.

Thanks!

Thomas
Scott Fan June 3, 2016, 1:05 a.m. UTC | #2 
Hello Thomas,

Thanks for your attention to this patch again.
I will follow up on it later.

Scott Fan

On Tue, May 31, 2016 at 10:06 PM, Thomas Petazzoni <
thomas.petazzoni@free-electrons.com> wrote:

> Hello Scott,
>
> It's been a very long time since you posted this patch. It generally
> looks good, but there's one thing that I don't really like.
>
> On Sat,  2 Jan 2016 23:56:06 +0800, Scott Fan wrote:
>
> > diff --git a/package/remmina/remmina.mk b/package/remmina/remmina.mk
> > new file mode 100644
> > index 0000000..8f333ae
> > --- /dev/null
> > +++ b/package/remmina/remmina.mk
> > @@ -0,0 +1,36 @@
> >
> +################################################################################
> > +#
> > +# remmina
> > +#
> >
> +################################################################################
> > +
> > +REMMINA_VERSION = e7f665f2634939aaa9bed114569cbd13eac10b82
> > +REMMINA_SITE = $(call github,FreeRDP,Remmina,$(REMMINA_VERSION))
>
> There has been new upstream releases since then, could you try to use a
> more recent version? Also, we now add hash files for packages sourced
> from Github, so it would be good to add one.
>
> > +REMMINA_LICENSE = GPLv2+ with OpenSSL exception
> > +REMMINA_LICENSE_FILES = COPYING LICENSE LICENSE.OpenSSL
> > +
> > +REMMINA_CONF_OPTS = \
> > +     -DWITH_AVAHI=OFF \
> > +     -DWITH_APPINDICATOR=OFF \
> > +     -DWITH_GNOMEKEYRING=OFF \
> > +     -DWITH_TELEPATHY=OFF \
> > +     -DWITH_GCRYPT=OFF \
> > +     -DWITH_LIBSSH=OFF \
> > +     -DWITH_VTE=OFF
> > +
> > +REMMINA_DEPENDENCIES = \
> > +     libgtk3 libvncserver freerdp \
> > +     xlib_libX11 xlib_libXext xlib_libxkbfile
> > +
> > +ifeq ($(BR2_NEEDS_GETTEXT),y)
> > +REMMINA_DEPENDENCIES += gettext
> > +
> > +define REMMINA_POST_PATCH_FIXINTL
> > +     $(SED) 's/$${GTK_LIBRARIES}/$${GTK_LIBRARIES} -lintl/' \
> > +             $(@D)/remmina/CMakeLists.txt
> > +endef
>
> The thing I dislike is this hook. Could you instead work on changing
> the CMakeLists.txt so that it links with -lintl when needed? This way,
> it would be a solution that we can contribute upstream to the Remmina
> developers.
>
> I've added in Cc: to this e-mail Samuel Martin, who is our CMake expert
> in the Buildroot community. Hopefully he can help you achieve this.
>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
>
Samuel Martin June 6, 2016, 9:38 p.m. UTC | #3 
Hi Scott, Thomas, all,

Sorry for the delayed answer.

On Fri, Jun 3, 2016 at 3:05 AM, Scott Fan <fancp2007@gmail.com> wrote:
> Hello Thomas,
>
> Thanks for your attention to this patch again.
> I will follow up on it later.
>
> Scott Fan
>
> On Tue, May 31, 2016 at 10:06 PM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
>>
>> Hello Scott,
>>
>> It's been a very long time since you posted this patch. It generally
>> looks good, but there's one thing that I don't really like.
>>
>> On Sat,  2 Jan 2016 23:56:06 +0800, Scott Fan wrote:
>>
>> > diff --git a/package/remmina/remmina.mk b/package/remmina/remmina.mk
>> > new file mode 100644
>> > index 0000000..8f333ae
>> > --- /dev/null
>> > +++ b/package/remmina/remmina.mk
>> > @@ -0,0 +1,36 @@
>> >
>> > +################################################################################
>> > +#
>> > +# remmina
>> > +#
>> >
>> > +################################################################################
>> > +
>> > +REMMINA_VERSION = e7f665f2634939aaa9bed114569cbd13eac10b82
>> > +REMMINA_SITE = $(call github,FreeRDP,Remmina,$(REMMINA_VERSION))
>>
>> There has been new upstream releases since then, could you try to use a
>> more recent version? Also, we now add hash files for packages sourced
>> from Github, so it would be good to add one.

Also, this particular revision seems not exist any more on this remote
(i cannot checkout it :-[)

>>
>> > +REMMINA_LICENSE = GPLv2+ with OpenSSL exception
>> > +REMMINA_LICENSE_FILES = COPYING LICENSE LICENSE.OpenSSL
>> > +
>> > +REMMINA_CONF_OPTS = \
>> > +     -DWITH_AVAHI=OFF \
>> > +     -DWITH_APPINDICATOR=OFF \
>> > +     -DWITH_GNOMEKEYRING=OFF \
>> > +     -DWITH_TELEPATHY=OFF \
>> > +     -DWITH_GCRYPT=OFF \
>> > +     -DWITH_LIBSSH=OFF \
>> > +     -DWITH_VTE=OFF
>> > +
>> > +REMMINA_DEPENDENCIES = \
>> > +     libgtk3 libvncserver freerdp \
>> > +     xlib_libX11 xlib_libXext xlib_libxkbfile
>> > +
>> > +ifeq ($(BR2_NEEDS_GETTEXT),y)
>> > +REMMINA_DEPENDENCIES += gettext
>> > +
>> > +define REMMINA_POST_PATCH_FIXINTL
>> > +     $(SED) 's/$${GTK_LIBRARIES}/$${GTK_LIBRARIES} -lintl/' \
>> > +             $(@D)/remmina/CMakeLists.txt
>> > +endef
>>
>> The thing I dislike is this hook. Could you instead work on changing
>> the CMakeLists.txt so that it links with -lintl when needed? This way,
>> it would be a solution that we can contribute upstream to the Remmina
>> developers.

There is certainly a few things that could be done in the CMakeLists.txt:
- cmake provides a FindIntl.cmake module;
- the FindGTK*.cmake modules from Remmina use pkg_check_module
function, so if the corresponding *.pc already are correct, there may
be nothing to do ;)
- etc

Scott, if you have a more up-to-date patch, and possibly a defconfig
which triggers some issues because of the missing -lintl flag, it
would be nice to share them.

Thanks,
Scott Fan June 9, 2016, 12:28 p.m. UTC | #4 
Hi Samuel, Thomas, all,

Scott Fan

On Tue, Jun 7, 2016 at 5:38 AM, Samuel Martin <s.martin49@gmail.com> wrote:

> Hi Scott, Thomas, all,
>
> Sorry for the delayed answer.
>
> On Fri, Jun 3, 2016 at 3:05 AM, Scott Fan <fancp2007@gmail.com> wrote:
> > Hello Thomas,
> >
> > Thanks for your attention to this patch again.
> > I will follow up on it later.
> >
> > Scott Fan
> >
> > On Tue, May 31, 2016 at 10:06 PM, Thomas Petazzoni
> > <thomas.petazzoni@free-electrons.com> wrote:
> >>
> >> Hello Scott,
> >>
> >> It's been a very long time since you posted this patch. It generally
> >> looks good, but there's one thing that I don't really like.
> >>
> >> On Sat,  2 Jan 2016 23:56:06 +0800, Scott Fan wrote:
> >>
> >> > diff --git a/package/remmina/remmina.mk b/package/remmina/remmina.mk
> >> > new file mode 100644
> >> > index 0000000..8f333ae
> >> > --- /dev/null
> >> > +++ b/package/remmina/remmina.mk
> >> > @@ -0,0 +1,36 @@
> >> >
> >> >
> +################################################################################
> >> > +#
> >> > +# remmina
> >> > +#
> >> >
> >> >
> +################################################################################
> >> > +
> >> > +REMMINA_VERSION = e7f665f2634939aaa9bed114569cbd13eac10b82
> >> > +REMMINA_SITE = $(call github,FreeRDP,Remmina,$(REMMINA_VERSION))
> >>
> >> There has been new upstream releases since then, could you try to use a
> >> more recent version? Also, we now add hash files for packages sourced
> >> from Github, so it would be good to add one.
>
> Also, this particular revision seems not exist any more on this remote
> (i cannot checkout it :-[)
>
> I've checked the commit e7f665f2634939aaa9bed114569cbd13eac10b82 again, it
even been the tag 1.2.0.rcgit.7, but it had been moved to another. Here is
the origin:
https://github.com/FreeRDP/Remmina/commit/e7f665f2634939aaa9bed114569cbd13eac10b82

I am trying to move the latest tag 1.2.0.rcgit.13

> >>
> >> > +REMMINA_LICENSE = GPLv2+ with OpenSSL exception
> >> > +REMMINA_LICENSE_FILES = COPYING LICENSE LICENSE.OpenSSL
> >> > +
> >> > +REMMINA_CONF_OPTS = \
> >> > +     -DWITH_AVAHI=OFF \
> >> > +     -DWITH_APPINDICATOR=OFF \
> >> > +     -DWITH_GNOMEKEYRING=OFF \
> >> > +     -DWITH_TELEPATHY=OFF \
> >> > +     -DWITH_GCRYPT=OFF \
> >> > +     -DWITH_LIBSSH=OFF \
> >> > +     -DWITH_VTE=OFF
> >> > +
> >> > +REMMINA_DEPENDENCIES = \
> >> > +     libgtk3 libvncserver freerdp \
> >> > +     xlib_libX11 xlib_libXext xlib_libxkbfile
> >> > +
> >> > +ifeq ($(BR2_NEEDS_GETTEXT),y)
> >> > +REMMINA_DEPENDENCIES += gettext
> >> > +
> >> > +define REMMINA_POST_PATCH_FIXINTL
> >> > +     $(SED) 's/$${GTK_LIBRARIES}/$${GTK_LIBRARIES} -lintl/' \
> >> > +             $(@D)/remmina/CMakeLists.txt
> >> > +endef
> >>
> >> The thing I dislike is this hook. Could you instead work on changing
> >> the CMakeLists.txt so that it links with -lintl when needed? This way,
> >> it would be a solution that we can contribute upstream to the Remmina
> >> developers.
>
> There is certainly a few things that could be done in the CMakeLists.txt:
> - cmake provides a FindIntl.cmake module;
> - the FindGTK*.cmake modules from Remmina use pkg_check_module
> function, so if the corresponding *.pc already are correct, there may
> be nothing to do ;)
> - etc
>
> Scott, if you have a more up-to-date patch, and possibly a defconfig
> which triggers some issues because of the missing -lintl flag, it
> would be nice to share them.
>
> Thanks,
>
>
> --
> Samuel
>
diff mbox

Patch

diff --git a/package/Config.in b/package/Config.in
index f58f2be..ce3f75c 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -256,6 +256,7 @@  endif
 	source "package/fbterm/Config.in"
 	source "package/fbv/Config.in"
 	source "package/freerdp/Config.in"
+	source "package/remmina/Config.in"
 	source "package/imagemagick/Config.in"
 	source "package/linux-fusion/Config.in"
 	source "package/lite/Config.in"
diff --git a/package/remmina/Config.in b/package/remmina/Config.in
new file mode 100644
index 0000000..78e9435
--- /dev/null
+++ b/package/remmina/Config.in
@@ -0,0 +1,26 @@ 
+config BR2_PACKAGE_REMMINA
+	bool "remmina"
+	depends on BR2_PACKAGE_XORG7
+	depends on BR2_PACKAGE_LIBGTK3
+	depends on BR2_USE_MMU # libvncserver depends on BR2_USE_MMU
+	depends on BR2_USE_WCHAR # freerdp
+	depends on !BR2_STATIC_LIBS # freerdp
+	depends on BR2_TOOLCHAIN_HAS_THREADS # freerdp
+	select BR2_PACKAGE_XLIB_LIBX11
+	select BR2_PACKAGE_XLIB_LIBXEXT
+	select BR2_PACKAGE_XLIB_LIBXKBFILE
+	select BR2_PACKAGE_LIBVNCSERVER
+	select BR2_PACKAGE_FREERDP
+	select BR2_PACKAGE_GETTEXT if BR2_NEEDS_GETTEXT
+	help
+	  Remmina is a remote desktop client written in GTK+, aiming to be
+	  useful for system administrators and travellers, who need to work
+	  with lots of remote computers in front of either large monitors
+	  or tiny netbooks.
+
+	  Remmina supports multiple network protocols in an integrated and
+	  consistent user interface.
+
+	  Currently RDP, VNC, NX, XDMCP and SSH are supported.
+
+	  http://remmina.sourceforge.net/
diff --git a/package/remmina/remmina.mk b/package/remmina/remmina.mk
new file mode 100644
index 0000000..8f333ae
--- /dev/null
+++ b/package/remmina/remmina.mk
@@ -0,0 +1,36 @@ 
+################################################################################
+#
+# remmina
+#
+################################################################################
+
+REMMINA_VERSION = e7f665f2634939aaa9bed114569cbd13eac10b82
+REMMINA_SITE = $(call github,FreeRDP,Remmina,$(REMMINA_VERSION))
+REMMINA_LICENSE = GPLv2+ with OpenSSL exception
+REMMINA_LICENSE_FILES = COPYING LICENSE LICENSE.OpenSSL
+
+REMMINA_CONF_OPTS = \
+	-DWITH_AVAHI=OFF \
+	-DWITH_APPINDICATOR=OFF \
+	-DWITH_GNOMEKEYRING=OFF \
+	-DWITH_TELEPATHY=OFF \
+	-DWITH_GCRYPT=OFF \
+	-DWITH_LIBSSH=OFF \
+	-DWITH_VTE=OFF
+
+REMMINA_DEPENDENCIES = \
+	libgtk3 libvncserver freerdp \
+	xlib_libX11 xlib_libXext xlib_libxkbfile
+
+ifeq ($(BR2_NEEDS_GETTEXT),y)
+REMMINA_DEPENDENCIES += gettext
+
+define REMMINA_POST_PATCH_FIXINTL
+	$(SED) 's/$${GTK_LIBRARIES}/$${GTK_LIBRARIES} -lintl/' \
+		$(@D)/remmina/CMakeLists.txt
+endef
+
+REMMINA_POST_PATCH_HOOKS += REMMINA_POST_PATCH_FIXINTL
+endif
+
+$(eval $(cmake-package))