Discussion:
D17088: [thumbnailer appimage] Fix building with libappimage not in system path
Friedrich W. H. Kossebau
2018-11-22 00:38:48 UTC
Permalink
kossebau created this revision.
kossebau added reviewers: broulik, TheAssassin, azubieta.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.
kossebau requested review of this revision.

REVISION SUMMARY
The current CMake Config file of libappimage does not specify any
imported target nameed "appimage". Instead it provides a shared lib
target "libappimage" and a static lib target "libappimage_static".
Both though are also broken in that they have targets to further
dependencies in their link list, which though are only defined in the
internal build system, but not provided with the installed CMake Config
file.

Additionally the LIBAPPIMAGE_INCLUDE_DIRS is currently not set,
https://github.com/AppImage/libappimage/pull/17 hopefully fixes this
for future versions.

The previous simple "appimage" only worked if libappimage was installed
to an otherwise known prefix, so include dirs and library paths were among
the ones considered, and the linker would find libappimage via -lappimage.

TEST PLAN
Building against current version of libappimage installed to custom prefix
works, as well as building against the patched version.

REPOSITORY
R320 KIO Extras

BRANCH
fixappimagelinking

REVISION DETAIL
https://phabricator.kde.org/D17088

AFFECTED FILES
thumbnail/CMakeLists.txt

To: kossebau, broulik, TheAssassin, azubieta
Cc: kde-frameworks-devel, kfm-devel, alexde, sourabhboss, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov
Kai Uwe Broulik
2018-11-22 11:30:34 UTC
Permalink
broulik added a comment.


Don't know much cmake but still builds fine here with this change, so +1 from me

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D17088

To: kossebau, broulik, TheAssassin, azubieta
Cc: kde-frameworks-devel, kfm-devel, alexde, sourabhboss, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov
TheAssassin
2018-11-22 11:34:49 UTC
Permalink
TheAssassin added a comment.


There will be packages for libappimage (hopefully) soon, which provide a `.pc` file. This provides an alternative to using these generated CMake config files, which can be used through CMake's pkg-config module (`find_package(PkgConfig)`).

REPOSITORY
R320 KIO Extras

REVISION DETAIL
https://phabricator.kde.org/D17088

To: kossebau, broulik, TheAssassin, azubieta
Cc: kde-frameworks-devel, kfm-devel, alexde, sourabhboss, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov
Friedrich W. H. Kossebau
2018-11-23 03:58:02 UTC
Permalink
This revision was not accepted when it landed; it landed in state "Needs Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R320:6c2ebbb5b853: [thumbnailer appimage] Fix building with libappimage not in system path (authored by kossebau).

REPOSITORY
R320 KIO Extras

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17088?vs=45983&id=46055

REVISION DETAIL
https://phabricator.kde.org/D17088

AFFECTED FILES
thumbnail/CMakeLists.txt

To: kossebau, broulik, TheAssassin, azubieta
Cc: kde-frameworks-devel, kfm-devel, alexde, sourabhboss, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov
Loading...