From e903715d213f686ae507f5aa4f5d458e4e9a8fcd Mon Sep 17 00:00:00 2001
From: Simon Chopin <simon.chopin@canonical.com>
Date: Tue, 13 Feb 2024 19:23:14 +0100
Subject: [PATCH 1/4] apport-retrace: split out report loading into its own
 function

This reduces complexity in main() while also addressing the unbound crashid
issue that showed up in Noble, albeit only partially: now it's not
unbound, but it can be None, which is another can of worms.

Baby steps!
---
 bin/apport-retrace | 55 +++++++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/bin/apport-retrace b/bin/apport-retrace
index 00ac04b91..e7f591e3a 100755
--- a/bin/apport-retrace
+++ b/bin/apport-retrace
@@ -26,12 +26,14 @@ import tempfile
 import termios
 import tty
 import zlib
+from argparse import Namespace
 from gettext import gettext as _
 
 import apport
 import apport.fileutils
 import apport.sandboxutils
-from apport.crashdb import get_crashdb
+from apport import Report
+from apport.crashdb import CrashDatabase, get_crashdb
 
 
 def parse_args():
@@ -323,20 +325,10 @@ def print_traces(report):
         print(report["StacktraceSource"])
 
 
-# pylint: disable-next=missing-function-docstring
-def main():
-    # TODO: Split into smaller functions/methods
-    # pylint: disable=too-many-branches,too-many-locals,too-many-nested-blocks
-    # pylint: disable=too-many-statements
-    apport.memdbg("start")
-
-    gettext.textdomain("apport")
-
-    options = parse_args()
-
-    crashdb = get_crashdb(options.auth)
-    apport.memdbg("got crash DB")
-
+def load_report(
+    options: Namespace, crashdb: CrashDatabase
+) -> tuple[Report, int | None]:
+    """Load the initial report for the given CLI options"""
     # load the report
     if os.path.exists(options.report):
         try:
@@ -344,13 +336,17 @@ def main():
             with open(options.report, "rb") as f:
                 report.load(f, binary="compressed")
             apport.memdbg("loaded report from file")
+            return report, None
         except (MemoryError, TypeError, ValueError, OSError, zlib.error) as error:
             apport.fatal("Cannot open report file: %s", str(error))
     elif options.report.isdigit():
         # crash ID
         try:
-            report = crashdb.download(int(options.report))
+            crashid = int(options.report)
+            report = crashdb.download(crashid)
             apport.memdbg("downloaded report from crash DB")
+            options.report = None
+            return report, crashid
         except AssertionError as error:
             if "apport format data" in str(error):
                 apport.error("Broken report: %s", str(error))
@@ -389,14 +385,29 @@ Thank you for your understanding, and sorry for the inconvenience!
                 sys.exit(0)
             else:
                 raise
-
-        crashid = options.report
-        options.report = None
     else:
         apport.fatal(
             '"%s" is neither an existing report file nor a crash ID', options.report
         )
 
+
+# pylint: disable-next=missing-function-docstring
+def main():
+    # TODO: Split into smaller functions/methods
+    # pylint: disable=too-many-branches,too-many-locals,too-many-nested-blocks
+    # pylint: disable=too-many-statements
+    apport.memdbg("start")
+
+    gettext.textdomain("apport")
+
+    options = parse_args()
+
+    crashdb = get_crashdb(options.auth)
+    apport.memdbg("got crash DB")
+
+    # load the report
+    report, crashid = load_report(options, crashdb)
+
     if options.core_file:
         report["CoreDump"] = (os.path.abspath(options.core_file),)
     if options.executable:
@@ -617,7 +628,7 @@ Thank you for your understanding, and sorry for the inconvenience!
                 update_bug = True
                 if options.duplicate_db:
                     crashdb.init_duplicate_db(options.duplicate_db)
-                    res = crashdb.check_duplicate(int(crashid), report)
+                    res = crashdb.check_duplicate(crashid, report)
                     if res:
                         if res[1] is None:
                             version = "not fixed yet"
@@ -637,8 +648,8 @@ Thank you for your understanding, and sorry for the inconvenience!
                     if "Stacktrace" in report:
                         crashdb.update_traces(crashid, report)
                         apport.log(
-                            "New attachments uploaded to crash database LP: #"
-                            + crashid,
+                            f"New attachments uploaded to crash database"
+                            f"LP: #{crashid}",
                             options.timestamps,
                         )
                     else:

From 0237ff078c82949d0aaffd0a172557f4ec26d037 Mon Sep 17 00:00:00 2001
From: Simon Chopin <simon.chopin@canonical.com>
Date: Tue, 13 Feb 2024 20:05:37 +0100
Subject: [PATCH 2/4] retrace: directly test crashid relevance

As it turns out testing for `options.report` being falsy meant that
crashid was defined, but I'd rather test it directly as it's a bit
cumbersome to trace (and it made pylint crazy).
---
 bin/apport-retrace | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/bin/apport-retrace b/bin/apport-retrace
index e7f591e3a..87baa3c63 100755
--- a/bin/apport-retrace
+++ b/bin/apport-retrace
@@ -345,7 +345,6 @@ def load_report(
             crashid = int(options.report)
             report = crashdb.download(crashid)
             apport.memdbg("downloaded report from crash DB")
-            options.report = None
             return report, crashid
         except AssertionError as error:
             if "apport format data" in str(error):
@@ -617,7 +616,7 @@ Thank you for your understanding, and sorry for the inconvenience!
         modified = True
 
     if modified:
-        if not options.report and not options.output:
+        if crashid is not None and not options.output:
             if not options.auth:
                 apport.fatal(
                     "You need to specify --auth for uploading retraced results"

From c373689af63eb93cad13fd8bb9d1e99efc348575 Mon Sep 17 00:00:00 2001
From: Simon Chopin <simon.chopin@canonical.com>
Date: Tue, 13 Feb 2024 19:55:44 +0100
Subject: [PATCH 3/4] retrace: delegate sandbox management in a context manager

While the contextmanager method is still too complex, it's another step
towards reducing the overall complexity of the file.
---
 bin/apport-retrace | 371 ++++++++++++++++++++++++---------------------
 1 file changed, 199 insertions(+), 172 deletions(-)

diff --git a/bin/apport-retrace b/bin/apport-retrace
index 87baa3c63..c8f8f33d6 100755
--- a/bin/apport-retrace
+++ b/bin/apport-retrace
@@ -27,6 +27,9 @@ import termios
 import tty
 import zlib
 from argparse import Namespace
+from collections.abc import Iterator
+from contextlib import contextmanager
+from dataclasses import dataclass
 from gettext import gettext as _
 
 import apport
@@ -390,60 +393,35 @@ Thank you for your understanding, and sorry for the inconvenience!
         )
 
 
-# pylint: disable-next=missing-function-docstring
-def main():
-    # TODO: Split into smaller functions/methods
-    # pylint: disable=too-many-branches,too-many-locals,too-many-nested-blocks
-    # pylint: disable=too-many-statements
-    apport.memdbg("start")
+@dataclass
+class SandboxData:
+    """Data loosely related to the sandboxing of the retracing."""
 
-    gettext.textdomain("apport")
+    dir: str | None = None
+    cache: str | None = None
+    outdated_msg: str | None = None
+    gdb: str | None = None
 
-    options = parse_args()
 
-    crashdb = get_crashdb(options.auth)
-    apport.memdbg("got crash DB")
-
-    # load the report
-    report, crashid = load_report(options, crashdb)
-
-    if options.core_file:
-        report["CoreDump"] = (os.path.abspath(options.core_file),)
-    if options.executable:
-        report["ExecutablePath"] = options.executable
-    if options.procmaps:
-        with open(options.procmaps, "r", encoding="utf-8") as f:
-            report["ProcMaps"] = f.read()
-    if options.rebuild_package_info and "ExecutablePath" in report:
-        report.add_package_info()
-
-    apport.memdbg("processed extra options from command line")
-
-    # consistency checks
-    required_fields = set(
-        ["CoreDump", "ExecutablePath", "Package", "DistroRelease", "Architecture"]
-    )
-    if report["ProblemType"] == "KernelCrash":
-        if not set(["Package", "VmCore"]).issubset(set(report.keys())):
-            apport.error("report file does not contain the required fields")
-            sys.exit(0)
-        apport.error("KernelCrash processing not implemented yet")
-        sys.exit(0)
-    elif not required_fields.issubset(set(report.keys())):
-        missing_fields = []
-        for required_field in required_fields:
-            if required_field not in set(report.keys()):
-                missing_fields.append(required_field)
-        apport.error(
-            "report file does not contain one of the required fields: %s",
-            " ".join(missing_fields),
-        )
-        sys.exit(0)
+@contextmanager
+def load_sandbox(report: Report, options: Namespace) -> Iterator[SandboxData]:
+    # TODO: Split into smaller functions/methods
+    # pylint: disable=too-many-branches,too-many-nested-blocks
+    # pylint: disable=too-many-statements
+    """Create, load, and then clean up the retracing sandboxing
+    as specified by the CLI options"""
+    if not options.sandbox and not options.gdb_sandbox:
+        yield SandboxData()
+        return
 
-    apport.memdbg("consistency checks passed")
+    system_arch = apport.packaging.get_system_architecture()
+    target = None
+    sandbox = None
+    cache = None
+    outdated_msg = None
+    gdb_sandbox = None
 
     if options.gdb_sandbox:
-        system_arch = apport.packaging.get_system_architecture()
         if system_arch != "amd64":
             apport.error("gdb sandboxes are only implemented for amd64 hosts")
             sys.exit(0)
@@ -472,16 +450,10 @@ def main():
             options.timestamps,
             options.dynamic_origins,
         )
-    else:
-        sandbox = None
-        cache = None
-        outdated_msg = None
 
     if options.gdb_sandbox:
         if report["Architecture"] == system_arch:
-            if sandbox:
-                # gdb was installed in the sandbox
-                gdb_sandbox = sandbox
+            gdb_sandbox = sandbox
         else:
             gdb_packages = ["gdb", "gdb-multiarch"]
             fake_report = apport.Report()
@@ -510,7 +482,6 @@ def main():
         # Workaround LP: #1818918 (.gnu_debugaltlink being an absolute path),
         # for releases without a fix, by creating a symlink from the host's
         # .dwz directory to the machine specific one in the sandbox.
-        target = ""
         if report["DistroRelease"][-5:] in {"18.04", "20.04"}:
             if gdb_sandbox and sandbox:
                 machine = report["Uname"].split()[-1]
@@ -546,54 +517,10 @@ def main():
                         " gdb sandbox's. See LP: #1818918 for details."
                     )
                     sys.exit(0)
-    else:
-        gdb_sandbox = None
-
-    # interactive gdb session
-    if options.gdb:
-        gdb_cmd, environ = report.gdb_command(sandbox, gdb_sandbox)
-        if options.verbose:
-            # build a shell-style command
-            cmd = ""
-            for w in gdb_cmd:
-                if cmd:
-                    cmd += " "
-                if " " in w:
-                    cmd += f"'{w}'"
-                else:
-                    cmd += w
-            apport.log(f"Calling gdb command: {cmd}", options.timestamps)
-        apport.memdbg("before calling gdb")
-        subprocess.call(gdb_cmd, env=os.environ | environ)
-    else:
-        # regenerate gdb info
-        apport.memdbg("before collecting gdb info")
-        try:
-            report.add_gdb_info(sandbox, gdb_sandbox)
-        except OSError as error:
-            if not options.auth:
-                apport.fatal("%s", str(error))
-            if not options.confirm or confirm_traces(report):
-                invalid_msg = """Thank you for your report!
 
-However, processing it in order to get sufficient information for the
-developers failed as the report has a core dump which is invalid. The
-corruption may have happened on the system which the crash occurred or during
-transit.
-
-Thank you for your understanding, and sorry for the inconvenience!
-"""
-                crashdb.mark_retrace_failed(crashid, invalid_msg)
-            apport.fatal("%s", str(error))
-        if options.sandbox == "system":
-            apt_root = os.path.join(cache, "system", "apt")
-        elif options.sandbox:
-            apt_root = os.path.join(cache, report["DistroRelease"], "apt")
-        else:
-            apt_root = None
-        if options.stacktrace_source:
-            gen_source_stacktrace(report, apt_root)
-        report.add_kernel_crash_info()
+    yield SandboxData(
+        dir=sandbox, cache=cache, outdated_msg=outdated_msg, gdb=gdb_sandbox
+    )
 
     # Cleanup the .dwz machine symlink for LP: #1818918
     if gdb_sandbox and sandbox and target:
@@ -602,94 +529,194 @@ Thank you for your understanding, and sorry for the inconvenience!
         ):
             os.unlink(f"/usr/lib/debug/.dwz/{target}")
 
-    modified = False
 
-    apport.memdbg("information collection done")
+# pylint: disable-next=missing-function-docstring
+def main():
+    # TODO: Split into smaller functions/methods
+    # pylint: disable=too-many-branches,too-many-locals,too-many-nested-blocks
+    # pylint: disable=too-many-statements
+    apport.memdbg("start")
 
-    if options.remove_core:
-        del report["CoreDump"]
-        modified = True
+    gettext.textdomain("apport")
 
-    if options.stdout:
-        print_traces(report)
-    elif not options.gdb:
-        modified = True
+    options = parse_args()
 
-    if modified:
-        if crashid is not None and not options.output:
-            if not options.auth:
-                apport.fatal(
-                    "You need to specify --auth for uploading retraced results"
-                    " back to the crash database."
-                )
-            if not options.confirm or confirm_traces(report):
-                # check for duplicates
-                update_bug = True
-                if options.duplicate_db:
-                    crashdb.init_duplicate_db(options.duplicate_db)
-                    res = crashdb.check_duplicate(crashid, report)
-                    if res:
-                        if res[1] is None:
-                            version = "not fixed yet"
-                        elif res[1] == "":
-                            version = "fixed in latest version"
-                        else:
-                            version = f"fixed in version {res[1]}"
-                        apport.log(
-                            f"Report is a duplicate of #{res[0]} ({version})",
-                            options.timestamps,
-                        )
-                        update_bug = False
-                    else:
-                        apport.log("Duplicate check negative", options.timestamps)
-
-                if update_bug:
-                    if "Stacktrace" in report:
-                        crashdb.update_traces(crashid, report)
-                        apport.log(
-                            f"New attachments uploaded to crash database"
-                            f"LP: #{crashid}",
-                            options.timestamps,
-                        )
-                    else:
-                        # this happens when gdb crashes
-                        apport.log("No stack trace, invalid report", options.timestamps)
+    crashdb = get_crashdb(options.auth)
+    apport.memdbg("got crash DB")
 
-                    if not report.has_useful_stacktrace():
-                        if outdated_msg:
-                            invalid_msg = f"""Thank you for your report!
+    # load the report
+    report, crashid = load_report(options, crashdb)
 
-However, processing it in order to get sufficient information for the
-developers failed (it does not generate a useful symbolic stack trace). This
-might be caused by some outdated packages which were installed on your system
-at the time of the report:
+    if options.core_file:
+        report["CoreDump"] = (os.path.abspath(options.core_file),)
+    if options.executable:
+        report["ExecutablePath"] = options.executable
+    if options.procmaps:
+        with open(options.procmaps, "r", encoding="utf-8") as f:
+            report["ProcMaps"] = f.read()
+    if options.rebuild_package_info and "ExecutablePath" in report:
+        report.add_package_info()
 
-{outdated_msg}
+    apport.memdbg("processed extra options from command line")
 
-Please upgrade your system to the latest package versions. If you still
-encounter the crash, please file a new report.
+    # consistency checks
+    required_fields = set(
+        ["CoreDump", "ExecutablePath", "Package", "DistroRelease", "Architecture"]
+    )
+    if report["ProblemType"] == "KernelCrash":
+        if not set(["Package", "VmCore"]).issubset(set(report.keys())):
+            apport.error("report file does not contain the required fields")
+            sys.exit(0)
+        apport.error("KernelCrash processing not implemented yet")
+        sys.exit(0)
+    elif not required_fields.issubset(set(report.keys())):
+        missing_fields = []
+        for required_field in required_fields:
+            if required_field not in set(report.keys()):
+                missing_fields.append(required_field)
+        apport.error(
+            "report file does not contain one of the required fields: %s",
+            " ".join(missing_fields),
+        )
+        sys.exit(0)
+
+    apport.memdbg("consistency checks passed")
+
+    with load_sandbox(report, options) as sandbox:
+        # interactive gdb session
+        if options.gdb:
+            gdb_cmd, environ = report.gdb_command(sandbox.dir, sandbox.gdb)
+            if options.verbose:
+                # build a shell-style command
+                cmd = ""
+                for w in gdb_cmd:
+                    if cmd:
+                        cmd += " "
+                    if " " in w:
+                        cmd += f"'{w}'"
+                    else:
+                        cmd += w
+                apport.log(f"Calling gdb command: {cmd}", options.timestamps)
+            apport.memdbg("before calling gdb")
+            subprocess.call(gdb_cmd, env=os.environ | environ)
+        else:
+            # regenerate gdb info
+            apport.memdbg("before collecting gdb info")
+            try:
+                report.add_gdb_info(sandbox.dir, sandbox.gdb)
+            except OSError as error:
+                if not options.auth:
+                    apport.fatal("%s", str(error))
+                if not options.confirm or confirm_traces(report):
+                    invalid_msg = """Thank you for your report!
+
+However, processing it in order to get sufficient information for the
+developers failed as the report has a core dump which is invalid. The
+corruption may have happened on the system which the crash occurred or during
+transit.
 
 Thank you for your understanding, and sorry for the inconvenience!
 """
+                    crashdb.mark_retrace_failed(crashid, invalid_msg)
+                apport.fatal("%s", str(error))
+            if options.sandbox == "system":
+                apt_root = os.path.join(sandbox.cache, "system", "apt")
+            elif options.sandbox:
+                apt_root = os.path.join(sandbox.cache, report["DistroRelease"], "apt")
+            else:
+                apt_root = None
+            if options.stacktrace_source:
+                gen_source_stacktrace(report, apt_root)
+            report.add_kernel_crash_info()
+
+        modified = False
+
+        apport.memdbg("information collection done")
+
+        if options.remove_core:
+            del report["CoreDump"]
+            modified = True
+
+        if options.stdout:
+            print_traces(report)
+        elif not options.gdb:
+            modified = True
+
+        if modified:
+            if crashid is not None and not options.output:
+                if not options.auth:
+                    apport.fatal(
+                        "You need to specify --auth for uploading retraced results"
+                        " back to the crash database."
+                    )
+                if not options.confirm or confirm_traces(report):
+                    # check for duplicates
+                    update_bug = True
+                    if options.duplicate_db:
+                        crashdb.init_duplicate_db(options.duplicate_db)
+                        res = crashdb.check_duplicate(crashid, report)
+                        if res:
+                            if res[1] is None:
+                                version = "not fixed yet"
+                            elif res[1] == "":
+                                version = "fixed in latest version"
+                            else:
+                                version = f"fixed in version {res[1]}"
                             apport.log(
-                                "No crash signature and outdated packages,"
-                                " invalidating report",
+                                f"Report is a duplicate of #{res[0]} ({version})",
                                 options.timestamps,
                             )
-                            crashdb.mark_retrace_failed(crashid, invalid_msg)
+                            update_bug = False
                         else:
+                            apport.log("Duplicate check negative", options.timestamps)
+
+                    if update_bug:
+                        if "Stacktrace" in report:
+                            crashdb.update_traces(crashid, report)
                             apport.log(
-                                "Report has no crash signature,"
-                                " so retrace is flawed",
+                                f"New attachments uploaded to crash database"
+                                f"LP: #{crashid}",
                                 options.timestamps,
                             )
-                            crashdb.mark_retrace_failed(crashid)
+                        else:
+                            # this happens when gdb crashes
+                            apport.log("No stack trace, invalid report", options.timestamps)
 
-        elif options.output == "-":
-            report.write(sys.stdout.detach())
-        else:
-            with open(options.report, "wb") as out:
-                report.write(out)
+                        if not report.has_useful_stacktrace():
+                            if sandbox.outdated_msg:
+                                invalid_msg = f"""Thank you for your report!
+
+However, processing it in order to get sufficient information for the
+developers failed (it does not generate a useful symbolic stack trace). This
+might be caused by some outdated packages which were installed on your system
+at the time of the report:
+
+{sandbox.outdated_msg}
+
+Please upgrade your system to the latest package versions. If you still
+encounter the crash, please file a new report.
+
+Thank you for your understanding, and sorry for the inconvenience!
+"""
+                                apport.log(
+                                    "No crash signature and outdated packages,"
+                                    " invalidating report",
+                                    options.timestamps,
+                                )
+                                crashdb.mark_retrace_failed(crashid, invalid_msg)
+                            else:
+                                apport.log(
+                                    "Report has no crash signature,"
+                                    " so retrace is flawed",
+                                    options.timestamps,
+                                )
+                                crashdb.mark_retrace_failed(crashid)
+
+            elif options.output == "-":
+                report.write(sys.stdout.detach())
+            else:
+                with open(options.report, "wb") as out:
+                    report.write(out)
 
 
 if __name__ == "__main__":

From 7418eb2c2be20dd9774d4c4516239249c536d131 Mon Sep 17 00:00:00 2001
From: Simon Chopin <simon.chopin@canonical.com>
Date: Tue, 13 Feb 2024 20:10:04 +0100
Subject: [PATCH 4/4] retrace: move the duplicate checks and bug updates into
 helper functions

---
 bin/apport-retrace | 137 +++++++++++++++++++++++++--------------------
 1 file changed, 75 insertions(+), 62 deletions(-)

diff --git a/bin/apport-retrace b/bin/apport-retrace
index c8f8f33d6..630224669 100755
--- a/bin/apport-retrace
+++ b/bin/apport-retrace
@@ -530,6 +530,79 @@ def load_sandbox(report: Report, options: Namespace) -> Iterator[SandboxData]:
             os.unlink(f"/usr/lib/debug/.dwz/{target}")
 
 
+def update_bug(
+    report: Report,
+    options: Namespace,
+    sandbox: SandboxData,
+    crashid: int,
+    crashdb: CrashDatabase,
+) -> None:
+    """Update the bug in the crash db with the retracing data."""
+    # TODO: check behaviour on crashid = None
+    if "Stacktrace" in report:
+        if crashid is None:
+            apport.fatal("Cannot upload stacktrace without a valid crash ID")
+        crashdb.update_traces(crashid, report)
+        apport.log(
+            f"New attachments uploaded to crash database LP: #{crashid}",
+            options.timestamps,
+        )
+    else:
+        # this happens when gdb crashes
+        apport.log("No stack trace, invalid report", options.timestamps)
+
+    if not report.has_useful_stacktrace():
+        if sandbox.outdated_msg:
+            invalid_msg = f"""Thank you for your report!
+
+However, processing it in order to get sufficient information for the
+developers failed (it does not generate a useful symbolic stack trace). This
+might be caused by some outdated packages which were installed on your system
+at the time of the report:
+
+{sandbox.outdated_msg}
+
+Please upgrade your system to the latest package versions. If you still
+encounter the crash, please file a new report.
+
+Thank you for your understanding, and sorry for the inconvenience!
+"""
+            apport.log(
+                "No crash signature and outdated packages, invalidating report",
+                options.timestamps,
+            )
+            crashdb.mark_retrace_failed(crashid, invalid_msg)
+        else:
+            apport.log(
+                "Report has no crash signature, so retrace is flawed",
+                options.timestamps,
+            )
+            crashdb.mark_retrace_failed(crashid)
+
+
+def needs_update(
+    report: Report, options: Namespace, crashid: int, crashdb: CrashDatabase
+) -> bool:
+    """Checks whether we need to update a given bug report."""
+    # check for duplicates
+    if options.duplicate_db:
+        crashdb.init_duplicate_db(options.duplicate_db)
+        res = crashdb.check_duplicate(crashid, report)
+        if res:
+            if res[1] is None:
+                version = "not fixed yet"
+            elif res[1] == "":
+                version = "fixed in latest version"
+            else:
+                version = f"fixed in version {res[1]}"
+            apport.log(
+                f"Report is a duplicate of #{res[0]} ({version})", options.timestamps
+            )
+            return False
+        apport.log("Duplicate check negative", options.timestamps)
+    return True
+
+
 # pylint: disable-next=missing-function-docstring
 def main():
     # TODO: Split into smaller functions/methods
@@ -650,68 +723,8 @@ Thank you for your understanding, and sorry for the inconvenience!
                         " back to the crash database."
                     )
                 if not options.confirm or confirm_traces(report):
-                    # check for duplicates
-                    update_bug = True
-                    if options.duplicate_db:
-                        crashdb.init_duplicate_db(options.duplicate_db)
-                        res = crashdb.check_duplicate(crashid, report)
-                        if res:
-                            if res[1] is None:
-                                version = "not fixed yet"
-                            elif res[1] == "":
-                                version = "fixed in latest version"
-                            else:
-                                version = f"fixed in version {res[1]}"
-                            apport.log(
-                                f"Report is a duplicate of #{res[0]} ({version})",
-                                options.timestamps,
-                            )
-                            update_bug = False
-                        else:
-                            apport.log("Duplicate check negative", options.timestamps)
-
-                    if update_bug:
-                        if "Stacktrace" in report:
-                            crashdb.update_traces(crashid, report)
-                            apport.log(
-                                f"New attachments uploaded to crash database"
-                                f"LP: #{crashid}",
-                                options.timestamps,
-                            )
-                        else:
-                            # this happens when gdb crashes
-                            apport.log("No stack trace, invalid report", options.timestamps)
-
-                        if not report.has_useful_stacktrace():
-                            if sandbox.outdated_msg:
-                                invalid_msg = f"""Thank you for your report!
-
-However, processing it in order to get sufficient information for the
-developers failed (it does not generate a useful symbolic stack trace). This
-might be caused by some outdated packages which were installed on your system
-at the time of the report:
-
-{sandbox.outdated_msg}
-
-Please upgrade your system to the latest package versions. If you still
-encounter the crash, please file a new report.
-
-Thank you for your understanding, and sorry for the inconvenience!
-"""
-                                apport.log(
-                                    "No crash signature and outdated packages,"
-                                    " invalidating report",
-                                    options.timestamps,
-                                )
-                                crashdb.mark_retrace_failed(crashid, invalid_msg)
-                            else:
-                                apport.log(
-                                    "Report has no crash signature,"
-                                    " so retrace is flawed",
-                                    options.timestamps,
-                                )
-                                crashdb.mark_retrace_failed(crashid)
-
+                    if needs_update(report, options, crashid, crashdb):
+                        update_bug(report, options, sandbox, crashid, crashdb)
             elif options.output == "-":
                 report.write(sys.stdout.detach())
             else: