We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 522 - SDL_image should use .pc files when possible
Summary: SDL_image should use .pc files when possible
Status: RESOLVED FIXED
Alias: None
Product: SDL_image
Classification: Unclassified
Component: misc (show other bugs)
Version: unspecified
Hardware: All All
: P2 enhancement
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-12-10 16:47 UTC by Mike Frysinger
Modified: 2009-10-05 21:54 UTC (History)
0 users

See Also:


Attachments
sdl-image-find-lib-cross.patch (925 bytes, patch)
2007-12-29 18:12 UTC, Mike Frysinger
Details | Diff
libsdl-sdl-m4-pkg-config.patch (4.56 KB, patch)
2007-12-29 19:07 UTC, Mike Frysinger
Details | Diff
libsdl-sdl-m4-pkg-config.2.patch (4.72 KB, patch)
2009-10-03 00:24 UTC, Mike Frysinger
Details | Diff
sdl-image-pkg-config-png.patch (838 bytes, patch)
2009-10-04 22:28 UTC, Mike Frysinger
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Frysinger 2007-12-10 16:47:36 UTC
it's much easier to manage .pc files for cross-compiling setups than it is for random *-config scripts ... the cross-compiled pkg-config files can all be easily/tightly controlled and separate from the host pkg-config files.  the *-config files however are to be found in $PATH and can easily pick the wrong one.

can we get the sdl.m4 macro updated so that it checks for libsdl.pc first via the standard PKG_CHECK_MODULES() m4 macro, and if that fails, fall back to the normal sdl-config voodoo.

along these lines, the png code in configure.in should be updated to use PKG_CHECK_MODULES() rather than parsing the headers/libs directly ... the current code can break easily when libpng requires other libs (like -lz and/or -lm).
Comment 1 Sam Lantinga 2007-12-28 08:52:56 UTC
I'm not very familiar with the pkg-config system.  Can you supply a patch?

Thanks!
Comment 2 Mike Frysinger 2007-12-29 18:12:10 UTC
Created attachment 240 [details]
sdl-image-find-lib-cross.patch

sorry, i didnt see your reply

this patch makes it so find_lib() does not look in /usr/local/lib and /usr/lib when cross-compiling as that can easily cause troubles
Comment 3 Mike Frysinger 2007-12-29 19:07:08 UTC
Created attachment 241 [details]
libsdl-sdl-m4-pkg-config.patch

here's the fun one ... this updates libsdl's sdl.m4 so that it uses sdl.pc (and pkg-config) before sdl-config

the order preference is:
 - use sdl configure options first from user
 - try to use sdl.pc/pkg-config
 - try to use sdl-config/SDL_CONFIG

so really all this does is insert pkg-config before the normal default logic
Comment 4 Sam Lantinga 2009-10-02 05:32:45 UTC
I applied your configure.in patch, thanks!

The sdl.m4 patch didn't cleanly apply though, can you update it for the latest SDL snapshot?
http://www.libsdl.org/tmp/SDL-1.2.zip

Thanks!
Comment 5 Mike Frysinger 2009-10-03 00:24:03 UTC
Created attachment 382 [details]
libsdl-sdl-m4-pkg-config.2.patch

applied rev 3493 and rev 3532 manually to get the patch to apply
Comment 6 Sam Lantinga 2009-10-03 03:14:33 UTC
Are you creating the diff against the current SDL_image in subversion?

sam-lantingas-mac-pro:SDL_image hercules$ patch -l -p2 <~/Downloads/libsdl-sdl-m4-pkg-config.2.patch 
patching file acinclude/sdl.m4
Hunk #1 FAILED at 19.
Comment 7 Mike Frysinger 2009-10-03 08:06:20 UTC
this bug started out as "SDL_image should do XXX", but the m4 problems SDL_image isnt specific to it.  the problematic code comes from SDL itself, so the patch applies to trunk/SDL/sdl.m4.  once the original version gets fixed, it'll propagate to all the other packages.
Comment 8 Sam Lantinga 2009-10-03 21:42:33 UTC
It doesn't quite work here...
./configure: line 20603: syntax error near unexpected token `SDL,'
./configure: line 20603: `    PKG_CHECK_MODULES(SDL, sdl >= $min_sdl_version,'
Comment 9 Mike Frysinger 2009-10-03 22:48:43 UTC
do you not have pkg-config installed on your system ?  it provides that m4 macro via its pkg.m4 ...

guess the AM_PATH_SDL should be updated like so:
@@ -10,5 +10,6 @@
 dnl
 AC_DEFUN([AM_PATH_SDL],
-[dnl
+[AC_REQUIRE([PKG_PROG_PKG_CONFIG])
+dnl
 dnl Get the cflags and libraries from the sdl-config script
 dnl
Comment 10 Sam Lantinga 2009-10-03 22:57:19 UTC
Okay, I tried that...

configure.in:87: warning: PKG_PROG_PKG_CONFIG is m4_require'd but not m4_defun'd
acinclude/sdl.m4:203: AM_PATH_SDL is expanded from...
configure.in:87: the top level
configure.in:87: warning: PKG_PROG_PKG_CONFIG is m4_require'd but not m4_defun'd
acinclude/sdl.m4:203: AM_PATH_SDL is expanded from...
configure.in:87: the top level
configure.in:87: warning: PKG_PROG_PKG_CONFIG is m4_require'd but not m4_defun'd
acinclude/sdl.m4:203: AM_PATH_SDL is expanded from...
configure.in:87: the top level
configure.in:1: error: possibly undefined macro: dnl
      If this token and others are legitimate, please use m4_pattern_allow.
      See the Autoconf documentation.
configure:20608: error: possibly undefined macro: AC_MSG_RESULT
Comment 11 Mike Frysinger 2009-10-04 09:57:36 UTC
adding the AC_REQUIRE wont fix your original problem (no pkg.m4 on your system), it'll only make the failure obvious at an earlier point (from a sometimes-syntax error to an autoconf error)

you still need to install the pkgconfig program and the associated pkg.m4:
http://pkgconfig.freedesktop.org/wiki/

i dont think it's unreasonable to expect developers who are running autotools to have this installed
Comment 12 Sam Lantinga 2009-10-04 11:12:06 UTC
Okay, I applied your changes to sdl.m4 to SDL 1.3 and the SDL_* libraries I maintain, and added pkg.m4 to the libraries.

I didn't want to make that change to SDL 1.2 because anyone who updates to the release will suddenly have an unexpected pkg.m4 dependency.

Can you provide a patch for the png support?  It should fall back to the current system if pkg-config isn't available.

Thanks!
Comment 13 Mike Frysinger 2009-10-04 22:28:31 UTC
Created attachment 395 [details]
sdl-image-pkg-config-png.patch

i think the current handling of autotool files is broken ... but i guess it doesnt matter since i'm not the SDL maintainer.  since you're including pkg.m4 in svn, no one who updates will have a dependency on pkg.m4.

at any rate, this patch should convert SDL_image's png lookup to pkg-config
Comment 14 Sam Lantinga 2009-10-05 00:55:09 UTC
Should there be equivalent changes for libjpeg and libtiff?

Do you have suggestions on improving the autotools support?

Thanks!
Comment 15 Mike Frysinger 2009-10-05 09:28:43 UTC
the jpeg/tiff packages dont provide .pc files, so they wouldnt work

imo, generated autotool files shouldnt be in svn.  it does mean that people need more development packages installed in order to use the development svn, but i think that is how it should be.  i dont imagine you want to impose these higher requirements on people which is why you copy so much to acinclude/ and such.
Comment 16 Sam Lantinga 2009-10-05 21:54:17 UTC
Yes, that's true.  My general philosophy is to try to work on as many different environments as possible. :)