GNU bug report logs - #57704
[PATCH core-updates] guix: packages: Remove #f from inputs when sanitizing.

Previous Next

To reply to this bug, email your comments to 57704 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: guix-patches <at> gnu.org
Subject: [PATCH core-updates] guix: packages: Remove #f from inputs when
 sanitizing.
Date: Fri, 9 Sep 2022 17:56:20 +0200

This makes it so that new-style inputs can be optional using regular Guile
patterns, e.g. (and (target-x86-64?) rust).

* guix/packages.scm (sanitize-inputs): Filter inputs by identity before adding
labels.
---
Note that this patch was prepared using master, but since it affects the
package record, it needs to go to core-updates.  I don' think there should
be a merge conflict here.

 guix/packages.scm | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/guix/packages.scm b/guix/packages.scm
index 94e464cd01..5bb2e81e18 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -430,11 +430,12 @@ (define %cuirass-supported-systems
 (define-inlinable (sanitize-inputs inputs)
   "Sanitize INPUTS by turning it into a list of name/package tuples if it's
 not already the case."
-  (cond ((null? inputs) inputs)
-        ((and (pair? (car inputs))
-              (string? (caar inputs)))
-         inputs)
-        (else (map add-input-label inputs))))
+  (let ((inputs (filter identity inputs)))
+    (cond ((null? inputs) inputs)
+          ((and (pair? (car inputs))
+                (string? (caar inputs)))
+           inputs)
+          (else (map add-input-label inputs)))))
 
 (define-syntax current-location-vector
   (lambda (s)
-- 
2.37.2

Message #8 received at 57704 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 57704 <at> debbugs.gnu.org
Subject: Re: [bug#57704] [PATCH core-updates] guix: packages: Remove #f from
 inputs when sanitizing.
Date: Fri, 9 Sep 2022 20:54:09 +0200

[Message part 1 (text/plain, inline)]

On 09-09-2022 17:56, Liliana Marie Prikler wrote:
> This makes it so that new-style inputs can be optional using regular Guile
> patterns, e.g. (and (target-x86-64?) rust).

Seems useful.

> * guix/packages.scm (sanitize-inputs): Filter inputs by identity before adding
> labels.

Documentation is missing.

> ---
> Note that this patch was prepared using master, but since it affects the
> package record, it needs to go to core-updates.  I don' think there should
> be a merge conflict here.

It does affect the package record, but it doesn't cause any rebuilds, so 
master should be fine:

* There aren't any current uses of #false:

(use-modules (guix packages) (gnu packages))
(package
  (inherit (specification->package "hello"))
  (inputs (list #false)))
;; guix build -f [...] --> package ‘hello <at> 2.12.1’ has an invalid input

* In the absence of #false, the behaviour remains unchanged.

* guix/packages.scm is not used by any derivation
  (except for "guix pull" and the guix package)

As a test, I applied the patch and did
‘make && ./pre-inst-env guix build -n libreoffice’,
and it turned out I already have it installed.

Greetings,
Maxime.

[OpenPGP_0x49E3EE22191725EE.asc (application/pgp-keys, attachment)]

[OpenPGP_signature (application/pgp-signature, attachment)]

Message #11 received at 57704 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: 57704 <at> debbugs.gnu.org
Cc: Maxime Devos <maximedevos <at> telenet.be>
Subject: [PATCH v2] guix: packages: Remove #f from inputs when sanitizing.
Date: Fri, 9 Sep 2022 17:56:20 +0200

This makes it so that new-style inputs can be optional using regular Guile
patterns, e.g. (and (target-x86-64?) rust).

* guix/packages.scm (sanitize-inputs): Filter inputs by identity before adding
labels.
---
As noted by Maxime, this doesn't seem to be cause any rebuilds, so retargeting
master.  Also added missing documentation.

 guix/packages.scm | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/guix/packages.scm b/guix/packages.scm
index 94e464cd01..7569380610 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -428,13 +428,14 @@ (define %cuirass-supported-systems
   (fold delete %supported-systems '("mips64el-linux" "powerpc-linux" "riscv64-linux")))
 
 (define-inlinable (sanitize-inputs inputs)
-  "Sanitize INPUTS by turning it into a list of name/package tuples if it's
-not already the case."
-  (cond ((null? inputs) inputs)
-        ((and (pair? (car inputs))
-              (string? (caar inputs)))
-         inputs)
-        (else (map add-input-label inputs))))
+  "Sanitize INPUTS by removing falsy elements and turning it into a list of
+name/package tuples if it's not already the case."
+  (let ((inputs (filter identity inputs)))
+    (cond ((null? inputs) inputs)
+          ((and (pair? (car inputs))
+                (string? (caar inputs)))
+           inputs)
+          (else (map add-input-label inputs)))))
 
 (define-syntax current-location-vector
   (lambda (s)
-- 
2.37.2

Message #14 received at 57704 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 57704 <at> debbugs.gnu.org
Subject: Re: [PATCH v2] guix: packages: Remove #f from inputs when sanitizing.
Date: Sat, 10 Sep 2022 02:33:00 +0200

[Message part 1 (text/plain, inline)]

On 09-09-2022 17:56, Liliana Marie Prikler wrote:
> This makes it so that new-style inputs can be optional using regular Guile
> patterns, e.g. (and (target-x86-64?) rust).
> 
> * guix/packages.scm (sanitize-inputs): Filter inputs by identity before adding
> labels.
> ---
> As noted by Maxime, this doesn't seem to be cause any rebuilds, so retargeting
> master.  Also added missing documentation.

The docstring is nice, but with documentation, I meant the manual, 
presumably in ‘(guix)package Reference’, maybe also in the packaging 
tutorial in the cookbook.

Also, something I forgot: performance.  sanitize-inputs is called for 
every package, and the change adds additional memory allocations (due to 
the use of 'filter'), is there an observable performance impact (maybe 
"GUIX_PROFILING=gc time guix refresh -l pkg-config" would be a good 
test)?  If there is, some optimisations may be in order

Greetings,
Maxime.

[OpenPGP_0x49E3EE22191725EE.asc (application/pgp-keys, attachment)]

[OpenPGP_signature (application/pgp-signature, attachment)]

Message #17 received at 57704 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Maxime Devos <maximedevos <at> telenet.be>, 57704 <at> debbugs.gnu.org
Subject: Re: [PATCH v2] guix: packages: Remove #f from inputs when sanitizing.
Date: Sat, 10 Sep 2022 08:40:14 +0200

Am Samstag, dem 10.09.2022 um 02:33 +0200 schrieb Maxime Devos:
> The docstring is nice, but with documentation, I meant the manual, 
> presumably in ‘(guix)package Reference’, maybe also in the packaging 
> tutorial in the cookbook.
I don't see the current practice documented, so I think we're actually
good on this front.

> Also, something I forgot: performance.  sanitize-inputs is called for
> every package, and the change adds additional memory allocations (due
> to the use of 'filter'), is there an observable performance impact
> (maybe "GUIX_PROFILING=gc time guix refresh -l pkg-config" would be a
> good test)?  If there is, some optimisations may be in order
Looking at the numbers below

Garbage collection statistics:
  heap size:        212.66 MiB
  allocated:        739.15 MiB
  GC times:         20
  time spent in GC: 5.30 seconds (65% of user time)

real	0m3,606s
user	0m8,140s
sys	0m0,109s

Garbage collection statistics:
  heap size:        276.29 MiB
  allocated:        1292.10 MiB
  GC times:         28
  time spent in GC: 10.48 seconds (64% of user time)

real	0m11,638s
user	0m16,422s
sys	0m0,308s

it does appear that this increases times by a factor of two.  Use of
filter! instead of filter brings only marginal benefits.  I'll check if
we could instead simply ignore unspecified? values when collecting the
inputs – that would allow the natural use of (when) and (unless).

Cheers 

Message #20 received at 57704 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Maxime Devos <maximedevos <at> telenet.be>, 57704 <at> debbugs.gnu.org
Subject: Re: [PATCH v2] guix: packages: Remove #f from inputs when sanitizing.
Date: Sat, 10 Sep 2022 09:44:28 +0200

Am Samstag, dem 10.09.2022 um 08:40 +0200 schrieb Liliana Marie
Prikler:
> Looking at the numbers below
> [...]
> it does appear that this increases times by a factor of two.
It seems I've been comparing apples to oranges.  Running ./pre-inst-env
already increases the times for guix:

Garbage collection statistics:
  heap size:        276.29 MiB
  allocated:        1291.91 MiB
  GC times:         28
  time spent in GC: 9.39 seconds (66% of user time)

real	0m6,069s
user	0m14,172s
sys	0m0,140s

An alternative patch that I'll submit as v3 adds little to these times:

Garbage collection statistics:
  heap size:        276.29 MiB
  allocated:        1291.96 MiB
  GC times:         28
  time spent in GC: 9.32 seconds (66% of user time)

real	0m6,124s
user	0m14,138s
sys	0m0,147s

Cheers

Message #23 received at 57704 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: 57704 <at> debbugs.gnu.org
Cc: Maxime Devos <maximedevos <at> telenet.be>
Subject: [PATCH v3] guix: Filter unspecified inputs when sanitizing.
Date: Sat, 10 Sep 2022 09:41:39 +0200

* guix/packages.scm (sanitize-inputs): Filter inputs which are unspecified?
rather than adding a label.
---
 guix/packages.scm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/guix/packages.scm b/guix/packages.scm
index 94e464cd01..0975002c13 100644
--- a/guix/packages.scm
+++ b/guix/packages.scm
@@ -429,12 +429,15 @@ (define %cuirass-supported-systems
 
 (define-inlinable (sanitize-inputs inputs)
   "Sanitize INPUTS by turning it into a list of name/package tuples if it's
-not already the case."
+not already the case and removing unspecified inputs."
   (cond ((null? inputs) inputs)
         ((and (pair? (car inputs))
               (string? (caar inputs)))
          inputs)
-        (else (map add-input-label inputs))))
+        (else (filter-map (lambda (input)
+                            (if (unspecified? input) #f
+                                (add-input-label input)))
+                          inputs))))
 
 (define-syntax current-location-vector
   (lambda (s)
-- 
2.37.2

Message #26 received at 57704 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 57704 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] guix: Filter unspecified inputs when sanitizing.
Date: Sat, 10 Sep 2022 12:19:32 +0200

[Message part 1 (text/plain, inline)]

> Am Samstag, dem 10.09.2022 um 02:33 +0200 schrieb Maxime Devos:
>> The docstring is nice, but with documentation, I meant the manual, 
>> presumably in ‘(guix)package Reference’, maybe also in the packaging 
>> tutorial in the cookbook.
> I don't see the current practice documented, so I think we're actually
> good on this front.

That sounds bad to me -- the undocumented surface should be decreased, 
not increased.  Also, it is actually documented a little:

     ‘inputs’ (default: ‘'()’)
     ‘native-inputs’ (default: ‘'()’)
     ‘propagated-inputs’ (default: ‘'()’)
          These fields list dependencies of the package.  Each element
          of these lists is either a package, origin, or other
          “file-like object” (*note G-Expressions::); [...]

#false (or, in this case, *unspecified*) is neither a package, origin or 
other file-like object.  Maybe you can add that #false is also allowed 
but ignored?

On 10-09-2022 09:41, Liliana Marie Prikler wrote:
>            inputs)
> -        (else (map add-input-label inputs))))
> +        (else (filter-map (lambda (input)
> +                            (if (unspecified? input) #f
> +                                (add-input-label input)))
> +                          inputs))))

(when cond ...) / (unless cond ...) returning *unspecified* when (not 
cond)/cond is an implementation detail:

  * The return values(s) when (not cond)/cond is not documented in
    (guile)Conditionals
  * maybe: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=56799#17

    There is an interest in letting it return zero values instead of
    *unspecified*, see e.g. 
https://scheme-reports.scheme-reports.narkive.com/QSQtJSAh/unspecified-values
    and a ‘bug’ on bugs.gnu.org I cannot find anymore about actually
    doing this change.

    By assuming that when/unless returns *unspecified* here, an
    additional backwards-compatibility concern is introduced.

As such, I don't think relying on this to be a good idea.

Alternative proposal: instead of (when cond package), maybe
(and cond package)?

Greetings,
Maxime

[OpenPGP_0x49E3EE22191725EE.asc (application/pgp-keys, attachment)]

[OpenPGP_signature (application/pgp-signature, attachment)]

Message #29 received at 57704 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 57704 <at> debbugs.gnu.org
Subject: Re: bug#57704: [PATCH core-updates] guix: packages: Remove #f from
 inputs when sanitizing.
Date: Mon, 26 Sep 2022 22:51:43 +0200

Hi Liliana,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

> This makes it so that new-style inputs can be optional using regular Guile
> patterns, e.g. (and (target-x86-64?) rust).

I’d rather avoid that and make sure input lists are just plain lists,
remaining strict, and keeping the sanitize procedure simple (notably so
it can be optimized in common cases).

That means we have to live with idioms like:

  (append (list x y z)
          (if (target-x86-64?) (list rust) '()))

The ‘openmpi’ package has sugar to make that more concise.

Thoughts?

Thanks,
Ludo’.

Message #34 received at 57704 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 57704 <at> debbugs.gnu.org
Subject: Re: bug#57704: [PATCH core-updates] guix: packages: Remove #f from
 inputs when sanitizing.
Date: Sat, 20 Jan 2024 15:43:37 -0500

Hi Liliana,

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

> Hi Liliana,
>
> Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:
>
>> This makes it so that new-style inputs can be optional using regular Guile
>> patterns, e.g. (and (target-x86-64?) rust).
>
> I’d rather avoid that and make sure input lists are just plain lists,
> remaining strict, and keeping the sanitize procedure simple (notably so
> it can be optimized in common cases).
>
> That means we have to live with idioms like:
>
>   (append (list x y z)
>           (if (target-x86-64?) (list rust) '()))
>
> The ‘openmpi’ package has sugar to make that more concise.
>
> Thoughts?

Any plans to revisit this, or should we close it?

-- 
Thanks,
Maxim

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.