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

[PATCH 1/1] insane.bbclass: Added QA test for expanded ${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 | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index 0b45374..8224124 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 expanded_d \
             "
 
 ALL_QA = "${WARN_QA} ${ERROR_QA}"
@@ -906,6 +906,34 @@ def package_qa_check_deps(pkg, pkgdest, skip, d):
 
     return sane
 
+QAPATHTEST[expanded_d] = "package_qa_check_expanded_d"
+def package_qa_check_expanded_d(path,name,d,elf,messages):
+    """
+    Check for the expanded D (${D}) value in pkg_* and FILES
+    variables, warn the user to use it correctly.
+    """
+
+    sane = True
+    expanded_d = d.getVar('D',True)
+
+    # Get packages for current recipe and iterate
+    packages = d.getVar('PACKAGES', True).split(" ")
+    for pak in packages:
+    # Go through all variables and check if expanded D is found, warn the user accordingly
+        for var in 'FILES','pkg_preinst', 'pkg_postinst', 'pkg_prerm', 'pkg_postrm':
+            # Variables are actually var_${PN}
+            bbvar = d.getVar(var + "_" + pak)
+            if bbvar:
+                # Bitbake expands ${D} within bbvar during the previous step, so we check for its expanded value
+                if expanded_d in bbvar:
+                    if var == 'FILES':
+                        messages["expanded_d"] = "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:
+                        messages["expanded_d"] = "%s in %s recipe contains ${D}, it should be replaced by $D instead" % (var, pak)
+                        sane = False
+    return sane
+
 # The PACKAGE FUNC to scan each package
 python do_package_qa () {
     import subprocess
-- 
1.9.1

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

[Burton, Ross]

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

Hi Alejandro,

Looks good, but some small points:

On 11 December 2014 at 22:40, Alejandro Hernandez <
alejandro.hernandez@linux.intel.com> wrote:

> -            version-going-backwards \
> +            version-going-backwards expanded_d \
>
+QAPATHTEST[expanded_d] = "package_qa_check_expanded_d"
>

Rename this to expanded-d for consistency with the other symbols that use -
instead of _.


> +            # Variables are actually var_${PN}
>

No need to document idioms, remove this comment.


> +                        messages["expanded_d"] = "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"
>

This doesn't name the package which makes it tricky to find in large builds.

Ross

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

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

[Alejandro Hernandez]

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

Thank Ross, sending new patch soon.
On 07/01/15 12:27, Burton, Ross wrote:
> Hi Alejandro,
>
> Looks good, but some small points:
>
> On 11 December 2014 at 22:40, Alejandro Hernandez 
> <alejandro.hernandez@linux.intel.com 
> <mailto:alejandro.hernandez@linux.intel.com>> wrote:
>
>     -           version-going-backwards \
>     +            version-going-backwards expanded_d \
>
>     +QAPATHTEST[expanded_d] = "package_qa_check_expanded_d"
>
>
> Rename this to expanded-d for consistency with the other symbols that 
> use - instead of _.
>
>     +           # Variables are actually var_${PN}
>
>
> No need to document idioms, remove this comment.
>
>     +                       messages["expanded_d"] = "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"
>
>
> This doesn't name the package which makes it tricky to find in large 
> builds.
>
> Ross
>


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

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

[Randy Witt]

On 12/11/2014 02:40 PM, 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 | 30 +++++++++++++++++++++++++++++-
>   1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
> index 0b45374..8224124 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 expanded_d \
>               "
>
>   ALL_QA = "${WARN_QA} ${ERROR_QA}"
> @@ -906,6 +906,34 @@ def package_qa_check_deps(pkg, pkgdest, skip, d):
>
>       return sane
>
> +QAPATHTEST[expanded_d] = "package_qa_check_expanded_d"
> +def package_qa_check_expanded_d(path,name,d,elf,messages):
> +    """
> +    Check for the expanded D (${D}) value in pkg_* and FILES
> +    variables, warn the user to use it correctly.
> +    """
> +
> +    sane = True
> +    expanded_d = d.getVar('D',True)
> +
> +    # Get packages for current recipe and iterate
> +    packages = d.getVar('PACKAGES', True).split(" ")
> +    for pak in packages:
> +    # Go through all variables and check if expanded D is found, warn the user accordingly
> +        for var in 'FILES','pkg_preinst', 'pkg_postinst', 'pkg_prerm', 'pkg_postrm':
> +            # Variables are actually var_${PN}
> +            bbvar = d.getVar(var + "_" + pak)

Would it be simpler to do "bbvar = d.getVar(var + "_" + pak, False)" so you 
don't get the expanded value, and then just check for ${D}? I suppose it would 
save one extra variable and a couple of expansions.

> +            if bbvar:
> +                # Bitbake expands ${D} within bbvar during the previous step, so we check for its expanded value
> +                if expanded_d in bbvar:
> +                    if var == 'FILES':
> +                        messages["expanded_d"] = "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:
> +                        messages["expanded_d"] = "%s in %s recipe contains ${D}, it should be replaced by $D instead" % (var, pak)
> +                        sane = False
> +    return sane
> +
>   # The PACKAGE FUNC to scan each package
>   python do_package_qa () {
>       import subprocess
>

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

[Burton, Ross]

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

On 7 January 2015 at 22:02, Randy Witt <randy.e.witt@linux.intel.com> wrote:

> Would it be simpler to do "bbvar = d.getVar(var + "_" + pak, False)" so
> you don't get the expanded value, and then just check for ${D}? I suppose
> it would save one extra variable and a couple of expansions.
>

Alejandro answered this point in the original thread:

"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?"

Ross

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