diff options
| -rw-r--r-- | src/debputy/analysis/debian_dir.py | 1 | ||||
| -rw-r--r-- | src/debputy/commands/debputy_cmd/lint_and_lsp_cmds.py | 11 | ||||
| -rw-r--r-- | src/debputy/linting/lint_report_junit.py | 8 | ||||
| -rw-r--r-- | src/debputy/lsp/debputy_ls.py | 6 | ||||
| -rw-r--r-- | src/debputy/lsp/lsp_debian_control.py | 39 | ||||
| -rw-r--r-- | src/debputy/packager_provided_files.py | 63 | ||||
| -rw-r--r-- | tests/test_packager_provided_files.py | 416 |
7 files changed, 504 insertions, 40 deletions
diff --git a/src/debputy/analysis/debian_dir.py b/src/debputy/analysis/debian_dir.py index 7e5b37a..68a5c1b 100644 --- a/src/debputy/analysis/debian_dir.py +++ b/src/debputy/analysis/debian_dir.py @@ -152,6 +152,7 @@ def scan_debian_dir( ) details: PackagingFileInfo = { "path": key, + "binary-package": ppf.package_name, "pkgfile-stem": ppf.definition.stem, "pkgfile-explicit-package-name": ppf.uses_explicit_package_name, "pkgfile-is-active-in-build": ppf.definition.has_active_command, diff --git a/src/debputy/commands/debputy_cmd/lint_and_lsp_cmds.py b/src/debputy/commands/debputy_cmd/lint_and_lsp_cmds.py index d715e5a..e8e380e 100644 --- a/src/debputy/commands/debputy_cmd/lint_and_lsp_cmds.py +++ b/src/debputy/commands/debputy_cmd/lint_and_lsp_cmds.py @@ -1,3 +1,4 @@ +import random import textwrap from argparse import BooleanOptionalAction @@ -219,9 +220,17 @@ def lsp_editor_glue(context: CommandContext) -> None: alias_of = f" (short for: {payload})" content.append((editor_name, alias_of)) max_name = max(len(c[0]) for c in content) - print("This version of debputy has editor snippets for the following editors: ") + print( + "This version of debputy has instructions or editor config snippets for the following editors: " + ) + print() for editor_name, alias_of in content: print(f" * {editor_name:<{max_name}}{alias_of}") + print() + choice = random.Random().choice(list(_EDITOR_SNIPPETS)) + print( + f"Use `debputy editor-config {choice}` (as an example) to see the instructions for a concrete editor." + ) return result = _EDITOR_SNIPPETS[editor_name] while result in _EDITOR_SNIPPETS: diff --git a/src/debputy/linting/lint_report_junit.py b/src/debputy/linting/lint_report_junit.py index 8b1d635..2ad993a 100644 --- a/src/debputy/linting/lint_report_junit.py +++ b/src/debputy/linting/lint_report_junit.py @@ -51,6 +51,8 @@ class JunitLintReport(LintReport): diagnostic_results: Iterable[LintDiagnosticResult], duration: float, ) -> "TestCase": + if not duration: + duration = 0.000001 case = TestCase( filename, # The JUnit schema has `classname` as mandatory @@ -63,9 +65,13 @@ class JunitLintReport(LintReport): continue diagnostic = diagnostic_result.diagnostic severity = debputy_severity(diagnostic) + if diagnostic_result.is_file_level_diagnostic: + range_desc = "entire file" + else: + range_desc = str(diagnostic.range) output = textwrap.dedent( f"""\ - {filename} [{severity}] ({diagnostic.range}): {diagnostic.message} + {filename} [{severity}] ({range_desc}): {diagnostic.message} """ ) case.add_failure_info( diff --git a/src/debputy/lsp/debputy_ls.py b/src/debputy/lsp/debputy_ls.py index e195208..ed076cd 100644 --- a/src/debputy/lsp/debputy_ls.py +++ b/src/debputy/lsp/debputy_ls.py @@ -447,6 +447,10 @@ class DebputyLanguageServer(LanguageServer): cleaned_filename = path[last_idx:] if self.trust_language_ids and lang_id and not lang_id.isspace(): - return "editor-provided", lang_id, cleaned_filename + if lang_id not in ("fundamental",): + return "editor-provided", lang_id, cleaned_filename + _info( + f"Ignoring editor provided language ID: {lang_id} (reverting to filename based detection instead)" + ) return "filename", cleaned_filename, cleaned_filename diff --git a/src/debputy/lsp/lsp_debian_control.py b/src/debputy/lsp/lsp_debian_control.py index 370a14f..e91a43e 100644 --- a/src/debputy/lsp/lsp_debian_control.py +++ b/src/debputy/lsp/lsp_debian_control.py @@ -959,7 +959,7 @@ def _lint_debian_control( def _package_range_of_stanza( lint_state: LintState, binary_stanzas: List[Tuple[Deb822ParagraphElement, TEPosition]], -) -> Iterable[Tuple[str, Range]]: +) -> Iterable[Tuple[str, Optional[str], Range]]: for stanza, stanza_position in binary_stanzas: kvpair = stanza.get_kvpair_element("Package") if kvpair is None: @@ -971,7 +971,7 @@ def _package_range_of_stanza( lint_state.lines, te_range_to_lsp(representation_field_range), ) - yield stanza["Package"], representation_field_range + yield stanza["Package"], stanza.get("Architecture"), representation_field_range def _detect_misspelled_packaging_files( @@ -989,7 +989,8 @@ def _detect_misspelled_packaging_files( debian_dir, ) stanza_ranges = { - p: r for p, r in _package_range_of_stanza(lint_state, binary_stanzas_w_pos) + p: (a, r) + for p, a, r in _package_range_of_stanza(lint_state, binary_stanzas_w_pos) } for pkg_file_data in all_pkg_file_data: @@ -999,13 +1000,15 @@ def _detect_misspelled_packaging_files( stem = pkg_file_data.get("pkgfile-stem") if binary_package is None or stem is None: continue - diag_range = stanza_ranges.get(binary_package) + declared_arch, diag_range = stanza_ranges.get(binary_package) if diag_range is None: continue - path = pkg_file_data["path"] likely_typo_of = pkg_file_data.get("likely-typo-of") + arch_restriction = pkg_file_data.get("pkgfile-architecture-restriction") if likely_typo_of is not None: + # Handles arch_restriction == 'all' at the same time due to how + # the `likely-typo-of` is created diagnostics.append( Diagnostic( diag_range, @@ -1018,6 +1021,31 @@ def _detect_misspelled_packaging_files( ) ) continue + if declared_arch == "all" and arch_restriction is not None: + diagnostics.append( + Diagnostic( + diag_range, + f'The file "{path}" has an architecture restriction but is for an `arch:all` package, so' + f" the restriction does not make sense.", + severity=DiagnosticSeverity.Warning, + source="debputy", + data=DiagnosticData( + report_for_related_file=path, + ), + ) + ) + elif arch_restriction == "all": + diagnostics.append( + Diagnostic( + diag_range, + f'The file "{path}" has an architecture restriction of `all` rather than a real architecture', + severity=DiagnosticSeverity.Warning, + source="debputy", + data=DiagnosticData( + report_for_related_file=path, + ), + ) + ) if not pkg_file_data.get("pkgfile-is-active-in-build", True): diagnostics.append( @@ -1037,7 +1065,6 @@ def _detect_misspelled_packaging_files( if not explicit_package and name_segment is not None: basename = os.path.basename(path) alt_name = f"{binary_package}.{stem}" - arch_restriction = pkg_file_data.get("pkgfile-architecture-restriction") if arch_restriction is not None: alt_name = f"{alt_name}.{arch_restriction}" diagnostics.append( diff --git a/src/debputy/packager_provided_files.py b/src/debputy/packager_provided_files.py index 85fff11..8e0e0af 100644 --- a/src/debputy/packager_provided_files.py +++ b/src/debputy/packager_provided_files.py @@ -86,42 +86,42 @@ def _find_package_name_prefix( yield main_binary_package, path.name, False, False +def _iterate_stem_splits(basename: str) -> Tuple[str, str, int]: + stem = basename + period_count = stem.count(".") + yield stem, None, period_count + install_as_name = "" + while period_count > 0: + period_count -= 1 + install_as_name_part, stem = stem.split(".", 1) + install_as_name = ( + install_as_name + "." + install_as_name_part + if install_as_name != "" + else install_as_name_part + ) + yield stem, install_as_name, period_count + + def _find_definition( packager_provided_files: Mapping[str, PackagerProvidedFileClassSpec], basename: str, *, period2stems: Optional[Mapping[int, Sequence[str]]] = None, ) -> Tuple[Optional[str], Optional[PackagerProvidedFileClassSpec], Optional[str]]: - definition = packager_provided_files.get(basename) - if definition is not None: - return None, definition, None - if period2stems and "." not in basename: - stems = period2stems.get(0) - matches = detect_possible_typo(basename, stems) if stems else None - if matches and len(matches) == 1: - definition = packager_provided_files[matches[0]] - return None, definition, basename - install_as_name = basename - file_class = "" - period_count = -1 - while "." in install_as_name: - period_count += 1 - install_as_name, file_class_part = install_as_name.rsplit(".", 1) - file_class = ( - file_class_part + "." + file_class if file_class != "" else file_class_part - ) - definition = packager_provided_files.get(file_class) + for stem, install_as_name, period_count in _iterate_stem_splits(basename): + definition = packager_provided_files.get(stem) if definition is not None: return install_as_name, definition, None if not period2stems: continue stems = period2stems.get(period_count) + if not stems: continue - matches = detect_possible_typo(file_class, stems) - if matches and len(matches) == 1: + matches = detect_possible_typo(stem, stems) + if matches is not None and len(matches) == 1: definition = packager_provided_files[matches[0]] - return install_as_name, definition, file_class + return install_as_name, definition, stem return None, None, None @@ -260,13 +260,16 @@ def _split_path( if bug_950723 and not definition.bug_950723: continue - _check_mismatches( - path, - definition, - owning_package, - install_as_name, - arch_restriction is not None, - ) + if not allow_fuzzy_matches: + # LSP/Lint checks here but should not use `_check_mismatches` as + # the hard error disrupts them. + _check_mismatches( + path, + definition, + owning_package, + install_as_name, + arch_restriction is not None, + ) expected_path: Optional[str] = None if ( @@ -321,7 +324,7 @@ def _split_path( basename = f"{install_as_name}.{basename}" if explicit_package: basename = f"{package_prefix}.{basename}" - if arch_restriction is not None: + if arch_restriction is not None and arch_restriction != "all": basename = f"{basename}.{arch_restriction}" expected_path = f"{parent_path}{basename}" if fuzzy_match and path.name.endswith(".in"): diff --git a/tests/test_packager_provided_files.py b/tests/test_packager_provided_files.py index b0e075f..caf4bd1 100644 --- a/tests/test_packager_provided_files.py +++ b/tests/test_packager_provided_files.py @@ -1,11 +1,17 @@ +import os import random -from typing import cast, TYPE_CHECKING +from typing import cast, TYPE_CHECKING, Sequence, Optional, Mapping import pytest from debputy.packager_provided_files import detect_all_packager_provided_files from debputy.plugin.api import DebputyPluginInitializer from debputy.plugin.api.feature_set import PluginProvidedFeatureSet +from debputy.plugin.api.impl import plugin_metadata_for_debputys_own_plugin +from debputy.plugin.api.impl_types import ( + PackagerProvidedFileClassSpec, + DebputyPluginMetadata, +) from debputy.plugin.api.test_api import ( InitializedPluginUnderTest, build_virtual_file_system, @@ -13,6 +19,7 @@ from debputy.plugin.api.test_api import ( from debputy.plugin.api.test_api.test_impl import ( initialize_plugin_under_test_preloaded, ) +from lint_tests.lint_tutil import requires_levenshtein from tutil import faked_binary_package, binary_package_table if TYPE_CHECKING: @@ -210,9 +217,416 @@ def test_packager_provided_files_priority( assert expected_basename == matched.path.name +@pytest.mark.parametrize( + "main_package,secondary_package", + [ + ("foo", "foo-doc"), + ("foo.bar", "foo.bar-doc"), + ], +) +def test_detect_ppf_simple(main_package: str, secondary_package: str) -> None: + plugin_metadata = plugin_metadata_for_debputys_own_plugin() + binary_packages = binary_package_table( + faked_binary_package(main_package, is_main_package=True), + faked_binary_package(secondary_package), + ) + debian_dir = build_virtual_file_system( + [ + "install", + f"{secondary_package}.install", + f"{main_package}.docs", + f"{secondary_package}.docs", + "copyright", + ] + ) + ppf_defs = _ppfs( + _fake_PPFClassSpec( + plugin_metadata, + "install", + ), + _fake_PPFClassSpec( + plugin_metadata, + "docs", + ), + _fake_PPFClassSpec( + plugin_metadata, + "copyright", + packageless_is_fallback_for_all_packages=True, + ), + ) + + results = detect_all_packager_provided_files( + ppf_defs, + debian_dir, + binary_packages, + ) + assert main_package in results + main_matches = results[main_package].auto_installable + assert {m.path.name for m in main_matches} == { + "install", + f"{main_package}.docs", + "copyright", + } + + assert secondary_package in results + sec_matches = results[secondary_package].auto_installable + expected = { + f"{secondary_package}.install", + f"{secondary_package}.docs", + "copyright", + } + assert {m.path.name for m in sec_matches} == expected + + +def test_detect_ppf_name_segment() -> None: + plugin_metadata = plugin_metadata_for_debputys_own_plugin() + binary_packages = binary_package_table( + faked_binary_package("foo", is_main_package=True), + faked_binary_package("bar"), + ) + debian_dir = build_virtual_file_system( + [ + "fuu.install", + ] + ) + ppf_defs = _ppfs( + _fake_PPFClassSpec( + plugin_metadata, + "install", + ), + ) + + results = detect_all_packager_provided_files( + ppf_defs, + debian_dir, + binary_packages, + ) + assert "foo" in results + foo_matches = results["foo"].auto_installable + assert {m.path.name for m in foo_matches} == { + "fuu.install", + } + foo_install = foo_matches[0] + assert foo_install.name_segment == "fuu" + assert foo_install.package_name == "foo" + assert foo_install.architecture_restriction is None + assert not foo_install.fuzzy_match + + assert "bar" in results + assert not results["bar"].auto_installable + + ppf_defs = _ppfs( + _fake_PPFClassSpec( + plugin_metadata, + "install", + allow_name_segment=False, + ), + ) + + results = detect_all_packager_provided_files( + ppf_defs, + debian_dir, + binary_packages, + allow_fuzzy_matches=True, + ) + + assert "foo" in results + foo_matches = results["foo"].auto_installable + assert {m.path.name for m in foo_matches} == {"fuu.install"} + foo_install = foo_matches[0] + assert foo_install.name_segment == "fuu" + assert foo_install.package_name == "foo" + assert foo_install.architecture_restriction is None + + assert "bar" in results + assert not results["bar"].auto_installable + + +def test_detect_ppf_fuzzy_match_bug_950723() -> None: + plugin_metadata = plugin_metadata_for_debputys_own_plugin() + binary_packages = binary_package_table( + faked_binary_package("foo", is_main_package=True), + faked_binary_package("bar"), + ) + debian_dir = build_virtual_file_system( + [ + "bar.service", + "foo@.service", + ] + ) + ppf_defs = _ppfs( + _fake_PPFClassSpec( + plugin_metadata, + "service", + bug_950723=True, + ), + ) + + results = detect_all_packager_provided_files( + ppf_defs, + debian_dir, + binary_packages, + ) + assert "foo" in results + foo_matches = results["foo"].auto_installable + assert {m.path.name for m in foo_matches} == {"foo@.service"} + foo_at_service = foo_matches[0] + # Without bug#950723 AND fuzzy_match, it counts a name segment for the main package. + assert foo_at_service.name_segment == "foo@" + assert foo_at_service.package_name == "foo" + assert foo_at_service.architecture_restriction is None + assert not foo_at_service.fuzzy_match + + assert "bar" in results + bar_matches = results["bar"].auto_installable + assert {m.path.name for m in bar_matches} == {"bar.service"} + bar_service = bar_matches[0] + assert bar_service.name_segment is None + assert bar_service.package_name == "bar" + assert bar_service.architecture_restriction is None + assert not bar_service.fuzzy_match + + results = detect_all_packager_provided_files( + ppf_defs, + debian_dir, + binary_packages, + allow_fuzzy_matches=True, + ) + assert "foo" in results + foo_matches = results["foo"].auto_installable + assert {m.path.name for m in foo_matches} == {"foo@.service"} + foo_at_service = foo_matches[0] + assert foo_at_service.name_segment is None + assert foo_at_service.package_name == "foo" + assert foo_at_service.architecture_restriction is None + + assert "bar" in results + bar_matches = results["bar"].auto_installable + assert {m.path.name for m in bar_matches} == {"bar.service"} + bar_service = bar_matches[0] + assert bar_service.name_segment is None + assert bar_service.package_name == "bar" + assert bar_service.architecture_restriction is None + assert not bar_service.fuzzy_match + + +@requires_levenshtein +def test_detect_ppf_typo() -> None: + plugin_metadata = plugin_metadata_for_debputys_own_plugin() + binary_packages = binary_package_table( + faked_binary_package("foo", is_main_package=True), + faked_binary_package("bar"), + ) + debian_dir = build_virtual_file_system( + [ + "fuu.install", + "bar.intsall", + ] + ) + ppf_defs = _ppfs( + _fake_PPFClassSpec( + plugin_metadata, + "install", + ), + _fake_PPFClassSpec( + plugin_metadata, + "logcheck.violations.d", + ), + ) + + results = detect_all_packager_provided_files( + ppf_defs, + debian_dir, + binary_packages, + detect_typos=True, + ) + assert "foo" in results + foo_matches = results["foo"].auto_installable + assert {m.path.name for m in foo_matches} == {"fuu.install"} + foo_logcheck = foo_matches[0] + # Not a typo due to how name segments works with debhelper compat <= 13 + # (but should probably have been one. + assert foo_logcheck.name_segment == "fuu" + assert foo_logcheck.package_name == "foo" + assert foo_logcheck.architecture_restriction is None + + assert "bar" in results + bar_matches = results["bar"].auto_installable + assert {m.path.name for m in bar_matches} == {"bar.intsall"} + bar_logcheck = bar_matches[0] + assert os.path.basename(bar_logcheck.expected_path) == "bar.install" + assert bar_logcheck.name_segment is None + assert bar_logcheck.package_name == "bar" + assert bar_logcheck.architecture_restriction is None + + debian_dir = build_virtual_file_system( + [ + # Typo'ed by intention + "logchcek.violations.d", + "bar.logchcek.violations.d", + ] + ) + + results = detect_all_packager_provided_files( + ppf_defs, + debian_dir, + binary_packages, + detect_typos=True, + ) + + assert "foo" in results + foo_matches = results["foo"].auto_installable + assert {m.path.name for m in foo_matches} == {"logchcek.violations.d"} + foo_logcheck = foo_matches[0] + assert foo_logcheck.name_segment is None + assert foo_logcheck.package_name == "foo" + assert foo_logcheck.architecture_restriction is None + assert os.path.basename(foo_logcheck.expected_path) == "logcheck.violations.d" + + assert "bar" in results + bar_matches = results["bar"].auto_installable + assert {m.path.name for m in bar_matches} == { + "bar.logchcek.violations.d", + } + bar_logcheck = bar_matches[0] + assert bar_logcheck.name_segment is None + assert bar_logcheck.package_name == "bar" + assert bar_logcheck.architecture_restriction is None + assert os.path.basename(bar_logcheck.expected_path) == "bar.logcheck.violations.d" + + +def test_debhelper_overlapping_stems() -> None: + plugin_metadata = plugin_metadata_for_debputys_own_plugin() + binary_packages = binary_package_table( + faked_binary_package("foo", is_main_package=True), + ) + debian_dir = build_virtual_file_system( + [ + "foo.user.service", + ] + ) + ppf_defs = _ppfs( + _fake_PPFClassSpec( + plugin_metadata, + "service", + ), + _fake_PPFClassSpec( + plugin_metadata, + "user.service", + ), + ) + + results = detect_all_packager_provided_files( + ppf_defs, + debian_dir, + binary_packages, + ) + assert "foo" in results + foo_matches = results["foo"].auto_installable + assert {m.path.name for m in foo_matches} == { + "foo.user.service", + } + matched_file = foo_matches[0] + assert matched_file.definition.stem == "user.service" + assert matched_file.name_segment is None + assert matched_file.package_name == "foo" + assert matched_file.architecture_restriction is None + + debian_dir = build_virtual_file_system( + [ + "foo.named.service", + ] + ) + results = detect_all_packager_provided_files( + ppf_defs, + debian_dir, + binary_packages, + ) + assert "foo" in results + foo_matches = results["foo"].auto_installable + assert {m.path.name for m in foo_matches} == {"foo.named.service"} + matched_file = foo_matches[0] + assert matched_file.definition.stem == "service" + assert matched_file.name_segment == "named" + assert matched_file.package_name == "foo" + assert matched_file.architecture_restriction is None + + +@requires_levenshtein +def test_debhelper_overlapping_stems_typo_check() -> None: + plugin_metadata = plugin_metadata_for_debputys_own_plugin() + binary_packages = binary_package_table( + faked_binary_package("foo", is_main_package=True), + ) + debian_dir = build_virtual_file_system( + [ + "foo.uxxr.service", + ] + ) + ppf_defs = _ppfs( + _fake_PPFClassSpec( + plugin_metadata, + "service", + ), + _fake_PPFClassSpec( + plugin_metadata, + "user.service", + ), + ) + results = detect_all_packager_provided_files( + ppf_defs, + debian_dir, + binary_packages, + detect_typos=True, + ) + assert "foo" in results + foo_matches = results["foo"].auto_installable + assert {m.path.name for m in foo_matches} == {"foo.uxxr.service"} + foo_install = foo_matches[0] + # While it could just as well be a named segment, we assume it is a typo. + assert foo_install.definition.stem == "user.service" + assert foo_install.name_segment is None + assert foo_install.package_name == "foo" + assert foo_install.architecture_restriction is None + assert os.path.basename(foo_install.expected_path) == "foo.user.service" + + def _fetch_debputy_plugin_feature_set( plugin: InitializedPluginUnderTest, ) -> PluginProvidedFeatureSet: # Very much not public API, but we need it to avoid testing on production data (also, it is hard to find # relevant debputy files for all the cases we want to test). return cast("InitializedPluginUnderTestImpl", plugin)._feature_set + + +def _fake_PPFClassSpec( + debputy_plugin_metadata: DebputyPluginMetadata, + stem: str, + *, + default_priority: Optional[int] = None, + packageless_is_fallback_for_all_packages: bool = False, + bug_950723: bool = False, + has_active_command: bool = False, + allow_name_segment: bool = True, +) -> PackagerProvidedFileClassSpec: + return PackagerProvidedFileClassSpec( + debputy_plugin_metadata, + stem, + "not-a-real-ppf", + allow_architecture_segment=True, + allow_name_segment=allow_name_segment, + default_priority=default_priority, + default_mode=0o644, + post_formatting_rewrite=None, + packageless_is_fallback_for_all_packages=packageless_is_fallback_for_all_packages, + reservation_only=False, + formatting_callback=None, + bug_950723=bug_950723, + has_active_command=has_active_command, + ) + + +def _ppfs( + *ppfs: PackagerProvidedFileClassSpec, +) -> Mapping[str, PackagerProvidedFileClassSpec]: + return {ppf.stem: ppf for ppf in ppfs} |