Background:
As part of my work on reproducible builds for openSUSE, I check that software still gives identical build results in the future.
The usual offset is +16 years, because that is how long I expect some software will be used in some places.
This showed up failing tests in our package build.
See https://reproducible-builds.org/ for why this matters.
With GitHub Actions now providing macOS 14 runners with M1 chips, we can
build a separate Apple Silicon release there and upload it.
Universal wheels are currently not possible, see #8229 for details.
Closes#6478
We want to make sure that the selection model gets deleted when clearing
the model, since we are switching from doing that directly to having it
happen indirectly based off of signals we don't manage.
Hopefully this doesn't end up to be flaky. I think we are depending on
this happening in two different Qt even loop runs (`model.deleteLater()`
in the first one then `selmod.deleteLater()` gets called from the
`model.deleted` signal).
I tried using `qtbot.wait_signals()` with a list but it segfaulted in
some cleanup method. Perhaps is doesn't handle the deleted signal well?
Alternately we could maybe just wait for the selmodel one and check
`sip.isdelted(model)` to check the model is gone too.
This starts happening reliably with the "Using session completion" end
to end test after the previous change where I made it so we don't set
the completion widget model to none, just call `deleteLater()` on the old
one. The relevant part of the test is:
And I run :session-save hello
# This loads a completion model with one entry
When I run :cmd-set-text -s :session-load
# This one selects the entry, which clears the model because the
# session completion only applies to the first word after the
# command, which we've just selected.
And I run :completion-item-focus next
# This does nothing, since we have no completion model loaded,
# not sure why it is in the test, presumably making sure no
# exception is thrown.
And I run :completion-item-focus next
The log message is reliable in the test but it's a bit flaky to
reproduce manually. I've reproduced it via `for f in 1 2;do qute-send -i
/tmp/qutebrowser-basedir-npzdpupy/runtime/ipc-8075cf54f63b8e1bc05db34f41292c38
":completion-item-focus next" ;done` if I manually go through the
scenario a few times. Possibly it would reproduce easier if I pin it to
one process using `taskset`.
I think the reason for this is that the model is marked for deletion in
the next event loop, then a signal goes out, then the selection model is
marked for deletion on the next event loop. And possibly this only
happens between the model and the selection model being deleted?
To avoid a leak when calling `QTreeView.setModel(None)`, this commit switches
to relying on the `model.destroyed` signal to make sure related state is
cleaned up. Upstream bug: https://bugreports.qt.io/browse/QTBUG-49966
When you call `setModel(None)` on a QTreeView it causes a small memory leak
of `QItemSelectionModel` objects created by the QTreeView's child QHeaderView
object.
`QAbstractItemView` will create a new `QItemSelectionModel` whenever a model
is set, even if that model is `None`. When the new model is non-null the
new selection model will be set to be deleted when the new model is, but when
the new model is None the new selection model will be linked to the
static `QAbstractItemModelPrivate::staticEmptyModel()`. Since that empty model
lives forever, so do the related section models, unless callers take care to
clean them up themselves.
Both `QTreeView` and it's child `QHeaderView` implement `QAbstractItemView`
and have this behaviour. For the `QTreeView` we were making sure to delete
the old selection model ourselves (as of fe1215c74d). But for the one
created by the `QHeaderView` we can't get a reference it because
`QTreeView.setModel()` would call `QHeaderView.setModel()` and then
`QHeaderView.setSelectionModel()` right away to assign it's own
selection model to the child, leaving no references to the selection
model created by `QHeaderView.setModel()`.
I was previously using `header.findChildren(QItemSelectionModel)` to clean up
old orphaned selection models, but this approach is a lot simpler!
To observe this for yourself you can plonk something like this in
`set_model()` before the early return and switch between the old and new
implementation and see how it changes behaviour.
header = self.header()
header_children = header.findChildren(QItemSelectionModel)
our_children = self.findChildren(QItemSelectionModel)
print(f"{len(our_children)=} {len(header_children)=}")
You can also observer the selection models accumulating in Gammaray
(https://github.com/KDAB/GammaRay/) if you just open and close the selection a
lot and then filter the object view by "model".
The relevant code is in `QTreeView` and `QAbstractItemView`'s
`setModel()`, `setSlectionModel()` and `modelDestroyed()`. Bot mostly in
the `setModels()` where you can see the relevant signals being connected
and disconnected.
https://code.qt.io/cgit/qt/qtbase.git/tree/src/widgets/itemviews/qtreeview.cpp#n179Fixes: #7947
- Drop trailing comma inside trivial tuple
- Use r"""...""" for string containing ", as \" inside r"..." is taken literally
(I'm surprised it works!)
- Use ['"] instead of ('|")
- Also adjust the inner [^'] to [^'"] for consistency
This reverts commit 7144e72116.
Turns out this adds complexity, but doesn't really help with debugging that
much, and also needs us to guess what the correct ID might be.
We should rather go for a nicer (but more invasive) cleanup by having a separate
IPCConnection class that stores a socket and ID, see #8208.
As of at the very least the latest version the version string looks
like:
const pdfjsVersion = "4.2.67";
So change the regex to allow double quotes too.
I'm trying to update pdf.js in the bleeding edge CI jobs. It complains
that either it can't find PyQt or it can't find yaml depending on how I
invoke tox. Joy. Since dict stuff isn't run by default in this script
hopefully that is the only broken import path and moving it into the
function lets the pdfjs (and ace) bit of the script work fine.
Actually, looking at the stack traces below, both of them are from dict
related code!
tox exec -re bleeding -- python scripts/dev/update_3rdparty.py --gh-token ***
Traceback (most recent call last):
File "/__w/qutebrowser/qutebrowser/scripts/dev/update_3rdparty.py", line 20, in <module>
from scripts import dictcli
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/dictcli.py", line 25, in <module>
from qutebrowser.browser.webengine import spell
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/browser/webengine/spell.py", line 14, in <module>
from qutebrowser.utils import log, message, standarddir
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/utils/message.py", line 15, in <module>
from qutebrowser.qt.core import pyqtSignal, pyqtBoundSignal, QObject
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/qt/core.py", line 17, in <module>
machinery.init_implicit()
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/qt/machinery.py", line 278, in init_implicit
raise NoWrapperAvailableError(info)
qutebrowser.qt.machinery.NoWrapperAvailableError: No Qt wrapper was importable.
python scripts/dev/update_3rdparty.py --gh-token ***
Traceback (most recent call last):
File "/__w/qutebrowser/qutebrowser/scripts/dev/update_3rdparty.py", line 20, in <module>
from scripts import dictcli
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/dictcli.py", line 25, in <module>
from qutebrowser.browser.webengine import spell
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/browser/webengine/spell.py", line 14, in <module>
from qutebrowser.utils import log, message, standarddir
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/utils/message.py", line 17, in <module>
from qutebrowser.utils import usertypes, log
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/utils/usertypes.py", line 16, in <module>
from qutebrowser.utils import log, qtutils, utils
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/utils/qtutils.py", line 39, in <module>
from qutebrowser.utils import usertypes, utils
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/utils/utils.py", line 29, in <module>
import yaml
ModuleNotFoundError: No module named 'yaml'
This installs pdf.js in a selection of CI jobs. Previously the PDF.js
tests (in qutescheme.feature) were skipped in CI because it wasn't
installed anywhere. There has been a couple of recent cases where pdf.js
started depending on javascript features that are too new for even the
most recent QtWebEngine to support. The aim of this commit is to catch
that case. This doesn't add coverage for older webengine releases.
This also incidentally updates the ace editor in these test jobs, since that is
also updated by default by the update_3rdparty script. Hopefully that
doesn't cause issues.
The reasoning for installing on each type of job:
*ubuntu jobs*: not installed - while our main test runs are on ubuntu
there is an upstream issue where many assets used by pdf.js (like icons
used in the toolbar) aren't packaged, see #7904. This causes warning
messages because assets requested via qutescheme can't be found, which
causes the tests to fail. We could a) install pdf.js from source instead
of using the ubuntu one b) ignore the warning logs c) skip this
environment and rely on tests elsewhere. I've chosen to do (c). I don't
see a huge benefit in testing pdf.js across multiple environments if we
aren't using it installed from the OS anyway. We could install from
source but currently all the Qt < 6.5 tests are failing from some other
JS error, and I think fixing that is out of scope of this issue.
*docker Qt6*: installed - the archlinux pdfjs package works fine and we are
only testing the most recent Qt versions because arch users are expected
to stay up to date.
*docker Qt5*: not installed - doesn't support JS features required by
PDF.js. I guess we could install the legacy build from source here. I'm
mostly worried about catching new breakages for this commit though
*windows*: installed - we install pdf.js from source when making a
release so it would be nice to do that in tests too.
*macos*: not installed - the tests that were catching the breakages are
end2end tests which we don't run on mac. And I think there was an
error from the :versions tests here, don't remember.
*bleeding edge*: installed - from source
pdf.js tests fail on Qt < 6.5 with `Uncaught TypeError: Cannot read
properties of null (reading 'mainContainer')`
The `TestPDFJSVersion.test_real_file` unit tests currently fails because
`version._pdfjs_version()` returns `unknown (bundled)`, not sure why. I
think this is pre-existing and it also wasn't being run on CI.
The `test_cleanup()` test for guiprocess was triggering the early timer
warning messages because it was using a VeryCourse timer with a 100ms
interval. Changed it to to a normal Course timer (the default) for that
test.
For the `ipc-timeout` timer we know that is happening in the tests on
windows. It's being logged in lost of e2e tests, the elapsed times it's
logging are between 0 and 0.020s. I'm not sure if it's the right thing
to be changing the log level in production code or marking the messages
as expected in test code.
Since we are now checking for early timer firings in usertype.Timer I
figured why do it in two places, lets call the check method on Timer in
the handler in the IPC class too.
I also removed the platform and version check because while the bug only
happens under those conditions the check to ensure a timer event was
valid should work everywhere. Right? We can add them back of someone
wants but I'm not sure I see a point.
It would be nice if we could filter out invalid timer events in
usertypes.Timer and not require the consuming code to have to do
anything. But that would be more work, so lets wait until more places of
code actually need to care about it, chances are it'll be fixed in Qt
soon enough anyway.
I also think usertypes.Timer should be restarting the time so it gets
called at the proper time. But if we aren't filtering the events that
would mean it gets called twice.
Florian Bruhin
toofar
dependabot[bot]
qutebrowser bot
Bernhard M. Wiedemann