Skip to content

Commit

Permalink
utils/checkpackagelib: warn about ifdef on .mk
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Ricardo Martincoski <[email protected]>
Signed-off-by: Peter Korsgaard <[email protected]>
  • Loading branch information
ricardo-martincoski authored and jacmet committed Feb 6, 2023
1 parent 01c0fb3 commit 29c9b44
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 0 deletions.
1 change: 1 addition & 0 deletions package/busybox/busybox.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions package/uclibc/uclibc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions utils/checkpackagelib/lib_mk.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,24 @@
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)))
Expand Down
48 changes: 48 additions & 0 deletions utils/checkpackagelib/test_lib_mk.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,54 @@
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',
Expand Down

0 comments on commit 29c9b44

Please sign in to comment.