Merge pull request #16 from oatmealraisin/user_services

Fix bug in module user systemd unit handling

README.md

@@ -125,7 +125,7 @@ So far I have not created an AUR package for decman, because I'm not sure if oth
 
 ### Packages
 
-Decman can be used to install pacman packages. Decman will install all packages defined in the source and **remove** all packages not defined in the source. You can set packages to be ignored by decman, so that it won't install them nor remove them.
+Decman can be used to install pacman packages. Decman will install all packages defined in the source and **remove** all explicitly installed packages not defined in the source. You don't need to list dependencies as those will be handeled by pacman. You can set packages to be ignored by decman, so that it won't install them nor remove them.
 
 ```py
 # Include both foreign and pacman packages here.
@@ -148,7 +148,7 @@ decman.user_packages.append(
         pkgname="decman-git",
         # Note, this example may not be up to date
         provides=["decman"],
-        version="0.2.1",
+        version="0.3.1",
         dependencies=["python", "python-requests", "devtools", "pacman", "systemd", "git"],
         make_dependencies=[
             "python-setuptools", "python-build", "python-installer", "python-wheel"
@@ -163,6 +163,9 @@ Build packages are stored in a cache `/var/cache/decman`. By default decman keep
 
 ### Systemd units
 
+> [!NOTE]
+> Decman will only enable and disable systemd systemd. It will not start or stop them.
+
 Decman can enable systemd services, system wide or for a specific user. Decman will enable all units defined in the source, and disable them when they are removed from the source. If a unit is not defined in the source, decman will not touch it.
 
 ### Files
@@ -276,7 +279,7 @@ user_packages = [{
     pkgname="decman-git",
     # Note, this example may not be up to date
     provides=["decman"],
-    version="0.2.1",
+    version="0.3.1",
     dependencies=["python", "python-requests", "devtools", "pacman", "systemd", "git"],
     make_dependencies=[
         "python-setuptools", "python-build", "python-installer", "python-wheel"

example/my_module.py

@@ -82,7 +82,7 @@ class MyModule(Module):
         return [
             UserPackage(
                 pkgname="decman-git",
-                version="0.2.1",
+                version="0.3.1",
                 provides=["decman"],
                 dependencies=[
                     "python",

example/source.py

@@ -40,7 +40,7 @@ decman.config.makepkg_user = "kk"
 decman.user_packages.append(
     UserPackage(
         pkgname="decman-git",
-        version="0.2.1",
+        version="0.3.1",
         provides=["decman"],
         dependencies=[
             "python",
@@ -132,6 +132,23 @@ decman.config.suppress_command_output = True
 # Make output less verbose. Summaries are still printed.
 decman.config.quiet_output = False
 
+# Decman captures pacman command output, and any line (and adjacent lines) that contains any of
+# the following keywords (case-insensetive) will be printed after the pacman command finishes.
+#
+# REMEMBER: You should still generally pay attention to pacman output
+# since these keywords may not catch all cases.
+decman.config.pacman_output_keywords = [
+    "pacsave",
+    "pacnew",
+    # Additional keywords can be:
+    #"warning",
+    #"error",
+    #"note",
+    # They might cause too many highlights however.
+]
+# If you don't want to print lines that contain keywords, set this to False
+decman.config.print_pacman_output_highlights = True
+
 # The user which builds aur and user packages.
 # decman.config.makepkg_user = "nobody" # This was set in a previous example. Let's not override it.
 
@@ -163,26 +180,30 @@ class MyCommands(decman.config.Commands):
     def list_foreign_pkgs_versioned(self) -> list[str]:
         return ["pacman", "-Qm", "--color=never"]
 
+    # --color=always is used in many commands since --color=auto results in no color.
+    # It seems a sensible default for me, since decman already uses color and it can't be disabled.
+
     def install_pkgs(self, pkgs: list[str]) -> list[str]:
-        return ["pacman", "-S", "--needed"] + pkgs
+        return ["pacman", "-S", "--color=always", "--needed"] + pkgs
 
     def install_files(self, pkg_files: list[str]) -> list[str]:
-        return ["pacman", "-U", "--asdeps"] + pkg_files
+        return ["pacman", "-U", "--color=always", "--asdeps"] + pkg_files
 
     def set_as_explicitly_installed(self, pkgs: list[str]) -> list[str]:
         return ["pacman", "-D", "--asexplicit"] + pkgs
 
     def install_deps(self, deps: list[str]) -> list[str]:
-        return ["pacman", "-S", "--needed", "--asdeps"] + deps
+        return ["pacman", "-S", "--color=always", "--needed", "--asdeps"
+                ] + deps
 
     def is_installable(self, pkg: str) -> list[str]:
         return ["pacman", "-Sddp", pkg]
 
     def upgrade(self) -> list[str]:
-        return ["pacman", "-Syu"]
+        return ["pacman", "-Syu", "--color=always"]
 
     def remove(self, pkgs: list[str]) -> list[str]:
-        return ["pacman", "-Rs"] + pkgs
+        return ["pacman", "-Rs", "--color=always"] + pkgs
 
     def enable_units(self, units: list[str]) -> list[str]:
         return ["systemctl", "enable"] + units
@@ -209,6 +230,9 @@ class MyCommands(decman.config.Commands):
     def git_get_commit_id(self) -> list[str]:
         return ["git", "rev-parse", "HEAD"]
 
+    def git_log_commit_ids(self) -> list[str]:
+        return ["git", "log", "--format=format:%H"]
+
     def review_file(self, file: str) -> list[str]:
         return ["less", file]
 

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "decman"
-version = "0.2.1"
+version = "0.3.1"
 description = "Declarative package & configuration manager for Arch Linux."
 license = {file = "LICENSE"}
 authors = [

src/decman/app.py

@@ -218,6 +218,9 @@ class Core:
     def _disable_units(self):
         to_disable = self.source.units_to_disable(self.store)
         l.print_list("Disabling systemd units:", to_disable)
+        if to_disable:
+            l.print_info(
+                "Disabled systemd units won't be stopped automatically.")
         if not self.only_print:
             self.systemctl.disable_units(to_disable)
 
@@ -286,6 +289,9 @@ class Core:
     def _enable_units(self):
         to_enable = self.source.units_to_enable(self.store)
         l.print_list("Enabling systemd units:", to_enable)
+        if to_enable:
+            l.print_info(
+                "Enabled systemd units won't be started automatically.")
         if not self.only_print:
             self.systemctl.enable_units(to_enable)
 

src/decman/config.py

@@ -45,26 +45,27 @@ class Commands:
         """
         Running this command installs the given packages from pacman repositories.
         """
-        return ["pacman", "-S", "--needed"] + pkgs
+        return ["pacman", "-S", "--color=always", "--needed"] + pkgs
 
     def install_files(self, pkg_files: list[str]) -> list[str]:
         """
         Running this command installs the given packages files.
         """
-        return ["pacman", "-U", "--asdeps"] + pkg_files
+        return ["pacman", "-U", "--color=always", "--asdeps"] + pkg_files
 
     def set_as_explicitly_installed(self, pkgs: list[str]) -> list[str]:
         """
         Running this command installs sets the given as explicitly installed.
         """
-        return ["pacman", "-D", "--asexplicit"] + pkgs
+        return ["pacman", "-D", "--color=always", "--asexplicit"] + pkgs
 
     def install_deps(self, deps: list[str]) -> list[str]:
         """
         Running this command installs the given packages from pacman repositories.
         The packages are installed as dependencies.
         """
-        return ["pacman", "-S", "--needed", "--asdeps"] + deps
+        return ["pacman", "-S", "--color=always", "--needed", "--asdeps"
+                ] + deps
 
     def is_installable(self, pkg: str) -> list[str]:
         """
@@ -76,14 +77,14 @@ class Commands:
         """
         Running this command upgrades all pacman packages.
         """
-        return ["pacman", "-Syu"]
+        return ["pacman", "-Syu", "--color=always"]
 
     def remove(self, pkgs: list[str]) -> list[str]:
         """
         Running this command removes the given packages and their dependencies
         (that aren't required by other packages).
         """
-        return ["pacman", "-Rs"] + pkgs
+        return ["pacman", "-Rs", "--color=always"] + pkgs
 
     def enable_units(self, units: list[str]) -> list[str]:
         """
@@ -135,6 +136,12 @@ class Commands:
         """
         return ["git", "rev-parse", "HEAD"]
 
+    def git_log_commit_ids(self) -> list[str]:
+        """
+        Running this command outputs commit hashes of the repository.
+        """
+        return ["git", "log", "--format=format:%H"]
+
     def review_file(self, file: str) -> list[str]:
         """
         Running this command outputs a file for the user to see.
@@ -199,6 +206,16 @@ valid_pkgexts: list[str] = [
     ".pkg.tar.Z",
 ]
 
+pacman_output_keywords: list[str] = [
+    "pacsave",
+    "pacnew",
+    # These cause too many false positives IMO
+    #"warning",
+    #"error",
+    #"note",
+]
+print_pacman_output_highlights: bool = True
+
 makepkg_user: str = "nobody"
 build_dir: str = "/tmp/decman/build"
 pkg_cache_dir: str = "/var/cache/decman"

src/decman/lib/__init__.py

@@ -2,7 +2,8 @@
 Library module for decman.
 """
 
-import pwd
+import threading
+import sys
 import shutil
 import subprocess
 import json
@@ -679,11 +680,13 @@ class Source:
         return result
 
     def _all_user_units(self) -> dict[str, set[str]]:
-        result = {}
-        result.update(self.systemd_user_units)
-        for module in self.modules:
-            if module.enabled:
-                result.update(module.systemd_user_units())
+        result = self.systemd_user_units
+        for module in [m for m in self.modules if m.enabled]:
+            module_user_units: dict[str, list[str]] = module.systemd_user_units()
+            for user in module_user_units.keys():
+                if user not in result:
+                    result[user] = set()
+                result[user].update(module_user_units[user])
         return result
 
 
@@ -755,14 +758,23 @@ class Pacman:
         if not packages:
             return
 
+        returncode, output = echo_and_capture_command(
+            conf.commands.install_pkgs(packages))
+        if returncode != 0:
+            raise err.UserFacingError(
+                f"Failed to install packages using pacman. Process exited with code {returncode}."
+            )
+        if conf.print_pacman_output_highlights:
+            print_highlighted_pacman_messages(output)
+
         try:
-            subprocess.run(conf.commands.install_pkgs(packages), check=True)
             subprocess.run(conf.commands.set_as_explicitly_installed(packages),
                            check=True,
                            capture_output=conf.suppress_command_output)
         except subprocess.CalledProcessError as error:
             raise err.UserFacingError(
-                "Failed to install packages using pacman.") from error
+                "Failed to set packages as explicitly installed using pacman."
+            ) from error
 
     def install_dependencies(self, deps: list[str]):
         """
@@ -771,12 +783,14 @@ class Pacman:
         if not deps:
             return
 
-        try:
-            subprocess.run(conf.commands.install_deps(deps), check=True)
-        except subprocess.CalledProcessError as error:
+        returncode, output = echo_and_capture_command(
+            conf.commands.install_deps(deps))
+        if returncode != 0:
             raise err.UserFacingError(
-                "Failed to install packages as dependencies using pacman."
-            ) from error
+                f"Failed to install packages as dependencies using pacman. Process exited with code {returncode}."
+            )
+        if conf.print_pacman_output_highlights:
+            print_highlighted_pacman_messages(output)
 
     def install_files(self, files: list[str], as_explicit: list[str]):
         """
@@ -786,9 +800,16 @@ class Pacman:
         if not files:
             return
 
-        try:
-            subprocess.run(conf.commands.install_files(files), check=True)
+        returncode, output = echo_and_capture_command(
+            conf.commands.install_files(files))
+        if returncode != 0:
+            raise err.UserFacingError(
+                f"Failed to install package files using pacman. Process exited with code {returncode}."
+            )
+        if conf.print_pacman_output_highlights:
+            print_highlighted_pacman_messages(output)
 
+        try:
             if as_explicit:
                 subprocess.run(
                     conf.commands.set_as_explicitly_installed(as_explicit),
@@ -799,17 +820,20 @@ class Pacman:
                 print_error("Output:")
                 print_continuation(error.output)
             raise err.UserFacingError(
-                "Failed to install package files using pacman.") from error
+                "Failed to set packages as explicitly installed using pacman."
+            ) from error
 
     def upgrade(self):
         """
         Upgrades all packages.
         """
-        try:
-            subprocess.run(conf.commands.upgrade(), check=True)
-        except subprocess.CalledProcessError as error:
+        returncode, output = echo_and_capture_command(conf.commands.upgrade())
+        if returncode != 0:
             raise err.UserFacingError(
-                "Failed to upgrade packages using pacman.") from error
+                f"Failed to upgrade packages using pacman. Process exited with code {returncode}."
+            )
+        if conf.print_pacman_output_highlights:
+            print_highlighted_pacman_messages(output)
 
     def remove(self, packages: list[str]):
         """
@@ -817,11 +841,95 @@ class Pacman:
         """
         if not packages:
             return
-        try:
-            subprocess.run(conf.commands.remove(packages), check=True)
-        except subprocess.CalledProcessError as error:
+
+        returncode, output = echo_and_capture_command(
+            conf.commands.remove(packages))
+        if returncode != 0:
             raise err.UserFacingError(
-                "Failed to remove packages using pacman.") from error
+                f"Failed to remove packages using pacman. Process exited with code {returncode}."
+            )
+        if conf.print_pacman_output_highlights:
+            print_highlighted_pacman_messages(output)
+
+
+def print_highlighted_pacman_messages(output: str):
+    """
+    Prints lines that contain pacman output keywords.
+    """
+    print_summary("Pacman output highlights:")
+    lines = output.split("\n")
+    for index, line in enumerate(lines):
+        for keyword in conf.pacman_output_keywords:
+            if keyword.lower() in line.lower():
+                print_summary(f"lines: {index}-{index+2}")
+                if index >= 1:
+                    print_continuation(lines[index - 1])
+                print_continuation(line)
+                if index + 1 < len(lines):
+                    print_continuation(lines[index + 1])
+                print_continuation("")
+
+                # Break, as to not print the same line again if it contains multiple keywords
+                break
+
+
+def echo_and_capture_command(program: list[str]) -> tuple[int, str]:
+    """
+    Runs the given CLI program and arguments.
+
+    Returns a tuple containing the return code of the program as well as all output of the program.
+    """
+    with subprocess.Popen(program,
+                          stdin=subprocess.PIPE,
+                          stdout=subprocess.PIPE,
+                          stderr=subprocess.STDOUT) as process:
+        os.set_blocking(process.stdout.fileno(), False)
+
+        output_thread = _OutputCapturingThread(process.stdout)
+        output_thread.start()
+
+        os.set_blocking(sys.stdin.fileno(), False)
+
+        # Capture stdin and forward it to the process in a non-blocking manner
+        while process.poll() is None:
+            inp = sys.stdin.readline()
+
+            if inp:
+                process.stdin.write(inp.encode())
+                process.stdin.flush()
+
+            time.sleep(0.1)
+
+        os.set_blocking(sys.stdin.fileno(), True)
+
+        output_thread.done = True
+        output_thread.join()
+
+        # Capture any output that may not have been yet captured
+        output = output_thread.output
+        missing_output = process.stdout.read()
+        if missing_output:
+            output += missing_output.decode()
+
+        return (process.returncode, output)
+
+
+class _OutputCapturingThread(threading.Thread):
+
+    def __init__(self, stream):
+        super().__init__()
+        self._stream = stream
+        self.output = ""
+        self.done = False
+
+    def run(self):
+        while not self.done and not self._stream.closed:
+            output = self._stream.read(1000)
+            if output:
+                output = output.decode()
+                self.output += output
+                print(output, end="", flush=True)
+            time.sleep(0.1)
 
 
 class Systemd:

src/decman/lib/fpm.py

@@ -1078,7 +1078,14 @@ before and thus are found in the cache."
                                 default=True):
                 latest_reviewed_commit = self._store.pkgbuild_latest_reviewed_commits.get(
                     pkgbase)
-                if latest_reviewed_commit is None:
+
+                git_commit_ids = subprocess.run(
+                    conf.commands.git_log_commit_ids(),
+                    check=True,
+                    stdout=subprocess.PIPE,
+                ).stdout.decode().strip().split('\n')
+
+                if latest_reviewed_commit is None or latest_reviewed_commit not in git_commit_ids:
                     for file in os.scandir("."):
                         if file.is_file() and not file.name.startswith("."):
                             subprocess.run(conf.commands.review_file(

tests/test_source_resolution.py

@@ -264,3 +264,49 @@ class TestSource(unittest.TestCase):
             self.source.packages_to_remove(self.currently_installed_packages),
             ["p4", "A4", "M_A1", "M_A2"],
         )
+
+class TestModuleUserServices(unittest.TestCase):
+
+    class ModuleWithUserServiceOne(Module):
+
+        def __init__(self):
+            super().__init__("one", True, "0")
+
+        def systemd_user_units(self) -> dict[str, list[str]]:
+            return {
+                    "user": ['foo.service']
+                    }
+
+    class ModuleWithUserServiceTwo(Module):
+
+        def __init__(self):
+            super().__init__("two", True, "0")
+
+        def systemd_user_units(self) -> dict[str, list[str]]:
+            return {
+                    "user": ['bar.service']
+                    }
+
+    def setUp(self) -> None:
+        self.source = Source(
+                pacman_packages=set(),
+                aur_packages=set(),
+                user_packages=set(),
+                ignored_packages=set(),
+                systemd_units=set(),
+                systemd_user_units={},
+                files={},
+                directories={},
+                modules={                
+                         self.ModuleWithUserServiceOne(),
+                         self.ModuleWithUserServiceTwo()
+                         },
+            )
+        self.store = Store()
+
+
+    def test_user_units_to_enable(self):
+        self.assertDictEqual(
+            self.source.user_units_to_enable(self.store),
+            {"user": ["foo.service", "bar.service"]},
+        )