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

Fix CMake build on Apple Silicon #14

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

Conversation

madebr
Copy link
Owner

@madebr madebr commented May 1, 2024

On Apple silicon, cmake_host_system_information(RESULT HAVE_FPU QUERY HAS_FPU) returns HAVE_FPU=0.
I reported this upstream.

Without a detected fpu, the build fails when REAL_IS_FIXED is defined, and neon is detected.
https://github.com/madebr/mpg123/actions/runs/8916112597/job/24486953667#step:5:13

My fix moves the cpu arch detection earlier, and hard enables HAVE_FPU for apple arm64.

As an extra, I added a patch that adds macos CI.

madebr added 2 commits May 2, 2024 00:59
cmake_host_system_information returns false currently
@madebr madebr changed the base branch from master-with-github-ci to master May 1, 2024 23:15
Copy link

github-actions bot commented May 2, 2024

The mpg123-devel mailing list has been notified of the existence of this pr.

@sobukus
Copy link

sobukus commented May 2, 2024

Applied to svn, thanks. I don't consider this something urgent, though, as you can build mpg123 on Mac just fine using autotools (right?). Maybe we should have a CI target for that, too.

@madebr
Copy link
Owner Author

madebr commented May 2, 2024

Applied to svn, thanks. I don't consider this something urgent, though, as you can build mpg123 on Mac just fine using autotools (right?). Maybe we should have a CI target for that, too.

Yeah, autotools builds fine.
At least, homebrew users haven't noticed a break. I'll add autotools to the macos yml.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants