Mike HommeyHi,
I use git-rebase quite regularly, and I haven't used git-rebase --skip after a failed merge without first resetting the working tree. I was wondering if it wouldn't make sense to automatically do the reset when running git-rebase --skip.
Mike
Johannes SchindelinRe: git rebase --skip
Hi,
On Wed, 7 Nov 2007, Mike Hommey wrote:
> I use git-rebase quite regularly, and I haven't used git-rebase --skip > after a failed merge without first resetting the working tree. I was > wondering if it wouldn't make sense to automatically do the reset when > running git-rebase --skip.
Makes sense to me. The user is already required to pass "--skip" to say "scrap this patch".
FWIW --interactive already has this behaviour since it was added to "official" git.
Ciao, Dscho
Jeff KingRe: git rebase --skip
On Wed, Nov 07, 2007 at 11:21:05PM +0100, Mike Hommey wrote:
> I use git-rebase quite regularly, and I haven't used git-rebase --skip > after a failed merge without first resetting the working tree. I was > wondering if it wouldn't make sense to automatically do the reset when > running git-rebase --skip.
I have often been annoyed by this behavior, too, and I can't think of any situation where you _wouldn't_ want the reset to happen. But I would be more comfortable hearing confirmation from others that they can't think of such a situation.
-Peff
Jakub NarebskiRe: git rebase --skip
Jeff King wrote:
> On Wed, Nov 07, 2007 at 11:21:05PM +0100, Mike Hommey wrote: > >> I use git-rebase quite regularly, and I haven't used git-rebase --skip >> after a failed merge without first resetting the working tree. I was >> wondering if it wouldn't make sense to automatically do the reset when >> running git-rebase --skip. > > I have often been annoyed by this behavior, too, and I can't think of > any situation where you _wouldn't_ want the reset to happen. But I > would be more comfortable hearing confirmation from others that they > can't think of such a situation.
Perhaps "git rebase --force-skip" or "git rebase --force --skip" would be the way to fo above...
-- Jakub Narebski Warsaw, Poland ShadeHawk on #git
Björn SteinbrinkRe: git rebase --skip
On 2007.11.07 22:23:10 -0500, Jeff King wrote:
> On Wed, Nov 07, 2007 at 11:21:05PM +0100, Mike Hommey wrote: > > > I use git-rebase quite regularly, and I haven't used git-rebase --skip > > after a failed merge without first resetting the working tree. I was > > wondering if it wouldn't make sense to automatically do the reset when > > running git-rebase --skip. > > I have often been annoyed by this behavior, too, and I can't think of > any situation where you _wouldn't_ want the reset to happen. But I > would be more comfortable hearing confirmation from others that they > can't think of such a situation.
Let's take this history:
C---D---E topic
/
A---B masterYou then do: git rebase master topic
Now D causes conflicts, because B did a similar change, but not quite the same, maybe having a bug. So you want to keep parts of D, but it's no longer the same commit semantically and the original commit message would be bogus. So you resolve the conflicts and do:
git commit git rebase --skip
Because you replaced D with a new different commit, instead of really just rebasing it. With plain --continue, you'd have to go back and fix the commit message once the rebase is complete. And --continue after the commit is a no-go, too, because rebase will complain that there's nothing left to produce the rebased D commit.
And now imagine that you forget to commit but instead just --skip. Ouch, all the work is lost, time to restart the rebase. With the current behaviour, rebase won't just throw away your stuff.
Björn
Andreas EricssonRe: git rebase --skip
Björn Steinbrink wrote:
> On 2007.11.07 22:23:10 -0500, Jeff King wrote: >> On Wed, Nov 07, 2007 at 11:21:05PM +0100, Mike Hommey wrote: >> >>> I use git-rebase quite regularly, and I haven't used git-rebase --skip >>> after a failed merge without first resetting the working tree. I was >>> wondering if it wouldn't make sense to automatically do the reset when >>> running git-rebase --skip. >> I have often been annoyed by this behavior, too, and I can't think of >> any situation where you _wouldn't_ want the reset to happen. But I >> would be more comfortable hearing confirmation from others that they >> can't think of such a situation. > > Let's take this history: > > C---D---E topic > / > A---B master > > You then do: > git rebase master topic > > Now D causes conflicts, because B did a similar change, but not quite > the same, maybe having a bug. So you want to keep parts of D, but it's > no longer the same commit semantically and the original commit message > would be bogus. So you resolve the conflicts and do: > > git commit > git rebase --skip > > Because you replaced D with a new different commit, instead of really > just rebasing it. With plain --continue, you'd have to go back and fix > the commit message once the rebase is complete. And --continue after the > commit is a no-go, too, because rebase will complain that there's > nothing left to produce the rebased D commit. > > And now imagine that you forget to commit but instead just --skip. > Ouch, all the work is lost, time to restart the rebase. With the current > behaviour, rebase won't just throw away your stuff. >
How about if the state to skip was stashed, the patch reapplied and the differences compared. If they were identical, go ahead and force the reset --hard, otherwise abort. That way, --skip will dwim only when it's safe, and all the lost work can be automagically created by just re-applying the patch again?
-- Andreas Ericsson andreas.ericsson@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231
Björn SteinbrinkRe: git rebase --skip
On 2007.11.08 11:32:08 +0100, Andreas Ericsson wrote:
> Björn Steinbrink wrote: >> On 2007.11.07 22:23:10 -0500, Jeff King wrote: >>> On Wed, Nov 07, 2007 at 11:21:05PM +0100, Mike Hommey wrote: >>> >>>> I use git-rebase quite regularly, and I haven't used git-rebase --skip >>>> after a failed merge without first resetting the working tree. I was >>>> wondering if it wouldn't make sense to automatically do the reset when >>>> running git-rebase --skip. >>> I have often been annoyed by this behavior, too, and I can't think of >>> any situation where you _wouldn't_ want the reset to happen. But I >>> would be more comfortable hearing confirmation from others that they >>> can't think of such a situation. >> >> Let's take this history: >> >> C---D---E topic >> / >> A---B master >> >> You then do: >> git rebase master topic >> >> Now D causes conflicts, because B did a similar change, but not quite >> the same, maybe having a bug. So you want to keep parts of D, but it's >> no longer the same commit semantically and the original commit message >> would be bogus. So you resolve the conflicts and do: >> >> git commit >> git rebase --skip >> >> Because you replaced D with a new different commit, instead of really >> just rebasing it. With plain --continue, you'd have to go back and fix >> the commit message once the rebase is complete. And --continue after the >> commit is a no-go, too, because rebase will complain that there's >> nothing left to produce the rebased D commit. >> >> And now imagine that you forget to commit but instead just --skip. >> Ouch, all the work is lost, time to restart the rebase. With the current >> behaviour, rebase won't just throw away your stuff. >> > > How about if the state to skip was stashed, the patch reapplied and the > differences compared. If they were identical, go ahead and force the > reset --hard, otherwise abort. That way, --skip will dwim only when > it's safe, and all the lost work can be automagically created by > just re-applying the patch again?
I'd prefer the --force option suggested in some other mail. Maybe I'm just not manly enough, but messing up a rebase can mean lots of duplicated work, so I'm rather happy with no dwim at all. Maybe for the real manly users out there, add a rebase.alwaysForce option so they can laugh at me for not using that ;-)
Björn
Jeff KingRe: git rebase --skip
On Thu, Nov 08, 2007 at 11:44:03AM +0100, Björn Steinbrink wrote:
> > How about if the state to skip was stashed, the patch reapplied and the > > differences compared. If they were identical, go ahead and force the > > reset --hard, otherwise abort. That way, --skip will dwim only when > > it's safe, and all the lost work can be automagically created by > > just re-applying the patch again? > > I'd prefer the --force option suggested in some other mail. Maybe I'm > just not manly enough, but messing up a rebase can mean lots of > duplicated work, so I'm rather happy with no dwim at all. Maybe for the > real manly users out there, add a rebase.alwaysForce option so they can > laugh at me for not using that ;-)
Personally, I don't see the point of a --force option; it turns your work flow from:
1. git-rebase --skip 2. Oops, I guess I have to reset. 3. git-reset --hard; git-rebase --skip
to:
1. same as above 2. same as above 3. git-rebase --force --skip
I guess it's a little bit easier to explain to new users, but it in no way eliminates the annoyance of "I expected this to work, and it didn't, so now I have to think about what happened and enter another command."
AIUI, Andreas's proposal is not so much DWIM as "do the obvious thing, but include a safety valve to prevent throwing away work." Is there actually a case where it would not have the desired effect?
-Peff
Junio C HamanoRe: git rebase --skip
Jeff King <peff@peff.net> writes:
> Personally, I don't see the point of a --force option; it turns your work > flow from: > > 1. git-rebase --skip > 2. Oops, I guess I have to reset. > 3. git-reset --hard; git-rebase --skip > > to: > > 1. same as above > 2. same as above > 3. git-rebase --force --skip
I do not see it as improvement, either, for the same reason you state.
> AIUI, Andreas's proposal is not so much DWIM as "do the obvious thing, > but include a safety valve to prevent throwing away work." Is there > actually a case where it would not have the desired effect?
The user is explicitly saying --skip, so I do not think it is dangerous even if we unconditionally did "reset --hard" at that point.
Or we could introduce a new option "--drop" (that's "drop the current commit and continue") to do so, if people find that the word "skip" does not sound like a scary destructive operation.
But I do not think that is needed.
Björn SteinbrinkRe: git rebase --skip
On 2007.11.08 15:52:08 -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes: > > > Personally, I don't see the point of a --force option; it turns your work > > flow from: > > > > 1. git-rebase --skip > > 2. Oops, I guess I have to reset. > > 3. git-reset --hard; git-rebase --skip > > > > to: > > > > 1. same as above > > 2. same as above > > 3. git-rebase --force --skip > > I do not see it as improvement, either, for the same reason you > state. > > > AIUI, Andreas's proposal is not so much DWIM as "do the obvious thing, > > but include a safety valve to prevent throwing away work." Is there > > actually a case where it would not have the desired effect? > > The user is explicitly saying --skip, so I do not think it is > dangerous even if we unconditionally did "reset --hard" at that > point.
The user _must_ say --skip in the case I outlined. And I'm pretty sure that the first thing I'll (accidently) do once --skip implies "reset --hard" is to forget to commit. Murphy has never let me down.
How about adding that --amend option that someone mentioned? Or even just letting --continue act like --skip when there's nothing to commit. That way, you're no longer forced to use --skip.
Björn
Jeff KingRe: git rebase --skip
On Thu, Nov 08, 2007 at 03:52:08PM -0800, Junio C Hamano wrote:
> The user is explicitly saying --skip, so I do not think it is > dangerous even if we unconditionally did "reset --hard" at that > point.
Sure, I think the complaint is not that "reset --hard" is the wrong behavior, but that people are prone to type --skip in error. Right now we handle that error in a data-preserving way (we complain, and the user has to think and issue a "throw away this data" command), but automatic reset is less safe (even though there are fewer times when somebody meant to commit instead of just reset, the consequences are harder to recover from).
I've never personally run into this, but I think it is a reasonable thing to think about, and if it is easy to add an additional safety valve (either stashing the index/wt state, or checking before automatic "reset --hard" whether any work has been done towards resolving), then we probably should.
So I am fine with the original patch (unconditional reset --hard), but it would be nice to see the people who care submit concrete proposals for such a safety valve.
> Or we could introduce a new option "--drop" (that's "drop the > current commit and continue") to do so, if people find that the > word "skip" does not sound like a scary destructive operation.
I don't think the problem is "users don't realize how scary --skip can be", but rather "I use --skip to resolve this situation 99% of the time, so in the other 1%, I soetimes use it accidentally."
-Peff
Johannes SchindelinRe: git rebase --skip
Hi,
On Thu, 8 Nov 2007, Jeff King wrote:
> So I am fine with the original patch (unconditional reset --hard), but > it would be nice to see the people who care submit concrete proposals > for such a safety valve.
Isn't having to say "--skip" instead of "--continue" enough? Some people might complain that it's too easy to get your fingers wired to type --skip.
In that case, I might beg to differ for two reasons: --skip is definitely not the default operation, so the fingers do not get any chance to do that, and even if, they would get wired to --force --skip just as easily.
Besides, after my patch to rebase on a detached HEAD, it is very easy to go back to the original state and try again.
Ciao, Dscho
Re: git rebase --skip
On Fri, Nov 09, 2007 at 10:59:57AM +0000, Johannes Schindelin wrote:
> Hi, > > On Thu, 8 Nov 2007, Jeff King wrote: > > > So I am fine with the original patch (unconditional reset --hard), but > > it would be nice to see the people who care submit concrete proposals > > for such a safety valve. > > Isn't having to say "--skip" instead of "--continue" enough? Some people > might complain that it's too easy to get your fingers wired to type > --skip. > > In that case, I might beg to differ for two reasons: --skip is definitely > not the default operation, so the fingers do not get any chance to do > that, and even if, they would get wired to --force --skip just as easily. > > Besides, after my patch to rebase on a detached HEAD, it is very easy to > go back to the original state and try again.
We can't make --skip do an automatic reset --hard. I expect --skip to tell me if I haven't finished resolving conflicts. If it doesn't do that, I'll lose work. I can't be the only user with that expectation.
(To be more specific: the work I lose is the work spent resolving how however many conflicts I've dealt with so far. It isn't always the case that I get to a certain point in a rebase and say "ah, yes, I know this is already applied, I can reset and skip it." More often I want to go through each conflict and decide individually. So I do that for a few, I run --continue or --skip when I think I'm done (but don't actually check first, 'cause I count on rebase to do that), then fix up anything remaining.)
--b.
Re: git rebase --skip
On Fri, Nov 09, 2007 at 11:19:46AM -0500, J. Bruce Fields wrote:
> On Fri, Nov 09, 2007 at 10:59:57AM +0000, Johannes Schindelin wrote: > > Hi, > > > > On Thu, 8 Nov 2007, Jeff King wrote: > > > > > So I am fine with the original patch (unconditional reset --hard), but > > > it would be nice to see the people who care submit concrete proposals > > > for such a safety valve. > > > > Isn't having to say "--skip" instead of "--continue" enough? Some people > > might complain that it's too easy to get your fingers wired to type > > --skip. > > > > In that case, I might beg to differ for two reasons: --skip is definitely > > not the default operation, so the fingers do not get any chance to do > > that, and even if, they would get wired to --force --skip just as easily. > > > > Besides, after my patch to rebase on a detached HEAD, it is very easy to > > go back to the original state and try again. > > We can't make --skip do an automatic reset --hard. I expect --skip to > tell me if I haven't finished resolving conflicts. If it doesn't do > that, I'll lose work. I can't be the only user with that expectation. > > (To be more specific: the work I lose is the work spent resolving how > however many conflicts I've dealt with so far. It isn't always the case > that I get to a certain point in a rebase and say "ah, yes, I know this > is already applied, I can reset and skip it." More often I want to go > through each conflict and decide individually. So I do that for a few, > I run --continue or --skip when I think I'm done (but don't actually > check first, 'cause I count on rebase to do that), then fix up anything > remaining.)
(Eh, now that I think of it: what I actually usually do is run --continue, git says there's one more conflict, I fix that up and run --continue again, it tells me there's nothing to commit, I say huh? Then eventually I remember I need to run --skip instead, cursing it for not just figuring that out on its own.... So actually it may be that the only time I run --skip is after --continue has told me I need to. I can't *swear* that that's the case, so I'm still nervous about the idea of --skip automatically resetting. Oh well. Take that one user's experience for what it's worth.)
--b.
Jeff KingRe: git rebase --skip
On Fri, Nov 09, 2007 at 10:59:57AM +0000, Johannes Schindelin wrote:
> Isn't having to say "--skip" instead of "--continue" enough? Some people > might complain that it's too easy to get your fingers wired to type > --skip.
They already have.
> Besides, after my patch to rebase on a detached HEAD, it is very easy to > go back to the original state and try again.
But you are losing any work you did in resolving merges.
But I don't personally have this problem, so I will stop defending those who claim to, and let them speak for themselves (hopefully with patches!).
-Peff
Daniel BarkalowRe: git rebase --skip
On Thu, 8 Nov 2007, Björn Steinbrink wrote:
> On 2007.11.07 22:23:10 -0500, Jeff King wrote: > > On Wed, Nov 07, 2007 at 11:21:05PM +0100, Mike Hommey wrote: > > > > > I use git-rebase quite regularly, and I haven't used git-rebase --skip > > > after a failed merge without first resetting the working tree. I was > > > wondering if it wouldn't make sense to automatically do the reset when > > > running git-rebase --skip. > > > > I have often been annoyed by this behavior, too, and I can't think of > > any situation where you _wouldn't_ want the reset to happen. But I > > would be more comfortable hearing confirmation from others that they > > can't think of such a situation. > > Let's take this history: > > C---D---E topic > / > A---B master > > You then do: > git rebase master topic > > Now D causes conflicts, because B did a similar change, but not quite > the same, maybe having a bug. So you want to keep parts of D, but it's > no longer the same commit semantically and the original commit message > would be bogus. So you resolve the conflicts and do: > > git commit > git rebase --skip
I guess that works, and nothing else presently does. But I don't think that's at all intuitive as the correct thing to do (plus it feels too easy to get into losing your commit message). Maybe we should have a "git rebase --amend", which does the obvious thing (acts like --continue, but lets you edit the message). It's not like you just did something totally different; the commit is still the replacement for D, it's just less the same.
-Daniel *This .sig left intentionally blank*
Mike HommeyRe: git rebase --skip
On Thu, Nov 08, 2007 at 01:43:05PM -0500, Daniel Barkalow wrote:
> > git commit > > git rebase --skip > > I guess that works, and nothing else presently does. But I don't think > that's at all intuitive as the correct thing to do (plus it feels too easy > to get into losing your commit message). Maybe we should have a "git > rebase --amend", which does the obvious thing (acts like --continue, but > lets you edit the message). It's not like you just did something totally > different; the commit is still the replacement for D, it's just less the > same.
Maybe some commands such as commit should fail or at least emit warning when used during a rebase ?
Mike
Mike HommeyRe: git rebase --skip
On Thu, Nov 08, 2007 at 08:16:01PM +0100, Mike Hommey wrote:
> On Thu, Nov 08, 2007 at 01:43:05PM -0500, Daniel Barkalow wrote: > > > git commit > > > git rebase --skip > > > > I guess that works, and nothing else presently does. But I don't think > > that's at all intuitive as the correct thing to do (plus it feels too easy > > to get into losing your commit message). Maybe we should have a "git > > rebase --amend", which does the obvious thing (acts like --continue, but > > lets you edit the message). It's not like you just did something totally > > different; the commit is still the replacement for D, it's just less the > > same. > > Maybe some commands such as commit should fail or at least emit warning > when used during a rebase ?
Or, in the present case, doing the same as what "edit" would do in a git-rebase --interactive could be enough.
Mike
Johannes SchindelinRe: git rebase --skip
Hi,
On Thu, 8 Nov 2007, Mike Hommey wrote:
> Maybe some commands such as commit should fail or at least emit warning > when used during a rebase ?
No, that would disrupt comming (and valid!) workflow too much. Even a warning.
Ciao, Dscho
Mike Hommey[PATCH] Do git reset --hard HEAD when using git rebase --skip
When you have a merge conflict and want to bypass the commit causing it, you don't want to care about the dirty state of the working tree.
Also, don't git reset --hard HEAD in the rebase-skip test, so that the lack of support for this is detected.
Signed-off-by: Mike Hommey <mh@glandium.org> --- git-rebase.sh | 1 + t/t3403-rebase-skip.sh | 2 -- 2 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh index e3ad7de..8814be9 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -158,6 +158,7 @@ do exit ;; --skip) + git reset --hard HEAD || exit $? if test -d "$dotest" then git rerere clear diff --git a/t/t3403-rebase-skip.sh b/t/t3403-rebase-skip.sh index eab053c..becabfc 100755 --- a/t/t3403-rebase-skip.sh +++ b/t/t3403-rebase-skip.sh @@ -36,7 +36,6 @@ test_expect_failure 'rebase with git am -3 (default)' ' ' test_expect_success 'rebase --skip with am -3' ' - git reset --hard HEAD && git rebase --skip ' test_expect_success 'checkout skip-merge' 'git checkout -f skip-merge' @@ -44,7 +43,6 @@ test_expect_success 'checkout skip-merge' 'git checkout -f skip-merge' test_expect_failure 'rebase with --merge' 'git rebase --merge master' test_expect_success 'rebase --skip with --merge' ' - git reset --hard HEAD && git rebase --skip '
-- 1.5.3.5