Re: [PATCH 09/11] builtin rebase: start a new rebase only if none is in progress

[Stefan Beller <sbeller@google.com>]

On Wed, Aug 8, 2018 at 6:51 AM Pratik Karki <predatoramigo@gmail.com> wrote:
>
> To run a new rebase, there needs to be a check to assure that no other
> rebase is in progress. New rebase operation cannot start until an
> ongoing rebase operation completes or is terminated.
>
> Signed-off-by: Pratik Karki <predatoramigo@gmail.com>
> ---
>  builtin/rebase.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 8a7bf3d468..a261f552f1 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -87,6 +87,7 @@ struct rebase_options {
>                 REBASE_VERBOSE = 1<<1,
>                 REBASE_DIFFSTAT = 1<<2,
>                 REBASE_FORCE = 1<<3,
> +               REBASE_INTERACTIVE_EXPLICIT = 1<<4,
>         } flags;
>         struct strbuf git_am_opt;
>  };
> @@ -392,10 +393,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>                 .git_am_opt = STRBUF_INIT,
>         };
>         const char *branch_name;
> -       int ret, flags;
> +       int ret, flags, in_progress = 0;
>         int ok_to_skip_pre_rebase = 0;
>         struct strbuf msg = STRBUF_INIT;
>         struct strbuf revisions = STRBUF_INIT;
> +       struct strbuf buf = STRBUF_INIT;
>         struct object_id merge_base;
>         struct option builtin_rebase_options[] = {
>                 OPT_STRING(0, "onto", &options.onto_name,
> @@ -447,6 +449,30 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>
>         git_config(rebase_config, &options);
>
> +       if (is_directory(apply_dir())) {
> +               options.type = REBASE_AM;
> +               options.state_dir = apply_dir();
> +       } else if (is_directory(merge_dir())) {
> +               strbuf_reset(&buf);
> +               strbuf_addf(&buf, "%s/rewritten", merge_dir());
> +               if (is_directory(buf.buf)) {
> +                       options.type = REBASE_PRESERVE_MERGES;
> +                       options.flags |= REBASE_INTERACTIVE_EXPLICIT;
> +               } else {
> +                       strbuf_reset(&buf);
> +                       strbuf_addf(&buf, "%s/interactive", merge_dir());
> +                       if(file_exists(buf.buf)) {
> +                               options.type = REBASE_INTERACTIVE;
> +                               options.flags |= REBASE_INTERACTIVE_EXPLICIT;
> +                       } else
> +                               options.type = REBASE_MERGE;
> +               }
> +               options.state_dir = merge_dir();
> +       }
> +
> +       if (options.type != REBASE_UNSPECIFIED)
> +               in_progress = 1;
> +
>         argc = parse_options(argc, argv, prefix,
>                              builtin_rebase_options,
>                              builtin_rebase_usage, 0);
> @@ -455,6 +481,26 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
>                 usage_with_options(builtin_rebase_usage,
>                                    builtin_rebase_options);
>
> +       /* Make sure no rebase is in progress */

The faithful conversion doesn't even stop at the comments. ;-)
I shortly wondered if this is the best place for this comment,
but let's just keep it here to have the 1:1 rewrite.


> +       if (in_progress) {
[...]
> +                   state_dir_base, cmd_live_rebase,buf.buf);

In case a resend is needed, add a whitespace after the
comma and buf.buf, please.

So far I have not seen anything major that would warrant a resend.

Thanks,
Stefan