shared/install: return failure when enablement fails, but process as much as possible

So far we'd issue a warning (before this series, just in the logs on the server
side, and before this commit, on stderr on the caller's side), but return
success. It seems that successfull return was introduced by mistake in
aa0f357fd8 (my fault :( ), which was supposed to
be a refactoring without a functional change. I think it's better to fail,
because if enablement fails, the user will most likely want to diagnose the
issue.

Note that we still do partial enablement, as far as that is possible. So if
e.g. we have [Install] Alias=foo.service foobar, we'll create the symlink
'foo.service', but not 'foobar', since that's not a valid unit name. We'll
print info about the action taken, and about 'foobar' being invalid, and return
failure.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek
2022-03-29 16:17:56 +02:00
parent cbfdbffb61
commit 0d11db5982
2 changed files with 26 additions and 23 deletions
+6 -4
View File
@@ -1794,20 +1794,22 @@ static int install_info_symlink_alias(
q = install_name_printf(scope, info, *s, info->root, &dst);
if (q < 0) {
unit_file_changes_add(changes, n_changes, q, *s, NULL);
return q;
r = r < 0 ? r : q;
continue;
}
q = unit_file_verify_alias(info, dst, &dst_updated, changes, n_changes);
if (q < 0)
if (q < 0) {
r = r < 0 ? r : q;
continue;
}
alias_path = path_make_absolute(dst_updated ?: dst, config_path);
if (!alias_path)
return -ENOMEM;
q = create_symlink(lp, info->path, alias_path, force, changes, n_changes);
if (r == 0)
r = q;
r = r < 0 ? r : q;
}
return r;
+20 -19
View File
@@ -56,19 +56,27 @@ test ! -e "$root/etc/systemd/system/default.target.wants/test1.service"
test ! -e "$root/etc/systemd/system/special.target.requires/test1.service"
: -------aliases----------------------------------------------
"$systemctl" --root="$root" enable test1
test -h "$root/etc/systemd/system/default.target.wants/test1.service"
test -h "$root/etc/systemd/system/special.target.requires/test1.service"
cat >>"$root/etc/systemd/system/test1.service" <<EOF
Alias=test1-goodalias.service
Alias=test1@badalias.service
Alias=test1-badalias.target
Alias=test1-badalias.socket
# we have a series of good, bad, and then good again
Alias=test1-goodalias2.service
EOF
"$systemctl" --root="$root" enable test1 && { echo "Expected failure" >&2; exit 1; }
test -h "$root/etc/systemd/system/default.target.wants/test1.service"
test -h "$root/etc/systemd/system/special.target.requires/test1.service"
test ! -e "$root/etc/systemd/system/test1-goodalias.service"
test -h "$root/etc/systemd/system/test1-goodalias.service"
test ! -e "$root/etc/systemd/system/test1@badalias.service"
test ! -e "$root/etc/systemd/system/test1-badalias.target"
test ! -e "$root/etc/systemd/system/test1-badalias.socket"
test -h "$root/etc/systemd/system/test1-goodalias2.service"
: -------aliases in reeanble----------------------------------
"$systemctl" --root="$root" reenable test1
"$systemctl" --root="$root" reenable test1 && { echo "Expected failure" >&2; exit 1; }
test -h "$root/etc/systemd/system/default.target.wants/test1.service"
test ! -e "$root/etc/systemd/system/test1-goodalias.service"
test -h "$root/etc/systemd/system/test1-goodalias.service"
@@ -328,7 +336,7 @@ Alias=link4alias.service
Alias=link4alias2.service
EOF
"$systemctl" --root="$root" enable 'link4.service'
"$systemctl" --root="$root" enable 'link4.service' && { echo "Expected failure" >&2; exit 1; }
test ! -h "$root/etc/systemd/system/link4.service" # this is our file
test ! -h "$root/etc/systemd/system/link4@.service"
test ! -h "$root/etc/systemd/system/link4@inst.service"
@@ -343,18 +351,20 @@ test ! -h "$root/etc/systemd/system/link4alias.service"
test ! -h "$root/etc/systemd/system/link4alias2.service"
: -------systemctl enable on path to unit file----------------
cat >"$root/etc/systemd/system/link4.service" <<EOF
[Install]
Alias=link4alias.service
Alias=link4alias2.service
EOF
# Apparently this works. I'm not sure what to think.
"$systemctl" --root="$root" enable '/etc/systemd/system/link4.service'
test ! -h "$root/etc/systemd/system/link4.service" # this is our file
test ! -h "$root/etc/systemd/system/link4@.service"
test ! -h "$root/etc/systemd/system/link4@inst.service"
islink "$root/etc/systemd/system/link4alias.service" "/etc/systemd/system/link4.service"
islink "$root/etc/systemd/system/link4alias2.service" "/etc/systemd/system/link4.service"
"$systemctl" --root="$root" disable '/etc/systemd/system/link4.service'
test ! -h "$root/etc/systemd/system/link4.service"
test ! -h "$root/etc/systemd/system/link4@.service"
test ! -h "$root/etc/systemd/system/link4@inst.service"
test ! -h "$root/etc/systemd/system/link4alias.service"
test ! -h "$root/etc/systemd/system/link4alias2.service"
@@ -364,25 +374,18 @@ cat >"$root/etc/systemd/system/link5.service" <<EOF
[Install]
# FIXME: self-alias should be ignored
# Alias=link5.service
Alias=link5@.service
Alias=link5@inst.service
Alias=link5alias.service
Alias=link5alias2.service
EOF
"$systemctl" --root="$root" enable 'link5.service'
test ! -h "$root/etc/systemd/system/link5.service" # this is our file
test ! -h "$root/etc/systemd/system/link5@.service"
test ! -h "$root/etc/systemd/system/link5@inst.service"
# FIXME/CLARIFYME: will systemd think that link5alias2, link5alias, link5 are all aliases?
# https://github.com/systemd/systemd/issues/661#issuecomment-1057931149
islink "$root/etc/systemd/system/link5alias.service" "/etc/systemd/system/link5.service"
islink "$root/etc/systemd/system/link5alias2.service" "/etc/systemd/system/link5.service"
"$systemctl" --root="$root" disable 'link5.service'
test ! -h "$root/etc/systemd/system/link5.service"
test ! -h "$root/etc/systemd/system/link5@.service"
test ! -h "$root/etc/systemd/system/link5@inst.service"
test ! -h "$root/etc/systemd/system/link5alias.service"
test ! -h "$root/etc/systemd/system/link5alias2.service"
@@ -528,8 +531,6 @@ check_alias % '%' && { echo "Expected failure because % is not legal in unit nam
check_alias z 'z' && { echo "Expected failure because %z is not known" >&2; exit 1; }
# FIXME: if there's an invalid Alias=, we shouldn't preach about empty [Install]
# TODO: repeat the tests above for presets
: -------SYSTEMD_OS_RELEASE relative to root------------------