diff --git a/commodore/component/template.py b/commodore/component/template.py index fe1dfc99..bd85609e 100644 --- a/commodore/component/template.py +++ b/commodore/component/template.py @@ -83,6 +83,17 @@ def delete(self): abort=True, ) rmtree(component.target_directory) + # We check for other checkouts here, because our MultiDependency doesn't + # know if there's other dependencies which would be registered on it. + if not cdep.has_checkouts(): + # Also delete bare copy of component repo, if there's no other + # worktree checkouts for the same dependency repo. + rmtree(cdep.repo_directory) + else: + click.echo( + f" > Not deleting bare copy of repository {cdep.url}. " + + "Other worktrees refer to the same reposiotry." + ) click.secho(f"Component {self.slug} successfully deleted 🎉", bold=True) else: diff --git a/commodore/dependency_templater.py b/commodore/dependency_templater.py index fd346f1c..19c8a740 100644 --- a/commodore/dependency_templater.py +++ b/commodore/dependency_templater.py @@ -2,6 +2,8 @@ import datetime import re +import tempfile +import shutil from abc import ABC, abstractmethod from pathlib import Path @@ -11,6 +13,7 @@ from commodore.config import Config from commodore.gitrepo import GitRepo +from commodore.multi_dependency import MultiDependency SLUG_REGEX = re.compile("^[a-z][a-z0-9-]+[a-z0-9]$") @@ -132,20 +135,34 @@ def create(self) -> None: + f"{self.target_dir} already exists." ) - self.template_renderer( - self.template, - no_input=True, - output_dir=self.target_dir.parent, - extra_context=self.cookiecutter_args, + want_worktree = ( + self.config.inventory.dependencies_dir in self.target_dir.parents ) + if want_worktree: + md = MultiDependency(self.repo_url, self.config.inventory.dependencies_dir) + md.initialize_worktree(self.target_dir) + + with tempfile.TemporaryDirectory() as tmpdir: + self.template_renderer( + self.template, + no_input=True, + output_dir=Path(tmpdir), + extra_context=self.cookiecutter_args, + ) + shutil.copytree( + Path(tmpdir) / self.slug, self.target_dir, dirs_exist_ok=True + ) - self.commit("Initial commit") + self.commit("Initial commit", amend=want_worktree) click.secho( f"{self.deptype.capitalize()} {self.name} successfully added 🎉", bold=True ) - def commit(self, msg: str) -> None: - repo = GitRepo(self.repo_url, targetdir=self.target_dir, force_init=True) + def commit(self, msg: str, amend=False, init=True) -> None: + # If we're amending an existing commit, we don't want to force initialize the + # repo. + repo = GitRepo(self.repo_url, self.target_dir, force_init=not amend and init) + repo.stage_all() repo.stage_files(self.additional_files) - repo.commit(msg) + repo.commit(msg, amend=amend) diff --git a/commodore/gitrepo.py b/commodore/gitrepo.py index 0cad28d3..f554b252 100644 --- a/commodore/gitrepo.py +++ b/commodore/gitrepo.py @@ -235,6 +235,15 @@ def head_short_sha(self) -> str: sha = self._repo.head.commit.hexsha return self._repo.git.rev_parse(sha, short=6) + @property + def _author_env(self) -> dict[str, str]: + return { + Actor.env_author_name: self._author.name or "", + Actor.env_author_email: self._author.email or "", + Actor.env_committer_name: self._author.name or "", + Actor.env_committer_email: self._author.email or "", + } + @property def _null_tree(self) -> Tree: """Generate empty Tree for the repo. @@ -404,6 +413,51 @@ def checkout_worktree(self, worktree: Path, version: Optional[str]): # If the worktree directory doesn't exist yet, create the worktree self._create_worktree(worktree, version) + def initialize_worktree( + self, worktree: Path, initial_branch: Optional[str] = None + ) -> None: + if not initial_branch: + initial_branch = self._default_version() + + # We need an initial commit to be able to create a worktree. Create initial + # commit from empty tree. + initsha = self._repo.git.execute( # type: ignore[call-overload] + command=[ + "git", + "commit-tree", + "-m", + "Initial commit", + self._null_tree.hexsha, + ], + env=self._author_env, + ) + + # Create worktree using the provided branch name + self._repo.git.execute( + ["git", "worktree", "add", str(worktree), initsha, "-b", initial_branch] + ) + + @property + def worktrees(self) -> list[GitRepo]: + """List all worktrees for the repo""" + # First prune worktrees, to ensure repo worktree state is clean + self._repo.git.execute(["git", "worktree", "prune"]) + worktrees: list[GitRepo] = [] + wt_list = self._repo.git.execute( + ["git", "worktree", "list", "--porcelain"], + as_process=False, + with_extended_output=False, + stdout_as_string=True, + ).splitlines() + for line in wt_list: + if " " not in line: + continue + k, v = line.split(" ") + if k == "worktree": + worktrees.append(GitRepo(None, Path(v))) + + return worktrees + def checkout(self, version: Optional[str] = None): remote_heads = self.fetch() if not remote_heads: @@ -492,13 +546,28 @@ def stage_files(self, files: Sequence[str]): """Add provided list of files to index.""" self._repo.index.add(files) - def commit(self, commit_message: str): + def commit(self, commit_message: str, amend=False): author = self._author if self._trace: click.echo(f' > Using "{author.name} <{author.email}>" as commit author') - self._repo.index.commit(commit_message, author=author, committer=author) + if amend: + # We need to call out to `git commit` for amending + self._repo.git.execute( # type: ignore[call-overload] + [ + "git", + "commit", + "--amend", + "--no-edit", + "--reset-author", + "-m", + commit_message, + ], + env=self._author_env, + ) + else: + self._repo.index.commit(commit_message, author=author, committer=author) def push( self, remote: Optional[str] = None, version: Optional[str] = None diff --git a/commodore/multi_dependency.py b/commodore/multi_dependency.py index 5d5bb864..45438843 100644 --- a/commodore/multi_dependency.py +++ b/commodore/multi_dependency.py @@ -27,6 +27,10 @@ def url(self) -> str: def url(self, repo_url: str): self._repo.remote = repo_url + @property + def repo_directory(self) -> Path: + return Path(self._repo.repo.common_dir).resolve().absolute() + def get_component(self, name: str) -> Optional[Path]: return self._components.get(name) @@ -71,6 +75,13 @@ def checkout_package(self, name: str, version: str): raise ValueError(f"can't checkout unknown package {name}") self._repo.checkout_worktree(target_dir, version=version) + def initialize_worktree(self, target_dir: Path) -> None: + """Initialize a worktree in `target_dir`.""" + self._repo.initialize_worktree(target_dir) + + def has_checkouts(self) -> bool: + return len(self._repo.worktrees) > 1 + def dependency_dir(base_dir: Path, repo_url: str) -> Path: return base_dir / ".repos" / dependency_key(repo_url) diff --git a/commodore/package/template.py b/commodore/package/template.py index eb4463d2..c28f2062 100644 --- a/commodore/package/template.py +++ b/commodore/package/template.py @@ -1,8 +1,6 @@ from __future__ import annotations import json -import shutil -import tempfile from pathlib import Path from typing import Any, Optional, Sequence @@ -27,6 +25,7 @@ class PackageTemplater(Templater): template_commit: str test_cases: list[str] = ["defaults"] copyright_year: Optional[str] = None + _target_dir: Optional[Path] = None @classmethod def from_existing(cls, config: Config, package_path: Path): @@ -39,6 +38,7 @@ def from_existing(cls, config: Config, package_path: Path): t = PackageTemplater( config, cookiecutter_args["slug"], name=cookiecutter_args["name"] ) + t._target_dir = package_path t.output_dir = package_path.absolute().parent t.template_url = cruft_json["template"] if cruft_json["checkout"]: @@ -61,28 +61,13 @@ def _cruft_renderer( no_input: bool, output_dir: Path, ): - """Render package cookiecutter template in tempdir and move the results to - `output_dir/pkg.`, because as far as I can see we can't configure - cruft/cookiecutter to create a directory named `pkg.` except if we - change the slug itself, which would need a bunch of template changes. - """ - - # Because we render the template in a temp directory and move it to the desired - # target directory, we don't need argument `output_dir` which is set to - # `self.target_dir.parent` when the renderer function is called by the base - # class, and instead move the final rendered package to `self.target_dir` - # ourselves. - _ = output_dir - tmpdir = Path(tempfile.mkdtemp()) cruft_create( template_location, checkout=self.template_version, extra_context=extra_context, no_input=no_input, - output_dir=tmpdir, + output_dir=output_dir, ) - shutil.move(str(tmpdir / self.slug), self.target_dir) - shutil.rmtree(tmpdir) def _validate_slug(self, value: str): # First perform default slug checks @@ -124,6 +109,9 @@ def template_renderer(self) -> Renderer: @property def target_dir(self) -> Path: + if self._target_dir: + return self._target_dir + if self.output_dir: return self.output_dir / self.slug @@ -153,7 +141,8 @@ def update(self): self.commit( "Update from template\n\n" - + f"Template version: {self.template_version} ({self.template_commit[:7]})" + + f"Template version: {self.template_version} ({self.template_commit[:7]})", + init=False, ) click.secho( diff --git a/docs/modules/ROOT/pages/reference/commands.adoc b/docs/modules/ROOT/pages/reference/commands.adoc index 29948247..d59f7bd6 100644 --- a/docs/modules/ROOT/pages/reference/commands.adoc +++ b/docs/modules/ROOT/pages/reference/commands.adoc @@ -40,6 +40,7 @@ check with the user whether they really want to remove the items listed above. commodore component new SLUG This command creates a new component repository under `dependencies/` in Commodore's working directory. +Commodore creates a Git worktree checkout for the new component. The component repository is created using a Cookiecutter template which provides a skeleton for writing a new component. The command requires the argument `SLUG` to match the regular expression `^[a-z][a-z0-9-]+[a-z0-9]$`. Optionally, the template can be used to add a component library and postprocessing filter configuration. @@ -152,7 +153,8 @@ If necessary, the command will call `commodore login` internally to fetch a vali commodore package new SLUG This command creates a new config package repository. -If not specified explicitly, the command will create the new package under `inventory/classes/` in Commodore's working directory. +If not specified explicitly, the command will create the new package under `dependencies/` in Commodore's working directory. +If the new package is created in `dependencies`, Commodore will create a Git worktree checkout. The package repository is created using a Cookiecutter template which provides a skeleton for writing a new package. The command requires the argument `SLUG` to match the regular expression `^[a-z][a-z0-9-]+[a-z0-9]$`. Additionally, the command prevents users from creating packages using reserved names or prefixes. diff --git a/tests/test_component_template.py b/tests/test_component_template.py index 5cca1e33..02779c47 100644 --- a/tests/test_component_template.py +++ b/tests/test_component_template.py @@ -9,6 +9,7 @@ from subprocess import call from git import Repo +from conftest import RunnerFunc from test_component import setup_directory @@ -92,7 +93,16 @@ def test_run_component_new_command( ) for file in expected_files: assert (tmp_path / "dependencies" / component_name / file).exists() - # Check that there are no uncommited files in the component repo + # Check that we created a worktree + assert (tmp_path / "dependencies" / component_name / ".git").is_file() + # Verify that worktree and bare copy configs are correct + repo = Repo(tmp_path / "dependencies" / component_name) + assert not repo.bare + assert P(repo.working_tree_dir) == tmp_path / "dependencies" / component_name + md_repo = Repo(P(repo.common_dir).resolve()) + assert md_repo.bare + assert md_repo.working_tree_dir is None + # Check that there are no uncommitted files in the component repo repo = Repo(tmp_path / "dependencies" / component_name) assert not repo.is_dirty() assert not repo.untracked_files @@ -225,24 +235,22 @@ def test_run_component_new_command_with_illegal_slug(tmp_path: P, test_input): assert exit_status != 0 -def test_run_component_new_then_delete(tmp_path: P): +def test_run_component_new_then_delete(tmp_path: P, cli_runner: RunnerFunc): """ Create a new component, then immediately delete it. """ setup_directory(tmp_path) component_name = "test-component" - exit_status = call( - f"commodore -d {tmp_path} -vvv component new {component_name} --lib --pp", - shell=True, + result = cli_runner( + ["-d", tmp_path, "-vvv", "component", "new", component_name, "--lib", "--pp"] ) - assert exit_status == 0 + assert result.exit_code == 0 - exit_status = call( - f"commodore -d {tmp_path} -vvv component delete --force {component_name}", - shell=True, + result = cli_runner( + ["-d", tmp_path, "-vvv", "component", "delete", "--force", component_name] ) - assert exit_status == 0 + assert result.exit_code == 0 # Ensure the dependencies folder is gone. assert not (tmp_path / "dependencies" / component_name).exists() diff --git a/tests/test_gitrepo.py b/tests/test_gitrepo.py index 6c646f69..bd1a8f43 100644 --- a/tests/test_gitrepo.py +++ b/tests/test_gitrepo.py @@ -7,6 +7,8 @@ from dataclasses import dataclass from typing import Optional +import shutil + import click import git import pytest @@ -505,3 +507,55 @@ def test_gitrepo_checkout_worktree_no_remote(tmp_path): assert w.head.commit.hexsha == c.hexsha assert w.head.ref.name == "master" assert (tmp_path / "worktree" / "test.txt").is_file() + + +def test_gitrepo_list_worktrees(tmp_path: Path): + r, ri = setup_repo(tmp_path) + + initial_wts = r.worktrees + assert len(initial_wts) == 1 + assert initial_wts[0].working_tree_dir == r.working_tree_dir + assert initial_wts[0].repo.head.commit.hexsha == ri.commit_shas["master"] + + r.checkout_worktree(tmp_path / "worktree", version="test-branch") + + worktrees = r.worktrees + assert len(worktrees) == 2 + assert worktrees[0].working_tree_dir == initial_wts[0].working_tree_dir + assert worktrees[1].working_tree_dir == tmp_path / "worktree" + assert worktrees[1].repo.head.commit.hexsha == ri.commit_shas["test-branch"] + + shutil.rmtree(worktrees[1].working_tree_dir) + + worktrees_after_delete = r.worktrees + assert len(worktrees_after_delete) == 1 + assert worktrees_after_delete[0].working_tree_dir == r.working_tree_dir + assert worktrees_after_delete[0].repo.head.commit.hexsha == ri.commit_shas["master"] + + +def test_gitrepo_init_worktree(tmp_path): + repo_path = tmp_path / "repo.git" + r = gitrepo.GitRepo( + "ssh://git@git.example.com/repo.git", + repo_path, + author_name="John Doe", + author_email="john.doe@example.com", + bare=True, + ) + r.initialize_worktree(tmp_path / "repo") + + assert r.repo.head.commit.author.name == "John Doe" + assert r.repo.head.commit.author.email == "john.doe@example.com" + + +def test_gitrepo_commit_amend(tmp_path: Path): + r, ri = setup_repo(tmp_path) + r._author = git.Actor("John Doe", "john.doe@example.com") + + r.commit("Amended", amend=True) + + assert r.repo.head.commit.message == "Amended\n" + assert r.repo.head.commit.author.name == "John Doe" + assert r.repo.head.commit.author.email == "john.doe@example.com" + assert r.repo.head.commit.committer.name == "John Doe" + assert r.repo.head.commit.committer.email == "john.doe@example.com" diff --git a/tests/test_multi_dependency.py b/tests/test_multi_dependency.py index 60268842..3e491247 100644 --- a/tests/test_multi_dependency.py +++ b/tests/test_multi_dependency.py @@ -47,7 +47,7 @@ def test_dependency_key(tmp_path: Path, repo_url: str, expected: str): def test_multi_dependency_init(tmp_path: Path): repo_url, ri = setup_remote(tmp_path) - _ = multi_dependency.MultiDependency(repo_url, tmp_path) + md = multi_dependency.MultiDependency(repo_url, tmp_path) repo_url_parts = deconstruct_url(repo_url) print(repo_url, repo_url_parts) @@ -60,6 +60,8 @@ def test_multi_dependency_init(tmp_path: Path): assert (bare_clone_path / "config").exists() assert (bare_clone_path / "HEAD").exists() + assert md.repo_directory == bare_clone_path + b = Repo.init(bare_clone_path) assert b.bare assert b.working_tree_dir is None @@ -148,6 +150,7 @@ def test_multi_dependency_deregister(tmp_path: Path): def test_multi_dependency_checkout_component_exc(tmp_path: Path): repo_url, ri = setup_remote(tmp_path) md = multi_dependency.MultiDependency(repo_url, tmp_path) + assert not md.has_checkouts() with pytest.raises(ValueError) as e: md.checkout_component("test", "master") @@ -161,10 +164,12 @@ def test_multi_dependency_checkout_component(tmp_path: Path): target_dir = tmp_path / "test" assert not target_dir.is_dir() + assert not md.has_checkouts() md.register_component("test", target_dir) md.checkout_component("test", "master") + assert md.has_checkouts() assert target_dir.is_dir() assert (target_dir / ".git").is_file() assert (target_dir / "test.txt").is_file() @@ -177,6 +182,7 @@ def test_multi_dependency_checkout_component(tmp_path: Path): def test_multi_dependency_checkout_package_exc(tmp_path: Path): repo_url, ri = setup_remote(tmp_path) md = multi_dependency.MultiDependency(repo_url, tmp_path) + assert not md.has_checkouts() with pytest.raises(ValueError) as e: md.checkout_package("test", "master") @@ -187,6 +193,7 @@ def test_multi_dependency_checkout_package_exc(tmp_path: Path): def test_multi_dependency_checkout_package(tmp_path: Path): repo_url, ri = setup_remote(tmp_path) md = multi_dependency.MultiDependency(repo_url, tmp_path) + assert not md.has_checkouts() target_dir = tmp_path / "test" assert not target_dir.is_dir() @@ -194,6 +201,7 @@ def test_multi_dependency_checkout_package(tmp_path: Path): md.register_package("test", target_dir) md.checkout_package("test", "master") + assert md.has_checkouts() assert target_dir.is_dir() assert (target_dir / ".git").is_file() assert (target_dir / "test.txt").is_file() diff --git a/tests/test_package_template.py b/tests/test_package_template.py index 7b8f93fe..d28bb49b 100644 --- a/tests/test_package_template.py +++ b/tests/test_package_template.py @@ -6,6 +6,7 @@ import json import click +import git import pytest import yaml @@ -98,6 +99,7 @@ def test_run_package_new_command( ] + [Path("tests", f"{case}.yml") for case in additional_test_cases] assert pkg_dir.is_dir() + assert (pkg_dir / ".git").is_file() == (output_dir == "") for f in expected_files: assert (pkg_dir / f).is_file() @@ -191,20 +193,28 @@ def _verify_copyright_holder(pkg_dir: Path, holder="VSHN AG "): assert line == f"Copyright {year}, {holder}\n" +@pytest.mark.parametrize("output_dir", ["", "--output-dir={0}"]) def test_package_update_copyright_holder( tmp_path: Path, cli_runner: RunnerFunc, + output_dir: str, ): + if output_dir == "": + pkg_dir = tmp_path / "dependencies" / "pkg.test-package" + else: + pkg_dir = tmp_path / "test-package" + + output_dir = output_dir.format(tmp_path) call_package_new( tmp_path, cli_runner, - output_dir=f"--output-dir={tmp_path}", + output_dir=output_dir, ) - pkg_dir = tmp_path / "test-package" + _verify_copyright_holder(pkg_dir) new_copyright = "Test Corp. " - cli_runner( + result = cli_runner( [ "-d", tmp_path, @@ -215,9 +225,19 @@ def test_package_update_copyright_holder( str(pkg_dir), ] ) + assert result.exit_code == 0 _verify_copyright_holder(pkg_dir, new_copyright) + if output_dir == "": + # Verify that we don't mess up the bare copy config with `package update`. This + # check only makes sense when upadting a component which is a worktree checkout + # (i.e. for output_dir=="" in this test). + p_repo = git.Repo(pkg_dir) + assert not p_repo.bare + md_repo = git.Repo(Path(p_repo.common_dir).resolve()) + assert md_repo.bare + @pytest.mark.parametrize( "initial_test_cases,additional_test_cases,remove_test_cases",