diff --git a/script/git/git_tool.py b/script/git/git_tool.py index cbf68fd..446223a 100644 --- a/script/git/git_tool.py +++ b/script/git/git_tool.py @@ -116,6 +116,48 @@ class GitTool: repo_path=repo_path, add_root=add_root ) + def _with_root_repo( + self, repos: list, repo_path: str, add_root: bool + ) -> list: + """`repos` précédée du dépôt racine si `add_root`, puis triée. + + TOUS les chemins de sortie passent par ici. Le dépôt racine ne + dépend ni du manifeste ni des sous-modules : un retour anticipé qui + saute cette étape rend une liste VIDE alors que la racine était + demandée, et l'appelant en conclut qu'il n'a rien à faire. Sur un + checkout sans `.repo`, le remote du dépôt principal n'était alors + jamais changé, sans qu'aucun message ne le signale. + + Une origine absente vaut l'URL par défaut plutôt qu'une exception : + la liste sert à RÉÉCRIRE les remotes, et un dépôt sans origine est + précisément un de ceux qu'on vient corriger. + """ + if not add_root: + return sorted(repos, key=lambda k: k.get("name")) + repo_root = Repo(repo_path) + try: + url = repo_root.git.remote("get-url", "origin") + except Exception: + print( + f"{Fore.YELLOW}WARNING{Style.RESET_ALL}: Missing origin" + f" remote, use default url {DEFAULT_REMOTE_URL}. Suggest" + " to add a remote origin: \n> git remote add origin" + f" {DEFAULT_REMOTE_URL}" + ) + url = DEFAULT_REMOTE_URL + url, url_https, url_git = self.get_url(url) + repos.insert( + 0, + { + "url": url, + "url_https": url_https, + "url_git": url_git, + "path": repo_path, + "name": "", + }, + ) + return sorted(repos, key=lambda k: k.get("name")) + def get_repo_info_submodule( self, repo_path: str = ".", add_root: bool = False ) -> list: @@ -181,22 +223,7 @@ class GitTool: } repos.append(data) - if add_root: - repo_root = Repo(repo_path) - url = repo_root.git.remote("get-url", "origin") - url, url_https, url_git = self.get_url(url) - - data = { - "url": url, - "url_https": url_https, - "url_git": url_git, - "path": repo_path, - "name": "", - } - repos.insert(0, data) - # Sort - repos = sorted(repos, key=lambda k: k.get("name")) - return repos + return self._with_root_repo(repos, repo_path, add_root) def get_repo_info_manifest_xml( self, repo_path: str = ".", add_root: bool = False, filter_group=None @@ -219,7 +246,7 @@ class GitTool: filter_groups = filter_group.split(",") if filter_group else [] manifest_file = self.get_manifest_file(repo_path=repo_path) if not manifest_file: - return [] + return self._with_root_repo([], repo_path, add_root) if os.path.isabs(manifest_file): # This is a absolute path filename = manifest_file @@ -231,7 +258,7 @@ class GitTool: xml_dict = xmltodict.parse(xml_as_string) manifest_data = xml_dict.get("manifest") if not manifest_data: - return [] + return self._with_root_repo([], repo_path, add_root) if manifest_data.get("default"): default_remote = manifest_data.get("default").get("@remote") else: @@ -274,31 +301,7 @@ class GitTool: } repos.append(data) - if add_root: - repo_root = Repo(repo_path) - try: - url = repo_root.git.remote("get-url", "origin") - except Exception as e: - print( - f"{Fore.YELLOW}WARNING{Style.RESET_ALL}: Missing origin" - f" remote, use default url {DEFAULT_REMOTE_URL}. Suggest" - " to add a remote origin: \n> git remote add origin" - f" {DEFAULT_REMOTE_URL}" - ) - url = DEFAULT_REMOTE_URL - url, url_https, url_git = self.get_url(url) - - data = { - "url": url, - "url_https": url_https, - "url_git": url_git, - "path": repo_path, - "name": "", - } - repos.insert(0, data) - # Sort - repos = sorted(repos, key=lambda k: k.get("name")) - return repos + return self._with_root_repo(repos, repo_path, add_root) def get_manifest_xml_info( self, repo_path: str = ".", filename=None, add_root: bool = False diff --git a/test/test_git_tool.py b/test/test_git_tool.py index 7fa6f7d..c38c86c 100644 --- a/test/test_git_tool.py +++ b/test/test_git_tool.py @@ -2,10 +2,13 @@ # © 2026 TechnoLibre (http://www.technolibre.ca) # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) +import io import os +import subprocess import tempfile import unittest from collections import OrderedDict +from contextlib import redirect_stdout from unittest.mock import MagicMock, mock_open, patch from script.git.git_tool import ( @@ -265,6 +268,80 @@ class TestGetRepoInfoSubmodule(unittest.TestCase): self.assertIn("git@", result[0]["url_git"]) +class TestTheRootRepoSurvivesAMissingManifest(unittest.TestCase): + """Le dépôt racine ne dépend ni du manifeste ni des sous-modules. + + Une liste vide là où la racine était demandée fait conclure à + l'appelant qu'il n'a rien à faire : sur un checkout sans `.repo`, le + remote du dépôt principal n'était jamais réécrit, sans message. + """ + + def _depot(self, chemin, origine="https://github.com/ERPLibre/ERPLibre"): + """Un vrai dépôt git, sans manifeste. Rien n'est bouchonné : la + lecture de l'origine passe par git, et c'est elle qu'on vérifie.""" + subprocess.run(["git", "init", "-q", chemin], check=True) + if origine: + subprocess.run( + ["git", "-C", chemin, "remote", "add", "origin", origine], + check=True, + ) + return chemin + + def test_without_a_manifest_the_root_is_still_returned(self): + gt = GitTool() + with tempfile.TemporaryDirectory() as tmpdir: + self._depot(tmpdir) + result = gt.get_repo_info(tmpdir, add_root=True) + self.assertEqual(len(result), 1) + self.assertEqual(result[0]["name"], "") + self.assertEqual( + result[0]["url_git"], "git@github.com:ERPLibre/ERPLibre" + ) + + def test_without_the_root_asked_nothing_is_returned(self): + """Deux appelants internes lisent le manifeste SANS la racine : le + correctif ne doit rien rendre à qui n'en veut pas.""" + gt = GitTool() + with tempfile.TemporaryDirectory() as tmpdir: + self._depot(tmpdir) + self.assertEqual(gt.get_repo_info(tmpdir), []) + + def test_a_root_without_origin_falls_back_instead_of_raising(self): + """La liste sert à RÉÉCRIRE les remotes : un dépôt sans origine est + précisément un de ceux qu'on vient corriger.""" + gt = GitTool() + with tempfile.TemporaryDirectory() as tmpdir: + self._depot(tmpdir, origine="") + with redirect_stdout(io.StringIO()): + result = gt.get_repo_info(tmpdir, add_root=True) + self.assertEqual(len(result), 1) + self.assertTrue(result[0]["url_git"].startswith("git@")) + + def test_the_root_comes_first_alongside_the_manifest_projects(self): + """La racine porte un nom vide : le tri la met en tête, et le + script qui réécrit les remotes commence donc par elle.""" + xml_content = ( + '\n' + "\n" + ' \n' + ' \n' + ' \n' + "\n" + ) + gt = GitTool() + with tempfile.TemporaryDirectory() as tmpdir: + self._depot(tmpdir) + manifeste = os.path.join(tmpdir, "manifest_test.xml") + with open(manifeste, "w") as fh: + fh.write(xml_content) + with patch.object( + GitTool, "get_manifest_file", return_value=manifeste + ): + result = gt.get_repo_info(tmpdir, add_root=True) + self.assertEqual([r["name"] for r in result], ["", "addons/OCA_web"]) + + class TestGetManifestXmlInfo(unittest.TestCase): def test_parses_manifest(self): xml_content = """