diff --git a/.github/plan.md b/.github/plan.md index d698250..aa6128c 100644 --- a/.github/plan.md +++ b/.github/plan.md @@ -56,7 +56,7 @@ that contain a main driver script (`code/main.R`) plus vendored source repositor ### primary syncweaver commands - syncweaver list --lockfile .syncweaver-lock.json -- syncweaver add --path code/package1 --repo-url ccbr/package1 --ref [tag] --lockfile .syncweaver-lock.json +- syncweaver add --path code/package1 --repo ccbr/package1 --ref [tag] --lockfile .syncweaver-lock.json - syncweaver update --path code/package1 --ref [tag] --lockfile .syncweaver-lock.json - syncweaver remove --path code/package1 --lockfile .syncweaver-lock.json - syncweaver patch create --path code/package1 --lockfile .syncweaver-lock.json --patch-dir [optional-override] diff --git a/src/syncweaver/cli/add.py b/src/syncweaver/cli/add.py index ea1ea6c..5ba6124 100644 --- a/src/syncweaver/cli/add.py +++ b/src/syncweaver/cli/add.py @@ -13,6 +13,39 @@ from syncweaver.lockfile import read_lockfile, write_lockfile +def _resolve_repo_url_input(repo_url: str, cwd: pathlib.Path) -> tuple[str, str]: + """Resolve clone URL and tracked URL from a user-provided repo value.""" + normalized_input = repo_url.strip() + if not normalized_input: + raise ValueError("--repo-url cannot be empty") + + if normalized_input.startswith("file://"): + raise ValueError("--repo-url must not be a local filesystem path") + + if normalized_input.startswith(("./", "../", "/", "~")): + raise ValueError("--repo-url must not be a local filesystem path") + + candidate_path = pathlib.Path(normalized_input).expanduser() + if not candidate_path.is_absolute(): + candidate_path = cwd / candidate_path + if candidate_path.exists(): + raise ValueError("--repo-url must not be a local filesystem path") + elif ( + "://" not in normalized_input + and normalized_input.count("/") == 1 + and "@" not in normalized_input + ): + slug = normalized_input.removesuffix(".git") + clone_url = f"https://github.com/{slug}.git" + tracked_repo_url = f"https://github.com/{slug}" + elif "://" not in normalized_input and "@" not in normalized_input: + raise ValueError("--repo-url must be a remote URL or OWNER/REPO shorthand") + else: + clone_url = normalized_input + tracked_repo_url = normalized_input + return clone_url, tracked_repo_url + + def _copy_checked_out_repo(source: pathlib.Path, destination: pathlib.Path) -> None: """Copy a checked-out repository working tree without the .git directory.""" shutil.copytree( @@ -22,6 +55,31 @@ def _copy_checked_out_repo(source: pathlib.Path, destination: pathlib.Path) -> N ) +def _ensure_linguist_vendored_entry( + host_root: pathlib.Path, destination_path: pathlib.Path +) -> None: + """Ensure destination path is marked linguist-vendored in .gitattributes.""" + gitattributes_path = host_root / ".gitattributes" + entry_path = destination_path.as_posix() + + existing_lines: list[str] = [] + if gitattributes_path.exists(): + existing_lines = gitattributes_path.read_text().splitlines() + + has_destination_entry = False + for line in existing_lines: + stripped = line.strip() + if stripped and not stripped.startswith("#"): + tokens = stripped.split() + if tokens and tokens[0] == entry_path: + has_destination_entry = True + + if not has_destination_entry: + existing_lines.append(f"{entry_path} linguist-vendored") + + gitattributes_path.write_text("\n".join(existing_lines) + "\n") + + def _resolve_remote_source_path( checkout_root: pathlib.Path, remote_subdir: str | None ) -> pathlib.Path: @@ -53,6 +111,7 @@ def add_external_repository( cwd = pathlib.Path.cwd() destination = cwd / destination_path lockfile = cwd / lockfile_path + clone_repo_url, tracked_repo_url = _resolve_repo_url_input(repo_url, cwd) if destination.exists(): if not overwrite: @@ -66,7 +125,7 @@ def add_external_repository( with tempfile.TemporaryDirectory(prefix="syncweaver-add-") as temp_dir: temp_repo = pathlib.Path(temp_dir) / "repo" - run_git(["clone", "--quiet", "--no-checkout", repo_url, str(temp_repo)]) + run_git(["clone", "--quiet", "--no-checkout", clone_repo_url, str(temp_repo)]) selected_ref = ref if selected_ref: @@ -103,7 +162,7 @@ def add_external_repository( sources = lock_data.setdefault("sources", {}) source_key = destination_path.as_posix() sources[source_key] = { - "repo_url": repo_url, + "repo_url": tracked_repo_url, "ref": selected_ref, "git_sha": git_sha, "installed_by": ["syncweaver"], @@ -112,6 +171,7 @@ def add_external_repository( normalized_subdir = pathlib.PurePosixPath(remote_subdir.strip("/")).as_posix() sources[source_key]["remote_subdir"] = normalized_subdir write_lockfile(lockfile, lock_data) + _ensure_linguist_vendored_entry(cwd, destination_path) return destination, lockfile, selected_ref, git_sha @@ -125,9 +185,11 @@ def add_external_repository( help="Destination path in the host repository, e.g. code/package1.", ) @click.option( + "--repo", "--repo-url", + "repo", required=True, - help="External repository URL or local path to clone.", + help="External repository URL or OWNER/REPO shorthand.", ) @click.option( "--ref", @@ -157,7 +219,7 @@ def add_external_repository( ) def add_cmd( destination_path: pathlib.Path, - repo_url: str, + repo: str, ref: str | None, remote_subdir: str | None, lockfile: pathlib.Path, @@ -167,7 +229,7 @@ def add_cmd( try: dest, lock_path, selected_ref, git_sha = add_external_repository( destination_path=destination_path, - repo_url=repo_url, + repo_url=repo, ref=ref, remote_subdir=remote_subdir, lockfile_path=lockfile, diff --git a/src/syncweaver/cli/patch.py b/src/syncweaver/cli/patch.py index 83fa0c4..ce787ea 100644 --- a/src/syncweaver/cli/patch.py +++ b/src/syncweaver/cli/patch.py @@ -29,7 +29,9 @@ def patch_group() -> None: help="Tracked source path in the host repository, e.g. code/package1.", ) @click.option( + "--repo", "--repo-url", + "repo", required=True, help="Repository URL as it appears in .syncweaver-lock.json.", ) @@ -48,7 +50,7 @@ def patch_group() -> None: ) def create_cmd( source_path: pathlib.Path, - repo_url: str, + repo: str, lockfile: pathlib.Path, patch_dir: pathlib.Path | None, ) -> None: @@ -57,7 +59,7 @@ def create_cmd( tracked source path. """ try: - patch_path = create_patch(source_path, repo_url, lockfile, patch_dir) + patch_path = create_patch(source_path, repo, lockfile, patch_dir) except ( FileNotFoundError, KeyError, diff --git a/tests/test_add.py b/tests/test_add.py index 7316e5c..47b20bf 100644 --- a/tests/test_add.py +++ b/tests/test_add.py @@ -8,6 +8,7 @@ from click.testing import CliRunner +import syncweaver.cli.add as add_module from syncweaver.cli import cli @@ -39,24 +40,34 @@ def test_add_vendors_repository_and_updates_lockfile(tmp_path, monkeypatch): Returns: None: Assertions validate command behavior. """ - source_repo = tmp_path / "source" - source_repo.mkdir() - _init_git_repo(source_repo) - (source_repo / "pkg.py").write_text("VALUE = 1\n") - _run(["git", "add", "pkg.py"], cwd=source_repo) - _run(["git", "commit", "--no-verify", "-m", "add package file"], cwd=source_repo) - source_sha = subprocess.run( - ["git", "rev-parse", "HEAD"], - cwd=source_repo, - check=True, - capture_output=True, - text=True, - ).stdout.strip() + source_sha = "abc123" host_repo = tmp_path / "host-repo" host_repo.mkdir() _init_git_repo(host_repo, remote_url="https://github.com/CCBR/host-repo1.git") monkeypatch.chdir(host_repo) + original_run_git = add_module.run_git + + def _fake_run_git(args, cwd=None, env=None, redacted_values=None): + output = "" + if args[:1] == ["clone"]: + temp_repo = pathlib.Path(args[4]) + temp_repo.mkdir(parents=True, exist_ok=True) + (temp_repo / "pkg.py").write_text("VALUE = 1\n") + elif len(args) >= 3 and args[0] == "-C" and args[2] in {"fetch", "checkout"}: + output = "" + elif args[-2:] == ["rev-parse", "HEAD"]: + output = source_sha + else: + output = original_run_git( + args, + cwd=cwd, + env=env, + redacted_values=redacted_values, + ) + return output + + monkeypatch.setattr(add_module, "run_git", _fake_run_git) runner = CliRunner() result = runner.invoke( @@ -66,7 +77,7 @@ def test_add_vendors_repository_and_updates_lockfile(tmp_path, monkeypatch): "--path", "code/package1", "--repo-url", - str(source_repo), + "https://github.com/CCBR/package1", "--ref", "main", ], @@ -79,13 +90,16 @@ def test_add_vendors_repository_and_updates_lockfile(tmp_path, monkeypatch): assert lockfile["name"] == "CCBR/host-repo1" assert lockfile["homePage"] == "https://github.com/CCBR/host-repo1" source_entry = lockfile["sources"]["code/package1"] - assert source_entry["repo_url"] == str(source_repo) + assert source_entry["repo_url"] == "https://github.com/CCBR/package1" assert source_entry["ref"] == "main" assert source_entry["git_sha"] == source_sha assert source_entry["installed_by"] == ["syncweaver"] assert "patch" not in source_entry assert "patches" not in source_entry + gitattributes_lines = (host_repo / ".gitattributes").read_text().splitlines() + assert "code/package1 linguist-vendored" in gitattributes_lines + def test_add_refuses_to_overwrite_existing_destination(tmp_path, monkeypatch): """Verify `add` refuses to replace an existing destination by default. @@ -97,10 +111,6 @@ def test_add_refuses_to_overwrite_existing_destination(tmp_path, monkeypatch): Returns: None: Assertions validate command behavior. """ - source_repo = tmp_path / "source" - source_repo.mkdir() - _init_git_repo(source_repo) - host_repo = tmp_path / "host-repo" host_repo.mkdir() _init_git_repo(host_repo) @@ -117,7 +127,7 @@ def test_add_refuses_to_overwrite_existing_destination(tmp_path, monkeypatch): "--path", "code/package1", "--repo-url", - str(source_repo), + "https://github.com/CCBR/package1", ], ) @@ -137,24 +147,27 @@ def test_add_with_remote_subdir_vendors_only_subdir_and_tracks_metadata( Returns: None: Assertions validate command behavior. """ - source_repo = tmp_path / "source" - source_repo.mkdir() - _init_git_repo(source_repo) - - package_root = source_repo / "subprojects/package1" - package_root.mkdir(parents=True) - (package_root / "pkg.py").write_text("VALUE = 1\n") - (source_repo / "root-only.txt").write_text("do not vendor\n") - _run( - ["git", "add", "subprojects/package1/pkg.py", "root-only.txt"], cwd=source_repo - ) - _run(["git", "commit", "--no-verify", "-m", "add nested package"], cwd=source_repo) - host_repo = tmp_path / "host-repo" host_repo.mkdir() _init_git_repo(host_repo) monkeypatch.chdir(host_repo) + def _fake_run_git(args, cwd=None, env=None, redacted_values=None): + del cwd + del env + del redacted_values + output = "" + if args[:1] == ["clone"]: + temp_repo = pathlib.Path(args[4]) + (temp_repo / "subprojects/package1").mkdir(parents=True, exist_ok=True) + (temp_repo / "subprojects/package1/pkg.py").write_text("VALUE = 1\n") + (temp_repo / "root-only.txt").write_text("do not vendor\n") + elif args[-2:] == ["rev-parse", "HEAD"]: + output = "abc123" + return output + + monkeypatch.setattr(add_module, "run_git", _fake_run_git) + runner = CliRunner() result = runner.invoke( cli, @@ -163,7 +176,7 @@ def test_add_with_remote_subdir_vendors_only_subdir_and_tracks_metadata( "--path", "code/package1", "--repo-url", - str(source_repo), + "https://github.com/CCBR/package1", "--ref", "main", "--remote-subdir", @@ -178,3 +191,148 @@ def test_add_with_remote_subdir_vendors_only_subdir_and_tracks_metadata( lockfile = json.loads((host_repo / ".syncweaver-lock.json").read_text()) source_entry = lockfile["sources"]["code/package1"] assert source_entry["remote_subdir"] == "subprojects/package1" + + +def test_add_rejects_local_repo_path(tmp_path, monkeypatch): + """Verify `add` rejects local filesystem paths for --repo-url. + + Args: + tmp_path: Temporary directory fixture. + monkeypatch: Pytest monkeypatch fixture. + + Returns: + None: Assertions validate command behavior. + """ + source_repo = tmp_path / "source" + source_repo.mkdir() + + host_repo = tmp_path / "host-repo" + host_repo.mkdir() + _init_git_repo(host_repo) + monkeypatch.chdir(host_repo) + + runner = CliRunner() + result = runner.invoke( + cli, + [ + "add", + "--path", + "code/package1", + "--repo-url", + str(source_repo), + ], + ) + + assert result.exit_code != 0 + assert "must not be a local filesystem path" in result.output + + +def test_add_accepts_owner_repo_shorthand(tmp_path, monkeypatch): + """Verify `add --repo OWNER/REPO` resolves to a GitHub clone URL. + + Args: + tmp_path: Temporary directory fixture. + monkeypatch: Pytest monkeypatch fixture. + + Returns: + None: Assertions validate command behavior. + """ + host_repo = tmp_path / "host-repo" + host_repo.mkdir() + _init_git_repo(host_repo) + (host_repo / ".syncweaver-lock.json").write_text( + json.dumps({"name": "host-repo", "homePage": "", "sources": {}}, indent=2) + + "\n" + ) + monkeypatch.chdir(host_repo) + + clone_url_seen = {"value": ""} + + def _fake_run_git(args, cwd=None, env=None, redacted_values=None): + del cwd + del env + del redacted_values + output = "" + if args[:1] == ["clone"]: + clone_url_seen["value"] = args[3] + temp_repo = pathlib.Path(args[4]) + temp_repo.mkdir(parents=True, exist_ok=True) + (temp_repo / "pkg.py").write_text("VALUE = 1\n") + elif args[-2:] == ["rev-parse", "HEAD"]: + output = "abc123" + return output + + monkeypatch.setattr(add_module, "run_git", _fake_run_git) + + runner = CliRunner() + result = runner.invoke( + cli, + [ + "add", + "--path", + "code/package1", + "--repo", + "CCBR/package1", + "--ref", + "main", + ], + ) + + assert result.exit_code == 0, result.output + assert clone_url_seen["value"] == "https://github.com/CCBR/package1.git" + + lockfile = json.loads((host_repo / ".syncweaver-lock.json").read_text()) + source_entry = lockfile["sources"]["code/package1"] + assert source_entry["repo_url"] == "https://github.com/CCBR/package1" + + +def test_add_does_not_duplicate_existing_gitattributes_entry(tmp_path, monkeypatch): + """Verify `add` does not duplicate an existing linguist-vendored entry. + + Args: + tmp_path: Temporary directory fixture. + monkeypatch: Pytest monkeypatch fixture. + + Returns: + None: Assertions validate command behavior. + """ + host_repo = tmp_path / "host-repo" + host_repo.mkdir() + _init_git_repo(host_repo) + (host_repo / ".gitattributes").write_text( + "code/package1 linguist-vendored\n*.md text\n" + ) + monkeypatch.chdir(host_repo) + + def _fake_run_git(args, cwd=None, env=None, redacted_values=None): + del cwd + del env + del redacted_values + output = "" + if args[:1] == ["clone"]: + temp_repo = pathlib.Path(args[4]) + temp_repo.mkdir(parents=True, exist_ok=True) + (temp_repo / "pkg.py").write_text("VALUE = 1\n") + elif args[-2:] == ["rev-parse", "HEAD"]: + output = "abc123" + return output + + monkeypatch.setattr(add_module, "run_git", _fake_run_git) + + runner = CliRunner() + result = runner.invoke( + cli, + [ + "add", + "--path", + "code/package1", + "--repo-url", + "https://github.com/CCBR/package1", + "--ref", + "main", + ], + ) + + assert result.exit_code == 0, result.output + gitattributes_lines = (host_repo / ".gitattributes").read_text().splitlines() + assert gitattributes_lines.count("code/package1 linguist-vendored") == 1 diff --git a/tests/test_cli.py b/tests/test_cli.py index 219b484..bffbbbf 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -207,6 +207,32 @@ def test_update_help_includes_remote_subdir_option(): assert "--patch-conflict-strategy" in result.output +def test_add_help_includes_repo_aliases(): + """Verify `add --help` documents --repo and compatibility alias --repo-url. + + Returns: + None: Assertions validate command behavior. + """ + runner = CliRunner() + result = runner.invoke(cli, ["add", "--help"]) + assert result.exit_code == 0 + assert "--repo" in result.output + assert "--repo-url" in result.output + + +def test_patch_create_help_includes_repo_aliases(): + """Verify `patch create --help` documents --repo and --repo-url. + + Returns: + None: Assertions validate command behavior. + """ + runner = CliRunner() + result = runner.invoke(cli, ["patch", "create", "--help"]) + assert result.exit_code == 0 + assert "--repo" in result.output + assert "--repo-url" in result.output + + def test_templates_list(): """Verify template listing command prints YAML template names. diff --git a/tests/test_patch.py b/tests/test_patch.py index d05161c..ada6ac6 100644 --- a/tests/test_patch.py +++ b/tests/test_patch.py @@ -9,6 +9,7 @@ import pytest from click.testing import CliRunner +import syncweaver.cli.add as add_module from syncweaver.cli import cli from syncweaver.patch import _validate_patch_structure @@ -45,6 +46,11 @@ def _setup_source_and_host(tmp_path, monkeypatch): _init_git_repo(host_repo) monkeypatch.chdir(host_repo) + monkeypatch.setattr( + add_module, + "_resolve_repo_url_input", + lambda _repo_url, _cwd: (str(source_repo), str(source_repo)), + ) runner = CliRunner() add_result = runner.invoke( cli, @@ -84,7 +90,7 @@ def test_patch_create_generates_file_and_updates_lockfile(tmp_path, monkeypatch) "create", "--path", "code/package1", - "--repo-url", + "--repo", str(source_repo), ], ) @@ -379,6 +385,11 @@ def test_patch_create_uses_remote_subdir_baseline(tmp_path, monkeypatch): host_repo.mkdir() _init_git_repo(host_repo) monkeypatch.chdir(host_repo) + monkeypatch.setattr( + add_module, + "_resolve_repo_url_input", + lambda _repo_url, _cwd: (str(source_repo), str(source_repo)), + ) runner = CliRunner() add_result = runner.invoke( diff --git a/tests/test_remove.py b/tests/test_remove.py index 951a436..011558a 100644 --- a/tests/test_remove.py +++ b/tests/test_remove.py @@ -8,6 +8,7 @@ from click.testing import CliRunner +import syncweaver.cli.add as add_module from syncweaver.cli import cli @@ -40,6 +41,11 @@ def test_remove_deletes_vendored_tree_and_lockfile_entry(tmp_path, monkeypatch): host_repo.mkdir() _init_git_repo(host_repo) monkeypatch.chdir(host_repo) + monkeypatch.setattr( + add_module, + "_resolve_repo_url_input", + lambda _repo_url, _cwd: (str(source_repo), str(source_repo)), + ) runner = CliRunner() add_result = runner.invoke( diff --git a/tests/test_update.py b/tests/test_update.py index dd16443..86c0da9 100644 --- a/tests/test_update.py +++ b/tests/test_update.py @@ -8,6 +8,7 @@ from click.testing import CliRunner +import syncweaver.cli.add as add_module import syncweaver.cli.update as update_module from syncweaver.cli import cli @@ -54,6 +55,11 @@ def test_update_refreshes_tracked_subdir_and_lockfile(tmp_path, monkeypatch): host_repo.mkdir() _init_git_repo(host_repo) monkeypatch.chdir(host_repo) + monkeypatch.setattr( + add_module, + "_resolve_repo_url_input", + lambda _repo_url, _cwd: (str(source_repo), str(source_repo)), + ) runner = CliRunner() add_result = runner.invoke( @@ -126,6 +132,11 @@ def test_update_allows_remote_subdir_override(tmp_path, monkeypatch): host_repo.mkdir() _init_git_repo(host_repo) monkeypatch.chdir(host_repo) + monkeypatch.setattr( + add_module, + "_resolve_repo_url_input", + lambda _repo_url, _cwd: (str(source_repo), str(source_repo)), + ) runner = CliRunner() add_result = runner.invoke( @@ -187,6 +198,11 @@ def test_update_reapplies_tracked_patch_after_refresh(tmp_path, monkeypatch): host_repo.mkdir() _init_git_repo(host_repo) monkeypatch.chdir(host_repo) + monkeypatch.setattr( + add_module, + "_resolve_repo_url_input", + lambda _repo_url, _cwd: (str(source_repo), str(source_repo)), + ) runner = CliRunner() add_result = runner.invoke( @@ -273,6 +289,11 @@ def test_update_warn_strategy_continues_when_patch_conflicts(tmp_path, monkeypat host_repo.mkdir() _init_git_repo(host_repo) monkeypatch.chdir(host_repo) + monkeypatch.setattr( + add_module, + "_resolve_repo_url_input", + lambda _repo_url, _cwd: (str(source_repo), str(source_repo)), + ) runner = CliRunner() add_result = runner.invoke(