After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 643128 - Distcheck fails unless folks_released=1
Distcheck fails unless folks_released=1
Status: RESOLVED FIXED
Product: folks
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: Future
Assigned To: Jeremy Whiting
folks-maint
: 645521 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-02-23 21:47 UTC by Travis Reitter
Modified: 2012-08-14 20:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Suggested build clean-ups (2.90 KB, patch)
2011-03-17 18:38 UTC, Travis Reitter
committed Details | Review
0001-telepathy-fix-distcheck.patch (1.20 KB, patch)
2011-04-11 14:37 UTC, Alban Crequy
rejected Details | Review
Add .gir and .vapi files to DISTCLEANFILES if not building a release. (1.75 KB, patch)
2012-07-06 20:23 UTC, Jeremy Whiting
committed Details | Review

Description Travis Reitter 2011-02-23 21:47:46 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.
Comment 1 Travis Reitter 2011-03-13 23:34:45 UTC
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.
Comment 2 Philip Withnall 2011-03-15 19:56:31 UTC
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.)
Comment 3 Travis Reitter 2011-03-17 18:38:17 UTC
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.
Comment 4 Travis Reitter 2011-03-17 18:39:44 UTC
This latest patch still doesn't fix our problem, though it seems like a minor clean-up.
Comment 5 Travis Reitter 2011-03-22 15:13:29 UTC
*** Bug 645521 has been marked as a duplicate of this bug. ***
Comment 6 Travis Reitter 2011-03-25 23:12:32 UTC
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.
Comment 7 Travis Reitter 2011-03-25 23:13:08 UTC
(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 8 Philip Withnall 2011-03-29 22:37:24 UTC
Comment on attachment 183666 [details] [review]
Suggested build clean-ups

Marking committed as per comment #7.
Comment 9 Alban Crequy 2011-04-11 14:37:23 UTC
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?
Comment 10 Travis Reitter 2011-04-11 14:44:06 UTC
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).
Comment 11 Travis Reitter 2011-08-01 18:39:23 UTC
Punting bugs that won't be fixed by Folks 0.6.0.
Comment 12 Jeremy Whiting 2012-07-06 17:00:16 UTC
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?
Comment 13 Travis Reitter 2012-07-06 17:50:01 UTC
(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? :)
Comment 14 Jeremy Whiting 2012-07-06 20:23:38 UTC
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.
Comment 15 Travis Reitter 2012-07-06 21:45:35 UTC
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).
Comment 16 Jeremy Whiting 2012-07-20 19:33:20 UTC
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?
Comment 17 Philip Withnall 2012-07-30 09:47:08 UTC
(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*
Comment 18 Travis Reitter 2012-08-13 16:55:54 UTC
(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.
Comment 19 Philip Withnall 2012-08-14 20:03:19 UTC
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(-)