CMake: Properly declare target include directories for generated includes #554
Conversation
|
This is a very useful fix. Integrated in master. Danke schön. |
|
Reopening. Verification failed, unfortunately, and I had to revert. |
|
Oh yea, I did not test this on mac. I think all we need is the same change a couple of lines down in the "framework" section. But I am not sure. |
Same here -- but hey, the verification bots did. |
|
Is there a way we can try running the ci in the pr? I don't like to make the changes blind. |
I understand. Sorry I haven't set up a GitHub action for verification of pull requests, because of a bunch of reasons that are too long to explain here. One way for you to go is that you submit, then I'll verify it offline (i.e. run it on my own Mac machine), and let you know if the problem is fixed. Another way for you would be to set up your own free Travis CI account and link it to your own libpng clone. And then, the automation should automatically pick up your commit, without any further action from my side. In my opinion, that's a bit too involved for just one contributed commit. My advice: do make the changes blind, even if you don't have a Mac machine available. And then, Trust Me™ 😝 |
…udes. Previously the non targeted `include_directories()` was used, which had issue when using the `png_static` target in a submodule.
Green-Sky
force-pushed
the
cmake_include_fix
branch
from
f45bc97 to
7b51797
Compare
|
I did the change blind. Go ahead and test it :) |
Confirmed, not only by my machine, but also by Travis CI. Meanwhile, AppVeyor CI is still pending, but I have a feeling it's gonna to work. (Famous last words.) |
Previously the non targeted
include_directories()was used, which had issue when using thepng_statictarget in a submodule.I came across this issue why trying to link
sdl3_imagewith static vendored libs, which links against png_static.For some reason
png_sharedexported the binary include path just fine, butpng_staticnot, resulting inpnglibconf.hnot being found.So I cleaned up the cmake a little and explicitly added the include dirs to the targets directly.