utils/check-package: warn about overridden variables

For the general case, appending values to variables is OK and also a
good practice, like this:
|PACKAGE_VAR = value1
|ifeq ...
|PACKAGE_VAR += value2

or this, when the above is not possible:
|PACKAGE_VAR = value1
|ifeq ...
|PACKAGE_VAR := $(PACKAGE_VAR), value2

But this override is an error:
|PACKAGE_VAR = value1
|PACKAGE_VAR = value2

as well this one:
|ifeq ...
|PACKAGE_VAR += value1
|endif
|PACKAGE_VAR = value2

And this override is error-prone:
|PACKAGE_VAR = value1
|ifeq ...
|PACKAGE_VAR = value2

Create a check function to warn about overridden variables.

Some variables are likely to have a default value that gets overridden
in a conditional, so ignore them. The name of such variables end in
_ARCH, _CPU, _SITE, _SOURCE or _VERSION.

After ignoring these variable names, there are a few exceptions to this
rule in the tree. For them use the comment that disables the check.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Simon Dawson <spdawson@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Tested-by: Titouan Christophe <titouan.christophe@railnova.eu>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>

1 files changed, 60 insertions, 0 deletions

diff --git a/utils/checkpackagelib/lib_mk.py b/utils/checkpackagelib/lib_mk.py
index 51c6577d2c..00efeb7fb2 100644
--- a/utils/checkpackagelib/lib_mk.py
+++ b/utils/checkpackagelib/lib_mk.py
@@ -73,6 +73,66 @@ class Indent(_CheckFunction):
                         text]
 
 
+class OverriddenVariable(_CheckFunction):
+    CONCATENATING = re.compile("^([A-Z0-9_]+)\s*(\+|:|)=\s*\$\(\\1\)")
+    END_CONDITIONAL = re.compile("^\s*({})".format("|".join(end_conditional)))
+    OVERRIDING_ASSIGNMENTS = [':=', "="]
+    START_CONDITIONAL = re.compile("^\s*({})".format("|".join(start_conditional)))
+    VARIABLE = re.compile("^([A-Z0-9_]+)\s*((\+|:|)=)")
+    USUALLY_OVERRIDDEN = re.compile("^[A-Z0-9_]+({})".format("|".join([
+        "_ARCH\s*=\s*",
+        "_CPU\s*=\s*",
+        "_SITE\s*=\s*",
+        "_SOURCE\s*=\s*",
+        "_VERSION\s*=\s*"])))
+
+    def before(self):
+        self.conditional = 0
+        self.unconditionally_set = []
+        self.conditionally_set = []
+
+    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
+
+        m = self.VARIABLE.search(text)
+        if m is None:
+            return
+        variable, assignment = m.group(1, 2)
+
+        if self.conditional == 0:
+            if variable in self.conditionally_set:
+                self.unconditionally_set.append(variable)
+                if assignment in self.OVERRIDING_ASSIGNMENTS:
+                    return ["{}:{}: unconditional override of variable {} previously conditionally set"
+                            .format(self.filename, lineno, variable),
+                            text]
+
+            if variable not in self.unconditionally_set:
+                self.unconditionally_set.append(variable)
+                return
+            if assignment in self.OVERRIDING_ASSIGNMENTS:
+                return ["{}:{}: unconditional override of variable {}"
+                        .format(self.filename, lineno, variable),
+                        text]
+        else:
+            if variable not in self.unconditionally_set:
+                self.conditionally_set.append(variable)
+                return
+            if self.CONCATENATING.search(text):
+                return
+            if self.USUALLY_OVERRIDDEN.search(text):
+                return
+            if assignment in self.OVERRIDING_ASSIGNMENTS:
+                return ["{}:{}: conditional override of variable {}"
+                        .format(self.filename, lineno, variable),
+                        text]
+
+
 class PackageHeader(_CheckFunction):
     def before(self):
         self.skip = False