CMake: INCLUDE_DIRECTORIES -> TARGET_INCLUDE_DIRECTORIES - #4284

Why are you dropping the explicit linkages? This looks weird: libgit2_clar does not link to libgit2 and this suggests to me that it does. It's already not obvious that we do not link to libgit2 (we can't, we need to test internal functions). This makes that less obvious still.

If you're worried about this not being sufficiently DRY, then why not set a variable to this set of libraries and then let TARGET_LINK_LIBRARIES point to both?

I'd ask for not touching that part at all regarding linkage, as I'm already doing this in #4282 in the way @ethomson asks for. The other part regarding the includes is still viable, though

Agree. 👍 Thanks @pks-t

Ok, I will fix it. But it seems quite strange for me that tests for the library don't link to that library.

Again - they can't, at least not without making everything public. We want to be able to test our buffer implementation and our posix emulation on Windows, and we do not want those to be a public, exported part of the library.

I have reworked my commits to leave git history clean.

🎉 Awesome, thanks, @AndreyG !

@ethomson thank you for clarification!

I'd still prefer to pull only the commit regarding target_include_directories to keep conflicts with #4282 as small as possible.

@pks-t It will be OK for me if you just cherry-pick my commit 905d80f regarding target_include_directories to your branch #4282 and then close this pull request.

@AndreyG That's certainly helpful, thank you! I'll try to get Windows building with #4282 tomorrow, and if so I'll cherry-pick this. Otherwise I'll manually pick the single commit and merge

Manually merged via c26ce78. I've made a follow-up commit to also use TARGET_INCLUDE_DIRECTORIES for libgit2_clar.

Thanks for your work!