Skip to content

Commit

Permalink
Make the backing MultiDependency for Component non-optional
Browse files Browse the repository at this point in the history
This simplifies a lot of the implementation for `Component` and only
surfaces in a minor way in `component_compile` where we may need to
create a fake `MultiDependency` if we compile a component outside a Git
repo.
  • Loading branch information
simu committed Feb 14, 2025
1 parent 6217d08 commit 19906ca
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 127 deletions.
89 changes: 33 additions & 56 deletions commodore/component/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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}")
Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand Down
12 changes: 11 additions & 1 deletion commodore/component/compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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,
Expand All @@ -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:
Expand Down
84 changes: 14 additions & 70 deletions tests/test_component.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -527,6 +472,5 @@ 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()

0 comments on commit 19906ca

Please sign in to comment.