Skip to content

Fix CI builds#1435

Merged
derselbst merged 7 commits intomasterfrom
fix-ci
Nov 23, 2024
Merged

Fix CI builds#1435
derselbst merged 7 commits intomasterfrom
fix-ci

Conversation

@derselbst
Copy link
Copy Markdown
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
Copy Markdown
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.

@sonarqubecloud
Copy link
Copy Markdown

@derselbst
Copy link
Copy Markdown
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
@derselbst derselbst deleted the fix-ci branch November 23, 2024 10:52
@pedrolcl
Copy link
Copy Markdown
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/chuck-users@lists.cs.princeton.edu/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".

DominusExult added a commit to DominusExult/fluidsynth-sans-glib that referenced this pull request Dec 27, 2024
* master: (66 commits)
  Add portamento test files
  Restore discovery of libsndfile on Ubuntu (FluidSynth#1454)
  Fix crash on startup when there are no MIDI devices  (FluidSynth#1447)
  Replace VERSIONINFO resource cmake macro (FluidSynth#1449)
  Fix filter processing order in DSP chain (FluidSynth#1444)
  Render the nervous filter with additional soundfonts
  Add Christian's SF2 spec test to regression tests
  Bump testdata ref
  Migrate test files from LFS to submodule
  Delete test files from Git LFS
  Bump to 2.4.1
  Smooth linear filter parameter change (FluidSynth#1432)
  Reenable AWE32 NRPN 23 and 24 (FluidSynth#1430)
  indentation issue
  Disable chorus for Uplift - it's too noisy in the middle part
  Add Klerg's renderings of Altitude and Uplift
  updated cmake build system * debug output for libsndfile, quieter output for pulseaudio * enhanced build summary report * revised building win32 binaries without unicode support
  add cmake output for sndfile issues
  Fix CI builds (FluidSynth#1435)
  Add test files for AWE32 NRPN (FluidSynth#1434)
  ...
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