[Buildroot] [PATCH v4 15/15] utils/check-package: warn about disabled install

[Angelo Compagnucci <angelo.compagnucci@gmail.com>]

From: Ricardo Martincoski <ricardo.martincoski@gmail.com>

When a package defines one of these variables:
<pkg>_INSTALL_IMAGES_CMDS
<pkg>_PRE_INSTALL_IMAGES_HOOKS
<pkg>_POST_INSTALL_IMAGES_HOOKS
it also needs to enable <pkg>_INSTALL_IMAGES.

The same also occurs for <pkg>_INSTALL_STAGING.

And yet <pkg>_INSTALL_TARGET is a similar case, except that it defaults
to YES, while the other two default to NO.

So warn when a package defines a install command or hook that will never
be called because the equivalent flag is not set.
Take into account that some special cases have <pkg>_INSTALL_STAGING
defaulting to YES.
Also only warn when the script can determine for sure the flag to
install is disabled. So if a flag that defaults to YES is overridden to
NO inside a conditional, consider it may be enabled.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Co-authored-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
Signed-off-by: Angelo Compagnucci <angelo.compagnucci@gmail.com>
---
Changes v1 -> v4 (v1 was patch 28/29 of the "warn about symbols not
recognized by a package infra" series):
  - rebase on current master; the only file warned about,
    libglib2-bootstrap, is fixed by a previous patch.

 utils/checkpackagelib/lib_mk.py      |  89 ++++++++++++++++++
 utils/checkpackagelib/test_lib_mk.py | 129 +++++++++++++++++++++++++++
 2 files changed, 218 insertions(+)

diff --git a/utils/checkpackagelib/lib_mk.py b/utils/checkpackagelib/lib_mk.py
index 9e154e3049..aeb88cb32b 100644
--- a/utils/checkpackagelib/lib_mk.py
+++ b/utils/checkpackagelib/lib_mk.py
@@ -63,6 +63,95 @@ def get_package_prefix_from_filename(filename):
     return package, package_upper
 
 
+class DisabledInstall(_CheckFunction):
+    END_CONDITIONAL = re.compile(r"^\s*({})".format("|".join(end_conditional)))
+    START_CONDITIONAL = re.compile(r"^\s*({})".format("|".join(start_conditional)))
+    PATHS_WITH_INSTALL_STAGING_YES = [
+        "package/linux-tools/",
+        "toolchain/",
+    ]
+
+    def before(self):
+        _, self.package = get_package_prefix_from_filename(self.filename)
+
+        self.install_images = False
+        self.all_images_cmds = []
+        self.IMAGES_CMDS = re.compile(r"^\s*(define\s+)?{}({})".format(self.package, "|".join([
+            r"_PRE_INSTALL_IMAGES_HOOKS",
+            r"_INSTALL_IMAGES_CMDS",
+            r"_POST_INSTALL_IMAGES_HOOKS"])))
+        self.INSTALL_IMAGES = re.compile(r"\s*{}_INSTALL_IMAGES\s*=\s*YES".format(self.package))
+
+        self.install_staging = False
+        for path in self.PATHS_WITH_INSTALL_STAGING_YES:
+            if self.filename.startswith(path):
+                self.install_staging = True
+                break
+        self.all_staging_cmds = []
+        self.STAGING_CMDS = re.compile(r"^\s*(define\s+)?{}({})".format(self.package, "|".join([
+            r"_PRE_INSTALL_STAGING_HOOKS",
+            r"_INSTALL_STAGING_CMDS",
+            r"_POST_INSTALL_STAGING_HOOKS"])))
+        self.INSTALL_STAGING = re.compile(r"\s*{}_INSTALL_STAGING\s*=\s*YES".format(self.package))
+
+        self.conditional = 0
+        self.install_target = True
+        self.all_target_cmds = []
+        self.TARGET_CMDS = re.compile(r"^\s*(define\s+)?{}({})".format(self.package, "|".join([
+            r"_PRE_INSTALL_TARGET_HOOKS",
+            r"_INSTALL_TARGET_CMDS",
+            r"_POST_INSTALL_TARGET_HOOKS"])))
+        self.INSTALL_TARGET = re.compile(r"\s*{}_INSTALL_TARGET\s*=\s*NO".format(self.package))
+
+    def check_line(self, lineno, text):
+        if self.START_CONDITIONAL.search(text):
+            self.conditional += 1
+            return
+        if self.END_CONDITIONAL.search(text):
+            self.conditional -= 1
+            return
+
+        if self.INSTALL_IMAGES.search(text):
+            self.install_images = True
+            return
+        if self.INSTALL_STAGING.search(text):
+            self.install_staging = True
+            return
+        if self.INSTALL_TARGET.search(text):
+            if self.conditional == 0:
+                self.install_target = False
+            return
+
+        if self.IMAGES_CMDS.search(text):
+            self.all_images_cmds.append([lineno, text])
+            return
+        if self.STAGING_CMDS.search(text):
+            self.all_staging_cmds.append([lineno, text])
+            return
+        if self.TARGET_CMDS.search(text):
+            self.all_target_cmds.append([lineno, text])
+            return
+
+    def after(self):
+        warnings = []
+        if not self.install_images:
+            for lineno, text in self.all_images_cmds:
+                warnings.append(["{}:{}: never called because {}_INSTALL_IMAGES is disabled"
+                                 .format(self.filename, lineno, self.package),
+                                 text])
+        if not self.install_staging:
+            for lineno, text in self.all_staging_cmds:
+                warnings.append(["{}:{}: never called because {}_INSTALL_STAGING is disabled"
+                                 .format(self.filename, lineno, self.package),
+                                 text])
+        if not self.install_target:
+            for lineno, text in self.all_target_cmds:
+                warnings.append(["{}:{}: never called because {}_INSTALL_TARGET is disabled"
+                                 .format(self.filename, lineno, self.package),
+                                 text])
+        return warnings
+
+
 class Indent(_CheckFunction):
     COMMENT = re.compile(r"^\s*#")
     CONDITIONAL = re.compile(r"^\s*({})\s".format("|".join(start_conditional + end_conditional + continue_conditional)))
diff --git a/utils/checkpackagelib/test_lib_mk.py b/utils/checkpackagelib/test_lib_mk.py
index eca0a255f8..83ebb2f7bd 100644
--- a/utils/checkpackagelib/test_lib_mk.py
+++ b/utils/checkpackagelib/test_lib_mk.py
@@ -105,6 +105,135 @@ def test_get_package_prefix_from_filename(testname, filename, expected):
     assert [prefix_lower, prefix_upper] == expected
 
 
+DisabledInstall = [
+    ('ignore comment at beginning of line',
+     'any.mk',
+     '# very useful comment\n',
+     [[]]),
+    ('install images disabled (default)',
+     'any.mk',
+     'define ANY_INSTALL_IMAGES_CMDS\n'
+     'endef\n'
+     'ANY_PRE_INSTALL_IMAGES_HOOKS += MY_HOOK  \n'
+     'ANY_POST_INSTALL_IMAGES_HOOKS += MY_HOOK\t\n',
+     [[['any.mk:1: never called because ANY_INSTALL_IMAGES is disabled',
+        'define ANY_INSTALL_IMAGES_CMDS\n'],
+       ['any.mk:3: never called because ANY_INSTALL_IMAGES is disabled',
+        'ANY_PRE_INSTALL_IMAGES_HOOKS += MY_HOOK  \n'],
+       ['any.mk:4: never called because ANY_INSTALL_IMAGES is disabled',
+        'ANY_POST_INSTALL_IMAGES_HOOKS += MY_HOOK\t\n']]]),
+    ('install images disabled (explicit)',
+     'any.mk',
+     'ANY_INSTALL_IMAGES = NO\n'
+     'define ANY_INSTALL_IMAGES_CMDS\n'
+     'endef\n'
+     ' ANY_PRE_INSTALL_IMAGES_HOOKS += MY_HOOK\n'
+     '\tANY_POST_INSTALL_IMAGES_HOOKS += MY_HOOK\n',
+     [[['any.mk:2: never called because ANY_INSTALL_IMAGES is disabled',
+        'define ANY_INSTALL_IMAGES_CMDS\n'],
+       ['any.mk:4: never called because ANY_INSTALL_IMAGES is disabled',
+        ' ANY_PRE_INSTALL_IMAGES_HOOKS += MY_HOOK\n'],
+       ['any.mk:5: never called because ANY_INSTALL_IMAGES is disabled',
+        '\tANY_POST_INSTALL_IMAGES_HOOKS += MY_HOOK\n']]]),
+    ('install images enabled',
+     'any.mk',
+     '  ANY_INSTALL_IMAGES = YES  \n'
+     'define ANY_INSTALL_IMAGES_CMDS\n'
+     'endef\n'
+     'ANY_PRE_INSTALL_IMAGES_HOOKS += MY_HOOK\n'
+     'ANY_POST_INSTALL_IMAGES_HOOKS += MY_HOOK\n',
+     [[]]),
+    ('install staging enabled (default for toolchain)',
+     'toolchain/toolchain-any.mk',
+     'define ANY_INSTALL_STAGING_CMDS\n'
+     'endef\n'
+     'ANY_POST_INSTALL_STAGING_HOOKS += MY_HOOK\t\n',
+     [[]]),
+    ('install staging enabled (default for linux tools)',
+     'package/linux-tools/linux-tool-foo.mk.in',
+     'define FOO_INSTALL_STAGING_CMDS\n'
+     'endef\n',
+     [[]]),
+    ('install staging disabled (default)',
+     'any.mk',
+     'define ANY_INSTALL_STAGING_CMDS\n'
+     'endef\n'
+     'ANY_PRE_INSTALL_STAGING_HOOKS += MY_HOOK  \n'
+     'ANY_POST_INSTALL_STAGING_HOOKS += MY_HOOK\t\n',
+     [[['any.mk:1: never called because ANY_INSTALL_STAGING is disabled',
+        'define ANY_INSTALL_STAGING_CMDS\n'],
+       ['any.mk:3: never called because ANY_INSTALL_STAGING is disabled',
+        'ANY_PRE_INSTALL_STAGING_HOOKS += MY_HOOK  \n'],
+       ['any.mk:4: never called because ANY_INSTALL_STAGING is disabled',
+        'ANY_POST_INSTALL_STAGING_HOOKS += MY_HOOK\t\n']]]),
+    ('install staging disabled (explicit)',
+     'any.mk',
+     'ANY_INSTALL_STAGING = NO\n'
+     'define ANY_INSTALL_STAGING_CMDS\n'
+     'endef\n'
+     ' ANY_PRE_INSTALL_STAGING_HOOKS += MY_HOOK\n'
+     '\tANY_POST_INSTALL_STAGING_HOOKS += MY_HOOK\n',
+     [[['any.mk:2: never called because ANY_INSTALL_STAGING is disabled',
+        'define ANY_INSTALL_STAGING_CMDS\n'],
+       ['any.mk:4: never called because ANY_INSTALL_STAGING is disabled',
+        ' ANY_PRE_INSTALL_STAGING_HOOKS += MY_HOOK\n'],
+       ['any.mk:5: never called because ANY_INSTALL_STAGING is disabled',
+        '\tANY_POST_INSTALL_STAGING_HOOKS += MY_HOOK\n']]]),
+    ('install staging enabled',
+     'any.mk',
+     '  ANY_INSTALL_STAGING = YES  \n'
+     'define ANY_INSTALL_STAGING_CMDS\n'
+     'endef\n'
+     'ANY_PRE_INSTALL_STAGING_HOOKS += MY_HOOK\n'
+     'ANY_POST_INSTALL_STAGING_HOOKS += MY_HOOK\n',
+     [[]]),
+    ('install target disabled (explicit)',
+     'any.mk',
+     'ANY_INSTALL_TARGET = NO\n'
+     'define ANY_INSTALL_TARGET_CMDS\n'
+     'endef\n'
+     ' ANY_PRE_INSTALL_TARGET_HOOKS += MY_HOOK\n'
+     '\tANY_POST_INSTALL_TARGET_HOOKS += MY_HOOK\n',
+     [[['any.mk:2: never called because ANY_INSTALL_TARGET is disabled',
+        'define ANY_INSTALL_TARGET_CMDS\n'],
+       ['any.mk:4: never called because ANY_INSTALL_TARGET is disabled',
+        ' ANY_PRE_INSTALL_TARGET_HOOKS += MY_HOOK\n'],
+       ['any.mk:5: never called because ANY_INSTALL_TARGET is disabled',
+        '\tANY_POST_INSTALL_TARGET_HOOKS += MY_HOOK\n']]]),
+    ('install target enabled (explict)',
+     'any.mk',
+     'ANY_INSTALL_TARGET = YES\n'
+     'define ANY_INSTALL_TARGET_CMDS\n'
+     'endef\n'
+     'ANY_PRE_INSTALL_TARGET_HOOKS += MY_HOOK\n'
+     'ANY_POST_INSTALL_TARGET_HOOKS += MY_HOOK\n',
+     [[]]),
+    ('install target enabled (default)',
+     'any.mk',
+     'define ANY_INSTALL_TARGET_CMDS\n'
+     'endef\n'
+     'ANY_PRE_INSTALL_TARGET_HOOKS += MY_HOOK\n'
+     'ANY_POST_INSTALL_TARGET_HOOKS += MY_HOOK\n',
+     [[]]),
+    ('install target possibly enabled (conditional)',
+     'any.mk',
+     'ifeq ($(condition))\n'
+     'ANY_INSTALL_TARGET = NO\n'
+     'endif\n'
+     'define ANY_INSTALL_TARGET_CMDS\n'
+     'endef\n'
+     'ANY_PRE_INSTALL_TARGET_HOOKS += MY_HOOK\n'
+     'ANY_POST_INSTALL_TARGET_HOOKS += MY_HOOK\n',
+     [[]]),
+    ]
+
+
+@pytest.mark.parametrize('testname,filename,string,expected', DisabledInstall)
+def test_DisabledInstall(testname, filename, string, expected):
+    warnings = util.check_file(m.DisabledInstall, filename, string)
+    assert warnings == expected
+
+
 Indent = [
     ('ignore comment at beginning of line',
      'any',
-- 
2.53.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot