Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@

import structlog

from repowise.core.fs_walk import WalkSnapshot

from ..languages.specs.cpp import INCLUDE_FRAGMENT_EXTENSIONS
from .base import ExternalSystemRecord

Expand Down Expand Up @@ -595,6 +597,7 @@ def discover_cmake_reactor(
repo_root: Path,
*,
max_files: int = 2000,
snapshot: WalkSnapshot | None = None,
) -> list[CMakeFile]:
"""Walk the repo from ``repo_root`` following ``add_subdirectory`` links.

Expand Down Expand Up @@ -637,9 +640,9 @@ def visit(rel_dir: str) -> None:
skip_dirs = {".git", "build", "_build", "out", "_deps", "cmake-build-debug",
"cmake-build-release", "node_modules", ".venv", "venv"}
if len(out) < max_files:
from repowise.core.fs_walk import iter_glob
from repowise.core.fs_walk import glob_via

for cml in iter_glob(repo_root, "CMakeLists.txt"):
for cml in glob_via(snapshot, repo_root, "CMakeLists.txt"):
try:
rel = cml.resolve().relative_to(repo_root).as_posix()
except ValueError:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,11 @@ def build_cpp_workspace_index(ctx: ResolverContext) -> CppWorkspaceIndex:
# is what that path did before.
source_map = getattr(ctx, "source_map", None)

cmake_files = discover_cmake_reactor(repo_path)
# Same reason as ``source_map`` above: the stand-in context has no
# snapshot, and None means the live walk this always did.
cmake_files = discover_cmake_reactor(
repo_path, snapshot=getattr(ctx, "walk_snapshot", None)
)
file_api_targets = parse_cmake_file_api_reply(repo_path)
bazel_files = discover_bazel_packages(repo_path) if is_bazel_repo(repo_path) else []

Expand Down
16 changes: 13 additions & 3 deletions packages/core/src/repowise/core/ingestion/resolvers/ruby.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,26 @@ def _scan_gem_metadata(ctx: ResolverContext) -> tuple[frozenset[str], tuple[str,
names: set[str] = set()
lib_roots: set[str] = set()
if ctx.repo_path is not None:
from repowise.core.fs_walk import iter_glob
from repowise.core.fs_walk import glob_via

repo = ctx.repo_path.resolve()
candidates: list = []
try:
candidates.extend(
iter_glob(ctx.repo_path, "Gemfile", prune_nested_git=ctx.prune_nested_git)
glob_via(
ctx.walk_snapshot,
ctx.repo_path,
"Gemfile",
prune_nested_git=ctx.prune_nested_git,
)
)
candidates.extend(
iter_glob(ctx.repo_path, "*.gemspec", prune_nested_git=ctx.prune_nested_git)
glob_via(
ctx.walk_snapshot,
ctx.repo_path,
"*.gemspec",
prune_nested_git=ctx.prune_nested_git,
)
)
except OSError:
pass
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from pathlib import Path
from typing import TYPE_CHECKING

from repowise.core.fs_walk import iter_glob
from repowise.core.fs_walk import WalkSnapshot, glob_via

if TYPE_CHECKING:
from .context import ResolverContext
Expand Down Expand Up @@ -63,8 +63,11 @@
return result


def build_swift_targets(

Check warning on line 66 in packages/core/src/repowise/core/ingestion/resolvers/swift_spm.py

View check run for this annotation

Repowise Bot / Repowise / code health

Signature changed: build_swift_targets

1 caller outside this PR call `build_swift_targets`: `tests/unit/ingestion/test_swift_resolver.py::TestBuildSwiftTargets::test_merges_multiple_packages`. They are not part of this change, so nothing in this diff proves they still compile or still pass the right arguments.
repo_path: Path | None, *, prune_nested_git: bool = True
repo_path: Path | None,
*,
prune_nested_git: bool = True,
snapshot: WalkSnapshot | None = None,
) -> dict[str, str]:
"""Walk the repo for every ``Package.swift``, merge their target maps.

Expand All @@ -75,7 +78,9 @@
if repo_path is None or not repo_path.is_dir():
return {}
merged: dict[str, str] = {}
for pkg_swift in iter_glob(repo_path, "Package.swift", prune_nested_git=prune_nested_git):
for pkg_swift in glob_via(
snapshot, repo_path, "Package.swift", prune_nested_git=prune_nested_git
):
try:
pkg_dir = pkg_swift.parent.relative_to(repo_path).as_posix()
except ValueError:
Expand All @@ -90,7 +95,9 @@
cached = getattr(ctx, "_swift_targets", None)
if cached is not None:
return cached
mapping = build_swift_targets(ctx.repo_path, prune_nested_git=ctx.prune_nested_git)
mapping = build_swift_targets(
ctx.repo_path, prune_nested_git=ctx.prune_nested_git, snapshot=ctx.walk_snapshot
)
ctx._swift_targets = mapping # type: ignore[attr-defined]
return mapping

Expand Down
125 changes: 92 additions & 33 deletions packages/core/src/repowise/core/ingestion/traverser.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import configparser
import os
import threading
from collections.abc import Iterable, Iterator
from collections.abc import Callable, Iterable, Iterator
from dataclasses import dataclass, field
from datetime import datetime
from pathlib import Path
Expand Down Expand Up @@ -436,6 +436,8 @@ def __init__(
self._console_scripts_prune_nested = not (include_submodules or include_nested_repos)
self.stats = TraversalStats()
self._count_lock = threading.Lock()
self._console_scripts_lock = threading.Lock()
self._dir_ignore_lock = threading.Lock()
log.info(
"FileTraverser initialised",
repo_root=str(self.repo_root),
Expand All @@ -456,11 +458,34 @@ def _console_script_tables(self) -> ConsoleScriptTables:
"repowise.cli.augment_hook:main"``) has no in-repo importer, so
without this it reads as unreachable unless its filename happens to
match an entry-stem heuristic.

Double-checked, because the first caller is normally
:meth:`_build_file_info` under the ingestion thread pool —
:func:`~repowise.core.pipeline.incremental.build_repo_graph` maps it
over every path with ~2x cpu_count workers. Unsynchronised, every
worker reaching the ``is None`` check before the first one assigns
starts its own :func:`_collect_console_scripts`, and each of those is a
full :func:`~repowise.core.fs_walk.iter_glob` walk of the repo. On a
17k-file repo with 28 workers that was ~264s of file-info phase against
~8s once the value is computed a single time, for identical FileInfos.

``functools.cached_property`` is not a substitute: 3.12 dropped its
internal lock (it serialised across instances), so it permits the
duplicate computation this exists to prevent.

The fast path stays lock-free — once assigned, readers never acquire —
and the assignment publishes a finished tuple, so a reader racing it
sees either ``None`` or the whole object.
"""
if self._console_scripts is None:
self._console_scripts = _collect_console_scripts(
self.repo_root, prune_nested_git=self._console_scripts_prune_nested
)
with self._console_scripts_lock:
# Re-check under the lock: a racer may have filled it while
# this thread waited, and recomputing is the bug itself.
if self._console_scripts is None:
self._console_scripts = _collect_console_scripts(
self.repo_root,
prune_nested_git=self._console_scripts_prune_nested,
)
return self._console_scripts

@property
Expand Down Expand Up @@ -562,18 +587,31 @@ def _get_dir_ignore(self, dirpath: Path) -> pathspec.PathSpec:
against the immediate child name (see ``_should_skip_dir`` /
``_build_file_info``), consistent with the existing per-directory
``.repowiseIgnore`` handling.

Read outside the lock and written under it, for the same reason as
:meth:`_console_script_tables`: the callers are ``_build_file_info``
workers, one per path. A miss here is two ``exists()`` and a small
compile rather than a repo walk, so concurrent misses on one directory
waste little, but they are pure waste — and the directories holding the
most files are the ones every worker reaches at once.
"""
key = str(dirpath)
if key not in self._dir_ignore_cache:
spec = self._dir_ignore_cache.get(key)
if spec is None:
lines: list[str] = []
for name in (".gitignore", self._extra_ignore_filename):
ignore_file = dirpath / name
if ignore_file.exists():
lines.extend(
ignore_file.read_text(encoding="utf-8", errors="ignore").splitlines()
)
self._dir_ignore_cache[key] = _compile_gitignore(lines)
return self._dir_ignore_cache[key]
spec = _compile_gitignore(lines)
with self._dir_ignore_lock:
# setdefault, not assignment: a racer's spec is equivalent, and
# keeping the first published one means callers holding a
# reference always see the cached object.
spec = self._dir_ignore_cache.setdefault(key, spec)
return spec

def _should_skip_dir(
self,
Expand Down Expand Up @@ -810,9 +848,11 @@ def _detect_monorepo(self) -> tuple[list[PackageInfo], bool]:
if self.dir_chain_skipped(rel_pkg_path):
continue
seen_paths.add(rel_pkg)
lang = _primary_language_in(pkg_dir, prune_nested_git=prune_nested)
entry_pts = _find_entry_points_in(
pkg_dir, self.repo_root, prune_nested_git=prune_nested
lang, entry_pts = _scan_package_dir(
pkg_dir,
self.repo_root,
prune_nested_git=prune_nested,
is_pruned=self.dir_chain_skipped,
)
packages.append(
PackageInfo(
Expand Down Expand Up @@ -1151,41 +1191,60 @@ def _is_console_script_target(rel_path: str, modules: frozenset[str]) -> bool:
return False


def _primary_language_in(directory: Path, *, prune_nested_git: bool = True) -> LanguageTag:
def _scan_package_dir(
directory: Path,
repo_root: Path,
*,
prune_nested_git: bool = True,
is_pruned: Callable[[Path], bool],
) -> tuple[LanguageTag, list[str]]:
"""Primary language and entry-point paths for one package, in one walk.

Both answers come off the same pass because they are derived from the same
listing: language from the file extensions, entry points from the
filenames. Read separately they cost two walks of a tree that can be the
largest thing in the repo.

``is_pruned`` is the ignore-file layer :func:`~.package_roots.
scan_package_roots` already applies, required rather than optional because
a scan that skips it answers from files nothing indexes. It matters more
here than it does there.
:func:`~repowise.core.fs_walk.walk_repo` skips vendored trees and nested
checkouts but not gitignored ones, so without it this descends into build
output — and unlike a manifest scan, which only matches filenames, language
detection *opens* every file whose extension it does not recognise
(:func:`_detect_by_shebang`). A gitignored build tree is exactly where those
files are, and none of them can be indexed, so the reads buy nothing.

Skipping them is also the more correct answer: a package's primary language
and its entry points should describe the sources traversal indexes, not
artifacts a build wrote.
"""
from repowise.core.fs_walk import walk_repo

counts: dict[str, int] = {}
entry_points: list[str] = []
try:
for dirpath, _dirnames, filenames in walk_repo(
for dirpath, dirnames, filenames in walk_repo(
directory, prune_nested_git=prune_nested_git
):
# Prune in place so the walk never descends, matching
# scan_package_roots. Candidates are repo-relative because
# dir_chain_skipped tests each level against the repo root.
rel_dir = dirpath.relative_to(repo_root)
dirnames[:] = [d for d in dirnames if not is_pruned(rel_dir / d)]
for fname in filenames:
if fname in _ENTRY_POINT_NAMES:
entry_points.append((dirpath / fname).relative_to(repo_root).as_posix())
lang = _detect_language(dirpath / fname)
if lang not in ("unknown", "yaml", "json", "markdown", "toml"):
counts[lang] = counts.get(lang, 0) + 1
except OSError:
pass
if not counts:
return "unknown"
return max(counts, key=lambda k: counts[k]) # type: ignore[return-value]


def _find_entry_points_in(
directory: Path, repo_root: Path, *, prune_nested_git: bool = True
) -> list[str]:
from repowise.core.fs_walk import walk_repo

result: list[str] = []
try:
for dirpath, _dirnames, filenames in walk_repo(
directory, prune_nested_git=prune_nested_git
):
for fname in filenames:
if fname in _ENTRY_POINT_NAMES:
result.append((dirpath / fname).relative_to(repo_root).as_posix())
except OSError:
pass
return sorted(result)
language: LanguageTag = "unknown"
if counts:
language = max(counts, key=lambda k: counts[k]) # type: ignore[assignment]
return language, sorted(entry_points)


def _is_nested_git_repo(path: Path) -> bool:
Expand Down
44 changes: 44 additions & 0 deletions tests/unit/ingestion/external_systems/test_cmake.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,47 @@ def test_malformed_does_not_raise(tmp_path):
# Trailing-open paren is tolerated; either zero targets or the args are
# captured but we don't crash.
assert isinstance(cm.targets, list)


def test_orphan_glob_reads_the_snapshot_when_given_one(tmp_path):
"""A snapshot answers the orphan sweep, and answers it the same way.

``discover_cmake_reactor`` falls back to a repo-wide glob for
``CMakeLists.txt`` files no ``add_subdirectory`` chain reached. That glob
is the one every resolver used to pay for separately; it now reads a
shared :class:`WalkSnapshot` when the caller has one. Same files either
way — a caller must not be able to tell which path served it.
"""
from repowise.core.fs_walk import WalkSnapshot

_write(tmp_path, "CMakeLists.txt", "project(top)")
_write(tmp_path, "orphan/CMakeLists.txt", "add_library(orphan STATIC o.cc)")

live = cmake.discover_cmake_reactor(tmp_path)
shared = cmake.discover_cmake_reactor(tmp_path, snapshot=WalkSnapshot(tmp_path))

assert {f.path for f in shared} == {f.path for f in live}
assert "orphan/CMakeLists.txt" in {f.path for f in shared}


def test_orphan_glob_honours_the_snapshot_nested_repo_setting(tmp_path):
"""The snapshot decides whether a nested checkout is in scope.

Pruning nested git repos is the default, so an orphan under one stays
hidden. A snapshot built with ``prune_nested_git=False`` — what a repo
indexed with ``--include-nested-repos`` gets — surfaces it, which is the
point of sharing the traverser's own boundary rather than restating it.
"""
from repowise.core.fs_walk import WalkSnapshot

_write(tmp_path, "CMakeLists.txt", "project(top)")
_write(tmp_path, "vendored/.git", "gitdir: elsewhere")
_write(tmp_path, "vendored/CMakeLists.txt", "add_library(vend STATIC v.cc)")

pruned = cmake.discover_cmake_reactor(tmp_path, snapshot=WalkSnapshot(tmp_path))
included = cmake.discover_cmake_reactor(
tmp_path, snapshot=WalkSnapshot(tmp_path, prune_nested_git=False)
)

assert "vendored/CMakeLists.txt" not in {f.path for f in pruned}
assert "vendored/CMakeLists.txt" in {f.path for f in included}
Loading