[parted-devel] [PATCH] Skip all the interaction with the use if we are in script mode.

[Jim Meyering]

Joel Andres Granados <jgranado at redhat.com> wrote:
> ---
>  parted/parted.c |   86 ++++++++++++++++++++++++++++++++++---------------------
>  1 files changed, 53 insertions(+), 33 deletions(-)
>
> diff --git a/parted/parted.c b/parted/parted.c
> index 9f79ea4..81b7805 100644
> --- a/parted/parted.c
> +++ b/parted/parted.c
> @@ -776,23 +776,33 @@ do_mkpart (PedDevice** dev)
>                          start_sol = ped_unit_format (*dev, part->geom.start);
>                          end_sol   = ped_unit_format (*dev, part->geom.end);
>  
> -                        switch (ped_exception_throw (
> -                                PED_EXCEPTION_WARNING,
> -                                PED_EXCEPTION_YES_NO,
> -                                _("You requested a partition from %s to %s.\n"
> -                                  "The closest location we can manage is "
> -                                  "%s to %s.  "
> -                                  "Is this still acceptable to you?"),
> -                                start_usr, end_usr, start_sol, end_sol))
> -                        {
> -                                case PED_EXCEPTION_YES:
> -                                        /* all is well in this state */
> -                                        break;
> -                                case PED_EXCEPTION_NO:
> -                                case PED_EXCEPTION_UNHANDLED:
> -                                default:
> -                                        /* undo partition addition */
> -                                        goto error_remove_part;

Hi Joel,
The existing behavior is definitely a bug.
Thanks for working on it.

> +                        if(!opt_script_mode){
> +                                switch (ped_exception_throw (
> +                                        PED_EXCEPTION_WARNING,
> +                                        PED_EXCEPTION_YES_NO,
> +                                        _("You requested a partition from %s to %s.\n"
> +                                          "The closest location we can manage is "
> +                                          "%s to %s.  "
> +                                          "Is this still acceptable to you?"),
> +                                        start_usr, end_usr, start_sol, end_sol))
> +                                {
> +                                        case PED_EXCEPTION_YES:
> +                                                /* all is well in this state */
> +                                                break;
> +                                        case PED_EXCEPTION_NO:
> +                                        case PED_EXCEPTION_UNHANDLED:
> +                                        default:
> +                                                /* undo partition addition */
> +                                                goto error_remove_part;
> +                                }
> +                        } else {
> +                                ped_exception_throw (
> +                                        PED_EXCEPTION_WARNING,
> +                                        PED_EXCEPTION_OK,
> +                                        _("You requested a partition from %s to %s.\n"
> +                                        "The closest location we can manage is "
> +                                        "%s to %s."),
> +                                        start_usr, end_usr, start_sol, end_sol);
...

Rather than duplicating the ped_exception_throw call,
how about this smaller patch:

diff --git a/parted/parted.c b/parted/parted.c
index 9f79ea4..b85837f 100644
--- a/parted/parted.c
+++ b/parted/parted.c
@@ -778,12 +778,15 @@ do_mkpart (PedDevice** dev)

                         switch (ped_exception_throw (
                                 PED_EXCEPTION_WARNING,
-                                PED_EXCEPTION_YES_NO,
+                                (opt_script_mode
+                                 ? PED_EXCEPTION_CANCEL
+                                 : PED_EXCEPTION_YES_NO),
                                 _("You requested a partition from %s to %s.\n"
                                   "The closest location we can manage is "
-                                  "%s to %s.  "
-                                  "Is this still acceptable to you?"),
-                                start_usr, end_usr, start_sol, end_sol))
+                                  "%s to %s.%s"),
+                                start_usr, end_usr, start_sol, end_sol,
+                                (opt_script_mode ? ""
+                                 : _("  Is this still acceptable to you?"))))
                         {
                                 case PED_EXCEPTION_YES:
                                         /* all is well in this state */

When in script mode, failure to use the specified values
deserves a fatal error, so I used "PED_EXCEPTION_CANCEL" above.

When fixing a bug like this, it's better for everyone
if you can also write a test case to exercise each block
and significant branch of the affected code.

So for the above, you'd want to have two tests.
One run in script mode and the other run in interactive mode.
You already gave an example demonstrating the first case,
and I've just run it from the command line like this:

  $ dd if=/dev/zero of=testfile bs=512 count=100
  $ ./parted -s  testfile "mklabel gpt mkpart primary ext3 1s -1s"
  Warning: You requested a partition from 512B to 50.7kB.
  The closest location we can manage is 17.4kB to 33.8kB.
  [Exit 1]

To test the interactive case from a script, it helps to know about the
(deliberately) undocumented ---pretend-input-tty option.
For now, I've done it on the command line:

  $ dd if=/dev/zero of=testfile bs=512 count=100
  $ echo n|./parted ---pretend-input-tty testfile "mklabel gpt mkpart primary ext3 1s -1s"                                                                
  WARNING: You are not superuser.  Watch out for permissions.                      Warning: You requested a partition from 512B to 50.7kB.                   
  The closest location we can manage is 17.4kB to 33.8kB.  Is this still acceptable to you?
  Yes/No? [Exit 1]                                                          

Can you adjust the other hunks of your patch, and run
command-line tests like the above?  Bonus points if you
create/add a new test script to automate the running of
these new tests.  Otherwise, I'll do it, but that may take longer.

Finally, once you're happy with the code, please follow
the patch-preparation guidelines I wrote for coreutils:

  coreutils contribution/style guidelines
    http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=blob;f=HACKING;hb=HEAD

The idea is that once the patch passes review on the list,
I can apply it to my local repository with "git am YOUR_PATCH",
then push to the public repo, where it will appear with your
name listed as the Author: and your commit log-comments in
the permanent git history.