fix detecting & using utf8c++ 4.0.0 or newer#346
Conversation
|
Soooooo… it seems my solution doesn't actually work with a distro-packaged 3.x. Interesting. Will have to switch that case back to |
6cbbd23 to
67a2dbb
Compare
|
Alright, 67a2dbb works fine on:
…and the checks agree. @marillat & @ReillyBrogan I'd appreciate some testing on your end, too. Thanks a lot! |
|
While we're at it we could also bump the version we get from git in case of "no distro-packaged found" to 4.0.x & use |
|
Build fine for me, but with a cmake warning: |
There is a QUIET parameter that could be used, in both calls. In the end we show in the summary what we are using (system or internal). |
robUx4
left a comment
There was a problem hiding this comment.
OK for me. We need the same in the master branch.
|
After rebasing on v1.x we should see in the CI that the 4.x.x version is properly picked. |
Speaking as a packager I usually check the cmake output to see what new dependencies I may need to add to the build recipe. So at least one of them should continue to be not-QUIET so it's noticeable in the output. But honestly looking at repology utf8cpp >= 4.0 is already very widespread in the various distribution repos. It's in the latest stable Debian, the latest LTS Ubuntu, all supported Fedora releases, etc. Given that libebml appears to compile and work with it just fine why not just require 4.0 unconditionally? If a user is on a distribution that doesn't have it they can just enable networking so that fetch_content can pull down a 4.0 release. Another approach that could be used here is to use alias targets instead. Something like this: (note that I'm not sure what target the fetch_content for utf8cpp actually exports so that might need to be adjusted). Then if utf8cpp is ever referenced anywhere else in cmake you can just use |
`find_package` with version constraint `3.2.0` does not consider 4.x.y to be compatible. Therefore also look for 4.x.y in a secondary `find_package` call. The fallback for cloning from git if neither version is found via `cmake` remains in place. Furthermore, linking against a distro-packaged 4.x.y doesn't seem to work with the existing `target_link_libraries(ebml PRIVATE $<BUILD_INTERFACE:utf8cpp>)` & requires `PRIVATE utf8cpp::utf8cpp`. However, that doesn't work with a git-cloned copy nor with distro-packaged 3.2.x as those still requires `PRIVATE $<BUILD_INTERFACE:utf8cpp>`. Therefore make the argument depend on availability of the `utf8cpp::utf8cpp` target: it is available only for distro-packaged 4.x.y. For the other two cases (distro-packaged < 4.x.y & cloned from git) we stay with `$<BUILD_INTERFACE:utf8cpp>`. fixes #344 ; see #345
67a2dbb to
7cf88e1
Compare
|
Thank y'all very much for your feedback. I've taken it into account & produced a new version that a) gets rid of the version requirement altogether and As for the version requirement: I had investigated using Therefore I opt not to restrict the version number at all. At the moment all known-deployed versions work just fine, and I say that we just deal with the bug reports of something breaking with utf8cpp 5.whatever when those come in. Furthermore, there are two cases wrt. the target to use:
Given that version ≥ 4 is the future, we can simply check if the target …and while I write this I realize I could've simply moved that check to the location of As before I've verified this to be working on:
In the meantime @robUx4 has also added some more checks which now test for some of the same combinations, too. Again I'd appreciate some feedback before I merge this (and do the same for the |
|
Oh, BTW, I wasn't able to make the |
robUx4
left a comment
There was a problem hiding this comment.
Not a fan of dropping the version check but I guess if that ever happens some distro will find out and reach out to us (but they usually silently patch).
|
Build fine for me with commit 7cf88e1 |
find_packagewith version constraint3.2.0does not consider 4.x.y to be compatible. Therefore also look for 4.x.y in a secondaryfind_packagecall.The fallback for cloning from git if neither version is found via
cmakeremains in place.Furthermore, linking against a distro-provided 4.x.y doesn't seem to work with the existing
target_link_libraries(ebml PRIVATE $<BUILD_INTERFACE:utf8cpp>)& requiresPRIVATE utf8cpp::utf8cpp. However, that doesn't work with a git-cloned copy; that still requiresPRIVATE $<BUILD_INTERFACE:utf8cpp>; so make that argument depend on the package detection, too.fixes #344 ; see #345