search

FNA-XNA / FAudio

FAudio - Accuracy-focused XAudio reimplementation for open platforms
https://fna-xna.github.io/
Other
544 stars 73 forks source link

Replace Makefile with CMake #71

Closed flibitijibibo closed 5 years ago

flibitijibibo commented 5 years ago

I think enough cases have come up to consider replacing the Makefile... Typically I just write a handwritten Makefile because there's not much you're meant to do with the library, but FAudio has run into WAY more use cases than anything I've ever built before, so CMake is starting to make a little more sense now. There are many reasons for this, including easy Debug/Release configuration, a clear options list (FFmpeg, XNA_Song, etc), improving the utils/ builds, and if we can, integrating the cpp/ folder's builds.

Note that this is replacing ONLY the Makefiles. The VS/Xcode projects will stay as-is. (Maik, I'll still take the Android.mk if that's still useful)

I wrote up a basic file that covers the core sources, with a bunch of TODOs to fill in:

https://github.com/FNA-XNA/FAudio/tree/cmake

A bunch of people are involved, so here's a list so everyone has an idea of what's going on:

As far as mandatory requirements go, we're stuck supporting Debian's criminally old CMake version, 3.7. Yes, it really is that old. Aside from this limitation, there aren't really any hard rules I can think of.

I'm currently stuck on the code side of things, but I know a lot of people were getting frustrated with my crappy old build system so I thought I'd open this up for anyone who's ready to throw it in the dumpster.

Checklist:

cybik commented 5 years ago

The VS/Xcode projects will stay as-is.

If I may: keeping the XCodeBuild project as-is is of course good, but at the same time, there might be some cases where developers may want to reuse the CMake project, or an altered version of it tailored to their needs, on macOS, yet in a project where they use Frameworks instead of dylibs.

I do realize it's a super rare use case, but I believe having OSX Framework generation logic in the CMake project won't hurt.

NeroBurner commented 5 years ago

What a coincidence! I was close to do a PR. I was just unsure if it would be accepted (or wanted). I'd be happy to contribute the whole cmake project (or merge the efforts)

Another thing I wanted tk do after the inotial cmake PR is to use Hunter to get the SDL2 dependency. I'd have to update their package but this would make it possible to build FAudio on windows (mingw or msvc) with just cmake and a compiler installed. The sdl dependency would be downloaded and build the first time FAudio is buld on the system

aeikum commented 5 years ago

Please don't get too fancy with the build system. Having to deal with brand-new-and-buggy systems like Meson and Ninja has been a real chore in Proton.

flibitijibibo commented 5 years ago

Any patch that adds anything more than the one CMakeLists.txt with any additional build system dependencies will be rejected immediately. Keep in mind that you're competing with a system that technically works with zero dependencies; the addition of one and only one dependency is for simplicity and even adding a single dependency can be arguably more complicated. This is why we're keeping the VS/Xcode projects, for example; dependency tracking can be very annoying if you're not on a Unix system where finding SDL2 should be very easy (and if it isn't easy on that Unix system, that system is broken).

NeroBurner commented 5 years ago

Can I at least add the findFFmpeg file or has to be everything in the one cmakelists file? Can there be a second cmakelists file in the cpp folder?

flibitijibibo commented 5 years ago

Ideally no to the cpp/CMakeLists, though if there's a really really good argument for it I'll let it slide - AFAIK there isn't a standard FindFFmpeg as the official project doesn't support it, is there a standard one that's out there?

GloriousEggroll commented 5 years ago

using pkg-config --cflags --libs libavcodec libavutil should be enough for finding ffmpeg afaik

NeroBurner commented 5 years ago

The findffmpeg.cmake in my branch is a good allrounder. If there is a ffmpeg-config fine that one will be used. Then the pgkconfig will be used to to get hints, and then the standard cmake find-library calls are used. a find_package call is the most standard cmake way to find a dependency

In my oppinion the cmakefile should be in the folder where the cpp files to create the target (library or executable) are in. This keeps the cmake files shorter and makes it easier to keep the overview

GloriousEggroll commented 5 years ago

@NeroBurner my point with using pkg-config is that ffmpeg is not always installed as a system wide package. for example proton compiles its own version of ffmpeg. Thus if you use PKG_CONFIG_PATH=/SOME/PATH/TO/PKG/FILES before calling any pkg-config lines, it will pull the necessary information from .pc pkg config files in that directory instead of the system's default for cases such as custom versions of ffmpeg like proton has. When proton compiles ffmpeg it does not install it system wide, it installs it into an obj-ffmpeg folder, which then copies things over to the obj-tools folder. So you must specify PKG_CONFIG_PATH to let it know to look in this folder instead of the system default.

Also while the cpp folder may be specific to the com wrapper, FAudio itself, which is the main library, resides outside of it, and the com wrapper is not used at all when either compiling faudio or using faudio to build xaudio2 libraries as part of wine's build process. Afaik theres no reason to have the cmake in cpp folder unless specifically compiling the com wrapper - which you wouldnt do unless adding it to a pre-existing wine install (or wine prefix for the mingw version)

NeroBurner commented 5 years ago

The findffmpeg file uses the pkgconfig file, so it doesn't prevent the pkg-cogfig usage

regarding the cpp folder. I've added the option WITH_CPP which controls if the cpp folder is included in the build. I'd like to learn more about the com wrapper and all the targets the faudio repo builds

NeroBurner commented 5 years ago

As far as mandatory requirements go, we're stuck supporting Debian's criminally old CMake version, 3.7.

I fear you mean 2.7. And that's ANCIENT! How long is this dependency needed? Pre CMake 3.0 support adds verbosity and room for errors in the cmake configuration. Pre 2.8.11 even more so! I'd reconsider using and supporting only newer cmake versions and keep the Makefile until the Debian version gets end of life. With CMake 3.0 many improvements in readability, ease of use and usability were introduced.Even more so in the later updates. The move away from using just variables to using targets and defining properties on them (with build dependencies, public and private build flags and much more) was a HUGE improvement for CMake

flibitijibibo commented 5 years ago

The exact version is 3.7.2: https://packages.debian.org/stretch/cmake

It'll be needed until Buster ships, which won't happen until 2019 at the earliest. Backports provides 3.9 but we're doing our best to avoid that.

NeroBurner commented 5 years ago

THAT's recent for me! I had to support 2.8.11 until a year ago. And I'd never go back! I'm VERY happy to hear that! Thanks for the good great news! :)

pchome commented 5 years ago

Yeah, I know ... But in case someone interested: I played a bit with Meson https://github.com/pchome/dxvk-gentoo-overlay/blob/master/media-libs/faudio/files/0001-Initial-meson-build-system-support.patch

It used by media-libs/faudio ebuild, and seems builds/installs fine on Gentoo.

flibitijibibo commented 5 years ago

Added a checklist to the OP. It seems like #77 is the last real TODO item, then it's just final cleanup that's left.

One thing that's getting cut with this transition is Wine support (again), since we now have proper work done in the official Wine tree that we're going to try and mainline. It has some very particular things in it that the current wrapper doesn't account for and probably shouldn't account for (conflicts with native threads vs. Wine threads, for example).

flibitijibibo commented 5 years ago

Okay, #77 just got merged and that about covers all the features. Tomorrow I'll clean up the CMake files, update all the projects to deal with the new include folder, then this should be in master on Friday. We may just end up cutting all the premade projects if that works nicer, ask me again in 2 days...

flibitijibibo commented 5 years ago

Right, so the CMake stuff has been cleaned up and I've made sure everything that worked before still works now. So we've only got two things left, both of which are very simple. @NeroBurner, can you do these two things for me:

Overall I really like this new system, and I'm sure everyone else will too.

pchome commented 5 years ago

Overall I really like this new system, and I'm sure everyone else will too.

Except those, who tired to fix every single CMakeLists.txt file in their distro. https://github.com/gentoo/gentoo/search?l=Diff&q=cmake

Problem: /usr/lib64/ -- the correct destination on my Gentoo system Possible solution:

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -373,10 +373,10 @@
 install(
        TARGETS ${PROJECT_NAME}
        EXPORT ${PROJECT_NAME}-targets
-       INCLUDES DESTINATION include
-       RUNTIME DESTINATION bin
-       LIBRARY DESTINATION lib
-       ARCHIVE DESTINATION lib/static
+       INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
+       RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
+       LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
+       ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
 )

 # Generate cmake-config file, install CMake files
@@ -384,14 +384,14 @@
 configure_package_config_file(
        cmake/config.cmake.in
        ${CMAKE_CURRENT_BINARY_DIR}/generated/${PROJECT_NAME}-config.cmake
-       INSTALL_DESTINATION lib/cmake/${PROJECT_NAME}
+       INSTALL_DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME}
 )
 install(
        FILES ${CMAKE_CURRENT_BINARY_DIR}/generated/${PROJECT_NAME}-config.cmake
-       DESTINATION lib/cmake/${PROJECT_NAME}
+       DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME}
 )
 install(
        EXPORT ${PROJECT_NAME}-targets
        NAMESPACE ${PROJECT_NAME}::
-       DESTINATION lib/cmake/${PROJECT_NAME}
+       DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME}
 )

https://cmake.org/cmake/help/v3.13/module/GNUInstallDirs.html

flibitijibibo commented 5 years ago

@NeroBurner, this look good to you? If so, @pchome, send this as a PR and I'll get it into master.

NeroBurner commented 5 years ago

yes looks good :+1:

flibitijibibo commented 5 years ago

Excellent - @pchome, write up what works for your system and then send the final patch as a PR and I'll merge it, so the commit will be in your name.

pchome commented 5 years ago

@flibitijibibo

so the commit will be in your name

It's not really matter.

Also, there is some places I'm not sure about:

So, better check/make that changes by yourself.

NeroBurner commented 5 years ago

I think it would be OK to put all static-libs into lib instead of lib/static, at least for Linux. @flibitijibibo do you know if there are any problems on Windows (mingw-w64/msvc) build?

flibitijibibo commented 5 years ago

The static libs only exist for the COM wrapper, so that's thankfully not applicable:

https://github.com/FNA-XNA/FAudio/commit/5353573bf823e8d242ac729c0bb8f5d0b2b48266

I do see this one problem with the MinGW build that's really odd:

-- Installing: /usr/i686-w64-mingw32/sys-root/mingw/lib/libFAudio.dll.a
CMake Error at cmake_install.cmake:452 (file):
  file INSTALL cannot copy file
  "/home/flibitijibibo/Programming/csLibraries/FAudio/_build_mingw/libFAudio.dll.a"
  to "/usr/i686-w64-mingw32/sys-root/mingw/lib/libFAudio.dll.a".

For some strange reason CMAKE_INSTALL_LIBDIR doesn't get changed based on CMAKE_INSTALL_PREFIX?

NeroBurner commented 5 years ago

with mingw enabled CMAKE_INSTALL_LIBDIR is set to an absolute path (the mingw-system-library path) see PR https://github.com/FNA-XNA/FAudio/pull/89