GNU bug report logs - #65352
Fix time-machine and network

Previous Next

Package: guix-patches;

Reported by: Simon Tournier <zimon.toutoune <at> gmail.com>

Date: Thu, 17 Aug 2023 14:08:02 UTC

Severity: normal

Done: Simon Tournier <zimon.toutoune <at> gmail.com>

Bug is archived. No further changes may be made.

Full log


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

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 65352 <at> debbugs.gnu.org, Christopher Baines <guix <at> cbaines.net>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: [bug#65352] Fix time-machine and network
Date: Mon, 25 Sep 2023 17:58:05 +0200
Hi,

On Mon, 25 Sep 2023 at 17:01, Ludovic Courtès <ludo <at> gnu.org> wrote:

>>>   2. Short commit IDs are no longer handled in the 'commit case, as I
>>>      mentioned before in this thread (and then forgot).  Could you
>>>      reintroduce support for them?
>>
>> Short commit ID are handled by tag-or-commit (guix time-machine and
>> guix pull).  If there is a discrepancy elsewhere with short commit ID,
>> it should be fixed overthere, IMHO.
>>
>> Else, I do not understand what you are asking.  From my understanding,
>> it would not make sense to have short commit ID handled with (commit .
>> "abc123") for some part of the code and (tag-or-commit . "abc123") for
>> some other part.
>
> It the caller passes (commit . "1234"), this is no longer handled
> efficiently as it used to be.
>
> Maybe that’s a bit of a theoretical issue though because in practice
> CLIs would pass (tag-or-commit . "1234"), right?

Well, to my knowledge, ’guix pull’ and ’guix time-machine’ pass
'tag-or-commit for short commit ID.  Somehow, that’s the beginning of
all this. :-)

The story starts with 79ec651a286c71a3d4c72be33a1f80e76a560031 and
ecab937897385fce3e3ce0c5f128afba4304187c:

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

and then I just try to keep consistent some rest with these changes,
cleaning unnecessary network access.  The root of all is maybe this
change f36522416e69d95f222fdfa6404d1981eb5225b6, introducing
tag-or-commit.

Having all that in mind, we have to make clear how to internally
represent a short commit ID:

  + either (commit . "1234")
  + either (tag-or-commit . "1234")

but not both.  It appears to me a slippery slope with potential nasty
bugs if we mix the both.

From a CLI point of view, say “guix pull --comnit=foo123”, it is hard to
know beforehand if “foo123“ is a tag or a short commit ID, hence the
representation (tag-or-commit . "foo123") then resolved by the heuristic
implemented by ’resolve-reference’; as nicely implemented by f36522. :-)

Now, if elsewhere in the Guix code base, something is reading ‘foo123’
and constructs (commit . "foo123") in order to pass it to
’update-cached-checkout’, my opinion is that we need to correct it and
instead construct (tag-or-commit . "foo123").  Somehow, be in agreement
with 79ec65, ecab93 and f36522. :-)

To conclude, we had all this long thread discussion, partially because
REF is not explicitly specified but implicitly used here or there.

Therefore, to end this lengthy thread, I propose to send a patch
documenting these cases.  For example, update the docstring of
reference-available?.  Well, let close this « Fix time-machine and
network » since it is done, I guess.  And open another one for this
discussion about short commit ID internal representation.  WDYT?

Cheers,
simon




This bug report was last modified 1 year and 242 days ago.

Previous Next


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