GNOME Bugzilla – Bug 654395
glib-build.mk
Last modified: 2017-12-13 14:18:40 UTC
This is an attempt to make the world a better place by providing a canonical set of "perfect" genmarshal/mkenums/etc rules rather than having everyone have to copy+paste them everywhere themselves, and having different bugs in different modules. (I'd been poking at this for a while, and was inspired to finally post it, even though it's not done, because of bug 624910.) The basic idea is that you just do: include $(GLIB_MAKEFILE) GLIB_GENERATED = \ soup-marshal.c \ soup-marshal.h \ soup-enum-types.c \ soup-enum-types.h and then a miracle occurs. See comments at the top of Makefile.glib for more details. (My first attempt at this didn't use $(GLIB_GENERATED), and instead just added rules like %marshal.h: %marshal.list ... but it turns out that having % rules for header files doesn't mix well with automake's automatic dependency generation, and it will sometimes try to rebuild .h files in other directories. And anyway, using $(GLIB_GENERATED) is nice because it lets us automatically handle BUILT_SOURCES/CLEANFILES/EXTRA_DIST correctly.) Unfortunately (and ironically given the bug 624910 connection), it doesn't actually handle srcdir!=builddir builds perfectly yet. The problem is that we are shipping -marshal.[ch] and -enum-types.[ch] files in the tarball, but also shipping rules to clean them and to rebuild them, and so at a certain point in "make distcheck", it will end up trying to rebuild one of those files inside the read-only srcdir and fail. It seems to me like the answer is "they're generated files, so we shouldn't be shipping them", and so I should just arrange for them to be not included in the tarball. Does that sound right? (Unfortunately, this turns out to be tricky, since the "dist-hook" rule only gets generated if it is present at automake time, so as things are now, we can't use that rule from Makefile.glib. I haven't figured out how to work around that yet.) Other open issues: - Not sure about the autoconf macro name (GLIB_CONFIG). It should be GLIB_something, but what? GLIB_USE_MAKEFILE maybe? ("GLIB_MAKEFILE" would be nice, but having both a macro and a configure variable with the same name would probably lead to problems.) Also, I removed a lot of stuff that AM_PATH_GLIB_2_0 does that just seemed like cruft, but that could be put back... - Is there any reason to prefer one of foo.stamp or stamp-foo for stamp files? Anyway, does this make sense?
Created attachment 191725 [details] [review] Add Makefile.glib and GLIB_CONFIG configure macro Add a Makefile fragment containing rules to generate marshallers, enums files, etc, so that people don't have to copy and paste the complicated rules into every project's Makefile.am
(In reply to comment #0) > This is an attempt to make the world a better place by providing a > canonical set of "perfect" genmarshal/mkenums/etc rules rather than > having everyone have to copy+paste them everywhere themselves I think though step 0 of this is to at least have a standard Makefile that's at least copy+pasted around so we can gain experience with it, before putting it in GLib. Committing to "API" stability for a Makefile frightens me honestly too =) So...how about we patch gnome-shell and a few other modules to use this to try it out first?
Created attachment 207117 [details] [review] Add Makefile.glib and GLIB_CONFIG configure macro ==== Updated. I fixed the distcheck issues mentioned in comment 0 by having the generated files be cleaned by "make distclean" rather than "make clean", which is correct since we're disting them. Also, I had to get rid of the attempt to automatically handle BUILT_SOURCES and instead now force the user to set "BUILT_SOURCES = $(GLIB_GENERATED)", because there's just no other way to make it work. (If there isn't a "BUILT_SOURCES = ..." line visible at automake time, then automake won't output the rules you need to depend on to get the same effect.) libsoup in git is now using this, and I have patches for NetworkManager too.
Created attachment 208330 [details] [review] et_EE-locale-fix.patch While making the world better place, you stomped on us, Estonians ;) PS. This patch is against libsoup.
Comment on attachment 208330 [details] [review] et_EE-locale-fix.patch >When using Estonian (et_EE) locale, 'a-z' range skips 'tuv...' Crazy! :) Pushed to libsoup, NetworkManager, and my glib branch. Thanks.
(In reply to comment #5) > (From update of attachment 208330 [details] [review]) > >When using Estonian (et_EE) locale, 'a-z' range skips 'tuv...' > > Crazy! :) Though entirely offtopic, I cannot leave out the fact that 24th of February also happens to be our Independence Day. :)
bug 701241 inspired me to push an updated version of this, which now has glib-compile-resources support as well. will comment more there...
I have a fairly simple rule: if it's shipped with the tarball then: (a) it MUST be architecture/config-independent in every way and (b) it goes in srcdir, always. This means that regenerating the file, even in a srcdir!=builddir build puts it in srcdir. Since the file is never different depending on the setup of different build directories, this is safe.
(In reply to comment #8) > This means that regenerating the file, even in a srcdir!=builddir build puts it > in srcdir. You're not supposed to ever write into srcdir from a srcdir!=builddir build. In particular, srcdir might be read-only. But the arch/config-independent rule is good, and this is not necessarily true for marshal/enums files, since they might include or not include certain other files based on configure-time options. Unfortunately, automake will include generated source files in the tarball by default, and there's no way to undo that from Makefile.glib if the Makefile.am doesn't already include a "dist-hook" rule. (This is something that could be made better if Makefile.glib was included at automake time. But then we'd have to have some way to have it already be present before automake was run. Eg, cut+paste or "glib-makefileize", both of which I was trying to avoid...) (OTOH, the marshal and enum-types rules have a dependency on "Makefile", so it's guaranteed that the files will get regenerated into builddir at build time, overriding the srcdir ones, so the inclusion of possibly-config-dependent files in the source tree is merely pointless, not actively harmful.)
I think there is nodist_myprogram_SOURCES = exactly for this. (In reply to comment #9) > You're not supposed to ever write into srcdir from a srcdir!=builddir build. In > particular, srcdir might be read-only. Looks like automake easily runs afoul of this simple rule as a basic principle of operation: [desrt@moonpix glib]$ mkdir _build [desrt@moonpix glib]$ cd _build/ [desrt@moonpix _build]$ ../configure &> /dev/null [desrt@moonpix _build]$ touch ../Makefile.am [desrt@moonpix _build]$ make results in the updated Makefile.in being written into the srcdir. > Unfortunately, automake will include generated source files in the tarball by > default, and there's no way to undo that from Makefile.glib if the Makefile.am > doesn't already include a "dist-hook" rule. This is exactly what nodist_myprogram_SOURCES = is for. We just have to instruct people to ensure that they're using this.
For those of us who don't build glib/gtk from makefiles, will it still be possible to generate the enums and genmarshals at build time? For example, I build using MSVC. To build any auto-generated stuff I generally either pass control to perl - or to the appropriate (i.e. supplied) build app (such as glib-genmarshal.exe). I suppose what I'm asking is whether the curretn build methodologies are being reorganised but are basically going to be kept - or is everything going to get rationalised into some new build scheme? And if so, will it be able to accomodate developers who don't use 'make'?
The enums and marshallers are still generated at build time, using the same tools as were used to generate them before. This is just about making the Makefile rules cleverer/simpler. If you're not using make then it would have absolutely no effect on you.
It might be interesting to consider the possibility of disting the generated files. This would make it possible to do cross-builds where you don't have native glib tools on the build machine. Now that I've said that, it sounds like a bad idea. Let's not do that. So maybe, rather: let's make it clear that we considered this and rejected it. Specifically: the marshal.[ch] files that often end up in tarballs by accident (because of being in _SOURCES and not nodist__SOURCES) are accidents, and we should stop this.
I've pushed wip/glib-build.mk (https://git.gnome.org/browse/glib/log/?h=wip/glib-build.mk) with my current (working, make-distcheck-passing) status. It's now included at Makefile.am time, as a copyable fragment a la glib.mk and glib-tap.mk. So you do: include $(top_srcdir)/glib-build.mk ... nodist_libsoup_2_4_la_SOURCES = \ soup-enum-types.c \ soup-enum-types.h ... soup_enum_types_sources = $(libsoupinclude_HEADERS) $(libsoupgnomeinclude_HEADERS) soup_enum_types_MKENUMS_C_FLAGS = --fhead "\#define LIBSOUP_USE_UNSTABLE_REQUEST_API" and that's it. (glib-build.mk scans all Makefile variables named *_SOURCES for filenames that look like the things it knows how to generate, and then looks for *_sources variables with matching names, and if it finds them, it outputs rules to generate the files.)
*** Bug 705557 has been marked as a duplicate of this bug. ***
(In reply to comment #14) > I've pushed wip/glib-build.mk > (https://git.gnome.org/browse/glib/log/?h=wip/glib-build.mk) with my current > (working, make-distcheck-passing) status. Looks great to me, although you appear to not have added glib-build.mk to EXTRA_DIST.
*** Bug 744101 has been marked as a duplicate of this bug. ***
So what still needs doing to get this merged? Looks like the patch currently on the bug is outdated wrt the branch (https://git.gnome.org/browse/glib/log/?h=wip/glib-build.mk), which has gained glib-compile-resources support.
(In reply to Philip Withnall from comment #18) > So what still needs doing to get this merged? Looks like the patch currently > on the bug is outdated wrt the branch > (https://git.gnome.org/browse/glib/log/?h=wip/glib-build.mk), which has > gained glib-compile-resources support. In comment 2 Colin had suggested that this needed to be thoroughly tested before going into glib, but since it's now going to be working on a copy+paste model rather than being installed API, that is less urgent; if it turns out there are architectural problems with it, we can just change how things work in the future, and it won't break any existing users because they won't get the changes until they manually copy in a new version of glib-build.mk. (Also, libsoup and NetworkManager are both using [slightly out-of-date versions of] glib-build.mk with no problems.) So, IMHO, there's nothing blocking it from being merged, other than that none of the other maintainers have come out especially in favor of it yet. Well, and probably the documentation I wrote as comments in glib-build.mk should be copied out into the real glib docs.
(In reply to Dan Winship from comment #19) > In comment 2 Colin had suggested that this needed to be thoroughly tested > before going into glib, but since it's now going to be working on a > copy+paste model rather than being installed API, that is less urgent; if it > turns out there are architectural problems with it, we can just change how > things work in the future, and it won't break any existing users because > they won't get the changes until they manually copy in a new version of > glib-build.mk. Agreed. I suspect the best way to get feedback at this point would be to just commit it, blog about it, and let people start using it. > (Also, libsoup and NetworkManager are both using [slightly out-of-date > versions of] glib-build.mk with no problems.) I guess they should be updated to use the latest version when it’s committed, or people might cargo-cult the outdated version from them. Overall, I would prefer for the Makefile fragment to be installed as is done with gobject-introspection. At the moment we’re exchanging localised cargo-culting of specific rules for global cargo-culting of a whole file. However, that file is a lot easier to update, so it’s a good half-way house. The main problem I see is if any of the GLib utilities change how they’re invoked and the rules need updating across all projects. I guess that’s unlikely.
(In reply to Philip Withnall from comment #20) > The main problem I see is if any of the GLib utilities change how they’re > invoked and the rules need updating across all projects. I guess that’s > unlikely. We couldn't do that though; that would break existing hand-written Makefiles as well. The command-line utilities are effectively API.
(In reply to Dan Winship from comment #21) > (In reply to Philip Withnall from comment #20) > > The main problem I see is if any of the GLib utilities change how they’re > > invoked and the rules need updating across all projects. I guess that’s > > unlikely. > > We couldn't do that though; that would break existing hand-written Makefiles > as well. The command-line utilities are effectively API. I was thinking more in terms of new best practices (no examples come to mind), but you’re right. So, can this get committed now? :-)
I don't think glib needs to be in the business of shipping and installing makefile fragments. And you can use the just fine without it being committed to glib. Just copy it from libsoup. Or teach gnome-builder to use this fragment, if it is so awesome... To put a cynical perspective on it, the only thing we'll gain from making this part of glib is more glib bugs
(In reply to Matthias Clasen from comment #23) > I don't think glib needs to be in the business of shipping and installing > makefile fragments. And you can use the just fine without it being committed > to glib. Just copy it from libsoup. Or teach gnome-builder to use this > fragment, if it is so awesome... Fair enough. So we just need to merge the wip/glib-build.mk branch and do some documentation updates as Dan suggested in comment #19.
Times move on: we now have Meson, and Emmanuele has done bug #788948 and hence the glib-mkenums and glib-genmarshal man pages have examples of the right autotools magic for them. Rather than resurrecting and maintaining this .mk file in time for it to die, I’m happy to consider this fixed by bug #788948. If anyone disagrees, please re-open this bug report.