From 146d97d293c2d1870846f6267b57243e1d6901da Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 07 2017 16:03:08 +0000 Subject: [PATCH 1/51] don't import from __future__ They are superfluous because we only use Python 3. --- diff --git a/src/_fedmod/module_generator.py b/src/_fedmod/module_generator.py index 7d5e7f2..ee03dcc 100644 --- a/src/_fedmod/module_generator.py +++ b/src/_fedmod/module_generator.py @@ -1,5 +1,3 @@ -from __future__ import absolute_import - import sys import modulemd import logging diff --git a/src/_fedmod/module_repoquery.py b/src/_fedmod/module_repoquery.py index 30ca443..0f60f63 100644 --- a/src/_fedmod/module_repoquery.py +++ b/src/_fedmod/module_repoquery.py @@ -1,5 +1,3 @@ -from __future__ import absolute_import - import json import sys import modulemd From f6042d28f4d625b6727f48a06dd3839fdf48e351 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 07 2017 16:03:55 +0000 Subject: [PATCH 2/51] add 'lint' command This only checks the summary at the moment. --- diff --git a/src/_fedmod/cli.py b/src/_fedmod/cli.py index a6d3be5..b238d0a 100644 --- a/src/_fedmod/cli.py +++ b/src/_fedmod/cli.py @@ -4,6 +4,8 @@ import logging from .module_generator import ModuleGenerator from .module_repoquery import ModuleRepoquery +from .modulemd_linter import ModuleMDLinter + from . import _depchase, _repodata, _fetchrepodata # fedmod uses click for argument parsing, but currently does its own @@ -129,3 +131,11 @@ def rpms_from_srpm(pkg): """Reports which RPMS are generated from the given SRPM""" rq = ModuleRepoquery() rq.get_rpms_for_srpm(pkg) + +# Check a modulemd YAML file for validity. +@_cli_commands.command('lint') +@click.argument("modulemd") +def lint(modulemd): + """Validates a given modulemd YAML file""" + linter = ModuleMDLinter(modulemd_path=modulemd) + linter.lint() diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py new file mode 100644 index 0000000..109b134 --- /dev/null +++ b/src/_fedmod/modulemd_linter.py @@ -0,0 +1,120 @@ +import inspect +from functools import reduce + +import modulemd + + +class guidelines_link(object): + """Decorate a linter method with a link to the guidelines (can be a full + URL or just an anchor on the main wiki page).""" + + guidelines_base_url = "https://fedoraproject.org/wiki/Module:Guidelines" + + def __init__(self, link): + self.link = link + + def __call__(self, fn): + if "://" in self.link: + fn.lint_guidelines_url = self.link + else: + anchor = self.link + if anchor.startswith("#"): + anchor = anchor[1:] + fn.lint_guidelines_url = "{}#{}".format(self.guidelines_base_url, + anchor) + return fn + + +class LintLevelDecorator(object): + """Decorator to specify a 'severity' level for a linter method.""" + + all_levels = set() + + def __init__(self, level): + assert(level not in self.all_levels) + self.level = level + self.all_levels.add(level) + + def __call__(self, fn): + fn.lint_level = self.level + return fn + + +info = LintLevelDecorator("info") +warning = LintLevelDecorator("warning") +error = LintLevelDecorator("error") + + +class ModuleMDLinter(object): + """Linter for modulemd YAML files or data.""" + + def __init__(self, modulemd_path=None, modulemd_str=None): + if bool(modulemd_path) == bool(modulemd_str): + raise ValueError( + "specify exactly one of modulemd_path or modulemd_str") + + mmd = self.mmd = modulemd.ModuleMetadata() + + self.modulemd_path = modulemd_path + + if not modulemd_str: + with open(modulemd_path, "r") as mmdfile: + self.modulemd_str = mmdfile.read() + else: + self.modulemd_str = modulemd_str + + mmd.loads(self.modulemd_str) + + def lint(self, levels=LintLevelDecorator.all_levels): + assert set(levels) <= LintLevelDecorator.all_levels + + lint_method_names = [x for x in dir(self) if x.startswith("lint_")] + + lint_methods = [y for y in + (getattr(self, x) for x in lint_method_names) + if callable(y) and y.lint_level in levels] + + assert all(getattr(x, "lint_level", None) + in LintLevelDecorator.all_levels + for x in lint_methods) + + flagged = [] + + for method in lint_methods: + try: + method() + except Exception as e: + flagged.append((method.lint_level, inspect.getdoc(method), + method.lint_guidelines_url, e)) + + longest_level_len = reduce(lambda x, y: max(x, len(y)), + LintLevelDecorator.all_levels, 0) + + for i, (lvl, text, url, exc) in enumerate(flagged): + print("[{lvl: <{len}}] {url}".format(lvl=lvl, + len=longest_level_len, + url=url or "")) + for l in text.split("\n"): + print("\t" + l) + + if i < len(flagged) - 1: + print() + + @error + @guidelines_link('#Module_summary_and_description') + def lint_summary_exists(self): + """Every module MUST include a short summary.""" + assert self.mmd.summary and self.mmd.summary.strip() + + @error + @guidelines_link('#Module_summary_and_description') + def lint_summary_one_sentence(self): + """The module summary is a one sentence concise description of the + module.""" + assert "." not in self.mmd.summary[:-1] + + @warning + @guidelines_link('#Module_summary_and_description') + def lint_summary_no_trailing_period(self): + """The summary SHOULD NOT end in a period.""" + assert not self.mmd.summary.strip().endswith(".") From 270d46e909d13021be87c860243a9bfbe04f37f8 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 07 2017 16:03:58 +0000 Subject: [PATCH 3/51] check if description ends in a period --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 109b134..f29b3a0 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -118,3 +118,9 @@ class ModuleMDLinter(object): def lint_summary_no_trailing_period(self): """The summary SHOULD NOT end in a period.""" assert not self.mmd.summary.strip().endswith(".") + + @warning + @guidelines_link('#Module_summary_and_description') + def lint_description_ends_in_period(self): + """The description SHOULD end in a period.""" + assert self.mmd.description.strip().endswith(".") From 17fe7454df0c99ef478237b87bbafced9d7cf04a Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 07 2017 16:03:58 +0000 Subject: [PATCH 4/51] execute linter methods in order of their definition --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index f29b3a0..b2c62ea 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -30,6 +30,8 @@ class LintLevelDecorator(object): all_levels = set() + lint_order = 0 + def __init__(self, level): assert(level not in self.all_levels) self.level = level @@ -37,6 +39,8 @@ class LintLevelDecorator(object): def __call__(self, fn): fn.lint_level = self.level + fn.lint_order = LintLevelDecorator.lint_order + LintLevelDecorator.lint_order += 1 return fn @@ -70,9 +74,10 @@ class ModuleMDLinter(object): lint_method_names = [x for x in dir(self) if x.startswith("lint_")] - lint_methods = [y for y in - (getattr(self, x) for x in lint_method_names) - if callable(y) and y.lint_level in levels] + lint_methods = sorted( + (y for y in (getattr(self, x) for x in lint_method_names) + if callable(y) and y.lint_level in levels), + key=lambda x: x.lint_order) assert all(getattr(x, "lint_level", None) in LintLevelDecorator.all_levels From afb748bbd8188eefb432ad67c647a27d0e9f7cab Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:36:29 +0000 Subject: [PATCH 5/51] add detail decorator This allows pointing out what should be examined further, e.g. which part of a guideline may have been violated. --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index b2c62ea..48941ac 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -49,6 +49,17 @@ warning = LintLevelDecorator("warning") error = LintLevelDecorator("error") +class detail(object): + """Decorate a linter method with details about what needs to be fixed.""" + + def __init__(self, detail): + self.detail = detail + + def __call__(self, fn): + fn.lint_detail = self.detail + return fn + + class ModuleMDLinter(object): """Linter for modulemd YAML files or data.""" @@ -89,19 +100,29 @@ class ModuleMDLinter(object): try: method() except Exception as e: + try: + lint_detail = method.lint_detail + except AttributeError: + lint_detail = None flagged.append((method.lint_level, inspect.getdoc(method), - method.lint_guidelines_url, e)) + lint_detail, method.lint_guidelines_url, e)) longest_level_len = reduce(lambda x, y: max(x, len(y)), LintLevelDecorator.all_levels, 0) - for i, (lvl, text, url, exc) in enumerate(flagged): + for i, (lvl, text, detail, url, exc) in enumerate(flagged): print("[{lvl: <{len}}] {url}".format(lvl=lvl, len=longest_level_len, url=url or "")) for l in text.split("\n"): print("\t" + l) + if detail: + print() + print("\tDETAIL:") + for l in detail.split("\n"): + print("\t" + l) + if i < len(flagged) - 1: print() From 195e056b6fbb59ed612c21b185f02afa197f0d9c Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:38:27 +0000 Subject: [PATCH 6/51] reformat output a little --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 48941ac..47d95d6 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -1,5 +1,4 @@ import inspect -from functools import reduce import modulemd @@ -107,13 +106,11 @@ class ModuleMDLinter(object): flagged.append((method.lint_level, inspect.getdoc(method), lint_detail, method.lint_guidelines_url, e)) - longest_level_len = reduce(lambda x, y: max(x, len(y)), - LintLevelDecorator.all_levels, 0) - for i, (lvl, text, detail, url, exc) in enumerate(flagged): - print("[{lvl: <{len}}] {url}".format(lvl=lvl, - len=longest_level_len, - url=url or "")) + print("[{lvl}] {url}".format(lvl=lvl.upper(), + url=url or "")) + + print() for l in text.split("\n"): print("\t" + l) From 088ea2afb5ec327cd05815bc30518279da4688d6 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:38:29 +0000 Subject: [PATCH 7/51] load raw YAML dict for low-level checks --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 47d95d6..b9d44f5 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -1,6 +1,7 @@ import inspect import modulemd +import yaml class guidelines_link(object): @@ -79,6 +80,9 @@ class ModuleMDLinter(object): mmd.loads(self.modulemd_str) + self.yaml_doc = yaml.safe_load(self.modulemd_str) + self.yaml_data = self.yaml_doc['data'] + def lint(self, levels=LintLevelDecorator.all_levels): assert set(levels) <= LintLevelDecorator.all_levels diff --git a/src/setup.py b/src/setup.py index 40ad2d4..284c2ae 100644 --- a/src/setup.py +++ b/src/setup.py @@ -30,6 +30,7 @@ setup( 'requests-toolbelt', 'lxml', 'attrs', + 'yaml', ], packages=find_packages(), ) From c180fe6d69ed032e0a9734da4548351e4ed4b4b3 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:39:00 +0000 Subject: [PATCH 8/51] add license block checks --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index b9d44f5..3de56fd 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -151,3 +151,40 @@ class ModuleMDLinter(object): def lint_description_ends_in_period(self): """The description SHOULD end in a period.""" assert self.mmd.description.strip().endswith(".") + + @error + @guidelines_link('#Module_licensing') + def lint_license_block_exists(self): + """Every module MUST contain a license block.""" + assert 'license' in self.yaml_data + + @error + @guidelines_link('#Module_licensing') + @detail("The license block must be a dictionary (key/value).") + def lint_license_block_keys(self): + """Every module MUST contain a license block.""" + assert isinstance(self.yaml_data['license'], dict) + + @error + @guidelines_link('#Module_licensing') + @detail("The module license block must exist.") + def lint_license_module_block_exists(self): + """Every module MUST contain a license block and declare a list of + the module's licenses.""" + assert 'module' in self.yaml_data['license'] + + @error + @guidelines_link('#Module_licensing') + @detail("The module license block must be a list.") + def lint_license_module_block_is_list(self): + """Every module MUST contain a license block and declare a list of + the module's licenses.""" + assert isinstance(self.yaml_data['license']['module'], list) + + @error + @guidelines_link('#Module_licensing') + @detail("The content license block must be a list if it exists.") + def lint_license_content(self): + """The packager MAY also define a list of content licenses.""" + if 'content' in self.yaml_data['license']: + assert isinstance(self.yaml_data['license']['content'], list) From f82abd4338cfa40987b56815366b287937beb7df Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:39:02 +0000 Subject: [PATCH 9/51] check that description exists --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 3de56fd..d522a2c 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -146,6 +146,12 @@ class ModuleMDLinter(object): """The summary SHOULD NOT end in a period.""" assert not self.mmd.summary.strip().endswith(".") + @error + @guidelines_link('#Module_summary_and_description') + def lint_description_exists(self): + """Every module MUST include a description.""" + assert self.mmd.description and self.mmd.description.strip() + @warning @guidelines_link('#Module_summary_and_description') def lint_description_ends_in_period(self): From 839feea1c683a4d7e0d3fa8cd08f174c844942a3 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:41:02 +0000 Subject: [PATCH 10/51] add prerequisite decorator This lets the linter skip checks that would inevitably fail because of a more fundamental problem. E.g. if the license block is missing there is no need to perform checks on the module license block. The argument to the decorator is the name of the method or methods which are prerequisite to the decorated method, with or without the "lint_" prefix. --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index d522a2c..d69cb3d 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -60,6 +60,27 @@ class detail(object): return fn +class prerequisite(object): + """Decorate a linter method with what checks are a prerequisite so as to + not overwhelm a user with subsequent errors.""" + + def __init__(self, *prerequisites): + assert len(prerequisites) + + # Allow specification with and without "lint_" prefix + prerequisites = set(x if x.startswith("lint_") else "lint_" + x + for x in prerequisites) + + self.prerequisites = prerequisites + + def __call__(self, fn): + try: + fn.lint_prerequisites.update(self.prerequisites) + except AttributeError: + fn.lint_prerequisites = self.prerequisites + return fn + + class ModuleMDLinter(object): """Linter for modulemd YAML files or data.""" @@ -98,8 +119,20 @@ class ModuleMDLinter(object): for x in lint_methods) flagged = [] + failed_or_skipped = set() for method in lint_methods: + skip_check = False + + for prerequisite in getattr(method, 'lint_prerequisites', ()): + if prerequisite in failed_or_skipped: + skip_check = True + failed_or_skipped.add(method.__name__) + break + + if skip_check: + continue + try: method() except Exception as e: @@ -109,6 +142,7 @@ class ModuleMDLinter(object): lint_detail = None flagged.append((method.lint_level, inspect.getdoc(method), lint_detail, method.lint_guidelines_url, e)) + failed_or_skipped.add(method.__name__) for i, (lvl, text, detail, url, exc) in enumerate(flagged): print("[{lvl}] {url}".format(lvl=lvl.upper(), @@ -135,6 +169,7 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_summary_and_description') + @prerequisite('summary_exists') def lint_summary_one_sentence(self): """The module summary is a one sentence concise description of the module.""" @@ -142,6 +177,7 @@ class ModuleMDLinter(object): @warning @guidelines_link('#Module_summary_and_description') + @prerequisite('summary_exists') def lint_summary_no_trailing_period(self): """The summary SHOULD NOT end in a period.""" assert not self.mmd.summary.strip().endswith(".") @@ -154,6 +190,7 @@ class ModuleMDLinter(object): @warning @guidelines_link('#Module_summary_and_description') + @prerequisite('description_exists') def lint_description_ends_in_period(self): """The description SHOULD end in a period.""" assert self.mmd.description.strip().endswith(".") @@ -167,6 +204,7 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_licensing') @detail("The license block must be a dictionary (key/value).") + @prerequisite('license_block_exists') def lint_license_block_keys(self): """Every module MUST contain a license block.""" assert isinstance(self.yaml_data['license'], dict) @@ -174,6 +212,7 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_licensing') @detail("The module license block must exist.") + @prerequisite('license_block_keys') def lint_license_module_block_exists(self): """Every module MUST contain a license block and declare a list of the module's licenses.""" @@ -182,6 +221,7 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_licensing') @detail("The module license block must be a list.") + @prerequisite('license_module_block_exists') def lint_license_module_block_is_list(self): """Every module MUST contain a license block and declare a list of the module's licenses.""" @@ -190,6 +230,7 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_licensing') @detail("The content license block must be a list if it exists.") + @prerequisite('license_block_keys') def lint_license_content(self): """The packager MAY also define a list of content licenses.""" if 'content' in self.yaml_data['license']: From 8172e7ff04f3ed2f2bd3b18d0cd5b643ca533710 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:41:09 +0000 Subject: [PATCH 11/51] use check*() methods rather than assert Assert statements are skipped if optimized execution is requested (python -O), therefore they can't be used to throw exceptions for failed checks. --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index d69cb3d..109ffa1 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -81,8 +81,18 @@ class prerequisite(object): return fn +class LintCheckFailed(Exception): + pass + + class ModuleMDLinter(object): - """Linter for modulemd YAML files or data.""" + """Linter for modulemd YAML files or data. + + Contains one method for each check to perform and scaffolding. The names of + the methods performing checks begin with 'lint_', they do nothing if the + check succeeds but throw a LintCheckFailed exception otherwise. An easy way + to do this is using the check()/check_true(), check_false() methods which + all accept boolean expressions.""" def __init__(self, modulemd_path=None, modulemd_str=None): if bool(modulemd_path) == bool(modulemd_str): @@ -135,7 +145,7 @@ class ModuleMDLinter(object): try: method() - except Exception as e: + except LintCheckFailed as e: try: lint_detail = method.lint_detail except AttributeError: @@ -161,11 +171,26 @@ class ModuleMDLinter(object): if i < len(flagged) - 1: print() + def check_true(self, expr): + """Check that a value is True or true-like.""" + if not expr: + raise LintCheckFailed() + + def check(self, expr): + """Synonym for check_true().""" + self.check_true(expr) + + def check_false(self, expr): + """Check that a value is not True or true-like.""" + self.check_true(not expr) + + # lint checks below + @error @guidelines_link('#Module_summary_and_description') def lint_summary_exists(self): """Every module MUST include a short summary.""" - assert self.mmd.summary and self.mmd.summary.strip() + self.check(self.mmd.summary and self.mmd.summary.strip()) @error @guidelines_link('#Module_summary_and_description') @@ -173,33 +198,33 @@ class ModuleMDLinter(object): def lint_summary_one_sentence(self): """The module summary is a one sentence concise description of the module.""" - assert "." not in self.mmd.summary[:-1] + self.check("." not in self.mmd.summary[:-1]) @warning @guidelines_link('#Module_summary_and_description') @prerequisite('summary_exists') def lint_summary_no_trailing_period(self): """The summary SHOULD NOT end in a period.""" - assert not self.mmd.summary.strip().endswith(".") + self.check(not self.mmd.summary.strip().endswith(".")) @error @guidelines_link('#Module_summary_and_description') def lint_description_exists(self): """Every module MUST include a description.""" - assert self.mmd.description and self.mmd.description.strip() + self.check(self.mmd.description and self.mmd.description.strip()) @warning @guidelines_link('#Module_summary_and_description') @prerequisite('description_exists') def lint_description_ends_in_period(self): """The description SHOULD end in a period.""" - assert self.mmd.description.strip().endswith(".") + self.check(self.mmd.description.strip().endswith(".")) @error @guidelines_link('#Module_licensing') def lint_license_block_exists(self): """Every module MUST contain a license block.""" - assert 'license' in self.yaml_data + self.check('license' in self.yaml_data) @error @guidelines_link('#Module_licensing') @@ -207,7 +232,7 @@ class ModuleMDLinter(object): @prerequisite('license_block_exists') def lint_license_block_keys(self): """Every module MUST contain a license block.""" - assert isinstance(self.yaml_data['license'], dict) + self.check(isinstance(self.yaml_data['license'], dict)) @error @guidelines_link('#Module_licensing') @@ -216,7 +241,7 @@ class ModuleMDLinter(object): def lint_license_module_block_exists(self): """Every module MUST contain a license block and declare a list of the module's licenses.""" - assert 'module' in self.yaml_data['license'] + self.check('module' in self.yaml_data['license']) @error @guidelines_link('#Module_licensing') @@ -225,7 +250,7 @@ class ModuleMDLinter(object): def lint_license_module_block_is_list(self): """Every module MUST contain a license block and declare a list of the module's licenses.""" - assert isinstance(self.yaml_data['license']['module'], list) + self.check(isinstance(self.yaml_data['license']['module'], list)) @error @guidelines_link('#Module_licensing') @@ -234,4 +259,4 @@ class ModuleMDLinter(object): def lint_license_content(self): """The packager MAY also define a list of content licenses.""" if 'content' in self.yaml_data['license']: - assert isinstance(self.yaml_data['license']['content'], list) + self.check(isinstance(self.yaml_data['license']['content'], list)) From 7107fbf5b16f4feb6a31c277af0dac347c6d2721 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:42:06 +0000 Subject: [PATCH 12/51] add --min-level option Extend the handling of lint levels so that comparing them is possible. Skip checks below the specified level. --- diff --git a/src/_fedmod/cli.py b/src/_fedmod/cli.py index b238d0a..885b2ad 100644 --- a/src/_fedmod/cli.py +++ b/src/_fedmod/cli.py @@ -4,8 +4,8 @@ import logging from .module_generator import ModuleGenerator from .module_repoquery import ModuleRepoquery -from .modulemd_linter import ModuleMDLinter +from . import modulemd_linter as mmdl from . import _depchase, _repodata, _fetchrepodata # fedmod uses click for argument parsing, but currently does its own @@ -134,8 +134,14 @@ def rpms_from_srpm(pkg): # Check a modulemd YAML file for validity. @_cli_commands.command('lint') +@click.option("--min-level", metavar="MINLEVEL", default='info', + help="The minimum level of checks to be performed ({}).".format( + ", ".join(mmdl.LintLevel.all_levels))) @click.argument("modulemd") -def lint(modulemd): +def lint(modulemd, min_level): """Validates a given modulemd YAML file""" - linter = ModuleMDLinter(modulemd_path=modulemd) - linter.lint() + if min_level not in mmdl.LintLevel.all_levels: + raise ValueError("The minimum level must be one of: {}".format( + ", ".join(mmdl.LintLevel.all_levels))) + linter = mmdl.ModuleMDLinter(modulemd_path=modulemd) + linter.lint(min_level=min_level) diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 109ffa1..8af5630 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -25,28 +25,34 @@ class guidelines_link(object): return fn -class LintLevelDecorator(object): +class LintLevel(object): """Decorator to specify a 'severity' level for a linter method.""" - all_levels = set() + all_levels = {} + max_level = -1 + # We store this here because every linter method needs to be decorated with + # a lint level. lint_order = 0 - def __init__(self, level): + def __init__(self, level_name, level): assert(level not in self.all_levels) + self.level_name = level_name self.level = level - self.all_levels.add(level) + self.all_levels[level_name] = level + LintLevel.max_level = max(LintLevel.max_level, level) def __call__(self, fn): fn.lint_level = self.level - fn.lint_order = LintLevelDecorator.lint_order - LintLevelDecorator.lint_order += 1 + fn.lint_level_name = self.level_name + fn.lint_order = LintLevel.lint_order + LintLevel.lint_order += 1 return fn -info = LintLevelDecorator("info") -warning = LintLevelDecorator("warning") -error = LintLevelDecorator("error") +info = LintLevel("info", 0) +warning = LintLevel("warning", 1) +error = LintLevel("error", 2) class detail(object): @@ -114,20 +120,29 @@ class ModuleMDLinter(object): self.yaml_doc = yaml.safe_load(self.modulemd_str) self.yaml_data = self.yaml_doc['data'] - def lint(self, levels=LintLevelDecorator.all_levels): - assert set(levels) <= LintLevelDecorator.all_levels + def lint(self, min_level=None): + # ensure that min_level describes a valid level + if min_level is None: + min_level = LintLevel.max_level + elif isinstance(min_level, int): + assert 0 <= min_level <= LintLevel.max_level + else: + assert min_level in LintLevel.all_levels + min_level = LintLevel.all_levels[min_level] lint_method_names = [x for x in dir(self) if x.startswith("lint_")] + # ensure all lint methods are properly decorated + assert all( + 0 <= getattr(getattr(self, x), "lint_level", -1) + <= LintLevel.max_level + for x in lint_method_names) + lint_methods = sorted( (y for y in (getattr(self, x) for x in lint_method_names) - if callable(y) and y.lint_level in levels), + if callable(y) and y.lint_level >= min_level), key=lambda x: x.lint_order) - assert all(getattr(x, "lint_level", None) - in LintLevelDecorator.all_levels - for x in lint_methods) - flagged = [] failed_or_skipped = set() @@ -150,7 +165,7 @@ class ModuleMDLinter(object): lint_detail = method.lint_detail except AttributeError: lint_detail = None - flagged.append((method.lint_level, inspect.getdoc(method), + flagged.append((method.lint_level_name, inspect.getdoc(method), lint_detail, method.lint_guidelines_url, e)) failed_or_skipped.add(method.__name__) From 147c7eea2c97aaad87134f82235253e8a47b8f30 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:42:09 +0000 Subject: [PATCH 13/51] use click.Choice for validating --min-level --- diff --git a/src/_fedmod/cli.py b/src/_fedmod/cli.py index 885b2ad..0f93574 100644 --- a/src/_fedmod/cli.py +++ b/src/_fedmod/cli.py @@ -135,13 +135,11 @@ def rpms_from_srpm(pkg): # Check a modulemd YAML file for validity. @_cli_commands.command('lint') @click.option("--min-level", metavar="MINLEVEL", default='info', + type=click.Choice(mmdl.LintLevel.all_levels), help="The minimum level of checks to be performed ({}).".format( ", ".join(mmdl.LintLevel.all_levels))) @click.argument("modulemd") def lint(modulemd, min_level): """Validates a given modulemd YAML file""" - if min_level not in mmdl.LintLevel.all_levels: - raise ValueError("The minimum level must be one of: {}".format( - ", ".join(mmdl.LintLevel.all_levels))) linter = mmdl.ModuleMDLinter(modulemd_path=modulemd) linter.lint(min_level=min_level) From ea8f4631a5837a0f91d947861439c015baa10ff6 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:43:19 +0000 Subject: [PATCH 14/51] format guideline blurbs and problem details better --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 8af5630..4761128 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -1,9 +1,16 @@ import inspect +from shutil import get_terminal_size + +from click.formatting import wrap_text import modulemd + import yaml +TEXT_INDENT = " " + + class guidelines_link(object): """Decorate a linter method with a link to the guidelines (can be a full URL or just an anchor on the main wiki page).""" @@ -169,19 +176,25 @@ class ModuleMDLinter(object): lint_detail, method.lint_guidelines_url, e)) failed_or_skipped.add(method.__name__) + width, height = get_terminal_size() + for i, (lvl, text, detail, url, exc) in enumerate(flagged): print("[{lvl}] {url}".format(lvl=lvl.upper(), url=url or "")) print() - for l in text.split("\n"): - print("\t" + l) + print(wrap_text(text=text, width=width - 2, + initial_indent=TEXT_INDENT, + subsequent_indent=TEXT_INDENT, + preserve_paragraphs=True)) if detail: print() - print("\tDETAIL:") - for l in detail.split("\n"): - print("\t" + l) + print(TEXT_INDENT + "DETAIL:") + print(wrap_text(text=detail, width=width - 2, + initial_indent=TEXT_INDENT, + subsequent_indent=TEXT_INDENT, + preserve_paragraphs=True)) if i < len(flagged) - 1: print() From d952f4f9f64181788e6c48aebe86b3364a43bce3 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:43:58 +0000 Subject: [PATCH 15/51] mention recognized license keys --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 4761128..8a4e61b 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -256,7 +256,9 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_licensing') - @detail("The license block must be a dictionary (key/value).") + @detail( + "The license block must be a dictionary (key/value). Recognized keys" + " are 'module' and 'content'.") @prerequisite('license_block_exists') def lint_license_block_keys(self): """Every module MUST contain a license block.""" From 9d3cdefb902ce6799c2845f380916c9259c1368f Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:43:59 +0000 Subject: [PATCH 16/51] flag fields which should be set during build --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 8a4e61b..b8a996d 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -283,6 +283,23 @@ class ModuleMDLinter(object): self.check(isinstance(self.yaml_data['license']['module'], list)) @error + @guidelines_link( + '#Module_name.2C_update_stream.2C_version.2C_context_and_architecture') + def lint_no_manual_name_stream_version_context_arch(self): + """Module packagers MUST NOT define values for name, stream, version, + context or architecture manually but should rather expect the Module + Build Service to do it for them.""" + self.check_false(self.mmd.name or self.mmd.stream or self.mmd.version + or self.mmd.context or self.mmd.arch) + + @error + @guidelines_link('#Module_Service_Levels_and_End_of_Life') + def lint_no_manual_eol(self): + """Module packagers MUST NOT define the EOL in the modulemd but should + define it in other infrastructure tooling.""" + self.check_false(self.mmd.eol) + + @error @guidelines_link('#Module_licensing') @detail("The content license block must be a list if it exists.") @prerequisite('license_block_keys') From 759d75b1977d74dac72557121db52f7ca8f1523f Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:50:06 +0000 Subject: [PATCH 17/51] add checks for the dependencies block --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index b8a996d..71ba0d2 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -107,6 +107,8 @@ class ModuleMDLinter(object): to do this is using the check()/check_true(), check_false() methods which all accept boolean expressions.""" + yaml_scalar_types = (str, int, float) + def __init__(self, modulemd_path=None, modulemd_str=None): if bool(modulemd_path) == bool(modulemd_str): raise ValueError( @@ -307,3 +309,66 @@ class ModuleMDLinter(object): """The packager MAY also define a list of content licenses.""" if 'content' in self.yaml_data['license']: self.check(isinstance(self.yaml_data['license']['content'], list)) + + @error + @guidelines_link('#Module_dependencies') + @detail("The dependencies block must be a dictionary (key/value).") + def lint_dependencies_dict(self): + """Modules MAY depend on other modules. These module relationships are + listed in the dependencies block. Dependencies are expressed using + module names and their stream names.""" + if 'dependencies' in self.mmd: + self.check(isinstance(self.yaml_data['dependencies'], dict)) + + @error + @guidelines_link('#Module_dependencies') + @detail("The 'buildrequires' dependencies block must be a dictionary" + " (key/value).") + @prerequisite('dependencies_dict') + def lint_buildrequires(self): + """Modules MAY depend on other modules. These module relationships are + listed in the dependencies block. Dependencies are expressed using + module names and their stream names.""" + self.check(isinstance(self.yaml_data['dependencies']['buildrequires'], + dict)) + + @error + @guidelines_link('#Module_dependencies') + @detail("The 'buildrequires' dictionary must associate module name keys" + " with stream name values.") + @prerequisite('buildrequires') + def lint_buildrequires_items(self): + """Modules MAY depend on other modules. These module relationships are + listed in the dependencies block. Dependencies are expressed using + module names and their stream names.""" + self.check(all( + isinstance(k, self.yaml_scalar_types) + and isinstance(v, self.yaml_scalar_types) + for k, v in + self.yaml_data['dependencies']['buildrequires'].items())) + + @error + @guidelines_link('#Module_dependencies') + @detail("The 'requires' dependencies block must be a dictionary" + " (key/value).") + @prerequisite('dependencies') + def lint_requires(self): + """Modules MAY depend on other modules. These module relationships are + listed in the dependencies block. Dependencies are expressed using + module names and their stream names.""" + self.check(isinstance(self.yaml_data['dependencies']['requires'], + dict)) + + @error + @guidelines_link('#Module_dependencies') + @detail("The 'requires' dictionary must associate module name keys with" + " stream name values.") + @prerequisite('requires') + def lint_requires_items(self): + """Modules MAY depend on other modules. These module relationships are + listed in the dependencies block. Dependencies are expressed using + module names and their stream names.""" + self.check(all( + isinstance(k, self.yaml_scalar_types) + and isinstance(v, self.yaml_scalar_types) + for k, v in self.yaml_data['dependencies']['requires'].items())) From 745089524dc8b2b094b136a3d5e55449c9a042a7 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:50:08 +0000 Subject: [PATCH 18/51] add check_is_scalar() method and use it --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 71ba0d2..6928351 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -104,8 +104,7 @@ class ModuleMDLinter(object): Contains one method for each check to perform and scaffolding. The names of the methods performing checks begin with 'lint_', they do nothing if the check succeeds but throw a LintCheckFailed exception otherwise. An easy way - to do this is using the check()/check_true(), check_false() methods which - all accept boolean expressions.""" + to do this is using the available check*() methods.""" yaml_scalar_types = (str, int, float) @@ -214,6 +213,10 @@ class ModuleMDLinter(object): """Check that a value is not True or true-like.""" self.check_true(not expr) + def check_is_scalar(self, value): + """Check that a value is of a scalar type.""" + self.check_true(isinstance(value, self.yaml_scalar_types)) + # lint checks below @error @@ -341,11 +344,8 @@ class ModuleMDLinter(object): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" - self.check(all( - isinstance(k, self.yaml_scalar_types) - and isinstance(v, self.yaml_scalar_types) - for k, v in - self.yaml_data['dependencies']['buildrequires'].items())) + for v in self.yaml_data['dependencies']['buildrequires'].values(): + self.check_is_scalar(v) @error @guidelines_link('#Module_dependencies') @@ -368,7 +368,5 @@ class ModuleMDLinter(object): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" - self.check(all( - isinstance(k, self.yaml_scalar_types) - and isinstance(v, self.yaml_scalar_types) - for k, v in self.yaml_data['dependencies']['requires'].items())) + for v in self.yaml_data['dependencies']['requires'].values(): + self.check_is_scalar(v) From 7e2a76f0f3f473900679ed1971059ba26f124b28 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:50:08 +0000 Subject: [PATCH 19/51] add check_is_dict() and use it --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 6928351..f373450 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -217,6 +217,10 @@ class ModuleMDLinter(object): """Check that a value is of a scalar type.""" self.check_true(isinstance(value, self.yaml_scalar_types)) + def check_is_dict(self, value): + """Check that a value is a dictionary.""" + self.check_true(isinstance(value, dict)) + # lint checks below @error @@ -267,7 +271,7 @@ class ModuleMDLinter(object): @prerequisite('license_block_exists') def lint_license_block_keys(self): """Every module MUST contain a license block.""" - self.check(isinstance(self.yaml_data['license'], dict)) + self.check_is_dict(self.yaml_data['license']) @error @guidelines_link('#Module_licensing') @@ -321,7 +325,7 @@ class ModuleMDLinter(object): listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" if 'dependencies' in self.mmd: - self.check(isinstance(self.yaml_data['dependencies'], dict)) + self.check_is_dict(self.yaml_data['dependencies']) @error @guidelines_link('#Module_dependencies') @@ -332,8 +336,7 @@ class ModuleMDLinter(object): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" - self.check(isinstance(self.yaml_data['dependencies']['buildrequires'], - dict)) + self.check_is_dict(self.yaml_data['dependencies']['buildrequires']) @error @guidelines_link('#Module_dependencies') @@ -356,8 +359,7 @@ class ModuleMDLinter(object): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" - self.check(isinstance(self.yaml_data['dependencies']['requires'], - dict)) + self.check_is_dict(self.yaml_data['dependencies']['requires']) @error @guidelines_link('#Module_dependencies') From c7971cea04f0a11e8d589f0c96ddc19ba73ed0fe Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:50:08 +0000 Subject: [PATCH 20/51] don't use modulemd for linting The modulemd module trips over some errors. Instead, use the lower-level YAML tree/dict throughout. --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index f373450..9fcef32 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -3,8 +3,6 @@ from shutil import get_terminal_size from click.formatting import wrap_text -import modulemd - import yaml @@ -113,8 +111,6 @@ class ModuleMDLinter(object): raise ValueError( "specify exactly one of modulemd_path or modulemd_str") - mmd = self.mmd = modulemd.ModuleMetadata() - self.modulemd_path = modulemd_path if not modulemd_str: @@ -123,10 +119,9 @@ class ModuleMDLinter(object): else: self.modulemd_str = modulemd_str - mmd.loads(self.modulemd_str) - + # We intentionally don't use the modulemd module for linting. self.yaml_doc = yaml.safe_load(self.modulemd_str) - self.yaml_data = self.yaml_doc['data'] + self.mmd = self.yaml_doc['data'] def lint(self, min_level=None): # ensure that min_level describes a valid level @@ -227,7 +222,7 @@ class ModuleMDLinter(object): @guidelines_link('#Module_summary_and_description') def lint_summary_exists(self): """Every module MUST include a short summary.""" - self.check(self.mmd.summary and self.mmd.summary.strip()) + self.check(self.mmd.get('summary', "").strip()) @error @guidelines_link('#Module_summary_and_description') @@ -235,33 +230,34 @@ class ModuleMDLinter(object): def lint_summary_one_sentence(self): """The module summary is a one sentence concise description of the module.""" - self.check("." not in self.mmd.summary[:-1]) + self.check("." not in self.mmd['summary'][:-1]) @warning @guidelines_link('#Module_summary_and_description') @prerequisite('summary_exists') def lint_summary_no_trailing_period(self): """The summary SHOULD NOT end in a period.""" - self.check(not self.mmd.summary.strip().endswith(".")) + self.check(not self.mmd['summary'].strip().endswith(".")) @error @guidelines_link('#Module_summary_and_description') def lint_description_exists(self): """Every module MUST include a description.""" - self.check(self.mmd.description and self.mmd.description.strip()) + self.check(self.mmd['description'] + and self.mmd['description'].strip()) @warning @guidelines_link('#Module_summary_and_description') @prerequisite('description_exists') def lint_description_ends_in_period(self): """The description SHOULD end in a period.""" - self.check(self.mmd.description.strip().endswith(".")) + self.check(self.mmd['description'].strip().endswith(".")) @error @guidelines_link('#Module_licensing') def lint_license_block_exists(self): """Every module MUST contain a license block.""" - self.check('license' in self.yaml_data) + self.check('license' in self.mmd) @error @guidelines_link('#Module_licensing') @@ -271,7 +267,7 @@ class ModuleMDLinter(object): @prerequisite('license_block_exists') def lint_license_block_keys(self): """Every module MUST contain a license block.""" - self.check_is_dict(self.yaml_data['license']) + self.check_is_dict(self.mmd['license']) @error @guidelines_link('#Module_licensing') @@ -280,7 +276,7 @@ class ModuleMDLinter(object): def lint_license_module_block_exists(self): """Every module MUST contain a license block and declare a list of the module's licenses.""" - self.check('module' in self.yaml_data['license']) + self.check('module' in self.mmd['license']) @error @guidelines_link('#Module_licensing') @@ -289,7 +285,7 @@ class ModuleMDLinter(object): def lint_license_module_block_is_list(self): """Every module MUST contain a license block and declare a list of the module's licenses.""" - self.check(isinstance(self.yaml_data['license']['module'], list)) + self.check(isinstance(self.mmd['license']['module'], list)) @error @guidelines_link( @@ -298,15 +294,16 @@ class ModuleMDLinter(object): """Module packagers MUST NOT define values for name, stream, version, context or architecture manually but should rather expect the Module Build Service to do it for them.""" - self.check_false(self.mmd.name or self.mmd.stream or self.mmd.version - or self.mmd.context or self.mmd.arch) + self.check_false( + set(('name', 'stream', 'version', 'context', 'arch')) + & set(self.mmd)) @error @guidelines_link('#Module_Service_Levels_and_End_of_Life') def lint_no_manual_eol(self): """Module packagers MUST NOT define the EOL in the modulemd but should define it in other infrastructure tooling.""" - self.check_false(self.mmd.eol) + self.check_false('eol' in self.mmd) @error @guidelines_link('#Module_licensing') @@ -314,8 +311,8 @@ class ModuleMDLinter(object): @prerequisite('license_block_keys') def lint_license_content(self): """The packager MAY also define a list of content licenses.""" - if 'content' in self.yaml_data['license']: - self.check(isinstance(self.yaml_data['license']['content'], list)) + if 'content' in self.mmd['license']: + self.check(isinstance(self.mmd['license']['content'], list)) @error @guidelines_link('#Module_dependencies') @@ -325,7 +322,7 @@ class ModuleMDLinter(object): listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" if 'dependencies' in self.mmd: - self.check_is_dict(self.yaml_data['dependencies']) + self.check_is_dict(self.mmd['dependencies']) @error @guidelines_link('#Module_dependencies') @@ -336,7 +333,7 @@ class ModuleMDLinter(object): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" - self.check_is_dict(self.yaml_data['dependencies']['buildrequires']) + self.check_is_dict(self.mmd['dependencies']['buildrequires']) @error @guidelines_link('#Module_dependencies') @@ -347,7 +344,7 @@ class ModuleMDLinter(object): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" - for v in self.yaml_data['dependencies']['buildrequires'].values(): + for v in self.mmd['dependencies']['buildrequires'].values(): self.check_is_scalar(v) @error @@ -359,7 +356,7 @@ class ModuleMDLinter(object): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" - self.check_is_dict(self.yaml_data['dependencies']['requires']) + self.check_is_dict(self.mmd['dependencies']['requires']) @error @guidelines_link('#Module_dependencies') @@ -370,5 +367,5 @@ class ModuleMDLinter(object): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" - for v in self.yaml_data['dependencies']['requires'].values(): + for v in self.mmd['dependencies']['requires'].values(): self.check_is_scalar(v) From dd0e927cd64f507d35b457fb9a1fa571234be2cc Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:50:40 +0000 Subject: [PATCH 21/51] check that a potential 'xmd' block is a dict --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 9fcef32..3ec8080 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -369,3 +369,12 @@ class ModuleMDLinter(object): module names and their stream names.""" for v in self.mmd['dependencies']['requires'].values(): self.check_is_scalar(v) + + @error + @guidelines_link('#Extensible_module_metadata_block') + @detail("The 'xmd' block must be a dictionary (key/value).") + def lint_xmd_dict(self): + """Modules MAY also contain an extensible metadata block, a list of + vendor-defined key-value pairs.""" + if 'xmd' in self.mmd: + self.check_is_dict(self.mmd['xmd']) From 2cc280307a10354fe856d45a5bf8e9d031adf3e8 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:50:44 +0000 Subject: [PATCH 22/51] assert that every linter method has a docstring --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 3ec8080..e1ed05e 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -146,6 +146,9 @@ class ModuleMDLinter(object): if callable(y) and y.lint_level >= min_level), key=lambda x: x.lint_order) + # ensure all lint methods have a docstring + assert all(getattr(x, '__doc__', None) for x in lint_methods) + flagged = [] failed_or_skipped = set() From 3422a55668be9e639033a0406b37c95091efe7bc Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:50:44 +0000 Subject: [PATCH 23/51] make assertion errors more informative --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index e1ed05e..f492de3 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -139,7 +139,9 @@ class ModuleMDLinter(object): assert all( 0 <= getattr(getattr(self, x), "lint_level", -1) <= LintLevel.max_level - for x in lint_method_names) + for x in lint_method_names), ( + "All lint_*() methods must be decorated with one of the @info," + " @warning, @error decorators.") lint_methods = sorted( (y for y in (getattr(self, x) for x in lint_method_names) @@ -147,7 +149,8 @@ class ModuleMDLinter(object): key=lambda x: x.lint_order) # ensure all lint methods have a docstring - assert all(getattr(x, '__doc__', None) for x in lint_methods) + assert all(getattr(x, '__doc__', None) for x in lint_methods), ( + """All lint_*() methods must have a docstring.""") flagged = [] failed_or_skipped = set() From 2f77a84812705e43f96581f7459ff9fd7f736c81 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:50:44 +0000 Subject: [PATCH 24/51] let check_is_(dict|scalar) accept multiple values Makes it easier to check whole ranges of values. --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index f492de3..40b82ef 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -214,13 +214,15 @@ class ModuleMDLinter(object): """Check that a value is not True or true-like.""" self.check_true(not expr) - def check_is_scalar(self, value): - """Check that a value is of a scalar type.""" - self.check_true(isinstance(value, self.yaml_scalar_types)) + def check_is_scalar(self, *values): + """Check that 0..n values are of a scalar type.""" + for value in values: + self.check_true(isinstance(value, self.yaml_scalar_types)) - def check_is_dict(self, value): - """Check that a value is a dictionary.""" - self.check_true(isinstance(value, dict)) + def check_is_dict(self, *values): + """Check that 0..n values are of a dictionary type.""" + for value in values: + self.check_true(isinstance(value, dict)) # lint checks below @@ -350,8 +352,8 @@ class ModuleMDLinter(object): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" - for v in self.mmd['dependencies']['buildrequires'].values(): - self.check_is_scalar(v) + self.check_is_scalar( + *self.mmd['dependencies']['buildrequires'].values()) @error @guidelines_link('#Module_dependencies') @@ -373,8 +375,7 @@ class ModuleMDLinter(object): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" - for v in self.mmd['dependencies']['requires'].values(): - self.check_is_scalar(v) + self.check_is_scalar(*self.mmd['dependencies']['requires'].values()) @error @guidelines_link('#Extensible_module_metadata_block') From 50d4058545a92ced950e414d62cd9aff89f9bc87 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 13:56:16 +0000 Subject: [PATCH 25/51] warn about unknown license keys/blocks --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 40b82ef..3a6d762 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -269,18 +269,25 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_licensing') - @detail( - "The license block must be a dictionary (key/value). Recognized keys" - " are 'module' and 'content'.") + @detail("The license block must be a dictionary (key/value).") @prerequisite('license_block_exists') - def lint_license_block_keys(self): + def lint_license_block_dict(self): """Every module MUST contain a license block.""" self.check_is_dict(self.mmd['license']) + @warning + @guidelines_link('#Module_licensing') + @detail("The license block must be a dictionary (key/value). Recognized" + " keys are 'module' and 'content'.") + @prerequisite('license_block_dict') + def lint_license_block_keys(self): + """Every module MUST contain a license block.""" + self.check(set(self.mmd['license']) <= {'module', 'content'}) + @error @guidelines_link('#Module_licensing') @detail("The module license block must exist.") - @prerequisite('license_block_keys') + @prerequisite('license_block_dict') def lint_license_module_block_exists(self): """Every module MUST contain a license block and declare a list of the module's licenses.""" @@ -316,7 +323,7 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_licensing') @detail("The content license block must be a list if it exists.") - @prerequisite('license_block_keys') + @prerequisite('license_block_dict') def lint_license_content(self): """The packager MAY also define a list of content licenses.""" if 'content' in self.mmd['license']: From 39b420809f0cad10c5e920d7d86cdaa4235c5512 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 14:02:20 +0000 Subject: [PATCH 26/51] check that all license blocks are lists in one go --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 3a6d762..2c51377 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -286,21 +286,22 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_licensing') - @detail("The module license block must exist.") + @detail("All license blocks ('module' and 'content') must be lists.") @prerequisite('license_block_dict') - def lint_license_module_block_exists(self): + def lint_license_blocks_are_lists(self): """Every module MUST contain a license block and declare a list of the module's licenses.""" - self.check('module' in self.mmd['license']) + for block in self.mmd['license'].values(): + self.check(isinstance(block, list)) @error @guidelines_link('#Module_licensing') - @detail("The module license block must be a list.") - @prerequisite('license_module_block_exists') - def lint_license_module_block_is_list(self): + @detail("The module license block must exist.") + @prerequisite('license_block_dict') + def lint_license_module_block_exists(self): """Every module MUST contain a license block and declare a list of the module's licenses.""" - self.check(isinstance(self.mmd['license']['module'], list)) + self.check('module' in self.mmd['license']) @error @guidelines_link( @@ -321,15 +322,6 @@ class ModuleMDLinter(object): self.check_false('eol' in self.mmd) @error - @guidelines_link('#Module_licensing') - @detail("The content license block must be a list if it exists.") - @prerequisite('license_block_dict') - def lint_license_content(self): - """The packager MAY also define a list of content licenses.""" - if 'content' in self.mmd['license']: - self.check(isinstance(self.mmd['license']['content'], list)) - - @error @guidelines_link('#Module_dependencies') @detail("The dependencies block must be a dictionary (key/value).") def lint_dependencies_dict(self): From 5483253f02eed8731255661771e594df84891b87 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 14:03:01 +0000 Subject: [PATCH 27/51] check that all license elements are scalars --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 2c51377..d5c0342 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -296,6 +296,15 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_licensing') + @detail("Each license must be a scalar value.") + @prerequisite('license_blocks_are_lists') + def lint_licenses_scalar(self): + """Every module MUST contain a license block and declare a list of + the module's licenses.""" + self.check_is_scalar(*self.mmd['license']['module']) + + @error + @guidelines_link('#Module_licensing') @detail("The module license block must exist.") @prerequisite('license_block_dict') def lint_license_module_block_exists(self): From 1d1bc9ce433298fa57896d7da0faf35bb01af77e Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 14:05:52 +0000 Subject: [PATCH 28/51] check the references --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index d5c0342..3bda078 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -393,3 +393,36 @@ class ModuleMDLinter(object): vendor-defined key-value pairs.""" if 'xmd' in self.mmd: self.check_is_dict(self.mmd['xmd']) + + @error + @guidelines_link('#Module_references') + @detail("The 'references' block must be a dictionary (key/value).") + def lint_references_dict(self): + """Modules MAY define links referencing various upstream resources, + such as community website, project documentation or upstream bug + tracker.""" + if 'references' in self.mmd: + self.check_is_dict(self.mmd['references']) + + @warning + @guidelines_link('#Module_references') + @detail("The 'references' block must be a dictionary (key/value)." + " Recognized keys are 'community', 'documentation' and 'tracker'.") + def lint_references_keys(self): + """Modules MAY define links referencing various upstream resources, + such as community website, project documentation or upstream bug + tracker.""" + if 'references' in self.mmd: + self.check(set(self.mmd['references']) <= {'community', + 'documentation', + 'tracker'}) + + @error + @guidelines_link('#Module_references') + @detail("The references must be scalar values (URLs).") + def lint_references_urls(self): + """Modules MAY define links referencing various upstream resources, + such as community website, project documentation or upstream bug + tracker.""" + if 'references' in self.mmd: + self.check_is_scalar(*self.mmd['references'].values()) From af858db8c99d59cc877dfa104112a8dc93070bad Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 14:07:51 +0000 Subject: [PATCH 29/51] check the profiles --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 3bda078..ad9d775 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -426,3 +426,47 @@ class ModuleMDLinter(object): tracker.""" if 'references' in self.mmd: self.check_is_scalar(*self.mmd['references'].values()) + + @error + @guidelines_link('#Module_profiles') + @detail("The 'profiles' block must be a dictionary (key/value).") + def lint_profiles_dict(self): + """The module author MAY define lists of packages that would be + installed by default, and a minimum, when the module is enabled and the + particular profile is selected.""" + if 'profiles' in self.mmd: + self.check_is_dict(self.mmd['profiles']) + + @info + @guidelines_link('#Module_profiles') + @detail("Modules that are supposed to be user-installable should in most" + " cases have at least one profile.") + @prerequisite('profiles_dict') + def lint_profiles_exist(self): + """The module author MAY define lists of packages that would be + installed by default, and a minimum, when the module is enabled and the + particular profile is selected.""" + self.check('profiles' in self.mmd and len(self.mmd['profiles'])) + + @info + @guidelines_link('#Module_profiles') + @detail("Modules that are supposed to be user-installable should in most" + " cases have a 'default' profile.") + @prerequisite('profiles_exist') + def lint_profiles_has_default(self): + """The module author MAY define lists of packages that would be + installed by default, and a minimum, when the module is enabled and the + particular profile is selected.""" + self.check('profiles' in self.mmd + and 'default' in self.mmd['profiles']) + + @error + @guidelines_link('#Module_profiles') + @detail("The 'profiles' block must contain dictionary (key/value) blocks.") + @prerequisite('profiles_dict') + def lint_profiles_blocks_are_dicts(self): + """The module author MAY define lists of packages that would be + installed by default, and a minimum, when the module is enabled and the + particular profile is selected.""" + if 'profiles' in self.mmd: + self.check_is_dict(*self.mmd['profiles'].values()) From 796f0a77099af2fec47222df04ff55f8cec65fe5 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 15:37:05 +0000 Subject: [PATCH 30/51] add check_is_list() and use it --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index ad9d775..0774ae5 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -219,6 +219,11 @@ class ModuleMDLinter(object): for value in values: self.check_true(isinstance(value, self.yaml_scalar_types)) + def check_is_list(self, *values): + """Check that 0..n values are lists.""" + for value in values: + self.check_true(isinstance(value, list)) + def check_is_dict(self, *values): """Check that 0..n values are of a dictionary type.""" for value in values: @@ -291,8 +296,7 @@ class ModuleMDLinter(object): def lint_license_blocks_are_lists(self): """Every module MUST contain a license block and declare a list of the module's licenses.""" - for block in self.mmd['license'].values(): - self.check(isinstance(block, list)) + self.check_is_list(*self.mmd['license'].values()) @error @guidelines_link('#Module_licensing') From bfb32d9211684fb76e018833a88f3013bd80ea2a Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 16:32:36 +0000 Subject: [PATCH 31/51] check the API --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 0774ae5..eceb9a5 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -474,3 +474,52 @@ class ModuleMDLinter(object): particular profile is selected.""" if 'profiles' in self.mmd: self.check_is_dict(*self.mmd['profiles'].values()) + + @warning + @guidelines_link('#Module_API') + def lint_api_exists(self): + """Every module SHOULD define its public API.""" + self.check('api' in self.mmd) + + @error + @guidelines_link('#Module_API') + @detail("The 'api' block must be a dictionary (key/value).") + @prerequisite('api_exists') + def lint_api_dict(self): + """Every module SHOULD define its public API.""" + self.check_is_dict(self.mmd['api']) + + @warning + @guidelines_link('#Module_API') + @detail("The 'api' block should only have the 'rpms' key.") + @prerequisite('api_dict') + def lint_api_keys(self): + """Every module SHOULD define its public API. Currently the only + supported type of API are binary RPM packages.""" + self.check(set(self.mmd['api']) == {'rpms'}) + + @error + @guidelines_link('#Module_API') + @detail("All 'api' blocks must be lists.") + @prerequisite('api_dict') + def lint_api_blocks_are_lists(self): + """Every module SHOULD define its public API.""" + self.check_is_list(*self.mmd['api'].values()) + + @warning + @guidelines_link('#Module_API') + @detail("The list of API RPM packages is empty.") + @prerequisite('api_blocks_are_lists') + def lint_api_rpms_empty(self): + """Every module SHOULD define its public API.""" + if 'rpms' in self.mmd['api']: + self.check(len(self.mmd['api']['rpms']) > 0) + + @error + @guidelines_link('#Module_API') + @detail("Every 'api' block must be a list of scalar items.") + @prerequisite('api_blocks_are_lists') + def lint_api_blocks_items_are_scalar(self): + """Every module SHOULD define its public API.""" + for block in self.mmd['api'].values(): + self.check_is_scalar(*block) From c1ac8b4b16b365bd5a3b892d5d5def40180fb2ee Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 11 2017 16:32:36 +0000 Subject: [PATCH 32/51] check filters --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index eceb9a5..5842f9b 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -523,3 +523,55 @@ class ModuleMDLinter(object): """Every module SHOULD define its public API.""" for block in self.mmd['api'].values(): self.check_is_scalar(*block) + + @error + @guidelines_link('#Module_filters') + @detail("The 'filter' block must be a dictionary (key/value).") + def lint_filter_dict(self): + """Module filters define lists of components or other content that + should not be part of the resulting, composed module deliverable.""" + if 'filter' in self.mmd: + self.check_is_dict(self.mmd['filter']) + + @warning + @guidelines_link('#Module_filters') + @detail("The 'filter' block should only have the 'rpms' key.") + @prerequisite('filter_dict') + def lint_filter_rpms(self): + """Module filters define lists of components or other content that + should not be part of the resulting, composed module deliverable. + Currently the only supported type of filter are binary RPM packages.""" + if 'filter' in self.mmd: + self.check(set(self.mmd['filter']) == {'rpms'}) + + @error + @guidelines_link('#Module_filters') + @detail("All 'filter' blocks must be lists.") + @prerequisite('filter_dict') + def lint_filter_blocks_are_lists(self): + """Module filters define lists of components or other content that + should not be part of the resulting, composed module deliverable.""" + if 'filter' in self.mmd: + self.check_is_list(*self.mmd['filter'].values()) + + @warning + @guidelines_link('#Module_filters') + @detail("The list of filtered RPM packages is empty.") + @prerequisite('filter_blocks_are_lists') + def lint_filter_rpms_empty(self): + """Module filters define lists of components or other content that + should not be part of the resulting, composed module deliverable. + Currently the only supported type of filter are binary RPM packages.""" + if 'filter' in self.mmd and 'rpms' in self.mmd['filter']: + self.check(len(self.mmd['filter']['rpms']) > 0) + + @error + @guidelines_link('#Module_filter') + @detail("Every 'filter' block must be a list of scalar items.") + @prerequisite('filter_blocks_are_lists') + def lint_filter_blocks_items_are_scalar(self): + """Module filters define lists of components or other content that + should not be part of the resulting, composed module deliverable.""" + if 'filter' in self.mmd: + for block in self.mmd['filter'].values(): + self.check_is_scalar(*block) From f20473bfcee700adea7ec81ba3152c12b1a8aedf Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 12 2017 16:30:33 +0000 Subject: [PATCH 33/51] check components --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 5842f9b..6e28eb0 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -106,6 +106,11 @@ class ModuleMDLinter(object): yaml_scalar_types = (str, int, float) + recognized_rpm_keys = {'rationale', 'buildorder', 'repository', 'ref', + 'cache', 'arches', 'multilib'} + + recognized_module_keys = {'rationale', 'buildorder', 'repository', 'ref'} + def __init__(self, modulemd_path=None, modulemd_str=None): if bool(modulemd_path) == bool(modulemd_str): raise ValueError( @@ -575,3 +580,105 @@ class ModuleMDLinter(object): if 'filter' in self.mmd: for block in self.mmd['filter'].values(): self.check_is_scalar(*block) + + @error + @guidelines_link('#Module_components') + @detail("The 'components' block must be a dictionary (key/value).") + def lint_components_dict(self): + """Modules MAY, and most modules do contain a components block defining + the module's content.""" + if 'components' in self.mmd: + self.check_is_dict(self.mmd['components']) + + @warning + @guidelines_link('#Module_components') + @detail("The 'components' block only recognizes 'rpms' and 'modules' as" + " keys.") + @prerequisite('components_dict') + def lint_components_keys(self): + """Modules MAY, and most modules do contain a components block defining + the module's content. RPM packages and other modules are the only + currently supported content types.""" + if 'components' in self.mmd: + self.check(set(self.mmd['components']) <= {'rpms', 'modules'}) + + @error + @guidelines_link('#Module_components') + @detail("All 'components' blocks must be dictionaries (key/value).") + @prerequisite('components_dict') + def lint_components_blocks_dict(self): + """Modules MAY, and most modules do contain a components block defining + the module's content.""" + if 'components' in self.mmd: + self.check_is_dict(*self.mmd['components'].values()) + + @error + @guidelines_link('#Module_components') + @detail("All 'components/*' blocks must be dictionaries (key/value).") + @prerequisite('components_blocks_dict') + def lint_components_subblocks_dict(self): + """Modules MAY, and most modules do contain a components block defining + the module's content.""" + if 'components' in self.mmd: + self.check_is_dict(*(subblock + for block in self.mmd['components'].values() + for subblock in block.values())) + + @warning + @guidelines_link('#Module_components') + @detail("One or more 'components' block is empty.") + @prerequisite('components_blocks_dict') + def lint_components_blocks_empty(self): + """Modules MAY, and most modules do contain a components block defining + the module's content.""" + if 'components' in self.mmd: + self.check(all(self.mmd['components'].values())) + + @warning + @guidelines_link('#Module_components') + @detail("One or more 'components/*' block is empty.") + @prerequisite('components_blocks_dict') + def lint_components_subblocks_empty(self): + """Modules MAY, and most modules do contain a components block defining + the module's content.""" + if 'components' in self.mmd: + self.check(all(subblock + for block in self.mmd['components'].values() + for subblock in block.values())) + + @error + @guidelines_link('#Module_components') + @prerequisite('components_subblocks_dict') + def lint_components_blocks_mandatory_keys(self): + """Every component MUST declare why it was added to the module, its + 'rationale'.""" + if 'components' in self.mmd: + for block in self.mmd['components'].values(): + for subblock in self.mmd['components']['rpms'].values(): + self.check('rationale' in subblock) + self.check_is_scalar(subblock['rationale']) + + @warning + @guidelines_link('#RPM_content') + @detail("Recognized keys are: {}".format( + ", ".join("'{}'".format(k) for k in recognized_rpm_keys))) + @prerequisite('components_subblocks_dict') + def lint_components_rpms_recognized_keys(self): + """Module RPM content is defined in the 'rpms' block of + 'components'.""" + if 'rpms' in self.mmd.get('components', {}): + self.check(all(set(b) <= self.recognized_rpm_keys + for b in self.mmd['components']['rpms'].values())) + + @warning + @guidelines_link('#Module_content') + @detail("Recognized keys are: {}".format( + ", ".join("'{}'".format(k) for k in recognized_module_keys))) + @prerequisite('components_subblocks_dict') + def lint_components_modules_recognized_keys(self): + """Including modules as content is defined in the 'modules' block of + 'components'.""" + if 'modules' in self.mmd.get('components', {}): + self.check( + all(set(b) <= self.recognized_module_keys + for b in self.mmd['components']['modules'].values())) From ec535913c92ce18ee2a8fee916b27bb009754383 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 12 2017 16:57:41 +0000 Subject: [PATCH 34/51] check types of content components' metadata --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 6e28eb0..5617e20 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -106,8 +106,17 @@ class ModuleMDLinter(object): yaml_scalar_types = (str, int, float) - recognized_rpm_keys = {'rationale', 'buildorder', 'repository', 'ref', - 'cache', 'arches', 'multilib'} + components_keys_types = { + 'rationale': str, + 'buildorder': int, + 'repository': str, + 'ref': str, + 'cache': str, + 'arches': [str], + 'multilib': [str], + } + + recognized_rpm_keys = set(components_keys_types) recognized_module_keys = {'rationale', 'buildorder', 'repository', 'ref'} @@ -658,6 +667,38 @@ class ModuleMDLinter(object): self.check('rationale' in subblock) self.check_is_scalar(subblock['rationale']) + @error + @guidelines_link('#Module_components') + @detail("Illegal type of component key(s): 'buildorder' must be an" + " integer, 'arches' and 'multilib' a list of strings and all" + " others strings.") + @prerequisite('components_subblocks_dict') + def lint_components_blocks_keys_types(self): + """Modules MAY, and most modules do contain a components block defining + the module's content.""" + if 'components' not in self.mmd: + return + + for block in self.mmd['components'].values(): + for comp_md in block.values(): + for k, v in comp_md.items(): + t = self.components_keys_types.get(k) + if not t: + # unknown key + continue + + if isinstance(t, list): + self.check_is_list(v) + t = t[0] + else: + v = [v] + + if issubclass(t, str): + t = self.yaml_scalar_types + + for value in v: + self.check(isinstance(value, t)) + @warning @guidelines_link('#RPM_content') @detail("Recognized keys are: {}".format( From dcafa5f713538349011448e70fd4bdd048972da8 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 12 2017 16:58:14 +0000 Subject: [PATCH 35/51] more comments --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 5617e20..65e65c2 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -104,8 +104,10 @@ class ModuleMDLinter(object): check succeeds but throw a LintCheckFailed exception otherwise. An easy way to do this is using the available check*() methods.""" + # PyYAML decodes numerical strings as int or float yaml_scalar_types = (str, int, float) + # 'str' for any scalar type components_keys_types = { 'rationale': str, 'buildorder': int, From 8974fc3558d7e0c7b9e59cde8a4c9cb909eef2f3 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 13 2017 12:42:49 +0000 Subject: [PATCH 36/51] add license blurb --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 65e65c2..98ddcdd 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -1,3 +1,26 @@ +# -*- coding: utf-8 -*- +# +# modulemd_linter - Linter for ModuleMD files +# +# Copyright © 2017 Red Hat, Inc. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +# Author: +# Nils Philippsen + +import fnmatch import inspect from shutil import get_terminal_size From 36370bf30eb6dcff8abcc6f77a3d37c50ef886a6 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 13 2017 12:45:39 +0000 Subject: [PATCH 37/51] add @option decorator This allows implementing linter methods which check if an optional block exists. Later linter methods can have it as a prerequisite which allows for less boiler plate code, especially in conjunction with the @prerequisite_for decorator which is to follow. --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 98ddcdd..9d9555a 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -78,6 +78,7 @@ class LintLevel(object): return fn +option = LintLevel("option", -1) info = LintLevel("info", 0) warning = LintLevel("warning", 1) error = LintLevel("error", 2) @@ -176,20 +177,23 @@ class ModuleMDLinter(object): # ensure all lint methods are properly decorated assert all( - 0 <= getattr(getattr(self, x), "lint_level", -1) - <= LintLevel.max_level + -1 <= getattr(getattr(self, x), "lint_level", -2) + <= LintLevel.max_level for x in lint_method_names), ( - "All lint_*() methods must be decorated with one of the @info," - " @warning, @error decorators.") + "All lint_*() methods must be decorated with one of the" + " @option, @info, @warning, @error decorators.") lint_methods = sorted( (y for y in (getattr(self, x) for x in lint_method_names) - if callable(y) and y.lint_level >= min_level), + if callable(y) and (y.lint_level == -1 + or y.lint_level >= min_level)), key=lambda x: x.lint_order) # ensure all lint methods have a docstring - assert all(getattr(x, '__doc__', None) for x in lint_methods), ( - """All lint_*() methods must have a docstring.""") + assert all( + getattr(x, '__doc__', None) + for x in lint_methods if x.lint_level >= 0), ( + """All non-option lint_*() methods must have a docstring.""") flagged = [] failed_or_skipped = set() @@ -209,12 +213,14 @@ class ModuleMDLinter(object): try: method() except LintCheckFailed as e: - try: - lint_detail = method.lint_detail - except AttributeError: - lint_detail = None - flagged.append((method.lint_level_name, inspect.getdoc(method), - lint_detail, method.lint_guidelines_url, e)) + if method.lint_level >= 0: + try: + lint_detail = method.lint_detail + except AttributeError: + lint_detail = None + flagged.append((method.lint_level_name, + inspect.getdoc(method), lint_detail, + method.lint_guidelines_url, e)) failed_or_skipped.add(method.__name__) width, height = get_terminal_size() From f22460f095739b9f3b6ded6f550ecb4398b9c07c Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 13 2017 12:55:18 +0000 Subject: [PATCH 38/51] add @prerequisite_for decorator This reverse of the @prerequisite decorator accepts globs and allows to apply a prerequisite to many linter methods at once. This is useful e.g. for linter methods which are only to be run if an optional block exists, if the method names somehow map to the hierarchical level of what they check. It also allows checking that a modulemd YAML file is processed to begin with (or skip all other tests if not). --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 9d9555a..6a06243 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -74,6 +74,8 @@ class LintLevel(object): fn.lint_level = self.level fn.lint_level_name = self.level_name fn.lint_order = LintLevel.lint_order + if not hasattr(fn, 'lint_prerequisites'): + fn.lint_prerequisites = set() LintLevel.lint_order += 1 return fn @@ -95,26 +97,39 @@ class detail(object): return fn -class prerequisite(object): - """Decorate a linter method with what checks are a prerequisite so as to - not overwhelm a user with subsequent errors.""" +class prerequisite_base(object): + """Base class for the @prerequisite, @prerequisite_for decorators.""" - def __init__(self, *prerequisites): - assert len(prerequisites) + attr_name = None - # Allow specification with and without "lint_" prefix - prerequisites = set(x if x.startswith("lint_") else "lint_" + x - for x in prerequisites) + def __init__(self, *names): + assert len(names) - self.prerequisites = prerequisites + # Allow specification with and without "lint_" prefix + self.names = set(x if x.startswith("lint_") else "lint_" + x + for x in names) def __call__(self, fn): try: - fn.lint_prerequisites.update(self.prerequisites) + getattr(fn, self.attr_name).update(self.names) except AttributeError: - fn.lint_prerequisites = self.prerequisites + setattr(fn, self.attr_name, self.names) return fn +class prerequisite(prerequisite_base): + """Decorate a linter method with what checks are a prerequisite so as to + not overwhelm a user with subsequent errors.""" + + attr_name = 'lint_prerequisites' + + +class prerequisite_for(prerequisite_base): + """Decorate a linter method with what checks have it as a prerequisite. + + Accepts globs.""" + + attr_name = 'lint_prerequisites_for' + class LintCheckFailed(Exception): pass @@ -189,6 +204,20 @@ class ModuleMDLinter(object): or y.lint_level >= min_level)), key=lambda x: x.lint_order) + names_to_methods = {m.__name__: m for m in lint_methods} + + # add names of methods decorated with @prerequisite_for to the list of + # prerequisites of matching methods + for method in lint_methods: + name = method.__name__ + for glob in getattr(method, 'lint_prerequisites_for', ()): + matching_names = fnmatch.filter(names_to_methods, glob) + for n in matching_names: + # don't apply to decorated method + if n == name: + continue + names_to_methods[n].lint_prerequisites.add(name) + # ensure all lint methods have a docstring assert all( getattr(x, '__doc__', None) From 74e36a9ec269a184d8601130ca8a925c4115fb90 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 13 2017 13:10:39 +0000 Subject: [PATCH 39/51] allow empty guidelines link This links the guidelines document without an anchor. --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 6a06243..0b64094 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -38,11 +38,11 @@ class guidelines_link(object): guidelines_base_url = "https://fedoraproject.org/wiki/Module:Guidelines" - def __init__(self, link): + def __init__(self, link=None): self.link = link def __call__(self, fn): - if "://" in self.link: + if not self.link or "://" in self.link: fn.lint_guidelines_url = self.link else: anchor = self.link From 86e6a121f102acf345992e97222a59e2796bf3d6 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 13 2017 13:11:35 +0000 Subject: [PATCH 40/51] load and process the file in linter methods Loads and processes the file data in three steps: 1. Read the file and verify its encoding is UTF-8 2. Attempt to parse as YAML. 3. Verify it conforms to the basic ModuleMD YAML structure. All further linting will depend on these, so we can bail out early. --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 0b64094..b2fa084 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -167,16 +167,7 @@ class ModuleMDLinter(object): "specify exactly one of modulemd_path or modulemd_str") self.modulemd_path = modulemd_path - - if not modulemd_str: - with open(modulemd_path, "r") as mmdfile: - self.modulemd_str = mmdfile.read() - else: - self.modulemd_str = modulemd_str - - # We intentionally don't use the modulemd module for linting. - self.yaml_doc = yaml.safe_load(self.modulemd_str) - self.mmd = self.yaml_doc['data'] + self.modulemd_str = modulemd_str def lint(self, min_level=None): # ensure that min_level describes a valid level @@ -306,6 +297,51 @@ class ModuleMDLinter(object): # lint checks below @error + @guidelines_link("http://yaml.org/spec/current.html#id2513364") + @detail("This file must be encoded in UTF-8 or UTF-16.") + def lint_document_is_unicode(self): + """A YAML processor must support the UTF-16 and UTF-8 character + encodings.""" + if not self.modulemd_str: + try: + with open(self.modulemd_path, "r") as mmdfile: + self.modulemd_str = mmdfile.read() + except UnicodeDecodeError: + raise LintCheckFailed() + else: + self.check(isinstance(self.modulemd_str, str)) + + @error + @guidelines_link() + @detail("The document is not a YAML file.") + @prerequisite('document_is_unicode') + @prerequisite_for('*') + def lint_document_is_yaml(self): + """Each module is defined by a single YAML file and comprises of a + number of key-value pairs describing the module's properties and + components it contains.""" + try: + self.yaml_doc = yaml.safe_load(self.modulemd_str) + except: + raise LintCheckFailed() + + @error + @guidelines_link() + @detail("The document is not a ModuleMD YAML file.") + @prerequisite_for('*') + def lint_document_is_modulemd_yaml(self): + """Each module is defined by a single YAML file and comprises of a + number of key-value pairs describing the module's properties and + components it contains.""" + try: + self.check(self.yaml_doc['document'] == 'modulemd') + self.check(str(self.yaml_doc['version']) == '1') + # We intentionally don't use the modulemd module for linting. + self.mmd = self.yaml_doc['data'] + except: + raise LintCheckFailed() + + @error @guidelines_link('#Module_summary_and_description') def lint_summary_exists(self): """Every module MUST include a short summary.""" From 20cb30fece958c2a29d09b5ae2a858e17348a188 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 13 2017 13:17:57 +0000 Subject: [PATCH 41/51] add checks for optional blocks Add linter methods using @option, make them a prerequisite for methods that need the optional block to exist and remove the now obsolete boilerplate. --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index b2fa084..596a579 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -444,6 +444,11 @@ class ModuleMDLinter(object): define it in other infrastructure tooling.""" self.check_false('eol' in self.mmd) + @option + @prerequisite_for('dependencies_*') + def lint_dependencies_exists(self): + self.check('dependencies' in self.mmd) + @error @guidelines_link('#Module_dependencies') @detail("The dependencies block must be a dictionary (key/value).") @@ -451,15 +456,14 @@ class ModuleMDLinter(object): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" - if 'dependencies' in self.mmd: - self.check_is_dict(self.mmd['dependencies']) + self.check_is_dict(self.mmd['dependencies']) @error @guidelines_link('#Module_dependencies') @detail("The 'buildrequires' dependencies block must be a dictionary" " (key/value).") @prerequisite('dependencies_dict') - def lint_buildrequires(self): + def lint_dependencies_buildrequires(self): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" @@ -470,7 +474,7 @@ class ModuleMDLinter(object): @detail("The 'buildrequires' dictionary must associate module name keys" " with stream name values.") @prerequisite('buildrequires') - def lint_buildrequires_items(self): + def lint_dependencies_buildrequires_items(self): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" @@ -482,7 +486,7 @@ class ModuleMDLinter(object): @detail("The 'requires' dependencies block must be a dictionary" " (key/value).") @prerequisite('dependencies') - def lint_requires(self): + def lint_dependencies_requires(self): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" @@ -493,20 +497,29 @@ class ModuleMDLinter(object): @detail("The 'requires' dictionary must associate module name keys with" " stream name values.") @prerequisite('requires') - def lint_requires_items(self): + def lint_dependencies_requires_items(self): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" self.check_is_scalar(*self.mmd['dependencies']['requires'].values()) + @option + @prerequisite_for('xmd_*') + def lint_xmd_exists(self): + self.check('xmd' in self.mmd) + @error @guidelines_link('#Extensible_module_metadata_block') @detail("The 'xmd' block must be a dictionary (key/value).") def lint_xmd_dict(self): """Modules MAY also contain an extensible metadata block, a list of vendor-defined key-value pairs.""" - if 'xmd' in self.mmd: - self.check_is_dict(self.mmd['xmd']) + self.check_is_dict(self.mmd['xmd']) + + @option + @prerequisite_for('references_*') + def lint_references_exists(self): + self.check('references' in self.mmd) @error @guidelines_link('#Module_references') @@ -515,8 +528,7 @@ class ModuleMDLinter(object): """Modules MAY define links referencing various upstream resources, such as community website, project documentation or upstream bug tracker.""" - if 'references' in self.mmd: - self.check_is_dict(self.mmd['references']) + self.check_is_dict(self.mmd['references']) @warning @guidelines_link('#Module_references') @@ -526,10 +538,8 @@ class ModuleMDLinter(object): """Modules MAY define links referencing various upstream resources, such as community website, project documentation or upstream bug tracker.""" - if 'references' in self.mmd: - self.check(set(self.mmd['references']) <= {'community', - 'documentation', - 'tracker'}) + self.check(set(self.mmd['references']) + <= {'community', 'documentation', 'tracker'}) @error @guidelines_link('#Module_references') @@ -538,8 +548,12 @@ class ModuleMDLinter(object): """Modules MAY define links referencing various upstream resources, such as community website, project documentation or upstream bug tracker.""" - if 'references' in self.mmd: - self.check_is_scalar(*self.mmd['references'].values()) + self.check_is_scalar(*self.mmd['references'].values()) + + @option + @prerequisite_for('profiles_*') + def lint_profiles_exists(self): + self.check('profiles' in self.mmd) @error @guidelines_link('#Module_profiles') @@ -548,31 +562,29 @@ class ModuleMDLinter(object): """The module author MAY define lists of packages that would be installed by default, and a minimum, when the module is enabled and the particular profile is selected.""" - if 'profiles' in self.mmd: - self.check_is_dict(self.mmd['profiles']) + self.check_is_dict(self.mmd['profiles']) @info @guidelines_link('#Module_profiles') @detail("Modules that are supposed to be user-installable should in most" " cases have at least one profile.") @prerequisite('profiles_dict') - def lint_profiles_exist(self): + def lint_profiles_not_empty(self): """The module author MAY define lists of packages that would be installed by default, and a minimum, when the module is enabled and the particular profile is selected.""" - self.check('profiles' in self.mmd and len(self.mmd['profiles'])) + self.check(len(self.mmd['profiles'])) @info @guidelines_link('#Module_profiles') @detail("Modules that are supposed to be user-installable should in most" " cases have a 'default' profile.") - @prerequisite('profiles_exist') + @prerequisite('profiles_dict') def lint_profiles_has_default(self): """The module author MAY define lists of packages that would be installed by default, and a minimum, when the module is enabled and the particular profile is selected.""" - self.check('profiles' in self.mmd - and 'default' in self.mmd['profiles']) + self.check('default' in self.mmd['profiles']) @error @guidelines_link('#Module_profiles') @@ -582,8 +594,7 @@ class ModuleMDLinter(object): """The module author MAY define lists of packages that would be installed by default, and a minimum, when the module is enabled and the particular profile is selected.""" - if 'profiles' in self.mmd: - self.check_is_dict(*self.mmd['profiles'].values()) + self.check_is_dict(*self.mmd['profiles'].values()) @warning @guidelines_link('#Module_API') @@ -619,11 +630,10 @@ class ModuleMDLinter(object): @warning @guidelines_link('#Module_API') @detail("The list of API RPM packages is empty.") - @prerequisite('api_blocks_are_lists') + @prerequisite('api_keys', 'api_blocks_are_lists') def lint_api_rpms_empty(self): """Every module SHOULD define its public API.""" - if 'rpms' in self.mmd['api']: - self.check(len(self.mmd['api']['rpms']) > 0) + self.check(len(self.mmd['api']['rpms']) > 0) @error @guidelines_link('#Module_API') @@ -634,14 +644,18 @@ class ModuleMDLinter(object): for block in self.mmd['api'].values(): self.check_is_scalar(*block) + @option + @prerequisite_for('filter_*') + def lint_filter_exists(self): + self.check('filter' in self.mmd) + @error @guidelines_link('#Module_filters') @detail("The 'filter' block must be a dictionary (key/value).") def lint_filter_dict(self): """Module filters define lists of components or other content that should not be part of the resulting, composed module deliverable.""" - if 'filter' in self.mmd: - self.check_is_dict(self.mmd['filter']) + self.check_is_dict(self.mmd['filter']) @warning @guidelines_link('#Module_filters') @@ -651,8 +665,7 @@ class ModuleMDLinter(object): """Module filters define lists of components or other content that should not be part of the resulting, composed module deliverable. Currently the only supported type of filter are binary RPM packages.""" - if 'filter' in self.mmd: - self.check(set(self.mmd['filter']) == {'rpms'}) + self.check(set(self.mmd['filter']) == {'rpms'}) @error @guidelines_link('#Module_filters') @@ -661,8 +674,7 @@ class ModuleMDLinter(object): def lint_filter_blocks_are_lists(self): """Module filters define lists of components or other content that should not be part of the resulting, composed module deliverable.""" - if 'filter' in self.mmd: - self.check_is_list(*self.mmd['filter'].values()) + self.check_is_list(*self.mmd['filter'].values()) @warning @guidelines_link('#Module_filters') @@ -672,8 +684,7 @@ class ModuleMDLinter(object): """Module filters define lists of components or other content that should not be part of the resulting, composed module deliverable. Currently the only supported type of filter are binary RPM packages.""" - if 'filter' in self.mmd and 'rpms' in self.mmd['filter']: - self.check(len(self.mmd['filter']['rpms']) > 0) + self.check(len(self.mmd['filter']['rpms']) > 0) @error @guidelines_link('#Module_filter') @@ -682,9 +693,13 @@ class ModuleMDLinter(object): def lint_filter_blocks_items_are_scalar(self): """Module filters define lists of components or other content that should not be part of the resulting, composed module deliverable.""" - if 'filter' in self.mmd: - for block in self.mmd['filter'].values(): - self.check_is_scalar(*block) + for block in self.mmd['filter'].values(): + self.check_is_scalar(*block) + + @option + @prerequisite_for('components_*') + def lint_components_exists(self): + self.check('components' in self.mmd) @error @guidelines_link('#Module_components') @@ -692,8 +707,7 @@ class ModuleMDLinter(object): def lint_components_dict(self): """Modules MAY, and most modules do contain a components block defining the module's content.""" - if 'components' in self.mmd: - self.check_is_dict(self.mmd['components']) + self.check_is_dict(self.mmd['components']) @warning @guidelines_link('#Module_components') @@ -704,8 +718,7 @@ class ModuleMDLinter(object): """Modules MAY, and most modules do contain a components block defining the module's content. RPM packages and other modules are the only currently supported content types.""" - if 'components' in self.mmd: - self.check(set(self.mmd['components']) <= {'rpms', 'modules'}) + self.check(set(self.mmd['components']) <= {'rpms', 'modules'}) @error @guidelines_link('#Module_components') @@ -714,8 +727,7 @@ class ModuleMDLinter(object): def lint_components_blocks_dict(self): """Modules MAY, and most modules do contain a components block defining the module's content.""" - if 'components' in self.mmd: - self.check_is_dict(*self.mmd['components'].values()) + self.check_is_dict(*self.mmd['components'].values()) @error @guidelines_link('#Module_components') @@ -724,10 +736,9 @@ class ModuleMDLinter(object): def lint_components_subblocks_dict(self): """Modules MAY, and most modules do contain a components block defining the module's content.""" - if 'components' in self.mmd: - self.check_is_dict(*(subblock - for block in self.mmd['components'].values() - for subblock in block.values())) + self.check_is_dict(*(subblock + for block in self.mmd['components'].values() + for subblock in block.values())) @warning @guidelines_link('#Module_components') @@ -736,8 +747,7 @@ class ModuleMDLinter(object): def lint_components_blocks_empty(self): """Modules MAY, and most modules do contain a components block defining the module's content.""" - if 'components' in self.mmd: - self.check(all(self.mmd['components'].values())) + self.check(all(self.mmd['components'].values())) @warning @guidelines_link('#Module_components') @@ -746,10 +756,9 @@ class ModuleMDLinter(object): def lint_components_subblocks_empty(self): """Modules MAY, and most modules do contain a components block defining the module's content.""" - if 'components' in self.mmd: - self.check(all(subblock - for block in self.mmd['components'].values() - for subblock in block.values())) + self.check(all(subblock + for block in self.mmd['components'].values() + for subblock in block.values())) @error @guidelines_link('#Module_components') @@ -757,11 +766,10 @@ class ModuleMDLinter(object): def lint_components_blocks_mandatory_keys(self): """Every component MUST declare why it was added to the module, its 'rationale'.""" - if 'components' in self.mmd: - for block in self.mmd['components'].values(): - for subblock in self.mmd['components']['rpms'].values(): - self.check('rationale' in subblock) - self.check_is_scalar(subblock['rationale']) + for block in self.mmd['components'].values(): + for subblock in self.mmd['components']['rpms'].values(): + self.check('rationale' in subblock) + self.check_is_scalar(subblock['rationale']) @error @guidelines_link('#Module_components') @@ -772,9 +780,6 @@ class ModuleMDLinter(object): def lint_components_blocks_keys_types(self): """Modules MAY, and most modules do contain a components block defining the module's content.""" - if 'components' not in self.mmd: - return - for block in self.mmd['components'].values(): for comp_md in block.values(): for k, v in comp_md.items(): @@ -795,6 +800,11 @@ class ModuleMDLinter(object): for value in v: self.check(isinstance(value, t)) + @option + @prerequisite_for('components_rpms_*') + def lint_components_rpms_exists(self): + self.check('rpms' in self.mmd['components']) + @warning @guidelines_link('#RPM_content') @detail("Recognized keys are: {}".format( @@ -803,9 +813,13 @@ class ModuleMDLinter(object): def lint_components_rpms_recognized_keys(self): """Module RPM content is defined in the 'rpms' block of 'components'.""" - if 'rpms' in self.mmd.get('components', {}): - self.check(all(set(b) <= self.recognized_rpm_keys - for b in self.mmd['components']['rpms'].values())) + self.check(all(set(b) <= self.recognized_rpm_keys + for b in self.mmd['components']['rpms'].values())) + + @option + @prerequisite_for('components_modules_*') + def lint_components_modules_exists(self): + self.check('modules' in self.mmd['components']) @warning @guidelines_link('#Module_content') @@ -815,7 +829,6 @@ class ModuleMDLinter(object): def lint_components_modules_recognized_keys(self): """Including modules as content is defined in the 'modules' block of 'components'.""" - if 'modules' in self.mmd.get('components', {}): - self.check( - all(set(b) <= self.recognized_module_keys - for b in self.mmd['components']['modules'].values())) + self.check( + all(set(b) <= self.recognized_module_keys + for b in self.mmd['components']['modules'].values())) From e619e8adcd4fdb04adb8a6b0379fc27491e0bc57 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 13 2017 13:43:02 +0000 Subject: [PATCH 42/51] return a non-zero exit code on warnings, errors --- diff --git a/src/_fedmod/cli.py b/src/_fedmod/cli.py index 0f93574..8ecc1b5 100644 --- a/src/_fedmod/cli.py +++ b/src/_fedmod/cli.py @@ -142,4 +142,4 @@ def rpms_from_srpm(pkg): def lint(modulemd, min_level): """Validates a given modulemd YAML file""" linter = mmdl.ModuleMDLinter(modulemd_path=modulemd) - linter.lint(min_level=min_level) + return linter.lint(min_level=min_level) diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 596a579..a1892b7 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -266,6 +266,8 @@ class ModuleMDLinter(object): if i < len(flagged) - 1: print() + return any(lambda x: x[0] not in ('option', 'info') for x in flagged) + def check_true(self, expr): """Check that a value is True or true-like.""" if not expr: From 9c5d940d279a86ac4dc868ef66acc40f9fba7e4c Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 13 2017 13:43:34 +0000 Subject: [PATCH 43/51] we don't really support UTF-16 at the moment --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index a1892b7..a9028c0 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -300,7 +300,8 @@ class ModuleMDLinter(object): @error @guidelines_link("http://yaml.org/spec/current.html#id2513364") - @detail("This file must be encoded in UTF-8 or UTF-16.") + @detail("YAML files must be encoded in UTF-16 or UTF-8. However, we only" + " support UTF-8 encodings currently.") def lint_document_is_unicode(self): """A YAML processor must support the UTF-16 and UTF-8 character encodings.""" From 5152a57edce982114482a35d7deae4dbc348287b Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 13 2017 14:18:47 +0000 Subject: [PATCH 44/51] fix PyYAML dependency --- diff --git a/src/setup.py b/src/setup.py index 284c2ae..dc22804 100644 --- a/src/setup.py +++ b/src/setup.py @@ -30,7 +30,7 @@ setup( 'requests-toolbelt', 'lxml', 'attrs', - 'yaml', + 'PyYAML', ], packages=find_packages(), ) From f9bdaf23009d685acde8d243da78c4eabdee75ed Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 13 2017 14:54:29 +0000 Subject: [PATCH 45/51] fix some method/prerequisite names --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index a9028c0..e033b32 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -466,7 +466,7 @@ class ModuleMDLinter(object): @detail("The 'buildrequires' dependencies block must be a dictionary" " (key/value).") @prerequisite('dependencies_dict') - def lint_dependencies_buildrequires(self): + def lint_dependencies_buildrequires_dict(self): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" @@ -476,7 +476,7 @@ class ModuleMDLinter(object): @guidelines_link('#Module_dependencies') @detail("The 'buildrequires' dictionary must associate module name keys" " with stream name values.") - @prerequisite('buildrequires') + @prerequisite('dependencies_buildrequires_dict') def lint_dependencies_buildrequires_items(self): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using @@ -488,8 +488,8 @@ class ModuleMDLinter(object): @guidelines_link('#Module_dependencies') @detail("The 'requires' dependencies block must be a dictionary" " (key/value).") - @prerequisite('dependencies') - def lint_dependencies_requires(self): + @prerequisite('dependencies_dict') + def lint_dependencies_requires_dict(self): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using module names and their stream names.""" @@ -499,7 +499,7 @@ class ModuleMDLinter(object): @guidelines_link('#Module_dependencies') @detail("The 'requires' dictionary must associate module name keys with" " stream name values.") - @prerequisite('requires') + @prerequisite('dependencies_requires_dict') def lint_dependencies_requires_items(self): """Modules MAY depend on other modules. These module relationships are listed in the dependencies block. Dependencies are expressed using From 8b6bdff07bbf144dd7e4195892a20dc7b602e717 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 13 2017 14:54:47 +0000 Subject: [PATCH 46/51] check for optional dependencies/(build)requires --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index e033b32..e5d66db 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -461,6 +461,11 @@ class ModuleMDLinter(object): module names and their stream names.""" self.check_is_dict(self.mmd['dependencies']) + @option + @prerequisite_for('dependencies_buildrequires_*') + def lint_dependencies_buildrequires_exists(self): + self.check('buildrequires' in self.mmd['dependencies']) + @error @guidelines_link('#Module_dependencies') @detail("The 'buildrequires' dependencies block must be a dictionary" @@ -484,6 +489,11 @@ class ModuleMDLinter(object): self.check_is_scalar( *self.mmd['dependencies']['buildrequires'].values()) + @option + @prerequisite_for('dependencies_requires_*') + def lint_dependencies_requires_exists(self): + self.check('requires' in self.mmd['dependencies']) + @error @guidelines_link('#Module_dependencies') @detail("The 'requires' dependencies block must be a dictionary" From 3cef2effdeb9cc710b837cd646f6d6f4591c1693 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 13 2017 16:56:57 +0000 Subject: [PATCH 47/51] check that filter/rpms exists before accessing it --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index e5d66db..de2701b 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -689,6 +689,11 @@ class ModuleMDLinter(object): should not be part of the resulting, composed module deliverable.""" self.check_is_list(*self.mmd['filter'].values()) + @option + @prerequisite_for('filter_rpms_*') + def lint_filter_rpms_exists(self): + self.check('rpms' in self.mmd['filter']) + @warning @guidelines_link('#Module_filters') @detail("The list of filtered RPM packages is empty.") From 4812bf8e7b480881ffae90d91d73e57f3e3d1b57 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 13 2017 16:59:25 +0000 Subject: [PATCH 48/51] fix grammar --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index de2701b..28a0712 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -760,7 +760,7 @@ class ModuleMDLinter(object): @warning @guidelines_link('#Module_components') - @detail("One or more 'components' block is empty.") + @detail("One or more 'components' blocks are empty.") @prerequisite('components_blocks_dict') def lint_components_blocks_empty(self): """Modules MAY, and most modules do contain a components block defining @@ -769,7 +769,7 @@ class ModuleMDLinter(object): @warning @guidelines_link('#Module_components') - @detail("One or more 'components/*' block is empty.") + @detail("One or more 'components/*' blocks are empty.") @prerequisite('components_blocks_dict') def lint_components_subblocks_empty(self): """Modules MAY, and most modules do contain a components block defining From 9759daf932c5697008caaa14ac3ad9bbfefae571 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 13 2017 17:00:53 +0000 Subject: [PATCH 49/51] improve wording in some detailed descriptions --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 28a0712..5f991da 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -468,8 +468,8 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_dependencies') - @detail("The 'buildrequires' dependencies block must be a dictionary" - " (key/value).") + @detail("The 'buildrequires' block beneath 'dependencies' must be a" + " dictionary (key/value).") @prerequisite('dependencies_dict') def lint_dependencies_buildrequires_dict(self): """Modules MAY depend on other modules. These module relationships are @@ -496,7 +496,7 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_dependencies') - @detail("The 'requires' dependencies block must be a dictionary" + @detail("The 'requires' block beneath 'dependencies' must be a dictionary" " (key/value).") @prerequisite('dependencies_dict') def lint_dependencies_requires_dict(self): @@ -625,7 +625,7 @@ class ModuleMDLinter(object): @warning @guidelines_link('#Module_API') - @detail("The 'api' block should only have the 'rpms' key.") + @detail("The 'api' block should have the 'rpms' key (and only that).") @prerequisite('api_dict') def lint_api_keys(self): """Every module SHOULD define its public API. Currently the only @@ -634,7 +634,7 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_API') - @detail("All 'api' blocks must be lists.") + @detail("All blocks beneath 'api' must be lists.") @prerequisite('api_dict') def lint_api_blocks_are_lists(self): """Every module SHOULD define its public API.""" @@ -650,7 +650,7 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_API') - @detail("Every 'api' block must be a list of scalar items.") + @detail("Every block beneath 'api' must be a list of scalar items.") @prerequisite('api_blocks_are_lists') def lint_api_blocks_items_are_scalar(self): """Every module SHOULD define its public API.""" @@ -672,7 +672,7 @@ class ModuleMDLinter(object): @warning @guidelines_link('#Module_filters') - @detail("The 'filter' block should only have the 'rpms' key.") + @detail("The 'filter' block should have the 'rpms' key (and only that).") @prerequisite('filter_dict') def lint_filter_rpms(self): """Module filters define lists of components or other content that @@ -682,7 +682,7 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_filters') - @detail("All 'filter' blocks must be lists.") + @detail("All blocks beneath 'filter' must be lists.") @prerequisite('filter_dict') def lint_filter_blocks_are_lists(self): """Module filters define lists of components or other content that @@ -706,7 +706,7 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_filter') - @detail("Every 'filter' block must be a list of scalar items.") + @detail("Every block beneath 'filter' must be a list of scalar items.") @prerequisite('filter_blocks_are_lists') def lint_filter_blocks_items_are_scalar(self): """Module filters define lists of components or other content that @@ -740,7 +740,8 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_components') - @detail("All 'components' blocks must be dictionaries (key/value).") + @detail("All blocks beneath 'components' must be dictionaries" + " (key/value).") @prerequisite('components_dict') def lint_components_blocks_dict(self): """Modules MAY, and most modules do contain a components block defining @@ -749,7 +750,8 @@ class ModuleMDLinter(object): @error @guidelines_link('#Module_components') - @detail("All 'components/*' blocks must be dictionaries (key/value).") + @detail("All blocks two levels beneath 'components' must be dictionaries" + " (key/value).") @prerequisite('components_blocks_dict') def lint_components_subblocks_dict(self): """Modules MAY, and most modules do contain a components block defining @@ -760,7 +762,7 @@ class ModuleMDLinter(object): @warning @guidelines_link('#Module_components') - @detail("One or more 'components' blocks are empty.") + @detail("One or more blocks beneath 'components' are empty.") @prerequisite('components_blocks_dict') def lint_components_blocks_empty(self): """Modules MAY, and most modules do contain a components block defining @@ -769,7 +771,7 @@ class ModuleMDLinter(object): @warning @guidelines_link('#Module_components') - @detail("One or more 'components/*' blocks are empty.") + @detail("One or more blocks two levels beneath 'components' are empty.") @prerequisite('components_blocks_dict') def lint_components_subblocks_empty(self): """Modules MAY, and most modules do contain a components block defining From 779e94c7f537d830b066e24e7b1f6482f22aead4 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 14 2017 09:45:05 +0000 Subject: [PATCH 50/51] don't trip over 'filter: ~' --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index 5f991da..cd560ac 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -691,6 +691,7 @@ class ModuleMDLinter(object): @option @prerequisite_for('filter_rpms_*') + @prerequisite('filter_dict') def lint_filter_rpms_exists(self): self.check('rpms' in self.mmd['filter']) From 0d25caaeb791ff17b45338201dcd7eb0df0e7270 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Dec 14 2017 09:49:58 +0000 Subject: [PATCH 51/51] allow marking blocks as "explicitly empty" This doesn't run subsequent tests on blocks like 'filter: ~', e.g. it won't complain that it's not a dictionary. --- diff --git a/src/_fedmod/modulemd_linter.py b/src/_fedmod/modulemd_linter.py index cd560ac..8e3d6bc 100644 --- a/src/_fedmod/modulemd_linter.py +++ b/src/_fedmod/modulemd_linter.py @@ -450,7 +450,7 @@ class ModuleMDLinter(object): @option @prerequisite_for('dependencies_*') def lint_dependencies_exists(self): - self.check('dependencies' in self.mmd) + self.check(self.mmd.get('dependencies')) @error @guidelines_link('#Module_dependencies') @@ -464,7 +464,7 @@ class ModuleMDLinter(object): @option @prerequisite_for('dependencies_buildrequires_*') def lint_dependencies_buildrequires_exists(self): - self.check('buildrequires' in self.mmd['dependencies']) + self.check(self.mmd['dependencies'].get('buildrequires')) @error @guidelines_link('#Module_dependencies') @@ -492,7 +492,7 @@ class ModuleMDLinter(object): @option @prerequisite_for('dependencies_requires_*') def lint_dependencies_requires_exists(self): - self.check('requires' in self.mmd['dependencies']) + self.check(self.mmd['dependencies'].get('requires')) @error @guidelines_link('#Module_dependencies') @@ -519,7 +519,7 @@ class ModuleMDLinter(object): @option @prerequisite_for('xmd_*') def lint_xmd_exists(self): - self.check('xmd' in self.mmd) + self.check(self.mmd.get('xmd')) @error @guidelines_link('#Extensible_module_metadata_block') @@ -532,7 +532,7 @@ class ModuleMDLinter(object): @option @prerequisite_for('references_*') def lint_references_exists(self): - self.check('references' in self.mmd) + self.check(self.mmd.get('references')) @error @guidelines_link('#Module_references') @@ -566,7 +566,7 @@ class ModuleMDLinter(object): @option @prerequisite_for('profiles_*') def lint_profiles_exists(self): - self.check('profiles' in self.mmd) + self.check(self.mmd.get('profiles')) @error @guidelines_link('#Module_profiles') @@ -613,7 +613,7 @@ class ModuleMDLinter(object): @guidelines_link('#Module_API') def lint_api_exists(self): """Every module SHOULD define its public API.""" - self.check('api' in self.mmd) + self.check(self.mmd.get('api')) @error @guidelines_link('#Module_API') @@ -660,7 +660,7 @@ class ModuleMDLinter(object): @option @prerequisite_for('filter_*') def lint_filter_exists(self): - self.check('filter' in self.mmd) + self.check(self.mmd.get('filter')) @error @guidelines_link('#Module_filters') @@ -693,7 +693,7 @@ class ModuleMDLinter(object): @prerequisite_for('filter_rpms_*') @prerequisite('filter_dict') def lint_filter_rpms_exists(self): - self.check('rpms' in self.mmd['filter']) + self.check(self.mmd['filter'].get('rpms')) @warning @guidelines_link('#Module_filters') @@ -718,7 +718,7 @@ class ModuleMDLinter(object): @option @prerequisite_for('components_*') def lint_components_exists(self): - self.check('components' in self.mmd) + self.check(self.mmd.get('components')) @error @guidelines_link('#Module_components') @@ -824,7 +824,7 @@ class ModuleMDLinter(object): @option @prerequisite_for('components_rpms_*') def lint_components_rpms_exists(self): - self.check('rpms' in self.mmd['components']) + self.check(self.mmd['components'].get('rpms')) @warning @guidelines_link('#RPM_content') @@ -840,7 +840,7 @@ class ModuleMDLinter(object): @option @prerequisite_for('components_modules_*') def lint_components_modules_exists(self): - self.check('modules' in self.mmd['components']) + self.check(self.mmd['components'].get('modules')) @warning @guidelines_link('#Module_content')