[PATCH]: Teach etc/committer.scm.in some stuff.

  • Open
  • quality assurance status badge
Details
4 participants
  • Maxim Cournoyer
  • Maxime Devos
  • Xinglu Chen
  • Ricardo Wurmus
Owner
unassigned
Submitted by
Maxime Devos
Severity
normal
M
M
Maxime Devos wrote on 30 Apr 2021 16:36
(address . guix-patches@gnu.org)
cb4614f9e7fde30068113fc409b28fdda2882854.camel@telenet.be
Hi Guix,

I snarfed these from my cc-for-target branch.
Examples of generated commit messages, without any massaging:

<start snip>
gnu: ufoai-data: Use 'cc-for-target' and friends.

* gnu/packages/games.scm (ufoai-data)
[arguments]<#:configure-flags>: Use the C cross-compiler, instead of hardcoding "gcc". Use the C++ cross-compiler, instead of hardcoding "g++".

gnu: mlt: Use 'cc-for-target' and friends.

* gnu/packages/video.scm (mlt)
[arguments]<#:make-flags>: Use the C cross-compiler, instead of hardcoding "gcc". Use the C++ cross-compiler, instead of hardcoding "g++".

gnu: theorafile: Use the C cross-compiler.

* gnu/packages/video.scm (theorafile)
[arguments]<#:make-flags>: Use the C cross-compiler, instead of hardcoding "gcc".
[arguments]<#:phases>{check}: Only run tests when not requested.
<end snip>

etc/comitter.scm.in should probably do some line wrapping as well.

WDYT?

Greetings,
Maxime.
Attachment: file
From ae06b43db712697f7080cc8e5f4d3eecd9350211 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Fri, 30 Apr 2021 13:32:41 +0200
Subject: [PATCH 2/6] etc: Teach committer.scm about checking tests? in the
'check' phase.

* etc/committer.scm.in
(has-explicit-argument?): New procedure.
(change-commit-message)[explain-argument]<#:phases>: New case,
try explaining changes in #:phases.
(explain-phases/change): New procedure, explain the patch makes
sure 'tests?' is respected in the 'check' phase.
---
etc/committer.scm.in | 46 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)

Toggle diff (73 lines)
diff --git a/etc/committer.scm.in b/etc/committer.scm.in
index 75c82c9019..3fa70a81d0 100755
--- a/etc/committer.scm.in
+++ b/etc/committer.scm.in
@@ -229,6 +229,26 @@ Return false if all changes could be explained and truth otherwise."
(or (bitvector-position old-explained? #f)
(bitvector-position new-explained? #f))))
+(define (has-explicit-argument? argument-to-search-for argument-list)
+ "Test whether ARGUMENT-TO-SEARCH-FOR occurs in ARGUMENT-LIST."
+ (let loop ((argument-list argument-list))
+ ;; (lambda () exp)
+ (cond ((null? argument-list) #f)
+ ;; (lambda (x . rest) exp)
+ ((pair? argument-list)
+ (let ((argument-in-list (car argument-list))
+ (rest (cdr argument-list)))
+ (cond ((eq? argument-in-list argument-to-search-for)
+ #t)
+ ;; (lambda* (#:key (x . default) . rest) #f)
+ ((and (pair? argument-in-list)
+ (eq? (car argument-in-list) argument-to-search-for))
+ #t)
+ (#t (loop rest)))))
+ ;; (lambda _ exp)
+ ((symbol? argument-list) #f)
+ (#t (error "the argument list seems to be incorrect!")))))
+
(define* (change-commit-message file-name old new #:optional (port (current-output-port)))
"Print ChangeLog commit message for changes between OLD and NEW."
(define (get-values expr field)
@@ -292,6 +312,21 @@ Return false if all changes could be explained and truth otherwise."
" Use the C++ cross-compiler, instead of hardcoding \"g++\".")
#t)
(_ #f)))
+ (define (explain-phases/change x y)
+ (match (cons x y)
+ ;; "guix build" supports a --without-tests=PACKAGE option,
+ ;; for building a package without running tests. Also, tests
+ ;; can often not be run when cross-compiling. The 'check'
+ ;; phase needs to respect this, though. Maybe this patch is
+ ;; for ensuring the phase respects this.
+ ((('replace ''check ((or 'lambda* 'lambda) args/x . exps/x))
+ . ('replace ''check ((or 'lambda* 'lambda) args/y . exps/y)))
+ (when (and (not (has-explicit-argument? 'tests? args/x))
+ (has-explicit-argument? 'tests? args/y))
+ (format port
+ "[arguments]<#:phases>{check}: Only run tests when not requested.~%")
+ #t))
+ (_ #f)))
(define (explain-argument keyword old new)
(unless (equal? old new)
(case keyword
@@ -303,6 +338,17 @@ Return false if all changes could be explained and truth otherwise."
;; There were some unexplained changes.
(format port " Update.~%")
(format port "~%")))
+ ((#:phases)
+ ;; For each phase, a separate line will be printed.
+ (match (cons old new)
+ ;; _ can be %standard-phases, for example
+ ((('modify-phases _ . rest/old) . ('modify-phases _ . rest/new))
+ (if (explain-list-delta rest/old rest/new
+ #:pairwise/change explain-phases/change)
+ ;; There were some unexplained changes.
+ (format port "[arguments]<#:phases>: Update.~%")))
+ ;; There were some unexplained changes.
+ (_ (format port "[arguments]<#:phases>: Update.~%"))))
;; There were some unexplained changes.
(else (format port "[arguments]<~a>: Update.~%" keyword)))))
(let ((old-arguments (or (get-values/list old 'arguments) '()))
--
2.31.1
From 31c36574e73ba448b0a883f046dbb5021882273e Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Fri, 30 Apr 2021 13:56:17 +0200
Subject: [PATCH 3/6] etc: committer: Only claim to be updating a package when
it's true.

* etc/committer.scm.in (change-commit-message): If the patch does not
change the version, do not falsely say the package is updated, and
instead output a placeholder to be filled in by the user.
---
etc/committer.scm.in | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

Toggle diff (29 lines)
diff --git a/etc/committer.scm.in b/etc/committer.scm.in
index 3fa70a81d0..8027d9d0f1 100755
--- a/etc/committer.scm.in
+++ b/etc/committer.scm.in
@@ -274,12 +274,17 @@ Return false if all changes could be explained and truth otherwise."
", and " (first (take-right items 1))))))
(define variable-name
(second old))
- (define version
- (and=> ((sxpath '(// version *any*)) new)
+ (define (version exp)
+ (and=> ((sxpath '(// version *any*)) exp)
first))
- (format port
- "gnu: ~a: Update to ~a.~%~%* ~a (~a): Update to ~a.~%"
- variable-name version file-name variable-name version)
+ (define old-version (version old))
+ (define new-version (version new))
+ (cond ((not (equal? old-version new-version))
+ (format port "gnu: ~a: Update to ~a.~%~%* ~a (~a): Update to ~a.~%"
+ variable-name new-version file-name variable-name new-version))
+ (#t
+ (format port "gnu: ~a: <FIXME you need to write something here>~%~%* ~a (~a): FIXME!.~%"
+ variable-name file-name variable-name)))
(for-each (lambda (field)
(let ((old-values (get-values old field))
(new-values (get-values new field)))
--
2.31.1
Attachment: file
From 061175155983d73f1488f2cf22d6d52bd5ed3054 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Fri, 30 Apr 2021 15:32:08 +0200
Subject: [PATCH 5/6] etc: committer: Support (list exp ...) in #:make-flags.

* etc/committer.scm.in
(unwrap-list): New procedure, supporting 'quasiquote', 'quote'
and 'list'.
(change-commit-message/one-pass)[unwrap-list]: Use new procedure.
---
etc/committer.scm.in | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

Toggle diff (32 lines)
diff --git a/etc/committer.scm.in b/etc/committer.scm.in
index be23367f49..fc3c929f17 100755
--- a/etc/committer.scm.in
+++ b/etc/committer.scm.in
@@ -264,6 +264,15 @@ Return false if all changes could be explained and truth otherwise."
(define (make-patch-summary)
(%make-patch-summary #f #f #f))
+;; '(x ...) -> (x ...)
+;; `(x ...) -> (x ...)
+;; (list x ...) -> (x ...)
+(define (unwrap-list list)
+ (case (car list)
+ ((quasiquote quote) (second list))
+ ((list) (cdr list))
+ (else (error "I can't interpret that as a list!"))))
+
(define* (change-commit-message/one-pass
file-name old new summary
#:optional (port (current-output-port)))
@@ -374,8 +383,7 @@ SUMMARY: first using a ‘void port’, then with the ‘real’ output port."
(case keyword
((#:make-flags)
(format port "[arguments]<#:make-flags>:")
- ;; second: skip ' and `
- (if (explain-list-delta (second old) (second new)
+ (if (explain-list-delta (unwrap-list old) (unwrap-list new)
#:pairwise/change explain-make-flags/change)
;; There were some unexplained changes.
(format port " Update.~%")
--
2.31.1
From 295cca1e1745acfea49b54681d566dd3d0c1dd19 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Fri, 30 Apr 2021 15:45:02 +0200
Subject: [PATCH 6/6] etc: committer: Explain changes in #:configure-flags.

* etc/committer.scm.in
(change-commit-message/one-pass)[explain-argument]: Handle
#:configure-flags the same way as #:make-flags for now.
---
etc/committer.scm.in | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

Toggle diff (21 lines)
diff --git a/etc/committer.scm.in b/etc/committer.scm.in
index fc3c929f17..0fd383533d 100755
--- a/etc/committer.scm.in
+++ b/etc/committer.scm.in
@@ -381,8 +381,12 @@ SUMMARY: first using a ‘void port’, then with the ‘real’ output port."
(define (explain-argument keyword old new)
(unless (equal? old new)
(case keyword
- ((#:make-flags)
- (format port "[arguments]<#:make-flags>:")
+ ;; Sometimes, arguments like "CC=TARGET-gcc" are passed to the
+ ;; configure script. Their interpretation is sometimes the same
+ ;; as in makefiles. Hence, for now we unify the handling of
+ ;; #:make-flags and #:configure-flags.
+ ((#:make-flags #:configure-flags)
+ (format port "[arguments]<~a>:" keyword)
(if (explain-list-delta (unwrap-list old) (unwrap-list new)
#:pairwise/change explain-make-flags/change)
;; There were some unexplained changes.
--
2.31.1
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYIwV1RccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7sYQAP9swBcYjO7Hv2u6z5OjN+HbU1bi
xwc26EIrPg9AVaXIjwEAvu0G0glNyWFDawNqb0xORZXdsDD3N1jn9Q2Xf6JJ8gc=
=R2cu
-----END PGP SIGNATURE-----


M
M
Maxime Devos wrote on 6 May 2021 09:50
Re: [PATCH] Teach etc/committer.scm.in some stuff
(address . 48120@debbugs.gnu.org)
039cd7066fe774cb6c3faad5e745da158d865485.camel@telenet.be
New patch series, handling more edge cases.

It needs some changes in how it wraps lines now there
is a break-string procedure, but I don't have time to
work on this currently so I'll just submit it as-is for now.

Greetings,
Maxime.
Attachment: file
From 894dde9febbecaedd1a9596ff66653f9c9249ef4 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Fri, 30 Apr 2021 13:32:41 +0200
Subject: [PATCH 02/11] etc: Teach committer.scm about checking tests? in the
'check' phase.

* etc/committer.scm.in
(has-explicit-argument?): New procedure.
(change-commit-message)[explain-argument]<#:phases>: New case,
try explaining changes in #:phases.
(explain-phases/change): New procedure, explain the patch makes
sure 'tests?' is respected in the 'check' phase.
---
etc/committer.scm.in | 46 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)

Toggle diff (73 lines)
diff --git a/etc/committer.scm.in b/etc/committer.scm.in
index c2f27ef8c8..1f25c424dd 100755
--- a/etc/committer.scm.in
+++ b/etc/committer.scm.in
@@ -256,6 +256,26 @@ Return false if all changes could be explained and truth otherwise."
(or (bitvector-position old-explained? #f)
(bitvector-position new-explained? #f))))
+(define (has-explicit-argument? argument-to-search-for argument-list)
+ "Test whether ARGUMENT-TO-SEARCH-FOR occurs in ARGUMENT-LIST."
+ (let loop ((argument-list argument-list))
+ ;; (lambda () exp)
+ (cond ((null? argument-list) #f)
+ ;; (lambda (x . rest) exp)
+ ((pair? argument-list)
+ (let ((argument-in-list (car argument-list))
+ (rest (cdr argument-list)))
+ (cond ((eq? argument-in-list argument-to-search-for)
+ #t)
+ ;; (lambda* (#:key (x . default) . rest) #f)
+ ((and (pair? argument-in-list)
+ (eq? (car argument-in-list) argument-to-search-for))
+ #t)
+ (#t (loop rest)))))
+ ;; (lambda _ exp)
+ ((symbol? argument-list) #f)
+ (#t (error "the argument list seems to be incorrect!")))))
+
(define* (change-commit-message file-name old new #:optional (port (current-output-port)))
"Print ChangeLog commit message for changes between OLD and NEW."
(define (get-values expr field)
@@ -320,6 +340,21 @@ Return false if all changes could be explained and truth otherwise."
" Use the C++ cross-compiler, instead of hardcoding \"g++\".")
#t)
(_ #f)))
+ (define (explain-phases/change x y)
+ (match (cons x y)
+ ;; "guix build" supports a --without-tests=PACKAGE option,
+ ;; for building a package without running tests. Also, tests
+ ;; can often not be run when cross-compiling. The 'check'
+ ;; phase needs to respect this, though. Maybe this patch is
+ ;; for ensuring the phase respects this.
+ ((('replace ''check ((or 'lambda* 'lambda) args/x . exps/x))
+ . ('replace ''check ((or 'lambda* 'lambda) args/y . exps/y)))
+ (when (and (not (has-explicit-argument? 'tests? args/x))
+ (has-explicit-argument? 'tests? args/y))
+ (format port
+ "[arguments]<#:phases>{check}: Only run tests when not requested.~%")
+ #t))
+ (_ #f)))
(define (explain-argument keyword old new)
(unless (equal? old new)
(case keyword
@@ -331,6 +366,17 @@ Return false if all changes could be explained and truth otherwise."
;; There were some unexplained changes.
(format port " Update.~%")
(format port "~%")))
+ ((#:phases)
+ ;; For each phase, a separate line will be printed.
+ (match (cons old new)
+ ;; _ can be %standard-phases, for example
+ ((('modify-phases _ . rest/old) . ('modify-phases _ . rest/new))
+ (if (explain-list-delta rest/old rest/new
+ #:pairwise/change explain-phases/change)
+ ;; There were some unexplained changes.
+ (format port "[arguments]<#:phases>: Update.~%")))
+ ;; There were some unexplained changes.
+ (_ (format port "[arguments]<#:phases>: Update.~%"))))
;; There were some unexplained changes.
(else (format port "[arguments]<~a>: Update.~%" keyword)))))
(let ((old-arguments (or (get-values/list old 'arguments) '()))
--
2.31.1
From d190b8088908518e51ac3e2b2bfa92747a506aba Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Fri, 30 Apr 2021 13:56:17 +0200
Subject: [PATCH 03/11] etc: committer: Only claim to be updating a package
when it's true.

* etc/committer.scm.in (change-commit-message): If the patch does not
change the version, do not falsely say the package is updated, and
instead output a placeholder to be filled in by the user.
---
etc/committer.scm.in | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

Toggle diff (29 lines)
diff --git a/etc/committer.scm.in b/etc/committer.scm.in
index 1f25c424dd..02f7817bde 100755
--- a/etc/committer.scm.in
+++ b/etc/committer.scm.in
@@ -301,12 +301,17 @@ Return false if all changes could be explained and truth otherwise."
", and " (first (take-right items 1))))))
(define variable-name
(second old))
- (define version
- (and=> ((sxpath '(// version *any*)) new)
+ (define (version exp)
+ (and=> ((sxpath '(// version *any*)) exp)
first))
- (format port
- "gnu: ~a: Update to ~a.~%~%* ~a (~a): Update to ~a.~%"
- variable-name version file-name variable-name version)
+ (define old-version (version old))
+ (define new-version (version new))
+ (cond ((not (equal? old-version new-version))
+ (format port "gnu: ~a: Update to ~a.~%~%* ~a (~a): Update to ~a.~%"
+ variable-name new-version file-name variable-name new-version))
+ (#t
+ (format port "gnu: ~a: <FIXME you need to write something here>~%~%* ~a (~a): FIXME!.~%"
+ variable-name file-name variable-name)))
(for-each (lambda (field)
(let ((old-values (get-values old field))
(new-values (get-values new field)))
--
2.31.1
Attachment: file
From 1b8a1599c84f25bdca866fc4dc1a7f2217901c4f Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Fri, 30 Apr 2021 15:32:08 +0200
Subject: [PATCH 05/11] etc: committer: Support (list exp ...) in #:make-flags.

* etc/committer.scm.in
(unwrap-list): New procedure, supporting 'quasiquote', 'quote'
and 'list'.
(change-commit-message/one-pass)[unwrap-list]: Use new procedure.
---
etc/committer.scm.in | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

Toggle diff (32 lines)
diff --git a/etc/committer.scm.in b/etc/committer.scm.in
index 231f27fe4e..1f8f06b9f1 100755
--- a/etc/committer.scm.in
+++ b/etc/committer.scm.in
@@ -291,6 +291,15 @@ Return false if all changes could be explained and truth otherwise."
(define (make-patch-summary)
(%make-patch-summary #f #f #f))
+;; '(x ...) -> (x ...)
+;; `(x ...) -> (x ...)
+;; (list x ...) -> (x ...)
+(define (unwrap-list list)
+ (case (car list)
+ ((quasiquote quote) (second list))
+ ((list) (cdr list))
+ (else (error "I can't interpret that as a list!"))))
+
(define* (change-commit-message/one-pass
file-name old new summary
#:optional (port (current-output-port)))
@@ -402,8 +411,7 @@ SUMMARY: first using a ‘void port’, then with the ‘real’ output port."
(case keyword
((#:make-flags)
(format port "[arguments]<#:make-flags>:")
- ;; second: skip ' and `
- (if (explain-list-delta (second old) (second new)
+ (if (explain-list-delta (unwrap-list old) (unwrap-list new)
#:pairwise/change explain-make-flags/change)
;; There were some unexplained changes.
(format port " Update.~%")
--
2.31.1
From f55d1a148426fda3c1cbb16f93e36ef14a3be099 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Fri, 30 Apr 2021 15:45:02 +0200
Subject: [PATCH 06/11] etc: committer: Explain changes in #:configure-flags.

* etc/committer.scm.in
(change-commit-message/one-pass)[explain-argument]: Handle
#:configure-flags the same way as #:make-flags for now.
---
etc/committer.scm.in | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

Toggle diff (21 lines)
diff --git a/etc/committer.scm.in b/etc/committer.scm.in
index 1f8f06b9f1..3c35074141 100755
--- a/etc/committer.scm.in
+++ b/etc/committer.scm.in
@@ -409,8 +409,12 @@ SUMMARY: first using a ‘void port’, then with the ‘real’ output port."
(define (explain-argument keyword old new)
(unless (equal? old new)
(case keyword
- ((#:make-flags)
- (format port "[arguments]<#:make-flags>:")
+ ;; Sometimes, arguments like "CC=TARGET-gcc" are passed to the
+ ;; configure script. Their interpretation is sometimes the same
+ ;; as in makefiles. Hence, for now we unify the handling of
+ ;; #:make-flags and #:configure-flags.
+ ((#:make-flags #:configure-flags)
+ (format port "[arguments]<~a>:" keyword)
(if (explain-list-delta (unwrap-list old) (unwrap-list new)
#:pairwise/change explain-make-flags/change)
;; There were some unexplained changes.
--
2.31.1
From c6fd832887b271378b1cfc1909f233693ea70cb4 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Fri, 30 Apr 2021 22:53:12 +0200
Subject: [PATCH 07/11] etc: committer: Read #~, #$, #+ correctly.

* etc/committer.scm.in: Import (guix gexp).
---
etc/committer.scm.in | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

Toggle diff (16 lines)
diff --git a/etc/committer.scm.in b/etc/committer.scm.in
index 3c35074141..f03f0daf2b 100755
--- a/etc/committer.scm.in
+++ b/etc/committer.scm.in
@@ -38,7 +38,8 @@
(ice-9 match)
(ice-9 rdelim)
(ice-9 textual-ports)
- (rnrs control))
+ (rnrs control)
+ (guix gexp))
(define* (break-string str #:optional (max-line-length 70))
"Break the string STR into lines that are no longer than MAX-LINE-LENGTH.
--
2.31.1
From e77fc1c06876bc65e0782af3bd1ebdbef4bc1ea3 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Fri, 30 Apr 2021 22:55:41 +0200
Subject: [PATCH 08/11] etc: committer: Perform line-wrapping.

* etc/committer.scm.in (change-commit-message): Perform line wrapping.
---
etc/committer.scm.in | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)

Toggle diff (43 lines)
diff --git a/etc/committer.scm.in b/etc/committer.scm.in
index f03f0daf2b..de8f954f4e 100755
--- a/etc/committer.scm.in
+++ b/etc/committer.scm.in
@@ -39,7 +39,9 @@
(ice-9 rdelim)
(ice-9 textual-ports)
(rnrs control)
- (guix gexp))
+ (guix gexp)
+ (texinfo)
+ (texinfo plain-text))
(define* (break-string str #:optional (max-line-length 70))
"Break the string STR into lines that are no longer than MAX-LINE-LENGTH.
@@ -442,11 +444,23 @@ SUMMARY: first using a ‘void port’, then with the ‘real’ output port."
(define* (change-commit-message file-name old new
#:optional (port (current-output-port)))
"Like change-commit-message/one-pass, but without requiring to be run twice
-for a correct header."
+for a correct header. Also perform line wrapping."
(let ((summary (make-patch-summary)))
(change-commit-message/one-pass file-name old new summary
(%make-void-port "w"))
- (change-commit-message/one-pass file-name old new summary port)))
+ (let* ((unwrapped-message
+ (call-with-output-string
+ (lambda (port)
+ (change-commit-message/one-pass
+ file-name old new summary port))))
+ ;; Hack: use (texinfo) and (texinfo plain-text) for
+ ;; line wraping. Suggested by rekado on #guix.
+ (raw-message/stexi (texi-fragment->stexi unwrapped-message))
+ (wrapped-message
+ (with-fluid* *line-width* 70
+ (lambda ()
+ (stexi->plain-text raw-message/stexi)))))
+ (put-string port wrapped-message))))
(define* (add-commit-message file-name variable-name #:optional (port (current-output-port)))
"Print ChangeLog commit message for a change to FILE-NAME adding a definition."
--
2.31.1
From 0861ab498d87ce1314221fef3c304ab9f1a73ab0 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Fri, 30 Apr 2021 22:57:07 +0200
Subject: [PATCH 09/11] etc: committer: Don't crash if no keyword list is
detected.

This is required for some packages that use quasiquote and
unquote-splicing in #:configure-flags.

* etc/committer.scm.in (keyword-list->alist): Don't crash
if no keyword list structure could be detected, instead
produce a warning.
---
etc/committer.scm.in | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

Toggle diff (28 lines)
diff --git a/etc/committer.scm.in b/etc/committer.scm.in
index de8f954f4e..7fbeb1874e 100755
--- a/etc/committer.scm.in
+++ b/etc/committer.scm.in
@@ -41,7 +41,9 @@
(rnrs control)
(guix gexp)
(texinfo)
- (texinfo plain-text))
+ (texinfo plain-text)
+ (guix diagnostics)
+ (guix i18n))
(define* (break-string str #:optional (max-line-length 70))
"Break the string STR into lines that are no longer than MAX-LINE-LENGTH.
@@ -209,7 +211,9 @@ corresponding to the top-level definition containing the staged changes."
(match kwlist
(() '())
(((? keyword? k) object . rest)
- `((,k . ,object) . ,(keyword-list->alist rest)))))
+ `((,k . ,object) . ,(keyword-list->alist rest)))
+ (_ (warning (G_ "cannot interpret as keyword argument list: ‘~a’~%") '())
+ '())))
(define (pairwise-foreach-keyword proc . arguments)
"Apply PROC with each keyword argument and corresponding values
--
2.31.1
From a63e09363549b16ba22d14b41812a008ecbe4127 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Fri, 30 Apr 2021 22:59:07 +0200
Subject: [PATCH 10/11] etc: committer: Ignore let bindings.

Hopefully they aren't important in practice, for
deciding on the commit message!

* etc/committer.scm.in
(change-commit-message/one-pass)[explain-make-flags/change]: Ignore 'let'
and 'let*' bindings, only use the last expression.
(unwrap-list)[unwrap-list]: Ignore 'let' and 'let*' bindings, only use
the expression that is inside.
---
etc/committer.scm.in | 4 ++++
1 file changed, 4 insertions(+)

Toggle diff (24 lines)
diff --git a/etc/committer.scm.in b/etc/committer.scm.in
index 7fbeb1874e..c056de912c 100755
--- a/etc/committer.scm.in
+++ b/etc/committer.scm.in
@@ -305,6 +305,8 @@ Return false if all changes could be explained and truth otherwise."
(case (car list)
((quasiquote quote) (second list))
((list) (cdr list))
+ ;; Hopefully the bindings weren't important ...
+ ((let let*) (last list))
(else (error "I can't interpret that as a list!"))))
(define* (change-commit-message/one-pass
@@ -396,6 +398,8 @@ SUMMARY: first using a ‘void port’, then with the ‘real’ output port."
(format port
" Use the C++ cross-compiler, instead of hardcoding \"g++\".")
#t)
+ ((((or 'let 'let*) _ exp) . _) (explain-make-flags/change exp y))
+ ((_ . ((or 'let 'let*) _ exp)) (explain-make-flags/change x exp))
(_ #f)))
(define (explain-phases/change x y)
(match (cons x y)
--
2.31.1
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYJOf3hccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7hFrAQDeW9UgJ2r6zZsG1Jd4sHC6sHaf
R9BQuR2TqZA5uZ0V7AD/cToWIYFUbHACDi4ktIqY/OuRcIvWsTUsDMu02Pvs1w8=
=rzk+
-----END PGP SIGNATURE-----


X
X
Xinglu Chen wrote on 9 May 2021 20:34
Re: [bug#48120] [PATCH] Teach etc/committer.scm.in some stuff
87v97rhi38.fsf@yoctocell.xyz
On Thu, May 06 2021, Maxime Devos wrote:

Toggle quote (6 lines)
> New patch series, handling more edge cases.
>
> It needs some changes in how it wraps lines now there
> is a break-string procedure, but I don't have time to
> work on this currently so I'll just submit it as-is for now.

I don’t think I am qualified to review all of this, but it seems to work
after I made some minor fixes. I just used ‘cc-for-target’ instead of
hardcoding ‘gcc’ on a random package, this is what I got.

Toggle snippet (7 lines)
gnu: eigensoft: Use the C cross-compiler.
* gnu/packages/bioinformatics.scm (eigensoft)
[arguments]<#:make-flags>: Use the C cross-compiler, instead of
hardcoding "gcc".

Toggle quote (13 lines)
> +(define (keyword-list->alist kwlist)
> + (match kwlist
> + (() '())
> + (((? keyword? k) object . rest)
> + `((,k . ,object) . ,(keyword-list->alist rest)))))
> +
> +(define (pairwise-foreach-keyword proc . arguments)
> + "Apply PROC with each keyword argument and corresponding values
> +in ARGUMENTS. If a value is not present in a argument, pass #f instead."
> + (let* ((alists (map keyword-list->alist arguments))
> + (keywords (delete-duplicates
> + (apply append (map (cut map car <>) alists))

‘append-map’ instead of (apply append (map ...) ...) ?

Toggle quote (29 lines)
> + eq?)))
> + (for-each (lambda (keyword)
> + (apply proc keyword
> + (map (cut assoc-ref <> keyword) alists)))
> + keywords)))
> +
>
> [...]
>
> @@ -207,6 +263,14 @@ corresponding to the top-level definition containing the staged changes."
> (() '())
> ((first . rest)
> (map cadadr first))))
> + ;; Like get-values, but also allow quote and do not treat
> + ;; the value of the field as an alist.
> + (define (get-values/list expr field)
> + (match ((sxpath `(// ,field ,(node-or (sxpath '(quasiquote))
> + (sxpath '(quote))))) expr)
> + (() '())
> + ((first . rest)
> + (second first))))
> (define (listify items)
> (match items
> ((one) one)
> @@ -245,6 +309,34 @@ corresponding to the top-level definition containing the staged changes."
> (listify removed)
> (listify added))))))))))
> '(inputs propagated-inputs native-inputs)))

I think the parentheses are mismatched here, {M-x check-parens} should complain.

Toggle snippet (21 lines)
~/src/guix $ guile etc/committer.scm.in
;;; note: source file /home/yoctocell/src/guix/etc/committer.scm
;;; newer than compiled /home/yoctocell/.cache/guile/ccache/3.0-LE-8-4.4/home/yoctocell/src/guix/etc/committer.scm.go
;;; note: auto-compilation is enabled, set GUILE_AUTO_COMPILE=0
;;; or pass the --no-auto-compile argument to disable.
;;; compiling /home/yoctocell/src/guix/etc/committer.scm
;;; WARNING: compilation of /home/yoctocell/src/guix/etc/committer.scm failed:
;;; In procedure read_inner_expression: etc/committer.scm:465:47: unexpected ")"
Backtrace:
4 (primitive-load "/home/yoctocell/src/guix/etc/committer.scm")
In ice-9/eval.scm:
298:34 3 (_ #<directory (guile-user) 7efdd29e9c80>)
196:27 2 (_ #<directory (guile-user) 7efdd29e9c80>)
223:20 1 (proc #<directory (guile-user) 7efdd29e9c80>)
In unknown file:
0 (%resolve-variable (7 . get-values/no-unquote) #<directory (guile-user) 7efdd29e9c80>)

ERROR: In procedure %resolve-variable:
Unbound variable: get-values/no-unquote

Toggle quote (19 lines)
> From 5f0313c01121a0a1e7f39f447425b5a8b70fb8c0 Mon Sep 17 00:00:00 2001
> From: Maxime Devos <maximedevos@telenet.be>
> Date: Sat, 1 May 2021 12:19:05 +0200
> Subject: [PATCH 11/11] etc: committer: Handle substitute-keyword-arguments.
>
> * etc/committer.scm.in
> (keyword-list->alist): Rename to ...
> (keyword-list->alist/list): ..., and document the input format.
>
> [...]
>
> -(define (keyword-list->alist kwlist)
> +;; Input: a list of keywords and the corresponding values,
> +;; without an exterior quote, quasiquote or list.
> +(define (keyword-list->alist/list kwlist)
> (match kwlist
> (() '())
> (((? keyword? k) object . rest)
> `((,k . ,object) . ,(keyword-list->alist rest)))
^^^^^^^^^^^^^^^^^^^
‘keyword-list->alist/list’
M
M
Maxime Devos wrote on 17 Jun 2021 15:36
[PATCH] etc: committer: Read #~, #$ and #+ correctly.
(address . 48120@debbugs.gnu.org)
927c6dc244aea404128d54055663409bd7f1e7e1.camel@telenet.be
Hi Guix,

I haven't gotten around to making a new revision
of the whole patch series yet, but I want to note
that this individual patch is simple and functions
on its own.

Greetings,
Maxime.
From 3f6404ab0367d91bb665748a8eab17976e9a2c11 Mon Sep 17 00:00:00 2001
From: Maxime Devos <maximedevos@telenet.be>
Date: Fri, 30 Apr 2021 22:53:12 +0200
Subject: [PATCH] etc: committer: Read #~, #$ and #+ correctly.

Some package definitions use G-expressions (see, e.g., chez-scheme).
Import (guix gexp) such that Guile knows how to read those.
Otherwise, an exception such as the following might be raised:

ERROR: In procedure read:
In procedure scm_lreadr: gnu/services/networking.scm:480:16: Unknown # object: #\~

* etc/committer.scm.in: Import (guix gexp).
---
etc/committer.scm.in | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

Toggle diff (16 lines)
diff --git a/etc/committer.scm.in b/etc/committer.scm.in
index 1f19ccfd6d..b61121bc4b 100755
--- a/etc/committer.scm.in
+++ b/etc/committer.scm.in
@@ -36,7 +36,8 @@
(ice-9 popen)
(ice-9 match)
(ice-9 rdelim)
- (ice-9 textual-ports))
+ (ice-9 textual-ports)
+ (guix gexp))
(define (read-excursion port)
"Read an expression from PORT and reset the port position before returning
--
2.32.0
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYMtPwhccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7hSgAP9IRHUVIZxHoAmAWAtBiuw+EONk
k8ig3itSrQRsvTtqRQEAg40AuKDIRkkv/Wlcv9D/vY9RqQ0SqwOfEwv29LUjfQ4=
=kYq1
-----END PGP SIGNATURE-----


R
R
Ricardo Wurmus wrote on 7 Aug 2021 14:06
[PATCH]: Teach etc/committer.scm.in some stuff.
(address . 48120@debbugs.gnu.org)
87eeb5mpcn.fsf@elephly.net
I pushed that one change with commit
50c2dcd1c977f98681a4e457b2bcf09d96588eee to the master branch.

Thank you!

--
Ricardo
R
R
Ricardo Wurmus wrote on 23 Nov 2021 00:07
(address . 48120@debbugs.gnu.org)
878rxfvle6.fsf@elephly.net
Hi Maxime,

are you still interested in adjusting your patches? Or would you
prefer I make the suggested changes when applying them?

--
Ricardo
M
M
Maxim Cournoyer wrote on 22 May 2022 05:33
(name . Maxime Devos)(address . maximedevos@telenet.be)(address . 48120@debbugs.gnu.org)
87bkvq8cmt.fsf_-_@gmail.com
Hello,

Maxime Devos <maximedevos@telenet.be> writes:

Toggle quote (6 lines)
> New patch series, handling more edge cases.
>
> It needs some changes in how it wraps lines now there
> is a break-string procedure, but I don't have time to
> work on this currently so I'll just submit it as-is for now.

Gentle ping :-).

Maxim
M
M
Maxim Cournoyer wrote on 22 May 2022 05:33
control message for bug #48120
(address . control@debbugs.gnu.org)
87a6ba8cmj.fsf@gmail.com
tags 48120 + moreinfo
quit
M
M
Maxime Devos wrote on 22 May 2022 11:04
Re: bug#48120: [PATCH]: Teach etc/committer.scm.in some stuff.
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)(address . 48120@debbugs.gnu.org)
e3b2c47dbae79f0879ed1df58685cf7f861b9f81.camel@telenet.be
Maxim Cournoyer schreef op za 21-05-2022 om 23:33 [-0400]:
Toggle quote (14 lines)
> Hello,
>
> Maxime Devos <maximedevos@telenet.be> writes:
>
> > New patch series, handling more edge cases.
> >
> > It needs some changes in how it wraps lines now there
> > is a break-string procedure, but I don't have time to
> > work on this currently so I'll just submit it as-is for now.
>
> Gentle ping :-).
>
> Maxim

Currently working on other things. Also, IIRC, needs a rebase
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYon8tBccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7s0rAQDVx4Kr+EqZX3uHNeAG9S8SLzzm
wgc8TAIW0VJS7Q8sBAD/aM517o/xlVxiTizEveH+lN9JUTZv6wh1tmu61Jm6yg4=
=+wZk
-----END PGP SIGNATURE-----


M
M
Maxim Cournoyer wrote on 22 May 2022 15:13
(name . Maxime Devos)(address . maximedevos@telenet.be)(address . 48120@debbugs.gnu.org)
87r14l677q.fsf@gmail.com
Hello,

Maxime Devos <maximedevos@telenet.be> writes:

Toggle quote (17 lines)
> Maxim Cournoyer schreef op za 21-05-2022 om 23:33 [-0400]:
>> Hello,
>>
>> Maxime Devos <maximedevos@telenet.be> writes:
>>
>> > New patch series, handling more edge cases.
>> >
>> > It needs some changes in how it wraps lines now there
>> > is a break-string procedure, but I don't have time to
>> > work on this currently so I'll just submit it as-is for now.
>>
>> Gentle ping :-).
>>
>> Maxim
>
> Currently working on other things. Also, IIRC, needs a rebase

OK, no pressure, thanks for the update.

Maxim
?
Your comment

Commenting via the web interface is currently disabled.

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

To respond to this issue using the mumi CLI, first switch to it
mumi current 48120
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