Restore libtimidity #7454
Restore libtimidity #7454
Conversation
Milek7
force-pushed
the
libtimidity
branch
2 times, most recently
from
0418841 to
a425d99
Compare
|
Also needs |
Milek7
force-pushed
the
libtimidity
branch
2 times, most recently
from
0462942 to
e6498c5
Compare
|
I would like that we only accept this if we have some CI running this part of the code to validate it is still functioning correctly. Given there is no Debian port (which the CI runs), this might be a bit tricky. As mentioned on IRC, as this is part of EmScripten wasm port, possibly it is good to combine that: upstream wasm port, re-introduce libtimidity, and have a Docker compiling for wasm on every compile (or every release / night). That makes sure libtimidity keeps working. Without that, I am not sure we should accept this PR upstream. Having code that we cannot test is just asking for trouble :( |
|
Ok. |
|
I ... do not understand why it is broken. I would think too that a revert would "just work". Hmm .. puzzling. Looked at it for 10+ minutes, can't see what is wrong :( |
|
Same here, I fail to see why it tries to compile libtimidity.cpp when libtimidity is not found. The code clearly says it shouldn't. |
configure
Outdated
| @@ -121,6 +121,7 @@ AWKCOMMAND=' | |||
| "'$os'" != "CYGWIN" && "'$os'" != "MSVC") { next; } | |||
| if ($0 == "MSVC" && "'$os'" != "MSVC") { next; } | |||
| if ($0 == "DIRECTMUSIC" && "'$with_direct_music'" == "0") { next; } | |||
| if ($0 == "LIBTIMIDITY" && "'$with_libtimidity'" == "0" ) { next; } | |||
There was a problem hiding this comment.
The remove commit had this line:
if ($0 == "LIBTIMIDITY" && "'$libtimidity'" == "" ) { next; }
There was a problem hiding this comment.
With original line it doesn't use libtimidity even when detected. Commited fix for detect_pkg_config from TrueBrain.
There was a problem hiding this comment.
That was still wrong, "'$libtimidity_config'" == "" here and unchanged detect_pkg_config should be right.
|
If you don't mind, I am going to close this Pull Request. Not because I think it is bad, but because I would rather focus on bringing this in together with Emscripten (as mentioned earlier). I will also work on integrating that with the CI, so this library gets tested on a regular basis :) |
extmidi doesn't work on platforms such as emscripten, and fluidsynth requires tens of dependencies (including glib)