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

2021-11-11 Thread Anuj Mittal
On Thu, 2021-11-11 at 09:42 +, Peter Kjellerstedt wrote:
> > -Original Message-
> > From: Mittal, Anuj 
> > Sent: den 11 november 2021 04:06
> > To: Peter Kjellerstedt 
> > Subject: Re: [OE-core] [honister][PATCH] insane.bbclass: Add a
> > check for
> > directories that are expected to be empty
> > 
> > On Wed, 2021-11-10 at 20:53 +, Peter Kjellerstedt wrote:
> > > > -Original Message-
> > > > From: openembedded-core@lists.openembedded.org  > > > c...@lists.openembedded.org> On Behalf Of Anuj Mittal
> > > > Sent: den 9 november 2021 15:36
> > > > To: openembedded-core@lists.openembedded.org; Peter
> > > > Kjellerstedt
> > > > 
> > > > Subject: Re: [OE-core] [honister][PATCH] insane.bbclass: Add a
> > > > check for
> > > > directories that are expected to be empty
> > > > 
> > > > On Mon, 2021-11-08 at 18:10 +0100, Peter Kjellerstedt wrote:
> > > > > From: 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:,
> > > > > 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.
> > > > > 
> > > > > Change-Id: Ic61019528f4b22f26e42e78125a99666ae27c7f5
> > > > > Signed-off-by: Peter Kjellerstedt
> > > > > 
> > > > > ---
> > > > > 
> > > > > Compared to the corresponding patch for master, there are two
> > > > > differences:
> > > > > 
> > > > > * "/var/volatile" is not added to QA_EMPTY_DIRS by default.
> > > > > * "empty-dirs" is added to WARN_QA instead of ERROR_QA.
> > > > > 
> > > > > This should make it safe to add this QA test to Honister
> > > > > without
> > > > > introdusing any new QA errors, while still allowing the QA
> > > > > test to be
> > > > > activated for those who wants to use it.
> > > > 
> > > > Does it have to be enabled by default?
> > > 
> > > Well, it doesn't have to be enabled. However, without
> > > /var/volatile in
> > > QA_EMPTY_DIRS, there should be no warnings generated for OE-Core
> > > or
> > > OpenEmbedded so it should not hurt to have it in WARN_QA.
> > 
> > Right, but there could be unexpected warnings for other downstream
> > layers in release versions.
> 
> True, but they would still just be warnings.
> 
> > I think we can add the test but only for people to use it if they'd
> > like to.
> 
> Sure, if you prefer that. Do you want me to send an updated patch, or
> will 
> you take care of it?
> 

Please send an updated patch.

Thanks,

Anuj

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#158205): 
https://lists.openembedded.org/g/openembedded-core/message/158205
Mute This Topic: https://lists.openembedded.org/mt/86910911/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



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

2021-11-11 Thread Peter Kjellerstedt
> -Original Message-
> From: Richard Purdie 
> Sent: den 11 november 2021 11:31
> To: Peter Kjellerstedt ; Mittal, Anuj
> 
> Cc: OE Core (openembedded-core@lists.openembedded.org)  c...@lists.openembedded.org>
> Subject: Re: [OE-core] [honister][PATCH] insane.bbclass: Add a check for
> directories that are expected to be empty
> 
> On Thu, 2021-11-11 at 09:42 +, Peter Kjellerstedt wrote:
> > > -Original Message-
> > > From: Mittal, Anuj 
> > > Sent: den 11 november 2021 04:06
> > > To: Peter Kjellerstedt 
> > > Subject: Re: [OE-core] [honister][PATCH] insane.bbclass: Add a check
> for
> > > directories that are expected to be empty
> > >
> > > On Wed, 2021-11-10 at 20:53 +, Peter Kjellerstedt wrote:
> > > > > -Original Message-
> > > > > From: openembedded-core@lists.openembedded.org  > > > > c...@lists.openembedded.org> On Behalf Of Anuj Mittal
> > > > > Sent: den 9 november 2021 15:36
> > > > > To: openembedded-core@lists.openembedded.org; Peter Kjellerstedt
> > > > > 
> > > > > Subject: Re: [OE-core] [honister][PATCH] insane.bbclass: Add a
> > > > > check for
> > > > > directories that are expected to be empty
> > > > >
> > > > > On Mon, 2021-11-08 at 18:10 +0100, Peter Kjellerstedt wrote:
> > > > > > From: 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:,
> > > > > > 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.
> > > > > >
> > > > > > Change-Id: Ic61019528f4b22f26e42e78125a99666ae27c7f5
> > > > > > Signed-off-by: Peter Kjellerstedt 
> > > > > > ---
> > > > > >
> > > > > > Compared to the corresponding patch for master, there are two
> > > > > > differences:
> > > > > >
> > > > > > * "/var/volatile" is not added to QA_EMPTY_DIRS by default.
> > > > > > * "empty-dirs" is added to WARN_QA instead of ERROR_QA.
> > > > > >
> > > > > > This should make it safe to add this QA test to Honister without
> > > > > > introdusing any new QA errors, while still allowing the QA test
> to be
> > > > > > activated for those who wants to use it.
> > > > >
> > > > > Does it have to be enabled by default?
> > > >
> > > > Well, it doesn't have to be enabled. However, without /var/volatile
> in
> > > > QA_EMPTY_DIRS, there should be no warnings generated for OE-Core or
> > > > OpenEmbedded so it should not hurt to have it in WARN_QA.
> > >
> > > Right, but there could be unexpected warnings for other downstream
> > > layers in release versions.
> >
> > True, but they would still just be warnings.
> 
> In the context of backports, this needs to be handled carefully. You're
> effectively asking for new feature backport here and people aren't happy
> when stable branches suddenly show new warnings.
> 
> If the issue was of huge importance, that would be ok but I'm not sure
> that is the case here, we've managed with the code as is for a long time.
> 
> Cheers,
> 
> Richard

Sure, I'm perfectly fine with the feature being backported without it 
being enabled out-of-the-box as it will still allow me to remove our 
local solution for the same problem.

//Peter


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#158166): 
https://lists.openembedded.org/g/openembedded-core/message/158166
Mute This Topic: https://lists.openembedded.org/mt/86910911/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



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

2021-11-11 Thread Richard Purdie
On Thu, 2021-11-11 at 09:42 +, Peter Kjellerstedt wrote:
> > -Original Message-
> > From: Mittal, Anuj 
> > Sent: den 11 november 2021 04:06
> > To: Peter Kjellerstedt 
> > Subject: Re: [OE-core] [honister][PATCH] insane.bbclass: Add a check for
> > directories that are expected to be empty
> > 
> > On Wed, 2021-11-10 at 20:53 +, Peter Kjellerstedt wrote:
> > > > -Original Message-
> > > > From: openembedded-core@lists.openembedded.org  > > > c...@lists.openembedded.org> On Behalf Of Anuj Mittal
> > > > Sent: den 9 november 2021 15:36
> > > > To: openembedded-core@lists.openembedded.org; Peter Kjellerstedt
> > > > 
> > > > Subject: Re: [OE-core] [honister][PATCH] insane.bbclass: Add a
> > > > check for
> > > > directories that are expected to be empty
> > > > 
> > > > On Mon, 2021-11-08 at 18:10 +0100, Peter Kjellerstedt wrote:
> > > > > From: 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:,
> > > > > 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.
> > > > > 
> > > > > Change-Id: Ic61019528f4b22f26e42e78125a99666ae27c7f5
> > > > > Signed-off-by: Peter Kjellerstedt 
> > > > > ---
> > > > > 
> > > > > Compared to the corresponding patch for master, there are two
> > > > > differences:
> > > > > 
> > > > > * "/var/volatile" is not added to QA_EMPTY_DIRS by default.
> > > > > * "empty-dirs" is added to WARN_QA instead of ERROR_QA.
> > > > > 
> > > > > This should make it safe to add this QA test to Honister without
> > > > > introdusing any new QA errors, while still allowing the QA test to be
> > > > > activated for those who wants to use it.
> > > > 
> > > > Does it have to be enabled by default?
> > > 
> > > Well, it doesn't have to be enabled. However, without /var/volatile in
> > > QA_EMPTY_DIRS, there should be no warnings generated for OE-Core or
> > > OpenEmbedded so it should not hurt to have it in WARN_QA.
> > 
> > Right, but there could be unexpected warnings for other downstream
> > layers in release versions.
> 
> True, but they would still just be warnings.

In the context of backports, this needs to be handled carefully. You're
effectively asking for new feature backport here and people aren't happy when
stable branches suddenly show new warnings.

If the issue was of huge importance, that would be ok but I'm not sure that is
the case here, we've managed with the code as is for a long time.

Cheers,

Richard


-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#158162): 
https://lists.openembedded.org/g/openembedded-core/message/158162
Mute This Topic: https://lists.openembedded.org/mt/86910911/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



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

2021-11-11 Thread Peter Kjellerstedt
> -Original Message-
> From: Mittal, Anuj 
> Sent: den 11 november 2021 04:06
> To: Peter Kjellerstedt 
> Subject: Re: [OE-core] [honister][PATCH] insane.bbclass: Add a check for
> directories that are expected to be empty
> 
> On Wed, 2021-11-10 at 20:53 +, Peter Kjellerstedt wrote:
> > > -Original Message-
> > > From: openembedded-core@lists.openembedded.org  > > c...@lists.openembedded.org> On Behalf Of Anuj Mittal
> > > Sent: den 9 november 2021 15:36
> > > To: openembedded-core@lists.openembedded.org; Peter Kjellerstedt
> > > 
> > > Subject: Re: [OE-core] [honister][PATCH] insane.bbclass: Add a
> > > check for
> > > directories that are expected to be empty
> > >
> > > On Mon, 2021-11-08 at 18:10 +0100, Peter Kjellerstedt wrote:
> > > > From: 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:,
> > > > 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.
> > > >
> > > > Change-Id: Ic61019528f4b22f26e42e78125a99666ae27c7f5
> > > > Signed-off-by: Peter Kjellerstedt 
> > > > ---
> > > >
> > > > Compared to the corresponding patch for master, there are two
> > > > differences:
> > > >
> > > > * "/var/volatile" is not added to QA_EMPTY_DIRS by default.
> > > > * "empty-dirs" is added to WARN_QA instead of ERROR_QA.
> > > >
> > > > This should make it safe to add this QA test to Honister without
> > > > introdusing any new QA errors, while still allowing the QA test to be
> > > > activated for those who wants to use it.
> > >
> > > Does it have to be enabled by default?
> >
> > Well, it doesn't have to be enabled. However, without /var/volatile in
> > QA_EMPTY_DIRS, there should be no warnings generated for OE-Core or
> > OpenEmbedded so it should not hurt to have it in WARN_QA.
> 
> Right, but there could be unexpected warnings for other downstream
> layers in release versions.

True, but they would still just be warnings.

> I think we can add the test but only for people to use it if they'd
> like to.

Sure, if you prefer that. Do you want me to send an updated patch, or will 
you take care of it?

> Thanks,
> 
> Anuj

//Peter

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#158149): 
https://lists.openembedded.org/g/openembedded-core/message/158149
Mute This Topic: https://lists.openembedded.org/mt/86910911/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-



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

2021-11-09 Thread Anuj Mittal
On Mon, 2021-11-08 at 18:10 +0100, Peter Kjellerstedt wrote:
> From: 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:,
> 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.
> 
> Change-Id: Ic61019528f4b22f26e42e78125a99666ae27c7f5
> Signed-off-by: Peter Kjellerstedt 
> ---
> 
> Compared to the corresponding patch for master, there are two
> differences:
> 
> * "/var/volatile" is not added to QA_EMPTY_DIRS by default.
> * "empty-dirs" is added to WARN_QA instead of ERROR_QA.
> 
> This should make it safe to add this QA test to Honister without
> introdusing any new QA errors, while still allowing the QA test to be
> activated for those who wants to use it.

Does it have to be enabled by default?

Thanks,

Anuj

> 
>  meta/classes/insane.bbclass  | 32 +++-
>  meta/conf/documentation.conf |  2 ++
>  2 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/insane.bbclass
> b/meta/classes/insane.bbclass
> index f2d2ca3689..7d4ba28e40 100644
> --- a/meta/classes/insane.bbclass
> +++ b/meta/classes/insane.bbclass
> @@ -27,7 +27,7 @@ WARN_QA ?= " libdir xorg-driver-abi \
>  infodir build-deps src-uri-bad symlink-to-sysroot
> multilib \
>  invalid-packageconfig host-user-contaminated uppercase-
> pn patch-fuzz \
>  mime mime-xdg unlisted-pkg-lics unhandled-features-check
> \
> -    missing-update-alternatives native-last missing-ptest \
> +    missing-update-alternatives native-last missing-ptest
> empty-dirs \
>  "
>  ERROR_QA ?= "dev-so debug-deps dev-deps debug-files arch pkgconfig
> la \
>  perms dep-cmp pkgvarcheck perm-config perm-line perm-
> link \
> @@ -50,6 +50,20 @@ 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 \
> +"
> +# It is possible to specify why a directory is expected to be empty
> by defining
> +# QA_EMPTY_DIRS_RECOMMENDATION:, 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
> @@ -917,6 +931,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 c5a38b0764..d38a88fb49 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 (#158020): 
https://lists.openembedded.org/g/openembedded-core/message/158020
Mute This Topic: https://lists.openembedded.org/mt/86910911/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: