* git-rebase --undo-skip proposal @ 2018-02-13 20:22 Psidium Guajava 2018-02-13 20:42 ` Stefan Beller 0 siblings, 1 reply; 10+ messages in thread From: Psidium Guajava @ 2018-02-13 20:22 UTC (permalink / raw) To: git Hello git community, I'd like to add a new feature in git-rebase: --undo-skip. But first, I'd like to consult with the experts if it would be beneficial for the project and if my line of tought is correct. Imagine that you are working on a feature for a long time, but there are multiple bug fixes happening at `master` branch at the same time. After lots of commits on both ends, you decide to rebase your changes on top of the current `master` branch. After lots of conflict resolution steps, you mistakenly call `git-rebase --skip` instead of `git-rebase --continue`, thus losing a commit of your work, and possibly inserting bugs in your project. The only solution for this right now would be to abort the current rebase and start over. It seems that a feature like this have been requested once on the mail list [1]. I propose the existence of --undo-skip on git-rebase's `$action` domain. How I fixed it when that happened with me was (just after running the wrong skip): 1. I figured I was making a rebase that used `git-am` as a backend. 2. In the rebase-apply directory I searched for the patch file with the change I just skipped. 3. Found the `rebase-apply/next` file. 4. Wrote the number of the patch I skipped - 1 in rebase-apply/next. 5. run `git rebase --skip` again on the repository. This made the lost patch appear again and I could `--continue` it this time. I propose the addition of an action `--undo-skip`, that could be called only after a wrongfully called `--skip`. `git rebase --undo-skip`. I would implemented it to do programatically what I did by hand when that happened with me. Here are my questions for you: 1. Would this be beneficial for the users? 2. For `rebase--am`, I would need to change `git-rebase--am.sh` file, correct? 3. Can I assume `builtin/am.c` will always store its information on `$state_dir/next` and `$state_dir/$patchnumbers`? 4. How hard would it be to add that logic for `rebase--interactive` and `rebase--merge` backends? Also, a little unrelated with this issue: 5. What happened to the rewrite of rebase in C [2]? I couldn't find any information after 2016. [1] https://public-inbox.org/git/201311011522.44631.thomas@koch.ro/ [2] https://public-inbox.org/git/1457779597-6918-1-git-send-email-pyokagan@gmail.com/ Best Regards, Gabriel Borges ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-rebase --undo-skip proposal 2018-02-13 20:22 git-rebase --undo-skip proposal Psidium Guajava @ 2018-02-13 20:42 ` Stefan Beller 2018-02-15 0:36 ` Psidium Guajava 2018-02-15 10:11 ` Paul Tan 0 siblings, 2 replies; 10+ messages in thread From: Stefan Beller @ 2018-02-13 20:42 UTC (permalink / raw) To: Psidium Guajava, Paul Tan, Johannes Schindelin; +Cc: git On Tue, Feb 13, 2018 at 12:22 PM, Psidium Guajava <psiidium@gmail.com> wrote: > Hello git community, > > I'd like to add a new feature in git-rebase: --undo-skip. > But first, I'd like to consult with the experts if it would be > beneficial for the project and if my line of tought is correct. > > Imagine that you are working on a feature for a long time, but there > are multiple bug fixes happening at `master` branch at the same time. > After lots of commits on both ends, you decide to rebase your changes > on top of the current `master` branch. > After lots of conflict resolution steps, you mistakenly call > `git-rebase --skip` instead of `git-rebase --continue`, thus losing a > commit of your work, and possibly inserting bugs in your project. > The only solution for this right now would be to abort the current > rebase and start over. > > It seems that a feature like this have been requested once on the mail list [1]. > > I propose the existence of --undo-skip on git-rebase's `$action` domain. > > How I fixed it when that happened with me was (just after running the > wrong skip): > > 1. I figured I was making a rebase that used `git-am` as a backend. > 2. In the rebase-apply directory I searched for the patch file with > the change I just skipped. > 3. Found the `rebase-apply/next` file. > 4. Wrote the number of the patch I skipped - 1 in rebase-apply/next. > 5. run `git rebase --skip` again on the repository. > > This made the lost patch appear again and I could `--continue` it this time. I think this could also be done with "git rebase --edit-todo", which brings up the right file in your editor. > I propose the addition of an action `--undo-skip`, that could be > called only after a wrongfully called `--skip`. > `git rebase --undo-skip`. > I would implemented it to do programatically what I did by hand when > that happened with me. > > Here are my questions for you: > 1. Would this be beneficial for the users? I guess it is. > 2. For `rebase--am`, I would need to change `git-rebase--am.sh` file, correct? > 3. Can I assume `builtin/am.c` will always store its information on > `$state_dir/next` and `$state_dir/$patchnumbers`? > 4. How hard would it be to add that logic for `rebase--interactive` > and `rebase--merge` backends? cc'd Johannes who is currently working on revamping rebase. > > Also, a little unrelated with this issue: > 5. What happened to the rewrite of rebase in C [2]? I couldn't find > any information after 2016. > > [1] https://public-inbox.org/git/201311011522.44631.thomas@koch.ro/ > [2] https://public-inbox.org/git/1457779597-6918-1-git-send-email-pyokagan@gmail.com/ cc'd Paul Tan, maybe he recalls the situation. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-rebase --undo-skip proposal 2018-02-13 20:42 ` Stefan Beller @ 2018-02-15 0:36 ` Psidium Guajava 2018-02-15 0:53 ` Johannes Schindelin 2018-02-15 10:11 ` Paul Tan 1 sibling, 1 reply; 10+ messages in thread From: Psidium Guajava @ 2018-02-15 0:36 UTC (permalink / raw) To: Stefan Beller; +Cc: Paul Tan, Johannes Schindelin, git On 2018-02-13 18:42 GMT-02:00 Stefan Beller <sbeller@google.com> wrote: > On Tue, Feb 13, 2018 at 12:22 PM, Psidium Guajava <psiidium@gmail.com> wrote: > I think this could also be done with "git rebase --edit-todo", which brings > up the right file in your editor. Yeah that'd would only work if one started a rebase as a interactive one, not am or merge. > cc'd Paul Tan, maybe he recalls the situation. From what I've found on the archive, he didn't recently answer mails that come from the mail list, I could be wrong tho. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-rebase --undo-skip proposal 2018-02-15 0:36 ` Psidium Guajava @ 2018-02-15 0:53 ` Johannes Schindelin 2018-02-15 1:23 ` Jacob Keller 2018-02-15 1:50 ` Psidium Guajava 0 siblings, 2 replies; 10+ messages in thread From: Johannes Schindelin @ 2018-02-15 0:53 UTC (permalink / raw) To: Psidium Guajava; +Cc: Stefan Beller, Paul Tan, git Hi, On Wed, 14 Feb 2018, Psidium Guajava wrote: > On 2018-02-13 18:42 GMT-02:00 Stefan Beller <sbeller@google.com> wrote: > > On Tue, Feb 13, 2018 at 12:22 PM, Psidium Guajava <psiidium@gmail.com> wrote: > > I think this could also be done with "git rebase --edit-todo", which brings > > up the right file in your editor. > > Yeah that'd would only work if one started a rebase as a interactive > one, not am or merge. I agree that the original proposal was clearly for the non-interactive rebase (it talked about .git/rebase-apply/). The biggest problem with the feature request is not how useful it would be: I agree it would be useful. The biggest problem is that it is a little bit of an ill-defined problem. Imagine that you are rebasing 30 patches. Now, let's assume that patch #7 causes a merge conflict, and you mistakenly call `git rebase --skip`. Now, when is the next possible time you can call `git rebase --undo-skip`? It could be after a merge conflict in #8. Or in interactive rebase, after a `pick` that was turned into an `edit`. Or, and this is also entirely possible, after the rebase finished with #30 without problems and the rebase is actually no longer in progress. So I do not think that there is a way, in general, to implement this feature. Even if you try to remember the state where a `--skip` was called, you would remember it in the .git/rebase-apply/ (or .git/rebase-merge/) directory, which is cleaned up after the rebase concluded successfully. So even then the information required to implement the feature would not necessarily be there, still, when it would be needed. Ciao, Johannes ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-rebase --undo-skip proposal 2018-02-15 0:53 ` Johannes Schindelin @ 2018-02-15 1:23 ` Jacob Keller 2018-02-15 13:02 ` Johannes Schindelin 2018-02-15 1:50 ` Psidium Guajava 1 sibling, 1 reply; 10+ messages in thread From: Jacob Keller @ 2018-02-15 1:23 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Psidium Guajava, Stefan Beller, Paul Tan, git On Wed, Feb 14, 2018 at 4:53 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > Hi, > > On Wed, 14 Feb 2018, Psidium Guajava wrote: > >> On 2018-02-13 18:42 GMT-02:00 Stefan Beller <sbeller@google.com> wrote: >> > On Tue, Feb 13, 2018 at 12:22 PM, Psidium Guajava <psiidium@gmail.com> wrote: >> > I think this could also be done with "git rebase --edit-todo", which brings >> > up the right file in your editor. >> >> Yeah that'd would only work if one started a rebase as a interactive >> one, not am or merge. > > I agree that the original proposal was clearly for the non-interactive > rebase (it talked about .git/rebase-apply/). > > The biggest problem with the feature request is not how useful it would > be: I agree it would be useful. The biggest problem is that it is a little > bit of an ill-defined problem. > > Imagine that you are rebasing 30 patches. Now, let's assume that patch #7 > causes a merge conflict, and you mistakenly call `git rebase --skip`. > > Now, when is the next possible time you can call `git rebase --undo-skip`? > It could be after a merge conflict in #8. Or in interactive rebase, after > a `pick` that was turned into an `edit`. Or, and this is also entirely > possible, after the rebase finished with #30 without problems and the > rebase is actually no longer in progress. > > So I do not think that there is a way, in general, to implement this > feature. Even if you try to remember the state where a `--skip` was > called, you would remember it in the .git/rebase-apply/ (or > .git/rebase-merge/) directory, which is cleaned up after the rebase > concluded successfully. So even then the information required to implement > the feature would not necessarily be there, still, when it would be needed. > > Ciao, > Johannes Instead of an "--undo-skip", what if we ask the question of what the user actually wants? Generally I'd assume that the user wishes to go back to the rebase and "pick" the commit back in. So what if we just make "git rebase --skip" more verbose so that it more clearly spells out which commit is being skipped? Possibly even as extra lines of "the following patches were skipped during the rebase" after it completes? Then it's up to the user to determine what to do with those commits, and there are many tools they could use to solve it, "git rebase -i, git cherry-pick, git reflog to restore to a previous and re-run the rebase, etc". Thanks, Jake ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-rebase --undo-skip proposal 2018-02-15 1:23 ` Jacob Keller @ 2018-02-15 13:02 ` Johannes Schindelin 0 siblings, 0 replies; 10+ messages in thread From: Johannes Schindelin @ 2018-02-15 13:02 UTC (permalink / raw) To: Jacob Keller; +Cc: Psidium Guajava, Stefan Beller, Paul Tan, git Hi Jake, On Wed, 14 Feb 2018, Jacob Keller wrote: > On Wed, Feb 14, 2018 at 4:53 PM, Johannes Schindelin > <Johannes.Schindelin@gmx.de> wrote: > > > > On Wed, 14 Feb 2018, Psidium Guajava wrote: > > > >> On 2018-02-13 18:42 GMT-02:00 Stefan Beller <sbeller@google.com> wrote: > >> > On Tue, Feb 13, 2018 at 12:22 PM, Psidium Guajava > >> > <psiidium@gmail.com> wrote: I think this could also be done with > >> > "git rebase --edit-todo", which brings up the right file in your > >> > editor. > >> > >> Yeah that'd would only work if one started a rebase as a interactive > >> one, not am or merge. > > > > I agree that the original proposal was clearly for the non-interactive > > rebase (it talked about .git/rebase-apply/). > > > > The biggest problem with the feature request is not how useful it > > would be: I agree it would be useful. The biggest problem is that it > > is a little bit of an ill-defined problem. > > > > Imagine that you are rebasing 30 patches. Now, let's assume that patch > > #7 causes a merge conflict, and you mistakenly call `git rebase > > --skip`. > > > > Now, when is the next possible time you can call `git rebase > > --undo-skip`? It could be after a merge conflict in #8. Or in > > interactive rebase, after a `pick` that was turned into an `edit`. Or, > > and this is also entirely possible, after the rebase finished with #30 > > without problems and the rebase is actually no longer in progress. > > > > So I do not think that there is a way, in general, to implement this > > feature. Even if you try to remember the state where a `--skip` was > > called, you would remember it in the .git/rebase-apply/ (or > > .git/rebase-merge/) directory, which is cleaned up after the rebase > > concluded successfully. So even then the information required to > > implement the feature would not necessarily be there, still, when it > > would be needed. > > Instead of an "--undo-skip", what if we ask the question of what the > user actually wants? Heh, I thought in this instance, --undo-skip was what the user wanted ;-) > Generally I'd assume that the user wishes to go back to the rebase and > "pick" the commit back in. Right, and then replay whatever series of commits was picked since the one that was skipped. What *could* be done is to save a copy of the current todo list (with the skipped commit put back in, before the current first line) and save that together with `git rev-parse HEAD`. This should make it possible to implement `--undo-skip` for as long as the rebase did not finish. Theoretically, you could even save the commit name of the skipped commit somewhere else than $GIT_DIR/rebase-apply/ (or $GIT_DIR/rebase-merge/), say, as worktree-local `refs/rebase/skipped`, and then a `git rebase --undo-skip` could detect the absence of $GIT_DIR/rebase-*/ and fall back to `git cherry-pick refs/rebase/skipped`. You'd have to take pains to handle that ref in gc, and to record when the user edited the todo list via `git rebase --edit-todo` after skipping that commit (and warn loudly upon/prevent --undo-skip) because those todo list changes would then be lost. That's just one way how this feature could be implemented. It does strike me as awfully specific, though. And it would still only extend to the latest `git rebase --skip`. So I am not sure whether we really would want to go this direction, or whether we can maybe come up with something (probably based on your suggestion to give the user enough information) that would allow many more scenarios than just --undo-skip. > So what if we just make "git rebase --skip" more verbose so that it > more clearly spells out which commit is being skipped? Possibly even > as extra lines of "the following patches were skipped during the > rebase" after it completes? > > Then it's up to the user to determine what to do with those commits, > and there are many tools they could use to solve it, "git rebase -i, > git cherry-pick, git reflog to restore to a previous and re-run the > rebase, etc". I think this is a saner direction, as it will probably allow more scenarios to be addressed than just undoing the latest `git rebase --skip`. Ciao, Dscho ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-rebase --undo-skip proposal 2018-02-15 0:53 ` Johannes Schindelin 2018-02-15 1:23 ` Jacob Keller @ 2018-02-15 1:50 ` Psidium Guajava 2018-02-15 4:50 ` Jacob Keller 1 sibling, 1 reply; 10+ messages in thread From: Psidium Guajava @ 2018-02-15 1:50 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Stefan Beller, Paul Tan, git 2018-02-14 22:53 GMT-02:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>: > Now, when is the next possible time you can call `git rebase --undo-skip`? What if the scope were reduced from `--undo-skip` to `--undo-last-skip`? Also, further reduce the scope to only allow `--undo-last-skip` during a ongoing rebase, not caring about a finished one? But, this could be so niche that I have doubts if this would ever be used; ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-rebase --undo-skip proposal 2018-02-15 1:50 ` Psidium Guajava @ 2018-02-15 4:50 ` Jacob Keller 2018-02-15 13:06 ` Johannes Schindelin 0 siblings, 1 reply; 10+ messages in thread From: Jacob Keller @ 2018-02-15 4:50 UTC (permalink / raw) To: Psidium Guajava; +Cc: Johannes Schindelin, Stefan Beller, Paul Tan, git On Wed, Feb 14, 2018 at 5:50 PM, Psidium Guajava <psiidium@gmail.com> wrote: > 2018-02-14 22:53 GMT-02:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>: >> Now, when is the next possible time you can call `git rebase --undo-skip`? > > What if the scope were reduced from `--undo-skip` to `--undo-last-skip`? > Also, further reduce the scope to only allow `--undo-last-skip` during > a ongoing rebase, not caring about a finished one? > > But, this could be so niche that I have doubts if this would ever be used; I think this is too niche to actually be useful in practice, especially since figuring out exactly what to replay might be tricky.. I suppose it could keep track of where in the rebase the last skip was used, and then just go back to rebase and stop there again? That sounds like just redoing the rebase is easier.. (ie: use the reflog to go back to before the rebase started, and then re-run it again and don't skip this time). ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-rebase --undo-skip proposal 2018-02-15 4:50 ` Jacob Keller @ 2018-02-15 13:06 ` Johannes Schindelin 0 siblings, 0 replies; 10+ messages in thread From: Johannes Schindelin @ 2018-02-15 13:06 UTC (permalink / raw) To: Jacob Keller; +Cc: Psidium Guajava, Stefan Beller, Paul Tan, git Hi Jake, On Wed, 14 Feb 2018, Jacob Keller wrote: > On Wed, Feb 14, 2018 at 5:50 PM, Psidium Guajava <psiidium@gmail.com> wrote: > > 2018-02-14 22:53 GMT-02:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>: > >> Now, when is the next possible time you can call `git rebase --undo-skip`? > > > > What if the scope were reduced from `--undo-skip` to `--undo-last-skip`? > > Also, further reduce the scope to only allow `--undo-last-skip` during > > a ongoing rebase, not caring about a finished one? > > > > But, this could be so niche that I have doubts if this would ever be used; > > I think this is too niche to actually be useful in practice, > especially since figuring out exactly what to replay might be tricky.. > I suppose it could keep track of where in the rebase the last skip was > used, and then just go back to rebase and stop there again? That > sounds like just redoing the rebase is easier.. (ie: use the reflog to > go back to before the rebase started, and then re-run it again and > don't skip this time). Yes, and having a record of what commits were skipped would probably be very helpful not only in this instance. Maybe also a record of what commits caused merge conflicts, which a mapping between original and new commit (which does get a bit tricky with fixup!/squash! commits, though). Ciao, Dscho ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: git-rebase --undo-skip proposal 2018-02-13 20:42 ` Stefan Beller 2018-02-15 0:36 ` Psidium Guajava @ 2018-02-15 10:11 ` Paul Tan 1 sibling, 0 replies; 10+ messages in thread From: Paul Tan @ 2018-02-15 10:11 UTC (permalink / raw) To: Psidium Guajava; +Cc: Stefan Beller, Johannes Schindelin, git, jacob.keller Hi Gabriel, On Wed, Feb 14, 2018 at 4:42 AM, Stefan Beller <sbeller@google.com> wrote: > On Tue, Feb 13, 2018 at 12:22 PM, Psidium Guajava <psiidium@gmail.com> wrote: >> >> Also, a little unrelated with this issue: >> 5. What happened to the rewrite of rebase in C [2]? I couldn't find >> any information after 2016. >> >> [1] https://public-inbox.org/git/201311011522.44631.thomas@koch.ro/ >> [2] https://public-inbox.org/git/1457779597-6918-1-git-send-email-pyokagan@gmail.com/ > > cc'd Paul Tan, maybe he recalls the situation. It was discarded in favor of Johannes' rebase-helper approach, and I think parts of it are already in master. There's probably room for help there. I haven't had time to keep track of Git development, hence my inactivity. Sorry about that. Regards, Paul ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-02-15 13:06 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-02-13 20:22 git-rebase --undo-skip proposal Psidium Guajava 2018-02-13 20:42 ` Stefan Beller 2018-02-15 0:36 ` Psidium Guajava 2018-02-15 0:53 ` Johannes Schindelin 2018-02-15 1:23 ` Jacob Keller 2018-02-15 13:02 ` Johannes Schindelin 2018-02-15 1:50 ` Psidium Guajava 2018-02-15 4:50 ` Jacob Keller 2018-02-15 13:06 ` Johannes Schindelin 2018-02-15 10:11 ` Paul Tan
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).