dbry / WavPack

WavPack encode/decode library, command-line programs, and several plugins
BSD 3-Clause "New" or "Revised" License
363 stars 66 forks source link

Use target_link_options() to specify exported symbol lists #102

Closed colugomusic closed 3 years ago

colugomusic commented 3 years ago

These arguments should be added using target_link_options instead, otherwise CMake will append them to the link command when statically linking WavPack to a dynamic library. If the dynamic library specifies its own lists then this will cause a linker error. Or if it doesn't then any exported symbols will be erroneously hidden.

dbry commented 3 years ago

Sorry about my delay on this! I'm not ignoring it but just wanted to wait until the big autotools update went in (even though I know they're not strictly related). Thanks!

dbry commented 3 years ago

https://travis-ci.org/github/dbry/WavPack/builds/769133476

And it looks like it broke the Travis CI, but that error might be fixed when the Ubuntu and XCode versions are updated in the other PR, so I'll try again once that's finishished.

colugomusic commented 3 years ago

No worries. Yes target_link_options requires at least cmake 3.13, so the update to Ubuntu focal should fix the Travis build I think.

dbry commented 3 years ago

I tried locally and it builds fine, but the Travis CI build specifies -DBUILD_SHARED_LIBS=ON and that is broken only with your change. You can see the output at https://travis-ci.org/github/dbry/WavPack/jobs/770187642.

colugomusic commented 3 years ago

I can reproduce it on my Linux machine. Removing the single quotes around the file path seems to fix it and the symbols are exported correctly. I'm about to go to bed though but if you like I can open a new pull request tomorrow.

dbry commented 3 years ago

No hurry. Just push to this branch or do a new PR, either way. Thanks!

colugomusic commented 3 years ago

All passing now https://travis-ci.org/github/dbry/WavPack/builds/770263123

dbry commented 3 years ago

Thanks!