guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#64746] [PATCH v2 2/3] pull: Tag commit argument with 'tag-or-commit


From: Simon Tournier
Subject: [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@gmail.com> wrote:

> > On Wed, 16 Aug 2023 at 14:47, Maxim Cournoyer <maxim.cournoyer@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





reply via email to

[Prev in Thread] Current Thread [Next in Thread]