[Buildroot] [PATCH 3/5] utils/checkpackagelib/lib_sysv: check SysV init scripts
Ricardo Martincoski
ricardo.martincoski at gmail.com
Sun Dec 26 18:49:17 UTC 2021
Enable the common checks:
- consecutive empty lines
- empty last line
- missing new line at end of file
- trailing space
- warn for executable files, with the hint to instead use
'$(INSTALL) -D -m 0755' in the .mk file
Check indent with tabs:
- add a simple check function to warn only when the indent is done
using spaces or a mix of tabs and spaces. It does not check indenting
levels, but it already makes the review easier, since it
diferentiates spaces and tabs.
Check variables:
- check DAEMON is defined
- when DAEMON is defined, check the filename is in the form S01daemon
- when PIDFILE is defined, expect it to be in /var/run and defined
using $DAEMON.
Signed-off-by: Ricardo Martincoski <ricardo.martincoski at gmail.com>
---
Please read the cover letter before applying.
---
utils/check-package | 5 +
utils/checkpackagelib/base.py | 3 +
utils/checkpackagelib/lib_sysv.py | 66 +++++++++++++
utils/checkpackagelib/test_lib_sysv.py | 131 +++++++++++++++++++++++++
utils/checkpackagelib/test_tool.py | 25 +++++
utils/checkpackagelib/tool.py | 2 +-
6 files changed, 231 insertions(+), 1 deletion(-)
create mode 100644 utils/checkpackagelib/lib_sysv.py
create mode 100644 utils/checkpackagelib/test_lib_sysv.py
diff --git a/utils/check-package b/utils/check-package
index 5fb430902d..f64daed84c 100755
--- a/utils/check-package
+++ b/utils/check-package
@@ -13,6 +13,7 @@ import checkpackagelib.lib_config
import checkpackagelib.lib_hash
import checkpackagelib.lib_mk
import checkpackagelib.lib_patch
+import checkpackagelib.lib_sysv
VERBOSE_LEVEL_TO_SHOW_IGNORED_FILES = 3
flags = None # Command line arguments.
@@ -66,6 +67,8 @@ DO_NOT_CHECK_INTREE = re.compile(r"|".join([
r"toolchain/toolchain-external/pkg-toolchain-external\.mk$",
]))
+SYSV_INIT_SCRIPT_FILENAME = re.compile(r"/S\d\d[^/]+$")
+
def get_lib_from_filename(fname):
if flags.intree_only:
@@ -85,6 +88,8 @@ def get_lib_from_filename(fname):
return checkpackagelib.lib_mk
if fname.endswith(".patch"):
return checkpackagelib.lib_patch
+ if SYSV_INIT_SCRIPT_FILENAME.search(fname):
+ return checkpackagelib.lib_sysv
return None
diff --git a/utils/checkpackagelib/base.py b/utils/checkpackagelib/base.py
index 73da925a03..f666e4110b 100644
--- a/utils/checkpackagelib/base.py
+++ b/utils/checkpackagelib/base.py
@@ -24,3 +24,6 @@ class _Tool(object):
def run(self):
pass
+
+ def hint(self):
+ return ""
diff --git a/utils/checkpackagelib/lib_sysv.py b/utils/checkpackagelib/lib_sysv.py
new file mode 100644
index 0000000000..77e6283b25
--- /dev/null
+++ b/utils/checkpackagelib/lib_sysv.py
@@ -0,0 +1,66 @@
+import os
+import re
+
+from checkpackagelib.base import _CheckFunction
+from checkpackagelib.lib import ConsecutiveEmptyLines # noqa: F401
+from checkpackagelib.lib import EmptyLastLine # noqa: F401
+from checkpackagelib.lib import NewlineAtEof # noqa: F401
+from checkpackagelib.lib import TrailingSpace # noqa: F401
+from checkpackagelib.tool import NotExecutable as NotExecutable_base
+
+
+class Indent(_CheckFunction):
+ INDENTED_WITH_SPACES = re.compile(r"^[\t]* ")
+
+ def check_line(self, lineno, text):
+ if self.INDENTED_WITH_SPACES.search(text.rstrip()):
+ return ["{}:{}: should be indented with tabs, see package/busybox/S01syslogd".format(self.filename, lineno),
+ text]
+
+
+class NotExecutable(NotExecutable_base):
+ def hint(self):
+ return ", just make sure you use '$(INSTALL) -D -m 0755' in the .mk file"
+
+
+# avoid check-package running both the parent and child classes. NOTE: NotExecutable_base.__name__ returns 'NotExecutable'
+del NotExecutable_base
+
+
+class Variables(_CheckFunction):
+ DAEMON_VAR = re.compile(r"^DAEMON=[\"']{0,1}([^\"']*)[\"']{0,1}")
+ PIDFILE_PATTERN = re.compile(r"/var/run/(\$DAEMON|\$\{DAEMON\}).pid")
+ PIDFILE_VAR = re.compile(r"^PIDFILE=[\"']{0,1}([^\"']*)[\"']{0,1}")
+
+ def before(self):
+ self.name = None
+
+ def check_line(self, lineno, text):
+ name_found = self.DAEMON_VAR.search(text.rstrip())
+ if name_found:
+ if self.name:
+ return ["{}:{}: DAEMON variable redefined, see package/busybox/S01syslogd".format(self.filename, lineno),
+ text]
+ self.name = name_found.group(1)
+ if '/' in self.name:
+ self.name = os.path.basename(self.name) # to be used in after() to check the expected filename
+ return ["{}:{}: Do not include path in DAEMON, see package/busybox/S01syslogd".format(self.filename, lineno),
+ text,
+ 'DAEMON="{}"'.format(self.name)]
+ return
+
+ pidfile_found = self.PIDFILE_VAR.search(text.rstrip())
+ if pidfile_found:
+ pidfile = pidfile_found.group(1)
+ if not self.PIDFILE_PATTERN.match(pidfile):
+ return ["{}:{}: For PIDFILE use the same pattern found in package/busybox/S01syslogd".format(self.filename, lineno),
+ text,
+ 'PIDFILE="/var/run/$DAEMON.pid"']
+
+ def after(self):
+ if self.name is None:
+ return ["{}:0: DAEMON variable not defined, see package/busybox/S01syslogd".format(self.filename)]
+ expected_filename = re.compile(r"S\d\d{}$".format(self.name))
+ if not expected_filename.match(os.path.basename(self.filename)):
+ return ["{}:0: filename should be S<number><number><deamon name>, see package/busybox/S01syslogd".format(self.filename),
+ "expecting S<number><number>{}".format(self.name)]
diff --git a/utils/checkpackagelib/test_lib_sysv.py b/utils/checkpackagelib/test_lib_sysv.py
new file mode 100644
index 0000000000..290539c0e4
--- /dev/null
+++ b/utils/checkpackagelib/test_lib_sysv.py
@@ -0,0 +1,131 @@
+import os
+import pytest
+import re
+import tempfile
+import checkpackagelib.test_util as util
+import checkpackagelib.lib_sysv as m
+from checkpackagelib.test_tool import check_file as tool_check_file
+
+workdir = os.path.join(tempfile.mkdtemp(suffix='-checkpackagelib-test-sysv'))
+workdir_regex = re.compile(r'/tmp/tmp[^/]*-checkpackagelib-test-sysv')
+
+
+Indent = [
+ ('empty file',
+ 'any',
+ '',
+ []),
+ ('empty line',
+ 'any',
+ '\n',
+ []),
+ ('ignore whitespace',
+ 'any',
+ ' \n',
+ []),
+ ('spaces',
+ 'any',
+ 'case "$1" in\n'
+ ' start)',
+ [['any:2: should be indented with tabs, see package/busybox/S01syslogd',
+ ' start)']]),
+ ('tab',
+ 'any',
+ 'case "$1" in\n'
+ '\tstart)',
+ []),
+ ('tabs and spaces',
+ 'any',
+ 'case "$1" in\n'
+ '\t start)',
+ [['any:2: should be indented with tabs, see package/busybox/S01syslogd',
+ '\t start)']]),
+ ('spaces and tabs',
+ 'any',
+ 'case "$1" in\n'
+ ' \tstart)',
+ [['any:2: should be indented with tabs, see package/busybox/S01syslogd',
+ ' \tstart)']]),
+ ]
+
+
+ at pytest.mark.parametrize('testname,filename,string,expected', Indent)
+def test_Indent(testname, filename, string, expected):
+ warnings = util.check_file(m.Indent, filename, string)
+ assert warnings == expected
+
+
+NotExecutable = [
+ ('SysV',
+ 'sh-shebang.sh',
+ 0o775,
+ '#!/bin/sh',
+ ["dir/sh-shebang.sh:0: This file does not need to be executable,"
+ " just make sure you use '$(INSTALL) -D -m 0755' in the .mk file"]),
+ ]
+
+
+ at pytest.mark.parametrize('testname,filename,permissions,string,expected', NotExecutable)
+def test_NotExecutable(testname, filename, permissions, string, expected):
+ warnings = tool_check_file(m.NotExecutable, filename, string, permissions)
+ assert warnings == expected
+
+
+Variables = [
+ ('empty file',
+ 'any',
+ '',
+ [['any:0: DAEMON variable not defined, see package/busybox/S01syslogd']]),
+ ('daemon and pidfile ok',
+ 'package/busybox/S01syslogd',
+ 'DAEMON="syslogd"\n'
+ 'PIDFILE="/var/run/$DAEMON.pid"\n',
+ []),
+ ('wrong filename',
+ 'package/busybox/S01syslog',
+ 'DAEMON="syslogd"\n'
+ 'PIDFILE="/var/run/${DAEMON}.pid"\n',
+ [['package/busybox/S01syslog:0: filename should be S<number><number><deamon name>, see package/busybox/S01syslogd',
+ 'expecting S<number><number>syslogd']]),
+ ('no pidfile ok',
+ 'S99something',
+ 'DAEMON="something"\n',
+ []),
+ ('hardcoded pidfile',
+ 'S99something',
+ 'DAEMON="something"\n'
+ 'PIDFILE="/var/run/something.pid"\n',
+ [['S99something:2: For PIDFILE use the same pattern found in package/busybox/S01syslogd',
+ 'PIDFILE="/var/run/something.pid"\n',
+ 'PIDFILE="/var/run/$DAEMON.pid"']]),
+ ('redefined daemon',
+ 'S50any',
+ 'DAEMON="any"\n'
+ 'DAEMON="other"\n',
+ [['S50any:2: DAEMON variable redefined, see package/busybox/S01syslogd',
+ 'DAEMON="other"\n']]),
+ ('daemon name with dash',
+ 'S82cups-browsed',
+ 'DAEMON="cups-browsed"',
+ []),
+ ('daemon with path',
+ 'S50avahi-daemon',
+ 'DAEMON=/usr/sbin/avahi-daemon',
+ [['S50avahi-daemon:1: Do not include path in DAEMON, see package/busybox/S01syslogd',
+ 'DAEMON=/usr/sbin/avahi-daemon',
+ 'DAEMON="avahi-daemon"']]),
+ ('daemon with path and wrong filename',
+ 'S50avahi',
+ 'DAEMON=/usr/sbin/avahi-daemon',
+ [['S50avahi:1: Do not include path in DAEMON, see package/busybox/S01syslogd',
+ 'DAEMON=/usr/sbin/avahi-daemon',
+ 'DAEMON="avahi-daemon"'],
+ ['S50avahi:0: filename should be S<number><number><deamon name>, see package/busybox/S01syslogd',
+ 'expecting S<number><number>avahi-daemon']]),
+ ]
+
+
+ at pytest.mark.parametrize('testname,filename,string,expected', Variables)
+def test_Variables(testname, filename, string, expected):
+ warnings = util.check_file(m.Variables, filename, string)
+ assert warnings == expected
diff --git a/utils/checkpackagelib/test_tool.py b/utils/checkpackagelib/test_tool.py
index e9e0838103..aab7cb51c9 100644
--- a/utils/checkpackagelib/test_tool.py
+++ b/utils/checkpackagelib/test_tool.py
@@ -39,3 +39,28 @@ NotExecutable = [
def test_NotExecutable(testname, filename, permissions, string, expected):
warnings = check_file(m.NotExecutable, filename, string, permissions)
assert warnings == expected
+
+
+NotExecutable_hint = [
+ ('no hint',
+ "",
+ 'sh-shebang.sh',
+ 0o775,
+ '#!/bin/sh',
+ ["dir/sh-shebang.sh:0: This file does not need to be executable"]),
+ ('hint',
+ ", very special hint",
+ 'sh-shebang.sh',
+ 0o775,
+ '#!/bin/sh',
+ ["dir/sh-shebang.sh:0: This file does not need to be executable, very special hint"]),
+ ]
+
+
+ at pytest.mark.parametrize('testname,hint,filename,permissions,string,expected', NotExecutable_hint)
+def test_NotExecutable_hint(testname, hint, filename, permissions, string, expected):
+ class NotExecutable(m.NotExecutable):
+ def hint(self):
+ return hint
+ warnings = check_file(NotExecutable, filename, string, permissions)
+ assert warnings == expected
diff --git a/utils/checkpackagelib/tool.py b/utils/checkpackagelib/tool.py
index f2007be1ff..e931272554 100644
--- a/utils/checkpackagelib/tool.py
+++ b/utils/checkpackagelib/tool.py
@@ -5,4 +5,4 @@ from checkpackagelib.base import _Tool
class NotExecutable(_Tool):
def run(self):
if os.access(self.filename, os.X_OK):
- return ["{}:0: This file does not need to be executable".format(self.filename)]
+ return ["{}:0: This file does not need to be executable{}".format(self.filename, self.hint())]
--
2.25.1
More information about the buildroot
mailing list