GUACAMOLE-1026: Add support for FreeRDP3.

[aleitner]

Refactored to support both FreeRDP 2 and 3.

• Added version checking for FreeRDP 3.x and fallback to 2 if 3 doesn't exist
• Additional checks introduced for various FreeRDP 3.x feature support
  • _aligned_free and _aligned_malloc renamed to winpr_aligned_free and winpr_aligned_malloc
  • ReadColor and WriteColor renamed to FreeRDPReadColor and FreeRDPWriteColor
  • Various CLIPRDR properties moved to CLIPRDR_HEADER common struct for CLIPRDR structs.
  • Required use of getter/setter functions interacting with rdpSettings
    • freerdp_settings_set_pointer, freerdp_settings_set_bool, freerdp_settings_set_string, freerdp_settings_set_uint32, freerdp_settings_get_pointer_writable, freerdp_settings_get_bool, freerdp_settings_get_string, freerdp_settings_get_uint32
    • Note: freerdp_settings_set_pointer and freerdp_settings_get_pointer_writable were only added for 3.X where as the rest were available within 2.X.
    • getter/setter functions within 3.X now use updated enum types for keys prefixed with FreeRDP_.
  • FreeRDP 3.X moved context-related pointers (rdpSettings* settings, rdpInput* input) to a context structure in the freerdp* instance.
  • freerdp_shall_disconnect renamed to freerdp_shall_disconnect_context.
  • IDRDYNVC_ENTRY_POINTS.GetPluginData() return now requires const.
  • glyph.New and pointer.Set no longer require arguments to be const.

[necouchman]

A couple of initial comments - overall looks pretty clean!

[necouchman]

I need to pull the code down and actually run it through some testing before I'm good with it, I'll try to get that done this weekend.

Currently this is targeted at the main branch which would put it in the 1.6.0 release. In the past I think it was mentioned that this might go into a 2.0 release, so wanted to check and see if @mike-jumper or @jmuehlner had any thoughts on that? I'm fine with it going into 1.6.0 - it seems like it's a pretty minimal and compatible set of changes, just want to make sure we're all in agreement.

[jmuehlner]

I need to pull the code down and actually run it through some testing before I'm good with it, I'll try to get that done this weekend.

Currently this is targeted at the main branch which would put it in the 1.6.0 release. In the past I think it was mentioned that this might go into a 2.0 release, so wanted to check and see if @mike-jumper or @jmuehlner had any thoughts on that? I'm fine with it going into 1.6.0 - it seems like it's a pretty minimal and compatible set of changes, just want to make sure we're all in agreement.

It doesn't drop compatibility with FreeRDP 2.x so I'm fine putting it in main, and in 1.6.0.

[mike-jumper]

No objection from me for inclusion in 1.6.0 either as long as we maintain compatibility with FreeRDP 2.x.

[mike-jumper]

I've added this temporary build to help verify these changes against various 2.x and 3.x versions of FreeRDP:

https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/

The above is identical to our existing build except that it builds against this RDP-3 branch (not main) and includes FreeRDP's main and latest 3.x release as additional versions to be tested.

[jmuehlner]

I've added this temporary build to help verify these changes against various 2.x and 3.x versions of FreeRDP:

https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/

The above is identical to our existing build except that it builds against this RDP-3 branch (not main) and includes FreeRDP's main and latest 3.x release as additional versions to be tested.

Looks like the FreeRDP 3 test build failed: https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/FREERDP_BRANCH=3.5.1,JENKINS_LABEL_EXPRESSION=ubuntu/lastBuild/console

[aleitner]

I've added this temporary build to help verify these changes against various 2.x and 3.x versions of FreeRDP:
https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/
The above is identical to our existing build except that it builds against this RDP-3 branch (not main) and includes FreeRDP's main and latest 3.x release as additional versions to be tested.

Looks like the FreeRDP 3 test build failed: https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/FREERDP_BRANCH=3.5.1,JENKINS_LABEL_EXPRESSION=ubuntu/lastBuild/console

Huh that shouldn't happen. I just pushed changes a little bit ago to resolve that

[jmuehlner]

Huh that shouldn't happen. I just pushed changes a little bit ago to resolve that

Ah ok - looks like a new build did kick off a bit ago - maybe that'll fix it.
https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/2/

[jmuehlner]

Huh that shouldn't happen. I just pushed changes a little bit ago to resolve that

Ah ok - looks like a new build did kick off a bit ago - maybe that'll fix it. https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/2/

Looks like it failed again... https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/2/FREERDP_BRANCH=3.5.1,JENKINS_LABEL_EXPRESSION=ubuntu/

[necouchman]

Looks like it failed again... https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/2/FREERDP_BRANCH=3.5.1,JENKINS_LABEL_EXPRESSION=ubuntu/

Looks like it doesn't like the attempts to build FreeRDP within the source tree:

#8 16.99 CMake Error at cmake/PreventInSourceBuilds.cmake:28 (message):
#8 16.99   
#8 16.99 
#8 16.99   CMake must not to be run in the source directory.  Rather create a
#8 16.99   dedicated build directory and run CMake there.  CMake now already created
#8 16.99   some files, to clean up after this aborted in-source compilation:
#8 16.99 
#8 16.99      rm -r CMakeCache.txt CMakeFiles
#8 16.99 
#8 16.99   or
#8 16.99 
#8 16.99      git clean -xdf
#8 16.99 
#8 16.99   
#8 16.99 
#8 16.99   If you happen to require in-source-tree builds for some reason rerun with
#8 16.99   -DALLOW_IN_SOURCE_BUILD=ON

[necouchman]

Looks like it failed again... https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/2/FREERDP_BRANCH=3.5.1,JENKINS_LABEL_EXPRESSION=ubuntu/

Looks like it doesn't like the attempts to build FreeRDP within the source tree:

#8 16.99 CMake Error at cmake/PreventInSourceBuilds.cmake:28 (message):
#8 16.99   
#8 16.99 
#8 16.99   CMake must not to be run in the source directory.  Rather create a
#8 16.99   dedicated build directory and run CMake there.  CMake now already created
#8 16.99   some files, to clean up after this aborted in-source compilation:
#8 16.99 
#8 16.99      rm -r CMakeCache.txt CMakeFiles
#8 16.99 
#8 16.99   or
#8 16.99 
#8 16.99      git clean -xdf
#8 16.99 
#8 16.99   
#8 16.99 
#8 16.99   If you happen to require in-source-tree builds for some reason rerun with
#8 16.99   -DALLOW_IN_SOURCE_BUILD=ON

I tweaked the build script slightly to create a build_root directory, cd to that, and then cmake from the parent directory. We'll see what happens.

[necouchman]

I tweaked the build script slightly to create a build_root directory, cd to that, and then cmake from the parent directory. We'll see what happens.

I updated the script, but that doesn't seem to have triggered an automatic rebuild, and I don't seem to have permissions to force a rebuild. @mike-jumper you might have to kick it off, again.

[jmuehlner]

And it's now passing! https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/13/

Looks like it failed again... https://ci-builds.apache.org/job/Guacamole/job/guacamole-server-wip-freerdp3/2/FREERDP_BRANCH=3.5.1,JENKINS_LABEL_EXPRESSION=ubuntu/

[jmuehlner]

Looking good.

[necouchman]

Looking really good - I just have a question and a minor code style tweak, and I think it's ready to .

[necouchman]

Alright, I'm happy with it - tested it out with FreeRDP2 on my EL8 system and it appears to work okay, so I'll go ahead and merge it. Great work, @aleitner, it's really nice to tick this one off the list!!