automake-patches
[Top][All Lists]
Advanced

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

Re: Vala support for automake


From: Jürg Billeter
Subject: Re: Vala support for automake
Date: Sun, 05 Apr 2009 15:45:44 +0200

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 address@hidden";
> 
> $^ 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.
> >  
> >  
> > 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.

Done.

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

Done.

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

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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]