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

update: Make autopruned refs automatically removed #5632

Merged
merged 1 commit into from Jan 8, 2024

Conversation

TingPing
Copy link
Member

In order to maintain a system over time update automatically removes any EOL runtimes that are unused.

This extends it to also remove any autopruned refs. In practice this means removing no longer used driver versions as the system is updated.

Closes #5261

@TingPing TingPing force-pushed the pgriffis/remove-autoprune-on-update branch from 2d7182a to e1d072c Compare December 20, 2023 18:51
Copy link
Member

@GeorgesStavracas GeorgesStavracas 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 small comment, not a full review

common/flatpak-dir-private.h Outdated Show resolved Hide resolved
@TingPing TingPing force-pushed the pgriffis/remove-autoprune-on-update branch 2 times, most recently from b9503c2 to 95f3fb6 Compare December 22, 2023 00:47
Copy link
Member

@GeorgesStavracas GeorgesStavracas left a comment

Choose a reason for hiding this comment

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

Some indentation comments only. The core logic of the patch seems correct to me.

Have you considered adding a test for this? I suppose it's going to be difficult to test it, but who knows

common/flatpak-dir.c Outdated Show resolved Hide resolved
common/flatpak-dir.c Outdated Show resolved Hide resolved
common/flatpak-dir.c Show resolved Hide resolved
common/flatpak-dir.c Outdated Show resolved Hide resolved
@TingPing TingPing force-pushed the pgriffis/remove-autoprune-on-update branch from 95f3fb6 to 13413aa Compare December 25, 2023 00:19
@TingPing
Copy link
Member Author

Have you considered adding a test for this? I suppose it's going to be difficult to test it, but who knows

Turned out not so bad, added a test.

@TingPing TingPing force-pushed the pgriffis/remove-autoprune-on-update branch from 13413aa to cb7ad42 Compare December 25, 2023 00:24
Copy link
Member

@GeorgesStavracas GeorgesStavracas left a comment

Choose a reason for hiding this comment

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

LGTM

@GeorgesStavracas
Copy link
Member

@smcv how do you feel about this one?

@smcv
Copy link
Collaborator

smcv commented Jan 2, 2024

@smcv how do you feel about this one?

I mainly focus on the sandboxing layer of Flatpak, and I don't have a particularly good overview of the package management layer.

Copy link
Collaborator

@smcv smcv left a comment

Choose a reason for hiding this comment

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

This seems reasonable as far as I can tell, except for some gboolean logic issues.

common/flatpak-dir.c Outdated Show resolved Hide resolved
common/flatpak-dir.c Outdated Show resolved Hide resolved
common/flatpak-dir.c Outdated Show resolved Hide resolved
common/flatpak-installation.c Outdated Show resolved Hide resolved
@smcv smcv requested a review from alexlarsson January 2, 2024 12:08
In order to maintain a system over time update automatically removes any EOL runtimes that are unused.

This extends it to also remove any autopruned refs. In practice this means removing no longer used driver versions as the system is updated.

Closes #5261
@TingPing TingPing force-pushed the pgriffis/remove-autoprune-on-update branch from cb7ad42 to cbf666d Compare January 4, 2024 21:39
@TingPing TingPing merged commit 8c26798 into main Jan 8, 2024
9 checks passed
@TingPing TingPing deleted the pgriffis/remove-autoprune-on-update branch January 8, 2024 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Triaged
Status: Done
3 participants