Teach the builtin rebase about the builtin interactive rebase

The builtin rebase and the builtin interactive rebase have been developed independently, on purpose: Google Summer of Code rules specifically state that students have to work on independent projects, they cannot collaborate on the same project.

The reason is probably the very fine tradition in academia to prohibit teamwork, which makes grading easier (at the expense of not exactly preparing the students for the real world, unless they want to stay in academia).

One fallout is that the rebase-in-c and rebase-i-in-c patches cause no merge conflicts but a royal number of tests in the test suite to fail.

It is easy to explain why: rebase-in-c was developed under the assumption that all rebase backends are implemented in Unix shell script and can be sourced via . git-rebase--<backend>, which is no longer true with rebase-i-in-c, where git-rebase--interactive is a hard-linked builtin.

This patch fixes that.

Note: while this patch targets pk/rebase-in-c-6-final, it will not work correctly without ag/rebase-i-in-c. So my suggestion is to rewrite the pk/rebas-in-c-6-final branch by first merging ag/rebase-i-in-c, then applying this here patch, and only then cherry-pick "rebase: default to using the builtin rebase".

Johannes Schindelin (1):
builtin rebase: prepare for builtin rebase -i

 builtin/rebase.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

base-commit: a5bb2345d2d414aba04e18531de1e0f041f0434a
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-23%2Fdscho%2Frebase-in-c-6-final-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-23/dscho/rebase-in-c-6-final-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/23

gitgitgadget

From: Johannes Schindelin <[email protected]>

It is no longer a shell script, so we need to call it in a different way than the other backends.

---
 builtin/rebase.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

index c8d632b6f4..87590047b3 100644
@@ -326,6 +326,13 @@ static void add_var(struct strbuf *buf, const char *name, const char *value)
 	}
 }
 
+static const char *resolvemsg =
+N_("Resolve all conflicts manually, mark them as resolved with\n"
+"\"git add/rm <conflicted_files>\", then run \"git rebase --continue\".\n"
+"You can instead skip this commit: run \"git rebase --skip\".\n"
+"To abort and get back to the state before \"git rebase\", run "
+"\"git rebase --abort\".");
+
 static int run_specific_rebase(struct rebase_options *opts)
 {
 	const char *argv[] = { NULL, NULL };

@@ -333,6 +340,79 @@ static int run_specific_rebase(struct rebase_options *opts)
 	int status;
 	const char *backend, *backend_func;
 
+	if (opts->type == REBASE_INTERACTIVE) {
+		/* Run builtin interactive rebase */
+		struct child_process child = CHILD_PROCESS_INIT;
+
+		argv_array_pushf(&child.env_array, "GIT_CHERRY_PICK_HELP=%s",
+				 resolvemsg);
+		if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) {
+			argv_array_push(&child.env_array, "GIT_EDITOR=:");
+			opts->autosquash = 0;
+		}
+
+		child.git_cmd = 1;
+		argv_array_push(&child.args, "rebase--interactive");
+
+		if (opts->action)
+			argv_array_pushf(&child.args, "--%s", opts->action);
+		if (opts->keep_empty)
+			argv_array_push(&child.args, "--keep-empty");
+		if (opts->rebase_merges)
+			argv_array_push(&child.args, "--rebase-merges");
+		if (opts->rebase_cousins)
+			argv_array_push(&child.args, "--rebase-cousins");
+		if (opts->autosquash)
+			argv_array_push(&child.args, "--autosquash");
+		if (opts->flags & REBASE_VERBOSE)
+			argv_array_push(&child.args, "--verbose");
+		if (opts->flags & REBASE_FORCE)
+			argv_array_push(&child.args, "--no-ff");
+		if (opts->restrict_revision)
+			argv_array_pushf(&child.args,
+					 "--restrict-revision=^%s",
+					 oid_to_hex(&opts->restrict_revision->object.oid));
+		if (opts->upstream)
+			argv_array_pushf(&child.args, "--upstream=%s",
+					 oid_to_hex(&opts->upstream->object.oid));
+		if (opts->onto)
+			argv_array_pushf(&child.args, "--onto=%s",
+					 oid_to_hex(&opts->onto->object.oid));
+		if (opts->squash_onto)
+			argv_array_pushf(&child.args, "--squash-onto=%s",
+					 oid_to_hex(opts->squash_onto));
+		if (opts->onto_name)
+			argv_array_pushf(&child.args, "--onto-name=%s",
+					 opts->onto_name);
+		argv_array_pushf(&child.args, "--head-name=%s",
+				 opts->head_name ?
+				 opts->head_name : "detached HEAD");
+		if (opts->strategy)
+			argv_array_pushf(&child.args, "--strategy=%s",
+					 opts->strategy);
+		if (opts->strategy_opts)
+			argv_array_pushf(&child.args, "--strategy-opts=%s",
+					 opts->strategy_opts);
+		if (opts->switch_to)
+			argv_array_pushf(&child.args, "--switch-to=%s",
+					 opts->switch_to);
+		if (opts->cmd)
+			argv_array_pushf(&child.args, "--cmd=%s", opts->cmd);
+		if (opts->allow_empty_message)
+			argv_array_push(&child.args, "--allow-empty-message");
+		if (opts->allow_rerere_autoupdate > 0)
+			argv_array_push(&child.args, "--rerere-autoupdate");
+		else if (opts->allow_rerere_autoupdate == 0)
+			argv_array_push(&child.args, "--no-rerere-autoupdate");
+		if (opts->gpg_sign_opt)
+			argv_array_push(&child.args, opts->gpg_sign_opt);
+		if (opts->signoff)
+			argv_array_push(&child.args, "--signoff");
+
+		status = run_command(&child);
+		goto finished_rebase;
+	}
+
 	add_var(&script_snippet, "GIT_DIR", absolute_path(get_git_dir()));
 	add_var(&script_snippet, "state_dir", opts->state_dir);
 

@@ -418,6 +498,7 @@ static int run_specific_rebase(struct rebase_options *opts)
 	argv[0] = script_snippet.buf;
 
 	status = run_command_v_opt(argv, RUN_USING_SHELL);
+finished_rebase:
 	if (opts->dont_finish_rebase)
 		; /* do nothing */
 	else if (status == 0) {

gitgitgadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

Yup, that sounds like a very sensible structure.

Hi,

Johannes Schindelin wrote:

[nice description snipped]

Please include this information in the commit message. It's super helpful to find this kind of information about why a patch does what it does when encountering a patch later "in the wild" (in git log -S output).

Thanks,
Jonathan

Hi Jonathan,

I thought I did include the relevant part? As to the full back story: I was repeatedly dressed down by Junio in recent attempts to include more motivation in my commit messages. So I am reluctant to do as you say, because Junio is the BDFL here.

Ciao,
Dscho

I do recall discouraging you from including irrelevant rant/whine in the log message a few times in the recent past, and also I do recall you never listening to me. Don't make me an excuse.

I think what Jonathan finds helpful is the other half of the story of what you did write in [1/1]. You wrote that it is no longer a shell script and needs to follow a separate calling convention. What was missing from that description that was given in [0/1] is why the original "rebase-in-c" series was done while pretending that the other effort "rebase-i-in-c" did not even exist, which made it necessary to do this change as a separate step.

And I tend to agree that it _is_ a relevant story in this case.

Hi Junio,

Junio, I would really appreciate less emotional, and more professional conduct from you.

I will await Jonathan's clarification.

Ciao,
Dscho

Which part is unprofessional?

Being caught and corrected with truth immediately after badmouthing another by lying may hurt, but that is your problem, not mine.

[...]

Junio's understanding is correct.

More generally, I greatly appreciate the kind of motivation and backstory that you write in cover letters, and I wish that more of that would find its way into the commit messages instead. Really I wish (and don't take this the wrong way --- I am not asking you to write it unless it's your own itch) that GitGitGadget would put the cover letter in single-patch series after the "---" line in the individual patches, since that would make it easier for reviewers to point out what content from the cover letter would be useful to add to the commit message.

That said, this is minor and not a reason to reroll this patch. It was more that I wanted to give the hint for later patches.

Thanks much and hope that helps,
Jonathan

Hi Junio,

You did not catch me doing anything bad.

You caught me telling Jonathan about having been criticized by you, for including background information *I* found relevant and *you* found irrelevant. And that was very important in this context, as he asked me to include something that I expected you to find irrelevant, too.

Of course, confusingly, this time you found it relevant, even if it was as unrelated to the patch as in the previous case.

So I am getting the impression that your critique was not actually professionally motivated, but very personal. And I do not appreciate that.

I do not need to say anything more about this topic.

Ciao,
Dscho

Hi Jonathan,

It does.

I'll "rick-roll" a new iteration, as I just realized that (contrary to my recollection; I guess I'll need that vacation) the commit message is *seriously* lacking. I thought I had remembered that I copy-edited the commit message into the PR description. Clearly that was not the case.

Thanks for the clarification that triggered my looking, Dscho

The builtin rebase and the builtin interactive rebase have been developed independently, on purpose: Google Summer of Code rules specifically state that students have to work on independent projects, they cannot collaborate on the same project.

The reason is probably the very fine tradition in academia to prohibit teamwork, which makes grading easier (at the expense of not exactly preparing the students for the real world, unless they want to stay in academia).

One fallout is that the rebase-in-c and rebase-i-in-c patches cause no merge conflicts but a royal number of tests in the test suite to fail.

It is easy to explain why: rebase-in-c was developed under the assumption that all rebase backends are implemented in Unix shell script and can be sourced via . git-rebase--<backend>, which is no longer true with rebase-i-in-c, where git-rebase--interactive is a hard-linked builtin.

This patch fixes that.

Note: while this patch targets pk/rebase-in-c-6-final, it will not work correctly without ag/rebase-i-in-c. So my suggestion is to rewrite the pk/rebas-in-c-6-final branch by first merging ag/rebase-i-in-c, then applying this here patch, and only then cherry-pick "rebase: default to using the builtin rebase".

Changes since v1:

* replaced the too-terse commit message by a copy-edited version of this cover letter (leaving out only the rant about disallowing teamwork).

Johannes Schindelin (1):
builtin rebase: prepare for builtin rebase -i

 builtin/rebase.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

base-commit: ae497a044508ebaac1794dcdd7ad04f8685686b2
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-23%2Fdscho%2Frebase-in-c-6-final-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-23/dscho/rebase-in-c-6-final-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/23

Range-diff vs v1:

1: 29d49819fa ! 1: 5403014be7 builtin rebase: prepare for builtin rebase -i
@@ -2,8 +2,21 @@

builtin rebase: prepare for builtin rebase -i

- It is no longer a shell script, so we need to call it in a different way
- than the other backends.
+ The builtin rebase and the builtin interactive rebase have been
+ developed independently, on purpose: Google Summer of Code rules
+ specifically state that students have to work on independent projects,
+ they cannot collaborate on the same project.
+
+ One fallout is that the rebase-in-c and rebase-i-in-c patches cause no
+ merge conflicts but a royal number of tests in the test suite to fail.
+
+ It is easy to explain why: rebase-in-c was developed under the
+ assumption that all rebase backends are implemented in Unix shell script
+ and can be sourced via . git-rebase--<backend>, which is no longer
+ true with rebase-i-in-c, where git-rebase--interactive is a hard-linked
+ builtin.
+
+ This patch fixes that.

gitgitgadget

From: Johannes Schindelin <[email protected]>

The builtin rebase and the builtin interactive rebase have been developed independently, on purpose: Google Summer of Code rules specifically state that students have to work on independent projects, they cannot collaborate on the same project.

One fallout is that the rebase-in-c and rebase-i-in-c patches cause no merge conflicts but a royal number of tests in the test suite to fail.

It is easy to explain why: rebase-in-c was developed under the assumption that all rebase backends are implemented in Unix shell script and can be sourced via . git-rebase--<backend>, which is no longer true with rebase-i-in-c, where git-rebase--interactive is a hard-linked builtin.

This patch fixes that.

---
 builtin/rebase.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

index 4e69458161..99fd5d4017 100644
@@ -326,6 +326,13 @@ static void add_var(struct strbuf *buf, const char *name, const char *value)
 	}
 }
 
+static const char *resolvemsg =
+N_("Resolve all conflicts manually, mark them as resolved with\n"
+"\"git add/rm <conflicted_files>\", then run \"git rebase --continue\".\n"
+"You can instead skip this commit: run \"git rebase --skip\".\n"
+"To abort and get back to the state before \"git rebase\", run "
+"\"git rebase --abort\".");
+
 static int run_specific_rebase(struct rebase_options *opts)
 {
 	const char *argv[] = { NULL, NULL };

@@ -333,6 +340,79 @@ static int run_specific_rebase(struct rebase_options *opts)
 	int status;
 	const char *backend, *backend_func;
 
+	if (opts->type == REBASE_INTERACTIVE) {
+		/* Run builtin interactive rebase */
+		struct child_process child = CHILD_PROCESS_INIT;
+
+		argv_array_pushf(&child.env_array, "GIT_CHERRY_PICK_HELP=%s",
+				 resolvemsg);
+		if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) {
+			argv_array_push(&child.env_array, "GIT_EDITOR=:");
+			opts->autosquash = 0;
+		}
+
+		child.git_cmd = 1;
+		argv_array_push(&child.args, "rebase--interactive");
+
+		if (opts->action)
+			argv_array_pushf(&child.args, "--%s", opts->action);
+		if (opts->keep_empty)
+			argv_array_push(&child.args, "--keep-empty");
+		if (opts->rebase_merges)
+			argv_array_push(&child.args, "--rebase-merges");
+		if (opts->rebase_cousins)
+			argv_array_push(&child.args, "--rebase-cousins");
+		if (opts->autosquash)
+			argv_array_push(&child.args, "--autosquash");
+		if (opts->flags & REBASE_VERBOSE)
+			argv_array_push(&child.args, "--verbose");
+		if (opts->flags & REBASE_FORCE)
+			argv_array_push(&child.args, "--no-ff");
+		if (opts->restrict_revision)
+			argv_array_pushf(&child.args,
+					 "--restrict-revision=^%s",
+					 oid_to_hex(&opts->restrict_revision->object.oid));
+		if (opts->upstream)
+			argv_array_pushf(&child.args, "--upstream=%s",
+					 oid_to_hex(&opts->upstream->object.oid));
+		if (opts->onto)
+			argv_array_pushf(&child.args, "--onto=%s",
+					 oid_to_hex(&opts->onto->object.oid));
+		if (opts->squash_onto)
+			argv_array_pushf(&child.args, "--squash-onto=%s",
+					 oid_to_hex(opts->squash_onto));
+		if (opts->onto_name)
+			argv_array_pushf(&child.args, "--onto-name=%s",
+					 opts->onto_name);
+		argv_array_pushf(&child.args, "--head-name=%s",
+				 opts->head_name ?
+				 opts->head_name : "detached HEAD");
+		if (opts->strategy)
+			argv_array_pushf(&child.args, "--strategy=%s",
+					 opts->strategy);
+		if (opts->strategy_opts)
+			argv_array_pushf(&child.args, "--strategy-opts=%s",
+					 opts->strategy_opts);
+		if (opts->switch_to)
+			argv_array_pushf(&child.args, "--switch-to=%s",
+					 opts->switch_to);
+		if (opts->cmd)
+			argv_array_pushf(&child.args, "--cmd=%s", opts->cmd);
+		if (opts->allow_empty_message)
+			argv_array_push(&child.args, "--allow-empty-message");
+		if (opts->allow_rerere_autoupdate > 0)
+			argv_array_push(&child.args, "--rerere-autoupdate");
+		else if (opts->allow_rerere_autoupdate == 0)
+			argv_array_push(&child.args, "--no-rerere-autoupdate");
+		if (opts->gpg_sign_opt)
+			argv_array_push(&child.args, opts->gpg_sign_opt);
+		if (opts->signoff)
+			argv_array_push(&child.args, "--signoff");
+
+		status = run_command(&child);
+		goto finished_rebase;
+	}
+
 	add_var(&script_snippet, "GIT_DIR", absolute_path(get_git_dir()));
 	add_var(&script_snippet, "state_dir", opts->state_dir);
 

@@ -418,6 +498,7 @@ static int run_specific_rebase(struct rebase_options *opts)
 	argv[0] = script_snippet.buf;
 
 	status = run_command_v_opt(argv, RUN_USING_SHELL);
+finished_rebase:
 	if (opts->dont_finish_rebase)
 		; /* do nothing */
 	else if (status == 0) {

gitgitgadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

A much better description, especially without the less relevant "the reason probably is..." omitted from here. The author's personal guess, while adding it does not help understanding what is already in the above paragraph an iota, is still a fine reading material in the cover letter 0/1, though.

Will replace by doing:

    $ git checkout js/rebase-in-c-5.5-work-with-rebase-i-in-c
    $ git checkout HEAD^
    $ git am -s mbox
    $ git range-diff @{-1}...
    $ git checkout -B @{-1}

    $ git checkout pk/rebase-i-in-c-6-final
    $ git rebase --onto js/rebase-in-c-5.5-work-with-rebase-i-in-c \
          js/rebase-in-c-5.5-work-with-rebase-i-in-c@{1} HEAD^0
    $ git range-diff @{-1}...
    $ git checkout -B @{-1}

to update the two topics and then rebuilding the integration branches the usual way. I also need to replace the "other" topic used in this topic, so the actual procedure would be a bit more involved than the above, though.

Thanks.

Hi Junio,

I addressed Jonathan's concern, though.

Is there any reason why you avoid using git rebase -ir here? This should be so much easier via

	git checkout pk/rebase-i-in-c-6-final
	git rebase -ir js/rebase-in-c-5.5-work-with-rebase-i-in-c^

and then inserting this at the appropriate position, followed by the git range-diff @{-1}...:

git am -s mbox
git update-ref js/rebase-in-c-5.5-work-with-rebase-i-in-c HEAD

Ciao,
Dscho

Related discussion, including a fantasy tangent by me (downthread):

https://public-inbox.org/git/[email protected]/#t

-Peff

Hi Peff,

I have no idea what you meant there...

Ciao,
Dscho

I thought you were asking why Junio does not just use "git am" from inside "git rebase". I asked the same thing recently, and the answer is because he is afraid of how the two interact. I dug a little into it (the fantasy part is that I laid out a dream for how operations like this could safely stack).

-Peff

The builtin rebase and the builtin interactive rebase have been developed independently, on purpose: Google Summer of Code rules specifically state that students have to work on independent projects, they cannot collaborate on the same project.

The reason is probably the very fine tradition in academia to prohibit teamwork, which makes grading easier (at the expense of not exactly preparing the students for the real world, unless they want to stay in academia).

One fallout is that the rebase-in-c and rebase-i-in-c patches cause no merge conflicts but a royal number of tests in the test suite to fail.

It is easy to explain why: rebase-in-c was developed under the assumption that all rebase backends are implemented in Unix shell script and can be sourced via . git-rebase--<backend>, which is no longer true with rebase-i-in-c, where git-rebase--interactive is a hard-linked builtin.

This patch fixes that.

Note: while this patch targets pk/rebase-in-c-6-final, it will not work correctly without ag/rebase-i-in-c. So my suggestion is to rewrite the pk/rebas-in-c-6-final branch by first merging ag/rebase-i-in-c, then applying this here patch, and only then cherry-pick "rebase: default to using the builtin rebase".

Changes since v2:

* Prepare for the break command, by skipping the call to finish_rebase() for interactive rebases altogether (the built-in interactive rebase already takes care of that).
* Remove a no-longer-used case arm (skipped by the newly-introduced code).

Changes since v1:

* replaced the too-terse commit message by a copy-edited version of this cover letter (leaving out only the rant about disallowing teamwork).

Johannes Schindelin (1):
builtin rebase: prepare for builtin rebase -i

 builtin/rebase.c | 87 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 83 insertions(+), 4 deletions(-)

base-commit: 67e0df2f391ec4177942a4d8b70e530aa5653c0d
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-23%2Fdscho%2Frebase-in-c-6-final-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-23/dscho/rebase-in-c-6-final-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/23

Range-diff vs v2:

1: 5403014be7 ! 1: db1652ef3e builtin rebase: prepare for builtin rebase -i
@@ -18,6 +18,15 @@

This patch fixes that.

+ Please note that we also skip the finish_rebase() call for interactive
+ rebases because the built-in interactive rebase already takes care of
+ that. This is needed to support the upcoming break command that wants
+ to interrupt the rebase with exit code 0 (and naturally wants to keep
+ the state directory intact when doing so).
+
+ While at it, remove the case arm for the interactive rebase that is
+ now skipped in favor of the short-cut to the built-in rebase.
+

diff --git a/builtin/rebase.c b/builtin/rebase.c
@@ -117,6 +126,17 @@
add_var(&script_snippet, "GIT_DIR", absolute_path(get_git_dir())); add_var(&script_snippet, "state_dir", opts->state_dir);

+@@
+ backend = "git-rebase--am";
+ backend_func = "git_rebase__am";
+ break;
+- case REBASE_INTERACTIVE:
+- backend = "git-rebase--interactive";
+- backend_func = "git_rebase__interactive";
+- break;
+ case REBASE_MERGE:
+ backend = "git-rebase--merge";
+ backend_func = "git_rebase__merge";
@@
argv[0] = script_snippet.buf;

@@ -124,4 +144,8 @@
+finished_rebase:
if (opts->dont_finish_rebase)
; /* do nothing */
++ else if (opts->type == REBASE_INTERACTIVE)
++ ; /* interactive rebase cleans up after itself */
else if (status == 0) {
+ if (!file_exists(state_dir_path("stopped-sha", opts)))
+ finish_rebase(opts);

gitgitgadget

From: Johannes Schindelin <[email protected]>

The builtin rebase and the builtin interactive rebase have been developed independently, on purpose: Google Summer of Code rules specifically state that students have to work on independent projects, they cannot collaborate on the same project.

One fallout is that the rebase-in-c and rebase-i-in-c patches cause no merge conflicts but a royal number of tests in the test suite to fail.

It is easy to explain why: rebase-in-c was developed under the assumption that all rebase backends are implemented in Unix shell script and can be sourced via . git-rebase--<backend>, which is no longer true with rebase-i-in-c, where git-rebase--interactive is a hard-linked builtin.

This patch fixes that.

Please note that we also skip the finish_rebase() call for interactive rebases because the built-in interactive rebase already takes care of that. This is needed to support the upcoming break command that wants to interrupt the rebase with exit code 0 (and naturally wants to keep the state directory intact when doing so).

While at it, remove the case arm for the interactive rebase that is now skipped in favor of the short-cut to the built-in rebase.

---
 builtin/rebase.c | 87 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 83 insertions(+), 4 deletions(-)

index 1a697d70c9..20f7159cf2 100644
@@ -326,6 +326,13 @@ static void add_var(struct strbuf *buf, const char *name, const char *value)
 	}
 }
 
+static const char *resolvemsg =
+N_("Resolve all conflicts manually, mark them as resolved with\n"
+"\"git add/rm <conflicted_files>\", then run \"git rebase --continue\".\n"
+"You can instead skip this commit: run \"git rebase --skip\".\n"
+"To abort and get back to the state before \"git rebase\", run "
+"\"git rebase --abort\".");
+
 static int run_specific_rebase(struct rebase_options *opts)
 {
 	const char *argv[] = { NULL, NULL };

@@ -333,6 +340,79 @@ static int run_specific_rebase(struct rebase_options *opts)
 	int status;
 	const char *backend, *backend_func;
 
+	if (opts->type == REBASE_INTERACTIVE) {
+		/* Run builtin interactive rebase */
+		struct child_process child = CHILD_PROCESS_INIT;
+
+		argv_array_pushf(&child.env_array, "GIT_CHERRY_PICK_HELP=%s",
+				 resolvemsg);
+		if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) {
+			argv_array_push(&child.env_array, "GIT_EDITOR=:");
+			opts->autosquash = 0;
+		}
+
+		child.git_cmd = 1;
+		argv_array_push(&child.args, "rebase--interactive");
+
+		if (opts->action)
+			argv_array_pushf(&child.args, "--%s", opts->action);
+		if (opts->keep_empty)
+			argv_array_push(&child.args, "--keep-empty");
+		if (opts->rebase_merges)
+			argv_array_push(&child.args, "--rebase-merges");
+		if (opts->rebase_cousins)
+			argv_array_push(&child.args, "--rebase-cousins");
+		if (opts->autosquash)
+			argv_array_push(&child.args, "--autosquash");
+		if (opts->flags & REBASE_VERBOSE)
+			argv_array_push(&child.args, "--verbose");
+		if (opts->flags & REBASE_FORCE)
+			argv_array_push(&child.args, "--no-ff");
+		if (opts->restrict_revision)
+			argv_array_pushf(&child.args,
+					 "--restrict-revision=^%s",
+					 oid_to_hex(&opts->restrict_revision->object.oid));
+		if (opts->upstream)
+			argv_array_pushf(&child.args, "--upstream=%s",
+					 oid_to_hex(&opts->upstream->object.oid));
+		if (opts->onto)
+			argv_array_pushf(&child.args, "--onto=%s",
+					 oid_to_hex(&opts->onto->object.oid));
+		if (opts->squash_onto)
+			argv_array_pushf(&child.args, "--squash-onto=%s",
+					 oid_to_hex(opts->squash_onto));
+		if (opts->onto_name)
+			argv_array_pushf(&child.args, "--onto-name=%s",
+					 opts->onto_name);
+		argv_array_pushf(&child.args, "--head-name=%s",
+				 opts->head_name ?
+				 opts->head_name : "detached HEAD");
+		if (opts->strategy)
+			argv_array_pushf(&child.args, "--strategy=%s",
+					 opts->strategy);
+		if (opts->strategy_opts)
+			argv_array_pushf(&child.args, "--strategy-opts=%s",
+					 opts->strategy_opts);
+		if (opts->switch_to)
+			argv_array_pushf(&child.args, "--switch-to=%s",
+					 opts->switch_to);
+		if (opts->cmd)
+			argv_array_pushf(&child.args, "--cmd=%s", opts->cmd);
+		if (opts->allow_empty_message)
+			argv_array_push(&child.args, "--allow-empty-message");
+		if (opts->allow_rerere_autoupdate > 0)
+			argv_array_push(&child.args, "--rerere-autoupdate");
+		else if (opts->allow_rerere_autoupdate == 0)
+			argv_array_push(&child.args, "--no-rerere-autoupdate");
+		if (opts->gpg_sign_opt)
+			argv_array_push(&child.args, opts->gpg_sign_opt);
+		if (opts->signoff)
+			argv_array_push(&child.args, "--signoff");
+
+		status = run_command(&child);
+		goto finished_rebase;
+	}
+
 	add_var(&script_snippet, "GIT_DIR", absolute_path(get_git_dir()));
 	add_var(&script_snippet, "state_dir", opts->state_dir);
 

@@ -395,10 +475,6 @@ static int run_specific_rebase(struct rebase_options *opts)
 		backend = "git-rebase--am";
 		backend_func = "git_rebase__am";
 		break;
-	case REBASE_INTERACTIVE:
-		backend = "git-rebase--interactive";
-		backend_func = "git_rebase__interactive";
-		break;
 	case REBASE_MERGE:
 		backend = "git-rebase--merge";
 		backend_func = "git_rebase__merge";

@@ -418,8 +494,11 @@ static int run_specific_rebase(struct rebase_options *opts)
 	argv[0] = script_snippet.buf;
 
 	status = run_command_v_opt(argv, RUN_USING_SHELL);
+finished_rebase:
 	if (opts->dont_finish_rebase)
 		; /* do nothing */
+	else if (opts->type == REBASE_INTERACTIVE)
+		; /* interactive rebase cleans up after itself */
 	else if (status == 0) {
 		if (!file_exists(state_dir_path("stopped-sha", opts)))
 			finish_rebase(opts);

gitgitgadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

Is this a stale comment in the context of v3, where we pretty much know how the resulting topic should intertwine with other topics? I am trying to see if I have do to anything differently this time, or just replacing the single commit while keeping the structure around that commit the same is fine.

Also, I see there is a new iteration v8 of ag/rebase-i-in-c on the list, sent on Sep 27th (you were CC'ed but I suspect it was before you came back). Are you happy with that update? Otherwise, we should make sure that topic is solid enough before extending (meaning: I'd replace this one while keeping ag/rebase-i-in-c that has been cooking in 'pu', without updating the latter).

Thanks.

Hi Junio,

Just replacing the single commit. For technical reasons, I still have to target pk/rebase-in-c-6-final in my branch.

To be quite honest, I did not yet have time to look over it, but I verified that it has my suggested fix for the -S option.

Ciao,
Dscho