[7/9] ref-manual: add empty-dirs QA check and QA_EMPTY_DIRS*

Message ID 0fb99813b129f7239e08240d22397a50c53ed883.1650591341.git.paul.eggleton@linux.microsoft.com
State New
Headers show
Series [1/9] migration-3.4: add missing entry on EXTRA_USERS_PARAMS | expand

Commit Message

Paul Eggleton April 22, 2022, 1:40 a.m. UTC
From: Paul Eggleton <paul.eggleton@microsoft.com>

This check is new in kirkstone.

Signed-off-by: Paul Eggleton <paul.eggleton@microsoft.com>
---
 documentation/ref-manual/classes.rst   |  5 +++++
 documentation/ref-manual/qa-checks.rst | 10 +++++++++-
 documentation/ref-manual/variables.rst | 15 +++++++++++++++
 3 files changed, 29 insertions(+), 1 deletion(-)

Comments

Peter Kjellerstedt April 22, 2022, 9:50 a.m. UTC | #1
> -----Original Message-----
> From: docs@lists.yoctoproject.org <docs@lists.yoctoproject.org> On Behalf
> Of Paul Eggleton
> Sent: den 22 april 2022 03:41
> To: docs@lists.yoctoproject.org
> Subject: [docs] [PATCH 7/9] ref-manual: add empty-dirs QA check and
> QA_EMPTY_DIRS*
> 
> From: Paul Eggleton <paul.eggleton@microsoft.com>
> 
> This check is new in kirkstone.
> 
> Signed-off-by: Paul Eggleton <paul.eggleton@microsoft.com>
> ---
>  documentation/ref-manual/classes.rst   |  5 +++++
>  documentation/ref-manual/qa-checks.rst | 10 +++++++++-
>  documentation/ref-manual/variables.rst | 15 +++++++++++++++
>  3 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/documentation/ref-manual/classes.rst b/documentation/ref-manual/classes.rst
> index 13cc2bb..a084ace 100644
> --- a/documentation/ref-manual/classes.rst
> +++ b/documentation/ref-manual/classes.rst
> @@ -1039,6 +1039,11 @@ Here are the tests you can list with the :term:`WARN_QA` and
>     cases, such as dynamically loaded modules, these symlinks
>     are needed instead in the main package.
> 
> +-  ``empty-dirs:`` Checks that packages are not installing files to
> +   directories that are normally expected to be empty (such as ``/tmp``)
> +   The list of directories that are checked is specified by the
> +   :term:`QA_EMPTY_DIRS` variable.
> +
>  -  ``file-rdeps:`` Checks that file-level dependencies identified by
>     the OpenEmbedded build system at packaging time are satisfied. For
>     example, a shell script might start with the line ``#!/bin/bash``.
> diff --git a/documentation/ref-manual/qa-checks.rst b/documentation/ref-manual/qa-checks.rst
> index 3364311..e881697 100644
> --- a/documentation/ref-manual/qa-checks.rst
> +++ b/documentation/ref-manual/qa-checks.rst
> @@ -154,7 +154,15 @@ Errors and Warnings
>     ``FILES:${PN}-dbg``. See :term:`FILES` for additional
>     information on :term:`FILES`.
> 
> -
> +.. _qa-check-empty-dirs:
> +
> +-  ``<packagename> installs files in <path>, but it is expected to be empty [empty-dirs]``
> +
> +   The specified package is installing files into a directory that is
> +   normally expected to be empty (such as ``/tmp``). These files may
> +   be more appropriately installed to a different location, or
> +   perhaps alternatively not installed at all.
> +
>  .. _qa-check-arch:
> 
>  -  ``Architecture did not match (<file_arch>, expected <machine_arch>) in <file> [arch]``
> diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
> index 4803578..47c8a8f 100644
> --- a/documentation/ref-manual/variables.rst
> +++ b/documentation/ref-manual/variables.rst
> @@ -6084,6 +6084,21 @@ system and gives an overview of their function and contents.
>        In the previous example,
>        the version of the dependency is :term:`PYTHON_PN`.
> 
> +   :term:`QA_EMPTY_DIRS`
> +      Specifies a list of directories that are expected to be empty when
> +      packaging; if ``empty-dirs`` appears in :term:`ERROR_QA` or
> +      :term:`WARN_QA` these will be checked and an error or warning
> +      (respectively) will be produced.
> +
> +      The default :term:`QA_EMPTY_DIRS` value is set in
> +      :ref:`classes/insane.bbclass <ref-classes-insane>`.
> +
> +   :term:`QA_EMPTY_DIRS_RECOMMENDATION`
> +      Specifies a recommendation for a directory why it must be empty,
> +      which will be included in the error message if a specific directory
> +      is found to contain files. Must be overridden with the directory
> +      path to match on.

Add:

      If no recommendation is specified for a directory, then the default 
      "but it is expected to be empty" will be used.

      Example:

      QA_EMPTY_DIRS_RECOMMENDATION:/dev = "but all devices must be created at runtime"
     
(Please add formatting for the example, I don't know rst enough.)

> +
>     :term:`RANLIB`
>        The minimal command and arguments to run ``ranlib``.
> 
> --
> 1.8.3.1

//Peter
Quentin Schulz April 22, 2022, 10:51 a.m. UTC | #2
Hi Paul,

On 4/22/22 03:40, Paul Eggleton wrote:
> From: Paul Eggleton <paul.eggleton@microsoft.com>
> 
> This check is new in kirkstone.
> 
> Signed-off-by: Paul Eggleton <paul.eggleton@microsoft.com>
> ---
>   documentation/ref-manual/classes.rst   |  5 +++++
>   documentation/ref-manual/qa-checks.rst | 10 +++++++++-
>   documentation/ref-manual/variables.rst | 15 +++++++++++++++
>   3 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/documentation/ref-manual/classes.rst b/documentation/ref-manual/classes.rst
> index 13cc2bb..a084ace 100644
> --- a/documentation/ref-manual/classes.rst
> +++ b/documentation/ref-manual/classes.rst
> @@ -1039,6 +1039,11 @@ Here are the tests you can list with the :term:`WARN_QA` and
>      cases, such as dynamically loaded modules, these symlinks
>      are needed instead in the main package.
>   
> +-  ``empty-dirs:`` Checks that packages are not installing files to
> +   directories that are normally expected to be empty (such as ``/tmp``)
> +   The list of directories that are checked is specified by the
> +   :term:`QA_EMPTY_DIRS` variable.
> +
>   -  ``file-rdeps:`` Checks that file-level dependencies identified by
>      the OpenEmbedded build system at packaging time are satisfied. For
>      example, a shell script might start with the line ``#!/bin/bash``.
> diff --git a/documentation/ref-manual/qa-checks.rst b/documentation/ref-manual/qa-checks.rst
> index 3364311..e881697 100644
> --- a/documentation/ref-manual/qa-checks.rst
> +++ b/documentation/ref-manual/qa-checks.rst
> @@ -154,7 +154,15 @@ Errors and Warnings
>      ``FILES:${PN}-dbg``. See :term:`FILES` for additional
>      information on :term:`FILES`.
>   
> -
> +.. _qa-check-empty-dirs:
> +
> +-  ``<packagename> installs files in <path>, but it is expected to be empty [empty-dirs]``
> +
> +   The specified package is installing files into a directory that is
> +   normally expected to be empty (such as ``/tmp``). These files may
> +   be more appropriately installed to a different location, or
> +   perhaps alternatively not installed at all.
> +

Can we reference where those changes need to be made?

To ignore this warning for some dirs: QA_EMPTY_DIRS
To install in a different location: do_install + FILES: variables?

>   .. _qa-check-arch:
>   
>   -  ``Architecture did not match (<file_arch>, expected <machine_arch>) in <file> [arch]``
> diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
> index 4803578..47c8a8f 100644
> --- a/documentation/ref-manual/variables.rst
> +++ b/documentation/ref-manual/variables.rst
> @@ -6084,6 +6084,21 @@ system and gives an overview of their function and contents.
>         In the previous example,
>         the version of the dependency is :term:`PYTHON_PN`.
>   
> +   :term:`QA_EMPTY_DIRS`
> +      Specifies a list of directories that are expected to be empty when
> +      packaging; if ``empty-dirs`` appears in :term:`ERROR_QA` or
> +      :term:`WARN_QA` these will be checked and an error or warning
> +      (respectively) will be produced.
> +
> +      The default :term:`QA_EMPTY_DIRS` value is set in
> +      :ref:`classes/insane.bbclass <ref-classes-insane>`.
> +

we usually just use insane.bbclass and not the path to it in the git 
repo/layer.

> +   :term:`QA_EMPTY_DIRS_RECOMMENDATION`
> +      Specifies a recommendation for a directory why it must be empty,
> +      which will be included in the error message if a specific directory
> +      is found to contain files. Must be overridden with the directory
> +      path to match on.
> +

This is not explicit enough to me, we shall provide an example or 
rephrase the last sentence.

I can suggest:

path to match on::

   QA_EMPTY_DIRS_RECOMMENDATION:/tmp = "/tmp is expected to be a 
temporary directory and emptied upon booting"

or something like that.

Cheers,
Quentin

Patch

diff --git a/documentation/ref-manual/classes.rst b/documentation/ref-manual/classes.rst
index 13cc2bb..a084ace 100644
--- a/documentation/ref-manual/classes.rst
+++ b/documentation/ref-manual/classes.rst
@@ -1039,6 +1039,11 @@  Here are the tests you can list with the :term:`WARN_QA` and
    cases, such as dynamically loaded modules, these symlinks
    are needed instead in the main package.
 
+-  ``empty-dirs:`` Checks that packages are not installing files to
+   directories that are normally expected to be empty (such as ``/tmp``)
+   The list of directories that are checked is specified by the
+   :term:`QA_EMPTY_DIRS` variable.
+
 -  ``file-rdeps:`` Checks that file-level dependencies identified by
    the OpenEmbedded build system at packaging time are satisfied. For
    example, a shell script might start with the line ``#!/bin/bash``.
diff --git a/documentation/ref-manual/qa-checks.rst b/documentation/ref-manual/qa-checks.rst
index 3364311..e881697 100644
--- a/documentation/ref-manual/qa-checks.rst
+++ b/documentation/ref-manual/qa-checks.rst
@@ -154,7 +154,15 @@  Errors and Warnings
    ``FILES:${PN}-dbg``. See :term:`FILES` for additional
    information on :term:`FILES`.
 
-    
+.. _qa-check-empty-dirs:
+
+-  ``<packagename> installs files in <path>, but it is expected to be empty [empty-dirs]``
+
+   The specified package is installing files into a directory that is
+   normally expected to be empty (such as ``/tmp``). These files may
+   be more appropriately installed to a different location, or
+   perhaps alternatively not installed at all.
+
 .. _qa-check-arch:
 
 -  ``Architecture did not match (<file_arch>, expected <machine_arch>) in <file> [arch]``
diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
index 4803578..47c8a8f 100644
--- a/documentation/ref-manual/variables.rst
+++ b/documentation/ref-manual/variables.rst
@@ -6084,6 +6084,21 @@  system and gives an overview of their function and contents.
       In the previous example,
       the version of the dependency is :term:`PYTHON_PN`.
 
+   :term:`QA_EMPTY_DIRS`
+      Specifies a list of directories that are expected to be empty when
+      packaging; if ``empty-dirs`` appears in :term:`ERROR_QA` or
+      :term:`WARN_QA` these will be checked and an error or warning
+      (respectively) will be produced.
+
+      The default :term:`QA_EMPTY_DIRS` value is set in
+      :ref:`classes/insane.bbclass <ref-classes-insane>`.
+
+   :term:`QA_EMPTY_DIRS_RECOMMENDATION`
+      Specifies a recommendation for a directory why it must be empty,
+      which will be included in the error message if a specific directory
+      is found to contain files. Must be overridden with the directory
+      path to match on.
+
    :term:`RANLIB`
       The minimal command and arguments to run ``ranlib``.