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

Change CMake library name back to portaudio from PortAudio #963

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

donarturo11
Copy link
Contributor

@donarturo11 donarturo11 commented Sep 30, 2024

Fixes #939 .

EDIT (added by RossBencina):

The purpose of this patch is to revert back to the CMake library name portaudio (all lowercase) that is used in our current V19.7 release. This avoids breaking things for all users of V19.7 CMake support. Note that this is not a git revert but rather a manual renaming.

@philburk
Copy link
Collaborator

philburk commented Oct 3, 2024

I looked at the PR and it seems limited to changing the case of "PortAudio" to "portaudio".
That is consistent with the request by Ross and @donarturo11 agreed to in #939.

But the CMake build for Ubuntu and MacOS is failing with:

/home/runner/work/portaudio/portaudio/qa/loopback/src/paqa.c:45:10: fatal error: portaudio.h: No such file or directory

For MInGW:

Cannot specify link libraries for target "PortAudio" which is not built by

@RossBencina
Copy link
Collaborator

Thanks for the PR. It's failing CI, I'm not sure why, could you take a look please? It seems like there are still some active occurrences of PortAudio in the file, maybe that's it? Please do a case-sensitive search for PortAudio and review all of the results.

@donarturo11
Copy link
Contributor Author

donarturo11 commented Oct 4, 2024

@RossBencina @philburk Could You retry, please? I merged yet another commits. I fixed cases in tests, examples and CMakeLists.txt. On my CI is now running successful.

@philburk philburk added this to the V19.8 milestone Oct 10, 2024
@RossBencina RossBencina changed the title Undo cases in CMakeLists.txt Change CMake library name back to portaudio from PortAudio Oct 10, 2024
@RossBencina RossBencina requested a review from dmitrykos October 10, 2024 22:56
@RossBencina
Copy link
Collaborator

@dechamps @dmitrykos other CMake users: Please could you weigh in on this. Right now I see no reason to break V19.7 users by renaming the library but maybe I'm missing something.

@RossBencina RossBencina requested a review from Be-ing October 10, 2024 22:59
@RossBencina RossBencina added build-cmake CMake build system P2 Priority: High labels Oct 18, 2024
@philburk philburk removed the build-cmake CMake build system label Oct 18, 2024
@RossBencina RossBencina added the build-cmake CMake build system label Nov 15, 2024
Copy link
Collaborator

@philburk philburk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change can be considered as a bug fix for an earlier breaking change.
Lacking any objections from CMake experts, I think we should do this.

@RossBencina
Copy link
Collaborator

RossBencina commented Nov 25, 2024

I would have liked to hear some feedback from CMake users, but there has been none for over a month.

Without further information this PR effectively fixes a regression relative to V19.7. If there was a valid reason to rename the library to camel case I don't what it is, and without that information I have no way to evaluate the benefit of not merging this PR.

So we're going to go ahead and merge it.

@RossBencina RossBencina merged commit ec1f5a5 into PortAudio:master Nov 25, 2024
11 checks passed
@donarturo11 donarturo11 deleted the undo-cmakelists branch November 26, 2024 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-cmake CMake build system P2 Priority: High
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build portaudiocpp with cmake failed
3 participants