From 471944b924882e1cd771814a9724f82967bca96a Mon Sep 17 00:00:00 2001 From: Bernardo Meurer Date: Tue, 25 Feb 2020 17:39:35 -0800 Subject: [PATCH 1/3] tlp: 1.2.2 -> 1.3.1 --- pkgs/tools/misc/tlp/default.nix | 159 +++++++++++------- .../misc/tlp/patches/fix-makefile-sed.patch | 46 +++++ .../misc/tlp/patches/tlp-sleep-service.patch | 95 +++++++++++ 3 files changed, 243 insertions(+), 57 deletions(-) create mode 100644 pkgs/tools/misc/tlp/patches/fix-makefile-sed.patch create mode 100644 pkgs/tools/misc/tlp/patches/tlp-sleep-service.patch diff --git a/pkgs/tools/misc/tlp/default.nix b/pkgs/tools/misc/tlp/default.nix index e5932d9e9462..c79d72f9ec04 100644 --- a/pkgs/tools/misc/tlp/default.nix +++ b/pkgs/tools/misc/tlp/default.nix @@ -1,79 +1,124 @@ -{ 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 ]; license = licenses.gpl2Plus; diff --git a/pkgs/tools/misc/tlp/patches/fix-makefile-sed.patch b/pkgs/tools/misc/tlp/patches/fix-makefile-sed.patch new file mode 100644 index 000000000000..942c9a579f6f --- /dev/null +++ b/pkgs/tools/misc/tlp/patches/fix-makefile-sed.patch @@ -0,0 +1,46 @@ +commit c44347b3b813e209fff537b4d46d23430727a5e2 +Author: Bernardo Meurer +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 \ diff --git a/pkgs/tools/misc/tlp/patches/tlp-sleep-service.patch b/pkgs/tools/misc/tlp/patches/tlp-sleep-service.patch new file mode 100644 index 000000000000..b37c7280e382 --- /dev/null +++ b/pkgs/tools/misc/tlp/patches/tlp-sleep-service.patch @@ -0,0 +1,95 @@ +commit ca94cd56210067e2a55c1f413bd7713f7d338f9f +Author: Bernardo Meurer +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 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 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 From 07cc033524aeb2ea9b00009a33189a69b921d8ca Mon Sep 17 00:00:00 2001 From: Bernardo Meurer Date: Tue, 25 Feb 2020 20:03:16 -0800 Subject: [PATCH 2/3] tlp: add lovesegfault as a maintainer --- pkgs/tools/misc/tlp/default.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/tools/misc/tlp/default.nix b/pkgs/tools/misc/tlp/default.nix index c79d72f9ec04..5a467dd626da 100644 --- a/pkgs/tools/misc/tlp/default.nix +++ b/pkgs/tools/misc/tlp/default.nix @@ -120,7 +120,7 @@ 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; }; } From ee7becd91863609bb39055e3c8a9fa7451d0ac03 Mon Sep 17 00:00:00 2001 From: Bernardo Meurer Date: Tue, 25 Feb 2020 20:19:27 -0800 Subject: [PATCH 3/3] nixos/tlp: revamp --- nixos/modules/services/hardware/tlp.nix | 146 ++++++++++-------------- 1 file changed, 60 insertions(+), 86 deletions(-) diff --git a/nixos/modules/services/hardware/tlp.nix b/nixos/modules/services/hardware/tlp.nix index 955a60677997..3962d7b15989 100644 --- 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"; + }; + }; }; - }