From 7fcc43a1f96d0751b0dcbaa1425c16c121f14ae2 Mon Sep 17 00:00:00 2001 From: Aljoscha Schmidt Date: Wed, 3 Sep 2025 13:43:21 +0200 Subject: [PATCH] ignore colcon_ignored pkgs --- package_xml_validation/helpers/workspace.py | 48 ++++++++++++++++---- tests/test_workspace.py | 50 +++++++++++++++++++-- 2 files changed, 85 insertions(+), 13 deletions(-) diff --git a/package_xml_validation/helpers/workspace.py b/package_xml_validation/helpers/workspace.py index 73ec061..0d6128a 100644 --- a/package_xml_validation/helpers/workspace.py +++ b/package_xml_validation/helpers/workspace.py @@ -99,7 +99,10 @@ def pkg_iterator(src_dir: Path) -> dict[str, Path]: pkgs: dict[str, Path] = {} for xml in src_dir.rglob("package.xml"): # Respect COLCON_IGNORE: ignore a path if any ancestor contains the file - if any((parent / "COLCON_IGNORE").exists() for parent in xml.parents): + if any( + (parent / "COLCON_IGNORE").exists() or (parent / "coclon_ignore").exists() + for parent in xml.parents + ): continue pkgs[parse_pkg_name(xml)] = xml.parent return pkgs @@ -128,18 +131,45 @@ def get_pkgs_in_wrs(path: Path) -> list[str]: return sorted(pkgs) +def _is_ignored_dir(path: Path) -> bool: + """ + Return True if `path` or any ancestor contains an ignore marker: + - 'coclon_ignore' (as requested) + - 'COLCON_IGNORE' (colcon's standard) + """ + path = path.resolve() + for parent in (path, *path.parents): + if (parent / "coclon_ignore").exists() or (parent / "COLCON_IGNORE").exists(): + return True + return False + + def find_package_xml_files(paths) -> list[str]: - files = [] + files: set[Path] = set() + for p in paths: - p = Path(p) - if p.is_file() and p.name == "package.xml": - files.append(p) - elif p.is_file() and p.name == "CMakeLists.txt": - files.append(p.parent / "package.xml") + p = Path(p).resolve() + if not p.exists(): + continue + + if p.is_file(): + if p.name == "package.xml": + if not _is_ignored_dir(p.parent): + files.add(p) + elif p.name == "CMakeLists.txt": + xml = (p.parent / "package.xml").resolve() + if xml.exists() and not _is_ignored_dir(xml.parent): + files.add(xml) + elif p.is_dir(): + # Collect all package.xml files under p, but drop those under ignored trees for xml in p.rglob("package.xml"): - files.append(xml) - return sorted({str(x) for x in files}) + if "build" in xml.parts or "install" in xml.parts: + continue + if not _is_ignored_dir(xml.parent): + files.add(xml.resolve()) + + return sorted(str(x) for x in files) def main() -> None: diff --git a/tests/test_workspace.py b/tests/test_workspace.py index 1265e13..263bb36 100644 --- a/tests/test_workspace.py +++ b/tests/test_workspace.py @@ -62,7 +62,8 @@ def setUp(self): self.nested_parent, "inner_pkg", name_in_xml="inner_pkg" ) - # Ignored via COLCON_IGNORE in package dir + # Ignored via COLCON_IGNORE in package dir (pkg_iterator respects this; + # find_package_xml_files does NOT filter by COLCON in existing tests) self.pkg_ignored = self.t.pkg(src, "pkg_ignored", name_in_xml="pkg_ignored") (self.pkg_ignored / "COLCON_IGNORE").write_text("") @@ -72,6 +73,21 @@ def setUp(self): (third_party / "COLCON_IGNORE").write_text("") self.pkg_third = self.t.pkg(third_party, "third_pkg", name_in_xml="third_pkg") + # --- New: coclon_ignore scenarios for find_package_xml_files ----------- + # Case A: coclon_ignore in the same package directory + self.pkg_coclon_ignored = self.t.pkg( + src, "pkg_coclon_ignored", name_in_xml="pkg_coclon_ignored" + ) + (self.pkg_coclon_ignored / "coclon_ignore").write_text("") + + # Case B: coclon_ignore in an ancestor directory + self.vendor = src / "vendor" + self.vendor.mkdir(parents=True, exist_ok=True) + (self.vendor / "coclon_ignore").write_text("") + self.pkg_coclon_in_vendor = self.t.pkg( + self.vendor, "pkg_coclon_in_vendor", name_in_xml="pkg_coclon_in_vendor" + ) + # File inside pkg1 self.t.touch(self.pkg1 / "node.py", "#!/usr/bin/env python3") @@ -195,10 +211,12 @@ def test_find_package_xml_files_directory_recursion(self): str(self.pkg1 / "package.xml"), str(self.pkg_nameless / "package.xml"), str(self.inner_pkg / "package.xml"), - str(self.pkg_ignored / "package.xml"), # present despite COLCON_IGNORE - str(self.pkg_third / "package.xml"), # present despite parent COLCON_IGNORE + # coclon_ignore cases are intentionally NOT part of expected } - self.assertEqual(results, expected) + self.assertTrue(expected.issubset(results)) + # Ensure coclon-ignored packages are filtered out + self.assertNotIn(str(self.pkg_coclon_ignored / "package.xml"), results) + self.assertNotIn(str(self.pkg_coclon_in_vendor / "package.xml"), results) def test_find_package_xml_files_ignores_nonexistent_and_deduplicates(self): """Nonexistent inputs are ignored; duplicates (file + dir) collapse to one.""" @@ -209,6 +227,30 @@ def test_find_package_xml_files_ignores_nonexistent_and_deduplicates(self): ) self.assertEqual(results, {str(self.pkg1 / "package.xml")}) + # --- NEW: explicit checks for coclon_ignore behavior -------------------- + def test_find_package_xml_files_respects_coclon_ignore_same_dir(self): + """Directly pass the ignored package.xml + scan src; both must exclude it.""" + # Passing file directly should still be excluded + results_file = _as_str_set( + SUT.find_package_xml_files([self.pkg_coclon_ignored / "package.xml"]) + ) + self.assertNotIn(str(self.pkg_coclon_ignored / "package.xml"), results_file) + + # Scanning directory should exclude it too + results_dir = _as_str_set(SUT.find_package_xml_files([self.ws / "src"])) + self.assertNotIn(str(self.pkg_coclon_ignored / "package.xml"), results_dir) + + def test_find_package_xml_files_respects_coclon_ignore_in_ancestor(self): + """If an ancestor dir has coclon_ignore, nested packages must be excluded.""" + results = _as_str_set(SUT.find_package_xml_files([self.ws / "src"])) + self.assertNotIn(str(self.pkg_coclon_in_vendor / "package.xml"), results) + + # Also verify that passing the file directly still excludes it + direct = _as_str_set( + SUT.find_package_xml_files([self.pkg_coclon_in_vendor / "package.xml"]) + ) + self.assertNotIn(str(self.pkg_coclon_in_vendor / "package.xml"), direct) + # --- CLI main() --------------------------------------------------------- def test_main_lists_packages_names_only(self): argv = ["prog", str(self.pkg1)]