Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to work with vte 2.91 as well as 2.90 #371

Merged
merged 1 commit into from
Nov 11, 2014

Conversation

iainlane
Copy link
Contributor

@iainlane iainlane commented Nov 7, 2014

I #ifdefed the code to work with 2.90 too - perhaps it's a good idea to make the version build-time detected.

@antenore
Copy link
Member

antenore commented Nov 7, 2014

Thanks for the patch, would you mind preparing and testing the patch also with "next" ?

Meanwhile I'm going to test and merge it in master.

Thanks in advance!

@antenore
Copy link
Member

antenore commented Nov 7, 2014

@iainlane @giox069
On Fedora 20, this patch does not work out of the box without passing theses flags

-DVTE_INCLUDE_DIR=/usr/include/vte-2.90 -DVTE_LIBRARY=/usr/lib64/vte-2.90

I'm not yet sure that is an issue with the patch or with Fedora.

-- package 'vte-2.91' not found
CMake Error at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:108 (message):
Could NOT find VTE (missing: VTE_LIBRARY VTE_INCLUDE_DIR)
Call Stack (most recent call first):
/usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:315 (_FPHSA_FAILURE_MESSAGE)
cmake/FindVTE.cmake:46 (find_package_handle_standard_args)
cmake/FindOptionalPackage.cmake:37 (find_package)
remmina/CMakeLists.txt:146 (find_suggested_package)

I'll take a look better later, or the next days, meanwhile if you have any comments...

@giox069
Copy link
Contributor

giox069 commented Nov 7, 2014

The patch looks interesting for the "next" branch also. But I have the same problem as @antenore when compiling Remmina master branch on Debian 8 (both vte 2.90 and 2.91 are installed, but the -dev package installed is 2.90, so headers/libraries are for 2.90)

--   Disable this using "-DWITH_VTE=OFF".
-- checking for module 'vte-2.91'
--   package 'vte-2.91' not found
CMake Error at /usr/share/cmake-3.0/Modules/FindPackageHandleStandardArgs.cmake:136 (message):
  Could NOT find VTE (missing: VTE_LIBRARY VTE_INCLUDE_DIR)
Call Stack (most recent call first):
  /usr/share/cmake-3.0/Modules/FindPackageHandleStandardArgs.cmake:343 (_FPHSA_FAILURE_MESSAGE)
  cmake/FindVTE.cmake:46 (find_package_handle_standard_args)
  cmake/FindOptionalPackage.cmake:37 (find_package)
  remmina/CMakeLists.txt:146 (find_suggested_package)

@iainlane
Copy link
Contributor Author

Yes it's because of _VTE_VERSION_NUM. It sounds like you would like this to be dynamically detected, so let me do that.

@iainlane
Copy link
Contributor Author

Okay, how about that? I had to hand-roll it a little bit (not too much) because find_optional_package bails if the package can't be found...

@iainlane
Copy link
Contributor Author

FWIW it now works out of the box for me with either libvte-2.90-dev or libvte-2.91-dev on Debian

@giox069
Copy link
Contributor

giox069 commented Nov 10, 2014

Tested on Debian 8: compiles correctly and links with vte 2.90. Seems also to be running fine.

@antenore
Copy link
Member

Tomorrow I'll test this also on fedora and then I'll merge it if you all are agree..

@giox069
Copy link
Contributor

giox069 commented Nov 10, 2014

@antenore: yes, very well. @iainlane: many thanks for the patches.

@antenore antenore merged commit 9ec1aea into FreeRDP:master Nov 11, 2014
@antenore
Copy link
Member

merged with master as well as promised.

@iainlane
Copy link
Contributor Author

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants