Patchwork [evolve-ext:stable] evolve: don't try to relocate a node on top of itself

login
register
mail settings
Submitter Olle Lundberg
Date March 25, 2014, 11:06 p.m.
Message ID <5a2abe5bae7ae4f439d4.1395788802@se-c02kq0dadr55.lan>
Download mbox | patch
Permalink /patch/4067/
State Not Applicable
Headers show

Comments

Olle Lundberg - March 25, 2014, 11:06 p.m.
# HG changeset patch
# User Olle Lundberg <geek@nerd.sh>
# Date 1395788513 -3600
#      Wed Mar 26 00:01:53 2014 +0100
# Branch stable
# Node ID 5a2abe5bae7ae4f439d4a078b7891e3bcf6bcc48
# Parent  6a67606e1c3456c603707fe57e8404af5e33a1bd
evolve: don't try to relocate a node on top of itself

This fixes problems with "no support for evolution merge changes yet"
on a linear repo, where evolve thinks that a node is supposed to be
relocated on top of itself.
Angel Ezquerra - March 25, 2014, 11:10 p.m.
On Wed, Mar 26, 2014 at 12:06 AM, Olle Lundberg <olle.lundberg@gmail.com> wrote:
> # HG changeset patch
> # User Olle Lundberg <geek@nerd.sh>
> # Date 1395788513 -3600
> #      Wed Mar 26 00:01:53 2014 +0100
> # Branch stable
> # Node ID 5a2abe5bae7ae4f439d4a078b7891e3bcf6bcc48
> # Parent  6a67606e1c3456c603707fe57e8404af5e33a1bd
> evolve: don't try to relocate a node on top of itself
>
> This fixes problems with "no support for evolution merge changes yet"
> on a linear repo, where evolve thinks that a node is supposed to be
> relocated on top of itself.
>
> diff --git a/hgext/evolve.py b/hgext/evolve.py
> --- a/hgext/evolve.py
> +++ b/hgext/evolve.py
> @@ -1007,36 +1007,39 @@
>      targets = newer[0]
>      assert targets
>      if len(targets) > 1:
>          raise util.Abort(_("does not handle split parents yet\n"))
>          return 2
> -    target = targets[0]
> -    displayer = cmdutil.show_changeset(ui, repo, {'template': shorttemplate})
> -    target = repo[target]
> -    repo.ui.status(_('move:'))
> -    if not ui.quiet:
> -        displayer.show(orig)
> -    repo.ui.status(_('atop:'))
> -    if not ui.quiet:
> -        displayer.show(target)
> -    if progresscb: progresscb()
> -    todo = 'hg rebase -r %s -d %s\n' % (orig, target)
> -    if dryrun:
> -        repo.ui.write(todo)
> -    else:
> -        repo.ui.note(todo)
> -        if progresscb: progresscb()
> -        lock = repo.lock()
> -        try:
> -            relocate(repo, orig, target)
> -        except MergeFailure:
> -            repo.opener.write('graftstate', orig.hex() + '\n')
> -            repo.ui.write_err(_('evolve failed!\n'))
> -            repo.ui.write_err(_('fix conflict and run "hg evolve --continue"\n'))
> -            raise
> -        finally:
> -            lock.release()
> +    target = repo[targets[0]]
> +    # Target revision is same as original revision, it makes no sense to try
> +    #  to relocate a node on top of itself.
> +    if target.rev() != orig.rev()
> +        displayer = cmdutil.show_changeset(ui, repo, {'template': shorttemplate})
> +        repo.ui.status(_('move:'))
> +        if not ui.quiet:
> +            displayer.show(orig)
> +        repo.ui.status(_('atop:'))
> +        if not ui.quiet:
> +            displayer.show(target)
> +        if progresscb:
> +            progresscb()
> +        todo = 'hg rebase -r %s -d %s\n' % (orig, target)
> +        if dryrun:
> +            repo.ui.write(todo)
> +        else:
> +            repo.ui.note(todo)
> +            if progresscb: progresscb()
> +            lock = repo.lock()
> +            try:
> +                relocate(repo, orig, target)
> +            except MergeFailure:
> +                repo.opener.write('graftstate', orig.hex() + '\n')
> +                repo.ui.write_err(_('evolve failed!\n'))
> +                repo.ui.write_err(_('fix conflict and run "hg evolve --continue"\n'))
> +                raise
> +            finally:
> +                lock.release()
>
>  def _solvebumped(ui, repo, bumped, dryrun=False, progresscb=None):
>      """Stabilize a bumped changeset"""
>      # For now we deny bumped merge
>      if len(bumped.parents()) > 1:

Wouldn't it be better to just do the check and exit early (showing
some error message), and keep the rest of the code the same? That
would be a much smaller patch.

Cheers,

angel
Olle Lundberg - March 25, 2014, 11:12 p.m.
On Wed, Mar 26, 2014 at 12:10 AM, Angel Ezquerra
<angel.ezquerra@gmail.com>wrote:

> On Wed, Mar 26, 2014 at 12:06 AM, Olle Lundberg <olle.lundberg@gmail.com>
> wrote:
> > # HG changeset patch
> > # User Olle Lundberg <geek@nerd.sh>
> > # Date 1395788513 -3600
> > #      Wed Mar 26 00:01:53 2014 +0100
> > # Branch stable
> > # Node ID 5a2abe5bae7ae4f439d4a078b7891e3bcf6bcc48
> > # Parent  6a67606e1c3456c603707fe57e8404af5e33a1bd
> > evolve: don't try to relocate a node on top of itself
> >
> > This fixes problems with "no support for evolution merge changes yet"
> > on a linear repo, where evolve thinks that a node is supposed to be
> > relocated on top of itself.
> >
> > diff --git a/hgext/evolve.py b/hgext/evolve.py
> > --- a/hgext/evolve.py
> > +++ b/hgext/evolve.py
> > @@ -1007,36 +1007,39 @@
> >      targets = newer[0]
> >      assert targets
> >      if len(targets) > 1:
> >          raise util.Abort(_("does not handle split parents yet\n"))
> >          return 2
> > -    target = targets[0]
> > -    displayer = cmdutil.show_changeset(ui, repo, {'template':
> shorttemplate})
> > -    target = repo[target]
> > -    repo.ui.status(_('move:'))
> > -    if not ui.quiet:
> > -        displayer.show(orig)
> > -    repo.ui.status(_('atop:'))
> > -    if not ui.quiet:
> > -        displayer.show(target)
> > -    if progresscb: progresscb()
> > -    todo = 'hg rebase -r %s -d %s\n' % (orig, target)
> > -    if dryrun:
> > -        repo.ui.write(todo)
> > -    else:
> > -        repo.ui.note(todo)
> > -        if progresscb: progresscb()
> > -        lock = repo.lock()
> > -        try:
> > -            relocate(repo, orig, target)
> > -        except MergeFailure:
> > -            repo.opener.write('graftstate', orig.hex() + '\n')
> > -            repo.ui.write_err(_('evolve failed!\n'))
> > -            repo.ui.write_err(_('fix conflict and run "hg evolve
> --continue"\n'))
> > -            raise
> > -        finally:
> > -            lock.release()
> > +    target = repo[targets[0]]
> > +    # Target revision is same as original revision, it makes no sense
> to try
> > +    #  to relocate a node on top of itself.
> > +    if target.rev() != orig.rev()
> > +        displayer = cmdutil.show_changeset(ui, repo, {'template':
> shorttemplate})
> > +        repo.ui.status(_('move:'))
> > +        if not ui.quiet:
> > +            displayer.show(orig)
> > +        repo.ui.status(_('atop:'))
> > +        if not ui.quiet:
> > +            displayer.show(target)
> > +        if progresscb:
> > +            progresscb()
> > +        todo = 'hg rebase -r %s -d %s\n' % (orig, target)
> > +        if dryrun:
> > +            repo.ui.write(todo)
> > +        else:
> > +            repo.ui.note(todo)
> > +            if progresscb: progresscb()
> > +            lock = repo.lock()
> > +            try:
> > +                relocate(repo, orig, target)
> > +            except MergeFailure:
> > +                repo.opener.write('graftstate', orig.hex() + '\n')
> > +                repo.ui.write_err(_('evolve failed!\n'))
> > +                repo.ui.write_err(_('fix conflict and run "hg evolve
> --continue"\n'))
> > +                raise
> > +            finally:
> > +                lock.release()
> >
> >  def _solvebumped(ui, repo, bumped, dryrun=False, progresscb=None):
> >      """Stabilize a bumped changeset"""
> >      # For now we deny bumped merge
> >      if len(bumped.parents()) > 1:
>
> Wouldn't it be better to just do the check and exit early (showing
> some error message), and keep the rest of the code the same? That
> would be a much smaller patch.
>

Perhaps :)

Just wanted somewhere to start. I don't expect to land this directly.

I don't know if an error message would help you here anymore than the one
you already get?
I'm not sure what return value to send back either.

I think we need some input from pyd.

>
> Cheers,
>
> angel
>
Pierre-Yves David - March 25, 2014, 11:13 p.m.
On 03/25/2014 04:10 PM, Angel Ezquerra wrote:
> On Wed, Mar 26, 2014 at 12:06 AM, Olle Lundberg <olle.lundberg@gmail.com> wrote:
>> # HG changeset patch
>> # User Olle Lundberg <geek@nerd.sh>
>> # Date 1395788513 -3600
>> #      Wed Mar 26 00:01:53 2014 +0100
>> # Branch stable
>> # Node ID 5a2abe5bae7ae4f439d4a078b7891e3bcf6bcc48
>> # Parent  6a67606e1c3456c603707fe57e8404af5e33a1bd
>> evolve: don't try to relocate a node on top of itself
>>
>> This fixes problems with "no support for evolution merge changes yet"
>> on a linear repo, where evolve thinks that a node is supposed to be
>> relocated on top of itself.
>>
>> diff --git a/hgext/evolve.py b/hgext/evolve.py
>> --- a/hgext/evolve.py
>> +++ b/hgext/evolve.py
>> @@ -1007,36 +1007,39 @@
>>       targets = newer[0]
>>       assert targets
>>       if len(targets) > 1:
>>           raise util.Abort(_("does not handle split parents yet\n"))
>>           return 2
>> -    target = targets[0]
>> -    displayer = cmdutil.show_changeset(ui, repo, {'template': shorttemplate})
>> -    target = repo[target]
>> -    repo.ui.status(_('move:'))
>> -    if not ui.quiet:
>> -        displayer.show(orig)
>> -    repo.ui.status(_('atop:'))
>> -    if not ui.quiet:
>> -        displayer.show(target)
>> -    if progresscb: progresscb()
>> -    todo = 'hg rebase -r %s -d %s\n' % (orig, target)
>> -    if dryrun:
>> -        repo.ui.write(todo)
>> -    else:
>> -        repo.ui.note(todo)
>> -        if progresscb: progresscb()
>> -        lock = repo.lock()
>> -        try:
>> -            relocate(repo, orig, target)
>> -        except MergeFailure:
>> -            repo.opener.write('graftstate', orig.hex() + '\n')
>> -            repo.ui.write_err(_('evolve failed!\n'))
>> -            repo.ui.write_err(_('fix conflict and run "hg evolve --continue"\n'))
>> -            raise
>> -        finally:
>> -            lock.release()
>> +    target = repo[targets[0]]
>> +    # Target revision is same as original revision, it makes no sense to try
>> +    #  to relocate a node on top of itself.
>> +    if target.rev() != orig.rev()
>> +        displayer = cmdutil.show_changeset(ui, repo, {'template': shorttemplate})
>> +        repo.ui.status(_('move:'))
>> +        if not ui.quiet:
>> +            displayer.show(orig)
>> +        repo.ui.status(_('atop:'))
>> +        if not ui.quiet:
>> +            displayer.show(target)
>> +        if progresscb:
>> +            progresscb()
>> +        todo = 'hg rebase -r %s -d %s\n' % (orig, target)
>> +        if dryrun:
>> +            repo.ui.write(todo)
>> +        else:
>> +            repo.ui.note(todo)
>> +            if progresscb: progresscb()
>> +            lock = repo.lock()
>> +            try:
>> +                relocate(repo, orig, target)
>> +            except MergeFailure:
>> +                repo.opener.write('graftstate', orig.hex() + '\n')
>> +                repo.ui.write_err(_('evolve failed!\n'))
>> +                repo.ui.write_err(_('fix conflict and run "hg evolve --continue"\n'))
>> +                raise
>> +            finally:
>> +                lock.release()
>>
>>   def _solvebumped(ui, repo, bumped, dryrun=False, progresscb=None):
>>       """Stabilize a bumped changeset"""
>>       # For now we deny bumped merge
>>       if len(bumped.parents()) > 1:
>
> Wouldn't it be better to just do the check and exit early (showing
> some error message), and keep the rest of the code the same? That
> would be a much smaller patch.

What angel say, In theory, this should not happen at all. So the current 
step is to move from

- I'm happily performing a non-sense operation

to

- I detect this is non-sense and abort before doing anything bad.
   Informing the user he hit a bad bad an must use rebase by himself to 
solve the situation.

You current patch make a bad error  silent and this is bad bad bad

Thanks for looking at it however.

Patch

diff --git a/hgext/evolve.py b/hgext/evolve.py
--- a/hgext/evolve.py
+++ b/hgext/evolve.py
@@ -1007,36 +1007,39 @@ 
     targets = newer[0]
     assert targets
     if len(targets) > 1:
         raise util.Abort(_("does not handle split parents yet\n"))
         return 2
-    target = targets[0]
-    displayer = cmdutil.show_changeset(ui, repo, {'template': shorttemplate})
-    target = repo[target]
-    repo.ui.status(_('move:'))
-    if not ui.quiet:
-        displayer.show(orig)
-    repo.ui.status(_('atop:'))
-    if not ui.quiet:
-        displayer.show(target)
-    if progresscb: progresscb()
-    todo = 'hg rebase -r %s -d %s\n' % (orig, target)
-    if dryrun:
-        repo.ui.write(todo)
-    else:
-        repo.ui.note(todo)
-        if progresscb: progresscb()
-        lock = repo.lock()
-        try:
-            relocate(repo, orig, target)
-        except MergeFailure:
-            repo.opener.write('graftstate', orig.hex() + '\n')
-            repo.ui.write_err(_('evolve failed!\n'))
-            repo.ui.write_err(_('fix conflict and run "hg evolve --continue"\n'))
-            raise
-        finally:
-            lock.release()
+    target = repo[targets[0]]
+    # Target revision is same as original revision, it makes no sense to try
+    #  to relocate a node on top of itself.
+    if target.rev() != orig.rev()
+        displayer = cmdutil.show_changeset(ui, repo, {'template': shorttemplate})
+        repo.ui.status(_('move:'))
+        if not ui.quiet:
+            displayer.show(orig)
+        repo.ui.status(_('atop:'))
+        if not ui.quiet:
+            displayer.show(target)
+        if progresscb:
+            progresscb()
+        todo = 'hg rebase -r %s -d %s\n' % (orig, target)
+        if dryrun:
+            repo.ui.write(todo)
+        else:
+            repo.ui.note(todo)
+            if progresscb: progresscb()
+            lock = repo.lock()
+            try:
+                relocate(repo, orig, target)
+            except MergeFailure:
+                repo.opener.write('graftstate', orig.hex() + '\n')
+                repo.ui.write_err(_('evolve failed!\n'))
+                repo.ui.write_err(_('fix conflict and run "hg evolve --continue"\n'))
+                raise
+            finally:
+                lock.release()
 
 def _solvebumped(ui, repo, bumped, dryrun=False, progresscb=None):
     """Stabilize a bumped changeset"""
     # For now we deny bumped merge
     if len(bumped.parents()) > 1: