[PATCHv2 1/2] systemd: Do not install anything in /var/volatile

[PATCHv2 1/2] systemd: Do not install anything in /var/volatile

[Peter Kjellerstedt]

/var/log is typically a symbolic link to inside /var/volatile, which
is expected to be empty. Check ${VOLATILE_LOG_DIR} to see if it is ok
to install /var/log.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---

PATCHv2: Added check for ${VOLATILE_LOG_DIR} before removing /var/log.

 meta/recipes-core/systemd/systemd_249.5.bb | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/meta/recipes-core/systemd/systemd_249.5.bb b/meta/recipes-core/systemd/systemd_249.5.bb
index d87f54bf16..c0c1f9bcfc 100644
--- a/meta/recipes-core/systemd/systemd_249.5.bb
+++ b/meta/recipes-core/systemd/systemd_249.5.bb
@@ -270,13 +270,16 @@ do_install() {
 		install -Dm 0755 ${S}/src/systemctl/systemd-sysv-install.SKELETON ${D}${systemd_system_unitdir}d-sysv-install
 	fi
 
-	chown root:systemd-journal ${D}/${localstatedir}/log/journal
-
-	# Delete journal README, as log can be symlinked inside volatile.
-	rm -f ${D}/${localstatedir}/log/README
+	if "${@'true' if oe.types.boolean(d.getVar('VOLATILE_LOG_DIR')) else 'false'}"; then
+		# /var/log is typically a symbolic link to inside /var/volatile,
+		# which is expected to be empty.
+		rm -rf ${D}${localstatedir}/log
+	else
+		chown root:systemd-journal ${D}${localstatedir}/log/journal
 
-	# journal-remote creates this at start
-	rm -rf ${D}/${localstatedir}/log/journal/remote
+		# journal-remote creates this at start
+		rm -rf ${D}${localstatedir}/log/journal/remote
+	fi
 
 	install -d ${D}${systemd_system_unitdir}/graphical.target.wants
 	install -d ${D}${systemd_system_unitdir}/multi-user.target.wants

[PATCHv2 2/2] insane.bbclass: Add a check for directories that are expected to be empty

[Peter Kjellerstedt]

The empty-dirs QA check verifies that all directories specified in
QA_EMPTY_DIRS are empty. It is possible to specify why a directory is
expected to be empty by defining QA_EMPTY_DIRS_RECOMMENDATION:<path>,
which will then be included in the error message if the directory is
not empty. If it is not specified for a directory, then "but it is
expected to be empty" will be used.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---

PATCHv2: No changes.

 meta/classes/insane.bbclass  | 33 ++++++++++++++++++++++++++++++++-
 meta/conf/documentation.conf |  2 ++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index 1e2f1b768a..1675adf6ac 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -37,7 +37,7 @@ ERROR_QA ?= "dev-so debug-deps dev-deps debug-files arch pkgconfig la \
             configure-gettext perllocalpod shebang-size \
             already-stripped installed-vs-shipped ldflags compile-host-path \
             install-host-path pn-overrides unknown-configure-option \
-            useless-rpaths rpaths staticdev \
+            useless-rpaths rpaths staticdev empty-dirs \
             "
 # Add usrmerge QA check based on distro feature
 ERROR_QA:append = "${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', ' usrmerge', '', d)}"
@@ -50,6 +50,21 @@ ALL_QA = "${WARN_QA} ${ERROR_QA}"
 
 UNKNOWN_CONFIGURE_WHITELIST ?= "--enable-nls --disable-nls --disable-silent-rules --disable-dependency-tracking --with-libtool-sysroot --disable-static"
 
+# This is a list of directories that are expected to be empty.
+QA_EMPTY_DIRS ?= " \
+    /dev/pts \
+    /media \
+    /proc \
+    /run \
+    /tmp \
+    ${localstatedir}/run \
+    ${localstatedir}/volatile \
+"
+# It is possible to specify why a directory is expected to be empty by defining
+# QA_EMPTY_DIRS_RECOMMENDATION:<path>, which will then be included in the error
+# message if the directory is not empty. If it is not specified for a directory,
+# then "but it is expected to be empty" will be used.
+
 def package_qa_clean_path(path, d, pkg=None):
     """
     Remove redundant paths from the path for display.  If pkg isn't set then
@@ -885,6 +900,22 @@ def package_qa_check_unlisted_pkg_lics(package, d, messages):
                            "listed in LICENSE" % (package, ' '.join(unlisted)))
     return False
 
+QAPKGTEST[empty-dirs] = "package_qa_check_empty_dirs"
+def package_qa_check_empty_dirs(pkg, d, messages):
+    """
+    Check for the existence of files in directories that are expected to be
+    empty.
+    """
+
+    pkgd = oe.path.join(d.getVar('PKGDEST'), pkg)
+    for dir in (d.getVar('QA_EMPTY_DIRS') or "").split():
+        empty_dir = oe.path.join(pkgd, dir)
+        if os.path.exists(empty_dir) and os.listdir(empty_dir):
+            recommendation = (d.getVar('QA_EMPTY_DIRS_RECOMMENDATION:' + dir) or
+                              "but it is expected to be empty")
+            msg = "%s installs files in %s, %s" % (pkg, dir, recommendation)
+            oe.qa.add_message(messages, "empty-dirs", msg)
+
 def package_qa_check_encoding(keys, encode, d):
     def check_encoding(key, enc):
         sane = True
diff --git a/meta/conf/documentation.conf b/meta/conf/documentation.conf
index 80ad8e10d5..45cd01374a 100644
--- a/meta/conf/documentation.conf
+++ b/meta/conf/documentation.conf
@@ -345,6 +345,8 @@ PYPI_SRC_URI[doc] = "The URI to use to fetch from pypi, default uses pythonhoste
 
 #Q
 
+QA_EMPTY_DIRS[doc] = "A list of directories that are expected to be empty."
+QA_EMPTY_DIRS_RECOMMENDATION[doc] = "This specifies a recommendation for a directory why it must be empty, which will be included in the error message if the directory is not empty."
 QMAKE_PROFILES[doc] = "Specifies your own subset of .pro files to be built for use with qmake."
 
 #R

Re: [OE-core] [PATCHv2 2/2] insane.bbclass: Add a check for directories that are expected to be empty

[Martyn Welch]

On Wed, 2021-10-27 at 20:05 +0200, Peter Kjellerstedt wrote:
> The empty-dirs QA check verifies that all directories specified in
> QA_EMPTY_DIRS are empty. It is possible to specify why a directory is
> expected to be empty by defining QA_EMPTY_DIRS_RECOMMENDATION:<path>,
> which will then be included in the error message if the directory is
> not empty. If it is not specified for a directory, then "but it is
> expected to be empty" will be used.
> 
> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> ---
> 
> PATCHv2: No changes.
> 
>  meta/classes/insane.bbclass  | 33 ++++++++++++++++++++++++++++++++-
>  meta/conf/documentation.conf |  2 ++
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/insane.bbclass
> b/meta/classes/insane.bbclass
> index 1e2f1b768a..1675adf6ac 100644
> --- a/meta/classes/insane.bbclass
> +++ b/meta/classes/insane.bbclass
> @@ -37,7 +37,7 @@ ERROR_QA ?= "dev-so debug-deps dev-deps debug-files
> arch pkgconfig la \
>              configure-gettext perllocalpod shebang-size \
>              already-stripped installed-vs-shipped ldflags compile-
> host-path \
>              install-host-path pn-overrides unknown-configure-option
> \
> -            useless-rpaths rpaths staticdev \
> +            useless-rpaths rpaths staticdev empty-dirs \
>              "
>  # Add usrmerge QA check based on distro feature
>  ERROR_QA:append = "${@bb.utils.contains('DISTRO_FEATURES',
> 'usrmerge', ' usrmerge', '', d)}"
> @@ -50,6 +50,21 @@ ALL_QA = "${WARN_QA} ${ERROR_QA}"
>  
>  UNKNOWN_CONFIGURE_WHITELIST ?= "--enable-nls --disable-nls --
> disable-silent-rules --disable-dependency-tracking --with-libtool-
> sysroot --disable-static"
>  
> +# This is a list of directories that are expected to be empty.
> +QA_EMPTY_DIRS ?= " \
> +    /dev/pts \
> +    /media \
> +    /proc \
> +    /run \
> +    /tmp \
> +    ${localstatedir}/run \
> +    ${localstatedir}/volatile \
> +"
> +# It is possible to specify why a directory is expected to be empty
> by defining
> +# QA_EMPTY_DIRS_RECOMMENDATION:<path>, which will then be included
> in the error
> +# message if the directory is not empty. If it is not specified for
> a directory,
> +# then "but it is expected to be empty" will be used.
> +

One thing I've just noticed is that this is more strictly limited to
ensuring certain directories are empty, rather than ensuring certain
paths (which may include the existance of an empty directory or a
file). Could this be modified to allow wildcard entries, as with the
proposal I originally submitted?

>  def package_qa_clean_path(path, d, pkg=None):
>      """
>      Remove redundant paths from the path for display.  If pkg isn't
> set then
> @@ -885,6 +900,22 @@ def package_qa_check_unlisted_pkg_lics(package,
> d, messages):
>                             "listed in LICENSE" % (package, '
> '.join(unlisted)))
>      return False
>  
> +QAPKGTEST[empty-dirs] = "package_qa_check_empty_dirs"
> +def package_qa_check_empty_dirs(pkg, d, messages):
> +    """
> +    Check for the existence of files in directories that are
> expected to be
> +    empty.
> +    """
> +
> +    pkgd = oe.path.join(d.getVar('PKGDEST'), pkg)
> +    for dir in (d.getVar('QA_EMPTY_DIRS') or "").split():
> +        empty_dir = oe.path.join(pkgd, dir)
> +        if os.path.exists(empty_dir) and os.listdir(empty_dir):
> +            recommendation =
> (d.getVar('QA_EMPTY_DIRS_RECOMMENDATION:' + dir) or
> +                              "but it is expected to be empty")
> +            msg = "%s installs files in %s, %s" % (pkg, dir,
> recommendation)
> +            oe.qa.add_message(messages, "empty-dirs", msg)
> +
>  def package_qa_check_encoding(keys, encode, d):
>      def check_encoding(key, enc):
>          sane = True
> diff --git a/meta/conf/documentation.conf
> b/meta/conf/documentation.conf
> index 80ad8e10d5..45cd01374a 100644
> --- a/meta/conf/documentation.conf
> +++ b/meta/conf/documentation.conf
> @@ -345,6 +345,8 @@ PYPI_SRC_URI[doc] = "The URI to use to fetch from
> pypi, default uses pythonhoste
>  
>  #Q
>  
> +QA_EMPTY_DIRS[doc] = "A list of directories that are expected to be
> empty."
> +QA_EMPTY_DIRS_RECOMMENDATION[doc] = "This specifies a recommendation
> for a directory why it must be empty, which will be included in the
> error message if the directory is not empty."
>  QMAKE_PROFILES[doc] = "Specifies your own subset of .pro files to be
> built for use with qmake."
>  
>  #R
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#157564):
> https://lists.openembedded.org/g/openembedded-core/message/157564
> Mute This Topic: https://lists.openembedded.org/mt/86634477/6512429
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe:
> https://lists.openembedded.org/g/openembedded-core/unsub [
> martyn.welch@collabora.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 

Re: [OE-core] [PATCHv2 2/2] insane.bbclass: Add a check for directories that are expected to be empty

[Martyn Welch]

On Thu, 2021-10-28 at 09:22 +0100, Martyn Welch wrote:
> On Wed, 2021-10-27 at 20:05 +0200, Peter Kjellerstedt wrote:
> > The empty-dirs QA check verifies that all directories specified in
> > QA_EMPTY_DIRS are empty. It is possible to specify why a directory is
> > expected to be empty by defining QA_EMPTY_DIRS_RECOMMENDATION:<path>,
> > which will then be included in the error message if the directory is
> > not empty. If it is not specified for a directory, then "but it is
> > expected to be empty" will be used.
> > 
> > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > ---
> > 
> > PATCHv2: No changes.
> > 
> >  meta/classes/insane.bbclass  | 33 ++++++++++++++++++++++++++++++++-
> >  meta/conf/documentation.conf |  2 ++
> >  2 files changed, 34 insertions(+), 1 deletion(-)
> > 
> > diff --git a/meta/classes/insane.bbclass
> > b/meta/classes/insane.bbclass
> > index 1e2f1b768a..1675adf6ac 100644
> > --- a/meta/classes/insane.bbclass
> > +++ b/meta/classes/insane.bbclass
> > @@ -37,7 +37,7 @@ ERROR_QA ?= "dev-so debug-deps dev-deps debug-files
> > arch pkgconfig la \
> >              configure-gettext perllocalpod shebang-size \
> >              already-stripped installed-vs-shipped ldflags compile-
> > host-path \
> >              install-host-path pn-overrides unknown-configure-option
> > \
> > -            useless-rpaths rpaths staticdev \
> > +            useless-rpaths rpaths staticdev empty-dirs \
> >              "
> >  # Add usrmerge QA check based on distro feature
> >  ERROR_QA:append = "${@bb.utils.contains('DISTRO_FEATURES',
> > 'usrmerge', ' usrmerge', '', d)}"
> > @@ -50,6 +50,21 @@ ALL_QA = "${WARN_QA} ${ERROR_QA}"
> >  
> >  UNKNOWN_CONFIGURE_WHITELIST ?= "--enable-nls --disable-nls --
> > disable-silent-rules --disable-dependency-tracking --with-libtool-
> > sysroot --disable-static"
> >  
> > +# This is a list of directories that are expected to be empty.
> > +QA_EMPTY_DIRS ?= " \
> > +    /dev/pts \
> > +    /media \
> > +    /proc \
> > +    /run \
> > +    /tmp \
> > +    ${localstatedir}/run \
> > +    ${localstatedir}/volatile \
> > +"
> > +# It is possible to specify why a directory is expected to be empty
> > by defining
> > +# QA_EMPTY_DIRS_RECOMMENDATION:<path>, which will then be included
> > in the error
> > +# message if the directory is not empty. If it is not specified for
> > a directory,
> > +# then "but it is expected to be empty" will be used.
> > +
> 
> One thing I've just noticed is that this is more strictly limited to
> ensuring certain directories are empty, rather than ensuring certain
> paths (which may include the existance of an empty directory or a
> file). Could this be modified to allow wildcard entries, as with the
> proposal I originally submitted?
> 

Hmm, actually, this would probably really complicate the
QA_EMPTY_DIRS_RECOMMENDATION functionality and it looks like we really
only check for empty directories, so:

Acked-by: Martyn Welch <martyn.welch@collabora.com>
Tested-by: Martyn Welch <martyn.welch@collabora.com>

> >  def package_qa_clean_path(path, d, pkg=None):
> >      """
> >      Remove redundant paths from the path for display.  If pkg isn't
> > set then
> > @@ -885,6 +900,22 @@ def package_qa_check_unlisted_pkg_lics(package,
> > d, messages):
> >                             "listed in LICENSE" % (package, '
> > '.join(unlisted)))
> >      return False
> >  
> > +QAPKGTEST[empty-dirs] = "package_qa_check_empty_dirs"
> > +def package_qa_check_empty_dirs(pkg, d, messages):
> > +    """
> > +    Check for the existence of files in directories that are
> > expected to be
> > +    empty.
> > +    """
> > +
> > +    pkgd = oe.path.join(d.getVar('PKGDEST'), pkg)
> > +    for dir in (d.getVar('QA_EMPTY_DIRS') or "").split():
> > +        empty_dir = oe.path.join(pkgd, dir)
> > +        if os.path.exists(empty_dir) and os.listdir(empty_dir):
> > +            recommendation =
> > (d.getVar('QA_EMPTY_DIRS_RECOMMENDATION:' + dir) or
> > +                              "but it is expected to be empty")
> > +            msg = "%s installs files in %s, %s" % (pkg, dir,
> > recommendation)
> > +            oe.qa.add_message(messages, "empty-dirs", msg)
> > +
> >  def package_qa_check_encoding(keys, encode, d):
> >      def check_encoding(key, enc):
> >          sane = True
> > diff --git a/meta/conf/documentation.conf
> > b/meta/conf/documentation.conf
> > index 80ad8e10d5..45cd01374a 100644
> > --- a/meta/conf/documentation.conf
> > +++ b/meta/conf/documentation.conf
> > @@ -345,6 +345,8 @@ PYPI_SRC_URI[doc] = "The URI to use to fetch from
> > pypi, default uses pythonhoste
> >  
> >  #Q
> >  
> > +QA_EMPTY_DIRS[doc] = "A list of directories that are expected to be
> > empty."
> > +QA_EMPTY_DIRS_RECOMMENDATION[doc] = "This specifies a recommendation
> > for a directory why it must be empty, which will be included in the
> > error message if the directory is not empty."
> >  QMAKE_PROFILES[doc] = "Specifies your own subset of .pro files to be
> > built for use with qmake."
> >  
> >  #R
> > 
> > 
> > 
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#157579):
> https://lists.openembedded.org/g/openembedded-core/message/157579
> Mute This Topic: https://lists.openembedded.org/mt/86634477/6512429
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe:
> https://lists.openembedded.org/g/openembedded-core/unsub [martyn.welch@collabora.com
> ]
> -=-=-=-=-=-=-=-=-=-=-=-
> 

Re: [OE-core] [PATCHv2 2/2] insane.bbclass: Add a check for directories that are expected to be empty

[Alexandre Belloni]

Hi Peter,

This seems to fail on the AB:

https://autobuilder.yoctoproject.org/typhoon/#/builders/15/builds/4544/steps/11/logs/stdio

The same failure is seen in other builds too.

On 27/10/2021 20:05:22+0200, Peter Kjellerstedt wrote:
> The empty-dirs QA check verifies that all directories specified in
> QA_EMPTY_DIRS are empty. It is possible to specify why a directory is
> expected to be empty by defining QA_EMPTY_DIRS_RECOMMENDATION:<path>,
> which will then be included in the error message if the directory is
> not empty. If it is not specified for a directory, then "but it is
> expected to be empty" will be used.
> 
> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> ---
> 
> PATCHv2: No changes.
> 
>  meta/classes/insane.bbclass  | 33 ++++++++++++++++++++++++++++++++-
>  meta/conf/documentation.conf |  2 ++
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
> index 1e2f1b768a..1675adf6ac 100644
> --- a/meta/classes/insane.bbclass
> +++ b/meta/classes/insane.bbclass
> @@ -37,7 +37,7 @@ ERROR_QA ?= "dev-so debug-deps dev-deps debug-files arch pkgconfig la \
>              configure-gettext perllocalpod shebang-size \
>              already-stripped installed-vs-shipped ldflags compile-host-path \
>              install-host-path pn-overrides unknown-configure-option \
> -            useless-rpaths rpaths staticdev \
> +            useless-rpaths rpaths staticdev empty-dirs \
>              "
>  # Add usrmerge QA check based on distro feature
>  ERROR_QA:append = "${@bb.utils.contains('DISTRO_FEATURES', 'usrmerge', ' usrmerge', '', d)}"
> @@ -50,6 +50,21 @@ ALL_QA = "${WARN_QA} ${ERROR_QA}"
>  
>  UNKNOWN_CONFIGURE_WHITELIST ?= "--enable-nls --disable-nls --disable-silent-rules --disable-dependency-tracking --with-libtool-sysroot --disable-static"
>  
> +# This is a list of directories that are expected to be empty.
> +QA_EMPTY_DIRS ?= " \
> +    /dev/pts \
> +    /media \
> +    /proc \
> +    /run \
> +    /tmp \
> +    ${localstatedir}/run \
> +    ${localstatedir}/volatile \
> +"
> +# It is possible to specify why a directory is expected to be empty by defining
> +# QA_EMPTY_DIRS_RECOMMENDATION:<path>, which will then be included in the error
> +# message if the directory is not empty. If it is not specified for a directory,
> +# then "but it is expected to be empty" will be used.
> +
>  def package_qa_clean_path(path, d, pkg=None):
>      """
>      Remove redundant paths from the path for display.  If pkg isn't set then
> @@ -885,6 +900,22 @@ def package_qa_check_unlisted_pkg_lics(package, d, messages):
>                             "listed in LICENSE" % (package, ' '.join(unlisted)))
>      return False
>  
> +QAPKGTEST[empty-dirs] = "package_qa_check_empty_dirs"
> +def package_qa_check_empty_dirs(pkg, d, messages):
> +    """
> +    Check for the existence of files in directories that are expected to be
> +    empty.
> +    """
> +
> +    pkgd = oe.path.join(d.getVar('PKGDEST'), pkg)
> +    for dir in (d.getVar('QA_EMPTY_DIRS') or "").split():
> +        empty_dir = oe.path.join(pkgd, dir)
> +        if os.path.exists(empty_dir) and os.listdir(empty_dir):
> +            recommendation = (d.getVar('QA_EMPTY_DIRS_RECOMMENDATION:' + dir) or
> +                              "but it is expected to be empty")
> +            msg = "%s installs files in %s, %s" % (pkg, dir, recommendation)
> +            oe.qa.add_message(messages, "empty-dirs", msg)
> +
>  def package_qa_check_encoding(keys, encode, d):
>      def check_encoding(key, enc):
>          sane = True
> diff --git a/meta/conf/documentation.conf b/meta/conf/documentation.conf
> index 80ad8e10d5..45cd01374a 100644
> --- a/meta/conf/documentation.conf
> +++ b/meta/conf/documentation.conf
> @@ -345,6 +345,8 @@ PYPI_SRC_URI[doc] = "The URI to use to fetch from pypi, default uses pythonhoste
>  
>  #Q
>  
> +QA_EMPTY_DIRS[doc] = "A list of directories that are expected to be empty."
> +QA_EMPTY_DIRS_RECOMMENDATION[doc] = "This specifies a recommendation for a directory why it must be empty, which will be included in the error message if the directory is not empty."
>  QMAKE_PROFILES[doc] = "Specifies your own subset of .pro files to be built for use with qmake."
>  
>  #R

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#157564): https://lists.openembedded.org/g/openembedded-core/message/157564
> Mute This Topic: https://lists.openembedded.org/mt/86634477/3617179
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com