[PATCH 0 of 2 ] Fix for Rebase fails silently on immutable changesets (#1610)

[michals...@gmail.com]

Both are required to fix https://bitbucket.org/tortoisehg/thg/issue/1610/rebase-fails-silently-on-immutable

[michals...@gmail.com]

# HG changeset patch
# User Michal Sznajder <michals...@gmail.com>
# Date 1329671076 -3600
# Branch stable
# Node ID 350f771a7ef7a2df5a398f24c955f97a7c5c4fbd
# Parent a76cbd22598a904ce6517502cbe6c526b5c8096b
cmdui: use unsigned values as error codes

since Mercurial 2.1 (01611e7c36ff) hg does not return -1 as error code
but always value between 0 and 255 (unsigned byte)

diff --git a/tortoisehg/hgqt/cmdui.py b/tortoisehg/hgqt/cmdui.py
--- a/tortoisehg/hgqt/cmdui.py
+++ b/tortoisehg/hgqt/cmdui.py
@@ -245,7 +245,7 @@
if error == QProcess.FailedToStart:
self.output.emit(_('failed to start command\n'),
'ui.error')
- finished(-1)
+ finished(255)
elif error != QProcess.Crashed:
self.output.emit(_('error while running command\n'),
'ui.error')
@@ -723,7 +723,7 @@

@pyqtSlot(int)
def onCommandFinished(self, ret):
- if ret == -1:
+ if ret == 255:
self.makeLogVisible.emit(True)
self.setShowOutput(True)
self.commandFinished.emit(ret)

[michals...@gmail.com]

# HG changeset patch
# User Michal Sznajder <michals...@gmail.com>

# Date 1329671179 -3600
# Branch stable
# Node ID 41319573064295adfce11d01183b0cda8172078e
# Parent 350f771a7ef7a2df5a398f24c955f97a7c5c4fbd
rebase: notify user that rebase failed (closes #1610)

diff --git a/tortoisehg/hgqt/rebase.py b/tortoisehg/hgqt/rebase.py
--- a/tortoisehg/hgqt/rebase.py
+++ b/tortoisehg/hgqt/rebase.py
@@ -191,10 +191,15 @@

def commandFinished(self, ret):
self.repo.decrementBusyCount()
- if self.checkResolve() is False:
+

+ if ret == 255:

+ msg = _("Rebase failed")
+ elif self.checkResolve() is False:
msg = _('Rebase is complete')
if self.aborted:
msg = _('Rebase aborted')
+
+ if msg:
self.showMessage.emit(msg)
self.rebasebtn.setEnabled(True)
self.rebasebtn.setText(_('Close'))

[Angel Ezquerra]

Michal,

You should check how this interacts with a recent change I made which shows all errors on the info bar. I think I'll work fine but you should check just to he sure.

Cheers,

Angel

[Michał Sznajder]

Angel,

Our code does not overlap that much - so I found no issues during
tests so far. Your advanced
capturing works only with main window, my changes are in rebase dialog
which is a separate thing.

Michal

[Angel Ezquerra]

Ok, that is great. I thought there wouldn't be any problems but as they say it is better to be safe than sorry! :-)

Angel

[Yuya Nishihara]

01611e7c36ff doesn't affect TortoiseHg because we mainly use thread-based
runner (thread.CmdThread).

According to fe38c01bfcf8 and 8cdcbf3ef288, it purposely uses -1.
So I skipped this patch.

Regards,

[Yuya Nishihara]

michals...@gmail.com wrote:
> # HG changeset patch
> # User Michal Sznajder<michals...@gmail.com>
> # Date 1329671179 -3600
> # Branch stable
> # Node ID 41319573064295adfce11d01183b0cda8172078e
> # Parent 350f771a7ef7a2df5a398f24c955f97a7c5c4fbd
> rebase: notify user that rebase failed (closes #1610)
>
> diff --git a/tortoisehg/hgqt/rebase.py b/tortoisehg/hgqt/rebase.py
> --- a/tortoisehg/hgqt/rebase.py
> +++ b/tortoisehg/hgqt/rebase.py
> @@ -191,10 +191,15 @@
>
> def commandFinished(self, ret):
> self.repo.decrementBusyCount()
> - if self.checkResolve() is False:
> +
> + if ret == 255:
> + msg = _("Rebase failed")
> + elif self.checkResolve() is False:
> msg = _('Rebase is complete')
> if self.aborted:
> msg = _('Rebase aborted')

This breaks the conflict resolution feature.
Maybe we should try checkResolve() first?

Regards,

[Yuya Nishihara]

Yuya Nishihara <yu...@tcha.org>

michals...@gmail.com wrote:

# HG changeset patch
# User Michal Sznajder<michalsznajder@gmail.com>

# Date 1329671179 -3600
# Branch stable
# Node ID 41319573064295adfce11d01183b0cda8172078e
# Parent  350f771a7ef7a2df5a398f24c955f97a7c5c4fbd
rebase: notify user that rebase failed (closes #1610)

diff --git a/tortoisehg/hgqt/rebase.py b/tortoisehg/hgqt/rebase.py
--- a/tortoisehg/hgqt/rebase.py
+++ b/tortoisehg/hgqt/rebase.py
@@ -191,10 +191,15 @@

     def commandFinished(self, ret):
         self.repo.decrementBusyCount()
-        if self.checkResolve() is False:
+
+        if ret == 255:
+            msg = _("Rebase failed")
+        elif self.checkResolve() is False:
             msg = _('Rebase is complete')
             if self.aborted:
                 msg = _('Rebase aborted')

This breaks the conflict resolution feature.
Maybe we should try checkResolve() first?

I've pushed the fix which puts ret == 255 condition as last ditch.

https://bitbucket.org/tortoisehg/thg/changeset/9f19325eb440

Regards,

[Michał Sznajder]

My intention was to catch 255 as error message and then show log window.

After second thought maybe it should be done differently? Please compare:
https://bitbucket.org/tortoisehg/thg/src/6f54b09e7eb1/tortoisehg/hgqt/cmdui.py#cl-814
https://bitbucket.org/tortoisehg/thg/src/6f54b09e7eb1/tortoisehg/hgqt/cmdui.py#cl-725
https://bitbucket.org/tortoisehg/thg/src/6f54b09e7eb1/tortoisehg/hgqt/cmdui.py#cl-880
Each of them behave a little bit different in case of an error. I
think the last one is
the best way: log window is shown always if command finishes with an error value
(non 0).

In your final fix for #1610 (https://bitbucket.org/tortoisehg/thg/issue/1610/)
you show log window "manually" in the rebase dialog.

Michał

[Michał Sznajder]

On Thu, Mar 1, 2012 at 5:11 PM, Yuya Nishihara <yu...@tcha.org> wrote:
> Yuya Nishihara <yu...@tcha.org>
>
>> michals...@gmail.com wrote:
>>>
>>> # HG changeset patch

>>> # User Michal Sznajder<michals...@gmail.com>

>>> # Date 1329671179 -3600
>>> # Branch stable
>>> # Node ID 41319573064295adfce11d01183b0cda8172078e
>>> # Parent  350f771a7ef7a2df5a398f24c955f97a7c5c4fbd
>>> rebase: notify user that rebase failed (closes #1610)
>>>
>>> diff --git a/tortoisehg/hgqt/rebase.py b/tortoisehg/hgqt/rebase.py
>>> --- a/tortoisehg/hgqt/rebase.py
>>> +++ b/tortoisehg/hgqt/rebase.py
>>> @@ -191,10 +191,15 @@
>>>
>>>      def commandFinished(self, ret):
>>>          self.repo.decrementBusyCount()
>>> -        if self.checkResolve() is False:
>>> +
>>> +        if ret == 255:
>>> +            msg = _("Rebase failed")
>>> +        elif self.checkResolve() is False:
>>>              msg = _('Rebase is complete')
>>>              if self.aborted:
>>>                  msg = _('Rebase aborted')
>>
>>
>> This breaks the conflict resolution feature.
>> Maybe we should try checkResolve() first?
>
>
> I've pushed the fix which puts ret == 255 condition as last ditch.
>
> https://bitbucket.org/tortoisehg/thg/changeset/9f19325eb440

Thank's! I was off the grid for a few days.

Michal

[Yuya Nishihara]

Michał Sznajder <michals...@gmail.com>

There may be some inconsistency, but according to fe38c01bfcf8, cmdui.Widget is

implemented not to show command output automatically for normal abort.

In your final fix for #1610 (https://bitbucket.org/tortoisehg/thg/issue/1610/)
you show log window "manually" in the rebase dialog.

because it handles merge conflicts (=ret 255) manually by checkResolve().

Regards,