[bug#65352] Fix time-machine and network

[Simon Tournier]

Hi,

On Mon, 04 Sep 2023 at 11:32, Ludovic Courtès <ludo@gnu.org> wrote:

> In fact, now I recall why that procedure was written that way: it’s
> meant to say whether a given commit (and only a commit) is already in
> the checkout, meaning we don’t need to pull.  By definition, it’s an
> answer that can only be given for a specific commit; we cannot tell
> whether “master” or “HEAD” is available, that wouldn’t make sense.

Yeah, that’s the job of ’reference-available?’ to say if a given
reference is or not in the repository, IMHO.

The patch I proposed earlier fixes the issue you reported, I guess.


When debugging, I have noticed that this update-cached-checkout is
called many times.  For instance,
79ec651a286c71a3d4c72be33a1f80e76a560031 introduced a call.
Investigating, I notice that this new procedure is incorrect:

--8<---------------cut here---------------start------------->8---
       (define (validate-guix-channel channels)
         "Finds the Guix channel among CHANNELS, and validates that REF as
captured from the closure, a git reference specification such as a commit hash
or tag associated to CHANNEL, is valid and new enough to satisfy the 'guix
time-machine' requirements.  A `formatted-message' condition is raised
otherwise."
         (let* ((guix-channel (find guix-channel? channels))
                (checkout commit relation (update-cached-checkout
                                           (channel-url guix-channel)
                                           #:ref (or ref '())
                                           #:starting-commit
                                           %oldest-possible-commit)))
--8<---------------cut here---------------end--------------->8---

Here, the symbol ’ref’ is bound by:

            (ref          (assoc-ref opts 'ref))

which comes from:

         (option '("commit") #t #f
                 (lambda (opt name arg result)
                   (alist-cons 'ref `(tag-or-commit . ,arg) result)))
         (option '("branch") #t #f
                 (lambda (opt name arg result)
                   (alist-cons 'ref `(branch . ,arg) result)))

Therefore, it means that when none of the options --commit= or --branch=
is provided by the user at the CLI, this ’ref’ is bounded to #false.

Therefore, it can lead to unexpected behaviour when providing a
channels.scm file.

Well, instead, the correct something like:

         (let* ((guix-channel (find guix-channel? channels))
                (reference (or ref
                               (match (channel-commit guix-channel)
                                 (#f `(branch . ,(channel-branch guix-channel)))
                                 (commit `(tag-or-commit . ,commit)))))
                (checkout commit relation (update-cached-checkout
                                           (channel-url guix-channel)
                                           #:ref reference
                                           #:starting-commit
                                           %oldest-possible-commit)))

which works using my tests (with or without network).

The remaining issue is the order when displaying messages.  This
’validate-guix-channel’ happens before “Updating channel 'guix'”
therefore the progress bar appears before etc.

I have not investigated how to improve cached-channel-instance.  Let me
know if the current tiny fix tweaking resolve-interface is enough for
now waiting some rework of validate-guix-channel. :-)

Cheers,
simon