GNU bug report logs - #64746
[PATCH 2/2] scripts: time-machine: Error when attempting to visit too old commits.

Previous Next

Full log

Message #67 received at 64746 <at> debbugs.gnu.org (full text, mbox):

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Ludovic Courtès <ludo <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 64746 <at> debbugs.gnu.org, Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#64746] [PATCH v2 2/3] pull: Tag commit argument with
 'tag-or-commit.
Date: Thu, 17 Aug 2023 20:47:44 +0200

Re Maxim,

On Thu, 17 Aug 2023 at 20:16, Maxim Cournoyer <maxim.cournoyer <at> gmail.com> wrote:

> > On Wed, 16 Aug 2023 at 14:47, Maxim Cournoyer <maxim.cournoyer <at> gmail.com> wrote:
> >
> >>>>           (option '("commit") #t #f
> >>>>                   (lambda (opt name arg result)
> >>>> -                   (alist-cons 'ref `(commit . ,arg) result)))
> >>>> +                   (alist-cons 'ref `(tag-or-commit . ,arg) result)))
> >
> > [...]
> >
> >>                       (match ref
> >> -                       (('commit . commit)
> >> +                       ((or ('commit . commit)
> >> +                            ('tag-or-commit . commit))
> >
> >> The reason is to standardize the API of (guix pull) and (guix git),
> >> whose procedure had a different expectation for 'ref' objects.
> >
> > My point is that this ’or’ is useless, IIUC.  Well, I have removed it in
> > the series fixing the annoyance with the network access of “guix
> > time-machine”.
>
> It isn't, unless you meant after applying further changes :-) You should
> be able to see the problem by reverting that commit and running 'guix
> time-machine --commit=v1.4.0 -- describe', for example.

Yes for sure because you introduced this in guix/scripts/time-machine.scm,

                  (lambda (opt name arg result)
-                   (alist-cons 'ref `(commit . ,arg) result)))
+                   (alist-cons 'ref `(tag-or-commit . ,arg) result)))

with the previous commit 79ec651a286c71a3d4c72be33a1f80e76a560031.

Well, I feel there is a misunderstanding. :-)  And maybe I am missing
something... IIUC, the option parser:

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

implemented in guix/scrtips/pull.scm provides a way to construct this
REF.  This way should be compliant with the other one in
guix/scripts/time-machine.scm -- at least for being consistent.  And I
am missing the reason why you introduced this difference.

If the both option parsers use the same way, then the 'or' is useless.

As I said initially commenting this patch v2 2/3:

--8<---------------cut here---------------start------------->8---
> For compatibility with (guix git) procedures.
>
> * guix/scripts/pull.scm (channel-list): Also accept tag-or-commit tagged
> refspec.
> ---

I am not sure to understand these both changes.
--8<---------------cut here---------------end--------------->8---

Anyway, my other message [1] in #65352 contains the two alternatives
for closing this discussion. :-)

1: https://issues.guix.gnu.org/65352#4


Cheers,
simon

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.