Re: Dependency cycle issues when using a Gexp-based snippet

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

Hi Ludovic!

Thank you for the reply.

Ludovic Courtès <ludo@gnu.org> writes:

> Hello!
>
> maxim.cournoyer@gmail.com skribis:
>
>> While trying to move some of the patching done to qtbase into a snippet,
>> with the goal of having at least the ./configure script runnable in a
>> guix environment without having to manually run patching phases:
>
> [...]
>
>> I encountered the following issue, which seems similar to one
>> encountered by Ricardo in 2016 [0]:
>>
>> ice-9/eval.scm:293:34: error: canonical-package: unbound variable
>> hint: Did you forget a `use-modules' form?
>>
>>
>> The origin can be correctly built at the REPL, so the problem indeed
>> seems to be a dependency cycle.
>
> Indeed: the problem is that when loading this module, we try to resolve
> one of the variables referenced in the snippet, but that variable is not
> defined yet because it comes from a module that’s in a dependency circle
> with the one at hand.
>
>> Attempting a suggested fix by Ludovic in that same conversation [0],
>> namely, making the snippet field of the <origin> record a thunked one:
>>
>> modified   guix/packages.scm
>> @@ -250,7 +250,8 @@ as base32.  Otherwise, it must be a bytevector."
>>    (patches   origin-patches                       ; list of file names
>>               (default '()) (delayed))
>>
>> -  (snippet   origin-snippet (default #f))         ; sexp or #f
>> +  (snippet   origin-snippet
>> +             (default #f) (thunked))              ; sexp or #f
>>    (patch-flags  origin-patch-flags                ; list of strings
>>                  (default '("-p1")))
>
> We should check what this change costs in CPU and memory, but it’s
> probably worth it.  As Marius noted before, the snippets for
> ungoogled-chromium and linux-libre are contrived because of this
> limitation.  (Perhaps we can use ‘delayed’ instead of ‘thunked’.)

What is the difference between delayed and thunked? Would a thunked
capture the closure of its environment while delayed not?  Is the
closure useful to access record-bound values such as the version field
of a package?

I checked the usage at compilation and run time, using the 'time'
command (aliased to time+ on my system), and didn't find any meaningful
difference whether the snippet is made a thunked or delayed field, or
none (current situation):

On current master:

--8<---------------cut here---------------start------------->8---
time+ make -j8
2436.29user 56.47system 14:29.36elapsed 286%CPU (0avgtext+0avgdata 870828maxresident)k
5480inputs+405952outputs (71major+320522minor)pagefaults 0swaps

time+ ./pre-inst-env guix package -A | wc -l
9.87user 0.24system 0:06.51elapsed 155%CPU (0avgtext+0avgdata 281564maxresident)k
0inputs+0outputs (0major+25636minor)pagefaults 0swaps
14702
--8<---------------cut here---------------end--------------->8---

With delayed source field:

--8<---------------cut here---------------start------------->8---
time+ make -j8
2493.40user 51.16system 16:29.33elapsed 257%CPU (0avgtext+0avgdata 842008maxresident)k
48888inputs+413640outputs (158major+340632minor)pagefaults 0swaps

time+ ./pre-inst-env guix package -A | wc -l
9.68user 0.26system 0:06.68elapsed 148%CPU (0avgtext+0avgdata 293488maxresident)k
1008inputs+0outputs (7major+42943minor)pagefaults 0swaps
14702
--8<---------------cut here---------------end--------------->8---

And finally with thunked source field:
--8<---------------cut here---------------start------------->8---
time+ make -j8
2302.67user 27.44system 22:32.82elapsed 172%CPU (0avgtext+0avgdata 862336maxresident)k
2960inputs+416688outputs (51major+300689minor)pagefaults 0swaps

time+ ./pre-inst-env guix package -A | wc -l
10.02user 0.23system 0:07.46elapsed 137%CPU (0avgtext+0avgdata 283052maxresident)k
2984inputs+0outputs (101major+24685minor)pagefaults 0swaps
14702
--8<---------------cut here---------------end--------------->8---

Based on this, I would choose the most capable 'mode' for the source
field, which I presume is thunked.

>> It now seems a new cycle is introduced because trying to build anything
>> hangs using the CPU with slowly increasing memory usage:
>
> Hmm not sure exactly why, but look:
>
> +              (snippet
> +               (with-imported-modules '((guix build utils))
> +                 #~(begin
> +                     (use-modules (guix build utils))
> +                     ;; corelib uses bundled harfbuzz, md4, md5, sha3
> +                     (with-directory-excursion "src/3rdparty"
> +                       (for-each delete-file-recursively
> +                                 (list "double-conversion" "freetype" "harfbuzz-ng"
> +                                       "libpng" "libjpeg" "pcre2" "sqlite" "xcb"
> +                                       "zlib")))
> +
> +                     (let ((coreutils #+(canonical-package coreutils)))
> +                       (substitute* "configure"
> +                         (("/bin/pwd")
> +                          (string-append coreutils "/bin/pwd")))
> +                       (substitute* "src/corelib/global/global.pri"
> +                         (("/bin/ls")
> +                          (string-append coreutils "/bin/ls"))))
> +                     #t)))))
>
> Such substitutions are system-dependent; thus, they should be made in a
> phase, not in a snippet.  Perhaps we’ll sidestep the issue altogether?
> :-)

Indeed.  I didn't consider this aspect well.  Apart from being
inefficient (the sources of a package would be different for each
system) it would still technically work, no?

But yeah, that bloat is probably not worth the extra convenience (of
having a source that can be built straight from 'guix build --source').

Thanks,

Maxim