diff --git a/changelog.d/335-nested-package-discovery.fixed.md b/changelog.d/335-nested-package-discovery.fixed.md new file mode 100644 index 00000000..f39b139a --- /dev/null +++ b/changelog.d/335-nested-package-discovery.fixed.md @@ -0,0 +1,9 @@ +Make source-package discovery find nested package members, such as +`stats_nz/census_2023_family_type/rc_01`, not just `/` +directories. The alias-drift guard and the default-bundle coverage check rely on +discovery, so a nested member that lost its alias is now reported instead of +being silently left out. Symlinked directories are followed at every depth +(the old two-level glob followed them at its two levels), and a link back into +its own ancestry ends the walk. Also correct the `verify-raw` diagnostics docstring: an +error line that merely mentions a missing-object marker still counts as +missing. diff --git a/chronicle/artifacts.py b/chronicle/artifacts.py index 9b5b5444..b195b1d8 100644 --- a/chronicle/artifacts.py +++ b/chronicle/artifacts.py @@ -1019,8 +1019,9 @@ def _is_missing_object_diagnostic(stderr: str) -> bool: Colour codes are removed first, because Wrangler wraps words in them. A marker counts only as a whole phrase, and only on a line that holds nothing but markers (such as ``NoSuchKey: specified key does not exist``) or is an - error line, so unrelated text that merely mentions a marker stays a fetch - error. + error line. A marker on a line that reports no error stays a fetch error, + but an error line that merely mentions a marker still counts as missing: + both are failures, so neither can produce ``verified``. """ for raw_line in _ANSI_ESCAPE.sub("", stderr).lower().splitlines(): line = " ".join(raw_line.split()) diff --git a/chronicle/source_package.py b/chronicle/source_package.py index 61d6c61f..07d32e81 100644 --- a/chronicle/source_package.py +++ b/chronicle/source_package.py @@ -2365,19 +2365,34 @@ def source_package_root() -> Path: def discover_source_package_dirs(root: Path | None = None) -> list[Path]: - """Discover ``/`` directories under a packages root. + """Discover every directory holding a source package under a packages root. Returns repo-relative package paths (e.g. ``usda_snap/fy69_to_current``), - matching the shape of the alias-map values, sorted for determinism. This - is the authoring surface a complete bundle must cover. + matching the shape of the alias-map values, sorted for determinism. Nested + members (e.g. ``stats_nz/census_2023_family_type/rc_01``) are found too, so + the alias-drift guard sees every package a bundle could build. This is the + authoring surface a complete bundle must cover. + + Symlinked directories are followed at every depth and reported under their + link path, as the earlier two-level glob did at its two levels; + ``Path.rglob`` does not follow them by default. Only a link back into its + own ancestry is not entered, so a cycle ends the walk instead of recursing + forever. """ packages_root = root if root is not None else source_package_root() if not packages_root.exists(): return [] - discovered = { - manifest.parent.relative_to(packages_root) - for manifest in packages_root.glob(f"*/*/{SOURCE_PACKAGE_FILENAME}") - } + discovered: set[Path] = set() + ancestry: dict[str, frozenset[str]] = {} + for dirpath, dirnames, filenames in os.walk(packages_root, followlinks=True): + real = os.path.realpath(dirpath) + above = ancestry.get(os.path.dirname(dirpath), frozenset()) + if real in above: + dirnames[:] = [] + continue + ancestry[dirpath] = above | {real} + if SOURCE_PACKAGE_FILENAME in filenames: + discovered.add(Path(dirpath).relative_to(packages_root)) return sorted(discovered, key=str) diff --git a/tests/test_source_package_alias_drift.py b/tests/test_source_package_alias_drift.py index bd26c21c..613be10b 100644 --- a/tests/test_source_package_alias_drift.py +++ b/tests/test_source_package_alias_drift.py @@ -113,6 +113,71 @@ def test_discover_source_package_dirs_scans_given_root(tmp_path): assert discovered == {"alpha/one", "beta/two"} +def test_discover_source_package_dirs_finds_nested_members(tmp_path): + """A package family's nested members are discovered alongside its root.""" + _write_package(tmp_path, "alpha/family") + _write_package(tmp_path, "alpha/family/rc_01") + _write_package(tmp_path, "alpha/family/rc_02") + + discovered = {str(path) for path in discover_source_package_dirs(root=tmp_path)} + assert discovered == {"alpha/family", "alpha/family/rc_01", "alpha/family/rc_02"} + + +def test_find_alias_map_drift_flags_an_unmapped_nested_member(tmp_path): + """Dropping a nested member's alias is reported, not silently skipped.""" + _write_package(tmp_path, "alpha/family") + _write_package(tmp_path, "alpha/family/rc_01") + aliases = {"alpha-family": type(tmp_path)("alpha/family")} + + missing_dirs, unmapped_dirs = find_alias_map_drift(root=tmp_path, aliases=aliases) + + assert missing_dirs == [] + assert [str(path) for path in unmapped_dirs] == ["alpha/family/rc_01"] + + +def test_discover_source_package_dirs_follows_symlinked_packages(tmp_path): + """A symlinked package or family is found under its link path, nested members included.""" + root = tmp_path / "packages" + outside = tmp_path / "outside" + _write_package(outside, "linked_package") + _write_package(outside, "linked_family/rc_01") + (root / "alpha").mkdir(parents=True) + (root / "alpha" / "linked").symlink_to( + outside / "linked_package", target_is_directory=True + ) + (root / "beta").symlink_to(outside / "linked_family", target_is_directory=True) + + discovered = {str(path) for path in discover_source_package_dirs(root=root)} + assert discovered == {"alpha/linked", "beta/rc_01"} + + missing_dirs, unmapped_dirs = find_alias_map_drift(root=root, aliases={}) + assert missing_dirs == [] + assert [str(path) for path in unmapped_dirs] == ["alpha/linked", "beta/rc_01"] + + +def test_discover_source_package_dirs_reports_each_link_to_one_package(tmp_path): + """A directory and a link to it are separate discoveries, as under the old glob.""" + _write_package(tmp_path, "alpha/real") + (tmp_path / "beta").mkdir() + (tmp_path / "beta" / "alias").symlink_to( + tmp_path / "alpha" / "real", target_is_directory=True + ) + + discovered = {str(path) for path in discover_source_package_dirs(root=tmp_path)} + assert discovered == {"alpha/real", "beta/alias"} + + +def test_discover_source_package_dirs_stops_at_a_symlink_cycle(tmp_path): + """A link back into its own ancestry ends the walk instead of recursing forever.""" + _write_package(tmp_path, "alpha/family") + (tmp_path / "alpha" / "family" / "loop").symlink_to( + tmp_path / "alpha", target_is_directory=True + ) + + discovered = {str(path) for path in discover_source_package_dirs(root=tmp_path)} + assert discovered == {"alpha/family"} + + def test_find_alias_map_drift_flags_unmapped_directory(tmp_path): """An on-disk package with no alias entry is reported as unmapped.""" _write_package(tmp_path, "alpha/mapped")