updater: make UpdateListIter a proper iterator #238
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #238 +/- ##
==========================================
- Coverage 93.18% 93.17% -0.01%
==========================================
Files 58 58
Lines 11046 11048 +2
==========================================
+ Hits 10293 10294 +1
- Misses 753 754 +1 ☔ View full report in Codecov by Sentry. |
|
If that's a CPython bug, please report it properly (and then put bug reference in the commit message). But it's more likely buggy usage somewhere (possibly in this repo). Do you have the call trace of problematic situation? |
I would, if I was able to reproduce it. The bug went away when I reinstalled the package, so it seemed like something weird (stale
I don’t have the full stack trace, but I do remember that the error happened here: Python complained that an selected_num = 0
for row in self.list_store:
selected_num += i.selectedFurthermore, I found that |
DemiMarie
force-pushed
the
python-bug-workaround
branch
2 times, most recently
from
c920daf to
84fb5ac
Compare
|
openQA reproduced the issue: https://openqa.qubes-os.org/tests/123911#investigation says the python3 version in dom0 didn't change since last good run, but the desktop-linux-manager version did change, so it's most likely a regression in this repo - maybe in the other PR here? |
Possibly. From what I can tell, the existing code relies on undefined behavior, so it could break at any time. Did your merge scripts start including the PR message? In the past I have let that get stale: for instance, this PR is not a workaround for a CPython bug, but rather a bug fix for relying on undocumented behavior in CPython. Also, the PR message is meant to be viewed as markdown, rather than as plain text, so it won’t look as good when incorporated into a commit mesage. |
DemiMarie
changed the title
DemiMarie
force-pushed
the
python-bug-workaround
branch
from
84fb5ac to
41ba839
Compare
OpenQA test summaryComplete test suite and dependencies: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2025010404-4.3&flavor=pull-requests Test run included the following:
New failures, excluding unstableCompared to: https://openqa.qubes-os.org/tests/overview?distri=qubesos&version=4.3&build=2024111705-4.3&flavor=update
Failed tests22 failures
Fixed failuresCompared to: https://openqa.qubes-os.org/tests/119126#dependencies 3 fixed
Unstable tests
|
There was a problem hiding this comment.
Ugh, in the meantime black formatting is enforced and it doesn't like this: https://gitlab.com/QubesOS/qubes-desktop-linux-manager/-/jobs/8763230803
Python iterators are required to implement an __iter__() that returns self [1]. CPython doesn't check this consistently, but the requirement is still there, so the existing code is buggy. Python 3.13 started checking this in list comprehensions, resulting in exceptions being thrown. Fix the bug by having __iter__() return self, as required by the iterator protocol. This worked on Python 3.13 and below, but broke in 3.13.1 [2]. [1]: https://docs.python.org/3/glossary.html#term-iterator [2]: python/cpython#128211
DemiMarie
force-pushed
the
python-bug-workaround
branch
from
41ba839 to
1231efc
Compare
Python iterators are required to implement an
__iter__()that returnsself 1. CPython doesn't check this consistently, but the requirement
is still there, so the existing code is buggy. Python 3.13 started
checking this in list comprehensions, resulting in exceptions being
thrown. Fix the bug by having
__iter__()return self, as required bythe iterator protocol.
This worked on Python 3.13 and below, but broke in 3.13.1 2.