[Buildroot] [PATCHv2 1/1] utils/checkpackagelib: add function to check of the default package source variable

[Buildroot] [PATCHv2 1/1] utils/checkpackagelib: add function to check of the default package source variable

[Jerzy Grzegorek]

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> 
---
Changes v1 -> v2
 - remove unused variable (Ricardo Martincoski)
 - change warning url (Ricardo Martincoski)
 - add whitelist of packages (Ricardo Martincoski)

 utils/checkpackagelib/lib_mk.py | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/utils/checkpackagelib/lib_mk.py b/utils/checkpackagelib/lib_mk.py
index 817e809..a0ed1ae 100644
--- a/utils/checkpackagelib/lib_mk.py
+++ b/utils/checkpackagelib/lib_mk.py
@@ -99,6 +99,33 @@ class PackageHeader(_CheckFunction):
                         text]
 
 
+class RemoveDefaultPackageSourceVariable(_CheckFunction):
+    packages_that_may_contain_default_source = ["binutils","gcc","gdb"]
+    PACKAGE_NAME = re.compile("/([^/]+)\.mk")
+
+    def before(self):
+        package = self.PACKAGE_NAME.search(self.filename).group(1)
+        package_upper = package.replace("-", "_").upper()
+        self.package = package
+        self.FIND_SOURCE = re.compile(
+            "^{}_SOURCE\s*=\s*{}-\$\({}_VERSION\)\.tar\.gz"
+            .format(package_upper, package, package_upper))
+
+    def check_line(self, lineno, text):
+        if self.FIND_SOURCE.search(text):
+
+            if self.package in self.packages_that_may_contain_default_source:
+                return ["{}:{}: this package may contain default value of "
+                        "_SOURCE variable to remove ({}#generic-package-reference)"
+                        .format(self.filename, lineno, self.url_to_manual),
+                        text]
+
+            return ["{}:{}: remove default value of _SOURCE variable "
+                    "({}#generic-package-reference)"
+                    .format(self.filename, lineno, self.url_to_manual),
+                    text]
+
+
 class SpaceBeforeBackslash(_CheckFunction):
     TAB_OR_MULTIPLE_SPACES_BEFORE_BACKSLASH = re.compile(r"^.*(  |\t)\\$")
 
-- 
1.9.1

[Buildroot] [PATCHv2 1/1] utils/checkpackagelib: add function to check of the default package source variable

[Ricardo Martincoski]

Hello,

On Wed, Jan 10, 2018 at 07:36 PM, Jerzy Grzegorek wrote:

[snip]
> +class RemoveDefaultPackageSourceVariable(_CheckFunction):
> +    packages_that_may_contain_default_source = ["binutils","gcc","gdb"]

This line adds 2 style warnings (detected with flake8):
E231 missing whitespace after ','
Can you fix them?

Please ignore the other 4 warnings
F401 'lib.*' imported but unused
already present in the file. I will send a series to fix them and all other
style warnings detected by flake8 in the tree.

> +    PACKAGE_NAME = re.compile("/([^/]+)\.mk")
> +
> +    def before(self):
> +        package = self.PACKAGE_NAME.search(self.filename).group(1)
> +        package_upper = package.replace("-", "_").upper()
> +        self.package = package
> +        self.FIND_SOURCE = re.compile(
> +            "^{}_SOURCE\s*=\s*{}-\$\({}_VERSION\)\.tar\.gz"
> +            .format(package_upper, package, package_upper))
> +
> +    def check_line(self, lineno, text):
> +        if self.FIND_SOURCE.search(text):
> +
> +            if self.package in self.packages_that_may_contain_default_source:
> +                return ["{}:{}: this package may contain default value of "
> +                        "_SOURCE variable to remove ({}#generic-package-reference)"
> +                        .format(self.filename, lineno, self.url_to_manual),
> +                        text]

For the whitelist case we need to use just:
                return

Any value returned by a check function is counted as a warning, making the main
check-package script to exit with code 1 to the shell, that in turn makes the
job in gitlab to be marked as failed.


There is also the glibc.mk to remove the default value of _SOURCE variable
from, to be done in a separate patch. 'make printvars' can be helpful to test
it besides a build test.

Are you willing to work on this too?


Regards,
Ricardo

[Buildroot] [PATCHv2 1/1] utils/checkpackagelib: add function to check of the default package source variable

[Jerzy Grzegorek]

Hi Ricardo,

Thanks for your feedback.

> Hello,
>
> On Wed, Jan 10, 2018 at 07:36 PM, Jerzy Grzegorek wrote:
>
> [snip]
>> +class RemoveDefaultPackageSourceVariable(_CheckFunction):
>> +    packages_that_may_contain_default_source = ["binutils","gcc","gdb"]
> This line adds 2 style warnings (detected with flake8):
> E231 missing whitespace after ','
> Can you fix them?

will do

>
> Please ignore the other 4 warnings
> F401 'lib.*' imported but unused
> already present in the file. I will send a series to fix them and all other
> style warnings detected by flake8 in the tree.
>
>> +    PACKAGE_NAME = re.compile("/([^/]+)\.mk")
>> +
>> +    def before(self):
>> +        package = self.PACKAGE_NAME.search(self.filename).group(1)
>> +        package_upper = package.replace("-", "_").upper()
>> +        self.package = package
>> +        self.FIND_SOURCE = re.compile(
>> +            "^{}_SOURCE\s*=\s*{}-\$\({}_VERSION\)\.tar\.gz"
>> +            .format(package_upper, package, package_upper))
>> +
>> +    def check_line(self, lineno, text):
>> +        if self.FIND_SOURCE.search(text):
>> +
>> +            if self.package in self.packages_that_may_contain_default_source:
>> +                return ["{}:{}: this package may contain default value of "
>> +                        "_SOURCE variable to remove ({}#generic-package-reference)"
>> +                        .format(self.filename, lineno, self.url_to_manual),
>> +                        text]
> For the whitelist case we need to use just:
>                  return
>
> Any value returned by a check function is counted as a warning, making the main
> check-package script to exit with code 1 to the shell, that in turn makes the
> job in gitlab to be marked as failed.

ok

>
>
> There is also the glibc.mk to remove the default value of _SOURCE variable
> from, to be done in a separate patch. 'make printvars' can be helpful to test
> it besides a build test.
>
> Are you willing to work on this too?

I'll try to do it.

Regards,
Jerzy

>
>
> Regards,
> Ricardo