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.

Sign up for GitHub

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

Jump to bottom

do not enable a service disabled by the user #1849

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from
Open

do not enable a service disabled by the user #1849

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
266408e
do not enable a service disabled by the user
OniriCorpe May 23, 2024
9525023
also handle 'reload_or_restart' and 'restart'
OniriCorpe May 23, 2024
8bdaedf
Update helpers/systemd
OniriCorpe May 23, 2024
e8c6abb
Update helpers/systemd
OniriCorpe May 23, 2024
cfcbeb3
enable the $app service during install
OniriCorpe May 23, 2024
66f8e31
automatically ignore the service in diagnosis if it has been deactiva…
OniriCorpe Jun 29, 2024
c410b70
Revert "automatically ignore the service in diagnosis if it has been …
OniriCorpe Jun 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion helpers/systemd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ ynh_add_systemd_config() {

ynh_add_config --template="$template" --destination="/etc/systemd/system/$service.service"

systemctl enable $service --quiet
# do not enable a service disabled by the user
if ! yunohost service status $service | grep -q "start_on_boot: disabled"; then
systemctl enable $service --quiet
fi
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm I think that part should be entirely removed ... the "start_on_boot" thing is precisely the systemd's enabled/disabled flag, cf https://github.com/YunoHost/yunohost/blob/bookworm/src/service.py#L433

so this code is basically "if the service is enabled, enable it"

Suggested change
# do not enable a service disabled by the user
if ! yunohost service status $service | grep -q "start_on_boot: disabled"; then
systemctl enable $service --quiet
fi

Copy link
Member Author

Choose a reason for hiding this comment

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

So how can we do to not re-enable an intentionally disabled service on update?

Copy link
Member

Choose a reason for hiding this comment

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

We just remove the original systemctl enable $service like you already did ? ;P

Copy link
Member Author

Choose a reason for hiding this comment

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

hummm yes x)
why is he here in the first place?

Copy link
Member

Choose a reason for hiding this comment

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

Because we did not really think about the use case of "the user wants the service to be disabled" i think

Copy link
Member Author

Choose a reason for hiding this comment

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

ah, but now if a packages uses ynh_add_systemd_config at installation, the service will not be enabled :/

Copy link
Member Author

Choose a reason for hiding this comment

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

the issue is during upgrade, not install

systemctl daemon-reload
}

Expand Down Expand Up @@ -88,6 +91,12 @@ ynh_systemd_action() {
return 0
fi

# do not start a service disabled by the user
if [ "$action" == "start" ] || [ "$action" == "reload_or_restart" ] || [ "$action" == "restart" ] && \
! yunohost service status $service_name | grep -q "start_on_boot: disabled"; then
return 0
fi
OniriCorpe marked this conversation as resolved.
Show resolved Hide resolved

# Start to read the log
if [[ -n "$line_match" ]]; then
local templog="$(mktemp)"
Expand Down
Loading