insane.bbclass: Added QA test for expanded ${D}
PATCH 1/1 insane.bbclass: Added QA test for expanded ${D}
In reply to a message not in the archiveChecks 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
@@ -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
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
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]
Signed-off-by Alejandro Hernandez <[email protected]>--- meta/classes/insane.bbclass | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)meta/classes/insane.bbclass +8
-1@@ -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
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
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-backwardsexpanded_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