GNOME Bugzilla – Bug 761356
Improvements to library-template build system templates
Last modified: 2016-02-11 10:53:24 UTC
Various patches to improve the library-template build system templates. There are still further improvements which could be made, such as standardising the naming and directory structure and ensuring it produces parallel-installable results. This will do for now though. See: • https://developer.gnome.org/programming-guidelines/unstable/namespacing.html.en • https://developer.gnome.org/programming-guidelines/unstable/parallel-installability.html.en • https://developer.gnome.org/programming-guidelines/unstable/versioning.html.en I have not actually tested these patches, so they might need some tweaking. Additionally, I’m not entirely sure of the changes to the Python script, though they seem to fit with what’s there already. Basically, this is a drive-by patch set and I make no apologies. Sorry.
Created attachment 320109 [details] [review] library-template: Update autogen.sh to use latest GNOME recommendation Port the template autogen.sh to use the latest recommendation from https://wiki.gnome.org/Projects/GnomeCommon/Migration#autogen.sh.
Created attachment 320110 [details] [review] library-template: Port from glib-gettext/intltool to upstream gettext This requires version 0.19.6 of upstream gettext for AppData support: https://lists.gnu.org/archive/html/info-gnu/2015-09/msg00003.html It is an error to use glib-gettext and intltool together (as the template was previously doing) — either use one or the other. However, both are deprecated in favour of upstream gettext, which is alive again: https://wiki.gnome.org/Projects/GnomeCommon/Migration#line-204 This has not been extensively tested, but should be the right general shape.
Created attachment 320111 [details] [review] library-template: Fix macro quoting in configure.ac To keep this pedant happy. I can’t explain why it’s better than before.
Created attachment 320112 [details] [review] library-template: Conditionally add po to SUBDIRS if enable_i18n is true
Created attachment 320113 [details] [review] library-template: use AX_GENERATE_CHANGELOG macro to generate ChangeLog This removes some more boilerplate code. http://www.gnu.org/software/autoconf-archive/ax_generate_changelog.html
Created attachment 320114 [details] [review] library-template: Add AM_V_GEN to the AUTHORS rule This should expose it in the automake output so the user can see it’s been generated.
Created attachment 320115 [details] [review] library-template: Remove AUTHORS from .PHONY target dependencies .PHONY targets are targets which don’t really exist — AUTHORS is the name of a generated file, so it does exist. https://www.gnu.org/software/make/manual/html_node/Phony-Targets.html
Created attachment 320116 [details] [review] library-template: Add $(NULL) terminator to Makefile.am lists This makes it easier to add new list entries without causing noise in the diff by fiddling with backslashes.
Created attachment 320117 [details] [review] library-template: Automatically include API version in build system Rather than hard-coding version 1.0 everywhere, use the api_version (formerly interface_age) variable to do it magically.
Created attachment 320118 [details] [review] library-template: Condense some AC_SUBST calls in configure.ac
Created attachment 320119 [details] [review] library-template: Use AX_CHECK_ENABLE_DEBUG for debug compiler flags This removes another load of boilerplate. http://www.gnu.org/software/autoconf-archive/ax_check_enable_debug.html We use the AX_IS_RELEASE macro to check whether to enable debug by default, using the micro-version policy. http://www.gnu.org/software/autoconf-archive/ax_is_release.html
Created attachment 320120 [details] [review] library-template: Use AX_COMPILER_FLAGS for determining compiler flags This uses a standard set of warning flags, and makes them available in WARN_CFLAGS, WARN_LDFLAGS, WARN_CXXFLAGS and WARN_SCANNERFLAGS. http://www.gnu.org/software/autoconf-archive/ax_compiler_flags.html
Review of attachment 320111 [details] [review]: Looks good. I really like this style.
Review of attachment 320113 [details] [review]: Didn't know about this before. Nice feature in autoconf.
Review of attachment 320119 [details] [review]: looks good.
Review of attachment 320120 [details] [review]: +1
Review of attachment 320116 [details] [review]: also like this style.
(In reply to Igor Gnatenko from comment #13) > Review of attachment 320111 [details] [review] [review]: > > Looks good. I really like this style. If you want an example of the build system I’m copying all of this out of: https://github.com/pwithnall/dunfell Other things which could be added at some point: • AX_CODE_COVERAGE (http://www.gnu.org/software/autoconf-archive/ax_code_coverage.html) • AX_VALGRIND_CHECK (http://www.gnu.org/software/autoconf-archive/ax_valgrind_check.html)
Can I get some review on the other patches? Rebasing the accepted ones to be pushed first is a bit of a pain.
(In reply to Philip Withnall from comment #19) > Can I get some review on the other patches? Rebasing the accepted ones to be > pushed first is a bit of a pain. Sorry, it was a gumption trap, I'll take a look.
Review of attachment 320109 [details] [review]: LGTM
Review of attachment 320110 [details] [review]: LGTM
Review of attachment 320112 [details] [review]: You can also do {{if enable_i18n}}po {{end}} if you feel like keeping it on a single line. Either way, LGTM.
Review of attachment 320114 [details] [review]: LGTM
Review of attachment 320115 [details] [review]: Hrmm, not sure where i copied that from. Anyway, as long as distcheck works, im happy :)
Review of attachment 320117 [details] [review]: LGTM. I was always a bit torn on doing API_VERSION because we don't actually rev them very often. But either way, seems like good hygiene.
Review of attachment 320118 [details] [review]: Oh duh!
Go ahead and push! Thanks again!
Attachment 320109 [details] pushed as e404e91 - library-template: Update autogen.sh to use latest GNOME recommendation Attachment 320110 [details] pushed as ffadf66 - library-template: Port from glib-gettext/intltool to upstream gettext Attachment 320111 [details] pushed as e6d4e10 - library-template: Fix macro quoting in configure.ac Attachment 320112 [details] pushed as 51b5eb1 - library-template: Conditionally add po to SUBDIRS if enable_i18n is true Attachment 320113 [details] pushed as 6e7846e - library-template: use AX_GENERATE_CHANGELOG macro to generate ChangeLog Attachment 320114 [details] pushed as 94c2795 - library-template: Add AM_V_GEN to the AUTHORS rule Attachment 320115 [details] pushed as 6e777d5 - library-template: Remove AUTHORS from .PHONY target dependencies Attachment 320116 [details] pushed as 3e9c1c4 - library-template: Add $(NULL) terminator to Makefile.am lists Attachment 320117 [details] pushed as 8287551 - library-template: Automatically include API version in build system Attachment 320118 [details] pushed as b325386 - library-template: Condense some AC_SUBST calls in configure.ac Attachment 320119 [details] pushed as fb9387c - library-template: Use AX_CHECK_ENABLE_DEBUG for debug compiler flags Attachment 320120 [details] pushed as 6de6fd5 - library-template: Use AX_COMPILER_FLAGS for determining compiler flags