Update how the submodule update handles failed modules

The bot should watch for state updates of failed modules in the
event they are manually integrated after having been marked
as failed. This patch also introduces an additional repo status
of DONE_FAILED_DEPENDENCY to better differentiate between actual
failed modules, and those which were marked as failed due
to bad dependencies.

Change-Id: I8b949aa32cef109a51d7b53f7bc9942186034959
Reviewed-by: Daniel Smith <Daniel.Smith@qt.io>

5 files changed, 114 insertions, 48 deletions

diff --git a/util/dependency_updater/main.py b/util/dependency_updater/main.py
index 42d0752..67910cd 100644
--- a/util/dependency_updater/main.py
+++ b/util/dependency_updater/main.py
@@ -117,21 +117,6 @@ def main():
         config.rewind_module = toolbox.search_for_repo(config, config.args.rewind_module)
     # Load the state cache
     config.state_data = state.load_updates_state(config)
-    # Check to see if we should abort as finished-failed
-    if config.state_data.get("pause_on_finish_fail"):
-        if not any([config.args.retry_failed, config.args.rewind_module]):
-            print(
-                "Round is in Failed_finish state and this round was run in Paused On Finish Fail Mode.\n"
-                "To move the round forward, run the script with one of the following --reset,"
-                " --rewind, or --retry_failed")
-            parse_args(print_help=True)
-            exit()
-        # Continue the round and try again.
-        del config.state_data["pause_on_finish_fail"]
-        if config.args.retry_failed:
-            for module in [r for r in config.state_data.values()
-                           if r.progress == Repo.PROGRESS.DONE_FAILED_BLOCKING]:
-                toolbox.reset_module_properties(config, module)
     report_new_round = False
     if not config.state_data and config.args.update_default_repos:
         # State data is always empty if the round is fresh.
@@ -153,9 +138,43 @@ def main():
     config.state_data = state.update_state_data(config.state_data, repos)
 
     # Update the progress of all repos in the state since the last run of the tool.
+    changes_since_last_run = False
     for repo in config.state_data.values():
+        progress = repo.progress
         repo.progress, repo.proposal.merged_ref, repo.proposal.gerrit_status = \
             toolbox.get_check_progress(config, repo)
+        if progress != repo.progress:
+            changes_since_last_run = True
+            print(f"{repo.id} moved from {progress.name} to {repo.progress.name}.")
+
+    # Check to see if we should abort as finished-failed
+    if config.state_data.get("pause_on_finish_fail"):
+        # If none of the retry conditions are met, print the help and exit.
+        if not any([config.args.retry_failed, config.args.rewind_module, changes_since_last_run]):
+            print(
+                "Round is in Failed_finish state and this round was run in"
+                " Pause On Finish Fail Mode.\n"
+                "To move the round forward, run the script with one of the following --reset,"
+                " --rewind, or --retry_failed, or manually integrate any of the"
+                " failed changes below.")
+            print(toolbox.state_printer(config))
+            parse_args(print_help=True)
+            exit()
+        # Continue the round and try again.
+        del config.state_data["pause_on_finish_fail"]
+
+    # Reset failed modules as necessary or re-determine readiness.
+    if config.args.retry_failed or changes_since_last_run:
+        for module in [r for r in config.state_data.values()
+                       if r.progress in [Repo.PROGRESS.DONE_FAILED_BLOCKING,
+                                         Repo.PROGRESS.DONE_FAILED_NON_BLOCKING,
+                                         Repo.PROGRESS.DONE_FAILED_DEPENDENCY]]:
+            if config.args.retry_failed:
+                toolbox.reset_module_properties(config, module)
+            elif changes_since_last_run:
+                # Update the progress of repos such that previously failed
+                # modules which are now unblocked get set to READY.
+                module.progress, _, __ = dependency_resolver.determine_ready(config, module)
 
     # Collect necessary data if dropping a dependency from a repo.
     if config.args.drop_dependency:
diff --git a/util/dependency_updater/tools/dependency_resolver.py b/util/dependency_updater/tools/dependency_resolver.py
index 63b7fa4..11caf37 100644
--- a/util/dependency_updater/tools/dependency_resolver.py
+++ b/util/dependency_updater/tools/dependency_resolver.py
@@ -1,6 +1,7 @@
 # Copyright (C) 2020 The Qt Company Ltd.
 # SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
 import copy
+from typing import Union
 
 from tools import toolbox
 from .config import Config
@@ -32,7 +33,7 @@ def prepare_update(config: Config, repo: Repo) -> tuple[Repo, bool]:
     """Bump progress of a repo if it's dependencies are met,
     then create a proposal if it doesn't already exist."""
 
-    repo.progress, progress_changed = determine_ready(config, repo)
+    repo.progress, progress_changed, repo.failed_dependencies = determine_ready(config, repo)
     if repo.is_supermodule:
         return repo, False
     repo.proposal = retrieve_or_generate_proposal(config, repo)
@@ -51,7 +52,8 @@ def prepare_update(config: Config, repo: Repo) -> tuple[Repo, bool]:
 def retrieve_or_generate_proposal(config: Config, repo) -> Proposal:
     """Return the proposed YAML if it exists and should not be updated,
     otherwise, generate a new one with the latest shas."""
-    if repo.progress in [PROGRESS.DONE_FAILED_NON_BLOCKING, PROGRESS.DONE_FAILED_BLOCKING, PROGRESS.DONE, PROGRESS.DONE_NO_UPDATE,
+    if repo.progress in [PROGRESS.DONE_FAILED_DEPENDENCY, PROGRESS.DONE_FAILED_NON_BLOCKING,
+                         PROGRESS.DONE_FAILED_BLOCKING, PROGRESS.DONE, PROGRESS.DONE_NO_UPDATE,
                          PROGRESS.WAIT_DEPENDENCY, PROGRESS.WAIT_INCONSISTENT,
                          PROGRESS.IN_PROGRESS]:
         return repo.proposal
@@ -80,9 +82,12 @@ def check_subtree(config, source: Repo,
     Recurse for each dependency which is not the same as the source.
 
     :returns: the id of a target repo which has a mismatching sha to the source_ref"""
-    deps = target.deps_yaml.get(
-        "dependencies") if target.progress < PROGRESS.DONE else target.proposal.proposed_yaml.get(
-        "dependencies")
+    deps = target.deps_yaml.get("dependencies") \
+        if target.progress < PROGRESS.DONE \
+        or target.progress == Repo.progress.DONE_FAILED_DEPENDENCY \
+        or target.progress in (Repo.progress.DONE_FAILED_BLOCKING,
+                               Repo.progress.DONE_FAILED_NON_BLOCKING) \
+        else target.proposal.proposed_yaml.get("dependencies")
     for dependency in deps.keys():
         if source.name in dependency:
             if not source_ref == deps[dependency]["ref"]:
@@ -184,10 +189,14 @@ def discover_missing_dependencies(config: Config, repo: Repo) -> dict[str, Repo]
     return config.state_data
 
 
-def determine_ready(config: Config, repo: Repo) -> tuple[PROGRESS, bool]:
+def determine_ready(config: Config, repo: Repo) -> tuple[PROGRESS, bool, Union[list, None]]:
     """Check to see if a repo is waiting on another, or if all
     dependency conflicts have been resolved and/or updated."""
     worst_state = PROGRESS.READY
+    # Keep a list of failed dependencies so we can explain
+    # why a repo is being marked as failed without
+    # actually attempting an update.
+    failed_repos = set()
 
     def is_worse(state):
         nonlocal worst_state
@@ -196,19 +205,28 @@ def determine_ready(config: Config, repo: Repo) -> tuple[PROGRESS, bool]:
 
     if repo.proposal.inconsistent_set:
         is_worse(PROGRESS.WAIT_INCONSISTENT)
-    if repo.progress < PROGRESS.IN_PROGRESS:
+    if repo.progress < PROGRESS.IN_PROGRESS \
+            or repo.progress == PROGRESS.DONE_FAILED_DEPENDENCY:
         for dependency in repo.dep_list:
+            dep_repo = config.state_data[dependency]
             if dependency in config.state_data.keys():
-                if config.state_data[dependency].progress < PROGRESS.DONE:
+                # Recurse and update the progress in the case that we're trying
+                # to rewind with many previously failed dependencies.
+                dep_repo.progress, _, dep_repo.failed_dependencies \
+                    = determine_ready(config, dep_repo)
+
+                if dep_repo.progress < PROGRESS.DONE:
                     is_worse(PROGRESS.WAIT_DEPENDENCY)
-                elif config.state_data[dependency].progress == PROGRESS.DONE_FAILED_NON_BLOCKING:
+                elif dep_repo.progress == PROGRESS.DONE_FAILED_NON_BLOCKING:
                     print(f"WARN: {repo.id} dependency {dependency} is a non-blocking module which"
-                          f" failed. Marking {repo.id} as failed.")
-                    is_worse(PROGRESS.DONE_FAILED_NON_BLOCKING)
-                elif config.state_data[dependency].progress == PROGRESS.DONE_FAILED_BLOCKING:
+                          f" failed. Marking {repo.id} as Failed due to dependency.")
+                    failed_repos.add(dependency)
+                    is_worse(PROGRESS.DONE_FAILED_DEPENDENCY)
+                elif dep_repo.progress == PROGRESS.DONE_FAILED_BLOCKING:
                     print(f"WARN: {repo.id} dependency {dependency} is a blocking module which"
-                          f" failed. Marking {repo.id} as failed-blocking.")
-                    is_worse(PROGRESS.DONE_FAILED_BLOCKING)
-        return worst_state, repo.progress != worst_state
+                          f" failed. Marking {repo.id} as Failed due to dependency.")
+                    is_worse(PROGRESS.DONE_FAILED_DEPENDENCY)
+                    failed_repos.add(dependency)
+        return worst_state, repo.progress != worst_state, list(failed_repos)
     else:
-        return repo.progress, False
+        return repo.progress, False, None
diff --git a/util/dependency_updater/tools/repo.py b/util/dependency_updater/tools/repo.py
index 3b3c20f..1e6363c 100644
--- a/util/dependency_updater/tools/repo.py
+++ b/util/dependency_updater/tools/repo.py
@@ -22,7 +22,8 @@ class PROGRESS(IntEnum):
     DONE_NO_UPDATE = 8
     DONE_FAILED_NON_BLOCKING = 9
     DONE_FAILED_BLOCKING = 10
-    IGNORE_IS_META = 11
+    DONE_FAILED_DEPENDENCY = 11
+    IGNORE_IS_META = 12
 
 
 class Repo(Namespace):
@@ -37,6 +38,7 @@ class Repo(Namespace):
     proposal: Proposal = Proposal()
     to_stage: list[str]
     progress: PROGRESS = PROGRESS.UNSPECIFIED
+    failed_dependencies: list[str]
     stage_count: int = 0
     retry_count: int = 0
     is_supermodule: bool = False  # Bypasses dependency calculation
@@ -49,6 +51,7 @@ class Repo(Namespace):
         super().__init__(**kwargs)
         self.to_stage = list()
         self.dep_list = list()
+        self.failed_dependencies = list()
         self.id = unquote(id)
         self.prefix = prefix
         self.name = id.removeprefix(prefix)
diff --git a/util/dependency_updater/tools/teams_connector.py b/util/dependency_updater/tools/teams_connector.py
index 580f61e..fdb6bfc 100644
--- a/util/dependency_updater/tools/teams_connector.py
+++ b/util/dependency_updater/tools/teams_connector.py
@@ -8,21 +8,26 @@ from .repo import Repo, PROGRESS
 from typing import Union
 
 
-def gerrit_link_maker(config, change: Union[GerritChange.GerritChange, Repo]) -> tuple[str, str]:
+def gerrit_link_maker(config, change_or_repo: Union[GerritChange.GerritChange, Repo, str],
+                      change_id_override: str = None) -> tuple[str, str]:
     repo = ""
+    change_override = ""
     _change = None
-    if type(change) == GerritChange:
-        repo = change.project
-        _change = change
-    elif type(change) == Repo:
-        repo = change.id
-        _change = config.datasources.gerrit_client.changes.get(change.proposal.change_id)
+    if type(change_or_repo) == GerritChange:
+        repo = change_or_repo.project
+        _change = change_or_repo
+    elif type(change_or_repo) == Repo:
+        repo = change_or_repo.id
+    _change = _change \
+        or config.datasources.gerrit_client.changes.get(change_id_override
+                                                        or change_or_repo.proposal.change_id)
     if not repo:
         return "", ""
     subject = _change.subject
     mini_sha = _change.get_revision("current").get_commit().commit[:10]
     url = f"{config.GERRIT_HOST}c/{repo}/+/{_change._number}"
-    return f"({mini_sha}) {subject[:70]}{'...' if len(subject) > 70 else ''}", url
+    change_status = f"[{_change.status}]" if _change.status != "NEW" else ""
+    return f"{change_status}({mini_sha}) {subject[:70]}{'...' if len(subject) > 70 else ''}", url
 
 
 class TeamsConnector:
@@ -125,15 +130,24 @@ class TeamsConnector:
                 msteams.connectorcard.addPotentialAction(reset_section, retry)
                 message_card.addSection(reset_section)
             failed_section = msteams.cardsection()
-            failed_modules_text = "\n".join([r.id for r in config.state_data.values()
-                                             if r.progress == PROGRESS.DONE_FAILED_BLOCKING])
-            failed_section.text(f"```\nFailed Modules on {config.args.branch}:\n{failed_modules_text}")
+            # Join the list of failed modules with their failed dependencies if there are any.
+            # If there's no failed dependencies, it means the module itself had issues integrating.
+            failed_modules_text = "\n".join(r.id for r in config.state_data.values()
+                                            if r.progress == PROGRESS.DONE_FAILED_BLOCKING)
+            failed_modules_text += "\n\nThe following modules could not be updated due to failed" \
+                                   " dependencies:\n"
+            failed_modules_text += "\n".join(f"{r.id}: {r.failed_dependencies}"
+                                             for r in config.state_data.values()
+                                             if r.progress == PROGRESS.DONE_FAILED_DEPENDENCY)
+            failed_section.text(
+                f"```\nFailed Modules on {config.args.branch}:\n{failed_modules_text}")
             message_card.addSection(failed_section)
+            print(message_card.payload)
             message_card.send()
-            if message_card.last_http_status.status_code != 200:
-                print(
-                    f"WARN: Unable to send alert to webhook for Round Failed Finished on {config.args.branch}")
-                return False
+            # if message_card.last_http_status.status_code != 200:
+            #     print(
+            #         f"WARN: Unable to send alert to webhook for Round Failed Finished on {config.args.branch}")
+            #     return False
         return True
 
     def send_teams_webhook_basic(self, text: str, repo: Repo = None, reset_links=False):
diff --git a/util/dependency_updater/tools/toolbox.py b/util/dependency_updater/tools/toolbox.py
index 772cff4..34bb783 100644
--- a/util/dependency_updater/tools/toolbox.py
+++ b/util/dependency_updater/tools/toolbox.py
@@ -1204,6 +1204,7 @@ def reset_module_properties(config: Config, repo: Repo) -> Repo:
     print(f"Resetting module state for {repo.id}")
     repo.progress = PROGRESS.UNSPECIFIED
     repo.proposal = Proposal()
+    repo.failed_dependencies = list()
     repo.stage_count = 0
     repo.retry_count = 0
     repo.to_stage = list()
@@ -1289,12 +1290,23 @@ def state_printer(config: Config) -> tuple[dict[PROGRESS, int], str]:
     repos.clear()
     msg = "\nThe following repos failed to update:"
     for repo in config.state_data.keys():
-        if config.state_data[repo].progress >= PROGRESS.DONE_FAILED_NON_BLOCKING:
+        if config.state_data[repo].progress in (PROGRESS.DONE_FAILED_NON_BLOCKING,
+                                                PROGRESS.DONE_FAILED_BLOCKING):
             total_state[PROGRESS.DONE_FAILED_NON_BLOCKING.value] += 1
             repos.append(repo)
     if repos:
         ret_str += _print(msg)
         for repo in repos:
             ret_str += _print(f"\t{repo}")
+    repos.clear()
+    msg = "\nThe following repos cannot be updated due to a failed dependency:"
+    for repo in config.state_data.keys():
+        if config.state_data[repo].progress == PROGRESS.DONE_FAILED_DEPENDENCY:
+            total_state[PROGRESS.DONE_FAILED_DEPENDENCY.value] += 1
+            repos.append(repo)
+    if repos:
+        ret_str += _print(msg)
+        for repo in repos:
+            ret_str += _print(f"\t{repo}: {config.state_data[repo].failed_dependencies}")
 
     return total_state, ret_str