[PATCH] Add thinkfan.

[PATCH] Add thinkfan.

[Nicolas Goaziou]

[-- Attachment #1: Type: text/plain, Size: 677 bytes --]

Hello,

I'd like to submit the following patch for inclusion. There are a few
caveats

1. It has to be run as root.

2. There is no default configuration since it is hardware-dependant, and
   it will not run without one.

3. It requires to load a kernel module with a specific option, namely:
     modprobe thinkpad_acpi fan_control=1

I guess we cannot handle any of these requirements at the package
definition level, anyway.

As icing on the cake, one could define a service to load it at boot (the
tarball contains scripts for at least 4 init systems), but my service-fu
is feeble.


Regards,

-- 
Nicolas Goaziou                                                0x80A93738

[-- Attachment #2: Add thinkfan --]
[-- Type: text/x-diff, Size: 2584 bytes --]

From e82ba2f266a144e05f16cf41602b9c2bd203b3ec Mon Sep 17 00:00:00 2001
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Date: Sat, 30 Apr 2016 10:31:13 +0200
Subject: [PATCH] gnu: Add thinkfan.

* gnu/packages/linux.scm (thinkfan): New variable.
---
 gnu/packages/linux.scm | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index a26e641..4ed83d1 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -11,6 +11,7 @@
 ;;; Copyright © 2016 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2016 Raymond Nicholson <rain1@openmailbox.org>
 ;;; Copyright © 2016 Mathieu Lirzin <mthl@gnu.org>
+;;; Copyright © 2016 Nicolas Goaziou <mail@nicolasgoaziou.fr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -63,6 +64,7 @@
   #:use-module (gnu packages readline)
   #:use-module (gnu packages calendar)
   #:use-module (gnu packages tls)
+  #:use-module (gnu packages freedesktop)
   #:use-module (guix packages)
   #:use-module (guix download)
   #:use-module (guix utils)
@@ -2571,3 +2573,32 @@ where they are less likely to cause damage to the spinning disc.  Requires a
 drive that supports the ATA/ATAPI-7 IDLE IMMEDIATE command with unload
 feature, and a laptop with an accelerometer.  It has no effect on SSDs.")
     (license license:gpl2)))
+
+(define-public thinkfan
+  (package
+    (name "thinkfan")
+    (version "0.9.2")
+    (source (origin
+              (method url-fetch/tarbomb)
+              (file-name (string-append name "-" version))
+              (uri (string-append "mirror://sourceforge/thinkfan/"
+                                  version "/thinkfan-" version ".tar.gz"))
+              (sha256
+               (base32
+                "0ydgabk2758f6j64g1r9vdsd221nqsv5rwnphm81s7i2vgra1nlh"))))
+    (build-system cmake-build-system)
+    (arguments
+     `(#:tests? #f                      ;no test target
+       #:configure-flags
+       `("-DUSE_ATASMART:BOOL=ON")))
+    (inputs
+     `(("libatasmart" ,libatasmart)))
+    (home-page "http://thinkfan.sourceforge.net/")
+    (synopsis "Simple fan control program")
+    (description
+     "Thinkfan is a simple fan control program.  It reads temperatures,
+checks them against configured limits and switches to appropriate (also
+pre-configured) fan level.  It requires a working @code{thinkpad_acpi} or any
+other @code{hwmon} driver that enables temperature reading and fan control
+from userspace.")
+    (license license:gpl3+)))
-- 
2.7.4

Re: [PATCH] Add thinkfan.

[Leo Famulari]

On Sun, May 01, 2016 at 01:05:59AM +0200, Nicolas Goaziou wrote:
> * gnu/packages/linux.scm (thinkfan): New variable.

Thanks!

> +    (arguments
> +     `(#:tests? #f                      ;no test target
> +       #:configure-flags
> +       `("-DUSE_ATASMART:BOOL=ON")))

Can you add a brief comment explaining this flag?

Also, can you add a phase that installs the "rcscripts"? They will be
useful for Guix users on foreign distros.

I'm not sure where to install them. Since they will probably have to be
copied or symlinked out of ~/.guix-profile anyways, how about installing
them into 'share/misc'?

I'm open to better ideas!

> +    (synopsis "Simple fan control program")

Is it specific to ThinkPads? If so, can you change the synopsis to
"Simple fan control program for ThinkPads" and mention the limitation in
the description?

Re: [PATCH] Add thinkfan.

[Kei Yamashita]

On Sun, 1 May 2016 17:14:18 -0400
Leo Famulari <leo@famulari.name> wrote:

> On Sun, May 01, 2016 at 01:05:59AM +0200, Nicolas Goaziou wrote:
> > * gnu/packages/linux.scm (thinkfan): New variable.  
> 
> Thanks!
> 
> > +    (arguments
> > +     `(#:tests? #f                      ;no test target
> > +       #:configure-flags
> > +       `("-DUSE_ATASMART:BOOL=ON")))  
> 
> Can you add a brief comment explaining this flag?
> 
> Also, can you add a phase that installs the "rcscripts"? They will be
> useful for Guix users on foreign distros.
> 
> I'm not sure where to install them. Since they will probably have to
> be copied or symlinked out of ~/.guix-profile anyways, how about
> installing them into 'share/misc'?
> 
> I'm open to better ideas!
> 
> > +    (synopsis "Simple fan control program")  
> 
> Is it specific to ThinkPads? If so, can you change the synopsis to
> "Simple fan control program for ThinkPads" and mention the limitation
> in the description?
> 

It would be helpful if you told the user where and how to install each
script for each of the major distribution families, similar to how Guix
does this for SystemD and Upstart systems[1].

[1]
https://gnu.org/software/guix/manual/html_node/Binary-Installation.html

Re: [PATCH] Add thinkfan.

[Nicolas Goaziou]

[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]

Hello,

Leo Famulari <leo@famulari.name> writes:

> Can you add a brief comment explaining this flag?

Done.

> Also, can you add a phase that installs the "rcscripts"? They will be
> useful for Guix users on foreign distros.

Done.

> I'm not sure where to install them. Since they will probably have to be
> copied or symlinked out of ~/.guix-profile anyways, how about installing
> them into 'share/misc'?
>
> I'm open to better ideas!

What about share/thinkfan instead? I changed the patch accordingly. Feel
free to use share/misc instead if you think this is better.

>> +    (synopsis "Simple fan control program")
>
> Is it specific to ThinkPads?

No, it isn't. Quoting README:

  Thinkfan is a simple, lightweight fan control program. Originally designed
  specifically for IBM/Lenovo Thinkpads, it now supports any kind of system via
  the sysfs hwmon interface (/sys/class/hwmon).

Here is the updated patch. Thank you for the review.


Regards,

-- 
Nicolas Goaziou                                                0x80A93738

[-- Attachment #2: add thinkfan (take 2) --]
[-- Type: text/x-diff, Size: 3333 bytes --]

From b804f745864eb06613a51e6055b8b0b4c4865401 Mon Sep 17 00:00:00 2001
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Date: Sat, 30 Apr 2016 10:31:13 +0200
Subject: [PATCH] gnu: Add thinkfan.

* gnu/packages/linux.scm (thinkfan): New variable.
---
 gnu/packages/linux.scm | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index aa778be..97958eb 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -11,6 +11,7 @@
 ;;; Copyright © 2016 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2016 Raymond Nicholson <rain1@openmailbox.org>
 ;;; Copyright © 2016 Mathieu Lirzin <mthl@gnu.org>
+;;; Copyright © 2016 Nicolas Goaziou <mail@nicolasgoaziou.fr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -63,6 +64,7 @@
   #:use-module (gnu packages readline)
   #:use-module (gnu packages calendar)
   #:use-module (gnu packages tls)
+  #:use-module (gnu packages freedesktop)
   #:use-module (guix packages)
   #:use-module (guix download)
   #:use-module (guix utils)
@@ -2591,3 +2593,47 @@ where they are less likely to cause damage to the spinning disc.  Requires a
 drive that supports the ATA/ATAPI-7 IDLE IMMEDIATE command with unload
 feature, and a laptop with an accelerometer.  It has no effect on SSDs.")
     (license license:gpl2)))
+
+(define-public thinkfan
+  (package
+    (name "thinkfan")
+    (version "0.9.2")
+    (source (origin
+              (method url-fetch/tarbomb)
+              (file-name (string-append name "-" version))
+              (uri (string-append "mirror://sourceforge/thinkfan/"
+                                  version "/thinkfan-" version ".tar.gz"))
+              (sha256
+               (base32
+                "0ydgabk2758f6j64g1r9vdsd221nqsv5rwnphm81s7i2vgra1nlh"))))
+    (build-system cmake-build-system)
+    (arguments
+     `(#:modules ((guix build cmake-build-system)
+                  (guix build utils)
+                  (srfi srfi-1)
+                  (srfi srfi-26))
+       #:tests? #f                      ;no test target
+       #:configure-flags
+       ;; Allow reading temperatures from hard disks via S.M.A.R.T.
+       `("-DUSE_ATASMART:BOOL=ON")
+       #:phases
+       (modify-phases %standard-phases
+         ;; Install scripts for various foreign init systems.
+         (add-after 'install 'install-rc-scripts
+           (lambda* (#:key outputs #:allow-other-keys)
+             (for-each (cute install-file <>
+                             (string-append (assoc-ref outputs "out")
+                                            "/share/thinkfan"))
+                       (find-files "../source/rcscripts" ".*"))
+             #t)))))
+    (inputs
+     `(("libatasmart" ,libatasmart)))
+    (home-page "http://thinkfan.sourceforge.net/")
+    (synopsis "Simple fan control program")
+    (description
+     "Thinkfan is a simple fan control program.  It reads temperatures,
+checks them against configured limits and switches to appropriate (also
+pre-configured) fan level.  It requires a working @code{thinkpad_acpi} or any
+other @code{hwmon} driver that enables temperature reading and fan control
+from userspace.")
+    (license license:gpl3+)))
-- 
2.7.4

Re: [PATCH] Add thinkfan.

[Leo Famulari]

On Wed, May 04, 2016 at 12:12:48AM +0200, Nicolas Goaziou wrote:
> Leo Famulari <leo@famulari.name> writes:
> > Also, can you add a phase that installs the "rcscripts"? They will be
> > useful for Guix users on foreign distros.
> 
> Done.
> 
> > I'm not sure where to install them. Since they will probably have to be
> > copied or symlinked out of ~/.guix-profile anyways, how about installing
> > them into 'share/misc'?
> >
> > I'm open to better ideas!
> 
> What about share/thinkfan instead? I changed the patch accordingly. Feel
> free to use share/misc instead if you think this is better.

Yours is one of those "better ideas" I was talking about ;)

> * gnu/packages/linux.scm (thinkfan): New variable.

Looks good! But I noticed they *just* released a new version. Can you
take a look and submit a patch with that version if it works for you?

Re: [PATCH] Add thinkfan.

[Nicolas Goaziou]

[-- Attachment #1: Type: text/plain, Size: 298 bytes --]

Hello,

Leo Famulari <leo@famulari.name> writes:

> Looks good! But I noticed they *just* released a new version. Can you
> take a look and submit a patch with that version if it works for you?

Here it is.


Regards,

-- 
Nicolas Goaziou                                                0x80A93738
 

[-- Attachment #2: add thinkfan (take 3) --]
[-- Type: text/x-diff, Size: 3650 bytes --]

From 9c82df26841b281fc9eeb751db76871e31c94339 Mon Sep 17 00:00:00 2001
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Date: Sat, 30 Apr 2016 10:31:13 +0200
Subject: [PATCH] gnu: Add thinkfan.

* gnu/packages/linux.scm (thinkfan): New variable.
---
 gnu/packages/linux.scm | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index e80c900..fcea499 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -11,6 +11,7 @@
 ;;; Copyright © 2016 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2016 Raymond Nicholson <rain1@openmailbox.org>
 ;;; Copyright © 2016 Mathieu Lirzin <mthl@gnu.org>
+;;; Copyright © 2016 Nicolas Goaziou <mail@nicolasgoaziou.fr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -63,6 +64,7 @@
   #:use-module (gnu packages readline)
   #:use-module (gnu packages calendar)
   #:use-module (gnu packages tls)
+  #:use-module (gnu packages freedesktop)
   #:use-module (guix packages)
   #:use-module (guix download)
   #:use-module (guix utils)
@@ -2591,3 +2593,53 @@ where they are less likely to cause damage to the spinning disc.  Requires a
 drive that supports the ATA/ATAPI-7 IDLE IMMEDIATE command with unload
 feature, and a laptop with an accelerometer.  It has no effect on SSDs.")
     (license license:gpl2)))
+
+(define-public thinkfan
+  (package
+    (name "thinkfan")
+    (version "0.9.3")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "mirror://sourceforge/thinkfan/"
+                                  version "/thinkfan-" version ".tar.gz"))
+              (sha256
+               (base32
+                "0nz4c48f0i0dljpk5y33c188dnnwg8gz82s4grfl8l64jr4n675n"))
+              (modules '((guix build utils)))
+              ;; Fix erroneous man page location in Makefile leading to
+              ;; a compilation failure.
+              (snippet
+               '(substitute* "CMakeLists.txt"
+                  (("thinkfan\\.1") "src/thinkfan.1")))))
+    (build-system cmake-build-system)
+    (arguments
+     `(#:modules ((guix build cmake-build-system)
+                  (guix build utils)
+                  (srfi srfi-26))
+       #:tests? #f                      ;no test target
+       #:configure-flags
+       ;; Enable reading temperatures from hard disks via S.M.A.R.T.
+       `("-DUSE_ATASMART:BOOL=ON")
+       #:phases
+       (modify-phases %standard-phases
+         ;; Install scripts for various foreign init systems.
+         (add-after 'install 'install-rc-scripts
+           (lambda* (#:key outputs #:allow-other-keys)
+             (for-each (cute install-file <>
+                             (string-append (assoc-ref outputs "out")
+                                            "/share/thinkfan"))
+                       (find-files (string-append "../thinkfan-" ,version
+                                                  "/rcscripts")
+                                   ".*"))
+             #t)))))
+    (inputs
+     `(("libatasmart" ,libatasmart)))
+    (home-page "http://thinkfan.sourceforge.net/")
+    (synopsis "Simple fan control program")
+    (description
+     "Thinkfan is a simple fan control program.  It reads temperatures,
+checks them against configured limits and switches to appropriate (also
+pre-configured) fan level.  It requires a working @code{thinkpad_acpi} or any
+other @code{hwmon} driver that enables temperature reading and fan control
+from userspace.")
+    (license license:gpl3+)))
-- 
2.7.4

Re: [PATCH] Add thinkfan.

[Leo Famulari]

On Sat, May 07, 2016 at 01:42:22AM +0200, Nicolas Goaziou wrote:
> Hello,
> 
> Leo Famulari <leo@famulari.name> writes:
> 
> > Looks good! But I noticed they *just* released a new version. Can you
> > take a look and submit a patch with that version if it works for you?
> 
> Here it is.

> * gnu/packages/linux.scm (thinkfan): New variable.

Thanks! Applied as d17ae8c0c30.

Re: [PATCH] Add thinkfan.

[Nicolas Goaziou]

Hello,

Leo Famulari <leo@famulari.name> writes:

> On Sat, May 07, 2016 at 01:42:22AM +0200, Nicolas Goaziou wrote:
>> Hello,
>> 
>> Leo Famulari <leo@famulari.name> writes:
>> 
>> > Looks good! But I noticed they *just* released a new version. Can you
>> > take a look and submit a patch with that version if it works for you?
>> 
>> Here it is.
>
>> * gnu/packages/linux.scm (thinkfan): New variable.
>
> Thanks! Applied as d17ae8c0c30.

Thank you.

I realize there are hard-coded paths in init scripts. Would it make
sense to patch them so as to refer to `thinkfan' in store instead of
"/usr/sbin/thinkfan"?

Regards,

-- 
Nicolas Goaziou                                                0x80A93738

Re: [PATCH] Add thinkfan.

[Ludovic Courtès]

Hi!

Nicolas Goaziou <mail@nicolasgoaziou.fr> skribis:

> Leo Famulari <leo@famulari.name> writes:
>
>> On Sat, May 07, 2016 at 01:42:22AM +0200, Nicolas Goaziou wrote:
>>> Hello,
>>> 
>>> Leo Famulari <leo@famulari.name> writes:
>>> 
>>> > Looks good! But I noticed they *just* released a new version. Can you
>>> > take a look and submit a patch with that version if it works for you?
>>> 
>>> Here it is.
>>
>>> * gnu/packages/linux.scm (thinkfan): New variable.
>>
>> Thanks! Applied as d17ae8c0c30.
>
> Thank you.
>
> I realize there are hard-coded paths in init scripts. Would it make
> sense to patch them so as to refer to `thinkfan' in store instead of
> "/usr/sbin/thinkfan"?

Yes, I think so.

Ludo’.

Re: [PATCH] Add thinkfan.

[Nicolas Goaziou]

[-- Attachment #1: Type: text/plain, Size: 423 bytes --]

Hello,

ludo@gnu.org (Ludovic Courtès) writes:

> Nicolas Goaziou <mail@nicolasgoaziou.fr> skribis:

>> I realize there are hard-coded paths in init scripts. Would it make
>> sense to patch them so as to refer to `thinkfan' in store instead of
>> "/usr/sbin/thinkfan"?
>
> Yes, I think so.

Here's the patch, then.

Regards,

-- 
Nicolas Goaziou                                                0x80A93738

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: thinkfan: fix hard-coded paths --]
[-- Type: text/x-diff, Size: 1978 bytes --]

From f6e8845b46cd469a2e9a8540879436671944c87e Mon Sep 17 00:00:00 2001
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
Date: Tue, 10 May 2016 21:20:49 +0200
Subject: [PATCH] gnu: thinkfan: Fix daemon path in init scripts.

* gnu/packages/linux.scm (thinkfan): Fix daemon path in init scripts.
---
 gnu/packages/linux.scm | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index fcea499..5500681 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -2622,15 +2622,20 @@ feature, and a laptop with an accelerometer.  It has no effect on SSDs.")
        `("-DUSE_ATASMART:BOOL=ON")
        #:phases
        (modify-phases %standard-phases
-         ;; Install scripts for various foreign init systems.
+         ;; Install scripts for various foreign init systems. Also fix
+         ;; hard-coded path for daemon.
          (add-after 'install 'install-rc-scripts
            (lambda* (#:key outputs #:allow-other-keys)
-             (for-each (cute install-file <>
-                             (string-append (assoc-ref outputs "out")
-                                            "/share/thinkfan"))
-                       (find-files (string-append "../thinkfan-" ,version
-                                                  "/rcscripts")
-                                   ".*"))
+             (let ((out (assoc-ref outputs "out"))
+                   (files (find-files
+                           (string-append "../thinkfan-" ,version "/rcscripts")
+                           ".*")))
+               (substitute* files
+                 (("/usr/sbin/(\\$NAME|thinkfan)" _ name)
+                  (string-append out "/sbin/" name)))
+               (for-each (cute install-file <>
+                               (string-append out "/share/thinkfan"))
+                         files))
              #t)))))
     (inputs
      `(("libatasmart" ,libatasmart)))
-- 
2.7.4

Re: [PATCH] Add thinkfan.

[Ludovic Courtès]

Nicolas Goaziou <mail@nicolasgoaziou.fr> skribis:

> From f6e8845b46cd469a2e9a8540879436671944c87e Mon Sep 17 00:00:00 2001
> From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
> Date: Tue, 10 May 2016 21:20:49 +0200
> Subject: [PATCH] gnu: thinkfan: Fix daemon path in init scripts.
>
> * gnu/packages/linux.scm (thinkfan): Fix daemon path in init scripts.

Applied, thanks!

Ludo’.