| Message ID | 87ftl6cqfc.fsf@yucca |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#37346] Move diffoscope from package-management to it's own module. | expand |
Hi Vagrant, 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. Thanks!
Hello! Vagrant Cascadian <vagrant@reproducible-builds.org> skribis: > 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) …) > 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! > 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. > 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’.
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? live well, vagrant
On 2019-09-18, Vagrant Cascadian wrote: > 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
Hi Vagrant, On Wed, 18 Sep 2019 17:30:35 -0700 Vagrant Cascadian <vagrant@reproducible-builds.org> wrote: > 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.
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(+) 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