Re: [libvirt] [PATCH 2/4] virt-result.m4: Align string more generously
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 > > > --- > > > 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 > > > > 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] virt-result.m4: Align string more generously
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 > --- > m4/virt-result.m4 | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Daniel P. Berrangé 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] virt-result.m4: Align string more generously
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 --- 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 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 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] virt-result.m4: Align string more generously
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 > > > --- > > > 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 > > > > 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] virt-result.m4: Align string more generously
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 --- 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 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] virt-result.m4: Align string more generously
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 > --- > 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 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] virt-result.m4: Align string more generously
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 --- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
