All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files
@ 2019-10-05 12:22 Jerzy Grzegorek
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 01/13] utils/checkpackagelib: CommentsMenusPackagesOrder: remove '-comment' state before the '-menu' one Jerzy Grzegorek
                   ` (13 more replies)
  0 siblings, 14 replies; 20+ messages in thread
From: Jerzy Grzegorek @ 2019-10-05 12:22 UTC (permalink / raw)
  To: buildroot

The main purpose of this patch series is to improve alphabetical order
checking of comments, menus and packages in Config.in files.
Patches 1-6 are preliminary ones and add small improvements.
Patches 7, 9, 10 do the main work.
Patches 8, 11-13 fix issues in Config.in files.

Changes v1 -> v2:
  - change the subject prefix checkpackagelib/lib_config.py to utils/checkpackagelib
    in all patches (Ricardo)
  - drop patch: 
    utils/checkpackagelib: CommentsMenusPackagesOrder: drop function get_line (Ricardo) 
  - use package arrays initialize in before() (Ricardo)
  - improve the commit message of patch 5 (Ricardo)

Regards,
Jerzy

Jerzy Grzegorek (13):
  utils/checkpackagelib: CommentsMenusPackagesOrder: remove '-comment'
    state before the '-menu' one
  utils/checkpackagelib: CommentsMenusPackagesOrder: separate the lines
    support
  utils/checkpackagelib: CommentsMenusPackagesOrder: use '-' to describe
    state
  utils/checkpackagelib: CommentsMenusPackagesOrder: use package arrays
    initialize in before()
  utils/checkpackagelib: CommentsMenusPackagesOrder: initialize
    'menu_of_packages' array
  utils/checkpackagelib: CommentsMenusPackagesOrder: add more Config.in
    files to check
  utils/checkpackagelib: CommentsMenusPackagesOrder: check package
    ordering just before 'if ' statement
  package/Config.in: fix packages ordering
  utils/checkpackagelib: CommentsMenusPackagesOrder: check the order of
    comments menu
  utils/checkpackagelib: CommentsMenusPackagesOrder: check the order of
    menu of menus
  package/Config.in: fix menus ordering
  package/kodi/Config.in: fix menus ordering
  package/kodi/Config.in: fix menus ordering

 package/Config.in                   |  58 ++++++------
 package/kodi/Config.in              |  12 +--
 utils/checkpackagelib/lib_config.py | 133 ++++++++++++++++++++++++----
 3 files changed, 150 insertions(+), 53 deletions(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 01/13] utils/checkpackagelib: CommentsMenusPackagesOrder: remove '-comment' state before the '-menu' one
  2019-10-05 12:22 [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Jerzy Grzegorek
@ 2019-10-05 12:22 ` Jerzy Grzegorek
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 02/13] utils/checkpackagelib: CommentsMenusPackagesOrder: separate the lines support Jerzy Grzegorek
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jerzy Grzegorek @ 2019-10-05 12:22 UTC (permalink / raw)
  To: buildroot

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: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 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 fc0df3dd17..499660feb7 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -107,6 +107,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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 02/13] utils/checkpackagelib: CommentsMenusPackagesOrder: separate the lines support
  2019-10-05 12:22 [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Jerzy Grzegorek
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 01/13] utils/checkpackagelib: CommentsMenusPackagesOrder: remove '-comment' state before the '-menu' one Jerzy Grzegorek
@ 2019-10-05 12:22 ` Jerzy Grzegorek
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 03/13] utils/checkpackagelib: CommentsMenusPackagesOrder: use '-' to describe state Jerzy Grzegorek
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jerzy Grzegorek @ 2019-10-05 12:22 UTC (permalink / raw)
  To: buildroot

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: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 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 499660feb7..06c066c5ca 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -95,22 +95,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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 03/13] utils/checkpackagelib: CommentsMenusPackagesOrder: use '-' to describe state
  2019-10-05 12:22 [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Jerzy Grzegorek
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 01/13] utils/checkpackagelib: CommentsMenusPackagesOrder: remove '-comment' state before the '-menu' one Jerzy Grzegorek
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 02/13] utils/checkpackagelib: CommentsMenusPackagesOrder: separate the lines support Jerzy Grzegorek
@ 2019-10-05 12:22 ` Jerzy Grzegorek
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 04/13] utils/checkpackagelib: CommentsMenusPackagesOrder: use package arrays initialize in before() Jerzy Grzegorek
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jerzy Grzegorek @ 2019-10-05 12:22 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 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 06c066c5ca..d1d4a4f549 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -115,7 +115,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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 04/13] utils/checkpackagelib: CommentsMenusPackagesOrder: use package arrays initialize in before()
  2019-10-05 12:22 [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Jerzy Grzegorek
                   ` (2 preceding siblings ...)
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 03/13] utils/checkpackagelib: CommentsMenusPackagesOrder: use '-' to describe state Jerzy Grzegorek
@ 2019-10-05 12:22 ` Jerzy Grzegorek
  2019-10-06 23:52   ` Ricardo Martincoski
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 05/13] utils/checkpackagelib: CommentsMenusPackagesOrder: initialize 'menu_of_packages' array Jerzy Grzegorek
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 20+ messages in thread
From: Jerzy Grzegorek @ 2019-10-05 12:22 UTC (permalink / raw)
  To: buildroot

Also order them alphabetically.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 utils/checkpackagelib/lib_config.py | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index d1d4a4f549..c5d0dc0fdb 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -61,14 +61,13 @@ class AttributesOrder(_CheckFunction):
 
 
 class CommentsMenusPackagesOrder(_CheckFunction):
-    menu_of_packages = [""]
-    package = [""]
-    print_package_warning = [True]
-
     def before(self):
-        self.state = ""
         self.level = 0
+        self.menu_of_packages = [""]
         self.new_package = ""
+        self.package = [""]
+        self.print_package_warning = [True]
+        self.state = ""
 
     def get_level(self):
         return len(self.state.split('-')) - 1
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 05/13] utils/checkpackagelib: CommentsMenusPackagesOrder: initialize 'menu_of_packages' array
  2019-10-05 12:22 [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Jerzy Grzegorek
                   ` (3 preceding siblings ...)
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 04/13] utils/checkpackagelib: CommentsMenusPackagesOrder: use package arrays initialize in before() Jerzy Grzegorek
@ 2019-10-05 12:22 ` Jerzy Grzegorek
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 06/13] utils/checkpackagelib: CommentsMenusPackagesOrder: add more Config.in files to check Jerzy Grzegorek
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jerzy Grzegorek @ 2019-10-05 12:22 UTC (permalink / raw)
  To: buildroot

'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: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 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 c5d0dc0fdb..3a84828902 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -63,7 +63,7 @@ class AttributesOrder(_CheckFunction):
 class CommentsMenusPackagesOrder(_CheckFunction):
     def before(self):
         self.level = 0
-        self.menu_of_packages = [""]
+        self.menu_of_packages = ["The top level menu"]
         self.new_package = ""
         self.package = [""]
         self.print_package_warning = [True]
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 06/13] utils/checkpackagelib: CommentsMenusPackagesOrder: add more Config.in files to check
  2019-10-05 12:22 [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Jerzy Grzegorek
                   ` (4 preceding siblings ...)
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 05/13] utils/checkpackagelib: CommentsMenusPackagesOrder: initialize 'menu_of_packages' array Jerzy Grzegorek
@ 2019-10-05 12:22 ` Jerzy Grzegorek
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 07/13] utils/checkpackagelib: CommentsMenusPackagesOrder: check package ordering just before 'if ' statement Jerzy Grzegorek
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jerzy Grzegorek @ 2019-10-05 12:22 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 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 3a84828902..142e8f745d 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -88,8 +88,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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 07/13] utils/checkpackagelib: CommentsMenusPackagesOrder: check package ordering just before 'if ' statement
  2019-10-05 12:22 [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Jerzy Grzegorek
                   ` (5 preceding siblings ...)
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 06/13] utils/checkpackagelib: CommentsMenusPackagesOrder: add more Config.in files to check Jerzy Grzegorek
@ 2019-10-05 12:22 ` Jerzy Grzegorek
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 08/13] package/Config.in: fix packages ordering Jerzy Grzegorek
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jerzy Grzegorek @ 2019-10-05 12:22 UTC (permalink / raw)
  To: buildroot

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: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 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 142e8f745d..709dbae0fe 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -108,6 +108,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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 08/13] package/Config.in: fix packages ordering
  2019-10-05 12:22 [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Jerzy Grzegorek
                   ` (6 preceding siblings ...)
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 07/13] utils/checkpackagelib: CommentsMenusPackagesOrder: check package ordering just before 'if ' statement Jerzy Grzegorek
@ 2019-10-05 12:22 ` Jerzy Grzegorek
  2019-10-05 13:30   ` Arnout Vandecappelle
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 09/13] utils/checkpackagelib: CommentsMenusPackagesOrder: check the order of comments menu Jerzy Grzegorek
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 20+ messages in thread
From: Jerzy Grzegorek @ 2019-10-05 12:22 UTC (permalink / raw)
  To: buildroot

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: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 package/Config.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/Config.in b/package/Config.in
index b52b2a96e3..d540ac00bf 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -591,7 +591,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"
@@ -671,6 +670,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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 09/13] utils/checkpackagelib: CommentsMenusPackagesOrder: check the order of comments menu
  2019-10-05 12:22 [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Jerzy Grzegorek
                   ` (7 preceding siblings ...)
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 08/13] package/Config.in: fix packages ordering Jerzy Grzegorek
@ 2019-10-05 12:22 ` Jerzy Grzegorek
  2019-10-05 13:35   ` Arnout Vandecappelle
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 10/13] utils/checkpackagelib: CommentsMenusPackagesOrder: check the order of menu of menus Jerzy Grzegorek
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 20+ messages in thread
From: Jerzy Grzegorek @ 2019-10-05 12:22 UTC (permalink / raw)
  To: buildroot

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: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 utils/checkpackagelib/lib_config.py | 43 ++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index 709dbae0fe..06d45bd6ec 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -62,10 +62,13 @@ class AttributesOrder(_CheckFunction):
 
 class CommentsMenusPackagesOrder(_CheckFunction):
     def before(self):
+        self.comment = [""]
+        self.comments_order_checking = [False]
         self.level = 0
         self.menu_of_packages = ["The top level menu"]
         self.new_package = ""
         self.package = [""]
+        self.print_comment_warning = [True]
         self.print_package_warning = [True]
         self.state = ""
 
@@ -84,6 +87,16 @@ class CommentsMenusPackagesOrder(_CheckFunction):
 
     def initialize_level_elements(self, text):
         self.level = self.get_level()
+
+        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):
@@ -101,7 +114,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"
@@ -145,6 +163,29 @@ class CommentsMenusPackagesOrder(_CheckFunction):
             self.level = self.get_level()
 
         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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 10/13] utils/checkpackagelib: CommentsMenusPackagesOrder: check the order of menu of menus
  2019-10-05 12:22 [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Jerzy Grzegorek
                   ` (8 preceding siblings ...)
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 09/13] utils/checkpackagelib: CommentsMenusPackagesOrder: check the order of comments menu Jerzy Grzegorek
@ 2019-10-05 12:22 ` Jerzy Grzegorek
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 11/13] package/Config.in: fix menus ordering Jerzy Grzegorek
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jerzy Grzegorek @ 2019-10-05 12:22 UTC (permalink / raw)
  To: buildroot

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: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 utils/checkpackagelib/lib_config.py | 37 ++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/utils/checkpackagelib/lib_config.py b/utils/checkpackagelib/lib_config.py
index 06d45bd6ec..97d256bb1c 100644
--- a/utils/checkpackagelib/lib_config.py
+++ b/utils/checkpackagelib/lib_config.py
@@ -64,11 +64,14 @@ class CommentsMenusPackagesOrder(_CheckFunction):
     def before(self):
         self.comment = [""]
         self.comments_order_checking = [False]
+        self.first_menu = [True, True]
         self.level = 0
+        self.menu = [""]
         self.menu_of_packages = ["The top level menu"]
         self.new_package = ""
         self.package = [""]
         self.print_comment_warning = [True]
+        self.print_menu_warning = [True]
         self.print_package_warning = [True]
         self.state = ""
 
@@ -91,11 +94,17 @@ class CommentsMenusPackagesOrder(_CheckFunction):
         try:
             self.comment[self.level] = ""
             self.comments_order_checking[self.level] = False
+            self.first_menu[self.level+1] = True
+            self.menu[self.level] = ""
             self.print_comment_warning[self.level] = True
+            self.print_menu_warning[self.level] = True
         except IndexError:
             self.comment.append("")
             self.comments_order_checking.append(False)
+            self.first_menu.append(True)
+            self.menu.append("")
             self.print_comment_warning.append(True)
+            self.print_menu_warning.append(True)
 
         self.initialize_package_level_elements(text)
 
@@ -148,7 +157,33 @@ class CommentsMenusPackagesOrder(_CheckFunction):
 
             self.state += "-menu"
 
-            self.initialize_level_elements(text)
+            self.level = self.get_level()
+
+            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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 11/13] package/Config.in: fix menus ordering
  2019-10-05 12:22 [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Jerzy Grzegorek
                   ` (9 preceding siblings ...)
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 10/13] utils/checkpackagelib: CommentsMenusPackagesOrder: check the order of menu of menus Jerzy Grzegorek
@ 2019-10-05 12:22 ` Jerzy Grzegorek
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 12/13] package/kodi/Config.in: " Jerzy Grzegorek
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Jerzy Grzegorek @ 2019-10-05 12:22 UTC (permalink / raw)
  To: buildroot

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: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 package/Config.in | 56 +++++++++++++++++++++++------------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/package/Config.in b/package/Config.in
index d540ac00bf..ca381ffdb6 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -1438,34 +1438,6 @@ menu "Hardware handling"
 	source "package/wiringpi/Config.in"
 endmenu
 
-menu "Javascript"
-	source "package/angularjs/Config.in"
-if BR2_PACKAGE_ANGULARJS
-menu "External AngularJS plugins"
-	source "package/angular-websocket/Config.in"
-endmenu
-endif
-	source "package/bootstrap/Config.in"
-	source "package/duktape/Config.in"
-	source "package/explorercanvas/Config.in"
-	source "package/flot/Config.in"
-	source "package/jquery/Config.in"
-if BR2_PACKAGE_JQUERY
-menu "External jQuery plugins"
-	source "package/jquery-datetimepicker/Config.in"
-	source "package/jquery-keyboard/Config.in"
-	source "package/jquery-mobile/Config.in"
-	source "package/jquery-sidebar/Config.in"
-	source "package/jquery-sparkline/Config.in"
-	source "package/jquery-ui/Config.in"
-	source "package/jquery-ui-themes/Config.in"
-	source "package/jquery-validation/Config.in"
-endmenu
-endif
-	source "package/jsmin/Config.in"
-	source "package/json-javascript/Config.in"
-endmenu
-
 menu "JSON/XML"
 	source "package/benejson/Config.in"
 	source "package/cjson/Config.in"
@@ -1501,6 +1473,34 @@ menu "JSON/XML"
 	source "package/yaml-cpp/Config.in"
 endmenu
 
+menu "Javascript"
+	source "package/angularjs/Config.in"
+if BR2_PACKAGE_ANGULARJS
+menu "External AngularJS plugins"
+	source "package/angular-websocket/Config.in"
+endmenu
+endif
+	source "package/bootstrap/Config.in"
+	source "package/duktape/Config.in"
+	source "package/explorercanvas/Config.in"
+	source "package/flot/Config.in"
+	source "package/jquery/Config.in"
+if BR2_PACKAGE_JQUERY
+menu "External jQuery plugins"
+	source "package/jquery-datetimepicker/Config.in"
+	source "package/jquery-keyboard/Config.in"
+	source "package/jquery-mobile/Config.in"
+	source "package/jquery-sidebar/Config.in"
+	source "package/jquery-sparkline/Config.in"
+	source "package/jquery-ui/Config.in"
+	source "package/jquery-ui-themes/Config.in"
+	source "package/jquery-validation/Config.in"
+endmenu
+endif
+	source "package/jsmin/Config.in"
+	source "package/json-javascript/Config.in"
+endmenu
+
 menu "Logging"
 	source "package/glog/Config.in"
 	source "package/liblog4c-localtime/Config.in"
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 12/13] package/kodi/Config.in: fix menus ordering
  2019-10-05 12:22 [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Jerzy Grzegorek
                   ` (10 preceding siblings ...)
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 11/13] package/Config.in: fix menus ordering Jerzy Grzegorek
@ 2019-10-05 12:22 ` Jerzy Grzegorek
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 13/13] " Jerzy Grzegorek
  2019-10-07 21:59 ` [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Arnout Vandecappelle
  13 siblings, 0 replies; 20+ messages in thread
From: Jerzy Grzegorek @ 2019-10-05 12:22 UTC (permalink / raw)
  To: buildroot

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: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 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 e8a504312c..a586f130b8 100644
--- a/package/kodi/Config.in
+++ b/package/kodi/Config.in
@@ -303,17 +303,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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 13/13] package/kodi/Config.in: fix menus ordering
  2019-10-05 12:22 [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Jerzy Grzegorek
                   ` (11 preceding siblings ...)
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 12/13] package/kodi/Config.in: " Jerzy Grzegorek
@ 2019-10-05 12:22 ` Jerzy Grzegorek
  2019-10-07 21:59 ` [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Arnout Vandecappelle
  13 siblings, 0 replies; 20+ messages in thread
From: Jerzy Grzegorek @ 2019-10-05 12:22 UTC (permalink / raw)
  To: buildroot

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: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 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 a586f130b8..c222c8fdf5 100644
--- a/package/kodi/Config.in
+++ b/package/kodi/Config.in
@@ -308,12 +308,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"
@@ -334,6 +328,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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 08/13] package/Config.in: fix packages ordering
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 08/13] package/Config.in: fix packages ordering Jerzy Grzegorek
@ 2019-10-05 13:30   ` Arnout Vandecappelle
  2019-10-05 15:23     ` Jerzy Grzegorek
  0 siblings, 1 reply; 20+ messages in thread
From: Arnout Vandecappelle @ 2019-10-05 13:30 UTC (permalink / raw)
  To: buildroot

 Hi Jerzy,

On 05/10/2019 14:22, Jerzy Grzegorek wrote:
> 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: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> ---
>  package/Config.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/Config.in b/package/Config.in
> index b52b2a96e3..d540ac00bf 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -591,7 +591,6 @@ endif
>  	source "package/jimtcl/Config.in"
>  	source "package/lua/Config.in"
>  	source "package/luainterpreter/Config.in"
> -	source "package/luajit/Config.in"

 This change is not good. After this change, the Lua modules will appear *above*
luajit when luajit is selected, instead of below luajit like they should. In
other words, this patch should not be applied.

 Which also implies that the preceding patch (adding the check for conditions to
follow there 'source' statement) should be removed. I anyway don't think it's
that valuable: this is something that is easily caught during review (unlike the
alphabetical ordering in general).

 Regards,
 Arnout


>  if BR2_PACKAGE_HAS_LUAINTERPRETER && !BR2_STATIC_LIBS
>  # lua modules are dynamically loaded, so not available on static builds
>  menu "Lua libraries/modules"
> @@ -671,6 +670,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"
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 09/13] utils/checkpackagelib: CommentsMenusPackagesOrder: check the order of comments menu
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 09/13] utils/checkpackagelib: CommentsMenusPackagesOrder: check the order of comments menu Jerzy Grzegorek
@ 2019-10-05 13:35   ` Arnout Vandecappelle
  0 siblings, 0 replies; 20+ messages in thread
From: Arnout Vandecappelle @ 2019-10-05 13:35 UTC (permalink / raw)
  To: buildroot



On 05/10/2019 14:22, Jerzy Grzegorek wrote:
> 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.

 I think this and the following check (and therefore also the three subsequent
fixes) should not be done.

 Indeed, for the comments/menus, it's likely that there are other concerns than
alphabetical ordering which are more relevant. For example, it makes more sense
for JSON to come after Javascript then before it.


 Regards,
 Arnout

> 
> Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
> Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[snip]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 08/13] package/Config.in: fix packages ordering
  2019-10-05 13:30   ` Arnout Vandecappelle
@ 2019-10-05 15:23     ` Jerzy Grzegorek
  2019-10-07 21:03       ` Arnout Vandecappelle
  0 siblings, 1 reply; 20+ messages in thread
From: Jerzy Grzegorek @ 2019-10-05 15:23 UTC (permalink / raw)
  To: buildroot

Hi Arnout,

>   Hi Jerzy,
>
> On 05/10/2019 14:22, Jerzy Grzegorek wrote:
>> 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: Ricardo Martincoski <ricardo.martincoski@gmail.com>
>> ---
>>   package/Config.in | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/package/Config.in b/package/Config.in
>> index b52b2a96e3..d540ac00bf 100644
>> --- a/package/Config.in
>> +++ b/package/Config.in
>> @@ -591,7 +591,6 @@ endif
>>   	source "package/jimtcl/Config.in"
>>   	source "package/lua/Config.in"
>>   	source "package/luainterpreter/Config.in"
>> -	source "package/luajit/Config.in"
>   This change is not good. After this change, the Lua modules will appear *above*
> luajit when luajit is selected, instead of below luajit like they should. In
> other words, this patch should not be applied.
>
>   Which also implies that the preceding patch (adding the check for conditions to
> follow there 'source' statement) should be removed. I anyway don't think it's
> that valuable: this is something that is easily caught during review (unlike the
> alphabetical ordering in general).


You're right. I will drop this patch.
In the previous patch (patch 7) I will change the condition to
if BR2_PACKAGE_FOO ...
The condition is now met in all such cases.

Thanks for the review.

Regards,
Jerzy


>
>   Regards,
>   Arnout
>
>
>>   if BR2_PACKAGE_HAS_LUAINTERPRETER && !BR2_STATIC_LIBS
>>   # lua modules are dynamically loaded, so not available on static builds
>>   menu "Lua libraries/modules"
>> @@ -671,6 +670,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"
>>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 04/13] utils/checkpackagelib: CommentsMenusPackagesOrder: use package arrays initialize in before()
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 04/13] utils/checkpackagelib: CommentsMenusPackagesOrder: use package arrays initialize in before() Jerzy Grzegorek
@ 2019-10-06 23:52   ` Ricardo Martincoski
  0 siblings, 0 replies; 20+ messages in thread
From: Ricardo Martincoski @ 2019-10-06 23:52 UTC (permalink / raw)
  To: buildroot

Hello,

On Sat, Oct 05, 2019 at 09:22 AM, Jerzy Grzegorek wrote:

> Also order them alphabetically.
> 
> Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
> Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>

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


Regards,
Ricardo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 08/13] package/Config.in: fix packages ordering
  2019-10-05 15:23     ` Jerzy Grzegorek
@ 2019-10-07 21:03       ` Arnout Vandecappelle
  0 siblings, 0 replies; 20+ messages in thread
From: Arnout Vandecappelle @ 2019-10-07 21:03 UTC (permalink / raw)
  To: buildroot



On 05/10/2019 17:23, Jerzy Grzegorek wrote:
> Hi Arnout,
> 
>> ? Hi Jerzy,
>>
>> On 05/10/2019 14:22, Jerzy Grzegorek wrote:
>>> 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: Ricardo Martincoski <ricardo.martincoski@gmail.com>
>>> ---
>>> ? package/Config.in | 2 +-
>>> ? 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/package/Config.in b/package/Config.in
>>> index b52b2a96e3..d540ac00bf 100644
>>> --- a/package/Config.in
>>> +++ b/package/Config.in
>>> @@ -591,7 +591,6 @@ endif
>>> ????? source "package/jimtcl/Config.in"
>>> ????? source "package/lua/Config.in"
>>> ????? source "package/luainterpreter/Config.in"
>>> -??? source "package/luajit/Config.in"
>> ? This change is not good. After this change, the Lua modules will appear *above*
>> luajit when luajit is selected, instead of below luajit like they should. In
>> other words, this patch should not be applied.
>>
>> ? Which also implies that the preceding patch (adding the check for conditions to
>> follow there 'source' statement) should be removed. I anyway don't think it's
>> that valuable: this is something that is easily caught during review (unlike the
>> alphabetical ordering in general).
> 
> 
> You're right. I will drop this patch.
> In the previous patch (patch 7) I will change the condition to
> if BR2_PACKAGE_FOO ...
> The condition is now met in all such cases.

 That doesn't really help... It *accidentally* happens to be the case that all
current conditions immediately follow the corresponding package include except
for lua, and it also happens that lua has this additional !static condition, so
by coincidence checking for pure 'if BR2_PACKAGE_FOO' solves it. But in general,
it is very well possible that we'll have other cases similar to lua where the
condition does not immediatel follow the package include.

 So I stand by my earlier statement that patch 7 and following should be removed.

 Regards,
 Arnout


> 
> Thanks for the review.
> 
> Regards,
> Jerzy
> 
> 
>>
>> ? Regards,
>> ? Arnout
>>
>>
>>> ? if BR2_PACKAGE_HAS_LUAINTERPRETER && !BR2_STATIC_LIBS
>>> ? # lua modules are dynamically loaded, so not available on static builds
>>> ? menu "Lua libraries/modules"
>>> @@ -671,6 +670,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"
>>>
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files
  2019-10-05 12:22 [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Jerzy Grzegorek
                   ` (12 preceding siblings ...)
  2019-10-05 12:22 ` [Buildroot] [PATCH v2 13/13] " Jerzy Grzegorek
@ 2019-10-07 21:59 ` Arnout Vandecappelle
  13 siblings, 0 replies; 20+ messages in thread
From: Arnout Vandecappelle @ 2019-10-07 21:59 UTC (permalink / raw)
  To: buildroot



On 05/10/2019 14:22, Jerzy Grzegorek wrote:
> The main purpose of this patch series is to improve alphabetical order
> checking of comments, menus and packages in Config.in files.
> Patches 1-6 are preliminary ones and add small improvements.
> Patches 7, 9, 10 do the main work.
> Patches 8, 11-13 fix issues in Config.in files.
> 
> Changes v1 -> v2:
>   - change the subject prefix checkpackagelib/lib_config.py to utils/checkpackagelib
>     in all patches (Ricardo)
>   - drop patch: 
>     utils/checkpackagelib: CommentsMenusPackagesOrder: drop function get_line (Ricardo) 
>   - use package arrays initialize in before() (Ricardo)
>   - improve the commit message of patch 5 (Ricardo)
> 
> Regards,
> Jerzy
> 
> Jerzy Grzegorek (13):
>   utils/checkpackagelib: CommentsMenusPackagesOrder: remove '-comment'
>     state before the '-menu' one
>   utils/checkpackagelib: CommentsMenusPackagesOrder: separate the lines
>     support
>   utils/checkpackagelib: CommentsMenusPackagesOrder: use '-' to describe
>     state
>   utils/checkpackagelib: CommentsMenusPackagesOrder: use package arrays
>     initialize in before()
>   utils/checkpackagelib: CommentsMenusPackagesOrder: initialize
>     'menu_of_packages' array

 I applied these to master, sometimes with small improvements to the commit message.

>   utils/checkpackagelib: CommentsMenusPackagesOrder: add more Config.in
>     files to check

 I applied this one as well, with one exception: I removed boot/Config.in
because it is currently NOT correctly sorted (arm-trusted-firmware should come
before the at91 packages).

 Just for kicks, I removed the whitelist entirely and ran check-package on all
Config.in files. It turned up quite a few errors still, and some of these are in
fact relevant (in qt5 and in toolchain-external). So I'm thinking, it may be
useful to turn the whitelist into a blacklist (after fixing the incorrect
ordering, of course). And instead of maintaining the blacklist in
checkpackagelib, it is probably better to do it with comments in the files
themselves:

# CommentsMenusPackagesOrder off


and set some class variable to False if that comment is found.

 There are a few menus where we can still bikeshed on the proper ordering, e.g.
gstreamer1.


>   utils/checkpackagelib: CommentsMenusPackagesOrder: check package
>     ordering just before 'if ' statement

 These and the following I have marked as Rejected in patchwork as I believe we
shouldn't do this kind of check automatically.

 Regards,
 Arnout

>   package/Config.in: fix packages ordering
>   utils/checkpackagelib: CommentsMenusPackagesOrder: check the order of
>     comments menu
>   utils/checkpackagelib: CommentsMenusPackagesOrder: check the order of
>     menu of menus
>   package/Config.in: fix menus ordering
>   package/kodi/Config.in: fix menus ordering
>   package/kodi/Config.in: fix menus ordering
> 
>  package/Config.in                   |  58 ++++++------
>  package/kodi/Config.in              |  12 +--
>  utils/checkpackagelib/lib_config.py | 133 ++++++++++++++++++++++++----
>  3 files changed, 150 insertions(+), 53 deletions(-)
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2019-10-07 21:59 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-05 12:22 [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Jerzy Grzegorek
2019-10-05 12:22 ` [Buildroot] [PATCH v2 01/13] utils/checkpackagelib: CommentsMenusPackagesOrder: remove '-comment' state before the '-menu' one Jerzy Grzegorek
2019-10-05 12:22 ` [Buildroot] [PATCH v2 02/13] utils/checkpackagelib: CommentsMenusPackagesOrder: separate the lines support Jerzy Grzegorek
2019-10-05 12:22 ` [Buildroot] [PATCH v2 03/13] utils/checkpackagelib: CommentsMenusPackagesOrder: use '-' to describe state Jerzy Grzegorek
2019-10-05 12:22 ` [Buildroot] [PATCH v2 04/13] utils/checkpackagelib: CommentsMenusPackagesOrder: use package arrays initialize in before() Jerzy Grzegorek
2019-10-06 23:52   ` Ricardo Martincoski
2019-10-05 12:22 ` [Buildroot] [PATCH v2 05/13] utils/checkpackagelib: CommentsMenusPackagesOrder: initialize 'menu_of_packages' array Jerzy Grzegorek
2019-10-05 12:22 ` [Buildroot] [PATCH v2 06/13] utils/checkpackagelib: CommentsMenusPackagesOrder: add more Config.in files to check Jerzy Grzegorek
2019-10-05 12:22 ` [Buildroot] [PATCH v2 07/13] utils/checkpackagelib: CommentsMenusPackagesOrder: check package ordering just before 'if ' statement Jerzy Grzegorek
2019-10-05 12:22 ` [Buildroot] [PATCH v2 08/13] package/Config.in: fix packages ordering Jerzy Grzegorek
2019-10-05 13:30   ` Arnout Vandecappelle
2019-10-05 15:23     ` Jerzy Grzegorek
2019-10-07 21:03       ` Arnout Vandecappelle
2019-10-05 12:22 ` [Buildroot] [PATCH v2 09/13] utils/checkpackagelib: CommentsMenusPackagesOrder: check the order of comments menu Jerzy Grzegorek
2019-10-05 13:35   ` Arnout Vandecappelle
2019-10-05 12:22 ` [Buildroot] [PATCH v2 10/13] utils/checkpackagelib: CommentsMenusPackagesOrder: check the order of menu of menus Jerzy Grzegorek
2019-10-05 12:22 ` [Buildroot] [PATCH v2 11/13] package/Config.in: fix menus ordering Jerzy Grzegorek
2019-10-05 12:22 ` [Buildroot] [PATCH v2 12/13] package/kodi/Config.in: " Jerzy Grzegorek
2019-10-05 12:22 ` [Buildroot] [PATCH v2 13/13] " Jerzy Grzegorek
2019-10-07 21:59 ` [Buildroot] [PATCH v2 00/13] Improve alphabetical order checking of Config.in files Arnout Vandecappelle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.