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 CI builds #1435

Merged
merged 7 commits into from
Nov 23, 2024
Merged

Fix CI builds #1435

merged 7 commits into from
Nov 23, 2024

Conversation

derselbst
Copy link
Member

This PR fixes the Android CI build and it (almost) fixes the Windows CI build. It also fixes fluidsynth's build on Windows XP and older.

There is one remaining issue: If a depending software package (that fluidsynth discovers via pkg-config) depends on libsndfile, fluidsynth will try to find a matching sndfile.pc file, but fails to do so, because libsndfile currently doesn't distribute them in their official Windows binary builds:
https://github.com/libsndfile/libsndfile/blob/0d3f80b7394368623df558d8ba3fee6348584d4d/.github/workflows/action.yml#L173

This will then result in the following error:
https://dev.azure.com/tommbrt/tommbrt/_build/results?buildId=11000&view=results

I currently have no explanation why this only affects the x86 while the x64 build works fine. The requirement for sndfile originates from libinstpatch's .pc file, and there sndfile is clearly marked as a requirement for both x86 and x64 builds.

@pedrolcl Do you have an idea how to fix or workaround that issue? Any idea why libsndfile has explicitly disabled installing the pkg-config file? Else I would ask them to change it.

Fixes #1422

@derselbst derselbst added this to the 2.4 milestone Nov 17, 2024
@pedrolcl
Copy link
Contributor

pedrolcl commented Nov 17, 2024

Do you have an idea how to fix or workaround that issue?

You may disable libinstpatch for building this scenario. The problem may be in cmake_admin/FindInstPatch.cmake or in libinstpatch itself.

Any idea why libsndfile has explicitly disabled installing the pkg-config file? Else I would ask them to change it.

I remember requesting Erik to include the .pc file in the windows .zip package a long time ago. But since he left the project the new maintainers seem clueless, and this case in particular seems like an ideological decision.

@derselbst
Copy link
Member Author

I have created an issue at libsndfile/libsndfile#1047. Let's see what happens. For the moment, I have disabled libinstpatch for WinXP x86.

If my request is decline, the alternative to restore libinstpatch support would be to compile libsndfile from source with cmake flag INSTALL_PKGCONFIG_MODULE=1.

@derselbst derselbst merged commit 4cdd9af into master Nov 23, 2024
56 of 58 checks passed
@derselbst derselbst deleted the fix-ci branch November 23, 2024 10:52
@pedrolcl
Copy link
Contributor

Related to the libsndfile particularities, in particular to the macOS CI build failures, I've found this insightful historic message from Erik: https://lists.cs.princeton.edu/hyperkitty/list/[email protected]/message/5AMBYZG24LMQFCUKFUWZMO6WW5YEYO6D/

The latest OSX compilers however are able to compile a dual architecture
binary; a binary which contains code for both intel and pcc cpus. However,
for libsndfile at least, this is a bad idea. libsndfile has lots of
architecture and OS secific tweaks which will not be correctly detected
for one or the other of the two dual architectures.

Remember, Erik says, "Always do native compiles and always run make check
to make sure libsndfile is working correctly".

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.

Cannot build Fluidsynth with libsndfile on Windows.
2 participants