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

Build portaudiocpp with cmake failed #939

Closed
donarturo11 opened this issue Jul 7, 2024 · 10 comments · Fixed by #963
Closed

Build portaudiocpp with cmake failed #939

donarturo11 opened this issue Jul 7, 2024 · 10 comments · Fixed by #963
Labels
build-cmake CMake build system
Milestone

Comments

@donarturo11
Copy link
Contributor

donarturo11 commented Jul 7, 2024

Describe the bug
Trying to create cmake build directory for portaudiocpp gets message:

CMake Error at cmake/modules/FindPortAudio.cmake:20 (message):
  PortAudio_FOUND but not target PortAudio::portaudio

To Reproduce
Steps to reproduce the behavior. Include code if applicable.

  1. Install portaudio with cmake in any directory
cmake -S portaudio -B portaudio-build -DCMAKE_INSTALL_PREFIX=${PORTAUDIO_PATH}
cmake --build portaudio-build --config Release
cmake --install portaudio-build --config Release
  1. Configure building with cmake portaudiocpp
cmake -S portaudio/bindings/cpp -B portaudiocpp-build -DCMAKE_PREFIX_PATH=${PORTAUDIO_PATH} 

Expected behavior
Configure build directory with no errors.

Actual behavior
CMake throws error

CMake Error at cmake/modules/FindPortAudio.cmake:20 (message):
  PortAudio_FOUND but not target PortAudio::portaudio

Desktop (please complete the following information):

  • OS: Windows
  • OS Version: 10
  • PortAudio version: current #2fa0219
  • Host API : any

Proposed solution
Unify target names cases.

Note: PortAudio is a community supported project. If you have a solution, please create a Pull Request for us to consider.

@RossBencina
Copy link
Collaborator

Our CI doesn't fail because our CI doesn't use portaudiocpp. But this does suggest a regression since presumably portaudiocpp used to build.

@donarturo11 would you please check PortAudio v19.7 release and confirm that it does not have this problem?

@RossBencina
Copy link
Collaborator

It's not failing CI because portaudiocpp is not tested by CI.

My concern now is that there is a regression in the PortAudio library CMakeLists.txt, and that maybe we should undo the library library name case change in PortAudio -> portaudio in the main PortAudio CMakeLists.txt.

@dechamps any thoughts?

@donarturo11
Copy link
Contributor Author

I think too that undoing change case in main CMakeList will be better solution than mine.

@donarturo11
Copy link
Contributor Author

@donarturo11 would you please check PortAudio v19.7 release and confirm that it does not have this problem?

Yes sure. So, preparing and building portaudiocpp using source from main branch and stable version of portaudio are successful.
portaudiocpp-msvc

@RossBencina
Copy link
Collaborator

preparing and building portaudiocpp using source from main branch and stable version of portaudio are successful.

@donarturo11 thanks. So as it stands, am I correct to say that there is a regression in our main PortAudio CMakeLists.txt ?

@RossBencina
Copy link
Collaborator

RossBencina commented Jul 25, 2024

Seems that this was part of @Be-ing's big rewrite (which I'm now strongly thinking we should just roll-back since there is no maintainer and it has been shown to have multiple issues):

242a024#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aR12

old add_library(portaudio:

242a024#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aL374

old add_library(portaudio_static:

242a024#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aL383

new: add_library(PortAudio:

242a024#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aR12

@RossBencina RossBencina added this to the V19.8 milestone Jul 25, 2024
@RossBencina RossBencina added the build-cmake CMake build system label Jul 25, 2024
@donarturo11
Copy link
Contributor Author

@RossBencina Yes. In main CMakeLists.txt is a regression. Just undo cases in target names and portaudiocpp will built with no problem.

@RossBencina
Copy link
Collaborator

@donarturo11 Could you please close the old PR and create a new PR with our agreed fix?

@donarturo11
Copy link
Contributor Author

donarturo11 commented Sep 30, 2024

@RossBencina I'm sorry for huge delay, but I had a lot of work at home. I closed old PR and I'll create new soon as possible.

@donarturo11
Copy link
Contributor Author

So @RossBencina , could You now review a new PR, please? I hope that one will be more satysfying.

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

Successfully merging a pull request may close this issue.

2 participants