[OE-core] [PATCH v2] systemd: Allow custom coredump config options

[Andre McCurdy]

On Mon, Jul 23, 2018 at 4:16 PM, Alistair Francis <alistair23 at gmail.com> wrote:
> On Wed, Jul 18, 2018 at 4:48 PM, Andre McCurdy <armccurdy at gmail.com> wrote:
>> On Wed, Jul 18, 2018 at 3:53 PM, Alistair Francis
>> <alistair.francis at wdc.com> wrote:
>>> If the user has enabled coredump let's allow them to customise the
>>> config options.
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis at wdc.com>
>>> ---
>>>  meta/recipes-core/systemd/systemd_239.bb | 12 ++++++++++++
>>>  1 file changed, 12 insertions(+)
>>>
>>> diff --git a/meta/recipes-core/systemd/systemd_239.bb b/meta/recipes-core/systemd/systemd_239.bb
>>> index 7822548993..a625d060a9 100644
>>> --- a/meta/recipes-core/systemd/systemd_239.bb
>>> +++ b/meta/recipes-core/systemd/systemd_239.bb
>>> @@ -279,6 +279,18 @@ do_install() {
>>>                         chown polkitd:root ${D}${datadir}/polkit-1/rules.d
>>>                 fi
>>>         fi
>>> +
>>> +  # If coredump was enabled, enable it in the config.
>>> +  # This just sets the default, but can be used to customise the values.
>>> +  if ${@bb.utils.contains('PACKAGECONFIG', 'coredump', 'true', 'false', d)}; then
>>> +    sed 's|#Storage.*|Storage=external|g' -i ${D}${sysconfdir}/systemd/coredump.conf
>>> +    sed 's|#Compress.*|Compress=yes|g' -i ${D}${sysconfdir}/systemd/coredump.conf
>>> +    sed 's|#ProcessSizeMax.*|ProcessSizeMax=2G|g' -i ${D}${sysconfdir}/systemd/coredump.conf
>>> +    sed 's|#ExternalSizeMax.*|ExternalSizeMax=2G|g' -i ${D}${sysconfdir}/systemd/coredump.conf
>>> +    sed 's|#JournalSizeMax.*|JournalSizeMax=767M|g' -i ${D}${sysconfdir}/systemd/coredump.conf
>>> +    sed 's|#MaxUse.*|MaxUse=|g' -i ${D}${sysconfdir}/systemd/coredump.conf
>>> +    sed 's|#KeepFree.*|KeepFree=|g' -i ${D}${sysconfdir}/systemd/coredump.conf
>>
>> How does this allow the user to customise the config options?
>
> They can edit the sed command line and have different options.

Editing the sed commands won't help users who prefer to customise
their builds via a .bbappend or by over-riding variables from
local.conf.

For users who edit recipes directly, providing a place-holder may not
be very useful either since those users can edit the recipe to their
liking and add any sed commands etc they need anyway. So this change
would be useful to quite a narrow audience.

>> By forcing the options to the _current_ default values you are
>> creating a maintenance problem - ie you'll still be forcing these
>> values even if/when the upstream defaults are changed.
>
> That is a good point, is there a usual way to allow edits to config
> files like this?

There are a few different ways. If an upstream reference config file
is really unusable then it might be OK to patch it from the recipe.

Perhaps the ideal case though is for the configure process to provide
control over any key values etc in the files which get installed. e.g.
instead of shipping and installing a fixed "options.conf" file, the
upstream source would ship an "options.conf.in" file which would get
transformed into "options.conf" as part of running the configure
process, based on configure options or auto detection etc in the
configure script. Configuring a build via configure options is
generally the preferred approach (better than patching or running sed
on output files etc).