[OE-core] [PATCH] systemd: Enable coredump if it is in PACKAGECONFIG

[Khem Raj]

On 7/16/18 5:00 PM, Andre McCurdy wrote:
> On Mon, Jul 16, 2018 at 4:22 PM, Alistair Francis <alistair23 at gmail.com> wrote:
>> On Mon, Jul 16, 2018 at 4:19 PM, Andre McCurdy <armccurdy at gmail.com> wrote:
>>> On Mon, Jul 16, 2018 at 3:50 PM, Alistair Francis
>>> <alistair.francis at wdc.com> wrote:
>>>> Previously if coredump was enabled in PACKAGECONFIG the user would still
>>>> need to modify the conf file to enable it. Now the config will be
>>>> enabled if coredump is enalbed in the PACKAGECONFIG.
                             ^^^^^^^
another typo

>>>>
>>>> Signed-off-by: Alistair Francis <alistair.francis at wdc.com>
>>>> ---
>>>>   .../systemd/systemd/coredump.conf             | 21 +++++++++++++++++++
>>>>   meta/recipes-core/systemd/systemd_237.bb      |  5 +++++
>>>>   2 files changed, 26 insertions(+)
>>>>   create mode 100644 meta/recipes-core/systemd/systemd/coredump.conf
>>>>
>>>> diff --git a/meta/recipes-core/systemd/systemd/coredump.conf b/meta/recipes-core/systemd/systemd/coredump.conf
>>>> new file mode 100644
>>>> index 0000000000..4cbc19cdc5
>>>> --- /dev/null
>>>> +++ b/meta/recipes-core/systemd/systemd/coredump.conf
>>>> @@ -0,0 +1,21 @@
>>>> +#  This file is part of systemd.
>>>
>>> If this file is part of systemd, why does it need to be added to
>>> oe-core? Isn't there a copy somewhere in the systemd source directory
>>> which could be used?
>>
>> The file is included by default, but by default the coredump config is disabled.
>>
>> At first I was modifying the systemd source to enable it but I thought
>> that this would be a better approach as it allows explicit editing
>> instead of a diff which needs to be maintained.
> 
> A diff doesn't need to maintained until it fails to apply - and then
> it generally NEEDS to be maintained, or at least reviewed.
> 
> If you unconditionally over-write a file then there's not warning when
> your version becomes stale.
> 
> However, if you don't like patching, then an alternative could be to
> use sed, e.g. something like:
> 
>    sed 's/^#\(\(Storage\|Compress\|ProcessSizeMax\|ExternalSizeMax\).*=.*\)/\1/'
> -i ${D}${sysconfdir}/systemd/coredump.conf
> 
>>>> +#  systemd is free software; you can redistribute it and/or modify it
>>>> +#  under the terms of the GNU Lesser General Public License as published by
>>>> +#  the Free Software Foundation; either version 2.1 of the License, or
>>>> +#  (at your option) any later version.
>>>> +#
>>>> +# Entries in this file show the compile time defaults.
>>>> +# You can change settings by editing this file.
>>>> +# Defaults can be restored by simply deleting this file.
>>>> +#
>>>> +# See coredump.conf(5) for details.
>>>> +
>>>> +[Coredump]
>>>> +Storage=external
>>>> +Compress=yes
>>>> +ProcessSizeMax=2G
>>>> +ExternalSizeMax=2G
>>>> +#JournalSizeMax=767M
>>>> +#MaxUse=
>>>> +#KeepFree=

We also need an easy way to make these values configurable during build, 
since machines may not have these kind of resources to offer and would 
like to use smaller value set.

>>>> diff --git a/meta/recipes-core/systemd/systemd_237.bb b/meta/recipes-core/systemd/systemd_237.bb
>>>> index 3efca33e73..db2848fd65 100644
>>>> --- a/meta/recipes-core/systemd/systemd_237.bb
>>>> +++ b/meta/recipes-core/systemd/systemd_237.bb
>>>> @@ -276,6 +276,11 @@ do_install() {
>>>>                          chown polkitd:root ${D}${datadir}/polkit-1/rules.d
>>>>                  fi
>>>>          fi
>>>> +
>>>> +  # If coredump was enalbed, enable it in the config
>>>
>>> Typo.
>>
>> Thanks, I'll fix it.
>>
>>>
>>>> +  if ${@bb.utils.contains('PACKAGECONFIG', 'coredump', 'true', 'false', d)}; then
>>>> +    install -Dm 0755 ${FILE_DIRNAME}/${BPN}/coredump.conf ${D}${sysconfdir}/systemd/
>>>
>>> Installing from the meta layer directly to ${D} is not the typical
>>> approach and may cause unexpected problems (e.g. the recipe won't get
>>> rebuilt if the file is changed, etc).
>>
>> What is the preferred approach?
> 
> Add the file the SRC_URI and then install from ${WORKDIR}. There are
> many example of that if you read through recipes in oe-core.
> 
>> Alistair
>>
>>>
>>>> +  fi
>>>>   }
>>>>
>>>>
>>>> --
>>>> 2.17.1
>>>>
>>>> --
>>>> _______________________________________________
>>>> Openembedded-core mailing list
>>>> Openembedded-core at lists.openembedded.org
>>>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>>> --
>>> _______________________________________________
>>> Openembedded-core mailing list
>>> Openembedded-core at lists.openembedded.org
>>> http://lists.openembedded.org/mailman/listinfo/openembedded-core