https://bugs.kde.org/show_bug.cgi?id=521682

--- Comment #61 from Anthony Lee <[email protected]> ---
It was suggested so I've done some work on investigating this issue.

The issue seems to be as follows (and this is what I put in my commit message):

Before: `StandardBackendUpdater::refreshUpdateable()` skipped
`setSettingUp(false)` for a backend that invalidated before setup, but
`isFetchingUpdates()` tested only `fetchingUpdatesProgress() != 100` and
`m_settingUp`, so both being false let the Updates page finish normally.

Now: commit `401d8e26ac4509a7538c4382daf6063bc40c36a8` appended `||
!m_hasBeenPopulated` to that test while leaving the `if (m_settingUp)` guard
that prevents flow into that flag's only write site (`setSettingUp()`), so in
this scenario `m_hasBeenPopulated` is never written and stays false for the
life of the process, stranding `isFetchingUpdates()` true and leaving the page
on a filled bar reading "Fetching updates…".

Fix: the patch calls `setSettingUp(false)` unconditionally in the branch that
stands an updater down when its backend has reported itself permanently
invalid, so the flag is finally set and `fetchingChanged()` is emitted for
`ResourcesUpdatesModel::refreshFetching()` to re-evaluate, which clears
`isFetching` and lets the page proceed as it did before the regression.

The fix could look like this (in patch form)

> ---
>  libdiscover/resources/StandardBackendUpdater.cpp | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libdiscover/resources/StandardBackendUpdater.cpp 
> b/libdiscover/resources/StandardBackendUpdater.cpp
> index 54f786ab1..cbcd75d1c 100644
> --- a/libdiscover/resources/StandardBackendUpdater.cpp
> +++ b/libdiscover/resources/StandardBackendUpdater.cpp
> @@ -175,10 +175,12 @@ void StandardBackendUpdater::refreshUpdateable()
>  {
>      if (!m_backend->isValid()) {
>          qWarning() << "Invalidated backend, deactivating" << 
> m_backend->name();
> -        if (m_settingUp) {
> -            setSettingUp(false);
> +        const bool wasSettingUp = m_settingUp;
> +        setSettingUp(false);
> +        if (wasSettingUp) {
>              Q_EMIT progressingChanged(isProgressing());
>          }
> +        Q_EMIT fetchingChanged();
>          return;
>      }
>  
> -- 
> 2.55.0

The above is based on patching 6.7.3  6f1960f62 .
You can find the original commit here
https://invent.kde.org/orthogonaleety/discover/-/commit/71230e3f284ddc30d721eebe91f89064ad218d10

Now I'm not a dev, so I've used a software support approach of localisation and
risk managing the limitations while working with Claude.
I'm 100% satisfied with the fix for 'my issue', but I'm only 75% 'confident' in
it against the entirety of this bug, and I'm 0% confident it doesn't have a
side effect I haven't projected and don't understand, and can't test.

- I have built it.
- And tested it and it works to fix my issue when executing  `plasma-discover
--backends packagekit,flatpak,fwupd --mode update` (so including my problematic
fwupd backend.)

However, I don't have a fix written for master branch. Claude static analysis
reveals the patch would cherry-pick onto master without conflict, and the logic
of the fix still applies there - but it would have no effect. Master,
apparently moved this state into a cache, so the fix's logic as-is would still
leave a stale, still problematic state being tested.

I will continue to look at developing a fix for master, satisfying myself (not
just claude) that there aren't any side effects, but I'm not a dev, so no one
hold their breath or wait on that.
In lieu of such, I hope this is able to shortcut someone to a fix even so.

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to