Make the backing MultiDependency for Component non-optional

commodore/component/__init__.py

@@ -15,7 +15,7 @@
 class Component:
     _name: str
     _repo: Optional[GitRepo]
-    _dependency: Optional[MultiDependency] = None
+    _dependency: MultiDependency
     _version: Optional[str] = None
     _dir: P
     _sub_path: str
@@ -38,7 +38,7 @@ def clone(cls, cfg, clone_url: str, name: str, version: str = "master"):
     def __init__(
         self,
         name: str,
-        dependency: Optional[MultiDependency],
+        dependency: MultiDependency,
         work_dir: Optional[P] = None,
         version: Optional[str] = None,
         directory: Optional[P] = None,
@@ -53,9 +53,8 @@ def __init__(
             raise click.ClickException(
                 "Either `work_dir` or `directory` must be provided."
             )
-        if dependency:
-            self._dependency = dependency
-            self._dependency.register_component(self.name, self._dir)
+        self._dependency = dependency
+        self._dependency.register_component(self.name, self._dir)
         self.version = version
         self._sub_path = sub_path
         self._repo = None
@@ -69,18 +68,11 @@ def name(self) -> str:
     @property
     def repo(self) -> GitRepo:
         if not self._repo:
-            if self._dependency:
-                dep_repo = self._dependency.bare_repo
-                author_name = (
-                    dep_repo.author.name if hasattr(dep_repo, "author") else None
-                )
-                author_email = (
-                    dep_repo.author.email if hasattr(dep_repo, "author") else None
-                )
-            else:
-                # Fall back to author detection if we don't have a dependency
-                author_name = None
-                author_email = None
+            dep_repo = self._dependency.bare_repo
+            author_name = dep_repo.author.name if hasattr(dep_repo, "author") else None
+            author_email = (
+                dep_repo.author.email if hasattr(dep_repo, "author") else None
+            )
             self._repo = GitRepo(
                 None,
                 self._dir,
@@ -91,19 +83,13 @@ def repo(self) -> GitRepo:
 
     @property
     def dependency(self) -> MultiDependency:
-        if self._dependency is None:
-            raise ValueError(
-                f"Dependency for component {self._name} hasn't been initialized"
-            )
         return self._dependency
 
     @dependency.setter
-    def dependency(self, dependency: Optional[MultiDependency]):
+    def dependency(self, dependency: MultiDependency):
         """Update the GitRepo backing the component"""
-        if self._dependency:
-            self._dependency.deregister_component(self.name)
-        if dependency:
-            dependency.register_component(self.name, self._dir)
+        self._dependency.deregister_component(self.name)
+        dependency.register_component(self.name, self._dir)
         self._dependency = dependency
         # Clear worktree GitRepo wrapper when we update the component's backing
         # dependency. The new GitRepo wrapper will be created on the nex access of
@@ -112,10 +98,6 @@ def dependency(self, dependency: Optional[MultiDependency]):
 
     @property
     def repo_url(self) -> str:
-        if self._dependency is None:
-            raise ValueError(
-                f"Dependency for component {self._name} hasn't been initialized"
-            )
         return self._dependency.url
 
     @property
@@ -155,8 +137,6 @@ def defaults_file(self) -> P:
         return self.alias_defaults_file(self.name)
 
     def alias_directory(self, alias: str) -> P:
-        if not self._dependency:
-            return self._dir / self._sub_path
         apath = self._dependency.get_component(alias)
         if not apath:
             raise ValueError(f"unknown alias {alias} for component {self.name}")
@@ -204,27 +184,29 @@ def parameters_key(self):
         return component_parameters_key(self.name)
 
     def checkout(self):
-        if self._dependency is None:
-            raise ValueError(
-                f"Dependency for component {self._name} hasn't been initialized"
-            )
         self._dependency.checkout_component(self.name, self.version)
 
-    def register_alias(self, alias: str, version: str, sub_path: str = ""):
-        if not self._work_dir:
-            raise ValueError(
-                f"Can't register alias on component {self.name} "
-                + "which isn't configured with a working directory"
-            )
+    def register_alias(
+        self,
+        alias: str,
+        version: str,
+        sub_path: str = "",
+        target_dir: Optional[P] = None,
+    ):
+        alias_target_dir = target_dir
+        if not alias_target_dir:
+            if not self._work_dir:
+                raise ValueError(
+                    f"Can't register alias on component {self.name} "
+                    + "which isn't configured with a working directory"
+                )
+            alias_target_dir = component_dir(self._work_dir, alias)
         if alias in self._aliases:
             raise ValueError(
                 f"alias {alias} already registered on component {self.name}"
             )
         self._aliases[alias] = (version, sub_path)
-        if self._dependency:
-            self._dependency.register_component(
-                alias, component_dir(self._work_dir, alias)
-            )
+        self._dependency.register_component(alias, alias_target_dir)
 
     def checkout_alias(
         self, alias: str, alias_dependency: Optional[MultiDependency] = None
@@ -236,22 +218,17 @@ def checkout_alias(
 
         if alias_dependency:
             alias_dependency.checkout_component(alias, self._aliases[alias][0])
-        elif self._dependency:
+        else:
             self._dependency.checkout_component(alias, self._aliases[alias][0])
 
     def is_checked_out(self) -> bool:
         return self.target_dir is not None and self.target_dir.is_dir()
 
     def checkout_is_dirty(self) -> bool:
-        if self._dependency:
-            dep_repo = self._dependency.bare_repo
-            author_name = dep_repo.author.name
-            author_email = dep_repo.author.email
-            worktree = self._dependency.get_component(self.name)
-        else:
-            author_name = None
-            author_email = None
-            worktree = self.target_dir
+        dep_repo = self._dependency.bare_repo
+        author_name = dep_repo.author.name
+        author_email = dep_repo.author.email
+        worktree = self._dependency.get_component(self.name)
 
         if worktree and worktree.is_dir():
             r = GitRepo(

commodore/component/compile.py

@@ -22,6 +22,7 @@
 from commodore.helpers import kapitan_inventory, kapitan_compile, relsymlink, yaml_dump
 from commodore.inventory import Inventory
 from commodore.inventory.lint import check_removed_reclass_variables
+from commodore.multi_dependency import MultiDependency
 from commodore.postprocess import postprocess_components
 
 
@@ -142,16 +143,21 @@ def _setup_component(
         target_dir = P(cr.working_tree_dir)
         # compute subpath from Repo working tree dir and component path
         sub_path = str(component_path.absolute().relative_to(target_dir))
+        cdep = MultiDependency(cr.remote().url, target_dir.parent)
     except git.InvalidGitRepositoryError:
         click.echo(" > Couldn't determine Git repository for component")
         # Just treat `component_path` as a directory holding a component, don't care
         # about Git repo details here.
         target_dir = component_path
         sub_path = ""
+        # We need a MultiDependency, but it doesn't matter what it's contents are, since
+        # we never actually call any methods backed by the MultiDependency in
+        # `component_compile()`.
+        cdep = MultiDependency("https://fake.example.org", component_path)
 
     component = Component(
         component_name,
-        None,
+        cdep,
         # Use repo working tree as component "target directory", otherwise we get messy
         # results with duplicate subpaths.
         directory=target_dir,
@@ -161,6 +167,10 @@ def _setup_component(
     )
     config.register_component(component)
     config.register_component_aliases({instance_name: component_name})
+    if instance_name != component_name:
+        component.register_alias(
+            instance_name, "", sub_path=sub_path, target_dir=target_dir
+        )
 
     # Validate component libraries
     for lib in component.lib_files:

tests/test_component.py

@@ -416,95 +416,40 @@ def test_component_get_library(tmp_path: P, libfiles: Iterable[str]):
         assert c.get_library(f) == tmp_path / "tc1" / "lib" / f
 
 
-def test_component_no_dep_get_dependency(tmp_path: P):
-    c = Component("test-component", None, directory=tmp_path)
-    with pytest.raises(ValueError) as e:
-        _ = c.dependency
-
-    assert (
-        str(e.value)
-        == "Dependency for component test-component hasn't been initialized"
-    )
-
-
-def test_component_no_dep_get_url(tmp_path: P):
-    c = Component("test-component", None, directory=tmp_path)
-    with pytest.raises(ValueError) as e:
-        _ = c.repo_url
-
-    assert (
-        str(e.value)
-        == "Dependency for component test-component hasn't been initialized"
-    )
-
-
-def test_component_no_dep_checkout(tmp_path: P):
-    c = Component("test-component", None, directory=tmp_path)
-    with pytest.raises(ValueError) as e:
-        c.checkout()
-
-    assert (
-        str(e.value)
-        == "Dependency for component test-component hasn't been initialized"
-    )
-
-
-@pytest.mark.parametrize("init_dep", [False, True])
-@pytest.mark.parametrize("new_dep", [False, True])
-def test_component_update_dependency(tmp_path: P, init_dep: bool, new_dep: bool):
+def test_component_update_dependency(tmp_path: P):
     r1 = _setup_existing_component(tmp_path / "tc1")
     r2 = _setup_existing_component(tmp_path / "tc2")
 
-    idep = None
-    if init_dep:
-        idep = MultiDependency(f"file://{r1.common_dir}", tmp_path / "dependencies")
+    idep = MultiDependency(f"file://{r1.common_dir}", tmp_path / "dependencies")
 
     c = Component("tc1", idep, tmp_path)
 
-    if init_dep:
-        c.checkout()
-        assert c.dependency == idep
-        assert c.repo_url == idep.url
-        assert c.repo.repo.head.commit.hexsha == r1.head.commit.hexsha
-    else:
-        with pytest.raises(ValueError) as exc:
-            _ = c.dependency
-        assert str(exc.value) == "Dependency for component tc1 hasn't been initialized"
-
-    ndep = None
-    if new_dep:
-        ndep = MultiDependency(f"file://{r2.common_dir}", tmp_path / "dependencies")
+    c.checkout()
+    assert c.dependency == idep
+    assert c.repo_url == idep.url
+    assert c.repo.repo.head.commit.hexsha == r1.head.commit.hexsha
 
+    ndep = MultiDependency(f"file://{r2.common_dir}", tmp_path / "dependencies")
     c.dependency = ndep
 
-    if ndep:
-        c.checkout()
-        assert c.dependency == ndep
-        assert c.repo_url == ndep.url
-        assert c.repo.repo.head.commit.hexsha == r2.head.commit.hexsha
-    else:
-        with pytest.raises(ValueError) as exc:
-            _ = c.dependency
-        assert str(exc.value) == "Dependency for component tc1 hasn't been initialized"
+    c.checkout()
+    assert c.dependency == ndep
+    assert c.repo_url == ndep.url
+    assert c.repo.repo.head.commit.hexsha == r2.head.commit.hexsha
 
 
-@pytest.mark.parametrize("dep", [True, False])
-def test_component_repo(tmp_path: P, dep: bool):
+def test_component_repo(tmp_path: P):
     u = Repo.init(tmp_path / "bare.git")
     (tmp_path / "bare.git" / "x").touch()
     u.index.add(["x"])
     u.index.commit("Initial commit")
 
-    if dep:
-        md = MultiDependency(f"file://{tmp_path}/bare.git", tmp_path)
-    else:
-        md = None
+    md = MultiDependency(f"file://{tmp_path}/bare.git", tmp_path)
 
     c = Component(
         "test-component", md, directory=tmp_path / "test-component", version="master"
     )
-    if md:
-        c.checkout()
+    c.checkout()
 
     assert c.repo.working_tree_dir == tmp_path / "test-component"
 
@@ -527,6 +472,39 @@ def test_checkout_is_dirty(tmp_path: P, config: Config):
 
     c = Component.clone(config, clone_url, "test-component")
     c.checkout()
-    c._dependency = None
 
     assert not c.checkout_is_dirty()
+
+
+@pytest.mark.parametrize("workdir", [True, False])
+def test_component_register_alias_workdir(tmp_path: P, workdir: bool):
+    c = _setup_component(tmp_path, name="test-component")
+    assert not c.work_directory
+    if workdir:
+        c._work_dir = tmp_path
+
+    if not workdir:
+        with pytest.raises(ValueError) as exc:
+            c.register_alias("test-alias", c.version)
+
+        assert (
+            "Can't register alias on component test-component which isn't configured with a working directory"
+            in str(exc.value)
+        )
+    else:
+        c.register_alias("test-alias", c.version)
+        assert (
+            c.alias_directory("test-alias") == tmp_path / "dependencies" / "test-alias"
+        )
+
+
+@pytest.mark.parametrize("workdir", [True, False])
+def test_component_register_alias_targetdir(tmp_path: P, workdir: bool):
+    c = _setup_component(tmp_path, name="test-component")
+    assert not c.work_directory
+    if workdir:
+        c._work_dir = tmp_path
+
+    c.register_alias("test-alias", c.version, target_dir=tmp_path / "test-alias")
+    # explicit `target_dir` has precedence over the component's _work_dir
+    assert c.alias_directory("test-alias") == tmp_path / "test-alias"