After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 749124 - vinagre fails to build with freerdp-1.2.1
vinagre fails to build with freerdp-1.2.1
Status: RESOLVED FIXED
Product: vinagre
Classification: Applications
Component: RDP
unspecified
Other Linux
: Normal normal
: ---
Assigned To: vinagre-maint
vinagre-maint
Depends on:
Blocks:
 
 
Reported: 2015-05-08 15:33 UTC by Nick Andrade
Modified: 2015-05-23 17:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
vinagre-rdp-1.2.1.patch (517 bytes, patch)
2015-05-08 15:33 UTC, Nick Andrade
none Details | Review
vinagre-rdp-1.2.1.patch (559 bytes, patch)
2015-05-12 03:22 UTC, Nick Andrade
none Details | Review
vinagre-3.16.0-rdp-1.2.1-def.patch (1002 bytes, patch)
2015-05-16 06:50 UTC, Nick Andrade
committed Details | Review

Description Nick Andrade 2015-05-08 15:33:07 UTC
Created attachment 303086 [details] [review]
vinagre-rdp-1.2.1.patch

Downstream bug: https://bugs.gentoo.org/show_bug.cgi?id=548708

When compiling vinagre-3.14.3 with rdp support and freerdp 1.2.1 installed, compilation fails as follows:
<<<<<
plugins/rdp/vinagre-rdp-tab.c: In function ‘open_freerdp’:
plugins/rdp/vinagre-rdp-tab.c:890:11: error: ‘rdpSettings’ has no member named ‘DisableEncryption’
   settings->DisableEncryption = FALSE;
           ^
Makefile:1660: recipe for target 'plugins/rdp/vinagre_vinagre-vinagre-rdp-tab.o' failed
make[2]: *** [plugins/rdp/vinagre_vinagre-vinagre-rdp-tab.o] Error 1
>>>>>

The same failure is still present on 3.16.0.

I believe the failure is due to deprecated options from freerdp commit 6424599639425212096340cd24c1bc35380ecc60 (https://github.com/FreeRDP/FreeRDP/commit/6424599639425212096340cd24c1bc35380ecc60).  

Likewise, removing the line in plugins/rdp/vinagre-rdp-tab.c in vinagre resolves the issue and shouldn't cause any breakage.
Comment 1 David King 2015-05-09 07:18:33 UTC
(In reply to Nick Andrade from comment #0)
>… shouldn't cause any breakage.

I haven't got time to check while travelling, but I will need a bit more justification than that! At least some ifdefs would seem reasonable.
Comment 2 Nick Andrade 2015-05-12 03:21:01 UTC
While I didn't encounter breakage, I found a better fix.  The setting was not deleted in freerdp, but rather it has been renamed: https://github.com/FreeRDP/FreeRDP/pull/2273

Attached is a patch to use the new name (patch is for 3.14.3, but it still applies to 3.16.0.
Comment 3 Nick Andrade 2015-05-12 03:22:40 UTC
Created attachment 303240 [details] [review]
vinagre-rdp-1.2.1.patch

Updated patch that switches "DisableEncryption" to "UseRdpSecurityLayer" (see previous comment for relevant upstream pull).
Comment 4 David King 2015-05-12 03:25:51 UTC
Looks much better, thanks. I will test and commit later this week.
Comment 5 David King 2015-05-15 06:47:17 UTC
Review of attachment 303240 [details] [review]:

Looking at the pull request, it is is a bit difficult to see what version the change was made at. Can you find out, and add the relevant ifdefs so that the RDP plugin still builds on older versions?
Comment 6 Nick Andrade 2015-05-16 06:50:52 UTC
Created attachment 303458 [details] [review]
vinagre-3.16.0-rdp-1.2.1-def.patch

Updated patch using ifdefs.  UseRdpSecurityLayer seems to apply to freerdp >= 1.2.1.  

Tested with freerdp 1.1.0, 1.2.0, & 1.2.1.
Comment 7 David King 2015-05-16 07:06:30 UTC
Review of attachment 303458 [details] [review]:

Thanks for the updated patch! I added attribution information, and pushed to master and gnome-3-16 branches as commit 89cc8bfeaf47a24c57616021915b30bbc4d12d53 and commit c819f3e06363faa3e0c1503d7bbcf3ac88c6dedd.
Comment 8 Ting-Wei Lan 2015-05-17 06:15:06 UTC
We use FreeRDP 1.1.0-beta+2013071101 on FreeBSD, but freerdp/version.h is not available in this version.

plugins/rdp/vinagre-rdp-tab.c:900:10: fatal error: 'freerdp/version.h' file not found
#include <freerdp/version.h>
         ^
1 error generated.
Comment 9 David King 2015-05-17 08:30:23 UTC
Considering that FreeRDP version is nearly 2 years old, you should update it. However, if you can come up with a patch that works on all versions of FreeRDP, I would be happy to merge it.
Comment 10 Ting-Wei Lan 2015-05-23 17:06:20 UTC
I sent a patch to upgrade FreeRDP on FreeBSD to 1.2.0-beta1+android9. It was accepted and the build problem of vinagre was resolved.