[Powertop] [PATCH 01/12] configure: use vertical lists and AS_IF macro

[Powertop] [PATCH 01/12] configure: use vertical lists and AS_IF macro

[Sami Kerola]

[-- Attachment #1: Type: text/plain, Size: 5470 bytes --]

Horizontal lists has downside of being managed as a single entry in
version control, which makes commits that add and remove items difficult
to follow.  With vertical lists each entry is managed separately, which
is much nicer.

Additionally make the 'if' statements to use AS_IF() macro, add project
URL to autoconfig init, and pretty print macro expressions to have
indentation, and bracing that follows more or less in K&R style.

Signed-off-by: Sami Kerola <kerolasa(a)iki.fi>
---
 configure.ac | 102 +++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 78 insertions(+), 24 deletions(-)

diff --git a/configure.ac b/configure.ac
index 0b02ad2..d38f750 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2,15 +2,21 @@
 # Process this file with autoconf to produce a configure script.
 
 AC_PREREQ([2.68])
-AC_INIT([powertop], [2.6.1], [powertop(a)lists.01.org])
-AM_INIT_AUTOMAKE([-Wall foreign ])
+AC_INIT([powertop], [2.6.1], [powertop(a)lists.01.org], [], [https://01.org/powertop])
+AM_INIT_AUTOMAKE([-Wall foreign])
 AC_LANG([C++])
-AC_CONFIG_FILES([Makefile src/Makefile traceevent/Makefile po/Makefile.in doc/Makefile])
+AC_CONFIG_FILES([ \
+	Makefile \
+	src/Makefile \
+	traceevent/Makefile \
+	po/Makefile.in \
+	doc/Makefile
+])
 AC_CONFIG_SRCDIR([src/main.cpp])
 AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_HEADERS([config.h])
 GETTEXT_PACKAGE=powertop
-AC_SUBST(GETTEXT_PACKAGE)
+AC_SUBST([GETTEXT_PACKAGE])
 AM_SILENT_RULES([yes])
 AM_GNU_GETTEXT([external])
 AM_GNU_GETTEXT_VERSION([0.18])
@@ -26,7 +32,23 @@ AM_PROG_CC_C_O
 
 # Checks for libraries.
 # Checks for header files.
-AC_CHECK_HEADERS([fcntl.h libintl.h limits.h locale.h malloc.h stdint.h stdlib.h string.h sys/ioctl.h sys/socket.h sys/statfs.h sys/time.h termios.h unistd.h ncurses.h])
+AC_CHECK_HEADERS([ \
+	fcntl.h \
+	libintl.h \
+	limits.h \
+	locale.h \
+	malloc.h \
+	ncurses.h \
+	stdint.h \
+	stdlib.h \
+	string.h \
+	sys/ioctl.h \
+	sys/socket.h \
+	sys/statfs.h \
+	sys/time.h \
+	termios.h \
+	unistd.h \
+])
 
 # Checks for typedefs, structures, and compiler characteristics.
 AC_HEADER_STDBOOL
@@ -44,41 +66,73 @@ AC_FUNC_MALLOC
 AC_FUNC_MMAP
 AC_FUNC_REALLOC
 AC_FUNC_STRTOD
-AC_CHECK_FUNCS([fdatasync getpagesize gettimeofday memmove memset mkdir munmap pow realpath regcomp select setlocale socket sqrt strcasecmp strchr strdup strerror strncasecmp strstr strtoul strtoull])
+AC_CHECK_FUNCS([ \
+	fdatasync \
+	getpagesize \
+	gettimeofday \
+	memmove \
+	memset \
+	mkdir \
+	munmap \
+	pow \
+	realpath \
+	regcomp \
+	select \
+	setlocale \
+	socket \
+	sqrt \
+	strcasecmp \
+	strchr \
+	strdup \
+	strerror \
+	strncasecmp \
+	strstr \
+	strtoul \
+	strtoull \
+])
 
-PKG_CHECK_MODULES([NCURSES], [ncursesw ncurses], [LIBS="$LIBS $ncurses_LIBS"],
-	AC_SEARCH_LIBS([delwin], [ncursesw ncurses], [], AC_MSG_ERROR([ncurses is required but was not found]), []))
+PKG_CHECK_MODULES([NCURSES], [ncursesw ncurses], [LIBS="$LIBS $ncurses_LIBS"], [
+	AC_SEARCH_LIBS([delwin], [ncursesw ncurses], [], [
+		AC_MSG_ERROR([ncurses is required but was not found])
+	], [])
+])
 
 has_libpci=0
-PKG_CHECK_MODULES([PCIUTILS], [libpci],[has_libpci=1],[
-	AC_SEARCH_LIBS([pci_get_dev], [pci],[has_libpci=1], [has_libpci=0] )])
+PKG_CHECK_MODULES([PCIUTILS], [libpci],[has_libpci=1], [
+	AC_SEARCH_LIBS([pci_get_dev], [pci],[has_libpci=1], [has_libpci=0])
+])
  
 
 has_libnl_ver=0
-# libnl-2 provides only libnl-2.0.pc file, so we check for separate libnl-genl-3.0.pc
-# pkg-config file just for libnl-3.0 case.
-#
+dnl libnl-2 provides only libnl-2.0.pc file, so we check for separate
+dnl libnl-genl-3.0.pc pkg-config file just for libnl-3.0 case.
 PKG_CHECK_MODULES([LIBNL], [libnl-3.0 >= 3.0 libnl-genl-3.0 >= 3.0], [has_libnl_ver=3], [
 	PKG_CHECK_MODULES([LIBNL], [libnl-2.0 >= 2.0], [has_libnl_ver=2], [
-		PKG_CHECK_MODULES([LIBNL], [libnl-1], [has_libnl_ver=1], [has_libnl_ver=0])])])
-if (test "$has_libnl_ver" -eq 0); then
-	AC_MSG_ERROR(libnl and libnl-genl are required but were not found)
-fi
-if (test "$has_libnl_ver" -gt 1); then
+		PKG_CHECK_MODULES([LIBNL], [libnl-1], [has_libnl_ver=1], [has_libnl_ver=0])
+	])
+])
+AS_IF([test "$has_libnl_ver" -eq 0], [
+	AC_MSG_ERROR([libnl and libnl-genl are required but were not found])
+])
+AS_IF([test "$has_libnl_ver" -gt 1], [
 	AC_DEFINE([HAVE_LIBNL20], [1], [Define if you have libnl-2.0 or higher])
-fi
+])
 
-if (test "$has_libpci" -eq 0); then 
-	AC_DEFINE([HAVE_NO_PCI],[1],[Define if pci is not supported])
+AS_IF([test "$has_libpci" -eq 0], [
+	AC_DEFINE([HAVE_NO_PCI], [1], [Define if pci is not supported])
         AC_MSG_WARN([
 			************* LIBPCI SUPPORT NOT CONFIGURED**************
 			If you need or want pci support, please install libpci 
 			and re-configure PowerTOP. 
 			*********************************************************
 	])
-fi
+])
 
-AC_SEARCH_LIBS([pthread_create], [pthread], [], AC_MSG_ERROR([libpthread is required but was not found]), [])
-AC_SEARCH_LIBS([inet_aton], [resolv], [], AC_MSG_ERROR([libresolv is required but was not found]), [])
+AC_SEARCH_LIBS([pthread_create], [pthread], [], [
+	AC_MSG_ERROR([libpthread is required but was not found])
+], [])
+AC_SEARCH_LIBS([inet_aton], [resolv], [], [
+	AC_MSG_ERROR([libresolv is required but was not found])
+], [])
 
 AC_OUTPUT
-- 
2.0.3

Re: [Powertop] [PATCH 01/12] configure: use vertical lists and AS_IF macro

[Alexandra Yates]

[-- Attachment #1: Type: text/plain, Size: 2497 bytes --]


> On 2 August 2014 13:17, Magnus Fromreide <magfr(a)lysator.liu.se> wrote:
>> On Sat, Aug 02, 2014 at 12:54:06PM +0100, Sami Kerola wrote:
>>> Horizontal lists has downside of being managed as a single entry in
>>> version control, which makes commits that add and remove items
>>> difficult
>>> to follow.  With vertical lists each entry is managed separately, which
>>> is much nicer.
>>
>> I agree with this but there is a small issue - in autoconf you don't
>> need to
>> have \'s at the end of the lines.
>
> I am afraid backslashes are not optional. When they are removed the
> ./autogen.sh will produce ./configure file that fails with following
> error:
>
> checking how to hardcode library paths into programs... immediate
> ./configure: line 19421: syntax error near unexpected token `fcntl.h'
> ./configure: line 19421: `      fcntl.h'
>
> I have not tried would backslash removing work with old autoconf
> versions. Mine is from autoconf upstream, and compiled about month
> ago, so it should be more or less up to date.
>
>>> Additionally make the 'if' statements to use AS_IF() macro, add project
>>> URL to autoconfig init
>>
>> It is always better to separate patches :-)
>
> The commit is split, here is the AS_IF update.
>
> https://github.com/kerolasa/powertop/commit/cc5cb0f2b16414449c123738a7bf0da08bf1b042
>
>>> and pretty print macro expressions to have
>>> indentation, and bracing that follows more or less in K&R style.
>>
>> This is pretty much a style issue and I ain't certain about which style
>> I
>> prefer. I am also unsure about the messages - have you adde newlines to
>> any
>> message?
>
> No. The messaging to users should stay exactly as they've always been.
>
> The point why I like K&R bracing when writing m4 is that at least I
> find it easier to validate visually when brace opens & close's follow
> indentation level in which they are effective. That said style changes
> are matters of taste, and I could very well be wrong^W against common
> consensus what is according good taste in this project. If so I can
> drop the change from final merge request.
>
> --
> Sami Kerola
> http://www.iki.fi/kerolasa/
> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop
>

PowerTOP follows the Linux kernel coding style.

https://www.kernel.org/doc/Documentation/CodingStyle

Thank you,
Alexandra.

Re: [Powertop] [PATCH 01/12] configure: use vertical lists and AS_IF macro

[Sami Kerola]

[-- Attachment #1: Type: text/plain, Size: 2118 bytes --]

On 2 August 2014 13:17, Magnus Fromreide <magfr(a)lysator.liu.se> wrote:
> On Sat, Aug 02, 2014 at 12:54:06PM +0100, Sami Kerola wrote:
>> Horizontal lists has downside of being managed as a single entry in
>> version control, which makes commits that add and remove items difficult
>> to follow.  With vertical lists each entry is managed separately, which
>> is much nicer.
>
> I agree with this but there is a small issue - in autoconf you don't need to
> have \'s at the end of the lines.

I am afraid backslashes are not optional. When they are removed the
./autogen.sh will produce ./configure file that fails with following
error:

checking how to hardcode library paths into programs... immediate
./configure: line 19421: syntax error near unexpected token `fcntl.h'
./configure: line 19421: `      fcntl.h'

I have not tried would backslash removing work with old autoconf
versions. Mine is from autoconf upstream, and compiled about month
ago, so it should be more or less up to date.

>> Additionally make the 'if' statements to use AS_IF() macro, add project
>> URL to autoconfig init
>
> It is always better to separate patches :-)

The commit is split, here is the AS_IF update.

https://github.com/kerolasa/powertop/commit/cc5cb0f2b16414449c123738a7bf0da08bf1b042

>> and pretty print macro expressions to have
>> indentation, and bracing that follows more or less in K&R style.
>
> This is pretty much a style issue and I ain't certain about which style I
> prefer. I am also unsure about the messages - have you adde newlines to any
> message?

No. The messaging to users should stay exactly as they've always been.

The point why I like K&R bracing when writing m4 is that at least I
find it easier to validate visually when brace opens & close's follow
indentation level in which they are effective. That said style changes
are matters of taste, and I could very well be wrong^W against common
consensus what is according good taste in this project. If so I can
drop the change from final merge request.

-- 
Sami Kerola
http://www.iki.fi/kerolasa/

Re: [Powertop] [PATCH 01/12] configure: use vertical lists and AS_IF macro

[Magnus Fromreide]

[-- Attachment #1: Type: text/plain, Size: 854 bytes --]

On Sat, Aug 02, 2014 at 12:54:06PM +0100, Sami Kerola wrote:
> Horizontal lists has downside of being managed as a single entry in
> version control, which makes commits that add and remove items difficult
> to follow.  With vertical lists each entry is managed separately, which
> is much nicer.

I agree with this but there is a small issue - in autoconf you don't need to
have \'s at the end of the lines.

> Additionally make the 'if' statements to use AS_IF() macro, add project
> URL to autoconfig init

It is always better to separate patches :-)

> and pretty print macro expressions to have
> indentation, and bracing that follows more or less in K&R style.

This is pretty much a style issue and I ain't certain about which style I
prefer. I am also unsure about the messages - have you adde newlines to any
message?

/MF