Re: [PATCH v2 04/25] sequencer: future-proof remove_sequencer_state()

[Junio C Hamano]

Johannes Schindelin  writes:

> I briefly considered consolidating them and using .git/rebase-merge/ as
> state directory also for cherry-pick/revert, but that would cause
> problems: I am surely not the only user who cherry-picks commits manually
> while running interactive rebases.

Good thinking.

Re: [PATCH v2 04/25] sequencer: future-proof remove_sequencer_state()

[Johannes Schindelin]

Hi Junio,

On Mon, 12 Sep 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > +static const char *get_dir(const struct replay_opts *opts)
> > +{
> > +   return git_path_seq_dir();
> > +}
> 
> Presumably this is what "In a couple of commits" meant, i.e. opts
> will be used soonish.

Exactly. The sequencer code was taught to use a state directory different
from rebase -i's (even if it quite clearly imitated rebase -i's approach),
to allow for rebase -i to run at the same time as the sequencer (by
calling it via cherry-pick).

So we need to be able to use different seq_dir locations, depending on the
mode we are running in.

I briefly considered consolidating them and using .git/rebase-merge/ as
state directory also for cherry-pick/revert, but that would cause
problems: I am surely not the only user who cherry-picks commits manually
while running interactive rebases.

Ciao,
Dscho

Re: [PATCH v2 04/25] sequencer: future-proof remove_sequencer_state()

[Junio C Hamano]

Johannes Schindelin  writes:

> +static const char *get_dir(const struct replay_opts *opts)
> +{
> + return git_path_seq_dir();
> +}

Presumably this is what "In a couple of commits" meant, i.e. opts
will be used soonish.

> -static void remove_sequencer_state(void)
> +static void remove_sequencer_state(const struct replay_opts *opts)
>  {
> - struct strbuf seq_dir = STRBUF_INIT;
> + struct strbuf dir = STRBUF_INIT;
>  
> - strbuf_addstr(_dir, git_path_seq_dir());
> - remove_dir_recursively(_dir, 0);
> - strbuf_release(_dir);
> + strbuf_addf(, "%s", get_dir(opts));
> + remove_dir_recursively(, 0);
> + strbuf_release();
>  }

As long as "who called the sequencer" is the only thing that
determines where the state is kept, this should work fine, I would
think.  I wondered that it would introduce a chicken-and-egg problem
if we had to support "git sequencer --clear-state" command ;-) but
that is not the case.

Good.

[PATCH v2 04/25] sequencer: future-proof remove_sequencer_state()

[Johannes Schindelin]

In a couple of commits, we will teach the sequencer to handle the
nitty gritty of the interactive rebase, which keeps its state in a
different directory.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index c2fbf6f..8d272fb 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -27,6 +27,11 @@ static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
 static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
 static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
 
+static const char *get_dir(const struct replay_opts *opts)
+{
+   return git_path_seq_dir();
+}
+
 static int is_rfc2822_line(const char *buf, int len)
 {
int i;
@@ -109,13 +114,13 @@ static int has_conforming_footer(struct strbuf *sb, 
struct strbuf *sob,
return 1;
 }
 
-static void remove_sequencer_state(void)
+static void remove_sequencer_state(const struct replay_opts *opts)
 {
-   struct strbuf seq_dir = STRBUF_INIT;
+   struct strbuf dir = STRBUF_INIT;
 
-   strbuf_addstr(_dir, git_path_seq_dir());
-   remove_dir_recursively(_dir, 0);
-   strbuf_release(_dir);
+   strbuf_addf(, "%s", get_dir(opts));
+   remove_dir_recursively(, 0);
+   strbuf_release();
 }
 
 static const char *action_name(const struct replay_opts *opts)
@@ -940,7 +945,7 @@ static int sequencer_rollback(struct replay_opts *opts)
}
if (reset_for_rollback(sha1))
goto fail;
-   remove_sequencer_state();
+   remove_sequencer_state(opts);
strbuf_release();
return 0;
 fail:
@@ -1034,7 +1039,7 @@ static int pick_commits(struct commit_list *todo_list, 
struct replay_opts *opts)
 * Sequence of picks finished successfully; cleanup by
 * removing the .git/sequencer directory
 */
-   remove_sequencer_state();
+   remove_sequencer_state(opts);
return 0;
 }
 
@@ -1095,7 +1100,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 * one that is being continued
 */
if (opts->subcommand == REPLAY_REMOVE_STATE) {
-   remove_sequencer_state();
+   remove_sequencer_state(opts);
return 0;
}
if (opts->subcommand == REPLAY_ROLLBACK)
-- 
2.10.0.windows.1.10.g803177d