GNOME Bugzilla – Bug 672133
conflict between srcdir != builddir and generated enum types
Last modified: 2015-02-07 16:49:34 UTC
E.g. cygwin distributes some broken .gir files, namely Pango-1.0.gir and GdkPixbuf-2.0.gir, but probably some others. The breakage is that some enums aren't marked as havling glib:gtype, although they should have. This happens when: 1) building from released tarball, not from git checkout 2) building out of source tree gdk-pixbuf-enum-types.h exist in both sourcedir (from the tarball) and builddir (generated by glib-mkenum). The problem is that GdkPixbuf_2_0_gir_CFLAGS variable in Makefile.am either does not specify -I$(top_builddir) before -I$(top_srcdir). In this case, scanner gets builddir/gdk-pixbuf-enum-types.h on the commandline, but preprocessor uses the one from sourcedir, therefore scanner ignores all declarations from sourcedir/gdk-pixbuf-enum-types.h, because it was not specified on the commandline. This means that _get_type() functions for enums are not visible for the scanner and scanner generates non-gtype-based enums in resulting .gir file. Of course, the simple fix is to add -I$(top_builddir) before -I$(top_srcdir) in the Makefile.am in all related packages. However, this is very error-prone, and there is absolutely no diagnostics when this is done wrong. Therefore I wonder whether it would be possible to add some mechanism/diagnostics into the scanner to diagnose this situation.
Simple steps to reproduce: curl http://ftp.gnome.org/pub/GNOME/sources/pango/1.29/pango-1.29.4.tar.xz | tar xJf - cd pango-1.29.4 mkdir _build cd _build ../configure && make cd .. ./configure && make diff -u _build/pango/Pango-1.0.gir pango/Pango-1.0.gir
Created attachment 209839 [details] [review] Diff between broken and correct .gir file A sample showing difference between borken and correct .gir for Pango. This is result of the 'diff' from sample commands given in comment #2
That could be done in Makefile.introspection which I think would solve it for most users. Would that work?
Created attachment 209849 [details] [review] Always pass -I$(top_builddir) to scanner invocation Good idea. Following patch tries to achieve that. It certainly fixes the cass I have tested. I'm widely open to ideas on how to make this patch less 'hackish'.
Hmm...so we get two enum files. But why are they different? Investigating A few approaches here: 1) gtk+ has a --disable-rebuilds configure flag. We could add this to more modules. But yeah, kind of gross. 2) Somehow fix the makefiles to not build the enums if they're already in $(srcdir) 3) Patch affected modules to ensure they're using the same CPPFLAGS for the scanner as they are for building.
(In reply to comment #1) > Simple steps to reproduce: > > curl http://ftp.gnome.org/pub/GNOME/sources/pango/1.29/pango-1.29.4.tar.xz | > tar xJf - > cd pango-1.29.4 > mkdir _build > cd _build > ../configure && make > cd .. > ./configure && make > diff -u _build/pango/Pango-1.0.gir pango/Pango-1.0.gir By the way, you shouldn't build in both a builddir and the srcdir. autoconf will error out on your if you try to do srcdir then builddir. So the better instructions here are: wget http://ftp.gnome.org/pub/GNOME/sources/pango/1.29/pango-1.29.4.tar.xz tar zvf pango-1.29.4.tar.xz mv pango-1.29.4{,.srcdir-build} tar zvf pango-1.29.4.tar.xz mv pango-1.29.4{,.builddir-build} cd pango-1.29.4.srcdir-build ./configure && make cd ../pango-1.29.4.builddir-build mkdir _build ../configure && make
(In reply to comment #5) > Hmm...so we get two enum files. But why are they different? They have the same content, the problem is that they have different path. Scanner uses full file path to distinguish whether symbol from the file should be included in the list of exported symbols or not. Relevant code is here: http://git.gnome.org/browse/gobject-introspection/tree/giscanner/sourcescanner.c#n256 found_filename is FALSE in this case, because symbol's filename is [srcdir/type.h] and scanner's filename (in the list) is [blddir/type.h] > A few approaches here: > > 1) gtk+ has a --disable-rebuilds configure flag. We could add this to more > modules. But yeah, kind of gross. > 2) Somehow fix the makefiles to not build the enums if they're already in > $(srcdir) AFAICS both these approaches require all modules' makefiles to do some action, and I don't see any way for us to issue any diagnostics if they do it wrong. > 3) Patch affected modules to ensure they're using the same CPPFLAGS for the > scanner as they are for building. Unfortunately even this might not be enough - see the problem explanation above; C compiler does not care about the path of the compiled file in the same way as scanner does.
So, we have following options: 1) Tell all modules makefiles to add -I$(top_builddir) as the first option to _CFLAGS variable 2) Tell all modules makefiles to not generate files which are already pregenerated in the srcdir from the tarball 3) Tell all modules to stop shipping pregenerated sources in the tarballs 4) Tell all modules to implement --disable-rebuilds configure flag or 5) Use patch from comment #2 I think that 1-4 are unrealistic, given that the problem is rather hard to spot for developers working in git source trees, so this gives us 5) as the one ugly but working solution.
> 5) Use patch from comment #2 oops, of course I meant comment #4 - patch which adds -I$(top_builddir) unconditionally to all g_ir_scanner invocations in Makefile.introspection.
Clearly you're right that it's really a problem that we just silently fail. One thing we could do is add --sourcedir and --builddir options to g-ir-scanner. This is the approach that was taken for the glib-compile-resources tool. That would help a lot for projects that have to manually prefix source files right now. Even if we add the -I$(top_builddir) hack to g-ir-scanner though, the problem really calls for *also* fixing it in the modules. You'll still have people trying to compile newer versions of modules with older g-ir-scanner. So we should at least investigate what it'd take to avoid building the enums if they're in the srcdir for example. One thing that argues somewhat against your $(top_builddir) approach is that only works for modules that are structured like Pango where the name of the source directory matches at the toplevel, and they're using non-recursive automake. This is true for pango, gdk-pixbuf, and glib. But it's not true for some of my code (e.g. ostree, which uses non-recursive automake and a src/ prefix).
Concretely, some modules may need $(builddir), not $(top_builddir).
(In reply to comment #11) > Concretely, some modules may need $(builddir), not $(top_builddir). Good idea. I just tried pango and gdkpixbuf builds with -I$(builddir) instead of -I$(top_builddir), and it seems to work well too. In general, I agree that it would be good to fix modules, but I'm skeptical about practical feasibility to do so, because 1) sheer amount of modules even in gnome git repository, not even counting external 3rd party ones 2) it is rather hard to discover whether the module is 'bitten' by this problem 3) it seems to be impossible to add some diagnostic machinery for this problem So I'm a bit in favor of including -I$(builddir) hack into GI makefile machinery - note that it is not patched directly into g-ir-scanner, but into shared 'Makefile.introspection' make include file, so it is already quite specific for autofools usage, and we can expect $(builddir) to be set in a sane way.
I would have liked to do something for 3.4, but on the other hand just doing this in introspection now creates a risk of breakage. Let's do something here early in the 3.6 cycle.
Created attachment 211257 [details] [review] Fix sourcedir!=builddir .gir build from source tarball Setting -I$(top_builddir) before -I$(top_srcdir) causes that g-ir-scanner picks up boxing definitions generated by glib-mkenums.
Created attachment 211258 [details] [review] Fix sourcedir!=builddir .gir build from source tarball Setting -I$(top_builddir) before -I$(top_srcdir) causes that g-ir-scanner picks up boxing definitions generated by glib-mkenums.
Created attachment 211259 [details] [review] Fix sourcedir!=builddir .gir build from source tarball Setting -I$(top_builddir) before -I$(top_srcdir) causes that g-ir-scanner picks up boxing definitions generated by glib-mkenums.
I tried rebuild most of the .gir-generating packages in cygwin distribution with local patch to Makefile.introspection which includes '-I$(top_builddir) -I$(builddir)'. The result is that only 3 packages out of cca 100 ones make a difference with this setting - pango, gdk-pixbuf and gdk. So I think that it is not worth introducing hack into generic Makefile.introspection just because of these 3 packages, and fix these packages instead. Patches against these packages are attached here.
These patches all look correct to me, and I do think it makes sense to do this change in individual modules. (If however we can think of a clever way to detect the problem inside g-ir-scanner, I'd be happy too). The patches in theory should go through individual maintainers, but I think it'd probably be fine if we just committed them to master for 3.6.
Attachment 211257 [details] pushed as 94116ae - Fix sourcedir!=builddir .gir build from source tarball Attachment 211258 [details] pushed as 94116ae - Fix sourcedir!=builddir .gir build from source tarball Attachment 211259 [details] pushed as 94116ae - Fix sourcedir!=builddir .gir build from source tarball
Review of attachment 211258 [details] [review]: ::: pango/Makefile.am @@ +283,3 @@ PangoFT2_1_0_gir_INCLUDES = GObject-2.0 cairo-1.0 freetype2-2.0 fontconfig-2.0 PangoFT2_1_0_gir_LIBS = libpangoft2-1.0.la +PangoFT2_1_0_gir_CFLAGS = -I$(top_builddir) I$(top_srcdir) Is there a missing hyphen there? Should that be -I$(top_srcdir) ? @@ +334,3 @@ PangoXft_1_0_gir_INCLUDES = GObject-2.0 xft-2.0 xlib-2.0 PangoXft_1_0_gir_LIBS = libpangoxft-1.0.la +PangoXft_1_0_gir_CFLAGS = I$(top_builddir) -I$(top_srcdir) $(PANGO_CFLAGS) Is there a missing hyphen there? Should that be -I$(top_builddir) ?
(In reply to comment #20) > Is there a missing hyphen there? Should that be -I$(top_builddir) ? Oops, thanks for spotting. I just pushed the fix for these obvious bugs. Sorry for the breakage.
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]