You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by necouchman <gi...@git.apache.org> on 2017/07/11 16:33:15 UTC
[GitHub] incubator-guacamole-server pull request #102: GUACAMOLE-296: Fix linking iss...
GitHub user necouchman opened a pull request:
https://github.com/apache/incubator-guacamole-server/pull/102
GUACAMOLE-296: Fix linking issue that causes audio issues with FreeRDP 1.1
This is a very minor tweak to the configure.ac file that adds the winpr-utils library to the list of libraries linked when the winpr/stream.h file is found. This works correctly for FreeRDP 1.0 (winpr/stream.h is not present) and FreeRDP 1.1 (winpr/stream.h is present), but probably not FreeRDP 2.0, as winpr libraries got significantly collapsed there. But, I think there may be other issues with FreeRDP 2.0 and the Guacamole server.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/necouchman/incubator-guacamole-server GUACAMOLE-296
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-guacamole-server/pull/102.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #102
----
commit 0d72ef2ccb7b3c89666a2ed9fd6516b98e05b03d
Author: Nick Couchman <vn...@apache.org>
Date: 2017-07-11T16:20:59Z
GUACAMOLE-296: Fix linking issue that causes audio issues with FreeRDP 1.1
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-server pull request #102: GUACAMOLE-296: Fix linking iss...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-server/pull/102#discussion_r126821765
--- Diff: configure.ac ---
@@ -662,7 +662,7 @@ fi
# Check for stream support via WinPR
if test "x${have_freerdp}" = "xyes"
then
- AC_CHECK_HEADER(winpr/stream.h,,
+ AC_CHECK_HEADER(winpr/stream.h,[RDP_LIBS="$RDP_LIBS -lwinpr-utils"],
--- End diff --
This is unfortunately probably wrong.
This section is specific to one aspect of that WinPR library, so adding the library here is odd. It would make more sense to add the WinPR library to `RDP_LIBS` at the point where the final yes/no decision is made regarding whether WinPR-related parts of the build are enabled, a little bit later:
```
if test "x${have_freerdp}" = "xyes" -a "x${have_winpr}" = "xyes"
then
AC_DEFINE([ENABLE_WINPR],,
[Whether library support for WinPR types was found])
fi
```
I have not looked into the original issue further, but I'm also unsure whether "libwinpr-utils" is the correct library to add.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-server pull request #102: GUACAMOLE-296: Fix linking iss...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-guacamole-server/pull/102
---
[GitHub] incubator-guacamole-server pull request #102: GUACAMOLE-296: Fix linking iss...
Posted by necouchman <gi...@git.apache.org>.
Github user necouchman commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-server/pull/102#discussion_r126961905
--- Diff: configure.ac ---
@@ -662,7 +662,7 @@ fi
# Check for stream support via WinPR
if test "x${have_freerdp}" = "xyes"
then
- AC_CHECK_HEADER(winpr/stream.h,,
+ AC_CHECK_HEADER(winpr/stream.h,[RDP_LIBS="$RDP_LIBS -lwinpr-utils"],
--- End diff --
So, the rationale for adding it with the streams.h section is that this library (winpr-utils) is where the Stream_New and Stream_Free functions actually exist. If you look at the original ticket, you'll see that when you do not link against winpr-utils, you get two undefined symbols (Stream_New and Stream_Free, in addition to the odd third undefined one) - when you link against winpr-utils, these two symbols are defined.
I'll try to take a look at reworking it in with the final determination of WinPR presence later on, and split out WINPR_LIBS from RDP_LIBS and find the right places in the src/protocols/rdp/Makefile.am to put that in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-guacamole-server pull request #102: GUACAMOLE-296: Fix linking iss...
Posted by mike-jumper <gi...@git.apache.org>.
Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-server/pull/102#discussion_r126822057
--- Diff: configure.ac ---
@@ -662,7 +662,7 @@ fi
# Check for stream support via WinPR
if test "x${have_freerdp}" = "xyes"
then
- AC_CHECK_HEADER(winpr/stream.h,,
+ AC_CHECK_HEADER(winpr/stream.h,[RDP_LIBS="$RDP_LIBS -lwinpr-utils"],
--- End diff --
It would make even *more* sense to add a `WINPR_LIBS` variable, and modify `Makefile.am` accordingly. This is the pattern used elsewhere for libraries which may or may not be present. If they are not present/needed, then the variable just stays empty.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---