diff --git a/supervisor/exceptions.py b/supervisor/exceptions.py index 4fbb22caef9..c1d40daf998 100644 --- a/supervisor/exceptions.py +++ b/supervisor/exceptions.py @@ -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.""" diff --git a/supervisor/store/__init__.py b/supervisor/store/__init__.py index aefd7d05d79..a5da4d6020d 100644 --- a/supervisor/store/__init__.py +++ b/supervisor/store/__init__.py @@ -10,6 +10,7 @@ StoreError, StoreGitCloneError, StoreGitError, + StoreGitRemoteURLUpdateError, StoreInvalidAppRepo, StoreJobError, StoreNotFound, @@ -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: diff --git a/supervisor/store/const.py b/supervisor/store/const.py index 370e3e5605b..c173c3643a0 100644 --- a/supervisor/store/const.py +++ b/supervisor/store/const.py @@ -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}") diff --git a/supervisor/store/git.py b/supervisor/store/git.py index 61496caa337..dd9558aad70 100644 --- a/supervisor/store/git.py +++ b/supervisor/store/git.py @@ -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 @@ -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 @@ -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, @@ -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 + 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", @@ -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() @@ -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.""" @@ -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", @@ -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 diff --git a/supervisor/store/repository.py b/supervisor/store/repository.py index 6f718d91a1d..103dd8c4a75 100644 --- a/supervisor/store/repository.py +++ b/supervisor/store/repository.py @@ -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, @@ -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 ) @staticmethod diff --git a/tests/store/test_builtin_stores.py b/tests/store/test_builtin_stores.py index 74d140c8786..64ad0fa8079 100644 --- a/tests/store/test_builtin_stores.py +++ b/tests/store/test_builtin_stores.py @@ -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: @@ -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 diff --git a/tests/store/test_custom_repository.py b/tests/store/test_custom_repository.py index 0b2e5356c9c..b23ea22a027 100644 --- a/tests/store/test_custom_repository.py +++ b/tests/store/test_custom_repository.py @@ -11,6 +11,7 @@ StoreError, StoreGitCloneError, StoreGitError, + StoreGitRemoteURLUpdateError, StoreJobError, StoreNotFound, ) @@ -164,6 +165,23 @@ 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"), [ @@ -171,6 +189,7 @@ async def test_add_repository_with_git_error( (True, StoreGitError()), (False, StoreGitCloneError()), (False, StoreGitError()), + (False, StoreGitRemoteURLUpdateError()), ], ) async def test_error_on_repository_with_git_error( diff --git a/tests/store/test_repository_git.py b/tests/store/test_repository_git.py index 339800e92f2..04b076fe923 100644 --- a/tests/store/test_repository_git.py +++ b/tests/store/test_repository_git.py @@ -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", [