search

xiph / vorbis

Reference implementation of the Ogg Vorbis audio format.
BSD 3-Clause "New" or "Revised" License
467 stars 188 forks source link

CMake: Link libm to test binaries if available #82

Closed diizzyy closed 3 years ago

diizzyy commented 3 years ago

If libm is available link it to test binaries which will otherwise fail to compile Tested on FreeBSD 13-STABLE

Signed-off-by: Daniel Engberg daniel.engberg.lists@pyret.net

diizzyy commented 3 years ago

@evpobr

diizzyy commented 3 years ago

Not related? This issue occured before my PR

https://github.com/xiph/vorbis/commit/83a82dd9296400d811b78c06e9ca429e24dd1e5c --> https://ci.appveyor.com/project/rillian/vorbis/builds/36177388 --> https://ci.appveyor.com/project/rillian/vorbis/builds/36177388/job/t06shr5lh00ejakb

evpobr commented 3 years ago

Can you add that define for MSVC to check it?

rillian commented 3 years ago

IIRC this is correct. lib/os.h has a fallback definition of M_PI which allows the main library to compile, but test/util.c doesn't include it.

We could address the issue by

I'm not sure what the best approach is here. Happy to see a proposal.

diizzyy commented 3 years ago

I have no idea idea to fix it for MSVC and additionally no test box so unfortunately not.

evpobr commented 3 years ago

I guess it is already done in #77 and still not merged.

Probably you need to make pull request to https://gitlab.xiph.org/xiph/vorbis.

rillian commented 3 years ago

We've addressed the M_PI issue. Could you please rebase to confirm appveyor failure goes away?

diizzyy commented 3 years ago

Compiles fine now, thanks! :-)

diizzyy commented 3 years ago

@rillian Anything else you want me to do?

rillian commented 3 years ago

Nope. Merged upstream in 84c023699cdf023a32fa4ded32019f194afcdad0. Thanks for the patch!