Let's separate out the unit files copied from attached portable service
image files from the admin's own files. Let's introduce
/etc/systemd/system.attached/ + /run/systemd/system.attached/ for the
files of portable services, and leave /etc/systemd/system/ and
/run/systemd/system/ for the admin.
These lines are generally out-of-date, incomplete and unnecessary. With
SPDX and git repository much more accurate and fine grained information
about licensing and authorship is available, hence let's drop the
per-file copyright notice. Of course, removing copyright lines of others
is problematic, hence this commit only removes my own lines and leaves
all others untouched. It might be nicer if sooner or later those could
go away too, making git the only and accurate source of authorship
information.
This part of the copyright blurb stems from the GPL use recommendations:
https://www.gnu.org/licenses/gpl-howto.en.html
The concept appears to originate in times where version control was per
file, instead of per tree, and was a way to glue the files together.
Ultimately, we nowadays don't live in that world anymore, and this
information is entirely useless anyway, as people are very welcome to
copy these files into any projects they like, and they shouldn't have to
change bits that are part of our copyright header for that.
hence, let's just get rid of this old cruft, and shorten our codebase a
bit.
When we look into a portable service image it might contain the unit
files in split-usr directories rather than merged-usr directories as on
the host. Hence, let#s add a flag that checking all dirs can be forced.
Previously we were a bit sloppy with the index and size types of arrays,
we'd regularly use unsigned. While I don't think this ever resulted in
real issues I think we should be more careful there and follow a
stricter regime: unless there's a strong reason not to use size_t for
array sizes and indexes, size_t it should be. Any allocations we do
ultimately will use size_t anyway, and converting forth and back between
unsigned and size_t will always be a source of problems.
Note that on 32bit machines "unsigned" and "size_t" are equivalent, and
on 64bit machines our arrays shouldn't grow that large anyway, and if
they do we have a problem, however that kind of overly large allocation
we have protections for usually, but for overflows we do not have that
so much, hence let's add it.
So yeah, it's a story of the current code being already "good enough",
but I think some extra type hygiene is better.
This patch tries to be comprehensive, but it probably isn't and I missed
a few cases. But I guess we can cover that later as we notice it. Among
smaller fixes, this changes:
1. strv_length()' return type becomes size_t
2. the unit file changes array size becomes size_t
3. DNS answer and query array sizes become size_t
Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=76745
We check the same condition at various places. Let's add a trivial,
common helper for this, and use it everywhere.
It's not going to make things much faster or much shorter, but I think a
lot more readable
Files which are installed as-is (any .service and other unit files, .conf
files, .policy files, etc), are left as is. My assumption is that SPDX
identifiers are not yet that well known, so it's better to retain the
extended header to avoid any doubt.
I also kept any copyright lines. We can probably remove them, but it'd nice to
obtain explicit acks from all involved authors before doing that.
This macro will read a pointer of any type, return it, and set the
pointer to NULL. This is useful as an explicit concept of passing
ownership of a memory area between pointers.
This takes inspiration from Rust:
https://doc.rust-lang.org/std/option/enum.Option.html#method.take
and was suggested by Alan Jenkins (@sourcejedi).
It drops ~160 lines of code from our codebase, which makes me like it.
Also, I think it clarifies passing of ownership, and thus helps
readability a bit (at least for the initiated who know the new macro)
$ diff -u <(old/systemd-analyze --user unit-paths) <(new/systemd-analyze --user unit-paths)|colordiff
--- /proc/self/fd/14 2018-02-08 14:36:34.190046129 +0100
+++ /proc/self/fd/15 2018-02-08 14:36:34.190046129 +0100
@@ -1,5 +1,5 @@
-/home/zbyszek/.config/systemd/system.control
-/run/user/1000/systemd/system.control
+/home/zbyszek/.config/systemd/user.control
+/run/user/1000/systemd/user.control
/run/user/1000/systemd/transient
...
Strictly speaking, online upgrades of user instances through daemon-reexec will
be broken. We can get away with this since
a) reexecs of the user instance are not commonly done, at least package upgrade
scripts don't do this afawk.
b) cgroups aren't delegateable on cgroupsv1 there's little reason to use "systemctl
set-property" for --user mode
It's not good if the paths are in different order. With --user, we expect
more paths, but it must be a strict superset, and the order for the ones
that appear in both sets must be the same.
$ diff -u <(build/systemd-analyze --global unit-paths) <(build/systemd-analyze --user unit-paths)|colordiff
--- /proc/self/fd/14 2018-02-08 14:11:45.425353107 +0100
+++ /proc/self/fd/15 2018-02-08 14:11:45.426353116 +0100
@@ -1,6 +1,17 @@
+/home/zbyszek/.config/systemd/system.control
+/run/user/1000/systemd/system.control
+/run/user/1000/systemd/transient
+/run/user/1000/systemd/generator.early
+/home/zbyszek/.config/systemd/user
/etc/systemd/user
+/run/user/1000/systemd/user
/run/systemd/user
+/run/user/1000/systemd/generator
+/home/zbyszek/.local/share/systemd/user
+/home/zbyszek/.local/share/flatpak/exports/share/systemd/user
+/var/lib/flatpak/exports/share/systemd/user
/usr/local/share/systemd/user
/usr/share/systemd/user
/usr/local/lib/systemd/user
/usr/lib/systemd/user
+/run/user/1000/systemd/generator.late
A test is added so that we don't regress on this.
This doesn't matter that much, because set-property --global does not work,
so at least those paths wouldn't be used automatically. It is still possible
to create such snippets manually, so we better fix this.
This changes the unit search path logic to never drop the transient and
control directories from the unit search path. This is necessary as we
add new entries to both during runtime, due to the "systemctl
set-property" and transient unit logic.
Previously, the "transient" directory was created during early boot to
deal with this, but the "control" directories were not covered like
that. Creating the control directories early at boot is not possible
however, as /etc might be read-only then, and we do define a persistent
control directory. Hence, let's create these dirs on-demand when we need
them, and make sure the search path clean-up logic never drops them from
the search path even if they are initially missing.
(Also, always create these paths properly labelled)
The advantage is that is the name is mispellt, cpp will warn us.
$ git grep -Ee "conf.set\('(HAVE|ENABLE)_" -l|xargs sed -r -i "s/conf.set\('(HAVE|ENABLE)_/conf.set10('\1_/"
$ git grep -Ee '#ifn?def (HAVE|ENABLE)' -l|xargs sed -r -i 's/#ifdef (HAVE|ENABLE)/#if \1/; s/#ifndef (HAVE|ENABLE)/#if ! \1/;'
$ git grep -Ee 'if.*defined\(HAVE' -l|xargs sed -i -r 's/defined\((HAVE_[A-Z0-9_]*)\)/\1/g'
$ git grep -Ee 'if.*defined\(ENABLE' -l|xargs sed -i -r 's/defined\((ENABLE_[A-Z0-9_]*)\)/\1/g'
+ manual changes to meson.build
squash! build-sys: use #if Y instead of #ifdef Y everywhere
v2:
- fix incorrect setting of HAVE_LIBIDN2
We would not consider symlinks in /etc/systemd/user/*.{wants,requires}/
towards the user unit being "enabled", because the symlinks were not
located in "config" paths. But this is confusing to users, since those units
are clearly enabled and will be started. So let's muddle the definition of
enablement a bit to include the paths only accessible to root when looking for
enabled user units.
Fixes#4432.
When running through systemd-analyze verify or with --test, we would
not run generators (environment or unit). But at the end, we would nuke
the generator dirs anyway.
Simplify things by actually running generators of both types, but redirecting
their output to a temporary directory. This has the advantage that we test more
code, and the verification is more complete.
Since now we are not touching the real generator directories, we also don't
delete them, which fixes#5609.
Why the strange name: the prefix is necessary to follow our own advice that
environment generators should have numerical prefixes. I also put -d- in the
name because otherwise the name was very easy to mistake with
systemd.environment-generator. This additional letter clarifies that this
on special generator that supports environment.d files.
So far, if either $HOME or $XDG_RUNTIME_DIR is not set we wouldn't use
either, and fail acquire_config_dirs() and acquire_control_dirs() in
their entireties. With this change, let's make use of the variables we
can acquire, and don't bother with the other.
Specifically this means: in both acquire_config_dirs() and
acquire_control_dirs() handle ENXIO from user_config_dir() and
user_runtime_dir() directly, instead of propagating it up and handling
it in the caller.
Let's use get_home_dir() for figuring out the home directory, so that
there's a good chance we succeed figuring out unit locations even if
$HOME isn't set.
Fixes: #5260
CID #1354671: char **l would be leaked.
Also rename l to paths, to make the code easier to read,
and do strv deduplication immediately when extending. No need to allocate
strings to remove them a few lines down.
The SysV compat code checks whether there's a native unit file before looking
for a SysV init script. Since the newest rework generated units will show up in
the unit path, and hence the checks ended up assuming that there always was a
native unit file for each init script: the generated one.
With this change the generated unit file directory is suppressed from the
search path when this check is done, to avoid the confusion.
Let's keep the code that manipulates LookupPaths together, and move
generator_binary_paths() to the end of the .h and .c files, since it is not
strictly related to that.
This patch adds a concept of a "control" unit file directory, which is supposed
to be used as place for unit file drop-ins created by "systemctl set-property"
(note that this directory is not actually hooked up to "systemctl set-property"
yet, that's coming in a later patch).
The rationale for this: previously changes made by the user and by "systemctl
set-property" were done in the same directory, which made semantics very
unclear: the changes made by "systemctl set-property" were applied instantly,
and their drop-ins only written to not lose settings on a later "systemctl
daemon-reload", while drop-ins made by the user would only be in effect after
"systemctl daemon-reload". This is particular problematic as the changes made
by "systemctl set-property" would really apply immediately without any respect
for the unit search path. This meant that using "set-property" could have an
effect that is lsot as soon as "daemon-reload" is issued, in case there was a
"later" drop-in already in place.
With this change the directories are seperated, and the "control" directory
always takes the highest priority of all, to avoid any confusion.
This is too confusing, as this funciton returns the paths to the generator
binaries, while usually when we refer to the just the "generator path" we mean
the generated unit files. Let's clean this up.
Let's modernize these calls a bit.
Also, don't call them from user_dirs() anymore, as we already have both dirs in
the list a second time via the persistent_config and runtime_config function
parameters.
Previously, transient units were created below the normal runtime directory
/run/systemd/system. With this change they are created in a special transient
directory /run/systemd/transient, which only contains data for transient units.
This clarifies the life-cycle of transient units, and makes clear they are
distinct from user-provided runtime units. In particular, users may now
extend transient units via /run/systemd/system, without systemd interfering
with the life-cycle of these files.
This change also adds code so that when a transient unit exits only the
drop-ins in this new directory are removed, but nothing else.
Fixes: #2139
