* [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase @ 2018-08-22 21:35 Johannes Schindelin via GitGitGadget 2018-08-22 21:35 ` [PATCH 1/1] builtin rebase: prepare for builtin rebase -i Johannes Schindelin via GitGitGadget ` (3 more replies) 0 siblings, 4 replies; 22+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2018-08-22 21:35 UTC (permalink / raw) To: git; +Cc: Junio C Hamano 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/1] builtin rebase: prepare for builtin rebase -i 2018-08-22 21:35 [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase Johannes Schindelin via GitGitGadget @ 2018-08-22 21:35 ` Johannes Schindelin via GitGitGadget 2018-08-22 21:50 ` [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase Junio C Hamano ` (2 subsequent siblings) 3 siblings, 0 replies; 22+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2018-08-22 21:35 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> It is no longer a shell script, so we need to call it in a different way than the other backends. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- builtin/rebase.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/builtin/rebase.c b/builtin/rebase.c index c8d632b6f4..87590047b3 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase 2018-08-22 21:35 [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase Johannes Schindelin via GitGitGadget 2018-08-22 21:35 ` [PATCH 1/1] builtin rebase: prepare for builtin rebase -i Johannes Schindelin via GitGitGadget @ 2018-08-22 21:50 ` Junio C Hamano 2018-08-23 2:48 ` Jonathan Nieder 2018-08-29 14:31 ` [PATCH v2 " Johannes Schindelin via GitGitGadget 3 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2018-08-22 21:50 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget; +Cc: git "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > 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". Yup, that sounds like a very sensible structure. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase 2018-08-22 21:35 [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase Johannes Schindelin via GitGitGadget 2018-08-22 21:35 ` [PATCH 1/1] builtin rebase: prepare for builtin rebase -i Johannes Schindelin via GitGitGadget 2018-08-22 21:50 ` [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase Junio C Hamano @ 2018-08-23 2:48 ` Jonathan Nieder 2018-08-25 23:46 ` Johannes Schindelin 2018-08-29 14:31 ` [PATCH v2 " Johannes Schindelin via GitGitGadget 3 siblings, 1 reply; 22+ messages in thread From: Jonathan Nieder @ 2018-08-23 2:48 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget; +Cc: git, Junio C Hamano Hi, Johannes Schindelin wrote: [nice description snipped] > This patch fixes that. 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase 2018-08-23 2:48 ` Jonathan Nieder @ 2018-08-25 23:46 ` Johannes Schindelin 2018-08-27 17:48 ` Junio C Hamano 0 siblings, 1 reply; 22+ messages in thread From: Johannes Schindelin @ 2018-08-25 23:46 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Johannes Schindelin via GitGitGadget, git, Junio C Hamano Hi Jonathan, On Wed, 22 Aug 2018, Jonathan Nieder wrote: > Johannes Schindelin wrote: > > [nice description snipped] > > This patch fixes that. > > 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). 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase 2018-08-25 23:46 ` Johannes Schindelin @ 2018-08-27 17:48 ` Junio C Hamano 2018-08-28 12:53 ` Johannes Schindelin 0 siblings, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2018-08-27 17:48 UTC (permalink / raw) To: Johannes Schindelin Cc: Jonathan Nieder, Johannes Schindelin via GitGitGadget, git Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >> 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). > > 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. 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase 2018-08-27 17:48 ` Junio C Hamano @ 2018-08-28 12:53 ` Johannes Schindelin 2018-08-28 15:34 ` Junio C Hamano 2018-08-28 17:17 ` Jonathan Nieder 0 siblings, 2 replies; 22+ messages in thread From: Johannes Schindelin @ 2018-08-28 12:53 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jonathan Nieder, Johannes Schindelin via GitGitGadget, git Hi Junio, On Mon, 27 Aug 2018, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > >> 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). > > > > 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. > > 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. Junio, I would really appreciate less emotional, and more professional conduct from you. > I think what Jonathan finds helpful is the other half of the story I will await Jonathan's clarification. Ciao, Dscho > 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. > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase 2018-08-28 12:53 ` Johannes Schindelin @ 2018-08-28 15:34 ` Junio C Hamano 2018-08-29 13:24 ` Johannes Schindelin 2018-08-28 17:17 ` Jonathan Nieder 1 sibling, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2018-08-28 15:34 UTC (permalink / raw) To: Johannes Schindelin Cc: Jonathan Nieder, Johannes Schindelin via GitGitGadget, git Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >> 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. > > Junio, I would really appreciate less emotional, and more professional > conduct from you. 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase 2018-08-28 15:34 ` Junio C Hamano @ 2018-08-29 13:24 ` Johannes Schindelin 0 siblings, 0 replies; 22+ messages in thread From: Johannes Schindelin @ 2018-08-29 13:24 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jonathan Nieder, Johannes Schindelin via GitGitGadget, git Hi Junio, On Tue, 28 Aug 2018, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > >> 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. > > > > Junio, I would really appreciate less emotional, and more professional > > conduct from you. > > 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. 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase 2018-08-28 12:53 ` Johannes Schindelin 2018-08-28 15:34 ` Junio C Hamano @ 2018-08-28 17:17 ` Jonathan Nieder 2018-08-29 14:29 ` Johannes Schindelin 1 sibling, 1 reply; 22+ messages in thread From: Jonathan Nieder @ 2018-08-28 17:17 UTC (permalink / raw) To: Johannes Schindelin Cc: Junio C Hamano, Johannes Schindelin via GitGitGadget, git Johannes Schindelin wrote: > On Mon, 27 Aug 2018, Junio C Hamano wrote: >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: >>> Jonathan Nieder wrote: >>>> 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). [...] >> I think what Jonathan finds helpful is the other half of the story > > I will await Jonathan's clarification. 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase 2018-08-28 17:17 ` Jonathan Nieder @ 2018-08-29 14:29 ` Johannes Schindelin 0 siblings, 0 replies; 22+ messages in thread From: Johannes Schindelin @ 2018-08-29 14:29 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Junio C Hamano, Johannes Schindelin via GitGitGadget, git Hi Jonathan, On Tue, 28 Aug 2018, Jonathan Nieder wrote: > Johannes Schindelin wrote: > > On Mon, 27 Aug 2018, Junio C Hamano wrote: > >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > >>> Jonathan Nieder wrote: > > >>>> 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). > [...] > >> I think what Jonathan finds helpful is the other half of the story > > > > I will await Jonathan's clarification. > > 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, 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 0/1] Teach the builtin rebase about the builtin interactive rebase 2018-08-22 21:35 [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase Johannes Schindelin via GitGitGadget ` (2 preceding siblings ...) 2018-08-23 2:48 ` Jonathan Nieder @ 2018-08-29 14:31 ` Johannes Schindelin via GitGitGadget 2018-08-29 14:31 ` [PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i Johannes Schindelin via GitGitGadget 2018-10-05 15:54 ` [PATCH v3 0/1] Teach the builtin rebase about the builtin interactive rebase Johannes Schindelin via GitGitGadget 3 siblings, 2 replies; 22+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2018-08-29 14:31 UTC (permalink / raw) To: git; +Cc: Junio C Hamano 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. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> -- gitgitgadget ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i 2018-08-29 14:31 ` [PATCH v2 " Johannes Schindelin via GitGitGadget @ 2018-08-29 14:31 ` Johannes Schindelin via GitGitGadget 2018-08-29 22:40 ` Junio C Hamano 2018-10-05 15:54 ` [PATCH v3 0/1] Teach the builtin rebase about the builtin interactive rebase Johannes Schindelin via GitGitGadget 1 sibling, 1 reply; 22+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2018-08-29 14:31 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> 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. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- builtin/rebase.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/builtin/rebase.c b/builtin/rebase.c index 4e69458161..99fd5d4017 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i 2018-08-29 14:31 ` [PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i Johannes Schindelin via GitGitGadget @ 2018-08-29 22:40 ` Junio C Hamano 2018-08-30 11:03 ` Johannes Schindelin 0 siblings, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2018-08-29 22:40 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget; +Cc: git, Johannes Schindelin "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > 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. 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. > 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. > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > builtin/rebase.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 81 insertions(+) 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i 2018-08-29 22:40 ` Junio C Hamano @ 2018-08-30 11:03 ` Johannes Schindelin 2018-08-30 20:09 ` Jeff King 0 siblings, 1 reply; 22+ messages in thread From: Johannes Schindelin @ 2018-08-30 11:03 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git Hi Junio, On Wed, 29 Aug 2018, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > writes: > > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > > > 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. > > 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. I addressed Jonathan's concern, though. > > 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. > > > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > > --- > > builtin/rebase.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 81 insertions(+) > > > 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. 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i 2018-08-30 11:03 ` Johannes Schindelin @ 2018-08-30 20:09 ` Jeff King 2018-08-31 20:38 ` Johannes Schindelin 0 siblings, 1 reply; 22+ messages in thread From: Jeff King @ 2018-08-30 20:09 UTC (permalink / raw) To: Johannes Schindelin Cc: Junio C Hamano, Johannes Schindelin via GitGitGadget, git On Thu, Aug 30, 2018 at 01:03:41PM +0200, Johannes Schindelin wrote: > > 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. > > 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 Related discussion, including a fantasy tangent by me (downthread): https://public-inbox.org/git/20180727080807.GA11932@sigill.intra.peff.net/#t -Peff ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i 2018-08-30 20:09 ` Jeff King @ 2018-08-31 20:38 ` Johannes Schindelin 2018-08-31 20:48 ` Jeff King 0 siblings, 1 reply; 22+ messages in thread From: Johannes Schindelin @ 2018-08-31 20:38 UTC (permalink / raw) To: Jeff King; +Cc: Junio C Hamano, Johannes Schindelin via GitGitGadget, git Hi Peff, On Thu, 30 Aug 2018, Jeff King wrote: > On Thu, Aug 30, 2018 at 01:03:41PM +0200, Johannes Schindelin wrote: > > > > 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. > > > > 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 > > Related discussion, including a fantasy tangent by me (downthread): > > https://public-inbox.org/git/20180727080807.GA11932@sigill.intra.peff.net/#t I have no idea what you meant there... Ciao, Dscho ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i 2018-08-31 20:38 ` Johannes Schindelin @ 2018-08-31 20:48 ` Jeff King 0 siblings, 0 replies; 22+ messages in thread From: Jeff King @ 2018-08-31 20:48 UTC (permalink / raw) To: Johannes Schindelin Cc: Junio C Hamano, Johannes Schindelin via GitGitGadget, git On Fri, Aug 31, 2018 at 10:38:44PM +0200, Johannes Schindelin wrote: > > > 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 > > > > Related discussion, including a fantasy tangent by me (downthread): > > > > https://public-inbox.org/git/20180727080807.GA11932@sigill.intra.peff.net/#t > > I have no idea what you meant there... 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 0/1] Teach the builtin rebase about the builtin interactive rebase 2018-08-29 14:31 ` [PATCH v2 " Johannes Schindelin via GitGitGadget 2018-08-29 14:31 ` [PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i Johannes Schindelin via GitGitGadget @ 2018-10-05 15:54 ` Johannes Schindelin via GitGitGadget 2018-10-05 15:54 ` [PATCH v3 1/1] builtin rebase: prepare for builtin rebase -i Johannes Schindelin via GitGitGadget 2018-10-06 23:50 ` [PATCH v3 0/1] Teach the builtin rebase about the builtin interactive rebase Junio C Hamano 1 sibling, 2 replies; 22+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2018-10-05 15:54 UTC (permalink / raw) To: git; +Cc: Junio C Hamano 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. + Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3 1/1] builtin rebase: prepare for builtin rebase -i 2018-10-05 15:54 ` [PATCH v3 0/1] Teach the builtin rebase about the builtin interactive rebase Johannes Schindelin via GitGitGadget @ 2018-10-05 15:54 ` Johannes Schindelin via GitGitGadget 2018-10-06 23:50 ` [PATCH v3 0/1] Teach the builtin rebase about the builtin interactive rebase Junio C Hamano 1 sibling, 0 replies; 22+ messages in thread From: Johannes Schindelin via GitGitGadget @ 2018-10-05 15:54 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin From: Johannes Schindelin <johannes.schindelin@gmx.de> 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. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- builtin/rebase.c | 87 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 83 insertions(+), 4 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 1a697d70c9..20f7159cf2 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3 0/1] Teach the builtin rebase about the builtin interactive rebase 2018-10-05 15:54 ` [PATCH v3 0/1] Teach the builtin rebase about the builtin interactive rebase Johannes Schindelin via GitGitGadget 2018-10-05 15:54 ` [PATCH v3 1/1] builtin rebase: prepare for builtin rebase -i Johannes Schindelin via GitGitGadget @ 2018-10-06 23:50 ` Junio C Hamano 2018-10-12 7:59 ` Johannes Schindelin 1 sibling, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2018-10-06 23:50 UTC (permalink / raw) To: Johannes Schindelin via GitGitGadget; +Cc: git "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > 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". 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). > 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). Thanks. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3 0/1] Teach the builtin rebase about the builtin interactive rebase 2018-10-06 23:50 ` [PATCH v3 0/1] Teach the builtin rebase about the builtin interactive rebase Junio C Hamano @ 2018-10-12 7:59 ` Johannes Schindelin 0 siblings, 0 replies; 22+ messages in thread From: Johannes Schindelin @ 2018-10-12 7:59 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin via GitGitGadget, git Hi Junio, On Sun, 7 Oct 2018, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > writes: > > > 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". > > 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. Just replacing the single commit. For technical reasons, I still have to target pk/rebase-in-c-6-final in my branch. > 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? 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2018-10-12 7:59 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-22 21:35 [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase Johannes Schindelin via GitGitGadget 2018-08-22 21:35 ` [PATCH 1/1] builtin rebase: prepare for builtin rebase -i Johannes Schindelin via GitGitGadget 2018-08-22 21:50 ` [PATCH 0/1] Teach the builtin rebase about the builtin interactive rebase Junio C Hamano 2018-08-23 2:48 ` Jonathan Nieder 2018-08-25 23:46 ` Johannes Schindelin 2018-08-27 17:48 ` Junio C Hamano 2018-08-28 12:53 ` Johannes Schindelin 2018-08-28 15:34 ` Junio C Hamano 2018-08-29 13:24 ` Johannes Schindelin 2018-08-28 17:17 ` Jonathan Nieder 2018-08-29 14:29 ` Johannes Schindelin 2018-08-29 14:31 ` [PATCH v2 " Johannes Schindelin via GitGitGadget 2018-08-29 14:31 ` [PATCH v2 1/1] builtin rebase: prepare for builtin rebase -i Johannes Schindelin via GitGitGadget 2018-08-29 22:40 ` Junio C Hamano 2018-08-30 11:03 ` Johannes Schindelin 2018-08-30 20:09 ` Jeff King 2018-08-31 20:38 ` Johannes Schindelin 2018-08-31 20:48 ` Jeff King 2018-10-05 15:54 ` [PATCH v3 0/1] Teach the builtin rebase about the builtin interactive rebase Johannes Schindelin via GitGitGadget 2018-10-05 15:54 ` [PATCH v3 1/1] builtin rebase: prepare for builtin rebase -i Johannes Schindelin via GitGitGadget 2018-10-06 23:50 ` [PATCH v3 0/1] Teach the builtin rebase about the builtin interactive rebase Junio C Hamano 2018-10-12 7:59 ` Johannes Schindelin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.