utils/checkpackagelib: warn about ifdef on .mk
There are two legitimate cases to prefer ifdef over ifeq in package recipes: command-line overrides are allowed for busybox and uclibc configs. Except for that, all package in tree already use ifeq, so warn the developer adding/changing a package to use ifeq instead of ifdef, in order to keep consistence across packages. file.mk:2: use ifeq ($(SYMBOL),y) instead of ifdef SYMBOL file.mk:5: use ifneq ($(SYMBOL),y) instead of ifndef SYMBOL The difference between ifeq and ifdef is that ifdef doesn't expand recursively. Add comments to busybox and uclibc packages to avoid a warning in such special cases. Cc: Arnout Vandecappelle <arnout@mind.be> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
This commit is contained in:
parent
01c0fb3862
commit
29c9b44355
@ -110,6 +110,7 @@ BUSYBOX_MAKE_OPTS = \
|
||||
|
||||
# specifying BUSYBOX_CONFIG_FILE on the command-line overrides the .config
|
||||
# setting.
|
||||
# check-package disable Ifdef
|
||||
ifndef BUSYBOX_CONFIG_FILE
|
||||
BUSYBOX_CONFIG_FILE = $(call qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG))
|
||||
endif
|
||||
|
@ -22,6 +22,7 @@ UCLIBC_DEPENDENCIES = host-gcc-initial linux-headers
|
||||
|
||||
# specifying UCLIBC_CONFIG_FILE on the command-line overrides the .config
|
||||
# setting.
|
||||
# check-package disable Ifdef
|
||||
ifndef UCLIBC_CONFIG_FILE
|
||||
UCLIBC_CONFIG_FILE = $(call qstrip,$(BR2_UCLIBC_CONFIG))
|
||||
endif
|
||||
|
@ -21,6 +21,24 @@ continue_conditional = ["elif", "else"]
|
||||
end_conditional = ["endif"]
|
||||
|
||||
|
||||
class Ifdef(_CheckFunction):
|
||||
IFDEF = re.compile(r"^\s*(else\s+|)(ifdef|ifndef)\s")
|
||||
|
||||
def check_line(self, lineno, text):
|
||||
m = self.IFDEF.search(text)
|
||||
if m is None:
|
||||
return
|
||||
word = m.group(2)
|
||||
if word == 'ifdef':
|
||||
return ["{}:{}: use ifeq ($(SYMBOL),y) instead of ifdef SYMBOL"
|
||||
.format(self.filename, lineno),
|
||||
text]
|
||||
else:
|
||||
return ["{}:{}: use ifneq ($(SYMBOL),y) instead of ifndef SYMBOL"
|
||||
.format(self.filename, lineno),
|
||||
text]
|
||||
|
||||
|
||||
class Indent(_CheckFunction):
|
||||
COMMENT = re.compile(r"^\s*#")
|
||||
CONDITIONAL = re.compile(r"^\s*({})\s".format("|".join(start_conditional + end_conditional + continue_conditional)))
|
||||
|
@ -3,6 +3,54 @@ import checkpackagelib.test_util as util
|
||||
import checkpackagelib.lib_mk as m
|
||||
|
||||
|
||||
Ifdef = [
|
||||
('ignore commented line',
|
||||
'any',
|
||||
'# ifdef\n',
|
||||
[]),
|
||||
('simple',
|
||||
'any',
|
||||
'\n'
|
||||
'ifdef BR2_PACKAGE_FWTS_EFI_RUNTIME_MODULE\n'
|
||||
'endif\n',
|
||||
[['any:2: use ifeq ($(SYMBOL),y) instead of ifdef SYMBOL',
|
||||
'ifdef BR2_PACKAGE_FWTS_EFI_RUNTIME_MODULE\n']]),
|
||||
('ignore indentation',
|
||||
'any',
|
||||
' ifdef FOO\n'
|
||||
' endif\n'
|
||||
'\tifdef BAR\n'
|
||||
'endif\n',
|
||||
[['any:1: use ifeq ($(SYMBOL),y) instead of ifdef SYMBOL',
|
||||
' ifdef FOO\n'],
|
||||
['any:3: use ifeq ($(SYMBOL),y) instead of ifdef SYMBOL',
|
||||
'\tifdef BAR\n']]),
|
||||
('typo',
|
||||
'any',
|
||||
'\n'
|
||||
'ifndef ($(BR2_ENABLE_LOCALE),y)\n'
|
||||
'endif\n',
|
||||
[['any:2: use ifneq ($(SYMBOL),y) instead of ifndef SYMBOL',
|
||||
'ifndef ($(BR2_ENABLE_LOCALE),y)\n']]),
|
||||
('else ifdef',
|
||||
'any',
|
||||
'else ifdef SYMBOL # comment\n',
|
||||
[['any:1: use ifeq ($(SYMBOL),y) instead of ifdef SYMBOL',
|
||||
'else ifdef SYMBOL # comment\n']]),
|
||||
('else ifndef',
|
||||
'any',
|
||||
'\t else ifndef\t($(SYMBOL),y) # comment\n',
|
||||
[['any:1: use ifneq ($(SYMBOL),y) instead of ifndef SYMBOL',
|
||||
'\t else ifndef\t($(SYMBOL),y) # comment\n']]),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize('testname,filename,string,expected', Ifdef)
|
||||
def test_Ifdef(testname, filename, string, expected):
|
||||
warnings = util.check_file(m.Ifdef, filename, string)
|
||||
assert warnings == expected
|
||||
|
||||
|
||||
Indent = [
|
||||
('ignore comment at beginning of line',
|
||||
'any',
|
||||
|
Loading…
Reference in New Issue
Block a user