GNOME Bugzilla – Bug 643128
Distcheck fails unless folks_released=1
Last modified: 2012-08-14 20:03:53 UTC
Running 'make distcheck' in Folks for non-release tarballs results in this failure: ERROR: files left in build directory after distclean: ./tests/lib/telepathy/contactlist/tp-test-contactlist.gir ./tests/lib/telepathy/contactlist/tp-test-contactlist.vapi ./backends/telepathy/lib/tp-lowlevel.gir ./backends/telepathy/lib/tp-lowlevel.vapi make[1]: *** [distcleancheck] Error 1 make[1]: Leaving directory `/home/rgs/devel/folks/folks-0.3.6.1.20110223/_build' make: *** [distcheck] Error 1 ======================================== There's some side-effect of folks_released=1 in configure.ac that prevents us from hitting this problem within release tarballs.
Sifting carefully through the flood of make --debug=all output suggests this is a matter of: 1. folks_released=1 forces enable_vala=yes (by our rule in configure.ac) 2. enable_vala=yes checks for GOBJECT_INTROSPECTION, which sets HAVE_INTROSPECTION 3. this ends up forcing the re-generation of tp-lowlevel.gir and tp-test-contactlist.gir, which end up forcing re-generation of their vapi files: Successfully remade target file `libtp-lowlevel.la'. Finished prerequisites of target file `tp-lowlevel.gir'. Prerequisite `../../../../backends/telepathy/lib/tp-lowlevel.c' is older than target `tp-lowlevel.gir'. Prerequisite `../../../../backends/telepathy/lib/tp-lowlevel.h' is older than target `tp-lowlevel.gir'. Prerequisite `../../../../backends/telepathy/lib/tp-lowlevel.c' is older than target `tp-lowlevel.gir'. Prerequisite `../../../../backends/telepathy/lib/tp-lowlevel.h' is older than target `tp-lowlevel.gir'. Prerequisite `libtp-lowlevel.la' is newer than target `tp-lowlevel.gir'. Must remake target `tp-lowlevel.gir'. Ignoring VPATH name `../../../../backends/telepathy/lib/tp-lowlevel.gir'. Putting child 0x022b5fa0 (tp-lowlevel.gir) PID 24491 on the chain. Live child 0x022b5fa0 (tp-lowlevel.gir) PID 24491 GISCAN tp-lowlevel.gir ===================== However, when HAVE_INTROSPECTION is undefined, we get this make output. Note that there's no GISCAN command, so tp-lowlevel.gir isn't actually re-created (ie, this seems slightly bogus -- it really should have to be remade if its source library is newer than it). Successfully remade target file `libtp-lowlevel.la'. Finished prerequisites of target file `tp-lowlevel.gir'. Prerequisite `../../../../backends/telepathy/lib/tp-lowlevel.c' is older than target `tp-lowlevel.gir'. Prerequisite `../../../../backends/telepathy/lib/tp-lowlevel.h' is older than target `tp-lowlevel.gir'. Prerequisite `libtp-lowlevel.la' is newer than target `tp-lowlevel.gir'. Must remake target `tp-lowlevel.gir'. Ignoring VPATH name `../../../../backends/telepathy/lib/tp-lowlevel.gir'. Successfully remade target file `tp-lowlevel.gir'. 4. the files from 3. are getting copied to _build, instead of make just using the distributed files (which happens when HAVE_INTROSPECTION is undefined) This seems like a bug in the gobject-introspection autotools support. I'm not sure of a decent way to fix it or work around it, though.
From a quick poke in backends/telepathy/lib/Makefile.am: • CLEANFILES += $(gir_DATA) $(typelib_DATA) looks unused as gir_DATA and typelib_DATA are undefined. • The GIR files should be distributed but not installed, so I think they should be listed as follows: noinst_gir_DATA = $(INTROSPECTION_GIRS). • The same is true of the typelib files. (This is just from a quick examination of the makefile, so I could be completely wrong. The CLEANFILES is definitely wrong at the moment, though.)
Created attachment 183666 [details] [review] Suggested build clean-ups Patches from branch: http://git.collabora.co.uk/?p=user/treitter/folks.git;a=shortlog;h=refs/heads/build-cleanup These address the comment #2, but not the bug itself.
This latest patch still doesn't fix our problem, though it seems like a minor clean-up.
*** Bug 645521 has been marked as a duplicate of this bug. ***
Review of attachment 183666 [details] [review]: Lost in the Great Bugzilla Failure of 2011: --- Comment #6 from Philip Withnall <bugzilla@tecnocode.c.u> 2011-03-24 09:05:27 UTC --- Review of attachment 183666 [details] [review]: --> (https://bugzilla.gnome.org/review?bug=643128&attachment=183666) Looks good to me.
(In reply to comment #6) > Review of attachment 183666 [details] [review]: > > Lost in the Great Bugzilla Failure of 2011: > > --- Comment #6 from Philip Withnall <bugzilla@tecnocode.c.u> 2011-03-24 > 09:05:27 UTC --- > Review of attachment 183666 [details] [review]: > --> (https://bugzilla.gnome.org/review?bug=643128&attachment=183666) > > Looks good to me. Applied to master. Leaving bug open, as this doesn't actually fix it. commit 6a4f08292269cf7add9d5214960ad6e40c57bced Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Thu Mar 17 11:00:24 2011 -0700 Distribute (but not install) internal .gir and .vapi files backends/telepathy/lib/Makefile.am | 4 ++-- tests/lib/telepathy/contactlist/Makefile.am | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) commit bbaf9a0dd285d2bed3dca1423866cbe74caa5f3e Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Thu Mar 17 10:50:20 2011 -0700 Cut unused automake variables. backends/telepathy/lib/Makefile.am | 2 -- tests/lib/telepathy/contactlist/Makefile.am | 2 -- 2 files changed, 0 insertions(+), 4 deletions(-)
Comment on attachment 183666 [details] [review] Suggested build clean-ups Marking committed as per comment #7.
Created attachment 185706 [details] [review] 0001-telepathy-fix-distcheck.patch Adding the file in DISTCLEANFILES works for me: "make distcheck" passes. Is there anything wrong with DISTCLEANFILES?
Review of attachment 185706 [details] [review]: The problem here is, if someone does this from a release tarball, they will suddenly require Vala and Gobject-introspect to compile. We make a point to bundle the necessary files since we require fairly new versions of each, and not all major distros catch up quickly. If you can come up with a nice way to avoid the issue above, I'd be happy to re-examine this. Otherwise, it might be best to just wait until Vala and g-i are stable enough that we can require them for release builds (which will solve some of our other issues as well).
Punting bugs that won't be fixed by Folks 0.6.0.
Travis, So to summarize if folks_released=1 is set we need to not have DISTCLEANFILES = $(dist_noinst_DATA) but if it is not set we can add that, right? If that's the case why not just wrap that line in checks for folks_released to be set to 0?
(In reply to comment #12) > Travis, > > So to summarize if folks_released=1 is set we need to not have DISTCLEANFILES = > $(dist_noinst_DATA) but if it is not set we can add that, right? > > If that's the case why not just wrap that line in checks for folks_released to > be set to 0? No problem comes to mind. Want to give it a shot? :)
Created attachment 218202 [details] [review] Add .gir and .vapi files to DISTCLEANFILES if not building a release. This patch makes make distcheck work whether folks_release is 1 or 0, if it's 0, DISTCLEANFILES adds the .gir and .vapi files.
Review of attachment 218202 [details] [review]: Shouldn't it be the other way around? (Bundling the .gir and .vapi files for releases only - the idea is to ensure the source in the tarball can be built without valac (I believe gobject-introspection is a hard requirement for some other reason in the release tarballs). As I've said, I wish we could just make valac a hard dependency but the Automake Vala rules make it impossible to /not/ bundle the generated C files, so we might as well be consistent until we have a work-around for that).
Well, if it's not a release it adds those files to the DISTCLEANFILES, so it doesn't ship them. Just did a quick make dist with release bit on and off and the only thing different besides file contents was the lack of intltool.m4 one of them. With this patch distcheck passes with and witohut the release bit set, but maybe there's a better way to test? Configuring with --disable-vala or something maybe?
(In reply to comment #16) > Well, if it's not a release it adds those files to the DISTCLEANFILES, so it > doesn't ship them. Just did a quick make dist with release bit on and off and > the only thing different besides file contents was the lack of intltool.m4 one > of them. With this patch distcheck passes with and witohut the release bit > set, but maybe there's a better way to test? Configuring with --disable-vala > or something maybe? *pokes Travis*
(In reply to comment #17) > (In reply to comment #16) > > Well, if it's not a release it adds those files to the DISTCLEANFILES, so it > > doesn't ship them. Just did a quick make dist with release bit on and off and > > the only thing different besides file contents was the lack of intltool.m4 one > > of them. With this patch distcheck passes with and witohut the release bit > > set, but maybe there's a better way to test? Configuring with --disable-vala > > or something maybe? > > *pokes Travis* bug #681420 blocks me from thoroughly checking this, but if it works both with folks_released=0 and =1, and you can build tarballs from each distcheck without having valac available, that's fine with me.
Seems to work here. Pushed to master! commit c9f16d291d1053e15f968dd23403f0c12573d9b8 Author: Jeremy Whiting <jpwhiting@kde.org> Date: Fri Jul 6 14:21:02 2012 -0600 Add gir and .vapi files to DISTCLEANFILES if not building a release Closes: https://bugzilla.gnome.org/show_bug.cgi?id=643128 backends/telepathy/lib/Makefile.am | 4 ++++ configure.ac | 2 ++ tests/lib/telepathy/contactlist/Makefile.am | 4 ++++ 3 files changed, 10 insertions(+), 0 deletions(-)