gnome: Add support for gi-compile-repository for generate_gir #13997
Conversation
|
GLib does not currently expose the binary path in pkg-config, a future version may via https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4428 |
| 'glib-genmarshal': ('glib-2.0', 'glib_genmarshal'), | ||
| 'glib-mkenums': ('glib-2.0', 'glib_mkenums'), | ||
| # TODO: Use gi_compile_repository once GLib exposes it | ||
| 'gi-compile-repository': ('girepository-2.0', None), |
There was a problem hiding this comment.
The code as-written does not allow looking up tools which are not in pkg-config, this adapts the code to allow finding tools via binary lookup only
There was a problem hiding this comment.
- That's just
state.find_program()instead ofstate.find_tool()... - Upstream MR has been merged (not released) so I guess we can simplify this again.
There was a problem hiding this comment.
I've added a new kwarg,
use_glib_to_compile_repositories, which allows opting intogi-compile-repositorybut this is definitely open to bikeshedding and ideas. In general I think this should probably be an opt-in to projects usinggnomemodule even ifgi-compile-repositoryshould be a drop-in replacement.
What's the advantages and disadvantages of each? If gi-compile-repository is a drop-in replacement, why not unconditionally use it?
mesonbuild/modules/gnome.py
Outdated
| @@ -1112,6 +1124,7 @@ def _get_scanner_ldflags(ldflags: T.Iterable[str]) -> T.Iterable[str]: | |||
| KwargInfo('install_dir_gir', (str, bool, NoneType), | |||
| deprecated_values={False: ('0.61.0', 'Use install_gir to disable installation')}, | |||
| validator=lambda x: 'as boolean can only be false' if x is True else None), | |||
| KwargInfo('use_glib_to_compile_repositories', (bool), default=False), | |||
There was a problem hiding this comment.
This is not just a long name, it's an unfortunately long name...
There was a problem hiding this comment.
It is a stand-in, meant to be bikeshed :)
There was a problem hiding this comment.
Feels like use_{new,latest}_compiler, use_glib_compiler, or even use_glib would suffice, given that it's an opt-in flag for fairly specific functionality. It can always be explained more fully in the documentation.
Plus, it's meant to be temporary, surely?
There was a problem hiding this comment.
Why not just have this as part of the existing test?
There was a problem hiding this comment.
It changes the library/binary which is compiling all of the GIRs. Both cases seem important to test. If there is a way to parameterize tests in this suite or a suggestion you have to combine the source files, let me know (first time navigating the meson codebase)
|
@eli-schwartz Additionally |
ewlsh
force-pushed
the
ewlsh/gi-compile-repository
branch
from
efabb27 to
3034fff
Compare
ewlsh
force-pushed
the
ewlsh/gi-compile-repository
branch
from
3034fff to
b7ac866
Compare
Hmm. THAT makes it sound like the tooling should start with autodetecting the available compiler(s), and just automatically use what it finds. Only when both compilers are available, would it even make sense to make it selectable. (And even then, only if there's a reason to continue using the old one even when the newer is also an option. Though it sounds like there may be, from your other comments.) |
You're saying:
Well yeah but inherent in this premise is that we only unconditionally use it when it exists, and the "unconditional" was a contrast to "require that it exists, and also require opting in to it". Obviously I can't and couldn't possibly be suggesting that we modify meson to delete all support of g-ir-compiler and raise the minimum supported version of glib to 2.79.2? |
| if not self.gir_dep: | ||
| self.gir_dep = state.dependency('gobject-introspection-1.0') | ||
| self.giscanner = self._find_tool(state, 'g-ir-scanner') | ||
| self.gicompiler = self._find_tool(state, 'g-ir-compiler') | ||
|
|
||
| if not self.gicompiler: | ||
| if use_gi_repository_compile and supports_girepository_2: | ||
| self.gicompiler = self._find_tool(state, 'gi-compile-repository') | ||
| else: | ||
| self.gicompiler = self._find_tool(state, 'g-ir-compiler') | ||
|
|
||
| return self.gir_dep, self.giscanner, self.gicompiler |
There was a problem hiding this comment.
Implementation-wise, this is very problematic. The current design requires people to specify which compiler they want via a meson.build DSL keyword argument, but once passed for the first time it will cache the value globally.
This means for example that if you have a project which knows nothing about this new feature -- it supports meson 0.40.0, or more charitably, supports meson 1.6.0 -- it will configure to build a gir using g-ir-compiler, and then it invokes a subproject that has a minimum meson_version: '>=1.7.0' and makes heavy use of GIRepository 2.0 features and is simply incompatible with GIRepository 1.0... and fails to compile, even though it passes your use_gi_repository_compile: true kwarg, because meson simply detects that self.gicompiler is g-ir-compiler.
This is one of the reasons I was hoping it would be practical to simply use the newer tool by default. :)
Opening this MR to start considering how to support
gi-compile-repository(from glib) in place ofg-ir-compiler(from gobject-introspection).gi-compile-repositoryis the replacement forg-ir-compilerand is currently used by GLib to compile its GIR files. This is currently being done by overriding theg-ir-compilerexecutable but we want to incorporategi-compile-repositoryinto thegnomemeson module for all projects.I've added a new kwarg,
use_glib_to_compile_repositories, which allows opting intogi-compile-repositorybut this is definitely open to bikeshedding and ideas. In general I think this should probably be an opt-in to projects usinggnomemodule even ifgi-compile-repositoryshould be a drop-in replacement.