| Message ID | 1515620175-11260-1-git-send-email-jerzy.m.grzegorek@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [PATCHv2,1/1] utils/checkpackagelib: add function to check of the default package source variable | expand |
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
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
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)\\$")
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(+)