[fluidsynth] Update to 2.3.2#30625
Merged
BillyONeal merged 9 commits intomicrosoft:masterfrom Apr 19, 2023
Merged
Conversation
The usage requirements patch is no longer required as a similar patch has been merged upstream.
- List out the options per-platform. - Set the proper options for Android.
Contributor
Author
|
The linux CI errors seem to be related to ALSA: It should be noted that upstream has switched from I can't seem to be able to reproduce the failure locally on the |
jimwang118
added
the
category:port-update
Contributor
|
I cloned your branch code locally, and then compiled it on Ubuntu and had the same problem as CI. |
Contributor
Author
|
Thank you for the confirmation. It sorts of look like an ALSA issue, where ALSA depends on For the time being, what do you think about adding the following patch: diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 114fa163..2ea55ddc 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -365,7 +365,7 @@ if ( PULSE_SUPPORT )
endif()
if ( TARGET ALSA::ALSA AND ALSA_SUPPORT )
- target_link_libraries ( libfluidsynth-OBJ PUBLIC ALSA::ALSA )
+ target_link_libraries ( libfluidsynth-OBJ PUBLIC ALSA::ALSA ${CMAKE_DL_LIBS} )
endif()
if ( TARGET PortAudio::PortAudio AND PORTAUDIO_SUPPORT )While I still cannot reproduce the issue locally, the output looks sane to me:
|
FtZPetruska
force-pushed
the
bump-fluidsynth-2.3.2
branch
from
8f09cf1 to
71356cb
Compare
Contributor
|
The features and usage have been tested successfully locally. |
jimwang118
added
the
info:needs-maintainer-attention
BillyONeal
reviewed
Apr 19, 2023
ports/fluidsynth/portfile.cmake
Outdated
| SOURCE_PATH "${SOURCE_PATH}" | ||
| OPTIONS | ||
| "-DVCPKG_HOST_TRIPLET=${HOST_TRIPLET}" | ||
| -DVCPKG_HOST_TRIPLET="${HOST_TRIPLET}" |
Member
There was a problem hiding this comment.
I don't believe this transform is correct. At least, https://cmake.org/cmake/help/latest/manual/cmake-language.7.html#command-arguments seems to indicate that this will cause the argument to be broken into 2 arguments?
Contributor
Author
There was a problem hiding this comment.
I was checking and while the resulting configure command looks odd "-DVCPKG_HOST_TRIPLET="arm64-osx"", the value in the cache is properly set:
//No help, variable specified on the command line.
VCPKG_HOST_TRIPLET:UNINITIALIZED=arm64-osxBut it's probably safer to keep it fully quoted as it used to be.
Contributor
There was a problem hiding this comment.
Inner quotes are part of the CMake value and work with some build systems and shells. It is not portable. That's why we want quotes around the CMake value so that CMake can treat the value as needed when forwarding it to specific build systems and shells.
BillyONeal
reviewed
Apr 19, 2023
|
|
||
| if ( TARGET ALSA::ALSA AND ALSA_SUPPORT ) | ||
| - target_link_libraries ( libfluidsynth-OBJ PUBLIC ALSA::ALSA ) | ||
| + target_link_libraries ( libfluidsynth-OBJ PUBLIC ALSA::ALSA ${CMAKE_DL_LIBS} ) |
Member
There was a problem hiding this comment.
Should ALSA::ALSA already be including CMAKE_DL_LIBS if necessary? (Should that port be fixed rather than this one?)
Contributor
Author
There was a problem hiding this comment.
I am having a hard time reproducing the error in question locally, but I agree that it seems a lot like an ALSA issue. Its pkg-config file does include dl, but the upstream FindALSA module from Kitlab does not seem to search for any potential transitive usage requirements.
I will see about making a vcpkg-cmake-wrapper for ALSA.
Member
|
Thanks for the update! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
./vcpkg x-add-version --alland committing the result.The
add-usage-requirementspatch is no longer necessary as upstream now handles it.Update the logic for setting the CMake options:
MAYBE_UNUSED_VARIABLESmore easily.