bug#65788: poor information when updating using “guix time-machine”

bug#65788: poor information when updating using “guix time-machine”

[Simon Tournier]

Hi,

Tangential of bug#65787 [1], the annoyance is the order of the updates.
It leads to poor messages.  Let exemplify at the extreme case.

--8<---------------cut here---------------start------------->8---
$ guix describe
Generation 28	sept. 06 2023 14:54:50	(current)
  guix 6113e05
    repository URL: https://git.savannah.gnu.org/git/guix.git
    commit: 6113e0529d61df7425f64e30a6bf77f7cfdfe5a5

$ rm -fr ~/.cache/guix/checkouts/pjmkglp4t7znuugeurpurzikxq3tnlaywmisyr27shj7apsnalwq

$ guix time-machine -q --commit=6113e05 -- describe
receiving objects   2% ▕█▋
    …some time flies…
indexing objects  21% ▕███████████████████████                                                                                       ▏
    …some time flies…
Updating channel 'guix' from Git repository at 'https://git.savannah.gnu.org/git/guix.git'...
   …instant…
Computing Guix derivation for 'x86_64-linux'... \
--8<---------------cut here---------------end--------------->8---

The reason is because the logic:

        (when (procedure? validate-channels)
          (validate-channels channels))
        (run-with-store store
          (mlet* %store-monad ((instances
                                -> (latest-channel-instances store channels
                                                             #:authenticate?
                                                             authenticate?))

where ’validate-channels’ (validate-guix-channel) reads,

                (checkout commit relation (update-cached-checkout
                                           (channel-url guix-channel)
                                           #:ref reference
                                           #:starting-commit
                                           %oldest-possible-commit)))

and ’latest-channel-instances’ which is the ones that displays,

                     (format (current-error-port)
                             (G_ "Updating channel '~a' from Git repository at '~a'...~%")
                             (channel-name channel)
                             (channel-url channel))


this ’latest-channel-instances’ reads under the hood,

               ((checkout commit relation)
                (update-cached-checkout (channel-url channel)
                                        #:ref (channel-reference channel)
                                        #:starting-commit starting-commit)))


Why not move this ’validate-guix-channel’ to internals.  Somehow, it is
in guix/scripts/ because it captures ’ref’.  However, this capture is
redundant and is normally managed by ’channel-list’.  Therefore, I would
be tempted to have this validation for the reachable commit close to the
“Updating” message.

WDYT?

Cheers,
simon


1: <https://issues.guix.gnu.org/issue/65787>

bug#65788: poor information when updating using “guix time-machine”

[Ludovic Courtès]

Hi,

(Cc: Maxim.)

Simon Tournier <zimon.toutoune@gmail.com> skribis:

> $ guix time-machine -q --commit=6113e05 -- describe
> receiving objects   2% ▕█▋
>     …some time flies…
> indexing objects  21% ▕███████████████████████                                                                                       ▏
>     …some time flies…
> Updating channel 'guix' from Git repository at 'https://git.savannah.gnu.org/git/guix.git'...
>    …instant…
> Computing Guix derivation for 'x86_64-linux'... \

To be clear, the problem you see is that “Updating channel” is printed
too late, after “receiving objects” etc., right?

> Why not move this ’validate-guix-channel’ to internals.  Somehow, it is
> in guix/scripts/ because it captures ’ref’.  However, this capture is
> redundant and is normally managed by ’channel-list’.  Therefore, I would
> be tempted to have this validation for the reachable commit close to the
> “Updating” message.

Yes, that’s a good idea.

As I started looking into it, I realized we could reuse the existing
#:validate-pull mechanism of ‘latest-channel-instances’ for the purposes
of this commit check in ‘time-machine’.

The main advantage is that this would address a performance issue with
the implementation of ‘validate-guix-channel’ in commit
79ec651a286c71a3d4c72be33a1f80e76a560031, namely the fact that it opens
and traverses the repository one extra time for this check.  (The
#:validate-pull mechanism is integrated with ‘latest-channel-instances’
precisely to avoid this cost.)

Here’s my proposal to do that:

  https://issues.guix.gnu.org/66793

Ludo’.

PS: We should define rules for “Reviewed-by” tags because I don’t think
    I LGTM’d commit 79ec651a286c71a3d4c72be33a1f80e76a560031 (?).

bug#65788: poor information when updating using “guix time-machine”

[Maxim Cournoyer]

Hi,

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

[...]

> PS: We should define rules for “Reviewed-by” tags because I don’t think
>     I LGTM’d commit 79ec651a286c71a3d4c72be33a1f80e76a560031 (?).

It was a friendly credit added based on substantial comments I received
and acted upon from your review,
e.g. https://issues.guix.gnu.org/64746#7.

I don't mind if we codify to add these only when the reviewer added
their 'LGTM' approval stamp; there's an ongoing change adding some
guidelines for reviews in bug#66436, we should add this bit in.

-- 
Thanks,
Maxim

bug#65788: poor information when updating using “guix time-machine”

[Simon Tournier]

Hi Ludo,

On Sat, 28 Oct 2023 at 16:16, Ludovic Courtès <ludo@gnu.org> wrote:

>> Why not move this ’validate-guix-channel’ to internals.  Somehow, it is
>> in guix/scripts/ because it captures ’ref’.  However, this capture is
>> redundant and is normally managed by ’channel-list’.  Therefore, I would
>> be tempted to have this validation for the reachable commit close to the
>> “Updating” message.
>
> Yes, that’s a good idea.
>
> As I started looking into it, I realized we could reuse the existing
> #:validate-pull mechanism of ‘latest-channel-instances’ for the purposes
> of this commit check in ‘time-machine’.

[...]

> Here’s my proposal to do that:
>
>   https://issues.guix.gnu.org/66793

This improvement does not address this issue with
%oldest-possible-commit, right?

In addition, we also need to consider ’inferior-for-channels’ which
calls ’cached-channel-instance’ – currently with the default (const #t)
for #:validate-channels.

For an instance of bug with inferiors, please look at:

        Dependence on an old version of a package.
        Philippe Veber <philippe.veber@gmail.com>
        Sun, 10 Dec 2023 09:18:50 +0100
        id:CAOOOohRJu0QH+czx3qAwNxCY0X9JBd4NdUd9vjBvt-kDFCHkmA@mail.gmail.com
        https://lists.gnu.org/archive/html/help-guix/2023-12
        https://yhetil.org/guix/CAOOOohRJu0QH+czx3qAwNxCY0X9JBd4NdUd9vjBvt-kDFCHkmA@mail.gmail.com


Cheers,
simon