[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

* guix/scripts.scm (%disk-space-warning-absolute): New variable.
(warn-about-disk-space): Test against %disk-space-warning-absolute.
Fix error in display-hint due to extraneous 'profile' argument.
---
 guix/scripts.scm | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/guix/scripts.scm b/guix/scripts.scm
index 77cbf12350..f8cce3a542 100644
--- a/guix/scripts.scm
+++ b/guix/scripts.scm
@@ -188,25 +188,35 @@ Show what and how will/would be built."
                     (#f        .05)               ;5%
                     (threshold (/ threshold 100.)))))
 
+(define %disk-space-warning-absolute
+  ;; The decimal number of GiB of free disk space below which a warning is
+  ;; emitted.
+  (make-parameter (match (and=> (getenv "GUIX_DISK_SPACE_WARNING_ABSOLUTE")
+                                string->number)
+                    (#f        17.0)
+                    (threshold threshold))))
+
 (define* (warn-about-disk-space #:optional profile
                                 #:key
-                                (threshold (%disk-space-warning)))
+                                (relative-threshold (%disk-space-warning))
+                                (absolute-threshold (%disk-space-warning-absolute)))
   "Display a hint about 'guix gc' if less than THRESHOLD of /gnu/store is
 available."
   (let* ((stats      (statfs (%store-prefix)))
          (block-size (file-system-block-size stats))
          (available  (* block-size (file-system-blocks-available stats)))
          (total      (* block-size (file-system-block-count stats)))
-         (ratio      (/ available total 1.)))
-    (when (< ratio threshold)
-      (warning (G_ "only ~,1f% of free space available on ~a~%")
-               (* ratio 100) (%store-prefix))
+         (relative-threshold-in-bytes (* total relative-threshold))
+         (absolute-threshold-in-bytes (* 1024 1024 1024 absolute-threshold)))
+    (when (< available (min relative-threshold-in-bytes
+                            absolute-threshold-in-bytes))
+      (warning (G_ "only ~,1f GiB of free space available on ~a~%")
+               available (%store-prefix))
       (display-hint (format #f (G_ "Consider deleting old profile
 generations and collecting garbage, along these lines:
 
 @example
 guix gc --delete-generations=1m
-@end example\n")
-                            profile)))))
+@end example\n"))))))
 
 ;;; scripts.scm ends here
-- 
2.25.0

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Ludovic Courtès]

Hi!

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> * guix/scripts.scm (%disk-space-warning-absolute): New variable.
> (warn-about-disk-space): Test against %disk-space-warning-absolute.
> Fix error in display-hint due to extraneous 'profile' argument.

[...]

> +(define %disk-space-warning-absolute
> +  ;; The decimal number of GiB of free disk space below which a warning is
> +  ;; emitted.
> +  (make-parameter (match (and=> (getenv "GUIX_DISK_SPACE_WARNING_ABSOLUTE")
> +                                string->number)
> +                    (#f        17.0)
> +                    (threshold threshold))))

Perhaps we should arrange for ‘GUIX_DISK_SPACE_WARNING’ to handle both
cases?

That is, we’d first try to convert it with ‘size->number’; if that
works, it’s an absolute measure, and if it returns #f, then pass the
string to ‘string->number’ and assume it’s a fraction.

Does that make sense?

> +         (absolute-threshold-in-bytes (* 1024 1024 1024 absolute-threshold)))

Always use bytes internally; that is, convert to bytes at the UI border.

Thanks,
Ludo’.

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 2241 bytes --]

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

> Hi!
>
> Pierre Neidhardt <mail@ambrevar.xyz> skribis:
>
>> * guix/scripts.scm (%disk-space-warning-absolute): New variable.
>> (warn-about-disk-space): Test against %disk-space-warning-absolute.
>> Fix error in display-hint due to extraneous 'profile' argument.
>
> [...]
>
>> +(define %disk-space-warning-absolute
>> +  ;; The decimal number of GiB of free disk space below which a warning is
>> +  ;; emitted.
>> +  (make-parameter (match (and=> (getenv "GUIX_DISK_SPACE_WARNING_ABSOLUTE")
>> +                                string->number)
>> +                    (#f        17.0)
>> +                    (threshold threshold))))
>
> Perhaps we should arrange for ‘GUIX_DISK_SPACE_WARNING’ to handle both
> cases?
>
> That is, we’d first try to convert it with ‘size->number’; if that
> works, it’s an absolute measure, and if it returns #f, then pass the
> string to ‘string->number’ and assume it’s a fraction.

I thought of something similar too, but this needs discussion:

- If we change the meaning of a value like "1", we are breaking backward compatibility.

- GUIX_DISK_SPACE_WARNING is currently undocumented, so I guess it's OK
  to break backward compatibility.  However, it'd be nice to document it :)

- Currently (size->number "1.MiB") "leaves" the Guile instance on error.  Which
  I find quite weird, I'd expect it to return #f when it cannot parse
  the input.

- Currently (size->number "0.8") returns 1.  If we want to use your
  suggestion, we would need to change the behaviour so that it returns
  #f.

- Alternatively, since we are breaking backward compatibility anyways,
  we could parse a trailing percent sign "%" to decide whether the value
  is relative or absolute.

Anyways, the issue was originally about dealing with both small and big
partitions, and for this we need both a default absolute threshold and a
default relative threshold.  Does that make sense?

>> +         (absolute-threshold-in-bytes (* 1024 1024 1024 absolute-threshold)))
>
> Always use bytes internally; that is, convert to bytes at the UI border.

What do you mean?

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Ludovic Courtès]

Hello!

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Hi!
>>
>> Pierre Neidhardt <mail@ambrevar.xyz> skribis:
>>
>>> * guix/scripts.scm (%disk-space-warning-absolute): New variable.
>>> (warn-about-disk-space): Test against %disk-space-warning-absolute.
>>> Fix error in display-hint due to extraneous 'profile' argument.
>>
>> [...]
>>
>>> +(define %disk-space-warning-absolute
>>> +  ;; The decimal number of GiB of free disk space below which a warning is
>>> +  ;; emitted.
>>> +  (make-parameter (match (and=> (getenv "GUIX_DISK_SPACE_WARNING_ABSOLUTE")
>>> +                                string->number)
>>> +                    (#f        17.0)
>>> +                    (threshold threshold))))
>>
>> Perhaps we should arrange for ‘GUIX_DISK_SPACE_WARNING’ to handle both
>> cases?
>>
>> That is, we’d first try to convert it with ‘size->number’; if that
>> works, it’s an absolute measure, and if it returns #f, then pass the
>> string to ‘string->number’ and assume it’s a fraction.
>
> I thought of something similar too, but this needs discussion:
>
> - If we change the meaning of a value like "1", we are breaking backward compatibility.

We can assume values <= 100 are a percentage.

> - GUIX_DISK_SPACE_WARNING is currently undocumented, so I guess it's OK
>   to break backward compatibility.  However, it'd be nice to document it :)

Yup.

> - Currently (size->number "1.MiB") "leaves" the Guile instance on error.  Which
>   I find quite weird, I'd expect it to return #f when it cannot parse
>   the input.

Fixed.

> - Currently (size->number "0.8") returns 1.  If we want to use your
>   suggestion, we would need to change the behaviour so that it returns
>   #f.

Hmm.

> - Alternatively, since we are breaking backward compatibility anyways,
>   we could parse a trailing percent sign "%" to decide whether the value
>   is relative or absolute.

Right.

> Anyways, the issue was originally about dealing with both small and big
> partitions, and for this we need both a default absolute threshold and a
> default relative threshold.  Does that make sense?

It does!

So how about this:

  We check for a trailing “%”, and if there’s one, assume it’s a percentage.

  Else, call ‘size->number’.  If we get an integer < 100, assume it’s a
  percentage, otherwise assume it’s an absolute size in bytes.

How does that sound?

Thanks!

Ludo’.

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 573 bytes --]

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

> So how about this:
>
>   We check for a trailing “%”, and if there’s one, assume it’s a percentage.
>
>   Else, call ‘size->number’.  If we get an integer < 100, assume it’s a
>   percentage, otherwise assume it’s an absolute size in bytes.
>
> How does that sound?

I think there is a misunderstanding.  If we have only _one_ threshold,
there is no way to set sane defaults for both big and small partitions.

Or were you thinking of something else?

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Ludovic Courtès]

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> So how about this:
>>
>>   We check for a trailing “%”, and if there’s one, assume it’s a percentage.
>>
>>   Else, call ‘size->number’.  If we get an integer < 100, assume it’s a
>>   percentage, otherwise assume it’s an absolute size in bytes.
>>
>> How does that sound?
>
> I think there is a misunderstanding.  If we have only _one_ threshold,
> there is no way to set sane defaults for both big and small partitions.

Oh, I must have misunderstood.

You mean you’d want to be able to set both an absolute threshold and a
relative threshold?  (Why “big and small” actually?)

Why not, but it seems to me we might be going overboard for something
that’s just meant as a hint to help people learn about ‘guix gc’.  :-)

Ludo’.

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 801 bytes --]

See the original discussion:

https://lists.gnu.org/archive/html/guix-devel/2020-02/msg00303.html

Our users fall into 2 categories:

- Those with a "small" store partition, say 25 GiB.
- Those with a "big" store partition, say 1 TiB.

If we use a relative threshold, users with a big partition will get
a hint all the time.  This may mislead newcomers into thinking that they
should run "guix gc" while they still have 50 GiB of free space.

If we use an absolute threshold, users with a small partition will get a
hint all the time.

In both cases, the hints becomes irrelevant if it's triggered for the
wrong reason.

The fix is to use both thresholds and to check against the lowest one.

Does that make more sense?

Cheers!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Ludovic Courtès]

Hi!

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> See the original discussion:
>
> https://lists.gnu.org/archive/html/guix-devel/2020-02/msg00303.html

Oops, I’m lagging behind and had not seen it yet, sorry about that!

> Our users fall into 2 categories:
>
> - Those with a "small" store partition, say 25 GiB.
> - Those with a "big" store partition, say 1 TiB.
>
> If we use a relative threshold, users with a big partition will get
> a hint all the time.  This may mislead newcomers into thinking that they
> should run "guix gc" while they still have 50 GiB of free space.
>
> If we use an absolute threshold, users with a small partition will get a
> hint all the time.
>
> In both cases, the hints becomes irrelevant if it's triggered for the
> wrong reason.
>
> The fix is to use both thresholds and to check against the lowest one.
>
> Does that make more sense?

It does, yes.

So back to the original patch… I think we should use ‘size->number’ for
the absolute threshold.

I’m also still mildly reluctant to the addition of
‘GUIX_DISK_SPACE_WARNING_ABSOLUTE’ (one more variable and a weird
name—“absolute warning”?).  An option could be to allow
‘GUIX_DISK_SPACE_WARNING’ to optionally contain both thresholds.  These
would be valid values:

  1GiB;10%
  15            ;15% relative, and default absolute
  15%           ;same
  1G            ;1G absolute, and default relative

Thus ‘%disk-space-warning’ would hold a pair.

Does that make sense?

Apologies for the misunderstanding!

Ludo’.

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 108 bytes --]

Sounds good, I'll send a new patch as soon as possible! :)

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

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

> So back to the original patch… I think we should use ‘size->number’ for
> the absolute threshold.
>
> I’m also still mildly reluctant to the addition of
> ‘GUIX_DISK_SPACE_WARNING_ABSOLUTE’ (one more variable and a weird
> name—“absolute warning”?).  An option could be to allow
> ‘GUIX_DISK_SPACE_WARNING’ to optionally contain both thresholds.  These
> would be valid values:
>
>   1GiB;10%
>   15            ;15% relative, and default absolute
>   15%           ;same
>   1G            ;1G absolute, and default relative

The above logic implies that we test against size->number, and if it
fails then we set the relative threshold.

But a size->number quits Guile.  Should we error out instead?

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]

Pierre Neidhardt <mail@ambrevar.xyz> writes:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> So back to the original patch… I think we should use ‘size->number’ for
>> the absolute threshold.
>>
>> I’m also still mildly reluctant to the addition of
>> ‘GUIX_DISK_SPACE_WARNING_ABSOLUTE’ (one more variable and a weird
>> name—“absolute warning”?).  An option could be to allow
>> ‘GUIX_DISK_SPACE_WARNING’ to optionally contain both thresholds.  These
>> would be valid values:
>>
>>   1GiB;10%
>>   15            ;15% relative, and default absolute
>>   15%           ;same
>>   1G            ;1G absolute, and default relative

Something else: in your example it's not logically possible to make out
if a raw number is relative or absolute.
So I suggest to break backward compatibility and do this:

   1GiB;10%
   15            ;15 absolute (15 bytes), and default relative
   15%           ;15% relative, and default absolute
   1G            ;1G absolute, and default relative

Thoughts?

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Ludovic Courtès]

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> So back to the original patch… I think we should use ‘size->number’ for
>> the absolute threshold.
>>
>> I’m also still mildly reluctant to the addition of
>> ‘GUIX_DISK_SPACE_WARNING_ABSOLUTE’ (one more variable and a weird
>> name—“absolute warning”?).  An option could be to allow
>> ‘GUIX_DISK_SPACE_WARNING’ to optionally contain both thresholds.  These
>> would be valid values:
>>
>>   1GiB;10%
>>   15            ;15% relative, and default absolute
>>   15%           ;same
>>   1G            ;1G absolute, and default relative
>
> The above logic implies that we test against size->number, and if it
> fails then we set the relative threshold.
>
> But a size->number quits Guile.

More precisely, it throws a 'quit exception.  :-)

So you could always do: (false-if-exception (size->number x)).

HTH,
Ludo’.

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

* guix/scripts.scm (%disk-space-warning-absolute): New variable.
(warn-about-disk-space): Test against %disk-space-warning-absolute.
Fix error in display-hint due to extraneous 'profile' argument.
---
 guix/scripts.scm | 59 ++++++++++++++++++++++++++++++++++++------------
 1 file changed, 45 insertions(+), 14 deletions(-)

diff --git a/guix/scripts.scm b/guix/scripts.scm
index 77cbf12350..bfb378f93c 100644
--- a/guix/scripts.scm
+++ b/guix/scripts.scm
@@ -181,32 +181,63 @@ Show what and how will/would be built."
     (newline (guix-warning-port))))
 
 (define %disk-space-warning
-  ;; The fraction (between 0 and 1) of free disk space below which a warning
-  ;; is emitted.
-  (make-parameter (match (and=> (getenv "GUIX_DISK_SPACE_WARNING")
-                                string->number)
-                    (#f        .05)               ;5%
-                    (threshold (/ threshold 100.)))))
+  ;; Return a pair of absolute threshold (number of bytes) and relative
+  ;; threshold (fraction between 0 and 1) for the free disk space below which
+  ;; a warning is emitted.
+  ;; GUIX_DISK_SPACE_WARNING can contain both thresholds.  The following
+  ;; example values are valid:
+  ;; - 1GiB;10%      ;1 GiB absolute, and 10% relative.
+  ;; - 15G           ;15 absolute, and default relative.
+  ;; - 15%           ;15% relative, and default absolute.
+  (let* ((default-absolute-threshold (size->number "5GiB"))
+         (default-relative-threshold 0.05)
+         (percentage->float (lambda (percentage)
+                              (or (if (string? percentage)
+                                      (string->number
+                                       (car (string-split percentage #\%))))
+                                  default-relative-threshold)))
+         (size->number* (lambda (size)
+                          (or (false-if-exception (size->number size))
+                              default-absolute-threshold))))
+    (make-parameter
+     (match (getenv "GUIX_DISK_SPACE_WARNING")
+       (#f (list default-absolute-threshold
+                 default-relative-threshold))
+       (env-string (match (string-split env-string #\;)
+                     ((threshold)
+                      (if (string-contains threshold "%")
+                          (list default-absolute-threshold
+                                (percentage->float threshold))
+                          (list (size->number* threshold)
+                                default-relative-threshold)))
+                     ((threshold1 threshold2)
+                      (if (string-contains threshold1 "%")
+                          (list (size->number* threshold2)
+                                (percentage->float threshold1))
+                          (list (size->number* threshold1)
+                                (percentage->float threshold2))))))))))
 
 (define* (warn-about-disk-space #:optional profile
                                 #:key
-                                (threshold (%disk-space-warning)))
+                                (thresholds (%disk-space-warning)))
   "Display a hint about 'guix gc' if less than THRESHOLD of /gnu/store is
-available."
+available.
+THRESHOLD is a pair of (ABSOLUTE-THRESHOLD RELATIVE-THRESHOLD)."
   (let* ((stats      (statfs (%store-prefix)))
          (block-size (file-system-block-size stats))
          (available  (* block-size (file-system-blocks-available stats)))
          (total      (* block-size (file-system-block-count stats)))
-         (ratio      (/ available total 1.)))
-    (when (< ratio threshold)
-      (warning (G_ "only ~,1f% of free space available on ~a~%")
-               (* ratio 100) (%store-prefix))
+         (relative-threshold-in-bytes (* total (cadr thresholds)))
+         (absolute-threshold-in-bytes (* 1024 1024 1024 (car thresholds))))
+    (when (< available (min relative-threshold-in-bytes
+                            absolute-threshold-in-bytes))
+      (warning (G_ "only ~,1f GiB of free space available on ~a~%")
+               available (%store-prefix))
       (display-hint (format #f (G_ "Consider deleting old profile
 generations and collecting garbage, along these lines:
 
 @example
 guix gc --delete-generations=1m
-@end example\n")
-                            profile)))))
+@end example\n"))))))
 
 ;;; scripts.scm ends here
-- 
2.25.0

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 62 bytes --]

Update sent.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 483 bytes --]

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Ludovic Courtès]

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> Pierre Neidhardt <mail@ambrevar.xyz> writes:
>
>> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>>>   1GiB;10%
>>>   15            ;15% relative, and default absolute
>>>   15%           ;same
>>>   1G            ;1G absolute, and default relative
>
> Something else: in your example it's not logically possible to make out
> if a raw number is relative or absolute.
> So I suggest to break backward compatibility and do this:
>
>    1GiB;10%
>    15            ;15 absolute (15 bytes), and default relative
>    15%           ;15% relative, and default absolute
>    1G            ;1G absolute, and default relative
>
> Thoughts?

What about having the rule that a number below 100 is a percentage, as
discussed before?  (It doesn’t make much sense to have an absolute
threshold of 42 bytes.)

Ludo’.

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 310 bytes --]

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

> What about having the rule that a number below 100 is a percentage, as
> discussed before?  (It doesn’t make much sense to have an absolute
> threshold of 42 bytes.)

Could do that.  Seems a bit ad-hoc though.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

* guix/scripts.scm (%disk-space-warning-absolute): New variable.
(warn-about-disk-space): Test against %disk-space-warning-absolute.
Fix error in display-hint due to extraneous 'profile' argument.
---
 guix/scripts.scm | 66 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 52 insertions(+), 14 deletions(-)

diff --git a/guix/scripts.scm b/guix/scripts.scm
index 77cbf12350..df59f5ae82 100644
--- a/guix/scripts.scm
+++ b/guix/scripts.scm
@@ -181,32 +181,70 @@ Show what and how will/would be built."
     (newline (guix-warning-port))))
 
 (define %disk-space-warning
-  ;; The fraction (between 0 and 1) of free disk space below which a warning
-  ;; is emitted.
-  (make-parameter (match (and=> (getenv "GUIX_DISK_SPACE_WARNING")
-                                string->number)
-                    (#f        .05)               ;5%
-                    (threshold (/ threshold 100.)))))
+  ;; Return a pair of absolute threshold (number of bytes) and relative
+  ;; threshold (fraction between 0 and 1) for the free disk space below which
+  ;; a warning is emitted.
+  ;; GUIX_DISK_SPACE_WARNING can contain both thresholds.  A value in [0;100)
+  ;; is a relative threshold, otherwise it's absolute.  The following
+  ;; example values are valid:
+  ;; - 1GiB;10%      ;1 GiB absolute, and 10% relative.
+  ;; - 15G           ;15 GiB absolute, and default relative.
+  ;; - 99%           ;99% relative, and default absolute.
+  ;; - 99            ;Same.
+  ;; - 100           ;100 absolute, and default relative.
+  (let* ((default-absolute-threshold (size->number "5GiB"))
+         (default-relative-threshold 0.05)
+         (percentage->float (lambda (percentage)
+                              (or (and=> (string->number
+                                          (car (string-split percentage #\%)))
+                                         (lambda (n) (/ n 100.0)))
+                                  default-relative-threshold)))
+         (size->number* (lambda (size)
+                          (or (false-if-exception (size->number size))
+                              default-absolute-threshold)))
+         (absolute? (lambda (size)
+                      (if (string-suffix? "%" size)
+                          #f
+                          (false-if-exception (<= 100 (size->number size)))))))
+    (make-parameter
+     (match (getenv "GUIX_DISK_SPACE_WARNING")
+       (#f (list default-absolute-threshold
+                 default-relative-threshold))
+       (env-string (match (string-split env-string #\;)
+                     ((threshold)
+                      (if (absolute? threshold)
+                          (list (size->number* threshold)
+                                default-relative-threshold)
+                          (list default-absolute-threshold
+                                (percentage->float threshold))))
+                     ((threshold1 threshold2)
+                      (if (absolute? threshold1)
+                          (list (size->number* threshold1)
+                                (percentage->float threshold2))
+                          (list (size->number* threshold2)
+                                (percentage->float threshold1))))))))))
 
 (define* (warn-about-disk-space #:optional profile
                                 #:key
-                                (threshold (%disk-space-warning)))
+                                (thresholds (%disk-space-warning)))
   "Display a hint about 'guix gc' if less than THRESHOLD of /gnu/store is
-available."
+available.
+THRESHOLD is a pair of (ABSOLUTE-THRESHOLD RELATIVE-THRESHOLD)."
   (let* ((stats      (statfs (%store-prefix)))
          (block-size (file-system-block-size stats))
          (available  (* block-size (file-system-blocks-available stats)))
          (total      (* block-size (file-system-block-count stats)))
-         (ratio      (/ available total 1.)))
-    (when (< ratio threshold)
-      (warning (G_ "only ~,1f% of free space available on ~a~%")
-               (* ratio 100) (%store-prefix))
+         (relative-threshold-in-bytes (* total (cadr thresholds)))
+         (absolute-threshold-in-bytes (* 1024 1024 1024 (car thresholds))))
+    (when (< available (min relative-threshold-in-bytes
+                            absolute-threshold-in-bytes))
+      (warning (G_ "only ~,1f GiB of free space available on ~a~%")
+               available (%store-prefix))
       (display-hint (format #f (G_ "Consider deleting old profile
 generations and collecting garbage, along these lines:
 
 @example
 guix gc --delete-generations=1m
-@end example\n")
-                            profile)))))
+@end example\n"))))))
 
 ;;; scripts.scm ends here
-- 
2.25.0

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 64 bytes --]

Patch updated.

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Ludovic Courtès]

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> * guix/scripts.scm (%disk-space-warning-absolute): New variable.
> (warn-about-disk-space): Test against %disk-space-warning-absolute.
> Fix error in display-hint due to extraneous 'profile' argument.

[...]

> +  (let* ((default-absolute-threshold (size->number "5GiB"))
> +         (default-relative-threshold 0.05)
> +         (percentage->float (lambda (percentage)
> +                              (or (and=> (string->number
> +                                          (car (string-split percentage #\%)))

Mayes just: (string-drop-right percentage 1).

> +         (absolute? (lambda (size)
> +                      (if (string-suffix? "%" size)
> +                          #f
> +                          (false-if-exception (<= 100 (size->number size)))))))

Maybe: (not (or (string-suffix? …) (false-if-exception …)))

Otherwise LGTM.

Thank you!

Ludo’, who actually saw the warning way too often.  :-)

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 624 bytes --]

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

>> +  (let* ((default-absolute-threshold (size->number "5GiB"))
>> +         (default-relative-threshold 0.05)
>> +         (percentage->float (lambda (percentage)
>> +                              (or (and=> (string->number
>> +                                          (car (string-split percentage #\%)))
>
> Mayes just: (string-drop-right percentage 1).

This does not work if the value does not have a percent sign, e.g. it
turns "13" into "1".

Resubmitting the patch with the other change.
Feel free to merge! ;)

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

* guix/scripts.scm (%disk-space-warning-absolute): New variable.
(warn-about-disk-space): Test against %disk-space-warning-absolute.
Fix error in display-hint due to extraneous 'profile' argument.
---
 guix/scripts.scm | 65 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 51 insertions(+), 14 deletions(-)

diff --git a/guix/scripts.scm b/guix/scripts.scm
index 77cbf12350..7ad1d5194c 100644
--- a/guix/scripts.scm
+++ b/guix/scripts.scm
@@ -181,32 +181,69 @@ Show what and how will/would be built."
     (newline (guix-warning-port))))
 
 (define %disk-space-warning
-  ;; The fraction (between 0 and 1) of free disk space below which a warning
-  ;; is emitted.
-  (make-parameter (match (and=> (getenv "GUIX_DISK_SPACE_WARNING")
-                                string->number)
-                    (#f        .05)               ;5%
-                    (threshold (/ threshold 100.)))))
+  ;; Return a pair of absolute threshold (number of bytes) and relative
+  ;; threshold (fraction between 0 and 1) for the free disk space below which
+  ;; a warning is emitted.
+  ;; GUIX_DISK_SPACE_WARNING can contain both thresholds.  A value in [0;100)
+  ;; is a relative threshold, otherwise it's absolute.  The following
+  ;; example values are valid:
+  ;; - 1GiB;10%      ;1 GiB absolute, and 10% relative.
+  ;; - 15G           ;15 GiB absolute, and default relative.
+  ;; - 99%           ;99% relative, and default absolute.
+  ;; - 99            ;Same.
+  ;; - 100           ;100 absolute, and default relative.
+  (let* ((default-absolute-threshold (size->number "5GiB"))
+         (default-relative-threshold 0.05)
+         (percentage->float (lambda (percentage)
+                              (or (and=> (string->number
+                                          (car (string-split percentage #\%)))
+                                         (lambda (n) (/ n 100.0)))
+                                  default-relative-threshold)))
+         (size->number* (lambda (size)
+                          (or (false-if-exception (size->number size))
+                              default-absolute-threshold)))
+         (absolute? (lambda (size)
+                      (not (or (string-suffix? "%" size)
+                               (false-if-exception (< (size->number size) 100)))))))
+    (make-parameter
+     (match (getenv "GUIX_DISK_SPACE_WARNING")
+       (#f (list default-absolute-threshold
+                 default-relative-threshold))
+       (env-string (match (string-split env-string #\;)
+                     ((threshold)
+                      (if (absolute? threshold)
+                          (list (size->number* threshold)
+                                default-relative-threshold)
+                          (list default-absolute-threshold
+                                (percentage->float threshold))))
+                     ((threshold1 threshold2)
+                      (if (absolute? threshold1)
+                          (list (size->number* threshold1)
+                                (percentage->float threshold2))
+                          (list (size->number* threshold2)
+                                (percentage->float threshold1))))))))))
 
 (define* (warn-about-disk-space #:optional profile
                                 #:key
-                                (threshold (%disk-space-warning)))
+                                (thresholds (%disk-space-warning)))
   "Display a hint about 'guix gc' if less than THRESHOLD of /gnu/store is
-available."
+available.
+THRESHOLD is a pair of (ABSOLUTE-THRESHOLD RELATIVE-THRESHOLD)."
   (let* ((stats      (statfs (%store-prefix)))
          (block-size (file-system-block-size stats))
          (available  (* block-size (file-system-blocks-available stats)))
          (total      (* block-size (file-system-block-count stats)))
-         (ratio      (/ available total 1.)))
-    (when (< ratio threshold)
-      (warning (G_ "only ~,1f% of free space available on ~a~%")
-               (* ratio 100) (%store-prefix))
+         (relative-threshold-in-bytes (* total (cadr thresholds)))
+         (absolute-threshold-in-bytes (* 1024 1024 1024 (car thresholds))))
+    (when (< available (min relative-threshold-in-bytes
+                            absolute-threshold-in-bytes))
+      (warning (G_ "only ~,1f GiB of free space available on ~a~%")
+               available (%store-prefix))
       (display-hint (format #f (G_ "Consider deleting old profile
 generations and collecting garbage, along these lines:
 
 @example
 guix gc --delete-generations=1m
-@end example\n")
-                            profile)))))
+@end example\n"))))))
 
 ;;; scripts.scm ends here
-- 
2.25.0

bug#39734: [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Ludovic Courtès]

Hello,

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>>> +  (let* ((default-absolute-threshold (size->number "5GiB"))
>>> +         (default-relative-threshold 0.05)
>>> +         (percentage->float (lambda (percentage)
>>> +                              (or (and=> (string->number
>>> +                                          (car (string-split percentage #\%)))
>>
>> Mayes just: (string-drop-right percentage 1).
>
> This does not work if the value does not have a percent sign, e.g. it
> turns "13" into "1".
>
> Resubmitting the patch with the other change.
> Feel free to merge! ;)

Perfect, I pushed it on your behalf.

Thank you for your patience!

Ludo’.

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Ludovic Courtès]

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> * guix/scripts.scm (%disk-space-warning-absolute): New variable.
> (warn-about-disk-space): Test against %disk-space-warning-absolute.
> Fix error in display-hint due to extraneous 'profile' argument.

I pushed a followup as 71c3c3df92375ca9b4bd28b2be90dda67288fa5c, fixing
a couple of issues we had overlooked.

Ludo’.

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 441 bytes --]

Thanks, I noticed the issue as well on my last guix pull! :)

However, shouldn't it be `min` still?

On big partitions, 5% will be high, so we don't want to display a
warning then, we only display it when lower than 5GiB, the minimum.

On small partitions, 5GiB is too much, so we don't want to display a
warning then, we only display it when lower than 5%, the minimum.

Thoughts?

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Ludovic Courtès]

Howdy!

Pierre Neidhardt <mail@ambrevar.xyz> skribis:

> Thanks, I noticed the issue as well on my last guix pull! :)
>
> However, shouldn't it be `min` still?
>
> On big partitions, 5% will be high, so we don't want to display a
> warning then, we only display it when lower than 5GiB, the minimum.
>
> On small partitions, 5GiB is too much, so we don't want to display a
> warning then, we only display it when lower than 5%, the minimum.
>
> Thoughts?

Hmm you must be right, sorry for getting it wrong.  Please feel free to
push a fix!

Ludo’.

[bug#39734] [PATCH] scripts: Emit GC hint if free space is lower than absolute and relative threshold.

[Pierre Neidhardt]

[-- Attachment #1: Type: text/plain, Size: 63 bytes --]

Done, thanks!

-- 
Pierre Neidhardt
https://ambrevar.xyz/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]