* [bug#58855] [PATCH 0/5] Update mcron to latest commit @ 2022-10-29 3:47 Maxim Cournoyer 2022-10-29 4:16 ` [bug#58855] [PATCH 1/5] services: configuration: Re-order generated record fields Maxim Cournoyer 0 siblings, 1 reply; 20+ messages in thread From: Maxim Cournoyer @ 2022-10-29 3:47 UTC (permalink / raw) To: 58855; +Cc: Maxim Cournoyer Hi, This update mcrons to its latest commit, which includes these changes: --8<---------------cut here---------------start------------->8--- 5fd0ccd * origin/master tests: Check (mcron vixie-specification) d1a3e83 * vixie-time: Remove calls to 'pk' debugging facility 19ba0a8 * Lose hope of running against guile 2.2 or earlier. e2ecb80 * Give mcron --log option to turn logging on. a7a456c * base: Annotate output with job information. 9e99490 * Revert "Minor cosmetic simplification of case logic after previous patch." d5c021e * documentation: extensive editing of info manual after a note from Paul Vixie. 99a26e5 * Minor cosmetic simplification of case logic after previous patch. 8b27157 * base: Handle nonexistent user home directories. 271b1f2 * Clarify an error message 172f70e * documentation: Bug fix in a simple example. --8<---------------cut here---------------end--------------->8--- The one "base: Annotate output with job information." one gives us better logging capability, such as when a job is started and when it finishes, and its completion status: --8<---------------cut here---------------start------------->8--- 2022-10-28 22:55:00 5185 duckdns-update: completed in 0.349s 2022-10-28 23:00:00 6547 duckdns-update: running... 2022-10-28 23:00:00 6548 btrbk: running... 2022-10-28 23:00:00 6547 duckdns-update: completed in 0.350s 2022-10-28 23:05:01 7764 duckdns-update: running... 2022-10-28 23:05:01 7764 duckdns-update: completed in 0.417s 2022-10-28 23:10:00 8315 duckdns-update: running... 2022-10-28 23:10:00 8315 duckdns-update: completed in 0.454s --8<---------------cut here---------------end--------------->8--- We now see the pid, job name and some message about the current status (running/completed/failed). I hope you like it! Maxim Cournoyer (5): services: configuration: Re-order generated record fields. services: mcron: Add log? and log-format fields to mcron-configuration. gnu: mcron: Use gexps and strip trailing #t. gnu: Remove guile2.2-mcron. gnu: mcron: Update to 1.2.1-0.5fd0ccd. doc/guix.texi | 45 +++++++++++++---- gnu/home/services/mcron.scm | 47 +++++++++++++---- gnu/packages/guile-xyz.scm | 92 +++++++++++++++++----------------- gnu/services/configuration.scm | 10 ++-- gnu/services/mcron.scm | 47 +++++++++++++---- gnu/services/monitoring.scm | 4 +- 6 files changed, 162 insertions(+), 83 deletions(-) -- 2.37.3 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [bug#58855] [PATCH 1/5] services: configuration: Re-order generated record fields. 2022-10-29 3:47 [bug#58855] [PATCH 0/5] Update mcron to latest commit Maxim Cournoyer @ 2022-10-29 4:16 ` Maxim Cournoyer 2022-10-29 4:16 ` [bug#58855] [PATCH 2/5] services: mcron: Add log? and log-format fields to mcron-configuration Maxim Cournoyer ` (4 more replies) 0 siblings, 5 replies; 20+ messages in thread From: Maxim Cournoyer @ 2022-10-29 4:16 UTC (permalink / raw) To: 58855; +Cc: Maxim Cournoyer This is so that the first field of the generated record matches the first one declared, which makes 'define-configuration' record API compatible with define-record-type* ones. * gnu/services/configuration.scm (define-configuration-helper): Move the %location field below the ones declared by the user. * gnu/services/monitoring.scm (zabbix-front-end-config): Adjust match pattern accordingly. --- gnu/services/configuration.scm | 10 +++++----- gnu/services/monitoring.scm | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm index 636c49ccba..dacfc52ba9 100644 --- a/gnu/services/configuration.scm +++ b/gnu/services/configuration.scm @@ -242,17 +242,17 @@ (define-record-type* #,(id #'stem #'< #'stem #'>) stem #,(id #'stem #'make- #'stem) #,(id #'stem #'stem #'?) - (%location #,(id #'stem #'stem #'-location) - (default (and=> (current-source-location) - source-properties->location)) - (innate)) #,@(map (lambda (name getter def) #`(#,name #,getter (default #,def) (sanitize #,(id #'stem #'validate- #'stem #'- name)))) #'(field ...) #'(field-getter ...) - #'(field-default ...))) + #'(field-default ...)) + (%location #,(id #'stem #'stem #'-location) + (default (and=> (current-source-location) + source-properties->location)) + (innate))) (define #,(id #'stem #'stem #'-fields) (list (configuration-field diff --git a/gnu/services/monitoring.scm b/gnu/services/monitoring.scm index 9c8704092c..b19c6c9f18 100644 --- a/gnu/services/monitoring.scm +++ b/gnu/services/monitoring.scm @@ -622,8 +622,8 @@ (define-configuration zabbix-front-end-configuration (define (zabbix-front-end-config config) (match-record config <zabbix-front-end-configuration> - (%location db-host db-port db-name db-user db-password db-secret-file - zabbix-host zabbix-port) + (db-host db-port db-name db-user db-password db-secret-file + zabbix-host zabbix-port %location) (mixed-text-file "zabbix.conf.php" "\ <?php -- 2.37.3 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [bug#58855] [PATCH 2/5] services: mcron: Add log? and log-format fields to mcron-configuration. 2022-10-29 4:16 ` [bug#58855] [PATCH 1/5] services: configuration: Re-order generated record fields Maxim Cournoyer @ 2022-10-29 4:16 ` Maxim Cournoyer 2022-10-29 4:16 ` [bug#58855] [PATCH 3/5] gnu: mcron: Use gexps and strip trailing #t Maxim Cournoyer ` (3 subsequent siblings) 4 siblings, 0 replies; 20+ messages in thread From: Maxim Cournoyer @ 2022-10-29 4:16 UTC (permalink / raw) To: 58855; +Cc: Maxim Cournoyer * gnu/services/mcron.scm (list-of-gexps?): New predicate. (mcron-configuration): Rewrite using define-configuration. [log?, log-format]: New fields. (mcron-shepherd-services): Invoke mcron with the --log and --log-format arguments when log? is #t, (generate-doc): New procedure. * doc/guix.texi (Scheduled Job Execution): Update doc. (Mcron Home Service): Likewise. * gnu/home/services/mcron.scm: Keep in sync with the above changes to gnu/services/mcron.scm. --- doc/guix.texi | 45 ++++++++++++++++++++++++++++------- gnu/home/services/mcron.scm | 47 ++++++++++++++++++++++++++++--------- gnu/services/mcron.scm | 47 ++++++++++++++++++++++++++++--------- 3 files changed, 108 insertions(+), 31 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index 80fb3bc47f..aadd16dd53 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -18994,20 +18994,33 @@ words, it is possible to define services that provide additional mcron jobs to run. @end defvr +@c Generated via (generate-documentation) at the bottom of (gnu services +@c mcron). +@c %start of fragment @deftp {Data Type} mcron-configuration -Data type representing the configuration of mcron. +Available @code{mcron-configuration} fields are: @table @asis -@item @code{mcron} (default: @var{mcron}) +@item @code{mcron} (default: @code{mcron}) (type: file-like) The mcron package to use. -@item @code{jobs} +@item @code{jobs} (default: @code{()}) (type: list-of-gexps) This is a list of gexps (@pxref{G-Expressions}), where each gexp corresponds to an mcron job specification (@pxref{Syntax, mcron job -specifications,, mcron, GNU@tie{}mcron}). +specifications,, mcron,GNU@tie{}mcron}). + +@item @code{log?} (default: @code{#t}) (type: boolean) +Log messages to standard output. + +@item @code{log-format} (default: @code{"~1@@*~a ~a: ~a~%"}) (type: string) +@code{(ice-9 format)} format string for log messages. The default value +produces messages like "@samp{@var{pid} @var{name}: @var{message}"} +(@pxref{Invoking mcron, Invoking,, mcron,GNU@tie{}mcron}). Each message +is also prefixed by a timestamp by GNU Shepherd. + @end table @end deftp - +@c %end of fragment @node Log Rotation @subsection Log Rotation @@ -41015,18 +41028,32 @@ jobs to run. @end defvr @deftp {Data Type} home-mcron-configuration -Data type representing the configuration of mcron. +Available @code{home-mcron-configuration} fields are: +@c Auto-generated with (gnu home services mcron)'s +@c generate-documentation procedure. +@c %start of fragment @table @asis -@item @code{mcron} (default: @var{mcron}) +@item @code{mcron} (default: @code{mcron}) (type: file-like) The mcron package to use. -@item @code{jobs} +@item @code{jobs} (default: @code{()}) (type: list-of-gexps) This is a list of gexps (@pxref{G-Expressions}), where each gexp corresponds to an mcron job specification (@pxref{Syntax, mcron job -specifications,, mcron, GNU@tie{}mcron}). +specifications,, mcron,GNU@tie{}mcron}). + +@item @code{log?} (default: @code{#t}) (type: boolean) +Log messages to standard output. + +@item @code{log-format} (default: @code{"~1@@*~a ~a: ~a~%"}) (type: string) +@code{(ice-9 format)} format string for log messages. The default value +produces messages like "@samp{@var{pid} @var{name}: @var{message}"} +(@pxref{Invoking mcron, Invoking,, mcron,GNU@tie{}mcron}). Each message +is also prefixed by a timestamp by GNU Shepherd. + @end table @end deftp +@c %end of fragment @node Power Management Home Services @subsection Power Management Home Services diff --git a/gnu/home/services/mcron.scm b/gnu/home/services/mcron.scm index 0b3dbb810b..1d294a997c 100644 --- a/gnu/home/services/mcron.scm +++ b/gnu/home/services/mcron.scm @@ -1,6 +1,7 @@ ;;; GNU Guix --- Functional package management for GNU ;;; Copyright © 2021 Andrew Tropin <andrew@trop.in> ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz> +;;; Copyright © 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com> ;;; ;;; This file is part of GNU Guix. ;;; @@ -20,6 +21,7 @@ (define-module (gnu home services mcron) #:use-module (gnu packages guile-xyz) #:use-module (gnu home services) + #:use-module (gnu services configuration) #:use-module (gnu services shepherd) #:use-module (gnu home services shepherd) #:use-module (guix records) @@ -53,13 +55,23 @@ (define-module (gnu home services mcron) ;; ;;; Code: -(define-record-type* <home-mcron-configuration> home-mcron-configuration - make-home-mcron-configuration - home-mcron-configuration? - (package home-mcron-configuration-package ; package - (default mcron)) - (jobs home-mcron-configuration-jobs ; list of jobs - (default '()))) +(define list-of-gexps? + (list-of gexp?)) + +(define-configuration/no-serialization home-mcron-configuration + (mcron (file-like mcron) "The mcron package to use.") + (jobs + (list-of-gexps '()) + "This is a list of gexps (@pxref{G-Expressions}), where each gexp +corresponds to an mcron job specification (@pxref{Syntax, mcron job +specifications,, mcron, GNU@tie{}mcron}).") + (log? (boolean #t) "Log messages to standard output.") + (log-format + (string "~1@*~a ~a: ~a~%") + "@code{(ice-9 format)} format string for log messages. The default value +produces messages like \"@samp{@var{pid} @var{name}: +@var{message}\"} (@pxref{Invoking mcron, Invoking,, mcron, GNU@tie{}mcron}). +Each message is also prefixed by a timestamp by GNU Shepherd.")) (define job-files (@@ (gnu services mcron) job-files)) (define shepherd-schedule-action @@ -69,19 +81,23 @@ (define home-mcron-shepherd-services (match-lambda (($ <home-mcron-configuration> mcron '()) ; no jobs to run '()) - (($ <home-mcron-configuration> mcron jobs) + (($ <home-mcron-configuration> mcron jobs log? log-format) (let ((files (job-files mcron jobs))) (list (shepherd-service (documentation "User cron jobs.") (provision '(mcron)) (modules `((srfi srfi-1) (srfi srfi-26) - (ice-9 popen) ; for the 'schedule' action + (ice-9 popen) ; for the 'schedule' action (ice-9 rdelim) (ice-9 match) ,@%default-modules)) (start #~(make-forkexec-constructor - (list #$(file-append mcron "/bin/mcron") #$@files) + (list (string-append #$mcron "/bin/mcron") + #$@(if log? + #~("--log" "--log-format" #$log-format) + #~()) + #$@files) #:log-file (string-append (or (getenv "XDG_LOG_HOME") (format #f "~a/.local/var/log" @@ -91,7 +107,7 @@ (define home-mcron-shepherd-services (actions (list (shepherd-schedule-action mcron files))))))))) -(define home-mcron-profile (compose list home-mcron-configuration-package)) +(define home-mcron-profile (compose list home-mcron-configuration-mcron)) (define (home-mcron-extend config jobs) (home-mcron-configuration @@ -113,3 +129,12 @@ (define home-mcron-service-type (default-value (home-mcron-configuration)) (description "Install and configure the GNU mcron cron job manager."))) + +\f +;;; +;;; Generate documentation. +;;; +(define (generate-doc) + (configuration->documentation 'home-mcron-configuration)) + +;;; mcron.scm ends here diff --git a/gnu/services/mcron.scm b/gnu/services/mcron.scm index 23760ebda4..52332d6123 100644 --- a/gnu/services/mcron.scm +++ b/gnu/services/mcron.scm @@ -1,5 +1,6 @@ ;;; GNU Guix --- Functional package management for GNU ;;; Copyright © 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org> +;;; Copyright © 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com> ;;; ;;; This file is part of GNU Guix. ;;; @@ -18,6 +19,7 @@ (define-module (gnu services mcron) #:use-module (gnu services) + #:use-module (gnu services configuration) #:use-module (gnu services shepherd) #:use-module (gnu packages guile-xyz) #:use-module (guix deprecation) @@ -30,6 +32,8 @@ (define-module (gnu services mcron) mcron-configuration? mcron-configuration-mcron mcron-configuration-jobs + mcron-configuration-log? + mcron-configuration-log-format mcron-service-type)) @@ -48,13 +52,23 @@ (define-module (gnu services mcron) ;;; ;;; Code: -(define-record-type* <mcron-configuration> mcron-configuration - make-mcron-configuration - mcron-configuration? - (mcron mcron-configuration-mcron ;file-like - (default mcron)) - (jobs mcron-configuration-jobs ;list of <mcron-job> - (default '()))) +(define list-of-gexps? + (list-of gexp?)) + +(define-configuration/no-serialization mcron-configuration + (mcron (file-like mcron) "The mcron package to use.") + (jobs + (list-of-gexps '()) + "This is a list of gexps (@pxref{G-Expressions}), where each gexp +corresponds to an mcron job specification (@pxref{Syntax, mcron job +specifications,, mcron, GNU@tie{}mcron}).") + (log? (boolean #t) "Log messages to standard output.") + (log-format + (string "~1@*~a ~a: ~a~%") + "@code{(ice-9 format)} format string for log messages. The default value +produces messages like \"@samp{@var{pid} @var{name}: +@var{message}\"} (@pxref{Invoking mcron, Invoking,, mcron, GNU@tie{}mcron}). +Each message is also prefixed by a timestamp by GNU Shepherd.")) (define (job-files mcron jobs) "Return a list of file-like object for JOBS, a list of gexps." @@ -124,21 +138,25 @@ (define (shepherd-schedule-action mcron files) (define mcron-shepherd-services (match-lambda - (($ <mcron-configuration> mcron ()) ;nothing to do! + (($ <mcron-configuration> mcron ()) ;nothing to do! '()) - (($ <mcron-configuration> mcron jobs) + (($ <mcron-configuration> mcron jobs log? log-format) (let ((files (job-files mcron jobs))) (list (shepherd-service (provision '(mcron)) (requirement '(user-processes)) (modules `((srfi srfi-1) (srfi srfi-26) - (ice-9 popen) ;for the 'schedule' action + (ice-9 popen) ;for the 'schedule' action (ice-9 rdelim) (ice-9 match) ,@%default-modules)) (start #~(make-forkexec-constructor - (list (string-append #$mcron "/bin/mcron") #$@files) + (list (string-append #$mcron "/bin/mcron") + #$@(if log? + #~("--log" "--log-format" #$log-format) + #~()) + #$@files) ;; Disable auto-compilation of the job files and set a ;; sane value for 'PATH'. @@ -172,4 +190,11 @@ (define mcron-service-type jobs))))) (default-value (mcron-configuration)))) ;empty job list +\f +;;; +;;; Generate documentation. +;;; +(define (generate-doc) + (configuration->documentation 'mcron-configuration)) + ;;; mcron.scm ends here -- 2.37.3 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [bug#58855] [PATCH 3/5] gnu: mcron: Use gexps and strip trailing #t. 2022-10-29 4:16 ` [bug#58855] [PATCH 1/5] services: configuration: Re-order generated record fields Maxim Cournoyer 2022-10-29 4:16 ` [bug#58855] [PATCH 2/5] services: mcron: Add log? and log-format fields to mcron-configuration Maxim Cournoyer @ 2022-10-29 4:16 ` Maxim Cournoyer 2022-10-29 4:16 ` [bug#58855] [PATCH 4/5] gnu: Remove guile2.2-mcron Maxim Cournoyer ` (2 subsequent siblings) 4 siblings, 0 replies; 20+ messages in thread From: Maxim Cournoyer @ 2022-10-29 4:16 UTC (permalink / raw) To: 58855; +Cc: Maxim Cournoyer * gnu/packages/guile-xyz.scm (mcron) [phases]: Use gexps and strip trailing #t. --- gnu/packages/guile-xyz.scm | 41 +++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/gnu/packages/guile-xyz.scm b/gnu/packages/guile-xyz.scm index 209ba694d7..1b41eb24af 100644 --- a/gnu/packages/guile-xyz.scm +++ b/gnu/packages/guile-xyz.scm @@ -2667,27 +2667,26 @@ (define-public mcron "0bkn235g2ia4f7ispr9d55c7bc18282r3qd8ldhh5q2kiin75zi0")))) (build-system gnu-build-system) (arguments - '(#:phases (modify-phases %standard-phases - (add-before 'check 'adjust-tests - (lambda _ - (substitute* "tests/job-specifier.scm" - ;; (getpw) fails with "entry not found" in the build - ;; environment, so pass an argument. - (("\\(getpw\\)") - "(getpwnam (getuid))") - ;; The build environment lacks an entry for root in - ;; /etc/passwd. - (("\\(getpw 0\\)") - "(getpwnam \"nobody\")") - - ;; FIXME: Skip the 4 faulty tests (see above). - (("\\(test-equal \"next-year\"" all) - (string-append "(test-skip 4)\n" all))) - #t))))) - (native-inputs `(("pkg-config" ,pkg-config) - ("tzdata" ,tzdata-for-tests) - ("guile-native" ;for 'guild compile' - ,@(assoc-ref (package-inputs this-package) "guile")))) + (list + #:phases #~(modify-phases %standard-phases + (add-before 'check 'adjust-tests + (lambda _ + (substitute* "tests/job-specifier.scm" + ;; (getpw) fails with "entry not found" in the build + ;; environment, so pass an argument. + (("\\(getpw\\)") + "(getpwnam (getuid))") + ;; The build environment lacks an entry for root in + ;; /etc/passwd. + (("\\(getpw 0\\)") + "(getpwnam \"nobody\")") + ;; FIXME: Skip the 4 faulty tests (see above). + (("\\(test-equal \"next-year\"" all) + (string-append "(test-skip 4)\n" all)))))))) + (native-inputs (list guile-3.0 ;for 'guild compile' + pkg-config + tzdata-for-tests)) + (inputs (list guile-3.0)) (home-page "https://www.gnu.org/software/mcron/") (synopsis "Run jobs at scheduled times") -- 2.37.3 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [bug#58855] [PATCH 4/5] gnu: Remove guile2.2-mcron. 2022-10-29 4:16 ` [bug#58855] [PATCH 1/5] services: configuration: Re-order generated record fields Maxim Cournoyer 2022-10-29 4:16 ` [bug#58855] [PATCH 2/5] services: mcron: Add log? and log-format fields to mcron-configuration Maxim Cournoyer 2022-10-29 4:16 ` [bug#58855] [PATCH 3/5] gnu: mcron: Use gexps and strip trailing #t Maxim Cournoyer @ 2022-10-29 4:16 ` Maxim Cournoyer 2022-10-29 4:16 ` [bug#58855] [PATCH 5/5] gnu: mcron: Update to 1.2.1-0.5fd0ccd Maxim Cournoyer 2022-11-17 22:37 ` Layout of ‘define-configuration’ records Ludovic Courtès 4 siblings, 0 replies; 20+ messages in thread From: Maxim Cournoyer @ 2022-10-29 4:16 UTC (permalink / raw) To: 58855; +Cc: Maxim Cournoyer * gnu/packages/guile-xyz.scm (guile2.2-mcron): Delete variable. --- gnu/packages/guile-xyz.scm | 6 ------ 1 file changed, 6 deletions(-) diff --git a/gnu/packages/guile-xyz.scm b/gnu/packages/guile-xyz.scm index 1b41eb24af..41f93df155 100644 --- a/gnu/packages/guile-xyz.scm +++ b/gnu/packages/guile-xyz.scm @@ -2697,12 +2697,6 @@ (define-public mcron format is also supported.") (license license:gpl3+))) -(define-public guile2.2-mcron - (package - (inherit mcron) - (name "guile2.2-mcron") - (inputs (list guile-2.2)))) - (define-public guile-picture-language (let ((commit "a1322bf11945465241ca5b742a70893f24156d12") (revision "5")) -- 2.37.3 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [bug#58855] [PATCH 5/5] gnu: mcron: Update to 1.2.1-0.5fd0ccd. 2022-10-29 4:16 ` [bug#58855] [PATCH 1/5] services: configuration: Re-order generated record fields Maxim Cournoyer ` (2 preceding siblings ...) 2022-10-29 4:16 ` [bug#58855] [PATCH 4/5] gnu: Remove guile2.2-mcron Maxim Cournoyer @ 2022-10-29 4:16 ` Maxim Cournoyer 2022-11-17 22:37 ` Layout of ‘define-configuration’ records Ludovic Courtès 4 siblings, 0 replies; 20+ messages in thread From: Maxim Cournoyer @ 2022-10-29 4:16 UTC (permalink / raw) To: 58855; +Cc: Maxim Cournoyer * gnu/packages/guile-xyz.scm (mcron): Update to 1.2.1-0.5fd0ccd. [native-inputs]: Add autoconf, automake, help2man, and texinfo. --- gnu/packages/guile-xyz.scm | 85 +++++++++++++++++++++----------------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/gnu/packages/guile-xyz.scm b/gnu/packages/guile-xyz.scm index 41f93df155..fd2a26d3a5 100644 --- a/gnu/packages/guile-xyz.scm +++ b/gnu/packages/guile-xyz.scm @@ -2655,47 +2655,56 @@ (define-public guile2.0-commonmark (inputs (list guile-2.0)))) (define-public mcron - (package - (name "mcron") - (version "1.2.1") - (source (origin - (method url-fetch) - (uri (string-append "mirror://gnu/mcron/mcron-" - version ".tar.gz")) - (sha256 - (base32 - "0bkn235g2ia4f7ispr9d55c7bc18282r3qd8ldhh5q2kiin75zi0")))) - (build-system gnu-build-system) - (arguments - (list - #:phases #~(modify-phases %standard-phases - (add-before 'check 'adjust-tests - (lambda _ - (substitute* "tests/job-specifier.scm" - ;; (getpw) fails with "entry not found" in the build - ;; environment, so pass an argument. - (("\\(getpw\\)") - "(getpwnam (getuid))") - ;; The build environment lacks an entry for root in - ;; /etc/passwd. - (("\\(getpw 0\\)") - "(getpwnam \"nobody\")") - ;; FIXME: Skip the 4 faulty tests (see above). - (("\\(test-equal \"next-year\"" all) - (string-append "(test-skip 4)\n" all)))))))) - (native-inputs (list guile-3.0 ;for 'guild compile' - pkg-config - tzdata-for-tests)) - - (inputs (list guile-3.0)) - (home-page "https://www.gnu.org/software/mcron/") - (synopsis "Run jobs at scheduled times") - (description - "GNU Mcron is a complete replacement for Vixie cron. It is used to run + ;; Use the latest commits, as interesting changes haven't been released yet, + ;; such as improved logging. + (let ((revision "0") + (commit "5fd0ccde5a4cff70299999f988e6b5166584814d")) + (package + (name "mcron") + (version (git-version "1.2.1" revision commit)) + (source (origin + (method git-fetch) + (uri (git-reference + (url "https://git.savannah.gnu.org/git/mcron.git") + (commit commit))) + (file-name (git-file-name name version)) + (sha256 + (base32 + "0jl2w67a5hkphzssdzq3q4jcwv2b174b11d3w5i3khxq2vhzd6kk")))) + (build-system gnu-build-system) + (arguments + (list + #:phases #~(modify-phases %standard-phases + (add-before 'check 'adjust-tests + (lambda _ + (substitute* "tests/job-specifier.scm" + ;; (getpw) fails with "entry not found" in the build + ;; environment, so pass an argument. + (("\\(getpw\\)") + "(getpwnam (getuid))") + ;; The build environment lacks an entry for root in + ;; /etc/passwd. + (("\\(getpw 0\\)") + "(getpwnam \"nobody\")") + ;; FIXME: Skip the 4 faulty tests (see above). + (("\\(test-equal \"next-year\"" all) + (string-append "(test-skip 4)\n" all)))))))) + (native-inputs (list autoconf + automake + guile-3.0 ;for 'guild compile' + help2man + pkg-config + tzdata-for-tests + texinfo)) + (inputs (list guile-3.0)) + (home-page "https://www.gnu.org/software/mcron/") + (synopsis "Run jobs at scheduled times") + (description + "GNU Mcron is a complete replacement for Vixie cron. It is used to run tasks on a schedule, such as every hour or every Monday. Mcron is written in Guile, so its configuration can be written in Scheme; the original cron format is also supported.") - (license license:gpl3+))) + (license license:gpl3+)))) (define-public guile-picture-language (let ((commit "a1322bf11945465241ca5b742a70893f24156d12") -- 2.37.3 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Layout of ‘define-configuration’ records 2022-10-29 4:16 ` [bug#58855] [PATCH 1/5] services: configuration: Re-order generated record fields Maxim Cournoyer ` (3 preceding siblings ...) 2022-10-29 4:16 ` [bug#58855] [PATCH 5/5] gnu: mcron: Update to 1.2.1-0.5fd0ccd Maxim Cournoyer @ 2022-11-17 22:37 ` Ludovic Courtès 2022-11-18 16:44 ` Maxim Cournoyer 4 siblings, 1 reply; 20+ messages in thread From: Ludovic Courtès @ 2022-11-17 22:37 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: 58855, guix-devel Hi, Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > This is so that the first field of the generated record matches the first one > declared, which makes 'define-configuration' record API compatible with > define-record-type* ones. > > * gnu/services/configuration.scm (define-configuration-helper): Move the > %location field below the ones declared by the user. > * gnu/services/monitoring.scm (zabbix-front-end-config): Adjust match pattern > accordingly. [...] > +++ b/gnu/services/configuration.scm > @@ -242,17 +242,17 @@ (define-record-type* #,(id #'stem #'< #'stem #'>) > stem > #,(id #'stem #'make- #'stem) > #,(id #'stem #'stem #'?) > - (%location #,(id #'stem #'stem #'-location) > - (default (and=> (current-source-location) > - source-properties->location)) > - (innate)) > #,@(map (lambda (name getter def) > #`(#,name #,getter (default #,def) > (sanitize > #,(id #'stem #'validate- #'stem #'- name)))) > #'(field ...) > #'(field-getter ...) > - #'(field-default ...))) > + #'(field-default ...)) > + (%location #,(id #'stem #'stem #'-location) > + (default (and=> (current-source-location) > + source-properties->location)) > + (innate))) Moving the field last is problematic as we’ve seen for any user that uses ‘match’ on records—something that’s not recommended but still used a lot. > (define (zabbix-front-end-config config) > (match-record config <zabbix-front-end-configuration> > - (%location db-host db-port db-name db-user db-password db-secret-file > - zabbix-host zabbix-port) > + (db-host db-port db-name db-user db-password db-secret-file > + zabbix-host zabbix-port %location) This change has no effect because ‘match-record’ matches fields by name, precisely to avoid problems when changing the layout of record types. I’m not sure what was meant by “compatible” in the commit log; how fields are laid out is something user code should not be aware of. The only thing to keep in mind is that records must not be matched with ‘match’ because then the code silently breaks when the record type layout is changed. This is why ‘match-record’ was introduced: https://issues.guix.gnu.org/28960#4 One last thing: placing ‘%location’ first can let us implement: (define (configuration-location config) (struct-ref config 0)) As if there were type inheritance. I didn’t see any indication that this is actually done anywhere, but it could have been the case (it’s not an unusual pattern) and it’s still something we might want to do. Does that make sense? Thanks, Ludo’. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Layout of ‘define-configuration’ records 2022-11-17 22:37 ` Layout of ‘define-configuration’ records Ludovic Courtès @ 2022-11-18 16:44 ` Maxim Cournoyer 2022-11-19 21:25 ` Katherine Cox-Buday 2022-11-21 10:22 ` Ludovic Courtès 0 siblings, 2 replies; 20+ messages in thread From: Maxim Cournoyer @ 2022-11-18 16:44 UTC (permalink / raw) To: guix-devel Hi Ludo, Ludovic Courtès <ludo@gnu.org> writes: > Hi, > > Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > >> This is so that the first field of the generated record matches the first one >> declared, which makes 'define-configuration' record API compatible with >> define-record-type* ones. >> >> * gnu/services/configuration.scm (define-configuration-helper): Move the >> %location field below the ones declared by the user. >> * gnu/services/monitoring.scm (zabbix-front-end-config): Adjust match pattern >> accordingly. > > [...] > >> +++ b/gnu/services/configuration.scm >> @@ -242,17 +242,17 @@ (define-record-type* #,(id #'stem #'< #'stem #'>) >> stem >> #,(id #'stem #'make- #'stem) >> #,(id #'stem #'stem #'?) >> - (%location #,(id #'stem #'stem #'-location) >> - (default (and=> (current-source-location) >> - source-properties->location)) >> - (innate)) >> #,@(map (lambda (name getter def) >> #`(#,name #,getter (default #,def) >> (sanitize >> #,(id #'stem #'validate- #'stem #'- name)))) >> #'(field ...) >> #'(field-getter ...) >> - #'(field-default ...))) >> + #'(field-default ...)) >> + (%location #,(id #'stem #'stem #'-location) >> + (default (and=> (current-source-location) >> + source-properties->location)) >> + (innate))) > > Moving the field last is problematic as we’ve seen for any user that > uses ‘match’ on records—something that’s not recommended but still used > a lot. Yep. I had that on mind when I made the change, though I still missed a few occurrences. >> (define (zabbix-front-end-config config) >> (match-record config <zabbix-front-end-configuration> >> - (%location db-host db-port db-name db-user db-password db-secret-file >> - zabbix-host zabbix-port) >> + (db-host db-port db-name db-user db-password db-secret-file >> + zabbix-host zabbix-port %location) > > This change has no effect because ‘match-record’ matches fields by name, > precisely to avoid problems when changing the layout of record types. Good catch; I got confused there, although my confusion luckily had no side effect :-). > I’m not sure what was meant by “compatible” in the commit log; how > fields are laid out is something user code should not be aware of. I wanted match on define-configuration'd fields to be backward-compatible with fields migrated from define-record-type*, so that they such change can be made without worrying breakages. It seems good for consistency that both our define-record-type* and define-configuration fields share a similar internal layout, at least until all the old-fashion (ice-9 match) record matching usages are rewritten to use something like 'match-record'. I initially got tricked by that discrepancy and it took me quite some time hunting down a cryptic backtrace when authoring the new mcron configuration records. > The only thing to keep in mind is that records must not be matched with > ‘match’ because then the code silently breaks when the record type > layout is changed. This is why ‘match-record’ was introduced: > > https://issues.guix.gnu.org/28960#4 > > One last thing: placing ‘%location’ first can let us implement: > > (define (configuration-location config) > (struct-ref config 0)) Would this have worked? --8<---------------cut here---------------start------------->8--- scheme@(gnu services mcron)> (define config (mcron-configuration)) scheme@(gnu services mcron)> (struct-ref 0 config) ice-9/boot-9.scm:1685:16: In procedure raise-exception: In procedure struct-ref: Wrong type argument in position 1 (expecting struct): 0 Entering a new prompt. Type `,bt' for a backtrace or `,q' to continue. scheme@(gnu services mcron) [1]> ,q scheme@(gnu services mcron)> ,use (oop goops) scheme@(gnu services mcron)> (class-of config) $5 = #<<class> <<mcron-configuration>> 7fe20fd83e00> --8<---------------cut here---------------end--------------->8--- All in all, I think that's a rather small change that got our internal implementation of both type of records in sync between define-configuration and define-record-type*, that should pave the way for migrating more of the later to the former without risking breaking something, going forward. Alternatively, if we have a good reason to not to go with this, I think we should abstract all the internal-implementation dependent code from our code base (e.g., the match patterns directly matching on field slots). What do you think? -- Thanks, Maxim ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Layout of ‘define-configuration’ records 2022-11-18 16:44 ` Maxim Cournoyer @ 2022-11-19 21:25 ` Katherine Cox-Buday 2022-11-20 13:47 ` Maxim Cournoyer 2022-11-21 10:22 ` Ludovic Courtès 1 sibling, 1 reply; 20+ messages in thread From: Katherine Cox-Buday @ 2022-11-19 21:25 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: guix-devel Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: >> Moving the field last is problematic as we’ve seen for any user that >> uses ‘match’ on records—something that’s not recommended but still >> used a lot. Some feedback I hope is useful: I'm one such newbie, and had to stumble my way into discovering why some of my services suddenly broke. All I had to go on was with "invalid G-expression input" at a location in an `operating-system` declaration. Initially, because of the reference to gexps, I thought something had changed with `local-file` which I am using to deploy source code off of a local branch. Through trial and error and reading git logs, I discovered this was not the case, and saw that `define-configuration` had changed. From there I was able to discover the problem. > Yep. I had that on mind when I made the change, though I still missed > a few occurrences. I came looking for an announcement of this change somewhere on a mailing list (info-guix maybe?) or the Guix blog but couldn't find anything. I understand the focus is on in-repo code, but could we consider announcing changes which might affect channels and such? I don't always have time to stay up to date with the changes to the project :( -- Katherine ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Layout of ‘define-configuration’ records 2022-11-19 21:25 ` Katherine Cox-Buday @ 2022-11-20 13:47 ` Maxim Cournoyer 2022-11-21 10:26 ` Ludovic Courtès 2022-11-21 16:49 ` Katherine Cox-Buday 0 siblings, 2 replies; 20+ messages in thread From: Maxim Cournoyer @ 2022-11-20 13:47 UTC (permalink / raw) To: Katherine Cox-Buday; +Cc: guix-devel Hi Katherine, Katherine Cox-Buday <cox.katherine.e@gmail.com> writes: > Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: > >>> Moving the field last is problematic as we’ve seen for any user that >>> uses ‘match’ on records—something that’s not recommended but still >>> used a lot. > > Some feedback I hope is useful: > > I'm one such newbie, and had to stumble my way into discovering why some > of my services suddenly broke. All I had to go on was with "invalid > G-expression input" at a location in an `operating-system` declaration. > > Initially, because of the reference to gexps, I thought something had > changed with `local-file` which I am using to deploy source code off of > a local branch. Through trial and error and reading git logs, I > discovered this was not the case, and saw that `define-configuration` > had changed. From there I was able to discover the problem. > >> Yep. I had that on mind when I made the change, though I still missed >> a few occurrences. > > I came looking for an announcement of this change somewhere on a mailing > list (info-guix maybe?) or the Guix blog but couldn't find anything. I > understand the focus is on in-repo code, but could we consider > announcing changes which might affect channels and such? I don't always > have time to stay up to date with the changes to the project :( Apologies for causing grief! Ironically, the change was motivated by going through such pain myself when attempting to migrate the mcron-configuration from a define-record-type* to a define-configuration produced record, that's why I wanted to standardize their layout. I'm taking yours and Ludovic's feedback into account for the future and will reach out to guix-devel with heads-up when introducing breaking changes to Guix APIs. A side-note, it seems that Ludovic has been working toward eliminating the use of match patterns matching the fields directly, instead encouraging the use of 'match-record', see https://issues.guix.gnu.org/59390. I haven't checked if this is compatible with define-configuration records though. -- Thanks, Maxim ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Layout of ‘define-configuration’ records 2022-11-20 13:47 ` Maxim Cournoyer @ 2022-11-21 10:26 ` Ludovic Courtès 2022-11-21 20:08 ` Maxim Cournoyer 2022-11-21 16:49 ` Katherine Cox-Buday 1 sibling, 1 reply; 20+ messages in thread From: Ludovic Courtès @ 2022-11-21 10:26 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: Katherine Cox-Buday, guix-devel Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > A side-note, it seems that Ludovic has been > working toward eliminating the use of match patterns matching the fields > directly, instead encouraging the use of 'match-record', see > https://issues.guix.gnu.org/59390. I haven't checked if this is > compatible with define-configuration records though. It is: ‘define-configuration’ builds on ‘define-record-type*’, which builds on SRFI-9, which builds on Guile “records”, which builds on Guile “structs”. :-) --8<---------------cut here---------------start------------->8--- scheme@(guile-user)> ,m (gnu home services desktop) scheme@(gnu home services desktop)> (home-redshift-configuration) $1 = #<<home-redshift-configuration> redshift: #<package redshift@1.12 gnu/packages/xdisorg.scm:1430 7f155c3ae210> location-provider: geoclue2 adjustment-method: randr daytime-temperature: 6500 nighttime-temperature: 4500 daytime-brightness: %unset-marker% nighttime-brightness: %unset-marker% latitude: %unset-marker% longitude: %unset-marker% dawn-time: %unset-marker% dusk-time: %unset-marker% extra-content: "" %location: #f> scheme@(gnu home services desktop)> (match-record $1 <home-redshift-configuration> (adjustment-method nighttime-temperature) ... (list nighttime-temperature adjustment-method)) $2 = (4500 randr) --8<---------------cut here---------------end--------------->8--- Ludo’. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Layout of ‘define-configuration’ records 2022-11-21 10:26 ` Ludovic Courtès @ 2022-11-21 20:08 ` Maxim Cournoyer 0 siblings, 0 replies; 20+ messages in thread From: Maxim Cournoyer @ 2022-11-21 20:08 UTC (permalink / raw) To: Ludovic Courtès; +Cc: Katherine Cox-Buday, guix-devel Hi, Ludovic Courtès <ludo@gnu.org> writes: > Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > >> A side-note, it seems that Ludovic has been >> working toward eliminating the use of match patterns matching the fields >> directly, instead encouraging the use of 'match-record', see >> https://issues.guix.gnu.org/59390. I haven't checked if this is >> compatible with define-configuration records though. > > It is: ‘define-configuration’ builds on ‘define-record-type*’, which > builds on SRFI-9, which builds on Guile “records”, which builds on Guile > “structs”. :-) Excellent! Thanks for having confirmed that. -- Maxim ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Layout of ‘define-configuration’ records 2022-11-20 13:47 ` Maxim Cournoyer 2022-11-21 10:26 ` Ludovic Courtès @ 2022-11-21 16:49 ` Katherine Cox-Buday 2022-11-21 21:00 ` Maxim Cournoyer 1 sibling, 1 reply; 20+ messages in thread From: Katherine Cox-Buday @ 2022-11-21 16:49 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: guix-devel Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: > Apologies for causing grief! No worries at all, Maxim! The good you do far outweighs any grief, and even if that weren't the case, we're only human :) > I'm taking yours and Ludovic's feedback into account for the future > and will reach out to guix-devel with heads-up when introducing > breaking changes to Guix APIs. This seems like the logical place to me, but it's also a bit busy for announcements! I'm sure I'm not alone, but sometimes I'm very busy and distracted and I need a very loud signal to let me know I need to take action :) That's why I suggested info-guix. Its description reads: #+begin_quote Subscribe to the info-guix low-traffic mailing list to receive important announcements sent by the project maintainers (in English). #+end_quote Would it make sense to publish there? I don't think we break APIs very often? Also, do we need any kind of formalization around lead-time for these announcements? > A side-note, it seems that Ludovic has been working toward eliminating > the use of match patterns matching the fields directly, instead > encouraging the use of 'match-record', see > https://issues.guix.gnu.org/59390. I haven't checked if this is > compatible with define-configuration records though. Thanks, I'll have a look! -- Katherine ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Layout of ‘define-configuration’ records 2022-11-21 16:49 ` Katherine Cox-Buday @ 2022-11-21 21:00 ` Maxim Cournoyer 2022-11-22 14:52 ` zimoun 0 siblings, 1 reply; 20+ messages in thread From: Maxim Cournoyer @ 2022-11-21 21:00 UTC (permalink / raw) To: Katherine Cox-Buday; +Cc: guix-devel Hi Katherine, Katherine Cox-Buday <cox.katherine.e@gmail.com> writes: > Maxim Cournoyer <maxim.cournoyer@gmail.com> writes: > >> Apologies for causing grief! > > No worries at all, Maxim! The good you do far outweighs any grief, and > even if that weren't the case, we're only human :) > >> I'm taking yours and Ludovic's feedback into account for the future >> and will reach out to guix-devel with heads-up when introducing >> breaking changes to Guix APIs. > > This seems like the logical place to me, but it's also a bit busy for > announcements! I'm sure I'm not alone, but sometimes I'm very busy and > distracted and I need a very loud signal to let me know I need to take > action :) That's why I suggested info-guix. Its description reads: > > #+begin_quote > Subscribe to the info-guix low-traffic mailing list to receive important > announcements sent by the project maintainers (in English). > #+end_quote That sounds very appropriate indeed. I guess we could send announcements on API breaking changes to both places. I suppose not many people are registered to 'info-guix' (I wasn't myself until recently ^^'). > Would it make sense to publish there? I don't think we break APIs very often? > > Also, do we need any kind of formalization around lead-time for these > announcements? I guess that's a question of how disruptive the API change is, but it'd be convenient if it was 2 weeks to match the time the change might appear on guix-patches un-reviewed. I'll try to do this in the future (better yet, try to not affect APIs at all); if it works well for everybody, we could formalize that in our contributing section. -- Thanks, Maxim ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Layout of ‘define-configuration’ records 2022-11-21 21:00 ` Maxim Cournoyer @ 2022-11-22 14:52 ` zimoun 2022-11-25 15:18 ` Maxim Cournoyer 0 siblings, 1 reply; 20+ messages in thread From: zimoun @ 2022-11-22 14:52 UTC (permalink / raw) To: Maxim Cournoyer, Katherine Cox-Buday; +Cc: guix-devel Hi Maxim, On Mon, 21 Nov 2022 at 16:00, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote: > That sounds very appropriate indeed. I guess we could send > announcements on API breaking changes to both places. I suppose not > many people are registered to 'info-guix' (I wasn't myself until > recently ^^'). Well, more is better here, no? :-) > I guess that's a question of how disruptive the API change is, but it'd > be convenient if it was 2 weeks to match the time the change might > appear on guix-patches un-reviewed. Well, the process for API change could be: 1. + submit to guix-patches, + in the same time, announce to guix-devel; so people not subscribed to guix-patches can chime. 2. + after 2 weeks, or consensus, merge the change, + in the same time, announce to guix-devel, info-guix and --news. There is no extra burden and it smooths the change for many users, IMHO. WDYT? Cheers, simon ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Layout of ‘define-configuration’ records 2022-11-22 14:52 ` zimoun @ 2022-11-25 15:18 ` Maxim Cournoyer 0 siblings, 0 replies; 20+ messages in thread From: Maxim Cournoyer @ 2022-11-25 15:18 UTC (permalink / raw) To: zimoun; +Cc: Katherine Cox-Buday, guix-devel Hi Simon, zimoun <zimon.toutoune@gmail.com> writes: > Hi Maxim, > > On Mon, 21 Nov 2022 at 16:00, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote: > >> That sounds very appropriate indeed. I guess we could send >> announcements on API breaking changes to both places. I suppose not >> many people are registered to 'info-guix' (I wasn't myself until >> recently ^^'). > > Well, more is better here, no? :-) > > >> I guess that's a question of how disruptive the API change is, but it'd >> be convenient if it was 2 weeks to match the time the change might >> appear on guix-patches un-reviewed. > > Well, the process for API change could be: > > 1. + submit to guix-patches, > + in the same time, announce to guix-devel; so people not subscribed to > guix-patches can chime. > 2. + after 2 weeks, or consensus, merge the change, > + in the same time, announce to guix-devel, info-guix and --news. > > There is no extra burden and it smooths the change for many users, IMHO. > > WDYT? This sounds reasonable to me, with the addition that the "core" team members should be added as CC when submitting the API changing patch(es) (etc/teams.scm cc core). -- Thanks, Maxim ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Layout of ‘define-configuration’ records 2022-11-18 16:44 ` Maxim Cournoyer 2022-11-19 21:25 ` Katherine Cox-Buday @ 2022-11-21 10:22 ` Ludovic Courtès 2022-11-21 21:16 ` Maxim Cournoyer 1 sibling, 1 reply; 20+ messages in thread From: Ludovic Courtès @ 2022-11-21 10:22 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: guix-devel Hi Maxim, Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > Ludovic Courtès <ludo@gnu.org> writes: [...] >>> + (%location #,(id #'stem #'stem #'-location) >>> + (default (and=> (current-source-location) >>> + source-properties->location)) >>> + (innate))) >> >> Moving the field last is problematic as we’ve seen for any user that >> uses ‘match’ on records—something that’s not recommended but still used >> a lot. > > Yep. I had that on mind when I made the change, though I still missed a > few occurrences. [...] > I wanted match on define-configuration'd fields to be > backward-compatible with fields migrated from define-record-type*, so > that they such change can be made without worrying breakages. That had the opposite effect: it introduced breakage precisely because existing uses of ‘match’ on records need to be verified manually, one by one. That led me to improve ‘match-record’, to recommend it in the manual, and do “convert” some uses of ‘match’ to ‘match-record’: https://issues.guix.gnu.org/59390 That’s a good outcome, but I’d prefer not feeling this kind of pressure. > I initially got tricked by that discrepancy and it took me quite some > time hunting down a cryptic backtrace when authoring the new mcron > configuration records. I see. However, this is a wide-ranging change, so I think this should have been discussed separately from the mcron changes. I find it important to take time for review and discussion for such changes. >> One last thing: placing ‘%location’ first can let us implement: >> >> (define (configuration-location config) >> (struct-ref config 0)) > > Would this have worked? > > scheme@(gnu services mcron)> (define config (mcron-configuration)) > scheme@(gnu services mcron)> (struct-ref 0 config) You got the order wrong. :-) > All in all, I think that's a rather small change that got our internal > implementation of both type of records in sync between > define-configuration and define-record-type*, that should pave the way > for migrating more of the later to the former without risking breaking > something, going forward. Fundamentally, the layout of record types should not be visible to users. That it is visible via ‘match’ is the problem, and the solution is not to tweak record type layout but instead tp make sure ‘match’ uses on records vanish. I hope that makes sense! > scheme@(gnu services mcron)> ,use (oop goops) Speaking of which: there was a conscious decision to not use GOOPS in Guix from day one. Perhaps some day we’ll want to collectively question that, but let’s make sure we don’t add that dependency on a whim. For example: class-of -> struct-vtable class-name -> record-type-name See commit 50c17ddd9e2983d71c125d89b422fd20fca476e1 for an example. Thanks, Ludo’. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Layout of ‘define-configuration’ records 2022-11-21 10:22 ` Ludovic Courtès @ 2022-11-21 21:16 ` Maxim Cournoyer 2022-11-23 21:56 ` Ludovic Courtès 0 siblings, 1 reply; 20+ messages in thread From: Maxim Cournoyer @ 2022-11-21 21:16 UTC (permalink / raw) To: Ludovic Courtès; +Cc: guix-devel Hi, Ludovic Courtès <ludo@gnu.org> writes: > Hi Maxim, > > Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > >> Ludovic Courtès <ludo@gnu.org> writes: > > [...] > >>>> + (%location #,(id #'stem #'stem #'-location) >>>> + (default (and=> (current-source-location) >>>> + source-properties->location)) >>>> + (innate))) >>> >>> Moving the field last is problematic as we’ve seen for any user that >>> uses ‘match’ on records—something that’s not recommended but still used >>> a lot. >> >> Yep. I had that on mind when I made the change, though I still missed a >> few occurrences. > > [...] > >> I wanted match on define-configuration'd fields to be >> backward-compatible with fields migrated from define-record-type*, so >> that they such change can be made without worrying breakages. > > That had the opposite effect: it introduced breakage precisely because > existing uses of ‘match’ on records need to be verified manually, one by > one. Yes. C.f. "I missed a few occurrences" above :-). > That led me to improve ‘match-record’, to recommend it in the manual, > and do “convert” some uses of ‘match’ to ‘match-record’: > > https://issues.guix.gnu.org/59390 > > That’s a good outcome, but I’d prefer not feeling this kind of pressure. Neat (not the under pressure part)! Perhaps srfi-worthy? >> I initially got tricked by that discrepancy and it took me quite some >> time hunting down a cryptic backtrace when authoring the new mcron >> configuration records. > > I see. However, this is a wide-ranging change, so I think this should > have been discussed separately from the mcron changes. I find it > important to take time for review and discussion for such changes. > >>> One last thing: placing ‘%location’ first can let us implement: >>> >>> (define (configuration-location config) >>> (struct-ref config 0)) >> >> Would this have worked? >> >> scheme@(gnu services mcron)> (define config (mcron-configuration)) >> scheme@(gnu services mcron)> (struct-ref 0 config) > > You got the order wrong. :-) Ah! Thanks for pointing my silly mistake. Then the argument would become... if it's good for define-configuration, it should have been good for define-record-type* the same (why the discrepancy?). After your new documentation in place to guide users to DTRT with regards to matching records, if you think %location should be the first field, then we should make it so in both instances, perhaps? >> All in all, I think that's a rather small change that got our internal >> implementation of both type of records in sync between >> define-configuration and define-record-type*, that should pave the way >> for migrating more of the later to the former without risking breaking >> something, going forward. > > Fundamentally, the layout of record types should not be visible to > users. That it is visible via ‘match’ is the problem, and the solution > is not to tweak record type layout but instead tp make sure ‘match’ uses > on records vanish. > > I hope that makes sense! Yes, and I agree. >> scheme@(gnu services mcron)> ,use (oop goops) > > Speaking of which: there was a conscious decision to not use GOOPS in > Guix from day one. Perhaps some day we’ll want to collectively question > that, but let’s make sure we don’t add that dependency on a whim. > > For example: > > class-of -> struct-vtable > class-name -> record-type-name > > See commit 50c17ddd9e2983d71c125d89b422fd20fca476e1 for an example. Oops! Another point to add to our future coding style guidelines :-). Thanks for showing the simple workaround to goops. -- Thanks, Maxim ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Layout of ‘define-configuration’ records 2022-11-21 21:16 ` Maxim Cournoyer @ 2022-11-23 21:56 ` Ludovic Courtès 2022-11-25 15:15 ` Maxim Cournoyer 0 siblings, 1 reply; 20+ messages in thread From: Ludovic Courtès @ 2022-11-23 21:56 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: guix-devel Hi, Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > Ludovic Courtès <ludo@gnu.org> writes: [...] >>>> One last thing: placing ‘%location’ first can let us implement: >>>> >>>> (define (configuration-location config) >>>> (struct-ref config 0)) >>> >>> Would this have worked? >>> >>> scheme@(gnu services mcron)> (define config (mcron-configuration)) >>> scheme@(gnu services mcron)> (struct-ref 0 config) >> >> You got the order wrong. :-) > > Ah! Thanks for pointing my silly mistake. Then the argument would > become... if it's good for define-configuration, it should have been > good for define-record-type* the same (why the discrepancy?). ‘define-record-type*’ is generic; there’s no reason for it to add a ‘location’ field. > After your new documentation in place to guide users to DTRT with > regards to matching records, if you think %location should be the first > field, then we should make it so in both instances, perhaps? ‘%location’ only appears in ‘define-configuration’; what did you mean by “both instances”? > Oops! Another point to add to our future coding style guidelines :-). In the end, I guess the lesson is that, indeed, not all the design choices and rationales are properly documented. That’ll always be the case to a large extent though, so changes “close to the core” require more careful review and discussion to fully understand the implications of the change—it might look innocuous but have broader implications than expected. Thanks, Ludo’. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Layout of ‘define-configuration’ records 2022-11-23 21:56 ` Ludovic Courtès @ 2022-11-25 15:15 ` Maxim Cournoyer 0 siblings, 0 replies; 20+ messages in thread From: Maxim Cournoyer @ 2022-11-25 15:15 UTC (permalink / raw) To: Ludovic Courtès; +Cc: guix-devel Hi Ludovic, Ludovic Courtès <ludo@gnu.org> writes: > Hi, > > Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis: > >> Ludovic Courtès <ludo@gnu.org> writes: [...] >> Ah! Thanks for pointing my silly mistake. Then the argument would >> become... if it's good for define-configuration, it should have been >> good for define-record-type* the same (why the discrepancy?). > > ‘define-record-type*’ is generic; there’s no reason for it to add a > ‘location’ field. > >> After your new documentation in place to guide users to DTRT with >> regards to matching records, if you think %location should be the first >> field, then we should make it so in both instances, perhaps? > > ‘%location’ only appears in ‘define-configuration’; what did you mean by > “both instances”? Hmm, that's right. Nevermind, I thought the later had a %location "special" field too. >> Oops! Another point to add to our future coding style guidelines :-). > > In the end, I guess the lesson is that, indeed, not all the design > choices and rationales are properly documented. That’ll always be the > case to a large extent though, so changes “close to the core” require > more careful review and discussion to fully understand the implications > of the change—it might look innocuous but have broader implications than > expected. Agreed. I'll try to make better use of the etc/teams.scm script in the future to ping the right people for such changes. -- Thanks, Maxim ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2022-11-25 15:18 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-29 3:47 [bug#58855] [PATCH 0/5] Update mcron to latest commit Maxim Cournoyer 2022-10-29 4:16 ` [bug#58855] [PATCH 1/5] services: configuration: Re-order generated record fields Maxim Cournoyer 2022-10-29 4:16 ` [bug#58855] [PATCH 2/5] services: mcron: Add log? and log-format fields to mcron-configuration Maxim Cournoyer 2022-10-29 4:16 ` [bug#58855] [PATCH 3/5] gnu: mcron: Use gexps and strip trailing #t Maxim Cournoyer 2022-10-29 4:16 ` [bug#58855] [PATCH 4/5] gnu: Remove guile2.2-mcron Maxim Cournoyer 2022-10-29 4:16 ` [bug#58855] [PATCH 5/5] gnu: mcron: Update to 1.2.1-0.5fd0ccd Maxim Cournoyer 2022-11-17 22:37 ` Layout of ‘define-configuration’ records Ludovic Courtès 2022-11-18 16:44 ` Maxim Cournoyer 2022-11-19 21:25 ` Katherine Cox-Buday 2022-11-20 13:47 ` Maxim Cournoyer 2022-11-21 10:26 ` Ludovic Courtès 2022-11-21 20:08 ` Maxim Cournoyer 2022-11-21 16:49 ` Katherine Cox-Buday 2022-11-21 21:00 ` Maxim Cournoyer 2022-11-22 14:52 ` zimoun 2022-11-25 15:18 ` Maxim Cournoyer 2022-11-21 10:22 ` Ludovic Courtès 2022-11-21 21:16 ` Maxim Cournoyer 2022-11-23 21:56 ` Ludovic Courtès 2022-11-25 15:15 ` Maxim Cournoyer
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/guix.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.