Merge pull request #427 from lomnido/feat-report-missing-remote

introducing reporting of (missing remote) for DM and FM

tsrc/manifest.py

@@ -11,7 +11,7 @@
 from tsrc.errors import Error, InvalidConfigError, LoadManifestSchemaError
 from tsrc.file_system import Copy, FileSystemOperation, Link
 from tsrc.groups import GroupList
-from tsrc.manifest_common_data import ManifestsTypeOfData
+from tsrc.manifest_common_data import ManifestsTypeOfData, mtod_can_ignore_remotes
 from tsrc.repo import Remote, Repo
 
 
@@ -194,6 +194,32 @@ def validate_repo(data: Any) -> None:
         )
 
 
+def validate_repo_no_remote_required(data: Any) -> None:
+    copy_schema = {"file": str, schema.Optional("dest"): str}
+    symlink_schema = {"source": str, "target": str}
+    remote_schema = {"name": str, "url": str}
+    repo_schema = schema.Schema(
+        {
+            "dest": str,
+            schema.Optional("branch"): str,
+            schema.Optional("copy"): [copy_schema],
+            schema.Optional("symlink"): [symlink_schema],
+            schema.Optional("sha1"): str,
+            schema.Optional("tag"): str,
+            schema.Optional("ignore_submodules"): bool,
+            schema.Optional("remotes"): [remote_schema],
+            schema.Optional("url"): str,
+        }
+    )
+    repo_schema.validate(data)
+    url = data.get("url")
+    remotes = data.get("remotes")
+    if url and remotes:
+        raise schema.SchemaError(
+            "Repo config cannot contain both an url and a list of remotes"
+        )
+
+
 def load_manifest(manifest_path: Path) -> Manifest:
     """Main entry point: return a manifest instance by parsing
     a `manifest.yml` file.
@@ -229,7 +255,10 @@ def load_manifest_safe_mode(manifest_path: Path, mtod: ManifestsTypeOfData) -> M
     ignore such Repo (do not add it to Group).
     """
     remote_git_server_schema = {"url": str}
-    repo_schema = schema.Use(validate_repo)
+    if mtod in mtod_can_ignore_remotes():
+        repo_schema = schema.Use(validate_repo_no_remote_required)
+    else:
+        repo_schema = schema.Use(validate_repo)
     group_schema = {"repos": [str], schema.Optional("includes"): [str]}
     # Note: gitlab and github_enterprise_url keys are ignored,
     # and kept here only for backward compatibility reasons

tsrc/manifest_common_data.py

@@ -1,4 +1,5 @@
 from enum import Enum, unique
+from typing import List
 
 import cli_ui as ui
 
@@ -7,22 +8,36 @@
 class ManifestsTypeOfData(Enum):
     LOCAL = 1
     DEEP = 2
-    DEEP_BLOCK = 3
-    FUTURE = 4
+    DEEP_ON_UPDATE = 3  # do not put warning about missing element
+    DEEP_BLOCK = 4
+    FUTURE = 5
 
 
 def get_mtod_str(tod: ManifestsTypeOfData) -> str:
     if tod == ManifestsTypeOfData.LOCAL:
         return "Local Manifest"
     if tod == ManifestsTypeOfData.DEEP:
         return "Deep Manifest"
+    if tod == ManifestsTypeOfData.DEEP_ON_UPDATE:
+        return "Deep Manifest on UPDATE"
     if tod == ManifestsTypeOfData.DEEP_BLOCK:
         return "Deep Manifest's block"
     if tod == ManifestsTypeOfData.FUTURE:
         return "Future Manifest"
 
 
+def mtod_can_ignore_remotes() -> List[ManifestsTypeOfData]:
+    rl: List[ManifestsTypeOfData] = [
+        ManifestsTypeOfData.DEEP,
+        ManifestsTypeOfData.DEEP_ON_UPDATE,
+        ManifestsTypeOfData.DEEP_BLOCK,
+        ManifestsTypeOfData.FUTURE,
+    ]
+    return rl
+
+
 def get_main_color(tod: ManifestsTypeOfData) -> ui.Token:
+    # TODO: rename with prefix 'mtod'
     # for Local Manifest (using for Manifest's Marker color)
     if tod == ManifestsTypeOfData.LOCAL:
         return ui.reset

tsrc/repo.py

@@ -1,13 +1,23 @@
 """ Repo objects. """
 
 from dataclasses import dataclass
+from enum import Enum, unique
 from typing import List, Optional, Tuple
 
 import cli_ui as ui
 
 from tsrc.manifest_common_data import ManifestsTypeOfData, get_main_color
 
 
+@unique
+class DescribeToTokens(Enum):
+    NONE = 0
+    BRANCH = 1
+    SHA1 = 2
+    TAG = 3
+    MISSING_REMOTES = 4
+
+
 @dataclass(frozen=True)
 class Remote:
     name: str
@@ -51,50 +61,96 @@ def clone_url(self) -> str:
     """copy from 'git.py'"""
 
     def describe_to_tokens(
-        self, ljust: int = 0, tod: ManifestsTypeOfData = ManifestsTypeOfData.LOCAL
+        self, ljust: int = 0, mtod: ManifestsTypeOfData = ManifestsTypeOfData.LOCAL
     ) -> Tuple[List[ui.Token], List[ui.Token]]:
         """returns:
         1st: is properly left-align: for print
         2nd: is NOT align: for 1:1 comparsion"""
+
+        # 1st caluclate total length of all elements
+        sha1: str = ""
+        if self.sha1:
+            sha1 = self.sha1[:7]  # artificially shorten
+
+        present_dtt: List[DescribeToTokens] = []
+        if self.branch and (
+            self.is_default_branch is False or (not self.sha1 and not self.tag)
+        ):
+            present_dtt.append(DescribeToTokens.BRANCH)
+        elif self.sha1:
+            present_dtt.append(DescribeToTokens.SHA1)
+        if self.tag:
+            present_dtt.append(DescribeToTokens.TAG)
+        if not self.remotes:
+            # TODO: possibly consider FM as well
+            if mtod == ManifestsTypeOfData.DEEP or mtod == ManifestsTypeOfData.FUTURE:
+                present_dtt.append(DescribeToTokens.MISSING_REMOTES)
+        if not present_dtt:
+            present_dtt.append(DescribeToTokens.NONE)
+
+        # return res, able
+        return self._describe_to_token_output(present_dtt, ljust, mtod, sha1)
+
+    def _describe_to_token_output(
+        self,
+        present_dtt: List[DescribeToTokens],
+        ljust: int,
+        mtod: ManifestsTypeOfData,
+        sha1: str,
+    ) -> Tuple[ui.Token, ui.Token]:
         cb = ui.green  # color (for) branch
         cs = ui.red  # color (for) SHA1
         ct = ui.brown  # color (for) tag
-        if tod == ManifestsTypeOfData.DEEP or tod == ManifestsTypeOfData.FUTURE:
-            cb = cs = get_main_color(tod)
-
+        if mtod == ManifestsTypeOfData.DEEP or mtod == ManifestsTypeOfData.FUTURE:
+            cb = cs = get_main_color(mtod)
         res: List[ui.Token] = []
         able: List[ui.Token] = []
-        first_ljust = ljust
-        if self.tag:
-            first_ljust = 0
-        if self.branch and (
-            self.is_default_branch is False or (not self.sha1 and not self.tag)
-        ):
-            res += [cb, self.branch.ljust(first_ljust), ui.reset]
-            able += [ui.green, self.branch, ui.reset]
-            if first_ljust == 0:
+
+        # 2nd detect last element for 'ljust' to apply'
+        last_element: DescribeToTokens = present_dtt[-1]
+
+        # 3rd fill the 'res' and 'able'
+        for e in present_dtt:
+            this_ljust: int = 0
+            if e == last_element:
+                this_ljust = ljust
+            if e == DescribeToTokens.BRANCH and self.branch:
+                res += [cb, self.branch.ljust(this_ljust), ui.reset]
+                able += [ui.green, self.branch, ui.reset]
                 ljust -= len(self.branch) + 1
-        elif self.sha1:
-            sha1 = self.sha1[:7]  # artificially shorten
-            res += [cs, sha1.ljust(first_ljust), ui.reset]
-            able += [ui.red, sha1, ui.reset]
-            if first_ljust == 0:
+            elif e == DescribeToTokens.SHA1:
+                res += [cs, sha1.ljust(this_ljust), ui.reset]
+                able += [ui.red, sha1, ui.reset]
                 ljust -= len(sha1) + 1
-        if self.tag:
-            # we have to compensate for len("on ")
-            res += [ct, "on", self.tag.ljust(ljust - 3), ui.reset]
-            able += [ui.brown, "on", self.tag, ui.reset]
+            elif e == DescribeToTokens.TAG and self.tag:
+                res += [ct, "on", self.tag.ljust(this_ljust - 3), ui.reset]
+                able += [ui.brown, "on", self.tag, ui.reset]
+                ljust -= len(self.tag) + 1 + 2 + 1  # + " on "
+            elif e == DescribeToTokens.MISSING_REMOTES:
+                res += [ui.red, "(missing remote)".ljust(this_ljust), ui.reset]
+                able += [ui.red, "(missing remote)", ui.reset]
+                ljust -= 16 + 1  # len of "(missing remote) "
+            else:  # DescribeToTokens.NONE:
+                res += [" ".ljust(this_ljust)]
+                able += [" "]
         return res, able
 
-    def len_of_describe(self) -> int:
+    def len_of_describe(
+        self, mtod: ManifestsTypeOfData = ManifestsTypeOfData.LOCAL
+    ) -> int:
         len_: int = 0
         if self.branch and (
             self.is_default_branch is False or (not self.sha1 and not self.tag)
         ):
-            len_ += len(self.branch)
+            len_ += len(self.branch) + 1
         elif self.sha1:
             sha1 = self.sha1[:7]  # artificially shorten
-            len_ += len(sha1)
+            len_ += len(sha1) + 1
         if self.tag:
             len_ += len(self.tag) + 4  # " on "
+        if not self.remotes:
+            if mtod == ManifestsTypeOfData.DEEP or mtod == ManifestsTypeOfData.FUTURE:
+                len_ += 16 + 1
+        if len_ > 0:
+            len_ -= 1
         return len_

tsrc/test/cli/test_display_dm_fm_mm.py

@@ -131,7 +131,7 @@ def ad_hoc_update_dm_dest__for_status_2_x_mm(
         if isinstance(value, List):
             for x in value:
                 if isinstance(x, ruamel.yaml.comments.CommentedMap):
-                    if x["dest"] == "manifest":
+                    if "dest" in x and x["dest"] == "manifest":
                         x["dest"] = "FM_destination"
     # write the file down
     with open(manifest_path, "w") as file:
@@ -242,8 +242,8 @@ def ad_hoc_update_dm__for_status_dm_fm(
         if isinstance(value, List):
             for x in value:
                 if isinstance(x, ruamel.yaml.comments.CommentedMap):
-                    if x["dest"] == "repo1":
-                        if x["url"]:
+                    if "dest" in x and x["dest"] == "repo1":
+                        if "url" in x and x["url"]:
                             keep_url = x["url"]
 
     if keep_url:
@@ -269,8 +269,8 @@ def ad_hoc_update_dm_2__for_status_dm_fm(
         if isinstance(value, List):
             for x in value:
                 if isinstance(x, ruamel.yaml.comments.CommentedMap):
-                    if x["dest"] == "repo1":
-                        if x["url"]:
+                    if "dest" in x and x["dest"] == "repo1":
+                        if "url" in x and x["url"]:
                             keep_url = x["url"]
 
     if keep_url:
@@ -754,7 +754,7 @@ def ad_hoc_update_to_dm_dest__for_test_mm(
         if isinstance(value, List):
             for x in value:
                 if isinstance(x, ruamel.yaml.comments.CommentedMap):
-                    if x["dest"] == "manifest":
+                    if "dest" in x and x["dest"] == "manifest":
                         x["dest"] = "manifest-dm"
     # write the file down
     with open(manifest_path, "w") as file:
@@ -774,7 +774,7 @@ def ad_hoc_update_to_fm_dest__for_test_mm(
         if isinstance(value, List):
             for x in value:
                 if isinstance(x, ruamel.yaml.comments.CommentedMap):
-                    if x["dest"] == "manifest-dm":
+                    if "dest" in x and x["dest"] == "manifest-dm":
                         x["dest"] = "manifest-fm"
     # write the file down
     with open(manifest_path, "w") as file:
@@ -815,10 +815,12 @@ def test_dm_manifests_schema_error(
     # 4th: see if 'status' warns about it, while still prints the rest
     message_recorder.reset()
     tsrc_cli.run("status")
-    assert message_recorder.find(r"Warning: Failed to get Deep Manifest")
-    assert message_recorder.find(r"\* manifest master \(dirty\) ~~ MANIFEST")
-    assert message_recorder.find(r"\* repo1    master")
-    assert not message_recorder.find(r"=> Destination .*")
+    assert message_recorder.find(r"=> Destination \[Deep Manifest description\]")
+    assert message_recorder.find(r"\* repo2    \[ master                  \]  master")
+    assert message_recorder.find(
+        r"\* manifest \[ master                  \]= master \(dirty\) ~~ MANIFEST"
+    )
+    assert message_recorder.find(r"\* repo1    \[ master \(missing remote\) \]  master")
 
 
 def test_fm_manifests_schema_error(
@@ -832,14 +834,14 @@ def test_fm_manifests_schema_error(
 
     Scenario:
 
-    # 1st: Create repositories and Manifest repository as well
-    # 2nd: init Workspace on master
-    # 3rd: Manifest repo: checkout new branch: 'damaged'
-    # 4th: damage Manifest's repo
-    # 5th: Manifest's repo: commit + push
-    # 6th: go back to 'master' for Manifest's repo
-    # 7th: switch future branch to 'damaged'
-    # 8th: verify if 'status' return proper Warning
+    * 1st: Create repositories and Manifest repository as well
+    * 2nd: init Workspace on master
+    * 3rd: Manifest repo: checkout new branch: 'damaged'
+    * 4th: damage Manifest's repo
+    * 5th: Manifest's repo: commit + push
+    * 6th: go back to 'master' for Manifest's repo
+    * 7th: switch future branch to 'damaged'
+    * 8th: verify if 'status' return proper Warning
     """
     # 1st: Create repositories and Manifest repository as well
     git_server.add_repo("repo1")
@@ -873,15 +875,28 @@ def test_fm_manifests_schema_error(
     #   also with Warning
     message_recorder.reset()
     tsrc_cli.run("manifest", "--branch", "damaged")
-    assert message_recorder.find(r"Warning: Failed to get Future Manifest")
-    assert message_recorder.find(r"\* manifest \[ master \]= master ~~ MANIFEST")
+    assert message_recorder.find(
+        r"=> Destination \[Deep Manifest description\] \(Future Manifest description\)"
+    )
+    assert message_recorder.find(
+        r"\* manifest \[ master \]= \( master == master \) ~~ MANIFEST"
+    )
 
     # 8th: verify if 'status' return proper Warning
     message_recorder.reset()
     tsrc_cli.run("status")
-    assert message_recorder.find(r"Warning: Failed to get Future Manifest")
-    assert message_recorder.find(r"\* manifest \[ master \]= master ~~ MANIFEST")
-    assert message_recorder.find(r"\* repo1    \[ master \]  master")
+    assert message_recorder.find(
+        r"=> Destination \[Deep Manifest description\] \(Future Manifest description\)"
+    )
+    assert message_recorder.find(
+        r"\* repo1    \[ master \]  \( master \(missing remote\) << master \)"
+    )
+    assert message_recorder.find(
+        r"\* repo2    \[ master \]  \( master                  == master \)"
+    )
+    assert message_recorder.find(
+        r"\* manifest \[ master \]= \( master                  == master \) ~~ MANIFEST"
+    )
 
 
 def ad_hoc_delete_item_from_manifest(
@@ -896,7 +911,7 @@ def ad_hoc_delete_item_from_manifest(
         if isinstance(value, List):
             for x in value:
                 if isinstance(x, ruamel.yaml.comments.CommentedMap):
-                    if x["dest"] == "repo1":
+                    if "dest" in x and "url" in x and x["dest"] == "repo1":
                         del x["url"]
 
     # write the file down

tsrc/test/cli/test_sync_to_ref.py

@@ -39,7 +39,7 @@
 from pathlib import Path
 from typing import List
 
-import pytest
+# import pytest
 import ruamel.yaml
 from cli_ui.tests import MessageRecorder
 
@@ -264,7 +264,6 @@ def test_sync_to_ref_case_2(
     assert message_recorder.find(r"=> Destination \[Deep Manifest description\]")
 
 
-@pytest.mark.last
 def test_sync_bug_unique_case_3(
     tsrc_cli: CLI,
     git_server: GitServer,