GNOME Bugzilla – Bug 772027
Modernize autotools configuration
Last modified: 2016-10-01 19:37:04 UTC
https://wiki.gnome.org/Initiatives/GnomeGoals/ModernAutotools https://wiki.gnome.org/Projects/GnomeCommon/Migration Patches to follow soon
Created attachment 336583 [details] [review] build: Modernize autogen.sh This gets rid of the dependency on the old gnome-autogen.sh and moves instead to the suggested autogen.sh given as an example in the gnome-common migration guide: https://wiki.gnome.org/Projects/GnomeCommon/Migration
Created attachment 336584 [details] [review] build: Use AX_IS_RELEASE Check for the presence of a git repository to determine whether things like compiler warnings should be fatal. The warnings themselves will be hooked up later. This adds an implicit dependency on autoconf-archive or GNOME’s m4-common. These are pulled in by JHBuild, and the relevant macros are automatically distributed in tarballs — so the dependencies only need to be explicitly installed if you are building from git without using JHBuild. This is an unsupported use case. Note that AC_CONFIG_MACRO_DIR() has already been added to configure.ac. https://wiki.gnome.org/Projects/GnomeCommon/Migration
Created attachment 336585 [details] [review] build: Remove GNOME_MAINTAINER_MODE_DEFINES This macro does not do anything relevant anymore. See: https://wiki.gnome.org/Projects/GnomeCommon/Migration
Created attachment 336586 [details] [review] build: Use five-argument form of AC_INIT This adds the website location to the package's manifest. See: https://wiki.gnome.org/Initiatives/GnomeGoals/ModernAutotools This requires Autoconf 2.64 which is already many years old and available in all major distros.
Created attachment 336587 [details] [review] build: Require Automake 1.11.1 Automake 1.11 was already required in practice, since that was the version in which the dist-xz option was introduced. We further require 1.11.1 since 1.11 is known to have a critical security issue, http://lists.gnu.org/archive/html/automake/2009-12/msg00012.html See: https://wiki.gnome.org/Initiatives/GnomeGoals/ModernAutotools
Created attachment 336588 [details] [review] build: Use new LT_INIT syntax for Libtool This requires Libtool 2.2.0, which is many years old and available in all major distros. See: https://wiki.gnome.org/Initiatives/GnomeGoals/ModernAutotools
Created attachment 336589 [details] [review] build: Disable Automake warnings about GNU make We require GNU make already, so add the Automake option which disables the warnings about GNU-only features.
Created attachment 336590 [details] [review] build: Replace deprecated AC_CONFIG_HEADER This macro can be replaced straight up by AC_CONFIG_HEADERS.
Created attachment 336591 [details] [review] build: Remove C compiler checks We don't use a C compiler anymore, so don't check for it, nor for characteristics of C compilers that should be standard in C++. Also don't add to CFLAGS, which will be unused anyway.
Created attachment 336592 [details] [review] build: Check for -Bsymbolic-functions with macro There is an Autoconf Archive macro for checking for linker flags now, AX_APPEND_LINK_FLAGS, so we can simplify our check.
Created attachment 336593 [details] [review] build: Improve readline check The AC_TRY_CPP macro is deprecated, and Automake provides a better macro to use: AC_CHECK_HEADERS. This works despite the possibly extra required linker flags because we call it within the scope where we have readline's flags added to LIBS. AC_CHECK_HEADERS defines its own preprocessor symbol with the result, so we can use that. The AM_CONDITIONAL symbol was unused anyway, so that can be removed.
Created attachment 336594 [details] [review] build: Skip Gtk.js test if GTK not detected If GJS was not compiled with support for GTK, then there is no point in running this test.
Created attachment 336595 [details] [review] build: Add comment for future maintainers The ACLOCAL_AMFLAGS variable is obsolete but harmless starting with Automake 1.13, but will not be supported in 2.0. Leave a note so that future maintainers will know when to remove it.
Created attachment 336596 [details] [review] build: Use AX_CODE_COVERAGE macro Instead of the homebrew stuff, use AX_CODE_COVERAGE from the Autoconf Archive. We do require a few extra options in order to include the coverage data for the JS files. See: https://wiki.gnome.org/Initiatives/GnomeGoals/ModernAutotools
Created attachment 336597 [details] [review] build: Remove malloc.h check The code that introduced this has been removed.
Created attachment 336598 [details] [review] build: Remove obsolete AC_ISC_POSIX This macro does checks for an operating system that was end-of-lifed in 2006.
Created attachment 336599 [details] [review] build: Modernize AC_ARG_ENABLE and AC_ARG_WITH These macros can be used with much more simple syntax, since they define variables themselves. This makes all the configure arguments follow the suggested patterns in https://autotools.io/autoconf/arguments.html (from the Autotools Mythbuster.)
Created attachment 336600 [details] [review] build: Use modern pkg-config macros The PKG_INSTALLDIR macro is available in pkg-config 0.27 and PKG_CHECK_VAR was introduced in 0.28, allowing for slightly cleaner build code. This does not affect what version of pkg-config is required if you build from a tarball. If running autoconf to regenerate configure, you now require 0.28. There is an even newer pkg-config macro to check what version of the M4 macros you have, but I don't want to require 0.29 just to check for 0.28. (0.28 is four years old, while 0.29 was released this year.)
Created attachment 336601 [details] [review] build: Use two-argument form of AC_SUBST Slightly more readable code.
Created attachment 336602 [details] [review] build: Enable subdir-objects and symlink GI files Currently we compile some of gobject-introspection's installed test code into our tests. In the next major version of Automake, this will not be supported anymore; object files and other compilation byproducts will be put into the same directory as the source file, which would not be possible with the GI files since they live in the installed tree. With current Automake we can turn on forward compatibility with the future behaviour using the subdir-objects option. At configure time we make symlinks to the GI files into the build dir so that they can still be compiled using the new Automake behaviour.
Created attachment 336603 [details] [review] build: Remove unused Make variable
Created attachment 336604 [details] [review] build: Enable silent rules for generated files We have several files that must be processed by make-time sed scripts. Make nicer build logs by using AM_V_GEN to enable silent rules for these. Also make sure the directories exist before the files are placed into them. This is usually not a problem, but will bite you on a clean checkout with builddir != srcdir.
Created attachment 336605 [details] [review] build: Distribute test files unconditionally Previously the installed test files would only make it into the distributed tarball if the package was configured with --enable-installed-tests. This is bad, since it means that the contents of the official release tarballs depend on the current configuration of whichever maintainer is making them. Instead, distribute the test files unconditionally, but only install them if configured with --enable-installed-tests.
Created attachment 336606 [details] [review] build: Use := for Makefile vars with $(shell) If you use = to assign a value consisting of a $(shell) invocation, then Make will keep shelling out and running the same process over and over every time the variable is used. Instead use := which computes the value only once.
Created attachment 336607 [details] [review] build: Check for programs This adds Autoconf's sanity checks to ln -s and sed.
Created attachment 336608 [details] [review] build: Remove DESTDIR from normal Automake rule In normal Automake usage, $(DESTDIR) is not necessary. Automake takes care of it behind the scenes. You only need it when doing funny stuff inside hook or local rules.
Created attachment 336609 [details] [review] build: Use git.mk for automatic .gitignore file This exposes a few files that should probably be in MAINTAINERCLEANFILES.
Created attachment 336610 [details] [review] build: Include private headers along with sources Don't use noinst_HEADERS; best practice according to https://www.gnu.org/software/automake/manual/html_node/Headers.html
Created attachment 336611 [details] [review] build: Leading tabs in Makefiles Get rid of leading spaces in Makefiles, they are dangerous; they can cause editors to go into spaces mode, and then you can get accidental spaces in your Make rules, which can cause them not to work in the most frustrating of fashions.
There's one item remaining from the migration guide: replace GNOME_CXX_WARNINGS with AX_COMPILER_FLAGS. I'll have to take a rain check on that one, and open a separate bug for it: it exposes a lot of warnings, some of which are non-trivial.
Review of attachment 336583 [details] [review]: ::: autogen.sh @@ +8,2 @@ +# shellcheck disable=SC2016 +PKG_NAME=$(autoconf --trace 'AC_INIT:$1' configure.ac) Should be moved below after the configure.ac check?
Review of attachment 336584 [details] [review]: OK
Review of attachment 336585 [details] [review]: OK
Review of attachment 336586 [details] [review]: OK
Review of attachment 336587 [details] [review]: OK
Review of attachment 336588 [details] [review]: OK
Review of attachment 336589 [details] [review]: OK
Review of attachment 336590 [details] [review]: OK
Review of attachment 336591 [details] [review]: OK
Review of attachment 336592 [details] [review]: Nice
Review of attachment 336593 [details] [review]: OK
Review of attachment 336594 [details] [review]: OK
Review of attachment 336595 [details] [review]: OK
Review of attachment 336596 [details] [review]: ::: Makefile-test.am @@ +139,3 @@ + --prefix $(abs_top_builddir) \ + $(NULL) +@CODE_COVERAGE_RULES@ Should these rules not be under the if CODE_COVERAGE_ENABLED section?
Review of attachment 336597 [details] [review]: OK
Review of attachment 336598 [details] [review]: OK
Review of attachment 336599 [details] [review]: Nice
Review of attachment 336600 [details] [review]: OK
Review of attachment 336601 [details] [review]: OK
Review of attachment 336602 [details] [review]: OK
Review of attachment 336603 [details] [review]: OK
Review of attachment 336604 [details] [review]: OK
Review of attachment 336605 [details] [review]: OK
Review of attachment 336606 [details] [review]: Nice, didn't know that
Review of attachment 336607 [details] [review]: OK
Review of attachment 336608 [details] [review]: OK
Review of attachment 336609 [details] [review]: OK
Review of attachment 336610 [details] [review]: OK
Review of attachment 336611 [details] [review]: OK
(In reply to Cosimo Cecchi from comment #31) > Review of attachment 336583 [details] [review] [review]: > > ::: autogen.sh > @@ +8,2 @@ > +# shellcheck disable=SC2016 > +PKG_NAME=$(autoconf --trace 'AC_INIT:$1' configure.ac) > > Should be moved below after the configure.ac check? True. I moved it above because the $PKG_NAME variable was used in the error message when there is no configure.ac, but that puts the egg before the chicken. The "trace configure.ac to get the package name" is a little too fancy for my taste anyhow, maybe I'll get rid of it. Presumably whoever runs autogen.sh should be aware of what package they're compiling... (In reply to Cosimo Cecchi from comment #44) > Review of attachment 336596 [details] [review] [review]: > > ::: Makefile-test.am > @@ +139,3 @@ > + --prefix $(abs_top_builddir) \ > + $(NULL) > +@CODE_COVERAGE_RULES@ > > Should these rules not be under the if CODE_COVERAGE_ENABLED section? No, there's already a check included inside the rules that are substituted for @CODE_COVERAGE_RULES@. They provide a helpful error message if you try to run "make check-code-coverage" without coverage enabled. http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=blob;f=m4/ax_code_coverage.m4;h=c146cdc562fbdafa5738a1d90371ae7448f4d126;hb=HEAD#l249
Attachment 336584 [details] pushed as a38614d - build: Use AX_IS_RELEASE Attachment 336585 [details] pushed as cbe0ddd - build: Remove GNOME_MAINTAINER_MODE_DEFINES Attachment 336586 [details] pushed as 8a3baee - build: Use five-argument form of AC_INIT Attachment 336587 [details] pushed as 75b6c22 - build: Require Automake 1.11.1 Attachment 336588 [details] pushed as dc6d27a - build: Use new LT_INIT syntax for Libtool Attachment 336589 [details] pushed as 61686bf - build: Disable Automake warnings about GNU make Attachment 336590 [details] pushed as 69447e9 - build: Replace deprecated AC_CONFIG_HEADER Attachment 336591 [details] pushed as afbe531 - build: Remove C compiler checks Attachment 336592 [details] pushed as e18c26a - build: Check for -Bsymbolic-functions with macro Attachment 336593 [details] pushed as 0d40554 - build: Improve readline check Attachment 336594 [details] pushed as d2426d3 - build: Skip Gtk.js test if GTK not detected Attachment 336595 [details] pushed as 7b801fc - build: Add comment for future maintainers Attachment 336597 [details] pushed as 0ba39fe - build: Remove malloc.h check Attachment 336598 [details] pushed as 80b88d7 - build: Remove obsolete AC_ISC_POSIX Attachment 336599 [details] pushed as c9e8734 - build: Modernize AC_ARG_ENABLE and AC_ARG_WITH Attachment 336600 [details] pushed as 96aa7a5 - build: Use modern pkg-config macros Attachment 336601 [details] pushed as 1922d22 - build: Use two-argument form of AC_SUBST Attachment 336602 [details] pushed as 6f533d2 - build: Enable subdir-objects and symlink GI files Attachment 336603 [details] pushed as b6f41f0 - build: Remove unused Make variable Attachment 336604 [details] pushed as 35975d6 - build: Enable silent rules for generated files Attachment 336605 [details] pushed as 26b4400 - build: Distribute test files unconditionally Attachment 336606 [details] pushed as 1fb999d - build: Use := for Makefile vars with $(shell) Attachment 336607 [details] pushed as 601bf86 - build: Check for programs Attachment 336608 [details] pushed as 86fd4fa - build: Remove DESTDIR from normal Automake rule Attachment 336609 [details] pushed as 73e1a29 - build: Use git.mk for automatic .gitignore file Attachment 336610 [details] pushed as 05f5be3 - build: Include private headers along with sources Attachment 336611 [details] pushed as 631f2fd - build: Leading tabs in Makefiles
Created attachment 336713 [details] [review] build: Modernize autogen.sh This gets rid of the dependency on the old gnome-autogen.sh and moves instead to the suggested autogen.sh given as an example in the gnome-common migration guide: https://wiki.gnome.org/Projects/GnomeCommon/Migration
Created attachment 336714 [details] [review] build: Use AX_CODE_COVERAGE macro Instead of the homebrew stuff, use AX_CODE_COVERAGE from the Autoconf Archive. We do require a few extra options in order to include the coverage data for the JS files. See: https://wiki.gnome.org/Initiatives/GnomeGoals/ModernAutotools
Review of attachment 336713 [details] [review]: OK
Review of attachment 336714 [details] [review]: OK
Attachment 336713 [details] pushed as 538a7c3 - build: Modernize autogen.sh Attachment 336714 [details] pushed as 46686ba - build: Use AX_CODE_COVERAGE macro