[PATCH] Move diffoscope from package-management to it's own module.

  • Done
  • quality assurance status badge
Details
3 participants
  • Danny Milosavljevic
  • Ludovic Courtès
  • Vagrant Cascadian
Owner
unassigned
Submitted by
Vagrant Cascadian
Severity
normal
V
V
Vagrant Cascadian wrote on 9 Sep 2019 01:35
(address . guix-patches@gnu.org)
87ftl6cqfc.fsf@yucca
I was having infinite recursion issues importing additional modules such
as android.scm, bootloaders.scm and statistics.scm into
package-management.scm with use-modules calls, along these lines:

error: googletest: unbound variable
hint: Did you forget a `use-modules' form?
...
error: curl: unbound variable
hint: Did you forget a `use-modules' form?

guix build: error: diffoscope: unknown package

Moving diffoscope to it's own package module seemed to at least work
around the issue. Diffoscope itself attempts to deal with an arbitrary
and growing number of file types, so pulls in quite a few other package
modules, so at least splitting it into a separate module might limit the
impacts on other modules.

Attached are two patches attempting the split; comments welcome!

The first patch is the move itself, and also moves trydiffoscope; the
second patch adds the package modules that previously would fail to
import properly before the move.

This allows diffoscope to run a few additional tests, getting the test
suite down to only about 68 skipped tests (and falling); it was around
140 skipped tests before I started this mad rush...


live well,
vagrant
From c4ef8545514b4d594ab6fc083c954a22eace3786 Mon Sep 17 00:00:00 2001
From: Vagrant Cascadian <vagrant@reproducible-builds.org>
Date: Sun, 8 Sep 2019 15:35:33 -0700
Subject: [PATCH 2/2] gnu: diffoscope: Add additional test dependencies.

* gnu/packages/diffoscope (diffoscope)[native-inputs]: Add abootimg, dtc,
and r-minimal.
(use-module): Add android, bootloaders and statistics, respectively.
---
gnu/packages/diffoscope.scm | 6 ++++++
1 file changed, 6 insertions(+)

Toggle diff (49 lines)
diff --git a/gnu/packages/diffoscope.scm b/gnu/packages/diffoscope.scm
index 6eb5c1d9fe..828e06a818 100644
--- a/gnu/packages/diffoscope.scm
+++ b/gnu/packages/diffoscope.scm
@@ -25,8 +25,10 @@
#:use-module (gnu packages)
#:use-module (gnu packages acl)
#:use-module (gnu packages admin)
+ #:use-module (gnu packages android)
#:use-module (gnu packages backup)
#:use-module (gnu packages base)
+ #:use-module (gnu packages bootloaders)
#:use-module (gnu packages cdrom)
#:use-module (gnu packages check)
#:use-module (gnu packages compression)
@@ -50,6 +52,7 @@
#:use-module (gnu packages python-xyz)
#:use-module (gnu packages sqlite)
#:use-module (gnu packages ssh)
+ #:use-module (gnu packages statistics)
#:use-module (gnu packages textutils)
#:use-module (gnu packages video)
#:use-module (gnu packages vim)
@@ -137,6 +140,7 @@
(native-inputs `(("python-pytest" ,python-pytest)
("python-chardet" ,python-chardet)
;; test suite skips tests when tool is missing
+ ("abootimg" ,abootimg)
("bdb" ,bdb)
("binutils" ,binutils)
("bzip2" ,bzip2)
@@ -144,6 +148,7 @@
("colord" ,colord)
("cpio" ,cpio)
("docx2txt" ,docx2txt)
+ ("dtc" ,dtc)
("e2fsprogs" ,e2fsprogs)
("ffmpeg" ,ffmpeg)
("gettext" ,gettext-minimal)
@@ -163,6 +168,7 @@
("openssh" ,openssh)
("pgpdump" ,pgpdump)
("poppler" ,poppler)
+ ("r-minimal" ,r-minimal)
("rpm" ,rpm)
("sng" ,sng)
("sqlite" ,sqlite)
--
2.20.1
-----BEGIN PGP SIGNATURE-----

iHUEARYKAB0WIQRlgHNhO/zFx+LkXUXcUY/If5cWqgUCXXWQOAAKCRDcUY/If5cW
quCCAQCWSRWA/yRO9MzMkYpl78tzViv9ou9ugb06aSnKD2Ec2AEA3Zfj208QJGcb
0gj/xES0lSZgr31Z1lO1mn88NoeFlAM=
=U9Uj
-----END PGP SIGNATURE-----

D
D
Danny Milosavljevic wrote on 11 Sep 2019 01:46
(name . Vagrant Cascadian)(address . vagrant@reproducible-builds.org)(address . 37346-done@debbugs.gnu.org)
20190911014625.529c1738@scratchpost.org
Hi Vagrant,

On Sun, 08 Sep 2019 16:35:19 -0700
Vagrant Cascadian <vagrant@reproducible-builds.org> wrote:

Toggle quote (4 lines)
> I was having infinite recursion issues importing additional modules such
> as android.scm, bootloaders.scm and statistics.scm into
> package-management.scm

Yeah, I hate import cycles.

I've pushed your patches to guix master.

Thanks!
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEds7GsXJ0tGXALbPZ5xo1VCwwuqUFAl14NdEACgkQ5xo1VCww
uqXBJAf7BvUmLQfTeiXkFv1uRUrtU9FfobcO96sJelbWlYYto/nEqzS1uvh8AUWx
S/ZKkyYikSsVEBoFkR2Xwfw8ZiC/808AmvzBLgSXjc6iDnvCGbMdFJVshCtvu9Ai
aVIw7EmM6LnOWm2NJtITEhOlnUlXAtams4ThSHG8BEzSooD9RW/xlSjooKBstI4n
ONPKNZXLb6GsYTqkHpns3IRSlwpwRlSNetx+Wzjne/mv3A9BbcfdUHwP+yVSJTvQ
crXl1mh9omcyzvBXQguQVvXp8x7y4EeAY+vsBwW3/JlKhtFmJw/Of1Z2UOH8RQNd
lsbUL3fP5yYoV69sDO81ebCQBAW3Ow==
=qgW+
-----END PGP SIGNATURE-----


Closed
L
L
Ludovic Courtès wrote on 11 Sep 2019 14:13
(name . Vagrant Cascadian)(address . vagrant@reproducible-builds.org)(address . 37346@debbugs.gnu.org)
87woefvxn1.fsf@gnu.org
Hello!

Vagrant Cascadian <vagrant@reproducible-builds.org> skribis:

Toggle quote (7 lines)
> I was having infinite recursion issues importing additional modules such
> as android.scm, bootloaders.scm and statistics.scm into
> package-management.scm with use-modules calls, along these lines:
>
> error: googletest: unbound variable
> hint: Did you forget a `use-modules' form?

Circular module dependencies alone shouldn’t cause these problems.
Problems arise when those modules that depend on each other refer to
variables exported by one another *at the top level*.

That typically happens if, say, in android.scm you’d do:

(package (inherit diffoscope) …)

Toggle quote (8 lines)
> Moving diffoscope to it's own package module seemed to at least work
> around the issue. Diffoscope itself attempts to deal with an arbitrary
> and growing number of file types, so pulls in quite a few other package
> modules, so at least splitting it into a separate module might limit the
> impacts on other modules.
>
> Attached are two patches attempting the split; comments welcome!

The split is a good idea anyhow!

Toggle quote (14 lines)
> From 352058f2ef002e77df6633c00ba009088bf5e8bd Mon Sep 17 00:00:00 2001
> From: Vagrant Cascadian <vagrant@reproducible-builds.org>
> Date: Sun, 8 Sep 2019 14:36:33 -0700
> Subject: [PATCH 1/2] gnu: Move diffoscope and trydiffoscope to new
> diffoscope.scm.
>
> * gnu/packages/package-management (diffoscope): Remove variable.
> (trydiffoscope): Remove variable.
> (use-modules): Remove modules only needed by diffoscope.
> Update copyright information.
> * gnu/packages/diffoscope.scm: New file.
> (diffoscope): Add variable.
> (trydiffoscope): Add variable.

LGTM.

Toggle quote (9 lines)
> From c4ef8545514b4d594ab6fc083c954a22eace3786 Mon Sep 17 00:00:00 2001
> From: Vagrant Cascadian <vagrant@reproducible-builds.org>
> Date: Sun, 8 Sep 2019 15:35:33 -0700
> Subject: [PATCH 2/2] gnu: diffoscope: Add additional test dependencies.
>
> * gnu/packages/diffoscope (diffoscope)[native-inputs]: Add abootimg, dtc,
> and r-minimal.
> (use-module): Add android, bootloaders and statistics, respectively.

Note that we don’t usually mention ‘use-module’ changes in commit logs.

Anyway, LGTM, thanks!

Ludo’.
V
V
Vagrant Cascadian wrote on 18 Sep 2019 19:45
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)(address . 37346@debbugs.gnu.org)
87d0fxwlav.fsf@yucca
On 2019-09-11, Danny Milosavljevic wrote:
Toggle quote (11 lines)
> On Sun, 08 Sep 2019 16:35:19 -0700
> Vagrant Cascadian <vagrant@reproducible-builds.org> wrote:
>
>> I was having infinite recursion issues importing additional modules such
>> as android.scm, bootloaders.scm and statistics.scm into
>> package-management.scm
>
> Yeah, I hate import cycles.
>
> I've pushed your patches to guix master.

They don't seem to be in master as of commit:

cf48ea9539020bd6300033a8910ca951225582e6

I don't see any reverts, so curious what happened?


live well,
vagrant
-----BEGIN PGP SIGNATURE-----

iHUEARYKAB0WIQRlgHNhO/zFx+LkXUXcUY/If5cWqgUCXYJtKAAKCRDcUY/If5cW
qg6FAQCBpXKrOlAs9Cq2DiYBLsgBSkwdiNzR+fxTW4/yx+wnkAD/d5LHdtkBiTS8
BLpTayESWFfq95kyeP0TQ/JCP+sDDQ0=
=5ZUT
-----END PGP SIGNATURE-----

V
V
Vagrant Cascadian wrote on 19 Sep 2019 02:30
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)
874l19w2j8.fsf@yucca
On 2019-09-18, Vagrant Cascadian wrote:
Toggle quote (18 lines)
> On 2019-09-11, Danny Milosavljevic wrote:
>> On Sun, 08 Sep 2019 16:35:19 -0700
>> Vagrant Cascadian <vagrant@reproducible-builds.org> wrote:
>>
>>> I was having infinite recursion issues importing additional modules such
>>> as android.scm, bootloaders.scm and statistics.scm into
>>> package-management.scm
>>
>> Yeah, I hate import cycles.
>>
>> I've pushed your patches to guix master.
>
> They don't seem to be in master as of commit:
>
> cf48ea9539020bd6300033a8910ca951225582e6
>
> I don't see any reverts, so curious what happened?

I went ahead and (re?)pushed it, dropping the use-modules bits based on
ludo's review.

live well,
vagrant
-----BEGIN PGP SIGNATURE-----

iHUEARYKAB0WIQRlgHNhO/zFx+LkXUXcUY/If5cWqgUCXYLMLAAKCRDcUY/If5cW
quhQAPwJTRsjznxfzxDnQPt/yhq5uKKZKZvlF1yt1Hgcdyp7tQD/bOv/Un16pqA1
QFRWRouGfNOOAK7QYZpsRRNw8TlccgY=
=onoa
-----END PGP SIGNATURE-----

D
D
Danny Milosavljevic wrote on 19 Sep 2019 16:04
(name . Vagrant Cascadian)(address . vagrant@reproducible-builds.org)
20190919160404.62a8bbae@scratchpost.org
Hi Vagrant,

On Wed, 18 Sep 2019 17:30:35 -0700
Vagrant Cascadian <vagrant@reproducible-builds.org> wrote:

Toggle quote (3 lines)
> I went ahead and (re?)pushed it, dropping the use-modules bits based on
> ludo's review.

Thanks. Not sure what happened, but I've checked it now, seems to be OK.
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEds7GsXJ0tGXALbPZ5xo1VCwwuqUFAl2DitQACgkQ5xo1VCww
uqVrZwf+NEoQXnDoWnwP2zWSbNWUjiQhqPwvTo3MzozRrVPdxArc+baI8Q+EkQaZ
UHKzsJEPnFCTuark32W7735K5r5LI8Fi6+UxriyKNjvs/RSQq73lF+74c0np2J24
Nng7CZ8Y0cznPlAY/lmPq6TkMBoEFTvxYA595JFWxD5ACdDiLbUDeErZR0pLQaWe
p8O4RiSihhOrae3jM1iPaYxbag1ozakxQPZstxL+9tHBZCQ/xrnWfmf0zTdczkJ1
5QkhKpKkJQ8Q1Vjqp8hKaWHGGePOILisEL6MjTKtSFeH2P+lvpiz+9sA5h2DuicY
QbhmWF6cHUYhiz0AVYqlvU2XuHxegg==
=ze+e
-----END PGP SIGNATURE-----


?