Re: Vala support for automake

[Ralf Wildenhues]

Hello Jürg,

* 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.

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

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.

> It appears to work well with a simple test
> project including correct distcheck and maintainer-clean functionality.

We need to work on this.  It is by far the single way to shake out bugs
very quickly.  A list of things that need to be tested listed below.

> 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)?

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

> * 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.

> +# 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).

> @@ -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"

(should be tested with some non-GNU make).

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

The outer parentheses are not necessary here.

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).

> +        }
> +    }
> +
> +  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)?

> +  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?

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

$^ is not portable make.

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

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

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

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?

> +  $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.)

> +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.

> +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.
>  
>  
> address@hidden Vala Support
> address@hidden  node-name,  next,  previous,  up
> address@hidden Vala Support
> +
> address@hidden Vala Support
> address@hidden Support for Vala
> +
> +Automake provides support for Vala compilation

See question above about version information.

> +(@uref{http://www.vala-project.org/}).
> +
> address@hidden
> +foo_SOURCES = foo.vala bar.vala zardoc.c
> address@hidden 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.

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

> 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.

> +#
> +# 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.

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.

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.

> --- /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.

> +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.

> 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'.

> diff --git a/tests/vala4.test b/tests/vala4.test
> new file mode 100755
> index 0000000..a2e8c96
> --- /dev/null
> +++ b/tests/vala4.test
> @@ -0,0 +1,61 @@
> +#! /bin/sh
> +# Copyright (C) 2008  Free Software Foundation, Inc.

> +# Test AM_PROG_VALAC.
> +
> +required=libtoolize
> +. ./defs || Exit 1
> +
> +set -e
> +
> +cat >> configure.in << 'END'
> +AC_PROG_CC
> +AC_PROG_LIBTOOL
> +AM_PROG_VALAC([0.0.1])
> +AC_OUTPUT
> +END
> +
> +: > Makefile.am
> +
> +cat > valac << 'END'
> +#! /bin/sh
> +if test "x$1" = x--version; then
> +  echo 1.2.3
> +fi
> +exit 0
> +END
> +chmod +x valac
> +
> +libtoolize
> +$ACLOCAL
> +$AUTOMAKE -a
> +$AUTOCONF
> +./configure "VALAC=`pwd`/valac"
> +
> +sed 's/AM_PROG_VALAC.*/AM_PROG_VALAC([9999.9])/' < configure.in >t
> +mv -f t configure.in
> +$AUTOCONF --force
> +./configure "VALAC=`pwd`/valac" && Exit 1
> +
> +sed 's/AM_PROG_VALAC.*/AM_PROG_VALAC([1.2.3])/' < configure.in >t
> +mv -f t configure.in
> +$AUTOCONF --force
> +./configure "VALAC=`pwd`/valac"
> +:

Cheers,
Ralf