[PATCH 0/2] Add apcupsd

  • Open
  • quality assurance status badge
Details
2 participants
  • Maxim Cournoyer
  • Tomas Volf
Owner
unassigned
Submitted by
Tomas Volf
Severity
normal
T
T
Tomas Volf wrote on 13 Jan 00:03 +0100
(address . guix-patches@gnu.org)(name . Tomas Volf)(address . ~@wolfsden.cz)
cover.1736722765.git.~@wolfsden.cz
For machines running with an uninterruptible power supply (UPS), it is
necessary to be able to react to events from it. That allows to, for example,
safely shutdown the system when battery is getting low or get a notification
that battery needs to be replaced. This series adds package and service type
for apcupsd, daemon to manage UPS devices made by American Power Conversion
Corporation (APC).

Tomas Volf (2):
gnu: Add apcupsd.
services: Add power.

doc/guix.texi | 374 +++++++++++++++++++++-
gnu/local.mk | 2 +
gnu/packages/power.scm | 125 ++++++++
gnu/services/power.scm | 690 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 1188 insertions(+), 3 deletions(-)
create mode 100644 gnu/packages/power.scm
create mode 100644 gnu/services/power.scm

--
2.47.1
T
T
Tomas Volf wrote on 13 Jan 00:05 +0100
[PATCH 1/2] gnu: Add apcupsd.
(address . 75528@debbugs.gnu.org)(name . Tomas Volf)(address . ~@wolfsden.cz)
51cba7679af5c4aa9cf0e6d70453e369ff0909d6.1736722765.git.~@wolfsden.cz
* gnu/packages/power.scm (apcupsd): New variable.

Change-Id: I74f59cd1fa2a13954117ff1683a10a84576cc839
---
gnu/local.mk | 1 +
gnu/packages/power.scm | 125 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 126 insertions(+)
create mode 100644 gnu/packages/power.scm

Toggle diff (145 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 855f2acccc..6ca7bf68ac 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -557,6 +557,7 @@ GNU_SYSTEM_MODULES = \
%D%/packages/polkit.scm \
%D%/packages/popt.scm \
%D%/packages/potassco.scm \
+ %D%/packages/power.scm \
%D%/packages/printers.scm \
%D%/packages/profiling.scm \
%D%/packages/prolog.scm \
diff --git a/gnu/packages/power.scm b/gnu/packages/power.scm
new file mode 100644
index 0000000000..98dc98c6e4
--- /dev/null
+++ b/gnu/packages/power.scm
@@ -0,0 +1,125 @@
+;;; Copyright (C) 2025 Tomas Volf <~@wolfsden.cz>
+;;; Copyright (C) 2023 Raven Hallsby <karl@hallsby.com>
+
+;;;; Commentary:
+
+;;; Power-related packages.
+
+;;;; Code:
+
+(define-module (gnu packages power)
+ #:use-module (gnu)
+ #:use-module (gnu packages libusb)
+ #:use-module (gnu packages linux)
+ #:use-module (gnu packages pkg-config)
+ #:use-module (gnu packages python-xyz)
+ #:use-module (guix build-system gnu)
+ #:use-module (guix download)
+ #:use-module (guix gexp)
+ #:use-module ((guix licenses) #:prefix license:)
+ #:use-module (guix packages))
+
+(define-public apcupsd
+ (package
+ (name "apcupsd")
+ (version "3.14.14")
+ (source (origin
+ (method url-fetch)
+ (uri
+ (string-append
+ "mirror://sourceforge/" name "/" name " - Stable/" version
+ "/" name "-" version ".tar.gz"))
+ (sha256
+ (base32
+ "0rwqiyzlg9p0szf3x6q1ppvrw6f6dbpn2rc5z623fk3bkdalhxyv"))))
+ (native-inputs
+ (list pkg-config python-docutils))
+ (inputs
+ (list libusb libusb-compat))
+ (outputs '("out" "doc"))
+ (build-system gnu-build-system)
+ (arguments
+ (list
+ #:configure-flags
+ #~(list
+ ;; The configure script ignores --prefix for most of the paths.
+ (string-append "--exec-prefix=" #$output)
+ (string-append "--mandir=" #$output "/share/man")
+ (string-append "--sbindir=" #$output "/sbin")
+ (string-append "--sysconfdir=" #$output "/etc/apcupsd")
+ (string-append "--with-halpolicydir=" #$output "/share/halpolicy")
+
+ ;; Put us into the version string.
+ "--with-distname=GNU/Guix"
+ "--disable-install-distdir"
+
+ ;; State directories.
+ "--localstatedir=/var"
+ "--with-log-dir=/var/log"
+ "--with-pid-dir=/var/run"
+ "--with-lock-dir=/var/run/apcupsd/lock"
+ "--with-nologin=/var/run/apcupsd"
+ "--with-pwrfail-dir=/var/run/apcupsd"
+
+ ;; Configure requires these, but we do not use the genenerated
+ ;; apcupsd.conf, so in order to reduce dependencies of the package,
+ ;; provide fake values.
+ (string-append "ac_cv_path_SHUTDOWN=/nope")
+ (string-append "ac_cv_path_APCUPSD_MAIL=/nope")
+ ;; While `wall' is not expanded anywhere, it still is searched for.
+ (string-append "ac_cv_path_WALL=/nope")
+
+ ;; Enable additional drivers.
+ "--enable-test"
+ "--enable-usb"
+ "--enable-modbus-usb")
+ #:tests? #f ; There are no tests.
+ #:modules (cons '(ice-9 ftw) %default-gnu-modules)
+ #:phases
+ #~(modify-phases %standard-phases
+ ;; These are not installed, so trick Make into thinking they were
+ ;; already generated. Allows us not to depend on mandoc, util-linux.
+ (add-before 'configure 'touch-txt-docs
+ (lambda _
+ (for-each (lambda (f)
+ (call-with-output-file f close-port))
+ '("doc/apcupsd.man.txt"
+ "doc/apcaccess.man.txt"
+ "doc/apctest.man.txt"
+ "doc/apccontrol.man.txt"
+ "doc/apcupsd.conf.man.txt"))))
+ (add-after 'build 'build-manual
+ (lambda _
+ (invoke "make" "-C" "doc/manual" "manual.html")))
+ (add-after 'install-license-files 'move-doc
+ (lambda _
+ (let ((target (string-append #$output:doc
+ "/share/doc/"
+ (strip-store-file-name #$output))))
+ (mkdir-p target)
+ (for-each (lambda (f)
+ (copy-file (string-append "doc/manual/" f)
+ (string-append target "/" f)))
+ (scandir "doc/manual"
+ (lambda (f)
+ (or (string-suffix? ".png" f)
+ (string-suffix? ".html" f))))))))
+ ;; If sending mails is required, use proper mail program.
+ (add-after 'install 'remove-smtp
+ (lambda _
+ (delete-file (string-append #$output "/sbin/smtp"))))
+ ;; The configuration files and scripts are not really suitable for
+ ;; Guix, and our service provides its own version anyway. So nuke
+ ;; these to make sure `apcupsd' and `apctest' executed without any
+ ;; arguments fail. `apctest' actually segfaults, but only after
+ ;; printing an error ¯\_(?)_/¯.
+ (add-after 'install 'remove-etc-apcupsd
+ (lambda _
+ (delete-file-recursively (string-append #$output "/etc/apcupsd")))))))
+ (home-page "https://www.apcupsd.org")
+ (synopsis "A daemon for controlling APC UPSes")
+ (description "Apcupsd can be used for power mangement and controlling most
+of APC’s UPS models on Unix and Windows machines. Apcupsd works with most of
+APC’s Smart-UPS models as well as most simple signalling models such a Back-UPS,
+and BackUPS-Office.")
+ (license license:gpl2)))
--
2.47.1
T
T
Tomas Volf wrote on 13 Jan 00:05 +0100
[PATCH 2/2] services: Add power.
(address . 75528@debbugs.gnu.org)(name . Tomas Volf)(address . ~@wolfsden.cz)
05ba17f2babc772a26072dca72c2e6e6f852e0ad.1736722765.git.~@wolfsden.cz
* gnu/services/power.scm: New file.
* gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
* doc/guix.texi (Power Management Services): Document service and data types.

Change-Id: If205d19bea1d20a99309626e28521a2d6fe6702f
---
doc/guix.texi | 374 +++++++++++++++++++++-
gnu/local.mk | 1 +
gnu/services/power.scm | 690 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 1062 insertions(+), 3 deletions(-)
create mode 100644 gnu/services/power.scm

Toggle diff (543 lines)
diff --git a/doc/guix.texi b/doc/guix.texi
index d6e17c74cd..6cb11200cd 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -123,7 +123,7 @@
Copyright @copyright{} 2023 Thomas Ieong@*
Copyright @copyright{} 2023 Saku Laesvuori@*
Copyright @copyright{} 2023 Graham James Addis@*
-Copyright @copyright{} 2023, 2024 Tomas Volf@*
+Copyright @copyright{} 2023-2025 Tomas Volf@*
Copyright @copyright{} 2024 Herman Rimm@*
Copyright @copyright{} 2024 Matthew Trzcinski@*
Copyright @copyright{} 2024 Richard Sent@*
@@ -420,7 +420,7 @@ Top
* Network File System:: NFS related services.
* Samba Services:: Samba services.
* Continuous Integration:: Cuirass and Laminar services.
-* Power Management Services:: Extending battery life.
+* Power Management Services:: Extending battery life, etc.
* Audio Services:: The MPD.
* Virtualization Services:: Virtualization services.
* Version Control Services:: Providing remote access to Git repositories.
@@ -19255,7 +19255,7 @@ Services
* Network File System:: NFS related services.
* Samba Services:: Samba services.
* Continuous Integration:: Cuirass and Laminar services.
-* Power Management Services:: Extending battery life.
+* Power Management Services:: Extending battery life, etc.
* Audio Services:: The MPD.
* Virtualization Services:: Virtualization services.
* Version Control Services:: Providing remote access to Git repositories.
@@ -36263,6 +36263,374 @@ Power Management Services
@end table
@end deftp
+The @code{(gnu services power)} module provides a service definition for
+@uref{http://www.apcupsd.org/, apcupsd}, a utility to interact with APC
+UPSes. Apcupsd also works with some OEM-branded products manufactured
+by APC.
+
+@defvar apcupsd-service-type
+The service type for apcupsd. For USB UPSes no configuration is
+necessary, however tweaking some fields to better suit your needs might
+be desirable. The defaults are taken from the upstream configuration
+and they are not very conservative (@code{battery-level} of 5% is too
+low in my opinion).
+
+The default event handlers do send emails, read more in
+@ref{apcupsd-event-handlers}.
+
+@lisp
+(service apcupsd-service-type)
+@end lisp
+@end defvar
+
+@deftp {Data Type} apcupsd-configuration
+
+Available @code{apcupsd-configuration} fields are:
+
+@table @asis
+@item @code{package} (default: @code{apcupsd}) (type: package)
+Package to use.
+
+@item @code{shepherd-base-name} (default: @code{apcupsd}) (type: symbol)
+Base name of the shepherd service. You can add the service multiple
+times with different prefix to manage multiple UPSes.
+
+@item @code{auto-start?} (default: @code{#t}) (type: boolean)
+Should the shepherd service auto-start?
+
+@item @code{pid-file} (default: @code{"/var/run/apcupsd.pid"}) (type: string)
+Path to the pid file.
+
+@item @code{debug-level} (default: @code{0}) (type: integer)
+Logging verbosity. Bigger number means more logs. In the source code I
+saw up to @code{300}, so for all logs, @code{999} seems reasonable.
+
+@item @code{run-dir} (default: @code{"/var/run/apcupsd"}) (type: string)
+Directory containing runtime information. You need to change this if
+you desire to run multiple instances of the daemon.
+
+@item @code{name} (type: maybe-string)
+Use this to give your UPS a name in log files and such. This is
+particularly useful if you have multiple UPSes. This does not set the
+EEPROM. It should be 8 characters or less.
+
+@item @code{cable} (default: @code{usb}) (type: enum-cable)
+The type of cable connecting the UPS to your computer. Possible generic
+choices are @code{'simple}, @code{'smart}, @code{'ether} and
+@code{'usb}. Or a specific cable model number may be used:
+@code{'940-0119A}, @code{'940-0127A}, @code{'940-0128A},
+@code{'940-0020B}, @code{'940-0020C}, @code{'940-0023A},
+@code{'940-0024B}, @code{'940-0024C}, @code{'940-1524C},
+@code{'940-0024G}, @code{'940-0095A}, @code{'940-0095B},
+@code{'940-0095C}, @code{'940-0625A}, @code{'M-04-02-2000}.
+
+@item @code{type} (default: @code{usb}) (type: enum-type)
+Type of the UPS you have.
+
+@table @code
+@item apcsmart
+Newer serial character device, appropriate for SmartUPS models using a
+serial cable (not USB).
+
+@item usb
+Most new UPSes are USB.
+
+@item net
+Network link to a master apcupsd through apcupsd's Network Information
+Server. This is used if the UPS powering your computer is connected to
+a different computer for monitoring.
+
+@item snmp
+SNMP network link to an SNMP-enabled UPS device.
+
+@item netsnmp
+Same as SNMP above but requires use of the net-snmp library. Unless you
+have a specific need for this old driver, you should use @code{'snmp}
+instead.
+
+@item dumb
+Old serial character device for use with simple-signaling UPSes.
+
+@item pcnet
+PowerChute Network Shutdown protocol which can be used as an alternative
+to SNMP with the AP9617 family of smart slot cards.
+
+@item modbus
+Serial device for use with newest SmartUPS models supporting the MODBUS
+protocol.
+
+@end table
+
+@item @code{device} (default: @code{""}) (type: string)
+For USB UPSes, usually you want to set this to an empty string (the
+default). For other UPS types, you must specify an appropriate port or
+address.
+
+@table @code
+@item apcsmart
+Set to the appropriate @file{/dev/tty**} device.
+
+@item usb
+A null string setting enables autodetection, which is the best choice
+for most installations.
+
+@item net
+Set to @code{@var{hostname}:@var{port}}.
+
+@item snmp
+Set to @code{@var{hostname}:@var{port}:@var{vendor}:@var{community}}.
+@var{hostname} is the ip address or hostname of the UPS on the network.
+@var{vendor} can be can be "APC" or "APC_NOTRAP". "APC_NOTRAP" will
+disable SNMP trap catching; you usually want "APC". @var{port} is
+usually 161. @var{community} is usually "private".
+
+@item netsnmp
+Same as @code{'snmp}.
+
+@item dumb
+Set to the appropriate @file{/dev/tty**} device.
+
+@item pcnet
+Set to @code{@var{ipaddr}:@var{username}:@var{passphrase}:@var{port}}.
+@var{ipaddr} is the IP address of the UPS management card.
+@var{username} and @var{passphrase} are the credentials for which the
+card has been configured. @var{port} is the port number on which to
+listen for messages from the UPS, normally 3052. If this parameter is
+empty or missing, the default of 3052 will be used.
+
+@item modbus
+Set to the appropriate @file{/dev/tty**} device. You can also leave it
+empty for MODBUS over USB or set to the serial number of the UPS.
+
+@end table
+
+@item @code{poll-time} (default: @code{60}) (type: integer)
+Interval (in seconds) at which apcupsd polls the UPS for status. This
+setting applies both to directly-attached UPSes (apcsmart, usb, dumb)
+and networked UPSes (net, snmp). Lowering this setting will improve
+apcupsd's responsiveness to certain events at the cost of higher CPU
+utilization.
+
+@item @code{on-batery-delay} (default: @code{6}) (type: integer)
+The the time in seconds from when a power failure is detected until we
+react to it with an onbattery event. The @code{'powerout} event will be
+triggered immediately when a power failure is detected. However, the
+@code{'onbattery} event will be trigger only after this delay.
+
+@item @code{battery-level} (default: @code{5}) (type: integer)
+If during a power failure, the remaining battery percentage (as reported
+by the UPS) is below or equal to this value, apcupsd will initiate a
+system shutdown.
+
+@quotation Note
+@code{battery-level}, @code{remaining-minutes}, and @code{timeout} work
+in conjunction, so the first that occurs will cause the initation of a
+shutdown.
+@end quotation
+
+@item @code{remaining-minutes} (default: @code{3}) (type: integer)
+If during a power failure, the remaining runtime in minutes (as
+calculated internally by the UPS) is below or equal to this value,
+apcupsd will initiate a system shutdown.
+
+@quotation Note
+@code{battery-level}, @code{remaining-minutes}, and @code{timeout} work
+in conjunction, so the first that occurs will cause the initation of a
+shutdown.
+@end quotation
+
+@item @code{timeout} (default: @code{0}) (type: integer)
+If during a power failure, the UPS has run on batteries for this many
+seconds or longer, apcupsd will initiate a system shutdown. A value of
+0 disables this timer.
+
+@quotation Note
+@code{battery-level}, @code{remaining-minutes}, and @code{timeout} work
+in conjunction, so the first that occurs will cause the initation of a
+shutdown.
+@end quotation
+
+@item @code{annoy-interval} (default: @code{300}) (type: integer)
+Time in seconds between annoying users (via the @code{'annoyme} event)
+to sign off prior to system shutdown. 0 disables.
+
+@item @code{annoy-delay} (default: @code{60}) (type: integer)
+Initial delay in seconds after power failure before warning users to get
+off the system.
+
+@item @code{no-logon} (default: @code{disable}) (type: enum-no-logon)
+The condition which determines when users are prevented from logging in
+during a power failure.
+
+@item @code{kill-delay} (default: @code{0}) (type: integer)
+If this is non-zero, apcupsd will continue running after a shutdown has
+been requested, and after the specified time in seconds attempt to kill
+the power. This is for use on systems where apcupsd cannot regain
+control after a shutdown.
+
+@item @code{net-server} (default: @code{#f}) (type: boolean)
+If enabled, a network information server process will be started.
+
+@item @code{net-server-ip} (default: @code{"127.0.0.1"}) (type: string)
+IP address on which NIS server will listen for incoming connections.
+
+@item @code{net-server-port} (default: @code{3551}) (type: integer)
+IP port on which NIS server will listen for incoming connections.
+
+@item @code{net-server-events-file} (type: maybe-string)
+If you want the last few EVENTS to be available over the network by the
+network information server, you must set this to a file patch.
+
+@item @code{net-server-events-file-max-size} (default: @code{10}) (type: integer)
+Maximum size of the events file in kilobytes.
+
+@item @code{class} (default: @code{standalone}) (type: enum-class)
+Normally standalone unless you share an UPS using an APC ShareUPS card.
+
+@item @code{mode} (default: @code{disable}) (type: enum-mode)
+Normally disable unless you share an UPS using an APC ShareUPS card.
+
+@item @code{stat-time} (default: @code{0}) (type: integer)
+Time interval in seconds between writing the status file, 0 disables.
+
+@item @code{log-stats} (default: @code{#f}) (type: boolean)
+Also write the stats as a logs. This generates a lot of output.
+
+@item @code{data-time} (default: @code{0}) (type: integer)
+Time interval in seconds between writing the data records to the log
+file, 0 disables.
+
+@item @code{facility} (type: maybe-string)
+The logging facility for the syslog.
+
+@item @code{event-handlers} (type: apcupsd-event-handlers)
+Handlers for events produced by apcupsd.
+
+@end table
+
+@end deftp
+
+@anchor{apcupsd-event-handlers}
+@deftp {Data Type} apcupsd-event-handlers
+
+For description of the events please refer to the @command{apcupsd}'s
+manual, which can be found in the @samp{apcupsd-doc} package.
+
+Each handler shall be a gexp. It is spliced into the control script for
+the daemon. In addition to the standard Guile programming environment,
+these procedures and variables are also available.
+
+@table @code
+@item conf
+Variable containing path to the configuration file.
+
+@item powerfail-file
+Variable containing path to the powerfail file.
+
+@item cmd
+The event currently being handled.
+
+@item name
+The name of the UPS as specified in the configuration file.
+
+@item connected
+Is @code{"1"} if apcupsd is connected to the UPS via a serial port (or a
+USB port). In most configurations, this will be the case. In the case
+of a Slave machine where apcupsd is not directly connected to the UPS,
+this value will be @code{"0"}.
+
+@item powered
+Is @code{"1"} if the computer on which @command{apcupsd} is running is
+powered by the UPS and @code{"0"} if not. At the moment, this value is
+unimplemented and always @code{"0"}.
+
+@item (err @var{fmt} @var{args...})
+Wrapper around @code{format} outputting to @code{(current-error-port)}.
+
+@item (wall @var{fmt} @var{args...})
+Wrapper around @code{format} outputting via @command{wall}.
+
+@item (apcupsd @var{args...})
+Call @command{apcupsd} while passing the correct configuration file and
+all the arguments.
+
+@item (mail-to-root @var{subject} @var{body})
+Send an email to the local administrator. This procedure assumes the
+@command{sendmail} is located at @command{/run/privileged/bin/sendmail}
+(as would be the case with @code{opensmtpd-service-type}).
+
+@end table
+
+Available @code{apcupsd-event-handlers} fields are:
+
+@table @asis
+@item @code{killpower} (type: gexp)
+Handler for killpower event.
+
+@item @code{commfailure} (type: gexp)
+Handler for commfailure event.
+
+@item @code{commok} (type: gexp)
+Handler for commfailure event.
+
+@item @code{powerout} (type: gexp)
+Handler for powerout event.
+
+@item @code{onbattery} (type: gexp)
+Handler for onbattery event.
+
+@item @code{offbattery} (type: gexp)
+Handler for offbattery event.
+
+@item @code{mainsback} (type: gexp)
+Handler for mainsback event.
+
+@item @code{failing} (type: gexp)
+Handler for failing event.
+
+@item @code{timeout} (type: gexp)
+Handler for timeout event.
+
+@item @code{loadlimit} (type: gexp)
+Handler for loadlimit event.
+
+@item @code{runlimit} (type: gexp)
+Handler for runlimit event.
+
+@item @code{doreboot} (type: gexp)
+Handler for doreboot event.
+
+@item @code{doshutdown} (type: gexp)
+Handler for doshutdown event.
+
+@item @code{annoyme} (type: gexp)
+Handler for annoyme event.
+
+@item @code{emergency} (type: gexp)
+Handler for emergency event.
+
+@item @code{changeme} (type: gexp)
+Handler for changeme event.
+
+@item @code{remotedown} (type: gexp)
+Handler for remotedown event.
+
+@item @code{startselftest} (type: gexp)
+Handler for startselftest event.
+
+@item @code{endselftest} (type: gexp)
+Handler for endselftest event.
+
+@item @code{battdetach} (type: gexp)
+Handler for battdetach event.
+
+@item @code{battattach} (type: gexp)
+Handler for battattach event.
+
+@end table
+
+@end deftp
+
@node Audio Services
@subsection Audio Services
diff --git a/gnu/local.mk b/gnu/local.mk
index 6ca7bf68ac..dea0e43fe6 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -758,6 +758,7 @@ GNU_SYSTEM_MODULES = \
%D%/services/nix.scm \
%D%/services/nfs.scm \
%D%/services/pam-mount.scm \
+ %D%/services/power.scm \
%D%/services/science.scm \
%D%/services/security.scm \
%D%/services/security-token.scm \
diff --git a/gnu/services/power.scm b/gnu/services/power.scm
new file mode 100644
index 0000000000..a6f9259eb3
--- /dev/null
+++ b/gnu/services/power.scm
@@ -0,0 +1,690 @@
+;;; Copyright (C) 2025 Tomas Volf <~@wolfsden.cz>
+
+;;;; Commentary:
+
+;;; Power-related services.
+
+;;;; Code:
+
+(define-module (gnu services power)
+ #:use-module (srfi srfi-1)
+ #:use-module (srfi srfi-26)
+ #:use-module (ice-9 match)
+ #:use-module (gnu)
+ #:use-module (gnu packages admin)
+ #:use-module (gnu packages linux)
+ #:use-module (gnu packages power)
+ #:use-module (gnu services)
+ #:use-module (gnu services configuration)
+ #:use-module (gnu services shepherd)
+ #:use-module (guix packages)
+ #:use-module (guix records)
+ #:export (apcupsd-service-type
+
+ apcupsd-configuration
+ apcupsd-configuration-package
+ apcupsd-configuration-shepherd-base-name
+ apcupsd-configuration-auto-start?
+ apcupsd-configuration-pid-file
+ apcupsd-configuration-debug-level
+ apcupsd-configuration-run-dir
+ apcupsd-configuration-name
+ apcupsd-configuration-cable
+ apcupsd-configuration-type
+ apcupsd-configuration-device
+ apcupsd-configuration-poll-time
+ apcupsd-configuration-on-batery-delay
+ apcupsd-configuration-battery-level
+ apcupsd-configuration-remaining-minutes
+ apcupsd-configuration-timeout
+ apcupsd-configuration-annoy-interval
+ apcupsd-configuration-annoy-delay
+ apcupsd-configuration-no-logon
+ apcupsd-configuration-kill-delay
+ apcupsd-configuration-net-server
+ apcupsd-configuration-net-server-ip
+ apcupsd-configuration-net-server-port
+ apcupsd-configuration-net-server-events-file
+ apcupsd-configuration-net-server-events-file-max-size
+ apcupsd-configuration-class
+ apcupsd-configuration-mode
+ apcupsd-configuration-stat-time
+ apcupsd-configuration-log-stats
+ apcupsd-configuration-data-time
+ apcupsd-configuration-facility
+ apcupsd-configuration-event-handlers
+
+ apcupsd-event-handlers-annoyme
+ apcupsd-event-handlers-battattach
+ apcupsd-event-handlers-battdetach
+ apcupsd-event-handlers-changeme
+ apcupsd-event-handlers-commfailure
+ apcupsd-event-handlers-commok
+ apcupsd-event-handlers-doreboot
+ apcupsd-event-handlers-doshutdown
+ apcupsd-event-handlers-emergency
+ apcupsd-event-handlers-endselftest
+ apcupsd-event-handlers-failing
+ apcupsd-event-handlers-killpower
+ apcupsd-event-handlers-loadlimit
+ apcupsd-event-handlers-mainsback
+ apcupsd-event-handlers-offbattery
+ apcupsd-event-handlers-onbattery
+ apcupsd-event-handlers-powerout
+ apcupsd-event-handlers-remotedown
+ apcupsd-event-handlers-runlimit
+ apcupsd-event-handlers-startselftest
+ apcupsd-event-handlers-timeout))
+
+(define-configuration/no-serialization apcupsd-event-handlers
+ (killpower
+ (gexp
+ #~((wall "Apccontrol doing: apcupsd --killpower on UPS ~a" name)
+ (sleep 10)
+ (apcupsd "--killpower")
+ (wall "Apccontrol has done: apcupsd --killpower on UPS ~a" name)))
+ "Handler for killpower event.")
+ (commfailure
+ (gexp
+ #~((let ((msg (format #f "~a Communications with UPS ~a lost."
+ (gethostname) name)))
+ (mail-to-root msg msg))
+ (wall "Warning: communications lost with UPS ~a" name)))
+ "Handler for commfailure event.")
+ (commok
+ (gexp
+ #~((let ((msg (format #f "~a Communications with UPS ~a restored."
+ (gethostname) name)))
+ (mail-to-root msg msg))
+ (wall "Communications restored with UPS ~a" name)))
+ "Handler for commfailure event.")
+ (powerout
+ (gexp
+ #~(#t))
+ "Handler for powerout event.")
+ (onbattery
+ (gexp
+ #~((let ((msg (format #f "~a UPS ~a Power Failure !!!"
+ (gethostname) name)))
+ (mail-to-root msg msg))
+ (wall "Power failure on UPS ~a. Running on batteries." name)))
+ "Handler for onbattery event.")
+ (offbattery
+ (gexp
+ #~((let ((msg (format #f "~a UPS ~a Power has returned."
+ (gethostname) name)))
+ (mail-to-root msg msg))
+ (wall "Power has returned on UPS ~a..." name)))
+ "Handler for offbattery event.")
+ (mainsba
This message was truncated. Download the full message here.
M
M
Maxim Cournoyer wrote 6 days ago
Re: [bug#75528] [PATCH 1/2] gnu: Add apcupsd.
(name . Tomas Volf)(address . ~@wolfsden.cz)(address . 75528@debbugs.gnu.org)
87jza498sp.fsf@gmail.com
Hi Tomas,

Tomas Volf <~@wolfsden.cz> writes:

Toggle quote (2 lines)
> * gnu/packages/power.scm (apcupsd): New variable.

'guix lint' says:

Toggle snippet (4 lines)
gnu/packages/power.scm:120:14: apcupsd@3.14.14: no article allowed at the beginning of the synopsis
gnu/packages/power.scm:119:15: apcupsd@3.14.14: TLS certificate error: X.509 server certificate for 'www.apcupsd.org' does not match: CN=sf.net

Please fix these.

Toggle quote (17 lines)
> Change-Id: I74f59cd1fa2a13954117ff1683a10a84576cc839
> ---
> gnu/local.mk | 1 +
> gnu/packages/power.scm | 125 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 126 insertions(+)
> create mode 100644 gnu/packages/power.scm
>
> diff --git a/gnu/local.mk b/gnu/local.mk
> index 855f2acccc..6ca7bf68ac 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -557,6 +557,7 @@ GNU_SYSTEM_MODULES = \
> %D%/packages/polkit.scm \
> %D%/packages/popt.scm \
> %D%/packages/potassco.scm \
> + %D%/packages/power.scm \

This change should be mentioned in the change log as well, e.g.:

* gnu/local.mk (GNU_SYSTEM_MODULES): Register it.

[...]

Toggle quote (18 lines)
> +(define-public apcupsd
> + (package
> + (name "apcupsd")
> + (version "3.14.14")
> + (source (origin
> + (method url-fetch)
> + (uri
> + (string-append
> + "mirror://sourceforge/" name "/" name " - Stable/" version
> + "/" name "-" version ".tar.gz"))
> + (sha256
> + (base32
> + "0rwqiyzlg9p0szf3x6q1ppvrw6f6dbpn2rc5z623fk3bkdalhxyv"))))
> + (native-inputs
> + (list pkg-config python-docutils))
> + (inputs
> + (list libusb libusb-compat))

nitpick: If there are less than 5 and they fit on the same line, I
prefer to list them on the same line as the field name itself. That's
how 'guix style' does it, although it has a tendency to protrude past
our max column width of 80 guideline in other places (that's bug#62303).

Also, we conventionally list the input fields below the arguments field
of a package, although technically it's unimportant.

Toggle quote (8 lines)
> + (outputs '("out" "doc"))
> + (build-system gnu-build-system)
> + (arguments
> + (list
> + #:configure-flags
> + #~(list
> + ;; The configure script ignores --prefix for most of the paths.

Well done with the comment.

Toggle quote (9 lines)
> + (string-append "--exec-prefix=" #$output)
> + (string-append "--mandir=" #$output "/share/man")
> + (string-append "--sbindir=" #$output "/sbin")
> + (string-append "--sysconfdir=" #$output "/etc/apcupsd")
> + (string-append "--with-halpolicydir=" #$output "/share/halpolicy")
> +
> + ;; Put us into the version string.
> + "--with-distname=GNU/Guix"

The name is 'GNU Guix'.

Toggle quote (10 lines)
> + "--disable-install-distdir"
> +
> + ;; State directories.
> + "--localstatedir=/var"
> + "--with-log-dir=/var/log"
> + "--with-pid-dir=/var/run"
> + "--with-lock-dir=/var/run/apcupsd/lock"
> + "--with-nologin=/var/run/apcupsd"
> + "--with-pwrfail-dir=/var/run/apcupsd"

I think /var/run is deprecated in favor of just /run, so we should
configure the package to use this, I think.

Toggle quote (5 lines)
> + (string-append "ac_cv_path_SHUTDOWN=/nope")
> + (string-append "ac_cv_path_APCUPSD_MAIL=/nope")
> + ;; While `wall' is not expanded anywhere, it still is searched for.
> + (string-append "ac_cv_path_WALL=/nope")

I'm not sure if this package is actively developed, but that last issue
should ideally be reported (then cross-referenced here).

Toggle quote (3 lines)
> + ;; Enable additional drivers.
> + "--enable-test"

Is '--enable-test' useful? What does it do?

Toggle quote (18 lines)
> + "--enable-usb"
> + "--enable-modbus-usb")
> + #:tests? #f ; There are no tests.
> + #:modules (cons '(ice-9 ftw) %default-gnu-modules)
> + #:phases
> + #~(modify-phases %standard-phases
> + ;; These are not installed, so trick Make into thinking they were
> + ;; already generated. Allows us not to depend on mandoc, util-linux.
> + (add-before 'configure 'touch-txt-docs
> + (lambda _
> + (for-each (lambda (f)
> + (call-with-output-file f close-port))
> + '("doc/apcupsd.man.txt"
> + "doc/apcaccess.man.txt"
> + "doc/apctest.man.txt"
> + "doc/apccontrol.man.txt"
> + "doc/apcupsd.conf.man.txt"))))

I think I'd rather depend on these than introduce hacks like below.
These are only needed as native inputs anyway, right?

Toggle quote (23 lines)
> + (add-after 'build 'build-manual
> + (lambda _
> + (invoke "make" "-C" "doc/manual" "manual.html")))
> + (add-after 'install-license-files 'move-doc
> + (lambda _
> + (let ((target (string-append #$output:doc
> + "/share/doc/"
> + (strip-store-file-name #$output))))
> + (mkdir-p target)
> + (for-each (lambda (f)
> + (copy-file (string-append "doc/manual/" f)
> + (string-append target "/" f)))
> + (scandir "doc/manual"
> + (lambda (f)
> + (or (string-suffix? ".png" f)
> + (string-suffix? ".html" f))))))))
> + ;; If sending mails is required, use proper mail program.
> + (add-after 'install 'remove-smtp
> + (lambda _
> + (delete-file (string-append #$output "/sbin/smtp"))))
> + ;; The configuration files and scripts are not really suitable for
> + ;; Guix, and our service provides its own version anyway. So nuke

I'd use the more peaceful 'remove' or 'delete' instead of 'nuke'.

Toggle quote (4 lines)
> + ;; these to make sure `apcupsd' and `apctest' executed without any
> + ;; arguments fail. `apctest' actually segfaults, but only after
> + ;; printing an error ¯\_(?)_/¯.

Please don't embed emojis in the source :-).

Toggle quote (4 lines)
> + (add-after 'install 'remove-etc-apcupsd
> + (lambda _
> + (delete-file-recursively (string-append #$output "/etc/apcupsd")))))))

Break this long line so it fits under 80 columns (our code style
guideline).

Toggle quote (2 lines)
> + (home-page "https://www.apcupsd.org")

I'd use 'http', since their TLS cert is now invalid.

Toggle quote (2 lines)
> + (synopsis "A daemon for controlling APC UPSes")

s/A // (as hinted by 'guix lint').

Toggle quote (2 lines)
> + (description "Apcupsd can be used for power mangement and controlling most

s/mangement/management/

Toggle quote (5 lines)
> +of APC’s UPS models on Unix and Windows machines. Apcupsd works with most of
> +APC’s Smart-UPS models as well as most simple signalling models such a Back-UPS,
> +and BackUPS-Office.")
> + (license license:gpl2)))

I think it's actually license:gpl2+, according to
apcupsd-3.14.14/COPYING, which says:

Toggle snippet (9 lines)
Each version is given a distinguishing version number. If the Program
specifies a version number of this License which applies to it and "any
later version", you have the option of following the terms and conditions
either of that version or of any later version published by the Free
Software Foundation. If the Program does not specify a version number of
this License, you may choose any version ever published by the Free Software
Foundation.

In this case for most file the last sentence applies, some other
explicitly mention 'or any later version'. That's also supported by the
debian/copyright file.

Another thing found: the doc output doesn't build reproducibly, as
shown by:

Toggle snippet (4 lines)
$ ./pre-inst-env guix build --no-grafts --check -K
guix build: error: derivation `/gnu/store/r62j72bd3an8k2fbmaiil5hma32syxdy-apcupsd-3.14.14.drv' may not be deterministic: output `/gnu/store/bhwkrgkqahgs5h8crcfyacvnwp7rj390-apcupsd-3.14.14-doc' differs from `/gnu/store/bhwkrgkqahgs5h8crcfyacvnwp7rj390-apcupsd-3.14.14-doc-check'

Let's see why:

$ diff -r /gnu/store/bhwkrgkqahgs5h8crcfyacvnwp7rj390-apcupsd-3.14.14-doc{,-check}
diff -r /gnu/store/bhwkrgkqahgs5h8crcfyacvnwp7rj390-apcupsd-3.14.14-doc/share/doc/apcupsd-3.14.14/manual.html /gnu/store/bhwkrgkqahgs5h8crcfyacvnwp7rj390-apcupsd-3.14.14-doc-check/share/doc/apcupsd-3.14.14/manual.html
376c376
< <div class="line">February 5, 2025 06:54:22</div>
---
Toggle quote (2 lines)
> <div class="line">February 5, 2025 06:54:50</div>

Ah, the classic date time stamp. You'll want to neuter it in the source
or in the built html file (with a preference for the former).

Could you please send a v2?

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote 6 days ago
(name . Tomas Volf)(address . ~@wolfsden.cz)(address . 75528@debbugs.gnu.org)
87frks98fm.fsf@gmail.com
Hi,

Tomas Volf <~@wolfsden.cz> writes:

Toggle quote (6 lines)
> --- /dev/null
> +++ b/gnu/packages/power.scm
> @@ -0,0 +1,125 @@
> +;;; Copyright (C) 2025 Tomas Volf <~@wolfsden.cz>
> +;;; Copyright (C) 2023 Raven Hallsby <karl@hallsby.com>

One last thing; I don't see Raven listed in a 'Co-authored-by:' git
trailer in the commit message; should they? Or otherwise mention
plainly this work was based on their previous work, made available
'$where'.

--
Thanks,
Maxim
M
M
Maxim Cournoyer wrote 6 days ago
Re: [bug#75528] [PATCH 2/2] services: Add power.
(name . Tomas Volf)(address . ~@wolfsden.cz)
878qqk8q96.fsf@gmail.com
Hi Tomas,

Tomas Volf <~@wolfsden.cz> writes:

Toggle quote (2 lines)
> * gnu/services/power.scm: New file.

I see there's already a gnu/services/pm.scm file; it seems we don't need
another one? Or was it a conscious decision?

[...]

Toggle quote (5 lines)
> +The @code{(gnu services power)} module provides a service definition for
> +@uref{http://www.apcupsd.org/, apcupsd}, a utility to interact with APC
> +UPSes. Apcupsd also works with some OEM-branded products manufactured
> +by APC.

I'd introduce a few @acronym here to help readers, e.g. for OEM and UPS.

Toggle quote (7 lines)
> +@defvar apcupsd-service-type
> +The service type for apcupsd. For USB UPSes no configuration is
> +necessary, however tweaking some fields to better suit your needs might
> +be desirable. The defaults are taken from the upstream configuration
> +and they are not very conservative (@code{battery-level} of 5% is too
> +low in my opinion).

I'd say, (for example, the default @code{battery-level} of 5% may be
considered too low by some), to keep it neutral.

Toggle quote (16 lines)
> +The default event handlers do send emails, read more in
> +@ref{apcupsd-event-handlers}.
> +
> +@lisp
> +(service apcupsd-service-type)
> +@end lisp
> +@end defvar
> +
> +@deftp {Data Type} apcupsd-configuration
> +
> +Available @code{apcupsd-configuration} fields are:
> +
> +@table @asis
> +@item @code{package} (default: @code{apcupsd}) (type: package)
> +Package to use.

That's more conventionally named with the same of the package,
e.g. 'acpupsd', not 'package'. I'd word the description as 'The
@code{apcupsd} package to use'.

Toggle quote (4 lines)
> +@item @code{shepherd-base-name} (default: @code{apcupsd}) (type: symbol)
> +Base name of the shepherd service. You can add the service multiple
> +times with different prefix to manage multiple UPSes.

s/prefix/prefixes/
aaa
Toggle quote (7 lines)
> +
> +@item @code{auto-start?} (default: @code{#t}) (type: boolean)
> +Should the shepherd service auto-start?
> +
> +@item @code{pid-file} (default: @code{"/var/run/apcupsd.pid"}) (type: string)
> +Path to the pid file.

All file names becoming /run/ prefixed instead of /var/run, based on a
previous comment.

We don't use path in GNU to denote file names, we use 'file names'.
That's mentioned in info '(standards) GNU Manuals':

Please do not use the term "pathname" that is used in Unix
documentation; use "file name" (two words) instead. We use the term
"path" only for search paths, which are lists of directory names.

Toggle quote (5 lines)
> +
> +@item @code{debug-level} (default: @code{0}) (type: integer)
> +Logging verbosity. Bigger number means more logs. In the source code I
> +saw up to @code{300}, so for all logs, @code{999} seems reasonable.

I think it's best to not use first person pronoun in the manual to keep
it a bit more formal. So instead of 'I saw', this could be reworded to:
'The source code uses up to @code{300} as debug level value, so a value
of @code{999} seems reasonable to enable all logs.' or similar.

Toggle quote (15 lines)
> +
> +@item @code{run-dir} (default: @code{"/var/run/apcupsd"}) (type: string)
> +Directory containing runtime information. You need to change this if
> +you desire to run multiple instances of the daemon.
> +
> +@item @code{name} (type: maybe-string)
> +Use this to give your UPS a name in log files and such. This is
> +particularly useful if you have multiple UPSes. This does not set the
> +EEPROM. It should be 8 characters or less.
> +
> +@item @code{cable} (default: @code{usb}) (type: enum-cable)
> +The type of cable connecting the UPS to your computer. Possible generic
> +choices are @code{'simple}, @code{'smart}, @code{'ether} and
> +@code{'usb}. Or a specific cable model number may be used:

nitpick: I'd reword to 'Alternatively, a specific [...]', which reads
better to me.

[...]

Toggle quote (4 lines)
> +@item usb
> +A null string setting enables autodetection, which is the best choice
> +for most installations.

nitpick: My dictionary (aspell) prefers auto-detection.

[...]

Toggle quote (6 lines)
> +@anchor{apcupsd-event-handlers}
> +@deftp {Data Type} apcupsd-event-handlers
> +
> +For description of the events please refer to the @command{apcupsd}'s
> +manual, which can be found in the @samp{apcupsd-doc} package.

I think you also meant it is in the @samp{doc} output, not in a
different package, right? In any case, I'd refer them to the man page
of apccontrol, which details the events. Also, to avoid the odd
rendering as "the ‘apcupsd’’s manual" in info, I'd instead rephrase to:

For a description of the events please refer to @samp{man 8 apccontrol}.

Toggle quote (5 lines)
> +
> +Each handler shall be a gexp. It is spliced into the control script for
> +the daemon. In addition to the standard Guile programming environment,
> +these procedures and variables are also available.

s/these/the following/ [...] s/available./available:/
Since these are spliced in the control script and thus not at the top
level, users won't be able to import Guile modules they may want to use,
so it seems there should be a field to define which Guile modules should
be imported and used by default in the configuration.

Toggle quote (5 lines)
> +
> +@table @code
> +@item conf
> +Variable containing path to the configuration file.

s/path/file name/, anywhere this appears.

Toggle quote (4 lines)
> +
> +@item powerfail-file
> +Variable containing path to the powerfail file.

Ditto.

Toggle quote (9 lines)
> +@item cmd
> +The event currently being handled.
> +
> +@item name
> +The name of the UPS as specified in the configuration file.
> +
> +@item connected
> +Is @code{"1"} if apcupsd is connected to the UPS via a serial port (or a

@command{apcupsd}

Also, any reason why these integer values are expressed as strings?

Toggle quote (3 lines)
> +USB port). In most configurations, this will be the case. In the case
> +of a Slave machine where apcupsd is not directly connected to the UPS,

s/Slave/slave/ ? Could be nicer to use a neutral alternative like
'follower machine'.

Toggle quote (94 lines)
> +this value will be @code{"0"}.
> +
> +@item powered
> +Is @code{"1"} if the computer on which @command{apcupsd} is running is
> +powered by the UPS and @code{"0"} if not. At the moment, this value is
> +unimplemented and always @code{"0"}.
> +
> +@item (err @var{fmt} @var{args...})
> +Wrapper around @code{format} outputting to @code{(current-error-port)}.
> +
> +@item (wall @var{fmt} @var{args...})
> +Wrapper around @code{format} outputting via @command{wall}.
> +
> +@item (apcupsd @var{args...})
> +Call @command{apcupsd} while passing the correct configuration file and
> +all the arguments.
> +
> +@item (mail-to-root @var{subject} @var{body})
> +Send an email to the local administrator. This procedure assumes the
> +@command{sendmail} is located at @command{/run/privileged/bin/sendmail}
> +(as would be the case with @code{opensmtpd-service-type}).
> +
> +@end table
> +
> +Available @code{apcupsd-event-handlers} fields are:
> +
> +@table @asis
> +@item @code{killpower} (type: gexp)
> +Handler for killpower event.
> +
> +@item @code{commfailure} (type: gexp)
> +Handler for commfailure event.
> +
> +@item @code{commok} (type: gexp)
> +Handler for commfailure event.
> +
> +@item @code{powerout} (type: gexp)
> +Handler for powerout event.
> +
> +@item @code{onbattery} (type: gexp)
> +Handler for onbattery event.
> +
> +@item @code{offbattery} (type: gexp)
> +Handler for offbattery event.
> +
> +@item @code{mainsback} (type: gexp)
> +Handler for mainsback event.
> +
> +@item @code{failing} (type: gexp)
> +Handler for failing event.
> +
> +@item @code{timeout} (type: gexp)
> +Handler for timeout event.
> +
> +@item @code{loadlimit} (type: gexp)
> +Handler for loadlimit event.
> +
> +@item @code{runlimit} (type: gexp)
> +Handler for runlimit event.
> +
> +@item @code{doreboot} (type: gexp)
> +Handler for doreboot event.
> +
> +@item @code{doshutdown} (type: gexp)
> +Handler for doshutdown event.
> +
> +@item @code{annoyme} (type: gexp)
> +Handler for annoyme event.
> +
> +@item @code{emergency} (type: gexp)
> +Handler for emergency event.
> +
> +@item @code{changeme} (type: gexp)
> +Handler for changeme event.
> +
> +@item @code{remotedown} (type: gexp)
> +Handler for remotedown event.
> +
> +@item @code{startselftest} (type: gexp)
> +Handler for startselftest event.
> +
> +@item @code{endselftest} (type: gexp)
> +Handler for endselftest event.
> +
> +@item @code{battdetach} (type: gexp)
> +Handler for battdetach event.
> +
> +@item @code{battattach} (type: gexp)
> +Handler for battattach event.
> +
> +@end table
> +
> +@end deftp

nitpick: I'd remove the newline between the two @end above (I know, it
gets generated this way...).

[...]

Toggle quote (2 lines)
> +;;; Copyright (C) 2025 Tomas Volf <~@wolfsden.cz>

We use the unicode symbol (©) for copyright throughout Guix; please use
it too, for uniformity.

Toggle quote (21 lines)
> +
> +;;;; Commentary:
> +
> +;;; Power-related services.
> +
> +;;;; Code:
> +
> +(define-module (gnu services power)
> + #:use-module (srfi srfi-1)
> + #:use-module (srfi srfi-26)
> + #:use-module (ice-9 match)
> + #:use-module (gnu)
> + #:use-module (gnu packages admin)
> + #:use-module (gnu packages linux)
> + #:use-module (gnu packages power)
> + #:use-module (gnu services)
> + #:use-module (gnu services configuration)
> + #:use-module (gnu services shepherd)
> + #:use-module (guix packages)
> + #:use-module (guix records)

Please order lexicographically (that is, alphabetically but on things
which are not necessarily words :-)).

Toggle quote (66 lines)
> + #:export (apcupsd-service-type
> +
> + apcupsd-configuration
> + apcupsd-configuration-package
> + apcupsd-configuration-shepherd-base-name
> + apcupsd-configuration-auto-start?
> + apcupsd-configuration-pid-file
> + apcupsd-configuration-debug-level
> + apcupsd-configuration-run-dir
> + apcupsd-configuration-name
> + apcupsd-configuration-cable
> + apcupsd-configuration-type
> + apcupsd-configuration-device
> + apcupsd-configuration-poll-time
> + apcupsd-configuration-on-batery-delay
> + apcupsd-configuration-battery-level
> + apcupsd-configuration-remaining-minutes
> + apcupsd-configuration-timeout
> + apcupsd-configuration-annoy-interval
> + apcupsd-configuration-annoy-delay
> + apcupsd-configuration-no-logon
> + apcupsd-configuration-kill-delay
> + apcupsd-configuration-net-server
> + apcupsd-configuration-net-server-ip
> + apcupsd-configuration-net-server-port
> + apcupsd-configuration-net-server-events-file
> + apcupsd-configuration-net-server-events-file-max-size
> + apcupsd-configuration-class
> + apcupsd-configuration-mode
> + apcupsd-configuration-stat-time
> + apcupsd-configuration-log-stats
> + apcupsd-configuration-data-time
> + apcupsd-configuration-facility
> + apcupsd-configuration-event-handlers
> +
> + apcupsd-event-handlers-annoyme
> + apcupsd-event-handlers-battattach
> + apcupsd-event-handlers-battdetach
> + apcupsd-event-handlers-changeme
> + apcupsd-event-handlers-commfailure
> + apcupsd-event-handlers-commok
> + apcupsd-event-handlers-doreboot
> + apcupsd-event-handlers-doshutdown
> + apcupsd-event-handlers-emergency
> + apcupsd-event-handlers-endselftest
> + apcupsd-event-handlers-failing
> + apcupsd-event-handlers-killpower
> + apcupsd-event-handlers-loadlimit
> + apcupsd-event-handlers-mainsback
> + apcupsd-event-handlers-offbattery
> + apcupsd-event-handlers-onbattery
> + apcupsd-event-handlers-powerout
> + apcupsd-event-handlers-remotedown
> + apcupsd-event-handlers-runlimit
> + apcupsd-event-handlers-startselftest
> + apcupsd-event-handlers-timeout))
> +
> +(define-configuration/no-serialization apcupsd-event-handlers
> + (killpower
> + (gexp
> + #~((wall "Apccontrol doing: apcupsd --killpower on UPS ~a" name)
> + (sleep 10)
> + (apcupsd "--killpower")
> + (wall "Apccontrol has done: apcupsd --killpower on UPS ~a" name)))
> + "Handler for killpower event.")

In many places in your writing, I find that it skips adding 'the'
(definite article), which I've learned should be use when describing
something specific, such as here with the specific handlers:

"Handler for the @code{killpower} event."

In case it helps, I've found this page seems useful in briefly
explaining when to use 'the' [0].


Toggle quote (99 lines)
> + (commfailure
> + (gexp
> + #~((let ((msg (format #f "~a Communications with UPS ~a lost."
> + (gethostname) name)))
> + (mail-to-root msg msg))
> + (wall "Warning: communications lost with UPS ~a" name)))
> + "Handler for commfailure event.")
> + (commok
> + (gexp
> + #~((let ((msg (format #f "~a Communications with UPS ~a restored."
> + (gethostname) name)))
> + (mail-to-root msg msg))
> + (wall "Communications restored with UPS ~a" name)))
> + "Handler for commfailure event.")
> + (powerout
> + (gexp
> + #~(#t))
> + "Handler for powerout event.")
> + (onbattery
> + (gexp
> + #~((let ((msg (format #f "~a UPS ~a Power Failure !!!"
> + (gethostname) name)))
> + (mail-to-root msg msg))
> + (wall "Power failure on UPS ~a. Running on batteries." name)))
> + "Handler for onbattery event.")
> + (offbattery
> + (gexp
> + #~((let ((msg (format #f "~a UPS ~a Power has returned."
> + (gethostname) name)))
> + (mail-to-root msg msg))
> + (wall "Power has returned on UPS ~a..." name)))
> + "Handler for offbattery event.")
> + (mainsback
> + (gexp
> + #~((when (file-exists? powerfail-file)
> + (wall "Continuing with shutdown."))))
> + "Handler for mainsback event.")
> + (failing
> + (gexp
> + #~((wall "Battery power exhausted on UPS ~a. Doing shutdown." name)))
> + "Handler for failing event.")
> + (timeout
> + (gexp
> + #~((wall "Battery time limit exceeded on UPS ~a. Doing shutdown." name)))
> + "Handler for timeout event.")
> + (loadlimit
> + (gexp
> + #~((wall "Remaining battery charge below limit on UPS ~a. Doing shutdown." name)))
> + "Handler for loadlimit event.")
> + (runlimit
> + (gexp
> + #~((wall "Remaining battery runtime below limit on UPS ~a. Doing shutdown." name)))
> + "Handler for runlimit event.")
> + (doreboot
> + (gexp
> + #~((wall "UPS ~a initiating Reboot Sequence" name)
> + (system* #$(file-append shepherd "/sbin/reboot"))))
> + "Handler for doreboot event.")
> + (doshutdown
> + (gexp
> + #~((wall "UPS ~a initiated Shutdown Sequence" name)
> + (system* #$(file-append shepherd "/sbin/halt"))))
> + "Handler for doshutdown event.")
> + (annoyme
> + (gexp
> + #~((wall "Power problems with UPS ~a. Please logoff." name)))
> + "Handler for annoyme event.")
> + (emergency
> + (gexp
> + #~((wall "Emergency Shutdown. Possible battery failure on UPS ~a." name)))
> + "Handler for emergency event.")
> + (changeme
> + (gexp
> + #~((let ((msg (format #f "~a UPS ~a battery needs changing NOW."
> + (gethostname) name)))
> + (mail-to-root msg msg))
> + (wall "Emergency! Batteries have failed on UPS ~a. Change them NOW." name)))
> + "Handler for changeme event.")
> + (remotedown
> + (gexp
> + #~((wall "Remote Shutdown. Beginning Shutdown Sequence.")))
> + "Handler for remotedown event.")
> + (startselftest
> + (gexp
> + #~(#t))
> + "Handler for startselftest event.")
> + (endselftest
> + (gexp
> + #~(#t))
> + "Handler for endselftest event.")
> + (battdetach
> + (gexp
> + #~(#t))
> + "Handler for battdetach event.")
> + (battattach
> + (gexp
> + #~(#t))
> + "Handler for battattach event."))

Sounds fun :-). I was wondering if the handlers could be of the
maybe-gexp type, and when not provided the handler would not be called
at all? Or does the design of apcupsd mandates that all handlers be
defined?

Toggle quote (10 lines)
> +(define-syntax define-enum
> + (lambda (x)
> + (syntax-case x ()
> + ((_ name values)
> + (let* ((d/n (syntax->datum #'name))
> + (d/predicate (string->symbol
> + (format #f "enum-~a?" d/n)))
> + (d/serialize (string->symbol
> + (format #f "serialize-enum-~a" d/n))))

Even for internal bindings, better variable names would help reading the
code.

Toggle quote (8 lines)
> + (with-syntax
> + ((predicate (datum->syntax x d/predicate))
> + (serialize (datum->syntax x d/serialize)))
> + #'(begin
> + (define (predicate value)
> + (memq value values))
> + (define serialize serialize-symbol))))))))

That is a nice minimal enum for using with define-configuration. Note
that Guile has some kind of very limited enum from (rnrs enums), but I
don't think they'd be too useful here. See for example
'make-enumeration' from (rnrs enums), which I used in for the
ntp-server-types in (gnu services networking).

Toggle quote (38 lines)
> +(define mangle-field-name
> + (match-lambda
> + ('name "UPSNAME")
> + ('cable "UPSCABLE")
> + ('type "UPSTYPE")
> + ('device "DEVICE")
> + ('poll-time "POLLTIME")
> + ('lock-dir "LOCKFILE")
> + ('power-fail-dir "PWRFAILDIR")
> + ('no-login-dir "NOLOGINDIR")
> + ('on-batery-delay "ONBATTERYDELAY")
> + ('battery-level "BATTERYLEVEL")
> + ('remaining-minutes "MINUTES")
> + ('timeout "TIMEOUT")
> + ('annoy-interval "ANNOY")
> + ('annoy-delay "ANNOYDELAY")
> + ('no-logon "NOLOGON")
> + ('kill-delay "KILLDELAY")
> + ('net-server "NETSERVER")
> + ('net-server-ip "NISIP")
> + ('net-server-port "NISPORT")
> + ('net-server-events-file "EVENTSFILE")
> + ('net-server-events-file-max-size "EVENTSFILEMAX")
> + ('class "UPSCLASS")
> + ('mode "UPSMODE")
> + ('stat-time "STATTIME")
> + ('stat-file "STATFILE")
> + ('log-stats "LOGSTATS")
> + ('data-time "DATATIME")
> + ('facility "FACILITY")))
> +
> +(define (serialize-string field-name value)
> + #~(format #f "~a ~a\n" #$(mangle-field-name field-name) '#$value))
> +(define serialize-symbol serialize-string)
> +(define serialize-integer serialize-string)
> +(define (serialize-boolean field-name value)
> + #~(format #f "~a ~a\n" #$(mangle-field-name field-name) #$(if value "on" "off")))

You'll want to break the above line.

Toggle quote (15 lines)
> +
> +(define-maybe string)
> +
> +(define-enum cable '( simple smart ether usb
> + 940-0119A 940-0127A 940-0128A 940-0020B 940-0020C
> + 940-0023A 940-0024B 940-0024C 940-1524C 940-0024G
> + 940-0095A 940-0095B 940-0095C 940-0625A MAM-04-02-2000))
> +(define-enum type '(apcsmart usb net snmp netsnmp dumb pcnet modbus test))
> +(define-enum no-logon '(disable timeout percent minutes always))
> +(define-enum class '(standalone shareslave sharemaster))
> +(define-enum mode '(disable share))
> +
> +(define-configuration apcupsd-configuration
> + (package (package apcupsd) "Package to use.")

Please stick to the convention where the name of the package is the name
of the fi
This message was truncated. Download the full message here.
T
T
Tomas Volf wrote 34 hours ago
Re: [bug#75528] [PATCH 1/2] gnu: Add apcupsd.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)(address . 75528@debbugs.gnu.org)
87pljrf69g.fsf@wolfsden.cz
Hello Maxim,

thank you for the review, replies below.

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Toggle quote (14 lines)
> Hi Tomas,
>
> Tomas Volf <~@wolfsden.cz> writes:
>
>> * gnu/packages/power.scm (apcupsd): New variable.
>
> 'guix lint' says:
>
> gnu/packages/power.scm:120:14: apcupsd@3.14.14: no article allowed at the beginning of the synopsis
> gnu/packages/power.scm:119:15: apcupsd@3.14.14: TLS certificate error: X.509 server certificate for 'www.apcupsd.org' does not match: CN=sf.net
>
>
> Please fix these.

Both fixed.

Toggle quote (24 lines)
>
>> Change-Id: I74f59cd1fa2a13954117ff1683a10a84576cc839
>> ---
>> gnu/local.mk | 1 +
>> gnu/packages/power.scm | 125 +++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 126 insertions(+)
>> create mode 100644 gnu/packages/power.scm
>>
>> diff --git a/gnu/local.mk b/gnu/local.mk
>> index 855f2acccc..6ca7bf68ac 100644
>> --- a/gnu/local.mk
>> +++ b/gnu/local.mk
>> @@ -557,6 +557,7 @@ GNU_SYSTEM_MODULES = \
>> %D%/packages/polkit.scm \
>> %D%/packages/popt.scm \
>> %D%/packages/potassco.scm \
>> + %D%/packages/power.scm \
>
> This change should be mentioned in the change log as well, e.g.:
>
> * gnu/local.mk (GNU_SYSTEM_MODULES): Register it.
>
> [...]

I used slightly different message, since the previous line mentions "new
variable", but the object being registered is whole file.

Toggle quote (25 lines)
>
>> +(define-public apcupsd
>> + (package
>> + (name "apcupsd")
>> + (version "3.14.14")
>> + (source (origin
>> + (method url-fetch)
>> + (uri
>> + (string-append
>> + "mirror://sourceforge/" name "/" name " - Stable/" version
>> + "/" name "-" version ".tar.gz"))
>> + (sha256
>> + (base32
>> + "0rwqiyzlg9p0szf3x6q1ppvrw6f6dbpn2rc5z623fk3bkdalhxyv"))))
>> + (native-inputs
>> + (list pkg-config python-docutils))
>> + (inputs
>> + (list libusb libusb-compat))
>
> nitpick: If there are less than 5 and they fit on the same line, I
> prefer to list them on the same line as the field name itself. That's
> how 'guix style' does it, although it has a tendency to protrude past
> our max column width of 80 guideline in other places (that's
> bug#62303).

Fair enough. I do not have strong preference, so I have made it into a
single line.

I have to admit I do not run `guix style' (yes, yes, I know contributing
part of the manual mandates it), since the code produced is often subpar
compared to the hand-crafted variant.

Toggle quote (4 lines)
>
> Also, we conventionally list the input fields below the arguments field
> of a package, although technically it's unimportant.

Moved.

Toggle quote (22 lines)
>
>> + (outputs '("out" "doc"))
>> + (build-system gnu-build-system)
>> + (arguments
>> + (list
>> + #:configure-flags
>> + #~(list
>> + ;; The configure script ignores --prefix for most of the paths.
>
> Well done with the comment.
>
>> + (string-append "--exec-prefix=" #$output)
>> + (string-append "--mandir=" #$output "/share/man")
>> + (string-append "--sbindir=" #$output "/sbin")
>> + (string-append "--sysconfdir=" #$output "/etc/apcupsd")
>> + (string-append "--with-halpolicydir=" #$output "/share/halpolicy")
>> +
>> + ;; Put us into the version string.
>> + "--with-distname=GNU/Guix"
>
> The name is 'GNU Guix'.

I was afraid of the space. :) But seems to work fine, changed.

Toggle quote (14 lines)
>
>> + "--disable-install-distdir"
>> +
>> + ;; State directories.
>> + "--localstatedir=/var"
>> + "--with-log-dir=/var/log"
>> + "--with-pid-dir=/var/run"
>> + "--with-lock-dir=/var/run/apcupsd/lock"
>> + "--with-nologin=/var/run/apcupsd"
>> + "--with-pwrfail-dir=/var/run/apcupsd"
>
> I think /var/run is deprecated in favor of just /run, so we should
> configure the package to use this, I think.

It seems pretty much all programs on my system are using /var/run,
including Shepherd. Only programs that store information in /run seem
to be elogind and dbus.

Is deprecation of /var/run listed somewhere in the manual? I am
surprised Shepherd (since it is actively maintained and core component)
is still in /var/run. I looked and found 42 references to /var/run, but
none with the deprecation notice.

Assuming you will insist on moving it to /run, is there some structure
to follow in that directory? Or just s~/var/run~/run~? What about /var
and /var/log? Should those be moved somewhere as well?

Toggle quote (9 lines)
>
>> + (string-append "ac_cv_path_SHUTDOWN=/nope")
>> + (string-append "ac_cv_path_APCUPSD_MAIL=/nope")
>> + ;; While `wall' is not expanded anywhere, it still is searched for.
>> + (string-append "ac_cv_path_WALL=/nope")
>
> I'm not sure if this package is actively developed, but that last issue
> should ideally be reported (then cross-referenced here).

I did not found a way to report a bug. Nothing is listed in the manual,
and their apcupsd-users mailing list requires JavaScript to sign up (and
does not tell me the email address otherwise).

I did try to blindly send an email, will see if it will be accepted
without being subscribed. If that happens, I will add the link here.

Toggle quote (6 lines)
>
>> + ;; Enable additional drivers.
>> + "--enable-test"
>
> Is '--enable-test' useful? What does it do?

According to the manual

Toggle snippet (3 lines)
This turns on a test driver that is used only for debugging.

Since there does not seem to be any harm in having it on, I enabled it,
since Guix seems to aim for feature complete packages (I assume that is
the reason for everything being so large). But I have no use for it, so
I can turn it off if you would prefer.

Toggle quote (21 lines)
>
>> + "--enable-usb"
>> + "--enable-modbus-usb")
>> + #:tests? #f ; There are no tests.
>> + #:modules (cons '(ice-9 ftw) %default-gnu-modules)
>> + #:phases
>> + #~(modify-phases %standard-phases
>> + ;; These are not installed, so trick Make into thinking they were
>> + ;; already generated. Allows us not to depend on mandoc, util-linux.
>> + (add-before 'configure 'touch-txt-docs
>> + (lambda _
>> + (for-each (lambda (f)
>> + (call-with-output-file f close-port))
>> + '("doc/apcupsd.man.txt"
>> + "doc/apcaccess.man.txt"
>> + "doc/apctest.man.txt"
>> + "doc/apccontrol.man.txt"
>> + "doc/apcupsd.conf.man.txt"))))
>
> I think I'd rather depend on these than introduce hacks like below.

The "hacks" *below* would still be required. The HTML manual is not
built nor installed by default, and there is no target to invoke to
install them. So both 'build-manual and 'move-doc phases would still be
required.

We could get rid of the "hack" *above* ('touch-txt-docs), true. It is
worth those two additional inputs?

Toggle quote (2 lines)
> These are only needed as native inputs anyway, right?

Yes.

Toggle quote (26 lines)
>
>> + (add-after 'build 'build-manual
>> + (lambda _
>> + (invoke "make" "-C" "doc/manual" "manual.html")))
>> + (add-after 'install-license-files 'move-doc
>> + (lambda _
>> + (let ((target (string-append #$output:doc
>> + "/share/doc/"
>> + (strip-store-file-name #$output))))
>> + (mkdir-p target)
>> + (for-each (lambda (f)
>> + (copy-file (string-append "doc/manual/" f)
>> + (string-append target "/" f)))
>> + (scandir "doc/manual"
>> + (lambda (f)
>> + (or (string-suffix? ".png" f)
>> + (string-suffix? ".html" f))))))))
>> + ;; If sending mails is required, use proper mail program.
>> + (add-after 'install 'remove-smtp
>> + (lambda _
>> + (delete-file (string-append #$output "/sbin/smtp"))))
>> + ;; The configuration files and scripts are not really suitable for
>> + ;; Guix, and our service provides its own version anyway. So nuke
>
> I'd use the more peaceful 'remove' or 'delete' instead of 'nuke'.

Uh, sure. Done.

Toggle quote (7 lines)
>
>> + ;; these to make sure `apcupsd' and `apctest' executed without any
>> + ;; arguments fail. `apctest' actually segfaults, but only after
>> + ;; printing an error ¯\_(?)_/¯.
>
> Please don't embed emojis in the source :-).

Technically kaomoji (???), not emoji (???), but point taken. :)

Toggle quote (8 lines)
>
>> + (add-after 'install 'remove-etc-apcupsd
>> + (lambda _
>> + (delete-file-recursively (string-append #$output "/etc/apcupsd")))))))
>
> Break this long line so it fits under 80 columns (our code style
> guideline).

I thought (well, still do), that going 4 characters over the limit was
worth the increase in readability. Nevertheless, I have split it into
two lines.

Toggle quote (5 lines)
>
>> + (home-page "https://www.apcupsd.org")
>
> I'd use 'http', since their TLS cert is now invalid.

Done as part of resolving the ling warning.

Toggle quote (5 lines)
>
>> + (synopsis "A daemon for controlling APC UPSes")
>
> s/A // (as hinted by 'guix lint').

Same.

Toggle quote (5 lines)
>
>> + (description "Apcupsd can be used for power mangement and controlling most
>
> s/mangement/management/

Nice catch, fixed.

Toggle quote (22 lines)
>
>> +of APC’s UPS models on Unix and Windows machines. Apcupsd works with most of
>> +APC’s Smart-UPS models as well as most simple signalling models such a Back-UPS,
>> +and BackUPS-Office.")
>> + (license license:gpl2)))
>
> I think it's actually license:gpl2+, according to
> apcupsd-3.14.14/COPYING, which says:
>
> Each version is given a distinguishing version number. If the Program
> specifies a version number of this License which applies to it and "any
> later version", you have the option of following the terms and conditions
> either of that version or of any later version published by the Free
> Software Foundation. If the Program does not specify a version number of
> this License, you may choose any version ever published by the Free Software
> Foundation.
>
>
> In this case for most file the last sentence applies, some other
> explicitly mention 'or any later version'. That's also supported by the
> debian/copyright file.

I do not think this is correct. When I check
apcupsd-3.14.14/src/apcupsd.c file, I see this in its header:

Toggle snippet (17 lines)
* Copyright (C) 1999-2005 Kern Sibbald
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of version 2 of the GNU General
* Public License as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* General Public License for more details.
*
* You should have received a copy of the GNU General Public
* License along with this program; if not, write to the Free
* Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
* MA 02110-1335, USA.

There is no mention of "any later version". Does that not make the
combined work GPL-2.0-only, even if other files would be under
GPL-2.0-or-later (I did no check whether they are)?

I did not find debian/copyright file in the source code.

Toggle quote (11 lines)
>
> Another thing found: the doc output doesn't build reproducibly, as
> shown by:
>
> $ ./pre-inst-env guix build --no-grafts --check -K
> guix build: error: derivation `/gnu/store/r62j72bd3an8k2fbmaiil5hma32syxdy-apcupsd-3.14.14.drv' may not be deterministic: output `/gnu/store/bhwkrgkqahgs5h8crcfyacvnwp7rj390-apcupsd-3.14.14-doc' differs from `/gnu/store/bhwkrgkqahgs5h8crcfyacvnwp7rj390-apcupsd-3.14.14-doc-check'
>
> Let's see why:
>
> $ diff -r /gnu/store/bhwkrgkqahgs5h8crcfyacvnwp7rj390-apcupsd-3.14.14-doc{,-check}

This is great, I did not know that it names the store item -check. Very
useful, thank you.

Toggle quote (11 lines)
> diff -r
> /gnu/store/bhwkrgkqahgs5h8crcfyacvnwp7rj390-apcupsd-3.14.14-doc/share/doc/apcupsd-3.14.14/manual.html
> /gnu/store/bhwkrgkqahgs5h8crcfyacvnwp7rj390-apcupsd-3.14.14-doc-check/share/doc/apcupsd-3.14.14/manual.html
> 376c376
> < <div class="line">February 5, 2025 06:54:22</div>
> ---
>> <div class="line">February 5, 2025 06:54:50</div>
>
> Ah, the classic date time stamp. You'll want to neuter it in the source
> or in the built html file (with a preference for the former).

Nice find, I have missed this. I have patched the source code of the
manual to not embed the date and time, seems to work now.

Toggle quote (3 lines)
>
> Could you please send a v2?

Definitely, I will go through your review for the service file as well,
and once I have all the answers (both here and there), will send v2.

Once more thanks for the review,
Tomas

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.
-----BEGIN PGP SIGNATURE-----

iQJCBAEBCgAsFiEEt4NJs4wUfTYpiGikL7/ufbZ/wakFAmeo04sOHH5Ad29sZnNk
ZW4uY3oACgkQL7/ufbZ/wakexA/9FXapBD7LTzTio9FuXl/LZfl0plqmid/KZEtn
vGP4MBKSCb/IpQaYi1XQuXQEtBzoLMDprJraelLfMVck5I5v0Sqfsi5pZSSUHcHr
ZzN2/flboVWyaol/Wee5zTFJUcAM8H2Pc61lNemwR8360Gm0qLIWXgIEiVw/k2I+
ViBbPXDo8zJJjTYT6L2GMFwVu2Rn6Ww9khd4U1mUy0IXxcu7rVgzoAJsA3He7Q1f
TY7Sx4j5ggVZFcoTAc6VIYxRDHb78eiTCTObbPnSCODE2cCsmrLvuoW7qy+GhMKP
ZkU/SLKoxx+H7xQ0Wp6QKPshUsAj8mKkK5LWtq0uT6LuLsY06HxxEihIynNSvwjc
8KA+zW7RzUqSEfne4V3xi6YXpyR396A7QmkT44Vy5Knd1zKaK2TMtJw38gYzVR/Y
ACLmO0MylPthoHQgyKsdWBQT4Ynmstetw5jAbLGgqvxMhW9Otwhzo4auSbX0+qXI
+EeXCQN8IzEIFWUFcKDm/+QqHyPtTDMcg/FjjYlbH16bJQzT0YX2pWWUt7xuyLlD
FtSsvw80F4mdCaN0upKWRasFoqLarqmwGc+TRfEBZST4CBQBOZ8g5WsI+0/dZuAV
FpJ2wcmoUhNkzeFA3uSUfsNCNsIEE9SjhIgI+IB8HaBtWv7l+4uumMyt3DOw/fPf
OZSCClI=
=ft5W
-----END PGP SIGNATURE-----

T
T
Tomas Volf wrote 33 hours ago
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)(address . 75528@debbugs.gnu.org)
87lduff5jn.fsf@wolfsden.cz
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Toggle quote (15 lines)
> Hi,
>
> Tomas Volf <~@wolfsden.cz> writes:
>
>> --- /dev/null
>> +++ b/gnu/packages/power.scm
>> @@ -0,0 +1,125 @@
>> +;;; Copyright (C) 2025 Tomas Volf <~@wolfsden.cz>
>> +;;; Copyright (C) 2023 Raven Hallsby <karl@hallsby.com>
>
> One last thing; I don't see Raven listed in a 'Co-authored-by:' git
> trailer in the commit message; should they? Or otherwise mention
> plainly this work was based on their previous work, made available
> '$where'.

I did not know about co-authored trailer. Should it be used even in
situations where I just took a look at his version online without any
active involvement? Currently I have modified the commit message to:

Toggle snippet (14 lines)
gnu: Add apcupsd.

Some parts were taken or inspired by the work of Raven Hallsby available
here[0]. For that reason I have added his copyright as well (I asked for the
permission).

0: https://raw.githubusercontent.com/KarlJoad/guix/9013b5ac3fadb48fad2e7ef1fbfaa4848dcb922a/gnu/packages/power.scm

* gnu/packages/power.scm (apcupsd): New variable.
* gnu/local.mk (GNU_SYSTEM_MODULES): Register the new file.

Change-Id: I0e4b2f50c8adf0f96d140e2be0f79e3740f4955c

Is that sufficient?

Tomas

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.
-----BEGIN PGP SIGNATURE-----

iQJCBAEBCgAsFiEEt4NJs4wUfTYpiGikL7/ufbZ/wakFAmeo1ywOHH5Ad29sZnNk
ZW4uY3oACgkQL7/ufbZ/wakakQ/+Icqd2dgQ/CeDbt0YUQW5ZOoGvxusDcC37Cir
RyMl6sw3w/G+g9Om9KVRHFAu6yDgrrukGpEuHXv3bD7YgwwYUkbiyUD26qeMFZt2
oj23ujzH/gPoyDJP01N2nuFDZx2x4c5E6K4K9Hx3Eoh8B3Ot5KKHhIngbaPAO9TH
dnEmRXnUH5eHAzEOl7QoY8WpV6gn7TQuxHOQdhM/Fc4ypbgmgq2w+Z0cYJSBXe3V
HDNeJhxKb06+KJvsRsOsMUM1pZdZo9SUCdPzjoxy0hk+mT5t39TTNFGzk88rcC7T
37VElKMiDb6SzqXiJ+Ijiltv0CfibcrbSOZQCrRI4MfIIxOn0K+xAfrwcEwTg1rI
nqIc87aKQ5H9qsEzKKWPcCaXefbNoAb3c6J5XVml9LN8h1+/Uvh3HbwXKw/i/F+1
QDbmiTcjauP5Sa2uK88pNMqoQG5bGJuSu/xqKTKHVlu1tzQRF8URiKNnQ4eMuwM5
ns/NaRZTYnKQWZlCi1LxBwZHbqa+TSAziLixhpuc7VWIDTuZiMrzXuiM/jK7q1Ko
kMNqBPPtvY1tNxl63d41hVR0Tx5lnCmT+uG0UQPNYpYQYRJO7MbkxXTDdPZfp0NT
A/n1YWABfMkK35ALxF5E3JicQ707o4u5T77YQ4k+j1p/S4t8h6DR8qvPZr74Utbq
SqKLf08=
=po+1
-----END PGP SIGNATURE-----

?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 75528@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 75528
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch