Merge pull request #81087 from lovesegfault/tlp-1.3.1

tlp: 1.2.2 -> 1.3.1
2020-02-27 19:43:14 +00:00 · 2020-02-27 19:43:14 +00:00 · 76f4f6b95d
commit 76f4f6b95d
parent 29b495d4d7 ee7becd918
4 changed files with 304 additions and 144 deletions
--- a/nixos/modules/services/hardware/tlp.nix
+++ b/nixos/modules/services/hardware/tlp.nix
@ -1,39 +1,26 @@
 { config, lib, pkgs, ... }:
-
 with lib;
-
 let
-
-cfg = config.services.tlp;
-
-enableRDW = config.networking.networkmanager.enable;
-
-tlp = pkgs.tlp.override {
-  inherit enableRDW;
-};
-
-# XXX: We can't use writeTextFile + readFile here because it triggers
-# TLP build to get the .drv (even on --dry-run).
-confFile = pkgs.runCommand "tlp"
-  { config = cfg.extraConfig;
-    passAsFile = [ "config" ];
-    preferLocalBuild = true;
-  }
-  ''
-    cat ${tlp}/etc/default/tlp > $out
-    cat $configPath >> $out
-  '';
-
+  cfg = config.services.tlp;
+  enableRDW = config.networking.networkmanager.enable;
+  tlp = pkgs.tlp.override { inherit enableRDW; };
+  # TODO: Use this for having proper parameters in the future
+  mkTlpConfig = tlpConfig: generators.toKeyValue {
+    mkKeyValue = generators.mkKeyValueDefault {
+      mkValueString = val:
+        if isInt val then toString val
+        else if isString val then val
+        else if true == val then "1"
+        else if false == val then "0"
+        else if isList val then "\"" + (concatStringsSep " " val) + "\""
+        else err "invalid value provided to mkTlpConfig:" (toString val);
+    } "=";
+  } tlpConfig;
 in
-
 {
-
  ###### interface
-
  options = {
-
    services.tlp = {
-
      enable = mkOption {
        type = types.bool;
        default = false;
@ -45,77 +32,64 @@ in
        default = "";
        description = "Additional configuration variables for TLP";
      };
-
    };
-
  };

-
  ###### implementation
-
  config = mkIf cfg.enable {
+    boot.kernelModules = [ "msr" ];

-    powerManagement.scsiLinkPolicy = null;
-    powerManagement.cpuFreqGovernor = null;
-    powerManagement.cpufreq.max = null;
-    powerManagement.cpufreq.min = null;
+    environment.etc = {
+      "tlp.conf".text = cfg.extraConfig;
+    } // optionalAttrs enableRDW {
+      "NetworkManager/dispatcher.d/99tlp-rdw-nm".source =
+        "${tlp}/etc/NetworkManager/dispatcher.d/99tlp-rdw-nm";
+    };

-    systemd.sockets.systemd-rfkill.enable = false;
+    environment.systemPackages = [ tlp ];

-    systemd.services = {
-      "systemd-rfkill@".enable = false;
-      systemd-rfkill.enable = false;
-
-      tlp = {
-        description = "TLP system startup/shutdown";
-
-        after = [ "multi-user.target" ];
-        wantedBy = [ "multi-user.target" ];
-        before = [ "shutdown.target" ];
-        restartTriggers = [ confFile ];
-
-        serviceConfig = {
-          Type = "oneshot";
-          RemainAfterExit = true;
-          ExecStart = "${tlp}/bin/tlp init start";
-          ExecStop = "${tlp}/bin/tlp init stop";
-        };
-      };
-
-      tlp-sleep = {
-        description = "TLP suspend/resume";
-
-        wantedBy = [ "sleep.target" ];
-        before = [ "sleep.target" ];
-
-        unitConfig = {
-          StopWhenUnneeded = true;
-        };
-
-        serviceConfig = {
-          Type = "oneshot";
-          RemainAfterExit = true;
-          ExecStart = "${tlp}/bin/tlp suspend";
-          ExecStop = "${tlp}/bin/tlp resume";
-        };
-      };
+    # FIXME: When the config is parametrized we need to move these into a
+    # conditional on the relevant options being enabled.
+    powerManagement = {
+      scsiLinkPolicy = null;
+      cpuFreqGovernor = null;
+      cpufreq.max = null;
+      cpufreq.min = null;
    };

    services.udev.packages = [ tlp ];

-    environment.etc =
-      {
-        "default/tlp".source = confFile;
-      } // optionalAttrs enableRDW {
-        "NetworkManager/dispatcher.d/99tlp-rdw-nm" = {
-          source = "${tlp}/etc/NetworkManager/dispatcher.d/99tlp-rdw-nm";
-        };
+    systemd = {
+      packages = [ tlp ];
+      # XXX: These must always be disabled/masked according to [1].
+      #
+      # [1]: https://github.com/linrunner/TLP/blob/a9ada09e0821f275ce5f93dc80a4d81a7ff62ae4/tlp-stat.in#L319
+      sockets.systemd-rfkill.enable = false;
+      services.systemd-rfkill.enable = false;
+
+      services.tlp = {
+        # XXX: The service should reload whenever the configuration changes,
+        # otherwise newly set power options remain inactive until reboot (or
+        # manual unit restart.)
+        restartTriggers = [ config.environment.etc."tlp.conf".source ];
+        # XXX: When using systemd.packages (which we do above) the [Install]
+        # section of systemd units does not work (citation needed) so we manually
+        # enforce it here.
+        wantedBy = [ "multi-user.target" ];
      };

-    environment.systemPackages = [ tlp ];
-
-    boot.kernelModules = [ "msr" ];
-
+      services.tlp-sleep = {
+        # XXX: When using systemd.packages (which we do above) the [Install]
+        # section of systemd units does not work (citation needed) so we manually
+        # enforce it here.
+        before = [ "sleep.target" ];
+        wantedBy = [ "sleep.target" ];
+        # XXX: `tlp suspend` requires /var/lib/tlp to exist in order to save
+        # some stuff in there. There is no way, that I know of, to do this in
+        # the package itself, so we do it here instead making sure the unit
+        # won't fail due to the save dir not existing.
+        serviceConfig.StateDirectory = "tlp";
+      };
+    };
  };
-
 }
--- a/pkgs/tools/misc/tlp/default.nix
+++ b/pkgs/tools/misc/tlp/default.nix
@ -1,81 +1,126 @@
-{ stdenv, lib, fetchFromGitHub, perl, makeWrapper, file, systemd, iw, rfkill
-, hdparm, ethtool, inetutils , kmod, pciutils, smartmontools
-, x86_energy_perf_policy, gawk, gnugrep, coreutils, utillinux
-, checkbashisms, shellcheck
-, enableRDW ? false, networkmanager
-}:
-
-let
-  paths = lib.makeBinPath
-          ([ iw rfkill hdparm ethtool inetutils systemd kmod pciutils smartmontools
-             x86_energy_perf_policy gawk gnugrep coreutils utillinux
-           ]
-           ++ lib.optional enableRDW networkmanager
-          );
-
-in stdenv.mkDerivation rec {
+{ stdenv
+, lib
+, checkbashisms
+, coreutils
+, ethtool
+, fetchFromGitHub
+, gawk
+, gnugrep
+, gnused
+, hdparm
+, iw
+, kmod
+, makeWrapper
+, pciutils
+, perl
+, shellcheck
+, smartmontools
+, systemd
+, utillinux
+, x86_energy_perf_policy
+  # RDW only works with NetworkManager, and thus is optional with default off
+, enableRDW ? false
+, networkmanager
+}: stdenv.mkDerivation rec {
  pname = "tlp";
-  version = "1.2.2";
+  version = "1.3.1";

  src = fetchFromGitHub {
    owner = "linrunner";
    repo = "TLP";
    rev = version;
-    sha256 = "0vm31ca6kdak9xzwskz7a8hvdp67drfh2zcdwlz3260r8r2ypgg1";
+    sha256 = "14fcnaz9pw534v4d8dddqq4wcvpf1kghr8zlrk62r5lrl46sp1p5";
  };

-  outRef = placeholder "out";
-
-  makeFlags = [
-    "DESTDIR=${outRef}"
-    "TLP_SBIN=${outRef}/bin"
-    "TLP_BIN=${outRef}/bin"
-    "TLP_TLIB=${outRef}/share/tlp"
-    "TLP_FLIB=${outRef}/share/tlp/func.d"
-    "TLP_ULIB=${outRef}/lib/udev"
-    "TLP_NMDSP=${outRef}/etc/NetworkManager/dispatcher.d"
-    "TLP_SHCPL=${outRef}/share/bash-completion/completions"
-    "TLP_MAN=${outRef}/share/man"
-    "TLP_META=${outRef}/share/metainfo"
-
-    "TLP_NO_INIT=1"
-  ];
-
-  nativeBuildInputs = [ makeWrapper file ];
+  # XXX: See patch files for relevant explanations.
+  patches = [ ./patches/fix-makefile-sed.patch ./patches/tlp-sleep-service.patch ];

  buildInputs = [ perl ];
+  nativeBuildInputs = [ makeWrapper gnused ];

-  installTargets = [ "install-tlp" "install-man" ] ++ stdenv.lib.optional enableRDW "install-rdw";
+  # XXX: While [1] states that DESTDIR should not be used, and that the correct
+  # variable to set is, in fact, PREFIX, tlp thinks otherwise. The Makefile for
+  # tlp concerns itself only with DESTDIR [2] (possibly incorrectly) and so we set
+  # that as opposed to PREFIX, despite what [1] says.
+  #
+  # [1]: https://github.com/NixOS/nixpkgs/issues/65718
+  # [2]: https://github.com/linrunner/TLP/blob/ab788abf4936dfb44fbb408afc34af834230a64d/Makefile#L4-L46
+  makeFlags = [
+    "DESTDIR=${placeholder "out"}"

-  checkInputs = [
-    checkbashisms
-    shellcheck
+    "TLP_NO_INIT=1"
+    "TLP_WITH_ELOGIND=0"
+    "TLP_WITH_SYSTEMD=1"
+
+    "TLP_BIN=/bin"
+    "TLP_CONFDEF=/share/tlp/defaults.conf"
+    "TLP_FLIB=/share/tlp/func.d"
+    "TLP_MAN=/share/man"
+    "TLP_META=/share/metainfo"
+    "TLP_SBIN=/sbin"
+    "TLP_SHCPL=/share/bash-completion/completions"
+    "TLP_TLIB=/share/tlp"
  ];

-  doCheck = true;
+  installTargets = [ "install-tlp" "install-man" ]
+  ++ lib.optionals enableRDW [ "install-rdw" "install-man-rdw" ];
+
+  # XXX: This is disabled because it's basically just noise since upstream
+  # itself does not seem to care about the zillion shellcheck errors.
+  doCheck = false;
+  checkInputs = [ checkbashisms shellcheck ];
  checkTarget = [ "checkall" ];

-  postInstall = ''
-    cp -r $out/$out/* $out
-    rm -rf $out/$(echo "$NIX_STORE" | cut -d "/" -f2)
+  postInstall = let
+    paths = lib.makeBinPath (
+      [
+        coreutils
+        ethtool
+        gawk
+        gnugrep
+        gnused
+        hdparm
+        iw
+        kmod
+        pciutils
+        perl
+        smartmontools
+        systemd
+        utillinux
+        x86_energy_perf_policy
+      ] ++ lib.optional enableRDW networkmanager
+    );
+  in
+    ''
+      fixup_perl=(
+        $out/share/tlp/tlp-pcilist
+        $out/share/tlp/tlp-readconfs
+        $out/share/tlp/tlp-usblist
+        $out/share/tlp/tpacpi-bat
+      )
+      for f in "''${fixup_perl[@]}"; do
+        wrapProgram "$f" --prefix PATH : "${paths}"
+      done

-    for i in $out/bin/* $out/lib/udev/tlp-* ${lib.optionalString enableRDW "$out/etc/NetworkManager/dispatcher.d/*"}; do
-      if file "$i" | grep -q Perl; then
-        # Perl script; use wrapProgram
-        wrapProgram "$i" \
-          --prefix PATH : "${paths}"
-      else
-        # Bash script
-        sed -i '2iexport PATH=${paths}:$PATH' "$i"
-      fi
-    done
-  '';
+      fixup_bash=(
+        $out/bin/*
+        $out/etc/NetworkManager/dispatcher.d/*
+        $out/lib/udev/tlp-*
+        $out/sbin/*
+        $out/share/tlp/func.d/*
+        $out/share/tlp/tlp-func-base
+      )
+      for f in "''${fixup_bash[@]}"; do
+        sed -i '2iexport PATH=${paths}:$PATH' "$f"
+      done
+    '';

-  meta = with stdenv.lib; {
+  meta = with lib; {
    description = "Advanced Power Management for Linux";
-    homepage = https://linrunner.de/en/tlp/docs/tlp-linux-advanced-power-management.html;
+    homepage =
+      "https://linrunner.de/en/tlp/docs/tlp-linux-advanced-power-management.html";
    platforms = platforms.linux;
-    maintainers = with maintainers; [ abbradar ];
+    maintainers = with maintainers; [ abbradar lovesegfault ];
    license = licenses.gpl2Plus;
  };
 }
--- a/pkgs/tools/misc/tlp/patches/fix-makefile-sed.patch
+++ b/pkgs/tools/misc/tlp/patches/fix-makefile-sed.patch
@ -0,0 +1,46 @@
+commit c44347b3b813e209fff537b4d46d23430727a5e2
+Author: Bernardo Meurer <meurerbernardo@gmail.com>
+Date:   Tue Feb 25 21:27:39 2020 -0800
+
+    makefile: correctly sed paths
+    
+    The default Makefile for tlp makes a mess with catenating `DESTDIR` to
+    everything, but then not actualy using the catenated (_ prefixed)
+    variables to sed it's `.in` files.
+    
+    This patch makes sure that it correctly sets the paths, taking `DESTDIR`
+    in account where it makes sense (e.g. /bin where we want $out/bin) but
+    not where it doesn't (/etc/tlp.conf should be just that).
+    
+    The reason DESTDIR is used at all, as opposed to the more appropriate
+    PREFIX, is covered in the nix formula, and is (also) due to the Makefile
+    being a bit "different."
+
+diff --git a/Makefile b/Makefile
+index b5af74e..95122df 100644
+--- a/Makefile
+++ b/Makefile
+@@ -47,17 +47,17 @@ _TPACPIBAT = $(DESTDIR)$(TPACPIBAT)
+ 
+ SED = sed \
+ 	-e "s|@TLPVER@|$(TLPVER)|g" \
+-	-e "s|@TLP_SBIN@|$(TLP_SBIN)|g" \
+-	-e "s|@TLP_TLIB@|$(TLP_TLIB)|g" \
+-	-e "s|@TLP_FLIB@|$(TLP_FLIB)|g" \
+-	-e "s|@TLP_ULIB@|$(TLP_ULIB)|g" \
+	-e "s|@TLP_SBIN@|$(_SBIN)|g" \
+	-e "s|@TLP_TLIB@|$(_TLIB)|g" \
+	-e "s|@TLP_FLIB@|$(_FLIB)|g" \
+	-e "s|@TLP_ULIB@|$(_ULIB)|g" \
+ 	-e "s|@TLP_CONFUSR@|$(TLP_CONFUSR)|g" \
+ 	-e "s|@TLP_CONFDIR@|$(TLP_CONFDIR)|g" \
+-	-e "s|@TLP_CONFDEF@|$(TLP_CONFDEF)|g" \
+	-e "s|@TLP_CONFDEF@|$(_CONFDEF)|g" \
+ 	-e "s|@TLP_CONF@|$(TLP_CONF)|g" \
+ 	-e "s|@TLP_RUN@|$(TLP_RUN)|g"   \
+ 	-e "s|@TLP_VAR@|$(TLP_VAR)|g"   \
+-	-e "s|@TPACPIBAT@|$(TPACPIBAT)|g"
+	-e "s|@TPACPIBAT@|$(_TPACPIBAT)|g"
+ 
+ INFILES = \
+ 	tlp \
--- a/pkgs/tools/misc/tlp/patches/tlp-sleep-service.patch
+++ b/pkgs/tools/misc/tlp/patches/tlp-sleep-service.patch
@ -0,0 +1,95 @@
+commit ca94cd56210067e2a55c1f413bd7713f7d338f9f
+Author: Bernardo Meurer <meurerbernardo@gmail.com>
+Date:   Wed Feb 26 10:46:23 2020 -0800
+
+    tlp-sleep.service: reintroduce
+    
+    This patch reintroduces tlp-sleep as a systemd unit as opposed to a
+    systemd system-sleep hook script. This is due to the recommendation by
+    systemd itself to not use the hook scripts. As per the manual:
+    
+    > Note that scripts or binaries dropped in /usr/lib/systemd/system-sleep/
+    > are intended for local use only and should be considered hacks. If
+    > applications want to react to system suspend/hibernation and resume,
+    > they should rather use the Inhibitor interface[1].
+
+diff --git a/Makefile b/Makefile
+index 95122df..0e9230a 100644
+--- a/Makefile
+++ b/Makefile
+@@ -70,6 +70,7 @@ INFILES = \
+ 	tlp.rules \
+ 	tlp-readconfs \
+ 	tlp-run-on \
+	tlp-sleep.service \
+ 	tlp.service \
+ 	tlp-stat \
+ 	tlp.upstart \
+@@ -99,7 +100,6 @@ SHFILES = \
+ 	tlp-rdw-udev.in \
+ 	tlp-rf.in \
+ 	tlp-run-on.in \
+-	tlp-sleep \
+ 	tlp-sleep.elogind \
+ 	tlp-stat.in \
+ 	tlp-usb-udev.in
+@@ -147,7 +147,7 @@ ifneq ($(TLP_NO_INIT),1)
+ endif
+ ifneq ($(TLP_WITH_SYSTEMD),0)
+ 	install -D -m 644 tlp.service $(_SYSD)/tlp.service
+-	install -D -m 755 tlp-sleep $(_SDSL)/tlp
+	install -D -m 644 tlp-sleep.service $(_SYSD)/tlp-sleep.service
+ endif
+ ifneq ($(TLP_WITH_ELOGIND),0)
+ 	install -D -m 755 tlp-sleep.elogind $(_ELOD)/49-tlp-sleep
+@@ -204,7 +204,7 @@ uninstall-tlp:
+ 	rm $(_ULIB)/rules.d/85-tlp.rules
+ 	rm -f $(_SYSV)/tlp
+ 	rm -f $(_SYSD)/tlp.service
+-	rm -f $(_SDSL)/tlp-sleep
+	rm -f $(_SYSD)/tlp-sleep.service
+ 	rm -f $(_ELOD)/49-tlp-sleep
+ 	rm -f $(_SHCPL)/tlp-stat
+ 	rm -f $(_SHCPL)/bluetooth
+diff --git a/tlp-sleep b/tlp-sleep
+deleted file mode 100644
+index 3de85ce..0000000
+--- a/tlp-sleep
+++ /dev/null
+@@ -1,11 +0,0 @@
+-#!/bin/sh
+-
+-# tlp - systemd suspend/resume hook
+-#
+-# Copyright (c) 2020 Thomas Koch <linrunner at gmx.net> and others.
+-# This software is licensed under the GPL v2 or later.
+-
+-case $1 in
+-    pre)  tlp suspend ;;
+-    post) tlp resume  ;;
+-esac
+diff --git a/tlp-sleep.service.in b/tlp-sleep.service.in
+new file mode 100644
+index 0000000..4ac17bd
+--- /dev/null
+++ b/tlp-sleep.service.in
+@@ -0,0 +1,19 @@
+# tlp - systemd suspend/resume service
+#
+# Copyright (c) 2020 Thomas Koch <linrunner at gmx.net> and others.
+# This software is licensed under the GPL v2 or later.
+
+[Unit]
+Description=TLP suspend/resume
+Before=sleep.target
+StopWhenUnneeded=yes
+Documentation=https://linrunner.de/tlp
+
+[Service]
+Type=oneshot
+RemainAfterExit=yes
+ExecStart=@TLP_SBIN@/tlp suspend
+ExecStop=@TLP_SBIN@/tlp resume
+
+[Install]
+WantedBy=sleep.target