Skip to contentSkip to conversation

insane.bbclass: Added QA test for expanded ${D}

PATCH Superseded Alejandro Hernandez 3 participants 5 messages Dec 11, 2014 – Jan 13, 2015 1 message not in the archive

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

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

Checks in FILES and pkg_* variables, solves common mistake of

using ${D} instead of $D and warns the user accordingly.

[YOCTO #6642]

---
 meta/classes/insane.bbclass | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)
meta/classes/insane.bbclass +29 -1
index 0b45374..8224124 100644
@@ -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
Signature · 1 line
1.9.1
#2 lore raw

Hi Alejandro,

Looks good, but some small points:

meta/classes/insane.bbclass WARN_QA ?= "ldflags useless-rpaths rpaths staticdev libdir xorg-driver-abi \ · line 37

Quoted 3 lines · Alejandro Hernandez
meta/classes/insane.bbclass +1 -1
-            version-going-backwards \
+            version-going-backwards expanded_d \

+QAPATHTEST[expanded_d] = "package_qa_check_expanded_d"

Quoted 1 line

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

Quoted 2 lines

+ # Variables are actually var_${PN}

No need to document idioms, remove this comment.

Quoted 4 lines

+ 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

#3 lore raw

Thank Ross, sending new patch soon.

Quoted 29 lines · Ross

Hi Alejandro,

Looks good, but some small points:

On 11 December 2014 at 22:40, Alejandro Hernandez <[email protected] <mailto:[email protected]>> 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

#4 lore raw
In reply to Alejandro Hernandez
Quoted 35 lines · Alejandro Hernandez

Checks in FILES and pkg_* variables, solves common mistake of

using ${D} instead of $D and warns the user accordingly.

[YOCTO #6642]

---
  meta/classes/insane.bbclass | 30 +++++++++++++++++++++++++++++-
  1 file changed, 29 insertions(+), 1 deletion(-)
meta/classes/insane.bbclass +8 -1
index 0b45374..8224124 100644
@@ -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.
+    """
+

meta/classes/insane.bbclass def package_qa_check_deps(pkg, pkgdest, skip, d): · line 925

Quoted 10 lines
meta/classes/insane.bbclass +10 -0
+    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.

Quoted 15 lines
meta/classes/insane.bbclass +11 -0
+            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
#5 lore raw
Quoted 4 lines · Randy Witt

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