search

tomahawk-player / tomahawk

Tomahawk, the multi-source music player
http://tomahawk-player.org
GNU General Public License v3.0
2.95k stars 367 forks source link

Use local cmake modules first (fix build with ECM >=5.72.0) #676

Closed a17r closed 3 years ago

a17r commented 3 years ago

The latest version of ECM gained a module that is overriding Tomahawk's own, but variables are incompatible.

I also have another commit queued up to copy the ECM FindTaglib.cmake module and adapt cmake, until we can depend on that ECM version itself and drop the local module. However I can't make sense of CheckTagLibFileName.cmake and if it is still necessary today. I haven't seen such a check in any other package using [a variant of] FindTaglib.cmake.

dschmidt commented 3 years ago

Cool. I'll merge.

I can tell you what that check does, it checks whether TagLib::FileName can be constructed with a wchar passed as argument. It's what's used on Windows for filenames, utf8 encoded filenames in char* won't work. No idea when what changed in taglib and if that check still makes any sense these days... it's basically just important to pass wchars on Windows when available. If neccessary we can certainly raise the required taglib version.

a17r commented 3 years ago

I think this is not required since depending on 1.8.0 in a2ea7e8b4ab6c82a4a2beff0169af7af28e21024.

Looking at taglib upstream commit https://github.com/taglib/taglib/commit/0b0cbc2e34b37a727f2c79c239b144222762b400#diff-ce2909b7443f76377d90e6459cd4fb06R39

dschmidt commented 3 years ago

Yeah it goes back to at least 1.5 or so. Feel free to replace the ifdef with simply #ifdef _WIN32 just like in taglib itself.