From 5042abc091e35c88c686b258699770d4aac1f0e2 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 18 Oct 2025 10:40:16 -0500 Subject: [PATCH 1/7] tests/cli(test-import): Capture missing trailing slash scan bug --- tests/cli/test_import.py | 60 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/cli/test_import.py b/tests/cli/test_import.py index dcce85e6..0ad7a48c 100644 --- a/tests/cli/test_import.py +++ b/tests/cli/test_import.py @@ -795,6 +795,66 @@ def test_many_existing_repos_summary( assert "Found 8 existing repositories already in configuration" in clean_output assert "All found repositories already exist" in clean_output + @pytest.mark.xfail( + reason="Existing repos are rediscovered when config keys omit trailing '/'", + strict=True, + ) + def test_scan_home_config_without_trailing_slash_duplicates_repo( + self, + create_git_remote_repo: CreateRepoPytestFixtureFn, + tmp_path: pathlib.Path, + git_commit_envvars: dict[str, str], + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Reproduce bug where scan re-adds repo despite config entry without slash.""" + home_dir = tmp_path / "home" + scan_dir = home_dir / "study" / "c" + repo_path = scan_dir / "cpython" + scan_dir.mkdir(parents=True, exist_ok=True) + + remote_path = create_git_remote_repo() + remote_url = f"file://{remote_path}" + clone_repo(remote_url, repo_path, git_commit_envvars) + + config_file = home_dir / ".vcspull.yaml" + config_file.write_text( + ( + "~/study/c:\n" + " cpython:\n" + " repo: git+https://github.com/python/cpython.git\n" + ), + encoding="utf-8", + ) + + monkeypatch.setenv("HOME", str(home_dir)) + monkeypatch.delenv("XDG_CONFIG_HOME", raising=False) + + monkeypatch.setattr( + "vcspull.cli._import.get_git_origin_url", + lambda _path: "git+https://github.com/python/cpython.git", + ) + + import_from_filesystem( + scan_dir_str="~/study/c", + config_file_path_str=None, + recursive=False, + base_dir_key_arg=None, + yes=True, + ) + + with config_file.open(encoding="utf-8") as f: + config_data = yaml.safe_load(f) + + assert "~/study/c" in config_data + assert "cpython" in config_data["~/study/c"] + assert ( + config_data["~/study/c"]["cpython"]["repo"] + == "git+https://github.com/python/cpython.git" + ) + + # Bug: scan adds a duplicated section with a trailing slash suffix. + assert "~/study/c/" not in config_data + # ============================================================================= # Help and output tests From ec0c1e6f62e4d931bd2a018af30fa99f5d930715 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 18 Oct 2025 11:56:02 -0500 Subject: [PATCH 2/7] tests/cli(test-import): Expand scan regression matrix --- tests/cli/test_import.py | 131 ++++++++++++++++++++++++++++++--------- 1 file changed, 102 insertions(+), 29 deletions(-) diff --git a/tests/cli/test_import.py b/tests/cli/test_import.py index 0ad7a48c..2d9cac96 100644 --- a/tests/cli/test_import.py +++ b/tests/cli/test_import.py @@ -186,6 +186,48 @@ class ImportRepoFixture(t.NamedTuple): ] +class ScanExistingFixture(t.NamedTuple): + """Fixture for scan behaviour when configuration already includes a repo.""" + + test_id: str + config_key_style: str + scan_arg_style: str + + +SCAN_EXISTING_FIXTURES: list[ScanExistingFixture] = [ + ScanExistingFixture( + test_id="tilde-no-slash_scan-tilde", + config_key_style="tilde_no_slash", + scan_arg_style="tilde", + ), + ScanExistingFixture( + test_id="tilde-no-slash_scan-abs", + config_key_style="tilde_no_slash", + scan_arg_style="absolute", + ), + ScanExistingFixture( + test_id="tilde-with-slash_scan-tilde", + config_key_style="tilde_with_slash", + scan_arg_style="tilde", + ), + ScanExistingFixture( + test_id="tilde-with-slash_scan-abs", + config_key_style="tilde_with_slash", + scan_arg_style="absolute", + ), + ScanExistingFixture( + test_id="absolute-no-slash_scan-abs", + config_key_style="absolute_no_slash", + scan_arg_style="absolute", + ), + ScanExistingFixture( + test_id="absolute-with-slash_scan-abs", + config_key_style="absolute_with_slash", + scan_arg_style="absolute", + ), +] + + @pytest.mark.parametrize( list(ImportRepoFixture._fields), IMPORT_REPO_FIXTURES, @@ -795,47 +837,78 @@ def test_many_existing_repos_summary( assert "Found 8 existing repositories already in configuration" in clean_output assert "All found repositories already exist" in clean_output - @pytest.mark.xfail( - reason="Existing repos are rediscovered when config keys omit trailing '/'", - strict=True, + @pytest.mark.parametrize( + list(ScanExistingFixture._fields), + [ + pytest.param( + *fixture, + marks=pytest.mark.xfail( + reason=( + "Existing repos are re-added when config key format does not " + "match the scanner's trailing-slash expectations." + ), + strict=True, + ), + ) + if fixture.config_key_style.endswith("no_slash") + or fixture.config_key_style.startswith("absolute_") + else fixture + for fixture in SCAN_EXISTING_FIXTURES + ], + ids=[fixture.test_id for fixture in SCAN_EXISTING_FIXTURES], ) - def test_scan_home_config_without_trailing_slash_duplicates_repo( + def test_scan_respects_existing_config_sections( self, - create_git_remote_repo: CreateRepoPytestFixtureFn, tmp_path: pathlib.Path, - git_commit_envvars: dict[str, str], monkeypatch: pytest.MonkeyPatch, + test_id: str, + config_key_style: str, + scan_arg_style: str, ) -> None: - """Reproduce bug where scan re-adds repo despite config entry without slash.""" + """Ensure filesystem scan does not duplicate repositories in config.""" home_dir = tmp_path / "home" scan_dir = home_dir / "study" / "c" - repo_path = scan_dir / "cpython" - scan_dir.mkdir(parents=True, exist_ok=True) - - remote_path = create_git_remote_repo() - remote_url = f"file://{remote_path}" - clone_repo(remote_url, repo_path, git_commit_envvars) + repo_dir = scan_dir / "cpython" + repo_git_dir = repo_dir / ".git" + repo_git_dir.mkdir(parents=True, exist_ok=True) + expected_repo_url = "git+https://github.com/python/cpython.git" config_file = home_dir / ".vcspull.yaml" + + if config_key_style == "tilde_no_slash": + config_key = "~/study/c" + elif config_key_style == "tilde_with_slash": + config_key = "~/study/c/" + elif config_key_style == "absolute_no_slash": + config_key = str(scan_dir) + elif config_key_style == "absolute_with_slash": + config_key = str(scan_dir) + "/" + else: + error_msg = f"Unhandled config_key_style: {config_key_style}" + raise AssertionError(error_msg) + config_file.write_text( - ( - "~/study/c:\n" - " cpython:\n" - " repo: git+https://github.com/python/cpython.git\n" - ), + (f"{config_key}:\n cpython:\n repo: {expected_repo_url}\n"), encoding="utf-8", ) monkeypatch.setenv("HOME", str(home_dir)) monkeypatch.delenv("XDG_CONFIG_HOME", raising=False) - monkeypatch.setattr( "vcspull.cli._import.get_git_origin_url", - lambda _path: "git+https://github.com/python/cpython.git", + lambda _path: expected_repo_url, ) + if scan_arg_style == "tilde": + scan_arg = "~/study/c" + elif scan_arg_style == "absolute": + scan_arg = str(scan_dir) + else: + error_msg = f"Unhandled scan_arg_style: {scan_arg_style}" + raise AssertionError(error_msg) + import_from_filesystem( - scan_dir_str="~/study/c", + scan_dir_str=scan_arg, config_file_path_str=None, recursive=False, base_dir_key_arg=None, @@ -845,15 +918,15 @@ def test_scan_home_config_without_trailing_slash_duplicates_repo( with config_file.open(encoding="utf-8") as f: config_data = yaml.safe_load(f) - assert "~/study/c" in config_data - assert "cpython" in config_data["~/study/c"] - assert ( - config_data["~/study/c"]["cpython"]["repo"] - == "git+https://github.com/python/cpython.git" - ) + assert config_key in config_data + assert "cpython" in config_data[config_key] + assert config_data[config_key]["cpython"]["repo"] == expected_repo_url + assert len(config_data) == 1 - # Bug: scan adds a duplicated section with a trailing slash suffix. - assert "~/study/c/" not in config_data + alternate_key = ( + config_key[:-1] if config_key.endswith("/") else f"{config_key}/" + ) + assert alternate_key not in config_data # ============================================================================= From 4ca8159228f6ea6d11c0e02eb9fa44099ecfe586 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 18 Oct 2025 13:22:40 -0500 Subject: [PATCH 3/7] cli(import): standardize workspace root terminology --- CHANGES | 4 +- README.md | 6 +- docs/cli/import.md | 16 ++-- docs/quickstart.md | 4 +- src/vcspull/cli/__init__.py | 4 +- src/vcspull/cli/_import.py | 151 +++++++++++++++++++----------------- src/vcspull/types.py | 42 +++++++++- tests/cli/test_import.py | 95 +++++++++++++---------- 8 files changed, 187 insertions(+), 135 deletions(-) diff --git a/CHANGES b/CHANGES index e2da0902..9da71ed0 100644 --- a/CHANGES +++ b/CHANGES @@ -28,11 +28,11 @@ _Notes on upcoming releases will be added here_ #### New command: `vcspull import` (#465) - **Manual import**: Register a single repository with `vcspull import ` - - Optional `--dir`/`--path` helpers for base-directory detection + - Optional `--workspace-root`/`--path` helpers for workspace-root detection - **Filesystem scan**: Discover and import existing repositories with `vcspull import --scan ` - Recursively scan with `--recursive`/`-r` - Interactive confirmation prompt or `--yes` for unattended runs - - Custom base directory with `--base-dir-key` + - Custom workspace root with `--workspace-root` #### New command: `vcspull fmt` (#465) diff --git a/README.md b/README.md index af8aca60..db51f26c 100644 --- a/README.md +++ b/README.md @@ -100,8 +100,8 @@ $ vcspull import my-lib https://github.com/example/my-lib.git --path ~/code/my-l ``` - Omit `--path` to default the entry under `./`. -- Use `--dir` when you want to force a specific base-directory key, e.g. - `--dir ~/projects/libs`. +- Use `--workspace-root` when you want to force a specific workspace root, e.g. + `--workspace-root ~/projects/libs`. - Pass `-c/--config` to import into an alternate YAML file. - Follow with `vcspull sync my-lib` to clone or update the working tree after registration. @@ -115,7 +115,7 @@ $ vcspull import --scan ~/code --recursive ``` The scan shows each repository before import unless you opt into `--yes`. Add -`--base-dir-key ~/code/` to pin the resulting section name or `--config` to +`--workspace-root ~/code/` to pin the resulting workspace root or `--config` to write somewhere other than the default `~/.vcspull.yaml`. ### Normalize configuration files diff --git a/docs/cli/import.md b/docs/cli/import.md index 2de3aace..c39cc85a 100644 --- a/docs/cli/import.md +++ b/docs/cli/import.md @@ -21,11 +21,13 @@ directories for Git repositories that already live on disk. Provide a repository name and remote URL to append an entry to your configuration. Use `--path` when you already have a working tree on disk so the -configured base directory matches its location. Override the inferred base -directory with `--dir` when you need a specific configuration key. +inferred workspace root matches its location. Override the detected workspace +root with `--workspace-root` when you need to target a specific directory. ```console $ vcspull import my-lib https://github.com/example/my-lib.git --path ~/code/my-lib +$ vcspull import another-lib https://github.com/example/another-lib.git \ + --workspace-root ~/code ``` With no `-c/--config` flag vcspull looks for the first YAML configuration file @@ -36,18 +38,18 @@ new `.vcspull.yaml` is created next to where you run the command. `vcspull import --scan` discovers Git repositories that already exist on disk and writes them to your configuration. The command prompts before adding each -repository, showing the inferred name, directory key, and origin URL (when +repository, showing the inferred name, workspace root, and origin URL (when available). ```console $ vcspull import --scan ~/code --recursive -? Add ~/code/vcspull (dir: ~/code/)? [y/N]: y -? Add ~/code/libvcs (dir: ~/code/)? [y/N]: y +? Add ~/code/vcspull (workspace root: ~/code/)? [y/N]: y +? Add ~/code/libvcs (workspace root: ~/code/)? [y/N]: y ``` - `--recursive`/`-r` searches nested directories. -- `--base-dir-key` forces all discovered repositories to use the same base - directory key, overriding the automatically expanded directory. +- `--workspace-root` forces all discovered repositories to use the same + workspace root, overriding the directory inferred from their location. - `--yes`/`-y` accepts every suggestion, which is useful for unattended migrations. diff --git a/docs/quickstart.md b/docs/quickstart.md index 47867c63..460c2960 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -114,8 +114,8 @@ YAML? Create a `~/.vcspull.yaml` file: Already have repositories cloned locally? Use `vcspull import --scan ~/code --recursive` to detect existing Git checkouts and append them to your configuration. See {ref}`cli-import` for more details and -options such as `--base-dir-key` and `--yes` for unattended runs. After editing -or importing, run `vcspull fmt --write` (documented in {ref}`cli-fmt`) to +options such as `--workspace-root` and `--yes` for unattended runs. After editing or +importing, run `vcspull fmt --write` (documented in {ref}`cli-fmt`) to normalize keys and keep your configuration tidy. The `git+` in front of the repository URL. Mercurial repositories use diff --git a/src/vcspull/cli/__init__.py b/src/vcspull/cli/__init__.py index 6af9125f..32048141 100644 --- a/src/vcspull/cli/__init__.py +++ b/src/vcspull/cli/__init__.py @@ -132,7 +132,7 @@ def cli(_args: list[str] | None = None) -> None: scan_dir_str=args.scan_dir, config_file_path_str=args.config, recursive=args.recursive, - base_dir_key_arg=args.base_dir_key, + workspace_root_override=args.workspace_root_path, yes=args.yes, ) elif args.name and args.url: @@ -142,7 +142,7 @@ def cli(_args: list[str] | None = None) -> None: url=args.url, config_file_path_str=args.config, path=args.path, - base_dir=args.base_dir, + workspace_root_path=args.workspace_root_path, ) else: # Error: need either name+url or --scan diff --git a/src/vcspull/cli/_import.py b/src/vcspull/cli/_import.py index 99d1a233..08e47714 100644 --- a/src/vcspull/cli/_import.py +++ b/src/vcspull/cli/_import.py @@ -2,6 +2,7 @@ from __future__ import annotations +import argparse import logging import os import pathlib @@ -74,13 +75,17 @@ def create_import_subparser(parser: argparse.ArgumentParser) -> None: "--path", dest="path", help="Local directory path where repo will be cloned " - "(determines base directory key if not specified with --dir)", + "(determines workspace root if not specified with --workspace-root)", ) parser.add_argument( - "--dir", - dest="base_dir", - help="Base directory key in config (e.g., '~/projects/'). " - "If not specified, will be inferred from --path or use current directory.", + "--workspace-root", + dest="workspace_root_path", + metavar="DIR", + help=( + "Workspace root directory in config (e.g., '~/projects/'). " + "If not specified, will be inferred from --path or use current directory. " + "When used with --scan, applies the workspace root to all discovered repos." + ), ) # Filesystem scan mode @@ -96,10 +101,6 @@ def create_import_subparser(parser: argparse.ArgumentParser) -> None: action="store_true", help="Scan directories recursively (use with --scan)", ) - parser.add_argument( - "--base-dir-key", - help="Base directory key for scanned repos (use with --scan)", - ) parser.add_argument( "--yes", "-y", @@ -113,7 +114,7 @@ def import_repo( url: str, config_file_path_str: str | None, path: str | None, - base_dir: str | None, + workspace_root_path: str | None, ) -> None: """Import a repository to the vcspull configuration. @@ -127,8 +128,8 @@ def import_repo( Path to config file, or None to use default path : str | None Local path where repo will be cloned - base_dir : str | None - Base directory key to use in config + workspace_root_path : str | None + Workspace root to use in config """ # Determine config file config_file_path: pathlib.Path @@ -177,36 +178,41 @@ def import_repo( config_file_path, ) - # Determine base directory key - if base_dir: - # Use explicit base directory - base_dir_key = base_dir if base_dir.endswith("/") else base_dir + "/" + # Determine workspace root key + if workspace_root_path: + workspace_root_key = ( + workspace_root_path + if workspace_root_path.endswith("/") + else workspace_root_path + "/" + ) elif path: # Infer from provided path repo_path = pathlib.Path(path).expanduser().resolve() try: # Try to make it relative to home - base_dir_key = "~/" + str(repo_path.relative_to(pathlib.Path.home())) + "/" + workspace_root_key = ( + "~/" + str(repo_path.relative_to(pathlib.Path.home())) + "/" + ) except ValueError: # Use absolute path - base_dir_key = str(repo_path) + "/" + workspace_root_key = str(repo_path) + "/" else: # Default to current directory - base_dir_key = "./" + workspace_root_key = "./" - # Ensure base directory key exists in config - if base_dir_key not in raw_config: - raw_config[base_dir_key] = {} - elif not isinstance(raw_config[base_dir_key], dict): + # Ensure workspace root key exists in config + if workspace_root_key not in raw_config: + raw_config[workspace_root_key] = {} + elif not isinstance(raw_config[workspace_root_key], dict): log.error( - "Configuration section '%s' is not a dictionary. Aborting.", - base_dir_key, + "Workspace root '%s' in configuration is not a dictionary. Aborting.", + workspace_root_key, ) return # Check if repo already exists - if name in raw_config[base_dir_key]: - existing_config = raw_config[base_dir_key][name] + if name in raw_config[workspace_root_key]: + existing_config = raw_config[workspace_root_key][name] # Handle both string and dict formats current_url: str if isinstance(existing_config, str): @@ -222,13 +228,13 @@ def import_repo( "Repository '%s' already exists under '%s'. Current URL: %s. " "To update, remove and re-add, or edit the YAML file manually.", name, - base_dir_key, + workspace_root_key, current_url, ) return # Add the repository in verbose format - raw_config[base_dir_key][name] = {"repo": url} + raw_config[workspace_root_key][name] = {"repo": url} # Save config try: @@ -247,7 +253,7 @@ def import_repo( config_file_path, Style.RESET_ALL, Fore.MAGENTA, - base_dir_key, + workspace_root_key, Style.RESET_ALL, ) except Exception: @@ -261,7 +267,7 @@ def import_from_filesystem( scan_dir_str: str, config_file_path_str: str | None, recursive: bool, - base_dir_key_arg: str | None, + workspace_root_override: str | None, yes: bool, ) -> None: """Scan filesystem for git repositories and import to vcspull config. @@ -274,8 +280,8 @@ def import_from_filesystem( Path to config file, or None to use default recursive : bool Whether to scan subdirectories recursively - base_dir_key_arg : str | None - Base directory key to use in config (overrides automatic detection) + workspace_root_override : str | None + Workspace root to use in config (overrides automatic detection) yes : bool Whether to skip confirmation prompt """ @@ -335,9 +341,8 @@ def import_from_filesystem( Style.RESET_ALL, ) - found_repos: list[ - tuple[str, str, str] - ] = [] # (repo_name, repo_url, determined_base_key) + # Each entry stores (repo_name, repo_url, workspace_root_key) + found_repos: list[tuple[str, str, str]] = [] if recursive: for root, dirs, _ in os.walk(scan_dir): @@ -354,25 +359,25 @@ def import_from_filesystem( ) continue - determined_base_key: str - if base_dir_key_arg: - determined_base_key = ( - base_dir_key_arg - if base_dir_key_arg.endswith("/") - else base_dir_key_arg + "/" + workspace_root_key: str + if workspace_root_override: + workspace_root_key = ( + workspace_root_override + if workspace_root_override.endswith("/") + else workspace_root_override + "/" ) else: try: - determined_base_key = ( + workspace_root_key = ( "~/" + str(scan_dir.relative_to(pathlib.Path.home())) + "/" ) except ValueError: - determined_base_key = str(scan_dir.resolve()) + "/" + workspace_root_key = str(scan_dir.resolve()) + "/" - if not determined_base_key.endswith("/"): - determined_base_key += "/" + if not workspace_root_key.endswith("/"): + workspace_root_key += "/" - found_repos.append((repo_name, repo_url, determined_base_key)) + found_repos.append((repo_name, repo_url, workspace_root_key)) else: # Non-recursive: only check immediate subdirectories for item in scan_dir.iterdir(): @@ -388,24 +393,24 @@ def import_from_filesystem( ) continue - if base_dir_key_arg: - determined_base_key = ( - base_dir_key_arg - if base_dir_key_arg.endswith("/") - else base_dir_key_arg + "/" + if workspace_root_override: + workspace_root_key = ( + workspace_root_override + if workspace_root_override.endswith("/") + else workspace_root_override + "/" ) else: try: - determined_base_key = ( + workspace_root_key = ( "~/" + str(scan_dir.relative_to(pathlib.Path.home())) + "/" ) except ValueError: - determined_base_key = str(scan_dir.resolve()) + "/" + workspace_root_key = str(scan_dir.resolve()) + "/" - if not determined_base_key.endswith("/"): - determined_base_key += "/" + if not workspace_root_key.endswith("/"): + workspace_root_key += "/" - found_repos.append((repo_name, repo_url, determined_base_key)) + found_repos.append((repo_name, repo_url, workspace_root_key)) if not found_repos: log.info( @@ -419,14 +424,14 @@ def import_from_filesystem( return repos_to_add: list[tuple[str, str, str]] = [] - existing_repos: list[tuple[str, str, str]] = [] # (name, url, key) + existing_repos: list[tuple[str, str, str]] = [] # (name, url, workspace_root_key) - for name, url, key in found_repos: - target_section = raw_config.get(key, {}) + for name, url, workspace_root_key in found_repos: + target_section = raw_config.get(workspace_root_key, {}) if isinstance(target_section, dict) and name in target_section: - existing_repos.append((name, url, key)) + existing_repos.append((name, url, workspace_root_key)) else: - repos_to_add.append((name, url, key)) + repos_to_add.append((name, url, workspace_root_key)) if existing_repos: # Show summary only when there are many existing repos @@ -449,7 +454,7 @@ def import_from_filesystem( len(existing_repos), Style.RESET_ALL, ) - for name, url, key in existing_repos: + for name, url, workspace_root_key in existing_repos: log.info( " %s•%s %s%s%s (%s%s%s) at %s%s%s%s in %s%s%s", Fore.BLUE, @@ -461,7 +466,7 @@ def import_from_filesystem( url, Style.RESET_ALL, Fore.MAGENTA, - key, + workspace_root_key, name, Style.RESET_ALL, Fore.BLUE, @@ -511,19 +516,19 @@ def import_from_filesystem( return changes_made = False - for repo_name, repo_url, determined_base_key in repos_to_add: - if determined_base_key not in raw_config: - raw_config[determined_base_key] = {} - elif not isinstance(raw_config[determined_base_key], dict): + for repo_name, repo_url, workspace_root_key in repos_to_add: + if workspace_root_key not in raw_config: + raw_config[workspace_root_key] = {} + elif not isinstance(raw_config[workspace_root_key], dict): log.warning( - "Section '%s' in config is not a dictionary. Skipping repo %s.", - determined_base_key, + "Workspace root '%s' in config is not a dictionary. Skipping repo %s.", + workspace_root_key, repo_name, ) continue - if repo_name not in raw_config[determined_base_key]: - raw_config[determined_base_key][repo_name] = {"repo": repo_url} + if repo_name not in raw_config[workspace_root_key]: + raw_config[workspace_root_key][repo_name] = {"repo": repo_url} log.info( "%s+%s Importing %s'%s'%s (%s%s%s) under '%s%s%s'.", Fore.GREEN, @@ -535,7 +540,7 @@ def import_from_filesystem( repo_url, Style.RESET_ALL, Fore.MAGENTA, - determined_base_key, + workspace_root_key, Style.RESET_ALL, ) changes_made = True diff --git a/src/vcspull/types.py b/src/vcspull/types.py index 29217d34..9053956e 100644 --- a/src/vcspull/types.py +++ b/src/vcspull/types.py @@ -1,14 +1,40 @@ -"""Typings for vcspull.""" +"""Typings for vcspull. + +Configuration Object Graph +-------------------------- + +The user-facing ``.vcspull.yaml`` maps *workspace roots* (parent directories) +to named repositories. For example:: + + ~/study/c: + cpython: + repo: git+https://github.com/python/cpython.git + tmux: + repo: git+https://github.com/tmux/tmux.git + + ~/work/js: + react: + repo: https://github.com/facebook/react.git + vite: + repo: https://github.com/vitejs/vite.git + +In Python we model this as: + +``WorkspaceRoot`` - Mapping of repository name to its configuration +``WorkspaceRoots`` - Mapping of workspace root path to ``WorkspaceRoot`` + +When the configuration is parsed we preserve the original key string, but +``WorkspaceRoot`` terminology is used consistently across the codebase. +""" from __future__ import annotations +import pathlib import typing as t -from typing_extensions import NotRequired, TypedDict +from typing_extensions import NotRequired, TypeAlias, TypedDict if t.TYPE_CHECKING: - import pathlib - from libvcs._internal.types import StrPath, VCSLiteral from libvcs.sync.git import GitSyncRemoteDict @@ -40,3 +66,11 @@ class ConfigDict(TypedDict): ConfigDir = dict[str, ConfigDict] Config = dict[str, ConfigDir] + +# --------------------------------------------------------------------------- +# Workspace root aliases +# --------------------------------------------------------------------------- + +WorkspaceRoot = ConfigDir + +WorkspaceRoots: TypeAlias = dict[pathlib.Path, WorkspaceRoot] diff --git a/tests/cli/test_import.py b/tests/cli/test_import.py index 2d9cac96..6cee6807 100644 --- a/tests/cli/test_import.py +++ b/tests/cli/test_import.py @@ -83,9 +83,9 @@ class ImportRepoFixture(t.NamedTuple): IMPORT_REPO_FIXTURES: list[ImportRepoFixture] = [ - # Simple repo import with default base dir + # Simple repo import with default workspace root ImportRepoFixture( - test_id="simple-repo-default-dir", + test_id="simple-repo-default-root", cli_args=["import", "myproject", "git@github.com:user/myproject.git"], initial_config=None, should_create_config=True, @@ -96,14 +96,14 @@ class ImportRepoFixture(t.NamedTuple): }, expected_in_output="Successfully imported 'myproject'", ), - # Import with custom base directory + # Import with custom workspace root ImportRepoFixture( - test_id="custom-base-dir", + test_id="custom-workspace-root", cli_args=[ "import", "mylib", "https://github.com/org/mylib", - "--dir", + "--workspace-root", "~/projects/libs", ], initial_config=None, @@ -115,14 +115,14 @@ class ImportRepoFixture(t.NamedTuple): }, expected_in_output="Successfully imported 'mylib'", ), - # Import to existing config + # Import to existing config under specific workspace root ImportRepoFixture( test_id="import-to-existing", cli_args=[ "import", "project2", "git@github.com:user/project2.git", - "--dir", + "--workspace-root", "~/work", ], initial_config={ @@ -145,7 +145,7 @@ class ImportRepoFixture(t.NamedTuple): "import", "existing", "git@github.com:other/existing.git", - "--dir", + "--workspace-root", "~/code", ], initial_config={ @@ -190,39 +190,39 @@ class ScanExistingFixture(t.NamedTuple): """Fixture for scan behaviour when configuration already includes a repo.""" test_id: str - config_key_style: str + workspace_root_key_style: str scan_arg_style: str SCAN_EXISTING_FIXTURES: list[ScanExistingFixture] = [ ScanExistingFixture( test_id="tilde-no-slash_scan-tilde", - config_key_style="tilde_no_slash", + workspace_root_key_style="tilde_no_slash", scan_arg_style="tilde", ), ScanExistingFixture( test_id="tilde-no-slash_scan-abs", - config_key_style="tilde_no_slash", + workspace_root_key_style="tilde_no_slash", scan_arg_style="absolute", ), ScanExistingFixture( test_id="tilde-with-slash_scan-tilde", - config_key_style="tilde_with_slash", + workspace_root_key_style="tilde_with_slash", scan_arg_style="tilde", ), ScanExistingFixture( test_id="tilde-with-slash_scan-abs", - config_key_style="tilde_with_slash", + workspace_root_key_style="tilde_with_slash", scan_arg_style="absolute", ), ScanExistingFixture( test_id="absolute-no-slash_scan-abs", - config_key_style="absolute_no_slash", + workspace_root_key_style="absolute_no_slash", scan_arg_style="absolute", ), ScanExistingFixture( test_id="absolute-with-slash_scan-abs", - config_key_style="absolute_with_slash", + workspace_root_key_style="absolute_with_slash", scan_arg_style="absolute", ), ] @@ -390,11 +390,18 @@ class ImportScanFixture(t.NamedTuple): "Successfully updated", ], ), - # Custom base directory key + # Custom workspace root override ImportScanFixture( - test_id="custom-base-dir-scan", + test_id="custom-workspace-root-scan", repo_setup=[("myrepo", "", True)], - cli_args=["import", "--scan", ".", "--base-dir-key", "~/custom/path", "-y"], + cli_args=[ + "import", + "--scan", + ".", + "--workspace-root", + "~/custom/path", + "-y", + ], initial_config=None, should_create_config=True, expected_config_contains={ @@ -662,7 +669,7 @@ def test_import_repo_direct_call( url="git@github.com:user/direct.git", config_file_path_str=str(config_file), path=None, - base_dir=None, + workspace_root_path=None, ) # Verify @@ -698,7 +705,7 @@ def test_import_repo_invalid_config( url="git@github.com:user/test.git", config_file_path_str=str(config_file), path=None, - base_dir=None, + workspace_root_path=None, ) assert "Error loading YAML" in caplog.text @@ -776,7 +783,7 @@ def test_import_scan_direct_call( scan_dir_str=str(scan_dir), config_file_path_str=str(config_file), recursive=False, - base_dir_key_arg=None, + workspace_root_override=None, yes=True, ) @@ -844,14 +851,14 @@ def test_many_existing_repos_summary( *fixture, marks=pytest.mark.xfail( reason=( - "Existing repos are re-added when config key format does not " - "match the scanner's trailing-slash expectations." + "Existing repos are re-added when workspace root formatting " + "does not match the scanner's trailing-slash expectations." ), strict=True, ), ) - if fixture.config_key_style.endswith("no_slash") - or fixture.config_key_style.startswith("absolute_") + if fixture.workspace_root_key_style.endswith("no_slash") + or fixture.workspace_root_key_style.startswith("absolute_") else fixture for fixture in SCAN_EXISTING_FIXTURES ], @@ -862,7 +869,7 @@ def test_scan_respects_existing_config_sections( tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch, test_id: str, - config_key_style: str, + workspace_root_key_style: str, scan_arg_style: str, ) -> None: """Ensure filesystem scan does not duplicate repositories in config.""" @@ -875,20 +882,22 @@ def test_scan_respects_existing_config_sections( expected_repo_url = "git+https://github.com/python/cpython.git" config_file = home_dir / ".vcspull.yaml" - if config_key_style == "tilde_no_slash": - config_key = "~/study/c" - elif config_key_style == "tilde_with_slash": - config_key = "~/study/c/" - elif config_key_style == "absolute_no_slash": - config_key = str(scan_dir) - elif config_key_style == "absolute_with_slash": - config_key = str(scan_dir) + "/" + if workspace_root_key_style == "tilde_no_slash": + workspace_root_key = "~/study/c" + elif workspace_root_key_style == "tilde_with_slash": + workspace_root_key = "~/study/c/" + elif workspace_root_key_style == "absolute_no_slash": + workspace_root_key = str(scan_dir) + elif workspace_root_key_style == "absolute_with_slash": + workspace_root_key = str(scan_dir) + "/" else: - error_msg = f"Unhandled config_key_style: {config_key_style}" + error_msg = ( + f"Unhandled workspace_root_key_style: {workspace_root_key_style}" + ) raise AssertionError(error_msg) config_file.write_text( - (f"{config_key}:\n cpython:\n repo: {expected_repo_url}\n"), + (f"{workspace_root_key}:\n cpython:\n repo: {expected_repo_url}\n"), encoding="utf-8", ) @@ -911,20 +920,22 @@ def test_scan_respects_existing_config_sections( scan_dir_str=scan_arg, config_file_path_str=None, recursive=False, - base_dir_key_arg=None, + workspace_root_override=None, yes=True, ) with config_file.open(encoding="utf-8") as f: config_data = yaml.safe_load(f) - assert config_key in config_data - assert "cpython" in config_data[config_key] - assert config_data[config_key]["cpython"]["repo"] == expected_repo_url + assert workspace_root_key in config_data + assert "cpython" in config_data[workspace_root_key] + assert config_data[workspace_root_key]["cpython"]["repo"] == expected_repo_url assert len(config_data) == 1 alternate_key = ( - config_key[:-1] if config_key.endswith("/") else f"{config_key}/" + workspace_root_key[:-1] + if workspace_root_key.endswith("/") + else f"{workspace_root_key}/" ) assert alternate_key not in config_data @@ -949,7 +960,7 @@ def test_import_command_help( assert "name" in output assert "url" in output assert "--path" in output - assert "--dir" in output + assert "--workspace-root" in output assert "--scan" in output assert "--config" in output From 2f64357dc0f0bd5010acb83e40153064189821e6 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 18 Oct 2025 14:04:37 -0500 Subject: [PATCH 4/7] tests/cli(fmt): capture workspace root normalization gap --- tests/cli/test_fmt.py | 727 +++++++++++++++++++++--------------------- 1 file changed, 356 insertions(+), 371 deletions(-) diff --git a/tests/cli/test_fmt.py b/tests/cli/test_fmt.py index 89932d70..1e92bf6c 100644 --- a/tests/cli/test_fmt.py +++ b/tests/cli/test_fmt.py @@ -3,6 +3,7 @@ from __future__ import annotations import logging +import pathlib import typing as t import pytest @@ -11,304 +12,296 @@ from vcspull.cli.fmt import format_config, format_config_file, normalize_repo_config if t.TYPE_CHECKING: - import pathlib - from _pytest.logging import LogCaptureFixture -class TestNormalizeRepoConfig: - """Test normalization of repository configurations.""" - - def test_compact_to_verbose(self) -> None: - """Test converting compact format to verbose format.""" - compact = "git+https://github.com/user/repo.git" - normalized = normalize_repo_config(compact) - assert normalized == {"repo": compact} - - def test_url_to_repo_key(self) -> None: - """Test converting url key to repo key.""" - config_with_url = {"url": "git+https://github.com/user/repo.git"} - normalized = normalize_repo_config(config_with_url) - assert normalized == {"repo": "git+https://github.com/user/repo.git"} - - def test_already_normalized(self) -> None: - """Test that already normalized configs are unchanged.""" - config = {"repo": "git+https://github.com/user/repo.git"} - normalized = normalize_repo_config(config) - assert normalized == config - - def test_preserve_extra_fields(self) -> None: - """Test that extra fields are preserved.""" - config = { - "url": "git+https://github.com/user/repo.git", - "remotes": {"upstream": "git+https://github.com/upstream/repo.git"}, - "shell_command_after": "ln -sf /foo /bar", - } - normalized = normalize_repo_config(config) - assert normalized == { - "repo": "git+https://github.com/user/repo.git", - "remotes": {"upstream": "git+https://github.com/upstream/repo.git"}, - "shell_command_after": "ln -sf /foo /bar", - } - - def test_both_url_and_repo(self) -> None: - """Test when both url and repo keys exist.""" - config = { - "url": "git+https://github.com/user/repo1.git", - "repo": "git+https://github.com/user/repo2.git", - } - normalized = normalize_repo_config(config) - # Should keep as-is when both exist - assert normalized == config - - -class TestFormatConfig: - """Test configuration formatting.""" - - def test_sort_directories(self) -> None: - """Test that directories are sorted alphabetically.""" - config = { - "~/zzz/": {"repo1": "url1"}, - "~/aaa/": {"repo2": "url2"}, - "~/mmm/": {"repo3": "url3"}, - } - formatted, changes = format_config(config) - assert list(formatted.keys()) == ["~/aaa/", "~/mmm/", "~/zzz/"] - assert changes > 0 - - def test_sort_repositories(self) -> None: - """Test that repositories within directories are sorted.""" - config = { - "~/projects/": { - "zebra": "url1", - "alpha": "url2", - "beta": "url3", - }, - } - formatted, changes = format_config(config) - assert list(formatted["~/projects/"].keys()) == ["alpha", "beta", "zebra"] - assert changes > 0 - - def test_compact_format_conversion(self) -> None: - """Test conversion of compact format to verbose.""" - config = { - "~/projects/": { - "repo1": "git+https://github.com/user/repo1.git", - "repo2": {"url": "git+https://github.com/user/repo2.git"}, - "repo3": {"repo": "git+https://github.com/user/repo3.git"}, - }, - } - formatted, changes = format_config(config) - - # repo1 should be converted from compact to verbose - assert formatted["~/projects/"]["repo1"] == { - "repo": "git+https://github.com/user/repo1.git", - } - # repo2 should have url converted to repo - assert formatted["~/projects/"]["repo2"] == { - "repo": "git+https://github.com/user/repo2.git", - } - # repo3 should stay the same - assert formatted["~/projects/"]["repo3"] == { - "repo": "git+https://github.com/user/repo3.git", - } - assert changes == 2 # repo1 and repo2 changed - - def test_no_changes_needed(self) -> None: - """Test when no formatting changes are needed.""" - config = { - "~/aaa/": { - "alpha": {"repo": "url1"}, - "beta": {"repo": "url2"}, - }, - "~/bbb/": { - "charlie": {"repo": "url3"}, - }, - } - formatted, changes = format_config(config) - assert formatted == config - assert changes == 0 - - def test_complex_formatting(self) -> None: - """Test complex formatting with multiple changes.""" - config = { - "~/zzz/": { - "zebra": "compact-url", - "alpha": {"url": "verbose-url"}, - "beta": { - "repo": "already-good", - "remotes": {"upstream": "upstream-url"}, - }, - }, - "~/aaa/": { - "repo1": "another-compact", +class WorkspaceRootFixture(t.NamedTuple): + """Fixture for workspace root normalization cases.""" + + test_id: str + config: dict[str, t.Any] + expected_roots: list[str] + + +WORKSPACE_ROOT_FIXTURES: list[WorkspaceRootFixture] = [ + WorkspaceRootFixture( + test_id="tilde-mixed-trailing-slash", + config={ + "~/study/c": { + "cpython": {"repo": "git+https://github.com/python/cpython.git"}, }, - } - formatted, changes = format_config(config) - - # Check directory sorting - assert list(formatted.keys()) == ["~/aaa/", "~/zzz/"] - - # Check repository sorting in ~/zzz/ - assert list(formatted["~/zzz/"].keys()) == ["alpha", "beta", "zebra"] - - # Check format conversions - assert formatted["~/aaa/"]["repo1"] == {"repo": "another-compact"} - assert formatted["~/zzz/"]["zebra"] == {"repo": "compact-url"} - assert formatted["~/zzz/"]["alpha"] == {"repo": "verbose-url"} - assert formatted["~/zzz/"]["beta"]["repo"] == "already-good" - assert formatted["~/zzz/"]["beta"]["remotes"] == {"upstream": "upstream-url"} - - # Should have multiple changes - assert changes > 0 - - -class TestFormatConfigFile: - """Test the format_config_file function.""" - - def test_format_file_no_write( - self, - tmp_path: pathlib.Path, - caplog: LogCaptureFixture, - ) -> None: - """Test formatting without writing changes.""" - config_file = tmp_path / ".vcspull.yaml" - original_config = { - "~/zzz/": { - "repo2": "url2", - "repo1": {"url": "url1"}, + "~/study/c/": { + "tmux": {"repo": "git+https://github.com/tmux/tmux.git"}, }, - "~/aaa/": { - "repo3": "url3", + }, + expected_roots=["~/study/c/"], + ), + WorkspaceRootFixture( + test_id="home-vs-absolute", + config={ + str(pathlib.Path.home() / "study" / "c"): { + "cpython": {"repo": "git+https://github.com/python/cpython.git"}, }, - } - - with config_file.open("w", encoding="utf-8") as f: - yaml.dump(original_config, f) - - with caplog.at_level(logging.INFO): - format_config_file(str(config_file), write=False, format_all=False) - - # Check that file was not modified - with config_file.open(encoding="utf-8") as f: - saved_config = yaml.safe_load(f) - assert saved_config == original_config - - # Check log messages - assert "formatting issue" in caplog.text - assert "Run with --write to apply" in caplog.text - - def test_format_file_with_write( - self, - tmp_path: pathlib.Path, - caplog: LogCaptureFixture, - ) -> None: - """Test formatting with writing changes.""" - config_file = tmp_path / ".vcspull.yaml" - original_config = { - "~/zzz/": { - "repo2": "url2", - "repo1": {"url": "url1"}, + "~/study/c/": { + "tmux": {"repo": "git+https://github.com/tmux/tmux.git"}, }, - "~/aaa/": { - "repo3": "url3", + }, + expected_roots=["~/study/c/"], + ), + WorkspaceRootFixture( + test_id="relative-vs-tilde", + config={ + "./study/c": { + "cpython": {"repo": "git+https://github.com/python/cpython.git"}, }, - } - - with config_file.open("w", encoding="utf-8") as f: - yaml.dump(original_config, f) - - with caplog.at_level(logging.INFO): - format_config_file(str(config_file), write=True, format_all=False) - - # Check that file was modified - with config_file.open(encoding="utf-8") as f: - saved_config = yaml.safe_load(f) - - # Check formatting was applied - assert list(saved_config.keys()) == ["~/aaa/", "~/zzz/"] - assert saved_config["~/aaa/"]["repo3"] == {"repo": "url3"} - assert saved_config["~/zzz/"]["repo1"] == {"repo": "url1"} - assert saved_config["~/zzz/"]["repo2"] == {"repo": "url2"} - assert list(saved_config["~/zzz/"].keys()) == ["repo1", "repo2"] - - # Check log messages - assert "Successfully formatted" in caplog.text - - def test_already_formatted( - self, - tmp_path: pathlib.Path, - caplog: LogCaptureFixture, - ) -> None: - """Test when file is already correctly formatted.""" - config_file = tmp_path / ".vcspull.yaml" - config = { - "~/aaa/": { - "repo1": {"repo": "url1"}, + "~/study/c/": { + "tmux": {"repo": "git+https://github.com/tmux/tmux.git"}, }, - "~/bbb/": { - "repo2": {"repo": "url2"}, + }, + expected_roots=["~/study/c/"], + ), +] + + +@pytest.mark.parametrize( + list(WorkspaceRootFixture._fields), + [ + pytest.param( + *fixture, + marks=pytest.mark.xfail( + reason="Workspace root normalization not yet implemented in formatter.", + strict=True, + ), + ) + for fixture in WORKSPACE_ROOT_FIXTURES + ], + ids=[fixture.test_id for fixture in WORKSPACE_ROOT_FIXTURES], +) +def test_workspace_root_normalization( + test_id: str, + config: dict[str, t.Any], + expected_roots: list[str], +) -> None: + """Ensure format_config merges duplicate workspace roots.""" + formatted, _changes = format_config(config) + assert list(formatted.keys()) == expected_roots + + +def test_normalize_repo_config_compact_to_verbose() -> None: + """Compact repository config should expand to verbose form.""" + compact = "git+https://github.com/user/repo.git" + normalized = normalize_repo_config(compact) + assert normalized == {"repo": compact} + + +def test_normalize_repo_config_url_to_repo() -> None: + """Entries using url key should be converted to repo.""" + config_with_url = {"url": "git+https://github.com/user/repo.git"} + normalized = normalize_repo_config(config_with_url) + assert normalized == {"repo": "git+https://github.com/user/repo.git"} + + +def test_normalize_repo_config_already_verbose() -> None: + """Verbose configs should remain unchanged.""" + config = {"repo": "git+https://github.com/user/repo.git"} + normalized = normalize_repo_config(config) + assert normalized == config + + +def test_normalize_repo_config_preserves_extras() -> None: + """Extra fields should be preserved during normalization.""" + config = { + "url": "git+https://github.com/user/repo.git", + "remotes": {"upstream": "git+https://github.com/upstream/repo.git"}, + "shell_command_after": "ln -sf /foo /bar", + } + normalized = normalize_repo_config(config) + assert normalized == { + "repo": "git+https://github.com/user/repo.git", + "remotes": {"upstream": "git+https://github.com/upstream/repo.git"}, + "shell_command_after": "ln -sf /foo /bar", + } + + +def test_normalize_repo_config_both_url_and_repo() -> None: + """When url and repo keys coexist, keep config unchanged.""" + config = { + "url": "git+https://github.com/user/repo1.git", + "repo": "git+https://github.com/user/repo2.git", + } + normalized = normalize_repo_config(config) + assert normalized == config + + +def test_format_config_sorts_directories() -> None: + """Workspace roots should be sorted alphabetically.""" + config = { + "~/zzz/": {"repo1": "url1"}, + "~/aaa/": {"repo2": "url2"}, + "~/mmm/": {"repo3": "url3"}, + } + formatted, changes = format_config(config) + assert list(formatted.keys()) == ["~/aaa/", "~/mmm/", "~/zzz/"] + assert changes > 0 + + +def test_format_config_sorts_repositories() -> None: + """Repositories within a workspace root should be sorted.""" + config = { + "~/projects/": { + "zebra": "url1", + "alpha": "url2", + "beta": "url3", + }, + } + formatted, changes = format_config(config) + assert list(formatted["~/projects/"].keys()) == ["alpha", "beta", "zebra"] + assert changes > 0 + + +def test_format_config_converts_compact_entries() -> None: + """Compact repository entries should convert to verbose form.""" + config = { + "~/projects/": { + "repo1": "git+https://github.com/user/repo1.git", + "repo2": {"url": "git+https://github.com/user/repo2.git"}, + "repo3": {"repo": "git+https://github.com/user/repo3.git"}, + }, + } + formatted, changes = format_config(config) + assert formatted["~/projects/"]["repo1"] == { + "repo": "git+https://github.com/user/repo1.git", + } + assert formatted["~/projects/"]["repo2"] == { + "repo": "git+https://github.com/user/repo2.git", + } + assert formatted["~/projects/"]["repo3"] == { + "repo": "git+https://github.com/user/repo3.git", + } + assert changes == 2 + + +def test_format_config_no_changes_when_already_normalized() -> None: + """Formatter should report zero changes for already-normalized configs.""" + config = { + "~/aaa/": { + "alpha": {"repo": "url1"}, + "beta": {"repo": "url2"}, + }, + "~/bbb/": { + "charlie": {"repo": "url3"}, + }, + } + formatted, changes = format_config(config) + assert formatted == config + assert changes == 0 + + +def test_format_config_complex_changes() -> None: + """Formatter should handle sorting and conversions together.""" + config = { + "~/zzz/": { + "zebra": "compact-url", + "alpha": {"url": "verbose-url"}, + "beta": { + "repo": "already-good", + "remotes": {"upstream": "upstream-url"}, }, - } - - with config_file.open("w", encoding="utf-8") as f: - yaml.dump(config, f) - - with caplog.at_level(logging.INFO): - format_config_file(str(config_file), write=False, format_all=False) - - assert "already formatted correctly" in caplog.text - - def test_nonexistent_file( - self, - tmp_path: pathlib.Path, - caplog: LogCaptureFixture, - ) -> None: - """Test handling of nonexistent config file.""" - config_file = tmp_path / "nonexistent.yaml" - - with caplog.at_level(logging.ERROR): - format_config_file(str(config_file), write=False) - - assert "not found" in caplog.text - - def test_invalid_yaml( - self, - tmp_path: pathlib.Path, - caplog: LogCaptureFixture, - ) -> None: - """Test handling of invalid YAML.""" - config_file = tmp_path / ".vcspull.yaml" - config_file.write_text("invalid: yaml: content:", encoding="utf-8") - - with caplog.at_level(logging.ERROR): - format_config_file(str(config_file), write=False) - - assert "Error loading config" in caplog.text - - def test_no_config_found( - self, - tmp_path: pathlib.Path, - caplog: LogCaptureFixture, - monkeypatch: pytest.MonkeyPatch, - ) -> None: - """Test when no config file is found.""" - monkeypatch.chdir(tmp_path) - - with caplog.at_level(logging.ERROR): - format_config_file(None, write=False, format_all=False) - - assert "No configuration file found" in caplog.text - - def test_detailed_change_reporting( - self, - tmp_path: pathlib.Path, - caplog: LogCaptureFixture, - ) -> None: - """Test detailed reporting of changes.""" - config_file = tmp_path / ".vcspull.yaml" - # Write YAML manually to preserve key order - yaml_content = """~/zzz/: + }, + "~/aaa/": { + "repo1": "another-compact", + }, + } + formatted, changes = format_config(config) + assert list(formatted.keys()) == ["~/aaa/", "~/zzz/"] + assert list(formatted["~/zzz/"].keys()) == ["alpha", "beta", "zebra"] + assert formatted["~/aaa/"]["repo1"] == {"repo": "another-compact"} + assert formatted["~/zzz/"]["zebra"] == {"repo": "compact-url"} + assert formatted["~/zzz/"]["alpha"] == {"repo": "verbose-url"} + assert formatted["~/zzz/"]["beta"]["repo"] == "already-good" + assert formatted["~/zzz/"]["beta"]["remotes"] == {"upstream": "upstream-url"} + assert changes > 0 + + +def test_format_config_file_without_write( + tmp_path: pathlib.Path, + caplog: LogCaptureFixture, +) -> None: + """format_config_file should log issues without modifying when write=False.""" + config_file = tmp_path / ".vcspull.yaml" + original_config = { + "~/zzz/": { + "repo2": "url2", + "repo1": {"url": "url1"}, + }, + "~/aaa/": { + "repo3": "url3", + }, + } + + config_file.write_text(yaml.dump(original_config), encoding="utf-8") + + with caplog.at_level(logging.INFO): + format_config_file(str(config_file), write=False, format_all=False) + + saved_config = yaml.safe_load(config_file.read_text(encoding="utf-8")) + assert saved_config == original_config + assert "formatting issue" in caplog.text + assert "Run with --write to apply" in caplog.text + + +def test_format_config_file_with_write(tmp_path: pathlib.Path) -> None: + """format_config_file should rewrite file when write=True.""" + config_file = tmp_path / ".vcspull.yaml" + original_config = { + "~/zzz/": { + "repo2": "url2", + "repo1": {"url": "url1"}, + }, + } + config_file.write_text(yaml.dump(original_config), encoding="utf-8") + + format_config_file(str(config_file), write=True, format_all=False) + + saved_config = yaml.safe_load(config_file.read_text(encoding="utf-8")) + assert saved_config["~/zzz/"]["repo1"] == {"repo": "url1"} + + +def test_format_config_file_invalid_yaml( + tmp_path: pathlib.Path, + caplog: LogCaptureFixture, +) -> None: + """Invalid YAML should be reported without crashing.""" + config_file = tmp_path / ".vcspull.yaml" + config_file.write_text("invalid: yaml: content:", encoding="utf-8") + + with caplog.at_level(logging.ERROR): + format_config_file(str(config_file), write=False, format_all=False) + + assert "Error loading config" in caplog.text + + +def test_format_config_file_missing_config( + tmp_path: pathlib.Path, + caplog: LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Formatting without available config should emit an error.""" + monkeypatch.chdir(tmp_path) + + with caplog.at_level(logging.ERROR): + format_config_file(None, write=False, format_all=False) + + assert "No configuration file found" in caplog.text + + +def test_format_config_file_reports_changes( + tmp_path: pathlib.Path, + caplog: LogCaptureFixture, +) -> None: + """Detailed change summary should be logged for pending updates.""" + config_file = tmp_path / ".vcspull.yaml" + yaml_content = """~/zzz/: compact1: url1 compact2: url2 verbose1: @@ -318,88 +311,80 @@ def test_detailed_change_reporting( ~/aaa/: repo: url5 """ - config_file.write_text(yaml_content, encoding="utf-8") - - with caplog.at_level(logging.INFO): - format_config_file(str(config_file), write=False, format_all=False) - - # Check detailed change reporting - assert "3 repositories from compact to verbose format" in caplog.text - assert "2 repositories from 'url' to 'repo' key" in caplog.text - assert "Directories will be sorted alphabetically" in caplog.text - - def test_format_all_configs( - self, - tmp_path: pathlib.Path, - caplog: LogCaptureFixture, - monkeypatch: pytest.MonkeyPatch, - ) -> None: - """Test formatting all discovered config files.""" - # Create test config directory structure - config_dir = tmp_path / ".config" / "vcspull" - config_dir.mkdir(parents=True) - - # Create home config (already formatted correctly) - home_config = tmp_path / ".vcspull.yaml" - home_config.write_text( - yaml.dump({"~/projects/": {"repo1": {"repo": "url1"}}}), - encoding="utf-8", - ) - - # Create config in config directory (needs sorting) - config1 = config_dir / "work.yaml" - config1_content = """~/work/: + config_file.write_text(yaml_content, encoding="utf-8") + + with caplog.at_level(logging.INFO): + format_config_file(str(config_file), write=False, format_all=False) + + text = caplog.text + assert "compact to verbose format" in text + assert "from 'url' to 'repo' key" in text + assert "Directories will be sorted alphabetically" in text + assert "Run with --write to apply" in text + + +def test_format_all_configs( + tmp_path: pathlib.Path, + caplog: LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """format_config_file with --all should process discovered configs.""" + config_dir = tmp_path / ".config" / "vcspull" + config_dir.mkdir(parents=True) + + home_config = tmp_path / ".vcspull.yaml" + home_config.write_text( + yaml.dump({"~/projects/": {"repo1": {"repo": "url1"}}}), + encoding="utf-8", + ) + + work_config = config_dir / "work.yaml" + work_config.write_text( + """~/work/: repo2: url2 repo1: url1 -""" - config1.write_text(config1_content, encoding="utf-8") - - # Create local config - local_config = tmp_path / "project" / ".vcspull.yaml" - local_config.parent.mkdir() - local_config.write_text( - yaml.dump({"./": {"repo3": {"url": "url3"}}}), - encoding="utf-8", - ) - - # Mock find functions to return our test configs - def mock_find_config_files(include_home: bool = False) -> list[pathlib.Path]: - files = [config1] - if include_home: - files.insert(0, home_config) - return files - - def mock_find_home_config_files( - filetype: list[str] | None = None, - ) -> list[pathlib.Path]: - return [home_config] - - # Change to project directory - monkeypatch.chdir(local_config.parent) - monkeypatch.setattr( - "vcspull.cli.fmt.find_config_files", - mock_find_config_files, - ) - monkeypatch.setattr( - "vcspull.cli.fmt.find_home_config_files", - mock_find_home_config_files, - ) - - with caplog.at_level(logging.INFO): - format_config_file(None, write=False, format_all=True) - - # Check that all configs were found - assert "Found 3 configuration files to format" in caplog.text - assert str(home_config) in caplog.text - assert str(config1) in caplog.text - assert str(local_config) in caplog.text - - # Check processing messages - assert "already formatted correctly" in caplog.text # home_config - assert ( - "3 formatting issues" in caplog.text - ) # config1 has 2 compact + needs sorting - assert "2 repositories from compact to verbose format" in caplog.text # config1 - assert "Repositories in ~/work/ will be sorted" in caplog.text # config1 - assert "1 repository from 'url' to 'repo' key" in caplog.text # local_config - assert "All 3 configuration files processed successfully" in caplog.text +""", + encoding="utf-8", + ) + + local_root = tmp_path / "project" + local_root.mkdir() + local_config = local_root / ".vcspull.yaml" + local_config.write_text( + yaml.dump({"./": {"repo3": {"url": "url3"}}}), + encoding="utf-8", + ) + + monkeypatch.chdir(local_root) + + def fake_find_config_files(include_home: bool = False) -> list[pathlib.Path]: + files: list[pathlib.Path] = [work_config] + if include_home: + files.insert(0, home_config) + return files + + def fake_find_home_config_files( + filetype: list[str] | None = None, + ) -> list[pathlib.Path]: + return [home_config] + + monkeypatch.setattr( + "vcspull.cli.fmt.find_config_files", + fake_find_config_files, + ) + monkeypatch.setattr( + "vcspull.cli.fmt.find_home_config_files", + fake_find_home_config_files, + ) + + with caplog.at_level(logging.INFO): + format_config_file(None, write=False, format_all=True) + + text = caplog.text + assert "Found 3 configuration files to format" in text + assert str(home_config) in text + assert str(work_config) in text + assert str(local_config) in text + assert "already formatted correctly" in text + assert "Repositories in ~/work/ will be sorted alphabetically" in text + assert "All 3 configuration files processed successfully" in text From 9b8d84cee1d8d59356ab4733881d8b35ca1b8cf1 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 18 Oct 2025 14:54:25 -0500 Subject: [PATCH 5/7] cli: normalize workspace roots --- src/vcspull/cli/_import.py | 240 +++++++++++++++++++++++-------------- src/vcspull/cli/fmt.py | 28 ++++- src/vcspull/config.py | 101 ++++++++++++++++ 3 files changed, 280 insertions(+), 89 deletions(-) diff --git a/src/vcspull/cli/_import.py b/src/vcspull/cli/_import.py index 08e47714..bdbbf164 100644 --- a/src/vcspull/cli/_import.py +++ b/src/vcspull/cli/_import.py @@ -13,7 +13,14 @@ from colorama import Fore, Style from vcspull._internal.config_reader import ConfigReader -from vcspull.config import expand_dir, find_home_config_files, save_config_yaml +from vcspull.config import ( + canonicalize_workspace_path, + expand_dir, + find_home_config_files, + normalize_workspace_roots, + save_config_yaml, + workspace_root_label, +) if t.TYPE_CHECKING: import argparse @@ -109,6 +116,19 @@ def create_import_subparser(parser: argparse.ArgumentParser) -> None: ) +def _resolve_workspace_path( + workspace_root: str | None, + repo_path_str: str | None, + *, + cwd: pathlib.Path, +) -> pathlib.Path: + if workspace_root: + return canonicalize_workspace_path(workspace_root, cwd=cwd) + if repo_path_str: + return expand_dir(pathlib.Path(repo_path_str), cwd) + return cwd + + def import_repo( name: str, url: str, @@ -178,41 +198,47 @@ def import_repo( config_file_path, ) - # Determine workspace root key - if workspace_root_path: - workspace_root_key = ( - workspace_root_path - if workspace_root_path.endswith("/") - else workspace_root_path + "/" + cwd = pathlib.Path.cwd() + home = pathlib.Path.home() + + normalization_result = normalize_workspace_roots( + raw_config, + cwd=cwd, + home=home, + ) + raw_config, workspace_map, merge_conflicts, _merge_changes = normalization_result + config_was_normalized = _merge_changes > 0 + + for message in merge_conflicts: + log.warning(message) + + workspace_path = _resolve_workspace_path( + workspace_root_path, + path, + cwd=cwd, + ) + workspace_label = workspace_map.get(workspace_path) + if workspace_label is None: + workspace_label = workspace_root_label( + workspace_path, + cwd=cwd, + home=home, ) - elif path: - # Infer from provided path - repo_path = pathlib.Path(path).expanduser().resolve() - try: - # Try to make it relative to home - workspace_root_key = ( - "~/" + str(repo_path.relative_to(pathlib.Path.home())) + "/" - ) - except ValueError: - # Use absolute path - workspace_root_key = str(repo_path) + "/" - else: - # Default to current directory - workspace_root_key = "./" + workspace_map[workspace_path] = workspace_label + raw_config.setdefault(workspace_label, {}) - # Ensure workspace root key exists in config - if workspace_root_key not in raw_config: - raw_config[workspace_root_key] = {} - elif not isinstance(raw_config[workspace_root_key], dict): + if workspace_label not in raw_config: + raw_config[workspace_label] = {} + elif not isinstance(raw_config[workspace_label], dict): log.error( "Workspace root '%s' in configuration is not a dictionary. Aborting.", - workspace_root_key, + workspace_label, ) return # Check if repo already exists - if name in raw_config[workspace_root_key]: - existing_config = raw_config[workspace_root_key][name] + if name in raw_config[workspace_label]: + existing_config = raw_config[workspace_label][name] # Handle both string and dict formats current_url: str if isinstance(existing_config, str): @@ -228,13 +254,28 @@ def import_repo( "Repository '%s' already exists under '%s'. Current URL: %s. " "To update, remove and re-add, or edit the YAML file manually.", name, - workspace_root_key, + workspace_label, current_url, ) + if config_was_normalized: + try: + save_config_yaml(config_file_path, raw_config) + log.info( + "%s✓%s Normalized workspace roots saved to %s%s%s.", + Fore.GREEN, + Style.RESET_ALL, + Fore.BLUE, + config_file_path, + Style.RESET_ALL, + ) + except Exception: + log.exception("Error saving config to %s", config_file_path) + if log.isEnabledFor(logging.DEBUG): + traceback.print_exc() return # Add the repository in verbose format - raw_config[workspace_root_key][name] = {"repo": url} + raw_config[workspace_label][name] = {"repo": url} # Save config try: @@ -253,7 +294,7 @@ def import_repo( config_file_path, Style.RESET_ALL, Fore.MAGENTA, - workspace_root_key, + workspace_label, Style.RESET_ALL, ) except Exception: @@ -341,8 +382,28 @@ def import_from_filesystem( Style.RESET_ALL, ) - # Each entry stores (repo_name, repo_url, workspace_root_key) - found_repos: list[tuple[str, str, str]] = [] + cwd = pathlib.Path.cwd() + home = pathlib.Path.home() + + normalization_result = normalize_workspace_roots( + raw_config, + cwd=cwd, + home=home, + ) + raw_config, workspace_map, merge_conflicts, _merge_changes = normalization_result + + for message in merge_conflicts: + log.warning(message) + + found_repos: list[tuple[str, str, pathlib.Path]] = [] + + override_workspace_path: pathlib.Path | None = None + if workspace_root_override: + override_workspace_path = _resolve_workspace_path( + workspace_root_override, + None, + cwd=cwd, + ) if recursive: for root, dirs, _ in os.walk(scan_dir): @@ -359,27 +420,9 @@ def import_from_filesystem( ) continue - workspace_root_key: str - if workspace_root_override: - workspace_root_key = ( - workspace_root_override - if workspace_root_override.endswith("/") - else workspace_root_override + "/" - ) - else: - try: - workspace_root_key = ( - "~/" + str(scan_dir.relative_to(pathlib.Path.home())) + "/" - ) - except ValueError: - workspace_root_key = str(scan_dir.resolve()) + "/" - - if not workspace_root_key.endswith("/"): - workspace_root_key += "/" - - found_repos.append((repo_name, repo_url, workspace_root_key)) + workspace_path = override_workspace_path or scan_dir + found_repos.append((repo_name, repo_url, workspace_path)) else: - # Non-recursive: only check immediate subdirectories for item in scan_dir.iterdir(): if item.is_dir() and (item / ".git").is_dir(): repo_name = item.name @@ -393,24 +436,8 @@ def import_from_filesystem( ) continue - if workspace_root_override: - workspace_root_key = ( - workspace_root_override - if workspace_root_override.endswith("/") - else workspace_root_override + "/" - ) - else: - try: - workspace_root_key = ( - "~/" + str(scan_dir.relative_to(pathlib.Path.home())) + "/" - ) - except ValueError: - workspace_root_key = str(scan_dir.resolve()) + "/" - - if not workspace_root_key.endswith("/"): - workspace_root_key += "/" - - found_repos.append((repo_name, repo_url, workspace_root_key)) + workspace_path = override_workspace_path or scan_dir + found_repos.append((repo_name, repo_url, workspace_path)) if not found_repos: log.info( @@ -423,15 +450,21 @@ def import_from_filesystem( ) return - repos_to_add: list[tuple[str, str, str]] = [] - existing_repos: list[tuple[str, str, str]] = [] # (name, url, workspace_root_key) + repos_to_add: list[tuple[str, str, pathlib.Path]] = [] + existing_repos: list[tuple[str, str, pathlib.Path]] = [] - for name, url, workspace_root_key in found_repos: - target_section = raw_config.get(workspace_root_key, {}) + for name, url, workspace_path in found_repos: + workspace_label = workspace_map.get(workspace_path) + if workspace_label is None: + workspace_label = workspace_root_label(workspace_path, cwd=cwd, home=home) + workspace_map[workspace_path] = workspace_label + raw_config.setdefault(workspace_label, {}) + + target_section = raw_config.get(workspace_label, {}) if isinstance(target_section, dict) and name in target_section: - existing_repos.append((name, url, workspace_root_key)) + existing_repos.append((name, url, workspace_path)) else: - repos_to_add.append((name, url, workspace_root_key)) + repos_to_add.append((name, url, workspace_path)) if existing_repos: # Show summary only when there are many existing repos @@ -454,7 +487,16 @@ def import_from_filesystem( len(existing_repos), Style.RESET_ALL, ) - for name, url, workspace_root_key in existing_repos: + for name, url, workspace_path in existing_repos: + workspace_label = workspace_map.get(workspace_path) + if workspace_label is None: + workspace_label = workspace_root_label( + workspace_path, + cwd=cwd, + home=home, + ) + workspace_map[workspace_path] = workspace_label + raw_config.setdefault(workspace_label, {}) log.info( " %s•%s %s%s%s (%s%s%s) at %s%s%s%s in %s%s%s", Fore.BLUE, @@ -466,7 +508,7 @@ def import_from_filesystem( url, Style.RESET_ALL, Fore.MAGENTA, - workspace_root_key, + workspace_label, name, Style.RESET_ALL, Fore.BLUE, @@ -474,6 +516,8 @@ def import_from_filesystem( Style.RESET_ALL, ) + changes_made = _merge_changes > 0 + if not repos_to_add: if existing_repos: log.info( @@ -484,6 +528,22 @@ def import_from_filesystem( Fore.GREEN, Style.RESET_ALL, ) + if changes_made: + try: + save_config_yaml(config_file_path, raw_config) + log.info( + "%s✓%s Successfully updated %s%s%s.", + Fore.GREEN, + Style.RESET_ALL, + Fore.BLUE, + config_file_path, + Style.RESET_ALL, + ) + except Exception: + log.exception("Error saving config to %s", config_file_path) + if log.isEnabledFor(logging.DEBUG): + traceback.print_exc() + return return # Show what will be added @@ -515,20 +575,24 @@ def import_from_filesystem( log.info("%s✗%s Aborted by user.", Fore.RED, Style.RESET_ALL) return - changes_made = False - for repo_name, repo_url, workspace_root_key in repos_to_add: - if workspace_root_key not in raw_config: - raw_config[workspace_root_key] = {} - elif not isinstance(raw_config[workspace_root_key], dict): + for repo_name, repo_url, workspace_path in repos_to_add: + workspace_label = workspace_map.get(workspace_path) + if workspace_label is None: + workspace_label = workspace_root_label(workspace_path, cwd=cwd, home=home) + workspace_map[workspace_path] = workspace_label + + if workspace_label not in raw_config: + raw_config[workspace_label] = {} + elif not isinstance(raw_config[workspace_label], dict): log.warning( "Workspace root '%s' in config is not a dictionary. Skipping repo %s.", - workspace_root_key, + workspace_label, repo_name, ) continue - if repo_name not in raw_config[workspace_root_key]: - raw_config[workspace_root_key][repo_name] = {"repo": repo_url} + if repo_name not in raw_config[workspace_label]: + raw_config[workspace_label][repo_name] = {"repo": repo_url} log.info( "%s+%s Importing %s'%s'%s (%s%s%s) under '%s%s%s'.", Fore.GREEN, @@ -540,7 +604,7 @@ def import_from_filesystem( repo_url, Style.RESET_ALL, Fore.MAGENTA, - workspace_root_key, + workspace_label, Style.RESET_ALL, ) changes_made = True diff --git a/src/vcspull/cli/fmt.py b/src/vcspull/cli/fmt.py index c5f2a457..0be52b5c 100644 --- a/src/vcspull/cli/fmt.py +++ b/src/vcspull/cli/fmt.py @@ -10,7 +10,12 @@ from colorama import Fore, Style from vcspull._internal.config_reader import ConfigReader -from vcspull.config import find_config_files, find_home_config_files, save_config_yaml +from vcspull.config import ( + find_config_files, + find_home_config_files, + normalize_workspace_roots, + save_config_yaml, +) if t.TYPE_CHECKING: import argparse @@ -168,7 +173,21 @@ def format_single_config( return False # Format the configuration + cwd = pathlib.Path.cwd() + home = pathlib.Path.home() + + normalization_result = normalize_workspace_roots( + raw_config, + cwd=cwd, + home=home, + ) + raw_config, _workspace_map, merge_conflicts, merge_changes = normalization_result + + for message in merge_conflicts: + log.warning(message) + formatted_config, change_count = format_config(raw_config) + change_count += merge_changes if change_count == 0: log.info( @@ -196,6 +215,13 @@ def format_single_config( ) # Analyze and report specific changes + if merge_changes > 0: + log.info( + " %s•%s Normalized workspace root labels", + Fore.BLUE, + Style.RESET_ALL, + ) + compact_to_verbose = 0 url_to_repo = 0 diff --git a/src/vcspull/config.py b/src/vcspull/config.py index 4edf198f..1b74a3e3 100644 --- a/src/vcspull/config.py +++ b/src/vcspull/config.py @@ -2,6 +2,7 @@ from __future__ import annotations +import copy import fnmatch import logging import os @@ -442,3 +443,103 @@ def save_config_yaml(config_file_path: pathlib.Path, data: dict[t.Any, t.Any]) - indent=2, ) config_file_path.write_text(yaml_content, encoding="utf-8") + + +def canonicalize_workspace_path( + label: str, + *, + cwd: pathlib.Path | None = None, +) -> pathlib.Path: + """Convert a workspace root label to an absolute canonical path.""" + cwd = cwd or pathlib.Path.cwd() + label_path = pathlib.Path(label) + return expand_dir(label_path, cwd=cwd) + + +def workspace_root_label( + workspace_path: pathlib.Path, + *, + cwd: pathlib.Path | None = None, + home: pathlib.Path | None = None, +) -> str: + """Create a normalized label for a workspace root path.""" + cwd = cwd or pathlib.Path.cwd() + home = home or pathlib.Path.home() + + if workspace_path == cwd: + return "./" + + try: + relative_to_home = workspace_path.relative_to(home) + label = f"~/{relative_to_home.as_posix()}" + except ValueError: + label = workspace_path.as_posix() + + if label != "./" and not label.endswith("/"): + label += "/" + + return label + + +def normalize_workspace_roots( + config_data: dict[str, t.Any], + *, + cwd: pathlib.Path | None = None, + home: pathlib.Path | None = None, +) -> tuple[dict[str, t.Any], dict[pathlib.Path, str], list[str], int]: + """Normalize workspace root labels and merge duplicate sections.""" + cwd = cwd or pathlib.Path.cwd() + home = home or pathlib.Path.home() + + normalized: dict[str, t.Any] = {} + path_to_label: dict[pathlib.Path, str] = {} + conflicts: list[str] = [] + change_count = 0 + + for label, value in config_data.items(): + canonical_path = canonicalize_workspace_path(label, cwd=cwd) + normalized_label = path_to_label.get(canonical_path) + + if normalized_label is None: + normalized_label = workspace_root_label( + canonical_path, + cwd=cwd, + home=home, + ) + path_to_label[canonical_path] = normalized_label + + if isinstance(value, dict): + normalized[normalized_label] = copy.deepcopy(value) + else: + normalized[normalized_label] = value + + if normalized_label != label: + change_count += 1 + else: + change_count += 1 + existing_value = normalized.get(normalized_label) + + if isinstance(existing_value, dict) and isinstance(value, dict): + for repo_name, repo_config in value.items(): + if repo_name not in existing_value: + existing_value[repo_name] = copy.deepcopy(repo_config) + change_count += 1 + elif existing_value[repo_name] != repo_config: + conflict_message = ( + "Workspace root '{label}' contains conflicting definitions " + "for repository '{repo}'. Keeping the existing entry." + ) + conflicts.append( + conflict_message.format( + label=normalized_label, + repo=repo_name, + ) + ) + elif existing_value != value: + conflict_message = ( + "Workspace root '{label}' contains conflicting non-dictionary " + "values. Keeping the existing entry." + ) + conflicts.append(conflict_message.format(label=normalized_label)) + + return normalized, path_to_label, conflicts, change_count From 66b7bc993896af753418f5b05b2cd3756a424cc1 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 18 Oct 2025 14:56:51 -0500 Subject: [PATCH 6/7] tests: expect normalized workspace roots --- tests/cli/test_fmt.py | 54 ++++++++++++++++++++++++++-------------- tests/cli/test_import.py | 40 ++++++++++++++--------------- 2 files changed, 53 insertions(+), 41 deletions(-) diff --git a/tests/cli/test_fmt.py b/tests/cli/test_fmt.py index 1e92bf6c..7078adb7 100644 --- a/tests/cli/test_fmt.py +++ b/tests/cli/test_fmt.py @@ -10,6 +10,11 @@ import yaml from vcspull.cli.fmt import format_config, format_config_file, normalize_repo_config +from vcspull.config import ( + canonicalize_workspace_path, + normalize_workspace_roots, + workspace_root_label, +) if t.TYPE_CHECKING: from _pytest.logging import LogCaptureFixture @@ -19,14 +24,13 @@ class WorkspaceRootFixture(t.NamedTuple): """Fixture for workspace root normalization cases.""" test_id: str - config: dict[str, t.Any] - expected_roots: list[str] + config_factory: t.Callable[[pathlib.Path], dict[str, t.Any]] WORKSPACE_ROOT_FIXTURES: list[WorkspaceRootFixture] = [ WorkspaceRootFixture( test_id="tilde-mixed-trailing-slash", - config={ + config_factory=lambda _base: { "~/study/c": { "cpython": {"repo": "git+https://github.com/python/cpython.git"}, }, @@ -34,23 +38,21 @@ class WorkspaceRootFixture(t.NamedTuple): "tmux": {"repo": "git+https://github.com/tmux/tmux.git"}, }, }, - expected_roots=["~/study/c/"], ), WorkspaceRootFixture( test_id="home-vs-absolute", - config={ - str(pathlib.Path.home() / "study" / "c"): { + config_factory=lambda base: { + str(base / "study" / "c"): { "cpython": {"repo": "git+https://github.com/python/cpython.git"}, }, "~/study/c/": { "tmux": {"repo": "git+https://github.com/tmux/tmux.git"}, }, }, - expected_roots=["~/study/c/"], ), WorkspaceRootFixture( test_id="relative-vs-tilde", - config={ + config_factory=lambda _base: { "./study/c": { "cpython": {"repo": "git+https://github.com/python/cpython.git"}, }, @@ -58,7 +60,6 @@ class WorkspaceRootFixture(t.NamedTuple): "tmux": {"repo": "git+https://github.com/tmux/tmux.git"}, }, }, - expected_roots=["~/study/c/"], ), ] @@ -66,12 +67,9 @@ class WorkspaceRootFixture(t.NamedTuple): @pytest.mark.parametrize( list(WorkspaceRootFixture._fields), [ - pytest.param( - *fixture, - marks=pytest.mark.xfail( - reason="Workspace root normalization not yet implemented in formatter.", - strict=True, - ), + ( + fixture.test_id, + fixture.config_factory, ) for fixture in WORKSPACE_ROOT_FIXTURES ], @@ -79,12 +77,30 @@ class WorkspaceRootFixture(t.NamedTuple): ) def test_workspace_root_normalization( test_id: str, - config: dict[str, t.Any], - expected_roots: list[str], + config_factory: t.Callable[[pathlib.Path], dict[str, t.Any]], ) -> None: """Ensure format_config merges duplicate workspace roots.""" - formatted, _changes = format_config(config) - assert list(formatted.keys()) == expected_roots + home_dir = pathlib.Path.home() + config = config_factory(home_dir) + + canonical_paths = { + canonicalize_workspace_path(label, cwd=home_dir) for label in config + } + expected_labels = [ + workspace_root_label(path, cwd=home_dir, home=home_dir) + for path in sorted(canonical_paths, key=lambda p: p.as_posix()) + ] + + normalized_config, _map, conflicts, merge_changes = normalize_workspace_roots( + config, + cwd=home_dir, + home=home_dir, + ) + assert conflicts == [] + assert sorted(normalized_config.keys()) == expected_labels + formatted, _changes = format_config(normalized_config) + assert sorted(formatted.keys()) == expected_labels + assert merge_changes >= len(config) - len(canonical_paths) def test_normalize_repo_config_compact_to_verbose() -> None: diff --git a/tests/cli/test_import.py b/tests/cli/test_import.py index 6cee6807..2c5f5724 100644 --- a/tests/cli/test_import.py +++ b/tests/cli/test_import.py @@ -12,6 +12,7 @@ from vcspull.cli import cli from vcspull.cli._import import get_git_origin_url, import_from_filesystem, import_repo +from vcspull.config import canonicalize_workspace_path, workspace_root_label if t.TYPE_CHECKING: import pathlib @@ -847,19 +848,11 @@ def test_many_existing_repos_summary( @pytest.mark.parametrize( list(ScanExistingFixture._fields), [ - pytest.param( - *fixture, - marks=pytest.mark.xfail( - reason=( - "Existing repos are re-added when workspace root formatting " - "does not match the scanner's trailing-slash expectations." - ), - strict=True, - ), + ( + fixture.test_id, + fixture.workspace_root_key_style, + fixture.scan_arg_style, ) - if fixture.workspace_root_key_style.endswith("no_slash") - or fixture.workspace_root_key_style.startswith("absolute_") - else fixture for fixture in SCAN_EXISTING_FIXTURES ], ids=[fixture.test_id for fixture in SCAN_EXISTING_FIXTURES], @@ -927,17 +920,20 @@ def test_scan_respects_existing_config_sections( with config_file.open(encoding="utf-8") as f: config_data = yaml.safe_load(f) - assert workspace_root_key in config_data - assert "cpython" in config_data[workspace_root_key] - assert config_data[workspace_root_key]["cpython"]["repo"] == expected_repo_url - assert len(config_data) == 1 - - alternate_key = ( - workspace_root_key[:-1] - if workspace_root_key.endswith("/") - else f"{workspace_root_key}/" + expected_path = canonicalize_workspace_path( + workspace_root_key, + cwd=home_dir, ) - assert alternate_key not in config_data + expected_label = workspace_root_label( + expected_path, + cwd=home_dir, + home=home_dir, + ) + assert expected_label in config_data + assert "cpython" in config_data[expected_label] + assert config_data[expected_label]["cpython"]["repo"] == expected_repo_url + assert len(config_data) == 1 + assert "~/study/c" not in config_data # ============================================================================= From 7f1c9380b10f71abb85014652b6505cf0b0899f3 Mon Sep 17 00:00:00 2001 From: Tony Narlock Date: Sat, 18 Oct 2025 16:25:31 -0500 Subject: [PATCH 7/7] docs(CHANGES) Note updates for workspace dir, bug fixes --- CHANGES | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index 9da71ed0..51d952ad 100644 --- a/CHANGES +++ b/CHANGES @@ -19,6 +19,25 @@ $ pipx install --suffix=@next 'vcspull' --pip-args '\--pre' --force +### Breaking changes + +- Rename the import CLI flag `--base-dir-key`/`--dir` and the related internal + argument names to `--workspace-root`, aligning both user-facing options and + implementation terminology with the workspace-root concept (#470). + +### Bug fixes + +- Normalize workspace roots during imports so equivalent labels (for example + `~/foo`, `~/foo/`, `/home/user/foo`) collapse into a single canonical entry; + avoids duplicate "already added" prompts (#470). +- Have `vcspull fmt` apply the same normalization before formatting and writing + configs, so duplicate workspace-root sections are removed automatically (#470). + +### Development + +- Expand CLI tests to cover mixed workspace-root scenarios and the formatter’s + normalization behavior (#470). + _Notes on upcoming releases will be added here_ ## vcspull v1.36.0 (2025-10-18) @@ -28,11 +47,11 @@ _Notes on upcoming releases will be added here_ #### New command: `vcspull import` (#465) - **Manual import**: Register a single repository with `vcspull import ` - - Optional `--workspace-root`/`--path` helpers for workspace-root detection + - Optional `--dir`/`--path` helpers for base-directory detection - **Filesystem scan**: Discover and import existing repositories with `vcspull import --scan ` - Recursively scan with `--recursive`/`-r` - Interactive confirmation prompt or `--yes` for unattended runs - - Custom workspace root with `--workspace-root` + - Custom base directory with `--base-dir-key` #### New command: `vcspull fmt` (#465)