Libguestfs - Feb 2022 - [guestfs-tools PATCH] virt-resize: replace "wrap" calls with calls to "info"

Utilities shouldn't directly call "Std_utils.wrap" for printing
informative messages; the "Tools_utils.info" function handles that
better.

Because "info" prints a trailing newline automatically, strip one
newline
from the originally wrapped messages. While at it, rewrap (in the source
code) the "Resize operation completed with no errors" message, for
better
readability.

The patch introduces some visible (but, arguably, correct) changes to the
output:
> virt-resize: /dev/sda1: This partition will be resized from 1023.9M to
> 2.0G.  The filesystem ext4 on /dev/sda1 will be expanded using the
> ?resize2fs? method.
> [...]
> virt-resize: Resize operation completed with no errors.  Before deleting
> the old disk, carefully check that the resized disk boots and works
> correctly.
These messages now carry the "virt-resize: " prefix, and they are
printed
in magenta.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1820221
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 resize/resize.ml | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/resize/resize.ml b/resize/resize.ml
index dad453ff99b7..b77e680d49c8 100644
--- a/resize/resize.ml
+++ b/resize/resize.ml
@@ -938,7 +938,7 @@ read the man page virt-resize(1).
               ""
             ) in
 
-      wrap (text ^ "\n\n") in
+      info "%s" (text ^ "\n") in
 
     List.iter print_summary partitions;
 
@@ -969,7 +969,7 @@ read the man page virt-resize(1).
                 ""
               ) in
 
-            wrap (text ^ "\n\n")
+            info "%s" (text ^ "\n")
     ) lvs;
 
     if surplus > 0L then (
@@ -983,7 +983,7 @@ read the man page virt-resize(1).
         ) else
           s_"  The surplus space will be ignored.  Run a partitioning
program in the guest to partition this extra space if you want." in
 
-      wrap (text ^ "\n\n")
+      info "%s" (text ^ "\n")
     );
 
     printf "**********\n";
@@ -1440,7 +1440,9 @@ read the man page virt-resize(1).
 
   if not (quiet ()) then (
     print_newline ();
-    wrap (s_"Resize operation completed with no errors.  Before deleting
the old disk, carefully check that the resized disk boots and works
correctly.\n");
+    info "%s" (s_"Resize operation completed with no errors. 
Before deleting \
+               the old disk, carefully check that the resized disk boots and \
+               works correctly.");
   )
 
 let () = run_main_and_handle_errors main

base-commit: 7d5d5e921d3d483a997f40566c1ccabf8a689a8a
-- 
2.19.1.3.g30247aa5d201

On Wed, Feb 09, 2022 at 02:17:38PM +0100, Laszlo Ersek
wrote:
> Utilities shouldn't directly call "Std_utils.wrap" for
printing
> informative messages; the "Tools_utils.info" function handles
that better.
> 
> Because "info" prints a trailing newline automatically, strip one
newline
> from the originally wrapped messages. While at it, rewrap (in the source
> code) the "Resize operation completed with no errors" message,
for better
> readability.
> 
> The patch introduces some visible (but, arguably, correct) changes to the
> output:
> 
> > virt-resize: /dev/sda1: This partition will be resized from 1023.9M to
> > 2.0G.  The filesystem ext4 on /dev/sda1 will be expanded using the
> > ?resize2fs? method.
> > [...]
> > virt-resize: Resize operation completed with no errors.  Before
deleting
> > the old disk, carefully check that the resized disk boots and works
> > correctly.
> 
> These messages now carry the "virt-resize: " prefix, and they are
printed
> in magenta.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1820221
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
>  resize/resize.ml | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/resize/resize.ml b/resize/resize.ml
> index dad453ff99b7..b77e680d49c8 100644
> --- a/resize/resize.ml
> +++ b/resize/resize.ml
> @@ -938,7 +938,7 @@ read the man page virt-resize(1).
>                ""
>              ) in
>  
> -      wrap (text ^ "\n\n") in
> +      info "%s" (text ^ "\n") in
>  
>      List.iter print_summary partitions;
>  
> @@ -969,7 +969,7 @@ read the man page virt-resize(1).
>                  ""
>                ) in
>  
> -            wrap (text ^ "\n\n")
> +            info "%s" (text ^ "\n")
>      ) lvs;
>  
>      if surplus > 0L then (
> @@ -983,7 +983,7 @@ read the man page virt-resize(1).
>          ) else
>            s_"  The surplus space will be ignored.  Run a partitioning
program in the guest to partition this extra space if you want." in
>  
> -      wrap (text ^ "\n\n")
> +      info "%s" (text ^ "\n")
>      );
>  
>      printf "**********\n";
> @@ -1440,7 +1440,9 @@ read the man page virt-resize(1).
>  
>    if not (quiet ()) then (
>      print_newline ();
> -    wrap (s_"Resize operation completed with no errors.  Before
deleting the old disk, carefully check that the resized disk boots and works
correctly.\n");
> +    info "%s" (s_"Resize operation completed with no
errors.  Before deleting \
> +               the old disk, carefully check that the resized disk boots
and \
> +               works correctly.");
>    )
>  
>  let () = run_main_and_handle_errors main
> 
> base-commit: 7d5d5e921d3d483a997f40566c1ccabf8a689a8a
> -- 
> 2.19.1.3.g30247aa5d201
Yes this is fine, ACK.

Should we also hide the wrap function?  From a very quick look at the
code it seems we could move the function from mlstdutils/std_utils.ml
to mltools/tools_utils.ml, and of course drop the public interface in
mlstdutils/std_utils.mli.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW