Created attachment 232492 [details] [review]
Fix srcdir != builddir build

Review of attachment 232492 [details] [review]:

What kind of error was this triggering?
If you tested this with a module that uses vala and it doesn't generally break distcheck, feel free to push.

::: Makefile.am
@@ +143,3 @@
 if LIBGD_STATIC
+noinst_DATA += $(builddir)/Gd-1.0.gir
+MAINTAINERCLEANFILES += $(builddir)/Gd-1.0.gir

Why did you remove it from EXTRA_DIST?

(In reply to comment #2)
> Review of attachment 232492 [details] [review]:
> 
> What kind of error was this triggering?
> If you tested this with a module that uses vala and it doesn't generally break
> distcheck, feel free to push.

The error is: "no rule to make ../../libgd/Gd-1.0.gir", inside baobab, built with ostbuild ($(top_builddir) = $(top_srcdir)/_build)
distcheck is already broken, because during distcheck the srcdir is not writable, but OTOH distcheck does not trigger this bug because the vapi file is shipped in the tarball and nothing else depends on the .gir file.

> ::: Makefile.am
> @@ +143,3 @@
>  if LIBGD_STATIC
> +noinst_DATA += $(builddir)/Gd-1.0.gir
> +MAINTAINERCLEANFILES += $(builddir)/Gd-1.0.gir
> 
> Why did you remove it from EXTRA_DIST?

distcheck breaks if you try to dist files from builddir. Also, gir files are arch-dependent and should not be shipped.

Okay, thanks. Feel free to push to master.

Attachment 232492 [details] pushed as 680a9f7 - Fix srcdir != builddir build

Created attachment 235159 [details] [review]
build-sys: fix distcheck for vapi projects

Since 680a9f72a0c3019cf4013ed026270ee7b9f1abc9, not dist'ing gir files
means one can't dist vapi files either and those files have to be
built at build time, ie like regular build targets.

Giovanni, I am going to revert your changes to fix our distcheck. Since you acknowdge you created another bug in 691808, I think it's fair to come back to previous state here and try to find a better solution. I agree we need to improve this.

*** Bug 691808 has been marked as a duplicate of this bug. ***

libgd already build-requires vapigen, even if the generated vapi was shipped in the tarball. I guess it should be acceptable if we stop disting gir&vapi and require gir & vapigen at build time.

Created attachment 235162 [details] [review]
Fix srcdir != builddir build, don't dist vapi and gir.

Generated files go in $(builddir), not $(srcdir).

(That was a trick to avoid requiring gir & vapi generation at build
time. But since libgd already requires vapigen & g-i at configure
time, it shouldn't be a problem to require them too at build-time)

Review of attachment 235162 [details] [review]:

::: Makefile.am
@@ +143,2 @@
 if LIBGD_STATIC
+noinst_DATA += Gd-1.0.gir

now I wonder why it's not in BUILT_SOURCES instead..

(In reply to comment #11)
> Review of attachment 235162 [details] [review]:
> 
> ::: Makefile.am
> @@ +143,2 @@
>  if LIBGD_STATIC
> +noinst_DATA += Gd-1.0.gir
> 
> now I wonder why it's not in BUILT_SOURCES instead..

I don't think it makes much difference, as long as nothing from the same makefile references it.

*** Bug 697204 has been marked as a duplicate of this bug. ***

Marc-Andre, Giovanni: does the proposed patch here (or the one from duplicate bug 697204) work for you?
Given that I don't use libgd with Vala in any of my projects I'll leave it up to you to test and merge the most correct solution :)

I believe this version that strip the builddir/srcdir from the gir target is best. As of now /opt/gnome/share/gobject-introspection-1.0/Makefile.introspection build the gir targets with INTROSPECTION_GIRS items which are also used to compute the gir_name (and does not support directories in the INTROSPECTION_GIRS items).
Thus it makes sense to depends only on "gir-file/gir-name token" .
Otherwise one should define on its own the "<path>/<gir_file/gir_name token>:" target to avoid it only working in the srcdir != builddir or only in the srcdir == builddir use cases.

To sum up the fix in this report fits best with the current gobject-introspection make definitions. Though those requires the gir to get generated in the builddir (and would not fixing on the gobject introspection make definition side to do otherwise).

ping

Review of attachment 235162 [details] [review]:

For what it's worth, this looks good to me, and is necessary (although not sufficient, Bug #697206) to jhbuild gnome-boxes at the moment.

::: Makefile.am
@@ +143,2 @@
 if LIBGD_STATIC
+noinst_DATA += Gd-1.0.gir

BUILT_SOURCES are for when there's a dependency that Automake can't see at autogen time, for example GDBus generating header files that are #include'd by a known .c file: because Automake gets header dependencies as a side-effect of compilation, it can't know what is #include'd until after one build has succeeded. This is not that situation, so BUILT_SOURCES should be unnecessary.

Review of attachment 235162 [details] [review]:

Barring a few things GTK+ proper has subsumed libgd in a lot of places, and there's a mass exodus from Autotools going on at the moment. Given all that, I don't know if it is even possible to test this patch. eg., currently Boxes doesn't uses neither Autotools nor libgd.

Thankfully it still survives at three-way merge.

So, I will trust Simon's review, and since this doesn't break gnome-photos' distcheck (it doesn't use introspection at all, but still), I am pushing it to master.