* Detecting duplicate field initializers in guix record constructors
@ 2018-04-21 8:16 Mark H Weaver
2018-04-22 20:01 ` Ludovic Courtès
0 siblings, 1 reply; 2+ messages in thread
From: Mark H Weaver @ 2018-04-21 8:16 UTC (permalink / raw)
To: guix-devel
[-- Attachment #1: Type: text/plain, Size: 1674 bytes --]
Hello Guix,
Recently, when doing a merge of 'master' into 'core-updates', I noticed
that git's automatic merging sometimes results in duplicate field
initializers being introduced, without any merge conflict being
reported. This happens when a field is introduced independently in both
'core-updates' and 'master', but in different places within the
constructor.
So, I implemented duplicate field detection in (guix records).
See below for my draft patches.
This revealed 9 occurrences of this error in my private branch, which is
based on 'core-updates' with recent 'staging' and 'master' merged in.
I ran into another problem along the way. I found that after adding the
duplicate field detection to (guix records), building Guix from a clean
tree started failing with an apparently unrelated error. When the code
in build-aux/compile-all.scm attempted to _load_ (guix scripts pack), it
hit a fatal error, namely that 'gzip' was undefined, although it's
clearly importing the right module. I guess this is somehow related to
the cycles in our module dependency graph. I found that this problem
could be prevented by moving $(GNU_SYSTEM_MODULES) above the modules in
guix/{import,scripts} in MODULES in Makefile.am. The idea is that
modules in guix/{import,scripts} sometimes import package modules, but
never the other way around.
I've attached three draft patches below. The first applies the
aforementioned workaround in Makefile.am. The second fixes the 9
occurrences of duplicate field initializers in my private branch. The
third modifies (guix records) to raise an error if duplicate fields are
found.
Comments and suggestions welcome.
Mark
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH 1/3] DRAFT: build: Load $(GNU_SYSTEM_MODULES) before guix/{import,scripts} --]
[-- Type: text/x-patch, Size: 1128 bytes --]
From 5e4422d81d4fd5581bce8f8b29f4c75864e37bd0 Mon Sep 17 00:00:00 2001
From: Mark H Weaver <mhw@netris.org>
Date: Thu, 19 Apr 2018 16:18:26 -0400
Subject: [PATCH 1/3] DRAFT: build: Load $(GNU_SYSTEM_MODULES) before
guix/{import,scripts}.
This works around an issue where modules in guix/import and guix/scripts
sometimes depend on package definitions at module load time.
* Makefile.am (MODULES): Move $(GNU_SYSTEM_MODULES) above guix/import/* and
guix/scripts/*.
---
Makefile.am | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 86528e8fd..c6dca942f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -174,6 +174,7 @@ MODULES = \
guix/build/make-bootstrap.scm \
guix/search-paths.scm \
guix/packages.scm \
+ $(GNU_SYSTEM_MODULES) \
guix/import/print.scm \
guix/import/utils.scm \
guix/import/gnu.scm \
@@ -214,8 +215,7 @@ MODULES = \
guix/scripts/graph.scm \
guix/scripts/container.scm \
guix/scripts/container/exec.scm \
- guix.scm \
- $(GNU_SYSTEM_MODULES)
+ guix.scm
if HAVE_GUILE_JSON
--
2.17.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: [PATCH 2/3] DRAFT: Fix duplicate field initializers in guix record constructors --]
[-- Type: text/x-patch, Size: 6358 bytes --]
From 907cd4b4a485fbce7662c3149d8d4eeb0b4e7d0d Mon Sep 17 00:00:00 2001
From: Mark H Weaver <mhw@netris.org>
Date: Thu, 19 Apr 2018 16:41:45 -0400
Subject: [PATCH 2/3] DRAFT: Fix duplicate field initializers in guix record
constructors.
---
gnu/packages/bittorrent.scm | 2 --
gnu/packages/gcc.scm | 4 ++--
gnu/packages/glib.scm | 9 ++++-----
gnu/packages/haskell.scm | 3 +++
gnu/packages/python.scm | 4 ++++
gnu/packages/syncthing.scm | 2 --
gnu/packages/web.scm | 4 ++++
gnu/tests/install.scm | 1 -
8 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/gnu/packages/bittorrent.scm b/gnu/packages/bittorrent.scm
index ae9b3bc62..9df4f097a 100644
--- a/gnu/packages/bittorrent.scm
+++ b/gnu/packages/bittorrent.scm
@@ -315,8 +315,6 @@ Aria2 can be manipulated via built-in JSON-RPC and XML-RPC interfaces.")
(base32
"0919cf7lfk1djdl003cahqjvafdliv7v2l8r5wg95n4isqggdk75"))))
(build-system gnu-build-system)
- (native-inputs
- `(("intltool" ,intltool)))
(inputs
`(("curl" ,curl)
("gtk+" ,gtk+)
diff --git a/gnu/packages/gcc.scm b/gnu/packages/gcc.scm
index 3a34ffbb3..741cfab06 100644
--- a/gnu/packages/gcc.scm
+++ b/gnu/packages/gcc.scm
@@ -145,11 +145,11 @@ where the OS part is overloaded to denote a specific ABI---into GCC
(method url-fetch)
(uri (string-append "mirror://gnu/gcc/gcc-"
version "/gcc-" version ".tar.bz2"))
- (patches (search-patches "gcc-4-compile-with-gcc-5.patch"))
(sha256
(base32
"10k2k71kxgay283ylbbhhs51cl55zn2q38vj5pk4k950qdnirrlj"))
- (patches (search-patches "gcc-fix-texi2pod.patch"))))
+ (patches (search-patches "gcc-4-compile-with-gcc-5.patch"
+ "gcc-fix-texi2pod.patch"))))
(build-system gnu-build-system)
;; Separate out the run-time support libraries because all the
diff --git a/gnu/packages/glib.scm b/gnu/packages/glib.scm
index dfa7b06a8..25d66ec3d 100644
--- a/gnu/packages/glib.scm
+++ b/gnu/packages/glib.scm
@@ -321,10 +321,6 @@ dynamic loading, and an object system.")
"gobject-introspection-girepository.patch"
"gobject-introspection-absolute-shlib-path.patch"))))
(build-system gnu-build-system)
- (arguments
- ;; The build system has at least one race condition involving Gio-2.0.gir
- ;; which causes intermittent failures, as of 1.56.0.
- `(#:parallel-build? #f))
(inputs
`(("bison" ,bison)
("flex" ,flex)
@@ -343,7 +339,10 @@ dynamic loading, and an object system.")
(files '("lib/girepository-1.0")))))
(search-paths native-search-paths)
(arguments
- `(;; The patch 'gobject-introspection-absolute-shlib-path.patch' causes
+ `(;; The build system has at least one race condition involving Gio-2.0.gir
+ ;; which causes intermittent failures, as of 1.56.0.
+ #:parallel-build? #f
+ ;; The patch 'gobject-introspection-absolute-shlib-path.patch' causes
;; some tests to fail.
#:tests? #f))
(home-page "https://wiki.gnome.org/GObjectIntrospection")
diff --git a/gnu/packages/haskell.scm b/gnu/packages/haskell.scm
index f2c546d08..442d48dd0 100644
--- a/gnu/packages/haskell.scm
+++ b/gnu/packages/haskell.scm
@@ -3139,6 +3139,9 @@ writing to stdout and other handles.")
(base32
"1j6ahvrz1g5q89y2difyk838yhwjc8z67zr0v2z512qdznc3h38n"))))
(build-system haskell-build-system)
+ ;; FIXME: Determine whether the following redundant 'inputs' initializer,
+ ;; which had been ignored, should be incorporated in the other 'inputs'
+ #;
(inputs
`(("ghc-hunit" ,ghc-hunit)))
;; these inputs are necessary to use this library
diff --git a/gnu/packages/python.scm b/gnu/packages/python.scm
index 61a7c2f73..a8cbf71a3 100644
--- a/gnu/packages/python.scm
+++ b/gnu/packages/python.scm
@@ -3609,6 +3609,10 @@ toolkits.")
`(("python2-numpy" ,python2-numpy)
("python2-scipy" ,python2-scipy)
("python2-pandas" ,python2-pandas)))
+ ;; FIXME: Determine whether the following redundant 'native-inputs'
+ ;; initializer, which had been ignored, should be incorporated in the
+ ;; other 'native-inputs'.
+ #;
(native-inputs
`(("python2-cython" ,python2-cython)))
(native-inputs
diff --git a/gnu/packages/syncthing.scm b/gnu/packages/syncthing.scm
index 93390df94..0a90610ec 100644
--- a/gnu/packages/syncthing.scm
+++ b/gnu/packages/syncthing.scm
@@ -860,8 +860,6 @@ implements arithmetic over the Galois Field GF(256).")
`(#:import-path "github.com/vitrun/qart/qr"
#:unpack-path "github.com/vitrun/qart"))
(synopsis "Qart component for generating QR codes")
- (description "This package, a component of @code{qart}, provides
-@code{qr}, for QR code generation.")
(description "This package provides a library for embedding
human-meaningful graphics in QR codes. However, instead of scribbling on
redundant pieces and relying on error correction to preserve the meaning,
diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
index 88c3b4aa1..8a06ce1ca 100644
--- a/gnu/packages/web.scm
+++ b/gnu/packages/web.scm
@@ -3422,6 +3422,10 @@ either mocked HTTP or a locally spawned server.")
(base32
"1d11fx9155d5v17d5w7q3kj37b01l8yj2yb0g6b0z1vh938rrlcr"))))
(build-system perl-build-system)
+ ;; FIXME: Determine whether the following redundant 'native-inputs'
+ ;; initializer, which had been ignored, should be incorporated in the
+ ;; other 'native-inputs'.
+ #;
(native-inputs
`(("perl-test-exception" ,perl-test-exception)))
(native-inputs
diff --git a/gnu/tests/install.scm b/gnu/tests/install.scm
index e3bb1b46a..f72eae7f5 100644
--- a/gnu/tests/install.scm
+++ b/gnu/tests/install.scm
@@ -435,7 +435,6 @@ reboot\n")
(file-system
(device "none")
(title 'device)
- (type "tmpfs")
(mount-point "/home")
(type "tmpfs"))
%base-file-systems))
--
2.17.0
[-- Attachment #4: [PATCH 3/3] DRAFT: records: Detect duplicate field initializers --]
[-- Type: text/x-patch, Size: 3916 bytes --]
From 45e26da1e4c8559b843034de3fd2edef89f5349c Mon Sep 17 00:00:00 2001
From: Mark H Weaver <mhw@netris.org>
Date: Thu, 19 Apr 2018 12:33:25 -0400
Subject: [PATCH 3/3] DRAFT: records: Detect duplicate field initializers.
---
guix/records.scm | 33 +++++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/guix/records.scm b/guix/records.scm
index c02395f2a..d6f97b288 100644
--- a/guix/records.scm
+++ b/guix/records.scm
@@ -1,5 +1,6 @@
;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2018 Mark H Weaver <mhw@netris.org>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -74,9 +75,13 @@ fields, and DELAYED is the list of identifiers of delayed fields."
field+value)
car))
- ;; Make sure there are no unknown field names.
+ ;; Make sure there are no duplicates, and no unknown field names.
(let* ((fields (map (compose car syntax->datum) field+value))
+ (duplicates (find-duplicates fields))
(unexpected (lset-difference eq? fields '(expected ...))))
+ (when (pair? duplicates)
+ (record-error 'name s "duplicate field initializers ~a"
+ duplicates))
(when (pair? unexpected)
(record-error 'name s "extraneous field initializers ~a"
unexpected)))
@@ -127,23 +132,39 @@ fields, and DELAYED is the list of identifiers of delayed fields."
#,(wrap-field-value #'field #'value)))))
field+value))
+ (define (find-duplicates lst)
+ ;; Return all elements of LST that occur more than once.
+ ;; Elements are compared using 'eq?'.
+ (match lst
+ ((x . rest)
+ (if (memq x rest)
+ (lset-adjoin eq? (find-duplicates rest) x)
+ (find-duplicates rest)))
+ (()
+ '())))
+
(syntax-case s (inherit expected ...)
((_ (inherit orig-record) (field value) (... ...))
#`(let* #,(field-bindings #'((field value) (... ...)))
#,(record-inheritance #'orig-record
#'((field value) (... ...)))))
((_ (field value) (... ...))
- (let ((fields (map syntax->datum #'(field (... ...)))))
+ (let ()
(define (field-value f)
(or (find (lambda (x)
(eq? f (syntax->datum x)))
#'(field (... ...)))
(wrap-field-value f (field-default-value f))))
- (let ((fields (append fields (map car default-values))))
- (cond ((lset= eq? fields '(expected ...))
- #`(let* #,(field-bindings
- #'((field value) (... ...)))
+ (let* ((provided-fields (map syntax->datum #'(field (... ...))))
+ (duplicates (find-duplicates provided-fields))
+ (fields (append provided-fields (map car default-values))))
+ (cond ((pair? duplicates)
+ (record-error 'name s
+ "duplicate field initializers ~a"
+ duplicates))
+ ((lset= eq? fields '(expected ...))
+ #`(let* #,(field-bindings #'((field value) (... ...)))
(ctor #,@(map field-value '(expected ...)))))
((pair? (lset-difference eq? fields
'(expected ...)))
--
2.17.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: Detecting duplicate field initializers in guix record constructors
2018-04-21 8:16 Detecting duplicate field initializers in guix record constructors Mark H Weaver
@ 2018-04-22 20:01 ` Ludovic Courtès
0 siblings, 0 replies; 2+ messages in thread
From: Ludovic Courtès @ 2018-04-22 20:01 UTC (permalink / raw)
To: Mark H Weaver; +Cc: guix-devel
Hello!
Mark H Weaver <mhw@netris.org> skribis:
> Recently, when doing a merge of 'master' into 'core-updates', I noticed
> that git's automatic merging sometimes results in duplicate field
> initializers being introduced, without any merge conflict being
> reported. This happens when a field is introduced independently in both
> 'core-updates' and 'master', but in different places within the
> constructor.
>
> So, I implemented duplicate field detection in (guix records).
> See below for my draft patches.
Excellent, I’ve been missing this for too long. :-)
> This revealed 9 occurrences of this error in my private branch, which is
> based on 'core-updates' with recent 'staging' and 'master' merged in.
Woow.
> I ran into another problem along the way. I found that after adding the
> duplicate field detection to (guix records), building Guix from a clean
> tree started failing with an apparently unrelated error. When the code
> in build-aux/compile-all.scm attempted to _load_ (guix scripts pack), it
> hit a fatal error, namely that 'gzip' was undefined, although it's
> clearly importing the right module. I guess this is somehow related to
> the cycles in our module dependency graph. I found that this problem
> could be prevented by moving $(GNU_SYSTEM_MODULES) above the modules in
> guix/{import,scripts} in MODULES in Makefile.am. The idea is that
> modules in guix/{import,scripts} sometimes import package modules, but
> never the other way around.
(Note: this was reported at <https://bugs.gnu.org/29774>.)
I still don’t quite get it. The #:use-module (gnu packages compression)
in (guix scripts pack) should lead to loading things in the right order,
no? Do you have a way to reproduce it?
> From 5e4422d81d4fd5581bce8f8b29f4c75864e37bd0 Mon Sep 17 00:00:00 2001
> From: Mark H Weaver <mhw@netris.org>
> Date: Thu, 19 Apr 2018 16:18:26 -0400
> Subject: [PATCH 1/3] DRAFT: build: Load $(GNU_SYSTEM_MODULES) before
> guix/{import,scripts}.
>
> This works around an issue where modules in guix/import and guix/scripts
> sometimes depend on package definitions at module load time.
>
> * Makefile.am (MODULES): Move $(GNU_SYSTEM_MODULES) above guix/import/* and
> guix/scripts/*.
Let’s discuss this separately for 29774.
> From 907cd4b4a485fbce7662c3149d8d4eeb0b4e7d0d Mon Sep 17 00:00:00 2001
> From: Mark H Weaver <mhw@netris.org>
> Date: Thu, 19 Apr 2018 16:41:45 -0400
> Subject: [PATCH 2/3] DRAFT: Fix duplicate field initializers in guix record
> constructors.
LGTM!
> From 45e26da1e4c8559b843034de3fd2edef89f5349c Mon Sep 17 00:00:00 2001
> From: Mark H Weaver <mhw@netris.org>
> Date: Thu, 19 Apr 2018 12:33:25 -0400
> Subject: [PATCH 3/3] DRAFT: records: Detect duplicate field initializers.
LGTM too. Could you add a test in tests/records.scm? There’s already a
couple of ‘syntax-error’ tests there.
Thank you!
Ludo’.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-04-22 20:01 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-21 8:16 Detecting duplicate field initializers in guix record constructors Mark H Weaver
2018-04-22 20:01 ` Ludovic Courtès
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).