[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Detecting duplicate field initializers in guix record constructors
|
From: |
Mark H Weaver |
|
Subject: |
Detecting duplicate field initializers in guix record constructors |
|
Date: |
Sat, 21 Apr 2018 04:16:27 -0400 |
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
>From 5e4422d81d4fd5581bce8f8b29f4c75864e37bd0 Mon Sep 17 00:00:00 2001
From: Mark H Weaver <address@hidden>
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
>From 907cd4b4a485fbce7662c3149d8d4eeb0b4e7d0d Mon Sep 17 00:00:00 2001
From: Mark H Weaver <address@hidden>
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
address@hidden, 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
>From 45e26da1e4c8559b843034de3fd2edef89f5349c Mon Sep 17 00:00:00 2001
From: Mark H Weaver <address@hidden>
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
<address@hidden>
+;;; Copyright © 2018 Mark H Weaver <address@hidden>
;;;
;;; 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
- Detecting duplicate field initializers in guix record constructors,
Mark H Weaver <=