Subject: Re: [bug#65352] Fix time-machine and network

From: Simon Tournier <[email-masked]>
Date: Mon, 25 Sep 2023 17:58:05 +0200

Hi,

On Mon, 25 Sep 2023 at 17:01, Ludovic Courtès <[email-masked]> 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