From 9e18735096d7ce572cc23b261584916794a6bfc4 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Fri, 15 Dec 2023 14:11:08 +0100 Subject: [PATCH 01/10] remove lxml based schema linter now in galaxy core https://github.com/galaxyproject/galaxy/pull/17081 --- planemo/xml/validation.py | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/planemo/xml/validation.py b/planemo/xml/validation.py index 417d3bc58..3f2764a7f 100644 --- a/planemo/xml/validation.py +++ b/planemo/xml/validation.py @@ -4,7 +4,6 @@ import subprocess from collections import namedtuple -from galaxy.util import unicodify from galaxy.util.commands import which try: @@ -15,7 +14,7 @@ XMLLINT_COMMAND = "xmllint --noout --schema {0} {1} 2>&1" INSTALL_VALIDATOR_MESSAGE = ( "This feature requires an external dependency " - "to function, pleaes install xmllint (e.g 'brew " + "to function, please install xmllint (e.g 'brew " "install libxml2' or 'apt-get install " "libxml2-utils'." ) @@ -42,27 +41,11 @@ def enabled(self): ValidationResult = namedtuple("ValidationResult", ["passed", "output"]) -class LxmlValidator(XsdValidator): - """Validate XSD files using lxml library.""" - - def validate(self, schema_path, target_path): - try: - xsd_doc = etree.parse(schema_path) - xsd = etree.XMLSchema(xsd_doc) - xml = etree.parse(target_path) - passed = xsd.validate(xml) - return ValidationResult(passed, xsd.error_log) - except etree.XMLSyntaxError as e: - return ValidationResult(False, unicodify(e)) - - def enabled(self): - return etree is not None - - class XmllintValidator(XsdValidator): """Validate XSD files with the external tool xmllint.""" def validate(self, schema_path, target_path): + print("XmllintValidator") command = XMLLINT_COMMAND.format(schema_path, target_path) p = subprocess.Popen(command, stdout=subprocess.PIPE, shell=True) stdout, _ = p.communicate() @@ -73,7 +56,7 @@ def enabled(self): return bool(which("xmllint")) -VALIDATORS = [LxmlValidator(), XmllintValidator()] +VALIDATORS = [XmllintValidator()] def get_validator(require=True): From 06a8295b794b777c04708b8cca8b4fadf101c9b8 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Fri, 15 Dec 2023 14:13:22 +0100 Subject: [PATCH 02/10] disable xsd linting default --- planemo/options.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planemo/options.py b/planemo/options.py index eb77fb303..dc0308c3a 100644 --- a/planemo/options.py +++ b/planemo/options.py @@ -1377,7 +1377,7 @@ def shed_fail_fast_option(): def lint_xsd_option(): return planemo_option( - "--xsd/--no_xsd", is_flag=True, default=True, help=("Include tool XSD validation in linting process.") + "--xsd/--no_xsd", is_flag=True, default=False, help=("Include tool XSD validation in linting process.") ) From 800e67b297b149e0252a7e154cbb660baee2afb4 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Fri, 15 Dec 2023 14:32:35 +0100 Subject: [PATCH 03/10] planemo lint: add --skip_file new option allowing to specify one or more files listing linters to skip --- planemo/commands/cmd_lint.py | 2 +- planemo/commands/cmd_shed_lint.py | 2 +- planemo/lint.py | 10 +++++++++- planemo/options.py | 18 ++++++++++++++++++ 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/planemo/commands/cmd_lint.py b/planemo/commands/cmd_lint.py index c84f8f0b9..14185f1f5 100644 --- a/planemo/commands/cmd_lint.py +++ b/planemo/commands/cmd_lint.py @@ -15,7 +15,7 @@ @options.report_level_option() @options.report_xunit() @options.fail_level_option() -@options.skip_option() +@options.skip_options() @options.lint_xsd_option() @options.recursive_option() @click.option( diff --git a/planemo/commands/cmd_shed_lint.py b/planemo/commands/cmd_shed_lint.py index b25ea5988..4ae21ab93 100644 --- a/planemo/commands/cmd_shed_lint.py +++ b/planemo/commands/cmd_shed_lint.py @@ -14,7 +14,7 @@ @options.shed_realization_options() @options.report_level_option() @options.fail_level_option() -@options.skip_option() +@options.skip_options() @options.click.option( "--tools", is_flag=True, default=False, help=("Lint tools discovered in the process of linting repositories.") ) diff --git a/planemo/lint.py b/planemo/lint.py index 25a0a9c7f..a8df3504a 100644 --- a/planemo/lint.py +++ b/planemo/lint.py @@ -20,8 +20,16 @@ def build_lint_args(ctx, **kwds): skip = ctx.global_config.get("lint_skip", "") if isinstance(skip, list): skip = ",".join(skip) - skip_types = [s.strip() for s in skip.split(",")] + + for skip_file in kwds.get("skip_file", []): + with open(skip_file) as f: + for line in f.readlines(): + line = line.strip() + if not line or line.startswith("#"): + continue + skip_types.append(line) + lint_args = dict( level=report_level, fail_level=fail_level, diff --git a/planemo/options.py b/planemo/options.py index dc0308c3a..d38b1483c 100644 --- a/planemo/options.py +++ b/planemo/options.py @@ -1409,6 +1409,13 @@ def report_xunit(): ) +def skip_options(): + return _compose( + skip_option(), + skip_file_option(), + ) + + def skip_option(): return planemo_option( "-s", @@ -1422,6 +1429,17 @@ def skip_option(): ) +def skip_file_option(): + return planemo_option( + "--skip_file", + type=click.Path(exists=True, file_okay=True, dir_okay=False, resolve_path=True), + multiple=True, + help=( + "File containing a list of lint tests to skip" + ), + ) + + def fail_level_option(): return planemo_option("--fail_level", type=click.Choice(["warn", "error"]), default="warn") From 901b2895723a837a804bc5ab025bd50928602f8b Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Fri, 15 Dec 2023 16:34:14 +0100 Subject: [PATCH 04/10] Revert "remove lxml based schema linter" This reverts commit bc4a3f25f4801f4a931e3ff14d0b6b0626feede4. The xsd linting is used also to other xml files, tool_dependencies.xml. So I guess the changed default for `--xsd` should be suffiecient --- planemo/xml/validation.py | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/planemo/xml/validation.py b/planemo/xml/validation.py index 3f2764a7f..417d3bc58 100644 --- a/planemo/xml/validation.py +++ b/planemo/xml/validation.py @@ -4,6 +4,7 @@ import subprocess from collections import namedtuple +from galaxy.util import unicodify from galaxy.util.commands import which try: @@ -14,7 +15,7 @@ XMLLINT_COMMAND = "xmllint --noout --schema {0} {1} 2>&1" INSTALL_VALIDATOR_MESSAGE = ( "This feature requires an external dependency " - "to function, please install xmllint (e.g 'brew " + "to function, pleaes install xmllint (e.g 'brew " "install libxml2' or 'apt-get install " "libxml2-utils'." ) @@ -41,11 +42,27 @@ def enabled(self): ValidationResult = namedtuple("ValidationResult", ["passed", "output"]) +class LxmlValidator(XsdValidator): + """Validate XSD files using lxml library.""" + + def validate(self, schema_path, target_path): + try: + xsd_doc = etree.parse(schema_path) + xsd = etree.XMLSchema(xsd_doc) + xml = etree.parse(target_path) + passed = xsd.validate(xml) + return ValidationResult(passed, xsd.error_log) + except etree.XMLSyntaxError as e: + return ValidationResult(False, unicodify(e)) + + def enabled(self): + return etree is not None + + class XmllintValidator(XsdValidator): """Validate XSD files with the external tool xmllint.""" def validate(self, schema_path, target_path): - print("XmllintValidator") command = XMLLINT_COMMAND.format(schema_path, target_path) p = subprocess.Popen(command, stdout=subprocess.PIPE, shell=True) stdout, _ = p.communicate() @@ -56,7 +73,7 @@ def enabled(self): return bool(which("xmllint")) -VALIDATORS = [XmllintValidator()] +VALIDATORS = [LxmlValidator(), XmllintValidator()] def get_validator(require=True): From 30becfa59ac366721dbbfa51235510ec417ee040 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Fri, 15 Dec 2023 19:46:47 +0100 Subject: [PATCH 05/10] apply black --- planemo/options.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/planemo/options.py b/planemo/options.py index d38b1483c..bfde2dee5 100644 --- a/planemo/options.py +++ b/planemo/options.py @@ -1434,9 +1434,7 @@ def skip_file_option(): "--skip_file", type=click.Path(exists=True, file_okay=True, dir_okay=False, resolve_path=True), multiple=True, - help=( - "File containing a list of lint tests to skip" - ), + help=("File containing a list of lint tests to skip"), ) From 59a799c2a40b36b5090128e04ba89f21b6f3bd25 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 16 Jan 2024 10:56:57 +0100 Subject: [PATCH 06/10] fix multiple definition of `TEST_DATA_DIR` --- tests/test_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 17d8b4288..98362b41e 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -55,7 +55,6 @@ TEST_DIR = os.path.dirname(__file__) TEST_DATA_DIR = os.path.join(TEST_DIR, "data") TEST_AUTOPYGEN_DATA = os.path.join(TEST_DATA_DIR, "autopygen") -TEST_DATA_DIR = os.path.join(TEST_DIR, "data") TEST_REPOS_DIR = os.path.join(TEST_DATA_DIR, "repos") TEST_TOOLS_DIR = os.path.join(TEST_DATA_DIR, "tools") PROJECT_TEMPLATES_DIR = os.path.join(TEST_DIR, os.path.pardir, "project_templates") From 19f6e7b270fefe3fe1c1bf531a124c8fd465b0e5 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 16 Jan 2024 10:59:54 +0100 Subject: [PATCH 07/10] add test for skip_file --- tests/data/lint_skip_list.txt | 4 ++++ tests/test_lint.py | 6 ++++++ 2 files changed, 10 insertions(+) create mode 100644 tests/data/lint_skip_list.txt diff --git a/tests/data/lint_skip_list.txt b/tests/data/lint_skip_list.txt new file mode 100644 index 000000000..acfa09ee5 --- /dev/null +++ b/tests/data/lint_skip_list.txt @@ -0,0 +1,4 @@ +# check that comments work +xml_order +# check the white spaces are ignored + citations diff --git a/tests/test_lint.py b/tests/test_lint.py index dc8ec9aff..e5a63c823 100644 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -5,6 +5,7 @@ CliTestCase, PROJECT_TEMPLATES_DIR, skip_if_environ, + TEST_DATA_DIR, TEST_REPOS_DIR, TEST_TOOLS_DIR, ) @@ -61,6 +62,11 @@ def test_skips(self): lint_cmd = ["lint", "--skip", "xml_order, citations", fail_citation] self._check_exit_code(lint_cmd, exit_code=0) + # Check skip_file (containing the same skips) + skip_file = os.path.join(TEST_DATA_DIR, "lint_skip_list.txt") + lint_cmd = ["lint", "--skip_file", skip_file, fail_citation] + self._check_exit_code(lint_cmd, exit_code=0) + def test_recursive(self): nested_dir = os.path.join(TEST_REPOS_DIR, "multi_repos_nested") From fa286053c705a26257d3c3573ff973afdf18b6bf Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 16 Jan 2024 11:56:54 +0100 Subject: [PATCH 08/10] implement check if skipped linters are available will only work with 24.0 --- planemo/lint.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/planemo/lint.py b/planemo/lint.py index a8df3504a..cc337ed47 100644 --- a/planemo/lint.py +++ b/planemo/lint.py @@ -4,7 +4,10 @@ from urllib.request import urlopen import requests -from galaxy.tool_util.lint import LintContext +from galaxy.tool_util.lint import ( + LintContext, + Linter, +) from planemo.io import error from planemo.shed import find_urls_for_xml @@ -30,6 +33,11 @@ def build_lint_args(ctx, **kwds): continue skip_types.append(line) + linters = Linter.list_listers() + invalid_skip_types = list(set(skip_types) - set(linters)) + if len(invalid_skip_types): + error(f"Unknown linter type(s) {invalid_skip_types} in list of linters to be skipped. Known linters {linters}") + lint_args = dict( level=report_level, fail_level=fail_level, From 7ad8fb27264c79dccbf4f1988812e72ee1a8ac34 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 2 May 2024 15:58:50 +0200 Subject: [PATCH 09/10] fix indentation --- planemo/lint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planemo/lint.py b/planemo/lint.py index cc337ed47..5b85022de 100644 --- a/planemo/lint.py +++ b/planemo/lint.py @@ -36,7 +36,7 @@ def build_lint_args(ctx, **kwds): linters = Linter.list_listers() invalid_skip_types = list(set(skip_types) - set(linters)) if len(invalid_skip_types): - error(f"Unknown linter type(s) {invalid_skip_types} in list of linters to be skipped. Known linters {linters}") + error(f"Unknown linter type(s) {invalid_skip_types} in list of linters to be skipped. Known linters {linters}") lint_args = dict( level=report_level, From cb9d50f59f6e0dc7d462311a2bca1480a2529f34 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 8 May 2024 15:49:33 +0200 Subject: [PATCH 10/10] drop --xsd option --- planemo/commands/cmd_lint.py | 1 - planemo/commands/cmd_shed_lint.py | 1 - planemo/options.py | 6 ------ planemo/tool_lint.py | 2 -- 4 files changed, 10 deletions(-) diff --git a/planemo/commands/cmd_lint.py b/planemo/commands/cmd_lint.py index 14185f1f5..85a3c77cc 100644 --- a/planemo/commands/cmd_lint.py +++ b/planemo/commands/cmd_lint.py @@ -16,7 +16,6 @@ @options.report_xunit() @options.fail_level_option() @options.skip_options() -@options.lint_xsd_option() @options.recursive_option() @click.option( "--urls", diff --git a/planemo/commands/cmd_shed_lint.py b/planemo/commands/cmd_shed_lint.py index 4ae21ab93..80f420b31 100644 --- a/planemo/commands/cmd_shed_lint.py +++ b/planemo/commands/cmd_shed_lint.py @@ -18,7 +18,6 @@ @options.click.option( "--tools", is_flag=True, default=False, help=("Lint tools discovered in the process of linting repositories.") ) -@options.lint_xsd_option() @options.click.option( "--ensure_metadata", is_flag=True, diff --git a/planemo/options.py b/planemo/options.py index bfde2dee5..71f15fc81 100644 --- a/planemo/options.py +++ b/planemo/options.py @@ -1375,12 +1375,6 @@ def shed_fail_fast_option(): ) -def lint_xsd_option(): - return planemo_option( - "--xsd/--no_xsd", is_flag=True, default=False, help=("Include tool XSD validation in linting process.") - ) - - def lint_biocontainers_option(): return planemo_option( "biocontainer", diff --git a/planemo/tool_lint.py b/planemo/tool_lint.py index 5fce052fb..9e72d2646 100644 --- a/planemo/tool_lint.py +++ b/planemo/tool_lint.py @@ -51,8 +51,6 @@ def lint_tools_on_path(ctx, paths, lint_args, **kwds): def _lint_extra_modules(**kwds): linters = [] - if kwds.get("xsd", True): - linters.append(planemo.linters.xsd) if kwds.get("doi", False): linters.append(planemo.linters.doi)