Dependency cycle issues when using a Gexp-based snippet

Dependency cycle issues when using a Gexp-based snippet

[maxim.cournoyer]

Hello,

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:

--8<---------------cut here---------------start------------->8---
modified   gnu/packages/qt.scm
@@ -45,6 +45,7 @@
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system trivial)
   #:use-module (guix build-system python)
+  #:use-module (guix gexp)
   #:use-module (guix packages)
   #:use-module (guix deprecation)
   #:use-module (guix utils)
@@ -349,27 +350,36 @@ developers using C++ or QML, a CSS & JavaScript like language.")
     ;; TODO Remove ((gnu packages kde) qtbase-for-krita) when upgrading qtbase.
     (version "5.14.2")
     (source (origin
-             (method url-fetch)
-             (uri (string-append "https://download.qt.io/official_releases/qt/"
-                                 (version-major+minor version) "/" version
-                                 "/submodules/" name "-everywhere-src-"
-                                 version ".tar.xz"))
-             (sha256
-              (base32
-               "12mjsahlma9rw3vz9a6b5h2s6ylg8b34hxc2vnlna5ll429fgfa8"))
-             ;; Use TZDIR to avoid depending on package "tzdata".
-             (patches (search-patches "qtbase-use-TZDIR.patch"
-                                      "qtbase-moc-ignore-gcc-macro.patch"))
-             (modules '((guix build utils)))
-             (snippet
-               ;; corelib uses bundled harfbuzz, md4, md5, sha3
-              '(begin
-                (with-directory-excursion "src/3rdparty"
-                  (for-each delete-file-recursively
-                            (list "double-conversion" "freetype" "harfbuzz-ng"
-                                  "libpng" "libjpeg" "pcre2" "sqlite" "xcb"
-                                  "zlib"))
-                  #t)))))
+              (method url-fetch)
+              (uri (string-append "https://download.qt.io/official_releases/qt/"
+                                  (version-major+minor version) "/" version
+                                  "/submodules/" name "-everywhere-src-"
+                                  version ".tar.xz"))
+              (sha256
+               (base32
+                "12mjsahlma9rw3vz9a6b5h2s6ylg8b34hxc2vnlna5ll429fgfa8"))
+              ;; Use TZDIR to avoid depending on package "tzdata".
+              (patches (search-patches "qtbase-use-TZDIR.patch"
+                                       "qtbase-moc-ignore-gcc-macro.patch"))
+              (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)))))
     (build-system gnu-build-system)
     (propagated-inputs
      `(("mesa" ,mesa)
--8<---------------cut here---------------end--------------->8---

I encountered the following issue, which seems similar to one
encountered by Ricardo in 2016 [0]:

--8<---------------cut here---------------start------------->8---
ice-9/eval.scm:293:34: error: canonical-package: unbound variable
hint: Did you forget a `use-modules' form?
--8<---------------cut here---------------end--------------->8---

The origin can be correctly built at the REPL, so the problem indeed
seems to be a dependency cycle.

Attempting a suggested fix by Ludovic in that same conversation [0],
namely, making the snippet field of the <origin> record a thunked one:

--8<---------------cut here---------------start------------->8---
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")))
--8<---------------cut here---------------end--------------->8---

It now seems a new cycle is introduced because trying to build anything
hangs using the CPU with slowly increasing memory usage:

./pre-inst-env guix build hello

How can we make Gexp-based snippets such as the one shown above work
without introducing problematic cycles?

Thanks,

Maxim

[0]  http://logs.guix.gnu.org/guix/2016-09-30.log#163047

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

[Ludovic Courtès]

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

> 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?
:-)

Thanks,
Ludo’.

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

[Maxim Cournoyer]

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

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

[Ludovic Courtès]

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

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

‘Thunk’ uses an actual thunk (zero-argument procedure) that’s called
each time the field is accessed; ‘delayed’ uses a promise, which is
similar except that the result is memoized (info "(guile) Delayed
Evaluation").

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

What would be interesting is a comparison of the performance of
‘package-derivation’, which can be done with something like:

  time guix build -d --no-grafts libreoffice pandoc

For memory consumption, try:

  GUIX_PROFILING=gc guix build -d --no-grafts libreoffice pandoc

>> +              (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?

It would work, but it’s “not the right place” for that, aesthetically.

(Note that when there’s a snippet, we get different derivations for each
system anyway.)

Thanks,
Ludo’.

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

[Maxim Cournoyer]

Hello Ludovic,

[...]

>> 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?
>
> ‘Thunk’ uses an actual thunk (zero-argument procedure) that’s called
> each time the field is accessed; ‘delayed’ uses a promise, which is
> similar except that the result is memoized (info "(guile) Delayed
> Evaluation").

Thanks for the explanation!  Now I wonder why delayed should not always
be preferable to thunks?  Is there a reason to offer thunked as well?

[...]

> What would be interesting is a comparison of the performance of
> ‘package-derivation’, which can be done with something like:
>
>   time guix build -d --no-grafts libreoffice pandoc
>
> For memory consumption, try:
>
>   GUIX_PROFILING=gc guix build -d --no-grafts libreoffice pandoc

Thanks for these "benchmarking" tips :-).  Unfortunately, making the
'snippet' field either thunked or delayed causes 'guix build' to stop
working entirely, peaking the CPU and slowy eating RAM away (looks like
a typical dependency cycle).

I'm at a lost as to how this can be debugged.

That's unfortunate, because as it stands, Gexp-based snippets don't seem
very usable.

Thank you,

Maxim

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

[Ludovic Courtès]

Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

>>> 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?
>>
>> ‘Thunk’ uses an actual thunk (zero-argument procedure) that’s called
>> each time the field is accessed; ‘delayed’ uses a promise, which is
>> similar except that the result is memoized (info "(guile) Delayed
>> Evaluation").
>
> Thanks for the explanation!  Now I wonder why delayed should not always
> be preferable to thunks?  Is there a reason to offer thunked as well?

Thunked fields were introduced pretty much as a hack so one could look
up dynamic bindings ‘%current-system’ and ‘%current-target-system’ and
produce a result that’s a function of that.  (See
<https://hal.inria.fr/hal-00824004/en>.)

>> What would be interesting is a comparison of the performance of
>> ‘package-derivation’, which can be done with something like:
>>
>>   time guix build -d --no-grafts libreoffice pandoc
>>
>> For memory consumption, try:
>>
>>   GUIX_PROFILING=gc guix build -d --no-grafts libreoffice pandoc
>
> Thanks for these "benchmarking" tips :-).  Unfortunately, making the
> 'snippet' field either thunked or delayed causes 'guix build' to stop
> working entirely, peaking the CPU and slowy eating RAM away (looks like
> a typical dependency cycle).

Hmm.  In packages.scm, there’s a couple of places where <origin> are
matched directly, which means that you get to see the raw
thunk/promise.  The solution is to explicitly call the thunk/force the
promise there, like we do for the ‘patches’ field in this clause:

  ($ <origin> uri method hash name (= force (patches ...)) snippet
          (flags ...) inputs (modules ...) guile-for-build)

If that still doesn’t help, please send a patch!  :-)

Ludo’.

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

[Mark H Weaver]

Hi Ludovic,

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

> 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.
[...]
> 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’.)

I can't speak for ungoogled-chromium, but for Linux-libre and IceCat,
there's at least one other limitation in snippets that are preventing us
from using them: the names of the pre- and post-snippet directories must
be the same.  In the case of IceCat, that would force us to either (1)
mislabel the upstream firefox source as "icecat", or (2) mislabel the
transformed icecat source as "firefox".  Ditto for "linux-libre" vs
"linux".

     Thanks,
       Mark

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

[Ludovic Courtès]

Hi Mark,

Mark H Weaver <mhw@netris.org> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> 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.
> [...]
>> 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’.)
>
> I can't speak for ungoogled-chromium, but for Linux-libre and IceCat,
> there's at least one other limitation in snippets that are preventing us
> from using them: the names of the pre- and post-snippet directories must
> be the same.  In the case of IceCat, that would force us to either (1)
> mislabel the upstream firefox source as "icecat", or (2) mislabel the
> transformed icecat source as "firefox".  Ditto for "linux-libre" vs
> "linux".

Good point.  That’s something we can probably address without too much
difficulty on core-updates.

Thanks,
Ludo’.

What should "guix build --source" produce? (was Re: Dependency cycle issues when using a Gexp-based snippet)

[Mark H Weaver]

Hi Maxim,

maxim.cournoyer@gmail.com writes:

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

[...]

> +              (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)))))

Apart from the technical difficulties with cyclic modules, I'd like to
raise another issue.

In my opinion, "guix build --source PACKAGE" should produce sources that
can be used to build the package on any system that the upstream package
supports, not just on Guix systems.

Alternatively, Guix should at least have *some* command to do this.

Such a command would be especially useful for packages that we clean for
FSDG compliance.  For example, I've made sure that "guix build --source
icecat" produces a tarball that's suitable for any system that IceCat
supports, and incidentally I intend to use Guix to generate the official
IceCat source tarballs.

Such a command would be useful for 'ungoogled-chromium' as well, and for
many of our other packages that include snippets to remove
non-FSDG-compliant code.

The snippet that you proposed above would produce "sources" that can
only be built on Guix systems, and moreover, only on the same
architecture and core-updates cycle that produced it.

I think that we ought to think about what "corresponding sources" should
be, and put some care into making sure that "guix build --source"
produces something worthy of that name.

What do you think?

      Mark

Re: What should "guix build --source" produce? (was Re: Dependency cycle issues when using a Gexp-based snippet)

[Andreas Enge]

Hello Mark,

On Tue, Sep 08, 2020 at 12:07:01AM -0400, Mark H Weaver wrote:
> > +                     (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)))))
> 
> In my opinion, "guix build --source PACKAGE" should produce sources that
> can be used to build the package on any system that the upstream package
> supports, not just on Guix systems.

that sounds good to me, in particular taking your subsequent arguments into
account. Guix sources could be just as useful as Debian sources in this
respect. Could this not be the elusive boundary between preparing the source
and adding a phase to the build arguments?

Andreas

Re: What should "guix build --source" produce? (was Re: Dependency cycle issues when using a Gexp-based snippet)

[Maxim Cournoyer]

Andreas Enge <andreas@enge.fr> writes:

> Hello Mark,
>
> On Tue, Sep 08, 2020 at 12:07:01AM -0400, Mark H Weaver wrote:
>> > +                     (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)))))
>> 
>> In my opinion, "guix build --source PACKAGE" should produce sources that
>> can be used to build the package on any system that the upstream package
>> supports, not just on Guix systems.
>
> that sounds good to me, in particular taking your subsequent arguments into
> account. Guix sources could be just as useful as Debian sources in this
> respect. Could this not be the elusive boundary between preparing the source
> and adding a phase to the build arguments?

Yes, this sounds like it!  How about the following section, to help
contributors decide between the two?

--8<---------------cut here---------------start------------->8---
1 file changed, 14 insertions(+)
doc/contributing.texi | 14 ++++++++++++++

modified   doc/contributing.texi
@@ -369,6 +369,7 @@ needed is to review and apply the patch.
 * Package Naming::              What's in a name?
 * Version Numbers::             When the name is not enough.
 * Synopses and Descriptions::   Helping users find the right package.
+* Snippets versus Phases::      Whether to use a snippet, or a build phase.
 * Python Modules::              A touch of British comedy.
 * Perl Modules::                Little pearls.
 * Java Packages::               Coffee break.
@@ -599,6 +600,19 @@ Gettext}):
 for the X11 resize-and-rotate (RandR) extension. @dots{}")
 @end example
 
+@node Snippets versus phases
+@subsection Snippets versus phases
+
+@cindex snippets, when to use
+The boundary between using an origin snippet versus a build phase to
+modify the sources of a package can be elusive.  Origin snippets are
+typically used to remove unnecessary files such as bundled libraries,
+nonfree sources, or to apply simple substitutions.  The source derived
+from an origin should produce a source that can be used to build the
+package on any system that the upstream package supports (i.e., act as
+the corresponding source).  In particular, origin patches or snippets
+must not embed store items in the sources; such patching should rather
+be done using build phases.
 
 @node Python Modules
 @subsection Python Modules
--8<---------------cut here---------------end--------------->8---

Thanks,

Maxim

Re: What should "guix build --source" produce? (was Re: Dependency cycle issues when using a Gexp-based snippet)

[zimoun]

Hi,

On Fri, 11 Sep 2020 at 20:22, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

> Yes, this sounds like it!  How about the following section, to help
> contributors decide between the two?
>
> --8<---------------cut here---------------start------------->8---
> 1 file changed, 14 insertions(+)
> doc/contributing.texi | 14 ++++++++++++++
>
> modified   doc/contributing.texi
> @@ -369,6 +369,7 @@ needed is to review and apply the patch.
>  * Package Naming::              What's in a name?
>  * Version Numbers::             When the name is not enough.
>  * Synopses and Descriptions::   Helping users find the right package.
> +* Snippets versus Phases::      Whether to use a snippet, or a build phase.
>  * Python Modules::              A touch of British comedy.
>  * Perl Modules::                Little pearls.
>  * Java Packages::               Coffee break.
> @@ -599,6 +600,19 @@ Gettext}):
>  for the X11 resize-and-rotate (RandR) extension. @dots{}")
>  @end example
>
> +@node Snippets versus phases
> +@subsection Snippets versus phases
> +
> +@cindex snippets, when to use
> +The boundary between using an origin snippet versus a build phase to
> +modify the sources of a package can be elusive.  Origin snippets are
> +typically used to remove unnecessary files such as bundled libraries,
> +nonfree sources, or to apply simple substitutions.  The source derived
> +from an origin should produce a source that can be used to build the
> +package on any system that the upstream package supports (i.e., act as
> +the corresponding source).  In particular, origin patches or snippets
> +must not embed store items in the sources; such patching should rather
> +be done using build phases.
>
>  @node Python Modules
>  @subsection Python Modules
> --8<---------------cut here---------------end--------------->8---

It helps. :-)

And the origin snippet:

  http://guix.gnu.org/manual/devel/en/guix.html#origin-Reference

could be cross-linked to the subsection above.


Thank you for the insightful remark.
All the best,
simon

Re: What should "guix build --source" produce? (was Re: Dependency cycle issues when using a Gexp-based snippet)

[Andreas Enge]

Hello Maxim,

On Fri, Sep 11, 2020 at 02:22:44PM -0400, Maxim Cournoyer wrote:
> +@cindex snippets, when to use
> +The boundary between using an origin snippet versus a build phase to
> +modify the sources of a package can be elusive.  Origin snippets are
> +typically used to remove unnecessary files such as bundled libraries,
> +nonfree sources, or to apply simple substitutions.  The source derived
> +from an origin should produce a source that can be used to build the
> +package on any system that the upstream package supports (i.e., act as
> +the corresponding source).  In particular, origin patches or snippets
> +must not embed store items in the sources; such patching should rather
> +be done using build phases.

that sounds good to me! However, I would not use the word "patch" in
"origin patches or snippets must not embed store items",
since patches are yet something else as defined in the "patches"
field; almost by definition, they cannot embed store items. So in the end:
"In particular, origin snippets must not embed store items in the sources;
such modifications should rather be done using build phases."

Thanks,

Andreas

Re: What should "guix build --source" produce? (was Re: Dependency cycle issues when using a Gexp-based snippet)

[Maxim Cournoyer]

Hi Mark,

Mark H Weaver <mhw@netris.org> writes:

> Hi Maxim,
>
> maxim.cournoyer@gmail.com writes:
>
>> 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:
>
> [...]
>
>> +              (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)))))
>
> Apart from the technical difficulties with cyclic modules, I'd like to
> raise another issue.
>
> In my opinion, "guix build --source PACKAGE" should produce sources that
> can be used to build the package on any system that the upstream package
> supports, not just on Guix systems.
>
> Alternatively, Guix should at least have *some* command to do this.
>
> Such a command would be especially useful for packages that we clean for
> FSDG compliance.  For example, I've made sure that "guix build --source
> icecat" produces a tarball that's suitable for any system that IceCat
> supports, and incidentally I intend to use Guix to generate the official
> IceCat source tarballs.
>
> Such a command would be useful for 'ungoogled-chromium' as well, and for
> many of our other packages that include snippets to remove
> non-FSDG-compliant code.
>
> The snippet that you proposed above would produce "sources" that can
> only be built on Guix systems, and moreover, only on the same
> architecture and core-updates cycle that produced it.
>
> I think that we ought to think about what "corresponding sources" should
> be, and put some care into making sure that "guix build --source"
> produces something worthy of that name.
>
> What do you think?

Thanks for those insightful remarks about what the source returned by
'guix build --source' should be.  I agree with you, and as Andreas
pointed, your remarks seem useful in drawing the distinction between
when to use snippets vs build phases.

Maxim