mpv
mpv copied to clipboard
libmpv: use correct header directory name
libmpv headers are installed to mpv/, so why are we pretending it is libmpv?
Fixes documentation, libmpv meson dependency, tests.
Read this before you submit this pull request: https://github.com/mpv-player/mpv/blob/master/DOCS/contribute.md
Reading this link and following the rules will get your pull request reviewed and merged faster. Nobody wants lazy pull requests.
Download the artifacts for this pull request:
My only slight opposition is that it's not immediately obvious what the directory mpv is for to someone not familiar with the codebase.
I agree, but this imho is the most frictionless change to make libmpv_dep work correctly. Now it is impossible to reference the internal headers.
What happens if I have mpv/client.h installed to the system include path? Does this PR guarantee that when compling mpv, the client headers from the mpv source is used instead of the one from system include path (which can be from a wrong version)?
What happens if I have
mpv/client.hinstalled to the system include path? Does this PR guarantee that when compling mpv, the client headers from the mpv source is used instead of the one from system include path (which can be from a wrong version)?
Yes, it is Meson job to add -I in proper place, so that internal headers take precedence.
Although, I had problems with Meson in regards to subproject linking, where linker would pick installed version instead of internal one. Again, Meson responsibility.
For this to make sense, we probably should have any users that are interested in using mpv as a subproject. Maybe mpv-build or mpv-examples could test something like that. Can be revisited later.
I hit this issue again, when trying to link with libmpv.
Maybe mpv-build or mpv-examples could test something like that.
I was wrong here. We have multiple tests, that already fully test linking to dependency. After this commit of course, because before they were manually linked.
My only slight opposition is that it's not immediately obvious what the directory mpv is for to someone not familiar with the codebase.
Well, we install those headers to mpv, so it's even worse to understand from outside of the repository?
I tested it with external project too. @Dudemanguy @sfan5 do you have strong objection about this? I think it is correct way of doing things in meson and while historically meson was not alone, now it it, so we should support _dep and subprojects.
I was just unhappy with a top level mpv folder. include/mpv is good.
+1 for include/mpv I like that better than current as well
include/mpv works for me too.
Thank you.