| Message ID | pull.1492.git.1679237337683.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rebase -i: do not update "done" when rescheduling command | expand |
On 19.03.23 15:48, Phillip Wood via GitGitGadget wrote: > Stefan - thanks for reporting this, hopefully it will be easy enough to > update lazygit to check the last command in the done file as well as the > second to last command for older versions of git. Thanks for the fix. Yes, it should be easy to adapt lazygit to the new behavior. -Stefan
"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > As the sequencer executes todo commands it appends them to > .git/rebase-merge/done. This file is used by "git status" to show the > recently executed commands. Unfortunately when a command is rescheduled > the command preceding it is erroneously appended to the "done" file. > This means that when rebase stops after rescheduling "pick B" the "done" > file contains > > pick A > pick B > pick A > > instead of > > pick A > pick B Here it may not be clear what you meant with the verb "reschedule" to those who weren't closely following the previous discussion that led to this fix. Is it the same as "the command attempted to execute a step, couldn't complete it (e.g. due to conflicts), and gave control to the end user until they say 'git rebase --continue'"? What cases, other than interrupted step due to conflicts, involve "rescheduling"? > Note that the rescheduled command will still be appended to the "done" > file again when it is successfully executed. Arguably it would be better > not to do that but fixing it would be more involved. And without quite understanding what "reschedule" refers to, it is unclear why it is even arguable---it is perfectly sensible that a command that is rescheduled (hence not yet done) would not be sent to 'done'. If a command that was once rescheduled (hence it wasn't finished initially) gets finished now, shouldn't it be sent to 'done'? It is unclear why is it better not to. > -static int save_todo(struct todo_list *todo_list, struct replay_opts *opts) > +static int save_todo(struct todo_list *todo_list, struct replay_opts *opts, > + int reschedule) > { OK, all callers to save_todo() are in pick_commits() that knows what the value of "reschedule" is, and it is passed down to this helper ... > @@ -3389,7 +3390,7 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts) > * rebase -i writes "git-rebase-todo" without the currently executing > * command, appending it to "done" instead. > */ > - if (is_rebase_i(opts)) > + if (is_rebase_i(opts) && !reschedule) > next++; > > fd = hold_lock_file_for_update(&todo_lock, todo_path, 0); > @@ -3402,7 +3403,7 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts) > if (commit_lock_file(&todo_lock) < 0) > return error(_("failed to finalize '%s'"), todo_path); > > - if (is_rebase_i(opts) && next > 0) { > + if (is_rebase_i(opts) && !reschedule && next > 0) { > const char *done = rebase_path_done(); > int fd = open(done, O_CREAT | O_WRONLY | O_APPEND, 0666); > int ret = 0; ... and the change here is quite straight-forward. With reschedule, we do not advance because by definition we haven't finished the step yet. OK. > @@ -4648,7 +4649,7 @@ static int pick_commits(struct repository *r, > const char *arg = todo_item_get_arg(todo_list, item); > int check_todo = 0; > > - if (save_todo(todo_list, opts)) > + if (save_todo(todo_list, opts, 0)) > return -1; I wonder why we pass a hardcoded 0 here---shouldn't the value match the local variable 'reschedule'? here? The same question for the other two callers, but I admit that when the second one is called, the local variable "reschedule" is not set... > if (is_rebase_i(opts)) { > if (item->command != TODO_COMMENT) { > @@ -4695,8 +4696,7 @@ static int pick_commits(struct repository *r, > todo_list->current), > get_item_line(todo_list, > todo_list->current)); > - todo_list->current--; > - if (save_todo(todo_list, opts)) > + if (save_todo(todo_list, opts, 1)) > return -1; ... yet we call the helper with reschedule set to 1. Puzzled. > @@ -4788,8 +4788,7 @@ static int pick_commits(struct repository *r, > get_item_line_length(todo_list, > todo_list->current), > get_item_line(todo_list, todo_list->current)); > - todo_list->current--; > - if (save_todo(todo_list, opts)) > + if (save_todo(todo_list, opts, 1)) > return -1; At this point, reschedule is set and passing it instead of 1 would be OK. Thanks.
Hi Junio Thanks for your comments On 20/03/2023 17:46, Junio C Hamano wrote: > "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> As the sequencer executes todo commands it appends them to >> .git/rebase-merge/done. This file is used by "git status" to show the >> recently executed commands. Unfortunately when a command is rescheduled >> the command preceding it is erroneously appended to the "done" file. >> This means that when rebase stops after rescheduling "pick B" the "done" >> file contains >> >> pick A >> pick B >> pick A >> >> instead of >> >> pick A >> pick B > > Here it may not be clear what you meant with the verb "reschedule" > to those who weren't closely following the previous discussion that > led to this fix. > > Is it the same as "the command attempted to execute a step, couldn't > complete it (e.g. due to conflicts), and gave control to the end > user until they say 'git rebase --continue'"? What cases, other > than interrupted step due to conflicts, involve "rescheduling"? I'll expand the commit message to explain that if we cannot pick a commit because it would overwrite untracked files then we add the command back into the todo list and give control to the user until they say 'git rebase --continue'. Hopefully they'll have removed the problematic files and we try to pick the commit again it will succeed. We do the same if an exec command fails and --reschedule-failed-exec was given. For conflicts we don't add the command back into the todo list because the cherry-pick has happened the user "just" needs to fix the conflicts before continuing to the next command. >> Note that the rescheduled command will still be appended to the "done" >> file again when it is successfully executed. Arguably it would be better >> not to do that but fixing it would be more involved. > > And without quite understanding what "reschedule" refers to, it is > unclear why it is even arguable---it is perfectly sensible that a > command that is rescheduled (hence not yet done) would not be sent > to 'done'. If a command that was once rescheduled (hence it wasn't > finished initially) gets finished now, shouldn't it be sent to > 'done'? It is unclear why is it better not to. The command is only successfully executed once but may end up in 'done' multiple times. While that means we can see which commands ended up being rescheduled I'm not sure it is very useful and think really we're just cluttering the 'done' file with failed attempts. >> -static int save_todo(struct todo_list *todo_list, struct replay_opts *opts) >> +static int save_todo(struct todo_list *todo_list, struct replay_opts *opts, >> + int reschedule) >> { > > OK, all callers to save_todo() are in pick_commits() that knows what > the value of "reschedule" is, and it is passed down to this helper ... > >> @@ -3389,7 +3390,7 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts) >> * rebase -i writes "git-rebase-todo" without the currently executing >> * command, appending it to "done" instead. >> */ >> - if (is_rebase_i(opts)) >> + if (is_rebase_i(opts) && !reschedule) >> next++; >> >> fd = hold_lock_file_for_update(&todo_lock, todo_path, 0); >> @@ -3402,7 +3403,7 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts) >> if (commit_lock_file(&todo_lock) < 0) >> return error(_("failed to finalize '%s'"), todo_path); >> >> - if (is_rebase_i(opts) && next > 0) { >> + if (is_rebase_i(opts) && !reschedule && next > 0) { >> const char *done = rebase_path_done(); >> int fd = open(done, O_CREAT | O_WRONLY | O_APPEND, 0666); >> int ret = 0; > > ... and the change here is quite straight-forward. With reschedule, > we do not advance because by definition we haven't finished the > step yet. OK. > >> @@ -4648,7 +4649,7 @@ static int pick_commits(struct repository *r, >> const char *arg = todo_item_get_arg(todo_list, item); >> int check_todo = 0; >> >> - if (save_todo(todo_list, opts)) >> + if (save_todo(todo_list, opts, 0)) >> return -1; > > I wonder why we pass a hardcoded 0 here---shouldn't the value match > the local variable 'reschedule'? here? > > The same question for the other two callers, but I admit that when > the second one is called, the local variable "reschedule" is not > set... The rescheduling code is a bit of a mess as rescheduling commands that pick a commit does not use the "reschedule" variable and is handled separately to other commands like "reset", "merge" and "exec" which do use the "reschedule" varibale. I did try and add a preparatory step to fix that but failed to find a good way of doing so. The reason I went with hardcoded parameters is that for each call the purpose is fixed and as you noticed the "reschedule" variable is only used for rescheduling "reset", "merge" and "exec". I could expand the commit message or do you think a couple of code comments be more helpful? Best Wishes Phillip >> if (is_rebase_i(opts)) { >> if (item->command != TODO_COMMENT) { >> @@ -4695,8 +4696,7 @@ static int pick_commits(struct repository *r, >> todo_list->current), >> get_item_line(todo_list, >> todo_list->current)); >> - todo_list->current--; >> - if (save_todo(todo_list, opts)) >> + if (save_todo(todo_list, opts, 1)) >> return -1; > > ... yet we call the helper with reschedule set to 1. Puzzled. > >> @@ -4788,8 +4788,7 @@ static int pick_commits(struct repository *r, >> get_item_line_length(todo_list, >> todo_list->current), >> get_item_line(todo_list, todo_list->current)); >> - todo_list->current--; >> - if (save_todo(todo_list, opts)) >> + if (save_todo(todo_list, opts, 1)) >> return -1; > > At this point, reschedule is set and passing it instead of 1 would > be OK. > > Thanks.
Phillip Wood <phillip.wood123@gmail.com> writes: >>> Note that the rescheduled command will still be appended to the "done" >>> file again when it is successfully executed. Arguably it would be better >>> not to do that but fixing it would be more involved. >> And without quite understanding what "reschedule" refers to, it is >> unclear why it is even arguable---it is perfectly sensible that a >> command that is rescheduled (hence not yet done) would not be sent >> to 'done'. If a command that was once rescheduled (hence it wasn't >> finished initially) gets finished now, shouldn't it be sent to >> 'done'? It is unclear why is it better not to. > > The command is only successfully executed once but may end up in > 'done' multiple times. While that means we can see which commands > ended up being rescheduled I'm not sure it is very useful and think > really we're just cluttering the 'done' file with failed attempts. Sorry, but you utterly confused me. I thought the point of this change was to avoid such a failed attempt to be recorded in "done", and if that is the case, we (1) do not record any failing attempts, (2) record the successful execution, and (3) will not re-attempt once it is successful. And if all of these three hold, we wont clutter 'done' with failed attempts at all, no? >>> @@ -4648,7 +4649,7 @@ static int pick_commits(struct repository *r, >>> const char *arg = todo_item_get_arg(todo_list, item); >>> int check_todo = 0; >>> - if (save_todo(todo_list, opts)) >>> + if (save_todo(todo_list, opts, 0)) >>> return -1; >> I wonder why we pass a hardcoded 0 here---shouldn't the value match >> the local variable 'reschedule'? here? >> The same question for the other two callers, but I admit that when >> the second one is called, the local variable "reschedule" is not >> set... > > The rescheduling code is a bit of a mess as rescheduling commands that > pick a commit does not use the "reschedule" variable and is handled > separately to other commands like "reset", "merge" and "exec" which do > use the "reschedule" varibale. I did try and add a preparatory step to > fix that but failed to find a good way of doing so. I see. It may be a sign, taken together with the fact that I found that it was very hard---even after reading the patch twice---to understand the verb "reschedule" in the proposed commit log to explain the change, that the concept of "reschedule" in this codepath may not be clearly capturing what it is attempting to do in the first place. > The reason I went > with hardcoded parameters is that for each call the purpose is fixed > and as you noticed the "reschedule" variable is only used for > rescheduling "reset", "merge" and "exec". I could expand the commit > message or do you think a couple of code comments be more helpful? Yeah, at least it sounds like the code deserves a "NEEDSWORK: this is messy in such and such way and we need to clean it up to make it understandable" comment somehow. Thanks.
Hi Junio On 24/03/2023 15:49, Junio C Hamano wrote: > Phillip Wood <phillip.wood123@gmail.com> writes: > >>>> Note that the rescheduled command will still be appended to the "done" >>>> file again when it is successfully executed. Arguably it would be better >>>> not to do that but fixing it would be more involved. >>> And without quite understanding what "reschedule" refers to, it is >>> unclear why it is even arguable---it is perfectly sensible that a >>> command that is rescheduled (hence not yet done) would not be sent >>> to 'done'. If a command that was once rescheduled (hence it wasn't >>> finished initially) gets finished now, shouldn't it be sent to >>> 'done'? It is unclear why is it better not to. >> >> The command is only successfully executed once but may end up in >> 'done' multiple times. While that means we can see which commands >> ended up being rescheduled I'm not sure it is very useful and think >> really we're just cluttering the 'done' file with failed attempts. > > Sorry, but you utterly confused me. Perhaps I should have not have added that last paragraph to the commit message. > I thought the point of this > change was to avoid such a failed attempt to be recorded in "done", No. This change fixes a bug where when we add the failed command back into "git-rebase-todo" we accidentally add the previous command to the "done" file. If "pick A" succeeds and "pick B" fails because it would overwrite an untracked file then currently when the rebase stops after the failed "done" will contain pick A pick B pick A When it should contain pick A pick B i.e. the last line should be the last command we tried to execute. > and if that is the case, we (1) do not record any failing attempts, unfortunately "done" is updated just before we try and execute the command so all the failing attempts are recorded. I'm not trying to change that in this patch, I mentioned it in the commit message as a suggestion for a future improvement. > (2) record the successful execution, and (3) will not re-attempt > once it is successful. And if all of these three hold, we wont > clutter 'done' with failed attempts at all, no? Yes, unfortunately that's not how it works at the moment. >>>> @@ -4648,7 +4649,7 @@ static int pick_commits(struct repository *r, >>>> const char *arg = todo_item_get_arg(todo_list, item); >>>> int check_todo = 0; >>>> - if (save_todo(todo_list, opts)) >>>> + if (save_todo(todo_list, opts, 0)) >>>> return -1; >>> I wonder why we pass a hardcoded 0 here---shouldn't the value match >>> the local variable 'reschedule'? here? >>> The same question for the other two callers, but I admit that when >>> the second one is called, the local variable "reschedule" is not >>> set... >> >> The rescheduling code is a bit of a mess as rescheduling commands that >> pick a commit does not use the "reschedule" variable and is handled >> separately to other commands like "reset", "merge" and "exec" which do >> use the "reschedule" varibale. I did try and add a preparatory step to >> fix that but failed to find a good way of doing so. > > I see. It may be a sign, taken together with the fact that I found > that it was very hard---even after reading the patch twice---to > understand the verb "reschedule" in the proposed commit log to > explain the change, that the concept of "reschedule" in this > codepath may not be clearly capturing what it is attempting to do in > the first place. I'll try and come up with some better wording (if you have any suggestions please let me know). What's happening is that just before we try and execute a command it it removed from "git-rebase-todo" and added to "done". If the command then fails because it would overwrite an untracked file we need to add it back into "git-rebase-todo" before handing control to the user to remove the offending files. When the user runs "git rebase --continue" we'll try and execute the command again. It is the adding the command back into "git-rebase-todo" so that it is executed by "git rebase --continue" that "reschedule" was intended to capture. The basic problem is that rebase updates "git-rebase-todo" and "done" before it has successfully executed the command (cherry-pick and revert only remove a command from their todo list after it is executed successfully). I fear it may be too late to change that now though. >> The reason I went >> with hardcoded parameters is that for each call the purpose is fixed >> and as you noticed the "reschedule" variable is only used for >> rescheduling "reset", "merge" and "exec". I could expand the commit >> message or do you think a couple of code comments be more helpful? > > Yeah, at least it sounds like the code deserves a "NEEDSWORK: this > is messy in such and such way and we need to clean it up to make it > understandable" comment somehow. I'll have another think about how we could clean it up, if that fails I'll add a code comment. I'll be offline next week so I'll re-roll after that. Best Wishes Phillip > Thanks.
Hi Phillip, On Sun, 19 Mar 2023, Phillip Wood via GitGitGadget wrote: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > As the sequencer executes todo commands it appends them to > .git/rebase-merge/done. This file is used by "git status" to show the > recently executed commands. Unfortunately when a command is rescheduled > the command preceding it is erroneously appended to the "done" file. > This means that when rebase stops after rescheduling "pick B" the "done" > file contains > > pick A > pick B > pick A > > instead of > > pick A > pick B > > Fix this by not updating the "done" file when adding a rescheduled > command back into the "todo" file. A couple of the existing tests are > modified to improve their coverage as none of them trigger this bug or > check the "done" file. I am purposefully not yet focusing on the patch, as I have a concern about the reasoning above. When a command fails that needs to be rescheduled, I actually _like_ that there is a record in `done` about said command. It is very much like a `pick` that failed (but was not rescheduled) and was then `--skip`ed: it still shows up on `done`. I do understand the concern that the rescheduled command now shows up in both `done` and `git-rebase-todo` (which is very different from the failed `pick` command that would show up _only_ in `git-rebase-todo`). So maybe we can find some compromise, e.g. adding a commented-out line to `done` à la: # rescheduled: pick A What do you think? Ciao, Johannes
Hi Dscho Sorry for not replying sooner. On 27/03/2023 08:04, Johannes Schindelin wrote: > Hi Phillip, > > On Sun, 19 Mar 2023, Phillip Wood via GitGitGadget wrote: > >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> As the sequencer executes todo commands it appends them to >> .git/rebase-merge/done. This file is used by "git status" to show the >> recently executed commands. Unfortunately when a command is rescheduled >> the command preceding it is erroneously appended to the "done" file. >> This means that when rebase stops after rescheduling "pick B" the "done" >> file contains >> >> pick A >> pick B >> pick A >> >> instead of >> >> pick A >> pick B >> >> Fix this by not updating the "done" file when adding a rescheduled >> command back into the "todo" file. A couple of the existing tests are >> modified to improve their coverage as none of them trigger this bug or >> check the "done" file. > > I am purposefully not yet focusing on the patch, as I have a concern about > the reasoning above. > > When a command fails that needs to be rescheduled, I actually _like_ that > there is a record in `done` about said command. It is very much like a > `pick` that failed (but was not rescheduled) and was then `--skip`ed: it > still shows up on `done`. We still do that after this patch. What changes is that when "pick B" fails we don't add "pick A" to the "done" file when "pick B" is added back into "git-rebase-todo" > I do understand the concern that the rescheduled command now shows up in > both `done` and `git-rebase-todo` (which is very different from the failed > `pick` command that would show up _only_ in `git-rebase-todo`). So maybe > we can find some compromise, e.g. adding a commented-out line to `done` à > la: > > # rescheduled: pick A > > What do you think? If a commit is rescheduled we still end up with multiple entries in the "done". In the example above if "pick B" fails the first time it is executed and then succeeds on the second attempt "done" will contain pick A pick B pick B It might be nice to mark it as rescheduled as you suggest but this series focuses on removing the incorrect entry from the "done" file, not de-duplicating the "done" entities when a command fails. Best Wishes Phillip > Ciao, > Johannes
Hi Phillip, On Thu, 3 Aug 2023, Phillip Wood wrote: > On 27/03/2023 08:04, Johannes Schindelin wrote: > > > > On Sun, 19 Mar 2023, Phillip Wood via GitGitGadget wrote: > > > > > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > > > > > As the sequencer executes todo commands it appends them to > > > .git/rebase-merge/done. This file is used by "git status" to show the > > > recently executed commands. Unfortunately when a command is rescheduled > > > the command preceding it is erroneously appended to the "done" file. > > > This means that when rebase stops after rescheduling "pick B" the "done" > > > file contains > > > > > > pick A > > > pick B > > > pick A > > > > > > instead of > > > > > > pick A > > > pick B > > > > > > Fix this by not updating the "done" file when adding a rescheduled > > > command back into the "todo" file. A couple of the existing tests are > > > modified to improve their coverage as none of them trigger this bug or > > > check the "done" file. > > > > I am purposefully not yet focusing on the patch, as I have a concern about > > the reasoning above. > > > > When a command fails that needs to be rescheduled, I actually _like_ that > > there is a record in `done` about said command. It is very much like a > > `pick` that failed (but was not rescheduled) and was then `--skip`ed: it > > still shows up on `done`. > > We still do that after this patch. What changes is that when "pick B" fails we > don't add "pick A" to the "done" file when "pick B" is added back into > "git-rebase-todo" > > > I do understand the concern that the rescheduled command now shows up in > > both `done` and `git-rebase-todo` (which is very different from the failed > > `pick` command that would show up _only_ in `git-rebase-todo`). So maybe > > we can find some compromise, e.g. adding a commented-out line to `done` à > > la: > > > > # rescheduled: pick A > > > > What do you think? > > If a commit is rescheduled we still end up with multiple entries in the > "done". In the example above if "pick B" fails the first time it is executed > and then succeeds on the second attempt "done" will contain > > pick A > pick B > pick B > > It might be nice to mark it as rescheduled as you suggest but this series > focuses on removing the incorrect entry from the "done" file, not > de-duplicating the "done" entities when a command fails. After having had plenty of time to let this issue simmer in the back of my head, and after reviewing the latest iteration of the patch series, I am no longer concerned, as it now sounds more logical to me, too, that rescheduled commands don't show up in `done`. Thank you, Johannes
diff --git a/sequencer.c b/sequencer.c index 1c96a75b1e9..87eeda52595 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3379,7 +3379,8 @@ give_advice: return -1; } -static int save_todo(struct todo_list *todo_list, struct replay_opts *opts) +static int save_todo(struct todo_list *todo_list, struct replay_opts *opts, + int reschedule) { struct lock_file todo_lock = LOCK_INIT; const char *todo_path = get_todo_path(opts); @@ -3389,7 +3390,7 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts) * rebase -i writes "git-rebase-todo" without the currently executing * command, appending it to "done" instead. */ - if (is_rebase_i(opts)) + if (is_rebase_i(opts) && !reschedule) next++; fd = hold_lock_file_for_update(&todo_lock, todo_path, 0); @@ -3402,7 +3403,7 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts) if (commit_lock_file(&todo_lock) < 0) return error(_("failed to finalize '%s'"), todo_path); - if (is_rebase_i(opts) && next > 0) { + if (is_rebase_i(opts) && !reschedule && next > 0) { const char *done = rebase_path_done(); int fd = open(done, O_CREAT | O_WRONLY | O_APPEND, 0666); int ret = 0; @@ -4648,7 +4649,7 @@ static int pick_commits(struct repository *r, const char *arg = todo_item_get_arg(todo_list, item); int check_todo = 0; - if (save_todo(todo_list, opts)) + if (save_todo(todo_list, opts, 0)) return -1; if (is_rebase_i(opts)) { if (item->command != TODO_COMMENT) { @@ -4695,8 +4696,7 @@ static int pick_commits(struct repository *r, todo_list->current), get_item_line(todo_list, todo_list->current)); - todo_list->current--; - if (save_todo(todo_list, opts)) + if (save_todo(todo_list, opts, 1)) return -1; } if (item->command == TODO_EDIT) { @@ -4788,8 +4788,7 @@ static int pick_commits(struct repository *r, get_item_line_length(todo_list, todo_list->current), get_item_line(todo_list, todo_list->current)); - todo_list->current--; - if (save_todo(todo_list, opts)) + if (save_todo(todo_list, opts, 1)) return -1; if (item->commit) return error_with_patch(r, diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index ff0afad63e2..39c1ce51f69 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1276,20 +1276,27 @@ test_expect_success 'todo count' ' ' test_expect_success 'rebase -i commits that overwrite untracked files (pick)' ' - git checkout --force branch2 && + git checkout --force A && git clean -f && + cat >todo <<-EOF && + exec >file2 + pick $(git rev-parse B) B + pick $(git rev-parse C) C + pick $(git rev-parse D) D + exec cat .git/rebase-merge/done >actual + EOF ( - set_fake_editor && - FAKE_LINES="edit 1 2" git rebase -i A + set_replace_editor todo && + test_must_fail git rebase -i A ) && - test_cmp_rev HEAD F && - test_path_is_missing file6 && - >file6 && - test_must_fail git rebase --continue && - test_cmp_rev HEAD F && - rm file6 && + test_cmp_rev HEAD B && + head -n3 todo >expect && + test_cmp expect .git/rebase-merge/done && + rm file2 && git rebase --continue && - test_cmp_rev HEAD I + test_cmp_rev HEAD D && + tail -n3 todo >>expect && + test_cmp expect actual ' test_expect_success 'rebase -i commits that overwrite untracked files (squash)' ' diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index fa2a06c19f0..2ad1f9504da 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -128,14 +128,24 @@ test_expect_success 'generate correct todo list' ' ' test_expect_success '`reset` refuses to overwrite untracked files' ' - git checkout -b refuse-to-reset && + git checkout B && test_commit dont-overwrite-untracked && - git checkout @{-1} && - : >dont-overwrite-untracked.t && - echo "reset refs/tags/dont-overwrite-untracked" >script-from-scratch && + cat >script-from-scratch <<-EOF && + exec >dont-overwrite-untracked.t + pick $(git rev-parse B) B + reset refs/tags/dont-overwrite-untracked + pick $(git rev-parse C) C + exec cat .git/rebase-merge/done >actual + EOF test_config sequence.editor \""$PWD"/replace-editor.sh\" && - test_must_fail git rebase -ir HEAD && - git rebase --abort + test_must_fail git rebase -ir A && + test_cmp_rev HEAD B && + head -n3 script-from-scratch >expect && + test_cmp expect .git/rebase-merge/done && + rm dont-overwrite-untracked.t && + git rebase --continue && + tail -n3 script-from-scratch >>expect && + test_cmp expect actual ' test_expect_success '`reset` rejects trees' '