| Message ID | 87fttvl9b1.fsf@yahoo.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#34067] Add input-wacom/inputattach | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/applying patch | success | Successfully applied |
Hi Tim, Thanks for the patches! Here’s a quick review: >>From 9c5fc83d9d5a162fb3d4662c3e66cd77918159da Mon Sep 17 00:00:00 2001 > From: Tim Gesthuizen <tim.gesthuizen@yahoo.de> > Date: Wed, 9 Jan 2019 18:40:42 +0100 > Subject: [PATCH 1/7] gnu: Add linux-libre-fill-version > > linux-libre-fill-version adds a patch version to the version if it is > missing. > > * gnu/packages/linux.scm (linux-libre-fill-version): New function Could the ‘version-major+minor’ procedure play a similar role for what you want? >>From 24f65c10bcfc8349778d024f039528997c9e7da9 Mon Sep 17 00:00:00 2001 > From: Tim Gesthuizen <tim.gesthuizen@yahoo.de> > Date: Wed, 9 Jan 2019 17:56:21 +0100 > Subject: [PATCH 2/7] gnu: Add linux-libre-module-path > > Add a function that evaluates to the module path where the modules for > linux-libre in a specific version are stored. > > * gnu/packages/linux.scm (linux-libre-module-path): New function s/path/file-name/ :-) >>From e771153f957e1bd41dbef32bf6f7e997f9a732f5 Mon Sep 17 00:00:00 2001 > From: Tim Gesthuizen <tim.gesthuizen@yahoo.de> > Date: Fri, 4 Jan 2019 12:22:34 +0100 > Subject: [PATCH 3/7] gnu: linux-libre: Copy source to the store > > The source code is needed by some kernel modules to compile. The item in the > store only symlinks the build directory in /tmp which is not reachable later > on and is a source of non determinism for the store item. > This patch deletes the symlinks and copies the source to a separate output. > > * gnu/packages/linux.scm (linux-libre): > [outputs]: Add output source > [arguments]: Add phase to copy source to the store item. I’d rather avoid that and this would fill up stores. What about simply writing: (inputs `(("linux-libre-source" ,(package-source linux-libre)) …)) in the ‘input-wacom’ package? The downside is that the package definition of ‘input-wacom’ would have to unpack it, etc., but it seems worth it. >>From 4cc4535566f0496e24fcf567c73494c18d4b8a08 Mon Sep 17 00:00:00 2001 > From: Tim Gesthuizen <tim.gesthuizen@yahoo.de> > Date: Sat, 5 Jan 2019 00:32:16 +0100 > Subject: [PATCH 4/7] gnu: Add input-wacom > > * gnu/packages/firmware.scm (input-wacom): New variable [...] > + (synopsis "Linux kernel driver for various wacom touchscreens") So this should go to linux.scm rather than firmware.scm. :-) Could you please make sure the code doesn’t contain binary blobs—e.g., in the form of large ‘char’ arrays? You can maybe just check whether Debian “main” contains this driver. Then there’s the question of which kernel version to target. I guess it’s reasonable to just choose whatever ‘linux-libre’ points to. Thoughts? > + (description "A set of kernel drivers that add support for various wacom > +touchscreens. In combination with xf86-input-wacom and libwacom it forms a set > +of modules to support wacom touchscreens with the X server.") Please write full sentences and address the two-space-after-period issue that ‘guix lint’ tells you about. :-) >>From cb02272b9759426427ba1accc60915b455dfb357 Mon Sep 17 00:00:00 2001 > From: Tim Gesthuizen <tim.gesthuizen@yahoo.de> > Date: Sat, 5 Jan 2019 20:55:14 +0100 > Subject: [PATCH 5/7] gnu: Add inputattach > > * gnu/packages/firmware.scm (inputattach): New variable [...] > + (replace 'install > + (lambda* (#:key outputs #:allow-other-keys) > + (let ((target-dir (string-append > + (assoc-ref outputs "out") > + "/bin/"))) > + (mkdir-p target-dir) > + (copy-file "inputattach/inputattach" > + (string-append target-dir > + "inputattach")))))))) Please return #t. >>From 8a1bb6706be11cd9c1e683e6d242accad0346d6b Mon Sep 17 00:00:00 2001 > From: Tim Gesthuizen <tim.gesthuizen@yahoo.de> > Date: Sat, 5 Jan 2019 23:28:18 +0100 > Subject: [PATCH 6/7] gnu: Add inputattach service > > Add a service that runs inputattach as a daemon to translate events from > serial ports. > > * gnu/local.mk: Add gnu/services/hardware.scm > * gnu/services/hardware.scm (<inputattach-configuration>): New record type > * gnu/services/hardware.scm (inputattach-service-type): New service type > * gnu/services/hardware.scm (inputattach-service): New function > > squash Leftover. :-) Also, no need to repeat the file name when it’s the same. > +++ b/gnu/services/hardware.scm Do you think desktop.scm would make sense? > +(define-record-type* <inputattach-configuration> > + inputattach-configuration > + make-inputattach-configuration > + inputattach-configuration? > + (devicetype inputattach-configuration-devicetype > + (default "wacom")) ‘device-type’ > +(define inputattach-service-type > + (service-type > + (name 'inputattach) > + (extensions > + (list (service-extension shepherd-root-service-type > + inputattach-shepherd-service))) > + (default-value (inputattach-configuration)))) Please add a ‘description’ field so people can find out about it with ‘guix system search’. > +(define* (inputattach-service #:key (type "wacom") (device "/dev/ttyS0") (log-file #f)) This can be removed (we no longer provide such procedures.) >>From a9cd86ad2244fff023f0c1bf4038748872aeab13 Mon Sep 17 00:00:00 2001 > From: Tim Gesthuizen <tim.gesthuizen@yahoo.de> > Date: Sun, 6 Jan 2019 11:56:57 +0100 > Subject: [PATCH 7/7] doc: Add documentation for inputattach-service > > * doc/guix.texi (Miscellaneous Services): Add inputattach Service > subsubheading Can be squashed with the previous patch. > +The @code{(gnu services hardware)} module provides the following service. > + > +@deffn {Scheme Procedure} inputattach-service [#:type "wacom"] @ > + [#:device "/dev/ttyS0"] [#:log-file #f] > +Return a service that runs inputattach on @var{device} to decode events from > +@var{type}. > +@end deffn Here could you document ‘inputattach-service-type’ and ‘inputattach-configuration’? Thanks! Ludo’.
From a9cd86ad2244fff023f0c1bf4038748872aeab13 Mon Sep 17 00:00:00 2001 From: Tim Gesthuizen <tim.gesthuizen@yahoo.de> Date: Sun, 6 Jan 2019 11:56:57 +0100 Subject: [PATCH 7/7] doc: Add documentation for inputattach-service * doc/guix.texi (Miscellaneous Services): Add inputattach Service subsubheading --- doc/guix.texi | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/doc/guix.texi b/doc/guix.texi index c0cc8d416..e0921d34d 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -22020,6 +22020,17 @@ that enables sharing the clipboard with a vm and setting the guest display resolution when the graphical console window resizes. @end deffn +@cindex inputattach +@subsubheading inputattach Service + +The @code{(gnu services hardware)} module provides the following service. + +@deffn {Scheme Procedure} inputattach-service [#:type "wacom"] @ + [#:device "/dev/ttyS0"] [#:log-file #f] +Return a service that runs inputattach on @var{device} to decode events from +@var{type}. +@end deffn + @subsubsection Dictionary Services @cindex dictionary The @code{(gnu services dict)} module provides the following service: -- 2.20.1