diff mbox series

[V5] systemd: fix a dead link under /var/log

Message ID 20240227051155.2166683-1-changqing.li@windriver.com
State New
Headers show
Series [V5] systemd: fix a dead link under /var/log | expand

Commit Message

Changqing Li Feb. 27, 2024, 5:11 a.m. UTC
From: Changqing Li <changqing.li@windriver.com>

Commit 6fe23ff31c0 changed README to a symlink to README.logs, and
install README.logs under systemd doc dir.

But for OE, systemd doc dir is splited into package systemd-doc, when it
is not installed on the target, there will be an dead link:
Eg:
root@intel-x86-64:/var/log# ls -l README
lrwxrwxrwx 1 root root 39 Jun 20 08:57 README -> ../../usr/share/doc/systemd/README.logs
root@intel-x86-64:/var/log# ls -l ../../usr/share/doc/systemd/README.logs
ls: cannot access '../../usr/share/doc/systemd/README.logs': No such file or directory

First, package this link into systemd-doc to fix above issue.  Second,
Source link path created by systemd is not correct when VOLATILE_LOG_DIR
is true. Create the symlink using absolute path to cover both condition.

Signed-off-by: Changqing Li <changqing.li@windriver.com>
---
 meta/recipes-core/systemd/systemd_255.1.bb | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

Comments

Alexander Kanavin Feb. 27, 2024, 9:28 a.m. UTC | #1
The patch description doesn't match the patch content. The packaging
doesn't change for README items, and VOLATILE_LOG_DIR isn't checked
anywhere.

Also, don't tweak WATCHDOR_TIMEOUT block, it's not relevant, and hard
to tell if something changed there.

I would rather suggest packaging the needed README item into the same
package that references it from other files. What is beoding done with
sed and grep is *really* hard to understand.


Alex

On Tue, 27 Feb 2024 at 06:11, Changqing Li
<changqing.li@eng.windriver.com> wrote:
>
> From: Changqing Li <changqing.li@windriver.com>
>
> Commit 6fe23ff31c0 changed README to a symlink to README.logs, and
> install README.logs under systemd doc dir.
>
> But for OE, systemd doc dir is splited into package systemd-doc, when it
> is not installed on the target, there will be an dead link:
> Eg:
> root@intel-x86-64:/var/log# ls -l README
> lrwxrwxrwx 1 root root 39 Jun 20 08:57 README -> ../../usr/share/doc/systemd/README.logs
> root@intel-x86-64:/var/log# ls -l ../../usr/share/doc/systemd/README.logs
> ls: cannot access '../../usr/share/doc/systemd/README.logs': No such file or directory
>
> First, package this link into systemd-doc to fix above issue.  Second,
> Source link path created by systemd is not correct when VOLATILE_LOG_DIR
> is true. Create the symlink using absolute path to cover both condition.
>
> Signed-off-by: Changqing Li <changqing.li@windriver.com>
> ---
>  meta/recipes-core/systemd/systemd_255.1.bb | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/meta/recipes-core/systemd/systemd_255.1.bb b/meta/recipes-core/systemd/systemd_255.1.bb
> index 9e09c89355..73ef0f01b2 100644
> --- a/meta/recipes-core/systemd/systemd_255.1.bb
> +++ b/meta/recipes-core/systemd/systemd_255.1.bb
> @@ -382,10 +382,15 @@ do_install() {
>         # add a profile fragment to disable systemd pager with busybox less
>         install -Dm 0644 ${WORKDIR}/systemd-pager.sh ${D}${sysconfdir}/profile.d/systemd-pager.sh
>
> -    if [ -n "${WATCHDOG_TIMEOUT}" ]; then
> -        sed -i -e 's/#RebootWatchdogSec=10min/RebootWatchdogSec=${WATCHDOG_TIMEOUT}/' \
> -            ${D}/${sysconfdir}/systemd/system.conf
> -    fi
> +       if [ -n "${WATCHDOG_TIMEOUT}" ]; then
> +               sed -i -e 's/#RebootWatchdogSec=10min/RebootWatchdogSec=${WATCHDOG_TIMEOUT}/' \
> +                       ${D}/${sysconfdir}/systemd/system.conf
> +       fi
> +
> +       if grep -q '^L /var/log/README' ${D}${nonarch_libdir}/tmpfiles.d/legacy.conf 2>/dev/null; then
> +               sed -i -e '/^L \/var\/log\/README/d' ${D}${nonarch_libdir}/tmpfiles.d/legacy.conf
> +               echo "L ${localstatedir}/log/README - - - - ${datadir}/doc/systemd/README.logs" > ${D}${nonarch_libdir}/tmpfiles.d/legacy-doc.conf
> +       fi
>  }
>
>  python populate_packages:prepend (){
> @@ -622,6 +627,8 @@ FILES:${PN}-udev-rules = "\
>                          ${rootlibexecdir}/udev/rules.d/99-systemd.rules \
>  "
>
> +FILES:${PN}-doc  += "${nonarch_libdir}/tmpfiles.d/legacy-doc.conf"
> +
>  CONFFILES:${PN} = "${sysconfdir}/systemd/coredump.conf \
>         ${sysconfdir}/systemd/journald.conf \
>         ${sysconfdir}/systemd/logind.conf \
> --
> 2.25.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#196242): https://lists.openembedded.org/g/openembedded-core/message/196242
> Mute This Topic: https://lists.openembedded.org/mt/104598170/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Changqing Li Feb. 28, 2024, 3:19 a.m. UTC | #2
On 2/27/24 17:28, Alexander Kanavin wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> The patch description doesn't match the patch content. The packaging
> doesn't change for README items, and VOLATILE_LOG_DIR isn't checked
> anywhere.

This patch is for fixing two problems:

1.

Currently,  There is one line in /usr/lib/tmpfile.d/legacy.conf(this is 
in package systemd),

"L /var/log/README - - - - ../../usr/share/doc/systemd/README.logs"

This line will create an link to "${datadir}/doc/systemd/README.logs" 
during boot,

and ${datadir}/doc/systemd/README.logs is in package systemd-doc. So 
there will be dead link when systemd-doc is not installed.

So that's why I create another legacy-doc.conf packaged into systemd-doc

2.

The symlink in legacy.conf use relative path.  but for oe, 
whenVOLATILE_LOG_DIR is true, /var/log is a link to /var/volatile/log, so

/var/log/README need link to ../../../usr/share/doc/systemd/README.logs,

while VOLATILE_LOG_DIR is false, /var/log is a dir, so /var/log/README

need link to ../../usr/share/doc/systemd/README.logs.

After change it to absolute path, checking of VOLATILE_LOG_DIR is not 
needed.

>
> Also, don't tweak WATCHDOR_TIMEOUT block, it's not relevant, and hard
> to tell if something changed there.
ok
>
> I would rather suggest packaging the needed README item into the same
> package that references it from other files. What is beoding done with
> sed and grep is *really* hard to understand.

I think these is what I am doing.

Any more comments? if not, I can update the description of this patch

to make things more clear.

Thanks

//Changqing

>
>
> Alex
>
> On Tue, 27 Feb 2024 at 06:11, Changqing Li
> <changqing.li@eng.windriver.com>  wrote:
>> From: Changqing Li<changqing.li@windriver.com>
>>
>> Commit 6fe23ff31c0 changed README to a symlink to README.logs, and
>> install README.logs under systemd doc dir.
>>
>> But for OE, systemd doc dir is splited into package systemd-doc, when it
>> is not installed on the target, there will be an dead link:
>> Eg:
>> root@intel-x86-64:/var/log# ls -l README
>> lrwxrwxrwx 1 root root 39 Jun 20 08:57 README -> ../../usr/share/doc/systemd/README.logs
>> root@intel-x86-64:/var/log# ls -l ../../usr/share/doc/systemd/README.logs
>> ls: cannot access '../../usr/share/doc/systemd/README.logs': No such file or directory
>>
>> First, package this link into systemd-doc to fix above issue.  Second,
>> Source link path created by systemd is not correct when VOLATILE_LOG_DIR
>> is true. Create the symlink using absolute path to cover both condition.
>>
>> Signed-off-by: Changqing Li<changqing.li@windriver.com>
>> ---
>>   meta/recipes-core/systemd/systemd_255.1.bb | 15 +++++++++++----
>>   1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/meta/recipes-core/systemd/systemd_255.1.bb b/meta/recipes-core/systemd/systemd_255.1.bb
>> index 9e09c89355..73ef0f01b2 100644
>> --- a/meta/recipes-core/systemd/systemd_255.1.bb
>> +++ b/meta/recipes-core/systemd/systemd_255.1.bb
>> @@ -382,10 +382,15 @@ do_install() {
>>          # add a profile fragment to disable systemd pager with busybox less
>>          install -Dm 0644 ${WORKDIR}/systemd-pager.sh ${D}${sysconfdir}/profile.d/systemd-pager.sh
>>
>> -    if [ -n "${WATCHDOG_TIMEOUT}" ]; then
>> -        sed -i -e 's/#RebootWatchdogSec=10min/RebootWatchdogSec=${WATCHDOG_TIMEOUT}/' \
>> -            ${D}/${sysconfdir}/systemd/system.conf
>> -    fi
>> +       if [ -n "${WATCHDOG_TIMEOUT}" ]; then
>> +               sed -i -e 's/#RebootWatchdogSec=10min/RebootWatchdogSec=${WATCHDOG_TIMEOUT}/' \
>> +                       ${D}/${sysconfdir}/systemd/system.conf
>> +       fi
>> +
>> +       if grep -q '^L /var/log/README' ${D}${nonarch_libdir}/tmpfiles.d/legacy.conf 2>/dev/null; then
>> +               sed -i -e '/^L \/var\/log\/README/d' ${D}${nonarch_libdir}/tmpfiles.d/legacy.conf
>> +               echo "L ${localstatedir}/log/README - - - - ${datadir}/doc/systemd/README.logs" > ${D}${nonarch_libdir}/tmpfiles.d/legacy-doc.conf
>> +       fi
>>   }
>>
>>   python populate_packages:prepend (){
>> @@ -622,6 +627,8 @@ FILES:${PN}-udev-rules = "\
>>                           ${rootlibexecdir}/udev/rules.d/99-systemd.rules \
>>   "
>>
>> +FILES:${PN}-doc  += "${nonarch_libdir}/tmpfiles.d/legacy-doc.conf"
>> +
>>   CONFFILES:${PN} = "${sysconfdir}/systemd/coredump.conf \
>>          ${sysconfdir}/systemd/journald.conf \
>>          ${sysconfdir}/systemd/logind.conf \
>> --
>> 2.25.1
>>
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#196242):https://lists.openembedded.org/g/openembedded-core/message/196242
>> Mute This Topic:https://lists.openembedded.org/mt/104598170/1686489
>> Group Owner:openembedded-core+owner@lists.openembedded.org
>> Unsubscribe:https://lists.openembedded.org/g/openembedded-core/unsub  [alex.kanavin@gmail.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>
Alexander Kanavin Feb. 28, 2024, 9:45 a.m. UTC | #3
On Wed, 28 Feb 2024 at 04:19, Changqing Li <changqing.li@windriver.com> wrote:
> 1.
>
> Currently,  There is one line in /usr/lib/tmpfile.d/legacy.conf(this is in package systemd),
>
> "L /var/log/README - - - - ../../usr/share/doc/systemd/README.logs"
>
> This line will create an link to "${datadir}/doc/systemd/README.logs" during boot,
>
> and ${datadir}/doc/systemd/README.logs is in package systemd-doc. So there will be dead link when systemd-doc is not installed.
>
> So that's why I create another legacy-doc.conf packaged into systemd-doc

Then legacy.conf and README.logs should be placed into the same
package. I would suggest that README.logs just goes into main systemd
package. (the other option is placing legacy.conf into systemd-doc,
but that file is not doc-specific, and should stay in systemd package,
especially if more entries are added to it).

> 2.
>
> The symlink in legacy.conf use relative path.  but for oe, when VOLATILE_LOG_DIR is true, /var/log is a link to /var/volatile/log, so
>
> /var/log/README need link to ../../../usr/share/doc/systemd/README.logs,
>
> while VOLATILE_LOG_DIR is false, /var/log is a dir, so /var/log/README
>
> need link to ../../usr/share/doc/systemd/README.logs.
>
> After change it to absolute path, checking of VOLATILE_LOG_DIR is not needed.

That's ok, but can you make fixing the path to an absolute one a
separate, second commit then?

Thanks,
Alex
Changqing Li Feb. 29, 2024, 5:56 a.m. UTC | #4
On 2/28/24 17:45, Alexander Kanavin wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Wed, 28 Feb 2024 at 04:19, Changqing Li<changqing.li@windriver.com>  wrote:
>> 1.
>>
>> Currently,  There is one line in /usr/lib/tmpfile.d/legacy.conf(this is in package systemd),
>>
>> "L /var/log/README - - - - ../../usr/share/doc/systemd/README.logs"
>>
>> This line will create an link to "${datadir}/doc/systemd/README.logs" during boot,
>>
>> and ${datadir}/doc/systemd/README.logs is in package systemd-doc. So there will be dead link when systemd-doc is not installed.
>>
>> So that's why I create another legacy-doc.conf packaged into systemd-doc
> Then legacy.conf and README.logs should be placed into the same
> package. I would suggest that README.logs just goes into main systemd
> package. (the other option is placing legacy.conf into systemd-doc,
> but that file is not doc-specific, and should stay in systemd package,
> especially if more entries are added to it).

The solution is also good. But package "${docdir}/systemd/README.logs" 
in package systemd may make things more complicated.

since in bitabke .conf, ${PN}-doc is before ${PN} in PACKAGES, and  
FILES:${PN}-doc = "${docdir}  ...". we need to adjust the sequence,

and may have influences to current list of files and directories that 
are placed in a package.

So maybe this patch is more simple and reasonable, and will not have 
other impact.

>
>> 2.
>>
>> The symlink in legacy.conf use relative path.  but for oe, when VOLATILE_LOG_DIR is true, /var/log is a link to /var/volatile/log, so
>>
>> /var/log/README need link to ../../../usr/share/doc/systemd/README.logs,
>>
>> while VOLATILE_LOG_DIR is false, /var/log is a dir, so /var/log/README
>>
>> need link to ../../usr/share/doc/systemd/README.logs.
>>
>> After change it to absolute path, checking of VOLATILE_LOG_DIR is not needed.
> That's ok, but can you make fixing the path to an absolute one a
> separate, second commit then?

if  use solution in this patch,  it is better they are in one commit.  I 
can update description more detail.

Thanks

Changqing

> Thanks,
> Alex
Khem Raj Feb. 29, 2024, 6:04 a.m. UTC | #5
On Wed, Feb 28, 2024 at 9:56 PM Changqing Li
<changqing.li@eng.windriver.com> wrote:
>
>
> On 2/28/24 17:45, Alexander Kanavin wrote:
>
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Wed, 28 Feb 2024 at 04:19, Changqing Li <changqing.li@windriver.com> wrote:
>
> 1.
>
> Currently,  There is one line in /usr/lib/tmpfile.d/legacy.conf(this is in package systemd),
>
> "L /var/log/README - - - - ../../usr/share/doc/systemd/README.logs"
>
> This line will create an link to "${datadir}/doc/systemd/README.logs" during boot,
>
> and ${datadir}/doc/systemd/README.logs is in package systemd-doc. So there will be dead link when systemd-doc is not installed.
>
> So that's why I create another legacy-doc.conf packaged into systemd-doc
>
> Then legacy.conf and README.logs should be placed into the same
> package. I would suggest that README.logs just goes into main systemd
> package. (the other option is placing legacy.conf into systemd-doc,
> but that file is not doc-specific, and should stay in systemd package,
> especially if more entries are added to it).
>
> The solution is also good. But package "${docdir}/systemd/README.logs" in package systemd may make things more complicated.
>
> since in bitabke .conf, ${PN}-doc is before ${PN} in PACKAGES, and  FILES:${PN}-doc = "${docdir}  ...". we need to adjust the sequence,
>
> and may have influences to current list of files and directories that are placed in a package.
>
> So maybe this patch is more simple and reasonable, and will not have other impact.
>
> 2.
>
> The symlink in legacy.conf use relative path.  but for oe, when VOLATILE_LOG_DIR is true, /var/log is a link to /var/volatile/log, so
>
> /var/log/README need link to ../../../usr/share/doc/systemd/README.logs,
>
> while VOLATILE_LOG_DIR is false, /var/log is a dir, so /var/log/README
>
> need link to ../../usr/share/doc/systemd/README.logs.
>
> After change it to absolute path, checking of VOLATILE_LOG_DIR is not needed.
>
> That's ok, but can you make fixing the path to an absolute one a
> separate, second commit then?
>
> if  use solution in this patch,  it is better they are in one commit.  I can update description more detail.
>

You could also explore using -Dcreate-log-dirs=false via EXTRA_OEMESON

> Thanks
>
> Changqing
>
> Thanks,
> Alex
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#196414): https://lists.openembedded.org/g/openembedded-core/message/196414
> Mute This Topic: https://lists.openembedded.org/mt/104598170/1997914
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Changqing Li Feb. 29, 2024, 6:43 a.m. UTC | #6
On 2/29/24 14:04, Khem Raj wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Wed, Feb 28, 2024 at 9:56 PM Changqing Li
> <changqing.li@eng.windriver.com> wrote:
>>
>> On 2/28/24 17:45, Alexander Kanavin wrote:
>>
>> CAUTION: This email comes from a non Wind River email account!
>> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>>
>> On Wed, 28 Feb 2024 at 04:19, Changqing Li <changqing.li@windriver.com> wrote:
>>
>> 1.
>>
>> Currently,  There is one line in /usr/lib/tmpfile.d/legacy.conf(this is in package systemd),
>>
>> "L /var/log/README - - - - ../../usr/share/doc/systemd/README.logs"
>>
>> This line will create an link to "${datadir}/doc/systemd/README.logs" during boot,
>>
>> and ${datadir}/doc/systemd/README.logs is in package systemd-doc. So there will be dead link when systemd-doc is not installed.
>>
>> So that's why I create another legacy-doc.conf packaged into systemd-doc
>>
>> Then legacy.conf and README.logs should be placed into the same
>> package. I would suggest that README.logs just goes into main systemd
>> package. (the other option is placing legacy.conf into systemd-doc,
>> but that file is not doc-specific, and should stay in systemd package,
>> especially if more entries are added to it).
>>
>> The solution is also good. But package "${docdir}/systemd/README.logs" in package systemd may make things more complicated.
>>
>> since in bitabke .conf, ${PN}-doc is before ${PN} in PACKAGES, and  FILES:${PN}-doc = "${docdir}  ...". we need to adjust the sequence,
>>
>> and may have influences to current list of files and directories that are placed in a package.
>>
>> So maybe this patch is more simple and reasonable, and will not have other impact.
>>
>> 2.
>>
>> The symlink in legacy.conf use relative path.  but for oe, when VOLATILE_LOG_DIR is true, /var/log is a link to /var/volatile/log, so
>>
>> /var/log/README need link to ../../../usr/share/doc/systemd/README.logs,
>>
>> while VOLATILE_LOG_DIR is false, /var/log is a dir, so /var/log/README
>>
>> need link to ../../usr/share/doc/systemd/README.logs.
>>
>> After change it to absolute path, checking of VOLATILE_LOG_DIR is not needed.
>>
>> That's ok, but can you make fixing the path to an absolute one a
>> separate, second commit then?
>>
>> if  use solution in this patch,  it is better they are in one commit.  I can update description more detail.
>>
> You could also explore using -Dcreate-log-dirs=false via EXTRA_OEMESON

Thanks. I had do some research about turn off CREATE_LOG_DIR,

if turn off CREATE_LOG_DIR, " /var/log/journal/" will not be created.  
By default,  VOLATILE__LOG_DIR = "yes",  this change will have no 
influence, since we will remove content under /var/log.

if VOLATILE_LOG_DIR set to "no", the current default behavior will be 
changed,  log will saved from persistent  to memory.   our default 
Storage set to auto,  if " /var/log/journal/" not exist, the log will

save  in memory /run/log/journal.  Refer [1].  So I did not try to turn 
off it in case current default behavior is changed.

Refer:

[1] https://www.freedesktop.org/software/systemd/man/journald.conf.html

//Changqing

>
>> Thanks
>>
>> Changqing
>>
>> Thanks,
>> Alex
>>
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#196414): https://lists.openembedded.org/g/openembedded-core/message/196414
>> Mute This Topic: https://lists.openembedded.org/mt/104598170/1997914
>> Group Owner: openembedded-core+owner@lists.openembedded.org
>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>
Alexander Kanavin Feb. 29, 2024, 9 a.m. UTC | #7
On Thu, 29 Feb 2024 at 06:56, Changqing Li <changqing.li@windriver.com> wrote:
> The solution is also good. But package "${docdir}/systemd/README.logs" in package systemd may make things more complicated.
>
> since in bitabke .conf, ${PN}-doc is before ${PN} in PACKAGES, and  FILES:${PN}-doc = "${docdir}  ...". we need to adjust the sequence,
>
> and may have influences to current list of files and directories that are placed in a package.
>
> So maybe this patch is more simple and reasonable, and will not have other impact.

That's right. It's possible but not obvious how to override that (I
don't remember that from memory).

> That's ok, but can you make fixing the path to an absolute one a
> separate, second commit then?
>
> if  use solution in this patch,  it is better they are in one commit.  I can update description more detail.

Please no. These are two separate issues, let's deal with them as
separate changes.

Alex
Peter Kjellerstedt Feb. 29, 2024, 11:56 a.m. UTC | #8
> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Alexander Kanavin
> Sent: den 29 februari 2024 10:01
> To: Changqing Li <changqing.li@windriver.com>
> Cc: Changqing Li <changqing.li@eng.windriver.com>; openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH V5] systemd: fix a dead link under /var/log
> 
> On Thu, 29 Feb 2024 at 06:56, Changqing Li <changqing.li@windriver.com> wrote:
> > The solution is also good. But package "${docdir}/systemd/README.logs"
> > in package systemd may make things more complicated.
> >
> > since in bitabke .conf, ${PN}-doc is before ${PN} in PACKAGES, and
> > FILES:${PN}-doc = "${docdir}  ...". we need to adjust the sequence,
> >
> > and may have influences to current list of files and directories that
> > are placed in a package.
> >
> > So maybe this patch is more simple and reasonable, and will not have
> > other impact.
> 
> That's right. It's possible but not obvious how to override that (I
> don't remember that from memory).
> 
> > That's ok, but can you make fixing the path to an absolute one a
> > separate, second commit then?
> >
> > if  use solution in this patch,  it is better they are in one commit.  I
> > can update description more detail.
> 
> Please no. These are two separate issues, let's deal with them as
> separate changes.
> 
> Alex

Given how much churn this patch has generated for a file that probably 
no one will read, how about just not creating it in the first place? 
That is what we do with the following patch:

From 49483effbcb920d097487c58214518f2f3a9d479 Mon Sep 17 00:00:00 2001
From: Yang Lyu <yangl@axis.com>
Date: Mon, 20 Jun 2022 09:07:19 +0200
Subject: [PATCH] Do not create README in log directory

---
 tmpfiles.d/legacy.conf.in | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tmpfiles.d/legacy.conf.in b/tmpfiles.d/legacy.conf.in
index 4f2c0d7c43..62e2ae0986 100644
--- a/tmpfiles.d/legacy.conf.in
+++ b/tmpfiles.d/legacy.conf.in
@@ -12,9 +12,6 @@

 d /run/lock 0755 root root -
 L /var/lock - - - - ../run/lock
-{% if CREATE_LOG_DIRS %}
-L /var/log/README - - - - ../..{{DOC_DIR}}/README.logs
-{% endif %}

 # /run/lock/subsys is used for serializing SysV service execution, and
 # hence without use on SysV-less systems.

//Peter
Changqing Li March 1, 2024, 1:51 a.m. UTC | #9
On 2/29/24 19:56, Peter Kjellerstedt wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>> -----Original Message-----
>> From:openembedded-core@lists.openembedded.org  <openembedded-core@lists.openembedded.org>  On Behalf Of Alexander Kanavin
>> Sent: den 29 februari 2024 10:01
>> To: Changqing Li<changqing.li@windriver.com>
>> Cc: Changqing Li<changqing.li@eng.windriver.com>;openembedded-core@lists.openembedded.org
>> Subject: Re: [OE-core] [PATCH V5] systemd: fix a dead link under /var/log
>>
>> On Thu, 29 Feb 2024 at 06:56, Changqing Li<changqing.li@windriver.com>  wrote:
>>> The solution is also good. But package "${docdir}/systemd/README.logs"
>>> in package systemd may make things more complicated.
>>>
>>> since in bitabke .conf, ${PN}-doc is before ${PN} in PACKAGES, and
>>> FILES:${PN}-doc = "${docdir}  ...". we need to adjust the sequence,
>>>
>>> and may have influences to current list of files and directories that
>>> are placed in a package.
>>>
>>> So maybe this patch is more simple and reasonable, and will not have
>>> other impact.
>> That's right. It's possible but not obvious how to override that (I
>> don't remember that from memory).
>>
>>> That's ok, but can you make fixing the path to an absolute one a
>>> separate, second commit then?
>>>
>>> if  use solution in this patch,  it is better they are in one commit.  I
>>> can update description more detail.
>> Please no. These are two separate issues, let's deal with them as
>> separate changes.
>>
>> Alex
> Given how much churn this patch has generated for a file that probably
> no one will read, how about just not creating it in the first place?
> That is what we do with the following patch:
>
>  From 49483effbcb920d097487c58214518f2f3a9d479 Mon Sep 17 00:00:00 2001
> From: Yang Lyu<yangl@axis.com>
> Date: Mon, 20 Jun 2022 09:07:19 +0200
> Subject: [PATCH] Do not create README in log directory
>
> ---
>   tmpfiles.d/legacy.conf.in | 3 ---
>   1 file changed, 3 deletions(-)
>
> diff --git a/tmpfiles.d/legacy.conf.in b/tmpfiles.d/legacy.conf.in
> index 4f2c0d7c43..62e2ae0986 100644
> --- a/tmpfiles.d/legacy.conf.in
> +++ b/tmpfiles.d/legacy.conf.in
> @@ -12,9 +12,6 @@
>
>   d /run/lock 0755 root root -
>   L /var/lock - - - - ../run/lock
> -{% if CREATE_LOG_DIRS %}
> -L /var/log/README - - - - ../..{{DOC_DIR}}/README.logs
> -{% endif %}
>
>   # /run/lock/subsys is used for serializing SysV service execution, and
>   # hence without use on SysV-less systems.
>
> //Peter

Hi,

This is also my V1 patch solution.  Please see Richard's comments for V1,

he also recommeded turn off CREATE_LOG_DIRS, but this CREATE_LOG_DIRS is 
not only

used here, it will changed log save location default behavior when 
volatile is disabled.

Thanks

Changqing
Khem Raj March 1, 2024, 4:23 a.m. UTC | #10
On Wed, Feb 28, 2024 at 10:43 PM Changqing Li
<changqing.li@windriver.com> wrote:
>
>
> On 2/29/24 14:04, Khem Raj wrote:
> > CAUTION: This email comes from a non Wind River email account!
> > Do not click links or open attachments unless you recognize the sender and know the content is safe.
> >
> > On Wed, Feb 28, 2024 at 9:56 PM Changqing Li
> > <changqing.li@eng.windriver.com> wrote:
> >>
> >> On 2/28/24 17:45, Alexander Kanavin wrote:
> >>
> >> CAUTION: This email comes from a non Wind River email account!
> >> Do not click links or open attachments unless you recognize the sender and know the content is safe.
> >>
> >> On Wed, 28 Feb 2024 at 04:19, Changqing Li <changqing.li@windriver.com> wrote:
> >>
> >> 1.
> >>
> >> Currently,  There is one line in /usr/lib/tmpfile.d/legacy.conf(this is in package systemd),
> >>
> >> "L /var/log/README - - - - ../../usr/share/doc/systemd/README.logs"
> >>
> >> This line will create an link to "${datadir}/doc/systemd/README.logs" during boot,
> >>
> >> and ${datadir}/doc/systemd/README.logs is in package systemd-doc. So there will be dead link when systemd-doc is not installed.
> >>
> >> So that's why I create another legacy-doc.conf packaged into systemd-doc
> >>
> >> Then legacy.conf and README.logs should be placed into the same
> >> package. I would suggest that README.logs just goes into main systemd
> >> package. (the other option is placing legacy.conf into systemd-doc,
> >> but that file is not doc-specific, and should stay in systemd package,
> >> especially if more entries are added to it).
> >>
> >> The solution is also good. But package "${docdir}/systemd/README.logs" in package systemd may make things more complicated.
> >>
> >> since in bitabke .conf, ${PN}-doc is before ${PN} in PACKAGES, and  FILES:${PN}-doc = "${docdir}  ...". we need to adjust the sequence,
> >>
> >> and may have influences to current list of files and directories that are placed in a package.
> >>
> >> So maybe this patch is more simple and reasonable, and will not have other impact.
> >>
> >> 2.
> >>
> >> The symlink in legacy.conf use relative path.  but for oe, when VOLATILE_LOG_DIR is true, /var/log is a link to /var/volatile/log, so
> >>
> >> /var/log/README need link to ../../../usr/share/doc/systemd/README.logs,
> >>
> >> while VOLATILE_LOG_DIR is false, /var/log is a dir, so /var/log/README
> >>
> >> need link to ../../usr/share/doc/systemd/README.logs.
> >>
> >> After change it to absolute path, checking of VOLATILE_LOG_DIR is not needed.
> >>
> >> That's ok, but can you make fixing the path to an absolute one a
> >> separate, second commit then?
> >>
> >> if  use solution in this patch,  it is better they are in one commit.  I can update description more detail.
> >>
> > You could also explore using -Dcreate-log-dirs=false via EXTRA_OEMESON
>
> Thanks. I had do some research about turn off CREATE_LOG_DIR,
>
> if turn off CREATE_LOG_DIR, " /var/log/journal/" will not be created.
> By default,  VOLATILE__LOG_DIR = "yes",  this change will have no
> influence, since we will remove content under /var/log.
>
> if VOLATILE_LOG_DIR set to "no", the current default behavior will be
> changed,  log will saved from persistent  to memory.   our default
> Storage set to auto,  if " /var/log/journal/" not exist, the log will
>
> save  in memory /run/log/journal.  Refer [1].  So I did not try to turn
> off it in case current default behavior is changed.
>

The option -Dcreate-log-dirs will control if systemd build emits
relevant info into generated legacy.conf
or not thats all. Whatever logic you have in OE is high level which
might be covering it across the distro. so
what I am suggesting is to use the meson option to control it when
building systemd, the knob logic
to apply it will depend on OE's logic for log persistence.

> Refer:
>
> [1] https://www.freedesktop.org/software/systemd/man/journald.conf.html
>
> //Changqing
>
> >
> >> Thanks
> >>
> >> Changqing
> >>
> >> Thanks,
> >> Alex
> >>
> >>
> >> -=-=-=-=-=-=-=-=-=-=-=-
> >> Links: You receive all messages sent to this group.
> >> View/Reply Online (#196414): https://lists.openembedded.org/g/openembedded-core/message/196414
> >> Mute This Topic: https://lists.openembedded.org/mt/104598170/1997914
> >> Group Owner: openembedded-core+owner@lists.openembedded.org
> >> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com]
> >> -=-=-=-=-=-=-=-=-=-=-=-
> >>
diff mbox series

Patch

diff --git a/meta/recipes-core/systemd/systemd_255.1.bb b/meta/recipes-core/systemd/systemd_255.1.bb
index 9e09c89355..73ef0f01b2 100644
--- a/meta/recipes-core/systemd/systemd_255.1.bb
+++ b/meta/recipes-core/systemd/systemd_255.1.bb
@@ -382,10 +382,15 @@  do_install() {
 	# add a profile fragment to disable systemd pager with busybox less
 	install -Dm 0644 ${WORKDIR}/systemd-pager.sh ${D}${sysconfdir}/profile.d/systemd-pager.sh
 
-    if [ -n "${WATCHDOG_TIMEOUT}" ]; then
-        sed -i -e 's/#RebootWatchdogSec=10min/RebootWatchdogSec=${WATCHDOG_TIMEOUT}/' \
-            ${D}/${sysconfdir}/systemd/system.conf
-    fi
+	if [ -n "${WATCHDOG_TIMEOUT}" ]; then
+		sed -i -e 's/#RebootWatchdogSec=10min/RebootWatchdogSec=${WATCHDOG_TIMEOUT}/' \
+			${D}/${sysconfdir}/systemd/system.conf
+	fi
+
+	if grep -q '^L /var/log/README' ${D}${nonarch_libdir}/tmpfiles.d/legacy.conf 2>/dev/null; then
+		sed -i -e '/^L \/var\/log\/README/d' ${D}${nonarch_libdir}/tmpfiles.d/legacy.conf
+		echo "L ${localstatedir}/log/README - - - - ${datadir}/doc/systemd/README.logs" > ${D}${nonarch_libdir}/tmpfiles.d/legacy-doc.conf
+	fi
 }
 
 python populate_packages:prepend (){
@@ -622,6 +627,8 @@  FILES:${PN}-udev-rules = "\
                         ${rootlibexecdir}/udev/rules.d/99-systemd.rules \
 "
 
+FILES:${PN}-doc  += "${nonarch_libdir}/tmpfiles.d/legacy-doc.conf"
+
 CONFFILES:${PN} = "${sysconfdir}/systemd/coredump.conf \
 	${sysconfdir}/systemd/journald.conf \
 	${sysconfdir}/systemd/logind.conf \