sdl2_mixer: make libmodplug required instead of optional #21036
Conversation
|
ref #20746 |
Formula/sdl2_mixer.rb
Outdated
| @@ -18,7 +18,7 @@ class Sdl2Mixer < Formula | |||
| depends_on "flac" => :optional | |||
| depends_on "fluid-synth" => :optional | |||
| depends_on "libmikmod" => :optional | |||
| depends_on "libmodplug" => :optional | |||
| depends_on "libmodplug" | |||
There was a problem hiding this comment.
needs to go above libvorbis
|
also please bump the revision |
|
rebuild is not the same as revision. you shouldn't modify the bottle block manually. revision goes at formula global scope. |
|
@ilovezfs you mean :revision ? |
|
nope that's a component of urls
|
Formula/sdl2_mixer.rb
Outdated
| @@ -2,12 +2,12 @@ class Sdl2Mixer < Formula | |||
| desc "Sample multi-channel audio mixer library" | |||
| homepage "https://www.libsdl.org/projects/SDL_mixer/" | |||
| url "https://www.libsdl.org/projects/SDL_mixer/release/SDL2_mixer-2.0.2.tar.gz" | |||
| revision 1 | |||
|
Thanks. Please squash this down to one commit with the commit subject |
ilovezfs
changed the title
|
@BrewTestBot test this please |
Formula/sdl2_mixer.rb
Outdated
| @@ -30,14 +31,13 @@ def install | |||
| --disable-music-flac-shared | |||
| --disable-music-midi-fluidsynth-shared | |||
| --disable-music-mod-mikmod-shared | |||
| --disable-music-mod-modplug-shared | |||
There was a problem hiding this comment.
actually we want to keep this disabled so that the linkage is explicit
There was a problem hiding this comment.
I'm not sure I understand, this defines the -DMODPLUG_DYNAMIC, without this we need to compile libmodplug statically with mrboom, but we don't have a libmodplug.a to do that.
There was a problem hiding this comment.
The difference is not whether it's static or dynamic but whether the path to the dylib is explicit or not.
There was a problem hiding this comment.
With --disable-music-mod-modplug-shared, here's what you get:
iMac-TMP:~ joe$ brew linkage sdl2_mixer
System libraries:
/System/Library/Frameworks/AudioToolbox.framework/Versions/A/AudioToolbox
/System/Library/Frameworks/AudioUnit.framework/Versions/A/AudioUnit
/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
/System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices
/usr/lib/libSystem.B.dylib
Homebrew libraries:
/usr/local/opt/libmodplug/lib/libmodplug.1.dylib (libmodplug)
/usr/local/opt/libvorbis/lib/libvorbis.0.dylib (libvorbis)
/usr/local/opt/libvorbis/lib/libvorbisfile.3.dylib (libvorbis)
/usr/local/opt/sdl2/lib/libSDL2-2.0.0.dylib (sdl2)
iMac-TMP:~ joe$
There was a problem hiding this comment.
But it does compile sdl2_mixer differently if you look at music_modplug.c, is this OK?
There was a problem hiding this comment.
And I assume you tested it without the --with-libmodplug?
There was a problem hiding this comment.
By which I mean, are you sure the new dependency is actually required?
There was a problem hiding this comment.
|
@frranck Thanks for your first contribution to Homebrew! Without people like you submitting PRs we couldn't run this project. You rock! |
ref #20746
brew install --build-from-source <formula>, where<formula>is the name of the formula you're submitting?brew audit --strict <formula>(after doingbrew install <formula>)?