[PATCH 1/1] insane.bbclass: Added QA test for unexpanded ${D}

[PATCH 1/1] insane.bbclass: Added QA test for unexpanded ${D}

[Alejandro Hernandez]

Checks in FILES and pkg_* variables ,solves common mistake
of using ${D} instead of $D and warns the user accordingly.

[YOCTO #6642]

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

diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index 0b45374..419c89b 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -906,6 +906,39 @@ def package_qa_check_deps(pkg, pkgdest, skip, d):
 
     return sane
 
+def package_qa_check_unexpanded_d(d):
+    """
+    Check for unexpanded D variable in pkg_* and FILES
+    variables, warn the user to use it correctly.
+    """
+
+    sane = True
+
+    # Get variables for current package
+    pkg = d.getVar('PN', True)
+    localdata = bb.data.createCopy(d)
+    localdata.setVar('OVERRIDES', pkg)
+    bb.data.update_data(localdata)
+
+    # Go through all variables and check if unexpanded D is found, warn the user accordingly
+    for var in 'FILES','pkg_preinst', 'pkg_postinst', 'pkg_prerm', 'pkg_postrm':
+        bbvar = localdata.getVar(var, False)
+        if bbvar:
+            if "${D}" in bbvar:
+                if var == 'FILES':
+                    bb.error("FILES should not contain the ${D} variable as it references the local build directory not the target filesystem, best solution is to remove the ${D} reference")
+                    sane = False
+                else:
+                    bb.error("%s in %s recipe contains ${D}, it should be replaced by $D instead" % (var, pkg))
+                    sane = False
+    return sane
+
+# Actual task to check for ${D
+python do_qa_check_unexpanded_d(){
+    package_qa_check_unexpanded_d(d)
+}
+addtask qa_check_unexpanded_d before do_compile
+
 # The PACKAGE FUNC to scan each package
 python do_package_qa () {
     import subprocess
@@ -1000,7 +1033,6 @@ python do_package_qa () {
         if not package_qa_check_deps(package, pkgdest, skip, d):
             deps_sane = False
 
-
     if 'libdir' in d.getVar("ALL_QA", True).split():
         package_qa_check_libdir(d)
 
-- 
1.9.1

Re: [PATCH 1/1] insane.bbclass: Added QA test for unexpanded ${D}

[Mark Hatle]

On 12/3/14, 11:00 AM, Alejandro Hernandez wrote:
> Checks in FILES and pkg_* variables ,solves common mistake
> of using ${D} instead of $D and warns the user accordingly.
>
> [YOCTO #6642]
>
> Signed-off-by: Alejandro Hernandez <alejandro.hernandez@linux.intel.com>
> ---
>   meta/classes/insane.bbclass | 34 +++++++++++++++++++++++++++++++++-
>   1 file changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
> index 0b45374..419c89b 100644
> --- a/meta/classes/insane.bbclass
> +++ b/meta/classes/insane.bbclass
> @@ -906,6 +906,39 @@ def package_qa_check_deps(pkg, pkgdest, skip, d):
>
>       return sane
>
> +def package_qa_check_unexpanded_d(d):
> +    """
> +    Check for unexpanded D variable in pkg_* and FILES
> +    variables, warn the user to use it correctly.
> +    """
> +
> +    sane = True
> +
> +    # Get variables for current package
> +    pkg = d.getVar('PN', True)
> +    localdata = bb.data.createCopy(d)
> +    localdata.setVar('OVERRIDES', pkg)
> +    bb.data.update_data(localdata)
> +
> +    # Go through all variables and check if unexpanded D is found, warn the user accordingly
> +    for var in 'FILES','pkg_preinst', 'pkg_postinst', 'pkg_prerm', 'pkg_postrm':
> +        bbvar = localdata.getVar(var, False)
> +        if bbvar:
> +            if "${D}" in bbvar:
> +                if var == 'FILES':
> +                    bb.error("FILES should not contain the ${D} variable as it references the local build directory not the target filesystem, best solution is to remove the ${D} reference")
> +                    sane = False
> +                else:
> +                    bb.error("%s in %s recipe contains ${D}, it should be replaced by $D instead" % (var, pkg))
> +                    sane = False
> +    return sane
> +
> +# Actual task to check for ${D
> +python do_qa_check_unexpanded_d(){
> +    package_qa_check_unexpanded_d(d)
> +}
> +addtask qa_check_unexpanded_d before do_compile
> +

would it make more sense to have a qa check -before- compile, then add this to 
that task.. and then have one (current place) after install?

That way we could add additional checks (as needed) for things that are 
setup/confgiured incorrectly.. and checks for the build/install stuff..

--Mark

>   # The PACKAGE FUNC to scan each package
>   python do_package_qa () {
>       import subprocess
> @@ -1000,7 +1033,6 @@ python do_package_qa () {
>           if not package_qa_check_deps(package, pkgdest, skip, d):
>               deps_sane = False
>
> -
>       if 'libdir' in d.getVar("ALL_QA", True).split():
>           package_qa_check_libdir(d)
>
>

Re: [PATCH 1/1] insane.bbclass: Added QA test for unexpanded ${D}

[Richard Purdie]

On Wed, 2014-12-03 at 11:00 -0600, Alejandro Hernandez wrote:
> Checks in FILES and pkg_* variables ,solves common mistake
> of using ${D} instead of $D and warns the user accordingly.
> 
> [YOCTO #6642]
> 
> Signed-off-by: Alejandro Hernandez <alejandro.hernandez@linux.intel.com>
> ---
>  meta/classes/insane.bbclass | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
> index 0b45374..419c89b 100644
> --- a/meta/classes/insane.bbclass
> +++ b/meta/classes/insane.bbclass
> @@ -906,6 +906,39 @@ def package_qa_check_deps(pkg, pkgdest, skip, d):
>  
>      return sane
>  
> +def package_qa_check_unexpanded_d(d):
> +    """
> +    Check for unexpanded D variable in pkg_* and FILES
> +    variables, warn the user to use it correctly.
> +    """
> +
> +    sane = True
> +
> +    # Get variables for current package
> +    pkg = d.getVar('PN', True)

Rather than just PN here (which may or may not be a package), you need
to iterate over the list of packages in PACKAGES.


> +    localdata = bb.data.createCopy(d)
> +    localdata.setVar('OVERRIDES', pkg)
> +    bb.data.update_data(localdata)
> +
> +    # Go through all variables and check if unexpanded D is found, warn the user accordingly
> +    for var in 'FILES','pkg_preinst', 'pkg_postinst', 'pkg_prerm', 'pkg_postrm':
> +        bbvar = localdata.getVar(var, False)
> +        if bbvar:
> +            if "${D}" in bbvar:
> +                if var == 'FILES':
> +                    bb.error("FILES should not contain the ${D} variable as it references the local build directory not the target filesystem, best solution is to remove the ${D} reference")
> +                    sane = False
> +                else:
> +                    bb.error("%s in %s recipe contains ${D}, it should be replaced by $D instead" % (var, pkg))
> +                    sane = False
> +    return sane
> +
> +# Actual task to check for ${D
> +python do_qa_check_unexpanded_d(){
> +    package_qa_check_unexpanded_d(d)
> +}
> +addtask qa_check_unexpanded_d before do_compile
> +

Why are we doing this before compile, rather than using the existing
package variable handling infrastructure in the class?

The other benefit of using that is we can benefit from the QA_WARN and
QA_ERR variables to determine if this issue is a warning or a fatal
error.

Cheers,

Richard

Re: [PATCH 1/1] insane.bbclass: Added QA test for unexpanded ${D}

[Alejandro Hernandez]

Yes, I agree the function should iterate over all the PACKAGES, but for 
the second matter, using the existing package variable handling 
infrastructure within the class, ends up expanding the variables before 
being able to check them , using getVar('FOO', False) is useless in this 
case, any suggestions?

On 04/12/14 03:48, Richard Purdie wrote:
> On Wed, 2014-12-03 at 11:00 -0600, Alejandro Hernandez wrote:
>> Checks in FILES and pkg_* variables ,solves common mistake
>> of using ${D} instead of $D and warns the user accordingly.
>>
>> [YOCTO #6642]
>>
>> Signed-off-by: Alejandro Hernandez <alejandro.hernandez@linux.intel.com>
>> ---
>>   meta/classes/insane.bbclass | 34 +++++++++++++++++++++++++++++++++-
>>   1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
>> index 0b45374..419c89b 100644
>> --- a/meta/classes/insane.bbclass
>> +++ b/meta/classes/insane.bbclass
>> @@ -906,6 +906,39 @@ def package_qa_check_deps(pkg, pkgdest, skip, d):
>>   
>>       return sane
>>   
>> +def package_qa_check_unexpanded_d(d):
>> +    """
>> +    Check for unexpanded D variable in pkg_* and FILES
>> +    variables, warn the user to use it correctly.
>> +    """
>> +
>> +    sane = True
>> +
>> +    # Get variables for current package
>> +    pkg = d.getVar('PN', True)
> Rather than just PN here (which may or may not be a package), you need
> to iterate over the list of packages in PACKAGES.
>
>
>> +    localdata = bb.data.createCopy(d)
>> +    localdata.setVar('OVERRIDES', pkg)
>> +    bb.data.update_data(localdata)
>> +
>> +    # Go through all variables and check if unexpanded D is found, warn the user accordingly
>> +    for var in 'FILES','pkg_preinst', 'pkg_postinst', 'pkg_prerm', 'pkg_postrm':
>> +        bbvar = localdata.getVar(var, False)
>> +        if bbvar:
>> +            if "${D}" in bbvar:
>> +                if var == 'FILES':
>> +                    bb.error("FILES should not contain the ${D} variable as it references the local build directory not the target filesystem, best solution is to remove the ${D} reference")
>> +                    sane = False
>> +                else:
>> +                    bb.error("%s in %s recipe contains ${D}, it should be replaced by $D instead" % (var, pkg))
>> +                    sane = False
>> +    return sane
>> +
>> +# Actual task to check for ${D
>> +python do_qa_check_unexpanded_d(){
>> +    package_qa_check_unexpanded_d(d)
>> +}
>> +addtask qa_check_unexpanded_d before do_compile
>> +
> Why are we doing this before compile, rather than using the existing
> package variable handling infrastructure in the class?
>
> The other benefit of using that is we can benefit from the QA_WARN and
> QA_ERR variables to determine if this issue is a warning or a fatal
> error.
>
> Cheers,
>
> Richard
>
>