GNOME Bugzilla – Bug 624910
Mutter does not build out-of-tree.
Last modified: 2016-06-27 21:49:04 UTC
Created attachment 166263 [details] [review] Fix out-of-tree build. The problem is that mutter-enum-types.h and mutter-enum-types.c targets have dependencies on the respective .in files in the build directory instead of the source directory. A patch is attached that fixes this issue.
Review of attachment 166263 [details] [review]: This patch looks half-right to me; the changes to remove $(srcdir) from: --template $(srcdir)/mutter-enum-types.h.in are right ... the srcdir is extraneous since the code changes to $(srcdir) anyways and breaks the srcdir != builddir case. But the changes to the dependencies look wrong to me... see the docs for VPATH in the make manual. Could use a more descriptive git commit message.
Created attachment 167809 [details] [review] Fix out-of-tree build. You are right, the dependencies are correct. I've come across another issue with generating gobject-introspection data when building out-of-tree. I'm attaching a patch which fixes both issues and has a more verbose commit message.
You attached the original patch again. (http://fishsoup.net/software/git-bz/ makes it easy to avoid this kind of mistake, which otherwise is really easy to do :-)
Created attachment 167931 [details] [review] Fix out-of-tree build. This patch fixes two issues encountered when building mutter out-of-tree: * When generating mutter-enum-types.[ch], the glib-mkenums command is executed from $(srcdir), so it is wrong to prepend $(srcdir) to the template file. * When generating gobject-introspection data, the g-ir-scanner command is executed from $(srcdir) and fails to find generated source file which reside in the build directory. To fix this, g-ir-scanner is run from the build directory, and $(srcdir) is prepended to non-generated source and header files.
Review of attachment 167931 [details] [review]: Looks good, let me know if you don't have a GNOME git account and I'll push it for you. ::: src/Makefile.am @@ +207,3 @@ # so that libgirepository looks for symbols in the executable instead Meta-$(api_version).gir: $(G_IR_SCANNER) mutter $(libmutterinclude_HEADERS) $(mutter_SOURCES) + $(AM_V_GEN)$(G_IR_SCANNER) \ space after $(AM_V_GEN) @@ +221,3 @@ + $(addprefix $(srcdir)/,$(filter %.c, \ + $(filter-out $(mutter_built_sources), \ + $(mutter_SOURCES)))) \ Ouch. But don't have particularly better ways of doing this (you could split apart mutter_SOURCES into mutter_sources and mutter_built_sources, and save the filter-out, but not a ton better.) So, OK.
Created attachment 168028 [details] [review] Fix out-of-tree build. This patch fixes two issues encountered when building mutter out-of-tree: * When generating mutter-enum-types.[ch], the glib-mkenums command is executed from $(srcdir), so it is wrong to prepend $(srcdir) to the template file. * When generating gobject-introspection data, the g-ir-scanner command is executed from $(srcdir) and fails to find generated source file which reside in the build directory. To fix this, g-ir-scanner is run from the build directory, and $(srcdir) is prepended to non-generated source and header files.
I've reworked the patch according to your comments and I think splitting off mutter_SOURCES into mutter_sources and mutter_built_sources is a bit better. I do not have a GNOME git account, so I won't be able to push it.
(In reply to comment #6) > * When generating gobject-introspection data, the g-ir-scanner command > is executed from $(srcdir) and fails to find generated source file > which reside in the build directory. > > To fix this, g-ir-scanner is run from the build directory, and > $(srcdir) is prepended to non-generated source and header files. You shouldn't need to do any prepending. It can all be made to work via $VPATH magic. In particular, if you port it to use "include $(INTROSPECTION_MAKEFILE)" and then use the rules from there (as explained in $(prefix)/share/gobject-introspection-1.0/Makefile.introspection) rather than constructing the g-ir-scanner command line yourself, then you don't need to prefix $(srcdir) to anything.
Created attachment 168202 [details] [review] Fix out-of-tree build. This patch fixes two issues encountered when building mutter out-of-tree: * When generating mutter-enum-types.[ch], the glib-mkenums command is executed from $(srcdir), so it is wrong to prepend $(srcdir) to the template file. * Generate gobject-introspection data using Makefile.introspection as suggested by Dan Winship.
Comment on attachment 168202 [details] [review] Fix out-of-tree build. assuming that works, it seems right to me (the $(api_version) mangling is a bit ugly, but...)
Review of attachment 168202 [details] [review]: ::: src/Makefile.am @@ +197,3 @@ +INTROSPECTION_GIRS = Meta-$(api_version).gir.tmp + +Meta_$(subst .,_,$(api_version))_INCLUDES = \ This is really hard to read. Does it work to do: api_version_ := $(subst .,_,$(api_version)) Meta_$(api_version_)_INCLUDES = \ ?
That won't work, unfortunately, because automake will complain about bad characters in the variable name. The closest I can get to what you are proposing is something like this: api_version_ := $(subst .,_,$(api_version)) Meta_$(value api_version_)_INCLUDES = \ I will follow up with a patch that implements that approach, so you can see if you like it any better.
Created attachment 168267 [details] [review] Fix out-of-tree build. This patch fixes two issues encountered when building mutter out-of-tree: * When generating mutter-enum-types.[ch], the glib-mkenums command is executed from $(srcdir), so it is wrong to prepend $(srcdir) to the template file. * Generate gobject-introspection data using Makefile.introspection as suggested by Dan Winship.
Review of attachment 168267 [details] [review]: Sorry for the delay in handling this. The instrospection Makefile stuff was landed independently eventually: commit a66ae4ad551b6b9778d790dd66842a186cae129a Author: Dan Winship <danw@gnome.org> Date: Sat Mar 5 15:01:33 2011 -0500 Use standard introspection configure/Makefile bits This changes the introspection configure flag from --with/--without-introspection to --enable/--disable-introspection, and changes it so that trying to enable introspection when g-i is not installed results in an error, rather than being silently ignored. https://bugzilla.gnome.org/show_bug.cgi?id=643959 (with a configure.in substitution for the API version.) I'll extract out the rest of it and commit it. (It sneaked through the srcdir != builddir building for make distcheck because: builddir = mutter/mutter-3.1.3.1/_build/src srcdir = ../../src in srcdir, ../../src points to mutter/src/ So, it finds the files *outside* the test build tree)
Committed relevant parts of patch Attachment 168267 [details] pushed as 8033184 - Fix out-of-tree build.
Created attachment 191645 [details] [review] build: Modernize enum build rules The current rules are broken if srcdir != builddir, but this patch also kills off the the stamp files. They're a relic of an earlier age; there's no reason not to depend on GLib tools at build time now. I also use $@.tmp and $@ instead of the semi-random xgen-foo which have been subject to copy-paste error before. Note these rules depend on GNU Make, but the introspection build does anyways.
I ran into this recently too, and had written this patch before I updated. I think my patch is better, for the reasons mentioned in the commit message.
i think you meant to reopen this, not mark it verified (In reply to comment #16) > The current rules are broken if srcdir != builddir, but this patch > also kills off the the stamp files. They're a relic of an earlier > age; there's no reason not to depend on GLib tools at build time now. No, that relic does exist in some packages (--disable-rebuild in glib and gtk at least), but that's different from this. The stamp files are so that mutter-enum-types.h only gets regenerated when you add/remove/change an enum, rather than being regenerated any time you make *any* change to a header file, thereby saving you from unnecessary recompiles of the files that depend on mutter-enum-types.h.
(In reply to comment #18) > i think you meant to reopen this, not mark it verified > > (In reply to comment #16) > > The current rules are broken if srcdir != builddir, but this patch > > also kills off the the stamp files. They're a relic of an earlier > > age; there's no reason not to depend on GLib tools at build time now. > > No, that relic does exist in some packages (--disable-rebuild in glib and gtk > at least), but that's different from this. The stamp files are so that > mutter-enum-types.h only gets regenerated when you add/remove/change an enum, > rather than being regenerated any time you make *any* change to a header file, > thereby saving you from unnecessary recompiles of the files that depend on > mutter-enum-types.h. Ah, right. Well, in my patch, it looks like in that case we'll run glib-mkenums on every invocation of "make" since we use 'cmp' to avoid updating the header file. I dunno; most nontrivial projects end up having a lot of internal header inclusion, so in practice, you need to rebuild a lot if any header files change. The solution to this IMO is ccache; it is a *large* compilation speed win in many cases, and as a huge bonus it's 99% transparent. The stamp file hack here is a relative micro-optimization. Bottom line then I guess, we should either: 1) Remove the cmp in my patch, and unconditionally run glib-mkenums on header file changes (and I bet no one notices) 2) Not take my patch
Review of attachment 191645 [details] [review]: I do like being able to change a header file without recompiling everything from scratch. The stamp file approach is the best way we found to achieve that. The rest of the changes? - Use GNU make $(addprefix ...) rather than cd'ing to srcdir. Meh. - Use temporary names based on $@ rather than xgen-<whatever. Sure, that's better, a patch to just do that would be fine.