diff mbox series

[bug#45104] pull: Add a "with-substitutes" option.

Message ID 87eek1vd4g.fsf@gnu.org
State Accepted
Headers show
Series [bug#45104] pull: Add a "with-substitutes" option. | expand

Checks

Context Check Description
cbaines/submitting builds success
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Mathieu Othacehe Dec. 7, 2020, 3:39 p.m. UTC
Hello,

When "guix pull" is run before the CI server is done baking substitutes,
the user may need to build some derivations, which can be long on
low-end hardware.

This patch adds an option to "guix pull" so that it updates to the
latest commit with available substitutes.

This work is still in progress, but I'd like to gather some impressions
on that before going further.

Thanks,

Mathieu

Comments

Simon Tournier Dec. 7, 2020, 5:05 p.m. UTC | #1
Hi Mathieu,

On Mon, 07 Dec 2020 at 16:39, Mathieu Othacehe <othacehe@gnu.org> wrote:

> When "guix pull" is run before the CI server is done baking substitutes,
> the user may need to build some derivations, which can be long on
> low-end hardware.

Yeah…


> This patch adds an option to "guix pull" so that it updates to the
> latest commit with available substitutes.

Cool!  It improves my ugly bash to pull 2 weeks behind. :-)


> This work is still in progress, but I'd like to gather some impressions
> on that before going further.

With your patch, it is possible to never pull something.  Because the
push are faster than the CI is able to build.

Therefore, an improvement could to be to check the latest commit on the
CI, if not available fallback to the previous, repeat until one is
available or repeat X time and raise a warning.


Cheers,
simon
Christopher Baines Dec. 8, 2020, 7:17 p.m. UTC | #2
Mathieu Othacehe <othacehe@gnu.org> writes:

> When "guix pull" is run before the CI server is done baking substitutes,
> the user may need to build some derivations, which can be long on
> low-end hardware.
>
> This patch adds an option to "guix pull" so that it updates to the
> latest commit with available substitutes.
>
> This work is still in progress, but I'd like to gather some impressions
> on that before going further.

Hey!

I think it's definitely a nice idea, although I think there are some
things in the naming.

My first thought is that the option doesn't really do what it says it
will do. The option is named --with-substitutes, but that actually means
something like "with successful build on ci.guix.gnu.org". This could
cause confusion if you're not fetching substitutes from ci.guix.gnu.org,
and it starts building things, or perhaps if you're fetching substitutes
from two servers, one of which is ci.guix.gnu.org which doesn't have a
substitute, but the other server does, this option would fetch an older
revision than what's truly available.

Maybe part of what I've said above is incorrect if a users substitute
URLs are used, but there also seems to be an assuption that something
which provides substitutes provides a Cuirass compatible API, which
isn't always the case.

One more thought on this is that it's probably important to convey that
this doesn't pick a revision where substitutes will be available. I
think this is something some users would be eager to assume, and then be
confused when it doesn't work.

Thanks,

Chris
Ludovic Courtès Dec. 14, 2020, 11:05 a.m. UTC | #3
Hi!

Christopher Baines <mail@cbaines.net> skribis:

> My first thought is that the option doesn't really do what it says it
> will do. The option is named --with-substitutes, but that actually means
> something like "with successful build on ci.guix.gnu.org". This could
> cause confusion if you're not fetching substitutes from ci.guix.gnu.org,
> and it starts building things, or perhaps if you're fetching substitutes
> from two servers, one of which is ci.guix.gnu.org which doesn't have a
> substitute, but the other server does, this option would fetch an older
> revision than what's truly available.
>
> Maybe part of what I've said above is incorrect if a users substitute
> URLs are used, but there also seems to be an assuption that something
> which provides substitutes provides a Cuirass compatible API, which
> isn't always the case.
>
> One more thought on this is that it's probably important to convey that
> this doesn't pick a revision where substitutes will be available. I
> think this is something some users would be eager to assume, and then be
> confused when it doesn't work.

Agreed on these points.

Another option would be to leave it up to users to customize their
channel file to run pretty much the code you posted, like:

--8<---------------cut here---------------start------------->8---
(use-modules (guix ci)
             (srfi srfi-1)
             (ice-9 match))

(define (latest-commit-successfully-built)
  "Return the latest commit for which substitutes are (potentially)
available."
  (let* ((evaluations (filter (lambda (evaluation)
                                (and (evaluation-complete? evaluation)
                                     (string=? "guix-modular-master"
                                               (evaluation-spec
                                                evaluation))))
                              (latest-evaluations "https://ci.guix.gnu.org"))))
    (any (lambda (evaluation)
           (match (evaluation-checkouts evaluation)
             ((checkout)
              (checkout-commit checkout))
             (_ #f)))
         evaluations)))

;; Pull the latest commit fully built on berlin.guixsd.org.
(list (channel
       (name 'guix)
       (url "https://git.savannah.gnu.org/git/guix.git")
       (commit (pk 'commit (latest-commit-successfully-built)))))
--8<---------------cut here---------------end--------------->8---

We don’t want people to copy/paste all this, so we could instead provide
and document a procedure that takes a channel and substitute URL and
returns a channel, like:

  (channel-with-substitutes-available
    (channel (name 'guix) …)
    "https://ci.guix.gnu.org")

and optionally with a manifest or a list of packages that should be
available as substitutes:

  (channel-with-substitutes-available
    (channel (name 'guix) …)
    "https://ci.guix.gnu.org"
    (specifications->manifest '("emacs" "guile")))

WDYT?

It does mean that we’re asking users to do extra work.  Perhaps there
could still be a command-line option that would call
‘channel-with-substitutes-available’ for you, but at least it would take
an explicit URL and clarify what Chris mentioned?

BTW, doing all this is safer today because ‘guix pull’ will detect and
prevent downgrades.  Though an attacker who manages to break into
ci.guix.gnu.org could cause all the users of
‘channel-with-substitutes-available’ to no longer receive updates or to
receive them more slowly than they appear in Git simply by making CI
even slower than it currently is.

Thoughts?

Ludo’.
Simon Tournier Dec. 14, 2020, 11:39 a.m. UTC | #4
Hi,

On Mon, 14 Dec 2020 at 12:05, Ludovic Courtès <ludo@gnu.org> wrote:

>   (channel-with-substitutes-available
>     (channel (name 'guix) …)
>     "https://ci.guix.gnu.org")
>
> and optionally with a manifest or a list of packages that should be
> available as substitutes:
>
>   (channel-with-substitutes-available
>     (channel (name 'guix) …)
>     "https://ci.guix.gnu.org"
>     (specifications->manifest '("emacs" "guile")))

Sounds good to me.


> BTW, doing all this is safer today because ‘guix pull’ will detect and
> prevent downgrades.  Though an attacker who manages to break into
> ci.guix.gnu.org could cause all the users of
> ‘channel-with-substitutes-available’ to no longer receive updates or to
> receive them more slowly than they appear in Git simply by making CI
> even slower than it currently is.

As mentioned earlier, if “guix pull” completes only when substitutes is
available, then depending on the CI, the user can never complete the
“pull” and so stay “blocked“.

For example, reusing your example, Emacs is not currently available and
it has been since say 1 month.  Therefore until Emacs becomes available
for the fast moving HEAD, the user would be “blocked”.  Or I miss
something.

<https://data.guix.gnu.org/repository/1/branch/master/package/emacs/output-history>

All the best,
simon
Mathieu Othacehe Dec. 15, 2020, 10:24 a.m. UTC | #5
Hey Chris and Ludo,

> Agreed on these points.

Yes I think you are definitely right on that point.

>                                 (and (evaluation-complete? evaluation)
>                                      (string=? "guix-modular-master"
>                                                (evaluation-spec
>                                                 evaluation))))

On Berlin, evaluations can be completed for days, but the associated
builds never started. I think that searching directly for a completed
build provides a stronger guarantee of available substitutes.

> ;; Pull the latest commit fully built on berlin.guixsd.org.
> (list (channel
>        (name 'guix)
>        (url "https://git.savannah.gnu.org/git/guix.git")
>        (commit (pk 'commit (latest-commit-successfully-built)))))

Providing such a procedure definitely makes sense though.

>   (channel-with-substitutes-available
>     (channel (name 'guix) …)
>     "https://ci.guix.gnu.org"
>     (specifications->manifest '("emacs" "guile")))

Yes it would be the ultimate thing! However, while finding the latest
commit with an available substitute for a derivation is quite easy,
finding a commit with available derivations for N derivations seems way
more difficult.

> It does mean that we’re asking users to do extra work.  Perhaps there
> could still be a command-line option that would call
> ‘channel-with-substitutes-available’ for you, but at least it would take
> an explicit URL and clarify what Chris mentioned?

Yes, the user would then have to provide the channels that need
available substitutes, the URL to use for the substitution check and
maybe a manifest that also needs available substitutes.

The channels list could default to '("guix") and the URL to
"https://ci.guix.gnu.org" as it would be a sensible default for most
Guix users I think.

> BTW, doing all this is safer today because ‘guix pull’ will detect and
> prevent downgrades.  Though an attacker who manages to break into
> ci.guix.gnu.org could cause all the users of
> ‘channel-with-substitutes-available’ to no longer receive updates or to
> receive them more slowly than they appear in Git simply by making CI
> even slower than it currently is.

Yes, the downgrade check definitely helps here, as it's often what will
happen with our lagging CI. Regarding the security aspect, I think that
breaking into ci.guix.gnu.org can have other way more impacting
consequences.

Thanks,

Mathieu
Mathieu Othacehe Dec. 15, 2020, 10:30 a.m. UTC | #6
Hello zimoun,

> As mentioned earlier, if “guix pull” completes only when substitutes is
> available, then depending on the CI, the user can never complete the
> “pull” and so stay “blocked“.

There's nothing blocking in what I'm proposing. "guix pull" first asks
to the CI what's the latest commit with available substitutes for
"guix", and then tries to update to that commit. If it results in a
downgrade then, "guix pull" detects it.

The corner case where there are no commits with available substitutes is
not handled, but I guess we could decide to update to the latest commit
in that case.

Thanks,

Mathieu
Simon Tournier Dec. 15, 2020, 12:51 p.m. UTC | #7
Hi Mathieu,

On Tue, 15 Dec 2020 at 11:30, Mathieu Othacehe <othacehe@gnu.org> wrote:

> There's nothing blocking in what I'm proposing. "guix pull" first asks
> to the CI what's the latest commit with available substitutes for
> "guix", and then tries to update to that commit. If it results in a
> downgrade then, "guix pull" detects it.

Thanks for the explanation.

I have actually overlooked the code. Well, the proposal was what your
patch is doing somehow. :-)

Now, since I have played with “guix repl”, all is clear. ;-)

Aside, the option name ’--with-substitutes’ could be misleading.  Or
clearly state that it is substitutes for “guix”.


Cheers,
simon
Ludovic Courtès Dec. 15, 2020, 10:03 p.m. UTC | #8
Hi,

Mathieu Othacehe <othacehe@gnu.org> skribis:

>>                                 (and (evaluation-complete? evaluation)
>>                                      (string=? "guix-modular-master"
>>                                                (evaluation-spec
>>                                                 evaluation))))
>
> On Berlin, evaluations can be completed for days, but the associated
> builds never started. I think that searching directly for a completed
> build provides a stronger guarantee of available substitutes.

Yes, something like you proposed probably makes more sense.

My point is just that we could make the procedure available as part of
the API and document it as something people can use in their channels
file.

>> ;; Pull the latest commit fully built on berlin.guixsd.org.
>> (list (channel
>>        (name 'guix)
>>        (url "https://git.savannah.gnu.org/git/guix.git")
>>        (commit (pk 'commit (latest-commit-successfully-built)))))
>
> Providing such a procedure definitely makes sense though.
>
>>   (channel-with-substitutes-available
>>     (channel (name 'guix) …)
>>     "https://ci.guix.gnu.org"
>>     (specifications->manifest '("emacs" "guile")))
>
> Yes it would be the ultimate thing! However, while finding the latest
> commit with an available substitute for a derivation is quite easy,
> finding a commit with available derivations for N derivations seems way
> more difficult.

Right!

>> It does mean that we’re asking users to do extra work.  Perhaps there
>> could still be a command-line option that would call
>> ‘channel-with-substitutes-available’ for you, but at least it would take
>> an explicit URL and clarify what Chris mentioned?
>
> Yes, the user would then have to provide the channels that need
> available substitutes, the URL to use for the substitution check and
> maybe a manifest that also needs available substitutes.
>
> The channels list could default to '("guix") and the URL to
> "https://ci.guix.gnu.org" as it would be a sensible default for most
> Guix users I think.

Yes, choosing good defaults can make it less intimidating.

>> BTW, doing all this is safer today because ‘guix pull’ will detect and
>> prevent downgrades.  Though an attacker who manages to break into
>> ci.guix.gnu.org could cause all the users of
>> ‘channel-with-substitutes-available’ to no longer receive updates or to
>> receive them more slowly than they appear in Git simply by making CI
>> even slower than it currently is.
>
> Yes, the downgrade check definitely helps here, as it's often what will
> happen with our lagging CI. Regarding the security aspect, I think that
> breaking into ci.guix.gnu.org can have other way more impacting
> consequences.

Yeah, though here we’re opening a new vulnerability channel, independent
of substitutes.  It changes the threat model.

Thanks,
Ludo’.
diff mbox series

Patch

From d399f8dbb9e38a82241b9048b8b04758fae10005 Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <othacehe@gnu.org>
Date: Mon, 7 Dec 2020 16:12:22 +0100
Subject: [PATCH] scripts: pull: Add "with-substitutes" option.

---
 guix/scripts/pull.scm | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/guix/scripts/pull.scm b/guix/scripts/pull.scm
index 83cdc1d1eb..4609f8614e 100644
--- a/guix/scripts/pull.scm
+++ b/guix/scripts/pull.scm
@@ -2,6 +2,7 @@ 
 ;;; Copyright © 2013, 2014, 2015, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2017 Marius Bakke <mbakke@fastmail.com>
 ;;; Copyright © 2020 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2020 Mathieu Othacehe <othacehe@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -20,6 +21,7 @@ 
 
 (define-module (guix scripts pull)
   #:use-module ((guix ui) #:hide (display-profile-content))
+  #:use-module (guix ci)
   #:use-module (guix colors)
   #:use-module (guix utils)
   #:use-module ((guix status) #:select (with-status-verbosity))
@@ -64,6 +66,7 @@ 
   #:re-export (display-profile-content
                channel-commit-hyperlink)
   #:export (channel-list
+            find-lastest-commit-with-substitutes
             guix-pull))
 
 
@@ -169,6 +172,9 @@  Download and deploy the latest version of Guix.\n"))
                  (lambda (opt name arg result)
                    (alist-cons 'validate-pull warn-about-backward-updates
                                result)))
+         (option '("with-substitutes") #f #f
+                 (lambda (opt name arg result)
+                   (alist-cons 'with-substitutes? #t result)))
          (option '("disable-authentication") #f #f
                  (lambda (opt name arg result)
                    (alist-cons 'authenticate-channels? #f result)))
@@ -526,6 +532,26 @@  true, display what would be built without actually building it."
           (leave (G_ "while creating symlink '~a': ~a~%")
                  link (strerror (system-error-errno args))))))))
 
+
+;;;
+;;; Substitutes.
+;;;
+
+(define (find-lastest-commit-with-substitutes)
+  (let ((urls %default-substitute-urls))
+    (any (lambda (url)
+           (let* ((build (match (latest-builds url 1
+                                               #:job "guix.x86_64-linux"
+                                               #:status 0) ;success
+                           ((build) build)))
+                  (evaluation (evaluation url
+                                          (build-evaluation build)))
+                  (commit (match (evaluation-checkouts evaluation)
+                            ((checkout)
+                             (checkout-commit checkout)))))
+             commit))
+         urls)))
+
 
 ;;;
 ;;; Queries.
@@ -731,8 +757,9 @@  Use '~/.config/guix/channels.scm' instead."))
 
   (let ((ref (assoc-ref opts 'ref))
         (url (or (assoc-ref opts 'repository-url)
-                 (environment-variable))))
-    (if (or ref url)
+                 (environment-variable)))
+        (with-substitutes? (assoc-ref opts 'with-substitutes?)))
+    (if (or ref url with-substitutes?)
         (match (find guix-channel? channels)
           ((? channel? guix)
            ;; Apply '--url', '--commit', and '--branch' to the 'guix' channel.
@@ -745,7 +772,12 @@  Use '~/.config/guix/channels.scm' instead."))
                       (channel (inherit guix)
                                (url url) (commit #f) (branch branch)))
                      (#f
-                      (channel (inherit guix) (url url))))
+                      (let ((commit
+                             (and with-substitutes?
+                                  (find-lastest-commit-with-substitutes))))
+                        (channel (inherit guix)
+                                 (url url)
+                                 (commit commit)))))
                    (remove guix-channel? channels))))
           (#f                           ;no 'guix' channel, failure will ensue
            channels))
-- 
2.29.2