games-fps/gzdoom: Prevent build error on -fluidsynth #12248
Conversation
Copyright policy changePlease note that on 2018-09-15 Trustees have approved new Gentoo copyright policy. All contributions made to Gentoo need to follow this policy. If you include the Signed-off-by line in your commit message, you indicate that you have read the policy and agree to its terms. For more detailed explanation, please see the new Gentoo copyright policy explained article. Pull Request assignmentSubmitter: @vilhelmgray games-fps/gzdoom: @vilhelmgray, @gentoo/proxy-maint, @gentoo/games Linked bugsBugs linked: 687922 In order to force reassignment and/or bug reference scan, please append Docs: Code of Conduct ● Copyright policy (expl.) ● Devmanual ● GitHub PRs ● Proxy-maint guide |
gentoo-bot
added
self-maintained
|
Seems it doesn't affect runtime, only fixes compilation, so revision bump is not needed. |
vilhelmgray
force-pushed
the
fix_gzdoom-4.1.3_fluidsynth
branch
4 times, most recently
from
d3960da
to
c23712e
Compare
FluidSynth is a dependency of GZDoom and cannot be disabled. Similarly, OpenAL is a dependency of GZDoom on Linux and cannot be disabled. Closes: https://bugs.gentoo.org/687922 Package-Manager: Portage-2.3.67, Repoman-2.3.14 Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
Closes: https://bugs.gentoo.org/687990 Package-Manager: Portage-2.3.67, Repoman-2.3.14 Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
vilhelmgray
force-pushed
the
fix_gzdoom-4.1.3_fluidsynth
branch
from
c23712e
to
9814415
Compare
Pull request CI reportReport generated at: 2019-06-13 12:10 UTC No issues found |
|
Thanks for getting this patch merged upstream. I did think the sound dependencies looked a little odd before, definitely better now. They should have used SDL_mixer instead, I added FluidSynth support to that years ago! 😝 Just one problem, the fluidsynth2 patch you're referencing here is missing. |
|
The fluidsynth2 patch was added as part of commit 8aa71216840d5cd875f4321a8a03ce8ed98375df. I think that commit is already in this branch -- or am I mistaken (I can rebase again on master if need be)?
The GZDoom upstream team has some reservations against the concept of dynamic linking with external libraries. You can see some of the discussion and their reasons in this thread: https://forum.zdoom.org/viewtopic.php?f=2&t=64633 However, this fluidsynth 2 patch is simple, so upstream may merge it in after some consideration. I'll keep an eye on it, and remove the patch if it's merged in time for the next version bump.
The soundfont patch only affects the GZDoom makefile -- it only introduces a configurable installation location for the GZDoom soundfonts -- so no runtime changes have occurred. |
| diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt | ||
| index 0bb16f39e..4ff15062d 100644 | ||
| --- a/src/CMakeLists.txt | ||
| +++ b/src/CMakeLists.txt |
There was a problem hiding this comment.
While this is nice for upstreaming, wouldn't doins be simpler for Gentoo purposes?
There was a problem hiding this comment.
That is a fair point, but with this patch I can match upstream in preparation for the next release (the patch has already been merged upstream). That should allow the upcoming version bump to be a simple cp rename and removal of this patch from the PATCHES list; with the doins approach, I'll have to remove the doins and reimplement the INSTALL_SOUNDFONT_PATH definition line again for the version bump.
Closes: https://bugs.gentoo.org/687922
Package-Manager: Portage-2.3.67, Repoman-2.3.14
Signed-off-by: William Breathitt Gray vilhelm.gray@gmail.com