Date: Tue, 23 Oct 2012 20:29:50 +0000 (UTC) From: Chris Rees <crees@FreeBSD.org> To: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: svn commit: r241968 - user/crees/rclint Message-ID: <201210232029.q9NKTofY050724@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: crees (ports committer) Date: Tue Oct 23 20:29:50 2012 New Revision: 241968 URL: http://svn.freebsd.org/changeset/base/241968 Log: Add simple Makefile for laziness Stop if errors cascade Add warn method for errors Check for empty assignments Add base/ports mode differentiation Added: user/crees/rclint/Makefile (contents, props changed) Modified: user/crees/rclint/errors.en user/crees/rclint/rclint.py Added: user/crees/rclint/Makefile ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ user/crees/rclint/Makefile Tue Oct 23 20:29:50 2012 (r241968) @@ -0,0 +1,43 @@ +# $FreeBSD$ + +CP?= /bin/cp +CUT?= /usr/bin/cut +MKDIR?= /bin/mkdir +PYTHON?=/usr/bin/env python +RM?= /bin/rm +SCRIPT= rclint.py +SED?= /usr/bin/sed +TAR?= /usr/bin/tar + +VER!= ${PYTHON} ${SCRIPT} --version 2>&1 +.for V in ${VER} +VERSION?= ${V:C,^([^-]+).*,\1,} +.endfor +VERSION_MAJOR= ${VERSION:C,([0-9]+).*,\1,} +VERSION_MINOR= ${VERSION:C,[0-9]+\.([0-9]+).*,\1,} +VERSION_MICRO= ${VERSION:C,[0-9]+\.[0-9]+\.([0-9]+).*,\1,} + +FILES= ${SCRIPT} errors.en problems.en + +.PHONY: majorbump minorbump microbump release + +majorbump: + M=$$(expr ${VERSION_MAJOR} + 1); \ + ${SED} -i '' -e "s,^\(MAJOR = \).*,\1$$M," ${SCRIPT} + ${SED} -i '' -e "s,^\(MINOR = \).*,\10," ${SCRIPT} + ${SED} -i '' -e "s,^\(MICRO = \).*,\10," ${SCRIPT} + +minorbump: + M=$$(expr ${VERSION_MINOR} + 1); \ + ${SED} -i '' -e "s,^\(MINOR = \).*,\1$$M," ${SCRIPT} + ${SED} -i '' -e "s,^\(MICRO = \).*,\10," ${SCRIPT} + +microbump: + M=$$(expr ${VERSION_MICRO} + 1); \ + ${SED} -i '' -e "s,^\(MICRO = \).*,\1$$M," ${SCRIPT} + +tarball: + ${MKDIR} rclint-${VERSION} && \ + ${CP} ${FILES} rclint-${VERSION} && \ + ${TAR} cvfz rclint-${VERSION}.tar.gz rclint-${VERSION} && \ + ${RM} -rf rclint-${VERSION} Modified: user/crees/rclint/errors.en ============================================================================== --- user/crees/rclint/errors.en Tue Oct 23 19:47:07 2012 (r241967) +++ user/crees/rclint/errors.en Tue Oct 23 20:29:50 2012 (r241968) @@ -1,3 +1,4 @@ +# $FreeBSD$ # Rigid format of this file; tab-separated tuples to define # error messages and explanations. # Yes, tab isn't a great separator; alternatives that aren't @@ -7,16 +8,26 @@ error_id message explanation file_order Order of rc file incorrect Order of the rc file should be shebang/header/$FreeBSD$/sourcing rc_subr/name/rcvar/load_rc_config/setting defaults/setting other definitions/defining functions. Do not include unassociated shell commands, and blocks must be separated by single blank lines. Single blank lines may appear inside the defaults, definitions and functions blocks +functions_chown Useless use of chown? Using chown in functions can nearly always be replaced with appropriate use of install(1) +functions_inline_brace Inline brace in function Put the opening brace for a function ({) on its own line functions_neverending Unclosed function block Functions must end with a closing brace on its own line functions_problem Function incorrectly defined Functions should not have spaces in the definition functions_short One-line functions discouraged; put command directly in variable It is wasteful to write a function just for one command. It is possible to put commands directly inside declarations; name_prestart="install -d -o $name_user /var/run/$name" for example -orphaned_line Orphaned line Do not put unassociated shell commands inside rc scripts; put them inside functions +name_mismatch Mismatched filename/PROVIDE/$name Convention dictates that the filename, PROVIDE line and $name should be the same value + +orphaned_line Orphaned line Do not put unassociated shell commands inside rc scripts; put them inside functions. Normally they can go inside a start_precmd function rcorder_keyword_freebsd Do not include FreeBSD in the KEYWORD rcorder line Historically FreeBSD scripts were marked in the KEYWORD section. This is no longer necessary +rcorder_keyword_shutdown Persistent service? Needs shutdown KEYWORD Missing shutdown KEYWORD; if the script starts a service that is persistent, it should have KEYWORD: shutdown in rcorder block rcorder_order rcorder block in the wrong order; should be PROVIDE/REQUIRE/BEFORE/KEYWORD See the article on RC scripting + +rcsid Missing FreeBSD RCSId keyword All rc scripts must contain a line beginning # $FreeBSD$. Do not include blank lines without comment markers at the beginning (#) until the script begins + rcvar_incorrect rcvar is not set correctly rcvar must be directly set to name_enable. Do not quote, and do not use indirection; ${name}_enable is slower than example_enable +run_rc_argument Incorrect argument to run_rc_command The last line of the file should be run_rc_command $1 + shebang Incorrect shebang used All rc scripts must start with the correct shebang; #!/bin/sh variables_defaults_mandatory_colon Override blanks in mandatory values (:=/:- vs =/-) Values that must not be blank (such as _enable) should be set by default as ${var:=value}; thus disallowing blank values (man sh) @@ -24,17 +35,12 @@ variables_defaults_non_mandatory_colon D variables_defaults_old_style Prefer condensed version for setting default values of variables When setting the default value for a variable, it is much less verbose and clearer to use the : ${variable=var} notation, as well as it being obvious that the source and destination variable are the same variables_order Order of variables incorrect Order of the variables should be setting defaults then setting other variables +value_empty Empty variable assignment There is almost never a need to assign an empty value to a variable; sh does not require initialisation. This line can almost certainly be removed value_quoted Do not quote values unless necessary Unless there are spaces in the value, quotes are unnecessary. With the syntax ${variable=value}, value can even contain spaces and does not need quoting -rcvar_missing rcvar is set late or not at all Setting rcvar must be done straight after setting name -rcvar_quoted rcvar is quoted Do not quote the value of rcvar - -rcsid Missing FreeBSD RCSId keyword All rc scripts must contain a line beginning # $FreeBSD$. Do not include blank lines without comment markers at the beginning (#) until the script begins - -run_rc_argument Incorrect argument to run_rc_command The last line of the file should be run_rc_command $1 run_rc_followed run_rc_command line is not the last line in the file Do not write anything after the run_rc_command line run_rc_quoted Quoted argument to run_rc_command No need to quote the argument to run_rc_command Modified: user/crees/rclint/rclint.py ============================================================================== --- user/crees/rclint/rclint.py Tue Oct 23 19:47:07 2012 (r241967) +++ user/crees/rclint/rclint.py Tue Oct 23 20:29:50 2012 (r241968) @@ -28,7 +28,7 @@ __version__ = '$FreeBSD$' MAJOR = 0 MINOR = 0 -MICRO = 0 +MICRO = 1 DATADIR = '.' @@ -47,6 +47,8 @@ class Db: if not e or e[0] == '#': continue self._contents.append(e.split('\t')) + # Count the errors and bail if too many + self.count = 0 def _get(self, key, index): for c in self._contents: @@ -60,16 +62,27 @@ class Db: def explanation(self, key): return self._get(key, 2) - def give(self, key, num): + def give(self, key, num=-1, level='error'): err = self.error(key) if err: - logging.error('[%d]: %s ' % (num+1, err)) + if level == 'error': + logging.error('[%d]: %s ' % (num+1, err)) + if level == 'warn': + logging.warn('[%d]: %s ' % (num+1, err)) if verbosity > 0: print(textwrap.fill(self.explanation(key), initial_indent='==> ', subsequent_indent=' ')) else: logging.error('No such error: %s' % key) + self.count += 1 + if self.count > 10: + hint = ' Try rerunning with -v option for extra details.' if verbosity == 0 else '' + logging.error('Error threshold reached-- further errors are unlikely to be helpful. Fix the errors and rerun.' + hint) + exit() + + def warn(self, key, num=-1, level='warn'): + self.give(key, num, level) class Statement: def __init__(self, line, number): @@ -136,6 +149,9 @@ class Variable(Statement): match = re.match(r': \${([^\s:=]+)(:?=)([^}]+)}', line) return match.groups() if match else False + def is_empty(self): + return False if re.match('[\'"]?[^\'"]+[\'"]?', self.value) else True + class Comment: def __init__(self, line, number): self.value = line if line and line[0] == '#' else False @@ -178,7 +194,9 @@ class RcsId: class Function: def __init__(self, lines, num): - if lines[1] and lines[1][0] == '{': + if lines[0] and lines[0][-1] == '{': + error.give('functions_inline_brace', num) + elif lines[1] and lines[1][0] == '{': try: self.name = re.match(r'([\S_]+)\(\)$', lines[0]).group(1) except: @@ -195,7 +213,7 @@ class Function: # Remove { and } lines from length self.length -= 2 logging.debug('Found function %s' % self.name) - else: + if not hasattr(self, 'value'): self.value = False def short(self): @@ -207,6 +225,29 @@ class Function: def contains_line(self, line): return True if line in self.linenumbers() else False +def get(objlist, name): + for o in objlist: + if o.name == name: + return o + else: + return False + +def do_ports_checking(lineobj, filename): + logging.debug('Now on ports-specific section') + logging.debug('Checking for defaults clobbering blank values') + for var in lineobj['Variable']: + if var.type in ('longhand', 'shorthand'): + if var.name.split('_')[-1] not in ('enable') and var.clobber: + error.give('variables_defaults_non_mandatory_colon', var.line) + elif not var.clobber and var.name.split('_')[-1] in ('enable'): + error.give('variables_defaults_mandatory_colon', var.line) + if var.type == 'longhand' and var.name == var.source: + error.give('variables_defaults_old_style', var.line) + return + +def do_src_checking(lineobj, filename): + return + def do_rclint(filename): logging.debug('Suck in file %s' % filename) try: @@ -248,7 +289,11 @@ def do_rclint(filename): logging.debug('Checking shebang') if len(lineobj['Shebang']) < 1: - error.give('shebang', num) + error.give('shebang') + + logging.debug('Checking RcsId') + if len(lineobj['RcsId']) < 1: + error.give('rcsid') logging.debug('Checking order of file') linenumbers = [] @@ -279,7 +324,7 @@ def do_rclint(filename): linenumbers.append(s.line) if sorted(linenumbers) != linenumbers: - error.give('file_order', 0) + error.give('file_order') logging.debug('Checking all lines are accounted for') for obj in list(lineobj.keys()): @@ -299,9 +344,16 @@ def do_rclint(filename): if sorted(linenumbers) != linenumbers: error.give('rcorder_order') + shutdownkeyword = False for o in lineobj['Rcorder']: - if o.type == 'KEYWORD' and 'freebsd' in [v.lower() for v in o.value]: - error.give('rcorder_keyword_freebsd', o.line) + if o.type == 'KEYWORD': + if 'freebsd' in [v.lower() for v in o.value]: + error.give('rcorder_keyword_freebsd', o.line) + elif 'shutdown' in o.value: + shutdownkeyword = True + + if not shutdownkeyword: + error.warn('rcorder_keyword_shutdown') logging.debug('Checking order of variables') linenumbers = [] @@ -310,13 +362,17 @@ def do_rclint(filename): if var.type in typ: linenumbers.append(var.line) if sorted(linenumbers) != linenumbers: - error.give('variables_order', 0) + error.give('variables_order') - logging.debug('Checking for pointless quoting') + logging.debug('Checking for pointless quoting and empty variables') for obj in lineobj['Variable']+lineobj['Statement']: if obj.pointless_quoted(): error.give('value_quoted', obj.line) + for v in lineobj['Variable']: + if v.is_empty(): + error.give('value_empty', v.line) + logging.debug('Checking for rcvar set correctly') for var in lineobj['Variable']: if var.name == 'name': @@ -328,28 +384,50 @@ def do_rclint(filename): except: error.give('file_order', var.line) - logging.debug('Checking for short functions') + logging.debug('Checking for function issues') for function in lineobj['Function']: if function.short(): error.give('functions_short', function.line) + for l in function.value: + if 'chown' in l: + error.warn('functions_chown', function.line) + + logging.debug('Checking for run_rc_command') + for s in lineobj['Statement']: + if s.type == 'run_rc_command': + if '$1' not in s.value: + error.give('run_rc_argument', s.line) + + # Strip .in from filename + logging.debug('Checking $name agrees with PROVIDE and filename') + fn = filename[:-3] if filename[-3:] == '.in' else filename + n = get(lineobj['Variable'], 'name').value + rcordervars = [] + for r in lineobj['Rcorder']: + if r.type != 'PROVIDE': + continue + for v in r.value: + rcordervars.append(v) + + if n != fn or n not in rcordervars: + error.give('name_mismatch') + + if mode == 'ports': + do_ports_checking(lineobj, filename) + if mode == 'base': + do_base_checking(lineobj, filename) - logging.debug('Checking for defaults clobbering blank values') - for var in lineobj['Variable']: - if var.type in ('longhand', 'shorthand'): - if var.name.split('_')[-1] not in ('enable') and var.clobber: - error.give('variables_defaults_non_mandatory_colon', var.line) - elif not var.clobber and var.name.split('_')[-1] in ('enable'): - error.give('variables_defaults_mandatory_colon', var.line) - if var.type == 'longhand' and var.name == var.source: - error.give('variables_defaults_old_style', var.line) parser = argparse.ArgumentParser() -parser.add_argument('filenames', nargs = '+') -parser.add_argument('--language', nargs = 1, type=str, default = ['en'], help = 'sets the language that errors are reported in') +parser.add_argument('filenames', nargs='+') +parser.add_argument('--language', nargs=1, type=str, default=['en'], help='sets the language that errors are reported in') parser.add_argument('-v', action='count', help='raises debug level; provides detailed explanations of errors') parser.add_argument('--version', action='version', version='%s.%s.%s-%s'%(MAJOR, MINOR, MICRO, __version__)) +parser.add_argument('-b', action='store_true', help='chooses base RC script mode') +parser.add_argument('-p', action='store_true', help='chooses ports RC script mode (default)') args = parser.parse_args() +mode = 'base' if args.b else 'ports' verbosity = args.v logging.basicConfig(level=logging.DEBUG if verbosity > 1 else logging.WARN)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201210232029.q9NKTofY050724>