git rebase --skip

git rebase --skip

[Mike Hommey]

Hi,

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

Re: git rebase --skip

[Johannes Schindelin]

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

Re: git rebase --skip

[Jeff King]

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

Re: git rebase --skip

[Jakub Narebski]

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

Re: git rebase --skip

[Björn Steinbrink]

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.

Björn

Re: git rebase --skip

[Andreas Ericsson]

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

Re: git rebase --skip

[Björn Steinbrink]

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

Re: git rebase --skip

[Jeff King]

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

Re: git rebase --skip

[Junio C Hamano]

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.

Re: git rebase --skip

[Björn Steinbrink]

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

Re: git rebase --skip

[Jeff King]

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

Re: git rebase --skip

[Johannes Schindelin]

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

[J. Bruce Fields]

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

[J. Bruce Fields]

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.

Re: git rebase --skip

[Jeff King]

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

Re: git rebase --skip

[Daniel Barkalow]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1662 bytes --]

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*

Re: git rebase --skip

[Mike Hommey]

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

Re: git rebase --skip

[Mike Hommey]

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

Re: git rebase --skip

[Johannes Schindelin]

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

[PATCH] Do git reset --hard HEAD when using git rebase --skip

[Mike Hommey]

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