Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement system updates #88

Merged
merged 21 commits into from
Jan 24, 2024
Merged

Implement system updates #88

merged 21 commits into from
Jan 24, 2024

Conversation

leolost2605
Copy link
Member

@leolost2605 leolost2605 commented Jan 22, 2024

Featurewise this should be pretty much on par with what appcenter currently does. In my testing it was very reliable and quite fast since it only has to deal with packagekit and not appstream and/or flatpak.

It will send notifications about updates, restart required and update failed.

This goes towards addressing elementary/appcenter#2107. Since we need packagekit for ubuntudrivers anyways I started with this (and because I know packagekit and don't know ubuntu drivers or the distro agnostic driver management mentioned there. The only thing I could find in this direction was an old solus project 🤷)

@leolost2605 leolost2605 marked this pull request as ready for review January 22, 2024 22:04
@danirabbit
Copy link
Member

Another piece of information I'd like here is last refresh time so we can display a string like we do here: https://github.com/elementary/appcenter/blob/b33d12c6485644512d7e3d52264397562a89f605/src/Views/AppListUpdateView.vala#L256C1-L260C23

Copy link

@Conan-Kudo Conan-Kudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks mostly good, I assume this triggers PackageKit's offline-updates mechanism?

src/Application.vala Outdated Show resolved Hide resolved
@leolost2605
Copy link
Member Author

This looks mostly good, I assume this triggers PackageKit's offline-updates mechanism?

Yup that's right

@leolost2605
Copy link
Member Author

Another piece of information I'd like here is last refresh time so we can display a string like we do here: https://github.com/elementary/appcenter/blob/b33d12c6485644512d7e3d52264397562a89f605/src/Views/AppListUpdateView.vala#L256C1-L260C23

Done however it's not used to delay checks for updates but I also don't think that's necessary or is it?

@danirabbit
Copy link
Member

@leolost2605 I think AppCenter checks when the timer runs if it's been more than 24 hours since the last refresh time and skips if it hasn't so that you don't get it checking multiple times if you've already checked manually. I'm not sure how much it really costs for us to check again 🤷‍♀️

@leolost2605
Copy link
Member Author

@danirabbit yeah I think performance wise it's pretty much negligible if you've got more than 100 kB download.
However I've seen some issues about too frequent update notifications so I wonder whether we should address that?

Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few things mainly about notifications

src/Application.vala Outdated Show resolved Hide resolved
src/Backends/SystemUpdate.vala Show resolved Hide resolved
src/Backends/SystemUpdate.vala Outdated Show resolved Hide resolved
src/Backends/SystemUpdate.vala Outdated Show resolved Hide resolved
src/Backends/SystemUpdate.vala Outdated Show resolved Hide resolved
src/Backends/SystemUpdate.vala Outdated Show resolved Hide resolved
src/Backends/SystemUpdate.vala Outdated Show resolved Hide resolved
src/Backends/SystemUpdate.vala Outdated Show resolved Hide resolved
src/Backends/SystemUpdate.vala Outdated Show resolved Hide resolved
@leolost2605
Copy link
Member Author

Ok this uses systemd timers now. However when testing them with sudo systemctl start io.elementary.settings-daemon.check-for-system-updates.service they fail with this error message:

Failed to set bus address: $DBUS_SESSION_BUS_ADDRESS and $XDG_RUNTIME_DIR not defined (consider using --machine=<user>@.h>

Same with the already existing firmware timers or rather their service files

I did some googling and found it works when installed in the user unit dir and then started with --user but the current version still has system unit dir because I'm pretty sure I missed something obvious since I have pretty much no experience with systemd units.

Btw maybe totally unrelated but both timers also don't get autostarted for me...?

Anybody any ideas here?

@danirabbit
Copy link
Member

@Marukesu or @tintou maybe?

@danirabbit
Copy link
Member

@leolost2605 perhaps we should revert back to the previous timer method and then we can try to switch to systemd timers in a follow up branch so that we're not blocking this feature while we wait for someone with more expertise

@leolost2605
Copy link
Member Author

Done!

Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really happy with this feature! I think we can iterate/expand/etc after merging but this all works as expected. Great work! 🚀

@danirabbit danirabbit merged commit b3ddc3a into master Jan 24, 2024
4 checks passed
@danirabbit danirabbit deleted the system-updates branch January 24, 2024 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants