[fluidsynth] Update to 2.2.4

[NancyLi1013]

Describe the pull request

• What does your PR fix?

  Fixes [FluidSynth] update to 2.2.3 #20868

Update to 2.2.3

Add set(CMAKE_DISABLE_SOURCE_CHANGES OFF) to the patch to fix the error like this:

CMake Error at C:/Program Files (x86)/CMake/share/cmake-3.21/Modules/ExternalProject.cmake:1738 (file):
  file attempted to create a directory:
  F:/vcpkg_0826/vcpkg/buildtrees/fluidsynth/src/272d57e624-a4c0bbde24/src/gentables
  into a source directory.
Call Stack (most recent call first):
  C:/Program Files (x86)/CMake/share/cmake-3.21/Modules/ExternalProject.cmake:3652 (_ep_set_directories)
  src/CMakeLists.txt:439 (ExternalProject_Add)

Upstream issue FluidSynth/fluidsynth#993

Note: No feature needs to test.

[BillyONeal]

Add set(CMAKE_DISABLE_SOURCE_CHANGES OFF) to the patch to fix the error like this:

I don't think we should do that, I think we should use DISABLE_PARALLEL_CONFIGURE since this indicates a write into the source tree which is likely to race (as I did in #21012 )

[NancyLi1013]

Thanks for your suggestion @BillyONeal. I have verified and DISABLE_PARALLEL_CONFIGURE can solve the problem.

For the failure on arm64-windows, the workaround is reasonable?

Remove -G "${CMAKE_GENERATOR} from CONFIGURE_COMMAND

[BillyONeal]

For the failure on arm64-windows, the workaround is reasonable?

Considering the comment there says that the whole purpose of passing the -G there is for arm64 I would be wary to make it not do that...

[BillyONeal]

For the failure on arm64-windows, the workaround is reasonable?

Considering the comment there says that the whole purpose of passing the -G there is for arm64 I would be wary to make it not do that...

On the other hand the comment explicitly says "in vcpkg" and there we were using msbuild on arm64; using vcpkg_cmake_configure we're now using Ninja so perhaps the -A part is no longer necessary...

[BillyONeal]

Ultimately the issue here is that this project is running a program that it builds during its build on the host, which fails for cross compilation scenarios. We're explicitly defeating that for arm and arm64, forcing it to use x86 built bits, which fails if anything funky is going on with how we invoke cmake.

In my view there are 2 possible fixes:

1. Split the port into 2 ports, 1 of which builds this gentables part, and 1 of which builds the rest, and set up a host dependency between them.
2. Effectively the same thing but with features in the same port.

I have a mild preference for 1.

[NancyLi1013]

Thanks for your proposal @BillyONeal. Sounds great!

I will try to use the first way to update this PR.

[kcgen]

Hi @NancyLi1013,

I know the plan was to split the FluidSynth port into an x86-specific portion (say, "FluidSynth x86 table generator") and a cross-platform port "FluidSynth" that depends on the prior.

Fortunately, this complexity isn't needed! (this is a relief).

The table generator can be built and run inline with the local FluidSynth build, all without x86 hardware. We're successfully building FluidSynth 2.2.3, including the table genrator, on the Raspberry Pi and MacOS M1.

Here's how to build and run the table generator first thing, after fetching the sources.

1. cd fluidsynth-2.2.3/src
2. In one-shot, compile gentables\gen_conv.c, gentables\gen_rvoice_dsp.c, and gentables\make_tables.c into a make_tables executable. Some minor notes:
   • The current src directory should be part of the include paths (-I.)
   • Building w/ GCC or Clang needs libm (-lm), while MSVC needs _USE_MATH_DEFINES defined.
   • Here's the full example with GCC: gcc gentables/gen_conv.c gentables/gen_rvoice_dsp.c gentables/make_tables.c -o make_tables.exe -I. -lm so msvc is probably pretty similar.
3. You now have make_tables.exe in the current directory (src)
4. Run it, passing the current directory as an argument: make_tables.exe ./
5. This will generate two headers: fluid_conv_tables.inc.h and fluid_rvoice_dsp_tables.inc.h
6. The generated headers are used by these corresponding sources:
   • ./fluidsynth-2.2.3/src/rvoice/fluid_rvoice_dsp.c
   • ./fluidsynth-2.2.3/src/utils/fluid_conv.c
7. All that's left is making sure this top-level src directory is part of the include path(s) when compiling the project itself.

The compiled make_table executable runs on all platforms supported by DOSBox Staging:

• PPC
• x86-32
• x64
• Raspberry Pi (ARM 32bit and ARM 64bit)
• Apple M1 (ARM 64bit)

[JackBoosY]

I will take over this PR.

[kcgen]

I will take over this PR.

Thank you @NancyLi1013 for all the work to get it to this point, and to @JackBoosY for continuing 👍

[github-actions]

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 13156b1cf6e2bbc02f02d74db905c63cad8b955d -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 011a7c6..ba607f9 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -2197,8 +2197,8 @@
       "port-version": 2
     },
     "fluidsynth": {
-      "baseline": "2.2.1",
-      "port-version": 1
+      "baseline": "2.2.3",
+      "port-version": 0
     },
     "fmem": {
       "baseline": "c-libs-2ccee3d2fb",
diff --git a/versions/f-/fluidsynth.json b/versions/f-/fluidsynth.json
index d129246..1494e96 100644
--- a/versions/f-/fluidsynth.json
+++ b/versions/f-/fluidsynth.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "bd8159cebc597c1a3be328f28dbaadef9ba4f6af",
+      "version": "2.2.3",
+      "port-version": 0
+    },
     {
       "git-tree": "d9e09f93f0880db415faf7f65f4dd483a5b2882a",
       "version": "2.2.1",

[github-actions]

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 14543ba18235846803ab1d3534cf2f63c6024b20 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 011a7c6..ba607f9 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -2197,8 +2197,8 @@
       "port-version": 2
     },
     "fluidsynth": {
-      "baseline": "2.2.1",
-      "port-version": 1
+      "baseline": "2.2.3",
+      "port-version": 0
     },
     "fmem": {
       "baseline": "c-libs-2ccee3d2fb",
diff --git a/versions/f-/fluidsynth.json b/versions/f-/fluidsynth.json
index d129246..1494e96 100644
--- a/versions/f-/fluidsynth.json
+++ b/versions/f-/fluidsynth.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "bd8159cebc597c1a3be328f28dbaadef9ba4f6af",
+      "version": "2.2.3",
+      "port-version": 0
+    },
     {
       "git-tree": "d9e09f93f0880db415faf7f65f4dd483a5b2882a",
       "version": "2.2.1",

[github-actions]

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 14543ba18235846803ab1d3534cf2f63c6024b20 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 011a7c6..ba607f9 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -2197,8 +2197,8 @@
       "port-version": 2
     },
     "fluidsynth": {
-      "baseline": "2.2.1",
-      "port-version": 1
+      "baseline": "2.2.3",
+      "port-version": 0
     },
     "fmem": {
       "baseline": "c-libs-2ccee3d2fb",
diff --git a/versions/f-/fluidsynth.json b/versions/f-/fluidsynth.json
index d129246..c3d28d0 100644
--- a/versions/f-/fluidsynth.json
+++ b/versions/f-/fluidsynth.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "2ec966c18425466e8c120c69a935c0a7c37e19ba",
+      "version": "2.2.3",
+      "port-version": 0
+    },
     {
       "git-tree": "d9e09f93f0880db415faf7f65f4dd483a5b2882a",
       "version": "2.2.1",

[JackBoosY]

Since the upstream use ExternalProject in its cmake files, we need to call twice vcpkg_cmake_* and always disable gentables in their cmake config file.

[dg0yt]

A split seems better.

[dg0yt]

The logs from this config and install stage will be overwritten by the regular build stage. This is a problem when a failure in the second stage is caused by an unnoticed misbehaviour in the first stage.

[JackBoosY]

We don't have a better way, log_base is not an options with vcpkg_cmake_install currently.

[dg0yt]

Option A) split port.
Option B) patch build system.

Considering that you want to add the tool to the PATH and run i, I don't understand how it can work in a cross build without splitting.

[JackBoosY]

@dg0yt Tool gentables is only used in this port, so split it to a new port isn't a good idea.
For option B, such a big change is not worth it.
In the cross build. it will prepend the host path to environment PATH so cmake will find the host tool first.

[BillyONeal]

@strega-nil and I discussed this and agree 1 port has better "ergonomics" but also that dropping the logs is problematic. The right thing is probably to add disambiguator flags to vcpkg_cmake_xxx so that they don't stomp on each other in situations like this. @dg0yt How would you feel about merging this as is and starting a new PR immediately following which adds that?

[BillyONeal]

I see that I don't need to open that can of worms; we can call the existing vcpkg_cmake_build which does support LOGFILE_BASE.

[JackBoosY]

Ping @dg0yt @BillyONeal for review this again.

[BillyONeal]

I would prefer to have the the PR adding the log disambiguators at least staged before merging this. If nobody looks at that before I return to the office on Monday, 2021-12-27, I will take a look at it.

[JackBoosY]

I checked the file list, it;s okay.

[BillyONeal]

Thanks for the review @JackBoosY