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  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 
Sun, 10 Dec 2023 09:18:50 +0100
id:cahrju0qh+czx3qawnxcy0x9jbd4ndud9vjbvt-kdfch...@mail.gmail.com
https://lists.gnu.org/archive/html/help-guix/2023-12

https://yhetil.org/guix/cahrju0qh+czx3qawnxcy0x9jbd4ndud9vjbvt-kdfch...@mail.gmail.com


Cheers,
simon

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

[Maxim Cournoyer]

Hi,

Ludovic Courtès  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”

[Ludovic Courtès]

Hi,

(Cc: Maxim.)

Simon Tournier  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”

[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: