| Summary: | Wrong #include for tremor (ivorbisfile.h) | ||
|---|---|---|---|
| Product: | SDL_mixer | Reporter: | Maarten ter Huurne <maarten> |
| Component: | misc | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | ||
| Version: | 2.0.0 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Attachments: | Patch to correct Tremor include path | ||
Like the modplug patch in bug 2136, this patch breaks building on all non-UNIX systems, which don't have a system installed copy of tremor. $ find . -name ivorbisfile.h ./external/libvorbisidec-1.2.1/ivorbisfile.h If nobody has a better idea, I'll leave the patch for the include path to the Linux distribution maintainers. I think the root of the problem is that in general there is no guarantee that you can build applications against the source tree of a library, while that is what is currently being attempted for some platforms. Do all of the other systems even use Tremor? I would assume anything targeting x86(-64) would use the normal Vorbis library instead. That is also what I see under "VisualC/external/" and "Xcode/Frameworks/". So I think only Android would break if you change the #include. My guess is that the simplest way to fix Android is to run "configure ; make ; make install" using DESTDIR to redirect the install to a non-system directory. If for some reason autotools doesn't play well with the Android NDK, you could have "Android.mk" create an "include/" directory and copy the required headers there in their expected subdirectories, or create that directory manually (as seems to have been done for Visual C and Xcode). A less elegant solution would be to use "#ifdef __ANDROID__" around the #include path. In any case, I don't think having the distros patch this is a good solution, since that would essentially be shipping a broken SDL_mixer and then requiring packagers to fix it with patches. I agree, which is why I haven't closed this bug. Maybe having a single local "install" directory shared by all non-UNIX builds is the way to go? For some builds (Visual C, Xcode) library binaries are included with SDL2_mixer; to avoid problems it would be best if the headers used in those builds always come from the same library version that the binaries were compiled from. So taking headers from a local "install" directory would require updating all library binaries at the same time.
Some libraries rely on headers generated by their build; I don't think this is the case for the libraries used by SDL2_mixer, but if you're looking for a generic solution it would be something to consider.
I think it would be best to deal with several different build categories in different ways:
- when binaries are provided (Visual C, Xcode), provide headers that match those binaries
- when external libraries can be built via the build system of the libraries themselves (GNU autotools for most), let those builds (Linux cross-distro) create the local "install" dir using "make install DESTDIR=the_install_dir"
- when external libraries are built from source but not using the build system of the libraries themselves (Android), use either a prepared local "install" dir or let the alternative build ("Android.mk") create one by copying headers from the library source trees
- when building against system-wide libraries (Linux one-distro, *BSD), use system-wide headers (via pkg-config and default include paths)
Okay, I thought through a good solution, thanks! https://hg.libsdl.org/SDL_mixer/rev/6a5e6d8d6a35 |
Created attachment 1732 [details] Patch to correct Tremor include path The Tremor header should be included as "tremor/ivorbisfile.h" instead of just "ivorbisfile.h". The configure script does use the right path, but the code does not. See attached patch.