scripts: sphinx-pre-install: rework install command logic

Cleanup the code to remove some redundancy and to let it be
clearer about the command install instructions.

Ensure that special instructions will be shown only once,
before the actual install command.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
Link: https://lore.kernel.org/r/a6120449d9cc14346e867d1ef8944ae28ddbf3f6.1754992972.git.mchehab+huawei@kernel.org

scripts/sphinx-pre-install.py

@@ -213,7 +213,7 @@ class DepManager:
                 raise KeyError(f"ERROR!!!: invalid dtype for {progs}: {dtype}")
 
         if output_msg:
-            print(f"\n{output_msg}\n")
+            print(f"\n{output_msg}")
 
 class AncillaryMethods:
     """
@@ -583,23 +583,6 @@ class MissingCheckers(AncillaryMethods):
                 return f.read().strip()
         return ""
 
-    def check_missing(self, progs):
-        """
-        Check for missing dependencies using the provided program mapping.
-
-        The actual distro-specific programs are mapped via progs argument.
-
-        Returns True if there are missing dependencies, False otherwise.
-        """
-        self.install += self.deps.check_missing(progs)
-        if self.verbose_warn_install:
-            self.deps.warn_install()
-
-        if not self.deps.need and not self.deps.optional:
-            return False
-
-        return True
-
     def get_system_release(self):
         """
         Determine the system type. There's no unique way that would work
@@ -722,7 +705,7 @@ class SphinxDependencyChecker(MissingCheckers):
         self.recommend_python = None
 
         # Certain hints are meant to be shown only once
-        self.first_hint = True
+        self.distro_msg = None
 
         self.latest_avail_ver = (0, 0, 0)
         self.venv_ver = (0, 0, 0)
@@ -732,6 +715,33 @@ class SphinxDependencyChecker(MissingCheckers):
         self.conf = prefix + "Documentation/conf.py"
         self.requirement_file = prefix + "Documentation/sphinx/requirements.txt"
 
+    def get_install_progs(self, progs, cmd, extra=None):
+        """
+        Check for missing dependencies using the provided program mapping.
+
+        The actual distro-specific programs are mapped via progs argument.
+        """
+        install = self.deps.check_missing(progs)
+
+        if self.verbose_warn_install:
+            self.deps.warn_install()
+
+        if not install:
+            return
+
+        if cmd:
+            if self.verbose_warn_install:
+                msg = "You should run:"
+            else:
+                msg = ""
+
+            if extra:
+                msg += "\n\t" + extra.replace("\n", "\n\t")
+
+            return(msg + "\n\tsudo " + cmd + " " + install)
+
+        return None
+
     #
     # Distro-specific hints methods
     #
@@ -772,12 +782,7 @@ class SphinxDependencyChecker(MissingCheckers):
 
             self.check_program("dvipng", DepManager.PDF_MANDATORY)
 
-        if self.check_missing(progs):
+        return self.get_install_progs(progs, "apt-get install")
-            return
-
-        if self.verbose_warn_install:
-            print("You should run:")
-        print(f"\n\tsudo apt-get install {self.install}")
 
     def give_redhat_hints(self):
         """
@@ -838,18 +843,16 @@ class SphinxDependencyChecker(MissingCheckers):
                 self.deps.add_package("python39", DepManager.SYSTEM_MANDATORY)
                 self.recommend_python = True
 
-            if self.first_hint:
+            if not self.distro_msg:
-                print("Note: RHEL-based distros typically require extra repositories.\n" \
+                self.distro_msg = \
+                    "Note: RHEL-based distros typically require extra repositories.\n" \
                     "For most, enabling epel and crb are enough:\n" \
-                      "\tsudo dnf install -y epel-release", \
+                    "\tsudo dnf install -y epel-release\n" \
                     "\tsudo dnf config-manager --set-enabled crb\n" \
                     "Yet, some may have other required repositories. Those commands could be useful:\n" \
                     "\tsudo dnf repolist all\n" \
-                      "\tsudo dnf repoquery --available --info <pkgs>\n",
+                    "\tsudo dnf repoquery --available --info <pkgs>\n" \
-                      "\tsudo dnf config-manager --set-enabled '*' # enable all - probably not what you want")
+                    "\tsudo dnf config-manager --set-enabled '*' # enable all - probably not what you want"
-
-                self.first_hint = False
-
 
         if self.pdf:
             pdf_pkgs = [
@@ -868,13 +871,7 @@ class SphinxDependencyChecker(MissingCheckers):
             if not fedora and rel == 8:
                 self.deps.del_package("texlive-ctex")
 
-        if self.check_missing(progs):
+        return self.get_install_progs(progs, "dnf install")
-            return
-
-        if self.verbose_warn_install:
-            print("You should run:")
-
-        print(f"\n\tsudo dnf install -y {self.install}")
 
     def give_opensuse_hints(self):
         """
@@ -945,12 +942,7 @@ class SphinxDependencyChecker(MissingCheckers):
         if self.pdf:
             self.check_missing_tex()
 
-        if self.check_missing(progs):
+        return self.get_install_progs(progs, "zypper install --no-recommends")
-            return
-
-        if self.verbose_warn_install:
-            print("You should run:")
-        print(f"\n\tsudo zypper install --no-recommends {self.install}")
 
     def give_mageia_hints(self):
         """
@@ -995,12 +987,7 @@ class SphinxDependencyChecker(MissingCheckers):
             self.check_missing_file(pdf_pkgs, noto_sans, DepManager.PDF_MANDATORY)
             self.check_rpm_missing(tex_pkgs, DepManager.PDF_MANDATORY)
 
-        if self.check_missing(progs):
+        return self.get_install_progs(progs, packager_cmd)
-            return
-
-        if self.verbose_warn_install:
-            print("You should run:")
-        print(f"\n\tsudo {packager_cmd} {self.install}")
 
     def give_arch_linux_hints(self):
         """
@@ -1023,20 +1010,15 @@ class SphinxDependencyChecker(MissingCheckers):
         ]
 
         if self.pdf:
-            self.check_pacman_missing(archlinux_tex_pkgs, DepManager.PDF_MANDATORY)
+            self.check_pacman_missing(archlinux_tex_pkgs,
+                                      DepManager.PDF_MANDATORY)
 
-            self.check_missing_file(
+            self.check_missing_file(["/usr/share/fonts/noto-cjk/NotoSansCJK-Regular.ttc"],
-                ["/usr/share/fonts/noto-cjk/NotoSansCJK-Regular.ttc"],
                                     "noto-fonts-cjk",
-                2,
+                                    DepManager.PDF_MANDATORY)
-            )
 
-        if self.check_missing(progs):
-            return
 
-        if self.verbose_warn_install:
+        return self.get_install_progs(progs, "pacman -S")
-            print("You should run:")
-        print(f"\n\tsudo pacman -S {self.install}")
 
     def give_gentoo_hints(self):
         """
@@ -1065,13 +1047,6 @@ class SphinxDependencyChecker(MissingCheckers):
             for package, files in pdf_pkgs.items():
                 self.check_missing_file(files, package, DepManager.PDF_MANDATORY)
 
-        if self.check_missing(progs):
-            return
-
-        if self.verbose_warn_install:
-            print("You should run:")
-        print("\n")
-
         # Handling dependencies is a nightmare, as Gentoo refuses to emerge
         # some packages if there's no package.use file describing them.
         # To make it worse, compilation flags shall also be present there
@@ -1104,7 +1079,10 @@ class SphinxDependencyChecker(MissingCheckers):
             "zziblib": "dev-libs/zziplib sdl",
         }
 
-        if self.first_hint:
+        extra_cmds = ""
+        if not self.distro_msg:
+            self.distro_msg = "Note: Gentoo requires package.use to be adjusted before emerging packages"
+
             use_base = "/etc/portage/package.use"
             files = glob(f"{use_base}/*")
 
@@ -1148,23 +1126,23 @@ class SphinxDependencyChecker(MissingCheckers):
 
                 # emit a code to setup missing USE
                 if install:
-                    print(f"\tsudo su -c 'echo \"{portage}\" > {use_base}/{fname}'")
+                    extra_cmds += (f"sudo su -c 'echo \"{portage}\" > {use_base}/{fname}'\n")
-
-            self.first_hint = False
 
         # Now, we can use emerge and let it respect USE
-        print(f"\tsudo emerge --ask --changed-use --binpkg-respect-use=y {self.install}")
+        return self.get_install_progs(progs,
+                                      "emerge --ask --changed-use --binpkg-respect-use=y",
+                                      extra_cmds)
 
-    #
+    def get_install(self):
-    # Dispatch the check to an os_specific hinter
-    #
-
-    def check_distros(self):
         """
-        OS-specific hints logic. Seeks for a hinter. If found, provide
+        OS-specific hints logic. Seeks for a hinter. If found, use it to
-        package-manager-specific install commands.
+        provide package-manager specific install commands.
 
-        Otherwise, just lists the missing dependencies.
+        Otherwise, outputs install instructions for the meta-packages.
+
+        Returns a string with the command to be executed to install the
+        the needed packages, if distro found. Otherwise, return just a
+        list of packages that require installation.
         """
         os_hints = {
             re.compile("Red Hat Enterprise Linux"):   self.give_redhat_hints,
@@ -1195,9 +1173,7 @@ class SphinxDependencyChecker(MissingCheckers):
         # If the OS is detected, use per-OS hint logic
         for regex, os_hint in os_hints.items():
             if regex.search(self.system_release):
-                os_hint()
+                return os_hint()
-
-                return
 
         #
         # Fall-back to generic hint code for other distros
@@ -1207,11 +1183,12 @@ class SphinxDependencyChecker(MissingCheckers):
         if self.pdf:
             self.check_missing_tex()
 
-        self.check_missing(progs)
+        self.distro_msg = \
+            f"I don't know distro {self.system_release}.\n" \
+            "So, I can't provide you a hint with the install procedure.\n" \
+            "There are likely missing dependencies.\n"
 
-        print(f"I don't know distro {self.system_release}.")
+        return self.get_install_progs(progs, None)
-        print("So, I can't provide you a hint with the install procedure.")
-        print("There are likely missing dependencies.")
 
     #
     # Common dependencies
@@ -1336,7 +1313,6 @@ class SphinxDependencyChecker(MissingCheckers):
 
         self.pdf = False
         self.deps.optional = 0
-        self.install = ""
         old_verbose = self.verbose_warn_install
         self.verbose_warn_install = 0
 
@@ -1344,7 +1320,9 @@ class SphinxDependencyChecker(MissingCheckers):
 
         self.deps.add_package("python-sphinx", DepManager.PYTHON_MANDATORY)
 
-        self.check_distros()
+        cmd = self.get_install()
+        if cmd:
+            print(cmd)
 
         self.deps.need = old_need
         self.deps.optional = old_optional
@@ -1511,7 +1489,14 @@ class SphinxDependencyChecker(MissingCheckers):
             self.check_program("latexmk", DepManager.PDF_MANDATORY)
 
         # Do distro-specific checks and output distro-install commands
-        self.check_distros()
+        cmd = self.get_install()
+        if cmd:
+            print(cmd)
+
+        # If distro requires some special instructions, print here.
+        # Please notice that get_install() needs to be called first.
+        if self.distro_msg:
+            print("\n" + self.distro_msg)
 
         if not self.python_cmd:
             if self.need == 1: