|
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
0001-Initial-support-for-the-vala-programming-language.patch
Description: Text Data
0002-Support-Vala-in-non-recursive-builds-more-tests-and.patch
Description: Text Data
0003-Minor-fixups-for-Vala-support.patch
Description: Text Data
0004-Improve-Vala-support.patch
Description: Text Data
[Prev in Thread] | Current Thread | [Next in Thread] |