Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure to add SDL's library directory before building SDL things. #327

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bentley
Copy link

@bentley bentley commented Aug 3, 2017

This fixes the build on OpenBSD, which otherwise hits this error:

[47/71] : && /usr/ports/pobj/audiality2-1.9.2/bin/cc  -O2 -pipe -g -g   test/CMakeFiles/wavestress2.dir/wavestress2.c.o  -o test/wavestress2 -L/usr/ports/pobj/audiality2-1.9.2/build-amd64  -L/usr/ports/pobj/audiality2-1.9.2/build-amd64/src -Wl,-rpath,/usr/ports/pobj/audiality2-1.9.2/build-amd64:/usr/ports/pobj/audiality2-1.9.2/build-amd64/src  -laudiality2 -lm -lSDL2 -Wl,-rpath-link,/usr/X11R6/lib:/usr/local/lib && :
FAILED: test/wavestress2 
: && /usr/ports/pobj/audiality2-1.9.2/bin/cc  -O2 -pipe -g -g   test/CMakeFiles/wavestress2.dir/wavestress2.c.o  -o test/wavestress2 -L/usr/ports/pobj/audiality2-1.9.2/build-amd64  -L/usr/ports/pobj/audiality2-1.9.2/build-amd64/src -Wl,-rpath,/usr/ports/pobj/audiality2-1.9.2/build-amd64:/usr/ports/pobj/audiality2-1.9.2/build-amd64/src  -laudiality2 -lm -lSDL2 -Wl,-rpath-link,/usr/X11R6/lib:/usr/local/lib && :
/usr/bin/ld: cannot find -lSDL2
cc: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

@olofson
Copy link
Owner

olofson commented Aug 3, 2017

Hmm... Only 'a2test' and 'apistress' should have direct dependencies on SDL or SDL2. If the others need to explicitly link with SDL, it's because the audiality2 library uses SDL (which I assume it does here), and there is something wrong with how the library is linked.

I suppose this might have something to do with the tests being built "locally," rather than pulling the properly installed A2 lib in via find_package()... I just want to be able to build the tests without first installing the A2 lib, but it's a bit of a dodgy hack. (See #322.)

@bentley
Copy link
Author

bentley commented Aug 4, 2017

Hm, is it really the case that the library is linked wrong? Trying pkg-config --libs on a number of libraries I have installed suggests many of them require programs to also link against libraries they depend on. Some of them even require explicitly linking to libm.

$ pkg-config --libs libwapcaplet
-L/usr/local/lib -lwapcaplet
$ pkg-config --libs libhubbub   
-L/usr/local/lib -lhubbub -lparserutils -liconv
$ pkg-config --libs libparserutils
-L/usr/local/lib -lparserutils -liconv
$ pkg-config --libs libdom        
-L/usr/local/lib -ldom -lexpat -lhubbub -lparserutils -liconv
$ pkg-config --libs vpx
-L/usr/local/lib -lvpx -lm

@olofson
Copy link
Owner

olofson commented Aug 4, 2017

Yeah, that's how it works, technically. Some platforms automatically pull in dependencies as an application loads a library. Others (like ours) don't, so pkg-config needs to ask what extra libs to link with. The problem here is that we (as an application build script) can't safely assume anything here; we really should depend on pkg-config or equivalent to figure out the correct way of linking.

What I get here:

$ uname -a
Linux hex 4.4.0-87-lowlatency #110-Ubuntu SMP PREEMPT Tue Jul 18 13:54:56 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
$ pkg-config audiality2 --libs
-L/usr -laudiality2 -lSDL2

If you've built A2 with SDL2 support (it's optional) on a platform that doesn't automatically pull in lib dependencies at run time, there should be an -lSDL2 in there, and there should be no need for the application build script to detect and pull SDL2 in explicitly.

But, this is not an application build script, so it's not doing the right thing here. If you use the commented out lines, under "# For stand-alone build, linking to the system A2 library" instead, I suspect it's going to work as intended - but then you need to build and install the A2 library before even running cmake on the tests and tools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants