[Buildroot] [PATCH 01/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: rename variable 'm'

[Buildroot] [PATCH 01/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: rename variable 'm'

[Jerzy Grzegorek]

Rename variable 'm' for better readability.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 utils/checkpackagelib/lib_config.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index 225f92af13..a3fbd0e69e 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -77,7 +77,8 @@ class CommentsMenusPackagesOrder(_CheckFunction):
                                  "package/Config.in.host"]:
             return
 
-        m = re.match(r'^\s*source ".*/([^/]*)/Config.in(.host)?"', text)
+        source_line = re.match(r'^\s*source ".*/([^/]*)/Config.in(.host)?"', text)
+
         if text.startswith("comment ") or text.startswith("if ") or \
            text.startswith("menu "):
 
@@ -113,9 +114,9 @@ class CommentsMenusPackagesOrder(_CheckFunction):
             elif text.startswith("endmenu"):
                 self.state = self.state[:-5]
 
-        elif m:
+        elif source_line:
             level = self.get_level()
-            new_package = m.group(1)
+            new_package = source_line.group(1)
 
             # We order _ before A, so replace it with .
             new_package_ord = new_package.replace('_', '.')
-- 
2.17.1

[Buildroot] [PATCH 02/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: change the type of variable 'level'

[Jerzy Grzegorek]

Change the type of variable "level" to make it a class member.
It will be used not only locally.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 utils/checkpackagelib/lib_config.py | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index a3fbd0e69e..8fad36f5ac 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -93,12 +93,12 @@ class CommentsMenusPackagesOrder(_CheckFunction):
                 elif text.startswith("menu"):
                     self.state += "-menu"
 
-            level = self.get_level()
+            self.level = self.get_level()
 
             try:
-                self.menu_of_packages[level] = text[:-1]
-                self.package[level] = ""
-                self.print_package_warning[level] = True
+                self.menu_of_packages[self.level] = text[:-1]
+                self.package[self.level] = ""
+                self.print_package_warning[self.level] = True
             except IndexError:
                 self.menu_of_packages.append(text[:-1])
                 self.package.append("")
@@ -115,16 +115,16 @@ class CommentsMenusPackagesOrder(_CheckFunction):
                 self.state = self.state[:-5]
 
         elif source_line:
-            level = self.get_level()
+            self.level = self.get_level()
             new_package = source_line.group(1)
 
             # We order _ before A, so replace it with .
             new_package_ord = new_package.replace('_', '.')
 
-            if self.package[level] != "" and \
-               self.print_package_warning[level] and \
-               new_package_ord < self.package[level]:
-                self.print_package_warning[level] = False
+            if self.package[self.level] != "" and \
+               self.print_package_warning[self.level] and \
+               new_package_ord < self.package[self.level]:
+                self.print_package_warning[self.level] = False
                 prefix = "{}:{}: ".format(self.filename, lineno)
                 spaces = " " * len(prefix)
                 return ["{prefix}Packages in: {menu},\n"
@@ -132,11 +132,11 @@ class CommentsMenusPackagesOrder(_CheckFunction):
                         "{spaces}correct order: '-', '_', digits, capitals, lowercase;\n"
                         "{spaces}first incorrect package: {package}"
                         .format(prefix=prefix, spaces=spaces,
-                                menu=self.menu_of_packages[level],
+                                menu=self.menu_of_packages[self.level],
                                 package=new_package),
                         text]
 
-            self.package[level] = new_package_ord
+            self.package[self.level] = new_package_ord
 
 
 class HelpText(_CheckFunction):
-- 
2.17.1

[Buildroot] [PATCH 03/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: get value of variable 'level'

[Jerzy Grzegorek]

Get value of variable 'level' only just after the state change.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 utils/checkpackagelib/lib_config.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index 8fad36f5ac..e6a5f1b182 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -114,8 +114,9 @@ class CommentsMenusPackagesOrder(_CheckFunction):
             elif text.startswith("endmenu"):
                 self.state = self.state[:-5]
 
-        elif source_line:
             self.level = self.get_level()
+
+        elif source_line:
             new_package = source_line.group(1)
 
             # We order _ before A, so replace it with .
-- 
2.17.1

[Buildroot] [PATCH 04/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: add functions to initialize arrays elements

[Jerzy Grzegorek]

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 utils/checkpackagelib/lib_config.py | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index e6a5f1b182..f8bfe6a577 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -71,6 +71,20 @@ class CommentsMenusPackagesOrder(_CheckFunction):
     def get_level(self):
         return len(self.state.split('-')) - 1
 
+    def initialize_package_level_elements(self, text):
+        try:
+            self.menu_of_packages[self.level] = text[:-1]
+            self.package[self.level] = ""
+            self.print_package_warning[self.level] = True
+        except IndexError:
+            self.menu_of_packages.append(text[:-1])
+            self.package.append("")
+            self.print_package_warning.append(True)
+
+    def initialize_level_elements(self, text):
+        self.level = self.get_level()
+        self.initialize_package_level_elements(text)
+
     def check_line(self, lineno, text):
         # We only want to force sorting for the top-level menus
         if self.filename not in ["package/Config.in",
@@ -93,16 +107,7 @@ class CommentsMenusPackagesOrder(_CheckFunction):
                 elif text.startswith("menu"):
                     self.state += "-menu"
 
-            self.level = self.get_level()
-
-            try:
-                self.menu_of_packages[self.level] = text[:-1]
-                self.package[self.level] = ""
-                self.print_package_warning[self.level] = True
-            except IndexError:
-                self.menu_of_packages.append(text[:-1])
-                self.package.append("")
-                self.print_package_warning.append(True)
+            self.initialize_level_elements(text)
 
         elif text.startswith("endif") or text.startswith("endmenu"):
             if self.state.endswith("comment"):
-- 
2.17.1

[Buildroot] [PATCH 05/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: change the type of variable 'new_package'

[Jerzy Grzegorek]

Change the type of variable 'new_package' to make it a class member.
It will be used not only locally.
Also initialize it.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 utils/checkpackagelib/lib_config.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index f8bfe6a577..de5faa19a2 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -67,6 +67,7 @@ class CommentsMenusPackagesOrder(_CheckFunction):
 
     def before(self):
         self.state = ""
+        self.new_package = ""
 
     def get_level(self):
         return len(self.state.split('-')) - 1
@@ -122,10 +123,10 @@ class CommentsMenusPackagesOrder(_CheckFunction):
             self.level = self.get_level()
 
         elif source_line:
-            new_package = source_line.group(1)
+            self.new_package = source_line.group(1)
 
             # We order _ before A, so replace it with .
-            new_package_ord = new_package.replace('_', '.')
+            new_package_ord = self.new_package.replace('_', '.')
 
             if self.package[self.level] != "" and \
                self.print_package_warning[self.level] and \
@@ -139,7 +140,7 @@ class CommentsMenusPackagesOrder(_CheckFunction):
                         "{spaces}first incorrect package: {package}"
                         .format(prefix=prefix, spaces=spaces,
                                 menu=self.menu_of_packages[self.level],
-                                package=new_package),
+                                package=self.new_package),
                         text]
 
             self.package[self.level] = new_package_ord
-- 
2.17.1

[Buildroot] [PATCH 06/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: remove '-comment' state before the '-menu' one

[Jerzy Grzegorek]

The comment may contain packages or if-endif block(s) but not menu(s)
of packages, so lets remove the '-comment' state before the '-menu' one.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 utils/checkpackagelib/lib_config.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index de5faa19a2..d8f26ef435 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -106,6 +106,9 @@ class CommentsMenusPackagesOrder(_CheckFunction):
                     self.state += "-if"
 
                 elif text.startswith("menu"):
+                    if self.state.endswith("-comment"):
+                        self.state = self.state[:-8]
+
                     self.state += "-menu"
 
             self.initialize_level_elements(text)
-- 
2.17.1

[Buildroot] [PATCH 07/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: separate the lines support

[Jerzy Grzegorek]

The lines 'comment...', 'if ...' and 'menu ...' will be handled without
sharing any code so lets separate their handling.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 utils/checkpackagelib/lib_config.py | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index d8f26ef435..7f92f580b8 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -94,22 +94,22 @@ class CommentsMenusPackagesOrder(_CheckFunction):
 
         source_line = re.match(r'^\s*source ".*/([^/]*)/Config.in(.host)?"', text)
 
-        if text.startswith("comment ") or text.startswith("if ") or \
-           text.startswith("menu "):
+        if text.startswith("comment "):
+            if not self.state.endswith("-comment"):
+                self.state += "-comment"
 
-            if text.startswith("comment"):
-                if not self.state.endswith("-comment"):
-                    self.state += "-comment"
+            self.initialize_level_elements(text)
+
+        elif text.startswith("if "):
+            self.state += "-if"
 
-            elif text.startswith("if") or text.startswith("menu"):
-                if text.startswith("if"):
-                    self.state += "-if"
+            self.initialize_level_elements(text)
 
-                elif text.startswith("menu"):
-                    if self.state.endswith("-comment"):
-                        self.state = self.state[:-8]
+        elif text.startswith("menu "):
+            if self.state.endswith("-comment"):
+                self.state = self.state[:-8]
 
-                    self.state += "-menu"
+            self.state += "-menu"
 
             self.initialize_level_elements(text)
 
-- 
2.17.1

[Buildroot] [PATCH 08/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: use '-' to describe state

[Jerzy Grzegorek]

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 utils/checkpackagelib/lib_config.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index 7f92f580b8..3d8f08cac9 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -114,7 +114,7 @@ class CommentsMenusPackagesOrder(_CheckFunction):
             self.initialize_level_elements(text)
 
         elif text.startswith("endif") or text.startswith("endmenu"):
-            if self.state.endswith("comment"):
+            if self.state.endswith("-comment"):
                 self.state = self.state[:-8]
 
             if text.startswith("endif"):
-- 
2.17.1

[Buildroot] [PATCH 09/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: drop function get_line

[Jerzy Grzegorek]

Because function get_line() has only one line so use it inline.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 utils/checkpackagelib/lib_config.py | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index 3d8f08cac9..f39a4b0599 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -69,9 +69,6 @@ class CommentsMenusPackagesOrder(_CheckFunction):
         self.state = ""
         self.new_package = ""
 
-    def get_level(self):
-        return len(self.state.split('-')) - 1
-
     def initialize_package_level_elements(self, text):
         try:
             self.menu_of_packages[self.level] = text[:-1]
@@ -83,7 +80,7 @@ class CommentsMenusPackagesOrder(_CheckFunction):
             self.print_package_warning.append(True)
 
     def initialize_level_elements(self, text):
-        self.level = self.get_level()
+        self.level = len(self.state.split('-')) - 1
         self.initialize_package_level_elements(text)
 
     def check_line(self, lineno, text):
@@ -123,7 +120,7 @@ class CommentsMenusPackagesOrder(_CheckFunction):
             elif text.startswith("endmenu"):
                 self.state = self.state[:-5]
 
-            self.level = self.get_level()
+            self.level = len(self.state.split('-')) - 1
 
         elif source_line:
             self.new_package = source_line.group(1)
-- 
2.17.1

[Buildroot] [PATCH 10/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: initialize 'menu_of_packages' array

[Jerzy Grzegorek]

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 utils/checkpackagelib/lib_config.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index f39a4b0599..f1c709388c 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -61,7 +61,7 @@ class AttributesOrder(_CheckFunction):
 
 
 class CommentsMenusPackagesOrder(_CheckFunction):
-    menu_of_packages = [""]
+    menu_of_packages = ["The top level menu"]
     package = [""]
     print_package_warning = [True]
 
-- 
2.17.1

[Buildroot] [PATCH 11/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: add more Config.in files to check

[Jerzy Grzegorek]

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 utils/checkpackagelib/lib_config.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index f1c709388c..a356702ca8 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -85,8 +85,11 @@ class CommentsMenusPackagesOrder(_CheckFunction):
 
     def check_line(self, lineno, text):
         # We only want to force sorting for the top-level menus
-        if self.filename not in ["package/Config.in",
-                                 "package/Config.in.host"]:
+        if self.filename not in ["boot/Config.in",
+                                 "fs/Config.in",
+                                 "package/Config.in",
+                                 "package/Config.in.host",
+                                 "package/kodi/Config.in"]:
             return
 
         source_line = re.match(r'^\s*source ".*/([^/]*)/Config.in(.host)?"', text)
-- 
2.17.1

[Buildroot] [PATCH 12/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: check package ordering just before 'if ' statement

[Jerzy Grzegorek]

Because the statement

 if BR2_PACKAGE_..._FOO

(if exist) refers to the foo package, so the line

 source package/foo/Config.in

should go just before that statement.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 utils/checkpackagelib/lib_config.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index a356702ca8..1ef46774a9 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -105,6 +105,22 @@ class CommentsMenusPackagesOrder(_CheckFunction):
 
             self.initialize_level_elements(text)
 
+            if text.startswith("if BR2_PACKAGE_") and \
+               self.new_package.replace('-', '_').upper() not in text:
+                self.print_package_warning[self.level-1] = False
+                prefix = "{}:{}: ".format(self.filename, lineno)
+                spaces = " " * len(prefix)
+                return ["{prefix}Packages in: {menu},\n"
+                        "{spaces}are not alphabetically ordered;\n"
+                        "{spaces}first incorrect package: {package} ;\n"
+                        "{spaces}this package, placed just before if statement,\n"
+                        "{spaces}should match the one used in\n"
+                        "{spaces}{text}"
+                        .format(prefix=prefix, spaces=spaces, text=text,
+                                menu=self.menu_of_packages[self.level-1],
+                                package=self.new_package),
+                        text]
+
         elif text.startswith("menu "):
             if self.state.endswith("-comment"):
                 self.state = self.state[:-8]
-- 
2.17.1

[Buildroot] [PATCH 13/18] package/Config.in: fix packages ordering

[Jerzy Grzegorek]

Fixes:
package/Config.in:594: Packages in: menu "Interpreter languages and scripting",
                       are not alphabetically ordered;
                       first incorrect package: luajit ;
                       this package, placed just before if statement,
                       should match the one used in
                       if BR2_PACKAGE_HAS_LUAINTERPRETER && !BR2_STATIC_LIBS

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 package/Config.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/Config.in b/package/Config.in
index 710ed12be0..7bc97fff72 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -590,7 +590,6 @@ endif
 	source "package/jimtcl/Config.in"
 	source "package/lua/Config.in"
 	source "package/luainterpreter/Config.in"
-	source "package/luajit/Config.in"
 if BR2_PACKAGE_HAS_LUAINTERPRETER && !BR2_STATIC_LIBS
 # lua modules are dynamically loaded, so not available on static builds
 menu "Lua libraries/modules"
@@ -670,6 +669,7 @@ menu "Lua libraries/modules"
 	source "package/xavante/Config.in"
 endmenu
 endif
+	source "package/luajit/Config.in"
 	source "package/micropython/Config.in"
 	source "package/micropython-lib/Config.in"
 	source "package/moarvm/Config.in"
-- 
2.17.1

[Buildroot] [PATCH 14/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: check the order of comments menu

[Jerzy Grzegorek]

At any level if the 'comment ...' line occurs for the first time a new state
'-comment' is added and all arrays elements for that level are initialized.
For the second and subsequent time only packages arrays elements are initialized.
The menu of comments corresponds to menu of packages of lower level.
Only the valid comments are compared. The comment is valid only if there is
at least one a 'source ...' line after it and before the next 'comment ...',
'menu ...' or 'if ...' line.
The alphabetical order of comments at that level is checked until the first
error occurs.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 utils/checkpackagelib/lib_config.py | 44 ++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index 1ef46774a9..cb8061e371 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -61,6 +61,10 @@ class AttributesOrder(_CheckFunction):
 
 
 class CommentsMenusPackagesOrder(_CheckFunction):
+    comment = [""]
+    comments_order_checking = [False]
+    print_comment_warning = [True]
+
     menu_of_packages = ["The top level menu"]
     package = [""]
     print_package_warning = [True]
@@ -81,6 +85,16 @@ class CommentsMenusPackagesOrder(_CheckFunction):
 
     def initialize_level_elements(self, text):
         self.level = len(self.state.split('-')) - 1
+
+        try:
+            self.comment[self.level] = ""
+            self.comments_order_checking[self.level] = False
+            self.print_comment_warning[self.level] = True
+        except IndexError:
+            self.comment.append("")
+            self.comments_order_checking.append(False)
+            self.print_comment_warning.append(True)
+
         self.initialize_package_level_elements(text)
 
     def check_line(self, lineno, text):
@@ -98,7 +112,12 @@ class CommentsMenusPackagesOrder(_CheckFunction):
             if not self.state.endswith("-comment"):
                 self.state += "-comment"
 
-            self.initialize_level_elements(text)
+                self.initialize_level_elements(text)
+            else:
+                self.initialize_package_level_elements(text)
+
+            if self.print_comment_warning[self.level]:
+                self.comments_order_checking[self.level] = True
 
         elif text.startswith("if "):
             self.state += "-if"
@@ -142,6 +161,29 @@ class CommentsMenusPackagesOrder(_CheckFunction):
             self.level = len(self.state.split('-')) - 1
 
         elif source_line:
+            if self.comments_order_checking[self.level]:
+
+                self.comments_order_checking[self.level] = False
+
+                new_comment = self.menu_of_packages[self.level][9: -1:]
+
+                if self.comment[self.level] != "" and \
+                   self.print_comment_warning[self.level] and \
+                   new_comment < self.comment[self.level]:
+                    self.print_comment_warning[self.level] = False
+                    prefix = "{}:{}: ".format(self.filename, lineno)
+                    spaces = " " * len(prefix)
+                    return ["{prefix}Comments in: {menu},\n"
+                            "{spaces}are not alphabetically ordered;\n"
+                            "{spaces}correct order: '-', '_', digits, capitals, lowercase;\n"
+                            "{spaces}first incorrect comment: {comment}"
+                            .format(prefix=prefix, spaces=spaces,
+                                    menu=self.menu_of_packages[self.level-1],
+                                    comment=new_comment),
+                            text]
+
+                self.comment[self.level] = new_comment
+
             self.new_package = source_line.group(1)
 
             # We order _ before A, so replace it with .
-- 
2.17.1

[Buildroot] [PATCH 15/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: check the order of menu of menus

[Jerzy Grzegorek]

At any level if the 'menu ...' line occurs for the first time a new state '-menu'
is added and all arrays elements for that level are initialized. For the second
and subsequent times only packages arrays elements are initialized.
Because of this that condition must be checked before appending elements to arrays.
Therefore array's first_menu element must exist for that level and therefore this
array must have one more element than the others and in time of initialization
the higher level element of this array is initialized.
The menu of menus corresponds to menu of packages of lower level.
The alphabetical order of menus at that level is checked until the first error
occurs.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 utils/checkpackagelib/lib_config.py | 41 ++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index cb8061e371..ad0142d259 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -65,6 +65,10 @@ class CommentsMenusPackagesOrder(_CheckFunction):
     comments_order_checking = [False]
     print_comment_warning = [True]
 
+    first_menu = [True, True]
+    menu = [""]
+    print_menu_warning = [True]
+
     menu_of_packages = ["The top level menu"]
     package = [""]
     print_package_warning = [True]
@@ -95,6 +99,15 @@ class CommentsMenusPackagesOrder(_CheckFunction):
             self.comments_order_checking.append(False)
             self.print_comment_warning.append(True)
 
+        try:
+            self.first_menu[self.level+1] = True
+            self.menu[self.level] = ""
+            self.print_menu_warning[self.level] = True
+        except IndexError:
+            self.first_menu.append(True)
+            self.menu.append("")
+            self.print_menu_warning.append(True)
+
         self.initialize_package_level_elements(text)
 
     def check_line(self, lineno, text):
@@ -146,7 +159,33 @@ class CommentsMenusPackagesOrder(_CheckFunction):
 
             self.state += "-menu"
 
-            self.initialize_level_elements(text)
+            self.level = len(self.state.split('-')) - 1
+
+            if self.first_menu[self.level]:
+                self.first_menu[self.level] = False
+
+                self.initialize_level_elements(text)
+            else:
+                self.initialize_package_level_elements(text)
+
+            new_menu = text[6: -2:]
+
+            if self.menu[self.level] != "" and \
+               self.print_menu_warning[self.level] and \
+               new_menu < self.menu[self.level]:
+                self.print_menu_warning[self.level] = False
+                prefix = "{}:{}: ".format(self.filename, lineno)
+                spaces = " " * len(prefix)
+                return ["{prefix}Menus in: {menu},\n"
+                        "{spaces}are not alphabetically ordered;\n"
+                        "{spaces}correct order: '-', '_', digits, capitals, lowercase;\n"
+                        "{spaces}first incorrect menu: {comment}"
+                        .format(prefix=prefix, spaces=spaces,
+                                menu=self.menu_of_packages[self.level-1],
+                                comment=new_menu),
+                        text]
+
+            self.menu[self.level] = new_menu
 
         elif text.startswith("endif") or text.startswith("endmenu"):
             if self.state.endswith("-comment"):
-- 
2.17.1

[Buildroot] [PATCH 16/18] package/Config.in: fix menus ordering

[Jerzy Grzegorek]

Fixes:
package/Config.in:1455: Menus in: menu "Libraries",
                        are not alphabetically ordered;
                        correct order: '-', '_', digits, capitals, lowercase;
                        first incorrect menu: JSON/XML

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 package/Config.in | 70 +++++++++++++++++++++++------------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/package/Config.in b/package/Config.in
index 7bc97fff72..a1072990ca 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -1424,6 +1424,41 @@ menu "Hardware handling"
 	source "package/wiringpi/Config.in"
 endmenu
 
+menu "JSON/XML"
+        source "package/benejson/Config.in"
+        source "package/cjson/Config.in"
+        source "package/expat/Config.in"
+        source "package/ezxml/Config.in"
+        source "package/jansson/Config.in"
+        source "package/jose/Config.in"
+        source "package/jsmn/Config.in"
+        source "package/json-c/Config.in"
+        source "package/json-for-modern-cpp/Config.in"
+        source "package/json-glib/Config.in"
+        source "package/jsoncpp/Config.in"
+        source "package/libbson/Config.in"
+        source "package/libfastjson/Config.in"
+        source "package/libjson/Config.in"
+        source "package/libroxml/Config.in"
+        source "package/libucl/Config.in"
+        source "package/libxml2/Config.in"
+        source "package/libxmlpp/Config.in"
+        source "package/libxmlrpc/Config.in"
+        source "package/libxslt/Config.in"
+        source "package/libyaml/Config.in"
+        source "package/mxml/Config.in"
+        source "package/pugixml/Config.in"
+        source "package/rapidjson/Config.in"
+        source "package/rapidxml/Config.in"
+        source "package/raptor/Config.in"
+        source "package/tinyxml/Config.in"
+        source "package/tinyxml2/Config.in"
+        source "package/valijson/Config.in"
+        source "package/xerces/Config.in"
+        source "package/yajl/Config.in"
+        source "package/yaml-cpp/Config.in"
+endmenu
+
 menu "Javascript"
 	source "package/angularjs/Config.in"
 if BR2_PACKAGE_ANGULARJS
@@ -1452,41 +1487,6 @@ endif
 	source "package/json-javascript/Config.in"
 endmenu
 
-menu "JSON/XML"
-	source "package/benejson/Config.in"
-	source "package/cjson/Config.in"
-	source "package/expat/Config.in"
-	source "package/ezxml/Config.in"
-	source "package/jansson/Config.in"
-	source "package/jose/Config.in"
-	source "package/jsmn/Config.in"
-	source "package/json-c/Config.in"
-	source "package/json-for-modern-cpp/Config.in"
-	source "package/json-glib/Config.in"
-	source "package/jsoncpp/Config.in"
-	source "package/libbson/Config.in"
-	source "package/libfastjson/Config.in"
-	source "package/libjson/Config.in"
-	source "package/libroxml/Config.in"
-	source "package/libucl/Config.in"
-	source "package/libxml2/Config.in"
-	source "package/libxmlpp/Config.in"
-	source "package/libxmlrpc/Config.in"
-	source "package/libxslt/Config.in"
-	source "package/libyaml/Config.in"
-	source "package/mxml/Config.in"
-	source "package/pugixml/Config.in"
-	source "package/rapidjson/Config.in"
-	source "package/rapidxml/Config.in"
-	source "package/raptor/Config.in"
-	source "package/tinyxml/Config.in"
-	source "package/tinyxml2/Config.in"
-	source "package/valijson/Config.in"
-	source "package/xerces/Config.in"
-	source "package/yajl/Config.in"
-	source "package/yaml-cpp/Config.in"
-endmenu
-
 menu "Logging"
 	source "package/eventlog/Config.in"
 	source "package/glog/Config.in"
-- 
2.17.1

[Buildroot] [PATCH 17/18] package/kodi/Config.in: fix menus ordering

[Jerzy Grzegorek]

Fixes:
package/kodi/Config.in:313: Menus in: if BR2_PACKAGE_KODI,
                            are not alphabetically ordered;
                            correct order: '-', '_', digits, capitals, lowercase;
                            first incorrect menu: Inputstream addons

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 package/kodi/Config.in | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/package/kodi/Config.in b/package/kodi/Config.in
index 626038e8c8..8e30e2f3ea 100644
--- a/package/kodi/Config.in
+++ b/package/kodi/Config.in
@@ -304,17 +304,17 @@ menu "Audio encoder addons"
 	source "package/kodi-audioencoder-wav/Config.in"
 endmenu
 
+menu "Inputstream addons"
+        source "package/kodi-inputstream-adaptive/Config.in"
+        source "package/kodi-inputstream-rtmp/Config.in"
+endmenu
+
 menu "Peripheral addons"
 	source "package/kodi-peripheral-joystick/Config.in"
 	source "package/kodi-peripheral-steamcontroller/Config.in"
 	source "package/kodi-peripheral-xarcade/Config.in"
 endmenu
 
-menu "Inputstream addons"
-	source "package/kodi-inputstream-adaptive/Config.in"
-	source "package/kodi-inputstream-rtmp/Config.in"
-endmenu
-
 menu "PVR addons"
 	source "package/kodi-pvr-argustv/Config.in"
 	source "package/kodi-pvr-dvblink/Config.in"
-- 
2.17.1

[Buildroot] [PATCH 18/18] package/kodi/Config.in: fix menus ordering

[Jerzy Grzegorek]

Fixes:
package/kodi/Config.in:318: Menus in: if BR2_PACKAGE_KODI,
                            are not alphabetically ordered;
                            correct order: '-', '_', digits, capitals, lowercase;
                            first incorrect menu: PVR addons

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 package/kodi/Config.in | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/package/kodi/Config.in b/package/kodi/Config.in
index 8e30e2f3ea..a7b4723d46 100644
--- a/package/kodi/Config.in
+++ b/package/kodi/Config.in
@@ -309,12 +309,6 @@ menu "Inputstream addons"
         source "package/kodi-inputstream-rtmp/Config.in"
 endmenu
 
-menu "Peripheral addons"
-	source "package/kodi-peripheral-joystick/Config.in"
-	source "package/kodi-peripheral-steamcontroller/Config.in"
-	source "package/kodi-peripheral-xarcade/Config.in"
-endmenu
-
 menu "PVR addons"
 	source "package/kodi-pvr-argustv/Config.in"
 	source "package/kodi-pvr-dvblink/Config.in"
@@ -335,6 +329,12 @@ menu "PVR addons"
 	source "package/kodi-pvr-wmc/Config.in"
 endmenu
 
+menu "Peripheral addons"
+        source "package/kodi-peripheral-joystick/Config.in"
+        source "package/kodi-peripheral-steamcontroller/Config.in"
+        source "package/kodi-peripheral-xarcade/Config.in"
+endmenu
+
 menu "Screensavers"
 	source "package/kodi-screensaver-asteroids/Config.in"
 	source "package/kodi-screensaver-asterwave/Config.in"
-- 
2.17.1

[Buildroot] [PATCH 01/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: rename variable 'm'

[Thomas Petazzoni]

Hello Ricardo,

Do you think you could review this patch series that touches
checkpackagelib ? It would be very useful to have your feedback.

Thanks a lot!

Thomas

On Tue,  3 Sep 2019 23:13:24 +0200
Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com> wrote:

> Rename variable 'm' for better readability.
> 
> Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  utils/checkpackagelib/lib_config.py | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
> index 225f92af13..a3fbd0e69e 100644
> --- a/utils/checkpackagelib/lib_config.py
> +++ b/utils/checkpackagelib/lib_config.py
> @@ -77,7 +77,8 @@ class CommentsMenusPackagesOrder(_CheckFunction):
>                                   "package/Config.in.host"]:
>              return
>  
> -        m = re.match(r'^\s*source ".*/([^/]*)/Config.in(.host)?"', text)
> +        source_line = re.match(r'^\s*source ".*/([^/]*)/Config.in(.host)?"', text)
> +
>          if text.startswith("comment ") or text.startswith("if ") or \
>             text.startswith("menu "):
>  
> @@ -113,9 +114,9 @@ class CommentsMenusPackagesOrder(_CheckFunction):
>              elif text.startswith("endmenu"):
>                  self.state = self.state[:-5]
>  
> -        elif m:
> +        elif source_line:
>              level = self.get_level()
> -            new_package = m.group(1)
> +            new_package = source_line.group(1)
>  
>              # We order _ before A, so replace it with .
>              new_package_ord = new_package.replace('_', '.')



-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 01/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: rename variable 'm'

[Ricardo Martincoski]

Hello,

On Mon, Sep 23, 2019 at 06:17 PM, Thomas Petazzoni wrote:

> Do you think you could review this patch series that touches
> checkpackagelib ? It would be very useful to have your feedback.

Sure.
I will review all patches. Probably only few patches each day.


Regards,
Ricardo

[Buildroot] [PATCH 01/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: rename variable 'm'

[Ricardo Martincoski]

Hello,

On Tue, Sep 03, 2019 at 06:13 PM, Jerzy Grzegorek wrote:

> checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: rename variable 'm'

I guess a better prefix would be:
utils/checkpackagelib: CommentsMenusPackagesOrder: rename variable 'm'
Since it is a minor nit, I give my tag below and the maintainer who applies
this patch can decide to change or not.

> 
> Rename variable 'm' for better readability.
> 
> Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>

Regards,
Ricardo

[Buildroot] [PATCH 02/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: change the type of variable 'level'

[Ricardo Martincoski]

Hello,

On Tue, Sep 03, 2019 at 06:13 PM, Jerzy Grzegorek wrote:

> checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: change the type of variable 'level'

Perhaps:
utils/checkpackagelib: CommentsMenusPackagesOrder: change the type of variable 'level'
However I have no good suggestion to make the subject shorter.

> Change the type of variable "level" to make it a class member.
> It will be used not only locally.
> 
> Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  utils/checkpackagelib/lib_config.py | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
> index a3fbd0e69e..8fad36f5ac 100644
> --- a/utils/checkpackagelib/lib_config.py
> +++ b/utils/checkpackagelib/lib_config.py
> @@ -93,12 +93,12 @@ class CommentsMenusPackagesOrder(_CheckFunction):
>                  elif text.startswith("menu"):
>                      self.state += "-menu"
>  
> -            level = self.get_level()
> +            self.level = self.get_level()

Please also initialize this variable:
     def before(self):
         self.state = ""
+        self.level = 0
like you did in patch 5.

With the initialization fixed you can add:
 Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>

Regards,
Ricardo

[Buildroot] [PATCH 03/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: get value of variable 'level'

[Ricardo Martincoski]

Hello,

On Tue, Sep 03, 2019 at 06:13 PM, Jerzy Grzegorek wrote:

> Get value of variable 'level' only just after the state change.
> 
> Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  utils/checkpackagelib/lib_config.py | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
> index 8fad36f5ac..e6a5f1b182 100644
> --- a/utils/checkpackagelib/lib_config.py
> +++ b/utils/checkpackagelib/lib_config.py
> @@ -114,8 +114,9 @@ class CommentsMenusPackagesOrder(_CheckFunction):
>              elif text.startswith("endmenu"):
>                  self.state = self.state[:-5]
>  
> -        elif source_line:
>              self.level = self.get_level()
> +
> +        elif source_line:
>              new_package = source_line.group(1)

Without the initialization in previous patch, if the script finds a 'source'
line without a previous 'menu' or 'comment' it would raise a AttributeError. 
You can test this by momentarily removing the first 'menu' line from
package/Config.in.
This use case do matters IMO because it could be found when testing
package/Config.in in br2-external trees.

If you change previous patch, you can add this tag to this patch as-is:
 Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>

Regards,
Ricardo

[Buildroot] [PATCH 04/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: add functions to initialize arrays elements

[Ricardo Martincoski]

Hello,

On Tue, Sep 03, 2019 at 06:13 PM, Jerzy Grzegorek wrote:

> checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: add functions to initialize arrays elements

This subject is long, but I don't have a good replacement to suggest.

Perhaps a commit message like this would be good:
Factor out two functions to initialize arrays elements. They will be
reused by followup patches.

> Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

With some improvement to the commit message (not necessarily what I
wrote):
 Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>

Regards,
Ricardo

[Buildroot] [PATCH 05/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: change the type of variable 'new_package'

[Ricardo Martincoski]

Hello,

On Tue, Sep 03, 2019 at 06:13 PM, Jerzy Grzegorek wrote:

> Change the type of variable 'new_package' to make it a class member.
> It will be used not only locally.
> Also initialize it.
> 
> Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>

In this patch you correctly did the initialization I talked about in the
review to patch 2.


Regards,
Ricardo

[Buildroot] [PATCH 14/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: check the order of comments menu

[Ricardo Martincoski]

Hello,

On Tue, Sep 03, 2019 at 06:13 PM, Jerzy Grzegorek wrote:

[snip]
> +++ b/utils/checkpackagelib/lib_config.py
> @@ -61,6 +61,10 @@ class AttributesOrder(_CheckFunction):
>  
>  
>  class CommentsMenusPackagesOrder(_CheckFunction):
> +    comment = [""]
> +    comments_order_checking = [False]
> +    print_comment_warning = [True]

This is not good. Please use instance variables initialized in before().
See my comment on the next patch.


Regards,
Ricardo

[Buildroot] [PATCH 15/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: check the order of menu of menus

[Ricardo Martincoski]

Hello,

On Tue, Sep 03, 2019 at 06:13 PM, Jerzy Grzegorek wrote:

[snip]
> +++ b/utils/checkpackagelib/lib_config.py
> @@ -65,6 +65,10 @@ class CommentsMenusPackagesOrder(_CheckFunction):
>      comments_order_checking = [False]
>      print_comment_warning = [True]
>  
> +    first_menu = [True, True]
> +    menu = [""]
> +    print_menu_warning = [True]

This is not good. Please use instance variables initialized in before().
before() is called... well... *before* every file is processed :-)
This will ensure the correct initial state for each file being processed.

Here an example of result to avoid:
With the whole series applied.
$ utils/check-package package/Config.in >/dev/null
2285 lines processed
2 warnings generated
$ utils/check-package package/Config.in.host >/dev/null
78 lines processed
0 warnings generated
$ utils/check-package package/Config.in package/Config.in.host >/dev/null
2363 lines processed
3 warnings generated

So we get a false warning for package/Config.in.host depending in other file(s)
being processed before it or not.
If you initialize all variables in before(), this error does not occur.


Regards,
Ricardo

[Buildroot] [PATCH 15/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: check the order of menu of menus

[Jerzy Grzegorek]

Hi Ricardo,

> Hello,
>
> On Tue, Sep 03, 2019 at 06:13 PM, Jerzy Grzegorek wrote:
>
> [snip]
>> +++ b/utils/checkpackagelib/lib_config.py
>> @@ -65,6 +65,10 @@ class CommentsMenusPackagesOrder(_CheckFunction):
>>       comments_order_checking = [False]
>>       print_comment_warning = [True]
>>   
>> +    first_menu = [True, True]
>> +    menu = [""]
>> +    print_menu_warning = [True]
> This is not good. Please use instance variables initialized in before().
> before() is called... well... *before* every file is processed :-)
> This will ensure the correct initial state for each file being processed.

So, should I also use instance variables

 ??? menu_of_packages = [""]
 ??? package = [""]
 ??? print_package_warning = [True]

initialized in before()?

Regards,
Jerzy


> Here an example of result to avoid:
> With the whole series applied.
> $ utils/check-package package/Config.in >/dev/null
> 2285 lines processed
> 2 warnings generated
> $ utils/check-package package/Config.in.host >/dev/null
> 78 lines processed
> 0 warnings generated
> $ utils/check-package package/Config.in package/Config.in.host >/dev/null
> 2363 lines processed
> 3 warnings generated
>
> So we get a false warning for package/Config.in.host depending in other file(s)
> being processed before it or not.
> If you initialize all variables in before(), this error does not occur.
>
>
> Regards,
> Ricardo

[Buildroot] [PATCH 01/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: rename variable 'm'

[Thomas Petazzoni]

Hello,

On Tue,  3 Sep 2019 23:13:24 +0200
Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com> wrote:

> Rename variable 'm' for better readability.
> 
> Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  utils/checkpackagelib/lib_config.py | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

I have applied patches 1 to 5 (included), with Ricardo Acked-by, after
fixing PATCH 02/18 to initialize self.level, and also taking into
account the other comments made by Ricardo on the commit titles and
commit logs.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 15/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: check the order of menu of menus

[Ricardo Martincoski]

Hello,

My free time comes and goes, sorry about my slow review.

On Wed, Sep 25, 2019 at 05:01 AM, Jerzy Grzegorek wrote:

>> On Tue, Sep 03, 2019 at 06:13 PM, Jerzy Grzegorek wrote:
>>
>> [snip]
>>> +++ b/utils/checkpackagelib/lib_config.py
>>> @@ -65,6 +65,10 @@ class CommentsMenusPackagesOrder(_CheckFunction):
>>>       comments_order_checking = [False]
>>>       print_comment_warning = [True]
>>>   
>>> +    first_menu = [True, True]
>>> +    menu = [""]
>>> +    print_menu_warning = [True]
>> This is not good. Please use instance variables initialized in before().
>> before() is called... well... *before* every file is processed :-)
>> This will ensure the correct initial state for each file being processed.
> 
> So, should I also use instance variables
> 
>  ??? menu_of_packages = [""]
>  ??? package = [""]
>  ??? print_package_warning = [True]
> 
> initialized in before()?

I think so.
If you will include this in this series or not, it's up to you.
Right now I don't know a good example to demonstrate a false warning generated
by those variables. But the code would be more future-proof if we change them.


Regards,
Ricardo

[Buildroot] [PATCH 06/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: remove '-comment' state before the '-menu' one

[Ricardo Martincoski]

Hello,

On Tue, Sep 03, 2019 at 06:13 PM, Jerzy Grzegorek wrote:

If you resend, please use prefix:
utils/checkpackagelib:

> The comment may contain packages or if-endif block(s) but not menu(s)
> of packages, so lets remove the '-comment' state before the '-menu' one.
> 
> Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

With the prefix fixed:
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>


Regards,
Ricardo

[Buildroot] [PATCH 07/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: separate the lines support

[Ricardo Martincoski]

Hello,

On Tue, Sep 03, 2019 at 06:13 PM, Jerzy Grzegorek wrote:

If you resend, please use prefix:
utils/checkpackagelib:

> The lines 'comment...', 'if ...' and 'menu ...' will be handled without
> sharing any code so lets separate their handling.
> 
> Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

With the prefix fixed:
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>


Regards,
Ricardo

[Buildroot] [PATCH 09/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: drop function get_line

[Ricardo Martincoski]

Hello,

On Tue, Sep 03, 2019 at 06:13 PM, Jerzy Grzegorek wrote:

> Because function get_line() has only one line so use it inline.

IMO it was better before this patch, so I would drop it, but if your patch
convince a maintainer, I am OK too.
So I will not give a tag, but it is not incorrect.


Regards,
Ricardo

[Buildroot] [PATCH 10/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: initialize 'menu_of_packages' array

[Ricardo Martincoski]

Hello,

On Tue, Sep 03, 2019 at 06:13 PM, Jerzy Grzegorek wrote:

If you resend, please use prefix:
utils/checkpackagelib:

Perhaps the commit message could say something like this:
'source' without a previous 'menu' is common in package/Config.in in
br2-externals.

> Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

With the prefix fixed:
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>


Regards,
Ricardo

[Buildroot] [PATCH 11/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: add more Config.in files to check

[Ricardo Martincoski]

Hello,

On Tue, Sep 03, 2019 at 06:13 PM, Jerzy Grzegorek wrote:

If you resend, please use prefix:
utils/checkpackagelib:

> Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
> Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

With the prefix fixed:
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>


Regards,
Ricardo

[Buildroot] [PATCH 09/18] checkpackagelib/lib_config.py: CommentsMenusPackagesOrder: drop function get_line

[Jerzy Grzegorek]

Hi Ricardo,

> Hello,
>
> On Tue, Sep 03, 2019 at 06:13 PM, Jerzy Grzegorek wrote:
>
>> Because function get_line() has only one line so use it inline.
> IMO it was better before this patch, so I would drop it, but if your patch
> convince a maintainer, I am OK too.
> So I will not give a tag, but it is not incorrect.

Ok, so I'll drop this patch.

Thanks for the review.

Regards,
Jerzy

> Regards, Ricardo