GNOME Bugzilla – Bug 778612
Meson build has incomplete introspection and docs
Last modified: 2017-02-21 07:36:46 UTC
A comparison between 0.3.2+8 (6ffe445) and 0.3.3 shows that the gir files lost information: https://pkgbuild.com/~heftig/pkgdiffrepo.grilo-0.3.3-1-x86_64.YQeOuAIsXh/report.html Notably, the GRL_OPERATION_OPTION_* constants and grl_plugin_* functions are gone. In addition, when built with meson, more information is missing: https://pkgbuild.com/~heftig/grilo-meson/report.html ./configure --prefix=/usr --sysconfdir=/etc --libdir=/usr/lib \ --disable-static --disable-debug --enable-gtk-doc vs meson --prefix=/usr --buildtype=plain -Denable-gtk-doc=true ../grilo grilo 0.3.3 meson 0.38.1 Arch Linux x86_64
This is the commit that causes missing those constants in 0.3.3. commit 463d94d6022d3dd0acc8da8313ab10e01bfac7d9 Author: Bastien Nocera <hadess@hadess.net> Date: Mon Nov 7 16:33:01 2016 +0100 core: Don't include private headers in introspection data We should only use the C sources, not the private headers in the introspection, so as to avoid making private API available. :040000 040000 e4693a0c12215ee8b359fd416d8c07dd1f2b9bc3 1cdc2f22ec827d4212e2a5d39834d605cb63802d M src
So basically, GRL_OPERATION_OPTION_* was intended for private use inside Grilo. And just mistakenly exposed in the introspection, which previous commit fixes.
Same happens with grl_plugin_* functions: those functions that are not part of public API were removed; but the other ones are kept (grl_plugin_get_author(), grl_plugin_get_description(), and so on).
For the difference between autotools and meson, I don't think we are breaking introspection. Taking a look at the changes, mostly are about documentation, which in Meson has been removed. Waiting for other opinions, but I think this is a NOTABUG
So the autotools build is indeed fine. Thanks for clearing this up. I'll try to get the meson build product closer to the autotools one.
Created attachment 345901 [details] [review] meson: Improve docs and introspection This patch gets us closer, but still not perfect. https://pkgbuild.com/~heftig/grilo-meson2/report.html (Ignore the increased change percentage; I hacked pkgdiff to treat HTML as Text for the visual diff.) Still bad: The "writing apps" section in the docs is missing the program listings. This is due to builddir != srcdir. The relative includes of the example sources in writing-apps.xml don't work anymore. The autotools build should be affected by this, too.
Review of attachment 345901 [details] [review]: Mostly LGTM, except a couple of minor comments. ::: doc/grilo/meson.build @@ +15,3 @@ + join_paths(meson.source_root(), 'src'), + join_paths(meson.source_root(), 'libs'), + ], LGTM ::: libs/meson.build @@ +7,3 @@ +libs_inc = include_directories(meson.current_source_dir()) + LGTM ::: libs/net/meson.build @@ +47,2 @@ install: true, + extra_args: [ '--c-include=net/grl-net.h' ]) LGTM, except the dependencies line, which is not required. ::: libs/pls/meson.build @@ +27,1 @@ c_args: '-DHAVE_CONFIG_H', You just switched two lines, but no changes. So revert this change. @@ +43,1 @@ endif Except the "dependencies" line (which is not required), the other changes looks good for me ::: src/meson.build @@ +118,3 @@ identifier_prefix: 'Grl', symbol_prefix: 'grl', + dependencies: [ gobject_dep, gmodule_dep, gio_dep ], This line is not required, as it is added by Meson.
(In reply to Juan A. Suarez Romero from comment #7) As I understand (from reading meson code and examples) this is needed when the gir files aren't in the system paths. For each of the dependencies, meson will extract the girdir from pkg-config and add it to the scanner's search dirs. So, if say, libsoup is somewhere else, this is how you get from libsoup-2.4.pc to Soup-2.4.gir. Meson doesn't know there's a connection between 'Soup-2.4' and libsoup_dep. > You just switched two lines, but no changes. So revert this change. Yeah, that was just me reducing the differences between libs/net/meson.build and libs/pls/meson.build . Should I still revert that?
(In reply to Jan Alexander Steffens (heftig) from comment #8) > (In reply to Juan A. Suarez Romero from comment #7) > > As I understand (from reading meson code and examples) this is needed when > the gir files aren't in the system paths. For each of the dependencies, > meson will extract the girdir from pkg-config and add it to the scanner's > search dirs. > But our gir files are installed in system paths, right? > So, if say, libsoup is somewhere else, this is how you get from > libsoup-2.4.pc to Soup-2.4.gir. Meson doesn't know there's a connection > between 'Soup-2.4' and libsoup_dep. > The thing is that without adding those lines, I still get the correct dependencies. So not sure if we really need it. > > You just switched two lines, but no changes. So revert this change. > > Yeah, that was just me reducing the differences between libs/net/meson.build > and libs/pls/meson.build . Should I still revert that? Yes, please. Let's keep only required changes.
(In reply to Juan A. Suarez Romero from comment #9) > (In reply to Jan Alexander Steffens (heftig) from comment #8) > > (In reply to Juan A. Suarez Romero from comment #7) > > > > As I understand (from reading meson code and examples) this is needed when > > the gir files aren't in the system paths. For each of the dependencies, > > meson will extract the girdir from pkg-config and add it to the scanner's > > search dirs. > > > > But our gir files are installed in system paths, right? > > > > So, if say, libsoup is somewhere else, this is how you get from > > libsoup-2.4.pc to Soup-2.4.gir. Meson doesn't know there's a connection > > between 'Soup-2.4' and libsoup_dep. > > > > The thing is that without adding those lines, I still get the correct > dependencies. So not sure if we really need it. > Let's ping @nirbheek to get their feedback around this issue.
Yes, Meson doesn't know about the connection between Soup-2.4.gir and libsoup_dep, but it doesn't need to know either. g-ir-scanner is what needs to know. You need to add internal dependencies to the gnome.generate_gir invocation, otherwise g-ir-scanner will pick up the system-installed dependency. For deps that are always picked up from the system (PKG_CONFIG_PATH), you don't need to. Of course you will need to change this if you decide to also support building libsoup/glib as a fallback subproject if it's not detected by the system pkg-config. But that is something for another time. See: https://bugzilla.gnome.org/show_bug.cgi?id=774625 https://github.com/mesonbuild/meson/pull/1060
Review of attachment 345901 [details] [review]: ::: libs/meson.build @@ +7,3 @@ +libs_inc = include_directories(meson.current_source_dir()) + You don't need to pass meson.current_source_dir() to include_directories(). Just do include_directories('.'). It is very rarely that you actually need to use meson.current_source_dir() and friends, and if you find yourself needing to use it, consider that it might be a bug and report it please!
(In reply to Nirbheek Chauhan from comment #11) > Yes, Meson doesn't know about the connection between Soup-2.4.gir and > libsoup_dep, but it doesn't need to know either. g-ir-scanner is what needs > to know. > > You need to add internal dependencies to the gnome.generate_gir invocation, > otherwise g-ir-scanner will pick up the system-installed dependency. For > deps that are always picked up from the system (PKG_CONFIG_PATH), you don't > need to. > > Of course you will need to change this if you decide to also support > building libsoup/glib as a fallback subproject if it's not detected by the > system pkg-config. But that is something for another time. > Thanks for the feedback. So I understand that it is safe to include the dependencies then.
(In reply to Juan A. Suarez Romero from comment #13) > Thanks for the feedback. So I understand that it is safe to include the > dependencies then. Yes, it should be fine to do that.
Created attachment 346055 [details] [review] build: Improve docs and introspection Thanks for the reviews. New patch fixes the docs, too.
Meson uses the basename as the target for content_files while gtk-doc.make just appends the whole path to the builddir. This results in different target locations if the filename contains a directory component. Meson's behavior is actually more useful here, and I replicated that in the Makefile.am.
Review of attachment 346055 [details] [review]: LGTM
Review of attachment 346055 [details] [review]: ++
Attachment 346055 [details] pushed as c5eac49 - build: Improve docs and introspection