Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions supervisor/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1269,6 +1269,10 @@ class StoreGitError(StoreError):
"""Raise if something on git is happening."""


class StoreGitRemoteURLUpdateError(StoreGitError):
"""Raise if updating a repository remote URL fails."""


class StoreGitCloneError(StoreGitError):
"""Raise if error occurred while cloning repository."""

Expand Down
10 changes: 10 additions & 0 deletions supervisor/store/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
StoreError,
StoreGitCloneError,
StoreGitError,
StoreGitRemoteURLUpdateError,
StoreInvalidAppRepo,
StoreJobError,
StoreNotFound,
Expand Down Expand Up @@ -166,6 +167,15 @@ async def _add_repository(
await repository.remove()
raise err

except StoreGitRemoteURLUpdateError as err:
if issue_on_error:
_LOGGER.warning(
"Can't update origin URL for repository %s: %s", url, err
)
else:
await repository.remove()
raise err

except StoreGitError as err:
_LOGGER.error("Can't load data from repository %s due to %s", url, err)
if issue_on_error:
Expand Down
14 changes: 14 additions & 0 deletions supervisor/store/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,17 @@ def git_url(self) -> str:
if self == BuiltinRepository.CORE:
return URL_HASSIO_APPS
return self.value # For URL-based repos, value is the URL

@property
def slug(self) -> str:
"""Return fixed slug for this built-in repository."""
if self in (BuiltinRepository.LOCAL, BuiltinRepository.CORE):
return self.value
if self == BuiltinRepository.COMMUNITY_APPS:
return "a0d7b954"
if self == BuiltinRepository.ESPHOME:
return "5c53de3b"
if self == BuiltinRepository.MUSIC_ASSISTANT:
return "d5369777"

raise RuntimeError(f"Unknown built-in repository: {self}")
73 changes: 62 additions & 11 deletions supervisor/store/git.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Init file for Supervisor app Git."""

import asyncio
from contextlib import suppress
import functools as ft
import logging
from pathlib import Path
Expand All @@ -10,7 +11,12 @@

from ..const import ATTR_BRANCH, ATTR_URL
from ..coresys import CoreSys, CoreSysAttributes
from ..exceptions import StoreGitCloneError, StoreGitError, StoreJobError
from ..exceptions import (
StoreGitCloneError,
StoreGitError,
StoreGitRemoteURLUpdateError,
StoreJobError,
)
from ..jobs.decorator import Job, JobCondition
from ..resolution.const import ContextType, IssueType, SuggestionType
from ..utils import directory_missing_or_empty, remove_folder
Expand Down Expand Up @@ -57,7 +63,10 @@ async def load(self) -> None:
async with self.lock:
try:
_LOGGER.info("Loading app %s repository", self.path)
self.repo = await self.sys_run_in_executor(git.Repo, str(self.path))
repo: git.Repo = await self.sys_run_in_executor(
git.Repo, str(self.path)
)
self.repo = repo

except (
git.InvalidGitRepositoryError,
Expand All @@ -70,12 +79,51 @@ async def load(self) -> None:

# Fix possible corruption
async with self.lock:
_LOGGER.debug("Integrity check app %s repository", self.path)
await self.sys_run_in_executor(self._sync_origin_remote_url_and_fsck, repo)

def _sync_origin_remote_url_and_fsck(self, repo: git.Repo) -> None:
"""Sync origin URL and run fsck in a single executor invocation."""
self._sync_origin_remote_url(repo)
try:
repo.git.execute(["git", "fsck"])
except (
git.InvalidGitRepositoryError,
git.NoSuchPathError,
git.CommandError,
UnicodeDecodeError,
) as err:
_LOGGER.error("Integrity check on %s failed: %s.", self.path, err)
raise StoreGitError from err

def _sync_origin_remote_url(self, repo: git.Repo) -> None:
"""Ensure the clone's origin URL matches the configured repository URL."""
remotes = {remote.name for remote in repo.remotes}
if "origin" not in remotes:
return

origin = repo.remotes.origin

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accessing repo.remotes or origin.url properties can actually raise exceptions (corrupted repository), so they should be within the try block. So far a load did not touch .git/config, so this can really be a regression.

if origin.url != self.url:
try:
_LOGGER.debug("Integrity check app %s repository", self.path)
await self.sys_run_in_executor(self.repo.git.execute, ["git", "fsck"])
except git.CommandError as err:
_LOGGER.error("Integrity check on %s failed: %s.", self.path, err)
raise StoreGitError from err
_LOGGER.info(
"Updating app %s repository origin URL from %s to %s",
self.path,
origin.url,
self.url,
)
origin.set_url(self.url)
except (
git.InvalidGitRepositoryError,
git.NoSuchPathError,
git.CommandError,
UnicodeDecodeError,
) as err:
_LOGGER.warning(
"Failed to update app %s repository origin URL: %s",
self.path,
err,
)
raise StoreGitRemoteURLUpdateError from err

@Job(
name="git_repo_clone",
Expand Down Expand Up @@ -168,8 +216,12 @@ async def pull(self) -> bool:
_LOGGER.warning("No valid repository for %s", self.url)
return False

repo: git.Repo = self.repo

async with self.lock:
_LOGGER.info("Update app %s repository from %s", self.path, self.url)
with suppress(StoreGitRemoteURLUpdateError):
await self.sys_run_in_executor(self._sync_origin_remote_url, repo)

try:
git_cmd = git.Git()
Expand All @@ -179,7 +231,6 @@ async def pull(self) -> bool:
raise StoreGitError from err

try:
repo = self.repo

def _fetch_and_check() -> tuple[str, bool]:
"""Fetch from origin and check if changed."""
Expand All @@ -197,13 +248,13 @@ def _fetch_and_check() -> tuple[str, bool]:
if changed:
# Jump on top of that
await self.sys_run_in_executor(
ft.partial(self.repo.git.reset, f"origin/{branch}", hard=True)
ft.partial(repo.git.reset, f"origin/{branch}", hard=True)
)

# Update submodules
await self.sys_run_in_executor(
ft.partial(
self.repo.git.submodule,
repo.git.submodule,
"update",
"--init",
"--recursive",
Expand All @@ -213,7 +264,7 @@ def _fetch_and_check() -> tuple[str, bool]:
)

# Cleanup old data
await self.sys_run_in_executor(ft.partial(self.repo.git.clean, "-xdf"))
await self.sys_run_in_executor(ft.partial(repo.git.clean, "-xdf"))

return changed

Expand Down
19 changes: 4 additions & 15 deletions supervisor/store/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,7 @@

import voluptuous as vol

from ..const import (
ATTR_MAINTAINER,
ATTR_NAME,
ATTR_URL,
FILE_SUFFIX_CONFIGURATION,
REPOSITORY_CORE,
REPOSITORY_LOCAL,
)
from ..const import ATTR_MAINTAINER, ATTR_NAME, ATTR_URL, FILE_SUFFIX_CONFIGURATION
from ..coresys import CoreSys, CoreSysAttributes
from ..exceptions import (
ConfigurationFileError,
Expand Down Expand Up @@ -57,18 +50,14 @@ def create(coresys: CoreSys, repository: str) -> Repository:
def _create_builtin(coresys: CoreSys, builtin: BuiltinRepository) -> Repository:
"""Create builtin repository."""
if builtin == BuiltinRepository.LOCAL:
slug = REPOSITORY_LOCAL
local_path = coresys.config.path_apps_local
return RepositoryLocal(coresys, local_path, slug)
return RepositoryLocal(coresys, local_path, builtin.slug)
if builtin == BuiltinRepository.CORE:
slug = REPOSITORY_CORE
local_path = coresys.config.path_apps_core
else:
# For other builtin repositories (URL-based)
slug = get_hash_from_repository(builtin.value)
local_path = coresys.config.path_apps_git / slug
local_path = coresys.config.path_apps_git / builtin.slug
return RepositoryGitBuiltin(
coresys, builtin.value, local_path, slug, builtin.git_url
coresys, builtin.value, local_path, builtin.slug, builtin.git_url
)
Comment thread
mdegat01 marked this conversation as resolved.

@staticmethod
Expand Down
22 changes: 22 additions & 0 deletions tests/store/test_builtin_stores.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
"""Test local and core store."""

import pytest

from supervisor.coresys import CoreSys
from supervisor.store.const import BuiltinRepository
from supervisor.store.repository import Repository


def test_local_store(coresys: CoreSys, test_repository) -> None:
Expand All @@ -15,3 +19,21 @@ def test_core_store(coresys: CoreSys, test_repository) -> None:
assert coresys.store.get("core")

assert "core_samba" in coresys.apps.store


@pytest.mark.parametrize(
("builtin", "slug"),
[
(BuiltinRepository.LOCAL, "local"),
(BuiltinRepository.CORE, "core"),
(BuiltinRepository.COMMUNITY_APPS, "a0d7b954"),
(BuiltinRepository.ESPHOME, "5c53de3b"),
(BuiltinRepository.MUSIC_ASSISTANT, "d5369777"),
],
)
def test_builtin_repository_has_fixed_slug(
coresys: CoreSys, builtin: BuiltinRepository, slug: str
) -> None:
"""Test built-in repository slugs are fixed and independent from URL hashing."""
assert builtin.slug == slug
assert Repository.create(coresys, builtin.value).slug == slug
19 changes: 19 additions & 0 deletions tests/store/test_custom_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
StoreError,
StoreGitCloneError,
StoreGitError,
StoreGitRemoteURLUpdateError,
StoreJobError,
StoreNotFound,
)
Expand Down Expand Up @@ -164,13 +165,31 @@ async def test_add_repository_with_git_error(
assert coresys.resolution.suggestions[-1].type == suggestion_type


async def test_add_repository_with_remote_url_update_error_on_startup(
coresys: CoreSys, store_manager: StoreManager
):
"""Test startup path ignores remote URL update errors."""
current = coresys.store.repository_urls
with patch(
"supervisor.store.git.GitRepo.load",
side_effect=StoreGitRemoteURLUpdateError(),
):
await store_manager.update_repositories(
set(current) | {"http://example.com"}, issue_on_error=True
)

assert "http://example.com" in coresys.store.repository_urls
assert len(coresys.resolution.suggestions) == 0


@pytest.mark.parametrize(
("use_update", "git_error"),
[
(True, StoreGitCloneError()),
(True, StoreGitError()),
(False, StoreGitCloneError()),
(False, StoreGitError()),
(False, StoreGitRemoteURLUpdateError()),
],
)
async def test_error_on_repository_with_git_error(
Expand Down
29 changes: 29 additions & 0 deletions tests/store/test_repository_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,35 @@ async def test_git_load(coresys: CoreSys, tmp_path: Path):
assert mock_repo.call_count == 1


async def test_git_load_updates_origin_remote_url(coresys: CoreSys, tmp_path: Path):
"""Test git load updates an existing origin remote URL."""

class MockRemotes(list):
"""Minimal remote container with GitPython-style origin attribute."""

@property
def origin(self):
"""Return origin remote."""
return self[0]

repo = GitRepo(coresys, tmp_path, REPO_URL)

# Pretend we have a repo
(tmp_path / ".git").mkdir()

origin = MagicMock()
origin.name = "origin"
origin.url = "https://github.com/awesome-developer/old-repo"

mock_repo = MagicMock()
mock_repo.remotes = MockRemotes([origin])

with patch("git.Repo", return_value=mock_repo):
await repo.load()

origin.set_url.assert_called_once_with(REPO_URL)


@pytest.mark.parametrize(
"git_errors",
[
Expand Down