[PATCH] csinfo: fix crash when stripping commits

[Antonio Muci]

# HG changeset patch
# User muxator <a....@inwind.it>
# Date 1728023213 -7200
# Fri Oct 04 08:26:53 2024 +0200
# Branch stable
# Node ID e7b8ae80e35687186a550e2382ffff11c07c82a6
# Parent 1f17abd911a232e537b7b1c1982f7a43b4d2f025
csinfo: fix crash when stripping commits

This commit fixes a bug introduced in a015c03d91c6 ("cslist: fix
[signature-mismatch] issues overriding QWidget.update()"), which modified
cslit.ChangesetList. Invoking "Modify History | Strip" would result in a crash.

Discussions:
- with Matt at https://foss.heptapod.net/mercurial/tortoisehg/thg/-/issues/5991
- with Yuya at https://groups.google.com/g/thg-dev/c/HNuN_tuuVGo

diff --git a/tortoisehg/hgqt/csinfo.py b/tortoisehg/hgqt/csinfo.py
--- a/tortoisehg/hgqt/csinfo.py
+++ b/tortoisehg/hgqt/csinfo.py
@@ -9,6 +9,7 @@ from __future__ import annotations

import binascii
import re
+import sys

from .qtcore import (
QSize,
@@ -106,7 +107,7 @@ class Factory(object):
else:
widget = SummaryLabel(*args)
if self.withupdate:
- widget.update()
+ widget.updateItems()
return widget

class UnknownItem(Exception):
@@ -435,7 +436,21 @@ class SummaryPanel(SummaryBase, QWidget)
self.revlabel = None
self.expand_btn = qtlib.PMButton()

- def update(self, target=None, style=None, custom=None, repo=None):
+ # TODO: drop this when all callers are removed, since it hides update() in
+ # the superclass, which seems related to painting the component, or an area
+ # of it.
+ def update(self, target=None, style=None, custom=None, repo=None): # pytype: disable=signature-mismatch
+ import warnings
+
+ warnings.warn(
+ "SummaryPanel.update() should be changed to SummaryPanel.updateItems()",
+ DeprecationWarning,
+ 2,
+ )
+ sys.stderr.flush()
+ self.updateItems(target, style, custom, repo)
+
+ def updateItems(self, target=None, style=None, custom=None, repo=None):
SummaryBase.update(self, target, custom, repo)

layout = self.layout()
@@ -516,7 +531,21 @@ class SummaryLabel(SummaryBase, QLabel):

self.csstyle = style

- def update(self, target=None, style=None, custom=None, repo=None):
+ # TODO: drop this when all callers are removed, since it hides update() in
+ # the superclass, which seems related to painting the component, or an area
+ # of it.
+ def update(self, target=None, style=None, custom=None, repo=None): # pytype: disable=signature-mismatch
+ import warnings
+
+ warnings.warn(
+ "SummaryLabel.update() should be changed to SummaryLabel.updateItems()",
+ DeprecationWarning,
+ 2,
+ )
+ sys.stderr.flush()
+ self.updateItems(target, style, custom, repo)
+
+ def updateItems(self, target=None, style=None, custom=None, repo=None):
SummaryBase.update(self, target, custom, repo)

if style is not None:

[Antonio Muci]

On 10/5/24 11:08, 'Antonio Muci' via TortoiseHg Developers wrote:
> - widget.update()
> + widget.updateItems()
>
Matt, Yuya: I am not really sure if I understood correctly the advice
you gave me here about Factory.__call__().

[Yuya Nishihara]

This change looks good to me.

> class UnknownItem(Exception):
> @@ -435,7 +436,21 @@ class SummaryPanel(SummaryBase, QWidget)
> self.revlabel = None
> self.expand_btn = qtlib.PMButton()
>
> - def update(self, target=None, style=None, custom=None, repo=None):
> + # TODO: drop this when all callers are removed, since it hides update() in
> + # the superclass, which seems related to painting the component, or an area
> + # of it.
> + def update(self, target=None, style=None, custom=None, repo=None): # pytype: disable=signature-mismatch
> + import warnings
> +
> + warnings.warn(
> + "SummaryPanel.update() should be changed to SummaryPanel.updateItems()",
> + DeprecationWarning,
> + 2,
> + )
> + sys.stderr.flush()
> + self.updateItems(target, style, custom, repo)

Maybe this forwarding function can be implemented on SummaryBase instead?

SummaryBase:
update(**):
warn ...
self.updateItems(**)
updateItems: # renamed

SummaryPanel(SummaryBase, ..):
updateItems: # renamed
SummaryBase.updateItems() # call super
...

> +
> + def updateItems(self, target=None, style=None, custom=None, repo=None):
> SummaryBase.update(self, target, custom, repo)

I think SummaryBase.update() should also be renamed to updateItems().

[Matt Harbison]

Gentle ping on this; I didn't realize it wasn't landed.  Antonio- do you have time to follow up on this?  I don't think we should do the RC with this regression.

[Antonio Muci]

# HG changeset patch
# User Antonio Muci <a....@inwind.it>
# Date 1728743188 -7200
# Sat Oct 12 16:26:28 2024 +0200
# Branch stable
# Node ID ca19ea353bb476839dd3465bb54f3285fe84d1d1

# Parent 1f17abd911a232e537b7b1c1982f7a43b4d2f025
csinfo: fix crash when stripping commits

This commit fixes a bug introduced in a015c03d91c6 ("cslist: fix
[signature-mismatch] issues overriding QWidget.update()"), which modified
cslit.ChangesetList. Invoking "Modify History | Strip" would result in a crash.

Discussions:

With Matt:
- https://foss.heptapod.net/mercurial/tortoisehg/thg/-/issues/5991

With Yuya:
- https://groups.google.com/g/thg-dev/c/HNuN_tuuVGo
- https://groups.google.com/g/thg-dev/c/zWTdJAYSr1c

diff --git a/tortoisehg/hgqt/csinfo.py b/tortoisehg/hgqt/csinfo.py
--- a/tortoisehg/hgqt/csinfo.py
+++ b/tortoisehg/hgqt/csinfo.py
@@ -9,6 +9,7 @@ from __future__ import annotations

import binascii
import re
+import sys

from .qtcore import (
QSize,
@@ -106,7 +107,7 @@ class Factory(object):
else:
widget = SummaryLabel(*args)
if self.withupdate:
- widget.update()
+ widget.updateItems()

return widget

class UnknownItem(Exception):
@@ -396,7 +397,22 @@ class SummaryBase(object):
def set_revision(self, rev):
self.target = rev

- def update(self, target=None, custom=None, repo=None):
+ # TODO: drop this when all callers are removed, since it hides the update()
+ # method that SummaryPanel and SummaryLabel get through multiple inheritance
+ # from QWidget and QLabel respectively. That update() method seems related
+ # to painting the component, or an area of it.

+ def update(self, target=None, style=None, custom=None, repo=None): # pytype: disable=signature-mismatch
+ import warnings
+
+ warnings.warn(

+ "SummaryBase.update() should be changed to SummaryBase.updateItems()",

+ DeprecationWarning,
+ 2,
+ )
+ sys.stderr.flush()
+ self.updateItems(target, style, custom, repo)

+
+ def updateItems(self, target=None, custom=None, repo=None):
self.ctx = None
if target is not None:
self.target = target
@@ -435,8 +451,8 @@ class SummaryPanel(SummaryBase, QWidget)

self.revlabel = None
self.expand_btn = qtlib.PMButton()

- def update(self, target=None, style=None, custom=None, repo=None):

- SummaryBase.update(self, target, custom, repo)

+ def updateItems(self, target=None, style=None, custom=None, repo=None):

+ SummaryBase.updateItems(self, target, custom, repo)

layout = self.layout()
assert isinstance(layout, QHBoxLayout)
@@ -516,8 +532,8 @@ class SummaryLabel(SummaryBase, QLabel):

self.csstyle = style

- def update(self, target=None, style=None, custom=None, repo=None):

- SummaryBase.update(self, target, custom, repo)

+ def updateItems(self, target=None, style=None, custom=None, repo=None):

+ SummaryBase.updateItems(self, target, custom, repo)

if style is not None:

self.csstyle = style

[Yuya Nishihara]

On Wed, 13 Nov 2024 22:23:47 +0100, 'Antonio Muci' via TortoiseHg Developers wrote:
> # HG changeset patch
> # User Antonio Muci <a....@inwind.it>
> # Date 1728743188 -7200
> # Sat Oct 12 16:26:28 2024 +0200
> # Branch stable
> # Node ID ca19ea353bb476839dd3465bb54f3285fe84d1d1
> # Parent 1f17abd911a232e537b7b1c1982f7a43b4d2f025
> csinfo: fix crash when stripping commits

Queued, thanks.

> - def update(self, target=None, custom=None, repo=None):
> + # TODO: drop this when all callers are removed, since it hides the update()
> + # method that SummaryPanel and SummaryLabel get through multiple inheritance
> + # from QWidget and QLabel respectively. That update() method seems related
> + # to painting the component, or an area of it.
> + def update(self, target=None, style=None, custom=None, repo=None): # pytype: disable=signature-mismatch
> + import warnings
> +
> + warnings.warn(
> + "SummaryBase.update() should be changed to SummaryBase.updateItems()",
> + DeprecationWarning,
> + 2,
> + )
> + sys.stderr.flush()
> + self.updateItems(target, style, custom, repo)

might be safer to use *args, **kwargs (because signatures differ), but I'm not
sure. csinfo is a mess.

[Matt Harbison]

I didn't look into the details yet, but pytype is unhappy:

https://foss.heptapod.net/mercurial/tortoisehg/thg/-/jobs/2688144#L626