Detecting duplicate field initializers in guix record constructors

[Mark H Weaver]

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

[Ludovic Courtès]

Hello!

Post by Mark H Weaver
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. :-)

Post by Mark H Weaver
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.

Post by Mark H Weaver
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?

Post by Mark H Weaver
From 5e4422d81d4fd5581bce8f8b29f4c75864e37bd0 Mon Sep 17 00:00:00 2001
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.

Post by Mark H Weaver
From 907cd4b4a485fbce7662c3149d8d4eeb0b4e7d0d Mon Sep 17 00:00:00 2001
Date: Thu, 19 Apr 2018 16:41:45 -0400
Subject: [PATCH 2/3] DRAFT: Fix duplicate field initializers in guix record
constructors.

LGTM!

Post by Mark H Weaver
From 45e26da1e4c8559b843034de3fd2edef89f5349c Mon Sep 17 00:00:00 2001
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’.