Avoid using git submodules for meson build #60
Conversation
meson.build
Outdated
| subdir('py') | ||
| if get_option('pykms').enabled() | ||
| subdir('py') | ||
| endif | ||
|
|
There was a problem hiding this comment.
Why is this patch needed? pykms is not built even now if pykms is disabled. I guess you wanted to avoid calling dependency('pybind11') if pykms is not enabled?
I don't think this works, as pykms can be 'auto'. If that's the case then pykms won't ever be built.
There was a problem hiding this comment.
Sorry, was a quick fix for the buildroot case ;-), dropped this one for pybind11 required dependency only if pykms is enabled...
There was a problem hiding this comment.
It still doesn't work right. I don't have pybind11 installed on my system, and if I do:
meson build -Dsystem-pybind11=true -Dpykms=auto
Meson passes fine, but when building it'll fail as pybind11 is not found. This is because the "auto" setting doesn't specify if the dependency is actually needed or not.
This probably needs a bit more work to make it work right. Maybe the whole pybind11_dep stuff should be moved from the top level meson.build to the pykms meson.build. I think I added it to the top level meson.build just because it's referring to the ext/pybind11/ directory, and it felt kind of more natural to declare pybind11 "module" at the top level, instead of pointing to it from the py/pykms/ subdirectory.
But that's not a real issue, and as moving the pybind11_dep to pykms meson.build solves this one being discussed, I guess that's the way to go.
Btw, I guess you didn't notice, but some time back I sent "[PATCH v2 1/1] package/kmsxx: bump to latest" to buildroot list. These patches of yours will probably make the bump to latest a bit cleaner. So lets get kms++ upstream cleaned up properly so we'll get a nice, clean package to buildroot =).
There was a problem hiding this comment.
Your are right (only tested with -Dpykms=disabled), updated patch to build pykms only in case pybind11 dependency is found...
Sorry missed/forgot about your patch, but will take a closer look at it later (renaming an option needs Config.in.legacy handling, adding pykms should be done in an extra/follow-up patch)...
meson_options.txt
Outdated
| @@ -3,3 +3,5 @@ option('pykms', type : 'feature', value : 'auto') | |||
| option('omap', type : 'feature', value : 'auto') | |||
| option('static-libc', type : 'boolean', value : false) | |||
| option('utils', type : 'boolean', value : true) | |||
|
|
|||
| option('use-system-fmt', type : 'boolean', value : false) | |||
There was a problem hiding this comment.
I don't know if there's some common practice with the option names, but would "system-fmt" be enough? That would be more in line with the existing ones (e.g. we don't have "use-static-libc", but just "static-libc").
Same comment for the "use-system-pybind11".
meson_options.txt
Outdated
| @@ -3,3 +3,5 @@ option('pykms', type : 'feature', value : 'auto') | |||
| option('omap', type : 'feature', value : 'auto') | |||
| option('static-libc', type : 'boolean', value : false) | |||
| option('utils', type : 'boolean', value : true) | |||
|
|
|||
There was a problem hiding this comment.
Can you drop the extra empty line here
Signed-off-by: Peter Seiderer <ps.report@gmx.net> --- Changes v1 -> v2: - renamed use-system-fmt to system-fmt option
pseiderer
force-pushed
the
ps-devel-do-not-use-git-submodules-001
branch
from
43a968a
to
9ace8ea
Compare
|
I did some hacking, and pushed another kind of approach to: #61 Although, while working on those, I was wondering what's the point. Both fmt and pybind11 are header-only "libraries", so there's no benefit (on the output side) to use the system packaged ones. That's the reason I added them as git submodules in the first place. I guess the only reason for header-only libs is just that buildroot has decided that git submodules are not nice? |
Add system-pybind11 option (dependency only required if pykms is enabled). Signed-off-by: Peter Seiderer <ps.report@gmx.net> --- Changes v2 -> v3: - build pykms only if pybind11 dependency is found Changes v1 -> v2: - renamed use-system-pybind11 to system-pybind11 option - change system-pybind11 dependency only required if pykms is enabled
pseiderer
force-pushed
the
ps-devel-do-not-use-git-submodules-001
branch
from
9ace8ea
to
6a507ae
Compare
Will take a look at it...
The buildroot reasoning is keeping track of the different packages (including source versions), collect the licenses accordingly (see make legal-info), search for CVE's (easier to keep track of if the provided packages are used instead of searching all packages for sub-packages/versions and easier to fix if only needed to be done once instead of multiple times in different packages and/or for sub-packages with maybe custom changes, etc.) and this makes no difference if the library is headers-only a real one... |
Ok, makes sense. Using only system provided libfmt is fine for me. For pybind11, I like to have an option to use specific versions relatively easily, so either git submodule or meson subproject via meson wrap DB. I think latter is fine, as pybind11 has proper releases. These are what I did in PR #61. I need to fix travis, though... |
|
I have merged patches for this. |
Add meson options 'use-system-fmt' and 'use-system-pybind11' to avoid using git submodules and use the system provided library versions instead (as preferred e.g. for buildroot cross compile).