[Buildroot] [PATCH 8/8] utils/check-package: warn about overridden variables

Ricardo Martincoski ricardo.martincoski at gmail.com
Sun Jan 27 18:59:43 UTC 2019


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 at gmail.com>
Cc: Simon Dawson <spdawson at gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni at bootlin.com>
---
 package/gcc/gcc-final/gcc-final.mk |  2 +
 package/zmqpp/zmqpp.mk             |  1 +
 utils/checkpackagelib/lib_mk.py    | 60 ++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+)

diff --git a/package/gcc/gcc-final/gcc-final.mk b/package/gcc/gcc-final/gcc-final.mk
index 37b645d252..49f16f699e 100644
--- a/package/gcc/gcc-final/gcc-final.mk
+++ b/package/gcc/gcc-final/gcc-final.mk
@@ -55,36 +55,38 @@ endef
 # Languages supported by the cross-compiler
 GCC_FINAL_CROSS_LANGUAGES-y = c
 GCC_FINAL_CROSS_LANGUAGES-$(BR2_INSTALL_LIBSTDCPP) += c++
 GCC_FINAL_CROSS_LANGUAGES-$(BR2_TOOLCHAIN_BUILDROOT_FORTRAN) += fortran
 GCC_FINAL_CROSS_LANGUAGES = $(subst $(space),$(comma),$(GCC_FINAL_CROSS_LANGUAGES-y))
 
 HOST_GCC_FINAL_CONF_OPTS = \
 	$(HOST_GCC_COMMON_CONF_OPTS) \
 	--enable-languages=$(GCC_FINAL_CROSS_LANGUAGES) \
 	--with-build-time-tools=$(HOST_DIR)/$(GNU_TARGET_NAME)/bin
 
 HOST_GCC_FINAL_GCC_LIB_DIR = $(HOST_DIR)/$(GNU_TARGET_NAME)/lib*
 # The kernel wants to use the -m4-nofpu option to make sure that it
 # doesn't use floating point operations.
 ifeq ($(BR2_sh4)$(BR2_sh4eb),y)
 HOST_GCC_FINAL_CONF_OPTS += "--with-multilib-list=m4,m4-nofpu"
+# check-package OverriddenVariable
 HOST_GCC_FINAL_GCC_LIB_DIR = $(HOST_DIR)/$(GNU_TARGET_NAME)/lib/!m4*
 endif
 ifeq ($(BR2_sh4a)$(BR2_sh4aeb),y)
 HOST_GCC_FINAL_CONF_OPTS += "--with-multilib-list=m4a,m4a-nofpu"
+# check-package OverriddenVariable
 HOST_GCC_FINAL_GCC_LIB_DIR = $(HOST_DIR)/$(GNU_TARGET_NAME)/lib/!m4*
 endif
 
 ifeq ($(BR2_GCC_SUPPORTS_LIBCILKRTS),y)
 
 # libcilkrts does not support v8
 ifeq ($(BR2_sparc),y)
 HOST_GCC_FINAL_CONF_OPTS += --disable-libcilkrts
 endif
 
 # Pthreads are required to build libcilkrts
 ifeq ($(BR2_PTHREADS_NONE),y)
 HOST_GCC_FINAL_CONF_OPTS += --disable-libcilkrts
 endif
 
 ifeq ($(BR2_STATIC_LIBS),y)
diff --git a/package/zmqpp/zmqpp.mk b/package/zmqpp/zmqpp.mk
index 801766a7d8..ea6b50e826 100644
--- a/package/zmqpp/zmqpp.mk
+++ b/package/zmqpp/zmqpp.mk
@@ -6,32 +6,33 @@
 
 ZMQPP_VERSION = 4.2.0
 ZMQPP_SITE = $(call github,zeromq,zmqpp,$(ZMQPP_VERSION))
 ZMQPP_INSTALL_STAGING = YES
 ZMQPP_DEPENDENCIES = zeromq
 ZMQPP_LICENSE = MPL-2.0
 ZMQPP_LICENSE_FILES = LICENSE
 ZMQPP_MAKE_OPTS = LD="$(TARGET_CXX)" BUILD_PATH=./build PREFIX=/usr
 ZMQPP_LDFLAGS = $(TARGET_LDFLAGS) -lpthread
 ZMQPP_CONFIG = $(if $(BR2_ENABLE_DEBUG),debug,release)
 
 # gcc bug internal compiler error: in merge_overlapping_regs, at
 # regrename.c:304. This bug is fixed since gcc 6.
 # By setting CONFIG to empty, all optimizations such as -funroll-loops
 # -ffast-math -finline-functions -fomit-frame-pointer are disabled
 ifeq ($(BR2_or1k):$(BR2_TOOLCHAIN_GCC_AT_LEAST_6),y:)
+# check-package OverriddenVariable
 ZMQPP_CONFIG =
 endif
 
 ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
 ZMQPP_LDFLAGS += -latomic
 endif
 
 ifeq ($(BR2_PACKAGE_ZMQPP_CLIENT),y)
 ZMQPP_DEPENDENCIES += boost
 endif
 
 ifeq ($(BR2_STATIC_LIBS),y)
 ZMQPP_MAKE_OPTS += BUILD_STATIC=yes BUILD_SHARED=no
 else ifeq ($(BR2_SHARED_STATIC_LIBS),y)
 ZMQPP_MAKE_OPTS += BUILD_STATIC=yes BUILD_SHARED=yes
 else ifeq ($(BR2_SHARED_LIBS),y)
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
@@ -60,32 +60,92 @@ class Indent(_CheckFunction):
         # comment can be indented or not inside define ... endef, so ignore it
         if self.define and self.COMMENT.search(text):
             return
 
         if expect_tabs:
             if not text.startswith("\t"):
                 return ["{}:{}: expected indent with tabs"
                         .format(self.filename, lineno),
                         text]
         else:
             if text.startswith("\t"):
                 return ["{}:{}: unexpected indent with tabs"
                         .format(self.filename, lineno),
                         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
 
     def check_line(self, lineno, text):
         if self.skip or lineno > 6:
             return
 
         if lineno in [1, 5]:
             if lineno == 1 and text.startswith("include "):
                 self.skip = True
                 return
             if text.rstrip() != "#" * 80:
                 return ["{}:{}: should be 80 hashes ({}#writing-rules-mk)"
                         .format(self.filename, lineno, self.url_to_manual),
                         text,
-- 
2.17.1



More information about the buildroot mailing list