meson: use vapoursynth get-include to get include dir#18127
Conversation
|
/cc @CrendKing |
To build vf_vapoursynth headers are needed. With newer VS versions those are located in Python module, while for older they are installed in system. Try Python module first and fallback to pkg-config. Note that this requires vapoursynth to be available during build time. If you use venv or similar, make sure it's activated. Fixes: mpv-player#18126
This is what actually exposes VSScript4.h header we need. On newer versions we expect `get-include` to work, as vapoursynth-script.pc no longer exists.
|
Thanks for doing this. I wonder if we should suggest to VapourSynth to move their development-related files such as the headers and the pkg-config .pc file they ship to pkg-config like the old way? mpv can't be the only project suffering the blow. |
| vs_inc = vs_get_inc.stdout().strip() | ||
| if vs_get_inc.returncode() == 0 and fs.is_dir(vs_inc) | ||
| vs_incdirs = include_directories(vs_inc, is_system: true) | ||
| if cc.has_header('VSScript4.h', include_directories: vs_incdirs) |
There was a problem hiding this comment.
Since the include dir path is returned by VapourSynth itself, I feel we should just trust it and no need to verify the actual file existence. The likelihood user has both a broken R74 and a working R56 installed in the same system should be low.
There was a problem hiding this comment.
See #18126 (comment), I even added fs.is_dir(vs_inc) because of it.
There was a problem hiding this comment.
It seems an OS-specific problem and they found a solution? My point is, the only difference these checks can make is that user has broken R74 like the issue reporter and simultaneously has a working R56 installed, thus instead of failing the meson configure, we use R56 for dependency. In any other cases (R74 working or R56 not installed) the checks don't make a difference. And I assume there is not a security reason for these checks right?
There was a problem hiding this comment.
I think the main regression that happened for the user there, because it is working with 0.41.0 release version. Is that we no longer get vapoursynth-script dependency, and instead only vapoursynth. Which seems to point on different location.
|
Also, |
Yes, I know. The question is what is the canonical way to use this. Because ok, there is .pc file, that only set include dir, so we can do it ourselves. If distros don't want to use include-dir from python module, well tough luck. They can remove them, I guess. Additionally, there is no way to use custom pkg-config path in meson itself. So, it has to be done before calling it. That's fine, but something to remember. |
|
Just to say, there is multiple ways to make this work. It can be made backward compatible on package level, it can be pointed into .pc file from python module, it can query headers directly. I don't particularly care, but I feel like querying include dir directly, makes it the most portable. Also on Windows, where .pc never worked in fact. I don't see the need to detour through pkg-config, when we only need include dir. Currently we need the fallback for old version only. I don't want to force this change, but it seems clean. |
|
All good. The include dir method certainly works and portable. |
|
It looks like this will break cross-compilation. If vapoursynth is installed on host, it will find the host vapoursynth instead of in cross install prefix. |
How do you install Python project in you cross prefix? Can you show me your cross build with recent vapoursynth? |
We just manually copied vapoursynth's headers to prefix with custom .pc. This method actually works. |
Does it matter where it finds header files? They're only required for compilation. |
This would break seal and reproducibility. We currently explicitly target vapoursynth headers to R72, but if meson decided to include host headers, it will actually use a version that we cannot control. |
4 participants
To build vf_vapoursynth headers are needed. With newer VS versions those are located in Python module, while for older they are installed in system. Try Python module first and fallback to pkg-config.
Note that this requires vapoursynth to be available during build time. If you use venv or similar, make sure it's activated.
Fixes: #18126