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

meson: introduce a build option for customizing the install paths of … #29244

Merged

Conversation

fbuihuu
Copy link
Contributor

@fbuihuu fbuihuu commented Sep 20, 2023

…the main config files

This allows distros to install configuration file templates in /usr/lib/systemd for example.

Copy link
Member

@keszybz keszybz left a comment

Choose a reason for hiding this comment

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

The patch doesn't contain much justification, so here is my understanding of why this is useful:

Currently we install "empty" config files in /etc/systemd/. They serve two purposes:

  • the file contains commented-out values that show the default settings
  • it is easier to edit the right file if it is already there, the user doesn't have to type in the path correctly, and the basic file structure is already in place so it's easier to edit.

Things that have happened since this approach was put in place:

  • we support loading of the main config file from /usr and /usr/local, in addition to /etc and /run (or maybe this way always true, maybe we just spruced up the docs), so the actual location of the config file varies.
  • we started supporting drop-ins for config files, and drop-ins are the recommended way to create local configuration overrides.
  • we have systemd-analyze cat-config
  • because of the first two points, systemd-analyze cat-config is much better, because it takes care of finding all the drop-ins and figuring out the precedence. Looking at files manually is still possible of course, but not very convenient.
  • we have systemctl edit so the user does not have to specify any file/drop-in name.
  • systemctl edit shows the (commented out) contents of the existing config, so when a user edits an empty drop-in, they get the correct hints about syntax/option names.

The disadvantages of the current approach with "empty" files in /etc:

  • we clutter up /etc so it's harder to see what the local configuration actually is.
  • if a user edits the file, package updates will not override the file (e.g. systemd.rpm uses %config(noreplace). This means that the "documented defaults" will become stale over time, if the user ever edits the main config file.

Thus, I think that it's reasonable to:

  • install the main config file to /usr/lib so that it serves as reference for syntax and option names and default values and is properly updated on package upgrades
  • recommend to users to always use drop-ins for configuration and systemd-analyze cat-config to view the documentation.

EDIT: cross out the mention of systemctl edit as discussed below. We haven't implemented that yet for config files.

meson.build Show resolved Hide resolved
meson_options.txt Show resolved Hide resolved
@keszybz
Copy link
Member

keszybz commented Sep 21, 2023

With -Dconfigfiledir=/usr/lib/ I see the following in /etc:

So this patch seems to behave as expected.

@bluca
Copy link
Member

bluca commented Sep 21, 2023

The patch doesn't contain much justification, so here is my understanding of why this is useful:

Currently we install "empty" config files in /etc/systemd/. They serve two purposes:

  • the file contains commented-out values that show the default settings
  • it is easier to edit the right file if it is already there, the user doesn't have to type in the path correctly, and the basic file structure is already in place so it's easier to edit.

Things that have happened since this approach was put in place:

  • we support loading of the main config file from /usr and /usr/local, in addition to /etc and /run (or maybe this way always true, maybe we just spruced up the docs), so the actual location of the config file varies.
  • we started supporting drop-ins for config files, and drop-ins are the recommended way to create local configuration overrides.
  • we have systemd-analyze cat-config
  • because of the first two points, systemd-analyze cat-config is much better, because it takes care of finding all the drop-ins and figuring out the precedence. Looking at files manually is still possible of course, but not very convenient.
  • we have systemctl edit so the user does not have to specify any file/drop-in name.
  • systemctl edit shows the (commented out) contents of the existing config, so when a user edits an empty drop-in, they get the correct hints about syntax/option names.

The disadvantages of the current approach with "empty" files in /etc:

  • we clutter up /etc so it's harder to see what the local configuration actually is.
  • if a user edits the file, package updates will not override the file (e.g. systemd.rpm uses %config(noreplace). This means that the "documented defaults" will become stale over time, if the user ever edits the main config file.

Thus, I think that it's reasonable to:

  • install the main config file to /usr/lib so that it serves as reference for syntax and option names and default values and is properly updated on package upgrades
  • recommend to users to always use drop-ins for configuration and systemd-analyze cat-config to view the documentation.

Makes sense to me - @fbuihuu could you please add the above in the commit message so that it's recorded?

@fbuihuu
Copy link
Contributor Author

fbuihuu commented Sep 21, 2023

The patch doesn't contain much justification, so here is my understanding of why this is useful:

Thanks for writing this up.

Things that have happened since this approach was put in place:

[...]

  • we have systemctl edit so the user does not have to specify any file/drop-in name.
  • systemctl edit shows the (commented out) contents of the existing config, so when a user edits an empty drop-in, they get the correct hints about syntax/option names.

I'm a bit confused here since AFAIK systemctl edit works with unit files, not config files. But I think a similar command that would work with config files would make a lot sense. Hence systemd-analyze edit-config systemd/system.conf would show the content of the defaults (commented out) and all the hints about syntax/option names.

With such command and maybe a way to show the defaults (systemd-analyze cat-config --defaults ?) we wouldn't need to install the templates in /usr at all since their sole purpose is to document the defaults.

@YHNdnzj
Copy link
Member

YHNdnzj commented Sep 21, 2023

But I think a similar command that would work with config files would make a lot sense. Hence systemd-analyze edit-config systemd/system.conf would show the content of the defaults (commented out) and all the hints about syntax/option names.

With such command and maybe a way to show the defaults (systemd-analyze cat-config --defaults ?) we wouldn't need to install the templates in /usr at all since their sole purpose is to document the defaults.

Indeed. This seems more graceful to me.

@fbuihuu
Copy link
Contributor Author

fbuihuu commented Sep 21, 2023

  • we started supporting drop-ins for config files, and drop-ins are the recommended way to create local configuration overrides.

Indeed it's the recommended way because (IMU) using drop-ins exclusively is the only way for packages shipped by downstream to safely customize the defaults without risking to override users' customization. That is the reason why we want to get rid of the main config files (even if they're "empty) and use drop-ins exclusively.

@medhefgo
Copy link
Contributor

  • install the main config file to /usr/lib so that it serves as reference for syntax and option names and default values and is properly updated on package upgrades

Imho, if it's all commented out anyway (aka, systemd doesn't derive it's defaults from any files but has them built in) it should really go into /usr/share/doc. Other programs do that too for their example confs.

@bluca
Copy link
Member

bluca commented Sep 21, 2023

  • install the main config file to /usr/lib so that it serves as reference for syntax and option names and default values and is properly updated on package upgrades

Imho, if it's all commented out anyway (aka, systemd doesn't derive it's defaults from any files but has them built in) it should really go into /usr/share/doc. Other programs do that too for their example confs.

Because when we change defaults downstream it's very easy to just patch that file and uncomment the line, and then it will automatically work as expected

@fbuihuu
Copy link
Contributor Author

fbuihuu commented Sep 21, 2023

Because when we change defaults downstream it's very easy to just patch that file and uncomment the line, and then it will automatically work as expected

until a user creates his own /etc/systemd/xxx.conf (regardless of which setting is overriden).

@medhefgo
Copy link
Contributor

Because when we change defaults downstream it's very easy to just patch that file and uncomment the line, and then it will automatically work as expected

But those are two different things. Downstream ought to have a analog to /usr/etc where they can put their stuff. But that should not be the same as the upstream default config file examples.

In fact, I propose to get rid of these. The file formats aren't some NIH syndrome format like pipewire is using, so you don't need some giant default config dump so that users can get an idea of how it works. And all the options and their possible values and defaults are clearly documented in their respective manpages, which are also linked from the exe manpage that consumes them. These example configs are just some half-assed cargo cult bs that should stop (especially when dropped into /etc). Iff they contain anything important it should probably be moved into their respective manpage.

@keszybz
Copy link
Member

keszybz commented Sep 21, 2023

  • systemctl edit shows the (commented out) contents of the existing config, so when a user edits an empty drop-in, they get the correct hints about syntax/option names.

I'm a bit confused here since AFAIK systemctl edit works with unit files, not config files. But I think a similar command that would work with config files would make a lot sense. Hence systemd-analyze edit-config systemd/system.conf would show the content of the defaults (commented out) and all the hints about syntax/option names.

This was a bit of a mental shortcut. We have systemctl edit, machinectl edit, networkctl edit. You are of course correct that we don't have this for config files, but I think we could add something like this later.

With such command and maybe a way to show the defaults (systemd-analyze cat-config --defaults ?) we wouldn't need to install the templates in /usr at all since their sole purpose is to document the defaults.

I'm not sure. The nice thing about the current approach is that without it works uniformly, without any special functionality. In particular, the distro may provide packaged drop-ins, under /usr/lib/, and those will be automatically and correctly shown. It may also override something in the main config file, and then again, this will be automatically and correctly shown by systemd-analyze cat-config. I don't want to add yet-another verb that would require special support in various places.

@fbuihuu
Copy link
Contributor Author

fbuihuu commented Sep 22, 2023

I'm not sure. The nice thing about the current approach is that without it works uniformly, without any special functionality.

Given that the templates might be located in different directories (/etc vs /usr) or even not shipped at all, having a uniform way to retrieve the defaults for all cases sounds appealing to me.

We could even extend systemd-analyze so it actually doesn't show the defaults, which is sometimes easier to spot the actual overrides. Something equivalent to:

# systemd-analyze cat-config systemd/system.conf | egrep '^# /(etc|usr)|^[^#]' 
# /usr/lib/systemd/system.conf
[Manager]
# /usr/lib/systemd/system.conf.d/20-defaults-SUSE.conf
[Manager]
DefaultMemoryAccounting=no
# /etc/systemd/system.conf.d/60-local.conf
[Manager]
DefaultMemoryAccounting=yes

@keszybz
Copy link
Member

keszybz commented Oct 17, 2023

We could even extend systemd-analyze so it actually doesn't show the defaults

See #29553.

@keszybz keszybz force-pushed the build-option-for-main-config-file-paths branch from 503a464 to e37e657 Compare October 17, 2023 11:53
@keszybz
Copy link
Member

keszybz commented Oct 17, 2023

I force-pushed the commit with an updated commit message.

@YHNdnzj
Copy link
Member

YHNdnzj commented Oct 17, 2023

This would resolve #18420.

@fbuihuu
Copy link
Contributor Author

fbuihuu commented Oct 17, 2023

This would resolve #18420.

Yes indeed, I wasn't aware that someone already asked for moving the templates in /usr.

@bluca bluca removed the please-review PR is ready for (re-)review by a maintainer label Oct 17, 2023
@bluca bluca added the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Oct 17, 2023
@fbuihuu fbuihuu force-pushed the build-option-for-main-config-file-paths branch from e37e657 to 7fe6301 Compare October 17, 2023 13:46
This allows distros to install configuration file templates in /usr/lib/systemd
for example.

Currently we install "empty" config files in /etc/systemd/. They serve two
purposes:

- The file contains commented-out values that show the default settings.
- It is easier to edit the right file if it is already there, the user doesn't
  have to type in the path correctly, and the basic file structure is already in
  place so it's easier to edit.

Things that have happened since this approach was put in place:

- We started supporting drop-ins for config files, and drop-ins are the
  recommended way to create local configuration overrides.
- We have systemd-analyze cat-config which takes care of iterating over
  all possible locations (/etc, /run, /usr, /usr/local) and figuring out
  the right file.
- Because of the first two points, systemd-analyze cat-config is much better,
  because it takes care of finding all the drop-ins and figuring out the
  precedence. Looking at files manually is still possible of course, but not
  very convenient.

The disadvantages of the current approach with "empty" files in /etc:

- We clutter up /etc so it's harder to see what the local configuration actually is.
- If a user edits the file, package updates will not override the file (e.g.
  systemd.rpm uses %config(noreplace). This means that the "documented defaults"
  will become stale over time, if the user ever edits the main config file.

Thus, I think that it's reasonable to:

- Install the main config file to /usr/lib so that it serves as reference for
  syntax and option names and default values and is properly updated on package
  upgrades.
- Recommend to users to always use drop-ins for configuration and
  systemd-analyze cat-config to view the documentation.

This setting makes this change opt-in.

Fixes systemd#18420.

[zjs: add more text to the description]
@fbuihuu fbuihuu force-pushed the build-option-for-main-config-file-paths branch from 7fe6301 to a5cab7b Compare October 17, 2023 13:47
@keszybz
Copy link
Member

keszybz commented Oct 17, 2023

We discussed this today during a live meeting and the conclusion was to merge this. -Dinstall-sysconfdir=false can be used to create an installation without the only-comment files. To make this nicer we can add a meson install tag, but let's do that in a separate pull request since it's just a convenience feature.

@keszybz keszybz merged commit 6495361 into systemd:main Oct 17, 2023
45 of 48 checks passed
@keszybz keszybz removed the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Oct 17, 2023
@fbuihuu fbuihuu deleted the build-option-for-main-config-file-paths branch October 18, 2023 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants