diff --git a/CHANGELOG.md b/CHANGELOG.md index b6404ac..dc4da75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,12 +10,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added ### Changed +- Port scanning now skips dependency, cache and VCS directories (`node_modules`, `.venv`, `.git`, `__pycache__`, and similar) when collecting used ports, so ports are no longer reserved from `.env` files that sprout did not generate. A directory whose name is on that list is still walked when it is a worktree root, so a branch named `venv` keeps its own ports reserved +- `find_available_port()` accepts an optional set of already-used ports, letting callers scan the workspace once instead of once per allocated port +- `parse_env_template()` treats its `used_ports` argument as a replacement for the workspace scan rather than an addition to it, and scans the workspace only when the template actually contains `{{ auto_port() }}` ### Deprecated ### Removed ### Fixed +- `sprout create` no longer rescans every worktree for each `{{ auto_port() }}` placeholder, and the scan it does run no longer descends into dependency directories. Together, on a workspace with 8 auto-assigned ports and ~490k files under `.sprout/`, the `.env` generation step dropped from 97s to 2s ### Security diff --git a/README.md b/README.md index 1de0f6e..d0b48bb 100644 --- a/README.md +++ b/README.md @@ -168,6 +168,7 @@ sprout supports three types of placeholders in `.env.example`: - Avoids conflicts across ALL services in ALL sprout environments - Checks system port availability - Ensures global uniqueness even in monorepo setups + - Collects used ports from the `.env` files of every worktree, skipping dependency and cache directories (`node_modules`, `.venv`, `.git`, ...) 3. **Branch Name**: `{{ branch() }}` - Replaced with the current branch/subtree name diff --git a/docs/sprout-cli/usage.md b/docs/sprout-cli/usage.md index acd8ec3..b65be01 100644 --- a/docs/sprout-cli/usage.md +++ b/docs/sprout-cli/usage.md @@ -159,9 +159,9 @@ sprout create existing-feature sprout intelligently manages ports to avoid conflicts: -- Scans existing `.sprout/*/` directories for used ports +- Scans the `.env` files of existing `.sprout/*/` worktrees for used ports, skipping dependency and cache directories (`node_modules`, `.venv`, `.git`, and similar) - Checks system port availability -- Automatically assigns ports starting from 3000 +- Assigns a free port from the 1024-65535 range - Each `{{ auto_port() }}` gets a unique port Example with multiple services: diff --git a/src/sprout/utils.py b/src/sprout/utils.py index 5abc7d1..1306968 100644 --- a/src/sprout/utils.py +++ b/src/sprout/utils.py @@ -5,6 +5,7 @@ import re import socket import subprocess +from collections.abc import Iterator from datetime import datetime from pathlib import Path from typing import TypeAlias @@ -19,6 +20,27 @@ PortNumber: TypeAlias = int PortSet: TypeAlias = set[PortNumber] +# Directory names skipped when scanning worktrees for .env files. These hold +# dependencies, caches and VCS internals, never a .env that sprout generated, +# but they can each contain tens of thousands of files. Descending into them +# turns a port scan into a multi-second walk on a real workspace. +SCAN_EXCLUDED_DIRS: frozenset[str] = frozenset( + { + ".direnv", + ".git", + ".mypy_cache", + ".next", + ".pytest_cache", + ".ruff_cache", + ".terraform", + ".tox", + ".venv", + "__pycache__", + "node_modules", + "venv", + } +) + console = Console() @@ -75,6 +97,45 @@ def ensure_sprout_dir() -> Path: return sprout_dir +def _is_worktree_root(path: Path) -> bool: + """Check whether a directory is a git worktree root. + + Args: + path: Directory to inspect. + """ + # git worktree add always leaves a .git entry: a file in a linked worktree, + # a directory in a main checkout. + return (path / ".git").exists() + + +def iter_env_files(root: Path) -> Iterator[Path]: + """Yield .env files under root, skipping dependency and cache directories. + + An excluded name is still walked when the directory is a worktree root: + a branch named "venv" or "fix/venv" puts a real worktree behind an excluded + name, and skipping it would hand its ports out to the next worktree. + + Args: + root: Directory to walk. + + Yields: + Paths of the .env files found under root. + """ + for dir_path, dir_names, file_names in os.walk(root): + # Prune in place so os.walk does not descend into excluded directories + dir_names[:] = [ + name + for name in dir_names + if name not in SCAN_EXCLUDED_DIRS or _is_worktree_root(Path(dir_path) / name) + ] + for file_name in file_names: + if file_name.endswith(".env"): + env_file = Path(dir_path) / file_name + # Only regular files: a fifo named .env would block read_text() + if env_file.is_file(): + yield env_file + + def get_used_ports() -> PortSet: """Get all ports currently used by sprout worktrees.""" used_ports: PortSet = set() @@ -84,18 +145,17 @@ def get_used_ports() -> PortSet: return used_ports # Scan all .env files recursively in .sprout/ - for env_file in sprout_dir.rglob("*.env"): - if env_file.is_file(): - try: - content = env_file.read_text() - # Find all port assignments (e.g., PORT=8080) - port_matches = re.findall(r"=(\d{4,5})\b", content) - for port_str in port_matches: - port = int(port_str) - if 1024 <= port <= 65535: - used_ports.add(port) - except (OSError, ValueError): - continue + for env_file in iter_env_files(sprout_dir): + try: + content = env_file.read_text() + # Find all port assignments (e.g., PORT=8080) + port_matches = re.findall(r"=(\d{4,5})\b", content) + for port_str in port_matches: + port = int(port_str) + if 1024 <= port <= 65535: + used_ports.add(port) + except (OSError, ValueError): + continue return used_ports @@ -110,9 +170,18 @@ def is_port_available(port: PortNumber) -> bool: return False -def find_available_port() -> PortNumber: - """Find an available port that's not used by sprout or system.""" - used_ports = get_used_ports() +def find_available_port(used_ports: PortSet | None = None) -> PortNumber: + """Find an available port that's not used by sprout or system. + + Args: + used_ports: Ports to treat as taken, replacing the workspace scan rather + than adding to it. Scanning every worktree is the expensive part of + this call, so callers that allocate several ports in a row should + scan once and pass the result instead of paying for a fresh walk per + port. An empty set means nothing is taken; pass None to scan. + """ + if used_ports is None: + used_ports = get_used_ports() max_attempts = 1000 for _ in range(max_attempts): @@ -136,7 +205,11 @@ def parse_env_template( Args: template_path: Path to the .env.example template file silent: If True, use stderr for prompts to keep stdout clean - used_ports: Set of ports already in use (in addition to system-wide used ports) + used_ports: Ports already in use across the workspace. The set replaces + the workspace scan rather than adding to it, so passing a partial + set silently gives up collision detection against the other + worktrees. An empty set means nothing is taken; omit it to have this + function scan once, and only if the template asks for a port. branch_name: Branch name to use for {{ branch() }} placeholders """ if not template_path.exists(): @@ -148,11 +221,12 @@ def parse_env_template( raise SproutError(f"Failed to read .env.example: {e}") from e lines: list[str] = [] - # Track used ports within this file to avoid duplicates - file_ports: PortSet = set() - # Include any additional used ports passed in - if used_ports: - file_ports.update(used_ports) + # Ports that {{ auto_port() }} must avoid: the ones already taken across the + # workspace plus the ones handed out earlier in this file. Left as None + # until the first port is requested, so a template without auto_port() - + # and every caller that already knows the used ports - never walks the + # worktrees. Once filled, it is reused for the rest of the file. + file_ports: PortSet | None = set(used_ports) if used_ports is not None else None for line in content.splitlines(): # Process {{ auto_port() | default }} placeholders @@ -162,10 +236,13 @@ def replace_auto_port(match: re.Match[str]) -> str: if default_value is not None: default_value = default_value.strip() - # Generate available port - port = find_available_port() - while port in file_ports: - port = find_available_port() + # Generate available port. Scan the worktrees on the first + # placeholder only; from then on file_ports carries both those + # ports and the ones assigned earlier in this file. + nonlocal file_ports + if file_ports is None: + file_ports = get_used_ports() + port = find_available_port(file_ports) file_ports.add(port) return str(port) diff --git a/tests/test_multi_env.py b/tests/test_multi_env.py index ef581fa..4e75e6d 100644 --- a/tests/test_multi_env.py +++ b/tests/test_multi_env.py @@ -4,6 +4,7 @@ from typer.testing import CliRunner +import sprout.utils from sprout.cli import app from sprout.utils import get_used_ports @@ -146,6 +147,28 @@ def test_global_port_uniqueness_across_worktrees(self, git_repo, monkeypatch): # Ensure no overlap assert len(ports1.intersection(ports2)) == 0, "Ports should not overlap between worktrees" + def test_create_scans_the_workspace_once(self, git_repo, monkeypatch, mocker): # noqa: F811 + """Test one create walks the worktrees once, not once per port.""" + git_repo, _default_branch = git_repo + monkeypatch.chdir(git_repo) + monkeypatch.setenv("API_KEY", "test_key") # Set env var for root .env.example + + service = git_repo / "service" + service.mkdir() + (service / ".env.example").write_text( + "P1={{ auto_port() }}\nP2={{ auto_port() }}\nP3={{ auto_port() }}\n" + ) + subprocess.run(["git", "add", "."], cwd=git_repo, check=True) + subprocess.run(["git", "commit", "-m", "Add service"], cwd=git_repo, check=True) + + # Every scan funnels through iter_env_files, whichever module asked for it + spy = mocker.spy(sprout.utils, "iter_env_files") + + result = runner.invoke(app, ["create", "scan-once"]) + + assert result.exit_code == 0 + assert spy.call_count == 1 + def test_nested_directory_structure(self, git_repo, monkeypatch): # noqa: F811 """Test handling of nested directory structures.""" git_repo, default_branch = git_repo diff --git a/tests/test_utils.py b/tests/test_utils.py index ad92066..6601e0f 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -10,12 +10,14 @@ from sprout.exceptions import SproutError from sprout.utils import ( + SCAN_EXCLUDED_DIRS, branch_exists, find_available_port, get_git_root, get_used_ports, is_git_repository, is_port_available, + iter_env_files, parse_env_template, run_command, worktree_exists, @@ -100,6 +102,36 @@ def test_get_used_ports_empty(self, tmp_path, mocker): ports = get_used_ports() assert ports == set() + def test_get_used_ports_skips_dependency_directories(self, tmp_path, mocker): + """Test get_used_ports ignores .env files under dependency directories.""" + mocker.patch("sprout.utils.get_sprout_dir", return_value=tmp_path) + + (tmp_path / "branch1").mkdir() + (tmp_path / "branch1" / ".env").write_text("WEB_PORT=8080") + + # Dependency and cache trees can ship their own .env files, and walking + # them is what made port allocation slow on large worktrees. + vendored = tmp_path / "branch1" / "node_modules" / "some-package" + vendored.mkdir(parents=True) + (vendored / ".env").write_text("VENDOR_PORT=9999") + + virtualenv = tmp_path / "branch1" / ".venv" / "lib" + virtualenv.mkdir(parents=True) + (virtualenv / ".env").write_text("VENV_PORT=9998") + + assert get_used_ports() == {8080} + + def test_get_used_ports_ignores_unreadable_files(self, tmp_path, mocker): + """Test a .env that cannot be read is skipped, not fatal.""" + mocker.patch("sprout.utils.get_sprout_dir", return_value=tmp_path) + + (tmp_path / "branch1").mkdir() + (tmp_path / "branch1" / ".env").write_text("WEB_PORT=8080") + # Dangling symlink: reported by the walk, unreadable when opened + (tmp_path / "branch1" / "broken.env").symlink_to(tmp_path / "gone" / ".env") + + assert get_used_ports() == {8080} + def test_is_port_available_true(self): """Test is_port_available returns True for free port.""" # Find a likely free port @@ -137,6 +169,93 @@ def test_find_available_port_exhausted(self, mocker): with pytest.raises(SproutError, match="Could not find an available port"): find_available_port() + def test_find_available_port_accepts_precomputed_ports(self, mocker): + """Test find_available_port skips the workspace scan when given ports.""" + scan = mocker.patch("sprout.utils.get_used_ports", return_value=set()) + mocker.patch("sprout.utils.is_port_available", return_value=True) + mocker.patch("random.randint", side_effect=[8080, 8081]) + + port = find_available_port({8080}) + + assert port == 8081 + scan.assert_not_called() + + def test_find_available_port_empty_set_is_not_a_rescan(self, mocker): + """Test an empty set means 'nothing taken', not 'go and scan'.""" + scan = mocker.patch("sprout.utils.get_used_ports", return_value={8080}) + mocker.patch("sprout.utils.is_port_available", return_value=True) + mocker.patch("random.randint", side_effect=[8080]) + + assert find_available_port(set()) == 8080 + scan.assert_not_called() + + +class TestIterEnvFiles: + """Test the .env discovery walk.""" + + # Spelled out rather than derived from SCAN_EXCLUDED_DIRS: deriving it would + # make the test shrink along with the constant instead of failing. + EXCLUDED_DIRS = [ + ".direnv", + ".git", + ".mypy_cache", + ".next", + ".pytest_cache", + ".ruff_cache", + ".terraform", + ".tox", + ".venv", + "__pycache__", + "node_modules", + "venv", + ] + + def test_excluded_dirs_constant_is_in_sync(self): + """Test the spelled-out list still matches the implementation.""" + assert set(self.EXCLUDED_DIRS) == SCAN_EXCLUDED_DIRS + + @pytest.mark.parametrize("excluded", EXCLUDED_DIRS) + def test_iter_env_files_skips_excluded_directory(self, tmp_path, excluded): + """Test each excluded directory is left unwalked.""" + (tmp_path / "worktree").mkdir() + (tmp_path / "worktree" / ".env").write_text("PORT=8080") + + buried = tmp_path / "worktree" / excluded / "nested" + buried.mkdir(parents=True) + (buried / ".env").write_text("PORT=9999") + + assert list(iter_env_files(tmp_path)) == [tmp_path / "worktree" / ".env"] + + @pytest.mark.parametrize("excluded", EXCLUDED_DIRS) + def test_iter_env_files_walks_worktree_named_after_excluded_dir(self, tmp_path, excluded): + """Test a worktree is walked even when its name is on the exclude list.""" + # A branch named e.g. "venv" or "fix/venv" puts a real worktree behind an + # excluded name; its ports must still be collected. + worktree = tmp_path / excluded + worktree.mkdir() + (worktree / ".git").write_text("gitdir: /somewhere/.git/worktrees/venv\n") + (worktree / ".env").write_text("PORT=8080") + + assert list(iter_env_files(tmp_path)) == [worktree / ".env"] + + def test_iter_env_files_matches_suffixed_names(self, tmp_path): + """Test names ending in .env are matched, as rglob('*.env') did.""" + (tmp_path / ".env").write_text("A=1") + (tmp_path / "local.env").write_text("B=2") + (tmp_path / "env").write_text("C=3") + (tmp_path / "notes.txt").write_text("D=4") + + assert {path.name for path in iter_env_files(tmp_path)} == {".env", "local.env"} + + @pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="requires mkfifo") + def test_iter_env_files_skips_non_regular_files(self, tmp_path): + """Test a fifo named .env is skipped instead of blocking a read.""" + os.mkfifo(tmp_path / ".env") + (tmp_path / "real").mkdir() + (tmp_path / "real" / ".env").write_text("PORT=8080") + + assert list(iter_env_files(tmp_path)) == [tmp_path / "real" / ".env"] + class TestEnvTemplateParser: """Test .env template parsing.""" @@ -151,6 +270,70 @@ def test_parse_env_template_auto_port(self, tmp_path, mocker): result = parse_env_template(template) assert result == "WEB_PORT=8080\nAPI_PORT=3000" + def test_parse_env_template_scans_workspace_once(self, tmp_path, mocker): + """Test the workspace is scanned once, not once per auto_port().""" + scan = mocker.patch("sprout.utils.get_used_ports", return_value=set()) + mocker.patch("sprout.utils.is_port_available", return_value=True) + mocker.patch("random.randint", side_effect=[8080, 8081, 8082]) + + template = tmp_path / ".env.example" + template.write_text( + "A_PORT={{ auto_port() }}\nB_PORT={{ auto_port() }}\nC_PORT={{ auto_port() }}" + ) + + result = parse_env_template(template) + + assert result == "A_PORT=8080\nB_PORT=8081\nC_PORT=8082" + assert scan.call_count == 1 + + def test_parse_env_template_reuses_given_used_ports(self, tmp_path, mocker): + """Test a caller-supplied port set replaces the workspace scan.""" + scan = mocker.patch("sprout.utils.get_used_ports", return_value=set()) + mocker.patch("sprout.utils.is_port_available", return_value=True) + mocker.patch("random.randint", side_effect=[8080, 8081]) + + template = tmp_path / ".env.example" + template.write_text("WEB_PORT={{ auto_port() }}") + + result = parse_env_template(template, used_ports={8080}) + + assert result == "WEB_PORT=8081" + scan.assert_not_called() + + def test_parse_env_template_empty_used_ports_is_not_a_rescan(self, tmp_path, mocker): + """Test an empty port set is taken at face value, not as 'unscanned'.""" + scan = mocker.patch("sprout.utils.get_used_ports", return_value={8080}) + mocker.patch("sprout.utils.is_port_available", return_value=True) + mocker.patch("random.randint", side_effect=[8080]) + + template = tmp_path / ".env.example" + template.write_text("WEB_PORT={{ auto_port() }}") + + assert parse_env_template(template, used_ports=set()) == "WEB_PORT=8080" + scan.assert_not_called() + + def test_parse_env_template_without_auto_port_does_not_scan(self, tmp_path, mocker): + """Test a template with no auto_port() never touches the workspace.""" + scan = mocker.patch("sprout.utils.get_used_ports", return_value=set()) + + template = tmp_path / ".env.example" + template.write_text("NAME=static\nURL=${HOST}/path") + + assert parse_env_template(template) == "NAME=static\nURL=${HOST}/path" + scan.assert_not_called() + + def test_parse_env_template_never_repeats_a_port_within_a_file(self, tmp_path, mocker): + """Test a port assigned earlier in the file is excluded from later draws.""" + mocker.patch("sprout.utils.get_used_ports", return_value=set()) + mocker.patch("sprout.utils.is_port_available", return_value=True) + # The same port is drawn twice; the second placeholder must reject it + mocker.patch("random.randint", side_effect=[8080, 8080, 8081]) + + template = tmp_path / ".env.example" + template.write_text("A={{ auto_port() }}\nB={{ auto_port() }}") + + assert parse_env_template(template) == "A=8080\nB=8081" + def test_parse_env_template_variable_from_env(self, tmp_path, mocker): """Test parsing {{ VARIABLE }} from environment.""" mocker.patch.dict(os.environ, {"API_KEY": "secret123"})