All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH][RFC] insane.bbclass: Enhance file-rdeps QA check
@ 2015-01-28 23:49 Alejandro Hernandez
  2015-01-29 14:18 ` Saul Wold
  0 siblings, 1 reply; 4+ messages in thread
From: Alejandro Hernandez @ 2015-01-28 23:49 UTC (permalink / raw)
  To: openembedded-core

This is a proposed solution for [YOCTO #7126]

For the first item , I simply added the symlink-to-sysroot check to the QA_WARN

this gave me the QA warning as I belive it was supposed to.

For the second item, I changed filerdepends from set() to dict(), hence methods

for adding or deleting items had to change, I believe dicts are slower than sets

but I had to keep track of the key:value relationship, since sets are unordered

a better way didnt occur to me, all this to flag the issue along with the

offending file accordingly, due to this rdep_rprovides isnt needed anymore.

Signed-off-by: Alejandro Hernandez <alejandro.hernandez@linux.intel.com>
---
 meta/classes/insane.bbclass | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index 143ec46..542346a 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -34,7 +34,7 @@ WARN_QA ?= "ldflags useless-rpaths rpaths staticdev libdir xorg-driver-abi \
 ERROR_QA ?= "dev-so debug-deps dev-deps debug-files arch pkgconfig la \
             perms dep-cmp pkgvarcheck perm-config perm-line perm-link \
             split-strip packages-list pkgv-undefined var-undefined \
-            version-going-backwards \
+            version-going-backwards  symlink-to-sysroot\
             "
 
 ALL_QA = "${WARN_QA} ${ERROR_QA}"
@@ -604,7 +604,6 @@ def package_qa_check_symlink_to_sysroot(path, name, d, elf, messages):
             if target.startswith(tmpdir):
                 trimmed = path.replace(os.path.join (d.getVar("PKGDEST", True), name), "")
                 messages["symlink-to-sysroot"] = "Symlink %s in %s points to TMPDIR" % (trimmed, name)
-
 def package_qa_check_license(workdir, d):
     """
     Check for changes in the license files 
@@ -803,13 +802,14 @@ def package_qa_check_rdepends(pkg, pkgdest, skip, taskdeps, packages, d):
             if bb.data.inherits_class('nativesdk', d):
                 ignored_file_rdeps |= set(['/bin/bash', '/usr/bin/perl'])
             # For Saving the FILERDEPENDS
-            filerdepends = set()
+            filerdepends = {}
             rdep_data = oe.packagedata.read_subpkgdata(pkg, d)
             for key in rdep_data:
                 if key.startswith("FILERDEPENDS_"):
                     for subkey in rdep_data[key].split():
-                        filerdepends.add(subkey)
-            filerdepends -= ignored_file_rdeps
+                        if subkey not in ignored_file_rdeps:
+                            # We already know it starts with FILERDEPENDS_
+                            filerdepends[subkey] = key[13:]
 
             if filerdepends:
                 next = rdepends
@@ -841,31 +841,27 @@ def package_qa_check_rdepends(pkg, pkgdest, skip, taskdeps, packages, d):
                 # case there is a RDEPENDS_pkg = "python" in the recipe.
                 for py in [ d.getVar('MLPREFIX', True) + "python", "python" ]:
                     if py in done:
-                        filerdepends.discard("/usr/bin/python")
+                        filerdepends.pop("/usr/bin/python",None)
                         done.remove(py)
                 for rdep in done:
                     # For Saving the FILERPROVIDES, RPROVIDES and FILES_INFO
-                    rdep_rprovides = set()
                     rdep_data = oe.packagedata.read_subpkgdata(rdep, d)
                     for key in rdep_data:
                         if key.startswith("FILERPROVIDES_") or key.startswith("RPROVIDES_"):
                             for subkey in rdep_data[key].split():
-                                rdep_rprovides.add(subkey)
+                                filerdepends.pop(subkey,None)
                         # Add the files list to the rprovides
                         if key == "FILES_INFO":
                             # Use eval() to make it as a dict
                             for subkey in eval(rdep_data[key]):
-                                rdep_rprovides.add(subkey)
-                    filerdepends -= rdep_rprovides
+                                filerdepends.pop(subkey,None)
                     if not filerdepends:
                         # Break if all the file rdepends are met
                         break
-                    else:
-                        # Clear it for the next loop
-                        rdep_rprovides.clear()
             if filerdepends:
-                error_msg = "%s requires %s, but no providers in its RDEPENDS" % \
-                            (pkg, ', '.join(str(e) for e in filerdepends))
+                for key in filerdepends:
+                    error_msg = "%s contained in package %s requires %s, but no providers found in its RDEPENDS" % \
+                            (filerdepends[key],pkg, key)
                 sane = package_qa_handle_error("file-rdeps", error_msg, d)
 
     return sane
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH][RFC] insane.bbclass: Enhance file-rdeps QA check
  2015-01-28 23:49 [PATCH][RFC] insane.bbclass: Enhance file-rdeps QA check Alejandro Hernandez
@ 2015-01-29 14:18 ` Saul Wold
  2015-01-29 21:20   ` Alejandro Hernandez
  0 siblings, 1 reply; 4+ messages in thread
From: Saul Wold @ 2015-01-29 14:18 UTC (permalink / raw)
  To: Alejandro Hernandez, openembedded-core

On 01/28/2015 03:49 PM, Alejandro Hernandez wrote:
> This is a proposed solution for [YOCTO #7126]
>
> For the first item , I simply added the symlink-to-sysroot check to the QA_WARN
>
> this gave me the QA warning as I belive it was supposed to.
>
> For the second item, I changed filerdepends from set() to dict(), hence methods
>
> for adding or deleting items had to change, I believe dicts are slower than sets
>

Have you done before and after builds to assess the performance 
differences if any?

> but I had to keep track of the key:value relationship, since sets are unordered
>
> a better way didnt occur to me, all this to flag the issue along with the
>
> offending file accordingly, due to this rdep_rprovides isnt needed anymore.
>
What tool are you using that might cause double spacing in your commit 
messages.

> Signed-off-by: Alejandro Hernandez <alejandro.hernandez@linux.intel.com>
> ---
>   meta/classes/insane.bbclass | 26 +++++++++++---------------
>   1 file changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
> index 143ec46..542346a 100644
> --- a/meta/classes/insane.bbclass
> +++ b/meta/classes/insane.bbclass
> @@ -34,7 +34,7 @@ WARN_QA ?= "ldflags useless-rpaths rpaths staticdev libdir xorg-driver-abi \
>   ERROR_QA ?= "dev-so debug-deps dev-deps debug-files arch pkgconfig la \
>               perms dep-cmp pkgvarcheck perm-config perm-line perm-link \
>               split-strip packages-list pkgv-undefined var-undefined \
> -            version-going-backwards \
> +            version-going-backwards  symlink-to-sysroot\
>               "
>
We should also add this to the poky.conf, for the Poky distro since it 
overrides the ERROR_QA setting in meta-yocto/distro/conf/poky.conf. 
That patch should go to poky@yoctoproject.org

>   ALL_QA = "${WARN_QA} ${ERROR_QA}"
> @@ -604,7 +604,6 @@ def package_qa_check_symlink_to_sysroot(path, name, d, elf, messages):
>               if target.startswith(tmpdir):
>                   trimmed = path.replace(os.path.join (d.getVar("PKGDEST", True), name), "")
>                   messages["symlink-to-sysroot"] = "Symlink %s in %s points to TMPDIR" % (trimmed, name)
> -
>   def package_qa_check_license(workdir, d):
>       """
>       Check for changes in the license files
> @@ -803,13 +802,14 @@ def package_qa_check_rdepends(pkg, pkgdest, skip, taskdeps, packages, d):
>               if bb.data.inherits_class('nativesdk', d):
>                   ignored_file_rdeps |= set(['/bin/bash', '/usr/bin/perl'])
>               # For Saving the FILERDEPENDS
> -            filerdepends = set()
> +            filerdepends = {}
>               rdep_data = oe.packagedata.read_subpkgdata(pkg, d)
>               for key in rdep_data:
>                   if key.startswith("FILERDEPENDS_"):
>                       for subkey in rdep_data[key].split():
> -                        filerdepends.add(subkey)
> -            filerdepends -= ignored_file_rdeps
> +                        if subkey not in ignored_file_rdeps:
> +                            # We already know it starts with FILERDEPENDS_
> +                            filerdepends[subkey] = key[13:]
>
>               if filerdepends:
>                   next = rdepends
> @@ -841,31 +841,27 @@ def package_qa_check_rdepends(pkg, pkgdest, skip, taskdeps, packages, d):
>                   # case there is a RDEPENDS_pkg = "python" in the recipe.
>                   for py in [ d.getVar('MLPREFIX', True) + "python", "python" ]:
>                       if py in done:
> -                        filerdepends.discard("/usr/bin/python")
> +                        filerdepends.pop("/usr/bin/python",None)
>                           done.remove(py)
>                   for rdep in done:
>                       # For Saving the FILERPROVIDES, RPROVIDES and FILES_INFO
> -                    rdep_rprovides = set()
>                       rdep_data = oe.packagedata.read_subpkgdata(rdep, d)
>                       for key in rdep_data:
>                           if key.startswith("FILERPROVIDES_") or key.startswith("RPROVIDES_"):
>                               for subkey in rdep_data[key].split():
> -                                rdep_rprovides.add(subkey)
> +                                filerdepends.pop(subkey,None)
Should this be a pop here since you are removing a add()?

>                           # Add the files list to the rprovides
>                           if key == "FILES_INFO":
>                               # Use eval() to make it as a dict
>                               for subkey in eval(rdep_data[key]):
> -                                rdep_rprovides.add(subkey)
> -                    filerdepends -= rdep_rprovides
> +                                filerdepends.pop(subkey,None)
>                       if not filerdepends:
>                           # Break if all the file rdepends are met
>                           break
> -                    else:
> -                        # Clear it for the next loop
> -                        rdep_rprovides.clear()
>               if filerdepends:
> -                error_msg = "%s requires %s, but no providers in its RDEPENDS" % \
> -                            (pkg, ', '.join(str(e) for e in filerdepends))
> +                for key in filerdepends:
> +                    error_msg = "%s contained in package %s requires %s, but no providers found in its RDEPENDS" % \
> +                            (filerdepends[key],pkg, key)
>                   sane = package_qa_handle_error("file-rdeps", error_msg, d)
>
>       return sane
>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH][RFC] insane.bbclass: Enhance file-rdeps QA check
  2015-01-29 14:18 ` Saul Wold
@ 2015-01-29 21:20   ` Alejandro Hernandez
  2015-01-30 17:33     ` Burton, Ross
  0 siblings, 1 reply; 4+ messages in thread
From: Alejandro Hernandez @ 2015-01-29 21:20 UTC (permalink / raw)
  To: Saul Wold, openembedded-core


On 29/01/15 08:18, Saul Wold wrote:

> Have you done before and after builds to assess the performance 
> differences if any?

I checked, found no actual difference on performance.


> What tool are you using that might cause double spacing in your commit 
> messages.

Won't be happening again.

>
>> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
>> index 143ec46..542346a 100644
>> --- a/meta/classes/insane.bbclass
>> +++ b/meta/classes/insane.bbclass
>> @@ -34,7 +34,7 @@ WARN_QA ?= "ldflags useless-rpaths rpaths staticdev 
>> libdir xorg-driver-abi \
>>   ERROR_QA ?= "dev-so debug-deps dev-deps debug-files arch pkgconfig 
>> la \
>>               perms dep-cmp pkgvarcheck perm-config perm-line 
>> perm-link \
>>               split-strip packages-list pkgv-undefined var-undefined \
>> -            version-going-backwards \
>> +            version-going-backwards  symlink-to-sysroot\
>>               "
>>
> We should also add this to the poky.conf, for the Poky distro since it 
> overrides the ERROR_QA setting in meta-yocto/distro/conf/poky.conf. 
> That patch should go to poky@yoctoproject.org

Yes, a patch to poky is also required, I intended to send it once this 
gets accepted.


>> @@ -841,31 +841,27 @@ def package_qa_check_rdepends(pkg, pkgdest, 
>> skip, taskdeps, packages, d):
>>                   # case there is a RDEPENDS_pkg = "python" in the 
>> recipe.
>>                   for py in [ d.getVar('MLPREFIX', True) + "python", 
>> "python" ]:
>>                       if py in done:
>> -                        filerdepends.discard("/usr/bin/python")
>> + filerdepends.pop("/usr/bin/python",None)
>>                           done.remove(py)
>>                   for rdep in done:
>>                       # For Saving the FILERPROVIDES, RPROVIDES and 
>> FILES_INFO
>> -                    rdep_rprovides = set()
>>                       rdep_data = 
>> oe.packagedata.read_subpkgdata(rdep, d)
>>                       for key in rdep_data:
>>                           if key.startswith("FILERPROVIDES_") or 
>> key.startswith("RPROVIDES_"):
>>                               for subkey in rdep_data[key].split():
>> -                                rdep_rprovides.add(subkey)
>> +                                filerdepends.pop(subkey,None)
> Should this be a pop here since you are removing a add()?

Yes, it should be a pop, because it was adding subkey to rdep_rprovides, 
which were substracted from filerdepends afterwards, this worked on sets 
but not on dictionaries, the same result is achieved this way.



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH][RFC] insane.bbclass: Enhance file-rdeps QA check
  2015-01-29 21:20   ` Alejandro Hernandez
@ 2015-01-30 17:33     ` Burton, Ross
  0 siblings, 0 replies; 4+ messages in thread
From: Burton, Ross @ 2015-01-30 17:33 UTC (permalink / raw)
  To: Alejandro Hernandez; +Cc: OE-core

[-- Attachment #1: Type: text/plain, Size: 545 bytes --]

On 29 January 2015 at 21:20, Alejandro Hernandez <
alejandro.hernandez@linux.intel.com> wrote:

> We should also add this to the poky.conf, for the Poky distro since it
>> overrides the ERROR_QA setting in meta-yocto/distro/conf/poky.conf. That
>> patch should go to poky@yoctoproject.org
>>
>
> Yes, a patch to poky is also required, I intended to send it once this
> gets accepted.


Good news is that I'm sending a series that means we don't need to do this
anymore (poky.conf edits the defaults, instead of replacing).

Ross

[-- Attachment #2: Type: text/html, Size: 1107 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-01-30 17:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-28 23:49 [PATCH][RFC] insane.bbclass: Enhance file-rdeps QA check Alejandro Hernandez
2015-01-29 14:18 ` Saul Wold
2015-01-29 21:20   ` Alejandro Hernandez
2015-01-30 17:33     ` Burton, Ross

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.