media-tv/kodi-9999: Use cmake instead of autotools #3027
Conversation
gentoo-repo-qa-bot
added
the
assigned
|
Pull Request assignment Areas affected: ebuilds media-tv/kodi: @vapier, @gentoo/video |
|
I'm not a cmake or autotools expert by any means. However, I think I've set this up correctly (using Gentoo documentation and other ebuilds as guidance) but it doesn't work. Here's the error I'm getting: Could someone who knows a bit more about cmake and its use in ebuilds help me out? |
|
@mgorny based on the git log, you seem to be the authority on cmake-utils.eclass, and that's what's causing me confusion right now. The cmake command run by the ebuild is: which gives the error above. However, by removing the additions made by cmake-utils I get this command: which works. Do you have any tips as to what next steps I can take to figure out why the gentoo additions cause this problem? |
|
"sorry" for jumping in, but seeing i'm the autotools removal dude (in work clothes): this is not your fault. this is the kodi cmake buildsystem assuming that a toolchain file means you must be crosscompiling, and then things goes haywire for you obviously. this must be fixed before autotools removal. |
|
please try with pr ref'd above. |
mgorny
changed the title
|
I have some remarks: -> combine use flags cdio and optical: ENABLE_OPTICAL requires CDIO (I've chosen dvd in my local overlay) -> udev is preferred over usb (see project/cmake/CMakeLists.txt), so better force udev instead of usb -> keep use flag texturepacker, or at least add its built time dependencies on lzo, libpng, giflib and virtual/jpeg to DEPEND -> change udisks? ( sys-fs/udisks:0 ) to udisks? ( sys-fs/udisks ) -> remove "?? ( gles vaapi )" from REQUIRED_USE -> remove toolchain-funcs from inherit -> remove all ac_cv_* from ebuild -> remove export HELP2MAN from src_configure -> modify src_unpack see BZ#598150 -> add dev-db/sqlite to COMMON_DEPEND -> raise ffmpeg dep to >=3.1.6 -> move app-arch/bzip2, app-arch/unzip and app-arch/zip from COMMOND_DEPEND to DEPEND (need to check if these are still needed) -> remove \ from mycmakeargs (this should be an array) -> add cmake option for bluetooth to mycmakeargs -> -DENABLE_CACHE=OFF should read -DENABLE_CCACHE=OFF -> remove many more unneeded packages (BZ#598460):
|
Done
Based on my reading of the cmake setup, texturepacker isn't optional, so we can't have a use flag controlling it. libpng is the only dependency you mentioned that wasn't already present, so I've added that (I also double checked - kodi does use it so it definitely should be there).
Done
Done
Done
Done
Done
Done
I believe that's covered by
Done
Makes sense. From looking at the kodi source, I agree these aren't used at runtime. Done.
Done.
There is no cmake argument to enable/disable bluetooth (ENABLE_BLUETOOTH isn't an option). I think we just need a conditional dependency on dev-python/pybluez.
Done.
Also dropped dependencies on media-libs/libmpeg2, media-libs/tiff, and media-libs/jasper.
Done
Done
I believe pillow is used - can you please check again?
Are you sure corefonts are unnecessary?
Done.
Done.
Done
Done
Done
Done (dropped in xbmc/xbmc#8687 )
Done
Done
Looks like this may be used... https://github.com/xbmc/xbmc/blob/17.0b6-Krypton/xbmc/music/tags/TagLoaderTagLib.cpp#L46 Thoughts?
Looks like this may be used... https://github.com/xbmc/xbmc/blob/17.0b6-Krypton/tools/depends/xbmc-addons.include#L93 Thoughts?
Done
Done
Done
Thank you very much for this feedback - I look forward to further feedback from you. I've submitted some pull requests to Kodi requesting they update the documented dependencies: Would you mind doing the same for any others as well as letting me know? |
|
@notspiff Thanks for jumping in - I'm using notspiff/xbmc@578050b as a patch, and made good progress. However, I'm now getting this error: Notably, glib.h exists and is available at /usr/include/glib-2.0/glib.h. Therefore, I believe there's a problem with https://github.com/xbmc/xbmc/blob/master/project/cmake/modules/FindFribidi.cmake and how it sets up the include dir - can you help with that issue? |
The current ebuild hast +usb. But UDEV will be used, if it is found, because according to CMakeLists.txt, LibUSB is only searched, if UDEV_FOUND is false: Note: configure.ac does the same,:
I guess that is something like crossguid, perhaps there may be a standalone texturepacker. But I agree, as long as this isn't the case, we should hard depend on the texturepacker deps but only fuer built time.
I can't find any place where kodi uses libpng:
I have no clue why kodi depends on python[sqlite]. Kodi itself is using sqlite as the default database backend if I'm correct. It also has it as required_dep in CMakeLists.txt.
You're right. Only configure.ac has a switch for bluetooth. There is a FindBluetooth.cmake file, but it is not used. Perhaps we should report that to the kodi guys. ../xbmc/network/TCPServer.cpp has a #ifdef HAVE_LIBBLUETOOTH which is only set by configure.ac, while there is no occurence in project/cmake
I've removed the dep some time ago and kodi builds just fine
I've removed the dep some time ago and kodi builds just fine
The include is provided by taglib:
I don't know if this file is also used in cmake builds, I can find it only in a Makefile:
Thanks!
Which others do you mean? Perhaps we should ask kodi developers about pillow, wavpack, corefonts and glew. Even though wavpack and glew ar einstalled on my system, they are not used: |
|
@candrews , fribidi problem solved by xbmc/xbmc#11116 |
I see what you're saying, thanks for the clarification. Perhaps the
I agree, I've removed the dependency on libpng.
Reported: http://trac.kodi.tv/ticket/17129#ticket
Yes, but does it work right at run time? :-)
Removed that dependency.
glew was removed from kodi: xbmc/xbmc#8631 |
|
Thanks to the work here I've now been able to build kodi-17.0_beta6 with cmake, although I had to add USE="webserver" (more on that at the end).
Messages during build:
Then src_unpack() can be removed.
|
I have no idea how to deal with that. Perhaps there is some default behaviour which gentoo prefers.
TexturePacker requires libpng, so please do not remove it, add it to DEPEND
Haven't seen any change in behaviour so far. |
Not 100% sure, but this maybe because texturepacker packs all these images in one file.
Thats just an additional fontset defined which uses arial instead of noto.
Looks like arial is used as default for subtitles, so the depencendy on corefonts should stay in my opinion. Arial is also used if the configured font can not be found/loaded (see xbmc/guilib/GUIFontManager.cpp)
Didn't see that, thanks. Perhaps also a candidate for upstream kodi to look at.
This comes as a dependency from another used shared lib:
Yes, because tools/depends/native/TexturePacker/CMakeLists.txt requires it:
Looks like an error which upstream kodi should fix. |
|
|
We are giving you guys a run for your money. 😉 |
|
could you please tell us about:
|
Makes sense. Done.
Thanks for the details. Done.
Good point. Done.
Done.
Nope. I don't think it should be either. Done.
Oops. Done.
Oops. Done.
Another good catch - done.
Agreed - here's the bug for shairplay in Gentoo: https://bugs.gentoo.org/show_bug.cgi?id=538932
I think that's a bug in the Kodi, I reported it at http://trac.kodi.tv/ticket/17130
Since ENABLE_LIBUSB/HAVE_LIBUSB are only checked if ENABLE_UDEV is off, I fixed this by not specifying -DENABLE_LIBUSB unless the libusb use flag is specified.
Reported at http://trac.kodi.tv/ticket/17131
I'm not sure how that works for other distros... but I've included the patch in the ebuild here. |
it's due to legacy. lots of add-ons (used to) implicitly assume it's available in the kodi-hosted python env without explicitly depending on a script module. since we run on the system interpreter, it needs to be around on the system level. when these assumptions were made, kodi used its own embedded interpreter where it was always supplied, hence the reference to 'legacy'. |
|
@candrews re dev-libs/libplatform: The rename was done due to the upstream rename (which was neccessary due to some package naming conflict with debian IIRC), which also involved API breakage (namespace was moved from PLATFORM:: to P8PLATFORM::). Also, IIRC, there wasn't a live ebuild available for the platform lib at that time. Lastly, I wanted all Kodi related things in one place/overlay. Since the platform lib really is p8platform now, IMHO the ebuild should be named accordingly and also conflict with the old library, and also have the package name aligned with other distros like debian/ubuntu. |
|
@candrews Before this PR gets merged, you should pick herrnst/gentoo-kodi-overlay@14917a6 (or adjust accordingly) as the CMake project files were moved. |
candrews
force-pushed
the
kodicmake
branch
3 times, most recently
from
830dc58
to
e7be8a8
Compare
|
Thanks @herrnst! I've made that change. |
|
@SoapGentoo I think this is ready for merging - can you take a peek at it please? |
| || ( gles opengl ) | ||
| udev? ( !libusb ) | ||
| udisks? ( dbus ) | ||
| upower? ( dbus ) |
There was a problem hiding this comment.
missing PYTHON_REQUIRED_USE (which is very important for python-single-r1!)
There was a problem hiding this comment.
That's obvious... I wonder if repoman can be improved to catch that kind of thing.
| >=net-misc/curl-7.51.0 | ||
| nfs? ( net-fs/libnfs:= ) | ||
| opengl? ( media-libs/glu ) | ||
| openssl? ( >=dev-libs/openssl-1.0.2j:0= ) |
There was a problem hiding this comment.
is this the only SSL/TLS/whatever provider? If so, USE=ssl enables/disables SSL/TLS, whereas USE=openssl selects the impl, conditional on USE=ssl
There was a problem hiding this comment.
libressl also works, so I've updated it accordingly.
| } | ||
|
|
||
| src_unpack() { | ||
| [[ ${PV} == 9999 ]] && git-r3_src_unpack || default |
There was a problem hiding this comment.
is this really necessary? doesn't git-r3.eclass override it if its inherited in a -9999 ebuild? Just asking
There was a problem hiding this comment.
Indeed it does! That's really cool
| } | ||
|
|
||
| src_prepare() { | ||
| default |
There was a problem hiding this comment.
never use default with cmake-utils, always use cmake-utils_src_prepare!
| } | ||
|
|
||
| src_configure() { | ||
| local CMAKE_BUILD_TYPE=$(usex debug Debug RelWithDebInfo) |
There was a problem hiding this comment.
in general please totally avoid this. Gentoo sets its own CMAKE_BUILD_TYPE which is totally orthogonal to the classic ones, and if the build breaks, it makes too many assumptions about CMAKE_BUILD_TYPE. CMake-based build systems should be agnostic of CMAKE_BUILD_TYPE.
There was a problem hiding this comment.
While I agree in principal, Kodi only works with specific cmake build types; it fails if the cmake build type is set to Gentoo as happens by default. So I think we have to set CMAKE_BUILD_TYPE here, unfortunately.
There was a problem hiding this comment.
alright then, can you try and get this removed upstream then?
| } | ||
|
|
||
| src_compile() { | ||
| default |
There was a problem hiding this comment.
call cmake-utils_src_compile instead of default
|
|
||
| rm "${ED%/}"/usr/share/kodi/addons/skin.estuary/fonts/Roboto-Thin.ttf || die | ||
| dosym /usr/share/fonts/roboto/Roboto-Thin.ttf \ | ||
| usr/share/kodi/addons/skin.estuary/fonts/Roboto-Thin.ttf |
There was a problem hiding this comment.
I haven't compared them bitwise, but can't this be done in a loop to reduce the churn? Just a suggestion
There was a problem hiding this comment.
Makes sense and cuts down on the number of lines a bit
|
Isn't it recommended to use eclass functions provided by inheritance? E.g. from CMAKE-UTILS.ECLASS:
|
cmake-utils_use_* is banned in EAPI 6: https://groups.google.com/forum/#!topic/linux.gentoo.dev/QxCQJ_8taqg |
I see the point of the discussion. But I can't see the final decision to do so. According to the current version and documentation it still exists: |
candrews
force-pushed
the
kodicmake
branch
3 times, most recently
from
4797834
to
3d56e67
Compare
|
yes, all those eclass functions have been killed because they add no value |
| rm "${ED%/}"/usr/share/kodi/addons/skin.estuary/fonts/NotoSans-Regular.ttf || die | ||
| dosym /usr/share/fonts/noto/NotoSans-Regular.ttf \ | ||
| usr/share/kodi/addons/skin.estuary/fonts/NotoSans-Regular.ttf | ||
| for f in NotoMono-Regular.ttf NotoSans-Bold.ttf NotoSans-Regular.ttf ; do |
| dosym /usr/share/fonts/noto/NotoSans-Regular.ttf \ | ||
| usr/share/kodi/addons/skin.estouchy/fonts/NotoSans-Regular.ttf | ||
|
|
||
| for f in NotoMono-Regular.ttf NotoSans-Bold.ttf NotoSans-Regular.ttf ; do |
|
|
||
| rm "${ED%/}"/usr/share/kodi/addons/skin.estouchy/fonts/Roboto-Thin.ttf || die | ||
| dosym /usr/share/fonts/roboto/Roboto-Thin.ttf \ | ||
| usr/share/kodi/addons/skin.estuary/fonts/Roboto-Thin.ttf |
There was a problem hiding this comment.
can't you put everything in the loop with
local f
for f in NotoMono-Regular.ttf NotoSans-Bold.ttf NotoSans-Regular.ttf NotoSans-Regular.ttf Roboto-Thin.ttf; do
rm "${ED%/}"/usr/share/kodi/addons/skin.*/fonts/"${f}" || die
dosym /usr/share/fonts/noto/"${f}" \
usr/share/kodi/addons/skin.estuary/fonts/"${f}"
done
There was a problem hiding this comment.
I don't think so.
- "Roboto-Thin.ttf" isn't in /usr/share/fonts/noto/ so
dosymwould make a broken symlink - There are two skins, esuary and estouchy - this loop would remove the fonts from both estuary and estouchy, but only add them back into estuary.
Upstream has dropped support for autotools and requires the use of cmake. Gentoo-bug: 601738, 598460, 598460
Just wanted to let you know I opened a PR to get |
Upstream is dropping support for autotools and requiring the use of cmake:
see xbmc/PR10429
https://bugs.gentoo.org/show_bug.cgi?id=601738