[libvirt] [PATCH 0/4] Colourize configure output
An example how configure looks like with these changes applied: https://travis-ci.org/zippy2/libvirt/jobs/582549099#L4247 Michal Prívozník (4): configure: Prefer LIBVIRT_RESULT over AC_MSG_NOTICE virt-result.m4: Align string more generously configure: Colorize output virt-result.m4: Colourize summary printings configure.ac | 8 ++--- m4/virt-chrdev-lock-files.m4 | 2 +- m4/virt-colours.m4 | 62 ++++++++++++++++++++++++++++++++++++ m4/virt-debug.m4 | 2 +- m4/virt-default-editor.m4 | 2 +- m4/virt-dtrace.m4 | 2 +- m4/virt-host-validate.m4 | 2 +- m4/virt-init-script.m4 | 2 +- m4/virt-loader-nvram.m4 | 2 +- m4/virt-login-shell.m4 | 2 +- m4/virt-numad.m4 | 2 +- m4/virt-result.m4 | 21 ++++++++++-- 12 files changed, 93 insertions(+), 16 deletions(-) create mode 100644 m4/virt-colours.m4 -- 2.21.0
One of the advantages is that LIBVIRT_RESULT aligns the resulting message for us. The other is that in near future we will colour some parts of the message and thus it helps if we get arguments split in two. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- configure.ac | 8 ++++---- m4/virt-chrdev-lock-files.m4 | 2 +- m4/virt-debug.m4 | 2 +- m4/virt-default-editor.m4 | 2 +- m4/virt-dtrace.m4 | 2 +- m4/virt-host-validate.m4 | 2 +- m4/virt-init-script.m4 | 2 +- m4/virt-loader-nvram.m4 | 2 +- m4/virt-login-shell.m4 | 2 +- m4/virt-numad.m4 | 2 +- 10 files changed, 13 insertions(+), 13 deletions(-) diff --git a/configure.ac b/configure.ac index bf9e7681bc..a8f8b0517e 100644 --- a/configure.ac +++ b/configure.ac @@ -1047,14 +1047,14 @@ LIBVIRT_WIN_RESULT_WINDRES AC_MSG_NOTICE([]) AC_MSG_NOTICE([Test suite]) AC_MSG_NOTICE([]) -AC_MSG_NOTICE([ Coverage: $enable_test_coverage]) -AC_MSG_NOTICE([ Alloc OOM: $enable_test_oom]) +LIBVIRT_RESULT([Coverage], [$enable_test_coverage]) +LIBVIRT_RESULT([Alloc OOM], [$enable_test_oom]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Miscellaneous]) AC_MSG_NOTICE([]) LIBVIRT_RESULT_DEBUG -AC_MSG_NOTICE([ Use -Werror: $enable_werror]) -AC_MSG_NOTICE([ Warning Flags: $WARN_CFLAGS]) +LIBVIRT_RESULT([Use -Werror], [$enable_werror]) +LIBVIRT_RESULT([Warning Flags], [$WARN_CFLAGS]) LIBVIRT_RESULT_DTRACE LIBVIRT_RESULT_NUMAD LIBVIRT_RESULT_INIT_SCRIPT diff --git a/m4/virt-chrdev-lock-files.m4 b/m4/virt-chrdev-lock-files.m4 index 7d85c0e21b..5802136b9f 100644 --- a/m4/virt-chrdev-lock-files.m4 +++ b/m4/virt-chrdev-lock-files.m4 @@ -48,5 +48,5 @@ AC_DEFUN([LIBVIRT_CHECK_CHRDEV_LOCK_FILES], [ ]) AC_DEFUN([LIBVIRT_RESULT_CHRDEV_LOCK_FILES], [ - AC_MSG_NOTICE([ Char device locks: $with_chrdev_lock_files]) + LIBVIRT_RESULT([Char device locks], [$with_chrdev_lock_files]) ]) diff --git a/m4/virt-debug.m4 b/m4/virt-debug.m4 index d03cf10255..d3ac0564f2 100644 --- a/m4/virt-debug.m4 +++ b/m4/virt-debug.m4 @@ -29,5 +29,5 @@ AC_DEFUN([LIBVIRT_CHECK_DEBUG], [ ]) AC_DEFUN([LIBVIRT_RESULT_DEBUG], [ - AC_MSG_NOTICE([ Debug: $enable_debug]) + LIBVIRT_RESULT([Debug], [$enable_debug]) ]) diff --git a/m4/virt-default-editor.m4 b/m4/virt-default-editor.m4 index 0c8e1e0798..4b4cad4e87 100644 --- a/m4/virt-default-editor.m4 +++ b/m4/virt-default-editor.m4 @@ -28,5 +28,5 @@ AC_DEFUN([LIBVIRT_CHECK_DEFAULT_EDITOR], [ ]) AC_DEFUN([LIBVIRT_RESULT_DEFAULT_EDITOR], [ - AC_MSG_NOTICE([ Default Editor: $with_default_editor]) + LIBVIRT_RESULT([Default Editor], [$with_default_editor]) ]) diff --git a/m4/virt-dtrace.m4 b/m4/virt-dtrace.m4 index e072b639cd..3dc20d5343 100644 --- a/m4/virt-dtrace.m4 +++ b/m4/virt-dtrace.m4 @@ -41,5 +41,5 @@ AC_DEFUN([LIBVIRT_CHECK_DTRACE], [ ]) AC_DEFUN([LIBVIRT_RESULT_DTRACE], [ - AC_MSG_NOTICE([ DTrace: $with_dtrace]) + LIBVIRT_RESULT([DTrace], [$with_dtrace]) ]) diff --git a/m4/virt-host-validate.m4 b/m4/virt-host-validate.m4 index 643cd8f06e..e43cec5366 100644 --- a/m4/virt-host-validate.m4 +++ b/m4/virt-host-validate.m4 @@ -39,5 +39,5 @@ AC_DEFUN([LIBVIRT_CHECK_HOST_VALIDATE], [ ]) AC_DEFUN([LIBVIRT_RESULT_HOST_VALIDATE], [ - AC_MSG_NOTICE([virt-host-validate: $with_host_validate]) + LIBVIRT_RESULT([virt-host-validate], [$with_host_validate]) ]) diff --git a/m4/virt-init-script.m4 b/m4/virt-init-script.m4 index 91bbd68235..6eb81f0429 100644 --- a/m4/virt-init-script.m4 +++ b/m4/virt-init-script.m4 @@ -51,5 +51,5 @@ AC_DEFUN([LIBVIRT_CHECK_INIT_SCRIPT],[ ]) AC_DEFUN([LIBVIRT_RESULT_INIT_SCRIPT],[ - AC_MSG_NOTICE([ Init script: $with_init_script]) + LIBVIRT_RESULT([Init script], [$with_init_script]) ]) diff --git a/m4/virt-loader-nvram.m4 b/m4/virt-loader-nvram.m4 index 0eb77fa923..d7e0c8ca18 100644 --- a/m4/virt-loader-nvram.m4 +++ b/m4/virt-loader-nvram.m4 @@ -37,5 +37,5 @@ AC_DEFUN([LIBVIRT_CHECK_LOADER_NVRAM], [ ]) AC_DEFUN([LIBVIRT_RESULT_LOADER_NVRAM], [ - AC_MSG_NOTICE([ Loader/NVRAM: $with_loader_nvram]) + LIBVIRT_RESULT([Loader/NVRAM], [$with_loader_nvram]) ]) diff --git a/m4/virt-login-shell.m4 b/m4/virt-login-shell.m4 index 3baec5432f..713c488599 100644 --- a/m4/virt-login-shell.m4 +++ b/m4/virt-login-shell.m4 @@ -39,5 +39,5 @@ AC_DEFUN([LIBVIRT_CHECK_LOGIN_SHELL], [ ]) AC_DEFUN([LIBVIRT_RESULT_LOGIN_SHELL], [ - AC_MSG_NOTICE([ virt-login-shell: $with_login_shell]) + LIBVIRT_RESULT([virt-login-shell], [$with_login_shell]) ]) diff --git a/m4/virt-numad.m4 b/m4/virt-numad.m4 index e760dcc579..378eba3c97 100644 --- a/m4/virt-numad.m4 +++ b/m4/virt-numad.m4 @@ -54,5 +54,5 @@ AC_DEFUN([LIBVIRT_CHECK_NUMAD], [ ]) AC_DEFUN([LIBVIRT_RESULT_NUMAD], [ - AC_MSG_NOTICE([ numad: $with_numad]) + LIBVIRT_RESULT([numad], [$with_numad]) ]) -- 2.21.0
On Mon, Sep 09, 2019 at 09:49:39AM +0200, Michal Privoznik wrote:
One of the advantages is that LIBVIRT_RESULT aligns the resulting message for us. The other is that in near future we will colour some parts of the message and thus it helps if we get arguments split in two.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- configure.ac | 8 ++++---- m4/virt-chrdev-lock-files.m4 | 2 +- m4/virt-debug.m4 | 2 +- m4/virt-default-editor.m4 | 2 +- m4/virt-dtrace.m4 | 2 +- m4/virt-host-validate.m4 | 2 +- m4/virt-init-script.m4 | 2 +- m4/virt-loader-nvram.m4 | 2 +- m4/virt-login-shell.m4 | 2 +- m4/virt-numad.m4 | 2 +- 10 files changed, 13 insertions(+), 13 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
The times, when we had small CRTs are long gone. Now, in the era of wide screens we can be more generous when it comes to aligning the output of configure. The longest string before the colon is 'wireshark_dissector' which counts 19 characters. Therefore, align the strings at 20. At the same time, drop the useless result alignment. It behaves oddly - it puts a space at the end of each "no" because of the %-3s format we use. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- m4/virt-result.m4 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/m4/virt-result.m4 b/m4/virt-result.m4 index cc622fe35b..36973ba0b5 100644 --- a/m4/virt-result.m4 +++ b/m4/virt-result.m4 @@ -33,9 +33,9 @@ dnl LIBVIRT_RESULT([yajl], [yes], [-I/opt/yajl/include -lyajl]) dnl AC_DEFUN([LIBVIRT_RESULT], [ if test "$2" = "no" || test -z "$3" ; then - STR=`printf "%10s: %-3s" "$1" "$2"` + STR=`printf "%20s: %s" "$1" "$2"` else - STR=`printf "%10s: %-3s (%s)" "$1" "$2" "$3"` + STR=`printf "%20s: %s (%s)" "$1" "$2" "$3"` fi AC_MSG_NOTICE([$STR]) -- 2.21.0
On 9/9/19 3:49 AM, Michal Privoznik wrote:
The times, when we had small CRTs are long gone. Now, in the era of wide screens we can be more generous when it comes to aligning the output of configure. The longest string before the colon is 'wireshark_dissector' which counts 19 characters. Therefore, align the strings at 20.
At the same time, drop the useless result alignment. It behaves oddly - it puts a space at the end of each "no" because of the %-3s format we use.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- m4/virt-result.m4 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/m4/virt-result.m4 b/m4/virt-result.m4 index cc622fe35b..36973ba0b5 100644 --- a/m4/virt-result.m4 +++ b/m4/virt-result.m4 @@ -33,9 +33,9 @@ dnl LIBVIRT_RESULT([yajl], [yes], [-I/opt/yajl/include -lyajl]) dnl AC_DEFUN([LIBVIRT_RESULT], [ if test "$2" = "no" || test -z "$3" ; then - STR=`printf "%10s: %-3s" "$1" "$2"` + STR=`printf "%20s: %s" "$1" "$2"` else - STR=`printf "%10s: %-3s (%s)" "$1" "$2" "$3"` + STR=`printf "%20s: %s (%s)" "$1" "$2" "$3"` fi
AC_MSG_NOTICE([$STR])
For the first 2: Reviewed-by: Cole Robinson <crobinso@redhat.com> I like the look of the colors and I agree it speeds up visually scanning the configure output. But I'm neutral on whether adding more m4 to the build system to facilitate it is worth it. So I'll abstain from giving ack or nack on those. If you push the first two independently you may want to strip mention of colour from their commit messages - Cole
On 9/12/19 12:30 AM, Cole Robinson wrote:
On 9/9/19 3:49 AM, Michal Privoznik wrote:
The times, when we had small CRTs are long gone. Now, in the era of wide screens we can be more generous when it comes to aligning the output of configure. The longest string before the colon is 'wireshark_dissector' which counts 19 characters. Therefore, align the strings at 20.
At the same time, drop the useless result alignment. It behaves oddly - it puts a space at the end of each "no" because of the %-3s format we use.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- m4/virt-result.m4 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/m4/virt-result.m4 b/m4/virt-result.m4 index cc622fe35b..36973ba0b5 100644 --- a/m4/virt-result.m4 +++ b/m4/virt-result.m4 @@ -33,9 +33,9 @@ dnl LIBVIRT_RESULT([yajl], [yes], [-I/opt/yajl/include -lyajl]) dnl AC_DEFUN([LIBVIRT_RESULT], [ if test "$2" = "no" || test -z "$3" ; then - STR=`printf "%10s: %-3s" "$1" "$2"` + STR=`printf "%20s: %s" "$1" "$2"` else - STR=`printf "%10s: %-3s (%s)" "$1" "$2" "$3"` + STR=`printf "%20s: %s (%s)" "$1" "$2" "$3"` fi
AC_MSG_NOTICE([$STR])
For the first 2:
Reviewed-by: Cole Robinson <crobinso@redhat.com>
I like the look of the colors and I agree it speeds up visually scanning the configure output. But I'm neutral on whether adding more m4 to the build system to facilitate it is worth it. So I'll abstain from giving ack or nack on those.
Fair enough. When we switch to meson we'll get colours for free.
If you push the first two independently you may want to strip mention of colour from their commit messages
Yep, that was my plan. The first two patches make sense even without the rest. I'll push them shortly, thanks. Michal
On Thu, Sep 12, 2019 at 12:18:10PM +0200, Michal Privoznik wrote:
On 9/12/19 12:30 AM, Cole Robinson wrote:
On 9/9/19 3:49 AM, Michal Privoznik wrote:
The times, when we had small CRTs are long gone. Now, in the era of wide screens we can be more generous when it comes to aligning the output of configure. The longest string before the colon is 'wireshark_dissector' which counts 19 characters. Therefore, align the strings at 20.
At the same time, drop the useless result alignment. It behaves oddly - it puts a space at the end of each "no" because of the %-3s format we use.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- m4/virt-result.m4 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/m4/virt-result.m4 b/m4/virt-result.m4 index cc622fe35b..36973ba0b5 100644 --- a/m4/virt-result.m4 +++ b/m4/virt-result.m4 @@ -33,9 +33,9 @@ dnl LIBVIRT_RESULT([yajl], [yes], [-I/opt/yajl/include -lyajl]) dnl AC_DEFUN([LIBVIRT_RESULT], [ if test "$2" = "no" || test -z "$3" ; then - STR=`printf "%10s: %-3s" "$1" "$2"` + STR=`printf "%20s: %s" "$1" "$2"` else - STR=`printf "%10s: %-3s (%s)" "$1" "$2" "$3"` + STR=`printf "%20s: %s (%s)" "$1" "$2" "$3"` fi AC_MSG_NOTICE([$STR])
For the first 2:
Reviewed-by: Cole Robinson <crobinso@redhat.com>
I like the look of the colors and I agree it speeds up visually scanning the configure output. But I'm neutral on whether adding more m4 to the build system to facilitate it is worth it. So I'll abstain from giving ack or nack on those.
Fair enough. When we switch to meson we'll get colours for free.
I've not checked how meson handles it, but in Travis with this series applied, the raw logs are now full of escape codes: https://api.travis-ci.org/v3/job/584045926/log.txt checking for library containing forkpty... [33;1m-lutil[m checking whether strerror(0) succeeds... [32;1myes[m checking for strerror_r with POSIX signature... [31;1mno[m checking whether __xpg_strerror_r works... [32;1myes[m checking whether strerror_r is declared... [32;1myes[m checking for external symbol _system_configuration... [31;1mno[m checking for pthread_t... [32;1myes[m checking for pthread_spinlock_t... [32;1myes[m checking for PTHREAD_CREATE_DETACHED... [32;1myes[m checking for PTHREAD_MUTEX_RECURSIVE... [32;1myes[m which makes the raw log much less readable. This is also the case already for the automake 'make check' output in fact [0;32mPASS[m: test-fnmatch-h [0;32mPASS[m: test-fnmatch [0;32mPASS[m: test-fpurge [0;32mPASS[m: test-fputc [0;32mPASS[m: test-fread [0;32mPASS[m: test-freading [0;32mPASS[m: test-fseek2.sh On the flip side, the default log view honours the colors which is nice: https://travis-ci.org/berrange/libvirt/jobs/584045926 Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 9/12/19 12:33 PM, Daniel P. Berrangé wrote:
On Thu, Sep 12, 2019 at 12:18:10PM +0200, Michal Privoznik wrote:
On 9/12/19 12:30 AM, Cole Robinson wrote:
On 9/9/19 3:49 AM, Michal Privoznik wrote:
The times, when we had small CRTs are long gone. Now, in the era of wide screens we can be more generous when it comes to aligning the output of configure. The longest string before the colon is 'wireshark_dissector' which counts 19 characters. Therefore, align the strings at 20.
At the same time, drop the useless result alignment. It behaves oddly - it puts a space at the end of each "no" because of the %-3s format we use.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- m4/virt-result.m4 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/m4/virt-result.m4 b/m4/virt-result.m4 index cc622fe35b..36973ba0b5 100644 --- a/m4/virt-result.m4 +++ b/m4/virt-result.m4 @@ -33,9 +33,9 @@ dnl LIBVIRT_RESULT([yajl], [yes], [-I/opt/yajl/include -lyajl]) dnl AC_DEFUN([LIBVIRT_RESULT], [ if test "$2" = "no" || test -z "$3" ; then - STR=`printf "%10s: %-3s" "$1" "$2"` + STR=`printf "%20s: %s" "$1" "$2"` else - STR=`printf "%10s: %-3s (%s)" "$1" "$2" "$3"` + STR=`printf "%20s: %s (%s)" "$1" "$2" "$3"` fi AC_MSG_NOTICE([$STR])
For the first 2:
Reviewed-by: Cole Robinson <crobinso@redhat.com>
I like the look of the colors and I agree it speeds up visually scanning the configure output. But I'm neutral on whether adding more m4 to the build system to facilitate it is worth it. So I'll abstain from giving ack or nack on those.
Fair enough. When we switch to meson we'll get colours for free.
I've not checked how meson handles it, but in Travis with this series applied, the raw logs are now full of escape codes:
https://api.travis-ci.org/v3/job/584045926/log.txt
checking for library containing forkpty... [33;1m-lutil[m checking whether strerror(0) succeeds... [32;1myes[m checking for strerror_r with POSIX signature... [31;1mno[m checking whether __xpg_strerror_r works... [32;1myes[m checking whether strerror_r is declared... [32;1myes[m checking for external symbol _system_configuration... [31;1mno[m checking for pthread_t... [32;1myes[m checking for pthread_spinlock_t... [32;1myes[m checking for PTHREAD_CREATE_DETACHED... [32;1myes[m checking for PTHREAD_MUTEX_RECURSIVE... [32;1myes[m
which makes the raw log much less readable.
This is also the case already for the automake 'make check' output in fact > [0;32mPASS[m: test-fnmatch-h [0;32mPASS[m: test-fnmatch [0;32mPASS[m: test-fpurge [0;32mPASS[m: test-fputc [0;32mPASS[m: test-fread [0;32mPASS[m: test-freading [0;32mPASS[m: test-fseek2.sh
On the flip side, the default log view honours the colors which is nice:
This is a travis bug. I've noticed this when doing my own builds and the problem is that somehow, travis reads TTY output instead of doing something like ./configure | tee. Travis runs ./configure from a TTY and thus my code (and obviously 'make check' too) sees FD 1 opened and being a TTY (`test -t 1') and thus colours are enabled. Fortunately, the default log view is not affected. Michal
On Thu, Sep 12, 2019 at 12:18:10PM +0200, Michal Privoznik wrote:
On 9/12/19 12:30 AM, Cole Robinson wrote:
On 9/9/19 3:49 AM, Michal Privoznik wrote:
The times, when we had small CRTs are long gone. Now, in the era of wide screens we can be more generous when it comes to aligning the output of configure. The longest string before the colon is 'wireshark_dissector' which counts 19 characters. Therefore, align the strings at 20.
At the same time, drop the useless result alignment. It behaves oddly - it puts a space at the end of each "no" because of the %-3s format we use.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- m4/virt-result.m4 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/m4/virt-result.m4 b/m4/virt-result.m4 index cc622fe35b..36973ba0b5 100644 --- a/m4/virt-result.m4 +++ b/m4/virt-result.m4 @@ -33,9 +33,9 @@ dnl LIBVIRT_RESULT([yajl], [yes], [-I/opt/yajl/include -lyajl]) dnl AC_DEFUN([LIBVIRT_RESULT], [ if test "$2" = "no" || test -z "$3" ; then - STR=`printf "%10s: %-3s" "$1" "$2"` + STR=`printf "%20s: %s" "$1" "$2"` else - STR=`printf "%10s: %-3s (%s)" "$1" "$2" "$3"` + STR=`printf "%20s: %s (%s)" "$1" "$2" "$3"` fi AC_MSG_NOTICE([$STR])
For the first 2:
Reviewed-by: Cole Robinson <crobinso@redhat.com>
I like the look of the colors and I agree it speeds up visually scanning the configure output. But I'm neutral on whether adding more m4 to the build system to facilitate it is worth it. So I'll abstain from giving ack or nack on those.
Fair enough. When we switch to meson we'll get colours for free.
FWIW, I've no objection to people continuing to make autotools usage better. In it unreasonable to block patches people send, until the meson patches are actually complete & published for review. I merely caution that any investment has a time limited window for payback, but since you've already done the work that's not a problem. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Sep 09, 2019 at 09:49:40AM +0200, Michal Privoznik wrote:
The times, when we had small CRTs are long gone. Now, in the era of wide screens we can be more generous when it comes to aligning the output of configure. The longest string before the colon is 'wireshark_dissector' which counts 19 characters. Therefore, align the strings at 20.
At the same time, drop the useless result alignment. It behaves oddly - it puts a space at the end of each "no" because of the %-3s format we use.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- m4/virt-result.m4 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
If we're running from a TTY we can put some colors around 'yes', 'no' and other messages. Shamelessly copied from Ruby source code and modified a bit to comply with syntax-check. https://github.com/ruby/ruby/commit/e4879592873abd4cd8aeed56f4cbaa360a3d3736 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- m4/virt-colours.m4 | 62 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 m4/virt-colours.m4 diff --git a/m4/virt-colours.m4 b/m4/virt-colours.m4 new file mode 100644 index 0000000000..251778f2ff --- /dev/null +++ b/m4/virt-colours.m4 @@ -0,0 +1,62 @@ +dnl Colourful configure +dnl +dnl Copyright (C) 2015 Nobuyoshi Nakada +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>. + +AC_DEFUN([_COLORIZE_RESULT_PREPARE], [ + msg_checking= msg_result_yes= msg_result_no= msg_result_other= msg_reset= + AS_IF([test -t 1], [ + msg_begin="`tput smso 2>/dev/null`" + AS_CASE(["$msg_begin"], ['@<:@'*m], + [msg_begin="`echo "$msg_begin" | sed ['s/[0-9]*m$//']`" + msg_checking="${msg_begin}33m" + AS_IF([test ${TEST_COLORS:+set}], [ + msg_result_yes=[`expr ":$TEST_COLORS:" : ".*:pass=\([^:]*\):"`] + msg_result_no=[`expr ":$TEST_COLORS:" : ".*:fail=\([^:]*\):"`] + msg_result_other=[`expr ":$TEST_COLORS:" : ".*:skip=\([^:]*\):"`] + ]) + msg_result_yes="${msg_begin}${msg_result_yes:-32;1}m" + msg_result_no="${msg_begin}${msg_result_no:-31;1}m" + msg_result_other="${msg_begin}${msg_result_other:-33;1}m" + msg_reset="${msg_begin}m" + ]) + AS_UNSET(msg_begin) + ]) + AS_REQUIRE_SHELL_FN([colorize_result], + [AS_FUNCTION_DESCRIBE([colorize_result], [MSG], [Colorize result])], + [AS_CASE(["$[]1"], + [yes], [AS_ECHO(["${msg_result_yes}$[]1${msg_reset}]")], + [no], [AS_ECHO(["${msg_result_no}$[]1${msg_reset}]")], + [AS_ECHO(["${msg_result_other}$[]1${msg_reset}]")])]) +]) + +AC_DEFUN([COLORIZE_RESULT], [AC_REQUIRE([_COLORIZE_RESULT_PREPARE])dnl + AS_LITERAL_IF([$1], + [m4_case([$1], + [yes], [AS_ECHO(["${msg_result_yes}$1${msg_reset}"])], + [no], [AS_ECHO(["${msg_result_no}$1${msg_reset}"])], + [AS_ECHO(["${msg_result_other}$1${msg_reset}"])])], + [colorize_result "$1"]) dnl +]) + +AC_DEFUN([AC_CHECKING],[dnl +AC_REQUIRE([_COLORIZE_RESULT_PREPARE])dnl +AS_MESSAGE([checking ${msg_checking}$1${msg_reset}...])]) + +AC_DEFUN([AC_MSG_RESULT], [dnl +{ _AS_ECHO_LOG([result: $1]) +COLORIZE_RESULT([$1]); dnl +}]) -- 2.21.0
On Mon, Sep 09, 2019 at 09:49:41AM +0200, Michal Privoznik wrote:
If we're running from a TTY we can put some colors around 'yes', 'no' and other messages.
Shamelessly copied from Ruby source code and modified a bit to comply with syntax-check.
https://github.com/ruby/ruby/commit/e4879592873abd4cd8aeed56f4cbaa360a3d3736
Ruby is under the Ruby license terms, which gives a choice of using the 2-clause BSD license instead, which in turn lets you relicense to the GPL, so ok.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- m4/virt-colours.m4 | 62 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 m4/virt-colours.m4
diff --git a/m4/virt-colours.m4 b/m4/virt-colours.m4 new file mode 100644 index 0000000000..251778f2ff --- /dev/null +++ b/m4/virt-colours.m4 @@ -0,0 +1,62 @@ +dnl Colourful configure +dnl +dnl Copyright (C) 2015 Nobuyoshi Nakada +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>. + +AC_DEFUN([_COLORIZE_RESULT_PREPARE], [ + msg_checking= msg_result_yes= msg_result_no= msg_result_other= msg_reset= + AS_IF([test -t 1], [ + msg_begin="`tput smso 2>/dev/null`" + AS_CASE(["$msg_begin"], ['@<:@'*m], + [msg_begin="`echo "$msg_begin" | sed ['s/[0-9]*m$//']`" + msg_checking="${msg_begin}33m" + AS_IF([test ${TEST_COLORS:+set}], [ + msg_result_yes=[`expr ":$TEST_COLORS:" : ".*:pass=\([^:]*\):"`] + msg_result_no=[`expr ":$TEST_COLORS:" : ".*:fail=\([^:]*\):"`] + msg_result_other=[`expr ":$TEST_COLORS:" : ".*:skip=\([^:]*\):"`] + ]) + msg_result_yes="${msg_begin}${msg_result_yes:-32;1}m" + msg_result_no="${msg_begin}${msg_result_no:-31;1}m" + msg_result_other="${msg_begin}${msg_result_other:-33;1}m" + msg_reset="${msg_begin}m" + ]) + AS_UNSET(msg_begin) + ]) + AS_REQUIRE_SHELL_FN([colorize_result], + [AS_FUNCTION_DESCRIBE([colorize_result], [MSG], [Colorize result])], + [AS_CASE(["$[]1"], + [yes], [AS_ECHO(["${msg_result_yes}$[]1${msg_reset}]")], + [no], [AS_ECHO(["${msg_result_no}$[]1${msg_reset}]")], + [AS_ECHO(["${msg_result_other}$[]1${msg_reset}]")])]) +]) + +AC_DEFUN([COLORIZE_RESULT], [AC_REQUIRE([_COLORIZE_RESULT_PREPARE])dnl + AS_LITERAL_IF([$1], + [m4_case([$1], + [yes], [AS_ECHO(["${msg_result_yes}$1${msg_reset}"])], + [no], [AS_ECHO(["${msg_result_no}$1${msg_reset}"])], + [AS_ECHO(["${msg_result_other}$1${msg_reset}"])])], + [colorize_result "$1"]) dnl +]) + +AC_DEFUN([AC_CHECKING],[dnl +AC_REQUIRE([_COLORIZE_RESULT_PREPARE])dnl +AS_MESSAGE([checking ${msg_checking}$1${msg_reset}...])]) + +AC_DEFUN([AC_MSG_RESULT], [dnl +{ _AS_ECHO_LOG([result: $1]) +COLORIZE_RESULT([$1]); dnl +}])
I can't claim I attempted to understand that. I'm just assuming that since it works for Ruby, its bug-free :-) Clever how they redefine existing autoconf macros AC_CHECKING and AC_MSG_RESULT to make them colourful. Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
The LIBVIRT_RESULT function takes two or three arguments. The first one is the name of the result (aka CHECK_NAME). It is printed before the colon character. The rest of the arguments is printed after the character. To produce colourized output a couple of changes needs to be made. Firstly, we need to print the CHECK_NAME using "echo -n" so that the new line is not appended at the end of the message. To achieve this, AS_MESSAGE_N function is introduced. It's a verbatim copy of AS_MESSAGE (which is just another alias to AC_MSG_NOTICE) except it doesn't put '\n' at the EOL. The alias is defined at /usr/share/autoconf-*/autoconf/general.m4 and the AS_MESSAGE is then defined at /usr/share/autoconf-2.69/m4sugar/m4sh.m4. Secondly, the rest of the arguments are printed colourized and to achieve that and also keep printing them into the log file the _AS_ECHO and COLORIZE_RESULT functions need to be called. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- m4/virt-result.m4 | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/m4/virt-result.m4 b/m4/virt-result.m4 index 36973ba0b5..9115be5774 100644 --- a/m4/virt-result.m4 +++ b/m4/virt-result.m4 @@ -31,12 +31,27 @@ dnl eg dnl dnl LIBVIRT_RESULT([yajl], [yes], [-I/opt/yajl/include -lyajl]) dnl + +m4_defun_init([_AS_ECHO_LOG_N], +[AS_REQUIRE([_AS_LINENO_PREPARE])], +[_AS_ECHO_N([$as_me:${as_lineno-$LINENO}: $1], AS_MESSAGE_LOG_FD)]) + +m4_defun_init([AS_MESSAGE_N], +[AS_REQUIRE([_AS_ME_PREPARE])], +[m4_ifval(AS_MESSAGE_LOG_FD, + [{ _AS_ECHO_LOG_N([$1]) +_AS_ECHO_N([$as_me: $1], [$2]);}], + [_AS_ECHO_N([$as_me: $1], [$2])])[]]) + AC_DEFUN([LIBVIRT_RESULT], [ + STR=`printf "%20s: " "$1"` if test "$2" = "no" || test -z "$3" ; then - STR=`printf "%20s: %s" "$1" "$2"` + VAL=`printf "%s" "$2"` else - STR=`printf "%20s: %s (%s)" "$1" "$2" "$3"` + VAL=`printf "%s (%s)" "$2" "$3"` fi - AC_MSG_NOTICE([$STR]) + AS_MESSAGE_N([$STR]) + _AS_ECHO([$VAL], AS_MESSAGE_LOG_FD) + COLORIZE_RESULT([$VAL]) ]) -- 2.21.0
On Mon, Sep 09, 2019 at 09:49:42AM +0200, Michal Privoznik wrote:
The LIBVIRT_RESULT function takes two or three arguments. The first one is the name of the result (aka CHECK_NAME). It is printed before the colon character. The rest of the arguments is printed after the character. To produce colourized output a couple of changes needs to be made.
Firstly, we need to print the CHECK_NAME using "echo -n" so that the new line is not appended at the end of the message. To achieve this, AS_MESSAGE_N function is introduced. It's a verbatim copy of AS_MESSAGE (which is just another alias to AC_MSG_NOTICE) except it doesn't put '\n' at the EOL.
The alias is defined at /usr/share/autoconf-*/autoconf/general.m4 and the AS_MESSAGE is then defined at /usr/share/autoconf-2.69/m4sugar/m4sh.m4.
Secondly, the rest of the arguments are printed colourized and to achieve that and also keep printing them into the log file the _AS_ECHO and COLORIZE_RESULT functions need to be called.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- m4/virt-result.m4 | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Mon, Sep 09, 2019 at 09:49:38AM +0200, Michal Privoznik wrote:
An example how configure looks like with these changes applied:
https://travis-ci.org/zippy2/libvirt/jobs/582549099#L4247
Michal Prívozník (4): configure: Prefer LIBVIRT_RESULT over AC_MSG_NOTICE virt-result.m4: Align string more generously configure: Colorize output
Why on earth would anyone want that? Jano
virt-result.m4: Colourize summary printings
configure.ac | 8 ++--- m4/virt-chrdev-lock-files.m4 | 2 +- m4/virt-colours.m4 | 62 ++++++++++++++++++++++++++++++++++++ m4/virt-debug.m4 | 2 +- m4/virt-default-editor.m4 | 2 +- m4/virt-dtrace.m4 | 2 +- m4/virt-host-validate.m4 | 2 +- m4/virt-init-script.m4 | 2 +- m4/virt-loader-nvram.m4 | 2 +- m4/virt-login-shell.m4 | 2 +- m4/virt-numad.m4 | 2 +- m4/virt-result.m4 | 21 ++++++++++-- 12 files changed, 93 insertions(+), 16 deletions(-) create mode 100644 m4/virt-colours.m4
-- 2.21.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 9/9/19 10:52 AM, Ján Tomko wrote:
On Mon, Sep 09, 2019 at 09:49:38AM +0200, Michal Privoznik wrote:
An example how configure looks like with these changes applied:
https://travis-ci.org/zippy2/libvirt/jobs/582549099#L4247
Michal Prívozník (4): configure: Prefer LIBVIRT_RESULT over AC_MSG_NOTICE virt-result.m4: Align string more generously configure: Colorize output
Why on earth would anyone want that?
I find it easier to see at the first glance what features are enabled and what are disabled. Also, the first two patches are independent of the feature, so even if we don't want to merge the rest, the first two still derserve review IMO. Michal
On 9/9/19 4:52 AM, Ján Tomko wrote:
On Mon, Sep 09, 2019 at 09:49:38AM +0200, Michal Privoznik wrote:
An example how configure looks like with these changes applied:
https://travis-ci.org/zippy2/libvirt/jobs/582549099#L4247
Michal Prívozník (4): configure: Prefer LIBVIRT_RESULT over AC_MSG_NOTICE virt-result.m4: Align string more generously configure: Colorize output
Why on earth would anyone want that?
If you have an objection, please just state it plainly. IMO if an outsider looking in on libvir-list saw your comment they may conclude this is an unwelcoming place - Cole
On Wed, Sep 11, 2019 at 06:26:15PM -0400, Cole Robinson wrote:
On 9/9/19 4:52 AM, Ján Tomko wrote:
On Mon, Sep 09, 2019 at 09:49:38AM +0200, Michal Privoznik wrote:
An example how configure looks like with these changes applied:
https://travis-ci.org/zippy2/libvirt/jobs/582549099#L4247
Michal Prívozník (4): configure: Prefer LIBVIRT_RESULT over AC_MSG_NOTICE virt-result.m4: Align string more generously configure: Colorize output
Why on earth would anyone want that?
If you have an objection, please just state it plainly. IMO if an outsider looking in on libvir-list saw your comment they may conclude this is an unwelcoming place
My objection was aesthetical, sorry for not stating it as factually and eloquently as Peter. I'll try to spend more time on the messages in the future. (and preferably after lunch) Jano
participants (4)
-
Cole Robinson -
Daniel P. Berrangé -
Ján Tomko -
Michal Privoznik