Skip to content

feat: implement X-RestartIfChanged=false in service activation#438

Merged
picnoir merged 2 commits intomainfrom
feat/restart-if-changed
Apr 10, 2026
Merged

feat: implement X-RestartIfChanged=false in service activation#438
picnoir merged 2 commits intomainfrom
feat/restart-if-changed

Conversation

@jfroche
Copy link
Copy Markdown
Member

@jfroche jfroche commented Mar 26, 2026

Prevents the engine from restarting services like the auto-upgrade service mid-execution when their unit files change.

required for #436

@jfroche jfroche force-pushed the feat/restart-if-changed branch 2 times, most recently from 00a7211 to 29c2665 Compare March 27, 2026 00:58
@jfroche jfroche changed the base branch from pic/refactor-etc-files-linking to main March 27, 2026 00:59
@jfroche jfroche force-pushed the feat/restart-if-changed branch from 29c2665 to 3b57c94 Compare March 27, 2026 01:08
@jfroche jfroche force-pushed the feat/restart-if-changed branch from 3b57c94 to b202740 Compare April 8, 2026 17:05
Copy link
Copy Markdown
Member

@picnoir picnoir left a comment

Choose a reason for hiding this comment

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

Overall 👍

But I feel like the parser needs to do a little bit more than string matching.

return false;
}
if let Some(ref path) = service.store_path {
if unit_has_directive(path, "X-RestartIfChanged=false") {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[Service]
#X-RestartIfChanged=false

😛

Looking at man systemd.syntax, we can also have whitespaces around the equal sign:

Each file is a plain text file divided into sections, with configuration entries in the style key=value. Whitespace immediately before or after the "=" is ignored. Empty lines and lines starting with "#" or ";" are ignored, which may be used commenting.

Maybe check if there's a parser for that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ok I switched to freedesktop_entry_parser

We parse the directive from the unit file text and skip restarting the service if X-RestartIfChanged=false is set

required for #436
Comment thread crates/system-manager-engine/src/activate/services.rs Outdated
@jfroche jfroche force-pushed the feat/restart-if-changed branch from cc5011c to 516bd1c Compare April 10, 2026 16:49
This library has a small overhead and is more robust than the current parsing method and will be used in other places in the future.
@jfroche jfroche force-pushed the feat/restart-if-changed branch from 516bd1c to 4394d8d Compare April 10, 2026 16:54
@picnoir picnoir merged commit 08934c2 into main Apr 10, 2026
6 checks passed
@picnoir picnoir deleted the feat/restart-if-changed branch April 10, 2026 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants