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

Add basic CMake support for master branch #353

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

porkiedev
Copy link

Adds basic support for CMake. It's far from perfect but it does work.

This was heavily derived from the work done in #314 but with less complicated CPU feature detection and support for the latest master branch.

Unfortunately, this only builds the library. I am not a C programmer, nor do I have any experience with autotools or autotest.
Because of this, I couldn't implement and verify autotest functionality. I also couldn't implement benchmarks because I am building under MingW64 on Windows, and the sys/resource.h is not readily available.

That said, this still builds the library fine and seems to be working. Hopefully, this motivates someone more qualified than me with CMake and C to build upon this work.

@pfeatherstone
Copy link

Do you need a GetSources.cmake file? Can't you inline all that stuff in the main CMakeLists.txt script?
You probs also need to install a liquidConfig.cmake file and liquid.pc file.

@porkiedev
Copy link
Author

Hopefully, those commits address your liquidConfig.cmake concerns. I'm not very familiar with CMake so it took me a little bit to figure out.

As for getSources.cmake, no, it's not neccesary. I added it to reduce bloat in the main CMakeLists.txt file. The sources are already very long and the autotests and benchmarks aren't even included yet. That said, there's no reason why it couldn't be inline with CMakeLists.txt.

And lastly, liquid.pc. I'm not entirely sure what a .pc file is. If you have more info, I can look at implementing it.

@pfeatherstone
Copy link

The .pc file is for pkg-config

@porkiedev
Copy link
Author

pkg-config support has been added. I discovered some problems when building on Ubuntu 22.04, so I resolved those and then tested the pkg-config implementation. pkg-config seems to be finding liquid just fine.

Hope that helps! :)

@DSDR999
Copy link

DSDR999 commented Jul 17, 2024

@porkiedev have clone your branch. I run the following commands in msys2(mingW64)
cd liquid-dsp
mkdir build
cmake ..
Its results is
-- VAR IS C:/APP/msys64/home/28181/liquid-dsp/CMakeLists.txt
-- Configuring done (25.1s)
-- Generating done (0.1s)
-- Build files have been written to: C:/APP/msys64/home/admin/liquid-dsp/build
but i found there is no makefile in the build directory

make: *** No targets specified and no makefile found. Stop.

@porkiedev
Copy link
Author

I don't currently have access to a PC, and this branch was from several months ago, so I can't investigate for about a week. Please ping me again if I don't reply again after a week, and I'll see what I can do. :)

@DSDR999
Copy link

DSDR999 commented Jul 17, 2024

I don't currently have access to a PC, and this branch was from several months ago, so I can't investigate for about a week. Please ping me again if I don't reply again after a week, and I'll see what I can do. :)

Thank your quickly reply. if there is no response, i will contact you.

@porkiedev
Copy link
Author

I don't currently have access to a PC, and this branch was from several months ago, so I can't investigate for about a week. Please ping me again if I don't reply again after a week, and I'll see what I can do. :)

Thank your quickly reply. if there is no response, i will contact you.

Sorry for the slow response. I can't remember for sure, but, if I recall correctly, I had issues with generating a makefile, which is why running make doesn't work.
Try running cmake --build . from inside of the build directory and see if that works.

@xerpi
Copy link

xerpi commented Jul 25, 2024

Looking forward to CMake support, which would also make it easier to use Ninja! 🙏

@DSDR999
Copy link

DSDR999 commented Jul 26, 2024

I don't currently have access to a PC, and this branch was from several months ago, so I can't investigate for about a week. Please ping me again if I don't reply again after a week, and I'll see what I can do. :)

Thank your quickly reply. if there is no response, i will contact you.

Sorry for the slow response. I can't remember for sure, but, if I recall correctly, I had issues with generating a makefile, which is why running make doesn't work. Try running cmake --build . from inside of the build directory and see if that works.
I run cmake . at the project root.
Then cmake --build ./ in the build dir.
I have sucessfully build the library.
Thank you.
Maybe, it will be more powerful tool, if it can make the test and example directorie.

@Zosoworld
Copy link

I would also be really interested in finally adding CMake support to liquiddsp. It seems like this branch can be merged with the master one. I have been using it for a while with no problems. Big thanks to @porkiedev for the work.

@xerpi
Copy link

xerpi commented Aug 26, 2024

The more I have to deal with autotools, the more I think CMake support is really needed.
It's quite painful to integrate liquid-dsp with a project using CMake and git submodules right now...

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.

5 participants