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

2018-08-24 Thread Johannes Schindelin
Hi Stefan,

On Wed, 8 Aug 2018, Stefan Beller wrote:

> On Wed, Aug 8, 2018 at 6:51 AM Pratik Karki  wrote:
> >
> > diff --git a/builtin/rebase.c b/builtin/rebase.c
> > index 8a7bf3d468..a261f552f1 100644
> > --- a/builtin/rebase.c
> > +++ b/builtin/rebase.c
> > @@ -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. ;-)

Yes, I insisted on it.

TBH it is a bit of a shame that we cannot fix all those error messages
going to stdout, but... you know... One step after the other.

> 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.

It should probably be inside the conditional block, but as you say: the
original had it in a funny spot, and so does the converted code.

> > +   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.

I will fix this before sending the next iteration.

Thanks for the review!
Dscho


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

2018-08-08 Thread Stefan Beller
On Wed, Aug 8, 2018 at 6:51 AM Pratik Karki  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 
> ---
>  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", _name,
> @@ -447,6 +449,30 @@ int cmd_rebase(int argc, const char **argv, const char 
> *prefix)
>
> git_config(rebase_config, );
>
> +   if (is_directory(apply_dir())) {
> +   options.type = REBASE_AM;
> +   options.state_dir = apply_dir();
> +   } else if (is_directory(merge_dir())) {
> +   strbuf_reset();
> +   strbuf_addf(, "%s/rewritten", merge_dir());
> +   if (is_directory(buf.buf)) {
> +   options.type = REBASE_PRESERVE_MERGES;
> +   options.flags |= REBASE_INTERACTIVE_EXPLICIT;
> +   } else {
> +   strbuf_reset();
> +   strbuf_addf(, "%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


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

2018-08-08 Thread Pratik Karki
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 
---
 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", _name,
@@ -447,6 +449,30 @@ int cmd_rebase(int argc, const char **argv, const char 
*prefix)
 
git_config(rebase_config, );
 
+   if (is_directory(apply_dir())) {
+   options.type = REBASE_AM;
+   options.state_dir = apply_dir();
+   } else if (is_directory(merge_dir())) {
+   strbuf_reset();
+   strbuf_addf(, "%s/rewritten", merge_dir());
+   if (is_directory(buf.buf)) {
+   options.type = REBASE_PRESERVE_MERGES;
+   options.flags |= REBASE_INTERACTIVE_EXPLICIT;
+   } else {
+   strbuf_reset();
+   strbuf_addf(, "%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 */
+   if (in_progress) {
+   const char *last_slash = strrchr(options.state_dir, '/');
+   const char *state_dir_base =
+   last_slash ? last_slash + 1 : options.state_dir;
+   const char *cmd_live_rebase =
+   "git rebase (--continue | --abort | --skip)";
+   strbuf_reset();
+   strbuf_addf(, "rm -fr \"%s\"", options.state_dir);
+   die(_("It seems that there is already a %s directory, and\n"
+ "I wonder if you are in the middle of another rebase.  "
+ "If that is the\n"
+ "case, please try\n\t%s\n"
+ "If that is not the case, please\n\t%s\n"
+ "and run me again.  I am stopping in case you still "
+ "have something\n"
+ "valuable there.\n"),
+   state_dir_base, cmd_live_rebase,buf.buf);
+   }
+
if (!(options.flags & REBASE_NO_QUIET))
strbuf_addstr(_am_opt, " -q");
 
-- 
2.18.0