Re: Detecting duplicate field initializers in guix record constructors

[Ludovic Courtès]

Hello!

Mark H Weaver <address@hidden> 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 <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/*.

Let’s discuss this separately for 29774.

> 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.

LGTM!

> 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.

LGTM too.  Could you add a test in tests/records.scm?  There’s already a
couple of ‘syntax-error’ tests there.

Thank you!

Ludo’.