Subject: Re: bug#10894: Mixed C-Vala projects distcheck is broken

From: Stefano Lattarini <[email-masked]>
Date: Mon, 27 Feb 2012 22:33:05 +0100

Hi Quentin, thanks for the report.

On 02/26/2012 10:12 PM, Quentin Glidic wrote:
> Package: automake
> Version: 1.11.3
> 
> Hello,
> 
> The distcheck of a project which mix C and Vala targets in the same Makefile.am
> leads to a pass of C targets to valac, which is completely wrong and fails if
> you put your headers in _SOURCES (valac ignores *.c files only).
>
This is bad.  We had a test in the automake testsuite that tried to build a
project with mixed C/Vala sources, but it didn't use a C header as well :-(

I've thus added a new test case that exposes the bug (see the first attached
patch).

> Attached a patch that limits the vala stamps’ generation to targets with
> *.vala file.
>
Unfortunately, your patch doesn't seem to fix the issue.  I've attached a
second patch that seems to do the trick.  It will appear in the next automake
version (1.12) unless there are objections.

Thanks,
  Stefano

>From 7d335efe6f874af817c2f29ef85a803d8db8a43a Mon Sep 17 00:00:00 2001
Message-Id: <7d335efe6[email-masked]>
From: Stefano Lattarini <[email-masked]>
Date: Mon, 27 Feb 2012 21:16:22 +0100
Subject: [PATCH 1/2] vala: expose automake bug#10894

* tests/vala-mix2.test: New test.
* tests/list-of-tests.mk: Add it.
* tests/Makefile.am (XFAIL_TESTS): Likewise.
* THANKS: Update.
---
 THANKS                 |    1 +
 tests/Makefile.am      |    1 +
 tests/list-of-tests.mk |    1 +
 tests/vala-mix2.test   |  121 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 124 insertions(+), 0 deletions(-)
 create mode 100755 tests/vala-mix2.test

diff --git a/THANKS b/THANKS
index b512b6b..d54e9d4 100644
--- a/THANKS
+++ b/THANKS
@@ -264,6 +264,7 @@ Olivier Louchart-Fletcher [email-masked]
 Olly Betts		[email-masked]
 Oren Ben-Kiki		[email-masked]
 Owen Taylor		[email-masked]
+Quentin Glidic		[email-masked]
 Patrick Welche		[email-masked]
 Patrik Weiskircher	[email-masked]
 Paul Berrevoets		[email-masked]
diff --git a/tests/Makefile.am b/tests/Makefile.am
index eab45dc..8fd33f4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -30,6 +30,7 @@ pr8365-remake-timing.test \
 lex-subobj-nodep.test \
 remake-am-pr10111.test \
 remake-m4-pr10111.test \
+vala-mix2.test \
 txinfo5.test
 
 include $(srcdir)/parallel-tests.am
diff --git a/tests/list-of-tests.mk b/tests/list-of-tests.mk
index bc64d61..2b73de2 100644
--- a/tests/list-of-tests.mk
+++ b/tests/list-of-tests.mk
@@ -885,6 +885,7 @@ vala4.test \
 vala5.test \
 vala-vpath.test \
 vala-mix.test \
+vala-mix2.test \
 vars.test \
 vars3.test \
 vartar.test \
diff --git a/tests/vala-mix2.test b/tests/vala-mix2.test
new file mode 100755
index 0000000..88d2818
--- /dev/null
+++ b/tests/vala-mix2.test
@@ -0,0 +1,121 @@
+#! /bin/sh
+# Copyright (C) 2012 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Vala sources, C and C++ sources and C and C++ headers in the same
+# program.  Functional test.  See automake bug#10894.
+
+required='valac cc c++ GNUmake'
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in <<'END'
+AC_PROG_CC
+AC_PROG_CXX
+AM_PROG_VALAC
+AC_OUTPUT
+END
+
+cat > Makefile.am <<'END'
+bin_PROGRAMS = zardoz
+AM_VALAFLAGS = --profile=posix
+zardoz_SOURCES = zardoz.vala foo.h bar.c baz.c zen.hh master.cxx
+END
+
+cat > zardoz.vala <<'END'
+int main ()
+{
+  stdout.printf ("foo is alive\n");
+  return 0;
+}
+END
+
+cat > foo.h <<'END'
+int foo;
+int bar (void);
+int baz (void);
+END
+
+cat > bar.c <<'END'
+#include "foo.h"
+int bar (void) { return foo + baz (); }
+END
+
+cat > baz.c <<'END'
+#include "foo.h"
+extern int foo = 0;
+int baz (void) { return 0; }
+END
+
+cat > zen.hh <<'END'
+#include <iostream>
+END
+
+cat > master.cxx <<'END'
+#include "zen.hh"
+void chatty (void) { std::cout << "Hello, stranger!\n"; }
+END
+
+$ACLOCAL
+$AUTOMAKE -a
+$AUTOCONF
+
+./configure
+
+$MAKE all
+ls -l # For debugging.
+
+have_generated_files ()
+{
+  test -f zardoz_vala.stamp
+  test -f zardoz.c
+}
+
+# Our vala-related rules must create stamp files and intermediate
+# C files.
+have_generated_files
+
+# Remake rules are not uselessly triggered.
+$MAKE -q
+$MAKE -n | $FGREP vala.stamp && Exit 1
+
+# But are triggered when they should.
+for file in zardoz.vala foo.h bar.c baz.c zen.hh master.cxx; do
+  $sleep
+  echo '& choke me !' >> $file
+  $MAKE && Exit 1
+  $sleep
+  sed '$d' $file > t
+  mv -f t $file
+  $MAKE
+done
+
+# Check the distribution.
+$MAKE distcheck
+
+# Stamp files and intermediate C files should *not* be removed
+# by "make clean".
+$MAKE clean
+ls -l # For debugging.
+have_generated_files
+
+# But stamp files should be removed by "maintainer-clean" (the
+# behaviour w.r.t. intermediate C files is still unclear, and
+# better left undefined for the moment).
+$MAKE maintainer-clean
+ls *vala*.stamp | grep . && Exit 1
+
+:
-- 
1.7.9

>From 51f61dfb1e861062aaa1d73fab71278c85fe0594 Mon Sep 17 00:00:00 2001
Message-Id: <51f61dfb1[email-masked]>
In-Reply-To: <7d335efe6[email-masked]>
References: <7d335efe6[email-masked]>
From: Stefano Lattarini <[email-masked]>
Date: Mon, 27 Feb 2012 21:48:03 +0100
Subject: [PATCH 2/2] vala: fix 'valac' calls for projects with mixed Vala/C

Fixes automake bug#10894.

* automake.in (lang_vala_finish_target): Strip non-vala files
from the dependencies of vala stamps, and from the invocation
of valac compiler.  Related refactorings.
* tests/Makefile.am (XFAIL_TESTS): Remove 'vala-mix2.test'.
---
 automake.in       |   27 +++++++++++++--------------
 tests/Makefile.am |    1 -
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/automake.in b/automake.in
index cdbb820..5ae151b 100644
--- a/automake.in
+++ b/automake.in
@@ -6014,20 +6014,19 @@ sub lang_vala_finish_target ($$)
   my ($self, $name) = @_;
 
   my $derived = canonicalize ($name);
-  my $varname = $derived . '_SOURCES';
-  my $var = var ($varname);
+  my $var = var "${derived}_SOURCES";
+  return unless $var;
 
-  if ($var)
+  my @vala_sources = grep { /\.vala$/ } ($var->value_as_list_recursive);
+
+  foreach my $vala_file (@vala_sources)
     {
-      foreach my $file ($var->value_as_list_recursive)
-        {
-          $output_rules .= "\$(srcdir)/$file: \$(srcdir)/${derived}_vala.stamp\n"
-            . "\t\@if test -f \$@; then :; else rm -f \$(srcdir)/${derived}_vala.stamp; fi\n"
-            . "\t\@if test -f \$@; then :; else \\\n"
-            . "\t  \$(MAKE) \$(AM_MAKEFLAGS) \$(srcdir)/${derived}_vala.stamp; \\\n"
-	    . "\tfi\n"
-            if $file =~ s/(.*)\.vala$/$1.c/;
-        }
+      (my $c_file = $vala_file) =~ s/(.*)\.vala$/$1.c/;
+      $output_rules .= "\$(srcdir)/$c_file: \$(srcdir)/${derived}_vala.stamp\n"
+        . "\t\@if test -f \$@; then :; else rm -f \$(srcdir)/${derived}_vala.stamp; fi\n"
+        . "\t\@if test -f \$@; then :; else \\\n"
+        . "\t  \$(MAKE) \$(AM_MAKEFLAGS) \$(srcdir)/${derived}_vala.stamp; \\\n"
+        . "\tfi\n"
     }
 
   # Add rebuild rules for generated header and vapi files
@@ -6077,7 +6076,7 @@ sub lang_vala_finish_target ($$)
   my $silent = silent_flag ();
 
   $output_rules .=
-    "\$(srcdir)/${derived}_vala.stamp: \$(${derived}_SOURCES)\n".
+    "\$(srcdir)/${derived}_vala.stamp: @vala_sources\n".
 # Since the C files generated from the vala sources depend on the
 # ${derived}_vala.stamp file, we must ensure its timestamp is older than
 # those of the C files generated by the valac invocation below (this is
@@ -6085,7 +6084,7 @@ sub lang_vala_finish_target ($$)
 # Thus we need to create the stamp file *before* invoking valac, and to
 # move it to its final location only after valac has been invoked.
     "\t${silent}rm -f \$\@ && echo stamp > \$\@-t\n".
-    "\t${verbose}\$(am__cd) \$(srcdir) && ${compile} \$(${derived}_SOURCES)\n".
+    "\t${verbose}\$(am__cd) \$(srcdir) && $compile @vala_sources\n".
     "\t${silent}mv -f \$\@-t \$\@\n";
 
   push_dist_common ("${derived}_vala.stamp");
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 8fd33f4..eab45dc 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -30,7 +30,6 @@ pr8365-remake-timing.test \
 lex-subobj-nodep.test \
 remake-am-pr10111.test \
 remake-m4-pr10111.test \
-vala-mix2.test \
 txinfo5.test
 
 include $(srcdir)/parallel-tests.am
-- 
1.7.9