GNOME.org
 

Re: [Vala] Vala support for automake

Hi Ralf,

On Tue, 2009-03-31 at 23:15 +0200, Ralf Wildenhues wrote:

* Jürg Billeter wrote on Tue, Mar 31, 2009 at 01:11:55PM CEST:

I've updated the Vala support patches initially written by Mathias
Hasselmann to fix a few issues and to conform to the reworked header
file generation in Vala 0.7.


Thanks for your work on this.  I see your patch is based on current
Automake git master.  If you like, you can rebase it against the 'next'
branch, which has currently the most recent beta.  Or as a rediff
against the mh-vala-support branch, that would have the aesthetic
advantage of showing which parts were contributed by Mathias and which
parts by you.

Anyway, no need to do either of this.  The rebase to "next" should be
a trivial merge though, and all that should be fixed up is a 'lder'
entry in the register_language call, so that silent-rules mode works
nicely with valac.


Thanks for your prompt response. I finally found some time today to
address your comments. I've rebased mh-vala-support branch and my patch
against the 'next' branch and attached the full patch series. I've also
made the necessary changes to get silent-rules mode working with valac.


BTW, where can we read about the current semantics of valac?


There is no formal documentation at the moment, the changes performed
recently are described in bug 572536 [1].


Where can I get a recent vala compiler to install, and if there isn't an
unofficial Debian package for it yet (and the build-deps have changed
compared to 0.5.7.1), what other packages do I need to install it?
Thanks.


Tarball releases do not need anything exotic to build. Only a relatively
recent GLib version (2.12 or later) needs to be installed in addition to
common build tools. I'm intending to release Vala 0.7.0 today.


The Vala support is intended to be used with the upcoming Vala 0.7
release series (git master) and all later versions. Earlier versions may
work with some limitations (e.g., only recursive make). Vala 0.7.0 is
expected to be released this week.


Should the section about Vala support in the Automake manual (and the
NEWS addition) mention that 0.7 is the earliest Vala version this is
suitable with (if that is indeed the case)?


Done.


More comments inline.  Some are rather notes to self, but the more you
can address the better.


I've tried to address as many comments as possible, see my comments
inline.


* automake.in: Add %known_libraries, lang_vala_rewrite,
lang_vala_finish and lang_vala_target_hook to support the Vala
programming language. Register Vala language hooks.
* doc/automake.texi, NEWS: Document Vala support.
* lib/am/vala.am: Empty rules file to prevent creation of depend2
based rules for Vala code.
* lib/am/Makefile.am (dist_am_DATA): Add vala.am.
* m4/vala.m4: Provide AM_PROG_VALAC for detecting the Vala compiler.
* m4/Makefile.am (dist_m4data_DATA): Add vala.m4.
* tests/vala.test: Test Vala support.
* tests/vala1.test: Test .c file generation.
* tests/vala2.test: Test recursive make.
* tests/vala3.test: Test non-recursive make.
* tests/vala4.test: Test AM_PROG_VALAC.
* tests/Makefile.am: Update.
Based on patch by Mathias Hasselmann.


I'd like to add Mathias to ChangeLog, and both of you to THANKS before
committing.


Done.


+# Vala
+register_language ('name' => 'vala',
+              'Name' => 'Vala',
+              'config_vars' => ['VALAC'],
+              'flags' => ['VALAFLAGS'],
+              'compile' => '$(VALAC) $(AM_VALAFLAGS) $(VALAFLAGS)',
+              'compiler' => 'VALACOMPILE',
+              'extensions' => ['.vala'],
+              'output_extensions' => sub { (my $ext = $_[0]) =~ s/vala$/c/;
+                                           return ($ext,) },
+              'rule_file' => 'vala',
+              '_finish' => \&lang_vala_finish,
+              '_target_hook' => \&lang_vala_target_hook,
+              'nodist_specific' => 1);


The nodist_specific setting should be exposed in some test (i.e., after
following recommendations below, there should be a test that fails if
this setting is removed).


I'm not familiar enough with nodist_ sources, can you help here?


@@ -5646,6 +5676,73 @@ sub lang_c_finish
     }
 }
 
+sub lang_vala_finish_target ($$)
+{
+  my ($self, $name) = @_;
+
+  my $derived = canonicalize ($name);
+  my $varname = $derived . '_SOURCES';
+  my $var = var ($varname);
+
+  if ($var)
+    {
+      foreach my $file ($var->value_as_list_recursive)
+        {
+          $output_rules .= "$file: ${derived}_vala.stamp ;\n"


Hmm.  This doesn't look like one of the approaches mentioned in
  info Automake "Multiple Outputs"


I've added the recover from removal commands recommended in the
documentation.


+            if ($file =~ s/(.*)\.vala$/$1.c/);


The outer parentheses are not necessary here.


Fixed.


The translation here is not correct if per-target flags (VALAFLAGS) are
used.  The Makefile.in generated by vala.test shows this: DIST_COMMON
lists zardoz-zardoz.c and maintainer-clean removes it, but the rule is
only for zardoz.c.

This renaming of outputs is necessary for something like
  bin_PROGRAMS = foo bar
  foo_SOURCES = baz.vala
  bar_SOURCES = baz.vala
  bar_VALAFLAGS = -bla

where the -bla flag causes the .c files for foo and bar to be different.

This should be exposed in the testsuite (as XFAIL if it is not fixed).


Done (as XFAIL).


+        }
+    }
+
+  my $compile = $self->compile;
+
+  # Rewrite each occurrence of `AM_$flag' in the compile
+  # rule into `${derived}_$flag' if it exists.
+  for my $flag (@{$self->flags})
+    {
+      my $val = "${derived}_$flag";
+      $compile =~ s/\(AM_$flag\)/\($val\)/
+        if set_seen ($val);
+    }


Is this exposed in the testsuite additions (i.e., if you remove this
loop, does a test fail)?


No, the testsuite does not seem to test this. This loop appears to have
been copied by Mathias from other places in automake.in. I don't know
how to properly test this.


+  my $dirname = dirname ($name);
+
+  $compile .= " -C";


Can you add a short comment on what -C does and why it is needed here?
Also, testsuite exposure?


Done. Without -C build wouldn't work at all, so testsuite covers that.


+  $output_rules .=
+    "${derived}_vala.stamp: \$(${derived}_SOURCES)\n".
+    "\t${compile} \$^\n\ttouch \$ \n";


$^ is not portable make.


Fixed.


The outputs need to be renamed (see above) if per-target flags are used.


Per-target vala flags are marked as XFAIL as noted above.


+  &push_dist_common ("${derived}_vala.stamp");


No need for '&' before function names (we are slowly transitioning away
from this style, see HACKING).


Done.


If this is distributed, does that mean you intend to always distribute
the generated ${derived}_SOURCES?  Are those .c files
system-independent?  What about accompanying .h files?


Yes, the .c files are system-independent and distributed. The Vala
compiler does not generate .h files by default. The user will need to
use -H foo.h to generate a .h file for a library and have it distributed
and installed by adding foo.h to _HEADERS.


+  $clean_files{"${derived}_vala.stamp"} = MAINTAINER_CLEAN;
+}



+# This is a vala helper which is called after all source file
+# processing is done.


Rather than documenting when a function is called, it is usually better
to state what it does.  (You can also leave in the information on when
it is called, but that shouldn't be the most important bit.)


Done.


+sub lang_vala_finish
+{
+  my ($self) = @_;
+
+  foreach my $prog (keys %known_programs)
+    {
+      lang_vala_finish_target ($self, $prog);
+    }
+
+  while (my ($name) = each %known_libraries)
+    {
+      lang_vala_finish_target ($self, $name);
+    }
+}



+# This is a vala helper which is called whenever we have decided to
+# compile a vala file.


Likewise.


Done.


+sub lang_vala_target_hook
+{
+  my ($self, $aggregate, $output, $input, %transform) = @_;
+
+  $clean_files{$output} = MAINTAINER_CLEAN;
+}




--- a/doc/automake.texi
+++ b/doc/automake.texi



@@ -6523,6 +6525,56 @@ using the @option{--main=} option.  The easiest way to do this is to use
 the @code{_LDFLAGS} variable for the program.
 
 
+ node Vala Support
+ comment  node-name,  next,  previous,  up
+ section Vala Support
+
+ cindex Vala Support
+ cindex Support for Vala
+
+Automake provides support for Vala compilation


See question above about version information.


Done.


+(@uref{http://www.vala-project.org/}).
+
+ example
+foo_SOURCES = foo.vala bar.vala zardoc.c
+ end example
+
+Any @file{.vala} file listed in a @code{_SOURCE} variable will be
+compiled into C code by the Vala compiler.


I think it should be documented whether generated .c/.h files are
distributed, or whether the end user is expected to have a vala compiler
installed.


Done.


+Automake ships with an Autoconf macro called @code{AM_PROG_VALAC}
+that will locate the Vala compiler and optionally check its version
+number.
+
+ defmac AM_PROG_VALAC (@ovar{MINIMUM-VERSION})
+Try to find a Vala compiler in @env{PATH}. If it is found, the variable
+ code{VALAC} is set. Optionally a minimum release number of the compiler
+can be requested:
+
+ example
+AM_PROG_VALAC([0.7.0])
+ end example
+ end defmac
+
+There are a few variables that are used when compiling Vala sources:
+
+ vtable @code
+ item VALAC
+Path to the the Vala compiler.
+
+ item VALAFLAGS
+Additional arguments for the Vala compiler.
+
+ item AM_VALAFLAGS
+The maintainer's variant of @code{VALAFLAGS}.
+
+ example
+lib_LTLIBRARIES = libfoo.la
+libfoo_la_SOURCES = foo.vala
+ end example
+ end vtable



--- /dev/null
+++ b/lib/am/vala.am
@@ -0,0 +1,17 @@
+## automake - create Makefile.in from Makefile.am
+## Copyright (C) 2008  Free Software Foundation, Inc.


2009.  :-)


Done.


diff --git a/m4/vala.m4 b/m4/vala.m4
new file mode 100644
index 0000000..5606296
--- /dev/null
+++ b/m4/vala.m4
@@ -0,0 +1,29 @@
+# Autoconf support for the Vala compiler
+
+# Copyright (C) 2008 Free Software Foundation, Inc.


See above.


Done.


+#
+# This file is free software; the Free Software Foundation
+# gives unlimited permission to copy and/or distribute it,
+# with or without modifications, as long as this notice is preserved.
+
+# serial 3
+
+# Check whether the Vala compiler exists in `PATH'. If it is found, the
+# variable VALAC is set. Optionally a minimum release number of the
+# compiler can be requested.
+#
+# AM_PROG_VALAC([MINIMUM-VERSION])
+# --------------------------------
+AC_DEFUN([AM_PROG_VALAC],
+[AC_PATH_PROG([VALAC], [valac], [])
+ AS_IF([test -z "$VALAC"],
+   [AC_MSG_WARN([No Vala compiler found.  You will not be able to compile .vala source files.])],
+   [AS_IF([test -n "$1"],
+      [AC_MSG_CHECKING([$VALAC is at least version $1])
+       am__vala_version=`$VALAC --version`
+       AS_VERSION_COMPARE([$1], ["$am__vala_version"],
+         [AC_MSG_RESULT([yes])],
+         [AC_MSG_RESULT([yes])],
+         [AC_MSG_RESULT([no])
+          AC_MSG_ERROR([Vala $1 not found.])])])])
+])




--- /dev/null
+++ b/tests/vala.test
@@ -0,0 +1,59 @@
+#! /bin/sh
+# Copyright (C) 1996, 2001, 2002, 2006, 2008  Free Software Foundation,
+# Inc.



+# Test to make sure intermediate .c files are built from vala source.
+
+required="libtool"
+. ./defs || Exit 1
+
+set -e
+
+cat >> 'configure.in' << 'END'
+AC_PROG_CC
+AC_PROG_LIBTOOL
+AM_PROG_VALAC
+AC_OUTPUT
+END
+
+cat > 'Makefile.am' <<'END'
+bin_PROGRAMS = zardoz
+zardoz_SOURCES = zardoz.vala
+zardoz_VALAFLAGS = --debug
+
+lib_LTLIBRARIES = libzardoz.la
+libzardoz_la_SOURCES = zardoz-foo.vala zardoz-bar.vala
+END
+
+: > ltmain.sh
+: > config.sub
+: > config.guess
+
+$ACLOCAL
+$AUTOMAKE -a
+
+grep 'VALAC' Makefile.in
+grep 'am_zardoz_OBJECTS' Makefile.in
+grep 'am_libzardoz_la_OBJECTS' Makefile.in
+grep 'zardoz_vala.stamp' Makefile.in
+grep 'libzardoz_la_vala.stamp' Makefile.in
+grep 'zardoz\.c' Makefile.in
+grep 'zardoz-foo\.c' Makefile.in
+
diff --git a/tests/vala1.test b/tests/vala1.test
new file mode 100755
index 0000000..1ee0455
--- /dev/null
+++ b/tests/vala1.test
@@ -0,0 +1,58 @@
+#! /bin/sh
+# Copyright (C) 1996, 2001, 2002, 2006, 2008  Free Software Foundation,
+# Inc.



+# Test to make sure intermediate .c files are built from vala sources
+# in non-recursive automake mode.
+
+required="libtool"
+. ./defs || Exit 1
+
+set -e
+
+cat >> 'configure.in' << 'END'
+AC_PROG_CC
+AC_PROG_LIBTOOL
+AM_PROG_VALAC
+AC_OUTPUT
+END
+
+cat > 'Makefile.am' <<'END'
+bin_PROGRAMS = src/zardoz
+src_zardoz_SOURCES = src/zardoz.vala
+
+lib_LTLIBRARIES = src/libzardoz.la
+src_libzardoz_la_SOURCES = src/zardoz-foo.vala src/zardoz-bar.vala
+END
+
+: > ltmain.sh
+: > config.sub
+: > config.guess
+
+$ACLOCAL
+$AUTOMAKE -a
+
+grep 'VALAC' Makefile.in
+grep 'src_zardoz_OBJECTS' Makefile.in
+grep 'src_libzardoz_la_OBJECTS' Makefile.in
+grep 'src_zardoz_vala.stamp' Makefile.in
+grep 'src_libzardoz_la_vala.stamp' Makefile.in
+grep 'zardoz\.c' Makefile.in
+grep 'src/zardoz-foo\.c' Makefile.in


This is good, but we should really also have a test that builds this
stuff with subdir sources, i.e., including running valac and all.  Since
Vala that requires so many prerequisites, maybe it's better to do that
in a separate test; just copy this one, add stuff to $required.


vala3.test tests full build with non-recursive make and subdirs.



Then, another program and another library which each have per-target
flags, i.e., *VALAFLAGS in this case.

Then something like
 ./configure
  $MAKE
  $MAKE distcheck
  $MAKE distclean
  mkdir build
  cd build
  ../configure
  $MAKE
  $MAKE distcheck

Also, we need a similar run of commands with a setup that has
"AUTOMAKE_OPTIONS = subdir-objects" enabled.  It can be done within the
same test.


Done in vala3.test.


When all of that passes, then we are probably ok in this area.
Well, rebuilding could be tested: touch or remove one of the input files
or intermediates, ensure things are rebuilt as necessary.


I didn't add a test for this, wasn't sure how to properly check this.
Various manual tests worked fine.


--- /dev/null
+++ b/tests/vala2.test
@@ -0,0 +1,70 @@
+#! /bin/sh
+# Copyright (C) 1996, 2001, 2002, 2006, 2008  Free Software Foundation,
+# Inc.



+# Test to make sure compiling Vala code really works with recursive make.
+
+required="libtool libtoolize pkg-config valac gcc"
+. ./defs || Exit 1
+
+set -e
+
+mkdir src
+
+cat >> 'configure.in' << 'END'
+AC_PROG_CC
+AM_PROG_CC_C_O
+AC_PROG_LIBTOOL
+AM_PROG_VALAC
+PKG_CHECK_MODULES(GOBJECT,gobject-2.0 >= 2.10)


M4 [quoting] for macro arguments, please.


Done.


+AC_CONFIG_FILES([src/Makefile])
+AC_OUTPUT
+END
+
+cat > 'Makefile.am' <<'END'
+SUBDIRS = src
+END
+
+cat > 'src/Makefile.am' <<'END'
+bin_PROGRAMS = zardoz
+zardoz_CFLAGS = $(GOBJECT_CFLAGS)
+zardoz_LDADD = $(GOBJECT_LIBS)
+zardoz_SOURCES = zardoz.vala
+END
+
+cat > 'src/zardoz.vala' <<'END'
+using GLib;
+
+public class Zardoz {
+  public static void main () {
+    stdout.printf ("Zardoz!\n");
+  }
+}
+END
+
+libtoolize
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE -a
+
+./configure
+$MAKE


Adding "$MAKE distcheck" here will, with Vala 0.3.4, error because
src/zardoz.h is not distributed but src/zardoz.c and
src/zardoz_vala.stamp are.


Vala 0.7.0 is required.


diff --git a/tests/vala3.test b/tests/vala3.test
new file mode 100755
index 0000000..9cca476
--- /dev/null
+++ b/tests/vala3.test
@@ -0,0 +1,64 @@
+#! /bin/sh
+# Copyright (C) 1996, 2001, 2002, 2006  Free Software Foundation, Inc.
+#
+# This file is part of GNU Automake.



+# Test to make sure compiling Vala code really works with non-recursive make.
+
+required="libtool libtoolize pkg-config valac gcc"
+. ./defs || Exit 1
+
+set -e
+
+mkdir src
+
+cat >> 'configure.in' << 'END'
+AC_PROG_CC
+AM_PROG_CC_C_O
+AC_PROG_LIBTOOL
+AM_PROG_VALAC
+PKG_CHECK_MODULES(GOBJECT,gobject-2.0 >= 2.10)
+AC_OUTPUT
+END
+
+cat > 'Makefile.am' <<'END'
+bin_PROGRAMS = src/zardoz
+src_zardoz_CFLAGS = $(GOBJECT_CFLAGS)
+src_zardoz_LDADD = $(GOBJECT_LIBS)
+src_zardoz_SOURCES = src/zardoz.vala
+END
+
+cat > 'src/zardoz.vala' <<'END'
+using GLib;
+
+public class Zardoz {
+  public static void main () {
+    stdout.printf ("Zardoz!\n");
+  }
+}
+END
+
+libtoolize
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE -a
+
+./configure
+$MAKE


FWIW this test fails for me with Vala 0.3.4, because zardoz.[ch] are not
generated in -C.  This may be fixed in your current version, so you
might want to use AM_PROG_VALAC([XXX]) to specify the minimum required
versino here and './configure || Exit 77'.


Done.

Regards,
Jürg

[1] http://bugzilla.gnome.org/show_bug.cgi?id=572536

Attachment: 0001-Initial-support-for-the-vala-programming-language.patch
Description: Text Data

Attachment: 0002-Support-Vala-in-non-recursive-builds-more-tests-and.patch
Description: Text Data

Attachment: 0003-Minor-fixups-for-Vala-support.patch
Description: Text Data

Attachment: 0004-Improve-Vala-support.patch
Description: Text Data

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]